From 55b95261cfe591ef74ce67f5a4cbb6ca91393a0e Mon Sep 17 00:00:00 2001
From: Nasser Grainawi <nasser.grainawi@linaro.org>
Date: Mon, 26 Feb 2024 16:05:07 -0800
Subject: Consistently normalize PatchSetApprovals when persisting

Comparing an unnormalized psa with a normalized psa can create
unexpected differences, leading to error messages during the
copy-approvals command like:
java.lang.IllegalArgumentException: Approval that should be copied is not copied.

Fix those situations by applying normalization to the current approvals
before comparing them to the inferred approvals (which already have
normalization applied).

Release-Notes: Fix copy-approvals for cases where current approvals have different values when normalized
Change-Id: I17cccf537943654f89a4d02af13ec2dda768f0df
---
 java/com/google/gerrit/server/approval/ApprovalsUtil.java | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/java/com/google/gerrit/server/approval/ApprovalsUtil.java b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
index 870485a5a3..7744f49a66 100644
--- a/java/com/google/gerrit/server/approval/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
@@ -45,6 +45,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.ReviewerStatusUpdate;
+import com.google.gerrit.server.change.LabelNormalizer;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -102,6 +103,7 @@ public class ApprovalsUtil {
   private final PermissionBackend permissionBackend;
   private final ProjectCache projectCache;
   private final ApprovalCache approvalCache;
+  private final LabelNormalizer labelNormalizer;
 
   @VisibleForTesting
   @Inject
@@ -109,11 +111,13 @@ public class ApprovalsUtil {
       ApprovalInference approvalInference,
       PermissionBackend permissionBackend,
       ProjectCache projectCache,
-      ApprovalCache approvalCache) {
+      ApprovalCache approvalCache,
+      LabelNormalizer labelNormalizer) {
     this.approvalInference = approvalInference;
     this.permissionBackend = permissionBackend;
     this.projectCache = projectCache;
     this.approvalCache = approvalCache;
+    this.labelNormalizer = labelNormalizer;
   }
 
   /**
@@ -375,13 +379,15 @@ public class ApprovalsUtil {
       ChangeUpdate changeUpdate) {
     Set<PatchSetApproval> current =
         ImmutableSet.copyOf(notes.getApprovalsWithCopied().get(notes.getCurrentPatchSet().id()));
+    Iterable<PatchSetApproval> currentNormalized =
+        labelNormalizer.normalize(notes, current).getNormalized();
     Set<PatchSetApproval> inferred =
         ImmutableSet.copyOf(approvalInference.forPatchSet(notes, patchSet, revWalk, repoConfig));
 
     // Exempt granted timestamp from comparisson, otherwise, we would persist the copied
     // labels every time this method is called.
     Table<LabelId, Account.Id, Short> approvalTable = HashBasedTable.create();
-    for (PatchSetApproval psa : current) {
+    for (PatchSetApproval psa : currentNormalized) {
       Account.Id id = psa.accountId();
       approvalTable.put(psa.labelId(), id, psa.value());
     }
-- 
cgit v1.2.3