PatchSetApproval: Replace copy constructors with instance methods

In a few cases, change the semantics to avoid the premature optimization
of returning the same instance if the patch set already matches. In the
near future in this series, PatchSetApproval will become deeply
immutable, so there will be no functional difference between different
instances.

Change-Id: Iba9e3d6f5fbd7a99ef015144b6550b67a73400be

java/com/google/gerrit/reviewdb/client/PatchSetApproval.java

@@ -75,7 +75,7 @@ public final class PatchSetApproval {
     setGranted(ts);
   }
 
-  public PatchSetApproval(PatchSet.Id psId, PatchSetApproval src) {
+  private PatchSetApproval(PatchSet.Id psId, PatchSetApproval src) {
     key = key(psId, src.getAccountId(), src.getLabelId());
     value = src.getValue();
     granted = src.granted;
@@ -84,8 +84,12 @@ public final class PatchSetApproval {
     postSubmit = src.postSubmit;
   }
 
-  public PatchSetApproval(PatchSetApproval src) {
-    this(src.getPatchSetId(), src);
+  public PatchSetApproval copyWithPatchSet(PatchSet.Id psId) {
+    return new PatchSetApproval(psId, this);
+  }
+
+  public PatchSetApproval copy() {
+    return copyWithPatchSet(getPatchSetId());
   }
 
   public PatchSetApproval.Key getKey() {

java/com/google/gerrit/server/ApprovalCopier.java

@@ -140,7 +140,7 @@ public class ApprovalCopier {
             wontCopy.put(psa.getLabel(), psa.getAccountId(), psa);
             continue;
           }
-          byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.id()));
+          byUser.put(psa.getLabel(), psa.getAccountId(), psa.copyWithPatchSet(ps.id()));
         }
       }
       return labelNormalizer.normalize(notes, byUser.values()).getNormalized();
@@ -186,11 +186,4 @@ public class ApprovalCopier {
         return false;
     }
   }
-
-  private static PatchSetApproval copy(PatchSetApproval src, PatchSet.Id psId) {
-    if (src.getKey().patchSetId().equals(psId)) {
-      return src;
-    }
-    return new PatchSetApproval(psId, src);
-  }
 }

java/com/google/gerrit/server/change/LabelNormalizer.java

@@ -103,7 +103,7 @@ public class LabelNormalizer {
         deleted.add(psa);
         continue;
       }
-      PatchSetApproval copy = copy(psa);
+      PatchSetApproval copy = psa.copy();
       applyTypeFloor(label, copy);
       if (copy.getValue() != psa.getValue()) {
         updated.add(copy);
@@ -114,10 +114,6 @@ public class LabelNormalizer {
     return Result.create(unchanged, updated, deleted);
   }
 
-  private PatchSetApproval copy(PatchSetApproval src) {
-    return new PatchSetApproval(src.getPatchSetId(), src);
-  }
-
   private void applyTypeFloor(LabelType lt, PatchSetApproval a) {
     LabelValue atMin = lt.getMin();
     if (atMin != null && a.getValue() < atMin.getValue()) {

java/com/google/gerrit/server/notedb/ChangeNotes.java

@@ -341,7 +341,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       ImmutableListMultimap.Builder<PatchSet.Id, PatchSetApproval> b =
           ImmutableListMultimap.builder();
       for (Map.Entry<PatchSet.Id, PatchSetApproval> e : state.approvals()) {
-        b.put(e.getKey(), new PatchSetApproval(e.getValue()));
+        b.put(e.getKey(), e.getValue().copy());
       }
       approvals = b.build();
     }

java/com/google/gerrit/server/submit/SubmitStrategyOp.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
 import static java.util.Comparator.comparing;
 import static java.util.Objects.requireNonNull;
 
-import com.google.common.base.Function;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.exceptions.StorageException;
@@ -334,7 +333,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
     // approvals as well.
     if (!newPsId.equals(oldPsId)) {
       saveApprovals(normalized, newPsUpdate, true);
-      submitter = convertPatchSet(newPsId).apply(submitter);
+      submitter = submitter.copyWithPatchSet(newPsId);
     }
   }
 
@@ -383,16 +382,6 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
     }
   }
 
-  private static Function<PatchSetApproval, PatchSetApproval> convertPatchSet(
-      final PatchSet.Id psId) {
-    return psa -> {
-      if (psa.getPatchSetId().equals(psId)) {
-        return psa;
-      }
-      return new PatchSetApproval(psId, psa);
-    };
-  }
-
   private String getByAccountName() {
     requireNonNull(submitter, "getByAccountName called before submitter populated");
     Optional<Account> account =

javatests/com/google/gerrit/server/change/LabelNormalizerTest.java

@@ -193,7 +193,7 @@ public class LabelNormalizerTest {
   }
 
   private PatchSetApproval copy(PatchSetApproval src, int newValue) {
-    PatchSetApproval result = new PatchSetApproval(src.getKey().patchSetId(), src);
+    PatchSetApproval result = src.copy();
     result.setValue((short) newValue);
     return result;
   }