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,