From aedcb7e808238a0ba0aa7fc2725329d3b3a90198 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 25 Oct 2012 17:02:50 -0700 Subject: [PATCH] Support project notification using To or CC 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 --- Documentation/user-notify.txt | 10 ++++ .../gerrit/server/git/NotifyConfig.java | 13 +++++ .../gerrit/server/git/ProjectConfig.java | 6 ++ .../gerrit/server/mail/AbandonedSender.java | 2 +- .../gerrit/server/mail/ChangeEmail.java | 58 +++++++++++++------ .../gerrit/server/mail/CommentSender.java | 2 +- .../server/mail/CreateChangeSender.java | 22 +++---- .../gerrit/server/mail/MergedSender.java | 4 +- .../gerrit/server/mail/RestoredSender.java | 2 +- .../gerrit/server/mail/RevertedSender.java | 2 +- 10 files changed, 88 insertions(+), 33 deletions(-) diff --git a/Documentation/user-notify.txt b/Documentation/user-notify.txt index ae3c2d09c5..fc5e49bed5 100644 --- a/Documentation/user-notify.txt +++ b/Documentation/user-notify.txt @@ -99,6 +99,16 @@ are sent. + Like email, this variable may be a list of options. +[[notify.name.header]]notify..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..filter:: + link:user-search.html[Change search expression] to match changes that diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/NotifyConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/NotifyConfig.java index ba2833dc8e..3a0c278cc8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/NotifyConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/NotifyConfig.java @@ -24,10 +24,15 @@ import java.util.EnumSet; import java.util.Set; public class NotifyConfig implements Comparable { + public static enum Header { + TO, CC, BCC; + } + private String name; private EnumSet types = EnumSet.of(NotifyType.ALL); private String filter; + private Header header; private Set groups = Sets.newHashSet(); private Set
addresses = Sets.newHashSet(); @@ -63,6 +68,14 @@ public class NotifyConfig implements Comparable { } } + public Header getHeader() { + return header; + } + + public void setHeader(Header hdr) { + header = hdr; + } + public Set getGroups() { return groups; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index c6657aaee5..2d775e7354 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -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 { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java index 0eb3dfe367..b6ad8d8b8b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java @@ -39,7 +39,7 @@ public class AbandonedSender extends ReplyToChangeSender { ccAllApprovals(); bccStarredBy(); - bccWatches(NotifyType.ALL_COMMENTS); + includeWatchers(NotifyType.ALL_COMMENTS); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java index 18683abd90..1bcf7321c7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java @@ -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 accounts = Sets.newHashSet(); - protected final Set
emails = Sets.newHashSet(); + static class List { + protected final Set accounts = Sets.newHashSet(); + protected final Set
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 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 seen = Sets.newHashSet(); Queue 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()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java index bfba1b35a5..2d0046c4b1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java @@ -69,7 +69,7 @@ public class CommentSender extends ReplyToChangeSender { ccAllApprovals(); bccStarredBy(); - bccWatches(NotifyType.ALL_COMMENTS); + includeWatchers(NotifyType.ALL_COMMENTS); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java index 9b82c715a0..3ff2c2ac89 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java @@ -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); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java index 70b2d7fa2b..db56176329 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java @@ -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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java index 946c29fcda..55884e944c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java @@ -39,7 +39,7 @@ public class RestoredSender extends ReplyToChangeSender { ccAllApprovals(); bccStarredBy(); - bccWatches(NotifyType.ALL_COMMENTS); + includeWatchers(NotifyType.ALL_COMMENTS); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java index 033bd5656e..d62e82a493 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java @@ -38,7 +38,7 @@ public class RevertedSender extends ReplyToChangeSender { ccAllApprovals(); bccStarredBy(); - bccWatches(NotifyType.ALL_COMMENTS); + includeWatchers(NotifyType.ALL_COMMENTS); } @Override