opendev/gerrit
Code Issues Proposed changes

Merge changes Ie03b5b33,I09e61d81

Browse Source 
* changes:
  Refactor ReceiveCommits to use ApprovalsUtil
  Fix: Rebase did not mail all reviewers
This commit is contained in:
Saša Živkov
2012-05-09 09:47:20 -07:00
committed by gerrit code review
parent 027216e79e 4bac87c259
commit ba524c7cc8
3 changed files with 22 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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<PatchSetApproval> The previous approvals
*/
public void copyVetosToLatestPatchSet(Change change)
public List<PatchSetApproval> 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<PatchSetApproval> 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;
}

16
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -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<PatchSetApproval> patchSetApprovals = approvalsUtil.copyVetosToLatestPatchSet(change);
final Set<Account.Id> oldReviewers = new HashSet<Account.Id>();
final Set<Account.Id> oldCC = new HashSet<Account.Id>();
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());

27
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -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<PatchSetApproval> patchSetApprovals = approvalsUtil.copyVetosToLatestPatchSet(change);
final Set<Account.Id> haveApprovals = new HashSet<Account.Id>();
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 =