Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

Commit

Permalink
Middleware behaviour update
Browse files Browse the repository at this point in the history 
- Use If-Modified-Since instead of the incorrect If-Unmodified-Since header
- Always add IResponseCachingFeature before calling the next middleware
  • Loading branch information
@JunTaoLuo
JunTaoLuo committed Jan 23, 2017
1 parent f6b6efc commit a6909cc
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 39 deletions.
10 changes: 5 additions & 5 deletions src/Microsoft.AspNetCore.ResponseCaching/Internal/LoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal static class LoggerExtensions
private static Action<ILogger, int, Exception> _logResponseWithUnsuccessfulStatusCodeNotCacheable;
private static Action<ILogger, Exception> _logNotModifiedIfNoneMatchStar;
private static Action<ILogger, EntityTagHeaderValue, Exception> _logNotModifiedIfNoneMatchMatched;
private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfUnmodifiedSinceSatisfied;
private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfModifiedSinceSatisfied;
private static Action<ILogger, Exception> _logNotModifiedServed;
private static Action<ILogger, Exception> _logCachedResponseServed;
private static Action<ILogger, Exception> _logGatewayTimeoutServed;
Expand Down Expand Up @@ -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<DateTimeOffset, DateTimeOffset>(
_logNotModifiedIfModifiedSinceSatisfied = LoggerMessage.Define<DateTimeOffset, DateTimeOffset>(
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,
Expand Down Expand Up @@ -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)
Expand Down
41 changes: 29 additions & 12 deletions src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -318,6 +328,15 @@ internal Task OnResponseStartingAsync(ResponseCachingContext context)
}
}

internal static void AddResponseCachingFeature(HttpContext context)
{
if (context.Features.Get<IResponseCachingFeature>() != 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<IResponseCachingFeature>(new ResponseCachingFeature());
}

internal void ShimResponseStream(ResponseCachingContext context)
{
// Shim response stream
Expand All @@ -332,14 +351,12 @@ internal void ShimResponseStream(ResponseCachingContext context)
context.HttpContext.Features.Set<IHttpSendFileFeature>(new SendFileFeatureWrapper(context.OriginalSendFileFeature, context.ResponseCachingStream));
}

// Add IResponseCachingFeature
if (context.HttpContext.Features.Get<IResponseCachingFeature>() != 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<IResponseCachingFeature>(new ResponseCachingFeature());
AddResponseCachingFeature(context.HttpContext);
}

internal static void RemoveResponseCachingFeature(HttpContext context)
=> context.Features.Set<IResponseCachingFeature>(null);

internal static void UnshimResponseStream(ResponseCachingContext context)
{
// Unshim response stream
Expand All @@ -349,7 +366,7 @@ internal static void UnshimResponseStream(ResponseCachingContext context)
context.HttpContext.Features.Set(context.OriginalSendFileFeature);

// Remove IResponseCachingFeature
context.HttpContext.Features.Set<IResponseCachingFeature>(null);
RemoveResponseCachingFeature(context.HttpContext);
}

internal static bool ContentIsNotModified(ResponseCachingContext context)
Expand Down Expand Up @@ -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;
}
}
Expand Down
79 changes: 61 additions & 18 deletions test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingMiddlewareTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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<EntityTagHeaderValue>(new[] { EntityTagHeaderValue.Any });
Expand All @@ -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<EntityTagHeaderValue>(new[] { new EntityTagHeaderValue("\"E1\"") });
Expand Down Expand Up @@ -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<InvalidOperationException>(() => middleware.ShimResponseStream(context));
Assert.ThrowsAny<InvalidOperationException>(() => ResponseCachingMiddleware.AddResponseCachingFeature(httpContext));
}

private class FakeResponseFeature : HttpResponseFeature
{
public override void OnStarting(Func<object, Task> callback, object state) { }
}

[Fact]
public async Task Invoke_CacheableRequest_AddsResponseCachingFeature()
{
var responseCachingFeatureAdded = false;
var middleware = TestUtils.CreateTestMiddleware(next: httpContext =>
{
responseCachingFeatureAdded = httpContext.Features.Get<IResponseCachingFeature>() != null;
return TaskCache.CompletedTask;
},
policyProvider: new ResponseCachingPolicyProvider());

var context = new DefaultHttpContext();
context.Request.Method = HttpMethods.Get;
context.Features.Set<IHttpResponseFeature>(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<IResponseCachingFeature>() != null;
return TaskCache.CompletedTask;
},
policyProvider: new ResponseCachingPolicyProvider());

var context = new DefaultHttpContext();
context.Request.Method = HttpMethods.Post;
await middleware.Invoke(context);

Assert.True(responseCachingFeatureAdded);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions test/Microsoft.AspNetCore.ResponseCaching.Tests/TestUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,17 @@ internal static IEnumerable<IWebHostBuilder> 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();
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a6909cc

Please sign in to comment.