commit deec95f8f997bcabca047a3f26fdbbd3d89795a2
author Kamil Musin <kamilm@google.com> Mon Mar 27 16:56:23 2023 +0200
committer Kamil Musin <kamilm@google.com> Tue Mar 28 11:41:37 2023 +0200
tree 6b616ab05934155f432fcf8ad7aad06c740a3a75
parent 2ea0f85765e46fdf99017c1d26ae686c2566e36e

Move emailOnlyAuthors and emailiOnlyAttentionSetIfEnabled to isRecipientAllowed.

Currently they are special overrides of add, which increases complexity,
requires special handlnig in init() and prevents from simplifying iheritance stack.

Google-Bug-Id: b/273913864
Release-Notes: skip
Change-Id: If69cf4d013fff63788ffc6c8ef9b2b338df9fd22

java/com/google/gerrit/server/mail/send/ChangeEmail.java

diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 7af8063..e8c43dc 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -108,6 +108,8 @@
   protected Set<Account.Id> authors;
   protected boolean emailOnlyAuthors;
   protected boolean emailOnlyAttentionSetIfEnabled;
+  // Watchers ignore attention set rules.
+  protected Set<Account.Id> watchers = new HashSet<>();
 
   protected ChangeEmail(EmailArguments args, String messageClass, ChangeData changeData) {
     super(args, messageClass);
@@ -421,16 +423,17 @@
   }
 
   /** Add users or email addresses to the TO, CC, or BCC list. */
-  protected void addWatchers(RecipientType type, WatcherList watcherList) {
+  private void addWatchers(RecipientType type, WatcherList watcherList) {
+    watchers.addAll(watcherList.accounts);
     for (Account.Id user : watcherList.accounts) {
-      addWatcher(type, user);
+      addByAccountId(type, user);
     }
     for (Address addr : watcherList.emails) {
       addByEmail(type, addr);
     }
   }
 
-  protected final Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig) {
+  private final Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig) {
     if (!NotifyHandling.ALL.equals(notify.handling())) {
       return new Watchers();
     }
@@ -472,37 +475,11 @@
   }
 
   @Override
-  protected void addByAccountId(RecipientType rt, Account.Id to) {
-    addRecipient(rt, to, /* isWatcher= */ false);
-  }
-
-  /** This bypasses the EmailStrategy.ATTENTION_SET_ONLY strategy when adding the recipient. */
-  protected void addWatcher(RecipientType rt, Account.Id to) {
-    addRecipient(rt, to, /* isWatcher= */ true);
-  }
-
-  private void addRecipient(RecipientType rt, Account.Id to, boolean isWatcher) {
-    if (!isWatcher) {
-      Optional<AccountState> accountState = args.accountCache.get(to);
-      if (emailOnlyAttentionSetIfEnabled
-          && accountState.isPresent()
-          && accountState.get().generalPreferences().getEmailStrategy()
-              == EmailStrategy.ATTENTION_SET_ONLY
-          && !currentAttentionSet.contains(to)) {
-        return;
-      }
-    }
-    if (emailOnlyAuthors && !authors.contains(to)) {
-      return;
-    }
-    super.addByAccountId(rt, to);
-  }
-
-  @Override
   protected boolean isRecipientAllowed(Address addr) throws PermissionBackendException {
     if (!projectState.statePermitsRead()) {
       return false;
     }
+
     return args.permissionBackend
         .user(args.anonymousUser.get())
         .change(changeData)
@@ -514,6 +491,20 @@
     if (!projectState.statePermitsRead()) {
       return false;
     }
+    if (emailOnlyAuthors && !authors.contains(to)) {
+      return false;
+    }
+    if (!watchers.contains(to)) {
+      Optional<AccountState> accountState = args.accountCache.get(to);
+      if (emailOnlyAttentionSetIfEnabled
+          && accountState.isPresent()
+          && accountState.get().generalPreferences().getEmailStrategy()
+              == EmailStrategy.ATTENTION_SET_ONLY
+          && !currentAttentionSet.contains(to)) {
+        return false;
+      }
+    }
+
     return args.permissionBackend.absentUser(to).change(changeData).test(ChangePermission.READ);
   }