From bda3cc6aa862f2fbe6e05bb54c033368a6c04cc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 09:26:10 +0000 Subject: [PATCH 1/2] Invalidate every ref's lock snapshot when a relayed lock changes A lock listing is cached under the ?refspec= it was fetched with, but a create or unlock carries its ref in the JSON body, so LockRouteHandler invalidated the null-ref key and left the refspec-scoped listing stale for up to ListTtl: a new lock missing, a released lock still shown. ILockSnapshotStore.Invalidate now takes the upstream and repository and drops every snapshot for that repository whatever its ref. Both the relayed create/unlock and the batch fan-out use it, which also covers a client that lists under one ref and locks under another. The only cost is a refetch. Fixes #46 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016wsoxnwaqMuAzvkm2xnzYh --- .../Integration/LockListCachingTests.cs | 37 +++++++++++++++++++ GitLfsCache/Endpoints/LockRouteHandler.cs | 5 +-- GitLfsCache/Locks/ILockSnapshotStore.cs | 14 +++++-- GitLfsCache/Locks/LockFanOut.cs | 2 +- GitLfsCache/Locks/LockSnapshotStore.cs | 15 ++++++-- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/GitLfsCache.Tests/Integration/LockListCachingTests.cs b/GitLfsCache.Tests/Integration/LockListCachingTests.cs index 01e30c6..a4987d8 100644 --- a/GitLfsCache.Tests/Integration/LockListCachingTests.cs +++ b/GitLfsCache.Tests/Integration/LockListCachingTests.cs @@ -215,6 +215,43 @@ public async Task CreatingALock_InvalidatesTheSnapshot() Assert.HasCount(2, (await ListLocksAsync(fixture))["locks"]!.AsArray()); } + [TestMethod] + [DataRow("/locks", "{\"path\":\"b\",\"ref\":{\"name\":\"refs/heads/main\"}}", 2, DisplayName = "Create")] + [DataRow("/locks/1/unlock", "{\"ref\":{\"name\":\"refs/heads/main\"}}", 0, DisplayName = "Unlock")] + public async Task ChangingALock_InvalidatesASnapshotListedUnderARefspec(string change, string body, int expected) + { + // git-lfs lists with ?refspec= but carries the ref of a create or unlock in the body, so the + // change never names the snapshot the listing was cached under. + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + fixture.Upstream.Locks.Add("a"); + const string Refspec = "?refspec=refs/heads/main"; + + Assert.HasCount(1, (await ListLocksAsync(fixture, Refspec))["locks"]!.AsArray()); + + if (expected > 1) + { + fixture.Upstream.Locks.Add("b"); + } + else + { + fixture.Upstream.Locks.Clear(); + } + + using (HttpClient client = fixture.Client) + { + using HttpRequestMessage request = new(HttpMethod.Post, $"{LfsPath}{change}") + { + Content = new StringContent(body, Encoding.UTF8, "application/vnd.git-lfs+json"), + }; + + request.Headers.TryAddWithoutValidation("Authorization", Credential); + using HttpResponseMessage response = await client.SendAsync(request); + Assert.IsTrue(response.IsSuccessStatusCode, $"{(int)response.StatusCode}"); + } + + Assert.HasCount(expected, (await ListLocksAsync(fixture, Refspec))["locks"]!.AsArray()); + } + [TestMethod] public async Task LocksDisabled_RelaysExactlyAsBefore() { diff --git a/GitLfsCache/Endpoints/LockRouteHandler.cs b/GitLfsCache/Endpoints/LockRouteHandler.cs index daefcbd..a6c8d06 100644 --- a/GitLfsCache/Endpoints/LockRouteHandler.cs +++ b/GitLfsCache/Endpoints/LockRouteHandler.cs @@ -231,10 +231,7 @@ private void InvalidateIfChanged(HttpContext context, LfsRoute route) { if (context.Response.StatusCode is >= 200 and < 300) { - lockSnapshots.Invalidate(new LockSnapshotKey( - route.Upstream, - route.RepositoryPath, - context.Request.Query["refspec"].FirstOrDefault())); + lockSnapshots.Invalidate(route.Upstream, route.RepositoryPath); } } } diff --git a/GitLfsCache/Locks/ILockSnapshotStore.cs b/GitLfsCache/Locks/ILockSnapshotStore.cs index cdb5ad1..d97b88c 100644 --- a/GitLfsCache/Locks/ILockSnapshotStore.cs +++ b/GitLfsCache/Locks/ILockSnapshotStore.cs @@ -26,12 +26,20 @@ public interface ILockSnapshotStore public void Publish(LockSnapshotKey key, LockSnapshot snapshot); /// - /// Drops the snapshot for a repository, so the next read refreshes. + /// Drops every snapshot for a repository, whatever ref it was listed under, so the next read of + /// any of them refreshes. /// /// /// Called after a lock creation or release the proxy relayed successfully. Locks changed outside /// the proxy are not seen here and are bounded only by the listing lifetime. + /// + /// Every ref goes, not only the one the change named. A listing carries its ref in the query + /// string while a create or unlock carries it in the body, and a client can list under one ref + /// and lock under another, so the change cannot reliably name the snapshot it made wrong. Dropping + /// the rest only costs a refetch. + /// /// - /// The repository whose snapshot is now known to be wrong. - public void Invalidate(LockSnapshotKey key); + /// The upstream the repository is served from. + /// The repository whose snapshots are now known to be wrong. + public void Invalidate(string upstream, string repositoryPath); } diff --git a/GitLfsCache/Locks/LockFanOut.cs b/GitLfsCache/Locks/LockFanOut.cs index 15b1f12..a5727bc 100644 --- a/GitLfsCache/Locks/LockFanOut.cs +++ b/GitLfsCache/Locks/LockFanOut.cs @@ -86,7 +86,7 @@ await Parallel.ForAsync( // is about to look at them. if (results.Any(result => JsonValues.Bool(result?["ok"]) == true)) { - snapshots.Invalidate(key); + snapshots.Invalidate(key.Upstream, key.RepositoryPath); } JsonArray array = []; diff --git a/GitLfsCache/Locks/LockSnapshotStore.cs b/GitLfsCache/Locks/LockSnapshotStore.cs index 03777fa..101d383 100644 --- a/GitLfsCache/Locks/LockSnapshotStore.cs +++ b/GitLfsCache/Locks/LockSnapshotStore.cs @@ -33,9 +33,18 @@ public void Publish(LockSnapshotKey key, LockSnapshot snapshot) } /// - public void Invalidate(LockSnapshotKey key) + public void Invalidate(string upstream, string repositoryPath) { - Ensure.NotNull(key); - _snapshots.TryRemove(key, out _); + Ensure.NotNull(upstream); + Ensure.NotNull(repositoryPath); + + foreach (LockSnapshotKey key in _snapshots.Keys) + { + if (string.Equals(key.Upstream, upstream, StringComparison.Ordinal) + && string.Equals(key.RepositoryPath, repositoryPath, StringComparison.Ordinal)) + { + _snapshots.TryRemove(key, out _); + } + } } } From 8b34252a678a5ff97c4113baf08ee71a178a69fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 09:30:49 +0000 Subject: [PATCH 2/2] Filter the invalidated keys with Where Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016wsoxnwaqMuAzvkm2xnzYh --- GitLfsCache/Locks/LockSnapshotStore.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/GitLfsCache/Locks/LockSnapshotStore.cs b/GitLfsCache/Locks/LockSnapshotStore.cs index 101d383..f6fd868 100644 --- a/GitLfsCache/Locks/LockSnapshotStore.cs +++ b/GitLfsCache/Locks/LockSnapshotStore.cs @@ -38,13 +38,11 @@ public void Invalidate(string upstream, string repositoryPath) Ensure.NotNull(upstream); Ensure.NotNull(repositoryPath); - foreach (LockSnapshotKey key in _snapshots.Keys) + foreach (LockSnapshotKey key in _snapshots.Keys.Where(key => + string.Equals(key.Upstream, upstream, StringComparison.Ordinal) + && string.Equals(key.RepositoryPath, repositoryPath, StringComparison.Ordinal))) { - if (string.Equals(key.Upstream, upstream, StringComparison.Ordinal) - && string.Equals(key.RepositoryPath, repositoryPath, StringComparison.Ordinal)) - { - _snapshots.TryRemove(key, out _); - } + _snapshots.TryRemove(key, out _); } } }