commit 778fed6f9ebf289a52daaf50da0bc505431fb49b
author Edwin Kempin <ekempin@google.com> Fri Sep 30 11:26:37 2016 +0200
committer Edwin Kempin <ekempin@google.com> Fri Sep 30 11:53:49 2016 +0200
tree 246dd22a0a7c39a68f515f8bbb23305f2bfaa428
parent 7ec3ac1118fc4f2833d6d7b9d095378dd835fe30

Fix adding reviewer if dummy approval already exists

If NoteDb is enabled adding a reviewer sometimes fails because
inserting a dummy approval for this user in ReviewDb fails due to a
duplicate key exception. This means there is an inconsistency between
ReviewDb and NoteDb, the user is not found as existing reviewer when
reading from NoteDb, but according to ReviewDb the user is a reviewer
(dummy approval exists). Handle this situation gracefully by
overwriting the existing dummy approval in this case.

Change-Id: Iea29f869662f593eb6c35424ce30378fa82e1f02
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 312a93c..2475efa 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -42,6 +42,7 @@
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
@@ -71,7 +72,9 @@
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.LabelId;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.CurrentUser;
@@ -99,6 +102,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
@@ -839,6 +843,26 @@
   }
 
   @Test
+  public void addReviewerWithNoteDbWhenDummyApprovalInReviewDbExists()
+      throws Exception {
+    assume().that(notesMigration.enabled()).isTrue();
+
+    PushOneCommit.Result r = createChange();
+
+    // insert dummy approval in ReviewDb
+    PatchSetApproval psa =
+        new PatchSetApproval(new PatchSetApproval.Key(r.getPatchSetId(),
+            user.id, new LabelId("Code-Review")), (short) 0, TimeUtil.nowTs());
+    db.patchSetApprovals().insert(Collections.singleton(psa));
+
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes()
+        .id(r.getChangeId())
+        .addReviewer(in);
+  }
+
+  @Test
   public void addSelfAsReviewer() throws Exception {
     TestTimeUtil.resetWithClockStep(1, SECONDS);
     PushOneCommit.Result r = createChange();

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 97af09e..43f20cc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -225,7 +225,7 @@
           (short) 0, update.getWhen()));
       update.putReviewer(account, REVIEWER);
     }
-    db.patchSetApprovals().insert(cells);
+    db.patchSetApprovals().upsert(cells);
     return Collections.unmodifiableList(cells);
   }