Allow uploader to control who should be notified on upload

A new 'notify' option is added that can be specified on push:

  git push ssh://bot@gerrit:29418/foo HEAD:refs/for/master%notify=NONE

It supports the same values as the notify option in ReviewInput:

  NONE, OWNER, OWNER_REVIEWERS, ALL

This is useful to reduce email noise when bot users are automatically
uploading changes and patch sets. E.g. a bot that is able to forge
committer and author may want to upload a commit without cc-ing the
author and the committer of that commit.

Change-Id: I473703a439018c3097f5c3de8f47d3f8371551ab
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-01-29 10:56:07 +01:00
parent 33543b7ed8
commit 9e078d805b
6 changed files with 103 additions and 14 deletions

View File

@@ -146,6 +146,27 @@ Other users (e.g. project owners) who have configured Gerrit to
notify them of new changes will be automatically sent an email
message when the push is completed.
[[notify]]
Uploaders can control to whom email notifications are sent by setting
the `notify` option:
* `NONE`: No email notification will be sent to anyone.
* `OWNER`: Only the change owner is notified.
* `OWNER_REVIEWERS`: Only owners and reviewers will be notified. This
includes all reviewers, existing reviewers of the change and new
reviewers that are added by the `reviewer` option or by mentioning
in the commit message.
* `ALL`: All email notifications will be sent. This includes
notifications to watchers, users that have starred the change, CCs
and the committer and author of the uploaded commit.
By default all email notifications are sent.
====
git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE
====
[[topic]]
To include a short tag associated with all of the changes in the
same group, such as the local topic branch name, append it after

View File

@@ -22,6 +22,7 @@ import com.google.common.base.MoreObjects;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
@@ -102,6 +103,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private Iterable<String> groups;
private CommitValidators.Policy validatePolicy =
CommitValidators.Policy.GERRIT;
private NotifyHandling notify = NotifyHandling.ALL;
private Set<Account.Id> reviewers;
private Set<Account.Id> extraCC;
private Map<String, Short> approvals;
@@ -206,6 +208,11 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
return this;
}
public ChangeInserter setNotify(NotifyHandling notify) {
this.notify = notify;
return this;
}
public ChangeInserter setReviewers(Set<Account.Id> reviewers) {
this.reviewers = reviewers;
return this;
@@ -347,6 +354,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
createChangeSenderFactory.create(change.getId());
cm.setFrom(change.getOwner());
cm.setPatchSet(patchSet, patchSetInfo);
cm.setNotify(notify);
cm.addReviewers(reviewers);
cm.addExtraCC(extraCC);
cm.send();

View File

@@ -62,6 +62,7 @@ import com.google.gerrit.common.data.LabelTypes;
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.ReviewInput.NotifyHandling;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicMap.Entry;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -1188,6 +1189,12 @@ public class ReceiveCommits {
@Option(name = "--submit", usage = "immediately submit the change")
boolean submit;
@Option(name = "--notify",
usage = "Notify handling that defines to whom email notifications "
+ "should be sent. Allowed values are NONE, OWNER, "
+ "OWNER_REVIEWERS, ALL. If not set, the default is ALL.")
NotifyHandling notify = NotifyHandling.ALL;
@Option(name = "--reviewer", aliases = {"-r"}, metaVar = "EMAIL",
usage = "add reviewer to changes")
void reviewer(Account.Id id) {
@@ -2372,6 +2379,9 @@ public class ReceiveCommits {
cm.setFrom(me);
cm.setPatchSet(newPatchSet, info);
cm.setChangeMessage(msg);
if (magicBranch != null) {
cm.setNotify(magicBranch.notify);
}
cm.addReviewers(recipients.getReviewers());
cm.addExtraCC(recipients.getCcOnly());
cm.send();

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.mail;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
@@ -294,6 +295,10 @@ public abstract class ChangeEmail extends NotificationEmail {
/** BCC any user who has starred this change. */
protected void bccStarredBy() {
if (!NotifyHandling.ALL.equals(notify)) {
return;
}
try {
// BCC anyone who has starred this change.
//
@@ -311,6 +316,10 @@ public abstract class ChangeEmail extends NotificationEmail {
@Override
protected final Watchers getWatchers(NotifyType type) throws OrmException {
if (!NotifyHandling.ALL.equals(notify)) {
return new Watchers();
}
ProjectWatch watch = new ProjectWatch(
args, branch.getParentKey(), projectState, changeData);
return watch.getWatchers(type);
@@ -318,6 +327,11 @@ public abstract class ChangeEmail extends NotificationEmail {
/** Any user who has published comments on this change. */
protected void ccAllApprovals() {
if (!NotifyHandling.ALL.equals(notify)
&& !NotifyHandling.OWNER_REVIEWERS.equals(notify)) {
return;
}
try {
for (Account.Id id : changeData.reviewers().values()) {
add(RecipientType.CC, id);
@@ -329,6 +343,11 @@ public abstract class ChangeEmail extends NotificationEmail {
/** Users who have non-zero approval codes on the change. */
protected void ccExistingReviewers() {
if (!NotifyHandling.ALL.equals(notify)
&& !NotifyHandling.OWNER_REVIEWERS.equals(notify)) {
return;
}
try {
for (Account.Id id : changeData.reviewers().get(REVIEWER)) {
add(RecipientType.CC, id);
@@ -356,7 +375,11 @@ public abstract class ChangeEmail extends NotificationEmail {
protected Set<Account.Id> getAuthors() {
Set<Account.Id> authors = new HashSet<>();
authors.add(change.getOwner());
switch (notify) {
case NONE:
break;
case ALL:
default:
if (patchSet != null) {
authors.add(patchSet.getUploader());
}
@@ -368,6 +391,13 @@ public abstract class ChangeEmail extends NotificationEmail {
authors.add(patchSetInfo.getCommitter().getAccount());
}
}
//$FALL-THROUGH$
case OWNER_REVIEWERS:
case OWNER:
authors.add(change.getOwner());
break;
}
return authors;
}

View File

@@ -49,8 +49,19 @@ public abstract class NewChangeSender extends ChangeEmail {
setHeader("Message-ID", getChangeMessageThreadId());
add(RecipientType.TO, reviewers);
switch (notify) {
case NONE:
case OWNER:
break;
case ALL:
default:
add(RecipientType.CC, extraCC);
//$FALL-THROUGH$
case OWNER_REVIEWERS:
add(RecipientType.TO, reviewers);
break;
}
rcptToAuthors(RecipientType.CC);
}

View File

@@ -18,6 +18,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.Sets;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.EmailStrategy;
import com.google.gerrit.reviewdb.client.UserIdentity;
@@ -66,7 +67,7 @@ public abstract class OutgoingEmail {
protected final EmailArguments args;
protected Account.Id fromId;
protected NotifyHandling notify = NotifyHandling.ALL;
protected OutgoingEmail(EmailArguments ea, String mc) {
args = ea;
@@ -78,12 +79,20 @@ public abstract class OutgoingEmail {
fromId = id;
}
public void setNotify(NotifyHandling notify) {
this.notify = notify;
}
/**
* Format and enqueue the message for delivery.
*
* @throws EmailException
*/
public void send() throws EmailException {
if (NotifyHandling.NONE.equals(notify)) {
return;
}
if (!args.emailSender.isEnabled()) {
// Server has explicitly disabled email sending.
//