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 b87cce3834..c5a66bbed4 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 @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import java.io.IOException; @@ -65,8 +66,9 @@ public class ApprovalsUtil { * @param change Change to update * @throws OrmException * @throws IOException + * @return List The previous approvals */ - public void copyVetosToLatestPatchSet(Change change) + public List copyVetosToLatestPatchSet(Change change) throws OrmException, IOException { PatchSet.Id source; if (change.getNumberOfPatchSets() > 1) { @@ -76,16 +78,20 @@ public class ApprovalsUtil { } PatchSet.Id dest = change.currPatchSetId(); - for (PatchSetApproval a : db.patchSetApprovals().byPatchSet(source)) { + List patchSetApprovals = db.patchSetApprovals().byChange(change.getId()).toList(); + for (PatchSetApproval a : patchSetApprovals) { // ApprovalCategory.SUBMIT is still in db but not relevant in git-store if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { final ApprovalType type = approvalTypes.byId(a.getCategoryId()); - if (type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) { + if (a.getPatchSetId().equals(source) && + type.getCategory().isCopyMinScore() && + type.isMaxNegative(a)) { db.patchSetApprovals().insert( Collections.singleton(new PatchSetApproval(dest, a))); } } } + return patchSetApprovals; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 2812d390ae..ec28378487 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -383,18 +383,12 @@ public class ChangeUtil { replication.scheduleUpdate(change.getProject(), ru.getName()); - approvalsUtil.copyVetosToLatestPatchSet(change); - - final ChangeMessage cmsg = - new ChangeMessage(new ChangeMessage.Key(changeId, - ChangeUtil.messageUUID(db)), user.getAccountId(), patchSetId); - cmsg.setMessage("Patch Set " + patchSetId.get() + ": Rebased"); - db.changeMessages().insert(Collections.singleton(cmsg)); + List patchSetApprovals = approvalsUtil.copyVetosToLatestPatchSet(change); final Set oldReviewers = new HashSet(); final Set oldCC = new HashSet(); - for (PatchSetApproval a : db.patchSetApprovals().byChange(change.getId())) { + for (PatchSetApproval a : patchSetApprovals) { if (a.getValue() != 0) { oldReviewers.add(a.getAccountId()); } else { @@ -402,6 +396,12 @@ public class ChangeUtil { } } + final ChangeMessage cmsg = + new ChangeMessage(new ChangeMessage.Key(changeId, + ChangeUtil.messageUUID(db)), user.getAccountId(), patchSetId); + cmsg.setMessage("Patch Set " + patchSetId.get() + ": Rebased"); + db.changeMessages().insert(Collections.singleton(cmsg)); + final ReplacePatchSetSender cm = rebasedPatchSetSenderFactory.create(change); cm.setFrom(user.getAccountId()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 83877d0a48..095625d4ac 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -20,13 +20,10 @@ import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.PageLinks; -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.errors.NoSuchAccountException; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.ApprovalCategory; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; @@ -204,7 +201,6 @@ public class ReceiveCommits { private final IdentifiedUser currentUser; private final ReviewDb db; - private final ApprovalTypes approvalTypes; private final AccountResolver accountResolver; private final CreateChangeSender.Factory createChangeSenderFactory; private final MergedSender.Factory mergedSenderFactory; @@ -253,7 +249,7 @@ public class ReceiveCommits { private MessageSender messageSender; @Inject - ReceiveCommits(final ReviewDb db, final ApprovalTypes approvalTypes, + ReceiveCommits(final ReviewDb db, final AccountResolver accountResolver, final CreateChangeSender.Factory createChangeSenderFactory, final MergedSender.Factory mergedSenderFactory, @@ -276,7 +272,6 @@ public class ReceiveCommits { final SubmoduleOp.Factory subOpFactory) throws IOException { this.currentUser = (IdentifiedUser) projectControl.getCurrentUser(); this.db = db; - this.approvalTypes = approvalTypes; this.accountResolver = accountResolver; this.createChangeSenderFactory = createChangeSenderFactory; this.mergedSenderFactory = mergedSenderFactory; @@ -1387,33 +1382,21 @@ public class ReceiveCommits { result.patchSet = ps; result.info = info; + List patchSetApprovals = approvalsUtil.copyVetosToLatestPatchSet(change); + final Set haveApprovals = new HashSet(); oldReviewers.clear(); oldCC.clear(); - for (PatchSetApproval a : db.patchSetApprovals().byChange(change.getId())) { + for (PatchSetApproval a : patchSetApprovals) { haveApprovals.add(a.getAccountId()); - if (a.getValue() != 0) { oldReviewers.add(a.getAccountId()); } else { oldCC.add(a.getAccountId()); } - - // ApprovalCategory.SUBMIT is still in db but not relevant in git-store - if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { - final ApprovalType type = approvalTypes.byId(a.getCategoryId()); - if (a.getPatchSetId().equals(priorPatchSet)) { - if (type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) { - // If there was a negative vote on the prior patch set, carry it - // into this patch set. - // - db.patchSetApprovals().insert( - Collections.singleton(new PatchSetApproval(ps.getId(), a))); - } - } - } } + approvalsUtil.addReviewers(change, ps, info, reviewers, haveApprovals); msg =