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 b824d62ae5..2630c4fbfc 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 @@ -16,6 +16,8 @@ package com.google.gerrit.server.git; import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; +import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; +import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK; @@ -40,7 +42,6 @@ import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.PageLinks; 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.extensions.registration.DynamicSet; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; @@ -68,6 +69,7 @@ import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidationListener; import com.google.gerrit.server.git.validators.CommitValidationMessage; import com.google.gerrit.server.mail.CreateChangeSender; +import com.google.gerrit.server.mail.MailUtil.MailRecipients; import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.mail.ReplacePatchSetSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; @@ -143,8 +145,6 @@ public class ReceiveCommits { private static final Pattern NEW_PATCHSET = Pattern.compile("^refs/changes/(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$"); - private static final FooterKey REVIEWED_BY = new FooterKey("Reviewed-by"); - private static final FooterKey TESTED_BY = new FooterKey("Tested-by"); private static final FooterKey CHANGE_ID = new FooterKey("Change-Id"); private static final String COMMAND_REJECTION_MESSAGE_FOOTER = @@ -741,16 +741,6 @@ public class ReceiveCommits { return displayName; } - private Account.Id toAccountId(final String nameOrEmail) throws OrmException, - NoSuchAccountException { - final Account a = accountResolver.findByNameOrEmail(nameOrEmail); - if (a == null) { - throw new NoSuchAccountException("\"" + nameOrEmail - + "\" is not registered"); - } - return a.getId(); - } - private void parseCommands(final Collection commands) { for (final ReceiveCommand cmd : commands) { if (cmd.getResult() != NOT_ATTEMPTED) { @@ -1385,25 +1375,10 @@ public class ReceiveCommits { private void insertChange(ReviewDb db) throws OrmException { final Account.Id me = currentUser.getAccountId(); - final Set reviewers = new HashSet(reviewerId); - final Set cc = new HashSet(ccId); final List footerLines = commit.getFooterLines(); - if (!ps.isDraft()) { - for (final FooterLine footerLine : footerLines) { - try { - if (isReviewer(footerLine)) { - reviewers.add(toAccountId(footerLine.getValue().trim())); - } else if (footerLine.matches(FooterKey.CC)) { - cc.add(toAccountId(footerLine.getValue().trim())); - } - } catch (NoSuchAccountException e) { - continue; - } - } - } - reviewers.remove(me); - cc.remove(me); - cc.removeAll(reviewers); + final MailRecipients recipients = new MailRecipients(reviewerId, ccId); + recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); + recipients.remove(me); db.changes().beginTransaction(change.getId()); try { @@ -1412,7 +1387,7 @@ public class ReceiveCommits { db.changes().insert(Collections.singleton(change)); ChangeUtil.updateTrackingIds(db, change, trackingFooters, footerLines); approvalsUtil.addReviewers(db, change, ps, info, - reviewers, Collections. emptySet()); + recipients.getReviewers(), Collections. emptySet()); db.commit(); } finally { db.rollback(); @@ -1430,8 +1405,8 @@ public class ReceiveCommits { createChangeSenderFactory.create(change); cm.setFrom(me); cm.setPatchSet(ps, info); - cm.addReviewers(reviewers); - cm.addExtraCC(cc); + cm.addReviewers(recipients.getReviewers()); + cm.addExtraCC(recipients.getCcOnly()); cm.send(); } catch (Exception e) { log.error("Cannot send email for new change " + change.getId(), e); @@ -1446,13 +1421,6 @@ public class ReceiveCommits { } } - private static boolean isReviewer(final FooterLine candidateFooterLine) { - return candidateFooterLine.matches(FooterKey.SIGNED_OFF_BY) - || candidateFooterLine.matches(FooterKey.ACKED_BY) - || candidateFooterLine.matches(REVIEWED_BY) - || candidateFooterLine.matches(TESTED_BY); - } - private void preparePatchSetsForReplace() { try { readChangesForReplace(); @@ -1698,28 +1666,10 @@ public class ReceiveCommits { PatchSet.Id insertPatchSet(ReviewDb db) throws OrmException { final Account.Id me = currentUser.getAccountId(); - final Set reviewers = new HashSet(reviewerId); - final Set cc = new HashSet(ccId); final List footerLines = newCommit.getFooterLines(); - if (!newPatchSet.isDraft()) { - for (final FooterLine footerLine : footerLines) { - try { - if (isReviewer(footerLine)) { - reviewers.add(toAccountId(footerLine.getValue().trim())); - } else if (footerLine.matches(FooterKey.CC)) { - cc.add(toAccountId(footerLine.getValue().trim())); - } - } catch (NoSuchAccountException e) { - continue; - } - } - } - reviewers.remove(me); - cc.remove(me); - cc.removeAll(reviewers); - - final Set oldReviewers = new HashSet(); - final Set oldCC = new HashSet(); + final MailRecipients recipients = new MailRecipients(reviewerId, ccId); + recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines)); + recipients.remove(me); db.changes().beginTransaction(change.getId()); try { @@ -1739,22 +1689,11 @@ public class ReceiveCommits { List patchSetApprovals = approvalsUtil.copyVetosToPatchSet(db, newPatchSet.getId()); - - final Set haveApprovals = new HashSet(); - oldReviewers.clear(); - oldCC.clear(); - - for (PatchSetApproval a : patchSetApprovals) { - haveApprovals.add(a.getAccountId()); - if (a.getValue() != 0) { - oldReviewers.add(a.getAccountId()); - } else { - oldCC.add(a.getAccountId()); - } - } - + final MailRecipients oldRecipients = + getRecipientsFromApprovals(patchSetApprovals); approvalsUtil.addReviewers(db, change, newPatchSet, info, - reviewers, haveApprovals); + recipients.getReviewers(), oldRecipients.getAll()); + recipients.add(oldRecipients); msg = new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil @@ -1829,10 +1768,8 @@ public class ReceiveCommits { cm.setFrom(me); cm.setPatchSet(newPatchSet, info); cm.setChangeMessage(msg); - cm.addReviewers(reviewers); - cm.addExtraCC(cc); - cm.addReviewers(oldReviewers); - cm.addExtraCC(oldCC); + cm.addReviewers(recipients.getReviewers()); + cm.addExtraCC(recipients.getCcOnly()); cm.send(); } catch (Exception e) { log.error("Cannot send email for new patch set " + newPatchSet.getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java new file mode 100644 index 0000000000..f22c6e4024 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java @@ -0,0 +1,131 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.mail; + +import com.google.gerrit.common.errors.NoSuchAccountException; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.server.account.AccountResolver; +import com.google.gwtorm.server.OrmException; + +import org.eclipse.jgit.revwalk.FooterKey; +import org.eclipse.jgit.revwalk.FooterLine; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class MailUtil { + private static final FooterKey REVIEWED_BY = new FooterKey("Reviewed-by"); + private static final FooterKey TESTED_BY = new FooterKey("Tested-by"); + + public static MailRecipients getRecipientsFromFooters( + final AccountResolver accountResolver, final PatchSet ps, + final List footerLines) throws OrmException { + final MailRecipients recipients = new MailRecipients(); + if (!ps.isDraft()) { + for (final FooterLine footerLine : footerLines) { + try { + if (isReviewer(footerLine)) { + recipients.reviewers.add(toAccountId(accountResolver, footerLine + .getValue().trim())); + } else if (footerLine.matches(FooterKey.CC)) { + recipients.cc.add(toAccountId(accountResolver, footerLine + .getValue().trim())); + } + } catch (NoSuchAccountException e) { + continue; + } + } + } + return recipients; + } + + public static MailRecipients getRecipientsFromApprovals( + final List approvals) { + final MailRecipients recipients = new MailRecipients(); + for (PatchSetApproval a : approvals) { + if (a.getValue() != 0) { + recipients.reviewers.add(a.getAccountId()); + } else { + recipients.cc.add(a.getAccountId()); + } + } + return recipients; + } + + private static Account.Id toAccountId(final AccountResolver accountResolver, + final String nameOrEmail) throws OrmException, NoSuchAccountException { + final Account a = accountResolver.findByNameOrEmail(nameOrEmail); + if (a == null) { + throw new NoSuchAccountException("\"" + nameOrEmail + + "\" is not registered"); + } + return a.getId(); + } + + private static boolean isReviewer(final FooterLine candidateFooterLine) { + return candidateFooterLine.matches(FooterKey.SIGNED_OFF_BY) + || candidateFooterLine.matches(FooterKey.ACKED_BY) + || candidateFooterLine.matches(REVIEWED_BY) + || candidateFooterLine.matches(TESTED_BY); + } + + public static class MailRecipients { + private final Set reviewers; + private final Set cc; + + public MailRecipients() { + this.reviewers = new HashSet(); + this.cc = new HashSet(); + } + + public MailRecipients(final Set reviewers, + final Set cc) { + this.reviewers = new HashSet(reviewers); + this.cc = new HashSet(cc); + } + + public void add(final MailRecipients recipients) { + reviewers.addAll(recipients.reviewers); + cc.addAll(recipients.cc); + } + + public void remove(final Account.Id toRemove) { + reviewers.remove(toRemove); + cc.remove(toRemove); + } + + public Set getReviewers() { + return Collections.unmodifiableSet(reviewers); + } + + public Set getCcOnly() { + final Set cc = new HashSet(this.cc); + cc.removeAll(reviewers); + return Collections.unmodifiableSet(cc); + } + + public Set getAll() { + final Set all = + new HashSet(reviewers.size() + cc.size()); + all.addAll(reviewers); + all.addAll(cc); + return Collections.unmodifiableSet(all); + } + } +}