Make a few quality-of-life improvements in CloudTasksUtils (#1521)

* Make a few quality-of-life improvements in CloudTasksUtils 1. Update the method names. There are too many overloaded methods and it is hard to figure out which one does which without checking the javadoc. 2. Added a method in the task matcher to specify the delay time in DateTime, so the caller does not need to convert it to Timestamp. 3. Remove the expilict dependency on a clock when enqueueing a task with delay, the clock is now injected directly into the util instance itself.
2026-02-09 14:30:33 +00:00 · 2022-02-18 20:21:56 -05:00
parent 52c759d1db
commit bbac81996b
31 changed files with 119 additions and 123 deletions
--- a/core/src/test/java/google/registry/cron/CommitLogFanoutActionTest.java
+++ b/core/src/test/java/google/registry/cron/CommitLogFanoutActionTest.java
@@ -33,7 +33,7 @@ class CommitLogFanoutActionTest {

  private static final String ENDPOINT = "/the/servlet";
  private static final String QUEUE = "the-queue";
-  private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
+  private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(new FakeClock());

  @RegisterExtension
  final AppEngineExtension appEngineExtension =
@@ -58,7 +58,6 @@ class CommitLogFanoutActionTest {
    action.endpoint = ENDPOINT;
    action.queue = QUEUE;
    action.jitterSeconds = Optional.empty();
-    action.clock = new FakeClock();
    action.run();
    List<TaskMatcher> matchers = new ArrayList<>();
    for (int bucketId : CommitLogBucket.getBucketIds()) {
--- a/core/src/test/java/google/registry/cron/TldFanoutActionTest.java
+++ b/core/src/test/java/google/registry/cron/TldFanoutActionTest.java
@@ -45,7 +45,7 @@ class TldFanoutActionTest {
  private static final String ENDPOINT = "/the/servlet";
  private static final String QUEUE = "the-queue";
  private final FakeResponse response = new FakeResponse();
-  private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
+  private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(new FakeClock());

  @RegisterExtension
  final AppEngineExtension appEngine =
@@ -61,7 +61,6 @@ class TldFanoutActionTest {

  private void run(ImmutableListMultimap<String, String> params) {
    TldFanoutAction action = new TldFanoutAction();
-    action.clock = new FakeClock();
    action.params = params;
    action.endpoint = ENDPOINT;
    action.queue = QUEUE;
--- a/core/src/test/java/google/registry/reporting/billing/GenerateInvoicesActionTest.java
+++ b/core/src/test/java/google/registry/reporting/billing/GenerateInvoicesActionTest.java
@@ -23,7 +23,6 @@ import static org.mockito.Mockito.when;

 import com.google.cloud.tasks.v2.HttpMethod;
 import com.google.common.net.MediaType;
-import com.google.protobuf.util.Timestamps;
 import google.registry.beam.BeamActionTestBase;
 import google.registry.model.common.DatabaseMigrationStateSchedule.PrimaryDatabase;
 import google.registry.reporting.ReportingModule;
@@ -83,11 +82,9 @@ class GenerateInvoicesActionTest extends BeamActionTestBase {
            .param("jobId", "jobid")
            .param("yearMonth", "2017-10")
            .scheduleTime(
-                Timestamps.fromMillis(
-                    clock
-                        .nowUtc()
-                        .plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES))
-                        .getMillis())));
+                clock
+                    .nowUtc()
+                    .plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES))));
  }

  @TestOfyAndSql
--- a/core/src/test/java/google/registry/reporting/icann/IcannReportingStagingActionTest.java
+++ b/core/src/test/java/google/registry/reporting/icann/IcannReportingStagingActionTest.java
@@ -24,7 +24,6 @@ import static org.mockito.Mockito.when;
 import com.google.cloud.tasks.v2.HttpMethod;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.protobuf.util.Timestamps;
 import google.registry.bigquery.BigqueryJobFailureException;
 import google.registry.reporting.icann.IcannReportingModule.ReportType;
 import google.registry.request.HttpException.BadRequestException;
