Merge "Add notify section in project.config"

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

@@ -0,0 +1,104 @@
+// Copyright (C) 2012 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.git;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
+import com.google.gerrit.server.mail.Address;
+
+import java.util.EnumSet;
+import java.util.Set;
+
+public class NotifyConfig implements Comparable<NotifyConfig> {
+  private String name;
+  private EnumSet<NotifyType> types = EnumSet.of(NotifyType.ALL);
+  private String filter;
+
+  private Set<GroupReference> groups = Sets.newHashSet();
+  private Set<Address> addresses = Sets.newHashSet();
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public boolean isNotify(NotifyType type) {
+    return types.contains(type) || types.contains(NotifyType.ALL);
+  }
+
+  public EnumSet<NotifyType> getNotify() {
+    return types;
+  }
+
+  public void setTypes(EnumSet<NotifyType> newTypes) {
+    types = EnumSet.copyOf(newTypes);
+  }
+
+  public String getFilter() {
+    return filter;
+  }
+
+  public void setFilter(String filter) {
+    if ("*".equals(filter)) {
+      this.filter = null;
+    } else {
+      this.filter = Strings.emptyToNull(filter);
+    }
+  }
+
+  public Set<GroupReference> getGroups() {
+    return groups;
+  }
+
+  public Set<Address> getAddresses() {
+    return addresses;
+  }
+
+  public void addEmail(GroupReference group) {
+    groups.add(group);
+  }
+
+  public void addEmail(Address address) {
+    addresses.add(address);
+  }
+
+  @Override
+  public int compareTo(NotifyConfig o) {
+    return name.compareTo(o.name);
+  }
+
+  @Override
+  public int hashCode() {
+    return name.hashCode();
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof NotifyConfig) {
+      return compareTo((NotifyConfig) obj) == 0;
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return "NotifyConfig[" + name + " = " + addresses + " + " + groups + "]";
+  }
+}

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

@@ -16,6 +16,8 @@ package com.google.gerrit.server.git;
 
 import static com.google.gerrit.common.data.Permission.isPermission;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.ContributorAgreement;
 import com.google.gerrit.common.data.GlobalCapability;
@@ -23,17 +25,21 @@ import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.common.data.PermissionRule.Action;
+import com.google.gerrit.common.data.RefConfigSection;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.Project.State;
 import com.google.gerrit.reviewdb.client.Project.SubmitType;
 import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.common.data.RefConfigSection;
