Code cleanup response caching middleware (#6124)

* minor code cleanups

* comment public interface

* simplified some things using language features

src/Middleware/ResponseCaching/src/Internal/CacheEntry/CacheEntryHelpers .cs

@@ -7,7 +7,6 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 {
     internal static class CacheEntryHelpers
     {
-
         internal static long EstimateCachedResponseSize(CachedResponse cachedResponse)
         {
             if (cachedResponse == null)
@@ -25,7 +24,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
                 {
                     foreach (var item in cachedResponse.Headers)
                     {
-                        size += item.Key.Length * sizeof(char) + EstimateStringValuesSize(item.Value);
+                        size += (item.Key.Length * sizeof(char)) + EstimateStringValuesSize(item.Value);
                     }
                 }
 

src/Middleware/ResponseCaching/src/Internal/FastGuid.cs

@@ -15,7 +15,8 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 
         // Instance components
         private string _idString;
-        internal long IdValue { get; private set; }
+
+        internal long IdValue { get; }
 
         internal string IdString
         {

src/Middleware/ResponseCaching/src/Internal/Interfaces/IResponseCache.cs

@@ -8,10 +8,37 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 {
     public interface IResponseCache
     {
+        /// <summary>
+        /// Gets the cached response for the given key, if it exists.
+        /// If no cached response exists for the given key, <c>null</c> is returned.
+        /// </summary>
+        /// <param name="key">The cache key to look up.</param>
+        /// <returns>The response cache entry if it exists; otherwise <c>null</c>.</returns>
         IResponseCacheEntry Get(string key);
+
+        /// <summary>
+        /// Gets the cached response for the given key, if it exists.
+        /// If no cached response exists for the given key, <c>null</c> is returned.
+        /// </summary>
+        /// <param name="key">The cache key to look up.</param>
+        /// <returns>The response cache entry if it exists; otherwise <c>null</c>.</returns>
         Task<IResponseCacheEntry> GetAsync(string key);
 
+        /// <summary>
+        /// Stores the given response in the response cache.
+        /// </summary>
+        /// <param name="key">The cache key to store the response under.</param>
+        /// <param name="entry">The response cache entry to store.</param>
+        /// <param name="validFor">The amount of time the entry will be kept in the cache before expiring, relative to now.</param>
         void Set(string key, IResponseCacheEntry entry, TimeSpan validFor);
+
+        /// <summary>
+        /// Stores the given response in the response cache.
+        /// </summary>
+        /// <param name="key">The cache key to store the response under.</param>
+        /// <param name="entry">The response cache entry to store.</param>
+        /// <param name="validFor">The amount of time the entry will be kept in the cache before expiring, relative to now.</param>
+        /// <returns>No result is returned.</returns>
         Task SetAsync(string key, IResponseCacheEntry entry, TimeSpan validFor);
     }
 }

src/Middleware/ResponseCaching/src/Internal/LoggerExtensions.cs

@@ -12,35 +12,35 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
     /// </summary>
     internal static class LoggerExtensions
     {
-        private static Action<ILogger, string, Exception> _logRequestMethodNotCacheable;
+        private static readonly Action<ILogger, string, Exception> _logRequestMethodNotCacheable;
-        private static Action<ILogger, Exception> _logRequestWithAuthorizationNotCacheable;
+        private static readonly Action<ILogger, Exception> _logRequestWithAuthorizationNotCacheable;
-        private static Action<ILogger, Exception> _logRequestWithNoCacheNotCacheable;
+        private static readonly Action<ILogger, Exception> _logRequestWithNoCacheNotCacheable;
-        private static Action<ILogger, Exception> _logRequestWithPragmaNoCacheNotCacheable;
+        private static readonly Action<ILogger, Exception> _logRequestWithPragmaNoCacheNotCacheable;
-        private static Action<ILogger, TimeSpan, Exception> _logExpirationMinFreshAdded;
+        private static readonly Action<ILogger, TimeSpan, Exception> _logExpirationMinFreshAdded;
-        private static Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationSharedMaxAgeExceeded;
+        private static readonly Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationSharedMaxAgeExceeded;
-        private static Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationMustRevalidate;
+        private static readonly Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationMustRevalidate;
-        private static Action<ILogger, TimeSpan, TimeSpan, TimeSpan, Exception> _logExpirationMaxStaleSatisfied;
+        private static readonly Action<ILogger, TimeSpan, TimeSpan, TimeSpan, Exception> _logExpirationMaxStaleSatisfied;
-        private static Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationMaxAgeExceeded;
+        private static readonly Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationMaxAgeExceeded;
-        private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logExpirationExpiresExceeded;
+        private static readonly Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logExpirationExpiresExceeded;
-        private static Action<ILogger, Exception> _logResponseWithoutPublicNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithoutPublicNotCacheable;
-        private static Action<ILogger, Exception> _logResponseWithNoStoreNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithNoStoreNotCacheable;
-        private static Action<ILogger, Exception> _logResponseWithNoCacheNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithNoCacheNotCacheable;
-        private static Action<ILogger, Exception> _logResponseWithSetCookieNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithSetCookieNotCacheable;
-        private static Action<ILogger, Exception> _logResponseWithVaryStarNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithVaryStarNotCacheable;
-        private static Action<ILogger, Exception> _logResponseWithPrivateNotCacheable;
+        private static readonly Action<ILogger, Exception> _logResponseWithPrivateNotCacheable;
-        private static Action<ILogger, int, Exception> _logResponseWithUnsuccessfulStatusCodeNotCacheable;
+        private static readonly Action<ILogger, int, Exception> _logResponseWithUnsuccessfulStatusCodeNotCacheable;
-        private static Action<ILogger, Exception> _logNotModifiedIfNoneMatchStar;
+        private static readonly Action<ILogger, Exception> _logNotModifiedIfNoneMatchStar;
-        private static Action<ILogger, EntityTagHeaderValue, Exception> _logNotModifiedIfNoneMatchMatched;
+        private static readonly Action<ILogger, EntityTagHeaderValue, Exception> _logNotModifiedIfNoneMatchMatched;
-        private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfModifiedSinceSatisfied;
+        private static readonly Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfModifiedSinceSatisfied;
-        private static Action<ILogger, Exception> _logNotModifiedServed;
+        private static readonly Action<ILogger, Exception> _logNotModifiedServed;
-        private static Action<ILogger, Exception> _logCachedResponseServed;
+        private static readonly Action<ILogger, Exception> _logCachedResponseServed;
-        private static Action<ILogger, Exception> _logGatewayTimeoutServed;
+        private static readonly Action<ILogger, Exception> _logGatewayTimeoutServed;
-        private static Action<ILogger, Exception> _logNoResponseServed;
+        private static readonly Action<ILogger, Exception> _logNoResponseServed;
-        private static Action<ILogger, string, string, Exception> _logVaryByRulesUpdated;
+        private static readonly Action<ILogger, string, string, Exception> _logVaryByRulesUpdated;
-        private static Action<ILogger, Exception> _logResponseCached;
+        private static readonly Action<ILogger, Exception> _logResponseCached;
-        private static Action<ILogger, Exception> _logResponseNotCached;
+        private static readonly Action<ILogger, Exception> _logResponseNotCached;
-        private static Action<ILogger, Exception> _logResponseContentLengthMismatchNotCached;
+        private static readonly Action<ILogger, Exception> _logResponseContentLengthMismatchNotCached;
-        private static Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationInfiniteMaxStaleSatisfied;
+        private static readonly Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationInfiniteMaxStaleSatisfied;
 
         static LoggerExtensions()
         {

src/Middleware/ResponseCaching/src/Internal/MemoryResponseCache.cs

@@ -13,20 +13,14 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 
         public MemoryResponseCache(IMemoryCache cache)
         {
-            if (cache == null)
+            _cache = cache ?? throw new ArgumentNullException(nameof(cache));
-            {
-                throw new ArgumentNullException(nameof(cache));
-            }
-
-            _cache = cache;
         }
 
         public IResponseCacheEntry Get(string key)
         {
             var entry = _cache.Get(key);
 
-            var memoryCachedResponse = entry as MemoryCachedResponse;
+            if (entry is MemoryCachedResponse memoryCachedResponse)
-            if (memoryCachedResponse != null)
             {
                 return new CachedResponse
                 {
@@ -49,8 +43,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 
         public void Set(string key, IResponseCacheEntry entry, TimeSpan validFor)
         {
-            var cachedResponse = entry as CachedResponse;
+            if (entry is CachedResponse cachedResponse)
-            if (cachedResponse != null)
             {
                 var segmentStream = new SegmentWriteStream(StreamUtilities.BodySegmentSize);
                 cachedResponse.Body.CopyTo(segmentStream);
@@ -90,4 +83,4 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
             return Task.CompletedTask;
         }
     }
-}
+}

src/Middleware/ResponseCaching/src/Internal/ResponseCachingKeyProvider.cs

@@ -96,7 +96,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
                 throw new InvalidOperationException($"{nameof(CachedVaryByRules)} must not be null on the {nameof(ResponseCachingContext)}");
             }
 
-            if ((StringValues.IsNullOrEmpty(varyByRules.Headers) && StringValues.IsNullOrEmpty(varyByRules.QueryKeys)))
+            if (StringValues.IsNullOrEmpty(varyByRules.Headers) && StringValues.IsNullOrEmpty(varyByRules.QueryKeys))
             {
                 return varyByRules.VaryByKeyPrefix;
             }
@@ -204,7 +204,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 
         private class QueryKeyComparer : IComparer<KeyValuePair<string, StringValues>>
         {
-            private StringComparer _stringComparer;
+            private readonly StringComparer _stringComparer;
 
             public static QueryKeyComparer OrdinalIgnoreCase { get; } = new QueryKeyComparer(StringComparer.OrdinalIgnoreCase);
 

src/Middleware/ResponseCaching/src/Internal/ResponseCachingPolicyProvider.cs

@@ -47,7 +47,6 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
             else
             {
                 // Support for legacy HTTP 1.0 cache directive
-                var pragmaHeaderValues = request.Headers[HeaderNames.Pragma];
                 if (HeaderUtilities.ContainsCacheDirective(request.Headers[HeaderNames.Pragma], CacheControlHeaderValue.NoCacheString))
                 {
                     context.Logger.LogRequestWithPragmaNoCacheNotCacheable();

src/Middleware/ResponseCaching/src/Internal/SendFileFeatureWrapper.cs

@@ -19,10 +19,10 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
         }
 
         // Flush and disable the buffer if anyone tries to call the SendFile feature.
-        public Task SendFileAsync(string path, long offset, long? length, CancellationToken cancellation)
+        public Task SendFileAsync(string path, long offset, long? count, CancellationToken cancellation)
         {
             _responseCachingStream.DisableBuffering();
-            return _originalSendFileFeature.SendFileAsync(path, offset, length, cancellation);
+            return _originalSendFileFeature.SendFileAsync(path, offset, count, cancellation);
         }
     }
-}
+}

src/Middleware/ResponseCaching/src/Internal/StringBuilderExtensions.cs

@@ -21,4 +21,4 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
             return builder;
         }
     }
-}
+}

src/Middleware/ResponseCaching/src/Internal/SystemClock.cs

@@ -13,12 +13,6 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
         /// <summary>
         /// Retrieves the current system time in UTC.
         /// </summary>
-        public DateTimeOffset UtcNow
+        public DateTimeOffset UtcNow => DateTimeOffset.UtcNow;
-        {
-            get
-            {
-                return DateTimeOffset.UtcNow;
-            }
-        }
     }
 }

