From 58ec0f826d1e447e0852d76ba342c6e924e64029 Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Thu, 25 Jan 2024 16:02:04 -0500 Subject: [PATCH] Stop saving BSA empty refresh changes (#2307) * Stop saving BSA empty refresh changes We thought that as a way to verify the refresh job to be running, browsing the GCS bucket with empty files is easier than quering the DB or go to GCP logging dashboard, but there are too many of them to be useful. --- .../google/registry/bsa/BsaRefreshAction.java | 3 +-- .../registry/bsa/BsaRefreshFunctionalTest.java | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java index 2eb369b5e..a727d993a 100644 --- a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java +++ b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java @@ -117,13 +117,12 @@ public class BsaRefreshAction implements Runnable { case CHECK_FOR_CHANGES: ImmutableList blockabilityChanges = refresher.checkForBlockabilityChanges(); - // Always write even if no change. Easier for manual inspection of GCS bucket. - gcsClient.writeRefreshChanges(schedule.jobName(), blockabilityChanges.stream()); if (blockabilityChanges.isEmpty()) { logger.atInfo().log("No change to Unblockable domains found."); schedule.updateJobStage(RefreshStage.DONE); return null; } + gcsClient.writeRefreshChanges(schedule.jobName(), blockabilityChanges.stream()); schedule.updateJobStage(RefreshStage.APPLY_CHANGES); // Fall through case APPLY_CHANGES: diff --git a/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java b/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java index 401f2a26c..1e742e8a2 100644 --- a/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java +++ b/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java @@ -30,6 +30,7 @@ import static google.registry.testing.DatabaseHelper.deleteTestDomain; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -54,6 +55,7 @@ import google.registry.request.Response; import google.registry.testing.FakeClock; import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeResponse; +import java.io.UncheckedIOException; import java.util.Optional; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -293,7 +295,12 @@ class BsaRefreshFunctionalTest { action.run(); assertThat(queryUnblockableDomains()) .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); - assertThat(gcsClient.readRefreshChanges(jobName)).isEmpty(); + // Verify that refresh change file does not exist (404 error) since there is no change. + assertThat( + assertThrows( + UncheckedIOException.class, () -> gcsClient.readRefreshChanges(jobName).findAny())) + .hasMessageThat() + .contains("404"); verifyNoInteractions(bsaReportSender); } @@ -313,7 +320,12 @@ class BsaRefreshFunctionalTest { action.run(); assertThat(queryUnblockableDomains()) .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); - assertThat(gcsClient.readRefreshChanges(jobName)).isEmpty(); + // Verify that refresh change file does not exist (404 error) since there is no change. + assertThat( + assertThrows( + UncheckedIOException.class, () -> gcsClient.readRefreshChanges(jobName).findAny())) + .hasMessageThat() + .contains("404"); verifyNoInteractions(bsaReportSender); } }