diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs b/GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs index ff8239b..48fe136 100644 --- a/GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs +++ b/GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs @@ -12,11 +12,12 @@ namespace ktsu.GitBranchStateCache.Tests.Integration; /// What counts as "the same repository" once a request has been accepted. /// /// -/// 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. +/// The upstream registry and the allow-list both match case insensitively on purpose, so two callers +/// can address one repository by two spellings — of the repository path, of the upstream key, or of +/// both — 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 key 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. /// [TestClass] public class RepositoryIdentityTests @@ -32,6 +33,12 @@ public class RepositoryIdentityTests /// The same repository as a client that cloned it with different casing spells it. private const string CallerSpelling = "/v1/github/Studio/Game.git/state"; + /// + /// The same repository again, reached through the upstream key spelled the way a forge brands + /// itself rather than the way the configuration keys it. + /// + private const string UpstreamCallerSpelling = "/v1/GitHub/studio/game.git/state"; + private const string Body = $$"""{"base":"{{ClientBase}}","branchPatterns":["origin/main"]}"""; private static void Seed(ScriptedGit git) @@ -126,4 +133,77 @@ public async Task ARequestSpelledDifferentlyFromTheAllowList_MirrorsUnderTheCano "game.git", "mirror.git"))); } + + [TestMethod] + public async Task TwoSpellingsOfOneUpstream_AreBothServed() + { + // The premise for the upstream key, matching the one above for the repository path: the registry + // and the allow-list both resolve it case insensitively, so a caller that writes the forge's own + // branding rather than the configuration key is still addressing the configured upstream. + 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(UpstreamCallerSpelling)); + + Assert.AreEqual(HttpStatusCode.OK, configured.StatusCode); + Assert.AreEqual(HttpStatusCode.OK, caller.StatusCode); + } + + [TestMethod] + public async Task TwoSpellingsOfOneUpstream_ShareOneMirrorFetchDiffAndAdmission() + { + // The same duplication the repository-path case guards against, reached through the sibling half + // of the key instead. One repository, so one clone, one credential probe, and one 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(UpstreamCallerSpelling)); + + 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 TwoSpellingsOfOneUpstream_ProduceOneMirrorDirectory() + { + // Asserted against the volume, because an upstream key that is not canonicalised becomes a + // second top-level directory holding a complete duplicate of every repository under 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(UpstreamCallerSpelling)); + + IDirectory directory = fixture.FileSystem.Directory; + + Assert.HasCount( + 1, + directory.GetDirectories(ServiceFixture.MirrorRoot, "mirror.git", SearchOption.AllDirectories)); + } + + [TestMethod] + public async Task ARequestWithTheUpstreamSpelledDifferently_MirrorsUnderTheCanonicalUpstream() + { + // The canonical form is lower case for the upstream key as much as for the path, so the volume + // carries one directory per configured upstream whatever casing reached the service. + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + + using HttpResponseMessage response = await fixture.Client.SendAsync(Request(UpstreamCallerSpelling)); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + + Assert.IsTrue(fixture.FileSystem.Directory.Exists(fixture.FileSystem.Path.Combine( + ServiceFixture.MirrorRoot, + "github", + "studio", + "game.git", + "mirror.git"))); + } } diff --git a/GitBranchStateCache/Endpoints/BranchStateHandler.cs b/GitBranchStateCache/Endpoints/BranchStateHandler.cs index 90ce8e8..1b7599b 100644 --- a/GitBranchStateCache/Endpoints/BranchStateHandler.cs +++ b/GitBranchStateCache/Endpoints/BranchStateHandler.cs @@ -407,7 +407,7 @@ .. context.Request.Query["pattern"] return null; } - MirrorKey key = new(route.Upstream, Canonicalize(route.RepositoryPath)); + MirrorKey key = new(Canonicalize(route.Upstream), Canonicalize(route.RepositoryPath)); if (!mirrors.TryResolve(key, out string? directory) || !UpstreamUrl.TryCombine(upstreamBase!, route.RepositoryPath, out Uri? repositoryUrl)) @@ -419,26 +419,37 @@ .. context.Request.Query["pattern"] } /// - /// Reduces a repository path to the one spelling this service knows it by. + /// Reduces one half of a — the upstream key or the repository path — to the + /// one spelling this service knows it by. /// /// - /// 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. + /// This exists because the upstream resolution and the allow-list immediately above it both match + /// case insensitively, deliberately and for good reasons of their own, while every identity derived + /// from the key 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 + /// checks above have 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. + /// + /// Both halves of the key go through this, and for the same reason. Canonicalising only the + /// repository path leaves the identical duplication reachable through the upstream key instead, + /// because and + /// accept GitHub and github as one configured upstream while everything downstream of + /// them tells the two apart. + /// /// /// 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. + /// and this service should not be rewriting a URL on its behalf. The upstream key never reaches the + /// forge at all: the base URL comes from configuration, not from the caller's segment. /// /// - private static string Canonicalize(string repositoryPath) => repositoryPath.ToLowerInvariant(); + /// The upstream key or repository path as the caller spelled it. + /// The spelling this service records the request under. + private static string Canonicalize(string segment) => segment.ToLowerInvariant(); private static bool TryParsePatterns( IReadOnlyList? requested,