opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15' into stable-2.16

Browse Source 
* stable-2.15:
  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

Adjust newly added logging in OutgoingEmail to use Flogger. Inline the
logNotSending method.

Change-Id: I099c15cff79d8ab55fea098636ee9442b7d20a4d
This commit is contained in:
David Pursehouse
2019-10-31 16:22:45 +09:00
parent 15736bdad3 fd8b0f2ff2
commit b7e284a50f
1 changed files with 22 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
java/com/google/gerrit/server/mail/send/OutgoingEmail.java
View File

@@ -20,6 +20,7 @@ import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -97,13 +98,16 @@ public abstract class OutgoingEmail {
* @throws EmailException
*/
public void send() throws EmailException {
if (NotifyHandling.NONE.equals(notify) && accountsToNotify.isEmpty()) {
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 (NotifyHandling.NONE.equals(notify) && accountsToNotify.isEmpty()) {
logger.atFine().log("Not sending '%s': Notify handling is NONE", messageClass);
return;
}
@@ -154,6 +158,7 @@ public abstract class OutgoingEmail {
}
}
if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
logger.atFine().log("Not sending '%s': No SMTP recipients", messageClass);
return;
}
}
@@ -192,16 +197,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);
@@ -396,6 +411,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;
}
@@ -403,6 +419,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;
}
@@ -412,6 +429,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;
}