Skip to content

Key the cache by the configured upstream spelling, not the request's [minor] - #97

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/49-canonical-upstream-key
Oct 6, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/49-canonical-upstream-key

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #49

Before: UpstreamRegistry and RepositoryAllowList matched the upstream key regardless of case. After that, though, route.Upstream was used exactly as the request spelled it. It was the identity for store paths, FetchCoalescer keys, transfer-token binding, lock snapshot and admission keys, and metric labels. A client using /GitHub/… and another using /github/… therefore each filled their own cold cache on Linux. The same object was fetched from upstream twice and stored twice, and the lock snapshots and metrics were split.

After: GitLfsCacheHandler replaces route.Upstream with the key as it is configured, right after resolution. Every handler downstream receives the one spelling, so the two clients share one store directory, one coalescer entry, one lock snapshot and one set of metric labels. Rewritten batch hrefs also use the configured spelling. This is the same approach as GitBranchStateCache f22a1b0.

How: IUpstreamRegistry gains TryResolve(string key, out string canonicalKey, out Uri? baseUrl). It has a default implementation that returns the key unchanged, so existing implementers still compile and behave as before. UpstreamRegistry overrides it to report the configured key, and the existing two-argument TryResolve now delegates to it. The new public overload is why this is tagged [minor].

Tests:

  • ProxyFlowTests.Download_UpstreamKeyCasingDiffers_SharesOneCacheUnderTheConfiguredKey runs a batch and a download through /GitHub/…, then the same object through /github/…. It asserts one upstream fetch, an href under /github/, and the object stored under github. With the handler change reverted it fails: two upstream fetches.
  • UpstreamRegistryTests gains checks that the configured key is reported for a key in different casing, and that an unknown key comes back unchanged.
  • The full suite passes: 348 of 348.

Related: #50, lock-snapshot eviction, now sees one key per upstream rather than one per spelling.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hnw6WDDjwQ8HvxinnWS9CF


Generated by Claude Code

…[minor]

Upstream keys resolve regardless of case, but the request's own spelling
was then used as the identity for store paths, fetch coalescing, transfer
tokens, lock snapshots and metrics. On a case-sensitive filesystem a client
addressing /GitHub/ and another addressing /github/ each filled their own
cold cache and fetched the same object from upstream twice.

IUpstreamRegistry gains a TryResolve overload that reports the key as
configured (a default implementation keeps existing implementers working),
and the handler replaces the route's upstream with it right after
resolution, so everything downstream shares one identity.

Fixes #49

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Hnw6WDDjwQ8HvxinnWS9CF
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 3d4ae8d into main Oct 6, 2026
14 checks passed
@matt-edmondson
matt-edmondson deleted the fix/49-canonical-upstream-key branch October 6, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upstream key casing splits the object cache: the same object via /GitHub/ and /github/ is fetched and stored twice

2 participants