+import com.google.gerrit.server.config.ConfigUtil;
+import com.google.gerrit.server.mail.Address;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.util.StringUtils;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -41,6 +47,7 @@ import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -67,6 +74,11 @@ public class ProjectConfig extends VersionedMetaData {
   private static final String KEY_AUTO_VERIFY = "autoVerify";
   private static final String KEY_AGREEMENT_URL = "agreementUrl";
 
+  private static final String NOTIFY = "notify";
+  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 CAPABILITY = "capability";
 
   private static final String RECEIVE = "receive";
@@ -91,6 +103,7 @@ public class ProjectConfig extends VersionedMetaData {
   private Map<AccountGroup.UUID, GroupReference> groupsByUUID;
   private Map<String, AccessSection> accessSections;
   private Map<String, ContributorAgreement> contributorAgreements;
+  private Map<String, NotifyConfig> notifySections;
   private List<ValidationError> validationErrors;
   private ObjectId rulesId;
 
@@ -185,6 +198,10 @@ public class ProjectConfig extends VersionedMetaData {
     contributorAgreements.put(section.getName(), section);
   }
 
+  public Collection<NotifyConfig> getNotifyConfigs() {
+    return notifySections.values();
+  }
+
   public GroupReference resolve(AccountGroup group) {
     return resolve(GroupReference.forGroup(group));
   }
@@ -275,6 +292,7 @@ public class ProjectConfig extends VersionedMetaData {
     loadAccountsSection(rc, groupsByName);
     loadContributorAgreements(rc, groupsByName);
     loadAccessSections(rc, groupsByName);
+    loadNotifySections(rc, groupsByName);
   }
 
   private void loadAccountsSection(
@@ -318,6 +336,67 @@ public class ProjectConfig extends VersionedMetaData {
     }
   }
 
+  /**
+   * Parses the [notify] sections out of the configuration file.
+   *
+   * <pre>
+   *   [notify "reviewers"]
+   *     email = group Reviewers
+   *     type = new_changes
+   *
+   *   [notify "dev-team"]
+   *     email = dev-team@example.com
+   *     filter = branch:master
+   *
+   *   [notify "qa"]
+   *     email = qa@example.com
+   *     filter = branch:\"^(maint|stable)-.*\"
+   *     type = submitted_changes
+   * </pre>
+   */
+  private void loadNotifySections(
+      Config rc, Map<String, GroupReference> groupsByName) {
+    notifySections = Maps.newHashMap();
+    for (String sectionName : rc.getSubsections(NOTIFY)) {
+      NotifyConfig n = new NotifyConfig();
+      n.setName(sectionName);
+      n.setFilter(rc.getString(NOTIFY, sectionName, KEY_FILTER));
+
+      EnumSet<NotifyType> types = EnumSet.noneOf(NotifyType.class);
+      types.addAll(ConfigUtil.getEnumList(rc,
+          NOTIFY, sectionName, KEY_TYPE,
+          NotifyType.ALL));
+      n.setTypes(types);
+
+      for (String dst : rc.getStringList(NOTIFY, sectionName, KEY_EMAIL)) {
+        if (dst.startsWith("group ")) {
+          String groupName = dst.substring(6).trim();
+          GroupReference ref = groupsByName.get(groupName);
+          if (ref == null) {
+            ref = new GroupReference(null, groupName);
+            groupsByName.put(ref.getName(), ref);
+          }
+          if (ref.getUUID() != null) {
+            n.addEmail(ref);
+          } else {
+            error(new ValidationError(PROJECT_CONFIG,
+                "group \"" + ref.getName() + "\" not in " + GROUP_LIST));
+          }
+        } else if (dst.startsWith("user ")) {
+          error(new ValidationError(PROJECT_CONFIG, dst + " not supported"));
+        } else {
+          try {
+            n.addEmail(Address.parse(dst));
+          } catch (IllegalArgumentException err) {
+            error(new ValidationError(PROJECT_CONFIG,
+                "notify section \"" + sectionName + "\" has invalid email \"" + dst + "\""));
+          }
+        }
+      }
+      notifySections.put(sectionName, n);
+    }
+  }
+
   private void loadAccessSections(
       Config rc, Map<String, GroupReference> groupsByName) {
     accessSections = new HashMap<String, AccessSection>();
@@ -458,6 +537,7 @@ public class ProjectConfig extends VersionedMetaData {
     saveAccountsSection(rc, keepGroups);
     saveContributorAgreements(rc, keepGroups);
     saveAccessSections(rc, keepGroups);
+    saveNotifySections(rc, keepGroups);
     groupsByUUID.keySet().retainAll(keepGroups);
 
     saveConfig(PROJECT_CONFIG, rc);
@@ -493,6 +573,47 @@ public class ProjectConfig extends VersionedMetaData {
     }
   }
 
+  private void saveNotifySections(
+      Config rc, Set<AccountGroup.UUID> keepGroups) {
+    for (NotifyConfig nc : sort(notifySections.values())) {
+      List<String> email = Lists.newArrayList();
+      for (GroupReference gr : nc.getGroups()) {
+        if (gr.getUUID() != null) {
+          keepGroups.add(gr.getUUID());
+        }
+        email.add(new PermissionRule(gr).asString(false));
+      }
+      Collections.sort(email);
+
+      List<String> addrs = Lists.newArrayList();
+      for (Address addr : nc.getAddresses()) {
+        addrs.add(addr.toString());
+      }
+      Collections.sort(addrs);
+      email.addAll(addrs);
+
+      if (email.isEmpty()) {
+        rc.unset(NOTIFY, nc.getName(), KEY_EMAIL);
+      } else {
+        rc.setStringList(NOTIFY, nc.getName(), KEY_EMAIL, email);
+      }
+
+      if (nc.getNotify().equals(EnumSet.of(NotifyType.ALL))) {
+        rc.unset(NOTIFY, nc.getName(), KEY_TYPE);
+      } else {
+        List<String> types = Lists.newArrayListWithCapacity(4);
+        for (NotifyType t : NotifyType.values()) {
+          if (nc.isNotify(t)) {
+            types.add(StringUtils.toLowerCase(t.name()));
+          }
+        }
+        rc.setStringList(NOTIFY, nc.getName(), KEY_TYPE, types);
+      }
+
+      set(rc, NOTIFY, nc.getName(), KEY_FILTER, nc.getFilter());
+    }
+  }
+
   private List<String> ruleToStringList(
       List<PermissionRule> list, Set<AccountGroup.UUID> keepGroups) {
     List<String> rules = new ArrayList<String>();

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.mail;
 
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -38,7 +39,7 @@ public class AbandonedSender extends ReplyToChangeSender {
 
     ccAllApprovals();
     bccStarredBy();
-    bccWatchesNotifyAllComments();
+    bccWatches(NotifyType.ALL_COMMENTS);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java

@@ -54,6 +54,19 @@ public class Address {
     return email;
   }
 
+  @Override
+  public int hashCode() {
+    return email.hashCode();
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other instanceof Address) {
+      return email.equals(((Address) other).email);
+    }
+    return false;
+  }
+
   @Override
   public String toString() {
     try {

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

@@ -14,18 +14,25 @@
 
 package com.google.gerrit.server.mail;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountGroupInclude;
+import com.google.gerrit.reviewdb.client.AccountGroupMember;
 import com.google.gerrit.reviewdb.client.AccountProjectWatch;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.StarredChange;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.NotifyConfig;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListEntry;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
@@ -34,17 +41,17 @@ import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.SingleGroupUser;
 import com.google.gwtorm.server.OrmException;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.text.MessageFormat;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
-import java.util.List;
+import java.util.Queue;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -305,53 +312,148 @@ public abstract class ChangeEmail extends OutgoingEmail {
       // Just don't BCC 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 users that starred updated change", err);
     }
   }
 
-  /** BCC any user who has set "notify all comments" on this project. */
-  protected void bccWatchesNotifyAllComments() {
+  /** BCC users and groups that want notification of events. */
+  protected void bccWatches(NotifyType type) {
     try {
-      // BCC anyone else who has interest in this project's changes
-      //
-      for (final AccountProjectWatch w : getWatches()) {
-        if (w.isNotify(NotifyType.ALL_COMMENTS)) {
-          add(RecipientType.BCC, w.getAccountId());
-        }
+      Watchers matching = getWatches(type);
+      for (Account.Id user : matching.accounts) {
+        add(RecipientType.BCC, user);
+      }
+      for (Address addr : matching.emails) {
+        add(RecipientType.BCC, addr);
       }
     } 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 " + type, err);
     }
   }
 
   /** Returns all watches that are relevant */
-  protected final List<AccountProjectWatch> getWatches() throws OrmException {
+  protected final Watchers getWatches(NotifyType type) throws OrmException {
+    Watchers matching = new Watchers();
     if (changeData == null) {
-      return Collections.emptyList();
+      return matching;
     }
 
-    List<AccountProjectWatch> matching = new ArrayList<AccountProjectWatch>();
     Set<Account.Id> projectWatchers = new HashSet<Account.Id>();
 
     for (AccountProjectWatch w : args.db.get().accountProjectWatches()
         .byProject(change.getProject())) {
       projectWatchers.add(w.getAccountId());
-      add(matching, w);
-    }
-
-    for (AccountProjectWatch w : args.db.get().accountProjectWatches()
-        .byProject(args.allProjectsName)) {
-      if (!projectWatchers.contains(w.getAccountId())) {
+      if (w.isNotify(type)) {
         add(matching, w);
       }
     }
 
-    return Collections.unmodifiableList(matching);
+    for (AccountProjectWatch w : args.db.get().accountProjectWatches()
+        .byProject(args.allProjectsName)) {
+      if (!projectWatchers.contains(w.getAccountId()) && w.isNotify(type)) {
+        add(matching, w);
+      }
+    }
+
+    ProjectState state = projectState;
+    while (state != null) {
+      for (NotifyConfig nc : state.getConfig().getNotifyConfigs()) {
+        if (nc.isNotify(type)) {
+          try {
+            add(matching, nc, state.getProject().getNameKey());
+          } catch (QueryParseException e) {
+            log.warn(String.format(
+                "Project %s has invalid notify %s filter \"%s\": %s",
+                state.getProject().getName(), nc.getName(),
+                nc.getFilter(), e.getMessage()));
+          }
+        }
+      }
+      state = state.getParentState();
+    }
+
+    return matching;
+  }
+
+  protected static class Watchers {
+    protected final Set<Account.Id> accounts = Sets.newHashSet();
+    protected final Set<Address> emails = Sets.newHashSet();
   }
 
   @SuppressWarnings("unchecked")
-  private void add(List<AccountProjectWatch> matching, AccountProjectWatch w)
+  private void add(Watchers matching, NotifyConfig nc, Project.NameKey project)
+      throws OrmException, QueryParseException {
+    for (GroupReference ref : nc.getGroups()) {
+      AccountGroup group = args.groupCache.get(ref.getUUID());
+      if (group == null) {
+        log.warn(String.format(
+            "Project %s has invalid group %s in notify section %s",
+            project.get(), ref.getName(), nc.getName()));
+        continue;
+      }
+
+      if (group.getType() != AccountGroup.Type.INTERNAL) {
+        log.warn(String.format(
+            "Project %s cannot use group %s of type %s in notify section %s",
+            project.get(), ref.getName(), group.getType(), nc.getName()));
+        continue;
+      }
+
+      ChangeQueryBuilder qb = args.queryBuilder.create(new SingleGroupUser(
+          args.capabilityControlFactory,
+          ref.getUUID()));
+      qb.setAllowFile(true);
+      Predicate<ChangeData> p = qb.is_visible();
+      if (nc.getFilter() != null) {
+        p = Predicate.and(qb.parse(nc.getFilter()), p);
+        p = args.queryRewriter.get().rewrite(p);
+      }
+      if (p.match(changeData)) {
+        recursivelyAddAllAccounts(matching, group);
+      }
+    }
+
+    if (!nc.getAddresses().isEmpty()) {
+      if (nc.getFilter() != null) {
+        ChangeQueryBuilder qb = args.queryBuilder.create(args.anonymousUser);
+        qb.setAllowFile(true);
+        Predicate<ChangeData> p = qb.parse(nc.getFilter());
+        p = args.queryRewriter.get().rewrite(p);
+        if (p.match(changeData)) {
+          matching.emails.addAll(nc.getAddresses());
+        }
+      } else {
+        matching.emails.addAll(nc.getAddresses());
+      }
+    }
+  }
+
+  private void recursivelyAddAllAccounts(Watchers matching, AccountGroup group)
+      throws OrmException {
+    Set<AccountGroup.Id> seen = Sets.newHashSet();
+    Queue<AccountGroup.Id> scan = Lists.newLinkedList();
+    scan.add(group.getId());
+    seen.add(group.getId());
+    while (!scan.isEmpty()) {
+      AccountGroup.Id next = scan.remove();
+      for (AccountGroupMember m : args.db.get().accountGroupMembers()
+          .byGroup(next)) {
+        matching.accounts.add(m.getAccountId());
+      }
+      for (AccountGroupInclude m : args.db.get().accountGroupIncludes()
+          .byGroup(next)) {
+        if (seen.add(m.getIncludeId())) {
+          scan.add(m.getIncludeId());
+        }
+      }
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private void add(Watchers matching, AccountProjectWatch w)
       throws OrmException {
     IdentifiedUser user =
         args.identifiedUserFactory.create(args.db, w.getAccountId());
@@ -363,13 +465,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.add(w);
+          matching.accounts.add(w.getAccountId());
         }
       } catch (QueryParseException e) {
         // Ignore broken filter expressions.
       }
     } else if (p.match(changeData)) {
-      matching.add(w);
+      matching.accounts.add(w.getAccountId());
     }
   }
 

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.mail;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.patch.PatchFile;
 import com.google.gerrit.server.patch.PatchList;
@@ -68,7 +69,7 @@ public class CommentSender extends ReplyToChangeSender {
 
     ccAllApprovals();
     bccStarredBy();
-    bccWatchesNotifyAllComments();
+    bccWatches(NotifyType.ALL_COMMENTS);
   }
 
   @Override

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

@@ -15,74 +15,65 @@
 package com.google.gerrit.server.mail;
 
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
-import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.ssh.SshInfo;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
-import java.util.HashSet;
-import java.util.Set;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Notify interested parties of a brand new change. */
 public class CreateChangeSender extends NewChangeSender {
+  private static final Logger log =
+      LoggerFactory.getLogger(CreateChangeSender.class);
+
   public static interface Factory {
     public CreateChangeSender create(Change change);
   }
 
-  private final GroupCache groupCache;
-
   @Inject
   public CreateChangeSender(EmailArguments ea,
       @AnonymousCowardName String anonymousCowardName, SshInfo sshInfo,
-      GroupCache groupCache, @Assisted Change c) {
+      @Assisted Change c) {
     super(ea, anonymousCowardName, sshInfo, c);
-    this.groupCache = groupCache;
   }
 
   @Override
   protected void init() throws EmailException {
     super.init();
 
-    bccWatchers();
-  }
-
-  private void bccWatchers() {
     try {
+      // BCC anyone who has interest in this project's changes
       // Try to mark interested owners with a TO and not a BCC line.
       //
-      final Set<Account.Id> owners = new HashSet<Account.Id>();
-      for (AccountGroup.UUID uuid : getProjectOwners()) {
-        AccountGroup group = groupCache.get(uuid);
-        if (group != null) {
-          for (AccountGroupMember m : args.db.get().accountGroupMembers()
-              .byGroup(group.getId())) {
-            owners.add(m.getAccountId());
-          }
+      Watchers matching = getWatches(NotifyType.NEW_CHANGES);
+      for (Account.Id user : matching.accounts) {
+        if (isOwnerOfProjectOrBranch(user)) {
+          add(RecipientType.TO, user);
+        } else {
+          add(RecipientType.BCC, user);
         }
       }
-
-      // BCC anyone who has interest in this project's changes
-      //
-      for (final AccountProjectWatch w : getWatches()) {
-        if (w.isNotify(NotifyType.NEW_CHANGES)) {
-          if (owners.contains(w.getAccountId())) {
-            add(RecipientType.TO, w.getAccountId());
-          } else {
-            add(RecipientType.BCC, w.getAccountId());
-          }
-        }
+      for (Address addr : matching.emails) {
+        add(RecipientType.BCC, addr);
       }
     } 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);
     }
   }
+
+  private boolean isOwnerOfProjectOrBranch(Account.Id user) {
+    return projectState != null
+        && change != null
+        && projectState.controlFor(args.identifiedUserFactory.create(user))
+          .controlForRef(change.getDest())
+          .isOwner();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java

@@ -15,9 +15,11 @@
 package com.google.gerrit.server.mail;
 
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.IdentifiedUser.GenericFactory;
 import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.CapabilityControl;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.CanonicalWebUrl;
@@ -44,6 +46,8 @@ class EmailArguments {
   final EmailSender emailSender;
   final PatchSetInfoFactory patchSetInfoFactory;
   final IdentifiedUser.GenericFactory identifiedUserFactory;
+  final CapabilityControl.Factory capabilityControlFactory;
+  final AnonymousUser anonymousUser;
   final Provider<String> urlProvider;
   final AllProjectsName allProjectsName;
 
@@ -58,6 +62,8 @@ class EmailArguments {
       PatchListCache patchListCache, FromAddressGenerator fromAddressGenerator,
       EmailSender emailSender, PatchSetInfoFactory patchSetInfoFactory,
       GenericFactory identifiedUserFactory,
+      CapabilityControl.Factory capabilityControlFactory,
+      AnonymousUser anonymousUser,
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
       AllProjectsName allProjectsName,
       ChangeQueryBuilder.Factory queryBuilder,
@@ -72,6 +78,8 @@ class EmailArguments {
     this.emailSender = emailSender;
     this.patchSetInfoFactory = patchSetInfoFactory;
     this.identifiedUserFactory = identifiedUserFactory;
+    this.capabilityControlFactory = capabilityControlFactory;
+    this.anonymousUser = anonymousUser;
     this.urlProvider = urlProvider;
     this.allProjectsName = allProjectsName;
     this.queryBuilder = queryBuilder;

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

@@ -17,7 +17,6 @@ package com.google.gerrit.server.mail;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
 import com.google.gerrit.reviewdb.client.ApprovalCategory;
 import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
 import com.google.gerrit.reviewdb.client.Change;
@@ -53,8 +52,8 @@ public class MergedSender extends ReplyToChangeSender {
 
     ccAllApprovals();
     bccStarredBy();
-    bccWatchesNotifyAllComments();
-    bccWatchesNotifySubmittedChanges();
+    bccWatches(NotifyType.ALL_COMMENTS);
+    bccWatches(NotifyType.SUBMITTED_CHANGES);
   }
 
   @Override
@@ -140,20 +139,4 @@ public class MergedSender extends ReplyToChangeSender {
     }
     m.put(ca.getCategoryId(), ca);
   }
-
-  private void bccWatchesNotifySubmittedChanges() {
-    try {
-      // BCC anyone else who has interest in this project's changes
-      //
-      for (final AccountProjectWatch w : getWatches()) {
-        if (w.isNotify(NotifyType.SUBMITTED_CHANGES)) {
-          add(RecipientType.BCC, w.getAccountId());
-        }
-      }
-    } 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.
-    }
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.mail;
 
+import com.google.common.collect.Sets;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.server.account.AccountState;
@@ -34,14 +35,13 @@ import java.io.StringReader;
 import java.io.StringWriter;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /** Sends an email to one or more interested parties. */
 public abstract class OutgoingEmail {
@@ -53,7 +53,7 @@ public abstract class OutgoingEmail {
   protected String messageClass;
   private final HashSet<Account.Id> rcptTo = new HashSet<Account.Id>();
   private final Map<String, EmailHeader> headers;
-  private final List<Address> smtpRcptTo = new ArrayList<Address>();
+  private final Set<Address> smtpRcptTo = Sets.newHashSet();
   private Address smtpFromAddress;
   private StringBuilder body;
   protected VelocityContext velocityContext;
@@ -282,7 +282,7 @@ public abstract class OutgoingEmail {
       return false;
     }
 
-    if (rcptTo.size() == 1 && rcptTo.contains(fromId)) {
+    if (smtpRcptTo.size() == 1 && rcptTo.size() == 1 && rcptTo.contains(fromId)) {
       // If the only recipient is also the sender, don't bother.
       //
       return false;
@@ -324,14 +324,15 @@ public abstract class OutgoingEmail {
   protected void add(final RecipientType rt, final Address addr) {
     if (addr != null && addr.email != null && addr.email.length() > 0) {
       if (args.emailSender.canEmail(addr.email)) {
-        smtpRcptTo.add(addr);
-        switch (rt) {
-          case TO:
-            ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
-            break;
-          case CC:
-            ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
-            break;
+        if (smtpRcptTo.add(addr)) {
+          switch (rt) {
+            case TO:
+              ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
+              break;
+            case CC:
+              ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
+              break;
+          }
         }
       } else {
         log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.mail;
 
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -38,7 +39,7 @@ public class RestoredSender extends ReplyToChangeSender {
 
     ccAllApprovals();
     bccStarredBy();
-    bccWatchesNotifyAllComments();
+    bccWatches(NotifyType.ALL_COMMENTS);
   }
 
   @Override

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.mail;
 
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -37,7 +38,7 @@ public class RevertedSender extends ReplyToChangeSender {
 
     ccAllApprovals();
     bccStarredBy();
-    bccWatchesNotifyAllComments();
+    bccWatches(NotifyType.ALL_COMMENTS);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/query/Predicate.java

@@ -43,6 +43,12 @@ import java.util.List;
  * @type <T> type of object the predicate can evaluate in memory.
  */
 public abstract class Predicate<T> {
+  /** A predicate that matches any input, always, with no cost. */
+  @SuppressWarnings("unchecked")
+  public static <T> Predicate<T> any() {
+    return (Predicate<T>) Any.INSTANCE;
+  }
+
   /** Combine the passed predicates into a single AND node. */
   public static <T> Predicate<T> and(final Predicate<T>... that) {
     if (that.length == 1) {
@@ -120,4 +126,36 @@ public abstract class Predicate<T> {
 
   @Override
   public abstract boolean equals(Object other);
+
+  private static class Any<T> extends Predicate<T> {
+    private static final Any<Object> INSTANCE = new Any<Object>();
+
+    private Any() {
+    }
+
+    @Override
+    public Predicate<T> copy(Collection<? extends Predicate<T>> children) {
+      return this;
+    }
+
+    @Override
+    public boolean match(T object) {
+      return true;
+    }
+
+    @Override
+    public int getCost() {
+      return 0;
+    }
+
+    @Override
+    public int hashCode() {
+      return System.identityHashCode(this);
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      return other == this;
+    }
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/SingleGroupUser.java

@@ -27,15 +27,15 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Set;
 
-final class SingleGroupUser extends CurrentUser {
+public final class SingleGroupUser extends CurrentUser {
   private final GroupMembership groups;
 
-  SingleGroupUser(CapabilityControl.Factory capabilityControlFactory,
+  public SingleGroupUser(CapabilityControl.Factory capabilityControlFactory,
       AccountGroup.UUID groupId) {
     this(capabilityControlFactory, Collections.singleton(groupId));
   }
 
-  SingleGroupUser(CapabilityControl.Factory capabilityControlFactory,
+  public SingleGroupUser(CapabilityControl.Factory capabilityControlFactory,
       Set<AccountGroup.UUID> groups) {
     super(capabilityControlFactory, AccessPath.UNKNOWN);
     this.groups = new ListGroupMembership(groups);