From 673115a29292d6dfe8bb8f0974f6a16088c7772b Mon Sep 17 00:00:00 2001 From: John Luo Date: Thu, 15 Sep 2016 16:52:46 -0700 Subject: [PATCH] Remove todos and add a test Issues are now resolved or are tracked in issues. --- .../ResponseCacheHttpContextExtensions.cs | 1 - .../Internal/DistributedResponseCacheStore.cs | 11 +--- .../ResponseCacheFeature.cs | 1 - .../ResponseCacheMiddleware.cs | 10 --- .../ResponseCachePolicyProvider.cs | 11 ---- .../ResponseCacheTests.cs | 64 +++++++++++++++++++ 6 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.AspNetCore.ResponseCaching/Extensions/ResponseCacheHttpContextExtensions.cs b/src/Microsoft.AspNetCore.ResponseCaching/Extensions/ResponseCacheHttpContextExtensions.cs index 7c1915b14d..d8349bf594 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/Extensions/ResponseCacheHttpContextExtensions.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/Extensions/ResponseCacheHttpContextExtensions.cs @@ -5,7 +5,6 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.ResponseCaching { - // TODO: Temporary interface for endpoints to specify options for response caching public static class ResponseCacheHttpContextExtensions { public static ResponseCacheFeature GetResponseCacheFeature(this HttpContext httpContext) diff --git a/src/Microsoft.AspNetCore.ResponseCaching/Internal/DistributedResponseCacheStore.cs b/src/Microsoft.AspNetCore.ResponseCaching/Internal/DistributedResponseCacheStore.cs index aba8990b42..88fa05f3ea 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/Internal/DistributedResponseCacheStore.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/Internal/DistributedResponseCacheStore.cs @@ -28,7 +28,6 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal } catch { - // TODO: Log error return null; } } @@ -39,10 +38,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal { _cache.Remove(key); } - catch - { - // TODO: Log error - } + catch { } } public void Set(string key, object entry, TimeSpan validFor) @@ -57,10 +53,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal AbsoluteExpirationRelativeToNow = validFor }); } - catch - { - // TODO: Log error - } + catch { } } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs index 1233aff724..5646d15867 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.ResponseCaching { - // TODO: Temporary interface for endpoints to specify options for response caching public class ResponseCacheFeature { public StringValues VaryByParams { get; set; } diff --git a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheMiddleware.cs b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheMiddleware.cs index 85b8fec0ea..cd2ec42598 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheMiddleware.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheMiddleware.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Globalization; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; @@ -104,7 +103,6 @@ namespace Microsoft.AspNetCore.ResponseCaching } else { - // TODO: Invalidate resources for successful unsafe methods? Required by RFC await _next(httpContext); } } @@ -159,10 +157,6 @@ namespace Microsoft.AspNetCore.ResponseCaching return true; } - else - { - // TODO: Validate with endpoint instead - } return false; } @@ -313,8 +307,6 @@ namespace Microsoft.AspNetCore.ResponseCaching internal void ShimResponseStream(ResponseCacheContext context) { - // TODO: Consider caching large responses on disk and serving them from there. - // Shim response stream context.OriginalResponseStream = context.HttpContext.Response.Body; context.ResponseCacheStream = new ResponseCacheStream(context.OriginalResponseStream, _options.MaximumCachedBodySize); @@ -327,7 +319,6 @@ namespace Microsoft.AspNetCore.ResponseCaching context.HttpContext.Features.Set(new SendFileFeatureWrapper(context.OriginalSendFileFeature, context.ResponseCacheStream)); } - // TODO: Move this temporary interface with endpoint to HttpAbstractions context.HttpContext.AddResponseCacheFeature(); } @@ -339,7 +330,6 @@ namespace Microsoft.AspNetCore.ResponseCaching // Unshim IHttpSendFileFeature context.HttpContext.Features.Set(context.OriginalSendFileFeature); - // TODO: Move this temporary interface with endpoint to HttpAbstractions context.HttpContext.RemoveResponseCacheFeature(); } diff --git a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachePolicyProvider.cs b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachePolicyProvider.cs index f92527be20..912e6b48a4 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachePolicyProvider.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCachePolicyProvider.cs @@ -15,7 +15,6 @@ namespace Microsoft.AspNetCore.ResponseCaching public virtual bool IsRequestCacheable(ResponseCacheContext context) { // Verify the method - // TODO: RFC lists POST as a cacheable method when explicit freshness information is provided, but this is not widely implemented. Will revisit. var request = context.HttpContext.Request; if (!string.Equals("GET", request.Method, StringComparison.OrdinalIgnoreCase) && !string.Equals("HEAD", request.Method, StringComparison.OrdinalIgnoreCase)) @@ -24,14 +23,12 @@ namespace Microsoft.AspNetCore.ResponseCaching } // Verify existence of authorization headers - // TODO: The server may indicate that the response to these request are cacheable if (!StringValues.IsNullOrEmpty(request.Headers[HeaderNames.Authorization])) { return false; } // Verify request cache-control parameters - // TODO: no-cache requests can be retrieved upon validation with origin if (!StringValues.IsNullOrEmpty(request.Headers[HeaderNames.CacheControl])) { if (context.RequestCacheControlHeaderValue.NoCache) @@ -52,14 +49,12 @@ namespace Microsoft.AspNetCore.ResponseCaching } } - // TODO: Verify global middleware settings? Explicit ignore list, range requests, etc. return true; } public virtual bool IsResponseCacheable(ResponseCacheContext context) { // Only cache pages explicitly marked with public - // TODO: Consider caching responses that are not marked as public but otherwise cacheable? if (!context.ResponseCacheControlHeaderValue.Public) { return false; @@ -72,7 +67,6 @@ namespace Microsoft.AspNetCore.ResponseCaching } // Check no-cache - // TODO: Handle no-cache with headers if (context.ResponseCacheControlHeaderValue.NoCache) { return false; @@ -93,8 +87,6 @@ namespace Microsoft.AspNetCore.ResponseCaching return false; } - // TODO: public MAY override the cacheability checks for private and status codes - // Check private if (context.ResponseCacheControlHeaderValue.Private) { @@ -102,14 +94,12 @@ namespace Microsoft.AspNetCore.ResponseCaching } // Check response code - // TODO: RFC also lists 203, 204, 206, 300, 301, 404, 405, 410, 414, and 501 as cacheable by default if (response.StatusCode != StatusCodes.Status200OK) { return false; } // Check response freshness - // TODO: apparent age vs corrected age value if (context.TypedResponseHeaders.Date == null) { if (context.ResponseCacheControlHeaderValue.SharedMaxAge == null && @@ -180,7 +170,6 @@ namespace Microsoft.AspNetCore.ResponseCaching // Request allows stale values if (age < context.RequestCacheControlHeaderValue.MaxStaleLimit) { - // TODO: Add warning header indicating the response is stale return true; } diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheTests.cs index 54060d0fec..45fd90eb96 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheTests.cs @@ -518,6 +518,70 @@ namespace Microsoft.AspNetCore.ResponseCaching.Tests } } + [Fact] + public async void ServesFreshContent_IfCachedVaryByUpdated_OnCacheMiss() + { + var builder = TestUtils.CreateBuilderWithResponseCache(requestDelegate: async (context) => + { + context.Response.Headers[HeaderNames.Vary] = context.Request.Headers[HeaderNames.Pragma]; + await TestUtils.TestRequestDelegate(context); + }); + + using (var server = new TestServer(builder)) + { + var client = server.CreateClient(); + client.DefaultRequestHeaders.From = "user@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("From")); + client.DefaultRequestHeaders.MaxForwards = 1; + var initialResponse = await client.GetAsync(""); + client.DefaultRequestHeaders.From = "user2@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("Max-Forwards")); + client.DefaultRequestHeaders.MaxForwards = 2; + var otherResponse = await client.GetAsync(""); + client.DefaultRequestHeaders.From = "user@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("From")); + client.DefaultRequestHeaders.MaxForwards = 1; + var subsequentResponse = await client.GetAsync(""); + + await AssertResponseNotCachedAsync(initialResponse, subsequentResponse); + } + } + + [Fact] + public async void ServesCachedContent_IfCachedVaryByNotUpdated_OnCacheMiss() + { + var builder = TestUtils.CreateBuilderWithResponseCache(requestDelegate: async (context) => + { + context.Response.Headers[HeaderNames.Vary] = context.Request.Headers[HeaderNames.Pragma]; + await TestUtils.TestRequestDelegate(context); + }); + + using (var server = new TestServer(builder)) + { + var client = server.CreateClient(); + client.DefaultRequestHeaders.From = "user@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("From")); + client.DefaultRequestHeaders.MaxForwards = 1; + var initialResponse = await client.GetAsync(""); + client.DefaultRequestHeaders.From = "user2@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("From")); + client.DefaultRequestHeaders.MaxForwards = 2; + var otherResponse = await client.GetAsync(""); + client.DefaultRequestHeaders.From = "user@example.com"; + client.DefaultRequestHeaders.Pragma.Clear(); + client.DefaultRequestHeaders.Pragma.Add(new System.Net.Http.Headers.NameValueHeaderValue("From")); + client.DefaultRequestHeaders.MaxForwards = 1; + var subsequentResponse = await client.GetAsync(""); + + await AssertResponseCachedAsync(initialResponse, subsequentResponse); + } + } + private static async Task AssertResponseCachedAsync(HttpResponseMessage initialResponse, HttpResponseMessage subsequentResponse) { initialResponse.EnsureSuccessStatusCode();