perf(core): halve allocations on the coalesced cache-miss path

Stampede.Http.Tests/Caching/BackgroundRevalidationCoordinatorTests.cs

@@ -0,0 +1,107 @@
+using Stampede.Http.Caching;
+using FluentAssertions;
+
+namespace Stampede.Http.Tests.Caching;
+
+/// <summary>
+/// Verifies the stale-while-revalidate deduplication contract (RFC 5861 §3): a key has at most one
+/// background refresh in flight, and the claim is always released once that refresh finishes.
+/// </summary>
+public sealed class BackgroundRevalidationCoordinatorTests
+{
+    [Fact]
+    public async Task Schedule_WhileRefreshInFlight_DoesNotStartASecondOne()
+    {
+        BackgroundRevalidationCoordinator coordinator = new();
+        TaskCompletionSource gate = new(TaskCreationOptions.RunContinuationsAsynchronously);
+        int started = 0;
+
+        for (int i = 0; i < 10; i++)
+        {
+            coordinator.Schedule("key", async () =>
+            {
+                _ = Interlocked.Increment(ref started);
+                await gate.Task;
+            });
+        }
+
+        // Give the scheduled work a chance to run before asserting.
+        await Task.Delay(100, TestContext.Current.CancellationToken);
+
+        Volatile.Read(ref started).Should().Be(1, "a key already being revalidated must not be revalidated again");
+
+        gate.SetResult();
+    }
+
+    [Fact]
+    public async Task Schedule_DifferentKeys_RunIndependently()
+    {
+        BackgroundRevalidationCoordinator coordinator = new();
+        TaskCompletionSource gate = new(TaskCreationOptions.RunContinuationsAsynchronously);
+        int started = 0;
+
+        for (int i = 0; i < 5; i++)
+        {
+            coordinator.Schedule($"key-{i}", async () =>
+            {
+                _ = Interlocked.Increment(ref started);
+                await gate.Task;
+            });
+        }
+
+        await Task.Delay(100, TestContext.Current.CancellationToken);
+
+        Volatile.Read(ref started).Should().Be(5, "deduplication is per key, not global");
+
+        gate.SetResult();
+    }
+
+    [Fact]
+    public async Task Schedule_AfterPreviousRefreshCompletes_StartsAgain()
+    {
+        BackgroundRevalidationCoordinator coordinator = new();
+        int started = 0;
+
+        for (int i = 0; i < 20; i++)
+        {
+            coordinator.Schedule("key", () =>
+            {
+                _ = Interlocked.Increment(ref started);
+                return Task.CompletedTask;
+            });
+
+            // Let the refresh finish and release its claim before scheduling the next one.
+            await Task.Delay(20, TestContext.Current.CancellationToken);
+        }
+
+        // Regression: claiming the key from inside the background task allowed the refresh to finish before
+        // its own registration landed, leaving the key claimed forever and blocking all later revalidations.
+        Volatile.Read(ref started).Should().Be(20,
+            "each completed refresh must release its key so the entry can be revalidated again");
+    }
+
+    [Fact]
+    public async Task Schedule_WhenRefreshThrows_ReleasesTheKey()
+    {
+        BackgroundRevalidationCoordinator coordinator = new();
+        int started = 0;
+
+        coordinator.Schedule("key", () =>
+        {
+            _ = Interlocked.Increment(ref started);
+            throw new InvalidOperationException("origin unreachable");
+        });
+
+        await Task.Delay(50, TestContext.Current.CancellationToken);
+
+        coordinator.Schedule("key", () =>
+        {
+            _ = Interlocked.Increment(ref started);
+            return Task.CompletedTask;
+        });
+
+        await Task.Delay(50, TestContext.Current.CancellationToken);
+
+        Volatile.Read(ref started).Should().Be(2, "a failed refresh must not block the key permanently");
+    }
+}

Stampede.Http.Tests/Caching/HeadMethodCachingTests.cs

