From d3fc6063c997a4a69c9445b08752c7c01020b63c Mon Sep 17 00:00:00 2001 From: Rachel Guan Date: Thu, 10 Feb 2022 11:16:28 -0500 Subject: [PATCH] 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() --- .../sheet/SyncRegistrarsSheetAction.java | 12 +- .../module/frontend/FrontendComponent.java | 2 + .../registrar/RegistrarSettingsAction.java | 191 +++++++++++------- .../google/registry/server/TestServer.java | 20 +- .../registry/testing/CloudTasksHelper.java | 2 +- .../RegistrarSettingsActionTest.java | 48 ++--- .../RegistrarSettingsActionTestCase.java | 5 + 7 files changed, 166 insertions(+), 114 deletions(-) diff --git a/core/src/main/java/google/registry/export/sheet/SyncRegistrarsSheetAction.java b/core/src/main/java/google/registry/export/sheet/SyncRegistrarsSheetAction.java index e2b1ed249..fdb099327 100644 --- a/core/src/main/java/google/registry/export/sheet/SyncRegistrarsSheetAction.java +++ b/core/src/main/java/google/registry/export/sheet/SyncRegistrarsSheetAction.java @@ -14,8 +14,6 @@ package google.registry.export.sheet; -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; -import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static google.registry.request.Action.Method.POST; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -23,7 +21,6 @@ import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_OK; -import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.request.Action; @@ -100,7 +97,7 @@ public class SyncRegistrarsSheetAction implements Runnable { } public static final String PATH = "/_dr/task/syncRegistrarsSheet"; - private static final String QUEUE = "sheet"; + public static final String QUEUE = "sheet"; private static final String LOCK_NAME = "Synchronize registrars sheet"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -144,11 +141,4 @@ public class SyncRegistrarsSheetAction implements Runnable { Result.LOCKED.send(response, null); } } - - /** - * Enqueues a sync registrar sheet task targeting the App Engine service specified by hostname. - */ - public static void enqueueRegistrarSheetSync(String hostname) { - getQueue(QUEUE).add(withUrl(PATH).method(Method.GET).header("Host", hostname)); - } } diff --git a/core/src/main/java/google/registry/module/frontend/FrontendComponent.java b/core/src/main/java/google/registry/module/frontend/FrontendComponent.java index 54af5d0ad..ce50f0aa7 100644 --- a/core/src/main/java/google/registry/module/frontend/FrontendComponent.java +++ b/core/src/main/java/google/registry/module/frontend/FrontendComponent.java @@ -17,6 +17,7 @@ package google.registry.module.frontend; import com.google.monitoring.metrics.MetricReporter; import dagger.Component; import dagger.Lazy; +import google.registry.config.CloudTasksUtilsModule; import google.registry.config.CredentialModule; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.flows.ServerTridProviderModule; @@ -49,6 +50,7 @@ import javax.inject.Singleton; ConsoleConfigModule.class, CredentialModule.class, CustomLogicFactoryModule.class, + CloudTasksUtilsModule.class, DirectoryModule.class, DummyKeyringModule.class, FrontendRequestComponentModule.class, diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 48f0b26d2..d245eac38 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -19,7 +19,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Sets.difference; import static google.registry.config.RegistryEnvironment.PRODUCTION; -import static google.registry.export.sheet.SyncRegistrarsSheetAction.enqueueRegistrarSheetSync; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.security.JsonResponseHelper.Status.ERROR; @@ -32,18 +31,21 @@ import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryEnvironment; +import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.flows.certs.CertificateChecker; import google.registry.flows.certs.CertificateChecker.InsecureCertificateException; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; import google.registry.model.registrar.RegistrarContact.Type; import google.registry.request.Action; +import google.registry.request.Action.Service; import google.registry.request.HttpException.BadRequestException; import google.registry.request.HttpException.ForbiddenException; import google.registry.request.JsonActionRunner; @@ -58,6 +60,7 @@ import google.registry.ui.forms.FormFieldException; import google.registry.ui.server.RegistrarFormFields; import google.registry.ui.server.SendEmailUtils; import google.registry.util.AppEngineServiceUtils; +import google.registry.util.CloudTasksUtils; import google.registry.util.CollectionUtils; import google.registry.util.DiffUtils; import java.util.HashSet; @@ -88,6 +91,22 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA static final String ARGS_PARAM = "args"; static final String ID_PARAM = "id"; + /** + * Allows task enqueueing to be disabled when executing registrar console test cases. + * + *

The existing workflow in UI test cases triggers task enqueueing, which was not an issue with + * Task Queue since it's a native App Engine feature simulated by the App Engine SDK's + * environment. However, with Cloud Tasks, the server enqueues and fails to deliver to the actual + * Cloud Tasks endpoint due to lack of permission. + * + *

One way to allow enqueuing in backend test and avoid enqueuing in UI test is to disable + * enqueuing when the test server starts and enable enqueueing once the test server stops. This + * can be done by utilizing a ThreadLocal variable isInTestDriver, which is set to false + * by default. Enqueuing is allowed only if the value of isInTestDriver is false. It's set to true + * in start() and set to false in stop() inside TestDriver.java, a class used in testing. + */ + private static ThreadLocal isInTestDriver = ThreadLocal.withInitial(() -> false); + @Inject JsonActionRunner jsonActionRunner; @Inject AppEngineServiceUtils appEngineServiceUtils; @Inject RegistrarConsoleMetrics registrarConsoleMetrics; @@ -95,6 +114,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA @Inject AuthenticatedRegistrarAccessor registrarAccessor; @Inject AuthResult authResult; @Inject CertificateChecker certificateChecker; + @Inject CloudTasksUtils cloudTasksUtils; @Inject RegistrarSettingsAction() {} @@ -102,6 +122,14 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA return contact.getPhoneNumber() != null; } + public static void setIsInTestDriverToFalse() { + isInTestDriver.set(false); + } + + public static void setIsInTestDriverToTrue() { + isInTestDriver.set(true); + } + @Override public void run() { jsonActionRunner.run(this); @@ -170,6 +198,26 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } } + @AutoValue + abstract static class EmailInfo { + abstract Registrar registrar(); + + abstract Registrar updatedRegistrar(); + + abstract ImmutableSet contacts(); + + abstract ImmutableSet updatedContacts(); + + static EmailInfo create( + Registrar registrar, + Registrar updatedRegistrar, + ImmutableSet contacts, + ImmutableSet updatedContacts) { + return new AutoValue_RegistrarSettingsAction_EmailInfo( + registrar, updatedRegistrar, contacts, updatedContacts); + } + } + private RegistrarResult read(String registrarId) { return RegistrarResult.create("Success", loadRegistrarUnchecked(registrarId)); } @@ -183,72 +231,69 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } private RegistrarResult update(final Map args, String registrarId) { - tm().transact( - () -> { - // We load the registrar here rather than outside of the transaction - to make - // sure we have the latest version. This one is loaded inside the transaction, so it's - // guaranteed to not change before we update it. - Registrar registrar = loadRegistrarUnchecked(registrarId); - // Detach the registrar to avoid Hibernate object-updates, since we wish to email - // out the diffs between the existing and updated registrar objects - if (!tm().isOfy()) { - jpaTm().getEntityManager().detach(registrar); - } - // Verify that the registrar hasn't been changed. - // To do that - we find the latest update time (or null if the registrar has been - // deleted) and compare to the update time from the args. The update time in the args - // comes from the read that gave the UI the data - if it's out of date, then the UI - // had out of date data. - DateTime latest = registrar.getLastUpdateTime(); - DateTime latestFromArgs = - RegistrarFormFields.LAST_UPDATE_TIME.extractUntyped(args).get(); - if (!latestFromArgs.equals(latest)) { - logger.atWarning().log( - "Registrar changed since reading the data!" - + " Last updated at %s, but args data last updated at %s.", - latest, latestFromArgs); - throw new IllegalStateException( - "Registrar has been changed by someone else. Please reload and retry."); - } - - // Keep the current contacts so we can later check that no required contact was - // removed, email the changes to the contacts - ImmutableSet contacts = registrar.getContacts(); - - Registrar updatedRegistrar = registrar; - // Do OWNER only updates to the registrar from the request. - updatedRegistrar = checkAndUpdateOwnerControlledFields(updatedRegistrar, args); - // Do ADMIN only updates to the registrar from the request. - updatedRegistrar = checkAndUpdateAdminControlledFields(updatedRegistrar, args); - - // read the contacts from the request. - ImmutableSet updatedContacts = - readContacts(registrar, contacts, args); - - // Save the updated contacts - if (!updatedContacts.equals(contacts)) { - if (!registrarAccessor.hasRoleOnRegistrar(Role.OWNER, registrar.getRegistrarId())) { - throw new ForbiddenException("Only OWNERs can update the contacts"); - } - checkContactRequirements(contacts, updatedContacts); - RegistrarContact.updateContacts(updatedRegistrar, updatedContacts); - updatedRegistrar = - updatedRegistrar.asBuilder().setContactsRequireSyncing(true).build(); - } - - // Save the updated registrar - if (!updatedRegistrar.equals(registrar)) { - tm().put(updatedRegistrar); - } - - // Email the updates - sendExternalUpdatesIfNecessary( - registrar, contacts, updatedRegistrar, updatedContacts); - }); + // Email the updates + sendExternalUpdatesIfNecessary(tm().transact(() -> saveUpdates(args, registrarId))); // Reload the result outside of the transaction to get the most recent version return RegistrarResult.create("Saved " + registrarId, loadRegistrarUnchecked(registrarId)); } + /** Saves the updates and returns info needed for the update email */ + private EmailInfo saveUpdates(final Map args, String registrarId) { + // We load the registrar here rather than outside of the transaction - to make + // sure we have the latest version. This one is loaded inside the transaction, so it's + // guaranteed to not change before we update it. + Registrar registrar = loadRegistrarUnchecked(registrarId); + // Detach the registrar to avoid Hibernate object-updates, since we wish to email + // out the diffs between the existing and updated registrar objects + if (!tm().isOfy()) { + jpaTm().getEntityManager().detach(registrar); + } + // Verify that the registrar hasn't been changed. + // To do that - we find the latest update time (or null if the registrar has been + // deleted) and compare to the update time from the args. The update time in the args + // comes from the read that gave the UI the data - if it's out of date, then the UI + // had out of date data. + DateTime latest = registrar.getLastUpdateTime(); + DateTime latestFromArgs = RegistrarFormFields.LAST_UPDATE_TIME.extractUntyped(args).get(); + if (!latestFromArgs.equals(latest)) { + logger.atWarning().log( + "Registrar changed since reading the data!" + + " Last updated at %s, but args data last updated at %s.", + latest, latestFromArgs); + throw new IllegalStateException( + "Registrar has been changed by someone else. Please reload and retry."); + } + + // Keep the current contacts so we can later check that no required contact was + // removed, email the changes to the contacts + ImmutableSet contacts = registrar.getContacts(); + + Registrar updatedRegistrar = registrar; + // Do OWNER only updates to the registrar from the request. + updatedRegistrar = checkAndUpdateOwnerControlledFields(updatedRegistrar, args); + // Do ADMIN only updates to the registrar from the request. + updatedRegistrar = checkAndUpdateAdminControlledFields(updatedRegistrar, args); + + // read the contacts from the request. + ImmutableSet updatedContacts = readContacts(registrar, contacts, args); + + // Save the updated contacts + if (!updatedContacts.equals(contacts)) { + if (!registrarAccessor.hasRoleOnRegistrar(Role.OWNER, registrar.getRegistrarId())) { + throw new ForbiddenException("Only OWNERs can update the contacts"); + } + checkContactRequirements(contacts, updatedContacts); + RegistrarContact.updateContacts(updatedRegistrar, updatedContacts); + updatedRegistrar = updatedRegistrar.asBuilder().setContactsRequireSyncing(true).build(); + } + + // Save the updated registrar + if (!updatedRegistrar.equals(registrar)) { + tm().put(updatedRegistrar); + } + return EmailInfo.create(registrar, updatedRegistrar, contacts, updatedContacts); + } + private Map expandRegistrarWithContacts( Iterable contacts, Registrar registrar) { ImmutableSet> expandedContacts = @@ -575,26 +620,30 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA * sends an email with a diff of the changes to the configured notification email address and all * contact addresses and enqueues a task to re-sync the registrar sheet. */ - private void sendExternalUpdatesIfNecessary( - Registrar existingRegistrar, - ImmutableSet existingContacts, - Registrar updatedRegistrar, - ImmutableSet updatedContacts) { + private void sendExternalUpdatesIfNecessary(EmailInfo emailInfo) { + ImmutableSet existingContacts = emailInfo.contacts(); if (!sendEmailUtils.hasRecipients() && existingContacts.isEmpty()) { return; } - + Registrar existingRegistrar = emailInfo.registrar(); Map diffs = DiffUtils.deepDiff( expandRegistrarWithContacts(existingContacts, existingRegistrar), - expandRegistrarWithContacts(updatedContacts, updatedRegistrar), + expandRegistrarWithContacts(emailInfo.updatedContacts(), emailInfo.updatedRegistrar()), true); @SuppressWarnings("unchecked") Set changedKeys = (Set) diffs.keySet(); if (CollectionUtils.difference(changedKeys, "lastUpdateTime").isEmpty()) { return; } - enqueueRegistrarSheetSync(appEngineServiceUtils.getCurrentVersionHostname("backend")); + if (!isInTestDriver.get()) { + // Enqueues a sync registrar sheet task if enqueuing is not triggered by console tests and + // there's an update besides the lastUpdateTime + cloudTasksUtils.enqueue( + SyncRegistrarsSheetAction.QUEUE, + CloudTasksUtils.createGetTask( + SyncRegistrarsSheetAction.PATH, Service.BACKEND.toString(), ImmutableMultimap.of())); + } String environment = Ascii.toLowerCase(String.valueOf(RegistryEnvironment.get())); sendEmailUtils.sendEmail( String.format( diff --git a/core/src/test/java/google/registry/server/TestServer.java b/core/src/test/java/google/registry/server/TestServer.java index 21db930c0..44bded569 100644 --- a/core/src/test/java/google/registry/server/TestServer.java +++ b/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); diff --git a/core/src/test/java/google/registry/testing/CloudTasksHelper.java b/core/src/test/java/google/registry/testing/CloudTasksHelper.java index 2ca12fb3b..8df6794cc 100644 --- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java +++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java @@ -232,7 +232,7 @@ public class CloudTasksHelper implements Serializable { ImmutableMultimap.Builder 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( diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 2b3d7e155..639697b18 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/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 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 diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index a9b16558a..f12b88352 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/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));