From 8e24b766490fd04c044f55905c1ead813d0ed1fc Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 30 Jan 2019 13:47:50 -0800 Subject: [PATCH] Consistently pass NotifyResolver.Result instead of separate args Change-Id: I457fa6cc45002a88440e33adf609b52f8610b9eb --- .../gerrit/server/change/AbandonOp.java | 3 +- .../server/change/AddReviewersEmail.java | 3 +- .../gerrit/server/change/BatchAbandon.java | 17 +----- .../gerrit/server/change/ChangeInserter.java | 17 ++---- .../server/change/ConsistencyChecker.java | 3 +- .../server/change/EmailReviewComments.java | 16 ++---- .../gerrit/server/change/NotifyResolver.java | 8 ++- .../server/change/PatchSetInserter.java | 20 ++----- .../gerrit/server/change/RebaseChangeOp.java | 3 +- .../server/change/WorkInProgressOp.java | 11 ++-- .../gerrit/server/edit/ChangeEditUtil.java | 5 +- .../extensions/events/RevisionCreated.java | 7 +-- .../server/git/receive/ReceiveCommits.java | 53 +++++++++---------- .../gerrit/server/git/receive/ReplaceOp.java | 8 ++- .../server/mail/receive/MailProcessor.java | 6 +-- .../gerrit/server/mail/send/ChangeEmail.java | 14 ++--- .../server/mail/send/CommentSender.java | 4 +- .../server/mail/send/NewChangeSender.java | 2 +- .../server/mail/send/OutgoingEmail.java | 23 +++----- .../mail/send/ReplacePatchSetSender.java | 3 +- .../restapi/change/CherryPickChange.java | 6 +-- .../server/restapi/change/CreateChange.java | 3 +- .../restapi/change/CreateMergePatchSet.java | 7 ++- .../change/DeleteReviewerByEmailOp.java | 3 +- .../restapi/change/DeleteReviewerOp.java | 3 +- .../server/restapi/change/DeleteVote.java | 3 +- .../server/restapi/change/PostReview.java | 26 +++------ .../server/restapi/change/PutMessage.java | 3 +- .../gerrit/server/restapi/change/Revert.java | 22 +++----- .../gerrit/server/submit/EmailMerge.java | 19 +++---- .../server/submit/SubmitStrategyOp.java | 7 +-- .../server/change/ConsistencyCheckerIT.java | 6 +-- .../change/AbstractQueryChangesTest.java | 4 +- 33 files changed, 118 insertions(+), 220 deletions(-) diff --git a/java/com/google/gerrit/server/change/AbandonOp.java b/java/com/google/gerrit/server/change/AbandonOp.java index 7cabe25263..600f14d257 100644 --- a/java/com/google/gerrit/server/change/AbandonOp.java +++ b/java/com/google/gerrit/server/change/AbandonOp.java @@ -120,8 +120,7 @@ public class AbandonOp implements BatchUpdateOp { cm.setFrom(accountState.getAccount().getId()); } cm.setChangeMessage(message.getMessage(), ctx.getWhen()); - cm.setNotify(notify.handling()); - cm.setAccountsToNotify(notify.accounts()); + cm.setNotify(notify); cm.send(); } catch (Exception e) { logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId()); diff --git a/java/com/google/gerrit/server/change/AddReviewersEmail.java b/java/com/google/gerrit/server/change/AddReviewersEmail.java index da02646663..d9c5dade4f 100644 --- a/java/com/google/gerrit/server/change/AddReviewersEmail.java +++ b/java/com/google/gerrit/server/change/AddReviewersEmail.java @@ -58,8 +58,7 @@ public class AddReviewersEmail { try { AddReviewerSender cm = addReviewerSenderFactory.create(change.getProject(), change.getId()); - cm.setNotify(notify.handling()); - cm.setAccountsToNotify(notify.accounts()); + cm.setNotify(notify); cm.setFrom(userId); cm.addReviewers(toMail); cm.addReviewersByEmail(addedByEmail); diff --git a/java/com/google/gerrit/server/change/BatchAbandon.java b/java/com/google/gerrit/server/change/BatchAbandon.java index 6b7c43b7ba..0b3d8c6a90 100644 --- a/java/com/google/gerrit/server/change/BatchAbandon.java +++ b/java/com/google/gerrit/server/change/BatchAbandon.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.change; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Project; @@ -77,13 +76,7 @@ public class BatchAbandon { Collection changes, String msgTxt) throws RestApiException, UpdateException { - batchAbandon( - updateFactory, - project, - user, - changes, - msgTxt, - NotifyResolver.Result.create(NotifyHandling.ALL)); + batchAbandon(updateFactory, project, user, changes, msgTxt, NotifyResolver.Result.all()); } public void batchAbandon( @@ -92,12 +85,6 @@ public class BatchAbandon { CurrentUser user, Collection changes) throws RestApiException, UpdateException { - batchAbandon( - updateFactory, - project, - user, - changes, - "", - NotifyResolver.Result.create(NotifyHandling.ALL)); + batchAbandon(updateFactory, project, user, changes, "", NotifyResolver.Result.all()); } } diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java index fb92ec971a..822560ab7f 100644 --- a/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/java/com/google/gerrit/server/change/ChangeInserter.java @@ -24,16 +24,13 @@ import static java.util.Objects.requireNonNull; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.Iterables; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -123,8 +120,7 @@ public class ChangeInserter implements InsertChangeOp { private boolean workInProgress; private List groups = Collections.emptyList(); private boolean validate = true; - private NotifyHandling notify = NotifyHandling.ALL; - private ListMultimap accountsToNotify = ImmutableListMultimap.of(); + private NotifyResolver.Result notify = NotifyResolver.Result.all(); private Map approvals; private RequestScopePropagator requestScopePropagator; private boolean fireRevisionCreated; @@ -255,17 +251,11 @@ public class ChangeInserter implements InsertChangeOp { return this; } - public ChangeInserter setNotify(NotifyHandling notify) { + public ChangeInserter setNotify(NotifyResolver.Result notify) { this.notify = notify; return this; } - public ChangeInserter setAccountsToNotify( - ListMultimap accountsToNotify) { - this.accountsToNotify = requireNonNull(accountsToNotify); - return this; - } - public ChangeInserter setReviewersAndCcs( Iterable reviewers, Iterable ccs) { return setReviewersAndCcsAsStrings( @@ -457,7 +447,7 @@ public class ChangeInserter implements InsertChangeOp { @Override public void postUpdate(Context ctx) throws Exception { reviewerAdditions.postUpdate(ctx); - if (sendMail && (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty())) { + if (sendMail && notify.shouldNotify()) { Runnable sender = new Runnable() { @Override @@ -468,7 +458,6 @@ public class ChangeInserter implements InsertChangeOp { cm.setFrom(change.getOwner()); cm.setPatchSet(patchSet, patchSetInfo); cm.setNotify(notify); - cm.setAccountsToNotify(accountsToNotify); cm.addReviewers( reviewerAdditions .flattenResults(AddReviewersOp.Result::addedReviewers) diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java index 7a553e3a53..316acb3c95 100644 --- a/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -31,7 +31,6 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.changes.FixInput; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.common.ProblemInfo; import com.google.gerrit.extensions.common.ProblemInfo.Status; import com.google.gerrit.extensions.restapi.RestApiException; @@ -537,7 +536,7 @@ public class ConsistencyChecker { inserter .setValidate(false) .setFireRevisionCreated(false) - .setNotify(NotifyHandling.NONE) + .setNotify(NotifyResolver.Result.none()) .setAllowClosed(true) .setMessage("Patch set for merged commit inserted by consistency checker")); bu.addOp(notes.getChangeId(), new FixMergedOp(notFound)); diff --git a/java/com/google/gerrit/server/change/EmailReviewComments.java b/java/com/google/gerrit/server/change/EmailReviewComments.java index 7e063bcc97..8353501f93 100644 --- a/java/com/google/gerrit/server/change/EmailReviewComments.java +++ b/java/com/google/gerrit/server/change/EmailReviewComments.java @@ -16,12 +16,8 @@ package com.google.gerrit.server.change; import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER; -import com.google.common.collect.ListMultimap; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; -import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.api.changes.RecipientType; -import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.PatchSet; @@ -48,7 +44,6 @@ public class EmailReviewComments implements Runnable, RequestContext { // on the same set of inputs. /** * @param notify setting for handling notification. - * @param accountsToNotify detailed map of accounts to notify. * @param notes change notes. * @param patchSet patch set corresponding to the top-level op * @param user user the email should come from. @@ -63,8 +58,7 @@ public class EmailReviewComments implements Runnable, RequestContext { * @return handle for sending email. */ EmailReviewComments create( - NotifyHandling notify, - ListMultimap accountsToNotify, + NotifyResolver.Result notify, ChangeNotes notes, PatchSet patchSet, IdentifiedUser user, @@ -79,8 +73,7 @@ public class EmailReviewComments implements Runnable, RequestContext { private final CommentSender.Factory commentSenderFactory; private final ThreadLocalRequestContext requestContext; - private final NotifyHandling notify; - private final ListMultimap accountsToNotify; + private final NotifyResolver.Result notify; private final ChangeNotes notes; private final PatchSet patchSet; private final IdentifiedUser user; @@ -95,8 +88,7 @@ public class EmailReviewComments implements Runnable, RequestContext { PatchSetInfoFactory patchSetInfoFactory, CommentSender.Factory commentSenderFactory, ThreadLocalRequestContext requestContext, - @Assisted NotifyHandling notify, - @Assisted ListMultimap accountsToNotify, + @Assisted NotifyResolver.Result notify, @Assisted ChangeNotes notes, @Assisted PatchSet patchSet, @Assisted IdentifiedUser user, @@ -109,7 +101,6 @@ public class EmailReviewComments implements Runnable, RequestContext { this.commentSenderFactory = commentSenderFactory; this.requestContext = requestContext; this.notify = notify; - this.accountsToNotify = accountsToNotify; this.notes = notes; this.patchSet = patchSet; this.user = user; @@ -136,7 +127,6 @@ public class EmailReviewComments implements Runnable, RequestContext { cm.setPatchSetComment(patchSetComment); cm.setLabels(labels); cm.setNotify(notify); - cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception e) { logger.atSevere().withCause(e).log("Cannot email comments for %s", patchSet.getId()); diff --git a/java/com/google/gerrit/server/change/NotifyResolver.java b/java/com/google/gerrit/server/change/NotifyResolver.java index 5ad9b470e3..d75ba2b26c 100644 --- a/java/com/google/gerrit/server/change/NotifyResolver.java +++ b/java/com/google/gerrit/server/change/NotifyResolver.java @@ -41,15 +41,19 @@ import org.eclipse.jgit.errors.ConfigInvalidException; public class NotifyResolver { @AutoValue public abstract static class Result { - static Result none() { + public static Result none() { return create(NotifyHandling.NONE); } + public static Result all() { + return create(NotifyHandling.ALL); + } + public static Result create(NotifyHandling notifyHandling) { return create(notifyHandling, ImmutableListMultimap.of()); } - private static Result create( + public static Result create( NotifyHandling handling, ImmutableListMultimap recipients) { return new AutoValue_NotifyResolver_Result(handling, recipients); } diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java index ec11c1b505..d51c065541 100644 --- a/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -19,14 +19,10 @@ import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; import static java.util.Objects.requireNonNull; -import com.google.common.collect.ImmutableListMultimap; -import com.google.common.collect.ListMultimap; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; @@ -95,8 +91,7 @@ public class PatchSetInserter implements BatchUpdateOp { private boolean checkAddPatchSetPermission = true; private List groups = Collections.emptyList(); private boolean fireRevisionCreated = true; - private NotifyHandling notify = NotifyHandling.ALL; - private ListMultimap accountsToNotify = ImmutableListMultimap.of(); + private NotifyResolver.Result notify = NotifyResolver.Result.all(); private boolean allowClosed; // Fields set during some phase of BatchUpdate.Op. @@ -170,17 +165,11 @@ public class PatchSetInserter implements BatchUpdateOp { return this; } - public PatchSetInserter setNotify(NotifyHandling notify) { + public PatchSetInserter setNotify(NotifyResolver.Result notify) { this.notify = requireNonNull(notify); return this; } - public PatchSetInserter setAccountsToNotify( - ListMultimap accountsToNotify) { - this.accountsToNotify = requireNonNull(accountsToNotify); - return this; - } - public PatchSetInserter setAllowClosed(boolean allowClosed) { this.allowClosed = allowClosed; return this; @@ -229,7 +218,7 @@ public class PatchSetInserter implements BatchUpdateOp { psUtil.insert( ctx.getRevWalk(), ctx.getUpdate(psId), psId, commitId, newGroups, null, description); - if (notify != NotifyHandling.NONE) { + if (notify.handling() != NotifyHandling.NONE) { oldReviewers = approvalsUtil.getReviewers(ctx.getNotes()); } @@ -258,7 +247,7 @@ public class PatchSetInserter implements BatchUpdateOp { @Override public void postUpdate(Context ctx) throws OrmException { - if (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty()) { + if (notify.shouldNotify()) { try { ReplacePatchSetSender cm = replacePatchSetFactory.create(ctx.getProject(), change.getId()); cm.setFrom(ctx.getAccountId()); @@ -267,7 +256,6 @@ public class PatchSetInserter implements BatchUpdateOp { cm.addReviewers(oldReviewers.byState(REVIEWER)); cm.addExtraCC(oldReviewers.byState(CC)); cm.setNotify(notify); - cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception err) { logger.atSevere().withCause(err).log( diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java index a64900b69c..39c1adeb32 100644 --- a/java/com/google/gerrit/server/change/RebaseChangeOp.java +++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -16,7 +16,6 @@ package com.google.gerrit.server.change; import static com.google.common.base.Preconditions.checkState; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.restapi.MergeConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -183,7 +182,7 @@ public class RebaseChangeOp implements BatchUpdateOp { patchSetInserterFactory .create(notes, rebasedPatchSetId, rebasedCommit) .setDescription("Rebase") - .setNotify(NotifyHandling.NONE) + .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(fireRevisionCreated) .setCheckAddPatchSetPermission(checkAddPatchSetPermission) .setValidate(validate); diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java index 1da6d16347..cbcf9cb11f 100644 --- a/java/com/google/gerrit/server/change/WorkInProgressOp.java +++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.change; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.restapi.AuthException; @@ -92,7 +91,7 @@ public class WorkInProgressOp implements BatchUpdateOp { private final PatchSetUtil psUtil; private final boolean workInProgress; private final Input in; - private final NotifyHandling notify; + private final NotifyResolver.Result notify; private final WorkInProgressStateChanged stateChanged; private Change change; @@ -115,8 +114,9 @@ public class WorkInProgressOp implements BatchUpdateOp { this.workInProgress = workInProgress; this.in = in; notify = - MoreObjects.firstNonNull( - in.notify, workInProgress ? NotifyHandling.NONE : NotifyHandling.ALL); + NotifyResolver.Result.create( + MoreObjects.firstNonNull( + in.notify, workInProgress ? NotifyHandling.NONE : NotifyHandling.ALL)); } @Override @@ -160,13 +160,12 @@ public class WorkInProgressOp implements BatchUpdateOp { @Override public void postUpdate(Context ctx) { stateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen()); - if (workInProgress || notify.ordinal() < NotifyHandling.OWNER_REVIEWERS.ordinal()) { + if (workInProgress || notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) < 0) { return; } email .create( notify, - ImmutableListMultimap.of(), notes, ps, ctx.getIdentifiedUser(), diff --git a/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 3002565400..7b838bdbf7 100644 --- a/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -170,10 +170,7 @@ public class ChangeEditUtil { RevCommit squashed = squashEdit(rw, oi, edit.getEditCommit(), basePatchSet); PatchSet.Id psId = ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId()); PatchSetInserter inserter = - patchSetInserterFactory - .create(notes, psId, squashed) - .setNotify(notify.handling()) - .setAccountsToNotify(notify.accounts()); + patchSetInserterFactory.create(notes, psId, squashed).setNotify(notify); StringBuilder message = new StringBuilder("Patch Set ").append(inserter.getPatchSetId().get()).append(": "); diff --git a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java index e043e9fcd2..3fd69a2281 100644 --- a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java +++ b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java @@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.server.GpgException; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -46,7 +47,7 @@ public class RevisionCreated { PatchSet patchSet, AccountState uploader, Timestamp when, - NotifyHandling notify) {} + NotifyResolver.Result notify) {} }; private final PluginSetContext listeners; @@ -68,7 +69,7 @@ public class RevisionCreated { PatchSet patchSet, AccountState uploader, Timestamp when, - NotifyHandling notify) { + NotifyResolver.Result notify) { if (listeners.isEmpty()) { return; } @@ -79,7 +80,7 @@ public class RevisionCreated { util.revisionInfo(change.getProject(), patchSet), util.accountInfo(uploader), when, - notify); + notify.handling()); listeners.runEach(l -> l.onRevisionCreated(event)); } catch (PatchListObjectTooLargeException e) { logger.atWarning().log("Couldn't fire event: %s", e.getMessage()); diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 302c73baa6..deefd01aff 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -49,6 +49,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; @@ -94,6 +95,7 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.change.ChangeInserter; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.SetHashtagsOp; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.GerritServerConfig; @@ -1386,7 +1388,7 @@ class ReceiveCommits { "Notify handling that defines to whom email notifications " + "should be sent. Allowed values are NONE, OWNER, " + "OWNER_REVIEWERS, ALL. If not set, the default is ALL.") - private NotifyHandling notify; + private NotifyHandling notifyHandling; @Option( name = "--notify-to", @@ -1516,15 +1518,6 @@ class ReceiveCommits { .collect(toImmutableSet()); } - ListMultimap getAccountsToNotify() { - ListMultimap accountsToNotify = - MultimapBuilder.hashKeys().arrayListValues().build(); - accountsToNotify.putAll(RecipientType.TO, notifyTo); - accountsToNotify.putAll(RecipientType.CC, notifyCc); - accountsToNotify.putAll(RecipientType.BCC, notifyBcc); - return accountsToNotify; - } - boolean shouldPublishComments() { if (publishComments) { return true; @@ -1584,24 +1577,31 @@ class ReceiveCommits { return ref.substring(0, split); } - NotifyHandling getNotify() { - if (notify != null) { - return notify; - } - if (workInProgress) { - return NotifyHandling.OWNER; - } - return NotifyHandling.ALL; + NotifyResolver.Result getNotifyForNewChange() { + return getNotifyImpl(null); } - NotifyHandling getNotify(ChangeNotes notes) { - if (notify != null) { - return notify; + NotifyResolver.Result getNotify(ChangeNotes notes) { + return getNotifyImpl(requireNonNull(notes)); + } + + private NotifyResolver.Result getNotifyImpl(@Nullable ChangeNotes notes) { + NotifyHandling notifyHandling = this.notifyHandling; + if (notifyHandling == null) { + if (workInProgress || (notes != null && !ready && notes.getChange().isWorkInProgress())) { + notifyHandling = NotifyHandling.OWNER; + } else { + notifyHandling = NotifyHandling.ALL; + } } - if (workInProgress || (!ready && notes.getChange().isWorkInProgress())) { - return NotifyHandling.OWNER; - } - return NotifyHandling.ALL; + + return NotifyResolver.Result.create( + notifyHandling, + ImmutableListMultimap.builder() + .putAll(RecipientType.TO, notifyTo) + .putAll(RecipientType.CC, notifyCc) + .putAll(RecipientType.BCC, notifyBcc) + .build()); } } @@ -2446,8 +2446,7 @@ class ReceiveCommits { magicBranch.getCombinedCcs(fromFooters)) .setApprovals(approvals) .setMessage(msg.toString()) - .setNotify(magicBranch.getNotify()) - .setAccountsToNotify(magicBranch.getAccountsToNotify()) + .setNotify(magicBranch.getNotifyForNewChange()) .setRequestScopePropagator(requestScopePropagator) .setSendMail(true) .setPatchSetDescription(magicBranch.message)); diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index 0f68ba579e..430eac9fd6 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -25,7 +25,6 @@ import static org.eclipse.jgit.lib.Constants.R_HEADS; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; @@ -52,6 +51,7 @@ import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.change.AddReviewersOp; import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.change.EmailReviewComments; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.ReviewerAdder; import com.google.gerrit.server.change.ReviewerAdder.InternalAddReviewerInput; import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition; @@ -512,13 +512,12 @@ public class ReplaceOp implements BatchUpdateOp { } } - NotifyHandling notify = magicBranch != null ? magicBranch.getNotify(notes) : NotifyHandling.ALL; - + NotifyResolver.Result notify = + magicBranch != null ? magicBranch.getNotify(notes) : NotifyResolver.Result.all(); if (shouldPublishComments()) { emailCommentsFactory .create( notify, - magicBranch != null ? magicBranch.getAccountsToNotify() : ImmutableListMultimap.of(), notes, newPatchSet, ctx.getUser().asIdentifiedUser(), @@ -557,7 +556,6 @@ public class ReplaceOp implements BatchUpdateOp { cm.setChangeMessage(msg.getMessage(), ctx.getWhen()); if (magicBranch != null) { cm.setNotify(magicBranch.getNotify(notes)); - cm.setAccountsToNotify(magicBranch.getAccountsToNotify()); } cm.addReviewers( Streams.concat( diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java index a1d745e0f4..d0ae39f35a 100644 --- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java +++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java @@ -17,10 +17,8 @@ package com.google.gerrit.server.mail.receive; import static java.util.stream.Collectors.toList; import com.google.common.base.Strings; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.client.Side; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.Extension; @@ -47,6 +45,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.Emails; import com.google.gerrit.server.change.EmailReviewComments; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.config.UrlFormatter; import com.google.gerrit.server.extensions.events.CommentAdded; import com.google.gerrit.server.mail.MailFilter; @@ -315,8 +314,7 @@ public class MailProcessor { // Send email notifications outgoingMailFactory .create( - NotifyHandling.ALL, - ArrayListMultimap.create(), + NotifyResolver.Result.all(), notes, patchSet, ctx.getUser().asIdentifiedUser(), diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java index 62d629a7da..22923c09dc 100644 --- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java +++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java @@ -183,7 +183,7 @@ public abstract class ChangeEmail extends NotificationEmail { setChangeUrlHeader(); setCommitIdHeader(); - if (notify.ordinal() >= NotifyHandling.OWNER_REVIEWERS.ordinal()) { + if (notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) >= 0) { try { addByEmail( RecipientType.CC, changeData.reviewersByEmail().byState(ReviewerStateInternal.CC)); @@ -320,7 +320,7 @@ public abstract class ChangeEmail extends NotificationEmail { /** BCC any user who has starred this change. */ protected void bccStarredBy() { - if (!NotifyHandling.ALL.equals(notify)) { + if (!NotifyHandling.ALL.equals(notify.handling())) { return; } @@ -342,7 +342,7 @@ public abstract class ChangeEmail extends NotificationEmail { @Override protected final Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig) throws OrmException { - if (!NotifyHandling.ALL.equals(notify)) { + if (!NotifyHandling.ALL.equals(notify.handling())) { return new Watchers(); } @@ -352,7 +352,8 @@ public abstract class ChangeEmail extends NotificationEmail { /** Any user who has published comments on this change. */ protected void ccAllApprovals() { - if (!NotifyHandling.ALL.equals(notify) && !NotifyHandling.OWNER_REVIEWERS.equals(notify)) { + if (!NotifyHandling.ALL.equals(notify.handling()) + && !NotifyHandling.OWNER_REVIEWERS.equals(notify.handling())) { return; } @@ -367,7 +368,8 @@ public abstract class ChangeEmail extends NotificationEmail { /** Users who have non-zero approval codes on the change. */ protected void ccExistingReviewers() { - if (!NotifyHandling.ALL.equals(notify) && !NotifyHandling.OWNER_REVIEWERS.equals(notify)) { + if (!NotifyHandling.ALL.equals(notify.handling()) + && !NotifyHandling.OWNER_REVIEWERS.equals(notify.handling())) { return; } @@ -404,7 +406,7 @@ public abstract class ChangeEmail extends NotificationEmail { protected Set getAuthors() { Set authors = new HashSet<>(); - switch (notify) { + switch (notify.handling()) { case NONE: break; case ALL: diff --git a/java/com/google/gerrit/server/mail/send/CommentSender.java b/java/com/google/gerrit/server/mail/send/CommentSender.java index ec28bcbd6d..e2a7d92f79 100644 --- a/java/com/google/gerrit/server/mail/send/CommentSender.java +++ b/java/com/google/gerrit/server/mail/send/CommentSender.java @@ -153,10 +153,10 @@ public class CommentSender extends ReplyToChangeSender { protected void init() throws EmailException { super.init(); - if (notify.compareTo(NotifyHandling.OWNER_REVIEWERS) >= 0) { + if (notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) >= 0) { ccAllApprovals(); } - if (notify.compareTo(NotifyHandling.ALL) >= 0) { + if (notify.handling().compareTo(NotifyHandling.ALL) >= 0) { bccStarredBy(); includeWatchers(NotifyType.ALL_COMMENTS, !change.isWorkInProgress() && !change.isPrivate()); } diff --git a/java/com/google/gerrit/server/mail/send/NewChangeSender.java b/java/com/google/gerrit/server/mail/send/NewChangeSender.java index f94f1ca9e1..c45da40a35 100644 --- a/java/com/google/gerrit/server/mail/send/NewChangeSender.java +++ b/java/com/google/gerrit/server/mail/send/NewChangeSender.java @@ -59,7 +59,7 @@ public abstract class NewChangeSender extends ChangeEmail { setHeader("Message-ID", getChangeMessageThreadId()); - switch (notify) { + switch (notify.handling()) { case NONE: case OWNER: break; diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java index 043eee9faa..9c5f9775ce 100644 --- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java +++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java @@ -18,11 +18,8 @@ import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailSt import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED; import static java.util.Objects.requireNonNull; -import com.google.common.collect.ImmutableListMultimap; -import com.google.common.collect.ListMultimap; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.errors.EmailException; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat; @@ -33,6 +30,7 @@ import com.google.gerrit.mail.MailHeader; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.UserIdentity; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.validators.OutgoingEmailValidationListener; import com.google.gerrit.server.validators.ValidationException; @@ -65,13 +63,12 @@ public abstract class OutgoingEmail { private Address smtpFromAddress; private StringBuilder textBody; private StringBuilder htmlBody; - private ListMultimap accountsToNotify = ImmutableListMultimap.of(); protected Map soyContext; protected Map soyContextEmailData; protected List footers; protected final EmailArguments args; protected Account.Id fromId; - protected NotifyHandling notify = NotifyHandling.ALL; + protected NotifyResolver.Result notify = NotifyResolver.Result.all(); protected OutgoingEmail(EmailArguments ea, String mc) { args = ea; @@ -83,21 +80,17 @@ public abstract class OutgoingEmail { fromId = id; } - public void setNotify(NotifyHandling notify) { + public void setNotify(NotifyResolver.Result notify) { this.notify = requireNonNull(notify); } - public void setAccountsToNotify(ListMultimap accountsToNotify) { - this.accountsToNotify = requireNonNull(accountsToNotify); - } - /** * Format and enqueue the message for delivery. * * @throws EmailException */ public void send() throws EmailException { - if (NotifyHandling.NONE.equals(notify) && accountsToNotify.isEmpty()) { + if (!notify.shouldNotify()) { return; } @@ -129,7 +122,7 @@ public abstract class OutgoingEmail { // on their behalf to others. // add(RecipientType.CC, fromId); - } else if (!accountsToNotify.containsValue(fromId) && rcptTo.remove(fromId)) { + } else if (!notify.accounts().containsValue(fromId) && rcptTo.remove(fromId)) { // If they don't want a copy, but we queued one up anyway, // drop them from the recipient lists. // @@ -238,8 +231,8 @@ public abstract class OutgoingEmail { setHeader(FieldName.MESSAGE_ID, ""); setHeader(MailHeader.AUTO_SUBMITTED.fieldName(), "auto-generated"); - for (RecipientType recipientType : accountsToNotify.keySet()) { - add(recipientType, accountsToNotify.get(recipientType)); + for (RecipientType recipientType : notify.accounts().keySet()) { + add(recipientType, notify.accounts().get(recipientType)); } setHeader(MailHeader.MESSAGE_TYPE.fieldName(), messageClass); @@ -412,7 +405,7 @@ public abstract class OutgoingEmail { return false; } - if ((accountsToNotify == null || accountsToNotify.isEmpty()) + if (notify.accounts().isEmpty() && smtpRcptTo.size() == 1 && rcptTo.size() == 1 && rcptTo.contains(fromId)) { diff --git a/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java b/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java index 2398b82b3b..f2844c4e50 100644 --- a/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java +++ b/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java @@ -63,7 +63,8 @@ public class ReplacePatchSetSender extends ReplyToChangeSender { // reviewers.remove(fromId); } - if (notify == NotifyHandling.ALL || notify == NotifyHandling.OWNER_REVIEWERS) { + if (notify.handling() == NotifyHandling.ALL + || notify.handling() == NotifyHandling.OWNER_REVIEWERS) { add(RecipientType.TO, reviewers); add(RecipientType.CC, extraCC); } diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java index a33bcc11db..b898ab567a 100644 --- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java +++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java @@ -330,8 +330,7 @@ public class CherryPickChange { NotifyResolver.Result notify = resolveNotify(input); inserter .setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + ".") - .setNotify(notify.handling()) - .setAccountsToNotify(notify.accounts()); + .setNotify(notify); bu.addOp(destChange.getId(), inserter); return destChange.getId(); } @@ -356,8 +355,7 @@ public class CherryPickChange { .setWorkInProgress( (sourceChange != null && sourceChange.isWorkInProgress()) || !cherryPickCommit.getFilesWithGitConflicts().isEmpty()) - .setNotify(notify.handling()) - .setAccountsToNotify(notify.accounts()); + .setNotify(notify); if (input.keepReviewers && sourceChange != null) { ReviewerSet reviewerSet = approvalsUtil.getReviewers(changeNotesFactory.createChecked(sourceChange)); diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java index c7d2e8bc63..e7255d7ef6 100644 --- a/java/com/google/gerrit/server/restapi/change/CreateChange.java +++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java @@ -308,8 +308,7 @@ public class CreateChange NotifyResolver.Result notify = notifyResolver.resolve( firstNonNull(input.notify, NotifyHandling.ALL), input.notifyDetails); - ins.setNotify(notify.handling()); - ins.setAccountsToNotify(notify.accounts()); + ins.setNotify(notify); try (BatchUpdate bu = updateFactory.create(projectState.getNameKey(), me, now)) { bu.setRepository(git, rw, oi); bu.insertChange(ins); diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java index 6efe9598b4..3bf2ca81c5 100644 --- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java +++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.restapi.change; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.Iterables; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.MergeInput; @@ -41,6 +40,7 @@ import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.ChangeFinder; import com.google.gerrit.server.change.ChangeJson; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; @@ -185,9 +185,8 @@ public class CreateMergePatchSet bu.setRepository(git, rw, oi); psInserter .setMessage("Uploaded patch set " + nextPsId.get() + ".") - .setNotify(NotifyHandling.NONE) - .setCheckAddPatchSetPermission(false) - .setNotify(NotifyHandling.NONE); + .setNotify(NotifyResolver.Result.none()) + .setCheckAddPatchSetPermission(false); if (groups != null) { psInserter.setGroups(groups); } diff --git a/java/com/google/gerrit/server/restapi/change/DeleteReviewerByEmailOp.java b/java/com/google/gerrit/server/restapi/change/DeleteReviewerByEmailOp.java index 7a41f84aaf..42748cb4d8 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteReviewerByEmailOp.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteReviewerByEmailOp.java @@ -100,8 +100,7 @@ public class DeleteReviewerByEmailOp implements BatchUpdateOp { cm.setFrom(ctx.getAccountId()); cm.addReviewersByEmail(Collections.singleton(reviewer)); cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn()); - cm.setNotify(notify.handling()); - cm.setAccountsToNotify(notify.accounts()); + cm.setNotify(notify); cm.send(); } catch (Exception err) { logger.atSevere().withCause(err).log("Cannot email update for change %s", change.getId()); diff --git a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java index fb4c96ec10..3e265ba247 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java @@ -223,8 +223,7 @@ public class DeleteReviewerOp implements BatchUpdateOp { cm.setFrom(userId); cm.addReviewers(Collections.singleton(reviewer.getAccount().getId())); cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn()); - cm.setNotify(notify.handling()); - cm.setAccountsToNotify(notify.accounts()); + cm.setNotify(notify); cm.send(); } } diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java index 2de2f99ea9..dd617381e0 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java @@ -226,8 +226,7 @@ public class DeleteVote extends RetryingRestModifyView accountsToNotify; + private final NotifyResolver.Result notify; private IdentifiedUser user; private ChangeNotes notes; @@ -842,14 +840,11 @@ public class PostReview private Map oldApprovals = new HashMap<>(); private Op( - ProjectState projectState, - PatchSet.Id psId, - ReviewInput in, - ListMultimap accountsToNotify) { + ProjectState projectState, PatchSet.Id psId, ReviewInput in, NotifyResolver.Result notify) { this.projectState = projectState; this.psId = psId; this.in = in; - this.accountsToNotify = requireNonNull(accountsToNotify); + this.notify = requireNonNull(notify); } @Override @@ -872,18 +867,9 @@ public class PostReview if (message == null) { return; } - if (in.notify.compareTo(NotifyHandling.NONE) > 0 || !accountsToNotify.isEmpty()) { + if (notify.shouldNotify()) { email - .create( - in.notify, - accountsToNotify, - notes, - ps, - user, - message, - comments, - in.message, - labelDelta) + .create(notify, notes, ps, user, message, comments, in.message, labelDelta) .sendAsync(); } commentAdded.fire( diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java index 10192a96fa..aef6cafaba 100644 --- a/java/com/google/gerrit/server/restapi/change/PutMessage.java +++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java @@ -141,8 +141,7 @@ public class PutMessage String.format("Patch Set %s: Commit message was updated.", psId.getId())); inserter.setDescription("Edit commit message"); NotifyResolver.Result notify = resolveNotify(input, resource); - inserter.setNotify(notify.handling()); - inserter.setAccountsToNotify(notify.accounts()); + inserter.setNotify(notify); bu.addOp(resource.getChange().getId(), inserter); bu.execute(); } diff --git a/java/com/google/gerrit/server/restapi/change/Revert.java b/java/com/google/gerrit/server/restapi/change/Revert.java index 492e7410bd..def107eefd 100644 --- a/java/com/google/gerrit/server/restapi/change/Revert.java +++ b/java/com/google/gerrit/server/restapi/change/Revert.java @@ -19,10 +19,8 @@ import static com.google.gerrit.extensions.conditions.BooleanCondition.and; import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; import com.google.common.base.Strings; -import com.google.common.collect.ListMultimap; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.RevertInput; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -226,8 +224,7 @@ public class Revert extends RetryingRestModifyView accountsToNotify; + private final NotifyResolver.Result notify; - NotifyOp( - Change change, - ChangeInserter ins, - NotifyHandling notifyHandling, - ListMultimap accountsToNotify) { + NotifyOp(Change change, ChangeInserter ins, NotifyResolver.Result notify) { this.change = change; this.ins = ins; - this.notifyHandling = notifyHandling; - this.accountsToNotify = accountsToNotify; + this.notify = notify; } @Override @@ -298,8 +289,7 @@ public class Revert extends RetryingRestModifyView accountsToNotify); + NotifyResolver.Result notify); } private final ExecutorService sendEmailsExecutor; @@ -54,8 +51,7 @@ class EmailMerge implements Runnable, RequestContext { private final Project.NameKey project; private final Change.Id changeId; private final Account.Id submitter; - private final NotifyHandling notifyHandling; - private final ListMultimap accountsToNotify; + private final NotifyResolver.Result notify; @Inject EmailMerge( @@ -66,8 +62,7 @@ class EmailMerge implements Runnable, RequestContext { @Assisted Project.NameKey project, @Assisted Change.Id changeId, @Assisted @Nullable Account.Id submitter, - @Assisted NotifyHandling notifyHandling, - @Assisted ListMultimap accountsToNotify) { + @Assisted NotifyResolver.Result notify) { this.sendEmailsExecutor = executor; this.mergedSenderFactory = mergedSenderFactory; this.requestContext = requestContext; @@ -75,8 +70,7 @@ class EmailMerge implements Runnable, RequestContext { this.project = project; this.changeId = changeId; this.submitter = submitter; - this.notifyHandling = notifyHandling; - this.accountsToNotify = accountsToNotify; + this.notify = notify; } void sendAsync() { @@ -92,8 +86,7 @@ class EmailMerge implements Runnable, RequestContext { if (submitter != null) { cm.setFrom(submitter); } - cm.setNotify(notifyHandling); - cm.setAccountsToNotify(accountsToNotify); + cm.setNotify(notify); cm.send(); } catch (Exception e) { logger.atSevere().withCause(e).log("Cannot email merged notification for %s", changeId); diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index bc9109b0dd..5adefe6c72 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -501,12 +501,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { // have failed fast in one of the other steps. try { args.mergedSenderFactory - .create( - ctx.getProject(), - getId(), - submitter.getAccountId(), - args.notify.handling(), - args.notify.accounts()) + .create(ctx.getProject(), getId(), submitter.getAccountId(), args.notify) .sendAsync(); } catch (Exception e) { logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId()); diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index 8bc78c3a6a..148e15dcf0 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -26,7 +26,6 @@ import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.changes.FixInput; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ProblemInfo; @@ -38,6 +37,7 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ConsistencyChecker; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; @@ -753,7 +753,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { changeInserterFactory .create(id, commit, dest) .setValidate(false) - .setNotify(NotifyHandling.NONE) + .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(false) .setSendMail(false); bu.insertChange(ins).execute(); @@ -778,7 +778,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { .create(notes, nextPatchSetId(notes), commit) .setValidate(false) .setFireRevisionCreated(false) - .setNotify(NotifyHandling.NONE); + .setNotify(NotifyResolver.Result.none()); bu.addOp(notes.getChangeId(), ins).execute(); } return reload(notes); diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index af9a006be3..7f906549e6 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -49,7 +49,6 @@ import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.Changes.QueryRequest; import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.HashtagsInput; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; import com.google.gerrit.extensions.api.changes.StarsInput; @@ -90,6 +89,7 @@ import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeTriplet; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.meta.MetaDataUpdate; @@ -3176,7 +3176,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { PatchSetInserter inserter = patchSetFactory .create(changeNotesFactory.createChecked(c), new PatchSet.Id(c.getId(), n), commit) - .setNotify(NotifyHandling.NONE) + .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(false) .setValidate(false); try (BatchUpdate bu = updateFactory.create(c.getProject(), user, TimeUtil.nowTs());