Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  OutgoingEmail: Sanity check recipients of multipart and plaintext
  OutgoingEmail: Log when email is sent as multipart and plaintext
  OutgoingEmail: Log the reason that email is not being sent
  OutgoingEmail#send: Check first if email sending is enabled

Change-Id: I1e18e6f8db77900149d3bf0754b7fb4c64b408bf

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

@@ -18,6 +18,7 @@ import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailSt
 import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.exceptions.EmailException;
 import com.google.gerrit.extensions.api.changes.RecipientType;
@@ -90,13 +91,16 @@ public abstract class OutgoingEmail {
    * @throws EmailException
    */
   public void send() throws EmailException {
-    if (!notify.shouldNotify()) {
-      return;
-    }
-
     if (!args.emailSender.isEnabled()) {
       // Server has explicitly disabled email sending.
       //
+      logger.atFine().log(
+          "Not sending '%s': Email sending is disabled by server config", messageClass);
+      return;
+    }
+
+    if (!notify.shouldNotify()) {
+      logger.atFine().log("Not sending '%s': Notify handling is NONE", messageClass);
       return;
     }
 
@@ -147,6 +151,7 @@ public abstract class OutgoingEmail {
           }
         }
         if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
+          logger.atFine().log("Not sending '%s': No SMTP recipients", messageClass);
           return;
         }
       }
@@ -185,16 +190,26 @@ public abstract class OutgoingEmail {
         try {
           validator.validateOutgoingEmail(va);
         } catch (ValidationException e) {
+          logger.atFine().log(
+              "Not sending '%s': Rejected by outgoing email validator: %s",
+              messageClass, e.getMessage());
           return;
         }
       }
 
+      Set<Address> intersection = Sets.intersection(smtpRcptTo, smtpRcptToPlaintextOnly);
+      if (!intersection.isEmpty()) {
+        logger.atSevere().log("Email '%s' will be sent twice to %s", messageClass, intersection);
+      }
+
       if (!smtpRcptTo.isEmpty()) {
         // Send multipart message
+        logger.atFine().log("Sending multipart '%s'", messageClass);
         args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
       }
 
       if (!smtpRcptToPlaintextOnly.isEmpty()) {
+        logger.atFine().log("Sending plaintext '%s'", messageClass);
         // Send plaintext message
         Map<String, EmailHeader> shallowCopy = new HashMap<>();
         shallowCopy.putAll(headers);
@@ -389,6 +404,7 @@ public abstract class OutgoingEmail {
   protected boolean shouldSendMessage() {
     if (textBody.length() == 0) {
       // If we have no message body, don't send.
+      logger.atFine().log("Not sending '%s': No message body", messageClass);
       return false;
     }
 
@@ -396,6 +412,7 @@ public abstract class OutgoingEmail {
       // If we have nobody to send this message to, then all of our
       // selection filters previously for this type of message were
       // unable to match a destination. Don't bother sending it.
+      logger.atFine().log("Not sending '%s': No recipients", messageClass);
       return false;
     }
 
@@ -405,6 +422,7 @@ public abstract class OutgoingEmail {
         && rcptTo.contains(fromId)) {
       // If the only recipient is also the sender, don't bother.
       //
+      logger.atFine().log("Not sending '%s': Sender is only recipient", messageClass);
       return false;
     }