fix(caching): reset StoredAt on 304 revalidation so Age restarts (v2.2.1) - #5
Merged
Merged
Conversation
…2.1) When a stale entry was revalidated and the origin answered 304 Not Modified, the three refresh sites (foreground, background stale-while-revalidate, and HEAD) rebuilt the entry with a new ExpiresAt but kept the original StoredAt, so the Age response header kept growing past the revalidation (max-age=10 reporting Age: 65, 68, 70... on subsequent fresh hits, observed live against Kestrel + Redis). Per RFC 9111 §4.2.3 the age calculation restarts from the validation response, and §4.3.4 requires the stored response's header fields to be updated with those carried on the 304. The three duplicated refresh blocks are collapsed into a shared RefreshFromNotModified helper that resets StoredAt to the revalidation time, merges the 304's header fields (Date, Cache-Control, ETag, ...) into the stored headers, and recomputes freshness metadata as before. This also corrects the memory store's eviction window for refreshed entries, which was inflated by the stale StoredAt (new ExpiresAt - old StoredAt). All six regression tests fail against the pre-fix code and cover the foreground, background, and HEAD paths on both MemoryCacheStore and DistributedCacheStore (JSON round-trip included). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a stale entry was revalidated and the origin answered
304 Not Modified, the entry's freshness TTL was refreshed butStoredAtkept its original value, so theAgeresponse header kept growing past the revalidation. Observed live with the samples against Kestrel + Redis:/catalogwithmax-age=10, stale-if-error=60showedAge: 65at the moment of a successful 304 revalidation, thenAge: 68,70… on subsequent fresh cache hits instead of restarting near 0.Per RFC 9111 §4.2.3 the age calculation restarts from the validation response, and §4.3.4 requires the stored response to be updated with the 304's header fields.
Changes
RefreshFromNotModifiedhelper inCachingMiddlewarereplaces the three duplicated 304-refresh blocks (foregroundRevalidateAsync, background stale-while-revalidate, andHandleHeadAsync). On a 304 it now:StoredAtto the revalidation time, soAgerestarts from 0 (§4.2.3);Date,Cache-Control,ETag, …) into the stored headers (§4.3.4);ExpiresAt/ stale windows /MustRevalidateexactly as before.MemoryCacheStorecomputes its eviction TTL asExpiresAt − StoredAt, so refreshed entries previously got an inflated eviction window (new ExpiresAt − old StoredAt).DistributedCacheStorekeys off absoluteExpiresAtand is unaffected; the resetStoredAtround-trips through its JSON serialization (covered by a test).Tests
New
NotModifiedAgeResetTests(6 tests) covering: Age = 0 on the revalidated response, subsequent hits counting age from the revalidation time, §4.3.4 header updates, background (stale-while-revalidate) revalidation, HEAD-triggered revalidation, and theDistributedCacheStorepath. All six genuinely fail against the pre-fix code (verified by stashing the middleware change: 6/6 failed).Freshness logic runs on the injected
TimeProvider, so these useFakeTimeProvider— per the v2.1.0 lesson, only physical store-eviction timing needs the real clock, which isn't what's under test here. The distributed test seeds the fake clock with real now soMemoryDistributedCache's real-clock absolute expiration doesn't evict the entry.Verification
dotnet build Stampede.Http.slnx --no-incremental: 0 warnings, 0 errorsdotnet test: 349/349 passed (343 existing + 6 new)🤖 Generated with Claude Code