Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Add support for ResponseCache in Razor Pages #6627

Closed
wants to merge 2 commits into from
Closed

Add support for ResponseCache in Razor Pages #6627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5968b6d
Add support for ResponseCache in Razor Pages
pranavkm Aug 3, 2017
775bfee
Adding tests for ResponseCacheFilterApplicationModelProviderTest
pranavkm Aug 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/IResponseCacheFilter.cs
View file
Open in desktop
Expand Up @@ -6,9 +6,9 @@
namespace Microsoft.AspNetCore.Mvc.Internal
{
/// <summary>
/// An <see cref="IActionFilter"/> which sets the appropriate headers related to Response caching.
/// A filter which sets the appropriate headers related to Response caching.
/// </summary>
public interface IResponseCacheFilter : IActionFilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a different marker type for pages? I thought we were kinda ok changing this since it's in .Internal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! it is, cool. Ok that's all fine then

public interface IResponseCacheFilter : IFilterMetadata
{
}
}
125 changes: 15 additions & 110 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs
View file
Open in desktop
Expand Up @@ -2,27 +2,16 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Globalization;
using System.Linq;
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
{
/// <summary>
/// An <see cref="IActionFilter"/> which sets the appropriate headers related to response caching.
/// </summary>
public class ResponseCacheFilter : IResponseCacheFilter
public class ResponseCacheFilter : IActionFilter, IResponseCacheFilter
{
private readonly CacheProfile _cacheProfile;
private int? _cacheDuration;
private ResponseCacheLocation? _cacheLocation;
private bool? _cacheNoStore;
private string _cacheVaryByHeader;
private string[] _cacheVaryByQueryKeys;
private readonly ResponseCacheFilterExecutor _executor;

/// <summary>
/// Creates a new instance of <see cref="ResponseCacheFilter"/>
Expand All @@ -31,7 +20,7 @@ public class ResponseCacheFilter : IResponseCacheFilter
/// <see cref="ResponseCacheFilter"/>.</param>
public ResponseCacheFilter(CacheProfile cacheProfile)
{
_cacheProfile = cacheProfile;
_executor = new ResponseCacheFilterExecutor(cacheProfile);
}

/// <summary>
Expand All @@ -41,17 +30,17 @@ public ResponseCacheFilter(CacheProfile cacheProfile)
/// </summary>
public int Duration
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have these properties?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you can still construct the filter

{
get { return (_cacheDuration ?? _cacheProfile.Duration) ?? 0; }
set { _cacheDuration = value; }
get => _executor.Duration;
set => _executor.Duration = value;
}

/// <summary>
/// Gets or sets the location where the data from a particular URL must be cached.
/// </summary>
public ResponseCacheLocation Location
{
get { return (_cacheLocation ?? _cacheProfile.Location) ?? ResponseCacheLocation.Any; }
set { _cacheLocation = value; }
get => _executor.Location;
set => _executor.Location = value;
}

/// <summary>
Expand All @@ -62,17 +51,17 @@ public ResponseCacheLocation Location
/// </summary>
public bool NoStore
{
get { return (_cacheNoStore ?? _cacheProfile.NoStore) ?? false; }
set { _cacheNoStore = value; }
get => _executor.NoStore;
set => _executor.NoStore = value;
}

/// <summary>
/// Gets or sets the value for the Vary response header.
/// </summary>
public string VaryByHeader
{
get { return _cacheVaryByHeader ?? _cacheProfile.VaryByHeader; }
set { _cacheVaryByHeader = value; }
get => _executor.VaryByHeader;
set => _executor.VaryByHeader = value;
}

/// <summary>
Expand All @@ -83,8 +72,8 @@ public string VaryByHeader
/// </remarks>
public string[] VaryByQueryKeys
{
get { return _cacheVaryByQueryKeys ?? _cacheProfile.VaryByQueryKeys; }
set { _cacheVaryByQueryKeys = value; }
get => _executor.VaryByQueryKeys;
set => _executor.VaryByQueryKeys = value;
}

/// <inheritdoc />
Expand All @@ -97,101 +86,17 @@ public void OnActionExecuting(ActionExecutingContext context)

// If there are more filters which can override the values written by this filter,
// then skip execution of this filter.
if (IsOverridden(context))
if (ResponseCacheFilterExecutor.IsOverridden(this, context))
{
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
headers.Remove(HeaderNames.Vary);
headers.Remove(HeaderNames.CacheControl);
headers.Remove(HeaderNames.Pragma);

if (!string.IsNullOrEmpty(VaryByHeader))
{
headers[HeaderNames.Vary] = VaryByHeader;
}

if (VaryByQueryKeys != null)
{
var responseCachingFeature = context.HttpContext.Features.Get<IResponseCachingFeature>();
if (responseCachingFeature == null)
{
throw new InvalidOperationException(Resources.FormatVaryByQueryKeys_Requires_ResponseCachingMiddleware(nameof(VaryByQueryKeys)));
}
responseCachingFeature.VaryByQueryKeys = VaryByQueryKeys;
}

if (NoStore)
{
headers[HeaderNames.CacheControl] = "no-store";

// Cache-control: no-store, no-cache is valid.
if (Location == ResponseCacheLocation.None)
{
headers.AppendCommaSeparatedValues(HeaderNames.CacheControl, "no-cache");
headers[HeaderNames.Pragma] = "no-cache";
}
}
else
{
string cacheControlValue = null;
switch (Location)
{
case ResponseCacheLocation.Any:
cacheControlValue = "public";
break;
case ResponseCacheLocation.Client:
cacheControlValue = "private";
break;
case ResponseCacheLocation.None:
cacheControlValue = "no-cache";
headers[HeaderNames.Pragma] = "no-cache";
break;
}

cacheControlValue = string.Format(
CultureInfo.InvariantCulture,
"{0}{1}max-age={2}",
cacheControlValue,
cacheControlValue != null ? "," : null,
Duration);

if (cacheControlValue != null)
{
headers[HeaderNames.CacheControl] = cacheControlValue;
}
}
_executor.Execute(context);
}

/// <inheritdoc />
public void OnActionExecuted(ActionExecutedContext context)
{
}

// internal for Unit Testing purposes.
internal bool IsOverridden(ActionExecutingContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

// Return true if there are any filters which are after the current filter. In which case the current
// filter should be skipped.
return context.Filters.OfType<IResponseCacheFilter>().Last() != this;
}
}
}
153 changes: 153 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilterExecutor.cs
View file
Open in desktop
@@ -0,0 +1,153 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Diagnostics;
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
{
public class ResponseCacheFilterExecutor
{
private readonly CacheProfile _cacheProfile;
private int? _cacheDuration;
private ResponseCacheLocation? _cacheLocation;
private bool? _cacheNoStore;
private string _cacheVaryByHeader;
private string[] _cacheVaryByQueryKeys;

public ResponseCacheFilterExecutor(CacheProfile cacheProfile)
{
_cacheProfile = cacheProfile ?? throw new ArgumentNullException(nameof(cacheProfile));
}

public int Duration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than duplicating all of this, it would be better pass in the ResponseCacheFilter object directly - similar to how our action result executors work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have two different types of ResponseCacheFilter though. I didn't want to go down the inheritance route because that seemed gross.

{
get => _cacheDuration ?? _cacheProfile.Duration ?? 0;
set => _cacheDuration = value;
}

public ResponseCacheLocation Location
{
get => _cacheLocation ?? _cacheProfile.Location ?? ResponseCacheLocation.Any;
set => _cacheLocation = value;
}

public bool NoStore
{
get => _cacheNoStore ?? _cacheProfile.NoStore ?? false;
set => _cacheNoStore = value;
}

public string VaryByHeader
{
get => _cacheVaryByHeader ?? _cacheProfile.VaryByHeader;
set => _cacheVaryByHeader = value;
}

public string[] VaryByQueryKeys
{
get => _cacheVaryByQueryKeys ?? _cacheProfile.VaryByQueryKeys;
set => _cacheVaryByQueryKeys = value;
}

public void Execute(FilterContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

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
headers.Remove(HeaderNames.Vary);
headers.Remove(HeaderNames.CacheControl);
headers.Remove(HeaderNames.Pragma);

if (!string.IsNullOrEmpty(VaryByHeader))
{
headers[HeaderNames.Vary] = VaryByHeader;
}

if (VaryByQueryKeys != null)
{
var responseCachingFeature = context.HttpContext.Features.Get<IResponseCachingFeature>();
if (responseCachingFeature == null)
{
throw new InvalidOperationException(
Resources.FormatVaryByQueryKeys_Requires_ResponseCachingMiddleware(nameof(VaryByQueryKeys)));
}
responseCachingFeature.VaryByQueryKeys = VaryByQueryKeys;
}

if (NoStore)
{
headers[HeaderNames.CacheControl] = "no-store";

// Cache-control: no-store, no-cache is valid.
if (Location == ResponseCacheLocation.None)
{
headers.AppendCommaSeparatedValues(HeaderNames.CacheControl, "no-cache");
headers[HeaderNames.Pragma] = "no-cache";
}
}
else
{
string cacheControlValue;
switch (Location)
{
case ResponseCacheLocation.Any:
cacheControlValue = "public,";
break;
case ResponseCacheLocation.Client:
cacheControlValue = "private,";
break;
case ResponseCacheLocation.None:
cacheControlValue = "no-cache,";
headers[HeaderNames.Pragma] = "no-cache";
break;
default:
cacheControlValue = null;
break;
}

cacheControlValue = $"{cacheControlValue}max-age={Duration}";
headers[HeaderNames.CacheControl] = cacheControlValue;
}
}

public static bool IsOverridden(IResponseCacheFilter executingFilter, FilterContext context)
{
Debug.Assert(context != null);

// Return true if there are any filters which are after the current filter. In which case the current
// filter should be skipped.
for (var i = context.Filters.Count - 1; i >= 0; i--)
{
var filter = context.Filters[i];
if (filter is IResponseCacheFilter)
{
return !object.ReferenceEquals(executingFilter, filter);
}
}

Debug.Fail("The executing filter must be part of the filter context.");
return false;
}
}
}