Use CloudTasksUtils to enqueue in RegistrarSettingsAction (#1467)

* Use CloudTaskUtils to enqueue

* Add CloudTasksUtilsModule to FrontendComponent

* Fix Uri query issue

* Remove header and check service in matcher

* Use a ThreadLocal boolean in TestServer to determine enqueueing

* Extract enqueuing and email sending from tm().transact()

core/src/test/java/google/registry/server/TestServer.java

@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.HostAndPort;
 import com.google.common.util.concurrent.SimpleTimeLimiter;
+import google.registry.ui.server.registrar.RegistrarSettingsAction;
 import google.registry.util.UrlChecker;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -96,6 +97,7 @@ public final class TestServer {
   /** Starts the HTTP server in a new thread and returns once it's online. */
   public void start() {
     try {
+      RegistrarSettingsAction.setIsInTestDriverToTrue();
       server.start();
     } catch (Exception e) {
       throwIfUnchecked(e);
@@ -128,14 +130,16 @@ public final class TestServer {
   /** Stops the HTTP server. */
   public void stop() {
     try {
-      Void unusedReturnValue = SimpleTimeLimiter.create(newCachedThreadPool())
-          .callWithTimeout(
-              () -> {
-                server.stop();
-                return null;
-              },
-              SHUTDOWN_TIMEOUT_MS,
-              TimeUnit.MILLISECONDS);
+      Void unusedReturnValue =
+          SimpleTimeLimiter.create(newCachedThreadPool())
+              .callWithTimeout(
+                  () -> {
+                    server.stop();
+                    RegistrarSettingsAction.setIsInTestDriverToFalse();
+                    return null;
+                  },
+                  SHUTDOWN_TIMEOUT_MS,
+                  TimeUnit.MILLISECONDS);
     } catch (Exception e) {
       throwIfUnchecked(e);
       throw new RuntimeException(e);

core/src/test/java/google/registry/testing/CloudTasksHelper.java

@@ -232,7 +232,7 @@ public class CloudTasksHelper implements Serializable {
       ImmutableMultimap.Builder<String, String> paramBuilder = new ImmutableMultimap.Builder<>();
       // Note that UriParameters.parse() does not throw an IAE on a bad query string (e.g. one
       // where parameters are not properly URL-encoded); it always does a best-effort parse.
-      if (method == HttpMethod.GET) {
+      if (method == HttpMethod.GET && uri.getQuery() != null) {
         paramBuilder.putAll(UriParameters.parse(uri.getQuery()));
       } else if (method == HttpMethod.POST && !task.getAppEngineHttpRequest().getBody().isEmpty()) {
         assertThat(

core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java

@@ -18,13 +18,12 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
 import static google.registry.testing.DatabaseHelper.persistResource;
-import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
-import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
 import static google.registry.testing.TestDataHelper.loadFile;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
+import com.google.cloud.tasks.v2.HttpMethod;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -37,9 +36,9 @@ import google.registry.model.registrar.Registrar;
 import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
 import google.registry.testing.CertificateSamples;
+import google.registry.testing.CloudTasksHelper.TaskMatcher;
 import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.SystemPropertyExtension;
-import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import google.registry.testing.TestOfyAndSql;
 import google.registry.util.CidrAddressBlock;
 import google.registry.util.EmailMessage;
@@ -56,6 +55,7 @@ import org.mockito.ArgumentCaptor;
 @DualDatabaseTest
 class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
 
+
   @RegisterExtension
   final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension();
 
@@ -70,10 +70,12 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     ArgumentCaptor<EmailMessage> contentCaptor = ArgumentCaptor.forClass(EmailMessage.class);
     verify(emailService).sendEmail(contentCaptor.capture());
     assertThat(contentCaptor.getValue().body()).isEqualTo(expectedEmailBody);
-    assertTasksEnqueued("sheet", new TaskMatcher()
-        .url(SyncRegistrarsSheetAction.PATH)
-        .method("GET")
-        .header("Host", "backend.hostname"));
+    cloudTasksHelper.assertTasksEnqueued(
+        "sheet",
+        new TaskMatcher()
+            .url(SyncRegistrarsSheetAction.PATH)
+            .service("Backend")
+            .method(HttpMethod.GET));
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
   }
 
@@ -86,7 +88,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "results", ImmutableList.of(),
         "message",
         "One email address (etphonehome@example.com) cannot be used for multiple contacts");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: ContactRequirementException");
   }
 
@@ -103,7 +105,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "status", "ERROR",
             "results", ImmutableList.of(),
             "message", "TestUserId doesn't have access to registrar TheRegistrar");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "read", "[]", "ERROR: ForbiddenException");
   }
 
@@ -134,7 +136,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "field", "lastUpdateTime",
         "results", ImmutableList.of(),
         "message", "This field is required.");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
@@ -153,7 +155,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "field", "emailAddress",
         "results", ImmutableList.of(),
         "message", "This field is required.");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
@@ -171,7 +173,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "status", "ERROR",
             "results", ImmutableList.of(),
             "message", "TestUserId doesn't have access to registrar TheRegistrar");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[]", "ERROR: ForbiddenException");
   }
 
@@ -190,7 +192,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "field", "emailAddress",
         "results", ImmutableList.of(),
         "message", "Please enter a valid email address.");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
