Remove todos and add a test

Issues are now resolved or are tracked in issues.

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)

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
+            catch { }
-            {
-                // TODO: Log error
-            }
         }
 
         public void Set(string key, object entry, TimeSpan validFor)
@@ -57,10 +53,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
                         AbsoluteExpirationRelativeToNow = validFor
                     });
             }
-            catch
+            catch { }
-            {
-                // TODO: Log error
-            }
         }
     }
 }

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; }

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<IHttpSendFileFeature>(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();
         }
 

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;
                     }
 

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();