Add cc support to email service (#1230)

* Add cc support to email service

util/src/main/java/google/registry/util/EmailMessage.java

@@ -26,16 +26,6 @@ import javax.mail.internet.InternetAddress;
 @AutoValue
 public abstract class EmailMessage {
 
-  public abstract String subject();
-  public abstract String body();
-  public abstract ImmutableSet<InternetAddress> recipients();
-  public abstract InternetAddress from();
-
-  public abstract ImmutableSet<InternetAddress> bccs();
-
-  public abstract Optional<MediaType> contentType();
-  public abstract Optional<Attachment> attachment();
-
   public static Builder newBuilder() {
     return new AutoValue_EmailMessage.Builder();
   }
@@ -50,24 +40,48 @@ public abstract class EmailMessage {
         .build();
   }
 
+  public abstract String subject();
+
+  public abstract String body();
+
+  public abstract ImmutableSet<InternetAddress> recipients();
+
+  public abstract InternetAddress from();
+
+  public abstract ImmutableSet<InternetAddress> ccs();
+
+  public abstract ImmutableSet<InternetAddress> bccs();
+
+  public abstract Optional<MediaType> contentType();
+
+  public abstract Optional<Attachment> attachment();
+
   /** 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<InternetAddress> recipients);
+
     public abstract Builder setFrom(InternetAddress from);
 
     public abstract Builder setBccs(Collection<InternetAddress> bccs);
 
+    public abstract Builder setCcs(Collection<InternetAddress> ccs);
+
     public abstract Builder setContentType(MediaType contentType);
+
     public abstract Builder setAttachment(Attachment attachment);
 
     abstract ImmutableSet.Builder<InternetAddress> recipientsBuilder();
 
     abstract ImmutableSet.Builder<InternetAddress> bccsBuilder();
 
+    abstract ImmutableSet.Builder<InternetAddress> ccsBuilder();
+
     public Builder addRecipient(InternetAddress value) {
       recipientsBuilder().add(value);
       return this;
@@ -78,28 +92,36 @@ public abstract class EmailMessage {
       return this;
     }
 
+    public Builder addCc(InternetAddress cc) {
+      ccsBuilder().add(cc);
+      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();
     }
 
+    public abstract MediaType contentType();
+
+    public abstract String filename();
+
+    public abstract String content();
+
     /** 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();
     }
   }

util/src/main/java/google/registry/util/SendEmailService.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.util.Properties;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javax.mail.Address;
 import javax.mail.BodyPart;
 import javax.mail.Message;
 import javax.mail.Message.RecipientType;
@@ -50,8 +51,8 @@ public class SendEmailService {
   }
 
   /**
-   * Converts the provided message content into a {@link javax.mail.Message} and sends it with
+   * Converts the provided message content into a {@link javax.mail.Message} and sends it with retry
-   * retry on transient failures.
+   * on transient failures.
    */
   public void sendEmail(EmailMessage emailMessage) {
     retrier.callWithRetry(
@@ -78,9 +79,8 @@ public class SendEmailService {
             attachmentPart.setFileName(attachment.filename());
             multipart.addBodyPart(attachmentPart);
           }
-          for (InternetAddress bcc : emailMessage.bccs()) {
+          msg.addRecipients(RecipientType.BCC, toArray(emailMessage.bccs(), Address.class));
-            msg.addRecipient(RecipientType.BCC, bcc);
+          msg.addRecipients(RecipientType.CC, toArray(emailMessage.ccs(), Address.class));
-          }
           msg.setContent(multipart);
           msg.saveChanges();
           transportEmailSender.sendMessage(msg);

util/src/test/java/google/registry/util/SendEmailServiceTest.java

@@ -68,7 +68,28 @@ class SendEmailServiceTest {
   }
 
   @Test
-  void testSuccess_bcc() throws Exception {
+  void testSuccess_addBccs() throws Exception {
+    EmailMessage.Builder contentBuilder =
+        EmailMessage.newBuilder()
+            .setFrom(new InternetAddress("test@example.com"))
+            .setSubject("test subject")
+            .setBody("test body");
+    for (String email :
+        ImmutableList.of("bcc@example.com", "bcc1@example.com", "bcc2@example.com")) {
+      contentBuilder.addBcc(new InternetAddress(email));
+    }
+
+    sendEmailService.sendEmail(contentBuilder.build());
+    assertThat(getMessage().getRecipients(RecipientType.BCC))
+        .asList()
+        .containsExactly(
+            new InternetAddress("bcc@example.com"),
+            new InternetAddress("bcc1@example.com"),
+            new InternetAddress("bcc2@example.com"));
+  }
+
+  @Test
+  void testSuccess_setBccs() throws Exception {
     EmailMessage content =
         createBuilder()
             .setBccs(
@@ -77,19 +98,63 @@ class SendEmailServiceTest {
                     new InternetAddress("bcc2@example.com")))
             .build();
     sendEmailService.sendEmail(content);
-    Message message = getMessage();
+    assertThat(getMessage().getRecipients(RecipientType.BCC))
-    assertThat(message.getRecipients(RecipientType.BCC))
         .asList()
         .containsExactly(
             new InternetAddress("bcc@example.com"), new InternetAddress("bcc2@example.com"));
   }
 
   @Test
-  void testSuccess_contentType() throws Exception {
+  void testSuccess_emptyBcc() throws Exception {
-    EmailMessage content = createBuilder().setContentType(MediaType.HTML_UTF_8).build();
+    sendEmailService.sendEmail(createBuilder().setBccs(ImmutableList.of()).build());
+    assertThat(getMessage().getRecipients(RecipientType.BCC)).isNull();
+  }
+
+  @Test
+  void testSuccess_addCcs() throws Exception {
+    EmailMessage.Builder contentBuilder = EmailMessage.newBuilder();
+    contentBuilder
+        .setFrom(new InternetAddress("test@example.com"))
+        .setSubject("test subject")
+        .setBody("test body");
+    for (String email : ImmutableList.of("cc@example.com", "cc1@example.com", "cc2@example.com")) {
+      contentBuilder.addCc(new InternetAddress(email));
+    }
+    sendEmailService.sendEmail(contentBuilder.build());
+    assertThat(getMessage().getRecipients(RecipientType.CC))
+        .asList()
+        .containsExactly(
+            new InternetAddress("cc@example.com"),
+            new InternetAddress("cc1@example.com"),
+            new InternetAddress("cc2@example.com"));
+  }
+
+  @Test
+  void testSuccess_setCcs() throws Exception {
+    EmailMessage content =
+        createBuilder()
+            .setCcs(
+                ImmutableList.of(
+                    new InternetAddress("cc@example.com"), new InternetAddress("cc2@example.com")))
+            .build();
     sendEmailService.sendEmail(content);
-    Message message = getMessage();
+    assertThat(getMessage().getRecipients(RecipientType.CC))
-    assertThat(getInternalContent(message).getContentType()).isEqualTo("text/html; charset=utf-8");
+        .asList()
+        .containsExactly(
+            new InternetAddress("cc@example.com"), new InternetAddress("cc2@example.com"));
+  }
+
+  @Test
+  void testSuccess_emptyCC() throws Exception {
+    sendEmailService.sendEmail(createBuilder().setCcs(ImmutableList.of()).build());
+    assertThat(getMessage().getRecipients(RecipientType.CC)).isNull();
+  }
+
+  @Test
+  void testSuccess_contentType() throws Exception {
+    sendEmailService.sendEmail(createBuilder().setContentType(MediaType.HTML_UTF_8).build());
+    assertThat(getInternalContent(getMessage()).getContentType())
+        .isEqualTo("text/html; charset=utf-8");
   }
 
   @Test
@@ -117,17 +182,16 @@ class SendEmailServiceTest {
         .doNothing()
         .when(wrapper)
         .sendMessage(messageCaptor.capture());
-    EmailMessage content = createBuilder().build();
+    sendEmailService.sendEmail(createBuilder().build());
-    sendEmailService.sendEmail(content);
     assertThat(messageCaptor.getValue().getSubject()).isEqualTo("Subject");
   }
 
   @Test
   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));
+        assertThrows(
+            RuntimeException.class, () -> sendEmailService.sendEmail(createBuilder().build()));
     assertThat(thrown).hasMessageThat().isEqualTo("this is a runtime exception");
   }
 
@@ -137,9 +201,9 @@ class SendEmailServiceTest {
         .doThrow(new MessagingException("second"))
         .when(wrapper)
         .sendMessage(any());
-    EmailMessage content = createBuilder().build();
     RuntimeException thrown =
-        assertThrows(RuntimeException.class, () -> sendEmailService.sendEmail(content));
+        assertThrows(
+            RuntimeException.class, () -> sendEmailService.sendEmail(createBuilder().build()));
     assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("second");
     assertThat(thrown).hasCauseThat().isInstanceOf(MessagingException.class);
   }