Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions GitLfsCache.Tests/Integration/ProxyFlowTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@
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,
};
Expand Down Expand Up @@ -144,6 +145,27 @@
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)));

Check warning on line 159 in GitLfsCache.Tests/Integration/ProxyFlowTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Consider using the overload that accepts a CancellationToken and pass 'TestContext.CancellationToken'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitLfsCache&issues=AaES8svvNs-rcCYFFsrI&open=AaES8svvNs-rcCYFFsrI&pullRequest=97

Check warning on line 159 in GitLfsCache.Tests/Integration/ProxyFlowTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'Assert.AreSequenceEqual' instead of 'CollectionAssert.AreEqual'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitLfsCache&issues=AaES8svvNs-rcCYFFsrG&open=AaES8svvNs-rcCYFFsrG&pullRequest=97

JsonNode configuredBatch = await PostBatchAsync(fixture, "download", oid, content.Length);
CollectionAssert.AreEqual(content, await client.GetByteArrayAsync(Relative(HrefOf(configuredBatch, "download"))));

Check warning on line 162 in GitLfsCache.Tests/Integration/ProxyFlowTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'Assert.AreSequenceEqual' instead of 'CollectionAssert.AreEqual'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitLfsCache&issues=AaES8svvNs-rcCYFFsrH&open=AaES8svvNs-rcCYFFsrH&pullRequest=97

Check warning on line 162 in GitLfsCache.Tests/Integration/ProxyFlowTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Consider using the overload that accepts a CancellationToken and pass 'TestContext.CancellationToken'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitLfsCache&issues=AaES8svvNs-rcCYFFsrJ&open=AaES8svvNs-rcCYFFsrJ&pullRequest=97

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()
{
Expand Down
16 changes: 16 additions & 0 deletions GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
7 changes: 6 additions & 1 deletion GitLfsCache/Endpoints/GitLfsCacheHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions GitLfsCache/Upstreams/IUpstreamRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,23 @@ public interface IUpstreamRegistry
/// <param name="baseUrl">The configured base URL, or null when the key is unknown.</param>
/// <returns><see langword="true"/> when the key is configured.</returns>
public bool TryResolve(string key, out Uri? baseUrl);

/// <summary>
/// Resolves an upstream key and reports the spelling it is configured under.
/// </summary>
/// <remarks>
/// Keys match regardless of case, so <c>/GitHub/</c> and <c>/github/</c> reach the same upstream.
/// Everything keyed by the upstream afterwards (the object store, fetch coalescing, transfer tokens,
/// lock snapshots and metrics) must use <paramref name="canonicalKey"/>, or each spelling gets its
/// own cold cache. The default implementation returns <paramref name="key"/> unchanged.
/// </remarks>
/// <param name="key">The first path segment of the request.</param>
/// <param name="canonicalKey">The key as configured, or <paramref name="key"/> when it is unknown.</param>
/// <param name="baseUrl">The configured base URL, or null when the key is unknown.</param>
/// <returns><see langword="true"/> when the key is configured.</returns>
public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl)
{
canonicalKey = key;
return TryResolve(key, out baseUrl);
}
}
15 changes: 10 additions & 5 deletions GitLfsCache/Upstreams/UpstreamRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,31 @@ namespace ktsu.GitLfsCache.Upstreams;
/// <param name="options">The configured options.</param>
public sealed class UpstreamRegistry(IOptions<GitLfsCacheOptions> options) : IUpstreamRegistry
{
private readonly Dictionary<string, Uri> _upstreams = options.Value.Upstreams
private readonly Dictionary<string, (string Key, Uri BaseUrl)> _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);

/// <inheritdoc />
public bool TryResolve(string key, out Uri? baseUrl)
public bool TryResolve(string key, out Uri? baseUrl) => TryResolve(key, out _, out baseUrl);

/// <inheritdoc />
public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl)
{
canonicalKey = key;
baseUrl = null;

if (string.IsNullOrEmpty(key) || key.Contains('/', StringComparison.Ordinal))
{
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;
}

Expand Down
Loading