API, Core: Fix naming in fastForwardBranch/replaceBranch APIs (apache…

…#9134)

api/src/main/java/org/apache/iceberg/ManageSnapshots.java

@@ -163,26 +163,26 @@ default ManageSnapshots createBranch(String name) {
   ManageSnapshots replaceBranch(String name, long snapshotId);
 
   /**
-   * Replaces the branch with the given name to point to the source snapshot. The source branch will
-   * remain unchanged, the target branch will retain its retention properties.
+   * Replaces the {@code from} branch to point to the {@code to} snapshot. The {@code to} will
+   * remain unchanged, and {@code from} branch will retain its retention properties.
    *
-   * @param name Branch to replace
-   * @param source Source reference for the target to be replaced with
+   * @param from Branch to replace
+   * @param to The branch {@code from} should be replaced with
    * @return this for method chaining
    */
-  ManageSnapshots replaceBranch(String name, String source);
+  ManageSnapshots replaceBranch(String from, String to);
 
   /**
-   * Performs a fast-forward of the given target branch up to the source snapshot if target is an
-   * ancestor of source. The source branch will remain unchanged, the target branch will retain its
+   * Performs a fast-forward of {@code from} up to the {@code to} snapshot if {@code from} is an
+   * ancestor of {@code to}. The {@code to} will remain unchanged, and {@code from} will retain its
    * retention properties.
    *
-   * @param name Branch to fast-forward
-   * @param source Source reference for the target to be fast forwarded to
+   * @param from Branch to fast-forward
+   * @param to Ref for the {@code from} branch to be fast forwarded to
    * @return this for method chaining
-   * @throws IllegalArgumentException if the target branch is not an ancestor of source
+   * @throws IllegalArgumentException if {@code from} is not an ancestor of {@code to}
    */
-  ManageSnapshots fastForwardBranch(String name, String source);
+  ManageSnapshots fastForwardBranch(String from, String to);
 
   /**
    * Updates the minimum number of snapshots to keep for a branch.

core/src/main/java/org/apache/iceberg/SnapshotManager.java

@@ -137,14 +137,14 @@ public ManageSnapshots replaceBranch(String name, long snapshotId) {
   }
 
   @Override
-  public ManageSnapshots replaceBranch(String name, String source) {
-    updateSnapshotReferencesOperation().replaceBranch(name, source);
+  public ManageSnapshots replaceBranch(String from, String to) {
+    updateSnapshotReferencesOperation().replaceBranch(from, to);
     return this;
   }
 
   @Override
-  public ManageSnapshots fastForwardBranch(String name, String source) {
-    updateSnapshotReferencesOperation().fastForward(name, source);
+  public ManageSnapshots fastForwardBranch(String from, String to) {
+    updateSnapshotReferencesOperation().fastForward(from, to);
     return this;
   }
 

core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java

@@ -106,40 +106,41 @@ public UpdateSnapshotReferencesOperation replaceBranch(String name, long snapsho
     return this;
   }
 
-  public UpdateSnapshotReferencesOperation replaceBranch(String name, String source) {
-    return replaceBranch(name, source, false);
+  public UpdateSnapshotReferencesOperation replaceBranch(String from, String to) {
+    return replaceBranch(from, to, false);
   }
 
-  public UpdateSnapshotReferencesOperation fastForward(String name, String source) {
-    return replaceBranch(name, source, true);
+  public UpdateSnapshotReferencesOperation fastForward(String from, String to) {
+    return replaceBranch(from, to, true);
   }
 
   private UpdateSnapshotReferencesOperation replaceBranch(
-      String name, String source, boolean fastForward) {
-    Preconditions.checkNotNull(name, "Target branch cannot be null");
-    Preconditions.checkNotNull(source, "Source ref cannot be null");
-    SnapshotRef sourceRef = updatedRefs.get(source);
-    SnapshotRef refToUpdate = updatedRefs.get(name);
-    Preconditions.checkArgument(refToUpdate != null, "Target branch does not exist: %s", name);
-    Preconditions.checkArgument(sourceRef != null, "Ref does not exist: %s", source);
-    Preconditions.checkArgument(refToUpdate.isBranch(), "Ref %s is a tag not a branch", name);
+      String from, String to, boolean fastForward) {
+    Preconditions.checkNotNull(from, "Branch to update cannot be null");
+    Preconditions.checkNotNull(to, "Destination ref cannot be null");
+    SnapshotRef branchToUpdate = updatedRefs.get(from);
+    SnapshotRef toRef = updatedRefs.get(to);
+    Preconditions.checkArgument(
+        branchToUpdate != null, "Branch to update does not exist: %s", from);
+    Preconditions.checkArgument(toRef != null, "Ref does not exist: %s", to);
+    Preconditions.checkArgument(branchToUpdate.isBranch(), "Ref %s is a tag not a branch", from);
 
     // Nothing to replace
-    if (sourceRef.snapshotId() == refToUpdate.snapshotId()) {
+    if (toRef.snapshotId() == branchToUpdate.snapshotId()) {
       return this;
     }
 
-    SnapshotRef updatedRef = SnapshotRef.builderFrom(refToUpdate, sourceRef.snapshotId()).build();
+    SnapshotRef updatedRef = SnapshotRef.builderFrom(branchToUpdate, toRef.snapshotId()).build();
 
     if (fastForward) {
       boolean targetIsAncestor =
           SnapshotUtil.isAncestorOf(
-              sourceRef.snapshotId(), refToUpdate.snapshotId(), base::snapshot);
+              toRef.snapshotId(), branchToUpdate.snapshotId(), base::snapshot);
       Preconditions.checkArgument(
-          targetIsAncestor, "Cannot fast-forward: %s is not an ancestor of %s", name, source);
+          targetIsAncestor, "Cannot fast-forward: %s is not an ancestor of %s", from, to);
     }
 
-    updatedRefs.put(name, updatedRef);
+    updatedRefs.put(from, updatedRef);
     return this;
   }
 

core/src/test/java/org/apache/iceberg/TestSnapshotManager.java

@@ -409,15 +409,15 @@ public void testReplaceBranch() {
   }
 
   @Test
-  public void testReplaceBranchNonExistingTargetBranchFails() {
+  public void testReplaceBranchNonExistingBranchToUpdateFails() {
     Assertions.assertThatThrownBy(
             () -> table.manageSnapshots().replaceBranch("non-existing", "other-branch").commit())
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("Target branch does not exist: non-existing");
+        .hasMessage("Branch to update does not exist: non-existing");
   }
 
   @Test
-  public void testReplaceBranchNonExistingSourceFails() {
+  public void testReplaceBranchNonExistingToBranchFails() {
     table.newAppend().appendFile(FILE_A).commit();
     long snapshotId = table.currentSnapshot().snapshotId();
     table.manageSnapshots().createBranch("branch1", snapshotId).commit();
@@ -427,6 +427,26 @@ public void testReplaceBranchNonExistingSourceFails() {
         .hasMessage("Ref does not exist: non-existing");
   }
 
+  @Test
+  public void testFastForwardBranchNonExistingFromBranchFails() {
+    Assertions.assertThatThrownBy(
+            () ->
+                table.manageSnapshots().fastForwardBranch("non-existing", "other-branch").commit())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Branch to update does not exist: non-existing");
+  }
+
+  @Test
+  public void testFastForwardBranchNonExistingToFails() {
+    table.newAppend().appendFile(FILE_A).commit();
+    long snapshotId = table.currentSnapshot().snapshotId();
+    table.manageSnapshots().createBranch("branch1", snapshotId).commit();
+    Assertions.assertThatThrownBy(
+            () -> table.manageSnapshots().fastForwardBranch("branch1", "non-existing").commit())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Ref does not exist: non-existing");
+  }
+
   @Test
   public void testFastForward() {
     table.newAppend().appendFile(FILE_A).commit();
@@ -445,7 +465,7 @@ public void testFastForward() {
   }
 
   @Test
-  public void testFastForwardWhenTargetIsNotAncestorFails() {
+  public void testFastForwardWhenFromIsNotAncestorFails() {
     table.newAppend().appendFile(FILE_A).commit();
 
     table.newAppend().appendFile(FILE_B).set("wap.id", "123456789").stageOnly().commit();