diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index b153eff2c..9cb2f77f2 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -509,7 +509,7 @@ public final class RegistryConfig { /** * The email address that outgoing emails from the app are sent from. * - * @see google.registry.ui.server.registrar.SendEmailUtils + * @see google.registry.ui.server.SendEmailUtils */ @Provides @Config("gSuiteOutgoingEmailAddress") @@ -520,10 +520,10 @@ public final class RegistryConfig { /** * The display name that is used on outgoing emails sent by Nomulus. * - * @see google.registry.ui.server.registrar.SendEmailUtils + * @see google.registry.ui.server.SendEmailUtils */ @Provides - @Config("gSuiteOutoingEmailDisplayName") + @Config("gSuiteOutgoingEmailDisplayName") public static String provideGSuiteOutgoingEmailDisplayName(RegistryConfigSettings config) { return config.gSuite.outgoingEmailDisplayName; } diff --git a/java/google/registry/ui/server/BUILD b/java/google/registry/ui/server/BUILD index af1bc6881..e05f76536 100644 --- a/java/google/registry/ui/server/BUILD +++ b/java/google/registry/ui/server/BUILD @@ -13,12 +13,16 @@ java_library( "//java/google/registry/ui/css:registrar_dbg.css.js", ], deps = [ + "//java/google/registry/config", "//java/google/registry/model", "//java/google/registry/ui", "//java/google/registry/ui/forms", "//java/google/registry/util", "@com_google_appengine_api_1_0_sdk", "@com_google_code_findbugs_jsr305", + "@com_google_dagger", + "@com_google_flogger", + "@com_google_flogger_system_backend", "@com_google_guava", "@com_google_re2j", "@io_bazel_rules_closure//closure/templates", diff --git a/java/google/registry/ui/server/registrar/SendEmailUtils.java b/java/google/registry/ui/server/SendEmailUtils.java similarity index 65% rename from java/google/registry/ui/server/registrar/SendEmailUtils.java rename to java/google/registry/ui/server/SendEmailUtils.java index 90fcc5469..278ab0f4f 100644 --- a/java/google/registry/ui/server/registrar/SendEmailUtils.java +++ b/java/google/registry/ui/server/SendEmailUtils.java @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package google.registry.ui.server.registrar; +package google.registry.ui.server; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Iterables.toArray; import com.google.common.base.Joiner; -import com.google.common.collect.Streams; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.util.SendEmailService; @@ -37,30 +37,40 @@ public class SendEmailUtils { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final String gSuiteOutgoingEmailAddress; - private final String gSuiteOutoingEmailDisplayName; + private final String gSuiteOutgoingEmailDisplayName; private final SendEmailService emailService; + private final ImmutableList registrarChangesNotificationEmailAddresses; @Inject public SendEmailUtils( @Config("gSuiteOutgoingEmailAddress") String gSuiteOutgoingEmailAddress, - @Config("gSuiteOutoingEmailDisplayName") String gSuiteOutoingEmailDisplayName, + @Config("gSuiteOutgoingEmailDisplayName") String gSuiteOutgoingEmailDisplayName, + @Config("registrarChangesNotificationEmailAddresses") + ImmutableList registrarChangesNotificationEmailAddresses, SendEmailService emailService) { this.gSuiteOutgoingEmailAddress = gSuiteOutgoingEmailAddress; - this.gSuiteOutoingEmailDisplayName = gSuiteOutoingEmailDisplayName; + this.gSuiteOutgoingEmailDisplayName = gSuiteOutgoingEmailDisplayName; this.emailService = emailService; + this.registrarChangesNotificationEmailAddresses = registrarChangesNotificationEmailAddresses; } /** - * Sends an email from Nomulus to the specified recipient(s). Returns true iff sending was - * successful. + * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses. Returns true iff + * sending to at least 1 address was successful. + * + *

This means that if there are no recepients ({@link #hasRecepients} returns false), this will + * return false even thought no error happened. + * + *

This also means that if there are multiple recepients, it will return true even if some (but + * not all) of the recepients had an error. */ - public boolean sendEmail(Iterable addresses, final String subject, String body) { + public boolean sendEmail(final String subject, String body) { try { Message msg = emailService.createMessage(); msg.setFrom( - new InternetAddress(gSuiteOutgoingEmailAddress, gSuiteOutoingEmailDisplayName)); + new InternetAddress(gSuiteOutgoingEmailAddress, gSuiteOutgoingEmailDisplayName)); List emails = - Streams.stream(addresses) + registrarChangesNotificationEmailAddresses.stream() .map( emailAddress -> { try { @@ -85,9 +95,17 @@ public class SendEmailUtils { } catch (Throwable t) { logger.atSevere().withCause(t).log( "Could not email to addresses %s with subject '%s'.", - Joiner.on(", ").join(addresses), subject); + Joiner.on(", ").join(registrarChangesNotificationEmailAddresses), subject); return false; } return true; } + + /** + * Returns whether there are any recepients set up. {@link #sendEmail} will always return false if + * there are no recepients. + */ + public boolean hasRecepients() { + return !registrarChangesNotificationEmailAddresses.isEmpty(); + } } diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 7da38302b..1d3f9eaa4 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -33,7 +33,6 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryEnvironment; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; @@ -51,6 +50,7 @@ import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormException; import google.registry.ui.forms.FormFieldException; import google.registry.ui.server.RegistrarFormFields; +import google.registry.ui.server.SendEmailUtils; import google.registry.util.AppEngineServiceUtils; import google.registry.util.CollectionUtils; import google.registry.util.DiffUtils; @@ -92,8 +92,6 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA @Inject AuthResult authResult; @Inject RegistryEnvironment registryEnvironment; - @Inject @Config("registrarChangesNotificationEmailAddresses") ImmutableList - registrarChangesNotificationEmailAddresses; @Inject RegistrarSettingsAction() {} private static final Predicate HAS_PHONE = @@ -478,7 +476,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA ImmutableSet existingContacts, Registrar updatedRegistrar, ImmutableSet updatedContacts) { - if (registrarChangesNotificationEmailAddresses.isEmpty()) { + if (!sendEmailUtils.hasRecepients()) { return; } @@ -495,7 +493,6 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA enqueueRegistrarSheetSync(appEngineServiceUtils.getCurrentVersionHostname("backend")); String environment = Ascii.toLowerCase(String.valueOf(registryEnvironment)); sendEmailUtils.sendEmail( - registrarChangesNotificationEmailAddresses, String.format( "Registrar %s (%s) updated in %s", existingRegistrar.getRegistrarName(), diff --git a/javatests/google/registry/ui/server/BUILD b/javatests/google/registry/ui/server/BUILD index a9779cfb7..e5f1ea307 100644 --- a/javatests/google/registry/ui/server/BUILD +++ b/javatests/google/registry/ui/server/BUILD @@ -13,12 +13,15 @@ java_library( deps = [ "//java/google/registry/ui/forms", "//java/google/registry/ui/server", + "//java/google/registry/util", "//javatests/google/registry/testing", + "@com_google_appengine_api_1_0_sdk", "@com_google_guava", "@com_google_truth", "@com_google_truth_extensions_truth_java8_extension", "@junit", "@org_hamcrest_library", + "@org_mockito_all", ], ) diff --git a/javatests/google/registry/ui/server/registrar/SendEmailUtilsTest.java b/javatests/google/registry/ui/server/SendEmailUtilsTest.java similarity index 77% rename from javatests/google/registry/ui/server/registrar/SendEmailUtilsTest.java rename to javatests/google/registry/ui/server/SendEmailUtilsTest.java index 119554421..6ad50e5a1 100644 --- a/javatests/google/registry/ui/server/registrar/SendEmailUtilsTest.java +++ b/javatests/google/registry/ui/server/SendEmailUtilsTest.java @@ -12,11 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package google.registry.ui.server.registrar; +package google.registry.ui.server; import static com.google.common.truth.Truth.assertThat; -import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddress; -import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -51,16 +49,23 @@ public class SendEmailUtilsTest { public void init() { message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); when(emailService.createMessage()).thenReturn(message); + } + + private void setRecepients(ImmutableList recepients) { sendEmailUtils = new SendEmailUtils( - getGSuiteOutgoingEmailAddress(), getGSuiteOutgoingEmailDisplayName(), emailService); + "outgoing@registry.example", + "outgoing display name", + recepients, + emailService); } @Test public void testSuccess_sendToOneAddress() throws Exception { + setRecepients(ImmutableList.of("johnny@fakesite.tld")); + assertThat(sendEmailUtils.hasRecepients()).isTrue(); assertThat( sendEmailUtils.sendEmail( - ImmutableList.of("johnny@fakesite.tld"), "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); @@ -73,9 +78,10 @@ public class SendEmailUtilsTest { @Test public void testSuccess_sendToMultipleAddresses() throws Exception { + setRecepients(ImmutableList.of("foo@example.com", "bar@example.com")); + assertThat(sendEmailUtils.hasRecepients()).isTrue(); assertThat( sendEmailUtils.sendEmail( - ImmutableList.of("foo@example.com", "bar@example.com"), "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); @@ -87,9 +93,10 @@ public class SendEmailUtilsTest { @Test public void testSuccess_ignoresMalformedEmailAddress() throws Exception { + setRecepients(ImmutableList.of("foo@example.com", "1iñvalidemail")); + assertThat(sendEmailUtils.hasRecepients()).isTrue(); assertThat( sendEmailUtils.sendEmail( - ImmutableList.of("foo@example.com", "1iñvalidemail"), "Welcome to the Internet", "It is a dark and scary place.")) .isTrue(); @@ -99,10 +106,23 @@ public class SendEmailUtilsTest { } @Test - public void testFailure_onlyGivenMalformedAddress() throws Exception { + public void testFailure_noAddresses() throws Exception { + setRecepients(ImmutableList.of()); + assertThat(sendEmailUtils.hasRecepients()).isFalse(); + assertThat( + sendEmailUtils.sendEmail( + "Welcome to the Internet", + "It is a dark and scary place.")) + .isFalse(); + verify(emailService, never()).sendMessage(any(Message.class)); + } + + @Test + public void testFailure_onlyGivenMalformedAddress() throws Exception { + setRecepients(ImmutableList.of("1iñvalidemail")); + assertThat(sendEmailUtils.hasRecepients()).isTrue(); assertThat( sendEmailUtils.sendEmail( - ImmutableList.of("1iñvalidemail"), "Welcome to the Internet", "It is a dark and scary place.")) .isFalse(); @@ -111,10 +131,11 @@ public class SendEmailUtilsTest { @Test public void testFailure_exceptionThrownDuringSend() throws Exception { + setRecepients(ImmutableList.of("foo@example.com")); + assertThat(sendEmailUtils.hasRecepients()).isTrue(); doThrow(new MessagingException()).when(emailService).sendMessage(any(Message.class)); assertThat( sendEmailUtils.sendEmail( - ImmutableList.of("foo@example.com"), "Welcome to the Internet", "It is a dark and scary place.")) .isFalse(); diff --git a/javatests/google/registry/ui/server/registrar/BUILD b/javatests/google/registry/ui/server/registrar/BUILD index 7e2aa43eb..fd60ec5c6 100644 --- a/javatests/google/registry/ui/server/registrar/BUILD +++ b/javatests/google/registry/ui/server/registrar/BUILD @@ -19,6 +19,7 @@ java_library( "//java/google/registry/request", "//java/google/registry/request/auth", "//java/google/registry/security", + "//java/google/registry/ui/server", "//java/google/registry/ui/server/registrar", "//java/google/registry/util", "//javatests/google/registry/security", diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index 6764d04aa..0740ebc17 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -41,6 +41,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.InjectRule; import google.registry.testing.MockitoJUnitRule; +import google.registry.ui.server.SendEmailUtils; import google.registry.util.AppEngineServiceUtils; import google.registry.util.SendEmailService; import java.io.PrintWriter; @@ -97,11 +98,12 @@ public class RegistrarSettingsActionTestCase { when(appEngineServiceUtils.getCurrentVersionHostname("backend")).thenReturn("backend.hostname"); action.jsonActionRunner = new JsonActionRunner( ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); - action.registrarChangesNotificationEmailAddresses = ImmutableList.of( - "notification@test.example", "notification2@test.example"); action.sendEmailUtils = new SendEmailUtils( - getGSuiteOutgoingEmailAddress(), getGSuiteOutgoingEmailDisplayName(), emailService); + getGSuiteOutgoingEmailAddress(), + getGSuiteOutgoingEmailDisplayName(), + ImmutableList.of("notification@test.example", "notification2@test.example"), + emailService); action.registryEnvironment = RegistryEnvironment.get(); action.registrarConsoleMetrics = new RegistrarConsoleMetrics(); action.authResult =