From 41b51ccd9c740b4f93e0b94f8b9375b4f7a8d762 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 11 Jul 2013 11:47:40 +0900 Subject: [PATCH] Send notification email for new changes created by ChangeInserter New change notification was not being sent for changes created by the ChangeInserter. I.e. changes created by using the revert and cherry pick buttons on the Web UI. Update the ChangeInserter to send notification email to reviewers for new changes. Allow the caller to set extra CC recipients, or suppress sending of emails. Remove the new change email notification sending from ReceiveCommits as this is now handled by the ChangeInserter. Change-Id: I849fa533c17c7b4d3b7688832584490c942c76ba --- .../gerrit/server/change/ChangeInserter.java | 57 +++++++++++++++++++ .../gerrit/server/git/ReceiveCommits.java | 30 ++-------- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index dcda7f75e2..c645f71a23 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -29,9 +29,12 @@ import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.index.ChangeIndexer; +import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.RefControl; +import com.google.gerrit.server.util.RequestScopePropagator; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -39,6 +42,8 @@ import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.Set; @@ -48,12 +53,18 @@ public class ChangeInserter { ChangeInserter create(RefControl ctl, Change c, RevCommit rc); } + private static final Logger log = + LoggerFactory.getLogger(ChangeInserter.class); + private final Provider dbProvider; private final GitReferenceUpdated gitRefUpdated; private final ChangeHooks hooks; private final ApprovalsUtil approvalsUtil; private final TrackingFooters trackingFooters; private final ChangeIndexer indexer; + private final CreateChangeSender.Factory createChangeSenderFactory; + private final RequestScopePropagator requestScopePropagator; + private final WorkQueue workQueue; private final RefControl refControl; private final Change change; @@ -63,7 +74,9 @@ public class ChangeInserter { private ChangeMessage changeMessage; private Set reviewers; + private Set extraCC; private boolean runHooks; + private boolean sendMail; @Inject ChangeInserter(Provider dbProvider, @@ -73,6 +86,9 @@ public class ChangeInserter { ApprovalsUtil approvalsUtil, TrackingFooters trackingFooters, ChangeIndexer indexer, + CreateChangeSender.Factory createChangeSenderFactory, + RequestScopePropagator requestScopePropagator, + WorkQueue workQueue, @Assisted RefControl refControl, @Assisted Change change, @Assisted RevCommit commit) { @@ -82,11 +98,16 @@ public class ChangeInserter { this.approvalsUtil = approvalsUtil; this.trackingFooters = trackingFooters; this.indexer = indexer; + this.createChangeSenderFactory = createChangeSenderFactory; + this.requestScopePropagator = requestScopePropagator; + this.workQueue = workQueue; this.refControl = refControl; this.change = change; this.commit = commit; this.reviewers = Collections.emptySet(); + this.extraCC = Collections.emptySet(); this.runHooks = true; + this.sendMail = true; patchSet = new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID)); @@ -108,6 +129,11 @@ public class ChangeInserter { return this; } + public ChangeInserter setExtraCC(Set extraCC) { + this.extraCC = extraCC; + return this; + } + public ChangeInserter setDraft(boolean draft) { change.setStatus(draft ? Change.Status.DRAFT : Change.Status.NEW); patchSet.setDraft(draft); @@ -119,6 +145,11 @@ public class ChangeInserter { return this; } + public ChangeInserter setSendMail(boolean sendMail) { + this.sendMail = sendMail; + return this; + } + public PatchSet getPatchSet() { return patchSet; } @@ -149,8 +180,34 @@ public class ChangeInserter { indexer.index(change); gitRefUpdated.fire(change.getProject(), patchSet.getRefName(), ObjectId.zeroId(), commit); + if (runHooks) { hooks.doPatchsetCreatedHook(change, patchSet, db); } + + if (sendMail) { + workQueue.getDefaultQueue() + .submit(requestScopePropagator.wrap(new Runnable() { + @Override + public void run() { + try { + CreateChangeSender cm = + createChangeSenderFactory.create(change); + cm.setFrom(change.getOwner()); + cm.setPatchSet(patchSet, patchSetInfo); + cm.addReviewers(reviewers); + cm.addExtraCC(extraCC); + cm.send(); + } catch (Exception err) { + log.error("Cannot send email for new change " + change.getId(), err); + } + } + + @Override + public String toString() { + return "send-email newchange"; + } + })); + } } } 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 aed72ee9b2..bffc56123c 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 @@ -249,7 +249,6 @@ public class ReceiveCommits { private final SchemaFactory schemaFactory; private final AccountResolver accountResolver; private final CmdLineParser.Factory optionParserFactory; - private final CreateChangeSender.Factory createChangeSenderFactory; private final MergedSender.Factory mergedSenderFactory; private final ReplacePatchSetSender.Factory replacePatchSetFactory; private final GitReferenceUpdated gitRefUpdated; @@ -343,7 +342,6 @@ public class ReceiveCommits { this.schemaFactory = schemaFactory; this.accountResolver = accountResolver; this.optionParserFactory = optionParserFactory; - this.createChangeSenderFactory = createChangeSenderFactory; this.mergedSenderFactory = mergedSenderFactory; this.replacePatchSetFactory = replacePatchSetFactory; this.gitRefUpdated = gitRefUpdated; @@ -1523,32 +1521,12 @@ public class ReceiveCommits { recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); recipients.remove(me); - ins.setReviewers(recipients.getReviewers()).insert(); + ins + .setReviewers(recipients.getReviewers()) + .setExtraCC(recipients.getCcOnly()) + .insert(); created = true; - workQueue.getDefaultQueue() - .submit(requestScopePropagator.wrap(new Runnable() { - @Override - public void run() { - try { - CreateChangeSender cm = - createChangeSenderFactory.create(change); - cm.setFrom(me); - cm.setPatchSet(ps, ins.getPatchSetInfo()); - cm.addReviewers(recipients.getReviewers()); - cm.addExtraCC(recipients.getCcOnly()); - cm.send(); - } catch (Exception e) { - log.error("Cannot send email for new change " + change.getId(), e); - } - } - - @Override - public String toString() { - return "send-email newchange"; - } - })); - if (magicBranch != null && magicBranch.isSubmit()) { submit(projectControl.controlFor(change), ps); }