Sync output cache with API review

src/Middleware/OutputCaching/src/CacheVaryByRules.cs

@@ -21,9 +21,14 @@ public sealed class CacheVaryByRules
     public IDictionary<string, string> VaryByCustom => _varyByCustom ??= new();
 
     /// <summary>
-    /// Gets or sets the list of headers to vary by.
+    /// Gets or sets the list of route value names to vary by.
     /// </summary>
-    public StringValues Headers { get; set; }
+    public StringValues RouteValueNames { get; set; }
+
+    /// <summary>
+    /// Gets or sets the list of header names to vary by.
+    /// </summary>
+    public StringValues HeaderNames { get; set; }
 
     /// <summary>
     /// Gets or sets the list of query string keys to vary by.

src/Middleware/OutputCaching/src/LoggerExtensions.cs

@@ -11,59 +11,44 @@ namespace Microsoft.AspNetCore.OutputCaching;
 /// </summary>
 internal static partial class LoggerExtensions
 {
-    [LoggerMessage(1, LogLevel.Debug, "The request cannot be served from cache because it uses the HTTP method: {Method}.",
-        EventName = "RequestMethodNotCacheable")]
-    internal static partial void RequestMethodNotCacheable(this ILogger logger, string method);
-
-    [LoggerMessage(2, LogLevel.Debug, "The request cannot be served from cache because it contains an 'Authorization' header.",
-        EventName = "RequestWithAuthorizationNotCacheable")]
-    internal static partial void RequestWithAuthorizationNotCacheable(this ILogger logger);
-
-    [LoggerMessage(3, LogLevel.Debug, "Response is not cacheable because it contains a 'SetCookie' header.", EventName = "ResponseWithSetCookieNotCacheable")]
-    internal static partial void ResponseWithSetCookieNotCacheable(this ILogger logger);
-
-    [LoggerMessage(4, LogLevel.Debug, "Response is not cacheable because its status code {StatusCode} does not indicate success.",
-        EventName = "ResponseWithUnsuccessfulStatusCodeNotCacheable")]
-    internal static partial void ResponseWithUnsuccessfulStatusCodeNotCacheable(this ILogger logger, int statusCode);
-
-    [LoggerMessage(5, LogLevel.Debug, "The 'IfNoneMatch' header of the request contains a value of *.", EventName = "NotModifiedIfNoneMatchStar")]
+    [LoggerMessage(1, LogLevel.Debug, "The 'IfNoneMatch' header of the request contains a value of *.", EventName = "NotModifiedIfNoneMatchStar")]
     internal static partial void NotModifiedIfNoneMatchStar(this ILogger logger);
 
-    [LoggerMessage(6, LogLevel.Debug, "The ETag {ETag} in the 'IfNoneMatch' header matched the ETag of a cached entry.",
+    [LoggerMessage(2, LogLevel.Debug, "The ETag {ETag} in the 'IfNoneMatch' header matched the ETag of a cached entry.",
         EventName = "NotModifiedIfNoneMatchMatched")]
     internal static partial void NotModifiedIfNoneMatchMatched(this ILogger logger, EntityTagHeaderValue etag);
 
-    [LoggerMessage(7, LogLevel.Debug, "The last modified date of {LastModified} is before the date {IfModifiedSince} specified in the 'IfModifiedSince' header.",
+    [LoggerMessage(3, LogLevel.Debug, "The last modified date of {LastModified} is before the date {IfModifiedSince} specified in the 'IfModifiedSince' header.",
         EventName = "NotModifiedIfModifiedSinceSatisfied")]
     internal static partial void NotModifiedIfModifiedSinceSatisfied(this ILogger logger, DateTimeOffset lastModified, DateTimeOffset ifModifiedSince);
 
-    [LoggerMessage(8, LogLevel.Information, "The content requested has not been modified.", EventName = "NotModifiedServed")]
+    [LoggerMessage(4, LogLevel.Information, "The content requested has not been modified.", EventName = "NotModifiedServed")]
     internal static partial void NotModifiedServed(this ILogger logger);
 
-    [LoggerMessage(9, LogLevel.Information, "Serving response from cache.", EventName = "CachedResponseServed")]
+    [LoggerMessage(5, LogLevel.Information, "Serving response from cache.", EventName = "CachedResponseServed")]
     internal static partial void CachedResponseServed(this ILogger logger);
 
-    [LoggerMessage(10, LogLevel.Information, "No cached response available for this request and the 'only-if-cached' cache directive was specified.",
+    [LoggerMessage(6, LogLevel.Information, "No cached response available for this request and the 'only-if-cached' cache directive was specified.",
         EventName = "GatewayTimeoutServed")]
     internal static partial void GatewayTimeoutServed(this ILogger logger);
 
-    [LoggerMessage(11, LogLevel.Information, "No cached response available for this request.", EventName = "NoResponseServed")]
+    [LoggerMessage(7, LogLevel.Information, "No cached response available for this request.", EventName = "NoResponseServed")]
     internal static partial void NoResponseServed(this ILogger logger);
 
-    [LoggerMessage(12, LogLevel.Debug, "Vary by rules were updated. Headers: {Headers}, Query keys: {QueryKeys}", EventName = "VaryByRulesUpdated")]
-    internal static partial void VaryByRulesUpdated(this ILogger logger, string headers, string queryKeys);
+    [LoggerMessage(8, LogLevel.Debug, "Vary by rules were updated. Header names: {HeaderNames}, Query keys: {QueryKeys}, Route value names: {RouteValueNames}", EventName = "VaryByRulesUpdated")]
+    internal static partial void VaryByRulesUpdated(this ILogger logger, string headerNames, string queryKeys, string routeValueNames);
 
-    [LoggerMessage(13, LogLevel.Information, "The response has been cached.", EventName = "ResponseCached")]
+    [LoggerMessage(9, LogLevel.Information, "The response has been cached.", EventName = "ResponseCached")]
     internal static partial void ResponseCached(this ILogger logger);
 
-    [LoggerMessage(14, LogLevel.Information, "The response could not be cached for this request.", EventName = "ResponseNotCached")]
+    [LoggerMessage(10, LogLevel.Information, "The response could not be cached for this request.", EventName = "ResponseNotCached")]
     internal static partial void ResponseNotCached(this ILogger logger);
 
-    [LoggerMessage(15, LogLevel.Warning, "The response could not be cached for this request because the 'Content-Length' did not match the body length.",
+    [LoggerMessage(11, LogLevel.Warning, "The response could not be cached for this request because the 'Content-Length' did not match the body length.",
         EventName = "ResponseContentLengthMismatchNotCached")]
     internal static partial void ResponseContentLengthMismatchNotCached(this ILogger logger);
 
-    [LoggerMessage(16, LogLevel.Debug, "The response time of the entry is {ResponseTime} and has exceeded its expiry date.",
+    [LoggerMessage(12, LogLevel.Debug, "The response time of the entry is {ResponseTime} and has exceeded its expiry date.",
         EventName = "ExpirationExpiresExceeded")]
     internal static partial void ExpirationExpiresExceeded(this ILogger logger, DateTimeOffset responseTime);
 

src/Middleware/OutputCaching/src/OutputCacheAttribute.cs

@@ -45,9 +45,14 @@ public bool NoStore
     public string[]? VaryByQueryKeys { get; init; }
 
     /// <summary>
-    /// Gets or sets the headers to vary by.
+    /// Gets or sets the header names to vary by.
     /// </summary>
-    public string[]? VaryByHeaders { get; init; }
+    public string[]? VaryByHeaderNames { get; init; }
+
+    /// <summary>
+    /// Gets or sets the route value names to vary by.
+    /// </summary>
+    public string[]? VaryByRouteValueNames { get; init; }
 
     /// <summary>
     /// Gets or sets the value of the cache policy name.
@@ -78,6 +83,16 @@ internal IOutputCachePolicy BuildPolicy()
             builder.VaryByQuery(VaryByQueryKeys);
         }
 
+        if (VaryByHeaderNames != null)
+        {
+            builder.VaryByHeader(VaryByHeaderNames);
+        }
+
+        if (VaryByRouteValueNames != null)
+        {
+            builder.VaryByRouteValue(VaryByRouteValueNames);
+        }
+
         if (_duration != null)
         {
             builder.Expire(TimeSpan.FromSeconds(_duration.Value));

src/Middleware/OutputCaching/src/OutputCacheContext.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.AspNetCore.Http;
-using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.OutputCaching;
 
@@ -11,12 +10,8 @@ namespace Microsoft.AspNetCore.OutputCaching;
 /// </summary>
 public sealed class OutputCacheContext
 {
-    internal OutputCacheContext(HttpContext httpContext, IOutputCacheStore store, OutputCacheOptions options, ILogger logger)
+    public OutputCacheContext()
     {
-        HttpContext = httpContext;
-        Logger = logger;
-        Store = store;
-        Options = options;
     }
 
     /// <summary>
@@ -42,17 +37,17 @@ internal OutputCacheContext(HttpContext httpContext, IOutputCacheStore store, Ou
     /// <summary>
     /// Gets the <see cref="HttpContext"/>.
     /// </summary>
-    public HttpContext HttpContext { get; }
+    public required HttpContext HttpContext { get; init; }
 
     /// <summary>
-    /// Gets the response time.
+    /// Gets or sets the response time.
     /// </summary>
-    public DateTimeOffset? ResponseTime { get; internal set; }
+    public DateTimeOffset? ResponseTime { get; set; }
 
     /// <summary>
     /// Gets the <see cref="CacheVaryByRules"/> instance.
     /// </summary>
-    public CacheVaryByRules CacheVaryByRules { get; set; } = new();
+    public CacheVaryByRules CacheVaryByRules { get; } = new();
 
     /// <summary>
     /// Gets the tags of the cached response.
@@ -79,7 +74,4 @@ internal OutputCacheContext(HttpContext httpContext, IOutputCacheStore store, Ou
     internal Stream OriginalResponseStream { get; set; } = default!;
 
     internal OutputCacheStream OutputCacheStream { get; set; } = default!;
-    internal ILogger Logger { get; }
-    internal OutputCacheOptions Options { get; }
-    internal IOutputCacheStore Store { get; }
 }

src/Middleware/OutputCaching/src/OutputCacheKeyProvider.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Globalization;
 using System.Linq;
 using System.Text;
 using Microsoft.Extensions.ObjectPool;
@@ -28,7 +29,7 @@ internal OutputCacheKeyProvider(ObjectPoolProvider poolProvider, IOptions<Output
         _options = options.Value;
     }
 
-    // GET<delimiter>SCHEME<delimiter>HOST:PORT/PATHBASE/PATH<delimiter>H<delimiter>HeaderName=HeaderValue<delimiter>Q<delimiter>QueryName=QueryValue1<subdelimiter>QueryValue2
+    // GET<delimiter>SCHEME<delimiter>HOST:PORT/PATHBASE/PATH<delimiter>H<delimiter>HeaderName=HeaderValue<delimiter>Q<delimiter>QueryName=QueryValue1<subdelimiter>QueryValue2<delimiter>R<delimiter>RouteName1=RouteValue1<subdelimiter>RouteName2=RouteValue2
     public string CreateStorageKey(OutputCacheContext context)
     {
         ArgumentNullException.ThrowIfNull(_builderPool);
@@ -79,8 +80,8 @@ public string CreateStorageKey(OutputCacheContext context)
                 }
             }
 
-            // Vary by headers
-            var headersCount = varyByRules?.Headers.Count ?? 0;
+            // Vary by header names
+            var headersCount = varyByRules?.HeaderNames.Count ?? 0;
             if (headersCount > 0)
             {
                 // Append a group separator for the header segment of the cache key
@@ -90,7 +91,7 @@ public string CreateStorageKey(OutputCacheContext context)
                 var requestHeaders = context.HttpContext.Request.Headers;
                 for (var i = 0; i < headersCount; i++)
                 {
-                    var header = varyByRules!.Headers[i] ?? string.Empty;
+                    var header = varyByRules!.HeaderNames[i] ?? string.Empty;
                     var headerValues = requestHeaders[header];
                     builder.Append(KeyDelimiter)
                         .Append(header)
@@ -166,6 +167,29 @@ public string CreateStorageKey(OutputCacheContext context)
                 }
             }
 
+            // Vary by route value names
+            var routeValueNamesCount = varyByRules?.RouteValueNames.Count ?? 0;
+            if (routeValueNamesCount > 0)
+            {
+                // Append a group separator for the route values segment of the cache key
+                builder.Append(KeyDelimiter)
+                    .Append('R');
+
+                for (var i = 0; i < routeValueNamesCount; i++)
+                {
+                    // The lookup key can't be null
+                    var routeValueName = varyByRules!.RouteValueNames[i] ?? string.Empty;
+
+                    // RouteValueNames returns null if the key doesn't exist
+                    var routeValueValue = context.HttpContext.Request.RouteValues[routeValueName];
+
+                    builder.Append(KeyDelimiter)
+                        .Append(routeValueName)
+                        .Append('=')
+                        .Append(Convert.ToString(routeValueValue, CultureInfo.InvariantCulture));
+                }
+            }
+
             return builder.ToString();
         }
         finally

src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs

@@ -92,7 +92,7 @@ public Task Invoke(HttpContext httpContext)
 
     private async Task InvokeAwaited(HttpContext httpContext, IReadOnlyList<IOutputCachePolicy> policies)
     {
-        var context = new OutputCacheContext(httpContext, _store, _options, _logger);
+        var context = new OutputCacheContext { HttpContext = httpContext };
 
         // Add IOutputCacheFeature
         AddOutputCacheFeature(context);
@@ -247,7 +247,7 @@ internal async Task<bool> TryServeCachedResponseAsync(OutputCacheContext context
         // Validate expiration
         if (context.CachedEntryAge <= TimeSpan.Zero)
         {
-            context.Logger.ExpirationExpiresExceeded(context.ResponseTime!.Value);
+            _logger.ExpirationExpiresExceeded(context.ResponseTime!.Value);
             context.IsCacheEntryFresh = false;
         }
 
@@ -316,7 +316,7 @@ internal async Task<bool> TryServeFromCacheAsync(OutputCacheContext cacheContext
         // TODO: should it be part of the cache implementations or can we assume all caches would benefit from it?
         // It makes sense for caches that use IO (disk, network) or need to deserialize the state but could also be a global option
 
-        var cacheEntry = await _outputCacheEntryDispatcher.ScheduleAsync(cacheContext.CacheKey, cacheContext, static async (key, cacheContext) => await OutputCacheEntryFormatter.GetAsync(key, cacheContext.Store, cacheContext.HttpContext.RequestAborted));
+        var cacheEntry = await _outputCacheEntryDispatcher.ScheduleAsync(cacheContext.CacheKey, (Store: _store, CacheContext: cacheContext), static async (key, state) => await OutputCacheEntryFormatter.GetAsync(key, state.Store, state.CacheContext.HttpContext.RequestAborted));
 
         if (await TryServeCachedResponseAsync(cacheContext, cacheEntry, policies))
         {
@@ -341,32 +341,33 @@ internal void CreateCacheKey(OutputCacheContext context)
             return;
         }
 
-        var varyHeaders = context.CacheVaryByRules.Headers;
+        var varyHeaderNames = context.CacheVaryByRules.HeaderNames;
+        var varyRouteValueNames = context.CacheVaryByRules.RouteValueNames;
         var varyQueryKeys = context.CacheVaryByRules.QueryKeys;
-        var varyByCustomKeys = context.CacheVaryByRules.VaryByCustom;
+        var varyByCustomKeys = context.CacheVaryByRules.HasVaryByCustom ? context.CacheVaryByRules.VaryByCustom : null;
         var varyByPrefix = context.CacheVaryByRules.VaryByPrefix;
 
         // Check if any vary rules exist
-        if (!StringValues.IsNullOrEmpty(varyHeaders) || !StringValues.IsNullOrEmpty(varyQueryKeys) || !StringValues.IsNullOrEmpty(varyByPrefix) || varyByCustomKeys?.Count > 0)
+        if (!StringValues.IsNullOrEmpty(varyHeaderNames) || !StringValues.IsNullOrEmpty(varyRouteValueNames) || !StringValues.IsNullOrEmpty(varyQueryKeys) || !StringValues.IsNullOrEmpty(varyByPrefix) || varyByCustomKeys?.Count > 0)
         {
             // Normalize order and casing of vary by rules
-            var normalizedVaryHeaders = GetOrderCasingNormalizedStringValues(varyHeaders);
+            var normalizedVaryHeaderNames = GetOrderCasingNormalizedStringValues(varyHeaderNames);
+            var normalizedVaryRouteValueNames = GetOrderCasingNormalizedStringValues(varyRouteValueNames);
             var normalizedVaryQueryKeys = GetOrderCasingNormalizedStringValues(varyQueryKeys);
             var normalizedVaryByCustom = GetOrderCasingNormalizedDictionary(varyByCustomKeys);
 
             // Update vary rules with normalized values
-            context.CacheVaryByRules = new CacheVaryByRules
-            {
-                VaryByPrefix = varyByPrefix + normalizedVaryByCustom,
-                Headers = normalizedVaryHeaders,
-                QueryKeys = normalizedVaryQueryKeys
-            };
+            context.CacheVaryByRules.VaryByCustom.Clear();
+            context.CacheVaryByRules.VaryByPrefix = varyByPrefix + normalizedVaryByCustom;
+            context.CacheVaryByRules.HeaderNames = normalizedVaryHeaderNames;
+            context.CacheVaryByRules.RouteValueNames = normalizedVaryRouteValueNames;
+            context.CacheVaryByRules.QueryKeys = normalizedVaryQueryKeys;
 
             // TODO: Add same condition on LogLevel in Response Caching
             // Always overwrite the CachedVaryByRules to update the expiry information
             if (_logger.IsEnabled(LogLevel.Debug))
             {
-                _logger.VaryByRulesUpdated(normalizedVaryHeaders.ToString(), normalizedVaryQueryKeys.ToString());
+                _logger.VaryByRulesUpdated(normalizedVaryHeaderNames.ToString(), normalizedVaryQueryKeys.ToString(), normalizedVaryRouteValueNames.ToString());
             }
         }
 
@@ -516,7 +517,7 @@ internal static void UnshimResponseStream(OutputCacheContext context)
         RemoveOutputCacheFeature(context.HttpContext);
     }
 
-    internal static bool ContentIsNotModified(OutputCacheContext context)
+    internal bool ContentIsNotModified(OutputCacheContext context)
     {
         var cachedResponseHeaders = context.CachedResponse.Headers;
         var ifNoneMatchHeader = context.HttpContext.Request.Headers.IfNoneMatch;
@@ -525,7 +526,7 @@ internal static bool ContentIsNotModified(OutputCacheContext context)
         {
             if (ifNoneMatchHeader.Count == 1 && StringSegment.Equals(ifNoneMatchHeader[0], EntityTagHeaderValue.Any.Tag, StringComparison.OrdinalIgnoreCase))
             {
-                context.Logger.NotModifiedIfNoneMatchStar();
+                _logger.NotModifiedIfNoneMatchStar();
                 return true;
             }
 
@@ -538,7 +539,7 @@ internal static bool ContentIsNotModified(OutputCacheContext context)
                     var requestETag = ifNoneMatchEtags[i];
                     if (eTag.Compare(requestETag, useStrongComparison: false))
                     {
-                        context.Logger.NotModifiedIfNoneMatchMatched(requestETag);
+                        _logger.NotModifiedIfNoneMatchMatched(requestETag);
                         return true;
                     }
                 }
@@ -558,7 +559,7 @@ internal static bool ContentIsNotModified(OutputCacheContext context)
                 if (HeaderUtilities.TryParseDate(ifModifiedSince.ToString(), out var modifiedSince) &&
                     modified <= modifiedSince)
                 {
-                    context.Logger.NotModifiedIfModifiedSinceSatisfied(modified, modifiedSince);
+                    _logger.NotModifiedIfModifiedSinceSatisfied(modified, modifiedSince);
                     return true;
                 }
             }