Update RegistrarSettingsAction and RegistrarContact to SQL calls (#1042)

* Update RegistrarSettingsAction and RegistrarContact to SQL calls

Relevant potentially-unclear changes:
- Making sure the last update time is always correct and up to date in
the auto timestamp object
- Reloading the domain upon return when updating in a new transaction to
make sure that we use the properly-updated last update time (SQL returns
the correct result if retrieved within the same txn but DS does not)

core/src/main/java/google/registry/model/UpdateAutoTimestamp.java

@@ -58,7 +58,8 @@ public class UpdateAutoTimestamp extends ImmutableObject {
   @PreUpdate
   void setTimestamp() {
     if (autoUpdateEnabled() || lastUpdateTime == null) {
-      lastUpdateTime = DateTimeUtils.toZonedDateTime(jpaTm().getTransactionTime());
+      timestamp = jpaTm().getTransactionTime();
+      lastUpdateTime = DateTimeUtils.toZonedDateTime(timestamp);
     }
   }
 

core/src/main/java/google/registry/model/common/GaeUserIdConverter.java

@@ -16,8 +16,7 @@ package google.registry.model.common;
 
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.ofy.ObjectifyService.allocateId;
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.appengine.api.users.User;
 import com.google.common.base.Splitter;
@@ -56,13 +55,22 @@ public class GaeUserIdConverter extends ImmutableObject {
     try {
       // Perform these operations in a transactionless context to avoid enlisting in some outer
       // transaction (if any).
-      tm().doTransactionless(() -> ofy().saveWithoutBackup().entity(gaeUserIdConverter).now());
+      ofyTm()
+          .doTransactionless(
+              () -> {
+                ofyTm().putWithoutBackup(gaeUserIdConverter);
+                return null;
+              });
 
       // The read must be done in its own transaction to avoid reading from the session cache.
-      return tm()
+      return ofyTm().transactNew(() -> ofyTm().loadByEntity(gaeUserIdConverter).user.getUserId());
-          .transactNew(() -> ofy().load().entity(gaeUserIdConverter).safe().user.getUserId());
     } finally {
-      tm().doTransactionless(() -> ofy().deleteWithoutBackup().entity(gaeUserIdConverter).now());
+      ofyTm()
+          .doTransactionless(
+              () -> {
+                ofyTm().deleteWithoutBackup(gaeUserIdConverter);
+                return null;
+              });
     }
   }
 }

core/src/main/java/google/registry/model/registrar/RegistrarContact.java

@@ -23,6 +23,7 @@ import static com.google.common.io.BaseEncoding.base64;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registrar.Registrar.checkValidEmail;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableSortedCopy;
 import static google.registry.util.PasswordUtils.SALT_SUPPLIER;
@@ -200,17 +201,34 @@ public class RegistrarContact extends ImmutableObject
    * relevant Registrar entity with the {@link Registrar#contactsRequireSyncing} field set to true.
    */
   public static void updateContacts(
-      final Registrar registrar, final Set<RegistrarContact> contacts) {
+      final Registrar registrar, final ImmutableSet<RegistrarContact> contacts) {
     tm().transact(
             () -> {
-              ofy()
+              if (tm().isOfy()) {
-                  .delete()
+                ImmutableSet<Key<RegistrarContact>> existingKeys =
-                  .keys(
-                      difference(
                     ImmutableSet.copyOf(
-                              ofy().load().type(RegistrarContact.class).ancestor(registrar).keys()),
+                        ofy().load().type(RegistrarContact.class).ancestor(registrar).keys());
-                          contacts.stream().map(Key::create).collect(toImmutableSet())));
+                tm().delete(
-              ofy().save().entities(contacts);
+                        difference(
+                                existingKeys,
+                                contacts.stream().map(Key::create).collect(toImmutableSet()))
+                            .stream()
+                            .map(key -> VKey.createOfy(RegistrarContact.class, key))
+                            .collect(toImmutableSet()));
+              } else {
+                ImmutableSet<String> emailAddressesToKeep =
+                    contacts.stream()
+                        .map(RegistrarContact::getEmailAddress)
+                        .collect(toImmutableSet());
+                jpaTm()
+                    .query(
+                        "DELETE FROM RegistrarPoc WHERE registrarId = :registrarId AND "
+                            + "emailAddress NOT IN :emailAddressesToKeep")
+                    .setParameter("registrarId", registrar.getClientId())
+                    .setParameter("emailAddressesToKeep", emailAddressesToKeep)
+                    .executeUpdate();
+              }
+              tm().putAll(contacts);
             });
   }
 

core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java

@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static google.registry.model.common.GaeUserIdConverter.convertEmailAddressToGaeUserId;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.ui.server.SoyTemplateUtils.CSS_RENAMING_MAP_SUPPLIER;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
@@ -234,14 +233,13 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction {
               .setEmailAddress(consoleUserEmail.get())
               .setGaeUserId(gaeUserId)
               .build();
-      tm()
+      tm().transact(
-          .transact(
               () -> {
                 checkState(
                     !Registrar.loadByClientId(registrar.getClientId()).isPresent(),
                     "Registrar with client ID %s already exists",
                     registrar.getClientId());
-                ofy().save().entities(registrar, contact);
+                tm().putAll(registrar, contact);
               });
       data.put("password", password);
       data.put("passcode", phonePasscode);

core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java

@@ -20,7 +20,7 @@ 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.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.security.JsonResponseHelper.Status.ERROR;
 import static google.registry.security.JsonResponseHelper.Status.SUCCESS;
@@ -170,24 +170,28 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
   }
 
   private RegistrarResult read(String clientId) {
+    return RegistrarResult.create("Success", loadRegistrarUnchecked(clientId));
+  }
+
+  private Registrar loadRegistrarUnchecked(String registrarId) {
     try {
-      return RegistrarResult.create("Success", registrarAccessor.getRegistrar(clientId));
+      return registrarAccessor.getRegistrar(registrarId);
     } catch (RegistrarAccessDeniedException e) {
       throw new ForbiddenException(e.getMessage(), e);
     }
   }
 
   private RegistrarResult update(final Map<String, ?> args, String clientId) {
-    return tm().transact(
+    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;
+              Registrar registrar = loadRegistrarUnchecked(clientId);
-              try {
+              // Detach the registrar to avoid Hibernate object-updates, since we wish to email
-                registrar = registrarAccessor.getRegistrar(clientId);
+              // out the diffs between the existing and updated registrar objects
-              } catch (RegistrarAccessDeniedException e) {
+              if (!tm().isOfy()) {
-                throw new ForbiddenException(e.getMessage(), e);
+                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
@@ -233,14 +237,15 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
 
               // Save the updated registrar
               if (!updatedRegistrar.equals(registrar)) {
-                ofy().save().entity(updatedRegistrar);
+                tm().put(updatedRegistrar);
               }
 
-              // Email and return update.
+              // Email the updates
               sendExternalUpdatesIfNecessary(
                   registrar, contacts, updatedRegistrar, updatedContacts);
-              return RegistrarResult.create("Saved " + clientId, updatedRegistrar);
             });
+    // Reload the result outside of the transaction to get the most recent version
+    return RegistrarResult.create("Saved " + clientId, loadRegistrarUnchecked(clientId));
   }
 
   private Map<String, Object> expandRegistrarWithContacts(

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

@@ -40,9 +40,11 @@ import google.registry.request.auth.UserAuthInfo;
 import google.registry.security.XsrfTokenManager;
 import google.registry.testing.AppEngineExtension;
 import google.registry.testing.DeterministicStringGenerator;
+import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.SystemPropertyExtension;
+import google.registry.testing.TestOfyAndSql;
 import google.registry.ui.server.SendEmailUtils;
 import google.registry.util.EmailMessage;
 import google.registry.util.SendEmailService;
@@ -52,7 +54,6 @@ import javax.servlet.http.HttpServletRequest;
 import org.joda.money.CurrencyUnit;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.ArgumentCaptor;
@@ -60,6 +61,7 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
+@DualDatabaseTest
 final class ConsoleRegistrarCreatorActionTest {
 
   @RegisterExtension
@@ -124,7 +126,7 @@ final class ConsoleRegistrarCreatorActionTest {
     action.analyticsConfig = ImmutableMap.of("googleAnalyticsId", "sampleId");
   }
 
-  @Test
+  @TestOfyAndSql
   void testNoUser_redirect() {
     when(request.getRequestURI()).thenReturn("/test");
     action.authResult = AuthResult.NOT_AUTHENTICATED;
@@ -133,14 +135,14 @@ final class ConsoleRegistrarCreatorActionTest {
     assertThat(response.getHeaders().get(LOCATION)).isEqualTo("/_ah/login?continue=%2Ftest");
   }
 
-  @Test
+  @TestOfyAndSql
   void testGet_authorized() {
     action.run();
     assertThat(response.getPayload()).contains("<h1>Create Registrar</h1>");
     assertThat(response.getPayload()).contains("gtag('config', 'sampleId')");
   }
 
-  @Test
+  @TestOfyAndSql
   void testGet_authorized_onProduction() {
     RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension);
     action.run();
@@ -148,7 +150,7 @@ final class ConsoleRegistrarCreatorActionTest {
     assertThat(response.getPayload()).contains("gtag('config', 'sampleId')");
   }
 
-  @Test
+  @TestOfyAndSql
   void testGet_unauthorized() {
     action.registrarAccessor =
         AuthenticatedRegistrarAccessor.createForTesting(ImmutableSetMultimap.of());
@@ -157,7 +159,7 @@ final class ConsoleRegistrarCreatorActionTest {
     assertThat(response.getPayload()).contains("gtag('config', 'sampleId')");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_minimalAddress() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -225,7 +227,7 @@ final class ConsoleRegistrarCreatorActionTest {
                 .build());
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_allAddress() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -262,7 +264,7 @@ final class ConsoleRegistrarCreatorActionTest {
         .build());
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_multipleBillingLines() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -299,7 +301,7 @@ final class ConsoleRegistrarCreatorActionTest {
             CurrencyUnit.EUR, "billing-account-eur");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_repeatingCurrency_fails() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -327,7 +329,7 @@ final class ConsoleRegistrarCreatorActionTest {
                 + " JPY=billing-account-2 and JPY=billing-account-1");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_badCurrency_fails() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -354,7 +356,7 @@ final class ConsoleRegistrarCreatorActionTest {
         .contains("Failed: Error parsing billing accounts - Unknown currency &#39;XYZ&#39;");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_badBillingLine_fails() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -383,7 +385,7 @@ final class ConsoleRegistrarCreatorActionTest {
                 + " The format should be [currency]=[account ID]");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_authorized_setPassword() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -412,7 +414,7 @@ final class ConsoleRegistrarCreatorActionTest {
     assertThat(registrar.getPhonePasscode()).isEqualTo("10203");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_badEmailFails() {
     action.clientId = Optional.of("myclientid");
     action.name = Optional.of("registrar name");
@@ -433,7 +435,7 @@ final class ConsoleRegistrarCreatorActionTest {
         .contains("Failed: Provided email lolcat is not a valid email address");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_unauthorized() {
     action.registrarAccessor =
         AuthenticatedRegistrarAccessor.createForTesting(ImmutableSetMultimap.of());

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

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 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;
@@ -36,8 +37,10 @@ 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.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;
 import java.util.Map;
@@ -46,17 +49,17 @@ import java.util.function.Function;
 import org.joda.time.DateTime;
 import org.json.simple.JSONValue;
 import org.json.simple.parser.ParseException;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.ArgumentCaptor;
 
 /** Tests for {@link RegistrarSettingsAction}. */
+@DualDatabaseTest
 class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
 
   @RegisterExtension
   final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension();
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_updateRegistrarInfo_andSendsNotificationEmail() throws Exception {
     String expectedEmailBody = loadFile(getClass(), "update_registrar_email.txt");
     // This update changes some values on the admin contact and makes it a tech contact as well,
@@ -74,7 +77,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_updateRegistrarInfo_duplicateContacts() {
     Map<String, Object> response = action.handleJsonRequest(
         readJsonFromFile("update_registrar_duplicate_contacts.json", getLastUpdateTime()));
@@ -91,7 +94,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
    * Make sure that if someone spoofs a different registrar (they don't have access to), we fail.
    * Also relevant if the person's privilege were revoked after the page load.
    */
-  @Test
+  @TestOfyAndSql
   void testFailure_readRegistrarInfo_notAuthorized() {
     setUserWithoutAccess();
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID));
@@ -105,7 +108,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
   }
 
   /** This is the default read test for the registrar settings actions. */
-  @Test
+  @TestOfyAndSql
   void testSuccess_readRegistrarInfo_authorizedReadWrite() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID));
     assertThat(response)
@@ -116,7 +119,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "read", "[OWNER]", "SUCCESS");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_emptyJsonObject_errorLastUpdateTimeFieldRequired() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.remove("lastUpdateTime");
@@ -135,7 +138,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_noEmail_errorEmailFieldRequired() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.remove("emailAddress");
@@ -154,7 +157,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_updateRegistrarInfo_notAuthorized() {
     setUserWithoutAccess();
 
@@ -172,7 +175,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[]", "ERROR: ForbiddenException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_badEmail_errorEmailField() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.put("emailAddress", "lolcat");
@@ -191,7 +194,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_nonParsableTime_getsAngry() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.put("lastUpdateTime", "cookies");
@@ -210,7 +213,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormFieldException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testPost_nonAsciiCharacters_getsAngry() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.put("emailAddress", "ヘ(◕。◕ヘ)@example.com");
@@ -276,7 +279,11 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     // (We check it separately from the next assert to get better error message on failure)
     assertThat(getter.apply(updatedRegistrar)).isEqualTo(newValue);
     // ONLY that value changed:
-    assertThat(updatedRegistrar).isEqualTo(setter.apply(registrar.asBuilder(), newValue).build());
+    // (note: the setter won't properly update last update time, so ignore it)
+    assertAboutImmutableObjects()
+        .that(updatedRegistrar)
+        .isEqualExceptFields(
+            setter.apply(registrar.asBuilder(), newValue).build(), "lastUpdateTime");
     // We increased the correct metric
     assertMetric(CLIENT_ID, "update", String.format("[%s]", role), "SUCCESS");
   }
@@ -331,7 +338,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         CLIENT_ID, "update", allExceptCorrectRoles.toString(), "ERROR: ForbiddenException");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_whoisServer() {
     doTestUpdate(
         Role.OWNER,
@@ -340,24 +347,24 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setWhoisServer);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_phoneNumber() {
     doTestUpdate(
         Role.OWNER, Registrar::getPhoneNumber, "+1.2345678900", Registrar.Builder::setPhoneNumber);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_faxNumber() {
     doTestUpdate(
         Role.OWNER, Registrar::getFaxNumber, "+1.2345678900", Registrar.Builder::setFaxNumber);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_url() {
     doTestUpdate(Role.OWNER, Registrar::getUrl, "new-url.example", Registrar.Builder::setUrl);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_ipAddressAllowList() {
     doTestUpdate(
         Role.OWNER,
@@ -366,7 +373,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setIpAddressAllowList);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_clientCertificate() {
     clock.setTo(DateTime.parse("2020-11-02T00:00:00Z"));
     doTestUpdate(
@@ -376,7 +383,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         (builder, s) -> builder.setClientCertificate(s, clock.nowUtc()));
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_clientCertificateWithViolationsFails() {
     clock.setTo(DateTime.parse("2020-11-02T00:00:00Z"));
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -401,7 +408,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_clientCertificateWithMultipleViolationsFails() {
     clock.setTo(DateTime.parse("2055-11-01T00:00:00Z"));
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -426,7 +433,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_failoverClientCertificate() {
     clock.setTo(DateTime.parse("2020-11-02T00:00:00Z"));
     doTestUpdate(
@@ -436,7 +443,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         (builder, s) -> builder.setFailoverClientCertificate(s, clock.nowUtc()));
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_failoverClientCertificateWithViolationsAlreadyExistedSucceeds() {
     // TODO(sarahbot): remove this test after November 1, 2020.
 
@@ -469,7 +476,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_failoverClientCertificateWithViolationsFails() {
     clock.setTo(DateTime.parse("2020-11-02T00:00:00Z"));
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -494,7 +501,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_failoverClientCertificateWithMultipleViolationsFails() {
     clock.setTo(DateTime.parse("2055-11-01T00:00:00Z"));
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -519,7 +526,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_allowedTlds() {
     doTestUpdate(
         Role.ADMIN,
@@ -528,7 +535,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         (builder, s) -> builder.setAllowedTlds(s));
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_allowedTlds_failedWhenNoWhoisAbuseContactExists() {
     setUserAdmin();
     RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension);
@@ -551,7 +558,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_allowedTlds_failedWhenTldNotExist() {
     setUserAdmin();
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -573,7 +580,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_allowedTlds_failedWhenRemovingTld() {
     setUserAdmin();
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
@@ -595,7 +602,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertNoTasksEnqueued("sheet");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_allowedTlds_noChange_successWhenUserIsNotAdmin() {
     Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
     args.put("allowedTlds", ImmutableList.of("currenttld"));
@@ -615,7 +622,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_localizedAddress_city() {
     doTestUpdate(
         Role.OWNER,
@@ -624,7 +631,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setLocalizedAddress);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_localizedAddress_countryCode() {
     doTestUpdate(
         Role.OWNER,
@@ -633,7 +640,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setLocalizedAddress);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_localizedAddress_state() {
     doTestUpdate(
         Role.OWNER,
@@ -642,7 +649,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setLocalizedAddress);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_localizedAddress_street() {
     doTestUpdate(
         Role.OWNER,
@@ -655,7 +662,7 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
         Registrar.Builder::setLocalizedAddress);
   }
 
-  @Test
+  @TestOfyAndSql
   void testUpdate_localizedAddress_zip() {
     doTestUpdate(
         Role.OWNER,

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

@@ -75,9 +75,15 @@ public abstract class RegistrarSettingsActionTestCase {
 
   static final String CLIENT_ID = "TheRegistrar";
 
+  final FakeClock clock = new FakeClock(DateTime.parse("2014-01-01T00:00:00Z"));
+
   @RegisterExtension
   public final AppEngineExtension appEngine =
-      AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build();
+      AppEngineExtension.builder()
+          .withDatastoreAndCloudSql()
+          .withClock(clock)
+          .withTaskQueue()
+          .build();
 
   @RegisterExtension public final InjectExtension inject = new InjectExtension();
 
@@ -88,7 +94,6 @@ public abstract class RegistrarSettingsActionTestCase {
 
   final RegistrarSettingsAction action = new RegistrarSettingsAction();
   private final StringWriter writer = new StringWriter();
-  final FakeClock clock = new FakeClock(DateTime.parse("2014-01-01T00:00:00Z"));
 
   RegistrarContact techContact;