#2246 Review Core cache and redesign Regex caching

src/Ocelot.Cache.CacheManager/OcelotCacheManagerCache.cs

@@ -37,4 +37,9 @@ public void ClearRegion(string region)
     {
         _cacheManager.ClearRegion(region);
     }
+
+    public bool TryGetValue(string key, string region, out T value)
+    {
+        return _cacheManager.TryGetOrAdd(key, region, (a, b) => default, out value);
+    }
 }

src/Ocelot/Cache/DefaultMemoryCache.cs

@@ -5,12 +5,12 @@ namespace Ocelot.Cache;
 public class DefaultMemoryCache<T> : IOcelotCache<T>
 {
     private readonly IMemoryCache _memoryCache;
-    private readonly Dictionary<string, List<string>> _regions;
+    private readonly ConcurrentDictionary<string, ConcurrentBag<string>> _regions;
 
     public DefaultMemoryCache(IMemoryCache memoryCache)
     {
         _memoryCache = memoryCache;
-        _regions = new Dictionary<string, List<string>>();
+        _regions = new();
     }
 
     public void Add(string key, T value, TimeSpan ttl, string region)
@@ -21,29 +21,29 @@ public void Add(string key, T value, TimeSpan ttl, string region)
         }
 
         _memoryCache.Set(key, value, ttl);
-
         SetRegion(region, key);
     }
 
     public T Get(string key, string region)
     {
-        if (_memoryCache.TryGetValue(key, out T value))
+        if (TryGetValue(key, region, out T value))
         {
             return value;
         }
 
-        return default(T);
+        return default;
     }
 
     public void ClearRegion(string region)
     {
-        if (_regions.ContainsKey(region))
+        if (_regions.TryGetValue(region, out var keys))
         {
-            var keys = _regions[region];
             foreach (var key in keys)
             {
                 _memoryCache.Remove(key);
             }
+
+            keys.Clear();
         }
     }
 
@@ -59,17 +59,21 @@ public void AddAndDelete(string key, T value, TimeSpan ttl, string region)
 
     private void SetRegion(string region, string key)
     {
-        if (_regions.ContainsKey(region))
+        if (_regions.TryGetValue(region, out var current))
         {
-            var current = _regions[region];
             if (!current.Contains(key))
             {
                 current.Add(key);
             }
         }
         else
         {
-            _regions.Add(region, new List<string> { key });
+            _regions.TryAdd(region, new() { key });
         }
     }
+
+    public bool TryGetValue(string key, string region, out T value)
+    {
+        return _memoryCache.TryGetValue(key, out value);
+    }
 }

src/Ocelot/Cache/IOcelotCache.cs

@@ -9,4 +9,6 @@ public interface IOcelotCache<T>
     void ClearRegion(string region);
 
     void AddAndDelete(string key, T value, TimeSpan ttl, string region);
+
+    bool TryGetValue(string key, string region, out T value);
 }

src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs

@@ -1,4 +1,6 @@
+using Ocelot.Cache;
 using Ocelot.Configuration.File;
+using Ocelot.Infrastructure;
 using Ocelot.Values;
 
 namespace Ocelot.Configuration.Creator;
@@ -11,11 +13,16 @@ public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator
     private const string RegExIgnoreCase = "(?i)";
     private const string RegExForwardSlashOnly = "^/$";
     private const string RegExForwardSlashAndOnePlaceHolder = "^/.*";
+    private readonly IOcelotCache<Regex> _cache;
+
+    public UpstreamTemplatePatternCreator(IOcelotCache<Regex> cache)
+    {
+        _cache = cache;
+    }
 
     public UpstreamPathTemplate Create(IRoute route)
     {
         var upstreamTemplate = route.UpstreamPathTemplate;
-
         var placeholders = new List<string>();
 
         for (var i = 0; i < upstreamTemplate.Length; i++)
@@ -27,10 +34,10 @@ public UpstreamPathTemplate Create(IRoute route)
                 var placeHolderName = upstreamTemplate.Substring(i, difference);
                 placeholders.Add(placeHolderName);
 
-                //hack to handle /{url} case
+                // Hack to handle /{url} case
                 if (ForwardSlashAndOnePlaceHolder(upstreamTemplate, placeholders, postitionOfPlaceHolderClosingBracket))
                 {
-                    return new UpstreamPathTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false, route.UpstreamPathTemplate);
+                    return CreateTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false, route.UpstreamPathTemplate);
                 }
             }
         }
@@ -59,7 +66,7 @@ public UpstreamPathTemplate Create(IRoute route)
 
         if (upstreamTemplate == "/")
         {
-            return new UpstreamPathTemplate(RegExForwardSlashOnly, route.Priority, containsQueryString, route.UpstreamPathTemplate);
+            return CreateTemplate(RegExForwardSlashOnly, route.Priority, containsQueryString, route.UpstreamPathTemplate);
         }
 
         var index = upstreamTemplate.LastIndexOf('/'); // index of last forward slash