@@ -54,7 +53,8 @@ class IcannReportingStagingActionTest {
  private YearMonth yearMonth = new YearMonth(2017, 6);
  private String subdir = "default/dir";
  private IcannReportingStagingAction action;
-  private CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
+  private FakeClock clock = new FakeClock(DateTime.parse("2021-01-02T11:00:00Z"));
+  private CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(clock);

  @RegisterExtension
  final AppEngineExtension appEngine =
@@ -77,7 +77,6 @@ class IcannReportingStagingActionTest {
    action.recipient = new InternetAddress("recipient@example.com");
    action.emailService = mock(SendEmailService.class);
    action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
-    action.clock = new FakeClock(DateTime.parse("2021-01-02T11:00:00Z"));

    when(stager.stageReports(yearMonth, subdir, ReportType.ACTIVITY))
        .thenReturn(ImmutableList.of("a", "b"));
@@ -91,9 +90,7 @@ class IcannReportingStagingActionTest {
        new TaskMatcher()
            .url("/_dr/task/icannReportingUpload")
            .method(HttpMethod.POST)
-            .scheduleTime(
-                Timestamps.fromMillis(
-                    action.clock.nowUtc().plus(Duration.standardMinutes(2)).getMillis())));
+            .scheduleTime(clock.nowUtc().plus(Duration.standardMinutes(2))));
  }

  @Test
--- a/core/src/test/java/google/registry/reporting/spec11/GenerateSpec11ReportActionTest.java
+++ b/core/src/test/java/google/registry/reporting/spec11/GenerateSpec11ReportActionTest.java
@@ -21,7 +21,6 @@ import static org.mockito.Mockito.when;

 import com.google.cloud.tasks.v2.HttpMethod;
 import com.google.common.net.MediaType;
-import com.google.protobuf.util.Timestamps;
 import google.registry.beam.BeamActionTestBase;
 import google.registry.model.common.DatabaseMigrationStateSchedule.PrimaryDatabase;
 import google.registry.reporting.ReportingModule;
@@ -44,7 +43,7 @@ class GenerateSpec11ReportActionTest extends BeamActionTestBase {
      AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build();

  private final FakeClock clock = new FakeClock(DateTime.parse("2018-06-11T12:23:56Z"));
-  private CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
+  private CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(clock);
  private CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
  private GenerateSpec11ReportAction action;

@@ -101,11 +100,9 @@ class GenerateSpec11ReportActionTest extends BeamActionTestBase {
            .param("jobId", "jobid")
            .param("date", "2018-06-11")
            .scheduleTime(
-                Timestamps.fromMillis(
-                    clock
-                        .nowUtc()
-                        .plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES))
-                        .getMillis())));
+                clock
+                    .nowUtc()
+                    .plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES))));
  }

  @Test
--- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java
+++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java
@@ -40,6 +40,7 @@ import com.google.common.net.HttpHeaders;
 import com.google.common.net.MediaType;
 import com.google.common.truth.Truth8;
 import com.google.protobuf.Timestamp;
+import com.google.protobuf.util.Timestamps;
 import google.registry.model.ImmutableObject;
 import google.registry.util.CloudTasksUtils;
 import google.registry.util.Retrier;
@@ -60,6 +61,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import javax.annotation.Nonnull;
+import org.joda.time.DateTime;

 /**
 * Static utility functions for testing task queues.
@@ -92,13 +94,22 @@ public class CloudTasksHelper implements Serializable {
  private static final String PROJECT_ID = "test-project";
  private static final String LOCATION_ID = "test-location";

-  private final Retrier retrier = new Retrier(new FakeSleeper(new FakeClock()), 1);
  private final int instanceId = nextInstanceId.getAndIncrement();
-  private final CloudTasksUtils cloudTasksUtils =
-      new CloudTasksUtils(retrier, PROJECT_ID, LOCATION_ID, new FakeCloudTasksClient());
+  private final CloudTasksUtils cloudTasksUtils;
+
+  public CloudTasksHelper(FakeClock clock) {
+    this.cloudTasksUtils =
+        new CloudTasksUtils(
+            new Retrier(new FakeSleeper(clock), 1),
+            clock,
+            PROJECT_ID,
+            LOCATION_ID,
+            new FakeCloudTasksClient());
+    testTasks.put(instanceId, Multimaps.synchronizedListMultimap(LinkedListMultimap.create()));
+  }

  public CloudTasksHelper() {
-    testTasks.put(instanceId, Multimaps.synchronizedListMultimap(LinkedListMultimap.create()));
+    this(new FakeClock());
  }

  public CloudTasksUtils getTestCloudTasksUtils() {
@@ -302,6 +313,10 @@ public class CloudTasksHelper implements Serializable {
      return this;
    }

+    public TaskMatcher scheduleTime(DateTime scheduleTime) {
+      return scheduleTime(Timestamps.fromMillis(scheduleTime.getMillis()));
+    }
+
    public TaskMatcher param(String key, String value) {
      checkNotNull(value, "Test error: A param can never have a null value, so don't assert it");
      expected.params.put(key, value);