@@ -77,6 +77,61 @@ public async Task Head_FromCache_ResponseBodyIsEmpty()
         body.Should().BeEmpty("HEAD responses must not include a body");
     }
 
+    [Fact]
+    public async Task Head_FromCache_PreservesContentHeaders()
+    {
+        (CachingMiddleware middleware, _) = BuildPipeline(_ =>
+            new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new StringContent("body text", System.Text.Encoding.UTF8, "application/json")
+            });
+
+        HttpMessageInvoker invoker = Invoker(middleware);
+        const string url = "https://api.test/head/content-headers";
+
+        HttpResponseMessage getResponse = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, url), CancellationToken.None);
+        HttpResponseMessage headResponse = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Head, url), CancellationToken.None);
+
+        // RFC 9110 §9.3.2 — HEAD repeats the header fields the equivalent GET would have sent
+        headResponse.Content.Headers.ContentType?.MediaType.Should().Be("application/json",
+            "a cached HEAD must repeat the Content-Type the equivalent GET would have sent");
+        headResponse.Content.Headers.ContentLength.Should().Be(getResponse.Content.Headers.ContentLength,
+            "a cached HEAD must report the same Content-Length as the equivalent GET");
+    }
+
+    [Fact]
+    public async Task Head_AfterRevalidation_PreservesContentHeaders()
+    {
+        int callCount = 0;
+        (CachingMiddleware middleware, _) = BuildPipeline(_ =>
+        {
+            callCount++;
+            if (callCount == 1)
+            {
+                HttpResponseMessage r = new(HttpStatusCode.OK)
+                {
+                    Content = new StringContent("data", System.Text.Encoding.UTF8, "application/json")
+                };
+                r.Headers.ETag = new EntityTagHeaderValue("\"v1\"");
+                r.Headers.CacheControl = new CacheControlHeaderValue { MaxAge = TimeSpan.Zero };
+                return r;
+            }
+
+            return new HttpResponseMessage(HttpStatusCode.NotModified);
+        });
+
+        HttpMessageInvoker invoker = Invoker(middleware);
+        const string url = "https://api.test/head/reval-content-headers";
+
+        _ = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, url), CancellationToken.None);
+        HttpResponseMessage headResponse = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Head, url), CancellationToken.None);
+
+        headResponse.Content.Headers.ContentType?.MediaType.Should().Be("application/json",
+            "a HEAD refreshed by a 304 must still repeat the stored Content-Type");
+        byte[] body = await headResponse.Content.ReadAsByteArrayAsync(TestContext.Current.CancellationToken);
+        body.Should().BeEmpty("HEAD response body must be empty even after revalidation");
+    }
+
     [Fact]
     public async Task Head_FromCache_AgeHeaderPresent()
     {

Stampede.Http.Tests/Caching/MemoryCacheStoreExpirationTests.cs

@@ -39,17 +39,44 @@ private static CacheEntry BuildEntry(
     // ── AbsoluteExpiration placement ─────────────────────────────────────────
 
     [Fact]
-    public void Set_WithoutStaleWindow_PastExpiry_EntryStaysForConditionalRevalidation()
+    public void Set_WithoutStaleWindow_PastExpiry_NoValidator_EntryNotRetained()
     {
         MemoryCacheStore store = CreateStore();
-        // ExpiresAt in the past and no stale window → evictionTtl ≤ 0 → no AbsoluteExpiration set.
-        // Entry must remain so its ETag / Last-Modified can be used for conditional revalidation.
+        // ExpiresAt in the past, no stale window and no validator → the entry can neither be served
+        // (it is already stale) nor revalidated (nothing to condition on), so it must not be retained.
         CacheEntry entry = BuildEntry(expiresAt: DateTimeOffset.UtcNow.AddSeconds(-1));
 
         store.Set("key1", entry);
 
         bool found = store.TryGetValue("key1", out _);
-        found.Should().BeTrue("past-ExpiresAt with no stale window should stay for conditional revalidation (LRU eviction only)");
+        found.Should().BeFalse("an entry that can never be served nor revalidated must not occupy the cache");
+    }
+
+    [Fact]
+    public void Set_WithoutStaleWindow_PastExpiry_WithValidator_RetainedByGrace()
+    {
+        MemoryCacheStore store = CreateStore();
+        // Same as above but with an ETag: the revalidation grace period keeps it available so the next
+        // request can send a conditional If-None-Match instead of a full refetch.
+        CacheEntry entry = BuildEntry(expiresAt: DateTimeOffset.UtcNow.AddSeconds(-1), eTag: "\"v1\"");
+
+        store.Set("key1-validator", entry);
+
+        bool found = store.TryGetValue("key1-validator", out _);
+        found.Should().BeTrue("a validator-carrying entry is retained by RevalidationGraceSeconds for conditional revalidation");
+    }
+
+    [Fact]
+    public void Set_UnusableEntry_RemovesPreviousRepresentation()
+    {
+        MemoryCacheStore store = CreateStore();
+        store.Set("superseded", BuildEntry(expiresAt: DateTimeOffset.UtcNow.AddMinutes(5)));
+
+        // Storing an unusable representation must not silently leave the previous one behind.
+        store.Set("superseded", BuildEntry(expiresAt: DateTimeOffset.UtcNow.AddSeconds(-1)));
+
+        bool found = store.TryGetValue("superseded", out _);
+        found.Should().BeFalse("a superseded representation must not keep being served after an unusable store");
     }
 
     [Fact]
@@ -99,11 +126,11 @@ public void Set_LargerStaleWindow_UsedAsDeadline()
     }
 
     [Fact]
-    public void Set_BothStaleWindowsZero_PastExpiry_EntryStaysForConditionalRevalidation()
+    public void Set_BothStaleWindowsZero_PastExpiry_NoValidator_EntryNotRetained()
     {
         MemoryCacheStore store = CreateStore();
-        // Both stale windows are zero and ExpiresAt is in the past → evictionTtl ≤ 0 → no expiration
-        // set on the IMemoryCache entry. Entry must remain for conditional revalidation.
+        // Both stale windows are zero, ExpiresAt is in the past and there is no validator → the entry has
+        // no usable window at all and must not be retained.
         CacheEntry entry = BuildEntry(
             expiresAt: DateTimeOffset.UtcNow.AddSeconds(-1),
             staleIfErrorSeconds: 0,
@@ -112,7 +139,34 @@ public void Set_BothStaleWindowsZero_PastExpiry_EntryStaysForConditionalRevalida
         store.Set("key5", entry);
 
         bool found = store.TryGetValue("key5", out _);
-        found.Should().BeTrue("past-ExpiresAt with no stale windows should stay for conditional revalidation (LRU eviction only)");
+        found.Should().BeFalse("an entry with no freshness, no stale window and no validator must not occupy the cache");
+    }
+
+    [Fact]
+    public void Set_MaxAgeZeroWithoutValidator_NotRetainedEvenWithoutSizeLimit()
+    {
+        // Regression: without MaxCacheSize the IMemoryCache has no SizeLimit and therefore no LRU eviction,
+        // so an entry stored with no expiration would live for the lifetime of the process.
+        MemoryCacheStore store = CreateStore();
+        DateTimeOffset now = DateTimeOffset.UtcNow;
+
+        for (int i = 0; i < 100; i++)
+        {
+            store.Set($"max-age-zero-{i}", new CacheEntry
+            {
+                StatusCode = (int)HttpStatusCode.OK,
+                Body = [1, 2, 3],
+                Headers = new Dictionary<string, string[]>(),
+                ExpiresAt = now,
+                StoredAt = now
+            });
+        }
+
+        for (int i = 0; i < 100; i++)
+        {
+            store.TryGetValue($"max-age-zero-{i}", out _)
+                .Should().BeFalse("max-age=0 responses without a validator must not accumulate in memory");
+        }
     }
 
     // ── Revalidation grace (real clock — IMemoryCache evicts on its own clock)

Stampede.Http.Tests/Caching/UnsafeMethodInvalidationTests.cs

@@ -269,8 +269,99 @@ public async Task UnsafeMethod_OnlyInvalidatesTargetUri_OtherEntriesPreserved()
         usersCalls.Should().Be(1, "/users should still be served from cache");
     }
 
+    // ── Invalidation issues no redundant reads ───────────────────────────────
+
+    [Fact]
+    public async Task Invalidation_DoesNotReadTheEntryBeforeRemovingIt()
+    {
+        // Probing with a read before removing only served to make the log/metric count confirmed
+        // deletions, at the cost of fetching the whole stored body over the network on a distributed
+        // store. Removal is idempotent, so the read must not happen.
+        CountingCacheStore counting = new(new MemoryCacheStore(new MemoryCache(new MemoryCacheOptions())));
+        StubHandler stub = new(_ => new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("body") });
+        CachingMiddleware middleware = new(counting, _keyBuilder, _options) { InnerHandler = stub };
+        HttpMessageInvoker invoker = new(middleware);
+
+        _ = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "https://api.test/probe"), CancellationToken.None);
+
+        counting.Reset();
+
+        HttpRequestMessage post = new(HttpMethod.Post, "https://api.test/probe")
+        {
+            Content = new StringContent("payload")
+        };
+        _ = await invoker.SendAsync(post, CancellationToken.None);
+
+        counting.Removes.Should().Be(1, "the effective request URI must be invalidated");
+        counting.Gets.Should().Be(0, "invalidation must not read the entry it is about to remove");
+    }
+
+    [Fact]
+    public async Task Invalidation_WithLocationAndContentLocation_IssuesOneRemovePerUri()
+    {
+        CountingCacheStore counting = new(new MemoryCacheStore(new MemoryCache(new MemoryCacheOptions())));
+        StubHandler stub = new(req =>
+        {
+            if (req.Method == HttpMethod.Get)
+            {
+                return new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("body") };
+            }
+
+            HttpResponseMessage created = new(HttpStatusCode.Created)
+            {
+                Content = new StringContent("created")
+            };
+            created.Headers.Location = new Uri("https://api.test/items/42");
+            created.Content.Headers.ContentLocation = new Uri("https://api.test/items/canonical");
+            return created;
+        });
+        CachingMiddleware middleware = new(counting, _keyBuilder, _options) { InnerHandler = stub };
+        HttpMessageInvoker invoker = new(middleware);
+
+        counting.Reset();
+
+        HttpRequestMessage post = new(HttpMethod.Post, "https://api.test/items")
+        {
+            Content = new StringContent("payload")
+        };
+        _ = await invoker.SendAsync(post, CancellationToken.None);
+
+        counting.Removes.Should().Be(3, "effective request URI, Location and Content-Location are each invalidated once");
+        counting.Gets.Should().Be(0, "invalidation must not read the entries it is about to remove");
+    }
+
     // ── Helpers ──────────────────────────────────────────────────────────────
 
+    /// <summary>Counts the store operations issued by the middleware.</summary>
+    private sealed class CountingCacheStore(ICacheStore inner) : ICacheStore
+    {
+        private int _gets;
+        private int _removes;
+
+        public int Gets => Volatile.Read(ref _gets);
+        public int Removes => Volatile.Read(ref _removes);
+
+        public void Reset()
+        {
+            Volatile.Write(ref _gets, 0);
+            Volatile.Write(ref _removes, 0);
+        }
+
+        public bool TryGetValue(string key, out CacheEntry? entry)
+        {
+            _ = Interlocked.Increment(ref _gets);
+            return inner.TryGetValue(key, out entry);
+        }
+
+        public void Set(string key, CacheEntry entry) => inner.Set(key, entry);
+
+        public void Remove(string key)
+        {
+            _ = Interlocked.Increment(ref _removes);
+            inner.Remove(key);
+        }
+    }
+
     private CachingMiddleware BuildMiddleware(Func<HttpRequestMessage, HttpResponseMessage> handler)
     {
         StubHandler stub = new(handler);