opendev/gerrit
Code Issues Proposed changes

Support project notification using To or CC

Browse Source 
Some teams want to have new change notifications be CC'd
to a mailing list so that reply-all goes to the list.
Add notify.*.header = cc support to allow this usage.

Change-Id: I037b823a4127fe4d2ba0248e6be7f7efd7544b1c
This commit is contained in:
Shawn O. Pearce 2012-10-25 17:02:50 -07:00
parent 6738e5f340
commit aedcb7e808
10 changed files with 88 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Documentation/user-notify.txt
View File

@ -99,6 +99,16 @@ are sent.
+
Like email, this variable may be a list of options.
[[notify.name.header]]notify.<name>.header::
+
Email header used to list the destination. If not set BCC is used.
Only one value may be specified. To use different headers for each
address list them in different notify blocks.
+
* `to`: The standard To field is used; addresses are visible to all.
* `cc`: The standard CC field is used; addresses are visible to all.
* `bcc`: SMTP RCPT TO is used to hide the address.
[[notify.name.filter]]notify.<name>.filter::
+
link:user-search.html[Change search expression] to match changes that

13
gerrit-server/src/main/java/com/google/gerrit/server/git/NotifyConfig.java
View File

@ -24,10 +24,15 @@ import java.util.EnumSet;
import java.util.Set;
public class NotifyConfig implements Comparable<NotifyConfig> {
public static enum Header {
TO, CC, BCC;
}
private String name;
private EnumSet<NotifyType> types = EnumSet.of(NotifyType.ALL);
private String filter;
private Header header;
private Set<GroupReference> groups = Sets.newHashSet();
private Set<Address> addresses = Sets.newHashSet();
@ -63,6 +68,14 @@ public class NotifyConfig implements Comparable<NotifyConfig> {
}
}
public Header getHeader() {
return header;
}
public void setHeader(Header hdr) {
header = hdr;
}
public Set<GroupReference> getGroups() {
return groups;
}

