diff --git a/src/Microsoft.AspNetCore.ResponseCaching/Internal/LoggerExtensions.cs b/src/Microsoft.AspNetCore.ResponseCaching/Internal/LoggerExtensions.cs index b900731..64c67f6 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/Internal/LoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/Internal/LoggerExtensions.cs @@ -31,7 +31,7 @@ internal static class LoggerExtensions private static Action _logResponseWithUnsuccessfulStatusCodeNotCacheable; private static Action _logNotModifiedIfNoneMatchStar; private static Action _logNotModifiedIfNoneMatchMatched; - private static Action _logNotModifiedIfUnmodifiedSinceSatisfied; + private static Action _logNotModifiedIfModifiedSinceSatisfied; private static Action _logNotModifiedServed; private static Action _logCachedResponseServed; private static Action _logGatewayTimeoutServed; @@ -119,10 +119,10 @@ static LoggerExtensions() logLevel: LogLevel.Debug, eventId: 19, formatString: $"The ETag {{ETag}} in the '{HeaderNames.IfNoneMatch}' header matched the ETag of a cached entry."); - _logNotModifiedIfUnmodifiedSinceSatisfied = LoggerMessage.Define( + _logNotModifiedIfModifiedSinceSatisfied = LoggerMessage.Define( logLevel: LogLevel.Debug, eventId: 20, - formatString: $"The last modified date of {{LastModified}} is before the date {{IfUnmodifiedSince}} specified in the '{HeaderNames.IfUnmodifiedSince}' header."); + formatString: $"The last modified date of {{LastModified}} is before the date {{IfModifiedSince}} specified in the '{HeaderNames.IfModifiedSince}' header."); _logNotModifiedServed = LoggerMessage.Define( logLevel: LogLevel.Information, eventId: 21, @@ -252,9 +252,9 @@ internal static void LogNotModifiedIfNoneMatchMatched(this ILogger logger, Entit _logNotModifiedIfNoneMatchMatched(logger, etag, null); } - internal static void LogNotModifiedIfUnmodifiedSinceSatisfied(this ILogger logger, DateTimeOffset lastModified, DateTimeOffset ifUnmodifiedSince) + internal static void LogNotModifiedIfModifiedSinceSatisfied(this ILogger logger, DateTimeOffset lastModified, DateTimeOffset ifModifiedSince) { - _logNotModifiedIfUnmodifiedSinceSatisfied(logger, lastModified, ifUnmodifiedSince, null); + _logNotModifiedIfModifiedSinceSatisfied(logger, lastModified, ifModifiedSince, null); } internal static void LogNotModifiedServed(this ILogger logger) diff --git a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs index dc01df4..d1c3a01 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs @@ -106,7 +106,17 @@ public async Task Invoke(HttpContext httpContext) } else { - await _next(httpContext); + // Add IResponseCachingFeature which may be required when the response is generated + AddResponseCachingFeature(httpContext); + + try + { + await _next(httpContext); + } + finally + { + RemoveResponseCachingFeature(httpContext); + } } } @@ -318,6 +328,15 @@ internal Task OnResponseStartingAsync(ResponseCachingContext context) } } + internal static void AddResponseCachingFeature(HttpContext context) + { + if (context.Features.Get() != null) + { + throw new InvalidOperationException($"Another instance of {nameof(IResponseCachingFeature)} already exists. Only one instance of {nameof(ResponseCachingMiddleware)} can be configured for an application."); + } + context.Features.Set(new ResponseCachingFeature()); + } + internal void ShimResponseStream(ResponseCachingContext context) { // Shim response stream @@ -332,14 +351,12 @@ internal void ShimResponseStream(ResponseCachingContext context) context.HttpContext.Features.Set(new SendFileFeatureWrapper(context.OriginalSendFileFeature, context.ResponseCachingStream)); } - // Add IResponseCachingFeature - if (context.HttpContext.Features.Get() != null) - { - throw new InvalidOperationException($"Another instance of {nameof(ResponseCachingFeature)} already exists. Only one instance of {nameof(ResponseCachingMiddleware)} can be configured for an application."); - } - context.HttpContext.Features.Set(new ResponseCachingFeature()); + AddResponseCachingFeature(context.HttpContext); } + internal static void RemoveResponseCachingFeature(HttpContext context) + => context.Features.Set(null); + internal static void UnshimResponseStream(ResponseCachingContext context) { // Unshim response stream @@ -349,7 +366,7 @@ internal static void UnshimResponseStream(ResponseCachingContext context) context.HttpContext.Features.Set(context.OriginalSendFileFeature); // Remove IResponseCachingFeature - context.HttpContext.Features.Set(null); + RemoveResponseCachingFeature(context.HttpContext); } internal static bool ContentIsNotModified(ResponseCachingContext context) @@ -379,13 +396,13 @@ internal static bool ContentIsNotModified(ResponseCachingContext context) } else { - var ifUnmodifiedSince = context.TypedRequestHeaders.IfUnmodifiedSince; - if (ifUnmodifiedSince != null) + var ifModifiedSince = context.TypedRequestHeaders.IfModifiedSince; + if (ifModifiedSince != null) { var lastModified = cachedResponseHeaders.LastModified ?? cachedResponseHeaders.Date; - if (lastModified <= ifUnmodifiedSince) + if (lastModified <= ifModifiedSince) { - context.Logger.LogNotModifiedIfUnmodifiedSinceSatisfied(lastModified.Value, ifUnmodifiedSince.Value); + context.Logger.LogNotModifiedIfModifiedSinceSatisfied(lastModified.Value, ifModifiedSince.Value); return true; } } diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingMiddlewareTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingMiddlewareTests.cs index fb74241..564e562 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingMiddlewareTests.cs @@ -5,8 +5,10 @@ using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Headers; using Microsoft.AspNetCore.ResponseCaching.Internal; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -156,14 +158,14 @@ public void ContentIsNotModified_NotConditionalRequest_False() } [Fact] - public void ContentIsNotModified_IfUnmodifiedSince_FallsbackToDateHeader() + public void ContentIsNotModified_IfModifiedSince_FallsbackToDateHeader() { var utcNow = DateTimeOffset.UtcNow; var sink = new TestSink(); var context = TestUtils.CreateTestContext(sink); context.CachedResponseHeaders = new ResponseHeaders(new HeaderDictionary()); - context.TypedRequestHeaders.IfUnmodifiedSince = utcNow; + context.TypedRequestHeaders.IfModifiedSince = utcNow; // Verify modifications in the past succeeds context.CachedResponseHeaders.Date = utcNow - TimeSpan.FromSeconds(10); @@ -182,19 +184,19 @@ public void ContentIsNotModified_IfUnmodifiedSince_FallsbackToDateHeader() // Verify logging TestUtils.AssertLoggedMessages( sink.Writes, - LoggedMessage.NotModifiedIfUnmodifiedSinceSatisfied, - LoggedMessage.NotModifiedIfUnmodifiedSinceSatisfied); + LoggedMessage.NotModifiedIfModifiedSinceSatisfied, + LoggedMessage.NotModifiedIfModifiedSinceSatisfied); } [Fact] - public void ContentIsNotModified_IfUnmodifiedSince_LastModifiedOverridesDateHeader() + public void ContentIsNotModified_IfModifiedSince_LastModifiedOverridesDateHeader() { var utcNow = DateTimeOffset.UtcNow; var sink = new TestSink(); var context = TestUtils.CreateTestContext(sink); context.CachedResponseHeaders = new ResponseHeaders(new HeaderDictionary()); - context.TypedRequestHeaders.IfUnmodifiedSince = utcNow; + context.TypedRequestHeaders.IfModifiedSince = utcNow; // Verify modifications in the past succeeds context.CachedResponseHeaders.Date = utcNow + TimeSpan.FromSeconds(10); @@ -216,20 +218,20 @@ public void ContentIsNotModified_IfUnmodifiedSince_LastModifiedOverridesDateHead // Verify logging TestUtils.AssertLoggedMessages( sink.Writes, - LoggedMessage.NotModifiedIfUnmodifiedSinceSatisfied, - LoggedMessage.NotModifiedIfUnmodifiedSinceSatisfied); + LoggedMessage.NotModifiedIfModifiedSinceSatisfied, + LoggedMessage.NotModifiedIfModifiedSinceSatisfied); } [Fact] - public void ContentIsNotModified_IfNoneMatch_Overrides_IfUnmodifiedSince_ToTrue() + public void ContentIsNotModified_IfNoneMatch_Overrides_IfModifiedSince_ToTrue() { var utcNow = DateTimeOffset.UtcNow; var sink = new TestSink(); var context = TestUtils.CreateTestContext(sink); context.CachedResponseHeaders = new ResponseHeaders(new HeaderDictionary()); - // This would fail the IfUnmodifiedSince checks - context.TypedRequestHeaders.IfUnmodifiedSince = utcNow; + // This would fail the IfModifiedSince checks + context.TypedRequestHeaders.IfModifiedSince = utcNow; context.CachedResponseHeaders.LastModified = utcNow + TimeSpan.FromSeconds(10); context.TypedRequestHeaders.IfNoneMatch = new List(new[] { EntityTagHeaderValue.Any }); @@ -240,15 +242,15 @@ public void ContentIsNotModified_IfNoneMatch_Overrides_IfUnmodifiedSince_ToTrue( } [Fact] - public void ContentIsNotModified_IfNoneMatch_Overrides_IfUnmodifiedSince_ToFalse() + public void ContentIsNotModified_IfNoneMatch_Overrides_IfModifiedSince_ToFalse() { var utcNow = DateTimeOffset.UtcNow; var sink = new TestSink(); var context = TestUtils.CreateTestContext(sink); context.CachedResponseHeaders = new ResponseHeaders(new HeaderDictionary()); - // This would pass the IfUnmodifiedSince checks - context.TypedRequestHeaders.IfUnmodifiedSince = utcNow; + // This would pass the IfModifiedSince checks + context.TypedRequestHeaders.IfModifiedSince = utcNow; context.CachedResponseHeaders.LastModified = utcNow - TimeSpan.FromSeconds(10); context.TypedRequestHeaders.IfNoneMatch = new List(new[] { new EntityTagHeaderValue("\"E1\"") }); @@ -717,14 +719,55 @@ public async Task FinalizeCacheBody_DoNotCache_IfBufferingDisabled() [Fact] public void ShimResponseStream_SecondInvocation_Throws() { - var middleware = TestUtils.CreateTestMiddleware(); - var context = TestUtils.CreateTestContext(); + var httpContext = new DefaultHttpContext(); // Should not throw - middleware.ShimResponseStream(context); + ResponseCachingMiddleware.AddResponseCachingFeature(httpContext); // Should throw - Assert.ThrowsAny(() => middleware.ShimResponseStream(context)); + Assert.ThrowsAny(() => ResponseCachingMiddleware.AddResponseCachingFeature(httpContext)); + } + + private class FakeResponseFeature : HttpResponseFeature + { + public override void OnStarting(Func callback, object state) { } + } + + [Fact] + public async Task Invoke_CacheableRequest_AddsResponseCachingFeature() + { + var responseCachingFeatureAdded = false; + var middleware = TestUtils.CreateTestMiddleware(next: httpContext => + { + responseCachingFeatureAdded = httpContext.Features.Get() != null; + return TaskCache.CompletedTask; + }, + policyProvider: new ResponseCachingPolicyProvider()); + + var context = new DefaultHttpContext(); + context.Request.Method = HttpMethods.Get; + context.Features.Set(new FakeResponseFeature()); + await middleware.Invoke(context); + + Assert.True(responseCachingFeatureAdded); + } + + [Fact] + public async Task Invoke_NonCacheableRequest_AddsResponseCachingFeature() + { + var responseCachingFeatureAdded = false; + var middleware = TestUtils.CreateTestMiddleware(next: httpContext => + { + responseCachingFeatureAdded = httpContext.Features.Get() != null; + return TaskCache.CompletedTask; + }, + policyProvider: new ResponseCachingPolicyProvider()); + + var context = new DefaultHttpContext(); + context.Request.Method = HttpMethods.Post; + await middleware.Invoke(context); + + Assert.True(responseCachingFeatureAdded); } [Fact] diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingTests.cs index f2a014b..21007aa 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingTests.cs @@ -619,7 +619,7 @@ public async void Serves304_IfIfModifiedSince_Satisfied() { var client = server.CreateClient(); var initialResponse = await client.GetAsync(""); - client.DefaultRequestHeaders.IfUnmodifiedSince = DateTimeOffset.MaxValue; + client.DefaultRequestHeaders.IfModifiedSince = DateTimeOffset.MaxValue; var subsequentResponse = await client.GetAsync(""); initialResponse.EnsureSuccessStatusCode(); @@ -639,7 +639,7 @@ public async void ServesCachedContent_IfIfModifiedSince_NotSatisfied() { var client = server.CreateClient(); var initialResponse = await client.GetAsync(""); - client.DefaultRequestHeaders.IfUnmodifiedSince = DateTimeOffset.MinValue; + client.DefaultRequestHeaders.IfModifiedSince = DateTimeOffset.MinValue; var subsequentResponse = await client.GetAsync(""); await AssertCachedResponseAsync(initialResponse, subsequentResponse); diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/TestUtils.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/TestUtils.cs index b50be5b..de24026 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/TestUtils.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/TestUtils.cs @@ -103,12 +103,17 @@ internal static IEnumerable CreateBuildersWithResponseCaching( } internal static ResponseCachingMiddleware CreateTestMiddleware( + RequestDelegate next = null, IResponseCache cache = null, ResponseCachingOptions options = null, TestSink testSink = null, IResponseCachingKeyProvider keyProvider = null, IResponseCachingPolicyProvider policyProvider = null) { + if (next == null) + { + next = httpContext => TaskCache.CompletedTask; + } if (cache == null) { cache = new TestResponseCache(); @@ -127,7 +132,7 @@ internal static ResponseCachingMiddleware CreateTestMiddleware( } return new ResponseCachingMiddleware( - httpContext => TaskCache.CompletedTask, + next, Options.Create(options), testSink == null ? (ILoggerFactory)NullLoggerFactory.Instance : new TestLoggerFactory(testSink, true), policyProvider, @@ -188,7 +193,7 @@ internal class LoggedMessage internal static LoggedMessage ResponseWithUnsuccessfulStatusCodeNotCacheable => new LoggedMessage(17, LogLevel.Debug); internal static LoggedMessage NotModifiedIfNoneMatchStar => new LoggedMessage(18, LogLevel.Debug); internal static LoggedMessage NotModifiedIfNoneMatchMatched => new LoggedMessage(19, LogLevel.Debug); - internal static LoggedMessage NotModifiedIfUnmodifiedSinceSatisfied => new LoggedMessage(20, LogLevel.Debug); + internal static LoggedMessage NotModifiedIfModifiedSinceSatisfied => new LoggedMessage(20, LogLevel.Debug); internal static LoggedMessage NotModifiedServed => new LoggedMessage(21, LogLevel.Information); internal static LoggedMessage CachedResponseServed => new LoggedMessage(22, LogLevel.Information); internal static LoggedMessage GatewayTimeoutServed => new LoggedMessage(23, LogLevel.Information);