Refactor common email sending utility

The main thrust of this is to create a common POJO that contains email content in a simple way, then have one class that converts that to an email and sends it. Any class that uses email should only have to deal with creating that POJO.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=237883643

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<String> recipients) {
+  private void setRecipients(ImmutableList<String> 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<EmailMessage> contentCaptor = ArgumentCaptor.forClass(EmailMessage.class);
+    verify(emailService).sendEmail(contentCaptor.capture());
+    EmailMessage emailMessage = contentCaptor.getValue();
+    ImmutableList.Builder<InternetAddress> 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);
   }
 }

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("<h1>OT&E successfully created for registrar myclientid!</h1>");
-    assertThat(message.getSubject()).isEqualTo("OT&E for registrar myclientid created in unittest");
-    assertThat(message.getContent())
+    ArgumentCaptor<EmailMessage> 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");

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("<h1>Successfully created Registrar myclientid</h1>");
 
-    assertThat(message.getSubject()).isEqualTo("Registrar myclientid created in unittest");
-    assertThat(message.getContent())
+    ArgumentCaptor<EmailMessage> 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");

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<EmailMessage> 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")

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<EmailMessage> 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"),