6
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@ -79,6 +79,7 @@ public class ProjectConfig extends VersionedMetaData {
private static final String KEY_EMAIL = "email";
private static final String KEY_FILTER = "filter";
private static final String KEY_TYPE = "type";
private static final String KEY_HEADER = "header";
private static final String CAPABILITY = "capability";
@ -368,6 +369,9 @@ public class ProjectConfig extends VersionedMetaData {
NOTIFY, sectionName, KEY_TYPE,
NotifyType.ALL));
n.setTypes(types);
n.setHeader(ConfigUtil.getEnum(rc,
NOTIFY, sectionName, KEY_HEADER,
NotifyConfig.Header.BCC));
for (String dst : rc.getStringList(NOTIFY, sectionName, KEY_EMAIL)) {
if (dst.startsWith("group ")) {
@ -593,6 +597,8 @@ public class ProjectConfig extends VersionedMetaData {
Collections.sort(addrs);
email.addAll(addrs);
set(rc, NOTIFY, nc.getName(), KEY_HEADER,
nc.getHeader(), NotifyConfig.Header.BCC);
if (email.isEmpty()) {
rc.unset(NOTIFY, nc.getName(), KEY_EMAIL);
} else {

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java
View File

@ -39,7 +39,7 @@ public class AbandonedSender extends ReplyToChangeSender {
ccAllApprovals();
bccStarredBy();
bccWatches(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.ALL_COMMENTS);
}
@Override

58
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@ -322,16 +322,13 @@ public abstract class ChangeEmail extends OutgoingEmail {
}
}
/** BCC users and groups that want notification of events. */
protected void bccWatches(NotifyType type) {
/** Include users and groups that want notification of events. */
protected void includeWatchers(NotifyType type) {
try {
Watchers matching = getWatches(type);
for (Account.Id user : matching.accounts) {
add(RecipientType.BCC, user);
}
for (Address addr : matching.emails) {
add(RecipientType.BCC, addr);
}
add(RecipientType.TO, matching.to);
add(RecipientType.CC, matching.cc);
add(RecipientType.BCC, matching.bcc);
} catch (OrmException err) {
// Just don't CC everyone. Better to send a partial message to those
// we already have queued up then to fail deliver entirely to people
@ -340,6 +337,16 @@ public abstract class ChangeEmail extends OutgoingEmail {
}
}
/** Add users or email addresses to the TO, CC, or BCC list. */
protected void add(RecipientType type, Watchers.List list) {
for (Account.Id user : list.accounts) {
add(type, user);
}
for (Address addr : list.emails) {
add(type, addr);
}
}
/** Returns all watches that are relevant */
protected final Watchers getWatches(NotifyType type) throws OrmException {
Watchers matching = new Watchers();
@ -385,8 +392,25 @@ public abstract class ChangeEmail extends OutgoingEmail {
}
protected static class Watchers {
protected final Set<Account.Id> accounts = Sets.newHashSet();
protected final Set<Address> emails = Sets.newHashSet();
static class List {
protected final Set<Account.Id> accounts = Sets.newHashSet();
protected final Set<Address> emails = Sets.newHashSet();
}
protected final List to = new List();
protected final List cc = new List();
protected final List bcc = new List();
List list(NotifyConfig.Header header) {
switch (header) {
case TO:
return to;
case CC:
return cc;
default:
case BCC:
return bcc;
}
}
}
@SuppressWarnings("unchecked")
@ -419,7 +443,7 @@ public abstract class ChangeEmail extends OutgoingEmail {
p = args.queryRewriter.get().rewrite(p);
}
if (p.match(changeData)) {
recursivelyAddAllAccounts(matching, group);
recursivelyAddAllAccounts(matching.list(nc.getHeader()), group);
}
}
@ -430,16 +454,16 @@ public abstract class ChangeEmail extends OutgoingEmail {
Predicate<ChangeData> p = qb.parse(nc.getFilter());
p = args.queryRewriter.get().rewrite(p);
if (p.match(changeData)) {
matching.emails.addAll(nc.getAddresses());
matching.list(nc.getHeader()).emails.addAll(nc.getAddresses());
}
} else {
matching.emails.addAll(nc.getAddresses());
matching.list(nc.getHeader()).emails.addAll(nc.getAddresses());
}
}
}
private void recursivelyAddAllAccounts(Watchers matching, AccountGroup group)
throws OrmException {
private void recursivelyAddAllAccounts(Watchers.List matching,
AccountGroup group) throws OrmException {
Set<AccountGroup.Id> seen = Sets.newHashSet();
Queue<AccountGroup.Id> scan = Lists.newLinkedList();
scan.add(group.getId());
@ -472,13 +496,13 @@ public abstract class ChangeEmail extends OutgoingEmail {
p = Predicate.and(qb.parse(w.getFilter()), p);
p = args.queryRewriter.get().rewrite(p);
if (p.match(changeData)) {
matching.accounts.add(w.getAccountId());
matching.bcc.accounts.add(w.getAccountId());
}
} catch (QueryParseException e) {
// Ignore broken filter expressions.
}
} else if (p.match(changeData)) {
matching.accounts.add(w.getAccountId());
matching.bcc.accounts.add(w.getAccountId());
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
View File

@ -69,7 +69,7 @@ public class CommentSender extends ReplyToChangeSender {
ccAllApprovals();
bccStarredBy();
bccWatches(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.ALL_COMMENTS);
}
@Override

22
gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java
View File

@ -14,6 +14,7 @@
package com.google.gerrit.server.mail;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Change;
@ -47,25 +48,26 @@ public class CreateChangeSender extends NewChangeSender {
super.init();
try {
// BCC anyone who has interest in this project's changes
// Try to mark interested owners with a TO and not a BCC line.
//
// Try to mark interested owners with TO and CC or BCC line.
Watchers matching = getWatches(NotifyType.NEW_CHANGES);
for (Account.Id user : matching.accounts) {
for (Account.Id user : Iterables.concat(
matching.to.accounts,
matching.cc.accounts,
matching.bcc.accounts)) {
if (isOwnerOfProjectOrBranch(user)) {
add(RecipientType.TO, user);
} else {
add(RecipientType.BCC, user);
}
}
for (Address addr : matching.emails) {
add(RecipientType.BCC, addr);
}
// Add everyone else. Owners added above will not be duplicated.
add(RecipientType.TO, matching.to);
add(RecipientType.CC, matching.cc);
add(RecipientType.BCC, matching.bcc);
} catch (OrmException err) {
// Just don't CC everyone. Better to send a partial message to those
// we already have queued up then to fail deliver entirely to people
// who have a lower interest in the change.
log.warn("Cannot BCC watchers for new change", err);
log.warn("Cannot notify watchers for new change", err);
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
View File

@ -52,8 +52,8 @@ public class MergedSender extends ReplyToChangeSender {
ccAllApprovals();
bccStarredBy();
bccWatches(NotifyType.ALL_COMMENTS);
bccWatches(NotifyType.SUBMITTED_CHANGES);
includeWatchers(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.SUBMITTED_CHANGES);
}
@Override

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java
View File

@ -39,7 +39,7 @@ public class RestoredSender extends ReplyToChangeSender {
ccAllApprovals();
bccStarredBy();
bccWatches(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.ALL_COMMENTS);
}
@Override

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java
View File

@ -38,7 +38,7 @@ public class RevertedSender extends ReplyToChangeSender {
ccAllApprovals();
bccStarredBy();
bccWatches(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.ALL_COMMENTS);
}
@Override