From cd07df425dda1a4636f8165e24f8a9daa9a88a16 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Thu, 1 Dec 2016 09:10:09 +0100 Subject: [PATCH] Allow to explicitly specify whom to notify on a change update When pushing changes/patch sets and when making change updates through the REST API callers can specify a 'notify' option to control who should be notified (not all REST endpoints support this option). This 'notify' option allows to choose if owner, reviewers, watchers and users that have starred the change should be notified, or if no notification should be send. For some tools this is not sufficient as they want to have full control over which users should be notified. E.g. if a CI integration is triggered by a certain label, the owner and the user that triggered the CI should be notified. To support this the REST API input entities that support the 'notify' option are extended by a 'notify_details' options that allows the caller to specify accounts that should be notified. These accounts are always notified, even if notifications are otherwise disabled by setting 'notify' to 'NONE'. The 'notify_details' option is a map that maps a recipient type to a 'NotifyInfo' entity. This means callers can control how the accounts should be notified, as TO, CC or BCC. The 'NotifyInfo' entity hosts the list of accounts that should be notified. It is an extra entity so that further entities that should be notified can be added in future. E.g. one could imagine to add a list of emails (for user without Gerrit account), a list of groups or a list of star labels (notify all users that have set this star label on the change). Also when pushing commits for review, specific accounts can be explicitly notified by setting 'notify-to=', 'notify-cc=' or 'notify-bcc=' in the push specification: git push origin HEAD:refs/for/master%notify-to=foo@bar.com These accounts are not added as reviewer or CC so that they are not signed up to receive notifications for futher updates of the change. Change-Id: Id112b70158b80f84f631b5022f33228871716108 Signed-off-by: Edwin Kempin --- Documentation/rest-api-changes.txt | 96 +++++++++----- Documentation/user-upload.txt | 18 +++ .../gerrit/acceptance/AbstractDaemonTest.java | 35 +++++ .../acceptance/api/change/ChangeIT.java | 69 ++++++++-- .../gerrit/acceptance/edit/ChangeEditIT.java | 19 ++- .../acceptance/git/AbstractPushForReview.java | 37 ++++++ .../extensions/api/changes/AbandonInput.java | 3 + .../api/changes/AddReviewerInput.java | 3 + .../api/changes/DeleteReviewerInput.java | 3 + .../api/changes/DeleteVoteInput.java | 3 + .../extensions/api/changes/NotifyInfo.java | 26 ++++ .../api/changes/PublishChangeEditInput.java | 3 + .../api/changes}/RecipientType.java | 2 +- .../extensions/api/changes/ReviewInput.java | 1 + .../extensions/api/changes/SubmitInput.java | 3 + .../google/gerrit/server/change/Abandon.java | 36 ++++-- .../gerrit/server/change/ChangeInserter.java | 15 ++- .../gerrit/server/change/DeleteReviewer.java | 9 +- .../gerrit/server/change/DeleteVote.java | 9 +- .../server/change/EmailReviewComments.java | 8 ++ .../gerrit/server/change/NotifyUtil.java | 122 ++++++++++++++++++ .../server/change/PatchSetInserter.java | 15 ++- .../gerrit/server/change/PostReview.java | 33 +++-- .../gerrit/server/change/PostReviewers.java | 47 +++++-- .../server/change/PublishChangeEdit.java | 8 +- .../gerrit/server/edit/ChangeEditUtil.java | 20 ++- .../google/gerrit/server/git/AbandonOp.java | 11 +- .../google/gerrit/server/git/EmailMerge.java | 12 +- .../com/google/gerrit/server/git/MergeOp.java | 15 ++- .../gerrit/server/git/ReceiveCommits.java | 24 ++++ .../google/gerrit/server/git/ReplaceOp.java | 3 +- .../server/git/strategy/SubmitStrategy.java | 7 + .../git/strategy/SubmitStrategyFactory.java | 11 +- .../server/git/strategy/SubmitStrategyOp.java | 2 +- .../gerrit/server/mail/send/AddKeySender.java | 2 +- .../server/mail/send/AddReviewerSender.java | 11 +- .../gerrit/server/mail/send/ChangeEmail.java | 2 +- .../server/mail/send/CreateChangeSender.java | 2 +- .../mail/send/DeleteReviewerSender.java | 2 +- .../server/mail/send/NewChangeSender.java | 2 +- .../server/mail/send/NotificationEmail.java | 2 +- .../server/mail/send/OutgoingEmail.java | 27 +++- .../mail/send/RegisterNewEmailSender.java | 2 +- .../mail/send/ReplacePatchSetSender.java | 2 +- .../server/mail/send/ReplyToChangeSender.java | 2 +- 45 files changed, 658 insertions(+), 126 deletions(-) create mode 100644 gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/NotifyInfo.java rename {gerrit-server/src/main/java/com/google/gerrit/server/mail => gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes}/RecipientType.java (92%) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/NotifyUtil.java diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index fb1a49ff7c..fcf60ed4c6 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -4753,17 +4753,20 @@ This can be: The `AbandonInput` entity contains information for abandoning a change. [options="header",cols="1,^1,5"] -|=========================== -|Field Name ||Description -|`message` |optional| +|============================= +|Field Name ||Description +|`message` |optional| Message to be added as review comment to the change when abandoning the change. -|`notify` |optional| +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the change is abandoned. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. -|=========================== +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[action-info]] === ActionInfo @@ -5206,14 +5209,17 @@ The `DeleteReviewerInput` entity contains options for the deletion of a reviewer. [options="header",cols="1,^1,5"] -|======================= -|Field Name||Description -|`notify` |optional| +|============================= +|Field Name ||Description +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the reviewer is deleted. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. -|======================= +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[delete-vote-input]] === DeleteVoteInput @@ -5221,17 +5227,20 @@ The `DeleteVoteInput` entity contains options for the deletion of a vote. [options="header",cols="1,^1,5"] -|======================= -|Field Name||Description -|`label` |optional| +|============================= +|Field Name ||Description +|`label` |optional| The label for which the vote should be deleted. + If set, must match the label in the URL. -|`notify` |optional| +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the vote is deleted. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. -|======================= +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[description-input]] === DescriptionInput @@ -5623,6 +5632,23 @@ The `MoveInput` entity contains information for moving a change to a new branch. A message to be posted in this change's comments |=========================== +[[notify-info]] +=== NotifyInfo +The `NotifyInfo` entity contains detailed information about who should +be notified about an update. These notifications are sent out even if a +`notify` option in the request input disables normal notifications. +`NotifyInfo` entities are normally contained in a `notify_details` map +in the request input where the key is the recipient type. The recipient +type can be `TO`, `CC` and `BCC`. + +[options="header",cols="1,^1,5"] +|======================= +|Field Name||Description +|`accounts`|optional| +A list of link:rest-api-accounts.html#account-id[account IDs] that +identify the accounts that should be should be notified. +|======================= + [[problem-info]] === ProblemInfo The `ProblemInfo` entity contains a description of a potential consistency problem @@ -5648,14 +5674,17 @@ The `PublishChangeEditInput` entity contains options for the publishing of change edit. [options="header",cols="1,^1,5"] -|======================= -|Field Name||Description -|`notify` |optional| +|============================= +|Field Name ||Description +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the change edit is published. + Allowed values are `NONE` and `ALL`. + If not set, the default is `ALL`. -|======================= +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[push-certificate-info]] === PushCertificateInfo @@ -5835,6 +5864,9 @@ Notify handling that defines to whom email notifications should be sent after the review is stored. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. +|`notify_details` |optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. |`omit_duplicate_comments`|optional| If `true`, comments with the same content at the same place will be omitted. |`on_behalf_of` |optional| @@ -5867,28 +5899,31 @@ The `ReviewerInput` entity contains information for adding a reviewer to a change. [options="header",cols="1,^1,5"] -|=========================== -|Field Name ||Description -|`reviewer` || +|============================= +|Field Name ||Description +|`reviewer` || The link:rest-api-accounts.html#account-id[ID] of one account that should be added as reviewer or the link:rest-api-groups.html#group-id[ ID] of one group for which all members should be added as reviewers. + If an ID identifies both an account and a group, only the account is added as reviewer to the change. -|`state` |optional| +|`state` |optional| Add reviewer in this state. Possible reviewer states are `REVIEWER` and `CC`. If not given, defaults to `REVIEWER`. -|`confirmed` |optional| +|`confirmed` |optional| Whether adding the reviewer is confirmed. + The Gerrit server may be configured to link:config-gerrit.html#addreviewer.maxWithoutConfirmation[require a confirmation] when adding a group as reviewer that has many members. -|`notify` |optional| +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the reviewer is added. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. -|=========================== +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[revision-info]] === RevisionInfo @@ -6014,20 +6049,23 @@ not with the identity of the CI account. The `SubmitInput` entity contains information for submitting a change. [options="header",cols="1,^1,5"] -|=========================== +|============================= |Field Name ||Description -|`on_behalf_of`|optional| +|`on_behalf_of` |optional| If set, submit the change on behalf of the given user. The value may take any format link:rest-api-accounts.html#account-id[accepted by the accounts REST API]. Using this option requires link:access-control.html#category_submit_on_behalf_of[Submit (On Behalf Of)] permission on the branch. -|`notify`|optional| +|`notify` |optional| Notify handling that defines to whom email notifications should be sent after the change is submitted. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + If not set, the default is `ALL`. -|=========================== +|`notify_details`|optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. +|============================= [[submit-record]] === SubmitRecord diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt index a0803d6f5b..6d0c47ae64 100644 --- a/Documentation/user-upload.txt +++ b/Documentation/user-upload.txt @@ -177,6 +177,24 @@ By default all email notifications are sent. git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE ---- +In addition uploaders can explicitly specify accounts that should be +notified, regardless of the value that is given for the `notify` +option. To notify a specific account specify it by an +`notify-to='email'`, `notify-cc='email'` or `notify-bcc='email'` +option. These options can be specified as many times as necessary to +cover all interested parties. Gerrit will automatically avoid sending +duplicate email notifications, such as if one of the specified accounts +had also requested to receive all new change notifications. The +accounts that are specified by `notify-to='email'`, `notify-cc='email'` +and `notify-bcc='email'` will only be notified about this one push. +They are not added as link:#reviewers[reviewers or CCs], hence they are +not automatically signed up to be notified on further updates of the +change. + +---- + git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE,notify-to=a@a.com +---- + [[topic]] ==== Topic diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index b65d7641f4..3dbc10f892 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -892,6 +892,13 @@ public abstract class AbstractDaemonTest { .review(ReviewInput.approve()); } + protected void recommend(String id) throws Exception { + gApi.changes() + .id(id) + .revision("current") + .review(ReviewInput.recommend()); + } + protected Map getActions(String id) throws Exception { return gApi.changes() .id(id) @@ -1171,4 +1178,32 @@ public abstract class AbstractDaemonTest { .containsExactlyElementsIn(Arrays.asList(expected)); } } + + protected void assertNotifyTo(TestAccount expected) { + assertThat(sender.getMessages()).hasSize(1); + Message m = sender.getMessages().get(0); + assertThat(m.rcpt()).containsExactly(expected.emailAddress); + assertThat( + ((EmailHeader.AddressList) m.headers().get("To")).getAddressList()) + .containsExactly(expected.emailAddress); + assertThat(m.headers().get("CC").isEmpty()).isTrue(); + } + + protected void assertNotifyCc(TestAccount expected) { + assertThat(sender.getMessages()).hasSize(1); + Message m = sender.getMessages().get(0); + assertThat(m.rcpt()).containsExactly(expected.emailAddress); + assertThat(m.headers().get("To").isEmpty()).isTrue(); + assertThat( + ((EmailHeader.AddressList) m.headers().get("CC")).getAddressList()) + .containsExactly(expected.emailAddress); + } + + protected void assertNotifyBcc(TestAccount expected) { + assertThat(sender.getMessages()).hasSize(1); + Message m = sender.getMessages().get(0); + assertThat(m.rcpt()).containsExactly(expected.emailAddress); + assertThat(m.headers().get("To").isEmpty()).isTrue(); + assertThat(m.headers().get("CC").isEmpty()).isTrue(); + } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index c52989d093..0a0d51348e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -43,6 +43,7 @@ import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.TimeUtil; @@ -52,7 +53,9 @@ import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.DeleteReviewerInput; import com.google.gerrit.extensions.api.changes.DeleteVoteInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.NotifyInfo; import com.google.gerrit.extensions.api.changes.RebaseInput; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.api.projects.BranchInput; @@ -1373,10 +1376,7 @@ public class ChangeIT extends AbstractDaemonTest { .review(ReviewInput.approve()); setApiUser(user); - gApi.changes() - .id(r.getChangeId()) - .revision(r.getCommit().name()) - .review(ReviewInput.recommend()); + recommend(r.getChangeId()); setApiUser(admin); sender.clear(); @@ -1425,10 +1425,7 @@ public class ChangeIT extends AbstractDaemonTest { .review(ReviewInput.approve()); setApiUser(user); - gApi.changes() - .id(r.getChangeId()) - .revision(r.getCommit().name()) - .review(ReviewInput.recommend()); + recommend(r.getChangeId()); setApiUser(admin); sender.clear(); @@ -1442,6 +1439,62 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(sender.getMessages()).hasSize(0); } + @Test + public void deleteVoteNotifyAccount() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.approve()); + + DeleteVoteInput in = new DeleteVoteInput(); + in.label = "Code-Review"; + in.notify = NotifyHandling.NONE; + + // notify unrelated account as TO + TestAccount user2 = accounts.user2(); + setApiUser(user); + recommend(r.getChangeId()); + setApiUser(admin); + sender.clear(); + in.notifyDetails = new HashMap<>(); + in.notifyDetails.put(RecipientType.TO, + new NotifyInfo(ImmutableList.of(user2.email))); + gApi.changes() + .id(r.getChangeId()) + .reviewer(user.getId().toString()) + .deleteVote(in); + assertNotifyTo(user2); + + // notify unrelated account as CC + setApiUser(user); + recommend(r.getChangeId()); + setApiUser(admin); + sender.clear(); + in.notifyDetails = new HashMap<>(); + in.notifyDetails.put(RecipientType.CC, + new NotifyInfo(ImmutableList.of(user2.email))); + gApi.changes() + .id(r.getChangeId()) + .reviewer(user.getId().toString()) + .deleteVote(in); + assertNotifyCc(user2); + + // notify unrelated account as BCC + setApiUser(user); + recommend(r.getChangeId()); + setApiUser(admin); + sender.clear(); + in.notifyDetails = new HashMap<>(); + in.notifyDetails.put(RecipientType.BCC, + new NotifyInfo(ImmutableList.of(user2.email))); + gApi.changes() + .id(r.getChangeId()) + .reviewer(user.getId().toString()) + .deleteVote(in); + assertNotifyBcc(user2); + } + @Test public void deleteVoteNotPermitted() throws Exception { PushOneCommit.Result r = createChange(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 7830b17eb8..bc848532aa 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -174,8 +175,10 @@ public class ChangeEditIT extends AbstractDaemonTest { .isEqualTo(RefUpdate.Result.NEW); assertThat( modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME, - RawInputUtil.create(CONTENT_NEW2))).isEqualTo(RefUpdate.Result.FORCED); - editUtil.publish(editUtil.byChange(change).get(), NotifyHandling.NONE); + RawInputUtil.create(CONTENT_NEW2))) + .isEqualTo(RefUpdate.Result.FORCED); + editUtil.publish(editUtil.byChange(change).get(), NotifyHandling.NONE, + ImmutableListMultimap.of()); Optional edit = editUtil.byChange(change); assertThat(edit.isPresent()).isFalse(); assertChangeMessages(change, @@ -386,7 +389,8 @@ public class ChangeEditIT extends AbstractDaemonTest { edit = editUtil.byChange(change); assertThat(edit.get().getEditCommit().getFullMessage()).isEqualTo(msg); - editUtil.publish(edit.get(), NotifyHandling.NONE); + editUtil.publish(edit.get(), NotifyHandling.NONE, + ImmutableListMultimap.of()); assertThat(editUtil.byChange(change).isPresent()).isFalse(); ChangeInfo info = get(changeId, ListChangesOption.CURRENT_COMMIT, @@ -429,7 +433,8 @@ public class ChangeEditIT extends AbstractDaemonTest { assertThat(readContentFromJson(r)).isEqualTo(commit.getFullMessage()); } - editUtil.publish(edit.get(), NotifyHandling.NONE); + editUtil.publish(edit.get(), NotifyHandling.NONE, + ImmutableListMultimap.of()); assertChangeMessages(change, ImmutableList.of("Uploaded patch set 1.", "Uploaded patch set 2.", @@ -732,7 +737,8 @@ public class ChangeEditIT extends AbstractDaemonTest { assertThat(modifier.modifyMessage(edit.get(), newMsg)) .isEqualTo(RefUpdate.Result.FORCED); edit = editUtil.byChange(change); - editUtil.publish(edit.get(), NotifyHandling.NONE); + editUtil.publish(edit.get(), NotifyHandling.NONE, + ImmutableListMultimap.of()); ChangeInfo info = get(changeId); assertThat(info.subject).isEqualTo(newSubj); @@ -759,7 +765,8 @@ public class ChangeEditIT extends AbstractDaemonTest { editUtil.delete(editUtil.byChange(change).get()); assertThat(queryEdits()).hasSize(1); - editUtil.publish(editUtil.byChange(change2).get(), NotifyHandling.NONE); + editUtil.publish(editUtil.byChange(change2).get(), NotifyHandling.NONE, + ImmutableListMultimap.of()); assertThat(queryEdits()).hasSize(0); setApiUser(user); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 1fe0304877..b9736fdfee 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -262,6 +262,43 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { m = sender.getMessages().get(0); assertThat(m.rcpt()).containsExactly(user.emailAddress, user2.emailAddress, user3.emailAddress); + + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + + user3.email); + r.assertOkStatus(); + assertNotifyTo(user3); + + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-cc=" + + user3.email); + r.assertOkStatus(); + assertNotifyCc(user3); + + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-bcc=" + + user3.email); + r.assertOkStatus(); + assertNotifyBcc(user3); + + // request that sender gets notified as TO, CC and BCC, email should be sent + // even if the sender is the only recipient + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + + admin.email); + assertNotifyTo(admin); + + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-cc=" + + admin.email); + r.assertOkStatus(); + assertNotifyCc(admin); + + sender.clear(); + r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-bcc=" + + admin.email); + r.assertOkStatus(); + assertNotifyBcc(admin); } @Test diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AbandonInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AbandonInput.java index 34726a81d5..62829f0b70 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AbandonInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AbandonInput.java @@ -16,9 +16,12 @@ package com.google.gerrit.extensions.api.changes; import com.google.gerrit.extensions.restapi.DefaultInput; +import java.util.Map; + public class AbandonInput { @DefaultInput public String message; public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerInput.java index 4c535d4eb9..a042757ecc 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerInput.java @@ -19,12 +19,15 @@ import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.restapi.DefaultInput; +import java.util.Map; + public class AddReviewerInput { @DefaultInput public String reviewer; public Boolean confirmed; public ReviewerState state; public NotifyHandling notify; + public Map notifyDetails; public boolean confirmed() { return (confirmed != null) ? confirmed : false; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteReviewerInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteReviewerInput.java index 6af0dbbbb1..bb942c75c3 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteReviewerInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteReviewerInput.java @@ -14,8 +14,11 @@ package com.google.gerrit.extensions.api.changes; +import java.util.Map; + /** Input passed to {@code DELETE /changes/[id]/reviewers/[id]}. */ public class DeleteReviewerInput { /** Who to send email notifications to after the reviewer is deleted. */ public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java index 671f43ea6b..ee5463bb5e 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java @@ -16,6 +16,8 @@ package com.google.gerrit.extensions.api.changes; import com.google.gerrit.extensions.restapi.DefaultInput; +import java.util.Map; + /** Input passed to {@code DELETE /changes/[id]/reviewers/[id]/votes/[label]}. */ public class DeleteVoteInput { @DefaultInput @@ -23,4 +25,5 @@ public class DeleteVoteInput { /** Who to send email notifications to after vote is deleted. */ public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/NotifyInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/NotifyInfo.java new file mode 100644 index 0000000000..ef49651bc0 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/NotifyInfo.java @@ -0,0 +1,26 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.extensions.api.changes; + +import java.util.List; + +/** Detailed information about who should be notified about an update. */ +public class NotifyInfo { + public List accounts; + + public NotifyInfo(List accounts) { + this.accounts = accounts; + } +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/PublishChangeEditInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/PublishChangeEditInput.java index fa6f18f7c5..66232818ca 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/PublishChangeEditInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/PublishChangeEditInput.java @@ -14,8 +14,11 @@ package com.google.gerrit.extensions.api.changes; +import java.util.Map; + /** Input passed to {@code POST /changes/[id]/edit:publish/}. */ public class PublishChangeEditInput { /** Who to send email notifications to after the change edit is published. */ public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; } \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RecipientType.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RecipientType.java similarity index 92% rename from gerrit-server/src/main/java/com/google/gerrit/server/mail/RecipientType.java rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RecipientType.java index ea0def0abf..e3b8a5368f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RecipientType.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RecipientType.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.gerrit.server.mail; +package com.google.gerrit.extensions.api.changes; public enum RecipientType { TO, CC, BCC diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java index 472559b520..dd392c59b1 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java @@ -57,6 +57,7 @@ public class ReviewInput { /** Who to send email notifications to after review is stored. */ public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; /** * If true check to make sure that the comments being posted aren't already diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmitInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmitInput.java index e415acb5b9..b1ad6e18b7 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmitInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmitInput.java @@ -14,6 +14,8 @@ package com.google.gerrit.extensions.api.changes; +import java.util.Map; + public class SubmitInput { /** Not used anymore, kept for backward compatibility */ @Deprecated @@ -22,4 +24,5 @@ public class SubmitInput { public String onBehalfOf; public NotifyHandling notify = NotifyHandling.ALL; + public Map notifyDetails; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 69edc7eb0b..1641a03259 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -14,9 +14,12 @@ package com.google.gerrit.server.change; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Multimap; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.api.changes.AbandonInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -51,17 +54,20 @@ public class Abandon implements RestModifyView, private final ChangeJson.Factory json; private final BatchUpdate.Factory batchUpdateFactory; private final AbandonOp.Factory abandonOpFactory; + private final NotifyUtil notifyUtil; @Inject Abandon( Provider dbProvider, ChangeJson.Factory json, BatchUpdate.Factory batchUpdateFactory, - AbandonOp.Factory abandonOpFactory) { + AbandonOp.Factory abandonOpFactory, + NotifyUtil notifyUtil) { this.dbProvider = dbProvider; this.json = json; this.batchUpdateFactory = batchUpdateFactory; this.abandonOpFactory = abandonOpFactory; + this.notifyUtil = notifyUtil; } @Override @@ -71,27 +77,32 @@ public class Abandon implements RestModifyView, if (!control.canAbandon(dbProvider.get())) { throw new AuthException("abandon not permitted"); } - Change change = abandon(control, input.message, input.notify); + Change change = abandon(control, input.message, input.notify, + notifyUtil.resolveAccounts(input.notifyDetails)); return json.create(ChangeJson.NO_OPTIONS).format(change); } public Change abandon(ChangeControl control) throws RestApiException, UpdateException { - return abandon(control, "", NotifyHandling.ALL); + return abandon(control, "", NotifyHandling.ALL, ImmutableListMultimap.of()); } public Change abandon(ChangeControl control, String msgTxt) throws RestApiException, UpdateException { - return abandon(control, msgTxt, NotifyHandling.ALL); + return abandon(control, msgTxt, NotifyHandling.ALL, + ImmutableListMultimap.of()); } public Change abandon(ChangeControl control, String msgTxt, - NotifyHandling notifyHandling) throws RestApiException, UpdateException { + NotifyHandling notifyHandling, + Multimap accountsToNotify) + throws RestApiException, UpdateException { CurrentUser user = control.getUser(); Account account = user.isIdentifiedUser() ? user.asIdentifiedUser().getAccount() : null; - AbandonOp op = abandonOpFactory.create(account, msgTxt, notifyHandling); + AbandonOp op = abandonOpFactory.create(account, msgTxt, notifyHandling, + accountsToNotify); try (BatchUpdate u = batchUpdateFactory.create( dbProvider.get(), @@ -113,7 +124,9 @@ public class Abandon implements RestModifyView, */ public void batchAbandon(Project.NameKey project, CurrentUser user, Collection controls, String msgTxt, - NotifyHandling notifyHandling) throws RestApiException, UpdateException { + NotifyHandling notifyHandling, + Multimap accountsToNotify) + throws RestApiException, UpdateException { if (controls.isEmpty()) { return; } @@ -132,7 +145,8 @@ public class Abandon implements RestModifyView, } u.addOp( control.getId(), - abandonOpFactory.create(account, msgTxt, notifyHandling)); + abandonOpFactory.create(account, msgTxt, notifyHandling, + accountsToNotify)); } u.execute(); } @@ -141,13 +155,15 @@ public class Abandon implements RestModifyView, public void batchAbandon(Project.NameKey project, CurrentUser user, Collection controls, String msgTxt) throws RestApiException, UpdateException { - batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL); + batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL, + ImmutableListMultimap.of()); } public void batchAbandon(Project.NameKey project, CurrentUser user, Collection controls) throws RestApiException, UpdateException { - batchAbandon(project, user, controls, "", NotifyHandling.ALL); + batchAbandon(project, user, controls, "", NotifyHandling.ALL, + ImmutableListMultimap.of()); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index a2f5df45bf..28d28ed106 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -20,10 +20,13 @@ import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID; import static java.util.stream.Collectors.toSet; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Multimap; 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.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; @@ -112,6 +115,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { private CommitValidators.Policy validatePolicy = CommitValidators.Policy.GERRIT; private NotifyHandling notify = NotifyHandling.ALL; + private Multimap accountsToNotify = + ImmutableListMultimap.of(); private Set reviewers; private Set extraCC; private Map approvals; @@ -235,6 +240,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { return this; } + public ChangeInserter setAccountsToNotify( + Multimap accountsToNotify) { + this.accountsToNotify = checkNotNull(accountsToNotify); + return this; + } + public ChangeInserter setReviewers(Set reviewers) { this.reviewers = reviewers; return this; @@ -401,7 +412,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { @Override public void postUpdate(Context ctx) throws OrmException, NoSuchChangeException { - if (sendMail) { + if (sendMail + && (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty())) { Runnable sender = new Runnable() { @Override public void run() { @@ -411,6 +423,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { cm.setFrom(change.getOwner()); cm.setPatchSet(patchSet, patchSetInfo); cm.setNotify(notify); + cm.setAccountsToNotify(accountsToNotify); cm.addReviewers(reviewers); cm.addExtraCC(extraCC); cm.send(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java index 14d9ae3748..47f5901717 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java @@ -79,6 +79,7 @@ public class DeleteReviewer private final Provider user; private final DeleteReviewerSender.Factory deleteReviewerSenderFactory; private final NotesMigration migration; + private final NotifyUtil notifyUtil; @Inject DeleteReviewer(Provider dbProvider, @@ -90,7 +91,8 @@ public class DeleteReviewer ReviewerDeleted reviewerDeleted, Provider user, DeleteReviewerSender.Factory deleteReviewerSenderFactory, - NotesMigration migration) { + NotesMigration migration, + NotifyUtil notifyUtil) { this.dbProvider = dbProvider; this.approvalsUtil = approvalsUtil; this.psUtil = psUtil; @@ -101,6 +103,7 @@ public class DeleteReviewer this.user = user; this.deleteReviewerSenderFactory = deleteReviewerSenderFactory; this.migration = migration; + this.notifyUtil = notifyUtil; } @Override @@ -198,7 +201,7 @@ public class DeleteReviewer @Override public void postUpdate(Context ctx) { - if (input.notify.compareTo(NotifyHandling.NONE) > 0) { + if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) { emailReviewers(ctx.getProject(), currChange, del, changeMessage); } reviewerDeleted.fire(currChange, currPs, reviewer, @@ -262,6 +265,8 @@ public class DeleteReviewer cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn()); cm.setNotify(input.notify); + cm.setAccountsToNotify( + notifyUtil.resolveAccounts(input.notifyDetails)); cm.send(); } catch (Exception err) { log.error("Cannot email update for change " + change.getId(), err); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java index e1d1caa50f..951635e194 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java @@ -73,6 +73,7 @@ public class DeleteVote private final IdentifiedUser.GenericFactory userFactory; private final VoteDeleted voteDeleted; private final DeleteVoteSender.Factory deleteVoteSenderFactory; + private final NotifyUtil notifyUtil; @Inject DeleteVote(Provider db, @@ -82,7 +83,8 @@ public class DeleteVote ChangeMessagesUtil cmUtil, IdentifiedUser.GenericFactory userFactory, VoteDeleted voteDeleted, - DeleteVoteSender.Factory deleteVoteSenderFactory) { + DeleteVoteSender.Factory deleteVoteSenderFactory, + NotifyUtil notifyUtil) { this.db = db; this.batchUpdateFactory = batchUpdateFactory; this.approvalsUtil = approvalsUtil; @@ -91,6 +93,7 @@ public class DeleteVote this.userFactory = userFactory; this.voteDeleted = voteDeleted; this.deleteVoteSenderFactory = deleteVoteSenderFactory; + this.notifyUtil = notifyUtil; } @Override @@ -208,13 +211,15 @@ public class DeleteVote } IdentifiedUser user = ctx.getIdentifiedUser(); - if (input.notify.compareTo(NotifyHandling.NONE) > 0) { + if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) { try { ReplyToChangeSender cm = deleteVoteSenderFactory.create( ctx.getProject(), change.getId()); cm.setFrom(user.getAccountId()); cm.setChangeMessage(changeMessage.getMessage(), ctx.getWhen()); cm.setNotify(input.notify); + cm.setAccountsToNotify( + notifyUtil.resolveAccounts(input.notifyDetails)); cm.send(); } catch (Exception e) { log.error("Cannot email update for change " + change.getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java index bffb61be4a..2c65e4d2c2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java @@ -16,7 +16,10 @@ package com.google.gerrit.server.change; import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER; +import com.google.common.collect.Multimap; 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,6 +51,7 @@ public class EmailReviewComments implements Runnable, RequestContext { interface Factory { EmailReviewComments create( NotifyHandling notify, + Multimap accountsToNotify, ChangeNotes notes, PatchSet patchSet, IdentifiedUser user, @@ -62,6 +66,7 @@ public class EmailReviewComments implements Runnable, RequestContext { private final ThreadLocalRequestContext requestContext; private final NotifyHandling notify; + private final Multimap accountsToNotify; private final ChangeNotes notes; private final PatchSet patchSet; private final IdentifiedUser user; @@ -77,6 +82,7 @@ public class EmailReviewComments implements Runnable, RequestContext { SchemaFactory schemaFactory, ThreadLocalRequestContext requestContext, @Assisted NotifyHandling notify, + @Assisted Multimap accountsToNotify, @Assisted ChangeNotes notes, @Assisted PatchSet patchSet, @Assisted IdentifiedUser user, @@ -88,6 +94,7 @@ public class EmailReviewComments implements Runnable, RequestContext { this.schemaFactory = schemaFactory; this.requestContext = requestContext; this.notify = notify; + this.accountsToNotify = accountsToNotify; this.notes = notes; this.patchSet = patchSet; this.user = user; @@ -112,6 +119,7 @@ public class EmailReviewComments implements Runnable, RequestContext { cm.setChangeMessage(message.getMessage(), message.getWrittenOn()); cm.setComments(comments); cm.setNotify(notify); + cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception e) { log.error("Cannot email comments for " + patchSet.getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/NotifyUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/NotifyUtil.java new file mode 100644 index 0000000000..2bab42794a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/NotifyUtil.java @@ -0,0 +1,122 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.change; + +import static java.util.stream.Collectors.joining; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Multimap; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.NotifyInfo; +import com.google.gerrit.extensions.api.changes.RecipientType; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.account.AccountResolver; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +@Singleton +public class NotifyUtil { + private final Provider dbProvider; + private final AccountResolver accountResolver; + + @Inject + NotifyUtil(Provider dbProvider, + AccountResolver accountResolver) { + this.dbProvider = dbProvider; + this.accountResolver = accountResolver; + } + + public static boolean shouldNotify(NotifyHandling notify, + @Nullable Map notifyDetails) { + if (!isNullOrEmpty(notifyDetails)) { + return true; + } + + return notify.compareTo(NotifyHandling.NONE) > 0; + } + + private static boolean isNullOrEmpty( + @Nullable Map notifyDetails) { + if (notifyDetails == null || notifyDetails.isEmpty()) { + return true; + } + + for (NotifyInfo notifyInfo : notifyDetails.values()) { + if (!isEmpty(notifyInfo)) { + return false; + } + } + + return true; + } + + private static boolean isEmpty(NotifyInfo notifyInfo) { + return notifyInfo.accounts == null || notifyInfo.accounts.isEmpty(); + } + + public Multimap resolveAccounts( + @Nullable Map notifyDetails) + throws OrmException, BadRequestException { + if (isNullOrEmpty(notifyDetails)) { + return ImmutableListMultimap.of(); + } + + Multimap m = null; + for (Entry e : notifyDetails.entrySet()) { + List accounts = e.getValue().accounts; + if (accounts != null) { + if (m == null) { + m = ArrayListMultimap.create(); + } + m.putAll(e.getKey(), find(dbProvider.get(), accounts)); + } + } + + return m != null ? m : ImmutableListMultimap.of(); + } + + private List find(ReviewDb db, List nameOrEmails) + throws OrmException, BadRequestException { + List missing = new ArrayList<>(nameOrEmails.size()); + List r = new ArrayList<>(nameOrEmails.size()); + for (String nameOrEmail : nameOrEmails) { + Account a = accountResolver.find(db, nameOrEmail); + if (a != null) { + r.add(a.getId()); + } else { + missing.add(nameOrEmail); + } + } + + if (!missing.isEmpty()) { + throw new BadRequestException( + "The following accounts that should be notified could not be resolved: " + + missing.stream().distinct().sorted().collect(joining(", "))); + } + + return r; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index e47013e65c..c8dd4296be 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -19,9 +19,13 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Multimap; 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; @@ -94,6 +98,8 @@ public class PatchSetInserter extends BatchUpdate.Op { private List groups = Collections.emptyList(); private boolean fireRevisionCreated = true; private NotifyHandling notify = NotifyHandling.ALL; + private Multimap accountsToNotify = + ImmutableListMultimap.of(); private boolean allowClosed; private boolean copyApprovals = true; @@ -165,6 +171,12 @@ public class PatchSetInserter extends BatchUpdate.Op { return this; } + public PatchSetInserter setAccountsToNotify( + Multimap accountsToNotify) { + this.accountsToNotify = checkNotNull(accountsToNotify); + return this; + } + public PatchSetInserter setAllowClosed(boolean allowClosed) { this.allowClosed = allowClosed; return this; @@ -246,7 +258,7 @@ public class PatchSetInserter extends BatchUpdate.Op { @Override public void postUpdate(Context ctx) throws OrmException { - if (notify != NotifyHandling.NONE) { + if (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty()) { try { ReplacePatchSetSender cm = replacePatchSetFactory.create( ctx.getProject(), change.getId()); @@ -256,6 +268,7 @@ public class PatchSetInserter extends BatchUpdate.Op { cm.addReviewers(oldReviewers.byState(REVIEWER)); cm.addExtraCC(oldReviewers.byState(CC)); cm.setNotify(notify); + cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception err) { log.error("Cannot send email for new patch set on change " diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index 8cfeff00f8..712f8d6181 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -30,6 +30,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import com.google.common.collect.Ordering; import com.google.common.hash.HashCode; import com.google.common.hash.Hashing; @@ -42,6 +43,7 @@ import com.google.gerrit.common.data.PermissionRange; 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; @@ -130,6 +132,7 @@ public class PostReview implements RestModifyView private final CommentAdded commentAdded; private final PostReviewers postReviewers; private final NotesMigration migration; + private final NotifyUtil notifyUtil; @Inject PostReview(Provider db, @@ -145,7 +148,8 @@ public class PostReview implements RestModifyView EmailReviewComments.Factory email, CommentAdded commentAdded, PostReviewers postReviewers, - NotesMigration migration) { + NotesMigration migration, + NotifyUtil notifyUtil) { this.db = db; this.batchUpdateFactory = batchUpdateFactory; this.changes = changes; @@ -160,6 +164,7 @@ public class PostReview implements RestModifyView this.commentAdded = commentAdded; this.postReviewers = postReviewers; this.migration = migration; + this.notifyUtil = notifyUtil; } @Override @@ -198,6 +203,9 @@ public class PostReview implements RestModifyView input.notify = NotifyHandling.NONE; } + Multimap accountsToNotify = + notifyUtil.resolveAccounts(input.notifyDetails); + Map reviewerJsonResults = null; List reviewerResults = Lists.newArrayList(); boolean hasError = false; @@ -274,23 +282,25 @@ public class PostReview implements RestModifyView bu.addOp(revision.getChange().getId(), selfAddition.op); } - bu.addOp( - revision.getChange().getId(), - new Op(revision.getPatchSet().getId(), input, reviewerResults)); + bu.addOp(revision.getChange().getId(), + new Op(revision.getPatchSet().getId(), input, accountsToNotify, + reviewerResults)); bu.execute(); for (PostReviewers.Addition reviewerResult : reviewerResults) { reviewerResult.gatherResults(); } - emailReviewers(revision.getChange(), reviewerResults, input.notify); + emailReviewers(revision.getChange(), reviewerResults, input.notify, + accountsToNotify); } return Response.ok(output); } private void emailReviewers(Change change, - List reviewerAdditions, NotifyHandling notify) { + List reviewerAdditions, NotifyHandling notify, + Multimap accountsToNotify) { List to = new ArrayList<>(); List cc = new ArrayList<>(); for (PostReviewers.Addition addition : reviewerAdditions) { @@ -300,7 +310,7 @@ public class PostReview implements RestModifyView cc.addAll(addition.op.reviewers.keySet()); } } - postReviewers.emailReviewers(change, to, cc, notify); + postReviewers.emailReviewers(change, to, cc, notify, accountsToNotify); } private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in) @@ -551,6 +561,7 @@ public class PostReview implements RestModifyView private class Op extends BatchUpdate.Op { private final PatchSet.Id psId; private final ReviewInput in; + private final Multimap accountsToNotify; private final List reviewerResults; private IdentifiedUser user; @@ -563,9 +574,11 @@ public class PostReview implements RestModifyView private Map oldApprovals = new HashMap<>(); private Op(PatchSet.Id psId, ReviewInput in, + Multimap accountsToNotify, List reviewerResults) { this.psId = psId; this.in = in; + this.accountsToNotify = checkNotNull(accountsToNotify); this.reviewerResults = reviewerResults; } @@ -584,13 +597,15 @@ public class PostReview implements RestModifyView } @Override - public void postUpdate(Context ctx) { + public void postUpdate(Context ctx) throws OrmException { if (message == null) { return; } - if (in.notify.compareTo(NotifyHandling.NONE) > 0) { + if (in.notify.compareTo(NotifyHandling.NONE) > 0 + || !accountsToNotify.isEmpty()) { email.create( in.notify, + accountsToNotify, notes, ps, user, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 0cdddca11c..a17447ff70 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -14,18 +14,22 @@ package com.google.gerrit.server.change; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gerrit.extensions.client.ReviewerState.CC; import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.errors.NoSuchGroupException; 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.ReviewerInfo; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.restapi.BadRequestException; @@ -102,6 +106,7 @@ public class PostReviewers private final ReviewerAdded reviewerAdded; private final NotesMigration migration; private final AccountCache accountCache; + private final NotifyUtil notifyUtil; @Inject PostReviewers(AccountsCollection accounts, @@ -120,7 +125,8 @@ public class PostReviewers ReviewerJson json, ReviewerAdded reviewerAdded, NotesMigration migration, - AccountCache accountCache) { + AccountCache accountCache, + NotifyUtil notifyUtil) { this.accounts = accounts; this.reviewerFactory = reviewerFactory; this.approvalsUtil = approvalsUtil; @@ -138,6 +144,7 @@ public class PostReviewers this.reviewerAdded = reviewerAdded; this.migration = migration; this.accountCache = accountCache; + this.notifyUtil = notifyUtil; } @Override @@ -181,24 +188,27 @@ public class PostReviewers .format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer)); } return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId), - input.state(), input.notify); + input.state(), input.notify, + notifyUtil.resolveAccounts(input.notifyDetails)); } Addition ccCurrentUser(CurrentUser user, RevisionResource revision) { return new Addition( user.getUserName(), revision.getChangeResource(), ImmutableMap.of(user.getAccountId(), revision.getControl()), - CC, NotifyHandling.NONE); + CC, NotifyHandling.NONE, ImmutableListMultimap.of()); } private Addition putAccount(String reviewer, ReviewerResource rsrc, - ReviewerState state, NotifyHandling notify) - throws UnprocessableEntityException { + ReviewerState state, NotifyHandling notify, + Multimap accountsToNotify) + throws UnprocessableEntityException { Account member = rsrc.getReviewerUser().getAccount(); ChangeControl control = rsrc.getReviewerControl(); if (isValidReviewer(member, control)) { return new Addition(reviewer, rsrc.getChangeResource(), - ImmutableMap.of(member.getId(), control), state, notify); + ImmutableMap.of(member.getId(), control), state, notify, + accountsToNotify); } if (member.isActive()) { throw new UnprocessableEntityException( @@ -256,7 +266,7 @@ public class PostReviewers } return new Addition(input.reviewer, rsrc, reviewers, input.state(), - input.notify); + input.notify, notifyUtil.resolveAccounts(input.notifyDetails)); } private boolean isValidReviewer(Account member, ChangeControl control) { @@ -287,12 +297,13 @@ public class PostReviewers private final Map reviewers; protected Addition(String reviewer) { - this(reviewer, null, null, REVIEWER, null); + this(reviewer, null, null, REVIEWER, null, ImmutableListMultimap.of()); } protected Addition(String reviewer, ChangeResource rsrc, Map reviewers, ReviewerState state, - NotifyHandling notify) { + NotifyHandling notify, + Multimap accountsToNotify) { result = new AddReviewerResult(reviewer); if (reviewers == null) { this.reviewers = ImmutableMap.of(); @@ -300,7 +311,7 @@ public class PostReviewers return; } this.reviewers = reviewers; - op = new Op(rsrc, reviewers, state, notify); + op = new Op(rsrc, reviewers, state, notify, accountsToNotify); } void gatherResults() throws OrmException { @@ -331,6 +342,7 @@ public class PostReviewers final Map reviewers; final ReviewerState state; final NotifyHandling notify; + final Multimap accountsToNotify; List addedReviewers; Collection addedCCs; @@ -338,11 +350,13 @@ public class PostReviewers private PatchSet patchSet; Op(ChangeResource rsrc, Map reviewers, - ReviewerState state, NotifyHandling notify) { + ReviewerState state, NotifyHandling notify, + Multimap accountsToNotify) { this.rsrc = rsrc; this.reviewers = reviewers; this.state = state; this.notify = notify; + this.accountsToNotify = checkNotNull(accountsToNotify); } @Override @@ -380,7 +394,7 @@ public class PostReviewers } emailReviewers(rsrc.getChange(), Lists.transform(addedReviewers, r -> r.getAccountId()), addedCCs, - notify); + notify, accountsToNotify); if (!addedReviewers.isEmpty()) { List reviewers = Lists.transform(addedReviewers, psa -> accountCache.get(psa.getAccountId()).getAccount()); @@ -392,7 +406,8 @@ public class PostReviewers } public void emailReviewers(Change change, Collection added, - Collection copied, NotifyHandling notify) { + Collection copied, NotifyHandling notify, + Multimap accountsToNotify) { if (added.isEmpty() && copied.isEmpty()) { return; } @@ -419,7 +434,11 @@ public class PostReviewers try { AddReviewerSender cm = addReviewerSenderFactory - .create(change.getProject(), change.getId(), notify); + .create(change.getProject(), change.getId()); + if (notify != null) { + cm.setNotify(notify); + } + cm.setAccountsToNotify(accountsToNotify); cm.setFrom(userId); cm.addReviewers(toMail); cm.addExtraCC(toCopy); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java index 6875287017..87f81fe532 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java @@ -76,10 +76,13 @@ public class PublishChangeEdit implements implements RestModifyView { private final ChangeEditUtil editUtil; + private final NotifyUtil notifyUtil; @Inject - Publish(ChangeEditUtil editUtil) { + Publish(ChangeEditUtil editUtil, + NotifyUtil notifyUtil) { this.editUtil = editUtil; + this.notifyUtil = notifyUtil; } @Override @@ -101,7 +104,8 @@ public class PublishChangeEdit implements if (in == null) { in = new PublishChangeEditInput(); } - editUtil.publish(edit.get(), in.notify); + editUtil.publish(edit.get(), in.notify, + notifyUtil.resolveAccounts(in.notifyDetails)); return Response.none(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 6beff6cbb6..8ab2135a65 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -16,12 +16,15 @@ package com.google.gerrit.server.edit; import static com.google.common.base.Preconditions.checkArgument; +import com.google.common.collect.Multimap; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.client.ChangeKind; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.PatchSet; @@ -155,19 +158,23 @@ public class ChangeEditUtil { } /** - * Promote change edit to patch set, by squashing the edit into - * its parent. + * Promote change edit to patch set, by squashing the edit into its parent. * * @param edit change edit to publish + * @param notify Notify handling that defines to whom email notifications + * should be sent after the change edit is published. + * @param accountsToNotify Accounts that should be notified after the change + * edit is published. * @throws NoSuchChangeException * @throws IOException * @throws OrmException * @throws UpdateException * @throws RestApiException */ - public void publish(final ChangeEdit edit, NotifyHandling notify) - throws NoSuchChangeException, IOException, OrmException, RestApiException, - UpdateException { + public void publish(final ChangeEdit edit, NotifyHandling notify, + Multimap accountsToNotify) + throws NoSuchChangeException, IOException, OrmException, + RestApiException, UpdateException { Change change = edit.getChange(); try (Repository repo = gitManager.openRepository(change.getProject()); RevWalk rw = new RevWalk(repo); @@ -185,7 +192,8 @@ public class ChangeEditUtil { ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId()); PatchSetInserter inserter = patchSetInserterFactory .create(ctl, psId, squashed) - .setNotify(notify); + .setNotify(notify) + .setAccountsToNotify(accountsToNotify); StringBuilder message = new StringBuilder("Patch Set ") .append(inserter.getPatchSetId().get()) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java index d4751eace8..84ac33b429 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java @@ -15,8 +15,10 @@ package com.google.gerrit.server.git; import com.google.common.base.Strings; +import com.google.common.collect.Multimap; 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.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; @@ -47,6 +49,7 @@ public class AbandonOp extends BatchUpdate.Op { private final String msgTxt; private final NotifyHandling notifyHandling; + private final Multimap accountsToNotify; private final Account account; private Change change; @@ -57,7 +60,8 @@ public class AbandonOp extends BatchUpdate.Op { AbandonOp create( @Assisted @Nullable Account account, @Assisted @Nullable String msgTxt, - @Assisted NotifyHandling notifyHandling); + @Assisted NotifyHandling notifyHandling, + @Assisted Multimap accountsToNotify); } @AssistedInject @@ -68,7 +72,8 @@ public class AbandonOp extends BatchUpdate.Op { ChangeAbandoned changeAbandoned, @Assisted @Nullable Account account, @Assisted @Nullable String msgTxt, - @Assisted NotifyHandling notifyHandling) { + @Assisted NotifyHandling notifyHandling, + @Assisted Multimap accountsToNotify) { this.abandonedSenderFactory = abandonedSenderFactory; this.cmUtil = cmUtil; this.psUtil = psUtil; @@ -77,6 +82,7 @@ public class AbandonOp extends BatchUpdate.Op { this.account = account; this.msgTxt = Strings.nullToEmpty(msgTxt); this.notifyHandling = notifyHandling; + this.accountsToNotify = accountsToNotify; } @Nullable @@ -127,6 +133,7 @@ public class AbandonOp extends BatchUpdate.Op { } cm.setChangeMessage(message.getMessage(), ctx.getWhen()); cm.setNotify(notifyHandling); + cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception e) { log.error("Cannot email update for change " + change.getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java index 6817bac21c..6273702703 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java @@ -14,8 +14,10 @@ package com.google.gerrit.server.git; +import com.google.common.collect.Multimap; 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; @@ -43,7 +45,8 @@ public class EmailMerge implements Runnable, RequestContext { public interface Factory { EmailMerge create(Project.NameKey project, Change.Id changeId, - Account.Id submitter, NotifyHandling notifyHandling); + Account.Id submitter, NotifyHandling notifyHandling, + Multimap accountsToNotify); } private final ExecutorService sendEmailsExecutor; @@ -56,6 +59,8 @@ public class EmailMerge implements Runnable, RequestContext { private final Change.Id changeId; private final Account.Id submitter; private final NotifyHandling notifyHandling; + private final Multimap accountsToNotify; + private ReviewDb db; @Inject @@ -67,7 +72,8 @@ public class EmailMerge implements Runnable, RequestContext { @Assisted Project.NameKey project, @Assisted Change.Id changeId, @Assisted @Nullable Account.Id submitter, - @Assisted NotifyHandling notifyHandling) { + @Assisted NotifyHandling notifyHandling, + @Assisted Multimap accountsToNotify) { this.sendEmailsExecutor = executor; this.mergedSenderFactory = mergedSenderFactory; this.schemaFactory = schemaFactory; @@ -77,6 +83,7 @@ public class EmailMerge implements Runnable, RequestContext { this.changeId = changeId; this.submitter = submitter; this.notifyHandling = notifyHandling; + this.accountsToNotify = accountsToNotify; } public void sendAsync() { @@ -92,6 +99,7 @@ public class EmailMerge implements Runnable, RequestContext { cm.setFrom(submitter); } cm.setNotify(notifyHandling); + cm.setAccountsToNotify(accountsToNotify); cm.send(); } catch (Exception e) { log.error("Cannot email merged notification for " + changeId, e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 0062694957..6637d83d10 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -33,11 +33,13 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitTypeRecord; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; @@ -47,6 +49,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.InternalUser; +import com.google.gerrit.server.change.NotifyUtil; import com.google.gerrit.server.git.BatchUpdate.ChangeContext; import com.google.gerrit.server.git.MergeOpRepoManager.OpenBranch; import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo; @@ -217,6 +220,7 @@ public class MergeOp implements AutoCloseable { private final SubmitStrategyFactory submitStrategyFactory; private final SubmoduleOp.Factory subOpFactory; private final MergeOpRepoManager orm; + private final NotifyUtil notifyUtil; private Timestamp ts; private RequestId submissionId; @@ -225,6 +229,7 @@ public class MergeOp implements AutoCloseable { private CommitStatus commits; private ReviewDb db; private SubmitInput submitInput; + private Multimap accountsToNotify; private Set allProjects; private boolean dryrun; @@ -237,7 +242,8 @@ public class MergeOp implements AutoCloseable { InternalChangeQuery internalChangeQuery, SubmitStrategyFactory submitStrategyFactory, SubmoduleOp.Factory subOpFactory, - MergeOpRepoManager orm) { + MergeOpRepoManager orm, + NotifyUtil notifyUtil) { this.cmUtil = cmUtil; this.batchUpdateFactory = batchUpdateFactory; this.internalUserFactory = internalUserFactory; @@ -247,6 +253,7 @@ public class MergeOp implements AutoCloseable { this.submitStrategyFactory = submitStrategyFactory; this.subOpFactory = subOpFactory; this.orm = orm; + this.notifyUtil = notifyUtil; } @Override @@ -398,6 +405,8 @@ public class MergeOp implements AutoCloseable { boolean checkSubmitRules, SubmitInput submitInput, boolean dryrun) throws OrmException, RestApiException { this.submitInput = submitInput; + this.accountsToNotify = + notifyUtil.resolveAccounts(submitInput.notifyDetails); this.dryrun = dryrun; this.caller = caller; this.ts = TimeUtil.nowTs(); @@ -545,8 +554,8 @@ public class MergeOp implements AutoCloseable { throws IntegrationException { return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins, or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller, - mergeTip, commits, submissionId, submitInput.notify, submoduleOp, - dryrun); + mergeTip, commits, submissionId, submitInput.notify, accountsToNotify, + submoduleOp, dryrun); } private Set getAlreadyAccepted(OpenRepo or, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 74310a4d9e..d951d79a5f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -46,6 +46,7 @@ import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.collect.SortedSetMultimap; @@ -58,6 +59,7 @@ import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; import com.google.gerrit.extensions.registration.DynamicMap; @@ -1250,6 +1252,18 @@ public class ReceiveCommits { + "OWNER_REVIEWERS, ALL. If not set, the default is ALL.") NotifyHandling notify = NotifyHandling.ALL; + @Option(name = "--notify-to", metaVar = "USER", + usage = "user that should be notified") + List tos = new ArrayList<>(); + + @Option(name = "--notify-cc", metaVar = "USER", + usage = "user that should be CC'd") + List ccs = new ArrayList<>(); + + @Option(name = "--notify-bcc", metaVar = "USER", + usage = "user that should be BCC'd") + List bccs = new ArrayList<>(); + @Option(name = "--reviewer", aliases = {"-r"}, metaVar = "EMAIL", usage = "add reviewer to changes") void reviewer(Account.Id id) { @@ -1311,6 +1325,15 @@ public class ReceiveCommits { return new MailRecipients(reviewer, cc); } + Multimap getAccountsToNotify() { + Multimap accountsToNotify = + ArrayListMultimap.create(); + accountsToNotify.putAll(RecipientType.TO, tos); + accountsToNotify.putAll(RecipientType.CC, ccs); + accountsToNotify.putAll(RecipientType.BCC, bccs); + return accountsToNotify; + } + String parse(CmdLineParser clp, Repository repo, Set refs, ListMultimap pushOptions) throws CmdLineException { String ref = RefNames.fullName( @@ -2080,6 +2103,7 @@ public class ReceiveCommits { .setApprovals(approvals) .setMessage(msg.toString()) .setNotify(magicBranch.notify) + .setAccountsToNotify(magicBranch.getAccountsToNotify()) .setRequestScopePropagator(requestScopePropagator) .setSendMail(true) .setUpdateRef(false) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java index e4a79cb291..32ec9410de 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java @@ -381,8 +381,9 @@ public class ReplaceOp extends BatchUpdate.Op { cm.setFrom(account.getId()); cm.setPatchSet(newPatchSet, info); cm.setChangeMessage(msg.getMessage(), ctx.getWhen()); - if (magicBranch != null && magicBranch.notify != null) { + if (magicBranch != null) { cm.setNotify(magicBranch.notify); + cm.setAccountsToNotify(magicBranch.getAccountsToNotify()); } cm.addReviewers(recipients.getReviewers()); cm.addExtraCC(recipients.getCcOnly()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java index 28e170f4f6..c77cb05ebb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java @@ -16,10 +16,13 @@ package com.google.gerrit.server.git.strategy; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; @@ -96,6 +99,7 @@ public abstract class SubmitStrategy { Set alreadyAccepted, RequestId submissionId, NotifyHandling notifyHandling, + Multimap accountsToNotify, SubmoduleOp submoduleOp, boolean dryrun); } @@ -129,6 +133,7 @@ public abstract class SubmitStrategy { final RequestId submissionId; final SubmitType submitType; final NotifyHandling notifyHandling; + final Multimap accountsToNotify; final SubmoduleOp submoduleOp; final ProjectState project; @@ -167,6 +172,7 @@ public abstract class SubmitStrategy { @Assisted RequestId submissionId, @Assisted SubmitType submitType, @Assisted NotifyHandling notifyHandling, + @Assisted Multimap accountsToNotify, @Assisted SubmoduleOp submoduleOp, @Assisted boolean dryrun) { this.accountCache = accountCache; @@ -198,6 +204,7 @@ public abstract class SubmitStrategy { this.submissionId = submissionId; this.submitType = submitType; this.notifyHandling = notifyHandling; + this.accountsToNotify = accountsToNotify; this.submoduleOp = submoduleOp; this.dryrun = dryrun; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java index f8ca32ad09..4591c9ebd3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java @@ -14,8 +14,11 @@ package com.google.gerrit.server.git.strategy; +import com.google.common.collect.Multimap; import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.client.SubmitType; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; @@ -55,11 +58,13 @@ public class SubmitStrategyFactory { RevFlag canMergeFlag, Set alreadyAccepted, Branch.NameKey destBranch, IdentifiedUser caller, MergeTip mergeTip, CommitStatus commits, RequestId submissionId, - NotifyHandling notifyHandling, SubmoduleOp submoduleOp, boolean dryrun) - throws IntegrationException { + NotifyHandling notifyHandling, + Multimap accountsToNotify, + SubmoduleOp submoduleOp, boolean dryrun) throws IntegrationException { SubmitStrategy.Arguments args = argsFactory.create(submitType, destBranch, commits, rw, caller, mergeTip, inserter, repo, canMergeFlag, db, - alreadyAccepted, submissionId, notifyHandling, submoduleOp, dryrun); + alreadyAccepted, submissionId, notifyHandling, accountsToNotify, + submoduleOp, dryrun); switch (submitType) { case CHERRY_PICK: return new CherryPick(args); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java index ee2fdc41f9..79642fc1bc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java @@ -499,7 +499,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op { try { args.mergedSenderFactory .create(ctx.getProject(), getId(), submitter.getAccountId(), - args.notifyHandling) + args.notifyHandling, args.accountsToNotify) .sendAsync(); } catch (Exception e) { log.error("Cannot email merged notification for " + getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddKeySender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddKeySender.java index bbcd6a58c2..47068eb9ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddKeySender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddKeySender.java @@ -16,10 +16,10 @@ package com.google.gerrit.server.mail.send; import com.google.common.base.Joiner; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.AccountSshKey; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.mail.Address; -import com.google.gerrit.server.mail.RecipientType; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddReviewerSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddReviewerSender.java index 79d02e00d5..67577de1c4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddReviewerSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AddReviewerSender.java @@ -14,9 +14,7 @@ package com.google.gerrit.server.mail.send; -import com.google.gerrit.common.Nullable; import com.google.gerrit.common.errors.EmailException; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; @@ -26,20 +24,15 @@ import com.google.inject.assistedinject.Assisted; /** Asks a user to review a change. */ public class AddReviewerSender extends NewChangeSender { public interface Factory { - AddReviewerSender create(Project.NameKey project, Change.Id id, - NotifyHandling notify); + AddReviewerSender create(Project.NameKey project, Change.Id id); } @Inject public AddReviewerSender(EmailArguments ea, @Assisted Project.NameKey project, - @Assisted Change.Id id, - @Assisted @Nullable NotifyHandling notify) + @Assisted Change.Id id) throws OrmException { super(ea, newChangeData(ea, project, id)); - if (notify != null) { - setNotify(notify); - } } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java index 83ffab66e8..1cb6922cc1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.send; import com.google.common.collect.Multimap; 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.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; @@ -29,7 +30,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.account.AccountState; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.patch.PatchList; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java index 98a03e3593..d01e89d528 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java @@ -16,11 +16,11 @@ package com.google.gerrit.server.mail.send; import com.google.common.collect.Iterables; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java index 28fb714fa3..c45682815f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java @@ -15,11 +15,11 @@ package com.google.gerrit.server.mail.send; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.mail.RecipientType; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java index 49a909a747..c858b8ea0d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java @@ -15,8 +15,8 @@ package com.google.gerrit.server.mail.send; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java index 0ded9d8159..c08f24d9f7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java @@ -16,11 +16,11 @@ package com.google.gerrit.server.mail.send; import com.google.common.collect.Iterables; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.server.mail.Address; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gwtorm.server.OrmException; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java index fcbd790c85..48aee9994b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java @@ -14,18 +14,21 @@ package com.google.gerrit.server.mail.send; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS; import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Multimap; 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.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.UserIdentity; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.mail.Address; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.mail.send.EmailHeader.AddressList; import com.google.gerrit.server.validators.OutgoingEmailValidationListener; import com.google.gerrit.server.validators.ValidationException; @@ -73,6 +76,8 @@ public abstract class OutgoingEmail { private Address smtpFromAddress; private StringBuilder textBody; private StringBuilder htmlBody; + private Multimap accountsToNotify = + ImmutableListMultimap.of(); protected VelocityContext velocityContext; protected Map soyContext; protected Map soyContextEmailData; @@ -92,7 +97,12 @@ public abstract class OutgoingEmail { } public void setNotify(NotifyHandling notify) { - this.notify = notify; + this.notify = checkNotNull(notify); + } + + public void setAccountsToNotify( + Multimap accountsToNotify) { + this.accountsToNotify = checkNotNull(accountsToNotify); } /** @@ -101,7 +111,7 @@ public abstract class OutgoingEmail { * @throws EmailException */ public void send() throws EmailException { - if (NotifyHandling.NONE.equals(notify)) { + if (NotifyHandling.NONE.equals(notify) && accountsToNotify.isEmpty()) { return; } @@ -132,7 +142,8 @@ public abstract class OutgoingEmail { // on their behalf to others. // add(RecipientType.CC, fromId); - } else if (rcptTo.remove(fromId)) { + } else if (!accountsToNotify.containsValue(fromId) + && rcptTo.remove(fromId)) { // If they don't want a copy, but we queued one up anyway, // drop them from the recipient lists. // @@ -199,6 +210,10 @@ public abstract class OutgoingEmail { headers.put(HDR_CC, new EmailHeader.AddressList()); setHeader("Message-ID", ""); + for (RecipientType recipientType : accountsToNotify.keySet()) { + add(recipientType, accountsToNotify.get(recipientType)); + } + if (fromId != null) { // If we have a user that this message is supposedly caused by // but the From header on the email does not match the user as @@ -381,7 +396,9 @@ public abstract class OutgoingEmail { return false; } - if (smtpRcptTo.size() == 1 && rcptTo.size() == 1 && rcptTo.contains(fromId)) { + if ((accountsToNotify == null || accountsToNotify.isEmpty()) + && smtpRcptTo.size() == 1 && rcptTo.size() == 1 + && rcptTo.contains(fromId)) { // If the only recipient is also the sender, don't bother. // return false; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java index 03e9b7e563..b665690937 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java @@ -17,10 +17,10 @@ package com.google.gerrit.server.mail.send; import static com.google.common.base.Preconditions.checkNotNull; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.EmailTokenVerifier; -import com.google.gerrit.server.mail.RecipientType; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java index 8fa8c57a9f..39affb70c8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java @@ -15,11 +15,11 @@ package com.google.gerrit.server.mail.send; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.mail.RecipientType; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java index 02d3f32746..a6e2fa7329 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java @@ -15,9 +15,9 @@ package com.google.gerrit.server.mail.send; import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.mail.RecipientType; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException;