fix: canonicalise the repository path so one repository is one mirror [patch]

GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs

@@ -0,0 +1,129 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.GitBranchStateCache.Tests.Integration;
+
+using System.IO.Abstractions;
+using System.Net;
+using System.Net.Http.Headers;
+using System.Text;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// What counts as "the same repository" once a request has been accepted.
+/// </summary>
+/// <remarks>
+/// The allow-list matches case insensitively on purpose, so two callers can address one repository by
+/// two spellings and both be served. Everything derived afterwards — the mirror directory, the
+/// coalescing key, the diff cache key, the admission key — compares ordinally, so unless the path is
+/// canonicalised once at the point it is accepted, one repository quietly becomes two of everything.
+/// That is the whole of this service's purpose inverted: it would do the work twice rather than once.
+/// </remarks>
+[TestClass]
+public class RepositoryIdentityTests
+{
+	private const string ClientBase = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+	private const string MainTip = "cccccccccccccccccccccccccccccccccccccccc";
+	private const string ForkPoint = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee";
+	private const string Credential = "Basic dXNlcjp0b2tlbg==";
+
+	/// <summary>The repository as the allow-list spells it.</summary>
+	private const string ConfiguredSpelling = "/v1/github/studio/game.git/state";
+
+	/// <summary>The same repository as a client that cloned it with different casing spells it.</summary>
+	private const string CallerSpelling = "/v1/github/Studio/Game.git/state";
+
+	private const string Body = $$"""{"base":"{{ClientBase}}","branchPatterns":["origin/main"]}""";
+
+	private static void Seed(ScriptedGit git)
+	{
+		git.Branches["main"] = MainTip;
+		git.Commits.Add(ClientBase);
+		git.MergeBases[$"{ClientBase} {MainTip}"] = ForkPoint;
+		git.Diffs[$"{ForkPoint} {MainTip}"] =
+			":100644 100644 2222222222222222222222222222222222222222 1111111111111111111111111111111111111111 M\0Content/Chars/Bar.uasset\0";
+	}
+
+	private static HttpRequestMessage Request(string url)
+	{
+		HttpRequestMessage request = new(HttpMethod.Post, url)
+		{
+			Content = new StringContent(Body, Encoding.UTF8, "application/json"),
+		};
+
+		request.Headers.Authorization = AuthenticationHeaderValue.Parse(Credential);
+		return request;
+	}
+
+	[TestMethod]
+	public async Task TwoSpellingsOfOneRepository_AreBothServed()
+	{
+		// The premise the rest of this class rests on: the allow-list is case insensitive, so the
+		// casing a client happened to clone with is not a reason to refuse it.
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync();
+		Seed(fixture.Git);
+
+		using HttpResponseMessage configured = await fixture.Client.SendAsync(Request(ConfiguredSpelling));
+		using HttpResponseMessage caller = await fixture.Client.SendAsync(Request(CallerSpelling));
+
+		Assert.AreEqual(HttpStatusCode.OK, configured.StatusCode);
+		Assert.AreEqual(HttpStatusCode.OK, caller.StatusCode);
+	}
+
+	[TestMethod]
+	public async Task TwoSpellingsOfOneRepository_ShareOneMirrorFetchDiffAndAdmission()
+	{
+		// Two callers, one repository, one of everything. Without a canonical path each of these counts
+		// is two: two bare clones of the same repository on the volume, two ls-remote probes of the
+		// same credential, and two computations of the same diff.
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync();
+		Seed(fixture.Git);
+
+		using HttpResponseMessage configured = await fixture.Client.SendAsync(Request(ConfiguredSpelling));
+		using HttpResponseMessage caller = await fixture.Client.SendAsync(Request(CallerSpelling));
+
+		Assert.AreEqual(HttpStatusCode.OK, configured.StatusCode);
+		Assert.AreEqual(HttpStatusCode.OK, caller.StatusCode);
+
+		Assert.AreEqual(1, fixture.Git.CountOf("clone"));
+		Assert.AreEqual(1, fixture.Git.CountOf("ls-remote"));
+		Assert.AreEqual(1, fixture.Git.CountOf("diff-tree"));
+	}
+
+	[TestMethod]
+	public async Task TwoSpellingsOfOneRepository_ProduceOneMirrorDirectory()
+	{
+		// Asserted against the volume rather than against a count of clones, because this is the cost
+		// that persists: the duplicate mirror stays on disk long after the request that made it.
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync();
+		Seed(fixture.Git);
+
+		using HttpResponseMessage configured = await fixture.Client.SendAsync(Request(ConfiguredSpelling));
+		using HttpResponseMessage caller = await fixture.Client.SendAsync(Request(CallerSpelling));
+
+		IDirectory directory = fixture.FileSystem.Directory;
+
+		Assert.HasCount(
+			1,
+			directory.GetDirectories(ServiceFixture.MirrorRoot, "mirror.git", SearchOption.AllDirectories));
+	}
+
+	[TestMethod]
+	public async Task ARequestSpelledDifferentlyFromTheAllowList_MirrorsUnderTheCanonicalPath()
+	{
+		// The canonical form is lower case, so an operator reading the volume sees one directory per
+		// repository whatever casing the clients that asked for it happened to use.
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync();
+		Seed(fixture.Git);
+
+		using HttpResponseMessage response = await fixture.Client.SendAsync(Request(CallerSpelling));
+
+		Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);
+
+		Assert.IsTrue(fixture.FileSystem.Directory.Exists(fixture.FileSystem.Path.Combine(
+			ServiceFixture.MirrorRoot,
+			"github",
+			"studio",
+			"game.git",
+			"mirror.git")));
+	}
+}