@@ -77,21 +84,40 @@ public UpstreamPathTemplate Create(IRoute route)
             ? $"^{upstreamTemplate}{RegExMatchEndString}"
             : $"^{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}";
 
-        return new UpstreamPathTemplate(template, route.Priority, containsQueryString, route.UpstreamPathTemplate);
+        return CreateTemplate(template, route.Priority, containsQueryString, route.UpstreamPathTemplate);
     }
 
-    private static bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List<string> placeholders, int postitionOfPlaceHolderClosingBracket)
+    /// <summary>Time-to-live for caching <see cref="Regex"/> to initialize the <see cref="UpstreamPathTemplate.Pattern"/> property.</summary>
+    /// <value>A constant <see cref="TimeSpan"/> structure, default absolute value is 1 minute.</value>
+    public static TimeSpan RegexCachingTTL { get; set; } = TimeSpan.FromMinutes(1.0D);
+
+    protected Regex GetRegex(string key)
     {
-        if (upstreamTemplate.Substring(0, 2) == "/{" && placeholders.Count == 1 && upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1)
+        if (string.IsNullOrEmpty(key))
         {
-            return true;
+            return null;
         }
 
-        return false;
+        if (!_cache.TryGetValue(key, nameof(UpstreamPathTemplate), out var rgx))
+        {
+            rgx = RegexGlobal.New(key, RegexOptions.Singleline);
+            _cache.Add(key, rgx, RegexCachingTTL, nameof(UpstreamPathTemplate));
+        }
+
+        return rgx;
     }
 
+    protected UpstreamPathTemplate CreateTemplate(string template, int priority, bool containsQueryString, string originalValue)
+        => new(template, priority, containsQueryString, originalValue)
+        {
+            Pattern = GetRegex(template),
+        };
+
+    private static bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List<string> placeholders, int postitionOfPlaceHolderClosingBracket)
+        => upstreamTemplate.Substring(0, 2) == "/{" &&
+            placeholders.Count == 1 &&
+            upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1;
+
     private static bool IsPlaceHolder(string upstreamTemplate, int i)
-    {
-        return upstreamTemplate[i] == '{';
-    }
+        => upstreamTemplate[i] == '{';
 }

src/Ocelot/DependencyInjection/Features.cs

@@ -30,6 +30,7 @@ public static IServiceCollection AddRateLimiting(this IServiceCollection service
     /// <param name="services">The services collection to add the feature to.</param>
     /// <returns>The same <see cref="IServiceCollection"/> object.</returns>
     public static IServiceCollection AddOcelotCache(this IServiceCollection services) => services
+        .AddSingleton<IOcelotCache<Regex>, DefaultMemoryCache<Regex>>()
         .AddSingleton<IOcelotCache<FileConfiguration>, DefaultMemoryCache<FileConfiguration>>()
         .AddSingleton<IOcelotCache<CachedResponse>, DefaultMemoryCache<CachedResponse>>()
         .AddSingleton<ICacheKeyGenerator, DefaultCacheKeyGenerator>()

src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs

@@ -37,8 +37,7 @@ public Response<DownstreamRouteHolder> Get(string upstreamUrlPath, string upstre
         {
             var urlMatch = _urlMatcher.Match(upstreamUrlPath, upstreamQueryString, route.UpstreamTemplatePattern);
             var headersMatch = _headerMatcher.Match(upstreamHeaders, route.UpstreamHeaderTemplates);
-
-            if (urlMatch.Data.Match && headersMatch)
+            if (urlMatch.Match && headersMatch)
             {
                 downstreamRoutes.Add(GetPlaceholderNamesAndValues(upstreamUrlPath, upstreamQueryString, route, upstreamHeaders));
             }

src/Ocelot/DownstreamRouteFinder/UrlMatcher/IUrlPathToUrlTemplateMatcher.cs

@@ -1,9 +1,8 @@
-using Ocelot.Responses;
 using Ocelot.Values;
 
 namespace Ocelot.DownstreamRouteFinder.UrlMatcher;
 
 public interface IUrlPathToUrlTemplateMatcher
 {
-    Response<UrlMatch> Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate);
+    UrlMatch Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate);
 }

src/Ocelot/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcher.cs

@@ -1,21 +1,11 @@
-using Ocelot.Responses;
-using Ocelot.Values;
+using Ocelot.Values;
 
 namespace Ocelot.DownstreamRouteFinder.UrlMatcher;
 
 public class RegExUrlMatcher : IUrlPathToUrlTemplateMatcher
 {
-    public Response<UrlMatch> Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate)
-    {
-        if (!pathTemplate.ContainsQueryString)
-        {
-            return pathTemplate.Pattern.IsMatch(upstreamUrlPath)
-                ? new OkResponse<UrlMatch>(new UrlMatch(true))
-                : new OkResponse<UrlMatch>(new UrlMatch(false));
-        }
-
-        return pathTemplate.Pattern.IsMatch($"{upstreamUrlPath}{upstreamQueryString}")
-            ? new OkResponse<UrlMatch>(new UrlMatch(true))
-            : new OkResponse<UrlMatch>(new UrlMatch(false));
-    }
+    public UrlMatch Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate)
+        => !pathTemplate.ContainsQueryString
+            ? new(pathTemplate.Pattern.IsMatch(upstreamUrlPath))
+            : new(pathTemplate.Pattern.IsMatch($"{upstreamUrlPath}{upstreamQueryString}"));
 }

