[RyoKusnadi]

Closes #10565

Change Description

Bug Fix

How it happens: cherry-pick a commit that has commit metadata (for example one made with "metadata":{"k":"a"} on the commits API) onto another branch. The server then reads that source commit from its commit cache and changes its metadata in memory, so the original commit's ID stops resolving: GET /repositories/rep/commits/<id> returns commit not found. A commit with no metadata is not affected. Steps are in the issue.

1. Problem - After POST .../branches/main/cherry-pick with a source commit that has metadata, that source commit can no longer be fetched by its ID (commit not found). Passing metadata in the cherry-pick overrides also changes the map the caller passed in.
2. Root cause - Graveler.CherryPick set commit.Metadata = commitRecord.Metadata, or the override map, and then wrote cherry-pick-origin and cherry-pick-committer into it. That is the same map the cached source commit holds (ref.Manager.GetCommit returns one shared *Commit), so the cached source commit gained the two keys.
3. Solution - Copy the map with maps.Clone before adding the two keys, so the new commit gets its own map. I did this at the write site instead of copying when reading the source commit, because the override map has the same problem.

Testing Details

• Added TestGraveler_CherryPickDoesNotModifySourceCommitMetadata: cherry-picks a commit with metadata {"k":"a"} and checks the source commit's metadata is unchanged. Before the fix it fails (the source commit gains cherry-pick-origin and cherry-pick-committer); after, it passes. It uses the in-memory ref manager fake, so it does not touch the real commit cache.
• Two existing tests (TestGraveler_CherryPickHooks, TestGravelerCherryPick/cherry-pick_partially_override_commit_fields) compared the new commit's metadata to the same map they passed in, so they only passed because the map was being changed. They now check the expected keys and that the passed map stays as it was.

Run: go test ./pkg/graveler/..., go vet ./pkg/graveler/, gofmt -l pkg/graveler. Not run: the full repo suite, the system tests, or the curl steps against a live server.

Breaking Change?

No. The new commit has the same metadata as before.

Additional info

Same pattern elsewhere: the merge path (graveler.go, MergeStrategyMetadataKey) also writes into commitParams.Metadata. That map comes from the request, not from a cached commit, so I left it out of this PR.

Contact Details

@RyoKusnadi

[RyoKusnadi]

@nopcoder friendly ping on this one when you have a moment. Happy to change anything.

[nopcoder]

Thanks, @RyoKusnadi. From a quick look, it seems we have the same issue in Import as well. This can be addressed in a separate PR.

I'll be looking into making the required checks pass and will merge this one.

[RyoKusnadi]

Thanks! I'll check the Import path and open a separate PR for it if it's the same bug.

[RyoKusnadi]

@nopcoder Following up on Import: I checked it and it doesn't have the same problem. Graveler.Import takes its metadata from the import request (catalog.go builds it from params.Commit.Metadata), not from a cached commit, so adding the merge-strategy key only touches that request's own map, which nothing reads afterwards. So no follow-up PR from me for this.