Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

Commit 48cbfa4

Browse files
committed
Feedback
1 parent 0ca7aac commit 48cbfa4

File tree

7 files changed

+84
-106
lines changed

7 files changed

+84
-106
lines changed

src/Microsoft.AspNetCore.ResponseCaching/Internal/Interfaces/IResponseCachingPolicyProvider.cs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,35 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
66
public interface IResponseCachingPolicyProvider
77
{
88
/// <summary>
9-
/// Determine wehther the response cache middleware should be executed for the incoming HTTP request.
9+
/// Determine whether the response caching logic should be attempted for the incoming HTTP request.
1010
/// </summary>
1111
/// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
12-
/// <returns><c>true</c> if the request is cacheable; otherwise <c>false</c>.</returns>
13-
bool AllowResponseCaching(ResponseCachingContext context);
12+
/// <returns><c>true</c> if response caching logic should be attempted; otherwise <c>false</c>.</returns>
13+
bool AttemptResponseCaching(ResponseCachingContext context);
1414

15+
/// <summary>
16+
/// Determine whether a cache lookup is allowed for the incoming HTTP request.
17+
/// </summary>
18+
/// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
19+
/// <returns><c>true</c> if cache lookup for this request is allowed; otherwise <c>false</c>.</returns>
1520
bool AllowCacheLookup(ResponseCachingContext context);
1621

22+
/// <summary>
23+
/// Determine whether storage of the response is allowed for the incoming HTTP request.
24+
/// </summary>
25+
/// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
26+
/// <returns><c>true</c> if storage of the response for this request is allowed; otherwise <c>false</c>.</returns>
1727
bool AllowCacheStorage(ResponseCachingContext context);
1828

1929
/// <summary>
20-
/// Determine whether the response received by the middleware be cached for future requests.
30+
/// Determine whether the response received by the middleware can be cached for future requests.
2131
/// </summary>
2232
/// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
2333
/// <returns><c>true</c> if the response is cacheable; otherwise <c>false</c>.</returns>
2434
bool IsResponseCacheable(ResponseCachingContext context);
2535

2636
/// <summary>
27-
/// Determine whether the response retrieved from the response cache is fresh and be served.
37+
/// Determine whether the response retrieved from the response cache is fresh and can be served.
2838
/// </summary>
2939
/// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
3040
/// <returns><c>true</c> if the cached entry is fresh; otherwise <c>false</c>.</returns>

src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingContext.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ internal ResponseCachingContext(HttpContext httpContext, ILogger logger)
3737

3838
internal ILogger Logger { get; }
3939

40-
internal bool AllowResponseCapture { get; set; }
41-
4240
internal bool ShouldCacheResponse { get; set; }
4341

4442
internal string BaseKey { get; set; }

src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingPolicyProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
1010
{
1111
public class ResponseCachingPolicyProvider : IResponseCachingPolicyProvider
1212
{
13-
public virtual bool AllowResponseCaching(ResponseCachingContext context)
13+
public virtual bool AttemptResponseCaching(ResponseCachingContext context)
1414
{
1515
var request = context.HttpContext.Request;
1616

@@ -212,7 +212,7 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
212212
var maxStaleExist = HeaderUtilities.ContainsCacheDirective(requestCacheControlHeaders, CacheControlHeaderValue.MaxStaleString);
213213
HeaderUtilities.TryParseSeconds(requestCacheControlHeaders, CacheControlHeaderValue.MaxStaleString, out requestMaxStale);
214214

215-
// Request allows stale values no age limit
215+
// Request allows stale values with no age limit
216216
if (maxStaleExist && !requestMaxStale.HasValue)
217217
{
218218
context.Logger.LogExpirationInfiniteMaxStaleSatisfied(age, lowestMaxAge.Value);

src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs

Lines changed: 45 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -74,50 +74,52 @@ public async Task Invoke(HttpContext httpContext)
7474
var context = new ResponseCachingContext(httpContext, _logger);
7575

7676
// Should we attempt any caching logic?
77-
if (_policyProvider.AllowResponseCaching(context))
77+
if (_policyProvider.AttemptResponseCaching(context))
7878
{
7979
// Can this request be served from cache?
8080
if (_policyProvider.AllowCacheLookup(context) && await TryServeFromCacheAsync(context))
8181
{
8282
return;
8383
}
8484

85-
context.AllowResponseCapture = _policyProvider.AllowCacheStorage(context);
85+
// Should we store the response to this request?
86+
if (_policyProvider.AllowCacheStorage(context))
87+
{
88+
// Hook up to listen to the response stream
89+
ShimResponseStream(context);
8690

87-
// Hook up to listen to the response stream
88-
ShimResponseStream(context);
91+
try
92+
{
93+
// Subscribe to OnStarting event
94+
httpContext.Response.OnStarting(_onStartingCallback, context);
8995

90-
try
91-
{
92-
// Subscribe to OnStarting event
93-
httpContext.Response.OnStarting(_onStartingCallback, context);
96+
await _next(httpContext);
9497

95-
await _next(httpContext);
98+
// If there was no response body, check the response headers now. We can cache things like redirects.
99+
await OnResponseStartingAsync(context);
96100

97-
// If there was no response body, check the response headers now. We can cache things like redirects.
98-
await OnResponseStartingAsync(context);
101+
// Finalize the cache entry
102+
await FinalizeCacheBodyAsync(context);
103+
}
104+
finally
105+
{
106+
UnshimResponseStream(context);
107+
}
99108

100-
// Finalize the cache entry
101-
await FinalizeCacheBodyAsync(context);
102-
}
103-
finally
104-
{
105-
UnshimResponseStream(context);
109+
return;
106110
}
107111
}
108-
else
109-
{
110-
// Add IResponseCachingFeature which may be required when the response is generated
111-
AddResponseCachingFeature(httpContext);
112112

113-
try
114-
{
115-
await _next(httpContext);
116-
}
117-
finally
118-
{
119-
RemoveResponseCachingFeature(httpContext);
120-
}
113+
// Response should not be captured but add IResponseCachingFeature which may be required when the response is generated
114+
AddResponseCachingFeature(httpContext);
115+
116+
try
117+
{
118+
await _next(httpContext);
119+
}
120+
finally
121+
{
122+
RemoveResponseCachingFeature(httpContext);
121123
}
122124
}
123125

@@ -327,7 +329,7 @@ internal async Task FinalizeCacheBodyAsync(ResponseCachingContext context)
327329

328330
internal Task OnResponseStartingAsync(ResponseCachingContext context)
329331
{
330-
if (context.AllowResponseCapture && !context.ResponseStarted)
332+
if (!context.ResponseStarted)
331333
{
332334
context.ResponseStarted = true;
333335
context.ResponseTime = _options.SystemClock.UtcNow;
@@ -351,19 +353,16 @@ internal static void AddResponseCachingFeature(HttpContext context)
351353

352354
internal void ShimResponseStream(ResponseCachingContext context)
353355
{
354-
if (context.AllowResponseCapture)
356+
// Shim response stream
357+
context.OriginalResponseStream = context.HttpContext.Response.Body;
358+
context.ResponseCachingStream = new ResponseCachingStream(context.OriginalResponseStream, _options.MaximumBodySize, StreamUtilities.BodySegmentSize);
359+
context.HttpContext.Response.Body = context.ResponseCachingStream;
360+
361+
// Shim IHttpSendFileFeature
362+
context.OriginalSendFileFeature = context.HttpContext.Features.Get<IHttpSendFileFeature>();
363+
if (context.OriginalSendFileFeature != null)
355364
{
356-
// Shim response stream
357-
context.OriginalResponseStream = context.HttpContext.Response.Body;
358-
context.ResponseCachingStream = new ResponseCachingStream(context.OriginalResponseStream, _options.MaximumBodySize, StreamUtilities.BodySegmentSize);
359-
context.HttpContext.Response.Body = context.ResponseCachingStream;
360-
361-
// Shim IHttpSendFileFeature
362-
context.OriginalSendFileFeature = context.HttpContext.Features.Get<IHttpSendFileFeature>();
363-
if (context.OriginalSendFileFeature != null)
364-
{
365-
context.HttpContext.Features.Set<IHttpSendFileFeature>(new SendFileFeatureWrapper(context.OriginalSendFileFeature, context.ResponseCachingStream));
366-
}
365+
context.HttpContext.Features.Set<IHttpSendFileFeature>(new SendFileFeatureWrapper(context.OriginalSendFileFeature, context.ResponseCachingStream));
367366
}
368367

369368
// Add IResponseCachingFeature
@@ -375,14 +374,11 @@ internal static void RemoveResponseCachingFeature(HttpContext context) =>
375374

376375
internal static void UnshimResponseStream(ResponseCachingContext context)
377376
{
378-
if (context.AllowResponseCapture)
379-
{
380-
// Unshim response stream
381-
context.HttpContext.Response.Body = context.OriginalResponseStream;
377+
// Unshim response stream
378+
context.HttpContext.Response.Body = context.OriginalResponseStream;
382379

383-
// Unshim IHttpSendFileFeature
384-
context.HttpContext.Features.Set(context.OriginalSendFileFeature);
385-
}
380+
// Unshim IHttpSendFileFeature
381+
context.HttpContext.Features.Set(context.OriginalSendFileFeature);
386382

387383
// Remove IResponseCachingFeature
388384
RemoveResponseCachingFeature(context.HttpContext);

0 commit comments

Comments
 (0)