GitBranchStateCache/Endpoints/BranchStateHandler.cs

@@ -42,7 +42,7 @@
 /// <param name="metrics">Service counters.</param>
 /// <param name="options">The configured options.</param>
 /// <param name="logger">Logger.</param>
 internal sealed class BranchStateHandler(
 	IUpstreamRegistry registry,
 	IRepositoryAllowList allowList,
 	IMirrorStore mirrors,
@@ -407,7 +407,7 @@
 			return null;
 		}
 
-		MirrorKey key = new(route.Upstream, route.RepositoryPath);
+		MirrorKey key = new(route.Upstream, Canonicalize(route.RepositoryPath));
 
 		if (!mirrors.TryResolve(key, out string? directory)
 			|| !UpstreamUrl.TryCombine(upstreamBase!, route.RepositoryPath, out Uri? repositoryUrl))
@@ -418,6 +418,28 @@
 		return new ResolvedRepository(key, directory!, repositoryUrl!, upstreamBase!);
 	}
 
+	/// <summary>
+	/// Reduces a repository path to the one spelling this service knows it by.
+	/// </summary>
+	/// <remarks>
+	/// This exists because the allow-list immediately above it matches case insensitively, deliberately
+	/// and for good reasons of its own, while every identity derived from the path afterwards compares
+	/// ordinally: the mirror directory on a case-sensitive volume, the coalescing key that keeps
+	/// concurrent work on one repository to a single fetch, the diff cache key built from it, and the
+	/// admission key. Pass the caller's literal spelling on and one repository addressed two ways is
+	/// two mirrors on disk, fetched twice per heartbeat, diffed twice, and probed twice — which is the
+	/// duplication this service exists to remove, reintroduced by a difference the allow-list has
+	/// already ruled irrelevant. So it is canonicalised once, here, at the only point that decides what
+	/// a request is about. Removing this does not simplify anything; it silently doubles the cost of
+	/// every repository whose callers do not agree on casing.
+	/// <para>
+	/// Only this service's own bookkeeping is canonicalised. What is sent to the forge keeps the
+	/// caller's spelling, because the forge is the authority on how it spells its own repository names
+	/// and this service should not be rewriting a URL on its behalf.
+	/// </para>
+	/// </remarks>
+	private static string Canonicalize(string repositoryPath) => repositoryPath.ToLowerInvariant();
+
 	private static bool TryParsePatterns(
 		IReadOnlyList<string>? requested,
 		out List<BranchPattern> patterns,

README.md

@@ -72,6 +72,8 @@ gitbranchstatecache --upstream github=https://github.com --allow github=studio/g
 
 `--allow` is required at least once per upstream and is also repeatable. Unlike `ktsu.GitLfsCache`, there is no pattern meaning every repository: every pattern must name at least one literal path segment. One request for a repository not on the list would clone a permanent mirror of it onto a shared volume, sized by the repository rather than by the request, that nothing ever evicts.
 
+Patterns match case insensitively, because forge repository names are, and a pattern that fails only because someone typed `Studio` is a support ticket rather than a control. An allowed repository path is then reduced to lower case before anything is derived from it, so clients that disagree about casing still share one mirror, one fetch and one cached diff, and the volume holds one directory per repository whatever casing was used to ask for it. What is sent to the forge keeps the caller's spelling.
+
 ### Asking for branch state
 
 ```http