#2054 #2059 Manage EnableContentHashing setting by global CacheOptions (#2058)

* EnableContentHashing not being considered from appsettings

* Adding CacheOptionsCreator, Injected IRegionCreator as Singleton. Should still add some acceptance tests that are definitely missing!

* Adding caching global configuration since we messed up, ignoring an important breaking change with EnableContentHashing set to false by default

* Adding some further acceptance tests, validating EnableContentHashing, validating global config too.

* removing some debug content

* TtlSeconds must be set

* updating documentation

* Update docs/features/caching.rst

Co-authored-by: Raman Maksimchuk <[email protected]>

* Update docs/features/caching.rst

Co-authored-by: Raman Maksimchuk <[email protected]>

* Removing RegionCreator, moving service collection extension method to dependencyInjection\Features etc.

* adding unit tests for FileCacheOptions

* some more null tests...

* slight refactoring, updating ICacheOptionsCreator signature

* some more design refactoring

* Update src/Ocelot/Configuration/Creator/CacheOptionsCreator.cs

Co-authored-by: Raman Maksimchuk <[email protected]>

* Code review by @raman-m

* Rename `FileCacheOptions` -> `CacheOptions`

* Subtly transition to `CacheOptions`, ensuring compatibility with `FileCacheOptions` to avoid a breaking change

* Not obsolete

---------

Co-authored-by: Guillaume Gnaegi <[email protected]>
Co-authored-by: Raman Maksimchuk <[email protected]>

Author: 3 people

docs/features/caching.rst

@@ -39,7 +39,8 @@ Finally, in order to use caching on a route in your Route configuration add this
     "FileCacheOptions": {
       "TtlSeconds": 15,
       "Region": "europe-central",
-      "Header": "Authorization"
+      "Header": "OC-Caching-Control",
+      "EnableContentHashing": false // my route has GET verb only, assigning 'true' for requests with body: POST, PUT etc.
     }
 
 In this example **TtlSeconds** is set to 15 which means the cache will expire after 15 seconds.
@@ -48,10 +49,38 @@ The **Region** represents a region of caching.
 Additionally, if a header name is defined in the **Header** property, that header value is looked up by the key (header name) in the ``HttpRequest`` headers,
 and if the header is found, its value will be included in caching key. This causes the cache to become invalid due to the header value changing.
 
+``EnableContentHashing`` option
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In version `23.0`_, the new property **EnableContentHashing** has been introduced. Previously, the request body was utilized to compute the cache key.
+However, due to potential performance issues arising from request body hashing, it has been disabled by default.
+Clearly, this constitutes a breaking change and presents challenges for users who require cache key calculations that consider the request body (e.g., for the POST method).
+To address this issue, it is recommended to enable the option either at the route level or globally in the :ref:`cch-global-configuration` section:
+
+.. code-block:: json
+
+    "CacheOptions": {
+      // ...
+      "EnableContentHashing": true
+    }
+
+.. _cch-global-configuration:
+
+Global Configuration
+--------------------
+
+The positive update is that copying Route-level properties for each route is no longer necessary, as version `23.3`_ allows for setting their values in the ``GlobalConfiguration``.
+This convenience extends to **Header** and **Region** as well.
+However, an alternative is still being sought for **TtlSeconds**, which must be explicitly set for each route to enable caching.
+
+Notes
+-----
+
 If you look at the example `here <https://github.com/ThreeMammals/Ocelot/blob/main/test/Ocelot.ManualTest/Program.cs>`_ you can see how the cache manager is setup and then passed into the Ocelot ``AddCacheManager`` configuration method.
 You can use any settings supported by the **CacheManager** package and just pass them in.
 
-Anyway, Ocelot currently supports caching on the URL of the downstream service and setting a TTL in seconds to expire the cache. You can also clear the cache for a region by calling Ocelot's administration API.
+Anyway, Ocelot currently supports caching on the URL of the downstream service and setting a TTL in seconds to expire the cache.
+You can also clear the cache for a region by calling Ocelot's administration API.
 
 Your Own Caching
 ----------------
