From cf0560607e63facfbdcd2e34041e56c0eccf54a9 Mon Sep 17 00:00:00 2001 From: sarahcaseybot Date: Thu, 10 Nov 2022 18:08:27 -0500 Subject: [PATCH] Send email for packages over create limit (#1835) * Send email for packages over create limit * Small change to query * Fix small nits --- .../batch/CheckPackagesComplianceAction.java | 91 +++++++++++++++---- .../registry/config/RegistryConfig.java | 12 +++ .../config/RegistryConfigSettings.java | 7 ++ .../registry/config/files/default-config.yaml | 18 ++++ .../registry/ui/server/SendEmailUtils.java | 41 +++++++-- .../CheckPackagesComplianceActionTest.java | 41 ++++++++- .../ui/server/SendEmailUtilsTest.java | 29 ++++++ 7 files changed, 213 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/google/registry/batch/CheckPackagesComplianceAction.java b/core/src/main/java/google/registry/batch/CheckPackagesComplianceAction.java index 623bc3f2f..6b39e4796 100644 --- a/core/src/main/java/google/registry/batch/CheckPackagesComplianceAction.java +++ b/core/src/main/java/google/registry/batch/CheckPackagesComplianceAction.java @@ -13,17 +13,24 @@ // limitations under the License. package google.registry.batch; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; -import google.registry.model.domain.DomainHistory; +import com.google.common.primitives.Ints; +import google.registry.config.RegistryConfig.Config; +import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.PackagePromotion; +import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.RegistrarPoc; import google.registry.request.Action; import google.registry.request.Action.Service; import google.registry.request.auth.Auth; -import java.util.List; +import google.registry.ui.server.SendEmailUtils; +import java.util.Optional; +import javax.inject.Inject; /** * An action that checks all {@link PackagePromotion} objects for compliance with their max create @@ -37,6 +44,22 @@ public class CheckPackagesComplianceAction implements Runnable { public static final String PATH = "/_dr/task/checkPackagesCompliance"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final SendEmailUtils sendEmailUtils; + private final String packageCreateLimitEmailSubjectText; + private final String packageCreateLimitEmailBodyText; + private final String registrySupportEmail; + + @Inject + public CheckPackagesComplianceAction( + SendEmailUtils sendEmailUtils, + @Config("packageCreateLimitEmailSubjectText") String packageCreateLimitEmailSubjectText, + @Config("packageCreateLimitEmailBodyText") String packageCreateLimitEmailBodyText, + @Config("registrySupportEmail") String registrySupportEmail) { + this.sendEmailUtils = sendEmailUtils; + this.packageCreateLimitEmailSubjectText = packageCreateLimitEmailSubjectText; + this.packageCreateLimitEmailBodyText = packageCreateLimitEmailBodyText; + this.registrySupportEmail = registrySupportEmail; + } @Override public void run() { @@ -46,19 +69,19 @@ public class CheckPackagesComplianceAction implements Runnable { ImmutableList.Builder packagesOverCreateLimit = new ImmutableList.Builder<>(); for (PackagePromotion packagePromo : packages) { - List creates = - jpaTm() - .query( - "FROM DomainHistory WHERE current_package_token = :token AND" - + " modificationTime >= :lastBilling AND type = 'DOMAIN_CREATE'", - DomainHistory.class) - .setParameter("token", packagePromo.getToken().getKey().toString()) - .setParameter( - "lastBilling", packagePromo.getNextBillingDate().minusYears(1)) - .getResultList(); - - if (creates.size() > packagePromo.getMaxCreates()) { - int overage = creates.size() - packagePromo.getMaxCreates(); + Long creates = + (Long) + jpaTm() + .query( + "SELECT COUNT(*) FROM DomainHistory WHERE current_package_token =" + + " :token AND modificationTime >= :lastBilling AND type =" + + " 'DOMAIN_CREATE'") + .setParameter("token", packagePromo.getToken().getKey().toString()) + .setParameter( + "lastBilling", packagePromo.getNextBillingDate().minusYears(1)) + .getSingleResult(); + if (creates > packagePromo.getMaxCreates()) { + int overage = Ints.saturatedCast(creates) - packagePromo.getMaxCreates(); logger.atInfo().log( "Package with package token %s has exceeded their max domain creation limit" + " by %d name(s).", @@ -72,9 +95,43 @@ public class CheckPackagesComplianceAction implements Runnable { logger.atInfo().log( "Found %d packages over their create limit.", packagesOverCreateLimit.build().size()); - // TODO(sarahbot@) Send email to registrar and registry informing of creation - // overage once email template is finalized. + for (PackagePromotion packagePromotion : packagesOverCreateLimit.build()) { + AllocationToken packageToken = tm().loadByKey(packagePromotion.getToken()); + Optional registrar = + Registrar.loadByRegistrarIdCached( + packageToken.getAllowedRegistrarIds().iterator().next()); + if (registrar.isPresent()) { + String body = + String.format( + packageCreateLimitEmailBodyText, + registrar.get().getRegistrarName(), + packageToken.getToken(), + registrySupportEmail); + sendNotification( + packageToken, packageCreateLimitEmailSubjectText, body, registrar.get()); + } else { + logger.atSevere().log( + String.format( + "Could not find registrar for package token %s", packageToken)); + } + } } }); } + + private void sendNotification( + AllocationToken packageToken, String subject, String body, Registrar registrar) { + logger.atInfo().log( + String.format( + "Compliance email sent to the %s registrar regarding the package with token" + " %s.", + registrar.getRegistrarName(), packageToken.getToken())); + sendEmailUtils.sendEmail( + subject, + body, + Optional.of(registrySupportEmail), + registrar.getContacts().stream() + .filter(c -> c.getTypes().contains(RegistrarPoc.Type.ADMIN)) + .map(RegistrarPoc::getEmailAddress) + .collect(toImmutableList())); + } } diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 0987b553b..0bb4693f2 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1309,6 +1309,18 @@ public final class RegistryConfig { public static int provideHibernateJdbcBatchSize(RegistryConfigSettings config) { return config.hibernate.jdbcBatchSize; } + + @Provides + @Config("packageCreateLimitEmailSubjectText") + public static String providePackageCreateLimitEmailSubjectText(RegistryConfigSettings config) { + return config.packageMonitoring.packageCreateLimitEmailSubjectText; + } + + @Provides + @Config("packageCreateLimitEmailBodyText") + public static String providePackageCreateLimitEmailBodyText(RegistryConfigSettings config) { + return config.packageMonitoring.packageCreateLimitEmailBodyText; + } } /** Returns the App Engine project ID, which is based off the environment name. */ diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index af5214884..143aa1f0d 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -43,6 +43,7 @@ public class RegistryConfigSettings { public SslCertificateValidation sslCertificateValidation; public ContactHistory contactHistory; public DnsUpdate dnsUpdate; + public PackageMonitoring packageMonitoring; /** Configuration options that apply to the entire App Engine project. */ public static class AppEngine { @@ -256,4 +257,10 @@ public class RegistryConfigSettings { public String registrySupportEmail; public String registryCcEmail; } + + /** Configuration for package compliance monitoring. */ + public static class PackageMonitoring { + public String packageCreateLimitEmailSubjectText; + public String packageCreateLimitEmailBodyText; + } } diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index da7c00659..fa2f0b345 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -535,3 +535,21 @@ sslCertificateValidation: allowedEcdsaCurves: - secp256r1 - secp384r1 + +# Configuration options for the package compliance monitoring +packageMonitoring: + # Email subject text to notify partners their package has exceeded the limit for domain creates + packageCreateLimitEmailSubjectText: "NOTICE: Your Package Is Being Upgraded" + # Email body text template notify partners their package has exceeded the limit for domain creates + packageCreateLimitEmailBodyText: > + Dear %1$s, + + We are contacting you to inform you that your package with the package token + %2$s has exceeded its limit for annual domain creations. + Your package will now be upgraded to the next tier. + + If you have any questions or require additional support, please contact us + at %3$s. + + Regards, + Example Registry diff --git a/core/src/main/java/google/registry/ui/server/SendEmailUtils.java b/core/src/main/java/google/registry/ui/server/SendEmailUtils.java index 08f970405..2eec76cb5 100644 --- a/core/src/main/java/google/registry/ui/server/SendEmailUtils.java +++ b/core/src/main/java/google/registry/ui/server/SendEmailUtils.java @@ -23,6 +23,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.util.Objects; +import java.util.Optional; import java.util.stream.Stream; import javax.inject.Inject; import javax.mail.internet.AddressException; @@ -54,8 +55,9 @@ public class SendEmailUtils { } /** - * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses and the specified - * additionalAddresses. Returns true iff sending to at least 1 address was successful. + * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses, the bcc address, + * and the specified additionalAddresses. Returns true iff sending to at least 1 address was + * successful. * *

This means that if there are no recipients ({@link #hasRecipients} returns false), this will * return false even thought no error happened. @@ -64,7 +66,10 @@ public class SendEmailUtils { * not all) of the recipients had an error. */ public boolean sendEmail( - final String subject, String body, ImmutableList additionalAddresses) { + final String subject, + String body, + Optional bcc, + ImmutableList additionalAddresses) { try { InternetAddress from = new InternetAddress( @@ -89,13 +94,22 @@ public class SendEmailUtils { if (recipients.isEmpty()) { return false; } - emailService.sendEmail( + EmailMessage.Builder emailMessage = EmailMessage.newBuilder() .setBody(body) .setSubject(subject) .setRecipients(recipients) - .setFrom(from) - .build()); + .setFrom(from); + if (bcc.isPresent()) { + try { + InternetAddress bccInternetAddress = new InternetAddress(bcc.get(), true); + emailMessage.addBcc(bccInternetAddress); + } catch (AddressException e) { + logger.atSevere().withCause(e).log( + "Could not send email to %s with subject '%s'.", bcc, subject); + } + } + emailService.sendEmail(emailMessage.build()); return true; } catch (Throwable t) { logger.atSevere().withCause(t).log( @@ -105,6 +119,21 @@ public class SendEmailUtils { } } + /** + * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses and the specified + * additionalAddresses. Returns true iff sending to at least 1 address was successful. + * + *

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

This also means that if there are multiple recipients, it will return true even if some (but + * not all) of the recipients had an error. + */ + public boolean sendEmail( + final String subject, String body, ImmutableList additionalAddresses) { + return sendEmail(subject, body, Optional.empty(), additionalAddresses); + } + /** * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses. * diff --git a/core/src/test/java/google/registry/batch/CheckPackagesComplianceActionTest.java b/core/src/test/java/google/registry/batch/CheckPackagesComplianceActionTest.java index e7e4b3252..752072d1a 100644 --- a/core/src/test/java/google/registry/batch/CheckPackagesComplianceActionTest.java +++ b/core/src/test/java/google/registry/batch/CheckPackagesComplianceActionTest.java @@ -13,13 +13,20 @@ // limitations under the License. package google.registry.batch; +import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistEppResource; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.LogsSubject.assertAboutLogs; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import com.google.common.collect.ImmutableList; import com.google.common.testing.TestLogHandler; import google.registry.model.billing.BillingEvent.RenewalPriceBehavior; import google.registry.model.contact.Contact; @@ -29,8 +36,12 @@ import google.registry.model.domain.token.PackagePromotion; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeClock; +import google.registry.ui.server.SendEmailUtils; +import google.registry.util.EmailMessage; +import google.registry.util.SendEmailService; import java.util.logging.Level; import java.util.logging.Logger; +import javax.mail.internet.InternetAddress; import org.joda.money.CurrencyUnit; import org.joda.money.Money; import org.joda.time.DateTime; @@ -38,12 +49,16 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.ArgumentCaptor; import org.testcontainers.shaded.com.google.common.collect.ImmutableSet; /** Unit tests for {@link CheckPackagesComplianceAction}. */ public class CheckPackagesComplianceActionTest { // This is the default creation time for test data. private final FakeClock clock = new FakeClock(DateTime.parse("2012-03-25TZ")); + private static final String CREATE_LIMIT_EMAIL_SUBJECT = "create limit subject"; + private static final String CREATE_LIMIT_EMAIL_BODY = "create limit body %1$s %2$s %3$s"; + private static final String SUPPORT_EMAIL = "registry@test.com"; @RegisterExtension public final AppEngineExtension appEngine = @@ -54,15 +69,26 @@ public class CheckPackagesComplianceActionTest { private final TestLogHandler logHandler = new TestLogHandler(); private final Logger loggerToIntercept = Logger.getLogger(CheckPackagesComplianceAction.class.getCanonicalName()); + private final SendEmailService emailService = mock(SendEmailService.class); private Contact contact; private PackagePromotion packagePromotion; + private SendEmailUtils sendEmailUtils; + private ArgumentCaptor emailCaptor = ArgumentCaptor.forClass(EmailMessage.class); @BeforeEach - void beforeEach() { + void beforeEach() throws Exception { loggerToIntercept.addHandler(logHandler); + sendEmailUtils = + new SendEmailUtils( + new InternetAddress("outgoing@registry.example"), + "UnitTest Registry", + ImmutableList.of("notification@test.example", "notification2@test.example"), + emailService); createTld("tld"); - action = new CheckPackagesComplianceAction(); + action = + new CheckPackagesComplianceAction( + sendEmailUtils, CREATE_LIMIT_EMAIL_SUBJECT, CREATE_LIMIT_EMAIL_BODY, SUPPORT_EMAIL); token = persistResource( new AllocationToken.Builder() @@ -102,13 +128,14 @@ public class CheckPackagesComplianceActionTest { .build()); action.run(); + verifyNoInteractions(emailService); assertAboutLogs() .that(logHandler) .hasLogAtLevelWithMessage(Level.INFO, "Found no packages over their create limit."); } @Test - void testSuccess_onePackageOverCreateLimit() { + void testSuccess_onePackageOverCreateLimit() throws Exception { // Create limit is 1, creating 2 domains to go over the limit persistEppResource( DatabaseHelper.newDomain("foo.tld", contact) @@ -131,6 +158,12 @@ public class CheckPackagesComplianceActionTest { Level.INFO, "Package with package token abc123 has exceeded their max domain creation limit by 1" + " name(s)."); + verify(emailService).sendEmail(emailCaptor.capture()); + EmailMessage emailMessage = emailCaptor.getValue(); + assertThat(emailMessage.subject()).isEqualTo(CREATE_LIMIT_EMAIL_SUBJECT); + assertThat(emailMessage.body()) + .isEqualTo( + String.format(CREATE_LIMIT_EMAIL_BODY, "The Registrar", "abc123", SUPPORT_EMAIL)); } @Test @@ -196,6 +229,7 @@ public class CheckPackagesComplianceActionTest { Level.INFO, "Package with package token token has exceeded their max domain creation limit by 1" + " name(s)."); + verify(emailService, times(2)).sendEmail(any(EmailMessage.class)); } @Test @@ -237,5 +271,6 @@ public class CheckPackagesComplianceActionTest { assertAboutLogs() .that(logHandler) .hasLogAtLevelWithMessage(Level.INFO, "Found no packages over their create limit."); + verifyNoInteractions(emailService); } } diff --git a/core/src/test/java/google/registry/ui/server/SendEmailUtilsTest.java b/core/src/test/java/google/registry/ui/server/SendEmailUtilsTest.java index 8ef57c614..954617e4e 100644 --- a/core/src/test/java/google/registry/ui/server/SendEmailUtilsTest.java +++ b/core/src/test/java/google/registry/ui/server/SendEmailUtilsTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableList; import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; +import java.util.Optional; import javax.mail.MessagingException; import javax.mail.internet.InternetAddress; import org.junit.jupiter.api.Test; @@ -119,6 +120,34 @@ class SendEmailUtilsTest { verifyMessageSent("foo@example.com"); } + @Test + void testSuccess_bcc() throws Exception { + setRecipients(ImmutableList.of("johnny@fakesite.tld")); + assertThat(sendEmailUtils.hasRecipients()).isTrue(); + sendEmailUtils.sendEmail( + "Welcome to the Internet", + "It is a dark and scary place.", + Optional.of("bar@example.com"), + ImmutableList.of("baz@example.com")); + + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + ImmutableList.Builder recipientBuilder = ImmutableList.builder(); + for (String expectedRecipient : ImmutableList.of("johnny@fakesite.tld", "baz@example.com")) { + recipientBuilder.add(new InternetAddress(expectedRecipient)); + } + EmailMessage expectedContent = + EmailMessage.newBuilder() + .setSubject("Welcome to the Internet") + .setBody("It is a dark and scary place.") + .setFrom(new InternetAddress("outgoing@registry.example")) + .addBcc(new InternetAddress("bar@example.com")) + .setRecipients(recipientBuilder.build()) + .build(); + assertThat(emailMessage).isEqualTo(expectedContent); + } + @Test void testAdditionalRecipients() throws Exception { setRecipients(ImmutableList.of("foo@example.com"));