diff --git a/java/com/google/gerrit/server/change/AbandonOp.java b/java/com/google/gerrit/server/change/AbandonOp.java index 600f14d257..a43690c25d 100644 --- a/java/com/google/gerrit/server/change/AbandonOp.java +++ b/java/com/google/gerrit/server/change/AbandonOp.java @@ -45,7 +45,6 @@ public class AbandonOp implements BatchUpdateOp { private final ChangeAbandoned changeAbandoned; private final String msgTxt; - private final NotifyResolver.Result notify; private final AccountState accountState; private Change change; @@ -54,9 +53,7 @@ public class AbandonOp implements BatchUpdateOp { public interface Factory { AbandonOp create( - @Assisted @Nullable AccountState accountState, - @Assisted @Nullable String msgTxt, - @Assisted NotifyResolver.Result notify); + @Assisted @Nullable AccountState accountState, @Assisted @Nullable String msgTxt); } @Inject @@ -66,8 +63,7 @@ public class AbandonOp implements BatchUpdateOp { PatchSetUtil psUtil, ChangeAbandoned changeAbandoned, @Assisted @Nullable AccountState accountState, - @Assisted @Nullable String msgTxt, - @Assisted NotifyResolver.Result notify) { + @Assisted @Nullable String msgTxt) { this.abandonedSenderFactory = abandonedSenderFactory; this.cmUtil = cmUtil; this.psUtil = psUtil; @@ -75,7 +71,6 @@ public class AbandonOp implements BatchUpdateOp { this.accountState = accountState; this.msgTxt = Strings.nullToEmpty(msgTxt); - this.notify = notify; } @Nullable @@ -114,6 +109,7 @@ public class AbandonOp implements BatchUpdateOp { @Override public void postUpdate(Context ctx) throws OrmException { + NotifyResolver.Result notify = ctx.getNotify(change.getId()); try { ReplyToChangeSender cm = abandonedSenderFactory.create(ctx.getProject(), change.getId()); if (accountState != null) { diff --git a/java/com/google/gerrit/server/change/AddReviewersOp.java b/java/com/google/gerrit/server/change/AddReviewersOp.java index 6cb5896c24..4a97c30eca 100644 --- a/java/com/google/gerrit/server/change/AddReviewersOp.java +++ b/java/com/google/gerrit/server/change/AddReviewersOp.java @@ -65,14 +65,10 @@ public class AddReviewersOp implements BatchUpdateOp { * @param accountIds account IDs to add. * @param addresses email addresses to add. * @param state resulting reviewer state. - * @param notify notification handling. * @return batch update operation. */ AddReviewersOp create( - Set accountIds, - Collection
addresses, - ReviewerState state, - NotifyResolver.Result notify); + Set accountIds, Collection
addresses, ReviewerState state); } @AutoValue @@ -112,7 +108,6 @@ public class AddReviewersOp implements BatchUpdateOp { private final Set accountIds; private final Collection
addresses; private final ReviewerState state; - private final NotifyResolver.Result notify; // Unlike addedCCs, addedReviewers is a PatchSetApproval because the AddReviewerResult returned // via the REST API is supposed to include vote information. @@ -121,6 +116,7 @@ public class AddReviewersOp implements BatchUpdateOp { private Collection addedCCs = ImmutableList.of(); private Collection
addedCCsByEmail = ImmutableList.of(); + private boolean sendEmail = true; private Change change; private PatchSet patchSet; private Result opResult; @@ -135,8 +131,7 @@ public class AddReviewersOp implements BatchUpdateOp { AddReviewersEmail addReviewersEmail, @Assisted Set accountIds, @Assisted Collection
addresses, - @Assisted ReviewerState state, - @Assisted NotifyResolver.Result notify) { + @Assisted ReviewerState state) { checkArgument(state == REVIEWER || state == CC, "must be %s or %s: %s", REVIEWER, CC, state); this.approvalsUtil = approvalsUtil; this.psUtil = psUtil; @@ -148,7 +143,13 @@ public class AddReviewersOp implements BatchUpdateOp { this.accountIds = accountIds; this.addresses = addresses; this.state = state; - this.notify = notify; + } + + // TODO(dborowitz): This mutable setter is ugly, but a) it's less ugly than adding boolean args + // all the way through the constructor stack, and b) this class is slated to be completely + // rewritten. + public void suppressEmail() { + this.sendEmail = false; } void setPatchSet(PatchSet patchSet) { @@ -237,14 +238,16 @@ public class AddReviewersOp implements BatchUpdateOp { .setAddedCCs(addedCCs) .setAddedCCsByEmail(addedCCsByEmail) .build(); - addReviewersEmail.emailReviewers( - ctx.getUser().asIdentifiedUser(), - change, - Lists.transform(addedReviewers, PatchSetApproval::getAccountId), - addedCCs, - addedReviewersByEmail, - addedCCsByEmail, - notify); + if (sendEmail) { + addReviewersEmail.emailReviewers( + ctx.getUser().asIdentifiedUser(), + change, + Lists.transform(addedReviewers, PatchSetApproval::getAccountId), + addedCCs, + addedReviewersByEmail, + addedCCsByEmail, + ctx.getNotify(change.getId())); + } if (!addedReviewers.isEmpty()) { List reviewers = addedReviewers diff --git a/java/com/google/gerrit/server/change/BatchAbandon.java b/java/com/google/gerrit/server/change/BatchAbandon.java index 0b3d8c6a90..8c675312fe 100644 --- a/java/com/google/gerrit/server/change/BatchAbandon.java +++ b/java/com/google/gerrit/server/change/BatchAbandon.java @@ -56,6 +56,7 @@ public class BatchAbandon { } AccountState accountState = user.isIdentifiedUser() ? user.asIdentifiedUser().state() : null; try (BatchUpdate u = updateFactory.create(project, user, TimeUtil.nowTs())) { + u.setNotify(notify); for (ChangeData change : changes) { if (!project.equals(change.project())) { throw new ResourceConflictException( @@ -63,7 +64,7 @@ public class BatchAbandon { "Project name \"%s\" doesn't match \"%s\"", change.project().get(), project.get())); } - u.addOp(change.getId(), abandonOpFactory.create(accountState, msgTxt, notify)); + u.addOp(change.getId(), abandonOpFactory.create(accountState, msgTxt)); } u.execute(); } diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java index 822560ab7f..544edcc251 100644 --- a/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/java/com/google/gerrit/server/change/ChangeInserter.java @@ -120,7 +120,6 @@ public class ChangeInserter implements InsertChangeOp { private boolean workInProgress; private List groups = Collections.emptyList(); private boolean validate = true; - private NotifyResolver.Result notify = NotifyResolver.Result.all(); private Map approvals; private RequestScopePropagator requestScopePropagator; private boolean fireRevisionCreated; @@ -251,11 +250,6 @@ public class ChangeInserter implements InsertChangeOp { return this; } - public ChangeInserter setNotify(NotifyResolver.Result notify) { - this.notify = notify; - return this; - } - public ChangeInserter setReviewersAndCcs( Iterable reviewers, Iterable ccs) { return setReviewersAndCcsAsStrings( @@ -447,6 +441,7 @@ public class ChangeInserter implements InsertChangeOp { @Override public void postUpdate(Context ctx) throws Exception { reviewerAdditions.postUpdate(ctx); + NotifyResolver.Result notify = ctx.getNotify(change.getId()); if (sendMail && notify.shouldNotify()) { Runnable sender = new Runnable() { diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java index 316acb3c95..7b911c4c67 100644 --- a/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -531,12 +531,12 @@ public class ConsistencyChecker { } } + bu.setNotify(NotifyResolver.Result.none()); bu.addOp( notes.getChangeId(), inserter .setValidate(false) .setFireRevisionCreated(false) - .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/NotifyResolver.java b/java/com/google/gerrit/server/change/NotifyResolver.java index d75ba2b26c..817fd7fd34 100644 --- a/java/com/google/gerrit/server/change/NotifyResolver.java +++ b/java/com/google/gerrit/server/change/NotifyResolver.java @@ -63,6 +63,10 @@ public class NotifyResolver { // TODO(dborowitz): Should be ImmutableSetMultimap. public abstract ImmutableListMultimap accounts(); + public Result withHandling(NotifyHandling notifyHandling) { + return create(notifyHandling, accounts()); + } + public boolean shouldNotify() { return !accounts().isEmpty() || handling().compareTo(NotifyHandling.NONE) > 0; } diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java index d51c065541..f62d943af1 100644 --- a/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -91,7 +91,6 @@ public class PatchSetInserter implements BatchUpdateOp { private boolean checkAddPatchSetPermission = true; private List groups = Collections.emptyList(); private boolean fireRevisionCreated = true; - private NotifyResolver.Result notify = NotifyResolver.Result.all(); private boolean allowClosed; // Fields set during some phase of BatchUpdate.Op. @@ -165,11 +164,6 @@ public class PatchSetInserter implements BatchUpdateOp { return this; } - public PatchSetInserter setNotify(NotifyResolver.Result notify) { - this.notify = requireNonNull(notify); - return this; - } - public PatchSetInserter setAllowClosed(boolean allowClosed) { this.allowClosed = allowClosed; return this; @@ -218,7 +212,7 @@ public class PatchSetInserter implements BatchUpdateOp { psUtil.insert( ctx.getRevWalk(), ctx.getUpdate(psId), psId, commitId, newGroups, null, description); - if (notify.handling() != NotifyHandling.NONE) { + if (ctx.getNotify(change.getId()).handling() != NotifyHandling.NONE) { oldReviewers = approvalsUtil.getReviewers(ctx.getNotes()); } @@ -247,6 +241,7 @@ public class PatchSetInserter implements BatchUpdateOp { @Override public void postUpdate(Context ctx) throws OrmException { + NotifyResolver.Result notify = ctx.getNotify(change.getId()); if (notify.shouldNotify()) { try { ReplacePatchSetSender cm = replacePatchSetFactory.create(ctx.getProject(), change.getId()); diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java index 39c1adeb32..61bdc76a4e 100644 --- a/java/com/google/gerrit/server/change/RebaseChangeOp.java +++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -182,7 +182,6 @@ public class RebaseChangeOp implements BatchUpdateOp { patchSetInserterFactory .create(notes, rebasedPatchSetId, rebasedCommit) .setDescription("Rebase") - .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(fireRevisionCreated) .setCheckAddPatchSetPermission(checkAddPatchSetPermission) .setValidate(validate); diff --git a/java/com/google/gerrit/server/change/ReviewerAdder.java b/java/com/google/gerrit/server/change/ReviewerAdder.java index 6bfffa73d6..6dd0db829a 100644 --- a/java/com/google/gerrit/server/change/ReviewerAdder.java +++ b/java/com/google/gerrit/server/change/ReviewerAdder.java @@ -38,7 +38,6 @@ import com.google.gerrit.extensions.api.changes.ReviewerInfo; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.AuthException; -import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.mail.Address; @@ -148,7 +147,6 @@ public class ReviewerAdder { private final AccountLoader.Factory accountLoaderFactory; private final Config cfg; private final ReviewerJson json; - private final NotifyResolver notifyResolver; private final ProjectCache projectCache; private final Provider anonymousProvider; private final AddReviewersOp.Factory addReviewersOpFactory; @@ -163,7 +161,6 @@ public class ReviewerAdder { AccountLoader.Factory accountLoaderFactory, @GerritServerConfig Config cfg, ReviewerJson json, - NotifyResolver notifyResolver, ProjectCache projectCache, Provider anonymousProvider, AddReviewersOp.Factory addReviewersOpFactory, @@ -175,7 +172,6 @@ public class ReviewerAdder { this.accountLoaderFactory = accountLoaderFactory; this.cfg = cfg; this.json = json; - this.notifyResolver = notifyResolver; this.projectCache = projectCache; this.anonymousProvider = anonymousProvider; this.addReviewersOpFactory = addReviewersOpFactory; @@ -201,23 +197,17 @@ public class ReviewerAdder { ChangeNotes notes, CurrentUser user, AddReviewerInput input, boolean allowGroup) throws OrmException, IOException, PermissionBackendException, ConfigInvalidException { requireNonNull(input.reviewer); - NotifyResolver.Result notify; - try { - notify = resolveNotify(notes, input); - } catch (BadRequestException e) { - return fail(input, FailureType.OTHER, e.getMessage()); - } boolean confirmed = input.confirmed(); boolean allowByEmail = projectCache .checkedGet(notes.getProjectName()) .is(BooleanProjectConfig.ENABLE_REVIEWER_BY_EMAIL); - ReviewerAddition byAccountId = addByAccountId(input, notes, user, notify); + ReviewerAddition byAccountId = addByAccountId(input, notes, user); ReviewerAddition wholeGroup = null; if (!byAccountId.exactMatchFound) { - wholeGroup = addWholeGroup(input, notes, user, notify, confirmed, allowGroup, allowByEmail); + wholeGroup = addWholeGroup(input, notes, user, confirmed, allowGroup, allowByEmail); if (wholeGroup != null && wholeGroup.exactMatchFound) { return wholeGroup; } @@ -239,17 +229,7 @@ public class ReviewerAdder { return wholeGroup; } - return addByEmail(input, notes, user, notify); - } - - private NotifyResolver.Result resolveNotify(ChangeNotes notes, AddReviewerInput input) - throws BadRequestException, OrmException, ConfigInvalidException, IOException { - NotifyHandling notifyHandling = input.notify; - if (notifyHandling == null) { - notifyHandling = - notes.getChange().isWorkInProgress() ? NotifyHandling.NONE : NotifyHandling.ALL; - } - return notifyResolver.resolve(notifyHandling, input.notifyDetails); + return addByEmail(input, notes, user); } public ReviewerAddition ccCurrentUser(CurrentUser user, RevisionResource revision) { @@ -259,13 +239,12 @@ public class ReviewerAdder { revision.getUser(), ImmutableSet.of(user.getAccountId()), null, - NotifyResolver.Result.none(), true); } @Nullable private ReviewerAddition addByAccountId( - AddReviewerInput input, ChangeNotes notes, CurrentUser user, NotifyResolver.Result notify) + AddReviewerInput input, ChangeNotes notes, CurrentUser user) throws OrmException, PermissionBackendException, IOException, ConfigInvalidException { IdentifiedUser reviewerUser; boolean exactMatchFound = false; @@ -283,13 +262,7 @@ public class ReviewerAdder { if (isValidReviewer(notes.getChange().getDest(), reviewerUser.getAccount())) { return new ReviewerAddition( - input, - notes, - user, - ImmutableSet.of(reviewerUser.getAccountId()), - null, - notify, - exactMatchFound); + input, notes, user, ImmutableSet.of(reviewerUser.getAccountId()), null, exactMatchFound); } return fail( input, @@ -302,7 +275,6 @@ public class ReviewerAdder { AddReviewerInput input, ChangeNotes notes, CurrentUser user, - NotifyResolver.Result notify, boolean confirmed, boolean allowGroup, boolean allowByEmail) @@ -374,12 +346,11 @@ public class ReviewerAdder { } } - return new ReviewerAddition(input, notes, user, reviewers, null, notify, true); + return new ReviewerAddition(input, notes, user, reviewers, null, true); } @Nullable - private ReviewerAddition addByEmail( - AddReviewerInput input, ChangeNotes notes, CurrentUser user, NotifyResolver.Result notify) + private ReviewerAddition addByEmail(AddReviewerInput input, ChangeNotes notes, CurrentUser user) throws PermissionBackendException { try { permissionBackend.user(anonymousProvider.get()).change(notes).check(ChangePermission.READ); @@ -397,7 +368,7 @@ public class ReviewerAdder { FailureType.NOT_FOUND, MessageFormat.format(ChangeMessages.get().reviewerInvalid, input.reviewer)); } - return new ReviewerAddition(input, notes, user, null, ImmutableList.of(adr), notify, true); + return new ReviewerAddition(input, notes, user, null, ImmutableList.of(adr), true); } private boolean isValidReviewer(Branch.NameKey branch, Account member) @@ -452,7 +423,6 @@ public class ReviewerAdder { CurrentUser caller, @Nullable Iterable reviewers, @Nullable Iterable
reviewersByEmail, - NotifyResolver.Result notify, boolean exactMatchFound) { checkArgument( reviewers != null || reviewersByEmail != null, @@ -467,7 +437,7 @@ public class ReviewerAdder { this.reviewersByEmail = reviewersByEmail == null ? ImmutableSet.of() : ImmutableSet.copyOf(reviewersByEmail); this.caller = caller.asIdentifiedUser(); - op = addReviewersOpFactory.create(this.reviewers, this.reviewersByEmail, state(), notify); + op = addReviewersOpFactory.create(this.reviewers, this.reviewersByEmail, state()); this.exactMatchFound = exactMatchFound; } @@ -557,7 +527,12 @@ public class ReviewerAdder { .collect(toImmutableList()); List additions = new ArrayList<>(); for (AddReviewerInput input : sorted) { - additions.add(prepare(notes, user, input, allowGroup)); + ReviewerAddition addition = prepare(notes, user, input, allowGroup); + if (addition.op != null) { + // Assume any callers preparing a list of batch insertions are handling their own email. + addition.op.suppressEmail(); + } + additions.add(addition); } return new ReviewerAdditionList(additions); } diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java index cbcf9cb11f..02870fb099 100644 --- a/java/com/google/gerrit/server/change/WorkInProgressOp.java +++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java @@ -14,7 +14,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.gerrit.common.Nullable; @@ -91,9 +90,9 @@ public class WorkInProgressOp implements BatchUpdateOp { private final PatchSetUtil psUtil; private final boolean workInProgress; private final Input in; - private final NotifyResolver.Result notify; private final WorkInProgressStateChanged stateChanged; + private boolean sendEmail = true; private Change change; private ChangeNotes notes; private PatchSet ps; @@ -113,10 +112,10 @@ public class WorkInProgressOp implements BatchUpdateOp { this.stateChanged = stateChanged; this.workInProgress = workInProgress; this.in = in; - notify = - NotifyResolver.Result.create( - MoreObjects.firstNonNull( - in.notify, workInProgress ? NotifyHandling.NONE : NotifyHandling.ALL)); + } + + public void suppressEmail() { + this.sendEmail = false; } @Override @@ -160,7 +159,10 @@ public class WorkInProgressOp implements BatchUpdateOp { @Override public void postUpdate(Context ctx) { stateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen()); - if (workInProgress || notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) < 0) { + NotifyResolver.Result notify = ctx.getNotify(change.getId()); + if (workInProgress + || notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) < 0 + || !sendEmail) { return; } email diff --git a/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 7b838bdbf7..898f42703c 100644 --- a/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -169,8 +169,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); + PatchSetInserter inserter = patchSetInserterFactory.create(notes, psId, squashed); StringBuilder message = new StringBuilder("Patch Set ").append(inserter.getPatchSetId().get()).append(": "); @@ -191,6 +190,7 @@ public class ChangeEditUtil { try (BatchUpdate bu = updateFactory.create(change.getProject(), user, TimeUtil.nowTs())) { bu.setRepository(repo, rw, oi); + bu.setNotify(notify); bu.addOp(change.getId(), inserter.setMessage(message.toString())); bu.addOp( change.getId(), diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index deefd01aff..f1a4f01c15 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -682,7 +682,7 @@ class ReceiveCommits { // Update superproject gitlinks if required. if (!branches.isEmpty()) { try (MergeOpRepoManager orm = ormProvider.get()) { - orm.setContext(TimeUtil.nowTs(), user); + orm.setContext(TimeUtil.nowTs(), user, NotifyResolver.Result.none()); SubmoduleOp op = subOpFactory.create(branches, orm); op.updateSuperProjects(); } catch (SubmoduleException e) { @@ -787,9 +787,15 @@ class ReceiveCommits { RevWalk rw = new RevWalk(reader)) { bu.setRepository(repo, rw, ins); bu.setRefLogMessage("push"); + if (magicBranch != null) { + bu.setNotify(magicBranch.getNotifyForNewChange()); + } logger.atFine().log("Adding %d replace requests", newChanges.size()); for (ReplaceRequest replace : replaceByChange.values()) { + if (magicBranch != null) { + bu.setNotifyHandling(replace.ontoChange, magicBranch.getNotifyHandling(replace.notes)); + } replace.addOps(bu, replaceProgress); } @@ -1578,31 +1584,25 @@ class ReceiveCommits { } NotifyResolver.Result getNotifyForNewChange() { - return getNotifyImpl(null); - } - - 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; - } - } - return NotifyResolver.Result.create( - notifyHandling, + firstNonNull(notifyHandling, workInProgress ? NotifyHandling.OWNER : NotifyHandling.ALL), ImmutableListMultimap.builder() .putAll(RecipientType.TO, notifyTo) .putAll(RecipientType.CC, notifyCc) .putAll(RecipientType.BCC, notifyBcc) .build()); } + + NotifyHandling getNotifyHandling(ChangeNotes notes) { + requireNonNull(notes); + if (notifyHandling != null) { + return notifyHandling; + } + if (workInProgress || (!ready && notes.getChange().isWorkInProgress())) { + return NotifyHandling.OWNER; + } + return NotifyHandling.ALL; + } } /** @@ -2440,13 +2440,13 @@ class ReceiveCommits { msg.append("\n").append(magicBranch.message); } + bu.setNotify(magicBranch.getNotifyForNewChange()); bu.insertChange( ins.setReviewersAndCcsAsStrings( magicBranch.getCombinedReviewers(fromFooters), magicBranch.getCombinedCcs(fromFooters)) .setApprovals(approvals) .setMessage(msg.toString()) - .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 430eac9fd6..84bab4a462 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -512,8 +512,7 @@ public class ReplaceOp implements BatchUpdateOp { } } - NotifyResolver.Result notify = - magicBranch != null ? magicBranch.getNotify(notes) : NotifyResolver.Result.all(); + NotifyResolver.Result notify = ctx.getNotify(notes.getChangeId()); if (shouldPublishComments()) { emailCommentsFactory .create( @@ -554,9 +553,7 @@ public class ReplaceOp implements BatchUpdateOp { cm.setFrom(ctx.getAccount().getAccount().getId()); cm.setPatchSet(newPatchSet, info); cm.setChangeMessage(msg.getMessage(), ctx.getWhen()); - if (magicBranch != null) { - cm.setNotify(magicBranch.getNotify(notes)); - } + cm.setNotify(ctx.getNotify(notes.getChangeId())); cm.addReviewers( Streams.concat( oldRecipients.getReviewers().stream(), diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java index d0ae39f35a..e087325701 100644 --- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java +++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java @@ -45,7 +45,6 @@ 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; @@ -314,7 +313,7 @@ public class MailProcessor { // Send email notifications outgoingMailFactory .create( - NotifyResolver.Result.all(), + ctx.getNotify(notes.getChangeId()), notes, patchSet, ctx.getUser().asIdentifiedUser(), diff --git a/java/com/google/gerrit/server/restapi/change/Abandon.java b/java/com/google/gerrit/server/restapi/change/Abandon.java index 7793b589eb..05de9e420e 100644 --- a/java/com/google/gerrit/server/restapi/change/Abandon.java +++ b/java/com/google/gerrit/server/restapi/change/Abandon.java @@ -120,8 +120,9 @@ public class Abandon extends RetryingRestModifyView user; private final DeleteReviewerSender.Factory deleteReviewerSenderFactory; - private final NotifyResolver notifyResolver; private final RemoveReviewerControl removeReviewerControl; private final ProjectCache projectCache; @@ -91,7 +90,6 @@ public class DeleteReviewerOp implements BatchUpdateOp { ReviewerDeleted reviewerDeleted, Provider user, DeleteReviewerSender.Factory deleteReviewerSenderFactory, - NotifyResolver notifyResolver, RemoveReviewerControl removeReviewerControl, ProjectCache projectCache, @Assisted AccountState reviewerAccount, @@ -103,7 +101,6 @@ public class DeleteReviewerOp implements BatchUpdateOp { this.reviewerDeleted = reviewerDeleted; this.user = user; this.deleteReviewerSenderFactory = deleteReviewerSenderFactory; - this.notifyResolver = notifyResolver; this.removeReviewerControl = removeReviewerControl; this.projectCache = projectCache; this.reviewer = reviewerAccount; @@ -170,15 +167,16 @@ public class DeleteReviewerOp implements BatchUpdateOp { @Override public void postUpdate(Context ctx) { - if (input.notify == null) { - if (currChange.isWorkInProgress()) { - input.notify = oldApprovals.isEmpty() ? NotifyHandling.NONE : NotifyHandling.OWNER; - } else { - input.notify = NotifyHandling.ALL; - } + NotifyResolver.Result notify = ctx.getNotify(currChange.getId()); + if (input.notify == null + && currChange.isWorkInProgress() + && !oldApprovals.isEmpty() + && notify.handling().compareTo(NotifyHandling.OWNER) < 0) { + // Override NotifyHandling from the context to notify owner if votes were removed on a WIP + // change. + notify = notify.withHandling(NotifyHandling.OWNER); } try { - NotifyResolver.Result notify = notifyResolver.resolve(input.notify, input.notifyDetails); if (notify.shouldNotify()) { emailReviewers(ctx.getProject(), currChange, changeMessage, notify); } @@ -193,7 +191,7 @@ public class DeleteReviewerOp implements BatchUpdateOp { changeMessage.getMessage(), newApprovals, oldApprovals, - input.notify, + notify.handling(), ctx.getWhen()); } diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java index dd617381e0..3a167bf46c 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java @@ -62,6 +62,7 @@ import com.google.inject.Singleton; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import org.eclipse.jgit.errors.ConfigInvalidException; @Singleton public class DeleteVote extends RetryingRestModifyView> { @@ -104,7 +105,7 @@ public class DeleteVote extends RetryingRestModifyView applyImpl( BatchUpdate.Factory updateFactory, VoteResource rsrc, DeleteVoteInput input) - throws RestApiException, UpdateException, IOException { + throws RestApiException, UpdateException, IOException, OrmException, ConfigInvalidException { if (input == null) { input = new DeleteVoteInput(); } @@ -124,6 +125,9 @@ public class DeleteVote extends RetryingRestModifyView reviewerJsonResults = null; List reviewerResults = Lists.newArrayList(); boolean hasError = false; @@ -264,12 +262,6 @@ public class PostReview if (input.reviewers != null) { reviewerJsonResults = Maps.newHashMap(); for (AddReviewerInput reviewerInput : input.reviewers) { - // Prevent individual AddReviewersOps from sending one email each. Instead, we call - // batchEmailReviewers at the very end to send out a single email. - // TODO(dborowitz): I think this still sends out separate emails if any of input.reviewers - // specifies explicit accountsToNotify. Unclear whether that's a good thing. - reviewerInput.notify = NotifyHandling.NONE; - ReviewerAddition result = reviewerAdder.prepare(revision.getNotes(), revision.getUser(), reviewerInput, true); reviewerJsonResults.put(reviewerInput.reviewer, result.result); @@ -312,6 +304,7 @@ public class PostReview // updated set of reviewers. Also keep track of whether the user added // themselves as a reviewer or to the CC list. for (ReviewerAddition reviewerResult : reviewerResults) { + reviewerResult.op.suppressEmail(); // Send a single batch email below. bu.addOp(revision.getChange().getId(), reviewerResult.op); if (!ccOrReviewer && reviewerResult.result.reviewers != null) { for (ReviewerInfo reviewerInfo : reviewerResult.result.reviewers) { @@ -336,6 +329,7 @@ public class PostReview // isn't being explicitly added, and isn't voting on any label. // Automatically CC them on this change so they receive replies. ReviewerAddition selfAddition = reviewerAdder.ccCurrentUser(revision.getUser(), revision); + selfAddition.op.suppressEmail(); bu.addOp(revision.getChange().getId(), selfAddition.op); } @@ -353,19 +347,21 @@ public class PostReview output.ready = true; } - // Suppress notifications in WorkInProgressOp, we'll take care of - // them in this endpoint. - WorkInProgressOp.Input wipIn = new WorkInProgressOp.Input(); - wipIn.notify = NotifyHandling.NONE; - bu.addOp( - revision.getChange().getId(), - workInProgressOpFactory.create(input.workInProgress, wipIn)); + WorkInProgressOp wipOp = + workInProgressOpFactory.create(input.workInProgress, new WorkInProgressOp.Input()); + wipOp.suppressEmail(); + bu.addOp(revision.getChange().getId(), wipOp); } // Add the review op. bu.addOp( revision.getChange().getId(), - new Op(projectState, revision.getPatchSet().getId(), input, notify)); + new Op(projectState, revision.getPatchSet().getId(), input)); + + // Notify based on ReviewInput, ignoring the notify settings from any AddReviewerInputs. + NotifyResolver.Result notify = + notifyResolver.resolve(getNotifyHandling(input, output, revision), input.notifyDetails); + bu.setNotify(notify); bu.execute(); @@ -382,6 +378,17 @@ public class PostReview return Response.ok(output); } + private NotifyHandling getNotifyHandling( + ReviewInput input, ReviewResult output, RevisionResource revision) { + if (input.notify != null) { + return input.notify; + } + if ((output.ready != null && output.ready) || !revision.getChange().isWorkInProgress()) { + return NotifyHandling.ALL; + } + return NotifyHandling.NONE; + } + private NotifyHandling defaultNotify(Change c, ReviewInput in) { boolean workInProgress = c.isWorkInProgress(); if (in.workInProgress) { @@ -828,7 +835,6 @@ public class PostReview private final ProjectState projectState; private final PatchSet.Id psId; private final ReviewInput in; - private final NotifyResolver.Result notify; private IdentifiedUser user; private ChangeNotes notes; @@ -839,12 +845,10 @@ public class PostReview private Map approvals = new HashMap<>(); private Map oldApprovals = new HashMap<>(); - private Op( - ProjectState projectState, PatchSet.Id psId, ReviewInput in, NotifyResolver.Result notify) { + private Op(ProjectState projectState, PatchSet.Id psId, ReviewInput in) { this.projectState = projectState; this.psId = psId; this.in = in; - this.notify = requireNonNull(notify); } @Override @@ -867,6 +871,7 @@ public class PostReview if (message == null) { return; } + NotifyResolver.Result notify = ctx.getNotify(notes.getChangeId()); if (notify.shouldNotify()) { email .create(notify, notes, ps, user, message, comments, in.message, labelDelta) diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java index fdfefabb2c..4aeb07fc78 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java +++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java @@ -16,10 +16,12 @@ package com.google.gerrit.server.restapi.change; import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerResult; +import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.ReviewerAdder; import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition; import com.google.gerrit.server.change.ReviewerResource; @@ -42,13 +44,18 @@ public class PostReviewers ChangeResource, ReviewerResource, AddReviewerInput, AddReviewerResult> { private final ChangeData.Factory changeDataFactory; + private final NotifyResolver notifyResolver; private final ReviewerAdder reviewerAdder; @Inject PostReviewers( - ChangeData.Factory changeDataFactory, RetryHelper retryHelper, ReviewerAdder reviewerAdder) { + ChangeData.Factory changeDataFactory, + RetryHelper retryHelper, + NotifyResolver notifyResolver, + ReviewerAdder reviewerAdder) { super(retryHelper); this.changeDataFactory = changeDataFactory; + this.notifyResolver = notifyResolver; this.reviewerAdder = reviewerAdder; } @@ -67,6 +74,7 @@ public class PostReviewers } try (BatchUpdate bu = updateFactory.create(rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { + bu.setNotify(resolveNotify(rsrc, input)); Change.Id id = rsrc.getChange().getId(); bu.addOp(id, addition.op); bu.execute(); @@ -76,4 +84,14 @@ public class PostReviewers addition.gatherResults(changeDataFactory.create(rsrc.getProject(), rsrc.getId())); return addition.result; } + + private NotifyResolver.Result resolveNotify(ChangeResource rsrc, AddReviewerInput input) + throws BadRequestException, OrmException, ConfigInvalidException, IOException { + NotifyHandling notifyHandling = input.notify; + if (notifyHandling == null) { + notifyHandling = + rsrc.getChange().isWorkInProgress() ? NotifyHandling.NONE : NotifyHandling.ALL; + } + return notifyResolver.resolve(notifyHandling, input.notifyDetails); + } } diff --git a/java/com/google/gerrit/server/restapi/change/PutAssignee.java b/java/com/google/gerrit/server/restapi/change/PutAssignee.java index 8babb2d6e2..a7dcc12e46 100644 --- a/java/com/google/gerrit/server/restapi/change/PutAssignee.java +++ b/java/com/google/gerrit/server/restapi/change/PutAssignee.java @@ -99,6 +99,7 @@ public class PutAssignee extends RetryingRestModifyView(); } - public void setContext(Timestamp ts, IdentifiedUser caller) { - this.ts = ts; - this.caller = caller; + public void setContext(Timestamp ts, IdentifiedUser caller, NotifyResolver.Result notify) { + this.ts = requireNonNull(ts); + this.caller = requireNonNull(caller); + this.notify = requireNonNull(notify); } public OpenRepo getRepo(Project.NameKey project) throws NoSuchProjectException, IOException { @@ -200,7 +205,7 @@ public class MergeOpRepoManager implements AutoCloseable { throws NoSuchProjectException, IOException { List updates = new ArrayList<>(projects.size()); for (Project.NameKey project : projects) { - updates.add(getRepo(project).getUpdate().setRefLogMessage("merged")); + updates.add(getRepo(project).getUpdate().setNotify(notify).setRefLogMessage("merged")); } return updates; } diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java index 974f5b89db..37858d541a 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java @@ -29,7 +29,6 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.change.LabelNormalizer; -import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.RebaseChangeOp; import com.google.gerrit.server.change.TestSubmitInput; import com.google.gerrit.server.extensions.events.ChangeMerged; @@ -92,7 +91,6 @@ public abstract class SubmitStrategy { Set incoming, RequestId submissionId, SubmitInput submitInput, - NotifyResolver.Result notify, SubmoduleOp submoduleOp, boolean dryrun); } @@ -124,7 +122,6 @@ public abstract class SubmitStrategy { final RequestId submissionId; final SubmitType submitType; final SubmitInput submitInput; - final NotifyResolver.Result notify; final SubmoduleOp submoduleOp; final ProjectState project; @@ -163,7 +160,6 @@ public abstract class SubmitStrategy { @Assisted RequestId submissionId, @Assisted SubmitType submitType, @Assisted SubmitInput submitInput, - @Assisted NotifyResolver.Result notify, @Assisted SubmoduleOp submoduleOp, @Assisted boolean dryrun) { this.accountCache = accountCache; @@ -192,7 +188,6 @@ public abstract class SubmitStrategy { this.submissionId = submissionId; this.submitType = submitType; this.submitInput = submitInput; - this.notify = notify; this.submoduleOp = submoduleOp; this.dryrun = dryrun; diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java index 00ea7a315b..30326f773d 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java @@ -19,7 +19,6 @@ import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.MergeTip; @@ -55,7 +54,6 @@ public class SubmitStrategyFactory { CommitStatus commitStatus, RequestId submissionId, SubmitInput submitInput, - NotifyResolver.Result notify, SubmoduleOp submoduleOp, boolean dryrun) throws IntegrationException { @@ -72,7 +70,6 @@ public class SubmitStrategyFactory { incoming, submissionId, submitInput, - notify, submoduleOp, dryrun); switch (submitType) { diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index 5adefe6c72..a49ddff78d 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -501,7 +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) + .create(ctx.getProject(), getId(), submitter.getAccountId(), ctx.getNotify(getId())) .sendAsync(); } catch (Exception e) { logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId()); diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java index 06397d7487..176ffcfa17 100644 --- a/java/com/google/gerrit/server/update/BatchUpdate.java +++ b/java/com/google/gerrit/server/update/BatchUpdate.java @@ -30,6 +30,7 @@ import com.google.common.collect.MultimapBuilder; import com.google.common.collect.Multiset; 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.config.FactoryModule; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; @@ -40,6 +41,7 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.validators.OnSubmitValidators; @@ -229,6 +231,12 @@ public class BatchUpdate implements AutoCloseable { public CurrentUser getUser() { return user; } + + @Override + public NotifyResolver.Result getNotify(Change.Id changeId) { + NotifyHandling notifyHandling = perChangeNotifyHandling.get(changeId); + return notifyHandling != null ? notify.withHandling(notifyHandling) : notify; + } } private class RepoContextImpl extends ContextImpl implements RepoContext { @@ -302,12 +310,14 @@ public class BatchUpdate implements AutoCloseable { MultimapBuilder.linkedHashKeys().arrayListValues().build(); private final Map newChanges = new HashMap<>(); private final List repoOnlyOps = new ArrayList<>(); + private final Map perChangeNotifyHandling = new HashMap<>(); private RepoView repoView; private BatchRefUpdate batchRefUpdate; private OnSubmitValidators onSubmitValidators; private PushCertificate pushCert; private String refLogMessage; + private NotifyResolver.Result notify = NotifyResolver.Result.all(); @Inject BatchUpdate( @@ -364,6 +374,32 @@ public class BatchUpdate implements AutoCloseable { return this; } + /** + * Set the default notification settings for all changes in the batch. + * + * @param notify notification settings. + * @return this. + */ + public BatchUpdate setNotify(NotifyResolver.Result notify) { + this.notify = requireNonNull(notify); + return this; + } + + /** + * Override the {@link NotifyHandling} on a per-change basis. + * + *

Only the handling enum can be overridden; all changes share the same value for {@link + * NotifyResolver.Result#accounts()}. + * + * @param changeId change ID. + * @param notifyHandling notify handling. + * @return this. + */ + public BatchUpdate setNotifyHandling(Change.Id changeId, NotifyHandling notifyHandling) { + this.perChangeNotifyHandling.put(changeId, requireNonNull(notifyHandling)); + return this; + } + /** * Add a validation step for intended ref operations, which will be performed at the end of {@link * RepoOnlyOp#updateRepo(RepoContext)} step. diff --git a/java/com/google/gerrit/server/update/Context.java b/java/com/google/gerrit/server/update/Context.java index c24c650992..8704cf03fe 100644 --- a/java/com/google/gerrit/server/update/Context.java +++ b/java/com/google/gerrit/server/update/Context.java @@ -17,10 +17,12 @@ package com.google.gerrit.server.update; import static java.util.Objects.requireNonNull; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.change.NotifyResolver; import java.io.IOException; import java.sql.Timestamp; import java.util.TimeZone; @@ -85,6 +87,18 @@ public interface Context { */ CurrentUser getUser(); + /** + * Get the notification settings configured by the caller. + * + *

If there are multiple changes in a batch, they may have different settings. For example, WIP + * changes may have reduced {@code NotifyHandling} levels, and may be in a batch with non-WIP + * changes. + * + * @param changeId change ID + * @return notification settings. + */ + NotifyResolver.Result getNotify(Change.Id changeId); + /** * Get the identified user performing the update. * diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index 148e15dcf0..2ee2d46535 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -749,11 +749,11 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { ChangeInserter ins; try (BatchUpdate bu = newUpdate(owner.getId())) { RevCommit commit = patchSetCommit(new PatchSet.Id(id, 1)); + bu.setNotify(NotifyResolver.Result.none()); ins = changeInserterFactory .create(id, commit, dest) .setValidate(false) - .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(false) .setSendMail(false); bu.insertChange(ins).execute(); @@ -773,12 +773,12 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { private ChangeNotes incrementPatchSet(ChangeNotes notes, RevCommit commit) throws Exception { PatchSetInserter ins; try (BatchUpdate bu = newUpdate(notes.getChange().getOwner())) { + bu.setNotify(NotifyResolver.Result.none()); ins = patchSetInserterFactory .create(notes, nextPatchSetId(notes), commit) .setValidate(false) - .setFireRevisionCreated(false) - .setNotify(NotifyResolver.Result.none()); + .setFireRevisionCreated(false); 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 7f906549e6..e40cfdb25a 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -3176,7 +3176,6 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { PatchSetInserter inserter = patchSetFactory .create(changeNotesFactory.createChecked(c), new PatchSet.Id(c.getId(), n), commit) - .setNotify(NotifyResolver.Result.none()) .setFireRevisionCreated(false) .setValidate(false); try (BatchUpdate bu = updateFactory.create(c.getProject(), user, TimeUtil.nowTs()); @@ -3184,6 +3183,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { ObjectReader reader = oi.newReader(); RevWalk rw = new RevWalk(reader)) { bu.setRepository(repo.getRepository(), rw, oi); + bu.setNotify(NotifyResolver.Result.none()); bu.addOp(c.getId(), inserter); bu.execute(); }