Don't send email notification when 0 uploads were attempted (#1058)

* Don't send email notification when 0 uploads were attempted

core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java

@@ -25,7 +25,7 @@ import static google.registry.batch.AsyncTaskMetrics.OperationType.DNS_REFRESH;
 import static google.registry.mapreduce.inputs.EppResourceInputs.createEntityInput;
 import static google.registry.model.EppResourceUtils.isActive;
 import static google.registry.model.EppResourceUtils.isDeleted;
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.DateTimeUtils.latestOf;
 import static java.util.concurrent.TimeUnit.DAYS;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -44,7 +44,6 @@ import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.flogger.FluentLogger;
-import com.googlecode.objectify.Key;
 import google.registry.batch.AsyncTaskMetrics.OperationResult;
 import google.registry.dns.DnsQueue;
 import google.registry.mapreduce.MapreduceRunner;
@@ -64,6 +63,7 @@ import google.registry.util.SystemClock;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.logging.Level;
 import javax.annotation.Nullable;
@@ -123,7 +123,7 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
     }
 
     ImmutableList.Builder<DnsRefreshRequest> requestsBuilder = new ImmutableList.Builder<>();
-    ImmutableList.Builder<Key<HostResource>> hostKeys = new ImmutableList.Builder<>();
+    ImmutableList.Builder<VKey<HostResource>> hostKeys = new ImmutableList.Builder<>();
     final List<DnsRefreshRequest> requestsToDelete = new ArrayList<>();
 
     for (TaskHandle task : tasks) {
@@ -204,10 +204,10 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
         emit(true, true);
         return;
       }
