diff --git a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs index dcc58e5..76c30d8 100644 --- a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs +++ b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs @@ -33,13 +33,14 @@ private static async Task PostBatchAsync( string operation, string oid, long size, - string? authorization = "Basic dXNlcjp0b2tlbg==") + string? authorization = "Basic dXNlcjp0b2tlbg==", + string lfsPath = LfsPath) { using HttpClient client = fixture.Client; using StringContent body = BatchRequest(operation, oid, size); - using HttpRequestMessage request = new(HttpMethod.Post, $"{LfsPath}/objects/batch") + using HttpRequestMessage request = new(HttpMethod.Post, $"{lfsPath}/objects/batch") { Content = body, }; @@ -144,6 +145,27 @@ public async Task Download_ColdThenWarm_FetchesUpstreamOnceAndServesFromTheStore Assert.AreEqual(1, fixture.Upstream.FetchCount(oid)); } + [TestMethod] + public async Task Download_UpstreamKeyCasingDiffers_SharesOneCacheUnderTheConfiguredKey() + { + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + (byte[] content, string oid) = Object("one object, two spellings"); + fixture.Upstream.AddObject(oid, content); + using HttpClient client = fixture.Client; + + // The fixture configures the upstream as "github"; this client addresses it as "GitHub". + JsonNode mixedCaseBatch = await PostBatchAsync(fixture, "download", oid, content.Length, lfsPath: "/GitHub/owner/repo.git/info/lfs"); + string mixedCaseHref = HrefOf(mixedCaseBatch, "download"); + CollectionAssert.AreEqual(content, await client.GetByteArrayAsync(Relative(mixedCaseHref))); + + JsonNode configuredBatch = await PostBatchAsync(fixture, "download", oid, content.Length); + CollectionAssert.AreEqual(content, await client.GetByteArrayAsync(Relative(HrefOf(configuredBatch, "download")))); + + Assert.AreEqual(1, fixture.Upstream.FetchCount(oid), "The second spelling should be served from the first spelling's cache"); + Assert.StartsWith($"https://cache.example/github/owner/repo.git/info/lfs/objects/{oid}?t=", mixedCaseHref); + Assert.IsTrue(fixture.Store.Exists("github", oid)); + } + [TestMethod] public async Task Download_NeverShortCircuitsTheBatchCall() { diff --git a/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs b/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs index 87dde8f..f98fbb0 100644 --- a/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs +++ b/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs @@ -32,6 +32,22 @@ public void TryResolve_KeyCasingDiffers_StillResolves() Assert.AreEqual(new Uri("https://github.com"), baseUrl); } + [TestMethod] + public void TryResolve_KeyCasingDiffers_ReportsTheConfiguredKey() + { + Assert.IsTrue(Registry().TryResolve("GitHub", out string canonicalKey, out Uri? baseUrl)); + Assert.AreEqual("github", canonicalKey); + Assert.AreEqual(new Uri("https://github.com"), baseUrl); + } + + [TestMethod] + public void TryResolve_UnknownKey_ReportsTheKeyUnchanged() + { + Assert.IsFalse(Registry().TryResolve("GitLab", out string canonicalKey, out Uri? baseUrl)); + Assert.AreEqual("GitLab", canonicalKey); + Assert.IsNull(baseUrl); + } + [TestMethod] public void TryResolve_UnknownKey_ReturnsFalseAndNull() { diff --git a/GitLfsCache/Endpoints/GitLfsCacheHandler.cs b/GitLfsCache/Endpoints/GitLfsCacheHandler.cs index 01f7067..7b0ddbb 100644 --- a/GitLfsCache/Endpoints/GitLfsCacheHandler.cs +++ b/GitLfsCache/Endpoints/GitLfsCacheHandler.cs @@ -49,13 +49,18 @@ public async Task HandleAsync(HttpContext context) return; } - if (!registry.TryResolve(route.Upstream, out Uri? resolved) || resolved is null) + if (!registry.TryResolve(route.Upstream, out string canonicalKey, out Uri? resolved) || resolved is null) { EndpointLog.UnknownUpstream(logger, route.Upstream); context.Response.StatusCode = StatusCodes.Status404NotFound; return; } + // The key matched regardless of case, so from here on it is the configured spelling. Everything + // downstream (store paths, fetch coalescing, transfer tokens, lock snapshots and metrics) is + // keyed by it, and /GitHub/ and /github/ must share one cache rather than each fill their own. + route = route with { Upstream = canonicalKey }; + // Checked here, once, rather than in each branch below, so batch, transfer, verify and relay // are all covered and any route added later inherits it. Before any upstream call, so a // refused path costs upstream nothing and cannot be used to learn anything about it. A 404 diff --git a/GitLfsCache/Upstreams/IUpstreamRegistry.cs b/GitLfsCache/Upstreams/IUpstreamRegistry.cs index 3c73d15..96876a5 100644 --- a/GitLfsCache/Upstreams/IUpstreamRegistry.cs +++ b/GitLfsCache/Upstreams/IUpstreamRegistry.cs @@ -14,4 +14,23 @@ public interface IUpstreamRegistry /// The configured base URL, or null when the key is unknown. /// when the key is configured. public bool TryResolve(string key, out Uri? baseUrl); + + /// + /// Resolves an upstream key and reports the spelling it is configured under. + /// + /// + /// Keys match regardless of case, so /GitHub/ and /github/ reach the same upstream. + /// Everything keyed by the upstream afterwards (the object store, fetch coalescing, transfer tokens, + /// lock snapshots and metrics) must use , or each spelling gets its + /// own cold cache. The default implementation returns unchanged. + /// + /// The first path segment of the request. + /// The key as configured, or when it is unknown. + /// The configured base URL, or null when the key is unknown. + /// when the key is configured. + public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl) + { + canonicalKey = key; + return TryResolve(key, out baseUrl); + } } diff --git a/GitLfsCache/Upstreams/UpstreamRegistry.cs b/GitLfsCache/Upstreams/UpstreamRegistry.cs index 72e9dc6..5a54fe2 100644 --- a/GitLfsCache/Upstreams/UpstreamRegistry.cs +++ b/GitLfsCache/Upstreams/UpstreamRegistry.cs @@ -16,16 +16,20 @@ namespace ktsu.GitLfsCache.Upstreams; /// The configured options. public sealed class UpstreamRegistry(IOptions options) : IUpstreamRegistry { - private readonly Dictionary _upstreams = options.Value.Upstreams + private readonly Dictionary _upstreams = options.Value.Upstreams .Where(pair => pair.Value.BaseUrl is not null) .ToDictionary( pair => pair.Key, - pair => pair.Value.BaseUrl!, + pair => (pair.Key, pair.Value.BaseUrl!), StringComparer.OrdinalIgnoreCase); /// - public bool TryResolve(string key, out Uri? baseUrl) + public bool TryResolve(string key, out Uri? baseUrl) => TryResolve(key, out _, out baseUrl); + + /// + public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl) { + canonicalKey = key; baseUrl = null; if (string.IsNullOrEmpty(key) || key.Contains('/', StringComparison.Ordinal)) @@ -33,9 +37,10 @@ public bool TryResolve(string key, out Uri? baseUrl) return false; } - if (_upstreams.TryGetValue(key, out Uri? resolved)) + if (_upstreams.TryGetValue(key, out (string Key, Uri BaseUrl) resolved)) { - baseUrl = resolved; + canonicalKey = resolved.Key; + baseUrl = resolved.BaseUrl; return true; }