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;