Brennan's

Author: sebastienros

src/Middleware/OutputCaching/samples/OutputCachingSample/Startup.cs

@@ -25,7 +25,7 @@
 
 app.MapGet("/nocache", Gravatar.WriteGravatar).CacheOutput(x => x.NoCache());
 
-app.MapGet("/profile", Gravatar.WriteGravatar).CacheOutput(x => x.Policy("NoCache"));
+app.MapGet("/profile", Gravatar.WriteGravatar).CacheOutput("NoCache");
 
 app.MapGet("/attribute", [OutputCache(PolicyName = "NoCache")] () => Gravatar.WriteGravatar);
 

src/Middleware/OutputCaching/src/CachedVaryByRules.cs renamed to src/Middleware/OutputCaching/src/CacheVaryByRules.cs

@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.OutputCaching;
 /// <summary>
 /// Represents vary-by rules.
 /// </summary>
-public sealed class CachedVaryByRules
+public sealed class CacheVaryByRules
 {
     private Dictionary<string, string>? _varyByCustom;
 

src/Middleware/OutputCaching/src/CachedResponseBody.cs

@@ -17,6 +17,8 @@ internal sealed class CachedResponseBody
     /// <param name="length">The length.</param>
     public CachedResponseBody(List<byte[]> segments, long length)
     {
+        ArgumentNullException.ThrowIfNull(segments);
+
         Segments = segments;
         Length = length;
     }
@@ -45,17 +47,7 @@ public async Task CopyToAsync(PipeWriter destination, CancellationToken cancella
         {
             cancellationToken.ThrowIfCancellationRequested();
 
-            Copy(segment, destination);
-
-            await destination.FlushAsync(cancellationToken);
+            await destination.WriteAsync(segment, cancellationToken);
         }
     }
-
-    private static void Copy(byte[] segment, PipeWriter destination)
-    {
-        var span = destination.GetSpan(segment.Length);
-
-        segment.CopyTo(span);
-        destination.Advance(segment.Length);
-    }
 }

src/Middleware/OutputCaching/src/IOutputCachePolicy.cs

@@ -13,18 +13,18 @@ public interface IOutputCachePolicy
     /// At that point the cache middleware can still be enabled or disabled for the request.
     /// </summary>
     /// <param name="context">The current request's cache context.</param>
-    ValueTask CacheRequestAsync(OutputCacheContext context);
+    ValueTask CacheRequestAsync(OutputCacheContext context, CancellationToken cancellation);
 
     /// <summary>
     /// Updates the <see cref="OutputCacheContext"/> before the cached response is used.
     /// At that point the freshness of the cached response can be updated.
     /// </summary>
     /// <param name="context">The current request's cache context.</param>
-    ValueTask ServeFromCacheAsync(OutputCacheContext context);
+    ValueTask ServeFromCacheAsync(OutputCacheContext context, CancellationToken cancellation);
 
     /// <summary>
     /// Updates the <see cref="OutputCacheContext"/> before the response is served and can be cached.
     /// At that point cacheability of the response can be updated.
     /// </summary>
-    ValueTask ServeResponseAsync(OutputCacheContext context);
+    ValueTask ServeResponseAsync(OutputCacheContext context, CancellationToken cancellation);
 }

src/Middleware/OutputCaching/src/Memory/MemoryOutputCacheStore.cs

@@ -1,15 +1,15 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Concurrent;
 using Microsoft.Extensions.Caching.Memory;
 
 namespace Microsoft.AspNetCore.OutputCaching.Memory;
 
 internal sealed class MemoryOutputCacheStore : IOutputCacheStore
 {
     private readonly IMemoryCache _cache;
-    private readonly ConcurrentDictionary<string, HashSet<string>> _taggedEntries = new();
+    private readonly Dictionary<string, HashSet<string>> _taggedEntries = new();
+    private readonly object _tagsLock = new();
 
     internal MemoryOutputCacheStore(IMemoryCache cache)
     {
@@ -22,11 +22,16 @@ public ValueTask EvictByTagAsync(string tag, CancellationToken cancellationToken
     {
         ArgumentNullException.ThrowIfNull(tag);
 
-        if (_taggedEntries.TryGetValue(tag, out var keys))
+        lock (_tagsLock)
         {
-            foreach (var key in keys)
+            if (_taggedEntries.TryGetValue(tag, out var keys))
             {
-                _cache.Remove(key);
+                foreach (var key in keys)
+                {
+                    _cache.Remove(key);
+                }
+
+                _taggedEntries.Remove(tag);
             }
         }
 
@@ -50,29 +55,41 @@ public ValueTask SetAsync(string key, byte[] value, string[]? tags, TimeSpan val
 
         if (tags != null)
         {
-            foreach (var tag in tags)
-            {
-                var keys = _taggedEntries.GetOrAdd(tag, _ => new HashSet<string>());
+            // Lock with SetEntry() to prevent EvictByTagAsync() from trying to remove a tag whose entry hasn't been added yet.
+            // It might be acceptable to not lock SetEntry() since in this case Remove(key) would just no-op and the user retry to evict.
 
-                // Copy the list of tags to prevent locking
-
-                var local = new HashSet<string>(keys)
+            lock (_tagsLock)
+            {
+                foreach (var tag in tags)
                 {
-                    key
-                };
+                    if (!_taggedEntries.TryGetValue(tag, out var keys))
+                    {
+                        keys = new HashSet<string>();
+                        _taggedEntries[tag] = keys;
+                    }
 
-                _taggedEntries[tag] = local;
+                    keys.Add(key);
+                }
+
+                SetEntry();
             }
         }
+        else
+        {
+            SetEntry();
+        }
 
-        _cache.Set(
+        void SetEntry()
+        {
+            _cache.Set(
             key,
             value,
             new MemoryCacheEntryOptions
             {
                 AbsoluteExpirationRelativeToNow = validFor,
                 Size = value.Length
             });
+        }
 
         return ValueTask.CompletedTask;
     }

src/Middleware/OutputCaching/src/Microsoft.AspNetCore.OutputCaching.csproj

@@ -4,7 +4,6 @@
     <Description>ASP.NET Core middleware for caching HTTP responses on the server.</Description>
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <IsAspNetCoreApp>true</IsAspNetCoreApp>
-    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <IsPackable>false</IsPackable>
     <IsTrimmable>true</IsTrimmable>
   </PropertyGroup>

src/Middleware/OutputCaching/src/OutputCacheAttribute.cs

@@ -6,6 +6,9 @@ namespace Microsoft.AspNetCore.OutputCaching;
 /// <summary>
 /// Specifies the parameters necessary for setting appropriate headers in output caching.
 /// </summary>
+/// <remarks>
+/// This attribute requires the output cache middleware.
+/// </remarks>
 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
 public sealed class OutputCacheAttribute : Attribute
 {
@@ -30,7 +33,7 @@ public int Duration
     /// Gets or sets the value which determines whether the reponse should be cached or not.
     /// When set to <see langword="true"/>, the response won't be cached.
     /// </summary>
-    public bool NoCache
+    public bool NoStore
     {
         get => _noCache ?? false;
         init => _noCache = value;
@@ -39,17 +42,11 @@ public bool NoCache
     /// <summary>
     /// Gets or sets the query keys to vary by.
     /// </summary>
-    /// <remarks>
-    /// <see cref="VaryByQueryKeys"/> requires the output cache middleware.
-    /// </remarks>
     public string[]? VaryByQueryKeys { get; init; }
 
     /// <summary>
     /// Gets or sets the headers to vary by.
     /// </summary>
-    /// <remarks>
-    /// <see cref="VaryByHeaders"/> requires the output cache middleware.
-    /// </remarks>
     public string[]? VaryByHeaders { get; init; }
 
     /// <summary>
@@ -66,14 +63,14 @@ internal IOutputCachePolicy BuildPolicy()
 
         var builder = new OutputCachePolicyBuilder();
 
-        if (_noCache != null && _noCache.Value)
+        if (PolicyName != null)
         {
-            builder.NoCache();
+            builder.AddPolicy(new NamedPolicy(PolicyName));
         }
 
-        if (PolicyName != null)
+        if (_noCache != null && _noCache.Value)
         {
-            builder.Policy(PolicyName);
+            builder.NoCache();
         }
 
         if (VaryByQueryKeys != null)

src/Middleware/OutputCaching/src/OutputCacheContext.cs

@@ -1,8 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-#nullable disable
-
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 
@@ -52,9 +50,9 @@ internal OutputCacheContext(HttpContext httpContext, IOutputCacheStore store, Ou
     public DateTimeOffset? ResponseTime { get; internal set; }
 
     /// <summary>
-    /// Gets the <see cref="CachedVaryByRules"/> instance.
+    /// Gets the <see cref="CacheVaryByRules"/> instance.
     /// </summary>
-    public CachedVaryByRules CachedVaryByRules { get; set; } = new();
+    public CacheVaryByRules CacheVaryByRules { get; set; } = new();
 
     /// <summary>
     /// Gets the tags of the cached response.
@@ -66,23 +64,22 @@ internal OutputCacheContext(HttpContext httpContext, IOutputCacheStore store, Ou
     /// </summary>
     public TimeSpan? ResponseExpirationTimeSpan { get; set; }
 
-    internal string CacheKey { get; set; }
+    internal string CacheKey { get; set; } = default!;
 
     internal TimeSpan CachedResponseValidFor { get; set; }
 
     internal bool IsCacheEntryFresh { get; set; }
 
     internal TimeSpan CachedEntryAge { get; set; }
 
-    internal OutputCacheEntry CachedResponse { get; set; }
+    internal OutputCacheEntry CachedResponse { get; set; } = default!;
 
     internal bool ResponseStarted { get; set; }
 
-    internal Stream OriginalResponseStream { get; set; }
+    internal Stream OriginalResponseStream { get; set; } = default!;
 
-    internal OutputCacheStream OutputCacheStream { get; set; }
+    internal OutputCacheStream OutputCacheStream { get; set; } = default!;
     internal ILogger Logger { get; }
     internal OutputCacheOptions Options { get; }
     internal IOutputCacheStore Store { get; }
-
 }

src/Middleware/OutputCaching/src/OutputCacheEntry.cs

@@ -5,7 +5,6 @@
 
 namespace Microsoft.AspNetCore.OutputCaching;
 
-/// <inheritdoc />
 internal sealed class OutputCacheEntry
 {
     /// <summary>
@@ -31,5 +30,5 @@ internal sealed class OutputCacheEntry
     /// <summary>
     /// Gets the tags of the cache entry.
     /// </summary>
-    public string[]? Tags { get; set; }
+    public string[] Tags { get; set; } = Array.Empty<string>();
 }

src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs

@@ -22,9 +22,7 @@ internal static class OutputCacheEntryFormatter
             return null;
         }
 
-        using var br = new MemoryStream(content);
-
-        var formatter = await JsonSerializer.DeserializeAsync(br, FormatterEntrySerializerContext.Default.FormatterEntry, cancellationToken: cancellationToken);
+        var formatter = JsonSerializer.Deserialize(content, FormatterEntrySerializerContext.Default.FormatterEntry);
 
         if (formatter == null)
         {
@@ -35,26 +33,27 @@ internal static class OutputCacheEntryFormatter
         {
             StatusCode = formatter.StatusCode,
             Created = formatter.Created,
-            Tags = formatter.Tags
+            Tags = formatter.Tags,
+            Headers = new(),
+            Body = new CachedResponseBody(formatter.Body, formatter.Body.Sum(x => x.Length))
         };
 
         if (formatter.Headers != null)
         {
-            outputCacheEntry.Headers = new();
-
             foreach (var header in formatter.Headers)
             {
                 outputCacheEntry.Headers.TryAdd(header.Key, header.Value);
             }
         }
-        var cachedResponseBody = new CachedResponseBody(formatter.Body, formatter.Body.Sum(x => x.Length));
-        outputCacheEntry.Body = cachedResponseBody;
+
         return outputCacheEntry;
     }
 
     public static async ValueTask StoreAsync(string key, OutputCacheEntry value, TimeSpan duration, IOutputCacheStore store, CancellationToken cancellationToken)
     {
         ArgumentNullException.ThrowIfNull(value);
+        ArgumentNullException.ThrowIfNull(value.Body);
+        ArgumentNullException.ThrowIfNull(value.Headers);
 
         var formatterEntry = new FormatterEntry
         {
@@ -73,9 +72,10 @@ public static async ValueTask StoreAsync(string key, OutputCacheEntry value, Tim
             }
         }
 
-        using var br = new MemoryStream();
+        using var bufferStream = new MemoryStream();
+
+        JsonSerializer.Serialize(bufferStream, formatterEntry, FormatterEntrySerializerContext.Default.FormatterEntry);
 
-        await JsonSerializer.SerializeAsync(br, formatterEntry, FormatterEntrySerializerContext.Default.FormatterEntry, cancellationToken);
-        await store.SetAsync(key, br.ToArray(), value.Tags ?? Array.Empty<string>(), duration, cancellationToken);
+        await store.SetAsync(key, bufferStream.ToArray(), value.Tags ?? Array.Empty<string>(), duration, cancellationToken);
     }
 }

src/Middleware/OutputCaching/src/OutputCacheKeyProvider.cs

@@ -33,10 +33,10 @@ public string CreateStorageKey(OutputCacheContext context)
     {
         ArgumentNullException.ThrowIfNull(_builderPool);
 
-        var varyByRules = context.CachedVaryByRules;
+        var varyByRules = context.CacheVaryByRules;
         if (varyByRules == null)
         {
-            throw new InvalidOperationException($"{nameof(CachedVaryByRules)} must not be null on the {nameof(OutputCacheContext)}");
+            throw new InvalidOperationException($"{nameof(CacheVaryByRules)} must not be null on the {nameof(OutputCacheContext)}");
         }
 
         var request = context.HttpContext.Request;