Change BSA job status notifications (#2385)

Add error notifications for BsaDownload.

Stop sending success notifications.

core/src/main/java/google/registry/bsa/BsaDownloadAction.java

@@ -14,6 +14,7 @@
 
 package google.registry.bsa;
 
+import static com.google.common.base.Throwables.getStackTraceAsString;
 import static google.registry.bsa.BlockListType.BLOCK;
 import static google.registry.bsa.BlockListType.BLOCK_PLUS;
 import static google.registry.bsa.api.JsonSerializations.toCompletedOrdersReport;
@@ -65,6 +66,7 @@ public class BsaDownloadAction implements Runnable {
   private final BsaReportSender bsaReportSender;
   private final GcsClient gcsClient;
   private final Lazy<IdnChecker> lazyIdnChecker;
+  private final BsaEmailSender emailSender;
   private final BsaLock bsaLock;
   private final Clock clock;
   private final int transactionBatchSize;
@@ -78,6 +80,7 @@ public class BsaDownloadAction implements Runnable {
       BsaReportSender bsaReportSender,
       GcsClient gcsClient,
       Lazy<IdnChecker> lazyIdnChecker,
+      BsaEmailSender emailSender,
       BsaLock bsaLock,
       Clock clock,
       @Config("bsaTxnBatchSize") int transactionBatchSize,
@@ -88,6 +91,7 @@ public class BsaDownloadAction implements Runnable {
     this.bsaReportSender = bsaReportSender;
     this.gcsClient = gcsClient;
     this.lazyIdnChecker = lazyIdnChecker;
+    this.emailSender = emailSender;
     this.bsaLock = bsaLock;
     this.clock = clock;
     this.transactionBatchSize = transactionBatchSize;
@@ -98,12 +102,13 @@ public class BsaDownloadAction implements Runnable {
   public void run() {
     try {
       if (!bsaLock.executeWithLock(this::runWithinLock)) {
-        logger.atInfo().log("Job is being executed by another worker.");
+        String message = "BSA download did not run: another BSA related task is running";
+        logger.atInfo().log("%s.", message);
+        emailSender.sendNotification(message, /* body= */ "");
       }
     } catch (Throwable throwable) {
-      // TODO(12/31/2023): consider sending an alert email.
-      // TODO: if unretriable errors, log at severe and send email.
-      logger.atWarning().withCause(throwable).log("Failed to update block lists.");
+      logger.atWarning().withCause(throwable).log("Failed to download and process BSA data.");
+      emailSender.sendNotification("BSA download aborted", getStackTraceAsString(throwable));
     }
     // Always return OK. Let the next cron job retry.
     response.setStatus(SC_OK);

core/src/main/java/google/registry/bsa/BsaRefreshAction.java

@@ -90,8 +90,6 @@ public class BsaRefreshAction implements Runnable {
         String message = "BSA refresh did not run: another BSA related task is running";
         logger.atInfo().log("%s.", message);
         emailSender.sendNotification(message, /* body= */ "");
-      } else {
-        emailSender.sendNotification("BSA refreshed successfully", "");
       }
     } catch (Throwable throwable) {
       logger.atWarning().withCause(throwable).log("Failed to refresh BSA data.");

core/src/main/java/google/registry/bsa/BsaValidateAction.java

@@ -132,9 +132,10 @@ public class BsaValidateAction implements Runnable {
         errors.isEmpty()
             ? "BSA validation completed: no errors found"
             : "BSA validation completed with errors";
-
-    emailValidationResults(resultSummary, downloadJobName.get(), errors);
-    logger.atInfo().log("Finished validating BSA with latest download: %s", downloadJobName.get());
+    if (!errors.isEmpty()) {
+      emailValidationResults(resultSummary, downloadJobName.get(), errors);
+    }
+    logger.atInfo().log("%s (latest download: %s)", resultSummary, downloadJobName.get());
     return null;
   }
 

core/src/test/java/google/registry/bsa/BsaDownloadFunctionalTest.java

@@ -66,6 +66,8 @@ class BsaDownloadFunctionalTest {
   @Mock BlockListFetcher blockListFetcher;
   @Mock BsaReportSender bsaReportSender;
 
+  @Mock BsaEmailSender bsaEmailSender;
+
   private final FakeClock fakeClock = new FakeClock(TEST_START_TIME);
 
   @RegisterExtension
@@ -95,6 +97,7 @@ class BsaDownloadFunctionalTest {
             bsaReportSender,
             gcsClient,
             () -> new IdnChecker(fakeClock),
+            bsaEmailSender,
             new BsaLock(
                 new FakeLockHandler(/* lockSucceeds= */ true), Duration.standardSeconds(30)),
             fakeClock,

core/src/test/java/google/registry/bsa/BsaRefreshActionTest.java

@@ -16,6 +16,7 @@ package google.registry.bsa;
 
 import static com.google.common.base.Throwables.getStackTraceAsString;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -96,14 +97,13 @@ public class BsaRefreshActionTest {
   }
 
   @Test
-  void notificationSent_success() {
+  void notification_notSent_whenNoError() {
     when(bsaLock.executeWithLock(any()))
         .thenAnswer(
             args -> {
               return true;
             });
     action.run();
-    verify(gmailClient, times(1))
-        .sendEmail(EmailMessage.create("BSA refreshed successfully", "", emailRecipient));
+    verify(gmailClient, never()).sendEmail(any());
   }
 }

core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java

@@ -149,7 +149,7 @@ class BsaRefreshFunctionalTest {
     verify(bsaReportSender, times(1))
         .addUnblockableDomainsUpdates("{\n  \"reserved\": [\n    \"blocked1.app\"\n  ]\n}");
 
-    verify(emailSender, times(1)).sendNotification("BSA refreshed successfully", "");
+    verify(emailSender, never()).sendNotification(anyString(), anyString());
   }
 
   @Test

core/src/test/java/google/registry/bsa/BsaValidateActionTest.java

@@ -26,6 +26,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.startsWith;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -345,7 +346,7 @@ public class BsaValidateActionTest {
   }
 
   @Test
-  void notificationSent_noError() {
+  void notification_notSent_WhenNoError() {
     when(bsaLock.executeWithLock(any()))
         .thenAnswer(
             args -> {
@@ -356,9 +357,6 @@ public class BsaValidateActionTest {
     action = spy(action);
     doReturn(ImmutableList.of()).when(action).checkBsaLabels(anyString());
     action.run();
-    verify(gmailClient, times(1)).sendEmail(emailCaptor.capture());
-    EmailMessage message = emailCaptor.getValue();
-    assertThat(message.subject()).isEqualTo("BSA validation completed: no errors found");
-    assertThat(message.body()).isEqualTo("Most recent download is 2023-11-09t020857.880z.\n\n");
+    verify(gmailClient, never()).sendEmail(any());
   }
 }