diff --git a/src/Microsoft.AspNetCore.Mvc.Core/CacheProfile.cs b/src/Microsoft.AspNetCore.Mvc.Core/CacheProfile.cs index 92422c2ac5..caef771693 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/CacheProfile.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/CacheProfile.cs @@ -11,7 +11,7 @@ public class CacheProfile /// /// Gets or sets the duration in seconds for which the response is cached. /// If this property is set to a non null value, - /// the "max-age" in "Cache-control" header is set in the + /// the "max-age" in "Cache-control" header is set in the /// . /// public int? Duration { get; set; } @@ -36,5 +36,10 @@ public class CacheProfile /// Gets or sets the value for the Vary header in . /// public string VaryByHeader { get; set; } + + /// + /// TODO: + /// + public string[] VaryByQueryKeys { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs index b27ec02e93..87ad29920e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.ResponseCaching; using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Mvc.Internal @@ -21,6 +22,7 @@ public class ResponseCacheFilter : IActionFilter, IResponseCacheFilter private ResponseCacheLocation? _cacheLocation; private bool? _cacheNoStore; private string _cacheVaryByHeader; + private string[] _cacheVaryByQueryKey; /// /// Creates a new instance of @@ -73,6 +75,15 @@ public string VaryByHeader set { _cacheVaryByHeader = value; } } + /// + /// TODO: + /// + public string[] VaryByQueryKeys + { + get { return _cacheVaryByQueryKey ?? _cacheProfile.VaryByQueryKeys; } + set { _cacheVaryByQueryKey = value; } + } + /// public void OnActionExecuting(ActionExecutingContext context) { @@ -110,6 +121,16 @@ public void OnActionExecuting(ActionExecutingContext context) headers[HeaderNames.Vary] = VaryByHeader; } + if (VaryByQueryKeys != null) + { + var responseCacheFeature = context.HttpContext.GetResponseCacheFeature(); // TODO: replace this call + if (responseCacheFeature == null) + { + throw new InvalidOperationException($"The response cache middleware must be added when using the {nameof(VaryByQueryKeys)} feature."); + } + responseCacheFeature.VaryByQueryKeys = VaryByQueryKeys; + } + if (NoStore) { headers[HeaderNames.CacheControl] = "no-store"; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ResponseCacheAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ResponseCacheAttribute.cs index 9bddb6a66b..ccc2128f66 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ResponseCacheAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ResponseCacheAttribute.cs @@ -77,6 +77,11 @@ public bool NoStore /// public string VaryByHeader { get; set; } + /// + /// TODO: + /// + public string[] VaryByQueryKeys { get; set; } + /// /// Gets or sets the value of the cache profile name. /// @@ -117,6 +122,7 @@ public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) _noStore = _noStore ?? selectedProfile?.NoStore; _location = _location ?? selectedProfile?.Location; VaryByHeader = VaryByHeader ?? selectedProfile?.VaryByHeader; + VaryByQueryKeys = VaryByQueryKeys ?? selectedProfile?.VaryByQueryKeys; // ResponseCacheFilter cannot take any null values. Hence, if there are any null values, // the properties convert them to their defaults and are passed on. @@ -125,7 +131,8 @@ public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) Duration = _duration, Location = _location, NoStore = _noStore, - VaryByHeader = VaryByHeader + VaryByHeader = VaryByHeader, + VaryByQueryKeys = VaryByQueryKeys }); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/project.json b/src/Microsoft.AspNetCore.Mvc.Core/project.json index 13d0fc38e8..404c51453c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/project.json +++ b/src/Microsoft.AspNetCore.Mvc.Core/project.json @@ -24,6 +24,7 @@ "Microsoft.AspNetCore.Hosting.Abstractions": "1.1.0-*", "Microsoft.AspNetCore.Http": "1.1.0-*", "Microsoft.AspNetCore.Mvc.Abstractions": "1.1.0-*", + "Microsoft.AspNetCore.ResponseCaching": "0.1.0-*", "Microsoft.AspNetCore.Routing": "1.1.0-*", "Microsoft.AspNetCore.Routing.DecisionTree.Sources": { "type": "build", diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterTest.cs index 4e8248266d..ae43492c64 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterTest.cs @@ -6,8 +6,10 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.ResponseCaching; using Microsoft.AspNetCore.Routing; using Xunit; +using System.Linq; namespace Microsoft.AspNetCore.Mvc.Internal { @@ -210,7 +212,7 @@ public void OnActionExecuting_DoesNotSetLocationOrDuration_IfNoStoreIsSet( Assert.Equal(output, context.HttpContext.Response.Headers["Cache-control"]); } - public static IEnumerable VaryData + public static IEnumerable VaryByHeaderData { get { @@ -267,8 +269,8 @@ public static IEnumerable VaryData } [Theory] - [MemberData(nameof(VaryData))] - public void ResponseCacheCanSetVary(ResponseCacheFilter cache, string varyOutput, string cacheControlOutput) + [MemberData(nameof(VaryByHeaderData))] + public void ResponseCacheCanSetVaryByHeader(ResponseCacheFilter cache, string varyOutput, string cacheControlOutput) { // Arrange var context = GetActionExecutingContext(new List { cache }); @@ -281,6 +283,125 @@ public void ResponseCacheCanSetVary(ResponseCacheFilter cache, string varyOutput Assert.Equal(cacheControlOutput, context.HttpContext.Response.Headers["Cache-control"]); } + public static IEnumerable VaryByQueryKeyData + { + get + { + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Any, + NoStore = false, VaryByQueryKeys = new string[0] + }), + new string[0], + "public,max-age=10" }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Any, + NoStore = false, VaryByQueryKeys = new string[] { null } + }), + new string[] { null }, + "public,max-age=10" }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Any, + NoStore = false, VaryByQueryKeys = new[] { "" } + }), + new[] { "" }, + "public,max-age=10" }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Any, + NoStore = false, VaryByQueryKeys = new[] { "Accept" } + }), + new[] { "Accept" }, + "public,max-age=10" }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 0, Location= ResponseCacheLocation.Any, + NoStore = true, VaryByQueryKeys = new[] { "Accept" } + }), + new[] { "Accept" }, + "no-store" + }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Client, + NoStore = false, VaryByQueryKeys = new[] { "Accept" } + }), + new[] { "Accept" }, + "private,max-age=10" + }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, Location = ResponseCacheLocation.Client, + NoStore = false, VaryByQueryKeys = new[] { "Accept", "Test" } + }), + new[] { "Accept", "Test" }, + "private,max-age=10" + }; + yield return new object[] { + new ResponseCacheFilter( + new CacheProfile + { + Duration = 31536000, Location = ResponseCacheLocation.Any, + NoStore = false, VaryByQueryKeys = new[] { "Accept", "Test" } + }), + new[] { "Accept", "Test" }, + "public,max-age=31536000" + }; + } + } + + [Theory] + [MemberData(nameof(VaryByQueryKeyData))] + public void ResponseCacheCanSetVaryByQueryKeys(ResponseCacheFilter cache, string[] varyOutput, string cacheControlOutput) + { + // Arrange + var context = GetActionExecutingContext(new List { cache }); + context.HttpContext.Features.Set(new ResponseCacheFeature()); + + // Act + cache.OnActionExecuting(context); + + // Assert + Assert.True(context.HttpContext.Features.Get().VaryByQueryKeys.SequenceEqual(varyOutput)); + Assert.Equal(cacheControlOutput, context.HttpContext.Response.Headers["Cache-control"]); + } + + [Fact] + public void NonEmptyVaryByQueryKeys_WithoutConfiguringMiddleware_Throws() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile + { + Duration = 0, + Location = ResponseCacheLocation.None, + NoStore = true, + VaryByHeader = null, + VaryByQueryKeys = new[] { "Test" } + }); + var context = GetActionExecutingContext(new List { cache }); + + // Act & Assert + var exception = Assert.Throws(() => cache.OnActionExecuting(context)); + Assert.Equal($"The response cache middleware must be added when using the VaryByQueryKeys feature.", exception.Message); + } + [Fact] public void SetsPragmaOnNoCache() { @@ -288,7 +409,10 @@ public void SetsPragmaOnNoCache() var cache = new ResponseCacheFilter( new CacheProfile { - Duration = 0, Location = ResponseCacheLocation.None, NoStore = true, VaryByHeader = null + Duration = 0, + Location = ResponseCacheLocation.None, + NoStore = true, + VaryByHeader = null }); var context = GetActionExecutingContext(new List { cache }); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ResponseCacheAttributeTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ResponseCacheAttributeTest.cs index 4994c304d6..1c15ee8080 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ResponseCacheAttributeTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ResponseCacheAttributeTest.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.ResponseCaching; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -66,18 +67,18 @@ public static IEnumerable OverrideData // When there are no cache profiles then the passed in data is returned unchanged yield return new object[] { new ResponseCacheAttribute() - { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept" }, + { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept", VaryByQueryKeys = new[] { "QueryKey" } }, null, new CacheProfile - { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept" } + { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept", VaryByQueryKeys = new[] { "QueryKey" } } }; yield return new object[] { new ResponseCacheAttribute() - { Duration = 0, Location = ResponseCacheLocation.None, NoStore = true, VaryByHeader = null }, + { Duration = 0, Location = ResponseCacheLocation.None, NoStore = true, VaryByHeader = null, VaryByQueryKeys = null }, null, new CacheProfile - { Duration = 0, Location = ResponseCacheLocation.None, NoStore = true, VaryByHeader = null } + { Duration = 0, Location = ResponseCacheLocation.None, NoStore = true, VaryByHeader = null, VaryByQueryKeys = null } }; // Everything gets overriden if attribute parameters are present, @@ -89,6 +90,7 @@ public static IEnumerable OverrideData Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept", + VaryByQueryKeys = new[] { "QueryKey" }, CacheProfileName = "TestCacheProfile" }, new Dictionary { { "TestCacheProfile", new CacheProfile @@ -96,10 +98,11 @@ public static IEnumerable OverrideData Duration = 10, Location = ResponseCacheLocation.Client, NoStore = true, - VaryByHeader = "Test" + VaryByHeader = "Test", + VaryByQueryKeys = new[] { "ProfileQueryKey" } } } }, new CacheProfile - { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept" } + { Duration = 20, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = "Accept", VaryByQueryKeys = new[] { "QueryKey" } } }; // Select parameters override the selected profile. @@ -114,10 +117,11 @@ public static IEnumerable OverrideData Duration = 10, Location = ResponseCacheLocation.Client, NoStore = false, - VaryByHeader = "Test" + VaryByHeader = "Test", + VaryByQueryKeys = new[] { "ProfileQueryKey" } } } }, new CacheProfile - { Duration = 534, Location = ResponseCacheLocation.Client, NoStore = false, VaryByHeader = "Test" } + { Duration = 534, Location = ResponseCacheLocation.Client, NoStore = false, VaryByHeader = "Test", VaryByQueryKeys = new[] { "ProfileQueryKey" } } }; // Duration parameter gets added to the selected profile. @@ -131,10 +135,11 @@ public static IEnumerable OverrideData { Location = ResponseCacheLocation.Client, NoStore = false, - VaryByHeader = "Test" + VaryByHeader = "Test", + VaryByQueryKeys = new[] { "ProfileQueryKey" } } } }, new CacheProfile - { Duration = 534, Location = ResponseCacheLocation.Client, NoStore = false, VaryByHeader = "Test" } + { Duration = 534, Location = ResponseCacheLocation.Client, NoStore = false, VaryByHeader = "Test", VaryByQueryKeys = new[] { "ProfileQueryKey" } } }; // Default values gets added for parameters which are absent @@ -146,7 +151,7 @@ public static IEnumerable OverrideData }, new Dictionary() { { "TestCacheProfile", new CacheProfile() } }, new CacheProfile - { Duration = 5234, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = null } + { Duration = 5234, Location = ResponseCacheLocation.Any, NoStore = false, VaryByHeader = null, VaryByQueryKeys = null } }; } } @@ -167,6 +172,14 @@ public void CreateInstance_HonorsOverrides( Assert.Equal(expectedProfile.Location, responseCacheFilter.Location); Assert.Equal(expectedProfile.NoStore, responseCacheFilter.NoStore); Assert.Equal(expectedProfile.VaryByHeader, responseCacheFilter.VaryByHeader); + if (expectedProfile.VaryByQueryKeys == null) + { + Assert.Null(responseCacheFilter.VaryByQueryKeys); + } + else + { + Assert.True(expectedProfile.VaryByQueryKeys.SequenceEqual(responseCacheFilter.VaryByQueryKeys)); + } } [Fact] @@ -191,14 +204,17 @@ public void CreateInstance_DoesNotThrowWhenTheDurationIsNotSet_WithNoStoreFalse( public void ResponseCache_SetsAllHeaders() { // Arrange + var varyByQueryKeys = new[] { "QueryKey" }; var responseCache = new ResponseCacheAttribute() { Duration = 100, Location = ResponseCacheLocation.Any, - VaryByHeader = "Accept" + VaryByHeader = "Accept", + VaryByQueryKeys = varyByQueryKeys }; var filter = (ResponseCacheFilter)responseCache.CreateInstance(GetServiceProvider(cacheProfiles: null)); var context = GetActionExecutingContext(filter); + context.HttpContext.Features.Set(new ResponseCacheFeature()); // Act filter.OnActionExecuting(context); @@ -212,6 +228,7 @@ public void ResponseCache_SetsAllHeaders() Assert.True(response.Headers.TryGetValue("Vary", out values)); data = Assert.Single(values); Assert.Equal("Accept", data); + Assert.True(varyByQueryKeys.SequenceEqual(context.HttpContext.Features.Get().VaryByQueryKeys)); } public static TheoryData CacheControlData