From eb5794bdcf0bef1fdc2b48b94ba57708bb60db6d Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Wed, 23 Jul 2025 08:54:45 +0000 Subject: [PATCH 1/5] feat(mail): Implement SMTP mail service and comprehensive tests This commit introduces a new implementation for the Mail Service that allows sending emails via an external SMTP server. This functionality is controlled by the environment variable. Key changes include: - : Updated to include the method which uses to construct and send MIME messages. It handles various scenarios including different body types (text, HTML), attachments, and custom headers. - : A new helper class to abstract system environment variable access, allowing for easier testing and mocking. - : A comprehensive new test suite for the SMTP mail service, covering core functionality, MIME structure, headers, configuration, and error handling. - : Updated to include necessary dependencies for testing, such as . The new implementation provides a more flexible and robust way to send emails from App Engine applications, especially in environments where the native App Engine Mail API is not available. --- api/pom.xml | 2 +- .../appengine/api/mail/MailServiceImpl.java | 200 ++++++ .../api/mail/SystemEnvironmentProvider.java | 27 + .../api/mail/MailServiceImplTest.java | 642 ++++++++++++++++++ 4 files changed, 870 insertions(+), 1 deletion(-) create mode 100644 api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java create mode 100644 api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java diff --git a/api/pom.xml b/api/pom.xml index e18ba55d2..95d6e5290 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -137,7 +137,7 @@ org.mockito - mockito-junit-jupiter + mockito-core test diff --git a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java index 55700f61b..2e4abfe6f 100644 --- a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java +++ b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java @@ -23,6 +23,26 @@ import com.google.apphosting.api.ApiProxy; import com.google.protobuf.ByteString; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Properties; +import javax.activation.DataHandler; +import javax.activation.DataSource; +import javax.mail.Address; +import javax.mail.BodyPart; +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.AddressException; +import javax.mail.internet.InternetAddress; +import javax.mail.internet.MimeBodyPart; +import javax.mail.internet.MimeMessage; +import javax.mail.internet.MimeMultipart; +import javax.mail.util.ByteArrayDataSource; /** * This class implements raw access to the mail service. @@ -33,6 +53,17 @@ */ class MailServiceImpl implements MailService { static final String PACKAGE = "mail"; + private final SystemEnvironmentProvider envProvider; + + /** Default constructor, used in production. */ + MailServiceImpl() { + this(new SystemEnvironmentProvider()); + } + + /** Constructor for testing, allowing a mock environment provider. */ + MailServiceImpl(SystemEnvironmentProvider envProvider) { + this.envProvider = envProvider; + } /** {@inheritDoc} */ @Override @@ -48,6 +79,171 @@ public void send(Message message) doSend(message, false); } + private void sendSmtp(Message message, boolean toAdmin) + throws IllegalArgumentException, IOException { + String smtpHost = envProvider.getenv("SMTP_HOST"); + if (smtpHost == null || smtpHost.isEmpty()) { + throw new IllegalArgumentException("SMTP_HOST environment variable is not set."); + } + Properties props = new Properties(); + props.put("mail.smtp.host", smtpHost); + props.put("mail.smtp.port", envProvider.getenv("SMTP_PORT")); + props.put("mail.smtp.auth", "true"); + if (Boolean.parseBoolean(envProvider.getenv("SMTP_USE_TLS"))) { + props.put("mail.smtp.starttls.enable", "true"); + } + + Session session = Session.getInstance(props, new javax.mail.Authenticator() { + protected javax.mail.PasswordAuthentication getPasswordAuthentication() { + return new javax.mail.PasswordAuthentication( + envProvider.getenv("SMTP_USER"), envProvider.getenv("SMTP_PASSWORD")); + } + }); + + try { + MimeMessage mimeMessage = new MimeMessage(session); + mimeMessage.setFrom(new InternetAddress(message.getSender())); + + List toRecipients = new ArrayList<>(); + List ccRecipients = new ArrayList<>(); + List bccRecipients = new ArrayList<>(); + + if (toAdmin) { + String adminRecipients = envProvider.getenv("ADMIN_EMAIL_RECIPIENTS"); + if (adminRecipients == null || adminRecipients.isEmpty()) { + throw new IllegalArgumentException("Admin recipients not configured."); + } + toRecipients.addAll(Arrays.asList(InternetAddress.parse(adminRecipients))); + } else { + if (message.getTo() != null) { + toRecipients.addAll(toInternetAddressList(message.getTo())); + } + if (message.getCc() != null) { + ccRecipients.addAll(toInternetAddressList(message.getCc())); + } + if (message.getBcc() != null) { + bccRecipients.addAll(toInternetAddressList(message.getBcc())); + } + } + + List
allTransportRecipients = new ArrayList<>(); + allTransportRecipients.addAll(toRecipients); + allTransportRecipients.addAll(ccRecipients); + allTransportRecipients.addAll(bccRecipients); + + if (allTransportRecipients.isEmpty()) { + throw new IllegalArgumentException("No recipients specified."); + } + + if (!toRecipients.isEmpty()) { + mimeMessage.setRecipients(RecipientType.TO, toRecipients.toArray(new Address[0])); + } + if (!ccRecipients.isEmpty()) { + mimeMessage.setRecipients(RecipientType.CC, ccRecipients.toArray(new Address[0])); + } + // Bcc recipients are not set on the MimeMessage to prevent them from being in the headers. + + if (message.getReplyTo() != null) { + mimeMessage.setReplyTo(new Address[] {new InternetAddress(message.getReplyTo())}); + } + + mimeMessage.setSubject(message.getSubject()); + + final boolean hasAttachments = message.getAttachments() != null && !message.getAttachments().isEmpty(); + final boolean hasHtmlBody = message.getHtmlBody() != null; + final boolean hasAmpHtmlBody = message.getAmpHtmlBody() != null; + final boolean hasTextBody = message.getTextBody() != null; + + // Case 1: Plain text only, no attachments. Simplest case. + if (hasTextBody && !hasHtmlBody && !hasAmpHtmlBody && !hasAttachments) { + mimeMessage.setText(message.getTextBody()); + } else { + // Case 2: Anything more complex requires multipart. + MimeMultipart topLevelMultipart = new MimeMultipart("mixed"); + + // The bodies (text, html, amp) are grouped in a "multipart/alternative" + if (hasTextBody || hasHtmlBody || hasAmpHtmlBody) { + MimeMultipart alternativeMultipart = new MimeMultipart("alternative"); + MimeBodyPart alternativeBodyPart = new MimeBodyPart(); + alternativeBodyPart.setContent(alternativeMultipart); + + if (hasTextBody) { + MimeBodyPart textPart = new MimeBodyPart(); + textPart.setText(message.getTextBody()); + alternativeMultipart.addBodyPart(textPart); + } else if (hasHtmlBody) { + // If there is an HTML body but no text body, add an empty text part for compatibility. + MimeBodyPart textPart = new MimeBodyPart(); + textPart.setText(""); + alternativeMultipart.addBodyPart(textPart); + } + + if (hasHtmlBody) { + MimeBodyPart htmlPart = new MimeBodyPart(); + htmlPart.setContent(message.getHtmlBody(), "text/html"); + alternativeMultipart.addBodyPart(htmlPart); + } + if (hasAmpHtmlBody) { + MimeBodyPart ampPart = new MimeBodyPart(); + ampPart.setContent(message.getAmpHtmlBody(), "text/x-amp-html"); + alternativeMultipart.addBodyPart(ampPart); + } + topLevelMultipart.addBodyPart(alternativeBodyPart); + } + + // Add attachments to the top-level mixed part. + if (hasAttachments) { + for (Attachment attachment : message.getAttachments()) { + MimeBodyPart attachmentBodyPart = new MimeBodyPart(); + DataSource source = + new ByteArrayDataSource(attachment.getData(), "application/octet-stream"); + attachmentBodyPart.setDataHandler(new DataHandler(source)); + attachmentBodyPart.setFileName(attachment.getFileName()); + if (attachment.getContentID() != null) { + attachmentBodyPart.setContentID(attachment.getContentID()); + } + topLevelMultipart.addBodyPart(attachmentBodyPart); + } + } + mimeMessage.setContent(topLevelMultipart); + } + + if (message.getHeaders() != null) { + for (Header header : message.getHeaders()) { + mimeMessage.addHeader(header.getName(), header.getValue()); + } + } + + // Update headers to match content, e.g., setting the Content-Type + mimeMessage.saveChanges(); + + Transport transport = session.getTransport("smtp"); + try { + transport.connect(); + transport.sendMessage(mimeMessage, allTransportRecipients.toArray(new Address[0])); + } finally { + if (transport != null) { + transport.close(); + } + } + + } catch (MessagingException e) { + if (e instanceof javax.mail.AuthenticationFailedException) { + throw new IllegalArgumentException("SMTP authentication failed: " + e.getMessage(), e); + } + throw new IOException("Error sending email via SMTP: " + e.getMessage(), e); + } + } + + private List toInternetAddressList(Collection addresses) + throws AddressException { + List list = new ArrayList<>(); + for (String address : addresses) { + list.add(new InternetAddress(address)); + } + return list; + } + /** * Does the actual sending of the message. * @param message The message to be sent. @@ -55,6 +251,10 @@ public void send(Message message) */ private void doSend(Message message, boolean toAdmin) throws IllegalArgumentException, IOException { + if ("true".equals(envProvider.getenv("USE_SMTP_MAIL_SERVICE"))) { + sendSmtp(message, toAdmin); + return; + } // Could perform basic checks to save on RPCs in case of missing args etc. // I'm not doing this on purpose, to make sure the semantics of the two // implementations stay the same. diff --git a/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java b/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java new file mode 100644 index 000000000..9a0096abd --- /dev/null +++ b/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java @@ -0,0 +1,27 @@ +package com.google.appengine.api.mail; + +/** + * A simple wrapper around {@link System} to allow for easier testing. + */ +class SystemEnvironmentProvider { + /** + * Gets the value of the specified environment variable. + * @param name the name of the environment variable + * @return the string value of the variable, or {@code null} if the variable is not defined + */ + public String getenv(String name) { + return System.getenv(name); + } + + /** + * Gets the value of the specified environment variable, returning a default value if the + * variable is not defined. + * @param name the name of the environment variable + * @param defaultValue the default value to return + * @return the string value of the variable, or the default value if the variable is not defined + */ + public String getenv(String name, String defaultValue) { + String value = System.getenv(name); + return value != null ? value : defaultValue; + } +} diff --git a/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java b/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java new file mode 100644 index 000000000..e0ad09c99 --- /dev/null +++ b/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java @@ -0,0 +1,642 @@ +package com.google.appengine.api.mail; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doThrow; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.Properties; +import javax.mail.Address; +import javax.mail.Message.RecipientType; +import javax.mail.MessagingException; +import javax.mail.Session; +import javax.mail.Transport; +import javax.mail.internet.MimeBodyPart; +import javax.mail.internet.MimeMessage; +import javax.mail.internet.MimeMultipart; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.MockitoJUnitRunner; + +// TODO: Remove Silent runner and use default runner once all tests are implemented and unnecessary stubs are removed. +@RunWith(MockitoJUnitRunner.Silent.class) +public class MailServiceImplTest { + + @Mock private Transport transport; + @Mock private Session session; + @Mock private SystemEnvironmentProvider envProvider; + + private MailServiceImpl mailService; + + @Before + public void setUp() { + mailService = new MailServiceImpl(envProvider); + // Mock environment variables + when(envProvider.getenv("USE_SMTP_MAIL_SERVICE")).thenReturn("true"); + when(envProvider.getenv("SMTP_HOST")).thenReturn("smtp.example.com"); + when(envProvider.getenv("SMTP_PORT")).thenReturn("587"); + when(envProvider.getenv("SMTP_USER")).thenReturn("user"); + when(envProvider.getenv("SMTP_PASSWORD")).thenReturn("password"); + when(envProvider.getenv("SMTP_USE_TLS")).thenReturn("true"); + } + + @Test + public void testSendSmtp_basic() throws IOException, MessagingException { + // Setup + // Use try-with-resources for MockedStatic for Session + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message to send + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo(Collections.singletonList("to@example.com")); + message.setCc(Collections.singletonList("cc@example.com")); + message.setBcc(Collections.singletonList("bcc@example.com")); + message.setSubject("Test Subject"); + message.setTextBody("Test Body"); + + // Act + // Call the method under test + mailService.send(message); + + // Assert + // Capture the arguments passed to transport.sendMessage + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + ArgumentCaptor recipientsCaptor = ArgumentCaptor.forClass(Address[].class); + verify(transport, times(1)) + .sendMessage(messageCaptor.capture(), recipientsCaptor.capture()); + + // Assertions for the MimeMessage + MimeMessage sentMessage = messageCaptor.getValue(); + assertEquals("Test Subject", sentMessage.getSubject()); + assertEquals("sender@example.com", sentMessage.getFrom()[0].toString()); + assertEquals("to@example.com", sentMessage.getRecipients(RecipientType.TO)[0].toString()); + assertEquals("cc@example.com", sentMessage.getRecipients(RecipientType.CC)[0].toString()); + + Address[] bccRecipients = sentMessage.getRecipients(RecipientType.BCC); + assertTrue( + "BCC recipients should not be in the message headers", + bccRecipients == null || bccRecipients.length == 0); + + // Assertions for the recipient list passed to the transport layer + Address[] allRecipients = recipientsCaptor.getValue(); + assertEquals(3, allRecipients.length); + assertTrue( + "Recipient list should contain TO address", + Arrays.stream(allRecipients).anyMatch(a -> a.toString().equals("to@example.com"))); + assertTrue( + "Recipient list should contain CC address", + Arrays.stream(allRecipients).anyMatch(a -> a.toString().equals("cc@example.com"))); + assertTrue( + "Recipient list should contain BCC address", + Arrays.stream(allRecipients).anyMatch(a -> a.toString().equals("bcc@example.com"))); + } + } + + @Test + public void testSendSmtp_multipleRecipients() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with multiple recipients + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo(Arrays.asList("to1@example.com", "to2@example.com")); + message.setCc(Arrays.asList("cc1@example.com", "cc2@example.com")); + message.setBcc(Arrays.asList("bcc1@example.com", "bcc2@example.com")); + message.setSubject("Multiple Recipients Test"); + message.setTextBody("Test Body"); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + ArgumentCaptor recipientsCaptor = ArgumentCaptor.forClass(Address[].class); + verify(transport, times(1)) + .sendMessage(messageCaptor.capture(), recipientsCaptor.capture()); + + // Assertions for the MimeMessage headers + MimeMessage sentMessage = messageCaptor.getValue(); + assertEquals("to1@example.com, to2@example.com", sentMessage.getHeader("To", ", ")); + assertEquals("cc1@example.com, cc2@example.com", sentMessage.getHeader("Cc", ", ")); + + // Assertions for the recipient list passed to the transport layer + Address[] allRecipients = recipientsCaptor.getValue(); + assertEquals(6, allRecipients.length); + assertTrue( + "Recipient list should contain all TO addresses", + Arrays.stream(allRecipients) + .map(Address::toString) + .anyMatch(s -> s.equals("to1@example.com") || s.equals("to2@example.com"))); + assertTrue( + "Recipient list should contain all CC addresses", + Arrays.stream(allRecipients) + .map(Address::toString) + .anyMatch(s -> s.equals("cc1@example.com") || s.equals("cc2@example.com"))); + assertTrue( + "Recipient list should contain all BCC addresses", + Arrays.stream(allRecipients) + .map(Address::toString) + .anyMatch(s -> s.equals("bcc1@example.com") || s.equals("bcc2@example.com"))); + } + } + + @Test + public void testSendSmtp_htmlAndPlainTextBodies() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with HTML and plain text bodies + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("HTML and Plain Text Test"); + message.setTextBody("This is the plain text body."); + message.setHtmlBody("

This is the HTML body.

"); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertTrue(sentMessage.getHeader("Content-Type")[0].startsWith("multipart/mixed")); + + // Further inspection of the multipart content can be added here + // For example, checking the content of each part of the multipart message + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + assertEquals(1, mixedMultipart.getCount()); + + // Check the nested multipart/alternative part + MimeBodyPart alternativePart = (MimeBodyPart) mixedMultipart.getBodyPart(0); + assertTrue(alternativePart.getContentType().startsWith("multipart/alternative")); + MimeMultipart alternativeMultipart = (MimeMultipart) alternativePart.getContent(); + assertEquals(2, alternativeMultipart.getCount()); + + // Check the plain text part + MimeBodyPart textPart = (MimeBodyPart) alternativeMultipart.getBodyPart(0); + assertTrue(textPart.isMimeType("text/plain")); + assertEquals("This is the plain text body.", textPart.getContent()); + + // Check the HTML part + MimeBodyPart htmlPart = (MimeBodyPart) alternativeMultipart.getBodyPart(1); + assertTrue(htmlPart.isMimeType("text/html")); + assertEquals("

This is the HTML body.

", htmlPart.getContent()); + } + } + + @Test + public void testSendSmtp_htmlBodyOnly() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with only an HTML body + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("HTML Body Only Test"); + message.setHtmlBody("

This is the HTML body.

"); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertTrue(sentMessage.getHeader("Content-Type")[0].startsWith("multipart/mixed")); + + // Check the multipart content + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + assertEquals(1, mixedMultipart.getCount()); + + // Check the nested multipart/alternative part + MimeBodyPart alternativePart = (MimeBodyPart) mixedMultipart.getBodyPart(0); + assertTrue(alternativePart.getContentType().startsWith("multipart/alternative")); + MimeMultipart alternativeMultipart = (MimeMultipart) alternativePart.getContent(); + assertEquals(2, alternativeMultipart.getCount()); + + // Check that the plain text part is empty + MimeBodyPart textPart = (MimeBodyPart) alternativeMultipart.getBodyPart(0); + assertTrue(textPart.isMimeType("text/plain")); + assertEquals("", textPart.getContent()); + + // Check the HTML part + MimeBodyPart htmlPart = (MimeBodyPart) alternativeMultipart.getBodyPart(1); + assertTrue(htmlPart.isMimeType("text/html")); + assertEquals("

This is the HTML body.

", htmlPart.getContent()); + } + } + + @Test + public void testSendSmtp_singleAttachment() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with an attachment + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Single Attachment Test"); + message.setTextBody("This is the body."); + + byte[] attachmentData = "This is an attachment.".getBytes(); + MailService.Attachment attachment = new MailService.Attachment("attachment.txt", attachmentData); + message.setAttachments(Collections.singletonList(attachment)); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertTrue(sentMessage.getContentType().startsWith("multipart/mixed")); + + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + assertEquals(2, mixedMultipart.getCount()); + + // Check the body part, which should be a multipart/alternative + MimeBodyPart bodyPart = (MimeBodyPart) mixedMultipart.getBodyPart(0); + assertTrue(bodyPart.getContentType().startsWith("multipart/alternative")); + MimeMultipart alternativeMultipart = (MimeMultipart) bodyPart.getContent(); + assertEquals(1, alternativeMultipart.getCount()); + MimeBodyPart textPart = (MimeBodyPart) alternativeMultipart.getBodyPart(0); + assertTrue(textPart.isMimeType("text/plain")); + assertEquals("This is the body.", textPart.getContent()); + + + // Check the attachment part + MimeBodyPart attachmentPart = (MimeBodyPart) mixedMultipart.getBodyPart(1); + assertEquals("attachment.txt", attachmentPart.getFileName()); + + // Verify the content of the attachment + byte[] actualAttachmentData = new byte[attachmentData.length]; + attachmentPart.getDataHandler().getInputStream().read(actualAttachmentData); + assertTrue(Arrays.equals(attachmentData, actualAttachmentData)); + } + } + + @Test + public void testSendSmtp_multipleAttachments() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with multiple attachments + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Multiple Attachments Test"); + message.setTextBody("This is the body."); + + byte[] attachmentData1 = "This is attachment 1.".getBytes(); + MailService.Attachment attachment1 = new MailService.Attachment("attachment1.txt", attachmentData1); + byte[] attachmentData2 = "This is attachment 2.".getBytes(); + MailService.Attachment attachment2 = new MailService.Attachment("attachment2.txt", attachmentData2); + message.setAttachments(Arrays.asList(attachment1, attachment2)); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertTrue(sentMessage.getContentType().startsWith("multipart/mixed")); + + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + assertEquals(3, mixedMultipart.getCount()); + + // Check the body part + MimeBodyPart bodyPart = (MimeBodyPart) mixedMultipart.getBodyPart(0); + assertTrue(bodyPart.getContentType().startsWith("multipart/alternative")); + + // Check the first attachment + MimeBodyPart attachmentPart1 = (MimeBodyPart) mixedMultipart.getBodyPart(1); + assertEquals("attachment1.txt", attachmentPart1.getFileName()); + byte[] actualAttachmentData1 = new byte[attachmentData1.length]; + attachmentPart1.getDataHandler().getInputStream().read(actualAttachmentData1); + assertTrue(Arrays.equals(attachmentData1, actualAttachmentData1)); + + // Check the second attachment + MimeBodyPart attachmentPart2 = (MimeBodyPart) mixedMultipart.getBodyPart(2); + assertEquals("attachment2.txt", attachmentPart2.getFileName()); + byte[] actualAttachmentData2 = new byte[attachmentData2.length]; + attachmentPart2.getDataHandler().getInputStream().read(actualAttachmentData2); + assertTrue(Arrays.equals(attachmentData2, actualAttachmentData2)); + } + } + + @Test + public void testSendSmtp_htmlBodyAndAttachments() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with HTML body and an attachment + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("HTML Body and Attachments Test"); + message.setHtmlBody("

This is the HTML body.

"); + + byte[] attachmentData = "This is an attachment.".getBytes(); + MailService.Attachment attachment = new MailService.Attachment("attachment.txt", attachmentData); + message.setAttachments(Collections.singletonList(attachment)); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertTrue(sentMessage.getContentType().startsWith("multipart/mixed")); + + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + assertEquals(2, mixedMultipart.getCount()); + + // Check the body part (multipart/alternative) + MimeBodyPart bodyPart = (MimeBodyPart) mixedMultipart.getBodyPart(0); + assertTrue(bodyPart.getContentType().startsWith("multipart/alternative")); + MimeMultipart alternativeMultipart = (MimeMultipart) bodyPart.getContent(); + assertEquals(2, alternativeMultipart.getCount()); + + // Check the plain text part (should be empty) + MimeBodyPart textPart = (MimeBodyPart) alternativeMultipart.getBodyPart(0); + assertTrue(textPart.isMimeType("text/plain")); + assertEquals("", textPart.getContent()); + + // Check the HTML part + MimeBodyPart htmlPart = (MimeBodyPart) alternativeMultipart.getBodyPart(1); + assertTrue(htmlPart.isMimeType("text/html")); + assertEquals("

This is the HTML body.

", htmlPart.getContent()); + + // Check the attachment part + MimeBodyPart attachmentPart = (MimeBodyPart) mixedMultipart.getBodyPart(1); + assertEquals("attachment.txt", attachmentPart.getFileName()); + byte[] actualAttachmentData = new byte[attachmentData.length]; + attachmentPart.getDataHandler().getInputStream().read(actualAttachmentData); + assertTrue(Arrays.equals(attachmentData, actualAttachmentData)); + } + } + + @Test + public void testSendSmtp_attachmentWithContentId() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with an attachment with a Content-ID + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Attachment with Content-ID Test"); + message.setTextBody("This is the body."); + + byte[] attachmentData = "This is an attachment.".getBytes(); + MailService.Attachment attachment = + new MailService.Attachment("attachment.txt", attachmentData, ""); + message.setAttachments(Collections.singletonList(attachment)); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + MimeMultipart mixedMultipart = (MimeMultipart) sentMessage.getContent(); + + // Check the attachment part + MimeBodyPart attachmentPart = (MimeBodyPart) mixedMultipart.getBodyPart(1); + assertEquals("", attachmentPart.getContentID()); + } + } + + @Test + public void testSendSmtp_replyToHeader() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with a Reply-To header + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Reply-To Test"); + message.setTextBody("This is the body."); + message.setReplyTo("reply-to@example.com"); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertEquals(1, sentMessage.getReplyTo().length); + assertEquals("reply-to@example.com", sentMessage.getReplyTo()[0].toString()); + } + } + + @Test + public void testSendSmtp_customHeaders() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create the message with custom headers + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Custom Headers Test"); + message.setTextBody("This is the body."); + message.setHeaders( + Collections.singletonList(new MailService.Header("X-Custom-Header", "my-value"))); + + // Act + mailService.send(message); + + // Assert + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(MimeMessage.class); + verify(transport).sendMessage(messageCaptor.capture(), any()); + + MimeMessage sentMessage = messageCaptor.getValue(); + assertEquals("my-value", sentMessage.getHeader("X-Custom-Header")[0]); + } + } + + @Test + public void testSendSmtp_disabledTls() throws IOException, MessagingException { + // Setup + when(envProvider.getenv("SMTP_USE_TLS")).thenReturn("false"); + try (MockedStatic mockedSession = mockStatic(Session.class)) { + ArgumentCaptor propsCaptor = ArgumentCaptor.forClass(Properties.class); + mockedSession + .when(() -> Session.getInstance(propsCaptor.capture(), any())) + .thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create a simple message + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Disabled TLS Test"); + message.setTextBody("This is the body."); + + // Act + mailService.send(message); + + // Assert + Properties props = propsCaptor.getValue(); + assertEquals(null, props.getProperty("mail.smtp.starttls.enable")); + } + } + + @Test + public void testSendSmtp_adminEmail() throws IOException, MessagingException { + // Setup + when(envProvider.getenv("ADMIN_EMAIL_RECIPIENTS")) + .thenReturn("admin1@example.com,admin2@example.com"); + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + + // Create a message with some recipients that should be ignored + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setCc("cc@example.com"); + message.setBcc("bcc@example.com"); + message.setSubject("Admin Email Test"); + message.setTextBody("This is the body."); + + // Act + mailService.sendToAdmins(message); + + // Assert + ArgumentCaptor recipientsCaptor = ArgumentCaptor.forClass(Address[].class); + verify(transport).sendMessage(any(MimeMessage.class), recipientsCaptor.capture()); + + Address[] recipients = recipientsCaptor.getValue(); + assertEquals(2, recipients.length); + assertTrue( + "Recipient list should contain admin1@example.com", + Arrays.stream(recipients).anyMatch(a -> a.toString().equals("admin1@example.com"))); + assertTrue( + "Recipient list should contain admin2@example.com", + Arrays.stream(recipients).anyMatch(a -> a.toString().equals("admin2@example.com"))); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testSendSmtp_adminEmailNoRecipients() throws IOException, MessagingException { + // Setup + when(envProvider.getenv("ADMIN_EMAIL_RECIPIENTS")).thenReturn(null); + + // Create a simple message + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setSubject("Admin Email No Recipients Test"); + message.setTextBody("This is the body."); + + // Act + mailService.sendToAdmins(message); + } + + @Test(expected = IllegalArgumentException.class) + public void testSendSmtp_authenticationFailure() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + doThrow(new javax.mail.AuthenticationFailedException("Authentication failed")) + .when(transport) + .connect(); + + // Create a simple message + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Authentication Failure Test"); + message.setTextBody("This is the body."); + + // Act + mailService.send(message); + } + } + + @Test(expected = IOException.class) + public void testSendSmtp_connectionFailure() throws IOException, MessagingException { + // Setup + try (MockedStatic mockedSession = mockStatic(Session.class)) { + mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); + when(session.getTransport("smtp")).thenReturn(transport); + doThrow(new MessagingException("Connection failed")).when(transport).connect(); + + // Create a simple message + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Connection Failure Test"); + message.setTextBody("This is the body."); + + // Act + mailService.send(message); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testSendSmtp_missingSmtpHost() throws IOException, MessagingException { + // Setup + when(envProvider.getenv("SMTP_HOST")).thenReturn(null); + + // Create a simple message + MailService.Message message = new MailService.Message(); + message.setSender("sender@example.com"); + message.setTo("to@example.com"); + message.setSubject("Missing SMTP Host Test"); + message.setTextBody("This is the body."); + + // Act + mailService.send(message); + } +} From e5fb4624be6f89ed674772d323b6c67397579d69 Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Fri, 25 Jul 2025 09:07:09 +0000 Subject: [PATCH 2/5] feat(mail): Introduce EnvironmentProvider interface for SMTP settings Introduced an interface to abstract the retrieval of environment variables for the SMTP mail service. Key changes: - Created a new interface with methods. - Refactored to implement the new interface. - Updated to depend on the interface instead of the concrete class. This change allows for easier mocking of environment variables in tests, improving the overall testability of the mail service and promoting a more decoupled design. --- .../api/mail/EnvironmentProvider.java | 22 +++++++++++++++++++ .../appengine/api/mail/MailServiceImpl.java | 4 ++-- .../api/mail/SystemEnvironmentProvider.java | 4 +++- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 api/src/main/java/com/google/appengine/api/mail/EnvironmentProvider.java diff --git a/api/src/main/java/com/google/appengine/api/mail/EnvironmentProvider.java b/api/src/main/java/com/google/appengine/api/mail/EnvironmentProvider.java new file mode 100644 index 000000000..74be3c2d1 --- /dev/null +++ b/api/src/main/java/com/google/appengine/api/mail/EnvironmentProvider.java @@ -0,0 +1,22 @@ +package com.google.appengine.api.mail; + +/** + * An interface for providing environment variables. + */ +interface EnvironmentProvider { + /** + * Gets the value of the specified environment variable. + * @param name the name of the environment variable + * @return the string value of the variable, or {@code null} if the variable is not defined + */ + String getenv(String name); + + /** + * Gets the value of the specified environment variable, returning a default value if the + * variable is not defined. + * @param name the name of the environment variable + * @param defaultValue the default value to return + * @return the string value of the variable, or the default value if the variable is not defined + */ + String getenv(String name, String defaultValue); +} diff --git a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java index 2e4abfe6f..1b4960494 100644 --- a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java +++ b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java @@ -53,7 +53,7 @@ */ class MailServiceImpl implements MailService { static final String PACKAGE = "mail"; - private final SystemEnvironmentProvider envProvider; + private final EnvironmentProvider envProvider; /** Default constructor, used in production. */ MailServiceImpl() { @@ -61,7 +61,7 @@ class MailServiceImpl implements MailService { } /** Constructor for testing, allowing a mock environment provider. */ - MailServiceImpl(SystemEnvironmentProvider envProvider) { + MailServiceImpl(EnvironmentProvider envProvider) { this.envProvider = envProvider; } diff --git a/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java b/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java index 9a0096abd..a5807bfa5 100644 --- a/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java +++ b/api/src/main/java/com/google/appengine/api/mail/SystemEnvironmentProvider.java @@ -3,12 +3,13 @@ /** * A simple wrapper around {@link System} to allow for easier testing. */ -class SystemEnvironmentProvider { +class SystemEnvironmentProvider implements EnvironmentProvider { /** * Gets the value of the specified environment variable. * @param name the name of the environment variable * @return the string value of the variable, or {@code null} if the variable is not defined */ + @Override public String getenv(String name) { return System.getenv(name); } @@ -20,6 +21,7 @@ public String getenv(String name) { * @param defaultValue the default value to return * @return the string value of the variable, or the default value if the variable is not defined */ + @Override public String getenv(String name, String defaultValue) { String value = System.getenv(name); return value != null ? value : defaultValue; From d6717b497485e6485ff7dae173a36fde0aa9035f Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Fri, 25 Jul 2025 18:26:42 +0000 Subject: [PATCH 3/5] feat(mail): Prefix SMTP environment variables with APPENGINE_ Prefixed all SMTP-related environment variables (e.g., , ) with to improve namespacing and prevent potential conflicts. The implementation in and the corresponding mocks in have been updated to use the new variable names. --- .../appengine/api/mail/MailServiceImpl.java | 12 +++++----- .../api/mail/MailServiceImplTest.java | 22 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java index 1b4960494..08af76ef1 100644 --- a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java +++ b/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java @@ -81,22 +81,22 @@ public void send(Message message) private void sendSmtp(Message message, boolean toAdmin) throws IllegalArgumentException, IOException { - String smtpHost = envProvider.getenv("SMTP_HOST"); + String smtpHost = envProvider.getenv("APPENGINE_SMTP_HOST"); if (smtpHost == null || smtpHost.isEmpty()) { throw new IllegalArgumentException("SMTP_HOST environment variable is not set."); } Properties props = new Properties(); props.put("mail.smtp.host", smtpHost); - props.put("mail.smtp.port", envProvider.getenv("SMTP_PORT")); + props.put("mail.smtp.port", envProvider.getenv("APPENGINE_SMTP_PORT")); props.put("mail.smtp.auth", "true"); - if (Boolean.parseBoolean(envProvider.getenv("SMTP_USE_TLS"))) { + if (Boolean.parseBoolean(envProvider.getenv("APPENGINE_SMTP_USE_TLS"))) { props.put("mail.smtp.starttls.enable", "true"); } Session session = Session.getInstance(props, new javax.mail.Authenticator() { protected javax.mail.PasswordAuthentication getPasswordAuthentication() { return new javax.mail.PasswordAuthentication( - envProvider.getenv("SMTP_USER"), envProvider.getenv("SMTP_PASSWORD")); + envProvider.getenv("APPENGINE_SMTP_USER"), envProvider.getenv("APPENGINE_SMTP_PASSWORD")); } }); @@ -109,7 +109,7 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { List bccRecipients = new ArrayList<>(); if (toAdmin) { - String adminRecipients = envProvider.getenv("ADMIN_EMAIL_RECIPIENTS"); + String adminRecipients = envProvider.getenv("APPENGINE_ADMIN_EMAIL_RECIPIENTS"); if (adminRecipients == null || adminRecipients.isEmpty()) { throw new IllegalArgumentException("Admin recipients not configured."); } @@ -251,7 +251,7 @@ private List toInternetAddressList(Collection addresses */ private void doSend(Message message, boolean toAdmin) throws IllegalArgumentException, IOException { - if ("true".equals(envProvider.getenv("USE_SMTP_MAIL_SERVICE"))) { + if ("true".equals(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE"))) { sendSmtp(message, toAdmin); return; } diff --git a/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java b/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java index e0ad09c99..087e9dacb 100644 --- a/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java +++ b/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java @@ -36,7 +36,7 @@ public class MailServiceImplTest { @Mock private Transport transport; @Mock private Session session; - @Mock private SystemEnvironmentProvider envProvider; + @Mock private EnvironmentProvider envProvider; private MailServiceImpl mailService; @@ -44,12 +44,12 @@ public class MailServiceImplTest { public void setUp() { mailService = new MailServiceImpl(envProvider); // Mock environment variables - when(envProvider.getenv("USE_SMTP_MAIL_SERVICE")).thenReturn("true"); - when(envProvider.getenv("SMTP_HOST")).thenReturn("smtp.example.com"); - when(envProvider.getenv("SMTP_PORT")).thenReturn("587"); - when(envProvider.getenv("SMTP_USER")).thenReturn("user"); - when(envProvider.getenv("SMTP_PASSWORD")).thenReturn("password"); - when(envProvider.getenv("SMTP_USE_TLS")).thenReturn("true"); + when(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE")).thenReturn("true"); + when(envProvider.getenv("APPENGINE_SMTP_HOST")).thenReturn("smtp.example.com"); + when(envProvider.getenv("APPENGINE_SMTP_PORT")).thenReturn("587"); + when(envProvider.getenv("APPENGINE_SMTP_USER")).thenReturn("user"); + when(envProvider.getenv("APPENGINE_SMTP_PASSWORD")).thenReturn("password"); + when(envProvider.getenv("APPENGINE_SMTP_USE_TLS")).thenReturn("true"); } @Test @@ -507,7 +507,7 @@ public void testSendSmtp_customHeaders() throws IOException, MessagingException @Test public void testSendSmtp_disabledTls() throws IOException, MessagingException { // Setup - when(envProvider.getenv("SMTP_USE_TLS")).thenReturn("false"); + when(envProvider.getenv("APPENGINE_SMTP_USE_TLS")).thenReturn("false"); try (MockedStatic mockedSession = mockStatic(Session.class)) { ArgumentCaptor propsCaptor = ArgumentCaptor.forClass(Properties.class); mockedSession @@ -534,7 +534,7 @@ public void testSendSmtp_disabledTls() throws IOException, MessagingException { @Test public void testSendSmtp_adminEmail() throws IOException, MessagingException { // Setup - when(envProvider.getenv("ADMIN_EMAIL_RECIPIENTS")) + when(envProvider.getenv("APPENGINE_ADMIN_EMAIL_RECIPIENTS")) .thenReturn("admin1@example.com,admin2@example.com"); try (MockedStatic mockedSession = mockStatic(Session.class)) { mockedSession.when(() -> Session.getInstance(any(Properties.class), any())).thenReturn(session); @@ -570,7 +570,7 @@ public void testSendSmtp_adminEmail() throws IOException, MessagingException { @Test(expected = IllegalArgumentException.class) public void testSendSmtp_adminEmailNoRecipients() throws IOException, MessagingException { // Setup - when(envProvider.getenv("ADMIN_EMAIL_RECIPIENTS")).thenReturn(null); + when(envProvider.getenv("APPENGINE_ADMIN_EMAIL_RECIPIENTS")).thenReturn(null); // Create a simple message MailService.Message message = new MailService.Message(); @@ -627,7 +627,7 @@ public void testSendSmtp_connectionFailure() throws IOException, MessagingExcept @Test(expected = IllegalArgumentException.class) public void testSendSmtp_missingSmtpHost() throws IOException, MessagingException { // Setup - when(envProvider.getenv("SMTP_HOST")).thenReturn(null); + when(envProvider.getenv("APPENGINE_SMTP_HOST")).thenReturn(null); // Create a simple message MailService.Message message = new MailService.Message(); From 3e06a590574e8932f5a32a6dc8d1bb1004fc7407 Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Mon, 4 Aug 2025 08:52:18 +0000 Subject: [PATCH 4/5] refactor(mail): Implement Strategy pattern for MailService Refactored the MailService to use the Strategy pattern, addressing reviewer feedback to create a more modular and testable design. The mail sending logic is now split into two distinct strategies: the legacy App Engine API and the new SMTP implementation. Key changes: - Renamed the original to to clarify that it uses the traditional App Engine API. - Created a new to encapsulate all logic for sending email via an external SMTP server. - Updated to act as a factory. It now checks the environment variable to determine which implementation ( or ) to provide. - Reorganized the tests to match the new structure: - Renamed to to focus exclusively on the SMTP implementation. - Created to verify the factory's selection logic. This new structure improves adherence to the Single Responsibility and Open/Closed principles, making the system more scalable and easier to maintain. --- .../api/mail/LegacyMailServiceImpl.java | 164 ++++++++++++++++++ .../api/mail/MailServiceFactoryImpl.java | 16 +- ...viceImpl.java => SmtpMailServiceImpl.java} | 143 ++------------- .../api/mail/MailServiceFactoryImplTest.java | 36 ++++ ...Test.java => SmtpMailServiceImplTest.java} | 7 +- 5 files changed, 229 insertions(+), 137 deletions(-) create mode 100644 api/src/main/java/com/google/appengine/api/mail/LegacyMailServiceImpl.java rename api/src/main/java/com/google/appengine/api/mail/{MailServiceImpl.java => SmtpMailServiceImpl.java} (59%) create mode 100644 api/src/test/java/com/google/appengine/api/mail/MailServiceFactoryImplTest.java rename api/src/test/java/com/google/appengine/api/mail/{MailServiceImplTest.java => SmtpMailServiceImplTest.java} (99%) diff --git a/api/src/main/java/com/google/appengine/api/mail/LegacyMailServiceImpl.java b/api/src/main/java/com/google/appengine/api/mail/LegacyMailServiceImpl.java new file mode 100644 index 000000000..f484eb858 --- /dev/null +++ b/api/src/main/java/com/google/appengine/api/mail/LegacyMailServiceImpl.java @@ -0,0 +1,164 @@ +/* + * Copyright 2021 Google LLC + * + * 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 + * + * https://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 com.google.appengine.api.mail; + +import com.google.appengine.api.mail.MailServicePb.MailAttachment; +import com.google.appengine.api.mail.MailServicePb.MailHeader; +import com.google.appengine.api.mail.MailServicePb.MailMessage; +import com.google.appengine.api.mail.MailServicePb.MailServiceError.ErrorCode; +import com.google.apphosting.api.ApiProxy; +import com.google.protobuf.ByteString; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Properties; +import javax.activation.DataHandler; +import javax.activation.DataSource; +import javax.mail.Address; +import javax.mail.BodyPart; +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.AddressException; +import javax.mail.internet.InternetAddress; +import javax.mail.internet.MimeBodyPart; +import javax.mail.internet.MimeMessage; +import javax.mail.internet.MimeMultipart; +import javax.mail.util.ByteArrayDataSource; + +/** + * This class implements raw access to the mail service. + * Applications that don't want to make use of Sun's JavaMail + * can use it directly -- but they will forego the typing and + * convenience methods that JavaMail provides. + * + */ +class LegacyMailServiceImpl implements MailService { + static final String PACKAGE = "mail"; + + /** Default constructor. */ + LegacyMailServiceImpl() {} + + /** {@inheritDoc} */ + @Override + public void sendToAdmins(Message message) + throws IllegalArgumentException, IOException { + doSend(message, true); + } + + /** {@inheritDoc} */ + @Override + public void send(Message message) + throws IllegalArgumentException, IOException { + doSend(message, false); + } + + /** + * Does the actual sending of the message. + * @param message The message to be sent. + * @param toAdmin Whether the message is to be sent to the admins. + */ + private void doSend(Message message, boolean toAdmin) + throws IllegalArgumentException, IOException { + // Could perform basic checks to save on RPCs in case of missing args etc. + // I'm not doing this on purpose, to make sure the semantics of the two + // implementations stay the same. + // The benefit of not doing basic checks here is that we will pick up + // changes in semantics (ie from address can now also be the logged-in user) + // for free. + + MailMessage.Builder msgProto = MailMessage.newBuilder(); + if (message.getSender() != null) { + msgProto.setSender(message.getSender()); + } + if (message.getTo() != null) { + msgProto.addAllTo(message.getTo()); + } + if (message.getCc() != null) { + msgProto.addAllCc(message.getCc()); + } + if (message.getBcc() != null) { + msgProto.addAllBcc(message.getBcc()); + } + if (message.getReplyTo() != null) { + msgProto.setReplyTo(message.getReplyTo()); + } + if (message.getSubject() != null) { + msgProto.setSubject(message.getSubject()); + } + if (message.getTextBody() != null) { + msgProto.setTextBody(message.getTextBody()); + } + if (message.getHtmlBody() != null) { + msgProto.setHtmlBody(message.getHtmlBody()); + } + if (message.getAmpHtmlBody() != null) { + msgProto.setAmpHtmlBody(message.getAmpHtmlBody()); + } + if (message.getAttachments() != null) { + for (Attachment attach : message.getAttachments()) { + MailAttachment.Builder attachProto = MailAttachment.newBuilder(); + attachProto.setFileName(attach.getFileName()); + attachProto.setData(ByteString.copyFrom(attach.getData())); + String contentId = attach.getContentID(); + if (contentId != null) { + attachProto.setContentID(contentId); + } + msgProto.addAttachment(attachProto); + } + } + if (message.getHeaders() != null) { + for (Header header : message.getHeaders()) { + msgProto.addHeader( + MailHeader.newBuilder().setName(header.getName()).setValue(header.getValue())); + } + } + + byte[] msgBytes = msgProto.buildPartial().toByteArray(); + try { + // Returns VoidProto -- just ignore the return value. + if (toAdmin) { + ApiProxy.makeSyncCall(PACKAGE, "SendToAdmins", msgBytes); + } else { + ApiProxy.makeSyncCall(PACKAGE, "Send", msgBytes); + } + } catch (ApiProxy.ApplicationException ex) { + // Pass all the error details straight through (same as python). + switch (ErrorCode.forNumber(ex.getApplicationError())) { + case BAD_REQUEST: + throw new IllegalArgumentException("Bad Request: " + + ex.getErrorDetail()); + case UNAUTHORIZED_SENDER: + throw new IllegalArgumentException("Unauthorized Sender: " + + ex.getErrorDetail()); + case INVALID_ATTACHMENT_TYPE: + throw new IllegalArgumentException("Invalid Attachment Type: " + + ex.getErrorDetail()); + case INVALID_HEADER_NAME: + throw new IllegalArgumentException("Invalid Header Name: " + + ex.getErrorDetail()); + case INTERNAL_ERROR: + default: + throw new IOException(ex.getErrorDetail()); + } + } + } +} diff --git a/api/src/main/java/com/google/appengine/api/mail/MailServiceFactoryImpl.java b/api/src/main/java/com/google/appengine/api/mail/MailServiceFactoryImpl.java index a3d05702b..bbf2862df 100644 --- a/api/src/main/java/com/google/appengine/api/mail/MailServiceFactoryImpl.java +++ b/api/src/main/java/com/google/appengine/api/mail/MailServiceFactoryImpl.java @@ -21,8 +21,22 @@ */ final class MailServiceFactoryImpl implements IMailServiceFactory { + private final EnvironmentProvider envProvider; + + MailServiceFactoryImpl() { + this.envProvider = new SystemEnvironmentProvider(); + } + + // For testing + MailServiceFactoryImpl(EnvironmentProvider envProvider) { + this.envProvider = envProvider; + } + @Override public MailService getMailService() { - return new MailServiceImpl(); + if ("true".equals(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE"))) { + return new SmtpMailServiceImpl(envProvider); + } + return new LegacyMailServiceImpl(); } } diff --git a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java b/api/src/main/java/com/google/appengine/api/mail/SmtpMailServiceImpl.java similarity index 59% rename from api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java rename to api/src/main/java/com/google/appengine/api/mail/SmtpMailServiceImpl.java index 08af76ef1..d3a0cd71d 100644 --- a/api/src/main/java/com/google/appengine/api/mail/MailServiceImpl.java +++ b/api/src/main/java/com/google/appengine/api/mail/SmtpMailServiceImpl.java @@ -16,12 +16,6 @@ package com.google.appengine.api.mail; -import com.google.appengine.api.mail.MailServicePb.MailAttachment; -import com.google.appengine.api.mail.MailServicePb.MailHeader; -import com.google.appengine.api.mail.MailServicePb.MailMessage; -import com.google.appengine.api.mail.MailServicePb.MailServiceError.ErrorCode; -import com.google.apphosting.api.ApiProxy; -import com.google.protobuf.ByteString; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -31,10 +25,8 @@ import javax.activation.DataHandler; import javax.activation.DataSource; import javax.mail.Address; -import javax.mail.BodyPart; 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.AddressException; @@ -45,38 +37,27 @@ import javax.mail.util.ByteArrayDataSource; /** - * This class implements raw access to the mail service. - * Applications that don't want to make use of Sun's JavaMail - * can use it directly -- but they will forego the typing and - * convenience methods that JavaMail provides. - * + * This class implements the MailService interface using an external SMTP server. */ -class MailServiceImpl implements MailService { - static final String PACKAGE = "mail"; +class SmtpMailServiceImpl implements MailService { private final EnvironmentProvider envProvider; - /** Default constructor, used in production. */ - MailServiceImpl() { - this(new SystemEnvironmentProvider()); - } - - /** Constructor for testing, allowing a mock environment provider. */ - MailServiceImpl(EnvironmentProvider envProvider) { + /** + * Constructor. + * @param envProvider The provider for environment variables. + */ + SmtpMailServiceImpl(EnvironmentProvider envProvider) { this.envProvider = envProvider; } - /** {@inheritDoc} */ @Override - public void sendToAdmins(Message message) - throws IllegalArgumentException, IOException { - doSend(message, true); + public void send(Message message) throws IOException { + sendSmtp(message, false); } - /** {@inheritDoc} */ @Override - public void send(Message message) - throws IllegalArgumentException, IOException { - doSend(message, false); + public void sendToAdmins(Message message) throws IOException { + sendSmtp(message, true); } private void sendSmtp(Message message, boolean toAdmin) @@ -141,7 +122,6 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { if (!ccRecipients.isEmpty()) { mimeMessage.setRecipients(RecipientType.CC, ccRecipients.toArray(new Address[0])); } - // Bcc recipients are not set on the MimeMessage to prevent them from being in the headers. if (message.getReplyTo() != null) { mimeMessage.setReplyTo(new Address[] {new InternetAddress(message.getReplyTo())}); @@ -154,14 +134,11 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { final boolean hasAmpHtmlBody = message.getAmpHtmlBody() != null; final boolean hasTextBody = message.getTextBody() != null; - // Case 1: Plain text only, no attachments. Simplest case. if (hasTextBody && !hasHtmlBody && !hasAmpHtmlBody && !hasAttachments) { mimeMessage.setText(message.getTextBody()); } else { - // Case 2: Anything more complex requires multipart. MimeMultipart topLevelMultipart = new MimeMultipart("mixed"); - // The bodies (text, html, amp) are grouped in a "multipart/alternative" if (hasTextBody || hasHtmlBody || hasAmpHtmlBody) { MimeMultipart alternativeMultipart = new MimeMultipart("alternative"); MimeBodyPart alternativeBodyPart = new MimeBodyPart(); @@ -172,7 +149,6 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { textPart.setText(message.getTextBody()); alternativeMultipart.addBodyPart(textPart); } else if (hasHtmlBody) { - // If there is an HTML body but no text body, add an empty text part for compatibility. MimeBodyPart textPart = new MimeBodyPart(); textPart.setText(""); alternativeMultipart.addBodyPart(textPart); @@ -191,7 +167,6 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { topLevelMultipart.addBodyPart(alternativeBodyPart); } - // Add attachments to the top-level mixed part. if (hasAttachments) { for (Attachment attachment : message.getAttachments()) { MimeBodyPart attachmentBodyPart = new MimeBodyPart(); @@ -214,7 +189,6 @@ protected javax.mail.PasswordAuthentication getPasswordAuthentication() { } } - // Update headers to match content, e.g., setting the Content-Type mimeMessage.saveChanges(); Transport transport = session.getTransport("smtp"); @@ -243,99 +217,4 @@ private List toInternetAddressList(Collection addresses } return list; } - - /** - * Does the actual sending of the message. - * @param message The message to be sent. - * @param toAdmin Whether the message is to be sent to the admins. - */ - private void doSend(Message message, boolean toAdmin) - throws IllegalArgumentException, IOException { - if ("true".equals(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE"))) { - sendSmtp(message, toAdmin); - return; - } - // Could perform basic checks to save on RPCs in case of missing args etc. - // I'm not doing this on purpose, to make sure the semantics of the two - // implementations stay the same. - // The benefit of not doing basic checks here is that we will pick up - // changes in semantics (ie from address can now also be the logged-in user) - // for free. - - MailMessage.Builder msgProto = MailMessage.newBuilder(); - if (message.getSender() != null) { - msgProto.setSender(message.getSender()); - } - if (message.getTo() != null) { - msgProto.addAllTo(message.getTo()); - } - if (message.getCc() != null) { - msgProto.addAllCc(message.getCc()); - } - if (message.getBcc() != null) { - msgProto.addAllBcc(message.getBcc()); - } - if (message.getReplyTo() != null) { - msgProto.setReplyTo(message.getReplyTo()); - } - if (message.getSubject() != null) { - msgProto.setSubject(message.getSubject()); - } - if (message.getTextBody() != null) { - msgProto.setTextBody(message.getTextBody()); - } - if (message.getHtmlBody() != null) { - msgProto.setHtmlBody(message.getHtmlBody()); - } - if (message.getAmpHtmlBody() != null) { - msgProto.setAmpHtmlBody(message.getAmpHtmlBody()); - } - if (message.getAttachments() != null) { - for (Attachment attach : message.getAttachments()) { - MailAttachment.Builder attachProto = MailAttachment.newBuilder(); - attachProto.setFileName(attach.getFileName()); - attachProto.setData(ByteString.copyFrom(attach.getData())); - String contentId = attach.getContentID(); - if (contentId != null) { - attachProto.setContentID(contentId); - } - msgProto.addAttachment(attachProto); - } - } - if (message.getHeaders() != null) { - for (Header header : message.getHeaders()) { - msgProto.addHeader( - MailHeader.newBuilder().setName(header.getName()).setValue(header.getValue())); - } - } - - byte[] msgBytes = msgProto.buildPartial().toByteArray(); - try { - // Returns VoidProto -- just ignore the return value. - if (toAdmin) { - ApiProxy.makeSyncCall(PACKAGE, "SendToAdmins", msgBytes); - } else { - ApiProxy.makeSyncCall(PACKAGE, "Send", msgBytes); - } - } catch (ApiProxy.ApplicationException ex) { - // Pass all the error details straight through (same as python). - switch (ErrorCode.forNumber(ex.getApplicationError())) { - case BAD_REQUEST: - throw new IllegalArgumentException("Bad Request: " + - ex.getErrorDetail()); - case UNAUTHORIZED_SENDER: - throw new IllegalArgumentException("Unauthorized Sender: " + - ex.getErrorDetail()); - case INVALID_ATTACHMENT_TYPE: - throw new IllegalArgumentException("Invalid Attachment Type: " + - ex.getErrorDetail()); - case INVALID_HEADER_NAME: - throw new IllegalArgumentException("Invalid Header Name: " + - ex.getErrorDetail()); - case INTERNAL_ERROR: - default: - throw new IOException(ex.getErrorDetail()); - } - } - } } diff --git a/api/src/test/java/com/google/appengine/api/mail/MailServiceFactoryImplTest.java b/api/src/test/java/com/google/appengine/api/mail/MailServiceFactoryImplTest.java new file mode 100644 index 000000000..d0ae0bb73 --- /dev/null +++ b/api/src/test/java/com/google/appengine/api/mail/MailServiceFactoryImplTest.java @@ -0,0 +1,36 @@ +package com.google.appengine.api.mail; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class MailServiceFactoryImplTest { + + @Mock private EnvironmentProvider envProvider; + + @Test + public void testGetMailService_smtp() { + when(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE")).thenReturn("true"); + MailServiceFactoryImpl factory = new MailServiceFactoryImpl(envProvider); + assertTrue(factory.getMailService() instanceof SmtpMailServiceImpl); + } + + @Test + public void testGetMailService_legacy() { + when(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE")).thenReturn("false"); + MailServiceFactoryImpl factory = new MailServiceFactoryImpl(envProvider); + assertTrue(factory.getMailService() instanceof LegacyMailServiceImpl); + } + + @Test + public void testGetMailService_legacy_null() { + when(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE")).thenReturn(null); + MailServiceFactoryImpl factory = new MailServiceFactoryImpl(envProvider); + assertTrue(factory.getMailService() instanceof LegacyMailServiceImpl); + } +} diff --git a/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java b/api/src/test/java/com/google/appengine/api/mail/SmtpMailServiceImplTest.java similarity index 99% rename from api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java rename to api/src/test/java/com/google/appengine/api/mail/SmtpMailServiceImplTest.java index 087e9dacb..aa78bce6e 100644 --- a/api/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java +++ b/api/src/test/java/com/google/appengine/api/mail/SmtpMailServiceImplTest.java @@ -32,19 +32,18 @@ // TODO: Remove Silent runner and use default runner once all tests are implemented and unnecessary stubs are removed. @RunWith(MockitoJUnitRunner.Silent.class) -public class MailServiceImplTest { +public class SmtpMailServiceImplTest { @Mock private Transport transport; @Mock private Session session; @Mock private EnvironmentProvider envProvider; - private MailServiceImpl mailService; + private SmtpMailServiceImpl mailService; @Before public void setUp() { - mailService = new MailServiceImpl(envProvider); + mailService = new SmtpMailServiceImpl(envProvider); // Mock environment variables - when(envProvider.getenv("APPENGINE_USE_SMTP_MAIL_SERVICE")).thenReturn("true"); when(envProvider.getenv("APPENGINE_SMTP_HOST")).thenReturn("smtp.example.com"); when(envProvider.getenv("APPENGINE_SMTP_PORT")).thenReturn("587"); when(envProvider.getenv("APPENGINE_SMTP_USER")).thenReturn("user"); From 33594d2c4fdfdb68a80d7f8f6890aa2f5c660fa5 Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Tue, 5 Aug 2025 19:47:39 +0000 Subject: [PATCH 5/5] fix(tests): Update api_dev test to use LegacyMailServiceImpl Updated the mail service test in the module to reflect the recent refactoring. This test was failing in the CI environment because it was still referencing the old class. - Renamed to . - Updated the test class to instantiate and test . This change resolves the build failures observed in the GitHub workflow. --- ...st.java => LegacyMailServiceImplTest.java} | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) rename api_dev/src/test/java/com/google/appengine/api/mail/{MailServiceImplTest.java => LegacyMailServiceImplTest.java} (92%) diff --git a/api_dev/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java b/api_dev/src/test/java/com/google/appengine/api/mail/LegacyMailServiceImplTest.java similarity index 92% rename from api_dev/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java rename to api_dev/src/test/java/com/google/appengine/api/mail/LegacyMailServiceImplTest.java index fe6215c03..29cc93a3d 100644 --- a/api_dev/src/test/java/com/google/appengine/api/mail/MailServiceImplTest.java +++ b/api_dev/src/test/java/com/google/appengine/api/mail/LegacyMailServiceImplTest.java @@ -42,11 +42,11 @@ import org.mockito.junit.MockitoRule; /** - * Unit tests for the MailServiceImpl class. Cloned from URLFetchService. + * Unit tests for the LegacyMailServiceImpl class. Cloned from URLFetchService. * */ @RunWith(JUnit4.class) -public class MailServiceImplTest { +public class LegacyMailServiceImplTest { @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); @Mock private ApiProxy.Delegate delegate; @@ -57,7 +57,7 @@ public class MailServiceImplTest { public void setUp() throws Exception { ApiProxy.setDelegate(delegate); ApiProxy.setEnvironmentForCurrentThread(environment); - service = new MailServiceImpl(); + service = new LegacyMailServiceImpl(); } @After @@ -119,10 +119,10 @@ public void testSendAllNull() throws Exception { when(delegate.makeSyncCall(any(), any(), any(), any())).thenReturn(new byte[0]); - new MailServiceImpl().send(msg); + new LegacyMailServiceImpl().send(msg); verify(delegate) .makeSyncCall( - same(environment), eq(MailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); + same(environment), eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); } /** Tests that a message with an attachment works correctly. */ @@ -145,7 +145,7 @@ public void testDoSend_withAttachment() throws Exception { verify(delegate) .makeSyncCall( same(environment), - eq(MailServiceImpl.PACKAGE), + eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.build().toByteArray())); } @@ -175,7 +175,7 @@ public void testDoSend_withContentIDAttachment() throws Exception { verify(delegate) .makeSyncCall( - same(environment), eq(MailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); + same(environment), eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); } /** Tests that sending a AMP Email message works correctly. */ @@ -199,7 +199,7 @@ public void testDoSend_ampEmail() throws Exception { service.send(msg); verify(delegate) .makeSyncCall( - same(environment), eq(MailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); + same(environment), eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); } /** Tests that a message with a header works correctly. */ @@ -220,7 +220,7 @@ public void testDoSend_withHeader() throws Exception { verify(delegate) .makeSyncCall( same(environment), - eq(MailServiceImpl.PACKAGE), + eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.build().toByteArray())); } @@ -237,7 +237,7 @@ public void testDoSend_replyToAddress() throws Exception { verify(delegate) .makeSyncCall( - same(environment), eq(MailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); + same(environment), eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray())); } @Test @@ -306,7 +306,7 @@ private void runRecipientTest(String to, String cc, String bcc) throws Exception verify(delegate) .makeSyncCall( same(environment), - eq(MailServiceImpl.PACKAGE), + eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.build().toByteArray())); } @@ -324,7 +324,7 @@ private MailService.Message setupSendCallWithApplicationException(ErrorCode code MailMessage msgProto = newMailMessage(msg); when(delegate.makeSyncCall( - same(environment), eq(MailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray()))) + same(environment), eq(LegacyMailServiceImpl.PACKAGE), eq("Send"), eq(msgProto.toByteArray()))) .thenThrow(new ApiProxy.ApplicationException(code.getNumber(), "detail")); return msg; @@ -365,7 +365,7 @@ public void testSendToAdmins() throws Exception { verify(delegate) .makeSyncCall( same(environment), - eq(MailServiceImpl.PACKAGE), + eq(LegacyMailServiceImpl.PACKAGE), eq("SendToAdmins"), eq(msgProto.toByteArray())); } @@ -383,8 +383,8 @@ public void testSendToAdmins_multiArgConstructor() throws Exception { verify(delegate) .makeSyncCall( same(environment), - eq(MailServiceImpl.PACKAGE), + eq(LegacyMailServiceImpl.PACKAGE), eq("SendToAdmins"), eq(msgProto.toByteArray())); } -} +} \ No newline at end of file