src/Middleware/ResponseCaching/src/ResponseCachingExtensions.cs

@@ -3,7 +3,6 @@
 
 using System;
 using Microsoft.AspNetCore.ResponseCaching;
-using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Builder
 {

src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs

@@ -138,8 +138,7 @@ namespace Microsoft.AspNetCore.ResponseCaching
 
         internal async Task<bool> TryServeCachedResponseAsync(ResponseCachingContext context, IResponseCacheEntry cacheEntry)
         {
-            var cachedResponse = cacheEntry as CachedResponse;
+            if (!(cacheEntry is CachedResponse cachedResponse))
-            if (cachedResponse == null)
             {
                 return false;
             }
@@ -199,8 +198,7 @@ namespace Microsoft.AspNetCore.ResponseCaching
             context.BaseKey = _keyProvider.CreateBaseKey(context);
             var cacheEntry = await _cache.GetAsync(context.BaseKey);
 
-            var cachedVaryByRules = cacheEntry as CachedVaryByRules;
+            if (cacheEntry is CachedVaryByRules cachedVaryByRules)
-            if (cachedVaryByRules != null)
             {
                 // Request contains vary rules, recompute key(s) and try again
                 context.CachedVaryByRules = cachedVaryByRules;

src/Middleware/ResponseCaching/src/Streams/ResponseCachingStream.cs

@@ -13,9 +13,9 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
         private readonly Stream _innerStream;
         private readonly long _maxBufferSize;
         private readonly int _segmentSize;
-        private SegmentWriteStream _segmentWriteStream;
+        private readonly SegmentWriteStream _segmentWriteStream;
-        private Action _startResponseCallback;
+        private readonly Action _startResponseCallback;
-        private Func<Task> _startResponseCallbackAsync;
+        private readonly Func<Task> _startResponseCallbackAsync;
 
         internal ResponseCachingStream(Stream innerStream, long maxBufferSize, int segmentSize, Action startResponseCallback, Func<Task> startResponseCallbackAsync)
         {

src/Middleware/ResponseCaching/src/Streams/SegmentReadStream.cs

@@ -19,12 +19,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 
         internal SegmentReadStream(List<byte[]> segments, long length)
         {
-            if (segments == null)
+            _segments = segments ?? throw new ArgumentNullException(nameof(segments));
-            {
-                throw new ArgumentNullException(nameof(segments));
-            }
-
-            _segments = segments;
             _length = length;
         }