Fix: source ref is not always a branch

projectnessie · Apr 21, 2023 · 08198cf · 08198cf
1 parent 1764e4f
commit 08198cf
Show file tree

Hide file tree

Showing 23 changed files with 81 additions and 74 deletions.
diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java
@@ -568,11 +568,13 @@ public MergeResponse transplantCommitsIntoBranch(
       checkArgument(!hashesToTransplant.isEmpty(), "No hashes given to transplant.");
 
       BranchName targetBranch = BranchName.of(branchName);
+
+      WithHash<NamedRef> namedRefWithHash =
+          namedRefWithHashOrThrow(
+              fromRefName, hashesToTransplant.get(hashesToTransplant.size() - 1));
+
       startAccessCheck()
-          .canViewReference(
-              namedRefWithHashOrThrow(
-                      fromRefName, hashesToTransplant.get(hashesToTransplant.size() - 1))
-                  .getValue())
+          .canViewReference(namedRefWithHash.getValue())
           .canCommitChangeAgainstReference(targetBranch)
           .checkAndThrow();
 
@@ -590,7 +592,7 @@ public MergeResponse transplantCommitsIntoBranch(
       MergeResult<Commit> result =
           getStore()
               .transplant(
-                  BranchName.of(fromRefName),
+                  namedRefWithHash.getValue(),
                   targetBranch,
                   toHash(expectedHash, true),
                   transplants,
@@ -631,15 +633,16 @@ public MergeResponse mergeRefIntoBranch(
       throws NessieNotFoundException, NessieConflictException {
     try {
       BranchName targetBranch = BranchName.of(branchName);
+      WithHash<NamedRef> namedRefWithHash = namedRefWithHashOrThrow(fromRefName, fromHash);
       startAccessCheck()
-          .canViewReference(namedRefWithHashOrThrow(fromRefName, fromHash).getValue())
+          .canViewReference(namedRefWithHash.getValue())
           .canCommitChangeAgainstReference(targetBranch)
           .checkAndThrow();
 
       MergeResult<Commit> result =
           getStore()
               .merge(
-                  BranchName.of(fromRefName),
+                  namedRefWithHash.getValue(),
                   toHash(fromRefName, fromHash),
                   targetBranch,
                   toHash(expectedHash, true),

diff --git a/...pter/src/main/java/org/projectnessie/versioned/persist/adapter/MetadataRewriteParams.java b/...pter/src/main/java/org/projectnessie/versioned/persist/adapter/MetadataRewriteParams.java
@@ -19,14 +19,14 @@
 import org.immutables.value.Value;
 import org.projectnessie.model.ContentKey;
 import org.projectnessie.nessie.relocated.protobuf.ByteString;
-import org.projectnessie.versioned.BranchName;
 import org.projectnessie.versioned.MergeType;
 import org.projectnessie.versioned.MetadataRewriter;
+import org.projectnessie.versioned.NamedRef;
 
 public interface MetadataRewriteParams extends ToBranchParams {
 
-  /** Branch to merge or transplant from. */
-  BranchName getFromBranch();
+  /** Ref to merge or transplant from. */
+  NamedRef getFromRef();
 
   /** Whether to keep the individual commits and do not squash the commits to merge. */
   @Value.Default
@@ -51,7 +51,7 @@ default MergeType getDefaultMergeType() {
 
   @SuppressWarnings({"override", "UnusedReturnValue"})
   interface Builder<B> extends ToBranchParams.Builder<B> {
-    B fromBranch(BranchName fromBranch);
+    B fromRef(NamedRef fromBranch);
 
     B keepIndividualCommits(boolean keepIndividualCommits);
 

diff --git a/.../main/java/org/projectnessie/versioned/persist/nontx/NonTransactionalDatabaseAdapter.java b/.../main/java/org/projectnessie/versioned/persist/nontx/NonTransactionalDatabaseAdapter.java
@@ -242,7 +242,7 @@ public MergeResult<CommitLogEntry> merge(MergeParams mergeParams)
                 ImmutableMergeResult.Builder<CommitLogEntry> mergeResult =
                     MergeResult.<CommitLogEntry>builder()
                         .resultType(ResultType.MERGE)
-                        .sourceBranch(mergeParams.getFromBranch());
+                        .sourceRef(mergeParams.getFromRef());
                 mergeResultHolder.set(mergeResult);
 
                 Hash currentHead = Hash.of(refHead.getHash());
@@ -306,7 +306,7 @@ public MergeResult<CommitLogEntry> transplant(TransplantParams transplantParams)
                 ImmutableMergeResult.Builder<CommitLogEntry> mergeResult =
                     MergeResult.<CommitLogEntry>builder()
                         .resultType(ResultType.TRANSPLANT)
-                        .sourceBranch(transplantParams.getFromBranch());
+                        .sourceRef(transplantParams.getFromRef());
                 mergeResultHolder.set(mergeResult);
 
                 Hash currentHead = Hash.of(refHead.getHash());

diff --git a/...st/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java b/...st/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java
@@ -172,7 +172,7 @@ public CommitResult<Commit> commit(
 
   @Override
   public MergeResult<Commit> transplant(
-      BranchName sourceBranch,
+      NamedRef sourceRef,
       BranchName targetBranch,
       Optional<Hash> referenceHash,
       List<Hash> sequenceToTransplant,
@@ -188,7 +188,7 @@ public MergeResult<Commit> transplant(
       MergeResult<CommitLogEntry> adapterMergeResult =
           databaseAdapter.transplant(
               TransplantParams.builder()
-                  .fromBranch(sourceBranch)
+                  .fromRef(sourceRef)
                   .toBranch(targetBranch)
                   .expectedHead(referenceHash)
                   .sequenceToTransplant(sequenceToTransplant)
@@ -210,7 +210,7 @@ public MergeResult<Commit> transplant(
 
   @Override
   public MergeResult<Commit> merge(
-      BranchName fromBranch,
+      NamedRef fromRef,
       Hash fromHash,
       BranchName toBranch,
       Optional<Hash> expectedHash,
@@ -226,7 +226,7 @@ public MergeResult<Commit> merge(
       MergeResult<CommitLogEntry> adapterMergeResult =
           databaseAdapter.merge(
               MergeParams.builder()
-                  .fromBranch(fromBranch)
+                  .fromRef(fromRef)
                   .toBranch(toBranch)
                   .expectedHead(expectedHash)
                   .mergeFromHash(fromHash)
@@ -279,7 +279,7 @@ private MergeResult<Commit> storeMergeResult(
     ImmutableMergeResult.Builder<Commit> storeResult =
         ImmutableMergeResult.<Commit>builder()
             .resultType(adapterMergeResult.getResultType())
-            .sourceBranch(adapterMergeResult.getSourceBranch())
+            .sourceRef(adapterMergeResult.getSourceRef())
             .targetBranch(adapterMergeResult.getTargetBranch())
             .effectiveTargetHash(adapterMergeResult.getEffectiveTargetHash())
             .commonAncestor(adapterMergeResult.getCommonAncestor())

diff --git a/...persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractEvents.java b/...persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractEvents.java
@@ -298,7 +298,7 @@ public void eventMerge(
         adapter.merge(
             MergeParams.builder()
                 .mergeFromHash(committed)
-                .fromBranch(source)
+                .fromRef(source)
                 .toBranch(main)
                 .updateCommitMetadata(updater)
                 .build());
@@ -375,7 +375,7 @@ public void eventTransplant(
             TransplantParams.builder()
                 .addSequenceToTransplant(committed)
                 .updateCommitMetadata(updater)
-                .fromBranch(source)
+                .fromRef(source)
                 .toBranch(main)
                 .build());
 

diff --git a/...ests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java b/...ests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java
@@ -87,7 +87,7 @@ void merge(int numCommits, boolean keepIndividualCommits) throws Exception {
         numCommits,
         (commitHashes, i) ->
             MergeParams.builder()
-                .fromBranch(sourceBranch)
+                .fromRef(sourceBranch)
                 .updateCommitMetadata(metadataUpdater)
                 .keepIndividualCommits(keepIndividualCommits)
                 .mergeFromHash(commitHashes[i]),
@@ -102,7 +102,7 @@ void merge(int numCommits, boolean keepIndividualCommits) throws Exception {
             () ->
                 databaseAdapter.merge(
                     MergeParams.builder()
-                        .fromBranch(sourceBranch)
+                        .fromRef(sourceBranch)
                         .toBranch(branch2)
                         .mergeFromHash(
                             databaseAdapter.hashOnReference(sourceBranch, Optional.empty()))
@@ -153,7 +153,7 @@ void transplant(int numCommits, boolean keepIndividualCommits) throws Exception
             numCommits,
             (commitHashes, i) ->
                 TransplantParams.builder()
-                    .fromBranch(sourceBranch)
+                    .fromRef(sourceBranch)
                     .updateCommitMetadata(metadataUpdater)
                     .keepIndividualCommits(keepIndividualCommits)
                     .sequenceToTransplant(Arrays.asList(commitHashes).subList(0, i + 1)),
@@ -170,7 +170,7 @@ void transplant(int numCommits, boolean keepIndividualCommits) throws Exception
         databaseAdapter
             .transplant(
                 TransplantParams.builder()
-                    .fromBranch(sourceBranch)
+                    .fromRef(sourceBranch)
                     .toBranch(conflict)
                     .expectedHead(Optional.of(noConflictHead))
                     .addSequenceToTransplant(commits)
@@ -187,7 +187,7 @@ void transplant(int numCommits, boolean keepIndividualCommits) throws Exception
         databaseAdapter
             .transplant(
                 TransplantParams.builder()
-                    .fromBranch(sourceBranch)
+                    .fromRef(sourceBranch)
                     .toBranch(conflict)
                     .addSequenceToTransplant(commits)
                     .updateCommitMetadata(metadataUpdater)
@@ -202,7 +202,7 @@ void transplant(int numCommits, boolean keepIndividualCommits) throws Exception
             () ->
                 databaseAdapter.transplant(
                     TransplantParams.builder()
-                        .fromBranch(sourceBranch)
+                        .fromRef(sourceBranch)
                         .toBranch(conflict)
                         .updateCommitMetadata(metadataUpdater)
                         .keepIndividualCommits(keepIndividualCommits)
@@ -347,7 +347,7 @@ Hash mergeTransplantSuccess(
           .isEqualTo(
               expectedMergeResult
                   .resultType(merge ? ResultType.MERGE : ResultType.TRANSPLANT)
-                  .sourceBranch(source)
+                  .sourceRef(source)
                   .targetBranch(target)
                   .resultantTargetHash(mainHead)
                   .addedCommits(mergeResult.getAddedCommits())
@@ -365,7 +365,7 @@ Hash mergeTransplantSuccess(
           .isEqualTo(
               expectedMergeResult
                   .resultType(merge ? ResultType.MERGE : ResultType.TRANSPLANT)
-                  .sourceBranch(source)
+                  .sourceRef(source)
                   .targetBranch(target)
                   .resultantTargetHash(targetHead)
                   .wasApplied(true)
@@ -469,7 +469,7 @@ private MergeResult<CommitLogEntry> conflictExpectedMergeResult(
         MergeResult.<CommitLogEntry>builder()
             .resultType(merge ? ResultType.MERGE : ResultType.TRANSPLANT)
             .resultantTargetHash(conflictHead)
-            .sourceBranch(BranchName.of("branch"))
+            .sourceRef(BranchName.of("branch"))
             .targetBranch(conflict)
             .effectiveTargetHash(conflictHead)
             .expectedHash(conflictBase)

diff --git a/...ed/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java b/...ed/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java
@@ -247,7 +247,7 @@ public MergeResult<CommitLogEntry> merge(MergeParams mergeParams)
                 ImmutableMergeResult.Builder<CommitLogEntry> mergeResult =
                     MergeResult.<CommitLogEntry>builder()
                         .resultType(ResultType.MERGE)
-                        .sourceBranch(mergeParams.getFromBranch());
+                        .sourceRef(mergeParams.getFromRef());
                 mergeResultHolder.set(mergeResult);
 
                 List<CommitLogEntry> writtenCommits = new ArrayList<>();
@@ -316,7 +316,7 @@ public MergeResult<CommitLogEntry> transplant(TransplantParams transplantParams)
                 ImmutableMergeResult.Builder<CommitLogEntry> mergeResult =
                     MergeResult.<CommitLogEntry>builder()
                         .resultType(ResultType.TRANSPLANT)
-                        .sourceBranch(transplantParams.getFromBranch());
+                        .sourceRef(transplantParams.getFromRef());
 
                 mergeResultHolder.set(mergeResult);
 

diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/EventsVersionStore.java b/versioned/spi/src/main/java/org/projectnessie/versioned/EventsVersionStore.java
@@ -68,7 +68,7 @@ public CommitResult<Commit> commit(
 
   @Override
   public MergeResult<Commit> transplant(
-      BranchName sourceBranch,
+      NamedRef sourceRef,
       BranchName targetBranch,
       Optional<Hash> referenceHash,
       List<Hash> sequenceToTransplant,
@@ -81,7 +81,7 @@ public MergeResult<Commit> transplant(
       throws ReferenceNotFoundException, ReferenceConflictException {
     MergeResult<Commit> result =
         delegate.transplant(
-            sourceBranch,
+            sourceRef,
             targetBranch,
             referenceHash,
             sequenceToTransplant,
@@ -99,7 +99,7 @@ public MergeResult<Commit> transplant(
 
   @Override
   public MergeResult<Commit> merge(
-      BranchName fromBranch,
+      NamedRef fromRef,
       Hash fromHash,
       BranchName toBranch,
       Optional<Hash> expectedHash,
@@ -112,7 +112,7 @@ public MergeResult<Commit> merge(
       throws ReferenceNotFoundException, ReferenceConflictException {
     MergeResult<Commit> result =
         delegate.merge(
-            fromBranch,
+            fromRef,
             fromHash,
             toBranch,
             expectedHash,

diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java b/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java
@@ -48,8 +48,8 @@ default boolean wasSuccessful() {
   @jakarta.annotation.Nullable
   Hash getCommonAncestor();
 
-  /** Name of the source branch. */
-  BranchName getSourceBranch();
+  /** The source ref. */
+  NamedRef getSourceRef();
 
   /** Name of the target branch. */
   BranchName getTargetBranch();

diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/MetricsVersionStore.java b/versioned/spi/src/main/java/org/projectnessie/versioned/MetricsVersionStore.java
@@ -96,7 +96,7 @@ public CommitResult<Commit> commit(
 
   @Override
   public MergeResult<Commit> transplant(
-      BranchName sourceBranch,
+      NamedRef sourceRef,
       BranchName targetBranch,
       Optional<Hash> referenceHash,
       List<Hash> sequenceToTransplant,
@@ -112,7 +112,7 @@ public MergeResult<Commit> transplant(
             "transplant",
             () ->
                 delegate.transplant(
-                    sourceBranch,
+                    sourceRef,
                     targetBranch,
                     referenceHash,
                     sequenceToTransplant,
@@ -126,7 +126,7 @@ public MergeResult<Commit> transplant(
 
   @Override
   public MergeResult<Commit> merge(
-      BranchName fromBranch,
+      NamedRef fromRef,
       Hash fromHash,
       BranchName toBranch,
       Optional<Hash> expectedHash,
@@ -142,7 +142,7 @@ public MergeResult<Commit> merge(
             "merge",
             () ->
                 delegate.merge(
-                    fromBranch,
+                    fromRef,
                     fromHash,
                     toBranch,
                     expectedHash,

diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/TracingVersionStore.java b/versioned/spi/src/main/java/org/projectnessie/versioned/TracingVersionStore.java
@@ -117,7 +117,7 @@ public CommitResult<Commit> commit(
 
   @Override
   public MergeResult<Commit> transplant(
-      BranchName sourceBranch,
+      NamedRef sourceRef,
       BranchName targetBranch,
       Optional<Hash> referenceHash,
       List<Hash> sequenceToTransplant,
@@ -138,7 +138,7 @@ public MergeResult<Commit> transplant(
                         .withTag(TAG_TRANSPLANTS, safeSize(sequenceToTransplant)),
                 () ->
                     delegate.transplant(
-                        sourceBranch,
+                        sourceRef,
                         targetBranch,
                         referenceHash,
                         sequenceToTransplant,
@@ -152,7 +152,7 @@ public MergeResult<Commit> transplant(
 
   @Override
   public MergeResult<Commit> merge(
-      BranchName fromBranch,
+      NamedRef fromRef,
       Hash fromHash,
       BranchName toBranch,
       Optional<Hash> expectedHash,
@@ -173,7 +173,7 @@ public MergeResult<Commit> merge(
                         .withTag(TAG_EXPECTED_HASH, safeToString(expectedHash)),
                 () ->
                     delegate.merge(
-                        fromBranch,
+                        fromRef,
                         fromHash,
                         toBranch,
                         expectedHash,