@@ -68,3 +97,6 @@ If you want to add your own caching method, implement the following interfaces a
 Please dig into the Ocelot source code to find more.
 We would really appreciate it if anyone wants to implement `Redis <https://redis.io/>`_, `Memcached <http://www.memcached.org/>`_ etc.
 Please, open a new `Show and tell <https://github.com/ThreeMammals/Ocelot/discussions/categories/show-and-tell>`_ thread in `Discussions <https://github.com/ThreeMammals/Ocelot/discussions>`_ space of the repository.
+
+.. _23.0: https://github.com/ThreeMammals/Ocelot/releases/tag/23.0.0
+.. _23.3: https://github.com/ThreeMammals/Ocelot/releases/tag/23.3.0

src/Ocelot/Cache/AspMemoryCache.cs renamed to src/Ocelot/Cache/DefaultMemoryCache.cs

@@ -2,12 +2,12 @@
 
 namespace Ocelot.Cache
 {
-    public class AspMemoryCache<T> : IOcelotCache<T>
+    public class DefaultMemoryCache<T> : IOcelotCache<T>
     {
         private readonly IMemoryCache _memoryCache;
         private readonly Dictionary<string, List<string>> _regions;
 
-        public AspMemoryCache(IMemoryCache memoryCache)
+        public DefaultMemoryCache(IMemoryCache memoryCache)
         {
             _memoryCache = memoryCache;
             _regions = new Dictionary<string, List<string>>();

src/Ocelot/Cache/IRegionCreator.cs

@@ -19,7 +19,7 @@ public class DownstreamRouteBuilder
     private List<ClaimToThing> _claimToDownstreamPath;
     private string _requestIdHeaderKey;
     private bool _isCached;
-    private CacheOptions _fileCacheOptions;
+    private CacheOptions _cacheOptions;
     private string _downstreamScheme;
     private LoadBalancerOptions _loadBalancerOptions;
     private QoSOptions _qosOptions;
@@ -87,7 +87,7 @@ public DownstreamRouteBuilder WithUpstreamPathTemplate(UpstreamPathTemplate inpu
 
     public DownstreamRouteBuilder WithUpstreamHttpMethod(List<string> input)
     {
-        _upstreamHttpMethod = (input.Count == 0) ? new List<HttpMethod>() : input.Select(x => new HttpMethod(x.Trim())).ToList();
+        _upstreamHttpMethod = input.Count == 0 ? new List<HttpMethod>() : input.Select(x => new HttpMethod(x.Trim())).ToList();
         return this;
     }
 
@@ -147,7 +147,7 @@ public DownstreamRouteBuilder WithIsCached(bool input)
 
     public DownstreamRouteBuilder WithCacheOptions(CacheOptions input)
     {
-        _fileCacheOptions = input;
+        _cacheOptions = input;
         return this;
     }
 
@@ -276,7 +276,7 @@ public DownstreamRoute Build()
             _downstreamScheme,
             _requestIdHeaderKey,
             _isCached,
-            _fileCacheOptions,
+            _cacheOptions,
             _loadBalancerOptions,
             _rateLimitOptions,
             _routeClaimRequirement,

src/Ocelot/Cache/RegionCreator.cs

@@ -1,39 +1,42 @@
 using Ocelot.Request.Middleware;
 
-namespace Ocelot.Configuration
-{
-    public class CacheOptions
-    {
-        internal CacheOptions() { }
+namespace Ocelot.Configuration;
 
-        public CacheOptions(int ttlSeconds, string region, string header)
-        {
-            TtlSeconds = ttlSeconds;
-            Region = region;
-            Header = header;
-        }
-
-        public CacheOptions(int ttlSeconds, string region, string header, bool enableContentHashing)
-        {
-            TtlSeconds = ttlSeconds;
-            Region = region;
-            Header = header;
-            EnableContentHashing = enableContentHashing;
-        }
+public class CacheOptions
+{
+    internal CacheOptions() { }
 
-        public int TtlSeconds { get; }
-        public string Region { get; }
-        public string Header { get; }
-
-        /// <summary>
-        /// Enables MD5 hash calculation of the <see cref="HttpRequestMessage.Content"/> of the <see cref="DownstreamRequest.Request"/> object.
-        /// </summary>
-        /// <remarks>
-        /// Default value is <see langword="false"/>. No hashing by default.
-        /// </remarks>
-        /// <value>
-        /// <see langword="true"/> if hashing is enabled, otherwise it is <see langword="false"/>.
-        /// </value>
-        public bool EnableContentHashing { get; }
+    /// <summary>
+    /// Initializes a new instance of the <see cref="CacheOptions"/> class.
+    /// </summary>
+    /// <remarks>
+    /// Internal defaults:
+    ///   <list type="bullet">
+    ///   <item>The default value for <see cref="EnableContentHashing"/> is <see langword="false"/>, but it is set to null for route-level configuration to allow global configuration usage.</item>
+    ///   <item>The default value for <see cref="TtlSeconds"/> is 0.</item>
+    ///   </list>
+    /// </remarks>
+    /// <param name="ttlSeconds">Time-to-live seconds. If not speciefied, zero value is used by default.</param>
+    /// <param name="region">The region of caching.</param>
+    /// <param name="header">The header name to control cached value.</param>
+    /// <param name="enableContentHashing">The switcher for content hashing. If not speciefied, false value is used by default.</param>
+    public CacheOptions(int? ttlSeconds, string region, string header, bool? enableContentHashing)
+    {
+        TtlSeconds = ttlSeconds ?? 0;
+        Region = region;
+        Header = header;
+        EnableContentHashing = enableContentHashing ?? false;
     }
-}
+
+    /// <summary>Time-to-live seconds.</summary>
+    /// <remarks>Default value is 0. No caching by default.</remarks>
+    /// <value>An <see cref="int"/> value of seconds.</value>
+    public int TtlSeconds { get; }
+    public string Region { get; }
+    public string Header { get; }
+
+    /// <summary>Enables MD5 hash calculation of the <see cref="HttpRequestMessage.Content"/> of the <see cref="DownstreamRequest.Request"/> object.</summary>
+    /// <remarks>Default value is <see langword="false"/>. No hashing by default.</remarks>
+    /// <value><see langword="true"/> if hashing is enabled, otherwise it is <see langword="false"/>.</value>
+    public bool EnableContentHashing { get; }
+}

src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs

@@ -0,0 +1,27 @@
+using Ocelot.Configuration.File;
+
+namespace Ocelot.Configuration.Creator;
+
+public class CacheOptionsCreator : ICacheOptionsCreator
+{
+    public CacheOptions Create(FileCacheOptions options, FileGlobalConfiguration global, string upstreamPathTemplate, IList<string> upstreamHttpMethods)
+    {
+        var region = GetRegion(options.Region ?? global?.CacheOptions.Region, upstreamPathTemplate, upstreamHttpMethods);
+        var header = options.Header ?? global?.CacheOptions.Header;
+        var ttlSeconds = options.TtlSeconds ?? global?.CacheOptions.TtlSeconds;
+        var enableContentHashing = options.EnableContentHashing ?? global?.CacheOptions.EnableContentHashing;
+
+        return new CacheOptions(ttlSeconds, region, header, enableContentHashing);
+    }
+
+    protected virtual string GetRegion(string region, string upstreamPathTemplate, IList<string> upstreamHttpMethod)
+    {
+        if (!string.IsNullOrEmpty(region))
+        {
+            return region;
+        }
+
+        var methods = string.Join(string.Empty, upstreamHttpMethod);
+        return $"{methods}{upstreamPathTemplate.Replace("/", string.Empty)}";
+    }
+}

src/Ocelot/Configuration/CacheOptions.cs

@@ -0,0 +1,19 @@
+using Ocelot.Configuration.File;
+
+namespace Ocelot.Configuration.Creator;
+
+/// <summary>
+/// This interface is used to create cache options.
+/// </summary>
+public interface ICacheOptionsCreator
+{
+    /// <summary>
+    /// Creates cache options based on the file cache options, upstream path template and upstream HTTP methods.</summary>
+    /// <remarks>Upstream path template and upstream HTTP methods are used to get the region name.</remarks>
+    /// <param name="options">The file cache options.</param>
+    /// <param name="global">The global configuration.</param>
+    /// <param name="upstreamPathTemplate">The upstream path template as string.</param>
+    /// <param name="upstreamHttpMethods">The upstream http methods as a list of strings.</param>
+    /// <returns>The generated cache options.</returns>
+    CacheOptions Create(FileCacheOptions options, FileGlobalConfiguration global, string upstreamPathTemplate, IList<string> upstreamHttpMethods);
+}

src/Ocelot/Configuration/Creator/CacheOptionsCreator.cs

@@ -17,6 +17,8 @@ public RouteOptions Create(FileRoute fileRoute)
                 && (!string.IsNullOrEmpty(authOpts.AuthenticationProviderKey)
                     || authOpts.AuthenticationProviderKeys?.Any(k => !string.IsNullOrWhiteSpace(k)) == true);
             var isAuthorized = fileRoute.RouteClaimsRequirement?.Any() == true;
+
+            // TODO: This sounds more like a hack, it might be better to refactor this at some point.
             var isCached = fileRoute.FileCacheOptions.TtlSeconds > 0;
             var enableRateLimiting = fileRoute.RateLimitOptions?.EnableRateLimiting == true;
             var useServiceDiscovery = !string.IsNullOrEmpty(fileRoute.ServiceName);

src/Ocelot/Configuration/Creator/ICacheOptionsCreator.cs

@@ -1,5 +1,4 @@
-using Ocelot.Cache;
-using Ocelot.Configuration.Builder;
+using Ocelot.Configuration.Builder;
 using Ocelot.Configuration.File;
 
 namespace Ocelot.Configuration.Creator
@@ -14,7 +13,7 @@ public class RoutesCreator : IRoutesCreator
         private readonly IQoSOptionsCreator _qosOptionsCreator;
         private readonly IRouteOptionsCreator _fileRouteOptionsCreator;
         private readonly IRateLimitOptionsCreator _rateLimitOptionsCreator;
-        private readonly IRegionCreator _regionCreator;
+        private readonly ICacheOptionsCreator _cacheOptionsCreator;
         private readonly IHttpHandlerOptionsCreator _httpHandlerOptionsCreator;
         private readonly IHeaderFindAndReplaceCreator _headerFAndRCreator;
         private readonly IDownstreamAddressesCreator _downstreamAddressesCreator;
@@ -30,21 +29,20 @@ public RoutesCreator(
             IQoSOptionsCreator qosOptionsCreator,
             IRouteOptionsCreator fileRouteOptionsCreator,
             IRateLimitOptionsCreator rateLimitOptionsCreator,
-            IRegionCreator regionCreator,
+            ICacheOptionsCreator cacheOptionsCreator,
             IHttpHandlerOptionsCreator httpHandlerOptionsCreator,
             IHeaderFindAndReplaceCreator headerFAndRCreator,
             IDownstreamAddressesCreator downstreamAddressesCreator,
             ILoadBalancerOptionsCreator loadBalancerOptionsCreator,
             IRouteKeyCreator routeKeyCreator,
             ISecurityOptionsCreator securityOptionsCreator,
-            IVersionCreator versionCreator
-            )
+            IVersionCreator versionCreator)
         {
             _routeKeyCreator = routeKeyCreator;
             _loadBalancerOptionsCreator = loadBalancerOptionsCreator;
             _downstreamAddressesCreator = downstreamAddressesCreator;
             _headerFAndRCreator = headerFAndRCreator;
-            _regionCreator = regionCreator;
+            _cacheOptionsCreator = cacheOptionsCreator;
             _rateLimitOptionsCreator = rateLimitOptionsCreator;
             _requestIdKeyCreator = requestIdKeyCreator;
             _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator;
@@ -93,8 +91,6 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf
 
             var rateLimitOption = _rateLimitOptionsCreator.Create(fileRoute.RateLimitOptions, globalConfiguration);
 
-            var region = _regionCreator.Create(fileRoute);
-
             var httpHandlerOptions = _httpHandlerOptionsCreator.Create(fileRoute.HttpHandlerOptions);
 
             var hAndRs = _headerFAndRCreator.Create(fileRoute);
@@ -107,6 +103,8 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf
 
             var downstreamHttpVersion = _versionCreator.Create(fileRoute.DownstreamHttpVersion);
 
+            var cacheOptions = _cacheOptionsCreator.Create(fileRoute.FileCacheOptions, globalConfiguration, fileRoute.UpstreamPathTemplate, fileRoute.UpstreamHttpMethod);
+
             var route = new DownstreamRouteBuilder()
                 .WithKey(fileRoute.Key)
                 .WithDownstreamPathTemplate(fileRoute.DownstreamPathTemplate)
@@ -122,7 +120,7 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf
                 .WithClaimsToDownstreamPath(claimsToDownstreamPath)
                 .WithRequestIdKey(requestIdKey)
                 .WithIsCached(fileRouteOptions.IsCached)
-                .WithCacheOptions(new CacheOptions(fileRoute.FileCacheOptions.TtlSeconds, region, fileRoute.FileCacheOptions.Header))
+                .WithCacheOptions(cacheOptions)
                 .WithDownstreamScheme(fileRoute.DownstreamScheme)
                 .WithLoadBalancerOptions(lbOptions)
                 .WithDownstreamAddresses(downstreamAddresses)

src/Ocelot/Configuration/Creator/RouteOptionsCreator.cs

@@ -1,21 +1,26 @@
-namespace Ocelot.Configuration.File
-{
-    public class FileCacheOptions
-    {
-        public FileCacheOptions()
-        {
-            Region = string.Empty;
-            TtlSeconds = 0;
-        }
+namespace Ocelot.Configuration.File;
 
-        public FileCacheOptions(FileCacheOptions from)
-        {
-            Region = from.Region;
-            TtlSeconds = from.TtlSeconds;
-        }
+public class FileCacheOptions
+{
+    public FileCacheOptions() { }
 
-        public int TtlSeconds { get; set; }
-        public string Region { get; set; }
-        public string Header { get; set; }
-    }
+    public FileCacheOptions(FileCacheOptions from)
+    {
+        Region = from.Region;
+        TtlSeconds = from.TtlSeconds;
+        Header = from.Header;
+        EnableContentHashing = from.EnableContentHashing;
+    }
+
+    /// <summary>Using <see cref="Nullable{T}"/> where T is <see cref="int"/> to have <see langword="null"/> as default value and allowing global configuration usage.</summary>
+    /// <remarks>If <see langword="null"/> then use global configuration with 0 by default.</remarks>
+    /// <value>The time to live seconds, with 0 by default.</value>
+    public int? TtlSeconds { get; set; }
+    public string Region { get; set; }
+    public string Header { get; set; }
+
+    /// <summary>Using <see cref="Nullable{T}"/> where T is <see cref="bool"/> to have <see langword="null"/> as default value and allowing global configuration usage.</summary>
+    /// <remarks>If <see langword="null"/> then use global configuration with <see langword="false"/> by default.</remarks>
+    /// <value><see langword="true"/> if content hashing is enabled; otherwise, <see langword="false"/>.</value>
+    public bool? EnableContentHashing { get; set; }
 }