diff --git a/gradle/proxy/gradle/dependency-locks/compile.lockfile b/gradle/proxy/gradle/dependency-locks/compile.lockfile index 718dd1e29..a315d4d8d 100644 --- a/gradle/proxy/gradle/dependency-locks/compile.lockfile +++ b/gradle/proxy/gradle/dependency-locks/compile.lockfile @@ -10,6 +10,7 @@ com.google.apis:google-api-services-storage:v1-rev150-1.22.0 com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 com.google.auto.value:auto-value-annotations:1.6.2 +com.google.auto.value:auto-value:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.code.gson:gson:2.8.5 com.google.dagger:dagger:2.21 diff --git a/gradle/proxy/gradle/dependency-locks/compileClasspath.lockfile b/gradle/proxy/gradle/dependency-locks/compileClasspath.lockfile index 718dd1e29..a315d4d8d 100644 --- a/gradle/proxy/gradle/dependency-locks/compileClasspath.lockfile +++ b/gradle/proxy/gradle/dependency-locks/compileClasspath.lockfile @@ -10,6 +10,7 @@ com.google.apis:google-api-services-storage:v1-rev150-1.22.0 com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 com.google.auto.value:auto-value-annotations:1.6.2 +com.google.auto.value:auto-value:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.code.gson:gson:2.8.5 com.google.dagger:dagger:2.21 diff --git a/gradle/util/build.gradle b/gradle/util/build.gradle index 98219bfaf..b6a650d57 100644 --- a/gradle/util/build.gradle +++ b/gradle/util/build.gradle @@ -2,6 +2,7 @@ dependencies { def deps = rootProject.dependencyMap compile deps['com.google.appengine:appengine-api-1.0-sdk'] compile deps['com.google.appengine:appengine-testing'] + compile deps['com.google.auto.value:auto-value-annotations'] compile deps['com.google.code.findbugs:jsr305'] compile deps['com.google.dagger:dagger'] compile deps['com.google.flogger:flogger'] @@ -13,6 +14,7 @@ dependencies { compile deps['javax.xml.bind:jaxb-api'] compile deps['joda-time:joda-time'] compile deps['org.yaml:snakeyaml'] + runtime deps['com.google.auto.value:auto-value'] testCompile deps['com.google.appengine:appengine-api-stubs'] testCompile deps['com.google.guava:guava-testlib'] testCompile deps['com.google.truth:truth'] @@ -23,7 +25,9 @@ dependencies { testCompile files("${rootDir}/../third_party/objectify/v4_1/objectify-4.1.3.jar") testCompile project(':third_party') testCompile project(path: ':core', configuration: 'testRuntime') + annotationProcessor deps['com.google.auto.value:auto-value'] annotationProcessor deps['com.google.dagger:dagger-compiler'] + testAnnotationProcessor deps['com.google.auto.value:auto-value'] testAnnotationProcessor deps['com.google.dagger:dagger-compiler'] } diff --git a/gradle/util/gradle/dependency-locks/annotationProcessor.lockfile b/gradle/util/gradle/dependency-locks/annotationProcessor.lockfile index d33f8270a..9d5ca31d9 100644 --- a/gradle/util/gradle/dependency-locks/annotationProcessor.lockfile +++ b/gradle/util/gradle/dependency-locks/annotationProcessor.lockfile @@ -3,6 +3,7 @@ # This file is expected to be part of source control. com.github.kevinstern:software-and-algorithms:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1 +com.google.auto.value:auto-value:1.6.2 com.google.auto:auto-common:0.10 com.google.code.findbugs:jFormatString:3.0.0 com.google.code.findbugs:jsr305:3.0.0 diff --git a/gradle/util/gradle/dependency-locks/compile.lockfile b/gradle/util/gradle/dependency-locks/compile.lockfile index 2ca207e3b..b6921e34a 100644 --- a/gradle/util/gradle/dependency-locks/compile.lockfile +++ b/gradle/util/gradle/dependency-locks/compile.lockfile @@ -3,6 +3,7 @@ # This file is expected to be part of source control. com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 +com.google.auto.value:auto-value-annotations:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.dagger:dagger:2.21 com.google.errorprone:error_prone_annotations:2.2.0 diff --git a/gradle/util/gradle/dependency-locks/compileClasspath.lockfile b/gradle/util/gradle/dependency-locks/compileClasspath.lockfile index 2ca207e3b..b6921e34a 100644 --- a/gradle/util/gradle/dependency-locks/compileClasspath.lockfile +++ b/gradle/util/gradle/dependency-locks/compileClasspath.lockfile @@ -3,6 +3,7 @@ # This file is expected to be part of source control. com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 +com.google.auto.value:auto-value-annotations:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.dagger:dagger:2.21 com.google.errorprone:error_prone_annotations:2.2.0 diff --git a/gradle/util/gradle/dependency-locks/default.lockfile b/gradle/util/gradle/dependency-locks/default.lockfile index 2ca207e3b..9feca863f 100644 --- a/gradle/util/gradle/dependency-locks/default.lockfile +++ b/gradle/util/gradle/dependency-locks/default.lockfile @@ -3,6 +3,8 @@ # This file is expected to be part of source control. com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 +com.google.auto.value:auto-value-annotations:1.6.2 +com.google.auto.value:auto-value:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.dagger:dagger:2.21 com.google.errorprone:error_prone_annotations:2.2.0 diff --git a/gradle/util/gradle/dependency-locks/runtime.lockfile b/gradle/util/gradle/dependency-locks/runtime.lockfile index 2ca207e3b..9feca863f 100644 --- a/gradle/util/gradle/dependency-locks/runtime.lockfile +++ b/gradle/util/gradle/dependency-locks/runtime.lockfile @@ -3,6 +3,8 @@ # This file is expected to be part of source control. com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 +com.google.auto.value:auto-value-annotations:1.6.2 +com.google.auto.value:auto-value:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.dagger:dagger:2.21 com.google.errorprone:error_prone_annotations:2.2.0 diff --git a/gradle/util/gradle/dependency-locks/runtimeClasspath.lockfile b/gradle/util/gradle/dependency-locks/runtimeClasspath.lockfile index 2ca207e3b..9feca863f 100644 --- a/gradle/util/gradle/dependency-locks/runtimeClasspath.lockfile +++ b/gradle/util/gradle/dependency-locks/runtimeClasspath.lockfile @@ -3,6 +3,8 @@ # This file is expected to be part of source control. com.google.appengine:appengine-api-1.0-sdk:1.9.48 com.google.appengine:appengine-testing:1.9.58 +com.google.auto.value:auto-value-annotations:1.6.2 +com.google.auto.value:auto-value:1.6.2 com.google.code.findbugs:jsr305:3.0.2 com.google.dagger:dagger:2.21 com.google.errorprone:error_prone_annotations:2.2.0 diff --git a/gradle/util/gradle/dependency-locks/testAnnotationProcessor.lockfile b/gradle/util/gradle/dependency-locks/testAnnotationProcessor.lockfile index d33f8270a..9d5ca31d9 100644 --- a/gradle/util/gradle/dependency-locks/testAnnotationProcessor.lockfile +++ b/gradle/util/gradle/dependency-locks/testAnnotationProcessor.lockfile @@ -3,6 +3,7 @@ # This file is expected to be part of source control. com.github.kevinstern:software-and-algorithms:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1 +com.google.auto.value:auto-value:1.6.2 com.google.auto:auto-common:0.10 com.google.code.findbugs:jFormatString:3.0.0 com.google.code.findbugs:jsr305:3.0.0 diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index afc6b4516..c40801fa9 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -15,6 +15,7 @@ package google.registry.config; import static com.google.common.base.Suppliers.memoize; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.config.ConfigUtils.makeUrl; import static google.registry.util.ResourceUtils.readResourceUtf8; import static java.lang.annotation.RetentionPolicy.RUNTIME; @@ -41,6 +42,8 @@ import javax.annotation.Nullable; import javax.inject.Named; import javax.inject.Qualifier; import javax.inject.Singleton; +import javax.mail.internet.AddressException; +import javax.mail.internet.InternetAddress; import org.joda.time.DateTimeConstants; import org.joda.time.Duration; @@ -510,8 +513,8 @@ public final class RegistryConfig { */ @Provides @Config("gSuiteOutgoingEmailAddress") - public static String provideGSuiteOutgoingEmailAddress(RegistryConfigSettings config) { - return config.gSuite.outgoingEmailAddress; + public static InternetAddress provideGSuiteOutgoingEmailAddress(RegistryConfigSettings config) { + return parseEmailAddress(config.gSuite.outgoingEmailAddress); } /** @@ -697,9 +700,11 @@ public final class RegistryConfig { */ @Provides @Config("invoiceEmailRecipients") - public static ImmutableList provideInvoiceEmailRecipients( + public static ImmutableList provideInvoiceEmailRecipients( RegistryConfigSettings config) { - return ImmutableList.copyOf(config.billing.invoiceEmailRecipients); + return config.billing.invoiceEmailRecipients.stream() + .map(RegistryConfig::parseEmailAddress) + .collect(toImmutableList()); } /** @@ -863,14 +868,13 @@ public final class RegistryConfig { *

This allows us to easily verify the success or failure of periodic tasks by passively * checking e-mail. * - * @see google.registry.reporting.icann.ReportingEmailUtils * @see google.registry.reporting.billing.BillingEmailUtils * @see google.registry.reporting.spec11.Spec11EmailUtils */ @Provides @Config("alertRecipientEmailAddress") - public static String provideAlertRecipientEmailAddress(RegistryConfigSettings config) { - return config.misc.alertRecipientEmailAddress; + public static InternetAddress provideAlertRecipientEmailAddress(RegistryConfigSettings config) { + return parseEmailAddress(config.misc.alertRecipientEmailAddress); } /** @@ -880,8 +884,8 @@ public final class RegistryConfig { */ @Provides @Config("spec11ReplyToEmailAddress") - public static String provideSpec11ReplyToEmailAddress(RegistryConfigSettings config) { - return config.misc.spec11ReplyToEmailAddress; + public static InternetAddress provideSpec11ReplyToEmailAddress(RegistryConfigSettings config) { + return parseEmailAddress(config.misc.spec11ReplyToEmailAddress); } /** @@ -931,7 +935,7 @@ public final class RegistryConfig { /** * Number of times to retry a GAE operation when {@code TransientFailureException} is thrown. * - *

The number of milliseconds it'll sleep before giving up is {@code 2^n - 2}. + *

The number of milliseconds it'll sleep before giving up is {@code (2^n - 2) * 100}. * *

Note that this uses {@code @Named} instead of {@code @Config} so that it can be used from * the low-level util package, which cannot have a dependency on the config package. @@ -940,8 +944,8 @@ public final class RegistryConfig { */ @Provides @Named("transientFailureRetries") - public static int provideTransientFailureRetries() { - return 12; // Four seconds. + public static int provideTransientFailureRetries(RegistryConfigSettings config) { + return config.misc.transientFailureRetries; } /** @@ -1489,8 +1493,8 @@ public final class RegistryConfig { } /** Returns the email address that outgoing emails from the app are sent from. */ - public static String getGSuiteOutgoingEmailAddress() { - return CONFIG_SETTINGS.get().gSuite.outgoingEmailAddress; + public static InternetAddress getGSuiteOutgoingEmailAddress() { + return parseEmailAddress(CONFIG_SETTINGS.get().gSuite.outgoingEmailAddress); } /** Returns the display name that outgoing emails from the app are sent from. */ @@ -1548,5 +1552,13 @@ public final class RegistryConfig { .collect(Collectors.joining("\n")); } + private static InternetAddress parseEmailAddress(String email) { + try { + return new InternetAddress(email); + } catch (AddressException e) { + throw new IllegalArgumentException(String.format("Could not parse email address %s.", email)); + } + } + private RegistryConfig() {} } diff --git a/java/google/registry/config/RegistryConfigSettings.java b/java/google/registry/config/RegistryConfigSettings.java index c7b7c1c8e..dda79a8c8 100644 --- a/java/google/registry/config/RegistryConfigSettings.java +++ b/java/google/registry/config/RegistryConfigSettings.java @@ -169,6 +169,7 @@ public class RegistryConfigSettings { public String alertRecipientEmailAddress; public String spec11ReplyToEmailAddress; public int asyncDeleteDelaySeconds; + public int transientFailureRetries; } /** Configuration for keyrings (used to store secrets outside of source). */ diff --git a/java/google/registry/config/files/default-config.yaml b/java/google/registry/config/files/default-config.yaml index e6a4c0f8b..8aa53a12f 100644 --- a/java/google/registry/config/files/default-config.yaml +++ b/java/google/registry/config/files/default-config.yaml @@ -361,6 +361,10 @@ misc: # hosts from being used on domains. asyncDeleteDelaySeconds: 90 + # Number of times to retry a GAE operation when a transient exception is thrown. + # The number of milliseconds it'll sleep before giving up is (2^n - 2) * 100. + transientFailureRetries: 12 + beam: # The default zone to run Apache Beam (Cloud Dataflow) jobs in. defaultJobZone: us-east1-c diff --git a/java/google/registry/config/files/nomulus-config-unittest.yaml b/java/google/registry/config/files/nomulus-config-unittest.yaml index 00501f26e..672ba01df 100644 --- a/java/google/registry/config/files/nomulus-config-unittest.yaml +++ b/java/google/registry/config/files/nomulus-config-unittest.yaml @@ -27,3 +27,7 @@ caching: # tests gSuite: supportGroupEmailAddress: + +misc: + # We would rather have failures than timeouts, so reduce the number of retries + transientFailureRetries: 3 diff --git a/java/google/registry/reporting/billing/BillingEmailUtils.java b/java/google/registry/reporting/billing/BillingEmailUtils.java index e15c26a72..7bc40a40c 100644 --- a/java/google/registry/reporting/billing/BillingEmailUtils.java +++ b/java/google/registry/reporting/billing/BillingEmailUtils.java @@ -20,23 +20,17 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.appengine.tools.cloudstorage.GcsFilename; import com.google.common.collect.ImmutableList; import com.google.common.io.CharStreams; +import com.google.common.net.MediaType; import google.registry.config.RegistryConfig.Config; import google.registry.gcs.GcsUtils; import google.registry.reporting.billing.BillingModule.InvoiceDirectoryPrefix; -import google.registry.util.Retrier; +import google.registry.util.EmailMessage; +import google.registry.util.EmailMessage.Attachment; import google.registry.util.SendEmailService; -import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import javax.inject.Inject; -import javax.mail.BodyPart; -import javax.mail.Message; -import javax.mail.Message.RecipientType; -import javax.mail.MessagingException; -import javax.mail.Multipart; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeBodyPart; -import javax.mail.internet.MimeMultipart; import org.joda.time.YearMonth; /** Utility functions for sending emails involving monthly invoices. */ @@ -44,27 +38,25 @@ class BillingEmailUtils { private final SendEmailService emailService; private final YearMonth yearMonth; - private final String outgoingEmailAddress; - private final String alertRecipientAddress; - private final ImmutableList invoiceEmailRecipients; + private final InternetAddress outgoingEmailAddress; + private final InternetAddress alertRecipientAddress; + private final ImmutableList invoiceEmailRecipients; private final String billingBucket; private final String invoiceFilePrefix; private final String invoiceDirectoryPrefix; private final GcsUtils gcsUtils; - private final Retrier retrier; @Inject BillingEmailUtils( SendEmailService emailService, YearMonth yearMonth, - @Config("gSuiteOutgoingEmailAddress") String outgoingEmailAddress, - @Config("alertRecipientEmailAddress") String alertRecipientAddress, - @Config("invoiceEmailRecipients") ImmutableList invoiceEmailRecipients, + @Config("gSuiteOutgoingEmailAddress") InternetAddress outgoingEmailAddress, + @Config("alertRecipientEmailAddress") InternetAddress alertRecipientAddress, + @Config("invoiceEmailRecipients") ImmutableList invoiceEmailRecipients, @Config("billingBucket") String billingBucket, @Config("invoiceFilePrefix") String invoiceFilePrefix, @InvoiceDirectoryPrefix String invoiceDirectoryPrefix, - GcsUtils gcsUtils, - Retrier retrier) { + GcsUtils gcsUtils) { this.emailService = emailService; this.yearMonth = yearMonth; this.outgoingEmailAddress = outgoingEmailAddress; @@ -74,50 +66,34 @@ class BillingEmailUtils { this.invoiceFilePrefix = invoiceFilePrefix; this.invoiceDirectoryPrefix = invoiceDirectoryPrefix; this.gcsUtils = gcsUtils; - this.retrier = retrier; } /** Sends an e-mail to all expected recipients with an attached overall invoice from GCS. */ void emailOverallInvoice() { try { - retrier.callWithRetry( - () -> { - String invoiceFile = - String.format("%s-%s.csv", invoiceFilePrefix, yearMonth); - GcsFilename invoiceFilename = - new GcsFilename(billingBucket, invoiceDirectoryPrefix + invoiceFile); - try (InputStream in = gcsUtils.openInputStream(invoiceFilename)) { - Message msg = emailService.createMessage(); - msg.setFrom(new InternetAddress(outgoingEmailAddress)); - for (String recipient : invoiceEmailRecipients) { - msg.addRecipient(RecipientType.TO, new InternetAddress(recipient)); - } - msg.setSubject( - String.format("Domain Registry invoice data %s", yearMonth)); - Multipart multipart = new MimeMultipart(); - BodyPart textPart = new MimeBodyPart(); - textPart.setText( - String.format( - "Attached is the %s invoice for the domain registry.", yearMonth)); - multipart.addBodyPart(textPart); - BodyPart invoicePart = new MimeBodyPart(); - String invoiceData = CharStreams.toString(new InputStreamReader(in, UTF_8)); - invoicePart.setContent(invoiceData, "text/csv; charset=utf-8"); - invoicePart.setFileName(invoiceFile); - multipart.addBodyPart(invoicePart); - msg.setContent(multipart); - msg.saveChanges(); - emailService.sendMessage(msg); - } - }, - IOException.class, - MessagingException.class); + String invoiceFile = String.format("%s-%s.csv", invoiceFilePrefix, yearMonth); + GcsFilename invoiceFilename = + new GcsFilename(billingBucket, invoiceDirectoryPrefix + invoiceFile); + try (InputStream in = gcsUtils.openInputStream(invoiceFilename)) { + emailService.sendEmail( + EmailMessage.newBuilder() + .setSubject(String.format("Domain Registry invoice data %s", yearMonth)) + .setBody( + String.format("Attached is the %s invoice for the domain registry.", yearMonth)) + .setFrom(outgoingEmailAddress) + .setRecipients(invoiceEmailRecipients) + .setAttachment( + Attachment.newBuilder() + .setContent(CharStreams.toString(new InputStreamReader(in, UTF_8))) + .setContentType(MediaType.CSV_UTF_8) + .setFilename(invoiceFile) + .build()) + .build()); + } } catch (Throwable e) { // Strip one layer, because callWithRetry wraps in a RuntimeException sendAlertEmail( - String.format( - "Emailing invoice failed due to %s", - getRootCause(e).getMessage())); + String.format("Emailing invoice failed due to %s", getRootCause(e).getMessage())); throw new RuntimeException("Emailing invoice failed", e); } } @@ -125,17 +101,13 @@ class BillingEmailUtils { /** Sends an e-mail to the provided alert e-mail address indicating a billing failure. */ void sendAlertEmail(String body) { try { - retrier.callWithRetry( - () -> { - Message msg = emailService.createMessage(); - msg.setFrom(new InternetAddress(outgoingEmailAddress)); - msg.addRecipient(RecipientType.TO, new InternetAddress(alertRecipientAddress)); - msg.setSubject(String.format("Billing Pipeline Alert: %s", yearMonth)); - msg.setText(body); - emailService.sendMessage(msg); - return null; - }, - MessagingException.class); + emailService.sendEmail( + EmailMessage.newBuilder() + .setSubject(String.format("Billing Pipeline Alert: %s", yearMonth)) + .setBody(body) + .addRecipient(alertRecipientAddress) + .setFrom(outgoingEmailAddress) + .build()); } catch (Throwable e) { throw new RuntimeException("The alert e-mail system failed.", e); } diff --git a/java/google/registry/reporting/icann/IcannReportingStagingAction.java b/java/google/registry/reporting/icann/IcannReportingStagingAction.java index c5595a05c..1815e16ec 100644 --- a/java/google/registry/reporting/icann/IcannReportingStagingAction.java +++ b/java/google/registry/reporting/icann/IcannReportingStagingAction.java @@ -30,14 +30,18 @@ import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import google.registry.bigquery.BigqueryJobFailureException; +import google.registry.config.RegistryConfig.Config; import google.registry.reporting.icann.IcannReportingModule.ReportType; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.EmailMessage; import google.registry.util.Retrier; +import google.registry.util.SendEmailService; import java.util.Optional; import javax.inject.Inject; +import javax.mail.internet.InternetAddress; import org.joda.time.Duration; import org.joda.time.YearMonth; import org.joda.time.format.DateTimeFormat; @@ -79,7 +83,10 @@ public final class IcannReportingStagingAction implements Runnable { @Inject IcannReportingStager stager; @Inject Retrier retrier; @Inject Response response; - @Inject ReportingEmailUtils emailUtils; + @Inject @Config("gSuiteOutgoingEmailAddress") InternetAddress sender; + @Inject @Config("alertRecipientEmailAddress") InternetAddress recipient; + @Inject SendEmailService emailService; + @Inject IcannReportingStagingAction() {} @Override @@ -96,11 +103,16 @@ public final class IcannReportingStagingAction implements Runnable { stager.createAndUploadManifest(subdir, manifestedFiles); logger.atInfo().log("Completed staging %d report files.", manifestedFiles.size()); - emailUtils.emailResults( - "ICANN Monthly report staging summary [SUCCESS]", - String.format( - "Completed staging the following %d ICANN reports:\n%s", - manifestedFiles.size(), Joiner.on('\n').join(manifestedFiles))); + emailService.sendEmail( + EmailMessage.newBuilder() + .setSubject("ICANN Monthly report staging summary [SUCCESS]") + .setBody( + String.format( + "Completed staging the following %d ICANN reports:\n%s", + manifestedFiles.size(), Joiner.on('\n').join(manifestedFiles))) + .addRecipient(recipient) + .setFrom(sender) + .build()); response.setStatus(SC_OK); response.setContentType(MediaType.PLAIN_TEXT_UTF_8); @@ -117,11 +129,13 @@ public final class IcannReportingStagingAction implements Runnable { }, BigqueryJobFailureException.class); } catch (Throwable e) { - emailUtils.emailResults( - "ICANN Monthly report staging summary [FAILURE]", - String.format( - "Staging failed due to %s, check logs for more details.", - getRootCause(e).toString())); + emailService.sendEmail( + EmailMessage.create( + "ICANN Monthly report staging summary [FAILURE]", + String.format( + "Staging failed due to %s, check logs for more details.", getRootCause(e)), + recipient, + sender)); response.setStatus(SC_INTERNAL_SERVER_ERROR); response.setContentType(MediaType.PLAIN_TEXT_UTF_8); response.setPayload( diff --git a/java/google/registry/reporting/icann/IcannReportingUploadAction.java b/java/google/registry/reporting/icann/IcannReportingUploadAction.java index e85522f79..351299ffe 100644 --- a/java/google/registry/reporting/icann/IcannReportingUploadAction.java +++ b/java/google/registry/reporting/icann/IcannReportingUploadAction.java @@ -34,11 +34,14 @@ import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.EmailMessage; import google.registry.util.Retrier; +import google.registry.util.SendEmailService; import java.io.IOException; import java.io.InputStream; import java.util.stream.Collectors; import javax.inject.Inject; +import javax.mail.internet.InternetAddress; /** * Action that uploads the monthly activity/transactions reports from GCS to ICANN via an HTTP PUT. @@ -74,8 +77,9 @@ public final class IcannReportingUploadAction implements Runnable { @Inject IcannHttpReporter icannReporter; @Inject Retrier retrier; @Inject Response response; - @Inject ReportingEmailUtils emailUtils; - + @Inject @Config("gSuiteOutgoingEmailAddress") InternetAddress sender; + @Inject @Config("alertRecipientEmailAddress") InternetAddress recipient; + @Inject SendEmailService emailService; @Inject IcannReportingUploadAction() {} @@ -112,19 +116,18 @@ public final class IcannReportingUploadAction implements Runnable { } private void emailUploadResults(ImmutableMap reportSummary) { - emailUtils.emailResults( - String.format( - "ICANN Monthly report upload summary: %d/%d succeeded", - reportSummary.values().stream().filter((b) -> b).count(), reportSummary.size()), + String subject = String.format( + "ICANN Monthly report upload summary: %d/%d succeeded", + reportSummary.values().stream().filter((b) -> b).count(), reportSummary.size()); + String body = String.format( "Report Filename - Upload status:\n%s", - reportSummary - .entrySet() - .stream() + reportSummary.entrySet().stream() .map( (e) -> String.format("%s - %s", e.getKey(), e.getValue() ? "SUCCESS" : "FAILURE")) - .collect(Collectors.joining("\n")))); + .collect(Collectors.joining("\n"))); + emailService.sendEmail(EmailMessage.create(subject, body, recipient, sender)); } private ImmutableList getManifestedFiles(String reportBucketname) { diff --git a/java/google/registry/reporting/icann/ReportingEmailUtils.java b/java/google/registry/reporting/icann/ReportingEmailUtils.java deleted file mode 100644 index c011b8bfa..000000000 --- a/java/google/registry/reporting/icann/ReportingEmailUtils.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// 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 google.registry.reporting.icann; - -import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig.Config; -import google.registry.util.SendEmailService; -import javax.inject.Inject; -import javax.mail.Message; -import javax.mail.Message.RecipientType; -import javax.mail.internet.InternetAddress; - -/** Static utils for emailing reporting results. */ -public class ReportingEmailUtils { - - @Inject @Config("gSuiteOutgoingEmailAddress") String sender; - @Inject @Config("alertRecipientEmailAddress") String recipient; - @Inject SendEmailService emailService; - @Inject ReportingEmailUtils() {} - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - void emailResults(String subject, String body) { - try { - Message msg = emailService.createMessage(); - logger.atInfo().log("Emailing %s", recipient); - msg.setFrom(new InternetAddress(sender)); - msg.addRecipient(RecipientType.TO, new InternetAddress(recipient)); - msg.setSubject(subject); - msg.setText(body); - emailService.sendMessage(msg); - } catch (Exception e) { - logger.atWarning().withCause(e).log("E-mail service failed."); - } - } -} diff --git a/java/google/registry/reporting/spec11/Spec11EmailUtils.java b/java/google/registry/reporting/spec11/Spec11EmailUtils.java index 82c91fb12..1521409ef 100644 --- a/java/google/registry/reporting/spec11/Spec11EmailUtils.java +++ b/java/google/registry/reporting/spec11/Spec11EmailUtils.java @@ -20,21 +20,19 @@ import static com.google.common.io.Resources.getResource; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.net.MediaType; import com.google.template.soy.SoyFileSet; import com.google.template.soy.parseinfo.SoyTemplateInfo; import com.google.template.soy.tofu.SoyTofu; import com.google.template.soy.tofu.SoyTofu.Renderer; import google.registry.config.RegistryConfig.Config; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; -import google.registry.util.Retrier; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; -import java.io.IOException; import java.util.List; import java.util.Map; import java.util.Set; import javax.inject.Inject; -import javax.mail.Message; -import javax.mail.Message.RecipientType; import javax.mail.MessagingException; import javax.mail.internet.InternetAddress; import org.joda.time.LocalDate; @@ -52,29 +50,26 @@ public class Spec11EmailUtils { .compileToTofu(); private final SendEmailService emailService; - private final String outgoingEmailAddress; - private final String alertRecipientAddress; - private final String spec11ReplyToAddress; + private final InternetAddress outgoingEmailAddress; + private final InternetAddress alertRecipientAddress; + private final InternetAddress spec11ReplyToAddress; private final ImmutableList spec11WebResources; private final String registryName; - private final Retrier retrier; @Inject Spec11EmailUtils( SendEmailService emailService, - @Config("gSuiteOutgoingEmailAddress") String outgoingEmailAddress, - @Config("alertRecipientEmailAddress") String alertRecipientAddress, - @Config("spec11ReplyToEmailAddress") String spec11ReplyToAddress, + @Config("gSuiteOutgoingEmailAddress") InternetAddress outgoingEmailAddress, + @Config("alertRecipientEmailAddress") InternetAddress alertRecipientAddress, + @Config("spec11ReplyToEmailAddress") InternetAddress spec11ReplyToAddress, @Config("spec11WebResources") ImmutableList spec11WebResources, - @Config("registryName") String registryName, - Retrier retrier) { + @Config("registryName") String registryName) { this.emailService = emailService; this.outgoingEmailAddress = outgoingEmailAddress; this.alertRecipientAddress = alertRecipientAddress; this.spec11ReplyToAddress = spec11ReplyToAddress; this.spec11WebResources = spec11WebResources; this.registryName = registryName; - this.retrier = retrier; } /** @@ -87,14 +82,9 @@ public class Spec11EmailUtils { String subject, Set registrarThreatMatchesSet) { try { - retrier.callWithRetry( - () -> { - for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) { - emailRegistrar(date, soyTemplateInfo, subject, registrarThreatMatches); - } - }, - IOException.class, - MessagingException.class); + for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) { + emailRegistrar(date, soyTemplateInfo, subject, registrarThreatMatches); + } } catch (Throwable e) { // Send an alert with the root cause, unwrapping the retrier's RuntimeException sendAlertEmail( @@ -113,16 +103,15 @@ public class Spec11EmailUtils { String subject, RegistrarThreatMatches registrarThreatMatches) throws MessagingException { - Message msg = emailService.createMessage(); - msg.setSubject(subject); - String content = getContent(date, soyTemplateInfo, registrarThreatMatches); - msg.setContent(content, "text/html"); - msg.setHeader("Content-Type", "text/html"); - msg.setFrom(new InternetAddress(outgoingEmailAddress)); - msg.addRecipient( - RecipientType.TO, new InternetAddress(registrarThreatMatches.registrarEmailAddress())); - msg.addRecipient(RecipientType.BCC, new InternetAddress(spec11ReplyToAddress)); - emailService.sendMessage(msg); + emailService.sendEmail( + EmailMessage.newBuilder() + .setSubject(subject) + .setBody(getContent(date, soyTemplateInfo, registrarThreatMatches)) + .setContentType(MediaType.HTML_UTF_8) + .setFrom(outgoingEmailAddress) + .addRecipient(new InternetAddress(registrarThreatMatches.registrarEmailAddress())) + .setBcc(spec11ReplyToAddress) + .build()); } private String getContent( @@ -144,7 +133,7 @@ public class Spec11EmailUtils { ImmutableMap.of( "date", date.toString(), "registry", registryName, - "replyToEmail", spec11ReplyToAddress, + "replyToEmail", spec11ReplyToAddress.getAddress(), "threats", threatMatchMap, "resources", spec11WebResources); renderer.setData(data); @@ -154,17 +143,13 @@ public class Spec11EmailUtils { /** Sends an e-mail indicating the state of the spec11 pipeline, with a given subject and body. */ void sendAlertEmail(String subject, String body) { try { - retrier.callWithRetry( - () -> { - Message msg = emailService.createMessage(); - msg.setFrom(new InternetAddress(outgoingEmailAddress)); - msg.addRecipient(RecipientType.TO, new InternetAddress(alertRecipientAddress)); - msg.setSubject(subject); - msg.setText(body); - emailService.sendMessage(msg); - return null; - }, - MessagingException.class); + emailService.sendEmail( + EmailMessage.newBuilder() + .setFrom(outgoingEmailAddress) + .addRecipient(alertRecipientAddress) + .setBody(body) + .setSubject(subject) + .build()); } catch (Throwable e) { throw new RuntimeException("The spec11 alert e-mail system failed.", e); } diff --git a/java/google/registry/ui/server/SendEmailUtils.java b/java/google/registry/ui/server/SendEmailUtils.java index 756e4ed7e..98ec745fe 100644 --- a/java/google/registry/ui/server/SendEmailUtils.java +++ b/java/google/registry/ui/server/SendEmailUtils.java @@ -15,18 +15,16 @@ package google.registry.ui.server; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.Iterables.toArray; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; -import java.util.List; import java.util.Objects; import java.util.stream.Stream; import javax.inject.Inject; -import javax.mail.Message; import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; @@ -37,14 +35,14 @@ public class SendEmailUtils { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final String gSuiteOutgoingEmailAddress; + private final InternetAddress gSuiteOutgoingEmailAddress; private final String gSuiteOutgoingEmailDisplayName; private final SendEmailService emailService; private final ImmutableList registrarChangesNotificationEmailAddresses; @Inject public SendEmailUtils( - @Config("gSuiteOutgoingEmailAddress") String gSuiteOutgoingEmailAddress, + @Config("gSuiteOutgoingEmailAddress") InternetAddress gSuiteOutgoingEmailAddress, @Config("gSuiteOutgoingEmailDisplayName") String gSuiteOutgoingEmailDisplayName, @Config("registrarChangesNotificationEmailAddresses") ImmutableList registrarChangesNotificationEmailAddresses, @@ -68,10 +66,10 @@ public class SendEmailUtils { public boolean sendEmail( final String subject, String body, ImmutableList additionalAddresses) { try { - Message msg = emailService.createMessage(); - msg.setFrom( - new InternetAddress(gSuiteOutgoingEmailAddress, gSuiteOutgoingEmailDisplayName)); - List emails = + InternetAddress from = + new InternetAddress( + gSuiteOutgoingEmailAddress.getAddress(), gSuiteOutgoingEmailDisplayName); + ImmutableList recipients = Stream.concat( registrarChangesNotificationEmailAddresses.stream(), additionalAddresses.stream()) .map( @@ -88,20 +86,23 @@ public class SendEmailUtils { }) .filter(Objects::nonNull) .collect(toImmutableList()); - if (emails.isEmpty()) { + if (recipients.isEmpty()) { return false; } - msg.addRecipients(Message.RecipientType.TO, toArray(emails, InternetAddress.class)); - msg.setSubject(subject); - msg.setText(body); - emailService.sendMessage(msg); + emailService.sendEmail( + EmailMessage.newBuilder() + .setBody(body) + .setSubject(subject) + .setRecipients(recipients) + .setFrom(from) + .build()); + return true; } catch (Throwable t) { logger.atSevere().withCause(t).log( "Could not email to addresses %s with subject '%s'.", Joiner.on(", ").join(registrarChangesNotificationEmailAddresses), subject); return false; } - return true; } /** Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses. diff --git a/java/google/registry/util/BUILD b/java/google/registry/util/BUILD index 1ceefbe53..98d19f304 100644 --- a/java/google/registry/util/BUILD +++ b/java/google/registry/util/BUILD @@ -11,6 +11,7 @@ java_library( "//third_party/jaxb", "//third_party/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk", + "@com_google_auto_value", "@com_google_code_findbugs_jsr305", "@com_google_dagger", "@com_google_errorprone_error_prone_annotations", diff --git a/java/google/registry/util/EmailMessage.java b/java/google/registry/util/EmailMessage.java new file mode 100644 index 000000000..e06660c2d --- /dev/null +++ b/java/google/registry/util/EmailMessage.java @@ -0,0 +1,94 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// 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 google.registry.util; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.net.MediaType; +import java.util.Collection; +import java.util.Optional; +import javax.mail.internet.InternetAddress; + +/** Value class representing the content and metadata of an email. */ +@AutoValue +public abstract class EmailMessage { + + public abstract String subject(); + public abstract String body(); + public abstract ImmutableList recipients(); + public abstract InternetAddress from(); + public abstract Optional bcc(); + public abstract Optional contentType(); + public abstract Optional attachment(); + + public static Builder newBuilder() { + return new AutoValue_EmailMessage.Builder(); + } + + public static EmailMessage create( + String subject, String body, InternetAddress recipient, InternetAddress from) { + return newBuilder() + .setSubject(subject) + .setBody(body) + .setRecipients(ImmutableList.of(recipient)) + .setFrom(from) + .build(); + } + + /** Builder for {@link EmailMessage}. */ + @AutoValue.Builder + public abstract static class Builder { + + public abstract Builder setSubject(String subject); + public abstract Builder setBody(String body); + public abstract Builder setRecipients(Collection recipients); + public abstract Builder setFrom(InternetAddress from); + public abstract Builder setBcc(InternetAddress bcc); + public abstract Builder setContentType(MediaType contentType); + public abstract Builder setAttachment(Attachment attachment); + + abstract ImmutableList.Builder recipientsBuilder(); + + public Builder addRecipient(InternetAddress value) { + recipientsBuilder().add(value); + return this; + } + + public abstract EmailMessage build(); + } + + /** An attachment to the email, if one exists. */ + @AutoValue + public abstract static class Attachment { + + public abstract MediaType contentType(); + public abstract String filename(); + public abstract String content(); + + public static Builder newBuilder() { + return new AutoValue_EmailMessage_Attachment.Builder(); + } + + /** Builder for {@link Attachment}. */ + @AutoValue.Builder + public abstract static class Builder { + + public abstract Builder setContentType(MediaType contentType); + public abstract Builder setFilename(String filename); + public abstract Builder setContent(String content); + public abstract Attachment build(); + } + } +} diff --git a/java/google/registry/util/SendEmailService.java b/java/google/registry/util/SendEmailService.java index 86c2a46d8..6654f9cea 100644 --- a/java/google/registry/util/SendEmailService.java +++ b/java/google/registry/util/SendEmailService.java @@ -14,29 +14,78 @@ package google.registry.util; +import static com.google.common.collect.Iterables.toArray; + +import com.google.common.net.MediaType; +import google.registry.util.EmailMessage.Attachment; +import java.io.IOException; import java.util.Properties; import javax.inject.Inject; import javax.inject.Singleton; +import javax.mail.BodyPart; import javax.mail.Message; +import javax.mail.Message.RecipientType; import javax.mail.MessagingException; +import javax.mail.Multipart; import javax.mail.Session; -import javax.mail.Transport; +import javax.mail.internet.InternetAddress; +import javax.mail.internet.MimeBodyPart; import javax.mail.internet.MimeMessage; +import javax.mail.internet.MimeMultipart; -/** Wrapper around javax.mail's Transport.send that can be mocked for testing purposes. */ +/** + * Wrapper around javax.mail's email creation and sending functionality. Encompasses a retry policy + * as well. + */ @Singleton public class SendEmailService { - @Inject - SendEmailService() {}; + private final Retrier retrier; + private final TransportEmailSender transportEmailSender; - /** Returns a new MimeMessage using default App Engine transport settings. */ - public Message createMessage() { - return new MimeMessage(Session.getDefaultInstance(new Properties(), null)); + @Inject + SendEmailService(Retrier retrier, TransportEmailSender transportEmailSender) { + this.retrier = retrier; + this.transportEmailSender = transportEmailSender; } - /** Sends a message using default App Engine transport. */ - public void sendMessage(Message msg) throws MessagingException { - Transport.send(msg); + /** + * Converts the provided message content into a {@link javax.mail.Message} and sends it with + * retry on transient failures. + */ + public void sendEmail(EmailMessage emailMessage) { + retrier.callWithRetry( + () -> { + Message msg = + new MimeMessage( + Session.getDefaultInstance(new Properties(), /* authenticator= */ null)); + msg.setFrom(emailMessage.from()); + msg.addRecipients( + RecipientType.TO, toArray(emailMessage.recipients(), InternetAddress.class)); + msg.setSubject(emailMessage.subject()); + + Multipart multipart = new MimeMultipart(); + BodyPart bodyPart = new MimeBodyPart(); + bodyPart.setContent( + emailMessage.body(), + emailMessage.contentType().orElse(MediaType.PLAIN_TEXT_UTF_8).toString()); + multipart.addBodyPart(bodyPart); + + if (emailMessage.attachment().isPresent()) { + Attachment attachment = emailMessage.attachment().get(); + BodyPart attachmentPart = new MimeBodyPart(); + attachmentPart.setContent(attachment.content(), attachment.contentType().toString()); + attachmentPart.setFileName(attachment.filename()); + multipart.addBodyPart(attachmentPart); + } + if (emailMessage.bcc().isPresent()) { + msg.addRecipient(RecipientType.BCC, emailMessage.bcc().get()); + } + msg.setContent(multipart); + msg.saveChanges(); + transportEmailSender.sendMessage(msg); + }, + IOException.class, + MessagingException.class); } } diff --git a/java/google/registry/util/TransportEmailSender.java b/java/google/registry/util/TransportEmailSender.java new file mode 100644 index 000000000..ed692ee57 --- /dev/null +++ b/java/google/registry/util/TransportEmailSender.java @@ -0,0 +1,34 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// 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 google.registry.util; + +import javax.inject.Inject; +import javax.inject.Singleton; +import javax.mail.Message; +import javax.mail.MessagingException; +import javax.mail.Transport; + +/** Wrapper for sending email so that we can test {@link google.registry.util.SendEmailService}. */ +@Singleton +class TransportEmailSender { + + @Inject + TransportEmailSender() {} + + /** Sends a message using default App Engine transport. */ + void sendMessage(Message msg) throws MessagingException { + Transport.send(msg); + } +} diff --git a/javatests/google/registry/reporting/billing/BillingEmailUtilsTest.java b/javatests/google/registry/reporting/billing/BillingEmailUtilsTest.java index 147a0f9c5..9189c2606 100644 --- a/javatests/google/registry/reporting/billing/BillingEmailUtilsTest.java +++ b/javatests/google/registry/reporting/billing/BillingEmailUtilsTest.java @@ -15,132 +15,94 @@ package google.registry.reporting.billing; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.testing.JUnitBackports.assertThrows; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.appengine.tools.cloudstorage.GcsFilename; import com.google.common.collect.ImmutableList; +import com.google.common.net.MediaType; import google.registry.gcs.GcsUtils; -import google.registry.testing.FakeClock; -import google.registry.testing.FakeSleeper; -import google.registry.util.Retrier; +import google.registry.util.EmailMessage; +import google.registry.util.EmailMessage.Attachment; import google.registry.util.SendEmailService; import java.io.ByteArrayInputStream; -import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Properties; -import javax.mail.BodyPart; -import javax.mail.Message; import javax.mail.MessagingException; -import javax.mail.Multipart; -import javax.mail.Session; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; import org.joda.time.YearMonth; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; /** Unit tests for {@link google.registry.reporting.billing.BillingEmailUtils}. */ @RunWith(JUnit4.class) public class BillingEmailUtilsTest { - private static final int RETRY_COUNT = 2; - private SendEmailService emailService; private BillingEmailUtils emailUtils; private GcsUtils gcsUtils; - private ArgumentCaptor msgCaptor; + private ArgumentCaptor contentCaptor; @Before - public void setUp() { + public void setUp() throws Exception { emailService = mock(SendEmailService.class); - when(emailService.createMessage()) - .thenReturn(new MimeMessage(Session.getDefaultInstance(new Properties(), null))); gcsUtils = mock(GcsUtils.class); when(gcsUtils.openInputStream(new GcsFilename("test-bucket", "results/REG-INV-2017-10.csv"))) .thenReturn( new ByteArrayInputStream("test,data\nhello,world".getBytes(StandardCharsets.UTF_8))); - msgCaptor = ArgumentCaptor.forClass(Message.class); + contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); emailUtils = new BillingEmailUtils( emailService, new YearMonth(2017, 10), - "my-sender@test.com", - "my-receiver@test.com", - ImmutableList.of("hello@world.com", "hola@mundo.com"), + new InternetAddress("my-sender@test.com"), + new InternetAddress("my-receiver@test.com"), + ImmutableList.of( + new InternetAddress("hello@world.com"), new InternetAddress("hola@mundo.com")), "test-bucket", "REG-INV", "results/", - gcsUtils, - new Retrier(new FakeSleeper(new FakeClock()), RETRY_COUNT)); + gcsUtils); } @Test - public void testSuccess_emailOverallInvoice() throws MessagingException, IOException { + public void testSuccess_emailOverallInvoice() throws MessagingException { emailUtils.emailOverallInvoice(); - // We inspect individual parameters because Message doesn't implement equals(). - verify(emailService).sendMessage(msgCaptor.capture()); - Message expectedMsg = msgCaptor.getValue(); - assertThat(expectedMsg.getFrom()) - .asList() - .containsExactly(new InternetAddress("my-sender@test.com")); - assertThat(expectedMsg.getAllRecipients()) - .asList() - .containsExactly( - new InternetAddress("hello@world.com"), new InternetAddress("hola@mundo.com")); - assertThat(expectedMsg.getSubject()).isEqualTo("Domain Registry invoice data 2017-10"); - assertThat(expectedMsg.getContent()).isInstanceOf(Multipart.class); - Multipart contents = (Multipart) expectedMsg.getContent(); - assertThat(contents.getCount()).isEqualTo(2); - assertThat(contents.getBodyPart(0)).isInstanceOf(BodyPart.class); - BodyPart textPart = contents.getBodyPart(0); - assertThat(textPart.getContentType()).isEqualTo("text/plain; charset=us-ascii"); - assertThat(textPart.getContent().toString()) - .isEqualTo("Attached is the 2017-10 invoice for the domain registry."); - assertThat(contents.getBodyPart(1)).isInstanceOf(BodyPart.class); - BodyPart attachmentPart = contents.getBodyPart(1); - assertThat(attachmentPart.getContentType()).endsWith("name=REG-INV-2017-10.csv"); - assertThat(attachmentPart.getContent().toString()).isEqualTo("test,data\nhello,world"); + + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + EmailMessage expectedContent = + EmailMessage.newBuilder() + .setFrom(new InternetAddress("my-sender@test.com")) + .setRecipients( + ImmutableList.of( + new InternetAddress("hello@world.com"), new InternetAddress("hola@mundo.com"))) + .setSubject("Domain Registry invoice data 2017-10") + .setBody("Attached is the 2017-10 invoice for the domain registry.") + .setAttachment( + Attachment.newBuilder() + .setContent("test,data\nhello,world") + .setContentType(MediaType.CSV_UTF_8) + .setFilename("REG-INV-2017-10.csv") + .build()) + .build(); + assertThat(emailMessage).isEqualTo(expectedContent); } @Test - public void testFailure_tooManyRetries_emailsAlert() throws MessagingException, IOException { - // This message throws whenever it tries to set content, to force the overall invoice to fail. - Message throwingMessage = mock(Message.class); - doThrow(new MessagingException("expected")) - .when(throwingMessage) - .setContent(any(Multipart.class)); - when(emailService.createMessage()).thenAnswer( - new Answer() { - private int count = 0; - - @Override - public Message answer(InvocationOnMock invocation) { - // Once we've failed the retry limit for the original invoice, return a normal message - // so we can properly check its contents. - if (count < RETRY_COUNT) { - count++; - return throwingMessage; - } else if (count == RETRY_COUNT) { - return new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - } else { - assertWithMessage("Attempted to generate too many messages!").fail(); - return null; - } - } - } - ); + public void testFailure_emailsAlert() throws MessagingException { + doThrow(new RuntimeException(new MessagingException("expected"))) + .doNothing() + .when(emailService) + .sendEmail(contentCaptor.capture()); RuntimeException thrown = assertThrows(RuntimeException.class, () -> emailUtils.emailOverallInvoice()); assertThat(thrown).hasMessageThat().isEqualTo("Emailing invoice failed"); @@ -149,26 +111,24 @@ public class BillingEmailUtilsTest { .hasMessageThat() .isEqualTo("javax.mail.MessagingException: expected"); // Verify we sent an e-mail alert - verify(emailService).sendMessage(msgCaptor.capture()); - validateAlertMessage(msgCaptor.getValue(), "Emailing invoice failed due to expected"); + verify(emailService, times(2)).sendEmail(contentCaptor.capture()); + validateAlertMessage(contentCaptor.getValue(), "Emailing invoice failed due to expected"); } @Test - public void testSuccess_sendAlertEmail() throws MessagingException, IOException { + public void testSuccess_sendAlertEmail() throws MessagingException { emailUtils.sendAlertEmail("Alert!"); - verify(emailService).sendMessage(msgCaptor.capture()); - validateAlertMessage(msgCaptor.getValue(), "Alert!"); + verify(emailService).sendEmail(contentCaptor.capture()); + validateAlertMessage(contentCaptor.getValue(), "Alert!"); } - private void validateAlertMessage(Message msg, String body) - throws MessagingException, IOException { - assertThat(msg.getFrom()).hasLength(1); - assertThat(msg.getFrom()[0]).isEqualTo(new InternetAddress("my-sender@test.com")); - assertThat(msg.getAllRecipients()) - .asList() + private void validateAlertMessage(EmailMessage emailMessage, String body) + throws MessagingException { + assertThat(emailMessage.from()).isEqualTo(new InternetAddress("my-sender@test.com")); + assertThat(emailMessage.recipients()) .containsExactly(new InternetAddress("my-receiver@test.com")); - assertThat(msg.getSubject()).isEqualTo("Billing Pipeline Alert: 2017-10"); - assertThat(msg.getContentType()).isEqualTo("text/plain"); - assertThat(msg.getContent().toString()).isEqualTo(body); + assertThat(emailMessage.subject()).isEqualTo("Billing Pipeline Alert: 2017-10"); + assertThat(emailMessage.contentType()).isEmpty(); + assertThat(emailMessage.body()).isEqualTo(body); } } diff --git a/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java b/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java index 3dfeafa00..8ba436f75 100644 --- a/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java +++ b/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java @@ -33,8 +33,11 @@ import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.FakeSleeper; import google.registry.testing.TaskQueueHelper.TaskMatcher; +import google.registry.util.EmailMessage; import google.registry.util.Retrier; +import google.registry.util.SendEmailService; import java.util.Optional; +import javax.mail.internet.InternetAddress; import org.joda.time.YearMonth; import org.junit.Before; import org.junit.Rule; @@ -48,7 +51,6 @@ public class IcannReportingStagingActionTest { FakeResponse response = new FakeResponse(); IcannReportingStager stager = mock(IcannReportingStager.class); - ReportingEmailUtils emailUtils = mock(ReportingEmailUtils.class); YearMonth yearMonth = new YearMonth(2017, 6); String subdir = "default/dir"; IcannReportingStagingAction action; @@ -69,7 +71,9 @@ public class IcannReportingStagingActionTest { action.response = response; action.stager = stager; action.retrier = new Retrier(new FakeSleeper(new FakeClock()), 3); - action.emailUtils = emailUtils; + action.sender = new InternetAddress("sender@example.com"); + action.recipient = new InternetAddress("recipient@example.com"); + action.emailService = mock(SendEmailService.class); when(stager.stageReports(yearMonth, subdir, ReportType.ACTIVITY)) .thenReturn(ImmutableList.of("a", "b")); @@ -92,10 +96,13 @@ public class IcannReportingStagingActionTest { action.run(); verify(stager).stageReports(yearMonth, subdir, ReportType.ACTIVITY); verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b")); - verify(emailUtils) - .emailResults( - "ICANN Monthly report staging summary [SUCCESS]", - "Completed staging the following 2 ICANN reports:\na\nb"); + verify(action.emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report staging summary [SUCCESS]", + "Completed staging the following 2 ICANN reports:\na\nb", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); assertUploadTaskEnqueued("default/dir"); } @@ -105,10 +112,13 @@ public class IcannReportingStagingActionTest { verify(stager).stageReports(yearMonth, subdir, ReportType.ACTIVITY); verify(stager).stageReports(yearMonth, subdir, ReportType.TRANSACTIONS); verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b", "c", "d")); - verify(emailUtils) - .emailResults( - "ICANN Monthly report staging summary [SUCCESS]", - "Completed staging the following 4 ICANN reports:\na\nb\nc\nd"); + verify(action.emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report staging summary [SUCCESS]", + "Completed staging the following 4 ICANN reports:\na\nb\nc\nd", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); assertUploadTaskEnqueued("default/dir"); } @@ -121,10 +131,13 @@ public class IcannReportingStagingActionTest { verify(stager, times(2)).stageReports(yearMonth, subdir, ReportType.ACTIVITY); verify(stager, times(2)).stageReports(yearMonth, subdir, ReportType.TRANSACTIONS); verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b", "c", "d")); - verify(emailUtils) - .emailResults( - "ICANN Monthly report staging summary [SUCCESS]", - "Completed staging the following 4 ICANN reports:\na\nb\nc\nd"); + verify(action.emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report staging summary [SUCCESS]", + "Completed staging the following 4 ICANN reports:\na\nb\nc\nd", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); assertUploadTaskEnqueued("default/dir"); } @@ -138,11 +151,14 @@ public class IcannReportingStagingActionTest { assertThat(thrown).hasMessageThat().isEqualTo("Staging action failed."); assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("Expected failure"); verify(stager, times(3)).stageReports(yearMonth, subdir, ReportType.ACTIVITY); - verify(emailUtils) - .emailResults( - "ICANN Monthly report staging summary [FAILURE]", - "Staging failed due to BigqueryJobFailureException: Expected failure," - + " check logs for more details."); + verify(action.emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report staging summary [FAILURE]", + "Staging failed due to BigqueryJobFailureException: Expected failure," + + " check logs for more details.", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); // Assert no upload task enqueued assertNoTasksEnqueued("retryable-cron-tasks"); } diff --git a/javatests/google/registry/reporting/icann/IcannReportingUploadActionTest.java b/javatests/google/registry/reporting/icann/IcannReportingUploadActionTest.java index c089e37ab..f8927075f 100644 --- a/javatests/google/registry/reporting/icann/IcannReportingUploadActionTest.java +++ b/javatests/google/registry/reporting/icann/IcannReportingUploadActionTest.java @@ -32,8 +32,11 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.FakeSleeper; +import google.registry.util.EmailMessage; import google.registry.util.Retrier; +import google.registry.util.SendEmailService; import java.io.IOException; +import javax.mail.internet.InternetAddress; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -51,18 +54,20 @@ public class IcannReportingUploadActionTest { private static final byte[] MANIFEST_PAYLOAD = "test-transactions-201706.csv\na-activity-201706.csv\n".getBytes(UTF_8); private final IcannHttpReporter mockReporter = mock(IcannHttpReporter.class); - private final ReportingEmailUtils emailUtils = mock(ReportingEmailUtils.class); + private final SendEmailService emailService = mock(SendEmailService.class); private final FakeResponse response = new FakeResponse(); private final GcsService gcsService = GcsServiceFactory.createGcsService(); - private IcannReportingUploadAction createAction() { + private IcannReportingUploadAction createAction() throws Exception { IcannReportingUploadAction action = new IcannReportingUploadAction(); action.icannReporter = mockReporter; action.gcsUtils = new GcsUtils(gcsService, 1024); action.retrier = new Retrier(new FakeSleeper(new FakeClock()), 3); action.subdir = "icann/monthly/2017-06"; action.reportingBucket = "basin"; - action.emailUtils = emailUtils; + action.emailService = emailService; + action.sender = new InternetAddress("sender@example.com"); + action.recipient = new InternetAddress("recipient@example.com"); action.response = response; return action; } @@ -94,12 +99,15 @@ public class IcannReportingUploadActionTest { verifyNoMoreInteractions(mockReporter); assertThat(((FakeResponse) action.response).getPayload()) .isEqualTo("OK, attempted uploading 2 reports"); - verify(emailUtils) - .emailResults( - "ICANN Monthly report upload summary: 1/2 succeeded", - "Report Filename - Upload status:\n" - + "test-transactions-201706.csv - SUCCESS\n" - + "a-activity-201706.csv - FAILURE"); + verify(emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report upload summary: 1/2 succeeded", + "Report Filename - Upload status:\n" + + "test-transactions-201706.csv - SUCCESS\n" + + "a-activity-201706.csv - FAILURE", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); } @Test @@ -114,12 +122,15 @@ public class IcannReportingUploadActionTest { verifyNoMoreInteractions(mockReporter); assertThat(((FakeResponse) action.response).getPayload()) .isEqualTo("OK, attempted uploading 2 reports"); - verify(emailUtils) - .emailResults( - "ICANN Monthly report upload summary: 1/2 succeeded", - "Report Filename - Upload status:\n" - + "test-transactions-201706.csv - SUCCESS\n" - + "a-activity-201706.csv - FAILURE"); + verify(emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report upload summary: 1/2 succeeded", + "Report Filename - Upload status:\n" + + "test-transactions-201706.csv - SUCCESS\n" + + "a-activity-201706.csv - FAILURE", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); } @Test @@ -133,15 +144,18 @@ public class IcannReportingUploadActionTest { verifyNoMoreInteractions(mockReporter); assertThat(((FakeResponse) action.response).getPayload()) .isEqualTo("OK, attempted uploading 2 reports"); - verify(emailUtils) - .emailResults( - "ICANN Monthly report upload summary: 0/2 succeeded", - "Report Filename - Upload status:\n" - + "test-transactions-201706.csv - FAILURE\n" - + "a-activity-201706.csv - FAILURE"); + verify(emailService) + .sendEmail( + EmailMessage.create( + "ICANN Monthly report upload summary: 0/2 succeeded", + "Report Filename - Upload status:\n" + + "test-transactions-201706.csv - FAILURE\n" + + "a-activity-201706.csv - FAILURE", + new InternetAddress("recipient@example.com"), + new InternetAddress("sender@example.com"))); } @Test - public void testFail_FileNotFound() { + public void testFail_FileNotFound() throws Exception { IcannReportingUploadAction action = createAction(); action.subdir = "somewhere/else"; IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, action::run); diff --git a/javatests/google/registry/reporting/icann/ReportingEmailUtilsTest.java b/javatests/google/registry/reporting/icann/ReportingEmailUtilsTest.java deleted file mode 100644 index 36c28340a..000000000 --- a/javatests/google/registry/reporting/icann/ReportingEmailUtilsTest.java +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// 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 google.registry.reporting.icann; - -import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import google.registry.util.SendEmailService; -import java.util.Properties; -import javax.mail.Message; -import javax.mail.Message.RecipientType; -import javax.mail.Session; -import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Unit tests for {@link google.registry.reporting.icann.ReportingEmailUtils}. */ -@RunWith(JUnit4.class) -public class ReportingEmailUtilsTest { - private Message msg; - private final SendEmailService emailService = mock(SendEmailService.class); - - @Before - public void setUp() { - msg = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - when(emailService.createMessage()).thenReturn(msg); - } - - private ReportingEmailUtils createEmailUtil() { - ReportingEmailUtils emailUtils = new ReportingEmailUtils(); - emailUtils.sender = "test-project.appspotmail.com"; - emailUtils.recipient = "email@example.com"; - emailUtils.emailService = emailService; - return emailUtils; - } - - @Test - public void testSuccess_sendsEmail() throws Exception { - ReportingEmailUtils emailUtils = createEmailUtil(); - emailUtils.emailResults("Subject", "Body"); - - assertThat(msg.getFrom()).hasLength(1); - assertThat(msg.getFrom()[0]) - .isEqualTo(new InternetAddress("test-project.appspotmail.com")); - assertThat(msg.getRecipients(RecipientType.TO)).hasLength(1); - assertThat(msg.getRecipients(RecipientType.TO)[0]) - .isEqualTo(new InternetAddress("email@example.com")); - assertThat(msg.getSubject()).isEqualTo("Subject"); - assertThat(msg.getContentType()).isEqualTo("text/plain"); - assertThat(msg.getContent().toString()).isEqualTo("Body"); - } -} diff --git a/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java b/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java index 55800fd05..7a513e748 100644 --- a/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java +++ b/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java @@ -15,10 +15,8 @@ package google.registry.reporting.spec11; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches; import static google.registry.testing.JUnitBackports.assertThrows; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -26,63 +24,47 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import com.google.common.net.MediaType; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; -import google.registry.testing.FakeClock; -import google.registry.testing.FakeSleeper; -import google.registry.util.Retrier; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; -import java.io.IOException; import java.util.List; -import java.util.Properties; -import javax.annotation.Nullable; -import javax.mail.Message; -import javax.mail.Message.RecipientType; +import java.util.Optional; import javax.mail.MessagingException; -import javax.mail.Session; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; import org.joda.time.LocalDate; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; /** Unit tests for {@link Spec11EmailUtils}. */ @RunWith(JUnit4.class) public class Spec11EmailUtilsTest { - private static final int RETRY_COUNT = 2; private static final ImmutableList FAKE_RESOURCES = ImmutableList.of("foo"); private SendEmailService emailService; private Spec11EmailUtils emailUtils; private Spec11RegistrarThreatMatchesParser parser; - private ArgumentCaptor gotMessage; + private ArgumentCaptor contentCaptor; private final LocalDate date = new LocalDate(2018, 7, 15); @Before public void setUp() throws Exception { emailService = mock(SendEmailService.class); - when(emailService.createMessage()) - .thenAnswer((args) -> new MimeMessage(Session.getInstance(new Properties(), null))); - parser = mock(Spec11RegistrarThreatMatchesParser.class); when(parser.getRegistrarThreatMatches(date)).thenReturn(sampleThreatMatches()); - - gotMessage = ArgumentCaptor.forClass(Message.class); - + contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); emailUtils = new Spec11EmailUtils( emailService, - "my-sender@test.com", - "my-receiver@test.com", - "my-reply-to@test.com", + new InternetAddress("my-sender@test.com"), + new InternetAddress("my-receiver@test.com"), + new InternetAddress("my-reply-to@test.com"), FAKE_RESOURCES, - "Super Cool Registry", - new Retrier(new FakeSleeper(new FakeClock()), RETRY_COUNT)); + "Super Cool Registry"); } @Test @@ -93,8 +75,8 @@ public class Spec11EmailUtilsTest { "Super Cool Registry Monthly Threat Detector [2018-07-15]", sampleThreatMatches()); // We inspect individual parameters because Message doesn't implement equals(). - verify(emailService, times(3)).sendMessage(gotMessage.capture()); - List capturedMessages = gotMessage.getAllValues(); + verify(emailService, times(3)).sendEmail(contentCaptor.capture()); + List capturedContents = contentCaptor.getAllValues(); String emailFormat = "Dear registrar partner,

Super Cool Registry previously notified you when the following " + "domains managed by your registrar were flagged for potential security concerns." @@ -113,31 +95,31 @@ public class Spec11EmailUtilsTest { + "uestions regarding this notice, please contact my-reply-to@test.com.

"; validateMessage( - capturedMessages.get(0), + capturedContents.get(0), "my-sender@test.com", "a@fake.com", - "my-reply-to@test.com", + Optional.of("my-reply-to@test.com"), "Super Cool Registry Monthly Threat Detector [2018-07-15]", String.format(emailFormat, "a.comMALWARE"), - "text/html"); + Optional.of(MediaType.HTML_UTF_8)); validateMessage( - capturedMessages.get(1), + capturedContents.get(1), "my-sender@test.com", "b@fake.com", - "my-reply-to@test.com", + Optional.of("my-reply-to@test.com"), "Super Cool Registry Monthly Threat Detector [2018-07-15]", String.format( emailFormat, "b.comMALWAREc.comMALWARE"), - "text/html"); + Optional.of(MediaType.HTML_UTF_8)); validateMessage( - capturedMessages.get(2), + capturedContents.get(2), "my-sender@test.com", "my-receiver@test.com", - null, + Optional.empty(), "Spec11 Pipeline Success 2018-07-15", "Spec11 reporting completed successfully.", - "text/plain"); + Optional.empty()); } @Test @@ -148,8 +130,8 @@ public class Spec11EmailUtilsTest { "Super Cool Registry Daily Threat Detector [2018-07-15]", sampleThreatMatches()); // We inspect individual parameters because Message doesn't implement equals(). - verify(emailService, times(3)).sendMessage(gotMessage.capture()); - List capturedMessages = gotMessage.getAllValues(); + verify(emailService, times(3)).sendEmail(contentCaptor.capture()); + List capturedMessages = contentCaptor.getAllValues(); String emailFormat = "Dear registrar partner,

" + "Super Cool Registry conducts a daily analysis of all domains registered in its TLDs " @@ -172,54 +154,36 @@ public class Spec11EmailUtilsTest { capturedMessages.get(0), "my-sender@test.com", "a@fake.com", - "my-reply-to@test.com", + Optional.of("my-reply-to@test.com"), "Super Cool Registry Daily Threat Detector [2018-07-15]", String.format(emailFormat, "a.comMALWARE"), - "text/html"); + Optional.of(MediaType.HTML_UTF_8)); validateMessage( capturedMessages.get(1), "my-sender@test.com", "b@fake.com", - "my-reply-to@test.com", + Optional.of("my-reply-to@test.com"), "Super Cool Registry Daily Threat Detector [2018-07-15]", String.format( emailFormat, "b.comMALWAREc.comMALWARE"), - "text/html"); + Optional.of(MediaType.HTML_UTF_8)); validateMessage( capturedMessages.get(2), "my-sender@test.com", "my-receiver@test.com", - null, + Optional.empty(), "Spec11 Pipeline Success 2018-07-15", "Spec11 reporting completed successfully.", - "text/plain"); + Optional.empty()); } @Test - public void testFailure_tooManyRetries_emailsAlert() throws MessagingException, IOException { - Message throwingMessage = mock(Message.class); - doThrow(new MessagingException("expected")).when(throwingMessage).setSubject(any(String.class)); - // Only return the throwingMessage enough times to force failure. The last invocation will - // be for the alert e-mail we're looking to verify. - when(emailService.createMessage()) - .thenAnswer( - new Answer() { - private int count = 0; - - @Override - public Message answer(InvocationOnMock invocation) { - if (count < RETRY_COUNT) { - count++; - return throwingMessage; - } else if (count == RETRY_COUNT) { - return new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - } else { - assertWithMessage("Attempted to generate too many messages!").fail(); - return null; - } - } - }); + public void testFailure_emailsAlert() throws MessagingException { + doThrow(new RuntimeException(new MessagingException("expected"))) + .doNothing() + .when(emailService) + .sendEmail(contentCaptor.capture()); RuntimeException thrown = assertThrows( RuntimeException.class, @@ -231,57 +195,52 @@ public class Spec11EmailUtilsTest { .hasCauseThat() .hasMessageThat() .isEqualTo("javax.mail.MessagingException: expected"); - // We should have created RETRY_COUNT failing messages and one final alert message - verify(emailService, times(RETRY_COUNT + 1)).createMessage(); // Verify we sent an e-mail alert - verify(emailService).sendMessage(gotMessage.capture()); + verify(emailService, times(2)).sendEmail(contentCaptor.capture()); validateMessage( - gotMessage.getValue(), + contentCaptor.getValue(), "my-sender@test.com", "my-receiver@test.com", - null, + Optional.empty(), "Spec11 Emailing Failure 2018-07-15", "Emailing spec11 reports failed due to expected", - "text/plain"); + Optional.empty()); } @Test - public void testSuccess_sendAlertEmail() throws MessagingException, IOException { + public void testSuccess_sendAlertEmail() throws MessagingException { emailUtils.sendAlertEmail("Spec11 Pipeline Alert: 2018-07", "Alert!"); - verify(emailService).sendMessage(gotMessage.capture()); + verify(emailService).sendEmail(contentCaptor.capture()); validateMessage( - gotMessage.getValue(), + contentCaptor.getValue(), "my-sender@test.com", "my-receiver@test.com", - null, + Optional.empty(), "Spec11 Pipeline Alert: 2018-07", "Alert!", - "text/plain"); + Optional.empty()); } private void validateMessage( - Message message, + EmailMessage message, String from, String recipient, - @Nullable String replyTo, + Optional replyTo, String subject, String body, - String contentType) - throws MessagingException, IOException { - assertThat(message.getFrom()).asList().containsExactly(new InternetAddress(from)); - assertThat(message.getRecipients(RecipientType.TO)) - .asList() - .containsExactly(new InternetAddress(recipient)); - if (replyTo == null) { - assertThat(message.getRecipients(RecipientType.BCC)).isNull(); - } else { - assertThat(message.getRecipients(RecipientType.BCC)) - .asList() - .containsExactly(new InternetAddress(replyTo)); + Optional contentType) + throws MessagingException { + EmailMessage.Builder expectedContentBuilder = + EmailMessage.newBuilder() + .setFrom(new InternetAddress(from)) + .addRecipient(new InternetAddress(recipient)) + .setSubject(subject) + .setBody(body); + + if (replyTo.isPresent()) { + expectedContentBuilder.setBcc(new InternetAddress(replyTo.get())); } - assertThat(message.getRecipients(RecipientType.CC)).isNull(); - assertThat(message.getSubject()).isEqualTo(subject); - assertThat(message.getContentType()).isEqualTo(contentType); - assertThat(message.getContent()).isEqualTo(body); + contentType.ifPresent(expectedContentBuilder::setContentType); + assertThat(message).isEqualTo(expectedContentBuilder.build()); } } diff --git a/javatests/google/registry/ui/server/SendEmailUtilsTest.java b/javatests/google/registry/ui/server/SendEmailUtilsTest.java index c09896a29..7593caaf6 100644 --- a/javatests/google/registry/ui/server/SendEmailUtilsTest.java +++ b/javatests/google/registry/ui/server/SendEmailUtilsTest.java @@ -20,41 +20,28 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; -import java.util.Properties; -import javax.mail.Message; -import javax.mail.Message.RecipientType; import javax.mail.MessagingException; -import javax.mail.Session; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; /** Unit tests for {@link SendEmailUtils}. */ @RunWith(JUnit4.class) public class SendEmailUtilsTest { private final SendEmailService emailService = mock(SendEmailService.class); - - private Message message; private SendEmailUtils sendEmailUtils; - @Before - public void init() { - message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - when(emailService.createMessage()).thenReturn(message); - } - - private void setRecipients(ImmutableList recipients) { + private void setRecipients(ImmutableList recipients) throws Exception { sendEmailUtils = new SendEmailUtils( - "outgoing@registry.example", + new InternetAddress("outgoing@registry.example"), "outgoing display name", recipients, emailService); @@ -69,11 +56,7 @@ public class SendEmailUtilsTest { "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); - verifyMessageSent(); - assertThat(message.getRecipients(RecipientType.TO)).asList() - .containsExactly(new InternetAddress("johnny@fakesite.tld")); - assertThat(message.getAllRecipients()).asList() - .containsExactly(new InternetAddress("johnny@fakesite.tld")); + verifyMessageSent("johnny@fakesite.tld"); } @Test @@ -85,10 +68,7 @@ public class SendEmailUtilsTest { "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); - verifyMessageSent(); - assertThat(message.getAllRecipients()).asList().containsExactly( - new InternetAddress("foo@example.com"), - new InternetAddress("bar@example.com")); + verifyMessageSent("foo@example.com", "bar@example.com"); } @Test @@ -100,9 +80,7 @@ public class SendEmailUtilsTest { "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); - verifyMessageSent(); - assertThat(message.getAllRecipients()).asList() - .containsExactly(new InternetAddress("foo@example.com")); + verifyMessageSent("foo@example.com"); } @Test @@ -114,7 +92,7 @@ public class SendEmailUtilsTest { "Welcome to the Internet", "It is a dark and scary place.")) .isFalse(); - verify(emailService, never()).sendMessage(any(Message.class)); + verify(emailService, never()).sendEmail(any()); } @Test @@ -126,23 +104,22 @@ public class SendEmailUtilsTest { "Welcome to the Internet", "It is a dark and scary place.")) .isFalse(); - verify(emailService, never()).sendMessage(any(Message.class)); + verify(emailService, never()).sendEmail(any()); } @Test public void testFailure_exceptionThrownDuringSend() throws Exception { setRecipients(ImmutableList.of("foo@example.com")); assertThat(sendEmailUtils.hasRecipients()).isTrue(); - doThrow(new MessagingException()).when(emailService).sendMessage(any(Message.class)); + doThrow(new RuntimeException(new MessagingException("expected"))) + .when(emailService) + .sendEmail(any()); assertThat( sendEmailUtils.sendEmail( "Welcome to the Internet", "It is a dark and scary place.")) .isFalse(); - verifyMessageSent(); - assertThat(message.getAllRecipients()) - .asList() - .containsExactly(new InternetAddress("foo@example.com")); + verifyMessageSent("foo@example.com"); } @Test @@ -155,13 +132,7 @@ public class SendEmailUtilsTest { "It is a dark and scary place.", ImmutableList.of("bar@example.com", "baz@example.com"))) .isTrue(); - verifyMessageSent(); - assertThat(message.getAllRecipients()) - .asList() - .containsExactly( - new InternetAddress("foo@example.com"), - new InternetAddress("bar@example.com"), - new InternetAddress("baz@example.com")); + verifyMessageSent("foo@example.com", "bar@example.com", "baz@example.com"); } @Test @@ -174,16 +145,24 @@ public class SendEmailUtilsTest { "It is a dark and scary place.", ImmutableList.of("bar@example.com", "baz@example.com"))) .isTrue(); - verifyMessageSent(); - assertThat(message.getAllRecipients()) - .asList() - .containsExactly( - new InternetAddress("bar@example.com"), new InternetAddress("baz@example.com")); + verifyMessageSent("bar@example.com", "baz@example.com"); } - private void verifyMessageSent() throws Exception { - verify(emailService).sendMessage(message); - assertThat(message.getSubject()).isEqualTo("Welcome to the Internet"); - assertThat(message.getContent()).isEqualTo("It is a dark and scary place."); + private void verifyMessageSent(String... expectedRecipients) throws Exception { + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + ImmutableList.Builder recipientBuilder = ImmutableList.builder(); + for (String expectedRecipient : expectedRecipients) { + recipientBuilder.add(new InternetAddress(expectedRecipient)); + } + EmailMessage expectedContent = + EmailMessage.newBuilder() + .setSubject("Welcome to the Internet") + .setBody("It is a dark and scary place.") + .setFrom(new InternetAddress("outgoing@registry.example")) + .setRecipients(recipientBuilder.build()) + .build(); + assertThat(emailMessage).isEqualTo(expectedContent); } } diff --git a/javatests/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java index d598566c8..939d1bca8 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java @@ -21,6 +21,7 @@ import static google.registry.model.registrar.Registrar.loadByClientId; import static google.registry.testing.DatastoreHelper.persistPremiumList; import static google.registry.testing.JUnitBackports.assertThrows; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; @@ -40,18 +41,17 @@ import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.ui.server.SendEmailUtils; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.util.Optional; -import java.util.Properties; -import javax.mail.Message; -import javax.mail.Session; -import javax.mail.internet.MimeMessage; +import javax.mail.internet.InternetAddress; import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -72,10 +72,9 @@ public final class ConsoleOteSetupActionTest { @Mock HttpServletRequest request; @Mock SendEmailService emailService; - Message message; @Before - public void setUp() { + public void setUp() throws Exception { persistPremiumList("default_sandbox_list", "sandbox,USD 1000"); action.req = request; @@ -90,7 +89,7 @@ public final class ConsoleOteSetupActionTest { action.registryEnvironment = RegistryEnvironment.UNITTEST; action.sendEmailUtils = new SendEmailUtils( - "outgoing@registry.example", + new InternetAddress("outgoing@registry.example"), "UnitTest Registry", ImmutableList.of("notification@test.example", "notification2@test.example"), emailService); @@ -101,9 +100,6 @@ public final class ConsoleOteSetupActionTest { action.optionalPassword = Optional.empty(); action.passwordGenerator = new DeterministicStringGenerator("abcdefghijklmnopqrstuvwxyz"); - - message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - when(emailService.createMessage()).thenReturn(message); } @Test @@ -136,7 +132,7 @@ public final class ConsoleOteSetupActionTest { } @Test - public void testPost_authorized() throws Exception { + public void testPost_authorized() { action.clientId = Optional.of("myclientid"); action.email = Optional.of("contact@registry.example"); action.method = Method.POST; @@ -150,8 +146,12 @@ public final class ConsoleOteSetupActionTest { .isEqualTo("contact@registry.example"); assertThat(response.getPayload()) .contains("

OT&E successfully created for registrar myclientid!

"); - assertThat(message.getSubject()).isEqualTo("OT&E for registrar myclientid created in unittest"); - assertThat(message.getContent()) + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + assertThat(emailMessage.subject()) + .isEqualTo("OT&E for registrar myclientid created in unittest"); + assertThat(emailMessage.body()) .isEqualTo( "" + "The following entities were created in unittest by TestUserId:\n" @@ -163,7 +163,7 @@ public final class ConsoleOteSetupActionTest { } @Test - public void testPost_authorized_setPassword() throws Exception { + public void testPost_authorized_setPassword() { action.clientId = Optional.of("myclientid"); action.email = Optional.of("contact@registry.example"); action.optionalPassword = Optional.of("SomePassword"); diff --git a/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java index 672be1e5f..a08278d23 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java @@ -20,6 +20,7 @@ import static google.registry.model.common.GaeUserIdConverter.convertEmailAddres import static google.registry.model.registrar.Registrar.loadByClientId; import static google.registry.testing.DatastoreHelper.persistPremiumList; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; @@ -41,12 +42,10 @@ import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.ui.server.SendEmailUtils; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.util.Optional; -import java.util.Properties; -import javax.mail.Message; -import javax.mail.Session; -import javax.mail.internet.MimeMessage; +import javax.mail.internet.InternetAddress; import javax.servlet.http.HttpServletRequest; import org.joda.money.CurrencyUnit; import org.junit.Before; @@ -54,6 +53,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -74,10 +74,9 @@ public final class ConsoleRegistrarCreatorActionTest { @Mock HttpServletRequest request; @Mock SendEmailService emailService; - Message message; @Before - public void setUp() { + public void setUp() throws Exception { persistPremiumList("default_sandbox_list", "sandbox,USD 1000"); action.req = request; @@ -92,7 +91,7 @@ public final class ConsoleRegistrarCreatorActionTest { action.registryEnvironment = RegistryEnvironment.UNITTEST; action.sendEmailUtils = new SendEmailUtils( - "outgoing@registry.example", + new InternetAddress("outgoing@registry.example"), "UnitTest Registry", ImmutableList.of("notification@test.example", "notification2@test.example"), emailService); @@ -120,9 +119,6 @@ public final class ConsoleRegistrarCreatorActionTest { action.passwordGenerator = new DeterministicStringGenerator("abcdefghijklmnopqrstuvwxyz"); action.passcodeGenerator = new DeterministicStringGenerator("314159265"); - - message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - when(emailService.createMessage()).thenReturn(message); } @Test @@ -156,7 +152,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_minimalAddress() throws Exception { + public void testPost_authorized_minimalAddress() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.billingAccount = Optional.of("USD=billing-account"); @@ -176,8 +172,11 @@ public final class ConsoleRegistrarCreatorActionTest { assertThat(response.getPayload()) .contains("

Successfully created Registrar myclientid

"); - assertThat(message.getSubject()).isEqualTo("Registrar myclientid created in unittest"); - assertThat(message.getContent()) + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + assertThat(emailMessage.subject()).isEqualTo("Registrar myclientid created in unittest"); + assertThat(emailMessage.body()) .isEqualTo( "" + "The following registrar was created in unittest by TestUserId:\n" @@ -221,7 +220,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_allAddress() throws Exception { + public void testPost_authorized_allAddress() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.billingAccount = Optional.of("USD=billing-account"); @@ -257,7 +256,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_multipleBillingLines() throws Exception { + public void testPost_authorized_multipleBillingLines() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.ianaId = Optional.of(12321); @@ -294,7 +293,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_repeatingCurrency_fails() throws Exception { + public void testPost_authorized_repeatingCurrency_fails() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.ianaId = Optional.of(12321); @@ -322,7 +321,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_badCurrency_fails() throws Exception { + public void testPost_authorized_badCurrency_fails() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.ianaId = Optional.of(12321); @@ -349,7 +348,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_badBillingLine_fails() throws Exception { + public void testPost_authorized_badBillingLine_fails() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.ianaId = Optional.of(12321); @@ -378,7 +377,7 @@ public final class ConsoleRegistrarCreatorActionTest { } @Test - public void testPost_authorized_setPassword() throws Exception { + public void testPost_authorized_setPassword() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.billingAccount = Optional.of("USD=billing-account"); diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 90a9cb8ba..dc1ae7ea3 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -37,6 +37,7 @@ import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role; import google.registry.testing.CertificateSamples; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.CidrAddressBlock; +import google.registry.util.EmailMessage; import java.util.Map; import java.util.function.BiFunction; import java.util.function.Function; @@ -45,6 +46,7 @@ import org.json.simple.parser.ParseException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; /** Tests for {@link RegistrarSettingsAction}. */ @RunWith(JUnit4.class) @@ -56,7 +58,9 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase action.handleJsonRequest(readJsonFromFile("update_registrar.json", getLastUpdateTime())); verify(rsp, never()).setStatus(anyInt()); verifyContactsNotified(); - assertThat(message.getContent()).isEqualTo(expectedEmailBody); + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + assertThat(contentCaptor.getValue().body()).isEqualTo(expectedEmailBody); assertTasksEnqueued("sheet", new TaskMatcher() .url(SyncRegistrarsSheetAction.PATH) .method("GET") diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index d8bc8aace..ed1d75fbc 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -44,14 +44,11 @@ import google.registry.testing.FakeClock; import google.registry.testing.InjectRule; import google.registry.ui.server.SendEmailUtils; import google.registry.util.AppEngineServiceUtils; +import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Properties; -import javax.mail.Message; -import javax.mail.Session; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.joda.time.DateTime; @@ -60,6 +57,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -81,9 +79,6 @@ public class RegistrarSettingsActionTestCase { @Mock HttpServletRequest req; @Mock HttpServletResponse rsp; @Mock SendEmailService emailService; - final User user = new User("user", "gmail.com"); - - Message message; final RegistrarSettingsAction action = new RegistrarSettingsAction(); final StringWriter writer = new StringWriter(); @@ -115,8 +110,6 @@ public class RegistrarSettingsActionTestCase { AuthLevel.USER, UserAuthInfo.create(new User("user@email.com", "email.com", "12345"), false)); inject.setStaticField(Ofy.class, "clock", clock); - message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); - when(emailService.createMessage()).thenReturn(message); when(req.getMethod()).thenReturn("POST"); when(rsp.getWriter()).thenReturn(new PrintWriter(writer)); when(req.getContentType()).thenReturn("application/json"); @@ -128,7 +121,7 @@ public class RegistrarSettingsActionTestCase { } @After - public void tearDown() throws Exception { + public void tearDown() { assertThat(RegistrarConsoleMetrics.settingsRequestMetric).hasNoOtherValues(); } @@ -162,10 +155,9 @@ public class RegistrarSettingsActionTestCase { /** Verifies that the original contact of TheRegistrar is among those notified of a change. */ protected void verifyContactsNotified() throws Exception { - verify(emailService).createMessage(); - verify(emailService).sendMessage(message); - Truth.assertThat(message.getAllRecipients()) - .asList() + ArgumentCaptor captor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(captor.capture()); + Truth.assertThat(captor.getValue().recipients()) .containsExactly( new InternetAddress("notification@test.example"), new InternetAddress("notification2@test.example"), diff --git a/javatests/google/registry/util/SendEmailServiceTest.java b/javatests/google/registry/util/SendEmailServiceTest.java new file mode 100644 index 000000000..342f15dca --- /dev/null +++ b/javatests/google/registry/util/SendEmailServiceTest.java @@ -0,0 +1,162 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// 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 google.registry.util; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.JUnitBackports.assertThrows; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import com.google.common.net.MediaType; +import google.registry.testing.FakeClock; +import google.registry.testing.FakeSleeper; +import google.registry.util.EmailMessage.Attachment; +import javax.mail.BodyPart; +import javax.mail.Message; +import javax.mail.Message.RecipientType; +import javax.mail.MessagingException; +import javax.mail.internet.InternetAddress; +import javax.mail.internet.MimeMultipart; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +/** Unit tests for {@link SendEmailService}. */ +@RunWith(JUnit4.class) +public class SendEmailServiceTest { + + @Rule + public final MockitoRule mocks = MockitoJUnit.rule(); + + private final Retrier retrier = new Retrier(new FakeSleeper(new FakeClock()), 2); + private final TransportEmailSender wrapper = mock(TransportEmailSender.class); + private final SendEmailService sendEmailService = new SendEmailService(retrier, wrapper); + + @Captor private ArgumentCaptor messageCaptor; + + @Test + public void testSuccess_simple() throws Exception { + EmailMessage content = createBuilder().build(); + sendEmailService.sendEmail(content); + Message message = getMessage(); + assertThat(message.getAllRecipients()) + .asList() + .containsExactly(new InternetAddress("fake@example.com")); + assertThat(message.getFrom()) + .asList() + .containsExactly(new InternetAddress("registry@example.com")); + assertThat(message.getRecipients(RecipientType.BCC)).isNull(); + assertThat(message.getSubject()).isEqualTo("Subject"); + assertThat(message.getContentType()).startsWith("multipart/mixed"); + assertThat(getInternalContent(message).getContent().toString()).isEqualTo("body"); + assertThat(getInternalContent(message).getContentType()).isEqualTo("text/plain; charset=utf-8"); + assertThat(((MimeMultipart) message.getContent()).getCount()).isEqualTo(1); + } + + @Test + public void testSuccess_bcc() throws Exception { + EmailMessage content = createBuilder().setBcc(new InternetAddress("bcc@example.com")).build(); + sendEmailService.sendEmail(content); + Message message = getMessage(); + assertThat(message.getRecipients(RecipientType.BCC)) + .asList() + .containsExactly(new InternetAddress("bcc@example.com")); + } + + @Test + public void testSuccess_contentType() throws Exception { + EmailMessage content = createBuilder().setContentType(MediaType.HTML_UTF_8).build(); + sendEmailService.sendEmail(content); + Message message = getMessage(); + assertThat(getInternalContent(message).getContentType()) + .isEqualTo("text/html; charset=utf-8"); + } + + @Test + public void testSuccess_attachment() throws Exception { + EmailMessage content = + createBuilder() + .setAttachment( + Attachment.newBuilder() + .setFilename("filename") + .setContent("foo,bar\nbaz,qux") + .setContentType(MediaType.CSV_UTF_8) + .build()) + .build(); + sendEmailService.sendEmail(content); + Message message = getMessage(); + assertThat(((MimeMultipart) message.getContent()).getCount()).isEqualTo(2); + BodyPart attachment = ((MimeMultipart) message.getContent()).getBodyPart(1); + assertThat(attachment.getContent()).isEqualTo("foo,bar\nbaz,qux"); + assertThat(attachment.getContentType()).endsWith("name=filename"); + } + + @Test + public void testSuccess_retry() throws Exception { + doThrow(new MessagingException("hi")) + .doNothing() + .when(wrapper) + .sendMessage(messageCaptor.capture()); + EmailMessage content = createBuilder().build(); + sendEmailService.sendEmail(content); + assertThat(messageCaptor.getValue().getSubject()).isEqualTo("Subject"); + } + + @Test + public void testFailure_wrongExceptionType() throws Exception { + doThrow(new RuntimeException("this is a runtime exception")).when(wrapper).sendMessage(any()); + EmailMessage content = createBuilder().build(); + RuntimeException thrown = + assertThrows(RuntimeException.class, () -> sendEmailService.sendEmail(content)); + assertThat(thrown).hasMessageThat().isEqualTo("this is a runtime exception"); + } + + @Test + public void testFailure_tooManyTries() throws Exception { + doThrow(new MessagingException("hi")) + .doThrow(new MessagingException("second")) + .when(wrapper) + .sendMessage(any()); + EmailMessage content = createBuilder().build(); + RuntimeException thrown = + assertThrows(RuntimeException.class, () -> sendEmailService.sendEmail(content)); + assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("second"); + assertThat(thrown).hasCauseThat().isInstanceOf(MessagingException.class); + } + + private EmailMessage.Builder createBuilder() throws Exception { + return EmailMessage.newBuilder() + .setFrom(new InternetAddress("registry@example.com")) + .addRecipient(new InternetAddress("fake@example.com")) + .setSubject("Subject") + .setBody("body"); + } + + private Message getMessage() throws MessagingException { + verify(wrapper).sendMessage(messageCaptor.capture()); + return messageCaptor.getValue(); + } + + private BodyPart getInternalContent(Message message) throws Exception { + return ((MimeMultipart) message.getContent()).getBodyPart(0); + } +}