@@ -209,7 +211,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "field", "lastUpdateTime",
         "results", ImmutableList.of(),
         "message", "Not a valid ISO date-time string.");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
@@ -228,7 +230,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         "field", "emailAddress",
         "results", ImmutableList.of(),
         "message", "Please only use ASCII-US characters.");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
@@ -405,7 +407,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "Certificate validity period is too long; it must be less than or equal to 398"
                 + " days.");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -430,7 +432,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "Certificate is expired.\nCertificate validity period is too long; it must be less"
                 + " than or equal to 398 days.");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -473,7 +475,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
 
     assertThat(response).containsEntry("status", "SUCCESS");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -498,7 +500,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "Certificate validity period is too long; it must be less than or equal to 398"
                 + " days.");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -523,7 +525,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "Certificate is expired.\nCertificate validity period is too long; it must be less"
                 + " than or equal to 398 days.");
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -555,7 +557,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "results", ImmutableList.of(),
             "message", "Cannot add allowed TLDs if there is no WHOIS abuse contact set.");
     assertMetric(CLIENT_ID, "update", "[ADMIN]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -577,7 +579,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "results", ImmutableList.of(),
             "message", "TLDs do not exist: invalidtld");
     assertMetric(CLIENT_ID, "update", "[ADMIN]", "ERROR: IllegalArgumentException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql
@@ -599,7 +601,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
             "results", ImmutableList.of(),
             "message", "Can't remove allowed TLDs using the console.");
     assertMetric(CLIENT_ID, "update", "[ADMIN]", "ERROR: ForbiddenException");
-    assertNoTasksEnqueued("sheet");
+    cloudTasksHelper.assertNoTasksEnqueued("sheet");
   }
 
   @TestOfyAndSql

core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java

@@ -46,6 +46,7 @@ import google.registry.request.auth.AuthResult;
 import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
 import google.registry.testing.AppEngineExtension;
+import google.registry.testing.CloudTasksHelper;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectExtension;
 import google.registry.ui.server.SendEmailUtils;
@@ -97,6 +98,8 @@ public abstract class RegistrarSettingsActionTestCase {
 
   RegistrarContact techContact;
 
+  CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
+
   @BeforeEach
   public void beforeEachRegistrarSettingsActionTestCase() throws Exception {
     // Registrar "TheRegistrar" has access to TLD "currenttld" but not to "newtld".
@@ -132,6 +135,8 @@ public abstract class RegistrarSettingsActionTestCase {
             2048,
             ImmutableSet.of("secp256r1", "secp384r1"),
             clock);
+    action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
+
     inject.setStaticField(Ofy.class, "clock", clock);
     when(req.getMethod()).thenReturn("POST");
     when(rsp.getWriter()).thenReturn(new PrintWriter(writer));