-      Key<HostResource> referencingHostKey = null;
+      VKey<HostResource> referencingHostKey = null;
       for (DnsRefreshRequest request : refreshRequests) {
         if (isActive(domain, request.lastUpdateTime())
-            && domain.getNameservers().contains(VKey.from(request.hostKey()))) {
+            && domain.getNameservers().contains(request.hostKey())) {
           referencingHostKey = request.hostKey();
           break;
         }
@@ -293,7 +293,8 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
 
     private static final long serialVersionUID = 1772812852271288622L;
 
-    abstract Key<HostResource> hostKey();
+    abstract VKey<HostResource> hostKey();
+
     abstract DateTime lastUpdateTime();
     abstract DateTime requestedTime();
     abstract boolean isRefreshNeeded();
@@ -301,7 +302,8 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
 
     @AutoValue.Builder
     abstract static class Builder {
-      abstract Builder setHostKey(Key<HostResource> hostKey);
+      abstract Builder setHostKey(VKey<HostResource> hostKey);
+
       abstract Builder setLastUpdateTime(DateTime lastUpdateTime);
       abstract Builder setRequestedTime(DateTime requestedTime);
       abstract Builder setIsRefreshNeeded(boolean isRefreshNeeded);
@@ -314,10 +316,12 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
      */
     static DnsRefreshRequest createFromTask(TaskHandle task, DateTime now) throws Exception {
       ImmutableMap<String, String> params = ImmutableMap.copyOf(task.extractParams());
-      Key<HostResource> hostKey =
-          Key.create(checkNotNull(params.get(PARAM_HOST_KEY), "Host to refresh not specified"));
+      VKey<HostResource> hostKey =
+          VKey.fromWebsafeKey(
+              checkNotNull(params.get(PARAM_HOST_KEY), "Host to refresh not specified"));
       HostResource host =
-          checkNotNull(ofy().load().key(hostKey).now(), "Host to refresh doesn't exist");
+          tm().transact(() -> tm().loadByKeyIfPresent(hostKey))
+              .orElseThrow(() -> new NoSuchElementException("Host to refresh doesn't exist"));
       boolean isHostDeleted =
           isDeleted(host, latestOf(now, host.getUpdateTimestamp().getTimestamp()));
       if (isHostDeleted) {

core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java

@@ -16,9 +16,10 @@ package google.registry.model.registry.label;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.common.collect.Streams;
+import google.registry.model.DatabaseMigrationUtils;
+import google.registry.model.common.DatabaseTransitionSchedule.TransitionId;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.PremiumList.PremiumListEntry;
 import google.registry.schema.tld.PremiumListSqlDao;
@@ -46,8 +47,7 @@ public class PremiumListDualDao {
    * or absent if no such list exists.
    */
   public static Optional<PremiumList> getLatestRevision(String premiumListName) {
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       return PremiumListDatastoreDao.getLatestRevision(premiumListName);
     } else {
       return PremiumListSqlDao.getLatestRevision(premiumListName);
@@ -68,16 +68,14 @@ public class PremiumListDualDao {
     }
     String premiumListName = registry.getPremiumList().getName();
     Optional<Money> primaryResult;
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       primaryResult =
           PremiumListDatastoreDao.getPremiumPrice(premiumListName, label, registry.getTldStr());
     } else {
       primaryResult = PremiumListSqlDao.getPremiumPrice(premiumListName, label);
     }
     // Also load the value from the secondary DB, compare the two results, and log if different.
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       suppressExceptionUnlessInTest(
           () -> {
             Optional<Money> secondaryResult =
@@ -120,8 +118,7 @@ public class PremiumListDualDao {
    */
   public static PremiumList save(String name, List<String> inputData) {
     PremiumList result;
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       result = PremiumListDatastoreDao.save(name, inputData);
       suppressExceptionUnlessInTest(
           () -> PremiumListSqlDao.save(name, inputData), "Error when saving premium list to SQL.");
@@ -141,8 +138,7 @@ public class PremiumListDualDao {
    * secondary database.
    */
   public static void delete(PremiumList premiumList) {
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       PremiumListDatastoreDao.delete(premiumList);
       suppressExceptionUnlessInTest(
           () -> PremiumListSqlDao.delete(premiumList),
@@ -159,8 +155,7 @@ public class PremiumListDualDao {
   public static boolean exists(String premiumListName) {
     // It may seem like overkill, but loading the list has ways been the way we check existence and
     // given that we usually load the list around the time we check existence, we'll hit the cache
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       return PremiumListDatastoreDao.getLatestRevision(premiumListName).isPresent();
     } else {
       return PremiumListSqlDao.getLatestRevision(premiumListName).isPresent();
@@ -179,8 +174,7 @@ public class PremiumListDualDao {
                 () ->
                     new IllegalArgumentException(
                         String.format("No premium list with name %s.", premiumListName)));
-    // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
-    if (tm().isOfy()) {
+    if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
       return PremiumListDatastoreDao.loadPremiumListEntriesUncached(premiumList);
     } else {
       CurrencyUnit currencyUnit = premiumList.getCurrency();

core/src/main/java/google/registry/model/server/Lock.java

@@ -15,19 +15,20 @@
 package google.registry.model.server;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static google.registry.model.ofy.ObjectifyService.ofy;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 import static google.registry.util.DateTimeUtils.isAtOrAfter;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.flogger.FluentLogger;
+import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
+import google.registry.persistence.VKey;
 import google.registry.schema.replay.DatastoreOnlyEntity;
 import google.registry.util.RequestStatusChecker;
 import google.registry.util.RequestStatusCheckerImpl;
@@ -190,12 +191,17 @@ public class Lock extends ImmutableObject implements DatastoreOnlyEntity, Serial
     // access to resources like GCS that can't be transactionally rolled back. Therefore, the lock
     // must be definitively acquired before it is used, even when called inside another transaction.
     AcquireResult acquireResult =
-        tm().transactNew(
+        ofyTm()
+            .transactNew(
                 () -> {
-                  DateTime now = tm().getTransactionTime();
+                  DateTime now = ofyTm().getTransactionTime();
 
                   // Checking if an unexpired lock still exists - if so, the lock can't be acquired.
-                  Lock lock = ofy().load().type(Lock.class).id(lockId).now();
+                  Lock lock =
+                      ofyTm()
+                          .loadByKeyIfPresent(
+                              VKey.createOfy(Lock.class, Key.create(Lock.class, lockId)))
+                          .orElse(null);
                   if (lock != null) {
                     logger.atInfo().log(
                         "Loaded existing lock: %s for request: %s", lock.lockId, lock.requestLogId);
@@ -218,7 +224,7 @@ public class Lock extends ImmutableObject implements DatastoreOnlyEntity, Serial
                   // Locks are not parented under an EntityGroupRoot (so as to avoid write
                   // contention) and
                   // don't need to be backed up.
-                  ofy().saveWithoutBackup().entity(newLock);
+                  ofyTm().putWithoutBackup(newLock);
 
                   return AcquireResult.create(now, lock, newLock, lockState);
                 });
@@ -231,21 +237,26 @@ public class Lock extends ImmutableObject implements DatastoreOnlyEntity, Serial
   /** Release the lock. */
   public void release() {
     // Just use the default clock because we aren't actually doing anything that will use the clock.
-    tm().transact(
+    ofyTm()
+        .transact(
             () -> {
               // To release a lock, check that no one else has already obtained it and if not
               // delete it. If the lock in Datastore was different then this lock is gone already;
               // this can happen if release() is called around the expiration time and the lock
               // expires underneath us.
-              Lock loadedLock = ofy().load().type(Lock.class).id(lockId).now();
+              Lock loadedLock =
+                  ofyTm()
+                      .loadByKeyIfPresent(
+                          VKey.createOfy(Lock.class, Key.create(Lock.class, lockId)))
+                      .orElse(null);
               if (Lock.this.equals(loadedLock)) {
                 // Use noBackupOfy() so that we don't create a commit log entry for deleting the
                 // lock.
                 logger.atInfo().log("Deleting lock: %s", lockId);
-                ofy().deleteWithoutBackup().entity(Lock.this);
+                ofyTm().deleteWithoutBackup(Lock.this);
 
                 lockMetrics.recordRelease(
-                    resourceName, tld, new Duration(acquiredTime, tm().getTransactionTime()));
+                    resourceName, tld, new Duration(acquiredTime, ofyTm().getTransactionTime()));
               } else {
                 logger.atSevere().log(
                     "The lock we acquired was transferred to someone else before we"

core/src/main/java/google/registry/persistence/VKey.java

@@ -223,4 +223,14 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   public static <T> VKey<T> from(Key<T> key) {
     return VKeyTranslatorFactory.createVKey(key);
   }
+
+  /**
+   * Construct a VKey from the string representation of an ofy key.
+   *
+   * <p>TODO(b/184350590): After migration, we'll want remove the ofy key dependency from this.
+   */
+  @Nullable
+  public static <T> VKey<T> fromWebsafeKey(String ofyKeyRepr) {
+    return from(Key.create(ofyKeyRepr));
+  }
 }

core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java

@@ -278,9 +278,15 @@ public final class IcannReportingUploadAction implements Runnable {
   }
 
   private void emailUploadResults(ImmutableMap<String, Boolean> reportSummary) {
-    String subject = String.format(
-        "ICANN Monthly report upload summary: %d/%d succeeded",
-        reportSummary.values().stream().filter((b) -> b).count(), reportSummary.size());
+    if (reportSummary.size() == 0) {
+      logger.atInfo().log("No uploads were attempted today; skipping notification email.");
+      return;
+    }
+    String subject =
+        String.format(
+            "ICANN Monthly report upload summary: %d/%d succeeded",
+            // This filter() does in fact do something: It counts only the trues.
+            reportSummary.values().stream().filter((b) -> b).count(), reportSummary.size());
     String body =
         String.format(
             "Report Filename - Upload status:\n%s",

core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java

@@ -19,7 +19,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME;
 import static google.registry.batch.AsyncTaskMetrics.OperationType.DNS_REFRESH;
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.newDomainBase;
 import static google.registry.testing.DatabaseHelper.newHostResource;
@@ -47,11 +47,14 @@ import google.registry.batch.AsyncTaskMetrics.OperationResult;
 import google.registry.batch.RefreshDnsOnHostRenameAction.RefreshDnsOnHostRenameReducer;
 import google.registry.model.host.HostResource;
 import google.registry.model.server.Lock;
+import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.FakeSleeper;
 import google.registry.testing.InjectExtension;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
+import google.registry.testing.TestOfyAndSql;
+import google.registry.testing.TestOfyOnly;
 import google.registry.testing.mapreduce.MapreduceTestCase;
 import google.registry.util.AppEngineServiceUtils;
 import google.registry.util.RequestStatusChecker;
@@ -62,11 +65,11 @@ import java.util.Optional;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.Mock;
 
 /** Unit tests for {@link RefreshDnsOnHostRenameAction}. */
+@DualDatabaseTest
 public class RefreshDnsOnHostRenameActionTest
     extends MapreduceTestCase<RefreshDnsOnHostRenameAction> {
 
@@ -109,7 +112,7 @@ public class RefreshDnsOnHostRenameActionTest
     executeTasksUntilEmpty("mapreduce", clock);
     sleeper.sleep(millis(50));
     clock.advanceBy(standardSeconds(5));
-    ofy().clearSessionCache();
+    tm().clearSessionCache();
   }
 
   /** Kicks off, but does not run, the mapreduce tasks. Useful for testing validation/setup. */
@@ -117,10 +120,13 @@ public class RefreshDnsOnHostRenameActionTest
     clock.advanceOneMilli();
     action.run();
     clock.advanceBy(standardSeconds(5));
-    ofy().clearSessionCache();
+    tm().clearSessionCache();
   }
 
-  @Test
+  // TODO(b/181662306) None of the map reduce tests work with SQL since our map-reduce setup is
+  // inherently Datastore oriented, but this is a bigger task.
+
+  @TestOfyOnly
   void testSuccess_dnsUpdateEnqueued() throws Exception {
     HostResource host = persistActiveHost("ns1.example.tld");
     persistResource(newDomainBase("example.tld", host));
@@ -137,7 +143,7 @@ public class RefreshDnsOnHostRenameActionTest
     verifyNoMoreInteractions(action.asyncTaskMetrics);
   }
 
-  @Test
+  @TestOfyOnly
   void testSuccess_multipleHostsProcessedInBatch() throws Exception {
     HostResource host1 = persistActiveHost("ns1.example.tld");
     HostResource host2 = persistActiveHost("ns2.example.tld");
@@ -161,7 +167,7 @@ public class RefreshDnsOnHostRenameActionTest
     verifyNoMoreInteractions(action.asyncTaskMetrics);
   }
 
-  @Test
+  @TestOfyOnly
   void testSuccess_deletedHost_doesntTriggerDnsRefresh() throws Exception {
     HostResource host = persistDeletedHost("ns11.fakesss.tld", clock.nowUtc().minusDays(4));
     persistResource(newDomainBase("example1.tld", host));
@@ -176,7 +182,7 @@ public class RefreshDnsOnHostRenameActionTest
     verifyNoMoreInteractions(action.asyncTaskMetrics);
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_noDnsTasksForDeletedDomain() throws Exception {
     HostResource renamedHost = persistActiveHost("ns1.example.tld");
     persistResource(
@@ -190,7 +196,7 @@ public class RefreshDnsOnHostRenameActionTest
     assertNoTasksEnqueued(QUEUE_ASYNC_HOST_RENAME);
   }
 
-  @Test
+  @TestOfyAndSql
   void testRun_hostDoesntExist_delaysTask() {
     HostResource host = newHostResource("ns1.example.tld");
     enqueuer.enqueueAsyncDnsRefresh(host, clock.nowUtc());
@@ -204,7 +210,7 @@ public class RefreshDnsOnHostRenameActionTest
     assertThat(acquireLock()).isPresent();
   }
 
-  @Test
+  @TestOfyAndSql
   void test_cannotAcquireLock() {
     // Make lock acquisition fail.
     acquireLock();
@@ -213,7 +219,7 @@ public class RefreshDnsOnHostRenameActionTest
     assertNoDnsTasksEnqueued();
   }
 
-  @Test
+  @TestOfyAndSql
   void test_mapreduceHasWorkToDo_lockIsAcquired() {
     HostResource host = persistActiveHost("ns1.example.tld");
     enqueuer.enqueueAsyncDnsRefresh(host, clock.nowUtc());
@@ -221,7 +227,7 @@ public class RefreshDnsOnHostRenameActionTest
     assertThat(acquireLock()).isEmpty();
   }
 
-  @Test
+  @TestOfyAndSql
   void test_noTasksToLease_releasesLockImmediately() {
     enqueueMapreduceOnly();
     assertNoDnsTasksEnqueued();

core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java

@@ -15,35 +15,40 @@
 package google.registry.model.registry.label;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.newRegistry;
 import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.Duration.standardDays;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.truth.Truth8;
 import google.registry.dns.writer.VoidDnsWriter;
+import google.registry.model.EntityTestCase;
+import google.registry.model.common.DatabaseTransitionSchedule;
+import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabase;
+import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabaseTransition;
+import google.registry.model.common.DatabaseTransitionSchedule.TransitionId;
+import google.registry.model.common.TimedTransitionProperty;
 import google.registry.model.pricing.StaticPremiumListPricingEngine;
 import google.registry.model.registry.Registry;
 import google.registry.schema.tld.PremiumListSqlDao;
-import google.registry.testing.AppEngineExtension;
 import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.TestCacheExtension;
 import google.registry.testing.TestOfyAndSql;
-import google.registry.testing.TestOfyOnly;
-import google.registry.testing.TestSqlOnly;
+import org.joda.time.DateTime;
+import org.joda.time.Duration;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 /** Unit tests for {@link PremiumListDualDao}. */
 @DualDatabaseTest
-public class PremiumListDualDaoTest {
-
-  @RegisterExtension
-  public final AppEngineExtension appEngine =
-      AppEngineExtension.builder().withDatastoreAndCloudSql().build();
+public class PremiumListDualDaoTest extends EntityTestCase {
 
   // Set long persist times on caches so they can be tested (cache times default to 0 in tests).
   @RegisterExtension
@@ -56,9 +61,23 @@ public class PremiumListDualDaoTest {
   @BeforeEach
   void before() {
     createTld("tld");
+
+    fakeClock.setTo(DateTime.parse("1984-12-21T00:00:00.000Z"));
+    DatabaseTransitionSchedule schedule =
+        DatabaseTransitionSchedule.create(
+            TransitionId.DOMAIN_LABEL_LISTS,
+            TimedTransitionProperty.fromValueMap(
+                ImmutableSortedMap.of(
+                    START_OF_TIME,
+                    PrimaryDatabase.DATASTORE,
+                    fakeClock.nowUtc().plusDays(1),
+                    PrimaryDatabase.CLOUD_SQL),
+                PrimaryDatabaseTransition.class));
+
+    tm().transactNew(() -> ofyTm().putWithoutBackup(schedule));
   }
 
-  @TestOfyOnly
+  @TestOfyAndSql
   void testGetPremiumPrice_secondaryLoadMissingSql() {
     PremiumListSqlDao.delete(PremiumListSqlDao.getLatestRevision("tld").get());
     assertThat(
@@ -71,8 +90,9 @@ public class PremiumListDualDaoTest {
                 + "(Optional[USD 20.00]) and secondary SQL db (Optional.empty).");
   }
 
-  @TestSqlOnly
+  @TestOfyAndSql
   void testGetPremiumPrice_secondaryLoadMissingOfy() {
+    fakeClock.advanceBy(Duration.standardDays(5));
     PremiumList premiumList = PremiumListDatastoreDao.getLatestRevision("tld").get();
     PremiumListDatastoreDao.delete(premiumList);
     assertThat(
@@ -85,7 +105,7 @@ public class PremiumListDualDaoTest {
                 + "and secondary Datastore db (Optional.empty).");
   }
 
-  @TestOfyOnly
+  @TestOfyAndSql
   void testGetPremiumPrice_secondaryDifferentSql() {
     PremiumListSqlDao.save("tld", ImmutableList.of("brass,USD 50"));
     assertThat(
@@ -98,8 +118,9 @@ public class PremiumListDualDaoTest {
                 + "(Optional[USD 20.00]) and secondary SQL db (Optional[USD 50.00]).");
   }
 
-  @TestSqlOnly
+  @TestOfyAndSql
   void testGetPremiumPrice_secondaryDifferentOfy() {
+    fakeClock.advanceBy(Duration.standardDays(5));
     PremiumListDatastoreDao.save("tld", ImmutableList.of("brass,USD 50"));
     assertThat(
             assertThrows(

core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java

@@ -210,13 +210,7 @@ class IcannReportingUploadActionTest {
     action.run();
     tm().clearSessionCache();
     verifyNoMoreInteractions(mockReporter);
-    verify(emailService)
-        .sendEmail(
-            EmailMessage.create(
-                "ICANN Monthly report upload summary: 0/0 succeeded",
-                "Report Filename - Upload status:\n",
-                new InternetAddress("recipient@example.com"),
-                new InternetAddress("sender@example.com")));
+    verifyNoMoreInteractions(emailService);
   }
 
   @TestOfyAndSql

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

@@ -123,7 +123,7 @@ class DualDatabaseTestInvocationContextProvider implements TestTemplateInvocatio
       }
       fieldBuilder.addAll(
           Stream.of(clazz.getDeclaredFields())
-              .filter(field -> field.getType().isAssignableFrom(AppEngineExtension.class))
+              .filter(field -> AppEngineExtension.class.isAssignableFrom(field.getType()))
               .collect(toImmutableList()));
       return fieldBuilder.build();
     }

core/src/test/java/google/registry/tools/server/ListPremiumListsActionTest.java

@@ -69,7 +69,7 @@ class ListPremiumListsActionTest extends ListActionTestCase {
         Optional.empty(),
         "^name\\s+labelsToPrices\\s*$",
         "^-+\\s+-+\\s*$",
-        "^how\\s+\\{richer=5000\\.00\\}$",
+        "^how\\s+\\{richer=5000\\}\\s+$",
         "^xn--q9jyb4c\\s+\\{rich=100\\.00\\}\\s+$");
   }
 

db/build.gradle

@@ -1,3 +1,5 @@
+import org.gradle.process.internal.ExecException
+
 // Copyright 2019 The Nomulus Authors. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -40,7 +42,7 @@ ext {
   }
 
   getSocketFactoryAccessInfo = { env ->
-    def cred = getCloudSqlCredential(env, 'admin').split(' ')
+    def cred = getCloudSqlCredential(env).split(' ')
     def sqlInstance = cred[0]
     println "Database set to Cloud SQL instance ${sqlInstance}."
     return [
@@ -65,26 +67,25 @@ ext {
     }
   }
 
-  // Retrieves Cloud SQL credential for a given role. Result is in the form of
-  // 'instancename username password'.
+  // Retrieves the Cloud SQL credential for the schema deployer. Result is in
+  // the form of 'instancename username password'.
   //
   // The env parameter may be one of the following: alpha, crash, sandbox, or
-  // production. The role parameter may be superuser. (More roles will be added
-  // later).
-  getCloudSqlCredential = { env, role ->
-    def devProject = rootProject.devProject
-    def gcpProject = rootProject.projects[env]
-    def keyProject = env in restrictedDbEnv? devProject : gcpProject
+  // production.
+  //
+  // User must make sure that the nomulus tool can be found on PATH. An alias
+  // will not work.
+  getCloudSqlCredential = { env ->
+    try {
+      execInBash('which nomulus', '/tmp')
+    } catch (ExecException e) {
+      throw new IllegalStateException(
+          'nomulus not found. Make sure it is on PATH, not just an alias.')
+    }
     def command =
-        """gsutil cp \
-           gs://${gcpProject}-beam/cloudsql/${role}_credential.enc - | \
-           base64 -d | \
-           gcloud kms decrypt --location global --keyring nomulus-tool-keyring \
-           --key nomulus-tool-key --plaintext-file=- \
-           --ciphertext-file=- \
-           --project=${keyProject}"""
+        "nomulus -e ${env} get_sql_credential --user schema_deployer"
 
-    return execInBash(command, '/tmp')
+    return execInBash(command, project.rootDir)
   }
 }
 

release/cloudbuild-schema-deploy.yaml

@@ -27,9 +27,9 @@
 # See https://github.com/spinnaker/spinnaker/issues/3028 for more information.
 steps:
 # Download and decrypt the nomulus tool credential, which has the privilege to
-# start Cloud SQL proxy to all environments.
-# Also download and decrypt the admin_credential file, which has the cloud
-# instance name and database login name and password.
+# start Cloud SQL proxy to all environments. This credential is also used to
+# authenticate the nomulus tool when fetching the schema deployer credential in
+# the next step.
 - name: 'gcr.io/$PROJECT_ID/builder:latest'
   volumes:
   - name: 'secrets'
@@ -45,13 +45,21 @@ steps:
         --ciphertext-file=- \
         --plaintext-file=/secrets/cloud_sql_credential.json \
         --location=global --keyring=nomulus-tool-keyring --key=nomulus-tool-key
-    gsutil cp gs://$PROJECT_ID-deploy/cloudsql-credentials/${_ENV}/admin_credential.enc - \
-      | base64 -d \
-      | gcloud kms decrypt \
-        --ciphertext-file=- \
-        --plaintext-file=/secrets/admin_credential.dec \
-        --location global --keyring=nomulus-tool-keyring \
-        --key=nomulus-tool-key
+# Fetch the Cloud SQL credential for schema_deployer
+- name: 'gcr.io/$PROJECT_ID/nomulus-tool:latest'
+  volumes:
+  - name: 'secrets'
+    path: '/secrets'
+  args:
+  - -e
+  - ${_ENV}
+  - --credential
+  - /secrets/cloud_sql_credential.json
+  - get_sql_credential
+  - --user
+  - schema_deployer
+  - --output
+  - /secrets/schema_deployer_credential.dec
 # Download the schema jar to be deployed.
 - name: 'gcr.io/$PROJECT_ID/builder:latest'
   volumes:

release/schema-deployer/deploy_sql_schema.sh

@@ -18,19 +18,19 @@
 # - /flyway/jars: the schema jar to be deployed.
 #
 # Database login info may be passed in two ways:
-# - Decrypt the admin_credential.enc file on GCS and map it as
-#   /secrets/admin_credential.dec
-# - Provide the content of the admin_credential as command line arguments
+# - Save it in the format of "cloud_sql_instance login password" in a file and
+#   map the file as /secrets/schema_deployer_credential.dec
+# - Provide the content of the credential as command line arguments
 
 set -e
 if [ "$#" -le 1 ]; then
-  if [ ! -f /secrets/admin_credential.dec ]; then
-    echo "Missing /secrets/admin_credential.dec"
+  if [ ! -f /secrets/schema_deployer_credential.dec ]; then
+    echo "Missing /secrets/schema_deployer_credential.dec"
     exit 1
   fi
-  cloud_sql_instance=$(cut -d' ' -f1 /secrets/admin_credential.dec)
-  db_user=$(cut -d' ' -f2 /secrets/admin_credential.dec)
-  db_password=$(cut -d' ' -f3 /secrets/admin_credential.dec)
+  cloud_sql_instance=$(cut -d' ' -f1 /secrets/schema_deployer_credential.dec)
+  db_user=$(cut -d' ' -f2 /secrets/schema_deployer_credential.dec)
+  db_password=$(cut -d' ' -f3 /secrets/schema_deployer_credential.dec)
   flyway_action=${1:-validate}
 elif [ "$#" -ge 3 ]; then
   cloud_sql_instance=$1

vnames.json

@@ -1,6 +1,6 @@
 [
   {
-    "pattern": "(build/[^/]+)/(.*)",
+    "pattern": "([^/]+/build/[^/]+)/(.*)",
     "vname": {
       "corpus": "github.com/google/nomulus",
       "path": "@2@",