opendev/gerrit
Code Issues Proposed changes

Fix adding reviewer if dummy approval already exists

Browse Source 
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>
This commit is contained in:
Edwin Kempin 2016-09-30 11:26:37 +02:00
parent 7ec3ac1118
commit 778fed6f9e
2 changed files with 25 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -42,6 +42,7 @@ import com.google.gerrit.acceptance.NoHttpd;
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.extensions.restapi.UnprocessableEntityException;
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.sql.Timestamp;
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;
@ -838,6 +842,26 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(rsrc.getChange().getLastUpdatedOn()).isNotEqualTo(oldTs);
}
@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);

2
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
View File

@ -225,7 +225,7 @@ public class ApprovalsUtil {
(short) 0, update.getWhen()));
update.putReviewer(account, REVIEWER);
}
db.patchSetApprovals().insert(cells);
db.patchSetApprovals().upsert(cells);
return Collections.unmodifiableList(cells);
}