Skip to content

Creating or unlocking a lock does not invalidate the cached lock list when the list was fetched with ?refspec= #46

Description

@matt-edmondson

What's wrong

LockRouteHandler.ListAsync caches lock lists under a key that includes the list request's refspec query parameter:

LockSnapshotKey key = new(route.Upstream, route.RepositoryPath,
    context.Request.Query["refspec"].FirstOrDefault());

After a relayed create (POST /locks) or unlock (POST /locks/:id/unlock), InvalidateIfChanged builds the key to clear from that request's query string:

lockSnapshots.Invalidate(new LockSnapshotKey(route.Upstream, route.RepositoryPath,
    context.Request.Query["refspec"].FirstOrDefault()));

In the Git LFS locking API, create and unlock carry the ref in the JSON body ({"path": ..., "ref": {"name": "refs/heads/main"}}), not in the query string. When the list was fetched with a refspec, it sits under (upstream, repo, "refs/heads/main"), but the change clears (upstream, repo, null), which is a different entry.

The batch fan-out path already gets this right: FanOutAsync builds its key from request.Ref in the body.

Failure scenario

  1. A git-lfs client lists locks with ?refspec=refs/heads/main, and the list is cached.
  2. The same user runs git lfs lock foo.psd. The upstream accepts the lock and the proxy clears the null-refspec key.
  3. git lfs locks is served the stale cached list without the new lock, for up to ListTtl. Unlocking has the same problem: a released lock keeps showing as held.

The existing LockListCachingTests.CreatingALock_InvalidatesTheSnapshot passes only because neither of its requests sends a refspec.

Suggested fix

Do one of the following:

  • In the create and unlock relay, buffer the request body, read ref.name, and invalidate that key.
  • Invalidate every cached snapshot for (upstream, repositoryPath) whatever the refspec. This is simpler and is safe, since invalidation only costs a refetch.

Acceptance criteria: a test lists with ?refspec=refs/heads/main, creates a lock whose body carries the same ref, and asserts that the next list includes the new lock.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

readyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions