From 70b56f157cc225b0c3668263189404068ad60eb9 Mon Sep 17 00:00:00 2001 From: CodingGorilla Date: Wed, 27 May 2015 00:30:50 -0400 Subject: [PATCH] Updated the ResponseCacheFilter class to store a reference to the CacheProfile passed into the contructor Updated the ResponseCacheFilter Duration, Location, NoStore, and VaryByHeader properties to return one of 3 values: the specific setting applied to the instance, the setting supplied by the CachePolicy (from the constructor), or a default value. The emphasis being not to change the outward function of these properties to consumers, but to defer the evaluation of these properties until the OnActionExecuting method. Updated the ResponseCacheFilter to check whether a cache duration has been supplied when the NoStore property is false and throw an InvalidOperationException when a duration has not been provided. Updated ResponseCacheFilterTest to reflect the changes in behavior in the ResponseCacheFilter Updated the ResponseCacheFilterAttributeTest to reflect the change in the behavior of the ResponseCacheFilter Added unit tests to ResponseCacheFilterTest.cs to verify that the CachePolicy properties are properly overriden with the properties set directly on the ResponseCacheFilter. Added functional tests to test the ability to override CacheProfile settings with properties on the ResponseCacheAttribute --- .../Filters/ResponseCacheFilter.cs | 57 ++++++--- .../Filters/ResponseCacheAttributeTest.cs | 13 +- .../Filters/ResponseCacheFilterTest.cs | 116 ++++++++++++++++-- .../ResponseCacheTest.cs | 79 ++++++++++++ .../CacheProfilesOverridesController.cs | 43 +++++++ test/WebSites/ResponseCacheWebSite/Startup.cs | 8 ++ 6 files changed, 280 insertions(+), 36 deletions(-) create mode 100644 test/WebSites/ResponseCacheWebSite/Controllers/CacheProfilesOverridesController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/ResponseCacheFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/ResponseCacheFilter.cs index 8b0fbd1d27..ebe249a72b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/ResponseCacheFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/ResponseCacheFilter.cs @@ -14,6 +14,12 @@ namespace Microsoft.AspNet.Mvc /// public class ResponseCacheFilter : IActionFilter, IResponseCacheFilter { + private readonly CacheProfile _cacheProfile; + private int? _cacheDuration; + private ResponseCacheLocation? _cacheLocation; + private bool? _cacheNoStore; + private string _cacheVaryByHeader; + /// /// Creates a new instance of /// @@ -21,20 +27,7 @@ namespace Microsoft.AspNet.Mvc /// . public ResponseCacheFilter(CacheProfile cacheProfile) { - if (!(cacheProfile.NoStore ?? false)) - { - // Duration MUST be set (either in the cache profile or in the attribute) unless NoStore is true. - if (cacheProfile.Duration == null) - { - throw new InvalidOperationException( - Resources.FormatResponseCache_SpecifyDuration(nameof(NoStore), nameof(Duration))); - } - } - - Duration = cacheProfile.Duration ?? 0; - Location = cacheProfile.Location ?? ResponseCacheLocation.Any; - NoStore = cacheProfile.NoStore ?? false; - VaryByHeader = cacheProfile.VaryByHeader; + _cacheProfile = cacheProfile; } /// @@ -42,12 +35,20 @@ namespace Microsoft.AspNet.Mvc /// This is a required parameter. /// This sets "max-age" in "Cache-control" header. /// - public int Duration { get; set; } + public int Duration + { + get { return (_cacheDuration ?? _cacheProfile.Duration) ?? 0; } + set { _cacheDuration = value; } + } /// /// Gets or sets the location where the data from a particular URL must be cached. /// - public ResponseCacheLocation Location { get; set; } + public ResponseCacheLocation Location + { + get { return (_cacheLocation ?? _cacheProfile.Location) ?? ResponseCacheLocation.Any; } + set { _cacheLocation = value; } + } /// /// Gets or sets the value which determines whether the data should be stored or not. @@ -55,13 +56,21 @@ namespace Microsoft.AspNet.Mvc /// Ignores the "Location" parameter for values other than "None". /// Ignores the "duration" parameter. /// - public bool NoStore { get; set; } + public bool NoStore + { + get { return (_cacheNoStore ?? _cacheProfile.NoStore) ?? false; } + set { _cacheNoStore = value; } + } /// /// Gets or sets the value for the Vary response header. /// - public string VaryByHeader { get; set; } - + public string VaryByHeader + { + get { return _cacheVaryByHeader ?? _cacheProfile.VaryByHeader; } + set { _cacheVaryByHeader = value; } + } + // public void OnActionExecuting([NotNull] ActionExecutingContext context) { @@ -72,6 +81,16 @@ namespace Microsoft.AspNet.Mvc return; } + if (!NoStore) + { + // Duration MUST be set (either in the cache profile or in this filter) unless NoStore is true. + if (_cacheProfile.Duration == null && _cacheDuration == null) + { + throw new InvalidOperationException( + Resources.FormatResponseCache_SpecifyDuration(nameof(NoStore), nameof(Duration))); + } + } + var headers = context.HttpContext.Response.Headers; // Clear all headers diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheAttributeTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheAttributeTest.cs index 9637f3b6c4..2353700bff 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheAttributeTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheAttributeTest.cs @@ -162,7 +162,7 @@ namespace Microsoft.AspNet.Mvc } [Fact] - public void CreateInstance_ThrowsWhenTheDurationIsNotSet_WithNoStoreFalse() + public void CreateInstance_DoesNotThrowWhenTheDurationIsNotSet_WithNoStoreFalse() { // Arrange var responseCache = new ResponseCacheAttribute() @@ -172,12 +172,11 @@ namespace Microsoft.AspNet.Mvc var cacheProfiles = new Dictionary(); cacheProfiles.Add("Test", new CacheProfile { NoStore = false }); - // Act & Assert - var ex = Assert.Throws( - () => responseCache.CreateInstance(GetServiceProvider(cacheProfiles))); - Assert.Equal( - "If the 'NoStore' property is not set to true, 'Duration' property must be specified.", - ex.Message); + // Act + var filter = responseCache.CreateInstance(GetServiceProvider(cacheProfiles)); + + // Assert + Assert.NotNull(filter); } private IServiceProvider GetServiceProvider(Dictionary cacheProfiles) diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheFilterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheFilterTest.cs index d1d932fd56..f79a6cb838 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheFilterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResponseCacheFilterTest.cs @@ -31,17 +31,34 @@ namespace Microsoft.AspNet.Mvc } [Fact] - public void ResponseCacheFilter_ThrowsIfDurationIsNotSet_WhenNoStoreIsFalse() + public void ResponseCacheFilter_DoesNotThrowIfDurationIsNotSet_WhenNoStoreIsFalse() { - // Arrange, Act & Assert - var ex = Assert.Throws( - () => new ResponseCacheFilter( - new CacheProfile - { - Duration = null - })); - Assert.Equal( - "If the 'NoStore' property is not set to true, 'Duration' property must be specified.", + // Arrange, Act + var cache = new ResponseCacheFilter( + new CacheProfile + { + Duration = null + }); + + // Assert + Assert.NotNull(cache); + } + + [Fact] + public void OnActionExecuting_ThrowsIfDurationIsNotSet_WhenNoStoreIsFalse() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile() + { + Duration = null + }); + + var context = GetActionExecutingContext(new List { cache }); + + // Act & Assert + var ex = Assert.Throws(() => cache.OnActionExecuting(context)); + Assert.Equal("If the 'NoStore' property is not set to true, 'Duration' property must be specified.", ex.Message); } @@ -306,6 +323,85 @@ namespace Microsoft.AspNet.Mvc Assert.False(cache.IsOverridden(context)); } + [Fact] + public void FilterDurationProperty_OverridesCachePolicySetting() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile + { + Duration = 10 + }); + cache.Duration = 20; + var context = GetActionExecutingContext(new List { cache }); + + // Act + cache.OnActionExecuting(context); + + // Assert + Assert.Equal("public,max-age=20", context.HttpContext.Response.Headers.Get("Cache-control")); + } + + [Fact] + public void FilterLocationProperty_OverridesCachePolicySetting() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile + { + Duration = 10, + Location = ResponseCacheLocation.None + }); + cache.Location = ResponseCacheLocation.Client; + var context = GetActionExecutingContext(new List { cache }); + + // Act + cache.OnActionExecuting(context); + + // Assert + Assert.Equal("private,max-age=10", context.HttpContext.Response.Headers.Get("Cache-control")); + } + + [Fact] + public void FilterNoStoreProperty_OverridesCachePolicySetting() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile + { + NoStore = true + }); + cache.NoStore = false; + cache.Duration = 10; + var context = GetActionExecutingContext(new List { cache }); + + // Act + cache.OnActionExecuting(context); + + // Assert + Assert.Equal("public,max-age=10", context.HttpContext.Response.Headers.Get("Cache-control")); + } + + [Fact] + public void FilterVaryByProperty_OverridesCachePolicySetting() + { + // Arrange + var cache = new ResponseCacheFilter( + new CacheProfile + { + NoStore = true, + VaryByHeader = "Accept" + }); + cache.VaryByHeader = "Test"; + var context = GetActionExecutingContext(new List { cache }); + + // Act + cache.OnActionExecuting(context); + + // Assert + Assert.Equal("Test", context.HttpContext.Response.Headers.Get("Vary")); + } + private ActionExecutingContext GetActionExecutingContext(List filters = null) { return new ActionExecutingContext( diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ResponseCacheTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ResponseCacheTest.cs index fa28c7aaab..27f80eacd2 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ResponseCacheTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ResponseCacheTest.cs @@ -280,5 +280,84 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests response.Headers.TryGetValues("Pragma", out pragmaValues); Assert.Null(pragmaValues); } + + // Cache profile overrides + [Fact] + public async Task ResponseCacheAttribute_OverridesProfileDuration_FromAttributeProperty() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/CacheProfileOverrides/PublicCache30SecTo15Sec"); + + // Assert + var data = Assert.Single(response.Headers.GetValues("Cache-control")); + Assert.Equal("public, max-age=15", data); + } + + [Fact] + public async Task ResponseCacheAttribute_OverridesProfileLocation_FromAttributeProperty() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/CacheProfileOverrides/PublicCache30SecToPrivateCache"); + + // Assert + var data = Assert.Single(response.Headers.GetValues("Cache-control")); + Assert.Equal("max-age=30, private", data); + } + + [Fact] + public async Task ResponseCacheAttribute_OverridesProfileNoStore_FromAttributeProperty() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/CacheProfileOverrides/PublicCache30SecToNoStore"); + + // Assert + var data = Assert.Single(response.Headers.GetValues("Cache-control")); + Assert.Equal("no-store", data); + } + + [Fact] + public async Task ResponseCacheAttribute_OverridesProfileVaryBy_FromAttributeProperty() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/CacheProfileOverrides/PublicCache30SecWithVaryByAcceptToVaryByTest"); + + // Assert + var cacheControl = Assert.Single(response.Headers.GetValues("Cache-control")); + Assert.Equal("public, max-age=30", cacheControl); + var vary = Assert.Single(response.Headers.GetValues("Vary")); + Assert.Equal("Test", vary); + } + + [Fact] + public async Task ResponseCacheAttribute_OverridesProfileVaryBy_FromAttributeProperty_AndRemovesVaryHeader() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/CacheProfileOverrides/PublicCache30SecWithVaryByAcceptToVaryByNone"); + + // Assert + var cacheControl = Assert.Single(response.Headers.GetValues("Cache-control")); + Assert.Equal("public, max-age=30", cacheControl); + Assert.Throws(() => response.Headers.GetValues("Vary")); + } } } \ No newline at end of file diff --git a/test/WebSites/ResponseCacheWebSite/Controllers/CacheProfilesOverridesController.cs b/test/WebSites/ResponseCacheWebSite/Controllers/CacheProfilesOverridesController.cs new file mode 100644 index 0000000000..3e7615bdc8 --- /dev/null +++ b/test/WebSites/ResponseCacheWebSite/Controllers/CacheProfilesOverridesController.cs @@ -0,0 +1,43 @@ +using System; +using Microsoft.AspNet.Mvc; + +namespace ResponseCacheWebSite.Controllers +{ + public class CacheProfilesOverridesController + { + [HttpGet("/CacheProfileOverrides/PublicCache30SecTo15Sec")] + [ResponseCache(CacheProfileName = "PublicCache30Sec", Duration = 15)] + public string PublicCache30SecTo15Sec() + { + return "Hello World!"; + } + + [HttpGet("/CacheProfileOverrides/PublicCache30SecToPrivateCache")] + [ResponseCache(CacheProfileName = "PublicCache30Sec", Location = ResponseCacheLocation.Client)] + public string PublicCache30SecToPrivateCache() + { + return "Hello World!"; + } + + [HttpGet("/CacheProfileOverrides/PublicCache30SecToNoStore")] + [ResponseCache(CacheProfileName = "PublicCache30Sec", NoStore = true)] + public string PublicCache30SecToNoStore() + { + return "Hello World!"; + } + + [HttpGet("/CacheProfileOverrides/PublicCache30SecWithVaryByAcceptToVaryByTest")] + [ResponseCache(CacheProfileName = "PublicCache30Sec", VaryByHeader = "Test")] + public string PublicCache30SecWithVaryByAcceptToVaryByTest() + { + return "Hello World!"; + } + + [HttpGet("/CacheProfileOverrides/PublicCache30SecWithVaryByAcceptToVaryByNone")] + [ResponseCache(CacheProfileName = "PublicCache30Sec", VaryByHeader = null)] + public string PublicCache30SecWithVaryByAcceptToVaryByNone() + { + return "Hello World!"; + } + } +} \ No newline at end of file diff --git a/test/WebSites/ResponseCacheWebSite/Startup.cs b/test/WebSites/ResponseCacheWebSite/Startup.cs index c56174faba..981a6e0c1b 100644 --- a/test/WebSites/ResponseCacheWebSite/Startup.cs +++ b/test/WebSites/ResponseCacheWebSite/Startup.cs @@ -37,6 +37,14 @@ namespace ResponseCacheWebSite Location = ResponseCacheLocation.None }); + options.CacheProfiles.Add( + "PublicCache30SecVaryByAcceptHeader", new CacheProfile + { + Duration = 30, + Location = ResponseCacheLocation.Any, + VaryByHeader = "Accept" + }); + options.Filters.Add(new ResponseCacheFilter(new CacheProfile { NoStore = true,