Identify merge-base instead of common-ancestor (#7035)

Enhances the existing logic to identify the common ancestor to respect merge parents.

Externalizes the logic into a new class `MergeBase`, which combines the logic for the common-ancestor and merge-base use cases.

Currently the logic for a merge operation is to identify the common ancestor and apply the diff between the common ancestor and the "from HEAD" onto the "target HEAD".

This change will use the "best merge base" instead of the "common ancestor".

The ability to perform "merges with individual commits" and "squashed transplants" has been removed. This behavior is the only one available in Nessie V2 API.

The change ensures that "already existing content" does not cause conflicts. When a (merge/transplant) commit wants to "add" the exact same content value that already exists on the target branch, that should not result in a conflict.

Fixes #4562

api/model/src/main/java/org/projectnessie/api/v1/params/BaseMergeTransplant.java

@@ -39,6 +39,7 @@ public interface BaseMergeTransplant {
   @Nullable
   @jakarta.annotation.Nullable
   @JsonInclude(Include.NON_NULL)
+  @Deprecated
   Boolean keepIndividualCommits();
 
   @Nullable

api/model/src/main/java/org/projectnessie/model/CommitMeta.java

@@ -191,6 +191,9 @@ public static CommitMeta fromMessage(String message) {
   }
 
   public interface Builder {
+
+    Builder message(String message);
+
     default ImmutableCommitMeta.Builder author(String author) {
       if (author != null) {
         return addAllAuthors(author);
@@ -234,6 +237,8 @@ default ImmutableCommitMeta.Builder putProperties(String key, String value) {
     ImmutableCommitMeta.Builder putAllProperties(String key, List<String> value);
 
     ImmutableCommitMeta.Builder allProperties(Map<String, ? extends List<String>> entries);
+
+    CommitMeta build();
   }
 
   /**

api/model/src/main/java/org/projectnessie/model/MergeResponse.java

@@ -55,7 +55,14 @@ default boolean wasSuccessful() {
   @jakarta.annotation.Nullable
   String getResultantTargetHash();
 
-  /** Commit-ID of the identified common ancestor, only returned for a merge operation. */
+  /**
+   * Commit-ID of the identified merge base, only returned for a merge operation.
+   *
+   * <p>Note: earlier Nessie versions only supported merging using the common ancestor, so only
+   * considering the direct commit parents (predecessors). Nessie identifies the "nearest"
+   * merge-base singe version 0.61.0 (with the new storage model), and allows "incremental merges".
+   * Since renaming a public API fields is not good practice, this field represents the merge-base.
+   */
   @Nullable
   @jakarta.annotation.Nullable
   String getCommonAncestor();

integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/AbstractNessieSparkSqlExtensionTest.java

@@ -15,6 +15,7 @@
  */
 package org.projectnessie.spark.extensions;
 
+import static java.util.Collections.emptyMap;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -500,6 +501,8 @@ private void mergeReferencesInto(String query)
             .map(SparkCommitLogEntry::relevantFromMerge)
             .get();
 
+    result = ImmutableSparkCommitLogEntry.builder().from(result).properties(emptyMap()).build();
+
     sql(query, refName);
     // here we are skipping commit time as its variable
 

servers/rest-services/src/main/java/org/projectnessie/services/rest/RestTreeResource.java

@@ -15,13 +15,15 @@
  */
 package org.projectnessie.services.rest;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static org.projectnessie.services.impl.RefUtil.toReference;
 import static org.projectnessie.services.spi.TreeService.MAX_COMMIT_LOG_ENTRIES;
 
 import com.fasterxml.jackson.annotation.JsonView;
 import javax.enterprise.context.RequestScoped;
 import javax.inject.Inject;
 import org.projectnessie.api.v1.http.HttpTreeApi;
+import org.projectnessie.api.v1.params.BaseMergeTransplant;
 import org.projectnessie.api.v1.params.CommitLogParams;
 import org.projectnessie.api.v1.params.EntriesParams;
 import org.projectnessie.api.v1.params.GetReferenceParams;
@@ -220,14 +222,14 @@ public void deleteReference(
   public MergeResponse transplantCommitsIntoBranch(
       String branchName, String expectedHash, String message, Transplant transplant)
       throws NessieNotFoundException, NessieConflictException {
+    validateKeepIndividual(transplant, true, "Transplant", "unsquashed");
     return resource()
         .transplantCommitsIntoBranch(
             branchName,
             expectedHash,
             message != null ? CommitMeta.fromMessage(message) : null,
             transplant.getHashesToTransplant(),
             transplant.getFromRefName(),
-            transplant.keepIndividualCommits(),
             transplant.getKeyMergeModes(),
             transplant.getDefaultKeyMergeMode(),
             transplant.isDryRun(),
@@ -239,13 +241,13 @@ public MergeResponse transplantCommitsIntoBranch(
   @Override
   public MergeResponse mergeRefIntoBranch(String branchName, String expectedHash, Merge merge)
       throws NessieNotFoundException, NessieConflictException {
+    validateKeepIndividual(merge, false, "Merge", "squashed");
     return resource()
         .mergeRefIntoBranch(
             branchName,
             expectedHash,
             merge.getFromRefName(),
             merge.getFromHash(),
-            merge.keepIndividualCommits(),
             null,
             merge.getKeyMergeModes(),
             merge.getDefaultKeyMergeMode(),
@@ -254,6 +256,17 @@ public MergeResponse mergeRefIntoBranch(String branchName, String expectedHash,
             merge.isReturnConflictAsResult());
   }
 
+  private void validateKeepIndividual(
+      BaseMergeTransplant base, boolean goodValue, String opsName, String supported) {
+    @SuppressWarnings("deprecation")
+    Boolean keep = base.keepIndividualCommits();
+    checkArgument(
+        keep == null || keep == goodValue,
+        "The parameter 'keepIndividualCommits' is deprecated. %s operations only support %s now.",
+        opsName,
+        supported);
+  }
+
   @JsonView(Views.V1.class)
   @Override
   public Branch commitMultipleOperations(

servers/rest-services/src/main/java/org/projectnessie/services/rest/RestV2TreeResource.java

@@ -356,7 +356,6 @@ public MergeResponse transplantCommitsIntoBranch(String branch, Transplant trans
             meta,
             transplant.getHashesToTransplant(),
             transplant.getFromRefName(),
-            true,
             transplant.getKeyMergeModes(),
             transplant.getDefaultKeyMergeMode(),
             transplant.isDryRun(),
@@ -390,7 +389,6 @@ public MergeResponse mergeRefIntoBranch(String branch, Merge merge)
             ref.hashWithRelativeSpec(),
             merge.getFromRefName(),
             merge.getFromHash(),
-            false,
             meta.build(),
             merge.getKeyMergeModes(),
             merge.getDefaultKeyMergeMode(),

servers/services/src/main/java/org/projectnessie/services/impl/BaseApiImpl.java

@@ -48,6 +48,7 @@
 import org.projectnessie.services.authz.BatchAccessChecker;
 import org.projectnessie.services.authz.ServerAccessContext;
 import org.projectnessie.services.config.ServerConfig;
+import org.projectnessie.versioned.DefaultMetadataRewriter;
 import org.projectnessie.versioned.DetachedRef;
 import org.projectnessie.versioned.GetNamedRefsParams;
 import org.projectnessie.versioned.Hash;
@@ -201,6 +202,6 @@ protected MetadataRewriter<CommitMeta> commitMetaUpdate(
       IntFunction<String> squashMessage) {
     Principal principal = getPrincipal();
     String committer = principal == null ? "" : principal.getName();
-    return new CommitMetaUpdater(committer, Instant.now(), commitMeta, squashMessage);
+    return new DefaultMetadataRewriter(committer, Instant.now(), commitMeta, squashMessage);
   }
 }

servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java

@@ -566,7 +566,6 @@ public MergeResponse transplantCommitsIntoBranch(
       @Nullable @jakarta.annotation.Nullable CommitMeta commitMeta,
       List<String> hashesToTransplant,
       String fromRefName,
-      Boolean keepIndividualCommits,
       Collection<MergeKeyBehavior> keyMergeBehaviors,
       MergeBehavior defaultMergeBehavior,
       Boolean dryRun,
@@ -592,7 +591,7 @@ public MergeResponse transplantCommitsIntoBranch(
         transplants = s.collect(Collectors.toList());
       }
 
-      if (Boolean.TRUE.equals(keepIndividualCommits) && transplants.size() > 1) {
+      if (transplants.size() > 1) {
         // Message overrides are not meaningful when transplanting more than one commit.
         // This matches old behaviour where `message` was ignored in all cases.
         commitMeta = null;
@@ -619,7 +618,6 @@ public MergeResponse transplantCommitsIntoBranch(
                                       lastHash,
                                       branchName,
                                       into.map(h -> " at " + h.asString()).orElse(""))))
-                      .keepIndividualCommits(Boolean.TRUE.equals(keepIndividualCommits))
                       .mergeKeyBehaviors(keyMergeBehaviors(keyMergeBehaviors))
                       .defaultMergeBehavior(defaultMergeBehavior(defaultMergeBehavior))
                       .dryRun(Boolean.TRUE.equals(dryRun))
@@ -647,7 +645,6 @@ public MergeResponse mergeRefIntoBranch(
       String expectedHash,
       String fromRefName,
       String fromHash,
-      Boolean keepIndividualCommits,
       @Nullable @jakarta.annotation.Nullable CommitMeta commitMeta,
       Collection<MergeKeyBehavior> keyMergeBehaviors,
       MergeBehavior defaultMergeBehavior,
@@ -680,14 +677,13 @@ public MergeResponse mergeRefIntoBranch(
                           commitMetaUpdate(
                               commitMeta,
                               numCommits ->
+                                  // numCommits is always 1 for merges
                                   String.format(
-                                      "Merged %d commits from %s at %s into %s%s",
-                                      numCommits,
+                                      "Merged %s at %s into %s%s",
                                       fromRefName,
                                       from.asString(),
                                       branchName,
                                       into.map(h -> " at " + h.asString()).orElse(""))))
-                      .keepIndividualCommits(Boolean.TRUE.equals(keepIndividualCommits))
                       .mergeKeyBehaviors(keyMergeBehaviors(keyMergeBehaviors))
                       .defaultMergeBehavior(defaultMergeBehavior(defaultMergeBehavior))
                       .dryRun(Boolean.TRUE.equals(dryRun))

servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java

@@ -214,7 +214,6 @@ MergeResponse transplantCommitsIntoBranch(
               regexp = REF_NAME_REGEX,
               message = REF_NAME_MESSAGE)
           String fromRefName,
-      Boolean keepIndividualCommits,
       Collection<MergeKeyBehavior> keyMergeBehaviors,
       MergeBehavior defaultMergeType,
       @Nullable @jakarta.annotation.Nullable Boolean dryRun,
@@ -259,7 +258,6 @@ MergeResponse mergeRefIntoBranch(
           @Pattern(regexp = HASH_REGEX, message = HASH_MESSAGE)
           @jakarta.validation.constraints.Pattern(regexp = HASH_REGEX, message = HASH_MESSAGE)
           String fromHash,
-      @Nullable @jakarta.annotation.Nullable Boolean keepIndividualCommits,
       @Nullable @jakarta.annotation.Nullable CommitMeta commitMeta,
       Collection<MergeKeyBehavior> keyMergeBehaviors,
       MergeBehavior defaultMergeType,

servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestAccessChecks.java

@@ -103,7 +103,7 @@ public Map<Check, String> check() {
 
   @Test
   public void commitMergeTransplantAccessChecks() throws BaseNessieClientServerException {
-    assumeThat(persist).isNotNull();
+    assumeThat(databaseAdapter).isNull();
 
     ContentKey keyUnrelated = ContentKey.of("unrelated");
     ContentKey keyNamespace1 = ContentKey.of("ns1");
@@ -209,7 +209,6 @@ public void commitMergeTransplantAccessChecks() throws BaseNessieClientServerExc
             common.getHash(),
             source.getName(),
             source.getHash(),
-            false,
             null,
             emptyList(),
             NORMAL,
@@ -237,7 +236,6 @@ public void commitMergeTransplantAccessChecks() throws BaseNessieClientServerExc
             null,
             asList(source1.getHash(), source.getHash()),
             source1.getName(),
-            true,
             emptyList(),
             NORMAL,
             false,
@@ -373,7 +371,6 @@ public Map<Check, String> check() {
                           merge.getHash(),
                           ref.getName(),
                           ref.getHash(),
-                          false,
                           null,
                           emptyList(),
                           NORMAL,
@@ -392,7 +389,6 @@ public Map<Check, String> check() {
                           null,
                           singletonList(ref.getHash()),
                           ref.getName(),
-                          false,
                           emptyList(),
                           NORMAL,
                           false,