Consistently pass NotifyResolver.Result instead of separate args

Change-Id: I457fa6cc45002a88440e33adf609b52f8610b9eb
This commit is contained in:
Dave Borowitz
2019-01-30 13:47:50 -08:00
parent 3949eea76e
commit 8e24b76649
33 changed files with 118 additions and 220 deletions

View File

@@ -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());

View File

@@ -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);

View File

@@ -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<ChangeData> 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<ChangeData> changes)
throws RestApiException, UpdateException {
batchAbandon(
updateFactory,
project,
user,
changes,
"",
NotifyResolver.Result.create(NotifyHandling.ALL));
batchAbandon(updateFactory, project, user, changes, "", NotifyResolver.Result.all());
}
}

View File

@@ -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<String> groups = Collections.emptyList();
private boolean validate = true;
private NotifyHandling notify = NotifyHandling.ALL;
private ListMultimap<RecipientType, Account.Id> accountsToNotify = ImmutableListMultimap.of();
private NotifyResolver.Result notify = NotifyResolver.Result.all();
private Map<String, Short> 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<RecipientType, Account.Id> accountsToNotify) {
this.accountsToNotify = requireNonNull(accountsToNotify);
return this;
}
public ChangeInserter setReviewersAndCcs(
Iterable<Account.Id> reviewers, Iterable<Account.Id> 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)

View File

@@ -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));

View File

@@ -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<RecipientType, Account.Id> 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<RecipientType, Account.Id> 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<RecipientType, Account.Id> 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());

View File

@@ -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<RecipientType, Account.Id> recipients) {
return new AutoValue_NotifyResolver_Result(handling, recipients);
}

View File

@@ -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<String> groups = Collections.emptyList();
private boolean fireRevisionCreated = true;
private NotifyHandling notify = NotifyHandling.ALL;
private ListMultimap<RecipientType, Account.Id> 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<RecipientType, Account.Id> 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(

View File

@@ -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);

View File

@@ -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(),

View File

@@ -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(": ");

View File

@@ -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<RevisionCreatedListener> 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());

View File

@@ -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<RecipientType, Account.Id> getAccountsToNotify() {
ListMultimap<RecipientType, Account.Id> 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.<RecipientType, Account.Id>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));

View File

@@ -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(

View File

@@ -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(),

View File

@@ -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<Account.Id> getAuthors() {
Set<Account.Id> authors = new HashSet<>();
switch (notify) {
switch (notify.handling()) {
case NONE:
break;
case ALL:

View File

@@ -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());
}

View File

@@ -59,7 +59,7 @@ public abstract class NewChangeSender extends ChangeEmail {
setHeader("Message-ID", getChangeMessageThreadId());
switch (notify) {
switch (notify.handling()) {
case NONE:
case OWNER:
break;

View File

@@ -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<RecipientType, Account.Id> accountsToNotify = ImmutableListMultimap.of();
protected Map<String, Object> soyContext;
protected Map<String, Object> soyContextEmailData;
protected List<String> 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<RecipientType, Account.Id> 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)) {

View File

@@ -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);
}

View File

@@ -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));

View File

@@ -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);

View File

@@ -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);
}

View File

@@ -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());

View File

@@ -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();
}
}

View File

@@ -226,8 +226,7 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
ReplyToChangeSender cm = deleteVoteSenderFactory.create(ctx.getProject(), change.getId());
cm.setFrom(user.getAccountId());
cm.setChangeMessage(changeMessage.getMessage(), ctx.getWhen());
cm.setNotify(notify.handling());
cm.setAccountsToNotify(notify.accounts());
cm.setNotify(notify);
cm.send();
}
} catch (Exception e) {

View File

@@ -29,7 +29,6 @@ import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
@@ -42,7 +41,6 @@ import com.google.gerrit.common.data.LabelTypes;
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.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
@@ -367,7 +365,7 @@ public class PostReview
// Add the review op.
bu.addOp(
revision.getChange().getId(),
new Op(projectState, revision.getPatchSet().getId(), input, notify.accounts()));
new Op(projectState, revision.getPatchSet().getId(), input, notify));
bu.execute();
@@ -830,7 +828,7 @@ public class PostReview
private final ProjectState projectState;
private final PatchSet.Id psId;
private final ReviewInput in;
private final ListMultimap<RecipientType, Account.Id> accountsToNotify;
private final NotifyResolver.Result notify;
private IdentifiedUser user;
private ChangeNotes notes;
@@ -842,14 +840,11 @@ public class PostReview
private Map<String, Short> oldApprovals = new HashMap<>();
private Op(
ProjectState projectState,
PatchSet.Id psId,
ReviewInput in,
ListMultimap<RecipientType, Account.Id> 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(

View File

@@ -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();
}

View File

@@ -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<ChangeResource, RevertInput,
.create(changeId, revertCommit, notes.getChange().getDest().get())
.setTopic(changeToRevert.getTopic());
ins.setMessage("Uploaded patch set 1.");
ins.setNotify(notify.handling());
ins.setAccountsToNotify(notify.accounts());
ins.setNotify(notify);
ReviewerSet reviewerSet = approvalsUtil.getReviewers(notes);
@@ -243,7 +240,7 @@ public class Revert extends RetryingRestModifyView<ChangeResource, RevertInput,
try (BatchUpdate bu = updateFactory.create(project, user, now)) {
bu.setRepository(git, revWalk, oi);
bu.insertChange(ins);
bu.addOp(changeId, new NotifyOp(changeToRevert, ins, notify.handling(), notify.accounts()));
bu.addOp(changeId, new NotifyOp(changeToRevert, ins, notify));
bu.addOp(changeToRevert.getId(), new PostRevertedMessageOp(computedChangeId));
bu.execute();
}
@@ -278,18 +275,12 @@ public class Revert extends RetryingRestModifyView<ChangeResource, RevertInput,
private class NotifyOp implements BatchUpdateOp {
private final Change change;
private final ChangeInserter ins;
private final NotifyHandling notifyHandling;
private final ListMultimap<RecipientType, Account.Id> accountsToNotify;
private final NotifyResolver.Result notify;
NotifyOp(
Change change,
ChangeInserter ins,
NotifyHandling notifyHandling,
ListMultimap<RecipientType, Account.Id> 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<ChangeResource, RevertInput,
try {
RevertedSender cm = revertedSenderFactory.create(ctx.getProject(), change.getId());
cm.setFrom(ctx.getAccountId());
cm.setNotify(notifyHandling);
cm.setAccountsToNotify(accountsToNotify);
cm.setNotify(notify);
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(

View File

@@ -14,16 +14,14 @@
package com.google.gerrit.server.submit;
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.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.change.NotifyResolver;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.MergedSender;
import com.google.gerrit.server.util.RequestContext;
@@ -42,8 +40,7 @@ class EmailMerge implements Runnable, RequestContext {
Project.NameKey project,
Change.Id changeId,
Account.Id submitter,
NotifyHandling notifyHandling,
ListMultimap<RecipientType, Account.Id> 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<RecipientType, Account.Id> 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<RecipientType, Account.Id> 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);

View File

@@ -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());

View File

@@ -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);

View File

@@ -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());