src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs

@@ -1,7 +1,6 @@
 using Microsoft.AspNetCore.Http;
 using Ocelot.Configuration;
 using Ocelot.DownstreamRouteFinder.UrlMatcher;
-using Ocelot.Infrastructure;
 using Ocelot.Logging;
 using Ocelot.Middleware;
 using Ocelot.Request.Middleware;
@@ -26,7 +25,7 @@ public DownstreamUrlCreatorMiddleware(
         RequestDelegate next,
         IOcelotLoggerFactory loggerFactory,
         IDownstreamPathPlaceholderReplacer replacer)
-            : base(loggerFactory.CreateLogger<DownstreamUrlCreatorMiddleware>())
+        : base(loggerFactory.CreateLogger<DownstreamUrlCreatorMiddleware>())
     {
         _next = next;
         _replacer = replacer;
@@ -84,7 +83,6 @@ public async Task Invoke(HttpContext httpContext)
             else
             {
                 RemoveQueryStringParametersThatHaveBeenUsedInTemplate(downstreamRequest, placeholders);
-
                 downstreamRequest.AbsolutePath = dsPath;
             }
         }
@@ -119,30 +117,32 @@ private static string MergeQueryStringsWithoutDuplicateValues(string queryString
     }
 
     private static string MapQueryParameter(KeyValuePair<string, string> pair) => $"{pair.Key}={pair.Value}";
-    private static readonly ConcurrentDictionary<string, Regex> _regex = new();
 
-    private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List<PlaceholderNameAndValue> templatePlaceholderNameAndValues)
+    /// <summary>
+    /// Feature <see href="https://github.com/ThreeMammals/Ocelot/pull/467">467</see>:
+    /// Added support for query string parameters in upstream path template.
+    /// </summary>
+    private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List<PlaceholderNameAndValue> templatePlaceholders)
     {
-        foreach (var nAndV in templatePlaceholderNameAndValues)
+        var builder = new StringBuilder();
+        foreach (var nAndV in templatePlaceholders)
         {
             var name = nAndV.Name.Trim(OpeningBrace, ClosingBrace);
-            var value = Regex.Escape(nAndV.Value); // to ensure a placeholder value containing special Regex characters from URL query parameters is safely used in a Regex constructor, it's necessary to escape the value
-            var pattern = $@"\b{name}={value}\b";
-            var rgx = _regex.AddOrUpdate(pattern,
-                        RegexGlobal.New(pattern),
-                        (key, oldValue) => oldValue);
-            if (rgx.IsMatch(downstreamRequest.Query))
+            var parameter = $"{name}={nAndV.Value}";
+            if (!downstreamRequest.Query.Contains(parameter))
             {
-                var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal);                    
-                downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty);
-                downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1);
-
-                if (!string.IsNullOrEmpty(downstreamRequest.Query))
-                {
-                    downstreamRequest.Query = QuestionMark + downstreamRequest.Query[1..];
-                }
+                continue;
             }
-        } 
+
+            int questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal);
+            builder.Clear()
+                .Append(downstreamRequest.Query)
+                .Replace(parameter, string.Empty)
+                .Remove(--questionMarkOrAmpersand, 1);
+            downstreamRequest.Query = builder.Length > 0
+                ? builder.Remove(0, 1).Insert(0, QuestionMark).ToString()
+                : string.Empty;
+        }
     }
 
     private static string GetPath(string downstreamPath)

src/Ocelot/Values/UpstreamPathTemplate.cs

@@ -12,27 +12,24 @@ public partial class UpstreamPathTemplate
     private static readonly Regex _regexNoTemplate = RegexGlobal.New("$^", RegexOptions.Singleline);
     private static Regex RegexNoTemplate() => _regexNoTemplate;
 #endif
-    private static readonly ConcurrentDictionary<string, Regex> _regex = new();
 
     public UpstreamPathTemplate(string template, int priority, bool containsQueryString, string originalValue)
     {
         Template = template;
         Priority = priority;
         ContainsQueryString = containsQueryString;
         OriginalValue = originalValue;
-        Pattern = template == null ? RegexNoTemplate() :
-            _regex.AddOrUpdate(template,
-                RegexGlobal.New(template, RegexOptions.Singleline),
-                (key, oldValue) => oldValue);
     }
 
     public string Template { get; }
-
     public int Priority { get; }
-
     public bool ContainsQueryString { get; }
-
     public string OriginalValue { get; }
 
-    public Regex Pattern { get; }
+    private Regex _pattern;
+    public Regex Pattern
+    {
+        get => _pattern;
+        set => _pattern = Template == null || value == null ? RegexNoTemplate() : value;
+    }
 }