Add a 'Host' parameter to the relock action enqueuer (#699)

* Add a 'Host' parameter to the relock action enqueuer

I believe this is why we are seeing 404s currently -- we should be
specifying the backend host as the target like we do for the
resave-entity async action.

.gitignore

@@ -79,6 +79,10 @@ nomulus.iml
 nomulus.ipr
 nomulus.iws
 
+# Auto-generated java classes by Intellij
+*/src/main/generated/
+*/src/test/generated_tests/
+
 # VScode
 .vscode
 

build.gradle

@@ -25,7 +25,7 @@ buildscript {
 
   dependencies {
     classpath 'com.google.cloud.tools:appengine-gradle-plugin:2.0.1'
-    classpath "net.ltgt.gradle:gradle-errorprone-plugin:0.6.1"
+    classpath 'net.ltgt.gradle:gradle-errorprone-plugin:0.6.1'
     classpath 'org.sonatype.aether:aether-api:1.13.1'
     classpath 'org.sonatype.aether:aether-impl:1.13.1'
   }
@@ -49,6 +49,7 @@ plugins {
   id 'com.diffplug.gradle.spotless' version '3.25.0'
 
   id 'jacoco'
+  id 'com.dorongold.task-tree' version '1.5'
 }
 
 wrapper {
@@ -443,6 +444,7 @@ task javaIncrementalFormatDryRun {
     println("${invokeJavaDiffFormatScript("show")}")
   }
 }
+tasks.build.dependsOn(tasks.javaIncrementalFormatCheck)
 
 // Checks if modified lines in Java source files need reformatting.
 // Note that this task processes modified Java files in the entire repository.
@@ -452,8 +454,6 @@ task javaIncrementalFormatApply {
   }
 }
 
-tasks.build.dependsOn(tasks.javaIncrementalFormatCheck)
-
 task javadoc(type: Javadoc) {
   source javadocSource
   classpath = files(javadocClasspath)
@@ -468,4 +468,16 @@ task javadoc(type: Javadoc) {
 
 tasks.build.dependsOn(tasks.javadoc)
 
+// Task for doing development on core Nomulus.
+// This fixes code formatting automatically as necessary, builds and tests the
+// core Nomulus codebase, and runs all presubmits.
+task coreDev {
+  dependsOn 'javaIncrementalFormatApply'
+  dependsOn 'javadoc'
+  dependsOn 'checkDependenciesDotGradle'
+  dependsOn 'checkLicense'
+  dependsOn ':core:check'
+  dependsOn 'assemble'
+}
+
 javadocDependentTasks.each { tasks.javadoc.dependsOn(it) }

buildSrc/gradle/dependency-locks/testCompile.lockfile

@@ -61,12 +61,12 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.hamcrest:hamcrest-core:1.3
 org.json:json:20160212
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.mockito:mockito-core:3.3.3
 org.objenesis:objenesis:2.6
 org.opentest4j:opentest4j:1.2.0

buildSrc/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -61,12 +61,12 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.hamcrest:hamcrest-core:1.3
 org.json:json:20160212
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.mockito:mockito-core:3.3.3
 org.objenesis:objenesis:2.6
 org.opentest4j:opentest4j:1.2.0

buildSrc/gradle/dependency-locks/testRuntime.lockfile

@@ -61,12 +61,12 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.hamcrest:hamcrest-core:1.3
 org.json:json:20160212
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.mockito:mockito-core:3.3.3
 org.objenesis:objenesis:2.6
 org.opentest4j:opentest4j:1.2.0

buildSrc/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -61,12 +61,12 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.hamcrest:hamcrest-core:1.3
 org.json:json:20160212
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.mockito:mockito-core:3.3.3
 org.objenesis:objenesis:2.6
 org.opentest4j:opentest4j:1.2.0

common/gradle/dependency-locks/testCompile.lockfile

@@ -20,10 +20,10 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.eclipse.jgit:org.eclipse.jgit:4.4.1.201607150455-r
 org.hamcrest:hamcrest-core:1.3
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.opentest4j:opentest4j:1.2.0

common/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -20,10 +20,10 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.eclipse.jgit:org.eclipse.jgit:4.4.1.201607150455-r
 org.hamcrest:hamcrest-core:1.3
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.opentest4j:opentest4j:1.2.0

common/gradle/dependency-locks/testRuntime.lockfile

@@ -21,10 +21,10 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.eclipse.jgit:org.eclipse.jgit:4.4.1.201607150455-r
 org.hamcrest:hamcrest-core:1.3
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.opentest4j:opentest4j:1.2.0

common/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -21,10 +21,10 @@ org.checkerframework:checker-compat-qual:2.5.5
 org.checkerframework:checker-qual:2.11.1
 org.eclipse.jgit:org.eclipse.jgit:4.4.1.201607150455-r
 org.hamcrest:hamcrest-core:1.3
-org.junit.jupiter:junit-jupiter-api:5.6.1
-org.junit.jupiter:junit-jupiter-engine:5.6.1
-org.junit.platform:junit-platform-commons:1.6.1
-org.junit.platform:junit-platform-engine:1.6.1
-org.junit.vintage:junit-vintage-engine:5.6.1
-org.junit:junit-bom:5.6.1
+org.junit.jupiter:junit-jupiter-api:5.6.2
+org.junit.jupiter:junit-jupiter-engine:5.6.2
+org.junit.platform:junit-platform-commons:1.6.2
+org.junit.platform:junit-platform-engine:1.6.2
+org.junit.vintage:junit-vintage-engine:5.6.2
+org.junit:junit-bom:5.6.2
 org.opentest4j:opentest4j:1.2.0

config/presubmits.py

@@ -77,9 +77,9 @@ PRESUBMITS = {
     PresubmitCheck(
         r".*Copyright 20\d{2} The Nomulus Authors\. All Rights Reserved\.",
         ("java", "js", "soy", "sql", "py", "sh", "gradle"), {
-            ".git", "/build/", "/generated/", "node_modules/",
-            "JUnitBackports.java", "registrar_bin.", "registrar_dbg.",
-            "google-java-format-diff.py",
+            ".git", "/build/", "/generated/", "/generated_tests/",
+            "node_modules/", "JUnitBackports.java", "registrar_bin.",
+            "registrar_dbg.", "google-java-format-diff.py",
             "nomulus.golden.sql", "soyutils_usegoog.js"
         }, REQUIRED):
         "File did not include the license header.",

core/build.gradle

@@ -312,6 +312,7 @@ dependencies {
   testCompile deps['org.junit.jupiter:junit-jupiter-api']
   testCompile deps['org.junit.jupiter:junit-jupiter-engine']
   testCompile deps['org.junit.jupiter:junit-jupiter-migrationsupport']
+  testCompile deps['org.junit.jupiter:junit-jupiter-params']
   testCompile deps['org.junit.platform:junit-platform-runner']
   testCompile deps['org.junit.platform:junit-platform-suite-api']
   testCompile deps['org.junit.vintage:junit-vintage-engine']

core/gradle/dependency-locks/testCompile.lockfile

@@ -246,6 +246,7 @@ org.json:json:20160810
 org.junit.jupiter:junit-jupiter-api:5.6.2
 org.junit.jupiter:junit-jupiter-engine:5.6.2
 org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
+org.junit.jupiter:junit-jupiter-params:5.6.2
 org.junit.platform:junit-platform-commons:1.6.2
 org.junit.platform:junit-platform-engine:1.6.2
 org.junit.platform:junit-platform-launcher:1.6.2

core/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -244,6 +244,7 @@ org.json:json:20160810
 org.junit.jupiter:junit-jupiter-api:5.6.2
 org.junit.jupiter:junit-jupiter-engine:5.6.2
 org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
+org.junit.jupiter:junit-jupiter-params:5.6.2
 org.junit.platform:junit-platform-commons:1.6.2
 org.junit.platform:junit-platform-engine:1.6.2
 org.junit.platform:junit-platform-launcher:1.6.2

core/gradle/dependency-locks/testRuntime.lockfile

@@ -249,6 +249,7 @@ org.json:json:20160810
 org.junit.jupiter:junit-jupiter-api:5.6.2
 org.junit.jupiter:junit-jupiter-engine:5.6.2
 org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
+org.junit.jupiter:junit-jupiter-params:5.6.2
 org.junit.platform:junit-platform-commons:1.6.2
 org.junit.platform:junit-platform-engine:1.6.2
 org.junit.platform:junit-platform-launcher:1.6.2

core/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -249,6 +249,7 @@ org.json:json:20160810
 org.junit.jupiter:junit-jupiter-api:5.6.2
 org.junit.jupiter:junit-jupiter-engine:5.6.2
 org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
+org.junit.jupiter:junit-jupiter-params:5.6.2
 org.junit.platform:junit-platform-commons:1.6.2
 org.junit.platform:junit-platform-engine:1.6.2
 org.junit.platform:junit-platform-launcher:1.6.2

core/src/main/java/google/registry/backup/AppEngineEnvironment.java

@@ -23,7 +23,8 @@ import java.lang.reflect.Proxy;
 
 /**
  * Sets up a placeholder {@link Environment} on a non-AppEngine platform so that Datastore Entities
- * can be deserialized. See {@code DatastoreEntityExtension} in test source for more information.
+ * can be converted from/to Objectify entities. See {@code DatastoreEntityExtension} in test source
+ * for more information.
  */
 public class AppEngineEnvironment implements Closeable {
 

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

@@ -169,10 +169,12 @@ public final class AsyncTaskEnqueuer {
         lock.getRelockDuration().isPresent(),
         "Lock with ID %s not configured for relock",
         lock.getRevisionId());
+    String backendHostname = appEngineServiceUtils.getServiceHostname("backend");
     addTaskToQueueWithRetry(
         asyncActionsPushQueue,
         TaskOptions.Builder.withUrl(RelockDomainAction.PATH)
             .method(Method.POST)
+            .header("Host", backendHostname)
             .param(
                 RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
                 String.valueOf(lock.getRevisionId()))

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

@@ -532,7 +532,7 @@ public class DeleteContactsAndHostsAction implements Runnable {
           resource.getClass().getSimpleName());
       return new AutoValue_DeleteContactsAndHostsAction_DeletionRequest.Builder()
           .setKey(resourceKey)
-          .setLastUpdateTime(resource.getUpdateAutoTimestamp().getTimestamp())
+          .setLastUpdateTime(resource.getUpdateTimestamp().getTimestamp())
           .setRequestingClientId(
               checkNotNull(
                   params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified"))

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

@@ -319,13 +319,13 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
       HostResource host =
           checkNotNull(ofy().load().key(hostKey).now(), "Host to refresh doesn't exist");
       boolean isHostDeleted =
-          isDeleted(host, latestOf(now, host.getUpdateAutoTimestamp().getTimestamp()));
+          isDeleted(host, latestOf(now, host.getUpdateTimestamp().getTimestamp()));
       if (isHostDeleted) {
         logger.atInfo().log("Host %s is already deleted, not refreshing DNS.", hostKey);
       }
       return new AutoValue_RefreshDnsOnHostRenameAction_DnsRefreshRequest.Builder()
           .setHostKey(hostKey)
-          .setLastUpdateTime(host.getUpdateAutoTimestamp().getTimestamp())
+          .setLastUpdateTime(host.getUpdateTimestamp().getTimestamp())
           .setRequestedTime(
               DateTime.parse(
                   checkNotNull(params.get(PARAM_REQUESTED_TIME), "Requested time not specified")))

core/src/main/java/google/registry/beam/initsql/BeamJpaModule.java

@@ -19,31 +19,26 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
 
 import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableList;
-import dagger.Binds;
 import dagger.Component;
 import dagger.Lazy;
 import dagger.Module;
 import dagger.Provides;
-import google.registry.beam.initsql.BeamJpaModule.BindModule;
 import google.registry.config.CredentialModule;
 import google.registry.config.RegistryConfig.Config;
+import google.registry.config.RegistryConfig.ConfigModule;
 import google.registry.keyring.kms.KmsModule;
 import google.registry.persistence.PersistenceModule;
 import google.registry.persistence.PersistenceModule.JdbcJpaTm;
 import google.registry.persistence.PersistenceModule.SocketFactoryJpaTm;
 import google.registry.persistence.transaction.JpaTransactionManager;
-import google.registry.util.Clock;
-import google.registry.util.Sleeper;
-import google.registry.util.SystemClock;
-import google.registry.util.SystemSleeper;
+import google.registry.util.UtilsModule;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.channels.Channels;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
-import javax.inject.Named;
+import javax.annotation.Nullable;
 import javax.inject.Singleton;
 import org.apache.beam.sdk.io.FileSystems;
 import org.apache.beam.sdk.io.fs.ResourceId;
@@ -53,27 +48,31 @@ import org.apache.beam.sdk.io.fs.ResourceId;
  *
  * <p>This module is intended for use in BEAM pipelines, and uses a BEAM utility to access GCS like
  * a regular file system.
- *
- * <p>Note that {@link google.registry.config.RegistryConfig.ConfigModule} cannot be used here,
- * since many bindings, especially KMS-related ones, are different.
  */
-@Module(includes = {BindModule.class})
-class BeamJpaModule {
+@Module
+public class BeamJpaModule {
 
   private static final String GCS_SCHEME = "gs://";
 
-  private final String credentialFilePath;
+  @Nullable private final String credentialFilePath;
 
   /**
    * Constructs a new instance of {@link BeamJpaModule}.
    *
+   * <p>Note: it is an unfortunately necessary antipattern to check for the validity of
+   * credentialFilePath in {@link #provideCloudSqlAccessInfo} rather than in the constructor.
+   * Unfortunately, this is a restriction imposed upon us by Dagger. Specifically, because we use
+   * this in at least one 1 {@link google.registry.tools.RegistryTool} command(s), it must be
+   * instantiated in {@code google.registry.tools.RegistryToolComponent} for all possible commands;
+   * Dagger doesn't permit it to ever be null. For the vast majority of commands, it will never be
+   * used (so a null credential file path is fine in those cases).
+   *
    * @param credentialFilePath the path to a Cloud SQL credential file. This must refer to either a
    *     real encrypted file on GCS as returned by {@link
    *     BackupPaths#getCloudSQLCredentialFilePatterns} or an unencrypted file on local filesystem
    *     with credentials to a test database.
    */
-  BeamJpaModule(String credentialFilePath) {
-    checkArgument(!isNullOrEmpty(credentialFilePath), "Null or empty credentialFilePath");
+  public BeamJpaModule(@Nullable String credentialFilePath) {
     this.credentialFilePath = credentialFilePath;
   }
 
@@ -85,6 +84,7 @@ class BeamJpaModule {
   @Provides
   @Singleton
   SqlAccessInfo provideCloudSqlAccessInfo(Lazy<CloudSqlCredentialDecryptor> lazyDecryptor) {
+    checkArgument(!isNullOrEmpty(credentialFilePath), "Null or empty credentialFilePath");
     String line = readOnlyLineFromCredentialFile();
     if (isCloudSqlCredential()) {
       line = lazyDecryptor.get().decrypt(line);
@@ -114,13 +114,13 @@ class BeamJpaModule {
   }
 
   @Provides
-  @Config("cloudSqlJdbcUrl")
+  @Config("beamCloudSqlJdbcUrl")
   String provideJdbcUrl(SqlAccessInfo sqlAccessInfo) {
     return sqlAccessInfo.jdbcUrl();
   }
 
   @Provides
-  @Config("cloudSqlInstanceConnectionName")
+  @Config("beamCloudSqlInstanceConnectionName")
   String provideSqlInstanceName(SqlAccessInfo sqlAccessInfo) {
     return sqlAccessInfo
         .cloudSqlInstanceName()
@@ -128,58 +128,45 @@ class BeamJpaModule {
   }
 
   @Provides
-  @Config("cloudSqlUsername")
+  @Config("beamCloudSqlUsername")
   String provideSqlUsername(SqlAccessInfo sqlAccessInfo) {
     return sqlAccessInfo.user();
   }
 
   @Provides
-  @Config("cloudSqlPassword")
+  @Config("beamCloudSqlPassword")
   String provideSqlPassword(SqlAccessInfo sqlAccessInfo) {
     return sqlAccessInfo.password();
   }
 
   @Provides
-  @Config("cloudKmsProjectId")
+  @Config("beamCloudKmsProjectId")
   static String kmsProjectId() {
     return "domain-registry-dev";
   }
 
   @Provides
-  @Config("cloudKmsKeyRing")
+  @Config("beamCloudKmsKeyRing")
   static String keyRingName() {
     return "nomulus-tool-keyring";
   }
 
   @Provides
-  @Config("defaultCredentialOauthScopes")
-  static ImmutableList<String> defaultCredentialOauthScopes() {
-    return ImmutableList.of("https://www.googleapis.com/auth/cloud-platform");
-  }
-
-  @Provides
-  @Named("transientFailureRetries")
-  static int transientFailureRetries() {
-    return 12;
-  }
-
-  @Module
-  interface BindModule {
-
-    @Binds
-    Sleeper sleeper(SystemSleeper sleeper);
-
-    @Binds
-    Clock clock(SystemClock clock);
+  @Config("beamHibernateHikariMaximumPoolSize")
+  static int getBeamHibernateHikariMaximumPoolSize() {
+    // TODO(weiminyu): make this configurable. Should be equal to number of cores.
+    return 4;
   }
 
   @Singleton
   @Component(
       modules = {
+        ConfigModule.class,
         CredentialModule.class,
         BeamJpaModule.class,
         KmsModule.class,
-        PersistenceModule.class
+        PersistenceModule.class,
+        UtilsModule.class
       })
   public interface JpaTransactionManagerComponent {
     @SocketFactoryJpaTm

core/src/main/java/google/registry/beam/initsql/CloudSqlCredentialDecryptor.java

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.api.services.cloudkms.v1.model.DecryptRequest;
 import com.google.common.base.Strings;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.kms.KmsConnection;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
@@ -34,7 +35,7 @@ public class CloudSqlCredentialDecryptor {
   private final KmsConnection kmsConnection;
 
   @Inject
-  CloudSqlCredentialDecryptor(KmsConnection kmsConnection) {
+  CloudSqlCredentialDecryptor(@Config("beamKmsConnection") KmsConnection kmsConnection) {
     this.kmsConnection = kmsConnection;
   }
 

core/src/main/java/google/registry/beam/initsql/Transforms.java

@@ -16,18 +16,40 @@ package google.registry.beam.initsql;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static google.registry.beam.initsql.BackupPaths.getCommitLogTimestamp;
 import static google.registry.beam.initsql.BackupPaths.getExportFilePatterns;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.setJpaTm;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static google.registry.util.DateTimeUtils.isBeforeOrAt;
+import static java.util.Comparator.comparing;
+import static org.apache.beam.sdk.values.TypeDescriptors.integers;
+import static org.apache.beam.sdk.values.TypeDescriptors.kvs;
+import static org.apache.beam.sdk.values.TypeDescriptors.strings;
 
 import avro.shaded.com.google.common.collect.Iterators;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Streams;
+import google.registry.backup.AppEngineEnvironment;
 import google.registry.backup.CommitLogImports;
 import google.registry.backup.VersionedEntity;
+import google.registry.model.ofy.ObjectifyService;
+import google.registry.model.ofy.Ofy;
+import google.registry.persistence.transaction.JpaTransactionManager;
 import google.registry.tools.LevelDbLogReader;
+import google.registry.util.SystemSleeper;
+import java.io.Serializable;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Supplier;
+import javax.persistence.OptimisticLockException;
 import org.apache.beam.sdk.coders.StringUtf8Coder;
 import org.apache.beam.sdk.io.Compression;
 import org.apache.beam.sdk.io.FileIO;
@@ -36,12 +58,24 @@ import org.apache.beam.sdk.io.fs.EmptyMatchTreatment;
 import org.apache.beam.sdk.io.fs.MatchResult.Metadata;
 import org.apache.beam.sdk.transforms.Create;
 import org.apache.beam.sdk.transforms.DoFn;
+import org.apache.beam.sdk.transforms.Flatten;
+import org.apache.beam.sdk.transforms.GroupByKey;
+import org.apache.beam.sdk.transforms.GroupIntoBatches;
+import org.apache.beam.sdk.transforms.MapElements;
 import org.apache.beam.sdk.transforms.PTransform;
 import org.apache.beam.sdk.transforms.ParDo;
 import org.apache.beam.sdk.transforms.ProcessFunction;
+import org.apache.beam.sdk.values.KV;
 import org.apache.beam.sdk.values.PBegin;
 import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.sdk.values.PCollectionList;
+import org.apache.beam.sdk.values.PCollectionTuple;
+import org.apache.beam.sdk.values.PDone;
+import org.apache.beam.sdk.values.TupleTag;
+import org.apache.beam.sdk.values.TupleTagList;
+import org.apache.beam.sdk.values.TypeDescriptor;
 import org.joda.time.DateTime;
+import org.joda.time.Duration;
 
 /**
  * {@link PTransform Pipeline transforms} used in pipelines that load from both Datastore export
@@ -57,6 +91,110 @@ public final class Transforms {
    */
   @VisibleForTesting static final long EXPORT_ENTITY_TIME_STAMP = START_OF_TIME.getMillis();
 
+  /**
+   * Returns a {@link TupleTag} that can be used to retrieve entities of the given {@code kind} from
+   * the Datastore snapshot returned by {@link #loadDatastoreSnapshot}.
+   */
+  public static TupleTag<VersionedEntity> createTagForKind(String kind) {
+    // When used with PCollectionTuple the result must retain generic type information.
+    // Both the Generic param and the empty bracket below are important.
+    return new TupleTag<VersionedEntity>(Transforms.class.getSimpleName() + ":" + kind) {};
+  }
+
+  /**
+   * Composite {@link PTransform transform} that loads the Datastore snapshot at {@code
+   * commitLogToTime} for caller specified {@code kinds}.
+   *
+   * <p>Caller must provide the location of a Datastore export that started AFTER {@code
+   * commitLogFromTime} and completed BEFORE {@code commitLogToTime}, as well as the root directory
+   * of all CommitLog files.
+   *
+   * <p>Selection of {@code commitLogFromTime} and {@code commitLogToTime} should follow the
+   * guidelines below to ensure that all incremental changes concurrent with the export are covered:
+   *
+   * <ul>
+   *   <li>Two or more CommitLogs should exist between {@code commitLogFromTime} and the starting
+   *       time of the Datastore export. This ensures that the earlier CommitLog file was complete
+   *       before the export started.
+   *   <li>Two or more CommitLogs should exit between the export completion time and {@code
+   *       commitLogToTime}.
+   * </ul>
+   *
+   * <p>The output from the returned transform is a {@link PCollectionTuple} consisting of {@link
+   * VersionedEntity VersionedEntities} grouped into {@link PCollection PCollections} by {@code
+   * kind}.
+   */
+  public static PTransform<PBegin, PCollectionTuple> loadDatastoreSnapshot(
+      String exportDir,
+      String commitLogDir,
+      DateTime commitLogFromTime,
+      DateTime commitLogToTime,
+      Set<String> kinds) {
+    checkArgument(kinds != null && !kinds.isEmpty(), "At least one kind is expected.");
+
+    // Create tags to collect entities by kind in final step.
+    final ImmutableMap<String, TupleTag<VersionedEntity>> outputTags =
+        kinds.stream()
+            .collect(ImmutableMap.toImmutableMap(kind -> kind, Transforms::createTagForKind));
+    // Arbitrarily select one tag as mainOutTag and put the remaining ones in a TupleTagList.
+    // This separation is required by ParDo's config API.
+    Iterator<TupleTag<VersionedEntity>> tagsIt = outputTags.values().iterator();
+    final TupleTag<VersionedEntity> mainOutputTag = tagsIt.next();
+    final TupleTagList additionalTags = TupleTagList.of(ImmutableList.copyOf(tagsIt));
+
+    return new PTransform<PBegin, PCollectionTuple>() {
+      @Override
+      public PCollectionTuple expand(PBegin input) {
+        PCollection<VersionedEntity> exportedEntities =
+            input
+                .apply("Get export file patterns", getDatastoreExportFilePatterns(exportDir, kinds))
+                .apply("Find export files", getFilesByPatterns())
+                .apply("Load export data", loadExportDataFromFiles());
+        PCollection<VersionedEntity> commitLogEntities =
+            input
+                .apply("Get commitlog file patterns", getCommitLogFilePatterns(commitLogDir))
+                .apply("Find commitlog files", getFilesByPatterns())
+                .apply(
+                    "Filter commitLog by time",
+                    filterCommitLogsByTime(commitLogFromTime, commitLogToTime))
+                .apply("Load commitlog data", loadCommitLogsFromFiles(kinds));
+        return PCollectionList.of(exportedEntities)
+            .and(commitLogEntities)
+            .apply("Merge exports and CommitLogs", Flatten.pCollections())
+            .apply(
+                "Key entities by Datastore Keys",
+                // Converting to KV<String, VE> instead of KV<Key, VE> b/c default coder for Key
+                // (SerializableCoder) is not deterministic and cannot be used with GroupBy.
+                MapElements.into(kvs(strings(), TypeDescriptor.of(VersionedEntity.class)))
+                    .via((VersionedEntity e) -> KV.of(e.key().toString(), e)))
+            .apply("Gather entities by key", GroupByKey.create())
+            .apply(
+                "Output latest version per entity",
+                ParDo.of(
+                        new DoFn<KV<String, Iterable<VersionedEntity>>, VersionedEntity>() {
+                          @ProcessElement
+                          public void processElement(
+                              @Element KV<String, Iterable<VersionedEntity>> kv,
+                              MultiOutputReceiver out) {
+                            Optional<VersionedEntity> latest =
+                                Streams.stream(kv.getValue())
+                                    .sorted(comparing(VersionedEntity::commitTimeMills).reversed())
+                                    .findFirst();
+                            // Throw to abort (after default retries). Investigate, fix, and rerun.
+                            checkState(
+                                latest.isPresent(), "Unexpected key with no data", kv.getKey());
+                            if (latest.get().isDelete()) {
+                              return;
+                            }
+                            String kind = latest.get().getEntity().get().getKind();
+                            out.get(outputTags.get(kind)).output(latest.get());
+                          }
+                        })
+                    .withOutputTags(mainOutputTag, additionalTags));
+      }
+    };
+  }
+
   /**
    * Returns a {@link PTransform transform} that can generate a collection of patterns that match
    * all Datastore CommitLog files.
@@ -96,7 +234,7 @@ public final class Transforms {
    * Returns CommitLog files with timestamps between {@code fromTime} (inclusive) and {@code
    * endTime} (exclusive).
    */
-  public static PTransform<PCollection<? extends String>, PCollection<String>>
+  public static PTransform<PCollection<? extends Metadata>, PCollection<Metadata>>
       filterCommitLogsByTime(DateTime fromTime, DateTime toTime) {
     return ParDo.of(new FilterCommitLogFileByTime(fromTime, toTime));
   }
@@ -114,11 +252,47 @@ public final class Transforms {
 
   /** Returns a {@link PTransform} from file {@link Metadata} to {@link VersionedEntity}. */
   public static PTransform<PCollection<Metadata>, PCollection<VersionedEntity>>
-      loadCommitLogsFromFiles() {
+      loadCommitLogsFromFiles(Set<String> kinds) {
     return processFiles(
-        new BackupFileReader(file -> CommitLogImports.loadEntities(file.open()).iterator()));
+        new BackupFileReader(
+            file ->
+                CommitLogImports.loadEntities(file.open()).stream()
+                    .filter(e -> kinds.contains(e.key().getKind()))
+                    .iterator()));
   }
 
+  /**
+   * Returns a {@link PTransform} that writes a {@link PCollection} of entities to a SQL database.
+   *
+   * @param transformId a unique ID for an instance of the returned transform
+   * @param maxWriters the max number of concurrent writes to SQL, which also determines the max
+   *     number of connection pools created
+   * @param batchSize the number of entities to write in each operation
+   * @param jpaSupplier supplier of a {@link JpaTransactionManager}
+   */
+  public static PTransform<PCollection<VersionedEntity>, PDone> writeToSql(
+      String transformId,
+      int maxWriters,
+      int batchSize,
+      SerializableSupplier<JpaTransactionManager> jpaSupplier) {
+    return new PTransform<PCollection<VersionedEntity>, PDone>() {
+      @Override
+      public PDone expand(PCollection<VersionedEntity> input) {
+        input
+            .apply(
+                "Shard data for " + transformId,
+                MapElements.into(kvs(integers(), TypeDescriptor.of(VersionedEntity.class)))
+                    .via(ve -> KV.of(ThreadLocalRandom.current().nextInt(maxWriters), ve)))
+            .apply("Batch output by shard " + transformId, GroupIntoBatches.ofSize(batchSize))
+            .apply("Write in batch for " + transformId, ParDo.of(new SqlBatchWriter(jpaSupplier)));
+        return PDone.in(input.getPipeline());
+      }
+    };
+  }
+
+  /** Interface for serializable {@link Supplier suppliers}. */
+  public interface SerializableSupplier<T> extends Supplier<T>, Serializable {}
+
   /**
    * Returns a {@link PTransform} that produces a {@link PCollection} containing all elements in the
    * given {@link Iterable}.
@@ -139,12 +313,11 @@ public final class Transforms {
         return input
             .apply(FileIO.readMatches().withCompression(Compression.UNCOMPRESSED))
             .apply(transformer.getClass().getSimpleName(), ParDo.of(transformer));
-        // TODO(weiminyu): reshuffle to enable dynamic work rebalance per beam dev guide
       }
     };
   }
 
-  private static class FilterCommitLogFileByTime extends DoFn<String, String> {
+  private static class FilterCommitLogFileByTime extends DoFn<Metadata, Metadata> {
     private final DateTime fromTime;
     private final DateTime toTime;
 
@@ -161,10 +334,10 @@ public final class Transforms {
     }
 
     @ProcessElement
-    public void processElement(@Element String fileName, OutputReceiver<String> out) {
-      DateTime timestamp = getCommitLogTimestamp(fileName);
+    public void processElement(@Element Metadata fileMeta, OutputReceiver<Metadata> out) {
+      DateTime timestamp = getCommitLogTimestamp(fileMeta.resourceId().toString());
       if (isBeforeOrAt(fromTime, timestamp) && timestamp.isBefore(toTime)) {
-        out.output(fileName);
+        out.output(fileMeta);
       }
     }
   }
@@ -197,4 +370,104 @@ public final class Transforms {
       }
     }
   }
+
+  /**
+   * Writes a batch of entities to a SQL database.
+   *
+   * <p>Note that an arbitrary number of instances of this class may be created and freed in
+   * arbitrary order in a single JVM. Due to the tech debt that forced us to use a static variable
+   * to hold the {@code JpaTransactionManager} instance, we must ensure that JpaTransactionManager
+   * is not changed or torn down while being used by some instance.
+   */
+  private static class SqlBatchWriter extends DoFn<KV<Integer, Iterable<VersionedEntity>>, Void> {
+
+    private static int instanceCount = 0;
+    private static JpaTransactionManager originalJpa;
+
+    private final SerializableSupplier<JpaTransactionManager> jpaSupplier;
+
+    private transient Ofy ofy;
+    private transient SystemSleeper sleeper;
+
+    SqlBatchWriter(SerializableSupplier<JpaTransactionManager> jpaSupplier) {
+      this.jpaSupplier = jpaSupplier;
+    }
+
+    @Setup
+    public void setup() {
+      sleeper = new SystemSleeper();
+
+      ObjectifyService.initOfy();
+      ofy = ObjectifyService.ofy();
+
+      synchronized (SqlBatchWriter.class) {
+        if (instanceCount == 0) {
+          originalJpa = jpaTm();
+          setJpaTm(jpaSupplier);
+        }
+        instanceCount++;
+      }
+    }
+
+    @Teardown
+    public void teardown() {
+      synchronized (SqlBatchWriter.class) {
+        instanceCount--;
+        if (instanceCount == 0) {
+          jpaTm().teardown();
+          setJpaTm(() -> originalJpa);
+        }
+      }
+    }
+
+    @ProcessElement
+    public void processElement(@Element KV<Integer, Iterable<VersionedEntity>> kv) {
+      try (AppEngineEnvironment env = new AppEngineEnvironment()) {
+        ImmutableList<Object> ofyEntities =
+            Streams.stream(kv.getValue())
+                .map(VersionedEntity::getEntity)
+                .map(Optional::get)
+                .map(ofy::toPojo)
+                .collect(ImmutableList.toImmutableList());
+        retry(() -> jpaTm().transact(() -> jpaTm().saveNewOrUpdateAll(ofyEntities)));
+      }
+    }
+
+    // TODO(b/160632289): Enhance Retrier and use it here.
+    private void retry(Runnable runnable) {
+      int maxAttempts = 5;
+      int initialDelayMillis = 100;
+      double jitterRatio = 0.2;
+
+      for (int attempt = 0; attempt < maxAttempts; attempt++) {
+        try {
+          runnable.run();
+          return;
+        } catch (Throwable throwable) {
+          throwIfNotCausedBy(throwable, OptimisticLockException.class);
+          int sleepMillis = (1 << attempt) * initialDelayMillis;
+          int jitter =
+              ThreadLocalRandom.current().nextInt((int) (sleepMillis * jitterRatio))
+                  - (int) (sleepMillis * jitterRatio / 2);
+          sleeper.sleepUninterruptibly(Duration.millis(sleepMillis + jitter));
+        }
+      }
+    }
+
+    /**
+     * Rethrows {@code throwable} if it is not (and does not have a cause of) {@code causeType};
+     * otherwise returns with no side effects.
+     */
+    private void throwIfNotCausedBy(Throwable throwable, Class<? extends Throwable> causeType) {
+      Throwable t = throwable;
+      while (t != null) {
+        if (causeType.isInstance(t)) {
+          return;
+        }
+        t = t.getCause();
+      }
+      Throwables.throwIfUnchecked(t);
+      throw new RuntimeException(t);
+    }
+  }
 }

core/src/main/java/google/registry/beam/invoicing/BillingEvent.java

@@ -235,6 +235,7 @@ public abstract class BillingEvent implements Serializable {
                 DATE_TIME_FORMATTER.format(eventTime()),
                 registrarId(),
                 billingId(),
+                poNumber(),
                 tld(),
                 action(),
                 domain(),

core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java

@@ -141,7 +141,7 @@ public class SafeBrowsingTransforms {
     @ProcessElement
     public void processElement(ProcessContext context) {
       Subdomain subdomain = context.element();
-      subdomainBuffer.put(subdomain.fullyQualifiedDomainName(), subdomain);
+      subdomainBuffer.put(subdomain.domainName(), subdomain);
       if (subdomainBuffer.size() >= BATCH_SIZE) {
         ImmutableSet<KV<Subdomain, ThreatMatch>> results = evaluateAndFlush();
         results.forEach(context::output);
@@ -239,7 +239,7 @@ public class SafeBrowsingTransforms {
             String url = match.getJSONObject("threat").getString("url");
             Subdomain subdomain = subdomainBuffer.get(url);
             resultBuilder.add(
-                KV.of(subdomain, ThreatMatch.create(match, subdomain.fullyQualifiedDomainName())));
+                KV.of(subdomain, ThreatMatch.create(match, subdomain.domainName())));
           }
         }
       }

core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java

@@ -77,7 +77,7 @@ public class Spec11Pipeline implements Serializable {
   public static final String REGISTRAR_EMAIL_FIELD = "registrarEmailAddress";
   /** The JSON object field into which we put the registrar's name for Spec11 reports. */
   public static final String REGISTRAR_CLIENT_ID_FIELD = "registrarClientId";
-  /** The JSON object field we put the threat match array for Spec11 reports. */
+  /** The JSON object field into which we put the threat match array for Spec11 reports. */
   public static final String THREAT_MATCHES_FIELD = "threatMatches";
 
   private final String projectId;
@@ -94,8 +94,7 @@ public class Spec11Pipeline implements Serializable {
       @Config("spec11TemplateUrl") String spec11TemplateUrl,
       @Config("reportingBucketUrl") String reportingBucketUrl,
       @LocalCredential GoogleCredentialsBundle googleCredentialsBundle,
-      Retrier retrier
-  ) {
+      Retrier retrier) {
     this.projectId = projectId;
     this.beamStagingUrl = beamStagingUrl;
     this.spec11TemplateUrl = spec11TemplateUrl;
@@ -177,9 +176,11 @@ public class Spec11Pipeline implements Serializable {
       PCollection<Subdomain> domains,
       EvaluateSafeBrowsingFn evaluateSafeBrowsingFn,
       ValueProvider<String> dateProvider) {
-    PCollection<KV<Subdomain, ThreatMatch>> subdomains =
+
+    /* Store ThreatMatch objects in JSON. */
+    PCollection<KV<Subdomain, ThreatMatch>> subdomainsJson =
         domains.apply("Run through SafeBrowsingAPI", ParDo.of(evaluateSafeBrowsingFn));
-    subdomains
+    subdomainsJson
         .apply(
             "Map registrar client ID to email/ThreatMatch pair",
             MapElements.into(
@@ -188,7 +189,7 @@ public class Spec11Pipeline implements Serializable {
                 .via(
                     (KV<Subdomain, ThreatMatch> kv) ->
                         KV.of(
-                            kv.getKey().registrarClientId(),
+                            kv.getKey().registrarId(),
                             EmailAndThreatMatch.create(
                                 kv.getKey().registrarEmailAddress(), kv.getValue()))))
         .apply("Group by registrar client ID", GroupByKey.create())

core/src/main/java/google/registry/beam/spec11/Subdomain.java

@@ -36,12 +36,14 @@ import org.apache.beam.sdk.io.gcp.bigquery.SchemaAndRecord;
 public abstract class Subdomain implements Serializable {
 
   private static final ImmutableList<String> FIELD_NAMES =
-      ImmutableList.of("fullyQualifiedDomainName", "registrarClientId", "registrarEmailAddress");
+      ImmutableList.of("domainName", "domainRepoId", "registrarId", "registrarEmailAddress");
 
   /** Returns the fully qualified domain name. */
-  abstract String fullyQualifiedDomainName();
-  /** Returns the client ID of the associated registrar for this domain. */
-  abstract String registrarClientId();
+  abstract String domainName();
+  /** Returns the domain repo ID (the primary key of the domain table). */
+  abstract String domainRepoId();
+  /** Returns the registrar ID of the associated registrar for this domain. */
+  abstract String registrarId();
   /** Returns the email address of the registrar associated with this domain. */
   abstract String registrarEmailAddress();
 
@@ -56,8 +58,9 @@ public abstract class Subdomain implements Serializable {
     checkFieldsNotNull(FIELD_NAMES, schemaAndRecord);
     GenericRecord record = schemaAndRecord.getRecord();
     return create(
-        extractField(record, "fullyQualifiedDomainName"),
-        extractField(record, "registrarClientId"),
+        extractField(record, "domainName"),
+        extractField(record, "domainRepoId"),
+        extractField(record, "registrarId"),
         extractField(record, "registrarEmailAddress"));
   }
 
@@ -69,9 +72,11 @@ public abstract class Subdomain implements Serializable {
    */
   @VisibleForTesting
   static Subdomain create(
-      String fullyQualifiedDomainName, String registrarClientId, String registrarEmailAddress) {
+      String domainName,
+      String domainRepoId,
+      String registrarId,
+      String registrarEmailAddress) {
     return new AutoValue_Subdomain(
-        fullyQualifiedDomainName, registrarClientId, registrarEmailAddress);
+        domainName, domainRepoId, registrarId, registrarEmailAddress);
   }
 }
-

core/src/main/java/google/registry/beam/spec11/sql/subdomains.sql

@@ -19,11 +19,13 @@
   -- email address.
 
 SELECT
-  domain.fullyQualifiedDomainName AS fullyQualifiedDomainName,
-  registrar.clientId AS registrarClientId,
+  domain.fullyQualifiedDomainName AS domainName,
+  domain.__key__.name AS domainRepoId,
+  registrar.clientId AS registrarId,
   COALESCE(registrar.emailAddress, '') AS registrarEmailAddress
 FROM ( (
     SELECT
+      __key__,
       fullyQualifiedDomainName,
       currentSponsorClientId,
       creationTime

core/src/main/java/google/registry/env/common/backend/WEB-INF/web.xml

@@ -367,6 +367,12 @@
     <url-pattern>/_dr/task/linkRdeHosts</url-pattern>
   </servlet-mapping>
 
+  <!-- Action to automatically re-lock a domain after unlocking it -->
+  <servlet-mapping>
+    <servlet-name>backend-servlet</servlet-name>
+    <url-pattern>/_dr/task/relockDomain</url-pattern>
+  </servlet-mapping>
+
   <!-- Security config -->
   <security-constraint>
     <web-resource-collection>

core/src/main/java/google/registry/env/production/pubapi/WEB-INF/appengine-web.xml

@@ -7,7 +7,7 @@
   <sessions-enabled>true</sessions-enabled>
   <instance-class>B4_1G</instance-class>
   <manual-scaling>
-    <instances>10</instances>
+    <instances>20</instances>
   </manual-scaling>
 
   <system-properties>

core/src/main/java/google/registry/env/sandbox/default/WEB-INF/cron.xml

@@ -18,7 +18,14 @@
       and streams it to cloud storage. When this job has finished successfully, it'll
       launch a separate task that uploads the deposit file to Iron Mountain via SFTP.
     </description>
-    <schedule>every day 00:07</schedule>
+    <!--
+      This only needs to run once per day, but we launch additional jobs in case the
+      cursor is lagging behind, so it'll catch up to the current date eventually.
+
+      See <a href="../../../production/default/WEB-INF/cron.xml">production config</a> for an
+      explanation of job starting times.
+    -->
+    <schedule>every 12 hours from 00:07 to 12:07</schedule>
     <target>backend</target>
   </cron>
 

core/src/main/java/google/registry/flows/poll/PollAckFlow.java

@@ -14,8 +14,8 @@
 
 package google.registry.flows.poll;
 
-import static com.google.common.base.Preconditions.checkState;
 import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
+import static google.registry.flows.poll.PollFlowUtils.ackPollMessage;
 import static google.registry.flows.poll.PollFlowUtils.getPollMessagesQuery;
 import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_NO_MESSAGES;
 import static google.registry.model.ofy.ObjectifyService.ofy;
@@ -100,27 +100,8 @@ public class PollAckFlow implements TransactionalFlow {
     // This keeps track of whether we should include the current acked message in the updated
     // message count that's returned to the user. The only case where we do so is if an autorenew
     // poll message is acked, but its next event is already ready to be delivered.
-    boolean includeAckedMessageInCount = false;
-    if (pollMessage instanceof PollMessage.OneTime) {
-      // One-time poll messages are deleted once acked.
-      ofy().delete().entity(pollMessage);
-    } else {
-      checkState(pollMessage instanceof PollMessage.Autorenew, "Unknown poll message type");
-      PollMessage.Autorenew autorenewPollMessage = (PollMessage.Autorenew) pollMessage;
+    boolean includeAckedMessageInCount = ackPollMessage(pollMessage);
 
-      // Move the eventTime of this autorenew poll message forward by a year.
-      DateTime nextEventTime = autorenewPollMessage.getEventTime().plusYears(1);
-
-      // If the next event falls within the bounds of the end time, then just update the eventTime
-      // and re-save it for future autorenew poll messages to be delivered. Otherwise, this
-      // autorenew poll message has no more events to deliver and should be deleted.
-      if (nextEventTime.isBefore(autorenewPollMessage.getAutorenewEndTime())) {
-        ofy().save().entity(autorenewPollMessage.asBuilder().setEventTime(nextEventTime).build());
-        includeAckedMessageInCount = isBeforeOrAt(nextEventTime, now);
-      } else {
-        ofy().delete().entity(autorenewPollMessage);
-      }
-    }
     // We need to return the new queue length. If this was the last message in the queue being
     // acked, then we return a special status code indicating that. Note that the query will
     // include the message being acked.

core/src/main/java/google/registry/flows/poll/PollFlowUtils.java

@@ -14,7 +14,10 @@
 
 package google.registry.flows.poll;
 
+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.util.DateTimeUtils.isBeforeOrAt;
 
 import com.googlecode.objectify.cmd.Query;
 import google.registry.model.poll.PollMessage;
@@ -33,4 +36,42 @@ public final class PollFlowUtils {
         .filter("eventTime <=", now.toDate())
         .order("eventTime");
   }
+
+  /**
+   * Acknowledges the given {@link PollMessage} and returns whether we should include the current
+   * acked message in the updated message count that's returned to the user.
+   *
+   * <p>The only case where we do so is if an autorenew poll message is acked, but its next event is
+   * already ready to be delivered.
+   */
+  public static boolean ackPollMessage(PollMessage pollMessage) {
+    checkArgument(
+        isBeforeOrAt(pollMessage.getEventTime(), tm().getTransactionTime()),
+        "Cannot ACK poll message with ID %s because its event time is in the future: %s",
+        pollMessage.getId(),
+        pollMessage.getEventTime());
+    boolean includeAckedMessageInCount = false;
+    if (pollMessage instanceof PollMessage.OneTime) {
+      // One-time poll messages are deleted once acked.
+      tm().delete(pollMessage.createVKey());
+    } else if (pollMessage instanceof PollMessage.Autorenew) {
+      PollMessage.Autorenew autorenewPollMessage = (PollMessage.Autorenew) pollMessage;
+
+      // Move the eventTime of this autorenew poll message forward by a year.
+      DateTime nextEventTime = autorenewPollMessage.getEventTime().plusYears(1);
+
+      // If the next event falls within the bounds of the end time, then just update the eventTime
+      // and re-save it for future autorenew poll messages to be delivered. Otherwise, this
+      // autorenew poll message has no more events to deliver and should be deleted.
+      if (nextEventTime.isBefore(autorenewPollMessage.getAutorenewEndTime())) {
+        tm().saveNewOrUpdate(autorenewPollMessage.asBuilder().setEventTime(nextEventTime).build());
+        includeAckedMessageInCount = isBeforeOrAt(nextEventTime, tm().getTransactionTime());
+      } else {
+        tm().delete(autorenewPollMessage.createVKey());
+      }
+    } else {
+      throw new IllegalArgumentException("Unknown poll message type: " + pollMessage.getClass());
+    }
+    return includeAckedMessageInCount;
+  }
 }

core/src/main/java/google/registry/keyring/kms/KmsConnectionImpl.java

@@ -25,11 +25,9 @@ import com.google.api.services.cloudkms.v1.model.DecryptRequest;
 import com.google.api.services.cloudkms.v1.model.EncryptRequest;
 import com.google.api.services.cloudkms.v1.model.KeyRing;
 import com.google.api.services.cloudkms.v1.model.UpdateCryptoKeyPrimaryVersionRequest;
-import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.api.KeyringException;
 import google.registry.util.Retrier;
 import java.io.IOException;
-import javax.inject.Inject;
 
 /** The {@link KmsConnection} which talks to Cloud KMS. */
 class KmsConnectionImpl implements KmsConnection {
@@ -44,12 +42,7 @@ class KmsConnectionImpl implements KmsConnection {
   private final String projectId;
   private final Retrier retrier;
 
-  @Inject
-  KmsConnectionImpl(
-      @Config("cloudKmsProjectId") String projectId,
-      @Config("cloudKmsKeyRing") String kmsKeyRingName,
-      Retrier retrier,
-      CloudKMS kms) {
+  KmsConnectionImpl(String projectId, String kmsKeyRingName, Retrier retrier, CloudKMS kms) {
     this.projectId = projectId;
     this.kmsKeyRingName = kmsKeyRingName;
     this.retrier = retrier;

core/src/main/java/google/registry/keyring/kms/KmsKeyring.java

@@ -21,6 +21,7 @@ import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.googlecode.objectify.Key;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.api.KeySerializer;
 import google.registry.keyring.api.Keyring;
 import google.registry.keyring.api.KeyringException;
@@ -86,7 +87,7 @@ public class KmsKeyring implements Keyring {
   private final KmsConnection kmsConnection;
 
   @Inject
-  KmsKeyring(KmsConnection kmsConnection) {
+  KmsKeyring(@Config("defaultKmsConnection") KmsConnection kmsConnection) {
     this.kmsConnection = kmsConnection;
   }
 

core/src/main/java/google/registry/keyring/kms/KmsModule.java

@@ -24,6 +24,7 @@ import google.registry.config.CredentialModule.DefaultCredential;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.api.Keyring;
 import google.registry.util.GoogleCredentialsBundle;
+import google.registry.util.Retrier;
 
 /** Dagger module for Cloud KMS. */
 @Module
@@ -32,9 +33,22 @@ public abstract class KmsModule {
   public static final String NAME = "KMS";
 
   @Provides
+  @Config("defaultKms")
   static CloudKMS provideKms(
       @DefaultCredential GoogleCredentialsBundle credentialsBundle,
       @Config("cloudKmsProjectId") String projectId) {
+    return createKms(credentialsBundle, projectId);
+  }
+
+  @Provides
+  @Config("beamKms")
+  static CloudKMS provideBeamKms(
+      @DefaultCredential GoogleCredentialsBundle credentialsBundle,
+      @Config("beamCloudKmsProjectId") String projectId) {
+    return createKms(credentialsBundle, projectId);
+  }
+
+  private static CloudKMS createKms(GoogleCredentialsBundle credentialsBundle, String projectId) {
     return new CloudKMS.Builder(
             credentialsBundle.getHttpTransport(),
             credentialsBundle.getJsonFactory(),
@@ -43,11 +57,28 @@ public abstract class KmsModule {
         .build();
   }
 
+  @Provides
+  @Config("defaultKmsConnection")
+  static KmsConnection provideKmsConnection(
+      @Config("cloudKmsProjectId") String projectId,
+      @Config("cloudKmsKeyRing") String keyringName,
+      Retrier retrier,
+      @Config("defaultKms") CloudKMS defaultKms) {
+    return new KmsConnectionImpl(projectId, keyringName, retrier, defaultKms);
+  }
+
+  @Provides
+  @Config("beamKmsConnection")
+  static KmsConnection provideBeamKmsConnection(
+      @Config("beamCloudKmsProjectId") String projectId,
+      @Config("beamCloudKmsKeyRing") String keyringName,
+      Retrier retrier,
+      @Config("beamKms") CloudKMS defaultKms) {
+    return new KmsConnectionImpl(projectId, keyringName, retrier, defaultKms);
+  }
+
   @Binds
   @IntoMap
   @StringKey(NAME)
   abstract Keyring provideKeyring(KmsKeyring keyring);
-
-  @Binds
-  abstract KmsConnection provideKmsConnection(KmsConnectionImpl kmsConnectionImpl);
 }

core/src/main/java/google/registry/keyring/kms/KmsUpdater.java

@@ -39,6 +39,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.google.common.collect.ImmutableMap;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.api.KeySerializer;
 import google.registry.keyring.kms.KmsKeyring.PrivateKeyLabel;
 import google.registry.keyring.kms.KmsKeyring.PublicKeyLabel;
@@ -64,7 +65,7 @@ public final class KmsUpdater {
   private final HashMap<String, byte[]> secretValues;
 
   @Inject
-  public KmsUpdater(KmsConnection kmsConnection) {
+  public KmsUpdater(@Config("defaultKmsConnection") KmsConnection kmsConnection) {
     this.kmsConnection = kmsConnection;
 
     // Use LinkedHashMap to preserve insertion order on update() to simplify testing and debugging

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

@@ -14,16 +14,21 @@
 
 package google.registry.model;
 
+import com.google.common.annotations.VisibleForTesting;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.MappedSuperclass;
 import javax.xml.bind.annotation.XmlTransient;
 
 /**
  * Base class for entities that are the root of a Registry 2.0 entity group that gets enrolled in
  * commit logs for backup purposes.
  *
- * <p>The commit log system needs to preserve the ordering of closely timed mutations to entities
- * in a single entity group. We require an {@link UpdateAutoTimestamp} field on the root of a group
- * so that we can enforce strictly increasing timestamps.
+ * <p>The commit log system needs to preserve the ordering of closely timed mutations to entities in
+ * a single entity group. We require an {@link UpdateAutoTimestamp} field on the root of a group so
+ * that we can enforce strictly increasing timestamps.
  */
+@MappedSuperclass
 public abstract class BackupGroupRoot extends ImmutableObject {
   /**
    * An automatically managed timestamp of when this object was last written to Datastore.
@@ -32,10 +37,14 @@ public abstract class BackupGroupRoot extends ImmutableObject {
    * that this is updated on every save, rather than only in response to an {@code <update>} command
    */
   @XmlTransient
+  // Prevents subclasses from unexpectedly accessing as property (e.g., HostResource), which would
+  // require an unnecessary non-private setter method.
+  @Access(AccessType.FIELD)
+  @VisibleForTesting
   UpdateAutoTimestamp updateTimestamp = UpdateAutoTimestamp.create(null);
 
   /** Get the {@link UpdateAutoTimestamp} for this entity. */
-  public final UpdateAutoTimestamp getUpdateAutoTimestamp() {
+  public UpdateAutoTimestamp getUpdateTimestamp() {
     return updateTimestamp;
   }
 }

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

@@ -155,7 +155,7 @@ public final class EppResourceUtils {
     // time for writes.
     return Optional.of(
         cloneProjectedAtTime(
-            resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp())));
+            resource, latestOf(now, resource.getUpdateTimestamp().getTimestamp())));
   }
 
   /**
@@ -298,7 +298,7 @@ public final class EppResourceUtils {
     // and returns it projected forward to exactly the desired timestamp, or null if the resource is
     // deleted at that timestamp.
     final Result<T> loadResult =
-        isAtOrAfter(timestamp, resource.getUpdateAutoTimestamp().getTimestamp())
+        isAtOrAfter(timestamp, resource.getUpdateTimestamp().getTimestamp())
             ? new ResultNow<>(resource)
             : loadMostRecentRevisionAtTime(resource, timestamp);
     return () -> {

core/src/main/java/google/registry/model/contact/ContactBase.java

@@ -0,0 +1,398 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model.contact;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static google.registry.model.EppResourceUtils.projectResourceOntoBuilderAtTime;
+
+import com.google.common.collect.ImmutableList;
+import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.IgnoreSave;
+import com.googlecode.objectify.annotation.Index;
+import com.googlecode.objectify.condition.IfNull;
+import google.registry.model.EppResource;
+import google.registry.model.EppResource.ResourceWithTransferData;
+import google.registry.model.transfer.ContactTransferData;
+import google.registry.persistence.VKey;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Stream;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.AttributeOverride;
+import javax.persistence.AttributeOverrides;
+import javax.persistence.Column;
+import javax.persistence.Embeddable;
+import javax.persistence.Embedded;
+import javax.persistence.MappedSuperclass;
+import javax.xml.bind.annotation.XmlElement;
+import org.joda.time.DateTime;
+
+/**
+ * A persistable contact resource including mutable and non-mutable fields.
+ *
+ * @see <a href="https://tools.ietf.org/html/rfc5733">RFC 5733</a>
+ *     <p>This class deliberately does not include an {@link javax.persistence.Id} so that any
+ *     foreign-keyed fields can refer to the proper parent entity's ID, whether we're storing this
+ *     in the DB itself or as part of another entity
+ */
+@MappedSuperclass
+@Embeddable
+@Access(AccessType.FIELD)
+public class ContactBase extends EppResource implements ResourceWithTransferData {
+
+  /**
+   * Unique identifier for this contact.
+   *
+   * <p>This is only unique in the sense that for any given lifetime specified as the time range
+   * from (creationTime, deletionTime) there can only be one contact in Datastore with this id.
+   * However, there can be many contacts with the same id and non-overlapping lifetimes.
+   */
+  String contactId;
+
+  /**
+   * Localized postal info for the contact. All contained values must be representable in the 7-bit
+   * US-ASCII character set. Personal info; cleared by {@link ContactResource.Builder#wipeOut}.
+   */
+  @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "addr_local_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "addr_local_org")),
+    @AttributeOverride(name = "type", column = @Column(name = "addr_local_type")),
+    @AttributeOverride(
+        name = "address.streetLine1",
+        column = @Column(name = "addr_local_street_line1")),
+    @AttributeOverride(
+        name = "address.streetLine2",
+        column = @Column(name = "addr_local_street_line2")),
+    @AttributeOverride(
+        name = "address.streetLine3",
+        column = @Column(name = "addr_local_street_line3")),
+    @AttributeOverride(name = "address.city", column = @Column(name = "addr_local_city")),
+    @AttributeOverride(name = "address.state", column = @Column(name = "addr_local_state")),
+    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_local_zip")),
+    @AttributeOverride(
+        name = "address.countryCode",
+        column = @Column(name = "addr_local_country_code"))
+  })
+  PostalInfo localizedPostalInfo;
+
+  /**
+   * Internationalized postal info for the contact. Personal info; cleared by {@link
+   * ContactResource.Builder#wipeOut}.
+   */
+  @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "addr_i18n_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "addr_i18n_org")),
+    @AttributeOverride(name = "type", column = @Column(name = "addr_i18n_type")),
+    @AttributeOverride(
+        name = "address.streetLine1",
+        column = @Column(name = "addr_i18n_street_line1")),
+    @AttributeOverride(
+        name = "address.streetLine2",
+        column = @Column(name = "addr_i18n_street_line2")),
+    @AttributeOverride(
+        name = "address.streetLine3",
+        column = @Column(name = "addr_i18n_street_line3")),
+    @AttributeOverride(name = "address.city", column = @Column(name = "addr_i18n_city")),
+    @AttributeOverride(name = "address.state", column = @Column(name = "addr_i18n_state")),
+    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_i18n_zip")),
+    @AttributeOverride(
+        name = "address.countryCode",
+        column = @Column(name = "addr_i18n_country_code"))
+  })
+  PostalInfo internationalizedPostalInfo;
+
+  /**
+   * Contact name used for name searches. This is set automatically to be the internationalized
+   * postal name, or if null, the localized postal name, or if that is null as well, null. Personal
+   * info; cleared by {@link ContactResource.Builder#wipeOut}.
+   */
+  @Index String searchName;
+
+  /** Contact’s voice number. Personal info; cleared by {@link ContactResource.Builder#wipeOut}. */
+  @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "phoneNumber", column = @Column(name = "voice_phone_number")),
+    @AttributeOverride(name = "extension", column = @Column(name = "voice_phone_extension")),
+  })
+  ContactPhoneNumber voice;
+
+  /** Contact’s fax number. Personal info; cleared by {@link ContactResource.Builder#wipeOut}. */
+  @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "phoneNumber", column = @Column(name = "fax_phone_number")),
+    @AttributeOverride(name = "extension", column = @Column(name = "fax_phone_extension")),
+  })
+  ContactPhoneNumber fax;
+
+  /** Contact’s email address. Personal info; cleared by {@link ContactResource.Builder#wipeOut}. */
+  @IgnoreSave(IfNull.class)
+  String email;
+
+  /** Authorization info (aka transfer secret) of the contact. */
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "pw.value", column = @Column(name = "auth_info_value")),
+    @AttributeOverride(name = "pw.repoId", column = @Column(name = "auth_info_repo_id")),
+  })
+  ContactAuthInfo authInfo;
+
+  /** Data about any pending or past transfers on this contact. */
+  ContactTransferData transferData;
+
+  /**
+   * The time that this resource was last transferred.
+   *
+   * <p>Can be null if the resource has never been transferred.
+   */
+  DateTime lastTransferTime;
+
+  // If any new fields are added which contain personal information, make sure they are cleared by
+  // the wipeOut() function, so that data is not kept around for deleted contacts.
+
+  /** Disclosure policy. */
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "disclose_types_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "disclose_types_org")),
+    @AttributeOverride(name = "addr", column = @Column(name = "disclose_types_addr")),
+    @AttributeOverride(name = "flag", column = @Column(name = "disclose_mode_flag")),
+    @AttributeOverride(name = "voice.marked", column = @Column(name = "disclose_show_voice")),
+    @AttributeOverride(name = "fax.marked", column = @Column(name = "disclose_show_fax")),
+    @AttributeOverride(name = "email.marked", column = @Column(name = "disclose_show_email"))
+  })
+  Disclose disclose;
+
+  @Override
+  public VKey<? extends ContactBase> createVKey() {
+    // TODO(mmuller): create symmetric keys if we can ever reload both sides.
+    return VKey.create(ContactBase.class, getRepoId(), Key.create(this));
+  }
+
+  public String getContactId() {
+    return contactId;
+  }
+
+  public PostalInfo getLocalizedPostalInfo() {
+    return localizedPostalInfo;
+  }
+
+  public PostalInfo getInternationalizedPostalInfo() {
+    return internationalizedPostalInfo;
+  }
+
+  public String getSearchName() {
+    return searchName;
+  }
+
+  public ContactPhoneNumber getVoiceNumber() {
+    return voice;
+  }
+
+  public ContactPhoneNumber getFaxNumber() {
+    return fax;
+  }
+
+  public String getEmailAddress() {
+    return email;
+  }
+
+  public ContactAuthInfo getAuthInfo() {
+    return authInfo;
+  }
+
+  public Disclose getDisclose() {
+    return disclose;
+  }
+
+  public String getCurrentSponsorClientId() {
+    return getPersistedCurrentSponsorClientId();
+  }
+
+  @Override
+  public ContactTransferData getTransferData() {
+    return Optional.ofNullable(transferData).orElse(ContactTransferData.EMPTY);
+  }
+
+  @Override
+  public DateTime getLastTransferTime() {
+    return lastTransferTime;
+  }
+
+  @Override
+  public String getForeignKey() {
+    return contactId;
+  }
+
+  /**
+   * Postal info for the contact.
+   *
+   * <p>The XML marshalling expects the {@link PostalInfo} objects in a list, but we can't actually
+   * persist them to Datastore that way because Objectify can't handle collections of embedded
+   * objects that themselves contain collections, and there's a list of streets inside. This method
+   * transforms the persisted format to the XML format for marshalling.
+   */
+  @XmlElement(name = "postalInfo")
+  public ImmutableList<PostalInfo> getPostalInfosAsList() {
+    return Stream.of(localizedPostalInfo, internationalizedPostalInfo)
+        .filter(Objects::nonNull)
+        .collect(toImmutableList());
+  }
+
+  @Override
+  public ContactBase cloneProjectedAtTime(DateTime now) {
+    return cloneContactProjectedAtTime(this, now);
+  }
+
+  /**
+   * Clones the contact (or subclass). A separate static method so that we can pass in and return a
+   * T without the compiler complaining.
+   */
+  protected static <T extends ContactBase> T cloneContactProjectedAtTime(T contact, DateTime now) {
+    Builder builder = contact.asBuilder();
+    projectResourceOntoBuilderAtTime(contact, builder, now);
+    return (T) builder.build();
+  }
+
+  @Override
+  public Builder asBuilder() {
+    return new Builder<>(clone(this));
+  }
+
+  /** A builder for constructing {@link ContactResource}, since it is immutable. */
+  public static class Builder<T extends ContactBase, B extends Builder<T, B>>
+      extends EppResource.Builder<T, B> implements BuilderWithTransferData<ContactTransferData, B> {
+
+    public Builder() {}
+
+    protected Builder(T instance) {
+      super(instance);
+    }
+
+    public B setContactId(String contactId) {
+      getInstance().contactId = contactId;
+      return thisCastToDerived();
+    }
+
+    public B setLocalizedPostalInfo(PostalInfo localizedPostalInfo) {
+      checkArgument(
+          localizedPostalInfo == null
+              || PostalInfo.Type.LOCALIZED.equals(localizedPostalInfo.getType()));
+      getInstance().localizedPostalInfo = localizedPostalInfo;
+      return thisCastToDerived();
+    }
+
+    public B setInternationalizedPostalInfo(PostalInfo internationalizedPostalInfo) {
+      checkArgument(
+          internationalizedPostalInfo == null
+              || PostalInfo.Type.INTERNATIONALIZED.equals(internationalizedPostalInfo.getType()));
+      getInstance().internationalizedPostalInfo = internationalizedPostalInfo;
+      return thisCastToDerived();
+    }
+
+    public B overlayLocalizedPostalInfo(PostalInfo localizedPostalInfo) {
+      return setLocalizedPostalInfo(
+          getInstance().localizedPostalInfo == null
+              ? localizedPostalInfo
+              : getInstance().localizedPostalInfo.overlay(localizedPostalInfo));
+    }
+
+    public B overlayInternationalizedPostalInfo(PostalInfo internationalizedPostalInfo) {
+      return setInternationalizedPostalInfo(
+          getInstance().internationalizedPostalInfo == null
+              ? internationalizedPostalInfo
+              : getInstance().internationalizedPostalInfo.overlay(internationalizedPostalInfo));
+    }
+
+    public B setVoiceNumber(ContactPhoneNumber voiceNumber) {
+      getInstance().voice = voiceNumber;
+      return thisCastToDerived();
+    }
+
+    public B setFaxNumber(ContactPhoneNumber faxNumber) {
+      getInstance().fax = faxNumber;
+      return thisCastToDerived();
+    }
+
+    public B setEmailAddress(String emailAddress) {
+      getInstance().email = emailAddress;
+      return thisCastToDerived();
+    }
+
+    public B setAuthInfo(ContactAuthInfo authInfo) {
+      getInstance().authInfo = authInfo;
+      return thisCastToDerived();
+    }
+
+    public B setDisclose(Disclose disclose) {
+      getInstance().disclose = disclose;
+      return thisCastToDerived();
+    }
+
+    @Override
+    public B setTransferData(ContactTransferData transferData) {
+      getInstance().transferData = transferData;
+      return thisCastToDerived();
+    }
+
+    @Override
+    public B setLastTransferTime(DateTime lastTransferTime) {
+      getInstance().lastTransferTime = lastTransferTime;
+      return thisCastToDerived();
+    }
+
+    /**
+     * Remove all personally identifying information about a contact.
+     *
+     * <p>This should be used when deleting a contact so that the soft-deleted entity doesn't
+     * contain information that the registrant requested to be deleted.
+     */
+    public B wipeOut() {
+      setEmailAddress(null);
+      setFaxNumber(null);
+      setInternationalizedPostalInfo(null);
+      setLocalizedPostalInfo(null);
+      setVoiceNumber(null);
+      return thisCastToDerived();
+    }
+
+    @Override
+    public T build() {
+      T instance = getInstance();
+      // If TransferData is totally empty, set it to null.
+      if (ContactTransferData.EMPTY.equals(instance.transferData)) {
+        setTransferData(null);
+      }
+      // Set the searchName using the internationalized and localized postal info names.
+      if ((instance.internationalizedPostalInfo != null)
+          && (instance.internationalizedPostalInfo.getName() != null)) {
+        instance.searchName = instance.internationalizedPostalInfo.getName();
+      } else if ((instance.localizedPostalInfo != null)
+          && (instance.localizedPostalInfo.getName() != null)) {
+        instance.searchName = instance.localizedPostalInfo.getName();
+      } else {
+        instance.searchName = null;
+      }
+      return super.build();
+    }
+  }
+}

core/src/main/java/google/registry/model/contact/ContactHistory.java

@@ -0,0 +1,88 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model.contact;
+
+import com.googlecode.objectify.Key;
+import google.registry.model.EppResource;
+import google.registry.model.reporting.HistoryEntry;
+import google.registry.persistence.VKey;
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+/**
+ * A persisted history entry representing an EPP modification to a contact.
+ *
+ * <p>In addition to the general history fields (e.g. action time, registrar ID) we also persist a
+ * copy of the host entity at this point in time. We persist a raw {@link ContactBase} so that the
+ * foreign-keyed fields in that class can refer to this object.
+ */
+@Entity
+@javax.persistence.Table(
+    indexes = {
+      @javax.persistence.Index(columnList = "creationTime"),
+      @javax.persistence.Index(columnList = "historyRegistrarId"),
+      @javax.persistence.Index(columnList = "historyType"),
+      @javax.persistence.Index(columnList = "historyModificationTime")
+    })
+public class ContactHistory extends HistoryEntry {
+  // Store ContactBase instead of ContactResource so we don't pick up its @Id
+  ContactBase contactBase;
+
+  @Column(nullable = false)
+  VKey<ContactResource> contactRepoId;
+
+  /** The state of the {@link ContactBase} object at this point in time. */
+  public ContactBase getContactBase() {
+    return contactBase;
+  }
+
+  /** The key to the {@link ContactResource} this is based off of. */
+  public VKey<ContactResource> getContactRepoId() {
+    return contactRepoId;
+  }
+
+  @Override
+  public Builder asBuilder() {
+    return new Builder(clone(this));
+  }
+
+  public static class Builder extends HistoryEntry.Builder<ContactHistory, ContactHistory.Builder> {
+
+    public Builder() {}
+
+    public Builder(ContactHistory instance) {
+      super(instance);
+    }
+
+    public Builder setContactBase(ContactBase contactBase) {
+      getInstance().contactBase = contactBase;
+      return this;
+    }
+
+    public Builder setContactRepoId(VKey<ContactResource> contactRepoId) {
+      getInstance().contactRepoId = contactRepoId;
+      contactRepoId.maybeGetOfyKey().ifPresent(parent -> getInstance().parent = parent);
+      return this;
+    }
+
+    // We can remove this once all HistoryEntries are converted to History objects
+    @Override
+    public Builder setParent(Key<? extends EppResource> parent) {
+      super.setParent(parent);
+      getInstance().contactRepoId = VKey.create(ContactResource.class, parent.getName(), parent);
+      return this;
+    }
+  }
+}

core/src/main/java/google/registry/model/contact/ContactResource.java

@@ -14,36 +14,16 @@
 
 package google.registry.model.contact;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static google.registry.model.EppResourceUtils.projectResourceOntoBuilderAtTime;
-
-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.IgnoreSave;
-import com.googlecode.objectify.annotation.Index;
-import com.googlecode.objectify.condition.IfNull;
-import google.registry.model.EppResource;
 import google.registry.model.EppResource.ForeignKeyedEppResource;
-import google.registry.model.EppResource.ResourceWithTransferData;
 import google.registry.model.annotations.ExternalMessagingName;
 import google.registry.model.annotations.ReportedOn;
-import google.registry.model.contact.PostalInfo.Type;
-import google.registry.model.transfer.ContactTransferData;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithStringVKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.stream.Stream;
 import javax.persistence.Access;
 import javax.persistence.AccessType;
-import javax.persistence.AttributeOverride;
-import javax.persistence.AttributeOverrides;
-import javax.persistence.Column;
-import javax.persistence.Embedded;
-import javax.xml.bind.annotation.XmlElement;
 import org.joda.time.DateTime;
 
 /**
@@ -66,140 +46,12 @@ import org.joda.time.DateTime;
 @ExternalMessagingName("contact")
 @WithStringVKey
 @Access(AccessType.FIELD)
-public class ContactResource extends EppResource
-    implements DatastoreAndSqlEntity, ForeignKeyedEppResource, ResourceWithTransferData {
-
-  /**
-   * Unique identifier for this contact.
-   *
-   * <p>This is only unique in the sense that for any given lifetime specified as the time range
-   * from (creationTime, deletionTime) there can only be one contact in Datastore with this id.
-   * However, there can be many contacts with the same id and non-overlapping lifetimes.
-   */
-  String contactId;
-
-  /**
-   * Localized postal info for the contact. All contained values must be representable in the 7-bit
-   * US-ASCII character set. Personal info; cleared by {@link Builder#wipeOut}.
-   */
-  @IgnoreSave(IfNull.class)
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "name", column = @Column(name = "addr_local_name")),
-    @AttributeOverride(name = "org", column = @Column(name = "addr_local_org")),
-    @AttributeOverride(name = "type", column = @Column(name = "addr_local_type")),
-    @AttributeOverride(
-        name = "address.streetLine1",
-        column = @Column(name = "addr_local_street_line1")),
-    @AttributeOverride(
-        name = "address.streetLine2",
-        column = @Column(name = "addr_local_street_line2")),
-    @AttributeOverride(
-        name = "address.streetLine3",
-        column = @Column(name = "addr_local_street_line3")),
-    @AttributeOverride(name = "address.city", column = @Column(name = "addr_local_city")),
-    @AttributeOverride(name = "address.state", column = @Column(name = "addr_local_state")),
-    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_local_zip")),
-    @AttributeOverride(
-        name = "address.countryCode",
-        column = @Column(name = "addr_local_country_code"))
-  })
-  PostalInfo localizedPostalInfo;
-
-  /**
-   * Internationalized postal info for the contact. Personal info; cleared by {@link
-   * Builder#wipeOut}.
-   */
-  @IgnoreSave(IfNull.class)
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "name", column = @Column(name = "addr_i18n_name")),
-    @AttributeOverride(name = "org", column = @Column(name = "addr_i18n_org")),
-    @AttributeOverride(name = "type", column = @Column(name = "addr_i18n_type")),
-    @AttributeOverride(
-        name = "address.streetLine1",
-        column = @Column(name = "addr_i18n_street_line1")),
-    @AttributeOverride(
-        name = "address.streetLine2",
-        column = @Column(name = "addr_i18n_street_line2")),
-    @AttributeOverride(
-        name = "address.streetLine3",
-        column = @Column(name = "addr_i18n_street_line3")),
-    @AttributeOverride(name = "address.city", column = @Column(name = "addr_i18n_city")),
-    @AttributeOverride(name = "address.state", column = @Column(name = "addr_i18n_state")),
-    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_i18n_zip")),
-    @AttributeOverride(
-        name = "address.countryCode",
-        column = @Column(name = "addr_i18n_country_code"))
-  })
-  PostalInfo internationalizedPostalInfo;
-
-  /**
-   * Contact name used for name searches. This is set automatically to be the internationalized
-   * postal name, or if null, the localized postal name, or if that is null as well, null. Personal
-   * info; cleared by {@link Builder#wipeOut}.
-   */
-  @Index
-  String searchName;
-
-  /** Contact’s voice number. Personal info; cleared by {@link Builder#wipeOut}. */
-  @IgnoreSave(IfNull.class)
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "phoneNumber", column = @Column(name = "voice_phone_number")),
-    @AttributeOverride(name = "extension", column = @Column(name = "voice_phone_extension")),
-  })
-  ContactPhoneNumber voice;
-
-  /** Contact’s fax number. Personal info; cleared by {@link Builder#wipeOut}. */
-  @IgnoreSave(IfNull.class)
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "phoneNumber", column = @Column(name = "fax_phone_number")),
-    @AttributeOverride(name = "extension", column = @Column(name = "fax_phone_extension")),
-  })
-  ContactPhoneNumber fax;
-
-  /** Contact’s email address. Personal info; cleared by {@link Builder#wipeOut}. */
-  @IgnoreSave(IfNull.class)
-  String email;
-
-  /** Authorization info (aka transfer secret) of the contact. */
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "pw.value", column = @Column(name = "auth_info_value")),
-    @AttributeOverride(name = "pw.repoId", column = @Column(name = "auth_info_repo_id")),
-  })
-  ContactAuthInfo authInfo;
-
-  /** Data about any pending or past transfers on this contact. */
-  ContactTransferData transferData;
-
-  /**
-   * The time that this resource was last transferred.
-   *
-   * <p>Can be null if the resource has never been transferred.
-   */
-  DateTime lastTransferTime;
-
-  // If any new fields are added which contain personal information, make sure they are cleared by
-  // the wipeOut() function, so that data is not kept around for deleted contacts.
-
-  /** Disclosure policy. */
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(name = "name", column = @Column(name = "disclose_types_name")),
-    @AttributeOverride(name = "org", column = @Column(name = "disclose_types_org")),
-    @AttributeOverride(name = "addr", column = @Column(name = "disclose_types_addr")),
-    @AttributeOverride(name = "flag", column = @Column(name = "disclose_mode_flag")),
-    @AttributeOverride(name = "voice.marked", column = @Column(name = "disclose_show_voice")),
-    @AttributeOverride(name = "fax.marked", column = @Column(name = "disclose_show_fax")),
-    @AttributeOverride(name = "email.marked", column = @Column(name = "disclose_show_email"))
-  })
-  Disclose disclose;
+public class ContactResource extends ContactBase
+    implements DatastoreAndSqlEntity, ForeignKeyedEppResource {
 
   @Override
   public VKey<ContactResource> createVKey() {
+    // TODO(mmuller): create symmetric keys if we can ever reload both sides.
     return VKey.create(ContactResource.class, getRepoId(), Key.create(this));
   }
 
@@ -210,81 +62,9 @@ public class ContactResource extends EppResource
     return super.getRepoId();
   }
 
-  public String getContactId() {
-    return contactId;
-  }
-
-  public PostalInfo getLocalizedPostalInfo() {
-    return localizedPostalInfo;
-  }
-
-  public PostalInfo getInternationalizedPostalInfo() {
-    return internationalizedPostalInfo;
-  }
-
-  public String getSearchName() {
-    return searchName;
-  }
-
-  public ContactPhoneNumber getVoiceNumber() {
-    return voice;
-  }
-
-  public ContactPhoneNumber getFaxNumber() {
-    return fax;
-  }
-
-  public String getEmailAddress() {
-    return email;
-  }
-
-  public ContactAuthInfo getAuthInfo() {
-    return authInfo;
-  }
-
-  public Disclose getDisclose() {
-    return disclose;
-  }
-
-  public final String getCurrentSponsorClientId() {
-    return getPersistedCurrentSponsorClientId();
-  }
-
-  @Override
-  public ContactTransferData getTransferData() {
-    return Optional.ofNullable(transferData).orElse(ContactTransferData.EMPTY);
-  }
-
-  @Override
-  public DateTime getLastTransferTime() {
-    return lastTransferTime;
-  }
-
-  @Override
-  public String getForeignKey() {
-    return contactId;
-  }
-
-  /**
-   * Postal info for the contact.
-   *
-   * <p>The XML marshalling expects the {@link PostalInfo} objects in a list, but we can't actually
-   * persist them to Datastore that way because Objectify can't handle collections of embedded
-   * objects that themselves contain collections, and there's a list of streets inside. This method
-   * transforms the persisted format to the XML format for marshalling.
-   */
-  @XmlElement(name = "postalInfo")
-  public ImmutableList<PostalInfo> getPostalInfosAsList() {
-    return Stream.of(localizedPostalInfo, internationalizedPostalInfo)
-        .filter(Objects::nonNull)
-        .collect(toImmutableList());
-  }
-
   @Override
   public ContactResource cloneProjectedAtTime(DateTime now) {
-    Builder builder = this.asBuilder();
-    projectResourceOntoBuilderAtTime(this, builder, now);
-    return builder.build();
+    return ContactBase.cloneContactProjectedAtTime(this, now);
   }
 
   @Override
@@ -293,116 +73,12 @@ public class ContactResource extends EppResource
   }
 
   /** A builder for constructing {@link ContactResource}, since it is immutable. */
-  public static class Builder extends EppResource.Builder<ContactResource, Builder>
-      implements BuilderWithTransferData<ContactTransferData, Builder> {
+  public static class Builder extends ContactBase.Builder<ContactResource, Builder> {
 
     public Builder() {}
 
     private Builder(ContactResource instance) {
       super(instance);
     }
-
-    public Builder setContactId(String contactId) {
-      getInstance().contactId = contactId;
-      return this;
-    }
-
-    public Builder setLocalizedPostalInfo(PostalInfo localizedPostalInfo) {
-      checkArgument(localizedPostalInfo == null
-          || Type.LOCALIZED.equals(localizedPostalInfo.getType()));
-      getInstance().localizedPostalInfo = localizedPostalInfo;
-      return this;
-    }
-
-    public Builder setInternationalizedPostalInfo(PostalInfo internationalizedPostalInfo) {
-      checkArgument(internationalizedPostalInfo == null
-          || Type.INTERNATIONALIZED.equals(internationalizedPostalInfo.getType()));
-      getInstance().internationalizedPostalInfo = internationalizedPostalInfo;
-      return this;
-    }
-
-    public Builder overlayLocalizedPostalInfo(PostalInfo localizedPostalInfo) {
-      return setLocalizedPostalInfo(getInstance().localizedPostalInfo == null
-          ? localizedPostalInfo
-          : getInstance().localizedPostalInfo.overlay(localizedPostalInfo));
-    }
-
-    public Builder overlayInternationalizedPostalInfo(PostalInfo internationalizedPostalInfo) {
-      return setInternationalizedPostalInfo(getInstance().internationalizedPostalInfo == null
-          ? internationalizedPostalInfo
-          : getInstance().internationalizedPostalInfo.overlay(internationalizedPostalInfo));
-    }
-
-    public Builder setVoiceNumber(ContactPhoneNumber voiceNumber) {
-      getInstance().voice = voiceNumber;
-      return this;
-    }
-
-    public Builder setFaxNumber(ContactPhoneNumber faxNumber) {
-      getInstance().fax = faxNumber;
-      return this;
-    }
-
-    public Builder setEmailAddress(String emailAddress) {
-      getInstance().email = emailAddress;
-      return this;
-    }
-
-    public Builder setAuthInfo(ContactAuthInfo authInfo) {
-      getInstance().authInfo = authInfo;
-      return this;
-    }
-
-    public Builder setDisclose(Disclose disclose) {
-      getInstance().disclose = disclose;
-      return this;
-    }
-
-    @Override
-    public Builder setTransferData(ContactTransferData transferData) {
-      getInstance().transferData = transferData;
-      return this;
-    }
-
-    @Override
-    public Builder setLastTransferTime(DateTime lastTransferTime) {
-      getInstance().lastTransferTime = lastTransferTime;
-      return thisCastToDerived();
-    }
-
-    /**
-     * Remove all personally identifying information about a contact.
-     *
-     * <p>This should be used when deleting a contact so that the soft-deleted entity doesn't
-     * contain information that the registrant requested to be deleted.
-     */
-    public Builder wipeOut() {
-      setEmailAddress(null);
-      setFaxNumber(null);
-      setInternationalizedPostalInfo(null);
-      setLocalizedPostalInfo(null);
-      setVoiceNumber(null);
-      return this;
-    }
-
-    @Override
-    public ContactResource build() {
-      ContactResource instance = getInstance();
-      // If TransferData is totally empty, set it to null.
-      if (ContactTransferData.EMPTY.equals(instance.transferData)) {
-        setTransferData(null);
-      }
-      // Set the searchName using the internationalized and localized postal info names.
-      if ((instance.internationalizedPostalInfo != null)
-          && (instance.internationalizedPostalInfo.getName() != null)) {
-        instance.searchName = instance.internationalizedPostalInfo.getName();
-      } else if ((instance.localizedPostalInfo != null)
-          && (instance.localizedPostalInfo.getName() != null)) {
-        instance.searchName = instance.localizedPostalInfo.getName();
-      } else {
-        instance.searchName = null;
-      }
-      return super.build();
-    }
   }
 }

core/src/main/java/google/registry/model/host/HostBase.java

@@ -125,7 +125,7 @@ public class HostBase extends EppResource {
   }
 
   @Override
-  public VKey<? extends EppResource> createVKey() {
+  public VKey<? extends HostBase> createVKey() {
     return VKey.create(HostBase.class, getRepoId(), Key.create(this));
   }
 

core/src/main/java/google/registry/model/host/HostHistory.java

@@ -73,7 +73,7 @@ public class HostHistory extends HistoryEntry {
       return this;
     }
 
-    public Builder setHostResourceId(VKey<HostResource> hostRepoId) {
+    public Builder setHostRepoId(VKey<HostResource> hostRepoId) {
       getInstance().hostRepoId = hostRepoId;
       hostRepoId.maybeGetOfyKey().ifPresent(parent -> getInstance().parent = parent);
       return this;
@@ -83,8 +83,7 @@ public class HostHistory extends HistoryEntry {
     @Override
     public Builder setParent(Key<? extends EppResource> parent) {
       super.setParent(parent);
-      getInstance().hostRepoId =
-          VKey.create(HostResource.class, parent.getName(), (Key<HostResource>) parent);
+      getInstance().hostRepoId = VKey.create(HostResource.class, parent.getName(), parent);
       return this;
     }
   }

core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java

@@ -168,7 +168,7 @@ class CommitLoggedWork<R> implements Runnable {
       DateTime transactionTime, Set<Entry<Key<BackupGroupRoot>, BackupGroupRoot>> bgrEntries) {
     ImmutableMap.Builder<Key<BackupGroupRoot>, DateTime> builder = new ImmutableMap.Builder<>();
     for (Entry<Key<BackupGroupRoot>, BackupGroupRoot> entry : bgrEntries) {
-      DateTime updateTime = entry.getValue().getUpdateAutoTimestamp().getTimestamp();
+      DateTime updateTime = entry.getValue().getUpdateTimestamp().getTimestamp();
       if (!updateTime.isBefore(transactionTime)) {
         builder.put(entry.getKey(), updateTime);
       }

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

@@ -15,6 +15,7 @@
 package google.registry.model.registry.label;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
@@ -30,6 +31,7 @@ import com.google.common.collect.Multiset;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Id;
+import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
@@ -42,6 +44,11 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import javax.annotation.Nullable;
+import javax.persistence.Column;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.MappedSuperclass;
+import javax.persistence.Transient;
 import org.joda.time.DateTime;
 
 /**
@@ -49,25 +56,46 @@ import org.joda.time.DateTime;
  *
  * @param <T> The type of the root value being listed, e.g. {@link ReservationType}.
  * @param <R> The type of domain label entry being listed, e.g. {@link ReservedListEntry} (note,
- *            must subclass {@link DomainLabelEntry}.
+ *     must subclass {@link DomainLabelEntry}.
  */
+@MappedSuperclass
 public abstract class BaseDomainLabelList<T extends Comparable<?>, R extends DomainLabelEntry<T, ?>>
     extends ImmutableObject implements Buildable {
 
+  @Ignore
+  @javax.persistence.Id
+  @GeneratedValue(strategy = GenerationType.IDENTITY)
+  Long revisionId;
+
   @Id
+  @Column(nullable = false)
   String name;
 
-  @Parent
-  Key<EntityGroupRoot> parent = getCrossTldKey();
+  @Parent @Transient Key<EntityGroupRoot> parent = getCrossTldKey();
 
-  DateTime creationTime;
+  @Transient DateTime creationTime;
 
+  // The list in Cloud SQL is immutable, we only have a creation_timestamp field and it should be
+  // set to the timestamp when the list is created. In Datastore, we have two fields and the
+  // lastUpdateTime is set to the current timestamp when creating and updating a list. So, we use
+  // lastUpdateTime as the creation_timestamp column during the dual-write phase for compatibility.
+  @Column(name = "creation_timestamp", nullable = false)
   DateTime lastUpdateTime;
 
+  /** Returns the ID of this revision, or throws if null. */
+  public long getRevisionId() {
+    checkState(
+        revisionId != null,
+        "revisionId is null because this object has not been persisted to the database yet");
+    return revisionId;
+  }
+
+  /** Returns the name of the reserved list. */
   public String getName() {
     return name;
   }
 
+  /** Returns the creation time of this revision of the reserved list. */
   public DateTime getCreationTime() {
     return creationTime;
   }
@@ -183,6 +211,9 @@ public abstract class BaseDomainLabelList<T extends Comparable<?>, R extends Dom
     @Override
     public T build() {
       checkArgument(!isNullOrEmpty(getInstance().name), "List must have a name");
+      // The list is immutable in Cloud SQL, so make sure the revision id is not set when the
+      // builder object is created from a list object
+      getInstance().revisionId = null;
       return super.build();
     }
   }

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

@@ -23,16 +23,20 @@ import com.google.common.net.InternetDomainName;
 import com.googlecode.objectify.annotation.Id;
 import google.registry.model.Buildable.GenericBuilder;
 import google.registry.model.ImmutableObject;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
 
 /**
  * Represents a label entry parsed from a line in a reserved/premium list txt file.
  *
  * @param <T> The type of the value stored for the domain label, e.g. {@link ReservationType}.
  */
+@MappedSuperclass
 public abstract class DomainLabelEntry<T extends Comparable<?>, D extends DomainLabelEntry<?, ?>>
     extends ImmutableObject implements Comparable<D> {
 
   @Id
+  @Column(name = "domain_label", insertable = false, updatable = false)
   String label;
 
   String comment;

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

@@ -16,11 +16,8 @@ package google.registry.model.registry.label;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
-import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -30,12 +27,8 @@ import com.google.common.base.Splitter;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.MapDifference;
-import com.google.common.collect.Maps;
-import com.google.common.flogger.FluentLogger;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Embed;
@@ -45,45 +38,58 @@ import com.googlecode.objectify.mapper.Mapper;
 import google.registry.model.Buildable;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.DomainLabelMetrics.MetricsReservedListMatch;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
-import google.registry.schema.tld.ReservedList.ReservedEntry;
-import google.registry.schema.tld.ReservedListDao;
+import google.registry.schema.replay.DatastoreAndSqlEntity;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import javax.annotation.Nullable;
+import javax.persistence.CollectionTable;
+import javax.persistence.Column;
+import javax.persistence.ElementCollection;
+import javax.persistence.Embeddable;
+import javax.persistence.Index;
+import javax.persistence.JoinColumn;
+import javax.persistence.MapKeyColumn;
+import javax.persistence.Table;
 import org.joda.time.DateTime;
 
 /**
- * A reserved list entity, persisted to Datastore, that is used to check domain label reservations.
+ * A list of reserved domain labels that are blocked from being registered for various reasons.
+ *
+ * <p>Note that the primary key of this entity is {@link #revisionId}, which is auto-generated by
+ * the database. So, if a retry of insertion happens after the previous attempt unexpectedly
+ * succeeds, we will end up with having two exact same reserved lists that differ only by
+ * revisionId. This is fine though, because we only use the list with the highest revisionId.
  */
 @Entity
+@javax.persistence.Entity
+@Table(indexes = {@Index(columnList = "name", name = "reservedlist_name_idx")})
 public final class ReservedList
     extends BaseDomainLabelList<ReservationType, ReservedList.ReservedListEntry>
-    implements DatastoreEntity {
-
-  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+    implements DatastoreAndSqlEntity {
 
   @Mapify(ReservedListEntry.LabelMapper.class)
+  @ElementCollection
+  @CollectionTable(
+      name = "ReservedEntry",
+      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
+  @MapKeyColumn(name = "domain_label")
   Map<String, ReservedListEntry> reservedListMap;
 
+  @Column(nullable = false)
   boolean shouldPublish = true;
 
-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // ReservedList is dual-written
-  }
-
   /**
    * A reserved list entry entity, persisted to Datastore, that represents a single label and its
    * reservation type.
    */
   @Embed
-  public static class ReservedListEntry
-      extends DomainLabelEntry<ReservationType, ReservedListEntry> implements Buildable {
+  @Embeddable
+  public static class ReservedListEntry extends DomainLabelEntry<ReservationType, ReservedListEntry>
+      implements Buildable {
 
+    @Column(nullable = false)
     ReservationType reservationType;
 
     /** Mapper for use with @Mapify */
@@ -150,6 +156,7 @@ public final class ReservedList
     return shouldPublish;
   }
 
+  /** Returns a {@link Map} of domain labels to {@link ReservedListEntry}. */
   public ImmutableMap<String, ReservedListEntry> getReservedListEntries() {
     return ImmutableMap.copyOf(nullToEmpty(reservedListMap));
   }
@@ -239,65 +246,10 @@ public final class ReservedList
               new CacheLoader<String, ReservedList>() {
                 @Override
                 public ReservedList load(String listName) {
-                  ReservedList datastoreList =
-                      ofy()
-                          .load()
-                          .type(ReservedList.class)
-                          .parent(getCrossTldKey())
-                          .id(listName)
-                          .now();
-                  // Also load the list from Cloud SQL, compare the two lists, and log if different.
-                  try {
-                    loadAndCompareCloudSqlList(datastoreList);
-                  } catch (Throwable t) {
-                    logger.atSevere().withCause(t).log("Error comparing reserved lists.");
-                  }
-                  return datastoreList;
+                  return ReservedListDualWriteDao.getLatestRevision(listName).orElse(null);
                 }
               });
 
-  private static final void loadAndCompareCloudSqlList(ReservedList datastoreList) {
-    Optional<google.registry.schema.tld.ReservedList> maybeCloudSqlList =
-        ReservedListDao.getLatestRevision(datastoreList.getName());
-    if (maybeCloudSqlList.isPresent()) {
-      Map<String, ReservedEntry> datastoreLabelsToReservations =
-          datastoreList.reservedListMap.entrySet().parallelStream()
-              .collect(
-                  toImmutableMap(
-                      Map.Entry::getKey,
-                      entry ->
-                          ReservedEntry.create(
-                              entry.getValue().reservationType, entry.getValue().comment)));
-
-      google.registry.schema.tld.ReservedList cloudSqlList = maybeCloudSqlList.get();
-      MapDifference<String, ReservedEntry> diff =
-          Maps.difference(datastoreLabelsToReservations, cloudSqlList.getLabelsToReservations());
-      if (!diff.areEqual()) {
-        if (diff.entriesDiffering().size() > 10) {
-          logger.atWarning().log(
-              String.format(
-                  "Unequal reserved lists detected, Cloud SQL list with revision"
-                      + " id %d has %d different records than the current"
-                      + " Datastore list.",
-                  cloudSqlList.getRevisionId(), diff.entriesDiffering().size()));
-        } else {
-          StringBuilder diffMessage = new StringBuilder("Unequal reserved lists detected:\n");
-          diff.entriesDiffering()
-              .forEach(
-                  (label, valueDiff) ->
-                      diffMessage.append(
-                          String.format(
-                              "Domain label %s has entry %s in Datastore and entry"
-                                  + " %s in Cloud SQL.\n",
-                              label, valueDiff.leftValue(), valueDiff.rightValue())));
-          logger.atWarning().log(diffMessage.toString());
-        }
-      }
-    } else {
-      logger.atWarning().log("Reserved list in Cloud SQL is empty.");
-    }
-  }
-
   /**
    * Gets the {@link ReservationType} of a label in a single ReservedList, or returns an absent
    * Optional if none exists in the list.

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

@@ -0,0 +1,124 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model.registry.label;
+
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
+
+import com.google.common.collect.MapDifference;
+import com.google.common.collect.MapDifference.ValueDifference;
+import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
+import com.googlecode.objectify.Key;
+import google.registry.model.registry.label.ReservedList.ReservedListEntry;
+import google.registry.persistence.VKey;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * A {@link ReservedList} DAO that does dual-write and dual-read against Datastore and Cloud SQL. It
+ * still uses Datastore as the primary storage and suppresses any exception thrown by Cloud SQL.
+ *
+ * <p>TODO(b/160993806): Delete this DAO and switch to use the SQL only DAO after migrating to Cloud
+ * SQL.
+ */
+public class ReservedListDualWriteDao {
+
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private ReservedListDualWriteDao() {}
+
+  /** Persist a new reserved list to Cloud SQL. */
+  public static void save(ReservedList reservedList) {
+    ofyTm().transact(() -> ofyTm().saveNewOrUpdate(reservedList));
+    try {
+      logger.atInfo().log("Saving reserved list %s to Cloud SQL", reservedList.getName());
+      ReservedListSqlDao.save(reservedList);
+      logger.atInfo().log(
+          "Saved reserved list %s with %d entries to Cloud SQL",
+          reservedList.getName(), reservedList.getReservedListEntries().size());
+    } catch (Throwable t) {
+      logger.atSevere().withCause(t).log("Error saving the reserved list to Cloud SQL.");
+    }
+  }
+
+  /**
+   * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
+   * exists.
+   */
+  public static Optional<ReservedList> getLatestRevision(String reservedListName) {
+    Optional<ReservedList> maybeDatastoreList =
+        ofyTm()
+            .maybeLoad(
+                VKey.createOfy(
+                    ReservedList.class,
+                    Key.create(getCrossTldKey(), ReservedList.class, reservedListName)));
+    try {
+      // Also load the list from Cloud SQL, compare the two lists, and log if different.
+      maybeDatastoreList.ifPresent(ReservedListDualWriteDao::loadAndCompareCloudSqlList);
+    } catch (Throwable t) {
+      logger.atSevere().withCause(t).log("Error comparing reserved lists.");
+    }
+    return maybeDatastoreList;
+  }
+
+  private static void loadAndCompareCloudSqlList(ReservedList datastoreList) {
+    Optional<ReservedList> maybeCloudSqlList =
+        ReservedListSqlDao.getLatestRevision(datastoreList.getName());
+    if (maybeCloudSqlList.isPresent()) {
+      Map<String, ReservedListEntry> datastoreLabelsToReservations =
+          datastoreList.reservedListMap.entrySet().parallelStream()
+              .collect(
+                  toImmutableMap(
+                      Map.Entry::getKey,
+                      entry ->
+                          ReservedListEntry.create(
+                              entry.getKey(),
+                              entry.getValue().reservationType,
+                              entry.getValue().comment)));
+
+      ReservedList cloudSqlList = maybeCloudSqlList.get();
+      MapDifference<String, ReservedListEntry> diff =
+          Maps.difference(datastoreLabelsToReservations, cloudSqlList.reservedListMap);
+      if (!diff.areEqual()) {
+        if (diff.entriesDiffering().size() > 10) {
+          logger.atWarning().log(
+              String.format(
+                  "Unequal reserved lists detected, Cloud SQL list with revision"
+                      + " id %d has %d different records than the current"
+                      + " Datastore list.",
+                  cloudSqlList.getRevisionId(), diff.entriesDiffering().size()));
+        } else {
+          StringBuilder diffMessage = new StringBuilder("Unequal reserved lists detected:\n");
+          diff.entriesDiffering().entrySet().stream()
+              .forEach(
+                  entry -> {
+                    String label = entry.getKey();
+                    ValueDifference<ReservedListEntry> valueDiff = entry.getValue();
+                    diffMessage.append(
+                        String.format(
+                            "Domain label %s has entry %s in Datastore and entry"
+                                + " %s in Cloud SQL.\n",
+                            label, valueDiff.leftValue(), valueDiff.rightValue()));
+                  });
+          logger.atWarning().log(diffMessage.toString());
+        }
+      }
+    } else {
+      logger.atWarning().log("Reserved list in Cloud SQL is empty.");
+    }
+  }
+}

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

@@ -12,20 +12,46 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package google.registry.schema.tld;
+package google.registry.model.registry.label;
 
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
-import com.google.common.util.concurrent.UncheckedExecutionException;
 import java.util.Optional;
-import java.util.concurrent.ExecutionException;
 
-/** Data access object class for {@link ReservedList} */
-public class ReservedListDao {
+/**
+ * A {@link ReservedList} DAO for Cloud SQL.
+ *
+ * <p>TODO(b/160993806): Rename this class to ReservedListDao after migrating to Cloud SQL.
+ */
+public class ReservedListSqlDao {
+
+  private ReservedListSqlDao() {}
 
   /** Persist a new reserved list to Cloud SQL. */
   public static void save(ReservedList reservedList) {
-    jpaTm().transact(() -> jpaTm().getEntityManager().persist(reservedList));
+    checkArgumentNotNull(reservedList, "Must specify reservedList");
+    jpaTm().transact(() -> jpaTm().saveNew(reservedList));
+  }
+
+  /**
+   * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
+   * exists.
+   */
+  public static Optional<ReservedList> getLatestRevision(String reservedListName) {
+    return jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                    .getEntityManager()
+                    .createQuery(
+                        "FROM ReservedList rl LEFT JOIN FETCH rl.reservedListMap WHERE"
+                            + " rl.revisionId IN (SELECT MAX(revisionId) FROM ReservedList subrl"
+                            + " WHERE subrl.name = :name)",
+                        ReservedList.class)
+                    .setParameter("name", reservedListName)
+                    .getResultStream()
+                    .findFirst());
   }
 
   /**
@@ -46,39 +72,4 @@ public class ReservedListDao {
                         .size()
                     > 0);
   }
-
-  /**
-   * Returns the most recent revision of the {@link ReservedList} with the specified name, if it
-   * exists. TODO(shicong): Change this method to package level access after dual-read phase.
-   */
-  public static Optional<ReservedList> getLatestRevision(String reservedListName) {
-    return jpaTm()
-        .transact(
-            () ->
-                jpaTm()
-                    .getEntityManager()
-                    .createQuery(
-                        "FROM ReservedList rl LEFT JOIN FETCH rl.labelsToReservations WHERE"
-                            + " rl.revisionId IN (SELECT MAX(revisionId) FROM ReservedList subrl"
-                            + " WHERE subrl.name = :name)",
-                        ReservedList.class)
-                    .setParameter("name", reservedListName)
-                    .getResultStream()
-                    .findFirst());
-  }
-
-  /**
-   * Returns the most recent revision of the {@link ReservedList} with the specified name, from
-   * cache.
-   */
-  public static Optional<ReservedList> getLatestRevisionCached(String reservedListName) {
-    try {
-      return ReservedListCache.cacheReservedLists.get(reservedListName);
-    } catch (ExecutionException e) {
-      throw new UncheckedExecutionException(
-          "Could not retrieve reserved list named " + reservedListName, e);
-    }
-  }
-
-  private ReservedListDao() {}
 }

core/src/main/java/google/registry/model/reporting/HistoryEntry.java

@@ -185,6 +185,10 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
   @Transient // domain-specific
   Set<DomainTransactionRecord> domainTransactionRecords;
 
+  public Long getId() {
+    return id;
+  }
+
   public Key<? extends EppResource> getParent() {
     return parent;
   }

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

@@ -94,6 +94,21 @@ public class PersistenceModule {
       @Config("cloudSqlJdbcUrl") String jdbcUrl,
       @Config("cloudSqlInstanceConnectionName") String instanceConnectionName,
       @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs) {
+    return createPartialSqlConfigs(jdbcUrl, instanceConnectionName, defaultConfigs);
+  }
+
+  @Provides
+  @Singleton
+  @BeamPipelineCloudSqlConfigs
+  static ImmutableMap<String, String> provideBeamPipelineCloudSqlConfigs(
+      @Config("beamCloudSqlJdbcUrl") String jdbcUrl,
+      @Config("beamCloudSqlInstanceConnectionName") String instanceConnectionName,
+      @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs) {
+    return createPartialSqlConfigs(jdbcUrl, instanceConnectionName, defaultConfigs);
+  }
+
+  private static ImmutableMap<String, String> createPartialSqlConfigs(
+      String jdbcUrl, String instanceConnectionName, ImmutableMap<String, String> defaultConfigs) {
     HashMap<String, String> overrides = Maps.newHashMap(defaultConfigs);
     overrides.put(Environment.URL, jdbcUrl);
     overrides.put(HIKARI_DS_SOCKET_FACTORY, "com.google.cloud.sql.postgres.SocketFactory");
@@ -135,13 +150,15 @@ public class PersistenceModule {
   @Singleton
   @SocketFactoryJpaTm
   static JpaTransactionManager provideSocketFactoryJpaTm(
-      @Config("cloudSqlUsername") String username,
-      @Config("cloudSqlPassword") String password,
-      @PartialCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
+      @Config("beamCloudSqlUsername") String username,
+      @Config("beamCloudSqlPassword") String password,
+      @Config("beamHibernateHikariMaximumPoolSize") int hikariMaximumPoolSize,
+      @BeamPipelineCloudSqlConfigs ImmutableMap<String, String> cloudSqlConfigs,
       Clock clock) {
     HashMap<String, String> overrides = Maps.newHashMap(cloudSqlConfigs);
     overrides.put(Environment.USER, username);
     overrides.put(Environment.PASS, password);
+    overrides.put(HIKARI_MAXIMUM_POOL_SIZE, String.valueOf(hikariMaximumPoolSize));
     return new JpaTransactionManagerImpl(create(overrides), clock);
   }
 
@@ -149,9 +166,9 @@ public class PersistenceModule {
   @Singleton
   @JdbcJpaTm
   static JpaTransactionManager provideLocalJpaTm(
-      @Config("cloudSqlJdbcUrl") String jdbcUrl,
-      @Config("cloudSqlUsername") String username,
-      @Config("cloudSqlPassword") String password,
+      @Config("beamCloudSqlJdbcUrl") String jdbcUrl,
+      @Config("beamCloudSqlUsername") String username,
+      @Config("beamCloudSqlPassword") String password,
       @DefaultHibernateConfigs ImmutableMap<String, String> defaultConfigs,
       Clock clock) {
     HashMap<String, String> overrides = Maps.newHashMap(defaultConfigs);
@@ -218,6 +235,11 @@ public class PersistenceModule {
   @Documented
   @interface PartialCloudSqlConfigs {}
 
+  /** Dagger qualifier for the Cloud SQL configs used by Beam pipelines. */
+  @Qualifier
+  @Documented
+  @interface BeamPipelineCloudSqlConfigs {}
+
   /** Dagger qualifier for the default Hibernate configurations. */
   // TODO(shicong): Change annotations in this class to none public or put them in a top level
   //  package

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

@@ -58,6 +58,31 @@ public class VKey<T> extends ImmutableObject implements Serializable {
     return new VKey(kind, null, sqlKey);
   }
 
+  /** Creates a {@link VKey} which only contains the ofy primary key. */
+  public static <T> VKey<T> createOfy(
+      Class<? extends T> kind, com.googlecode.objectify.Key<? extends T> ofyKey) {
+    checkArgumentNotNull(kind, "kind must not be null");
+    checkArgumentNotNull(ofyKey, "ofyKey must not be null");
+    return new VKey(kind, ofyKey, null);
+  }
+
+  /**
+   * Creates a {@link VKey} which only contains the ofy primary key by specifying the id of the
+   * {@link Key}.
+   */
+  public static <T> VKey<T> createOfy(Class<? extends T> kind, long id) {
+    return createOfy(kind, Key.create(kind, id));
+  }
+
+  /**
+   * Creates a {@link VKey} which only contains the ofy primary key by specifying the name of the
+   * {@link Key}.
+   */
+  public static <T> VKey<T> createOfy(Class<? extends T> kind, String name) {
+    checkArgumentNotNull(kind, "name must not be null");
+    return createOfy(kind, Key.create(kind, name));
+  }
+
   /** Creates a {@link VKey} which only contains both sql and ofy primary key. */
   public static <T> VKey<T> create(
       Class<? extends T> kind, Object sqlKey, com.googlecode.objectify.Key ofyKey) {

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java

@@ -25,4 +25,11 @@ public interface JpaTransactionManager extends TransactionManager {
 
   /** Deletes the entity by its id, throws exception if the entity is not deleted. */
   public abstract <T> void assertDelete(VKey<T> key);
+
+  /**
+   * Releases all resources and shuts down.
+   *
+   * <p>The errorprone check forbids injection of {@link java.io.Closeable} resources.
+   */
+  void teardown();
 }

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java

@@ -62,6 +62,11 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
     this.clock = clock;
   }
 
+  @Override
+  public void teardown() {
+    emf.close();
+  }
+
   @Override
   public EntityManager getEntityManager() {
     if (transactionInfo.get().entityManager == null) {
@@ -101,7 +106,8 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
       T result = work.get();
       txn.commit();
       return result;
-    } catch (RuntimeException e) {
+    } catch (RuntimeException | Error e) {
+      // Error is unchecked!
       try {
         txn.rollback();
         logger.atWarning().log("Error during transaction; transaction rolled back");

core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java

@@ -75,7 +75,12 @@ public class TransactionManagerFactory {
     return tm;
   }
 
-  /** Returns {@link JpaTransactionManager} instance. */
+  /**
+   * Returns {@link JpaTransactionManager} instance.
+   *
+   * <p>Between invocations of {@link TransactionManagerFactory#setJpaTm} every call to this method
+   * returns the same instance.
+   */
   public static JpaTransactionManager jpaTm() {
     return jpaTm.get();
   }
@@ -93,7 +98,7 @@ public class TransactionManagerFactory {
         RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST)
             || RegistryToolEnvironment.get() != null,
         "setJpamTm() should only be called by tools and tests.");
-    jpaTm = jpaTmSupplier;
+    jpaTm = Suppliers.memoize(jpaTmSupplier::get);
   }
 
   /** Sets the return of {@link #tm()} to the given instance of {@link TransactionManager}. */

core/src/main/java/google/registry/rde/RdeModule.java

@@ -89,7 +89,7 @@ public abstract class RdeModule {
   @Provides
   @Parameter(PARAM_LENIENT)
   static boolean provideLenient(HttpServletRequest req) {
-    return extractBooleanParameter(req, PARAM_REVISION);
+    return extractBooleanParameter(req, PARAM_LENIENT);
   }
 
   @Provides

core/src/main/java/google/registry/schema/tld/ReservedList.java

@@ -1,177 +0,0 @@
-// Copyright 2019 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.schema.tld;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.collect.ImmutableList.sortedCopyOf;
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
-
-import com.google.common.base.Joiner;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import google.registry.model.CreateAutoTimestamp;
-import google.registry.model.ImmutableObject;
-import google.registry.model.registry.label.ReservationType;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
-import java.util.Map;
-import java.util.stream.Stream;
-import javax.annotation.Nullable;
-import javax.persistence.CollectionTable;
-import javax.persistence.Column;
-import javax.persistence.ElementCollection;
-import javax.persistence.Embeddable;
-import javax.persistence.Entity;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
-import javax.persistence.Id;
-import javax.persistence.Index;
-import javax.persistence.JoinColumn;
-import javax.persistence.MapKeyColumn;
-import javax.persistence.Table;
-import org.joda.time.DateTime;
-
-/**
- * A list of reserved domain labels that are blocked from being registered for various reasons.
- *
- * <p>Note that the primary key of this entity is {@link #revisionId}, which is auto-generated by
- * the database. So, if a retry of insertion happens after the previous attempt unexpectedly
- * succeeds, we will end up with having two exact same reserved lists that differ only by
- * revisionId. This is fine though, because we only use the list with the highest revisionId.
- */
-@Entity
-@Table(indexes = {@Index(columnList = "name", name = "reservedlist_name_idx")})
-public class ReservedList extends ImmutableObject implements SqlEntity {
-
-  @Column(nullable = false)
-  private String name;
-
-  @Id
-  @GeneratedValue(strategy = GenerationType.IDENTITY)
-  @Column(nullable = false)
-  private Long revisionId;
-
-  @Column(nullable = false)
-  private CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
-
-  @Column(nullable = false)
-  private Boolean shouldPublish;
-
-  @ElementCollection
-  @CollectionTable(
-      name = "ReservedEntry",
-      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
-  @MapKeyColumn(name = "domainLabel")
-  private Map<String, ReservedEntry> labelsToReservations;
-
-  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // ReservedList is dual-written\
-  }
-
-  @Embeddable
-  public static class ReservedEntry extends ImmutableObject {
-    @Column(nullable = false)
-    private ReservationType reservationType;
-
-    @Column(nullable = true)
-    private String comment;
-
-    private ReservedEntry(ReservationType reservationType, @Nullable String comment) {
-      this.reservationType = reservationType;
-      this.comment = comment;
-    }
-
-    // Hibernate requires this default constructor.
-    private ReservedEntry() {}
-
-    /** Constructs a {@link ReservedEntry} object. */
-    public static ReservedEntry create(ReservationType reservationType, @Nullable String comment) {
-      return new ReservedEntry(reservationType, comment);
-    }
-
-    /** Returns the reservation type for this entry. */
-    public ReservationType getReservationType() {
-      return reservationType;
-    }
-
-    /** Returns the comment for this entry. Retruns null if there is no comment. */
-    public String getComment() {
-      return comment;
-    }
-  }
-
-  private ReservedList(
-      String name, Boolean shouldPublish, Map<String, ReservedEntry> labelsToReservations) {
-    this.name = name;
-    this.shouldPublish = shouldPublish;
-    this.labelsToReservations = labelsToReservations;
-  }
-
-  // Hibernate requires this default constructor.
-  private ReservedList() {}
-
-  /** Constructs a {@link ReservedList} object. */
-  public static ReservedList create(
-      String name, Boolean shouldPublish, Map<String, ReservedEntry> labelsToReservations) {
-    ImmutableList<String> invalidLabels =
-        labelsToReservations.entrySet().parallelStream()
-            .flatMap(
-                entry -> {
-                  String label = entry.getKey();
-                  if (label.equals(canonicalizeDomainName(label))) {
-                    return Stream.empty();
-                  } else {
-                    return Stream.of(label);
-                  }
-                })
-            .collect(toImmutableList());
-    checkArgument(
-        invalidLabels.isEmpty(),
-        "Label(s) [%s] must be in puny-coded, lower-case form",
-        Joiner.on(",").join(sortedCopyOf(invalidLabels)));
-    return new ReservedList(name, shouldPublish, labelsToReservations);
-  }
-
-  /** Returns the name of the reserved list. */
-  public String getName() {
-    return name;
-  }
-
-  /** Returns the ID of this revision, or throws if null. */
-  public Long getRevisionId() {
-    checkState(
-        revisionId != null,
-        "revisionId is null because this object has not been persisted to the database yet");
-    return revisionId;
-  }
-
-  /** Returns the creation time of this revision of the reserved list. */
-  public DateTime getCreationTimestamp() {
-    return creationTimestamp.getTimestamp();
-  }
-
-  /** Returns a {@link Map} of domain labels to {@link ReservedEntry}. */
-  public ImmutableMap<String, ReservedEntry> getLabelsToReservations() {
-    return ImmutableMap.copyOf(labelsToReservations);
-  }
-
-  /** Returns true if the reserved list should be published. */
-  public Boolean getShouldPublish() {
-    return shouldPublish;
-  }
-}

core/src/main/java/google/registry/schema/tld/ReservedListCache.java

@@ -1,54 +0,0 @@
-// Copyright 2019 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.schema.tld;
-
-import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import google.registry.util.NonFinalForTesting;
-import java.util.Optional;
-import org.joda.time.Duration;
-
-/** Caching utils for {@link ReservedList} */
-public class ReservedListCache {
-
-  /**
-   * In-memory cache for reserved lists.
-   *
-   * <p>This is cached for a shorter duration because we need to periodically reload from the DB to
-   * check if a new revision has been published, and if so, then use that.
-   */
-  @NonFinalForTesting
-  static LoadingCache<String, Optional<ReservedList>> cacheReservedLists =
-      createCacheReservedLists(getDomainLabelListCacheDuration());
-
-  @VisibleForTesting
-  static LoadingCache<String, Optional<ReservedList>> createCacheReservedLists(
-      Duration cachePersistDuration) {
-    return CacheBuilder.newBuilder()
-        .expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
-        .build(
-            new CacheLoader<String, Optional<ReservedList>>() {
-              @Override
-              public Optional<ReservedList> load(String reservedListName) {
-                return ReservedListDao.getLatestRevision(reservedListName);
-              }
-            });
-  }
-}

core/src/main/java/google/registry/tools/AckPollMessagesCommand.java

@@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.cmd.QueryKeys;
+import google.registry.flows.poll.PollFlowUtils;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.poll.PollMessage.Autorenew;
 import google.registry.model.poll.PollMessage.OneTime;
@@ -49,8 +50,7 @@ import javax.inject.Inject;
  * delete confirmations) and it is desired that the registrar be able to ACK the rest of the poll
  * messages in-band.
  *
- * <p>This command only ACKs {@link OneTime} poll messages because that's our only use case so far,
- * but it could be extended to ACK {@link Autorenew} poll messages as well if needed. The main
+ * <p>This command ACKs both {@link OneTime} and {@link Autorenew} poll messages. The main
  * difference is that one-time poll messages are deleted when ACKed whereas Autorenews are sometimes
  * modified and re-saved instead, if the corresponding domain is still active.
  *
@@ -85,18 +85,17 @@ final class AckPollMessagesCommand implements CommandWithRemoteApi {
   @Override
   public void run() {
     QueryKeys<PollMessage> query = getPollMessagesQuery(clientId, clock.nowUtc()).keys();
+    // TODO(b/160325686): Remove the batch logic after db migration.
     for (List<Key<PollMessage>> keys : Iterables.partition(query, BATCH_SIZE)) {
-      tm()
-          .transact(
+      tm().transact(
               () -> {
                 // Load poll messages and filter to just those of interest.
                 ImmutableList<PollMessage> pollMessages =
                     ofy().load().keys(keys).values().stream()
-                        .filter(pm -> pm instanceof OneTime)
                         .filter(pm -> isNullOrEmpty(message) || pm.getMsg().contains(message))
                         .collect(toImmutableList());
                 if (!dryRun) {
-                  ofy().delete().entities(pollMessages).now();
+                  pollMessages.forEach(PollFlowUtils::ackPollMessage);
                 }
                 pollMessages.forEach(
                     pm ->

core/src/main/java/google/registry/tools/AuthModule.java

@@ -50,7 +50,6 @@ import java.lang.annotation.RetentionPolicy;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import javax.annotation.Nullable;
-import javax.inject.Named;
 import javax.inject.Qualifier;
 import javax.inject.Singleton;
 
@@ -151,10 +150,10 @@ public class AuthModule {
   public static String provideLocalCredentialJson(
       Lazy<GoogleClientSecrets> clientSecrets,
       @StoredCredential Lazy<Credential> credential,
-      @Nullable @Named("credentialFileName") String credentialFilename) {
+      @Nullable @Config("credentialFilePath") String credentialFilePath) {
     try {
-      if (credentialFilename != null) {
-        return new String(Files.readAllBytes(Paths.get(credentialFilename)), UTF_8);
+      if (credentialFilePath != null) {
+        return new String(Files.readAllBytes(Paths.get(credentialFilePath)), UTF_8);
       } else {
         return new Gson()
             .toJson(

core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java

@@ -14,23 +14,12 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static google.registry.model.registry.label.BaseDomainLabelList.splitOnComment;
-import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
-
 import com.beust.jcommander.Parameter;
-import com.google.common.base.Splitter;
-import com.google.common.collect.HashMultiset;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Multiset;
 import com.google.common.flogger.FluentLogger;
-import google.registry.model.registry.label.ReservationType;
-import google.registry.schema.tld.ReservedList.ReservedEntry;
+import google.registry.model.registry.label.ReservedList;
+import google.registry.model.registry.label.ReservedListDualWriteDao;
 import google.registry.tools.params.PathParameter;
 import java.nio.file.Path;
-import java.util.List;
-import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
@@ -62,69 +51,22 @@ public abstract class CreateOrUpdateReservedListCommand extends MutatingCommand
       arity = 1)
   Boolean shouldPublish;
 
-  google.registry.schema.tld.ReservedList cloudSqlReservedList;
-
-  abstract void saveToCloudSql();
+  ReservedList reservedList;
 
   @Override
-  protected String execute() throws Exception {
-    // Save the list to Datastore and output its response.
-    String output = super.execute();
-    logger.atInfo().log(output);
-
-    String cloudSqlMessage =
+  protected String execute() {
+    String message =
         String.format(
             "Saved reserved list %s with %d entries",
-            name, cloudSqlReservedList.getLabelsToReservations().size());
+            name, reservedList.getReservedListEntries().size());
     try {
-      logger.atInfo().log("Saving reserved list to Cloud SQL for TLD %s", name);
-      saveToCloudSql();
-      logger.atInfo().log(cloudSqlMessage);
+      logger.atInfo().log("Saving reserved list for TLD %s", name);
+      ReservedListDualWriteDao.save(reservedList);
+      logger.atInfo().log(message);
     } catch (Throwable e) {
-      cloudSqlMessage =
-          "Unexpected error saving reserved list to Cloud SQL from nomulus tool command";
-      logger.atSevere().withCause(e).log(cloudSqlMessage);
+      message = "Unexpected error saving reserved list from nomulus tool command";
+      logger.atSevere().withCause(e).log(message);
     }
-    return cloudSqlMessage;
-  }
-
-  /** Turns the list CSV data into a map of labels to {@link ReservedEntry}. */
-  static ImmutableMap<String, ReservedEntry> parseToReservationsByLabels(Iterable<String> lines) {
-    Map<String, ReservedEntry> labelsToEntries = Maps.newHashMap();
-    Multiset<String> duplicateLabels = HashMultiset.create();
-    for (String originalLine : lines) {
-      List<String> lineAndComment = splitOnComment(originalLine);
-      if (lineAndComment.isEmpty()) {
-        continue;
-      }
-      String line = lineAndComment.get(0);
-      String comment = lineAndComment.get(1);
-      List<String> parts = Splitter.on(',').trimResults().splitToList(line);
-      checkArgument(
-          parts.size() == 2 || parts.size() == 3,
-          "Could not parse line in reserved list: %s",
-          originalLine);
-      String label = parts.get(0);
-      checkArgument(
-          label.equals(canonicalizeDomainName(label)),
-          "Label '%s' must be in puny-coded, lower-case form",
-          label);
-      ReservationType reservationType = ReservationType.valueOf(parts.get(1));
-      ReservedEntry reservedEntry = ReservedEntry.create(reservationType, comment);
-      // Check if the label was already processed for this list (which is an error), and if so,
-      // accumulate it so that a list of all duplicates can be thrown.
-      if (labelsToEntries.containsKey(label)) {
-        duplicateLabels.add(label, duplicateLabels.contains(label) ? 1 : 2);
-      } else {
-        labelsToEntries.put(label, reservedEntry);
-      }
-    }
-    if (!duplicateLabels.isEmpty()) {
-      throw new IllegalStateException(
-          String.format(
-              "Reserved list cannot contain duplicate labels. Dupes (with counts) were: %s",
-              duplicateLabels));
-    }
-    return ImmutableMap.copyOf(labelsToEntries);
+    return message;
   }
 }

core/src/main/java/google/registry/tools/CreateReservedListCommand.java

@@ -16,7 +16,6 @@ package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static google.registry.model.registry.Registries.assertTldExists;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.ListNamingUtils.convertFilePathToName;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.joda.time.DateTimeZone.UTC;
@@ -27,7 +26,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import google.registry.model.registry.label.ReservedList;
-import google.registry.schema.tld.ReservedListDao;
 import java.nio.file.Files;
 import java.util.List;
 import org.joda.time.DateTime;
@@ -50,15 +48,14 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
   protected void init() throws Exception {
     name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name;
     checkArgument(
-        !ReservedList.get(name).isPresent(),
-        "A reserved list already exists by this name");
+        !ReservedList.get(name).isPresent(), "A reserved list already exists by this name");
     if (!override) {
       validateListName(name);
     }
     DateTime now = DateTime.now(UTC);
     List<String> allLines = Files.readAllLines(input, UTF_8);
     boolean shouldPublish = this.shouldPublish == null || this.shouldPublish;
-    ReservedList reservedList =
+    reservedList =
         new ReservedList.Builder()
             .setName(name)
             .setReservedListMapFromLines(allLines)
@@ -66,23 +63,6 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
             .setCreationTime(now)
             .setLastUpdateTime(now)
             .build();
-    stageEntityChange(null, reservedList);
-    cloudSqlReservedList =
-        google.registry.schema.tld.ReservedList.create(
-            name, shouldPublish, parseToReservationsByLabels(allLines));
-  }
-
-  @Override
-  void saveToCloudSql() {
-    jpaTm()
-        .transact(
-            () -> {
-              checkArgument(
-                  !ReservedListDao.checkExists(cloudSqlReservedList.getName()),
-                  "A reserved list of this name already exists: %s.",
-                  cloudSqlReservedList.getName());
-              ReservedListDao.save(cloudSqlReservedList);
-            });
   }
 
   private static void validateListName(String name) {

core/src/main/java/google/registry/tools/DomainLockUtils.java

@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.googlecode.objectify.Key;
 import google.registry.batch.AsyncTaskEnqueuer;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.domain.DomainBase;
@@ -52,13 +53,16 @@ public final class DomainLockUtils {
   private static final int VERIFICATION_CODE_LENGTH = 32;
 
   private final StringGenerator stringGenerator;
+  private final String registryAdminRegistrarId;
   private final AsyncTaskEnqueuer asyncTaskEnqueuer;
 
   @Inject
   public DomainLockUtils(
       @Named("base58StringGenerator") StringGenerator stringGenerator,
+      @Config("registryAdminClientId") String registryAdminRegistrarId,
       AsyncTaskEnqueuer asyncTaskEnqueuer) {
     this.stringGenerator = stringGenerator;
+    this.registryAdminRegistrarId = registryAdminRegistrarId;
     this.asyncTaskEnqueuer = asyncTaskEnqueuer;
   }
 
@@ -217,7 +221,7 @@ public final class DomainLockUtils {
   private RegistryLock.Builder createLockBuilder(
       String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) {
     DateTime now = jpaTm().getTransactionTime();
-    DomainBase domainBase = getDomain(domainName, now);
+    DomainBase domainBase = getDomain(domainName, registrarId, now);
     verifyDomainNotLocked(domainBase);
 
     // Multiple pending actions are not allowed
@@ -242,7 +246,7 @@ public final class DomainLockUtils {
   private RegistryLock.Builder createUnlockBuilder(
       String domainName, String registrarId, boolean isAdmin, Optional<Duration> relockDuration) {
     DateTime now = jpaTm().getTransactionTime();
-    DomainBase domainBase = getDomain(domainName, now);
+    DomainBase domainBase = getDomain(domainName, registrarId, now);
     Optional<RegistryLock> lockOptional =
         RegistryLockDao.getMostRecentVerifiedLockByRepoId(domainBase.getRepoId());
 
@@ -303,10 +307,19 @@ public final class DomainLockUtils {
         domainBase.getDomainName());
   }
 
-  private static DomainBase getDomain(String domainName, DateTime now) {
-    return loadByForeignKeyCached(DomainBase.class, domainName, now)
-        .orElseThrow(
-            () -> new IllegalArgumentException(String.format("Unknown domain %s", domainName)));
+  private DomainBase getDomain(String domainName, String registrarId, DateTime now) {
+    DomainBase domain =
+        loadByForeignKeyCached(DomainBase.class, domainName, now)
+            .orElseThrow(
+                () -> new IllegalArgumentException(String.format("Unknown domain %s", domainName)));
+    // The user must have specified either the correct registrar ID or the admin registrar ID
+    checkArgument(
+        registryAdminRegistrarId.equals(registrarId)
+            || domain.getCurrentSponsorClientId().equals(registrarId),
+        "Domain %s is not owned by registrar %s",
+        domainName,
+        registrarId);
+    return domain;
   }
 
   private static RegistryLock getByVerificationCode(String verificationCode) {
@@ -317,8 +330,8 @@ public final class DomainLockUtils {
                     String.format("Invalid verification code %s", verificationCode)));
   }
 
-  private static void applyLockStatuses(RegistryLock lock, DateTime lockTime) {
-    DomainBase domain = getDomain(lock.getDomainName(), lockTime);
+  private void applyLockStatuses(RegistryLock lock, DateTime lockTime) {
+    DomainBase domain = getDomain(lock.getDomainName(), lock.getRegistrarId(), lockTime);
     verifyDomainNotLocked(domain);
 
     DomainBase newDomain =
@@ -330,8 +343,8 @@ public final class DomainLockUtils {
     saveEntities(newDomain, lock, lockTime, true);
   }
 
-  private static void removeLockStatuses(RegistryLock lock, boolean isAdmin, DateTime unlockTime) {
-    DomainBase domain = getDomain(lock.getDomainName(), unlockTime);
+  private void removeLockStatuses(RegistryLock lock, boolean isAdmin, DateTime unlockTime) {
+    DomainBase domain = getDomain(lock.getDomainName(), lock.getRegistrarId(), unlockTime);
     if (!isAdmin) {
       verifyDomainLocked(domain);
     }

core/src/main/java/google/registry/tools/RegistryCli.java

@@ -29,6 +29,7 @@ import com.google.appengine.tools.remoteapi.RemoteApiOptions;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import google.registry.beam.initsql.BeamJpaModule;
 import google.registry.config.RegistryConfig;
 import google.registry.model.ofy.ObjectifyService;
 import google.registry.persistence.transaction.TransactionManagerFactory;
@@ -153,11 +154,15 @@ final class RegistryCli implements AutoCloseable, CommandRunner {
       return;
     }
 
-    checkState(RegistryToolEnvironment.get() == environment,
+    checkState(
+        RegistryToolEnvironment.get() == environment,
         "RegistryToolEnvironment argument pre-processing kludge failed.");
 
     component =
-        DaggerRegistryToolComponent.builder().credentialFilename(credentialJson).build();
+        DaggerRegistryToolComponent.builder()
+            .credentialFilePath(credentialJson)
+            .beamJpaModule(new BeamJpaModule(credentialJson))
+            .build();
 
     // JCommander stores sub-commands as nested JCommander objects containing a list of user objects
     // to be populated.  Extract the subcommand by getting the JCommander wrapper and then

core/src/main/java/google/registry/tools/RegistryToolComponent.java

@@ -18,8 +18,10 @@ import dagger.BindsInstance;
 import dagger.Component;
 import dagger.Lazy;
 import google.registry.batch.BatchModule;
+import google.registry.beam.initsql.BeamJpaModule;
 import google.registry.bigquery.BigqueryModule;
 import google.registry.config.CredentialModule.LocalCredentialJson;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.config.RegistryConfig.ConfigModule;
 import google.registry.dns.writer.VoidDnsWriterModule;
 import google.registry.dns.writer.clouddns.CloudDnsWriterModule;
@@ -42,7 +44,6 @@ import google.registry.tools.AuthModule.LocalCredentialModule;
 import google.registry.util.UtilsModule;
 import google.registry.whois.WhoisModule;
 import javax.annotation.Nullable;
-import javax.inject.Named;
 import javax.inject.Singleton;
 
 /**
@@ -57,6 +58,7 @@ import javax.inject.Singleton;
       AppEngineAdminApiModule.class,
       AuthModule.class,
       BatchModule.class,
+      BeamJpaModule.class,
       BigqueryModule.class,
       ConfigModule.class,
       CloudDnsWriterModule.class,
@@ -130,7 +132,9 @@ interface RegistryToolComponent {
   @Component.Builder
   interface Builder {
     @BindsInstance
-    Builder credentialFilename(@Nullable @Named("credentialFileName") String credentialFilename);
+    Builder credentialFilePath(@Nullable @Config("credentialFilePath") String credentialFilePath);
+
+    Builder beamJpaModule(BeamJpaModule beamJpaModule);
 
     RegistryToolComponent build();
   }

core/src/main/java/google/registry/tools/UpdateReservedListCommand.java

@@ -15,18 +15,17 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.ListNamingUtils.convertFilePathToName;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.beust.jcommander.Parameters;
 import com.google.common.base.Strings;
 import google.registry.model.registry.label.ReservedList;
-import google.registry.schema.tld.ReservedListDao;
 import google.registry.util.SystemClock;
 import java.nio.file.Files;
 import java.util.List;
 import java.util.Optional;
+import org.joda.time.DateTime;
 
 /** Command to safely update {@link ReservedList} on Datastore. */
 @Parameters(separators = " =", commandDescription = "Update a ReservedList in Datastore.")
@@ -35,42 +34,20 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand
   @Override
   protected void init() throws Exception {
     name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name;
-    // TODO(shicong): Read existing entry from Cloud SQL
     Optional<ReservedList> existing = ReservedList.get(name);
     checkArgument(
         existing.isPresent(), "Could not update reserved list %s because it doesn't exist.", name);
     boolean shouldPublish =
         this.shouldPublish == null ? existing.get().getShouldPublish() : this.shouldPublish;
     List<String> allLines = Files.readAllLines(input, UTF_8);
+    DateTime now = new SystemClock().nowUtc();
     ReservedList.Builder updated =
         existing
             .get()
             .asBuilder()
             .setReservedListMapFromLines(allLines)
-            .setLastUpdateTime(new SystemClock().nowUtc())
+            .setLastUpdateTime(now)
             .setShouldPublish(shouldPublish);
-    stageEntityChange(existing.get(), updated.build());
-    cloudSqlReservedList =
-        google.registry.schema.tld.ReservedList.create(
-            name, shouldPublish, parseToReservationsByLabels(allLines));
-  }
-
-  @Override
-  void saveToCloudSql() {
-    jpaTm()
-        .transact(
-            () -> {
-              // This check is currently disabled because, during the Cloud SQL migration, we need
-              // to be able to update reserved lists in Datastore while simultaneously creating
-              // their first revision in Cloud SQL (i.e. if they haven't been migrated over yet).
-              // TODO(shicong): Re-instate this once all reserved lists are migrated to Cloud SQL,
-              //                 and add a unit test to verity that an exception will be thrown if
-              //                 the reserved list doesn't exist.
-              // checkArgument(
-              //     ReservedListDao.checkExists(cloudSqlReservedList.getName()),
-              //     "A reserved list of this name doesn't exist: %s.",
-              //     cloudSqlReservedList.getName());
-              ReservedListDao.save(cloudSqlReservedList);
-            });
+    reservedList = updated.build();
   }
 }

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

@@ -454,7 +454,13 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
                 .orElseThrow(
                     () ->
                         new FormException(
-                            "Not allowed to set registry lock password directly on new contact"));
+                            "Cannot set registry lock password directly on new contact"));
+        // Can't modify registry lock email address
+        if (!Objects.equals(
+            updatedContact.getRegistryLockEmailAddress(),
+            existingContact.getRegistryLockEmailAddress())) {
+          throw new FormException("Cannot modify registryLockEmailAddress through the UI");
+        }
         if (updatedContact.isRegistryLockAllowed()) {
           // the password must have been set before or the user was allowed to set it now
           if (!existingContact.isAllowedToSetRegistryLockPassword()
@@ -464,7 +470,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
         }
         if (updatedContact.isAllowedToSetRegistryLockPassword()) {
           if (!existingContact.isAllowedToSetRegistryLockPassword()) {
-            throw new FormException("Cannot set isAllowedToSetRegistryLockPassword through UI");
+            throw new FormException(
+                "Cannot modify isAllowedToSetRegistryLockPassword through the UI");
           }
         }
       }

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

@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.security.JsonResponseHelper.Status.ERROR;
 import static google.registry.security.JsonResponseHelper.Status.SUCCESS;
-import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID;
 import static google.registry.ui.server.registrar.RegistryLockGetAction.getContactMatchingLogin;
 import static google.registry.ui.server.registrar.RegistryLockGetAction.getRegistrarAndVerifyLockAccess;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
@@ -31,6 +30,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.flogger.FluentLogger;
 import com.google.gson.Gson;
 import google.registry.config.RegistryConfig.Config;
+import google.registry.flows.domain.DomainFlowUtils;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.request.Action;
@@ -116,11 +116,10 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
       checkArgumentNotNull(input, "Null JSON");
       RegistryLockPostInput postInput =
           GSON.fromJson(GSON.toJsonTree(input), RegistryLockPostInput.class);
-      checkArgument(
-          !Strings.isNullOrEmpty(postInput.clientId),
-          "Missing key for client: %s",
-          PARAM_CLIENT_ID);
+      String registrarId = postInput.registrarId;
+      checkArgument(!Strings.isNullOrEmpty(registrarId), "Missing key for registrarId");
       checkArgument(!Strings.isNullOrEmpty(postInput.domainName), "Missing key for domainName");
+      DomainFlowUtils.validateDomainName(postInput.domainName);
       checkNotNull(postInput.isLock, "Missing key for isLock");
       UserAuthInfo userAuthInfo =
           authResult
@@ -135,12 +134,12 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
                     postInput.isLock
                         ? domainLockUtils.saveNewRegistryLockRequest(
                             postInput.domainName,
-                            postInput.clientId,
+                            registrarId,
                             userEmail,
                             registrarAccessor.isAdmin())
                         : domainLockUtils.saveNewRegistryUnlockRequest(
                             postInput.domainName,
-                            postInput.clientId,
+                            registrarId,
                             registrarAccessor.isAdmin(),
                             Optional.ofNullable(postInput.relockDurationMillis).map(Duration::new));
                 sendVerificationEmail(registryLock, userEmail, postInput.isLock);
@@ -190,9 +189,9 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
       return user.getEmail();
     }
     // Verify that the user can access the registrar, that the user has
-    // registry lock enabled, and that the user providjed a correct password
+    // registry lock enabled, and that the user provided a correct password
     Registrar registrar =
-        getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.clientId, false);
+        getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.registrarId, false);
     RegistrarContact registrarContact =
         getContactMatchingLogin(user, registrar)
             .orElseThrow(
@@ -215,7 +214,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
 
   /** Value class that represents the expected input body from the UI request. */
   private static class RegistryLockPostInput {
-    private String clientId;
+    private String registrarId;
     private String domainName;
     private Boolean isLock;
     private String password;

core/src/main/javascript/google/registry/ui/externs/json.js

@@ -180,7 +180,8 @@ registry.json.RegistrarAddress;
  *   faxNumber: (string?|undefined),
  *   types: (string?|undefined),
  *   allowedToSetRegistryLockPassword: boolean,
- *   registryLockAllowed: boolean
+ *   registryLockAllowed: boolean,
+ *   registryLockEmailAddress: (string?|undefined)
  * }}
  */
 registry.json.RegistrarContact;

core/src/main/javascript/google/registry/ui/js/registrar/contact_settings.js

@@ -106,6 +106,7 @@ registry.registrar.ContactSettings.prototype.renderItem = function(rspObj) {
           registryLockAllowedForRegistrar: rspObj.registryLockAllowed
         });
     this.setupAppbar();
+    this.setupPasswordElemIfNecessary_(targetContactNdx);
   } else {
     var contactsByType = {};
     for (var c in contacts) {
@@ -251,3 +252,26 @@ registry.registrar.ContactSettings.prototype.handleDeleteResponse =
   }
   return rsp;
 };
+
+// Show or hide the password based on what the user chooses
+registry.registrar.ContactSettings.prototype.setupPasswordElemIfNecessary_ =
+    function(contactIndex) {
+  var showOrHidePasswordButton = goog.dom.getElement('showOrHideRegistryLockPassword')
+  var showOrHidePassword = function() {
+    var inputElement = goog.dom.getRequiredElement(
+        'contacts[' + contactIndex + '].registryLockPassword')
+    var type = inputElement.getAttribute('type')
+    if (type === 'password') {
+      showOrHidePasswordButton.text = 'Hide password';
+      inputElement.setAttribute('type', 'text');
+    } else {
+      showOrHidePasswordButton.text = 'Show password';
+      inputElement.setAttribute('type', 'password');
+    }
+  };
+
+  if (showOrHidePasswordButton != null) {
+    goog.events.listen(showOrHidePasswordButton,
+        goog.events.EventType.CLICK, showOrHidePassword, false, this);
+  }
+};

core/src/main/javascript/google/registry/ui/js/registrar/registry_lock.js

@@ -172,7 +172,7 @@ registry.registrar.RegistryLock.prototype.lockOrUnlockDomain_ = function(isLock,
       e => this.fillLocksPage_(e),
       'POST',
       goog.json.serialize({
-        'clientId': this.clientId,
+        'registrarId': this.clientId,
         'domainName': domain,
         'isLock': isLock,
         'password': password,

core/src/main/resources/META-INF/persistence.xml

@@ -22,6 +22,7 @@
     <class>google.registry.model.billing.BillingEvent$Cancellation</class>
     <class>google.registry.model.billing.BillingEvent$OneTime</class>
     <class>google.registry.model.billing.BillingEvent$Recurring</class>
+    <class>google.registry.model.contact.ContactHistory</class>
     <class>google.registry.model.contact.ContactResource</class>
     <class>google.registry.model.domain.DomainBase</class>
     <class>google.registry.model.host.HostHistory</class>
@@ -35,12 +36,12 @@
     <class>google.registry.schema.server.Lock</class>
     <class>google.registry.schema.tld.PremiumList</class>
     <class>google.registry.schema.tld.PremiumEntry</class>
-    <class>google.registry.schema.tld.ReservedList</class>
     <class>google.registry.model.domain.secdns.DelegationSignerData</class>
     <class>google.registry.model.domain.GracePeriod</class>
     <class>google.registry.model.poll.PollMessage</class>
     <class>google.registry.model.poll.PollMessage$OneTime</class>
     <class>google.registry.model.poll.PollMessage$Autorenew</class>
+    <class>google.registry.model.registry.label.ReservedList</class>
 
     <!-- Customized type converters -->
     <class>google.registry.persistence.converter.BillingCostTransitionConverter</class>

core/src/main/resources/google/registry/ui/soy/registrar/ContactSettings.soy

@@ -176,6 +176,10 @@
     {if isNonnull($item['gaeUserId'])}
       <input type="hidden" name="{$namePrefix}gaeUserId" value="{$item['gaeUserId']}">
     {/if}
+    {if isNonnull($item['registryLockEmailAddress'])}
+      <input type="hidden" name="{$namePrefix}registryLockEmailAddress"
+             value="{$item['registryLockEmailAddress']}">
+    {/if}
   </div>
 {/template}
 
@@ -256,6 +260,15 @@
       {param isPassword: true /}
       {param placeholder: $placeholder /}
     {/call}
+    {if $item['allowedToSetRegistryLockPassword']}
+      <tr>
+        <td></td>
+        <td>
+          <a id="showOrHideRegistryLockPassword">Show password</a>
+        </td>
+      </tr>
+    {/if}
+
   {/if}
   <input type="hidden" name="{$namePrefix}allowedToSetRegistryLockPassword"
       {if isNonnull($item['allowedToSetRegistryLockPassword'])}

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

@@ -43,7 +43,6 @@ import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeSleeper;
 import google.registry.testing.InjectRule;
-import google.registry.testing.ShardableTestCase;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import google.registry.util.AppEngineServiceUtils;
 import google.registry.util.CapturingLogHandler;
@@ -62,7 +61,7 @@ import org.mockito.junit.MockitoRule;
 
 /** Unit tests for {@link AsyncTaskEnqueuer}. */
 @RunWith(JUnit4.class)
-public class AsyncTaskEnqueuerTest extends ShardableTestCase {
+public class AsyncTaskEnqueuerTest {
 
   @Rule
   public final AppEngineRule appEngine =
@@ -169,6 +168,7 @@ public class AsyncTaskEnqueuerTest extends ShardableTestCase {
         new TaskMatcher()
             .url(RelockDomainAction.PATH)
             .method("POST")
+            .header("Host", "backend.hostname.fake")
             .param(
                 RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
                 String.valueOf(lock.getRevisionId()))

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

@@ -21,14 +21,13 @@ import static google.registry.batch.AsyncTaskMetrics.OperationType.CONTACT_AND_H
 
 import com.google.common.collect.ImmutableSet;
 import google.registry.testing.FakeClock;
-import google.registry.testing.ShardableTestCase;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 /** Unit tests for {@link AsyncTaskMetrics}. */
 @RunWith(JUnit4.class)
-public class AsyncTaskMetricsTest extends ShardableTestCase {
+public class AsyncTaskMetricsTest {
 
   private final FakeClock clock = new FakeClock();
   private final AsyncTaskMetrics asyncTaskMetrics = new AsyncTaskMetrics(clock);

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

@@ -63,6 +63,7 @@ public class RelockDomainActionTest {
   private final DomainLockUtils domainLockUtils =
       new DomainLockUtils(
           new DeterministicStringGenerator(Alphabets.BASE_58),
+          "adminreg",
           AsyncTaskEnqueuerTest.createForTesting(
               mock(AppEngineServiceUtils.class), clock, Duration.ZERO));
 

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

@@ -50,12 +50,12 @@ public class ResaveAllEppResourcesActionTest
   @Test
   public void test_mapreduceSuccessfullyResavesEntity() throws Exception {
     ContactResource contact = persistActiveContact("test123");
-    DateTime creationTime = contact.getUpdateAutoTimestamp().getTimestamp();
-    assertThat(ofy().load().entity(contact).now().getUpdateAutoTimestamp().getTimestamp())
+    DateTime creationTime = contact.getUpdateTimestamp().getTimestamp();
+    assertThat(ofy().load().entity(contact).now().getUpdateTimestamp().getTimestamp())
         .isEqualTo(creationTime);
     ofy().clearSessionCache();
     runMapreduce();
-    assertThat(ofy().load().entity(contact).now().getUpdateAutoTimestamp().getTimestamp())
+    assertThat(ofy().load().entity(contact).now().getUpdateTimestamp().getTimestamp())
         .isGreaterThan(creationTime);
   }
 

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

@@ -45,7 +45,6 @@ import google.registry.request.Response;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
-import google.registry.testing.ShardableTestCase;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import google.registry.util.AppEngineServiceUtils;
 import org.joda.time.DateTime;
@@ -61,7 +60,7 @@ import org.mockito.junit.MockitoRule;
 
 /** Unit tests for {@link ResaveEntityAction}. */
 @RunWith(JUnit4.class)
-public class ResaveEntityActionTest extends ShardableTestCase {
+public class ResaveEntityActionTest {
 
   @Rule
   public final AppEngineRule appEngine =

core/src/test/java/google/registry/beam/initsql/BeamJpaModuleTest.java

@@ -15,45 +15,48 @@
 package google.registry.beam.initsql;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.junit.Assume.assumeThat;
 
 import google.registry.persistence.NomulusPostgreSql;
 import google.registry.persistence.transaction.JpaTransactionManager;
+import google.registry.testing.DatastoreEntityExtension;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
 import org.apache.beam.sdk.io.FileSystems;
 import org.apache.beam.sdk.options.PipelineOptionsFactory;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
 import org.testcontainers.containers.PostgreSQLContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
 
 /** Unit tests for {@link BeamJpaModule}. */
-@RunWith(JUnit4.class) // TODO(weiminyu): upgrade to JUnit 5.
+@Testcontainers
 public class BeamJpaModuleTest {
 
-  @Rule
+  @Container
   public PostgreSQLContainer database = new PostgreSQLContainer(NomulusPostgreSql.getDockerTag());
 
-  @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+  @RegisterExtension
+  public DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension();
+
+  @TempDir File tempFolder;
 
   private File credentialFile;
 
-  @Before
+  @BeforeEach
   public void beforeEach() throws IOException {
-    credentialFile = temporaryFolder.newFile();
+    credentialFile = new File(tempFolder, "credential");
     new PrintStream(credentialFile)
         .printf("%s %s %s", database.getJdbcUrl(), database.getUsername(), database.getPassword())
         .close();
   }
 
   @Test
-  public void getJpaTransactionManager_local() {
+  void getJpaTransactionManager_local() {
     JpaTransactionManager jpa =
         DaggerBeamJpaModule_JpaTransactionManagerComponent.builder()
             .beamJpaModule(new BeamJpaModule(credentialFile.getAbsolutePath()))
@@ -76,10 +79,9 @@ public class BeamJpaModuleTest {
    * information.
    */
   @Test
+  @EnabledIfSystemProperty(named = "test.gcp_integration.env", matches = "\\S+")
   public void getJpaTransactionManager_cloudSql_authRequired() {
     String environmentName = System.getProperty("test.gcp_integration.env");
-    assumeThat(environmentName, notNullValue());
-
     FileSystems.setDefaultPipelineOptions(PipelineOptionsFactory.create());
     JpaTransactionManager jpa =
         DaggerBeamJpaModule_JpaTransactionManagerComponent.builder()

core/src/test/java/google/registry/beam/initsql/CommitLogTransformsTest.java

@@ -19,6 +19,7 @@ import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.newRegistry;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import google.registry.backup.VersionedEntity;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
@@ -27,6 +28,7 @@ import google.registry.model.registry.Registry;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
 import java.io.File;
+import java.io.IOException;
 import java.io.Serializable;
 import org.apache.beam.sdk.coders.StringUtf8Coder;
 import org.apache.beam.sdk.io.fs.MatchResult.Metadata;
@@ -152,28 +154,45 @@ public class CommitLogTransformsTest implements Serializable {
 
   @Test
   @Category(NeedsRunner.class)
-  public void filterCommitLogsByTime() {
+  public void filterCommitLogsByTime() throws IOException {
     ImmutableList<String> commitLogFilenames =
         ImmutableList.of(
-            "/commit_diff_until_2000-01-01T00:00:00.000Z",
-            "/commit_diff_until_2000-01-01T00:00:00.001Z",
-            "/commit_diff_until_2000-01-01T00:00:00.002Z",
-            "/commit_diff_until_2000-01-01T00:00:00.003Z",
-            "/commit_diff_until_2000-01-01T00:00:00.004Z");
+            "commit_diff_until_2000-01-01T00:00:00.000Z",
+            "commit_diff_until_2000-01-01T00:00:00.001Z",
+            "commit_diff_until_2000-01-01T00:00:00.002Z",
+            "commit_diff_until_2000-01-01T00:00:00.003Z",
+            "commit_diff_until_2000-01-01T00:00:00.004Z");
+
+    File commitLogDir = temporaryFolder.newFolder();
+    for (String name : commitLogFilenames) {
+      new File(commitLogDir, name).createNewFile();
+    }
+
     PCollection<String> filteredFilenames =
         pipeline
             .apply(
-                "Generate All Filenames",
-                Create.of(commitLogFilenames).withCoder(StringUtf8Coder.of()))
+                "Get commitlog file patterns",
+                Transforms.getCommitLogFilePatterns(commitLogDir.getAbsolutePath()))
+            .apply("Find commitlog files", Transforms.getFilesByPatterns())
             .apply(
                 "Filtered by Time",
                 Transforms.filterCommitLogsByTime(
                     DateTime.parse("2000-01-01T00:00:00.001Z"),
-                    DateTime.parse("2000-01-01T00:00:00.003Z")));
+                    DateTime.parse("2000-01-01T00:00:00.003Z")))
+            .apply(
+                "Extract path strings",
+                ParDo.of(
+                    new DoFn<Metadata, String>() {
+                      @ProcessElement
+                      public void processElement(
+                          @Element Metadata fileMeta, OutputReceiver<String> out) {
+                        out.output(fileMeta.resourceId().getFilename());
+                      }
+                    }));
     PAssert.that(filteredFilenames)
         .containsInAnyOrder(
-            "/commit_diff_until_2000-01-01T00:00:00.001Z",
-            "/commit_diff_until_2000-01-01T00:00:00.002Z");
+            "commit_diff_until_2000-01-01T00:00:00.001Z",
+            "commit_diff_until_2000-01-01T00:00:00.002Z");
 
     pipeline.run();
   }
@@ -187,7 +206,9 @@ public class CommitLogTransformsTest implements Serializable {
                 "Get CommitLog file patterns",
                 Transforms.getCommitLogFilePatterns(commitLogsDir.getAbsolutePath()))
             .apply("Find CommitLogs", Transforms.getFilesByPatterns())
-            .apply(Transforms.loadCommitLogsFromFiles());
+            .apply(
+                Transforms.loadCommitLogsFromFiles(
+                    ImmutableSet.of("Registry", "ContactResource", "DomainBase")));
 
     InitSqlTestUtils.assertContainsExactlyElementsIn(
         entities,
@@ -197,4 +218,24 @@ public class CommitLogTransformsTest implements Serializable {
 
     pipeline.run();
   }
+
+  @Test
+  @Category(NeedsRunner.class)
+  public void loadOneCommitLogFile_filterByKind() {
+    PCollection<VersionedEntity> entities =
+        pipeline
+            .apply(
+                "Get CommitLog file patterns",
+                Transforms.getCommitLogFilePatterns(commitLogsDir.getAbsolutePath()))
+            .apply("Find CommitLogs", Transforms.getFilesByPatterns())
+            .apply(
+                Transforms.loadCommitLogsFromFiles(ImmutableSet.of("Registry", "ContactResource")));
+
+    InitSqlTestUtils.assertContainsExactlyElementsIn(
+        entities,
+        KV.of(fakeClock.nowUtc().getMillis() - 2, store.loadAsDatastoreEntity(registry)),
+        KV.of(fakeClock.nowUtc().getMillis() - 1, store.loadAsDatastoreEntity(contact)));
+
+    pipeline.run();
+  }
 }

core/src/test/java/google/registry/beam/initsql/ExportloadingTransformsTest.java

@@ -19,6 +19,7 @@ import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.newRegistry;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.backup.VersionedEntity;
 import google.registry.model.contact.ContactResource;
@@ -63,8 +64,8 @@ public class ExportloadingTransformsTest implements Serializable {
 
   private static final ImmutableList<Class<?>> ALL_KINDS =
       ImmutableList.of(Registry.class, ContactResource.class, DomainBase.class);
-  private static final ImmutableList<String> ALL_KIND_STRS =
-      ALL_KINDS.stream().map(Key::getKind).collect(ImmutableList.toImmutableList());
+  private static final ImmutableSet<String> ALL_KIND_STRS =
+      ALL_KINDS.stream().map(Key::getKind).collect(ImmutableSet.toImmutableSet());
 
   @Rule public final transient TemporaryFolder exportRootDir = new TemporaryFolder();
 
@@ -172,6 +173,7 @@ public class ExportloadingTransformsTest implements Serializable {
   }
 
   @Test
+  @Category(NeedsRunner.class)
   public void loadDataFromFiles() {
     PCollection<VersionedEntity> entities =
         pipeline

core/src/test/java/google/registry/beam/initsql/InitSqlTestUtils.java

@@ -30,6 +30,7 @@ import google.registry.backup.AppEngineEnvironment;
 import google.registry.backup.VersionedEntity;
 import java.io.Serializable;
 import java.util.Collection;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Stream;
 import org.apache.beam.sdk.testing.PAssert;
 import org.apache.beam.sdk.transforms.DoFn;
@@ -43,6 +44,9 @@ import org.apache.beam.sdk.values.TypeDescriptor;
 /** Test helpers for populating SQL with Datastore backups. */
 public final class InitSqlTestUtils {
 
+  // Generates unique ids to distinguish reused transforms.
+  private static final AtomicInteger TRANSFORM_ID_GEN = new AtomicInteger(0);
+
   /** Converts a Datastore {@link Entity} to an Objectify entity. */
   public static Object datastoreToOfyEntity(Entity entity) {
     return ofy().load().fromEntity(entity);
@@ -114,11 +118,12 @@ public final class InitSqlTestUtils {
     PCollection<String> errMsgs =
         actual
             .apply(
+                "MapElements_" + TRANSFORM_ID_GEN.getAndIncrement(),
                 MapElements.into(kvs(strings(), TypeDescriptor.of(VersionedEntity.class)))
                     .via(rawEntity -> KV.of("The One Key", rawEntity)))
-            .apply(GroupByKey.create())
+            .apply("GroupByKey_" + TRANSFORM_ID_GEN.getAndIncrement(), GroupByKey.create())
             .apply(
-                "assertContainsExactlyElementsIn",
+                "assertContainsExactlyElementsIn_" + TRANSFORM_ID_GEN.getAndIncrement(),
                 ParDo.of(
                     new DoFn<KV<String, Iterable<VersionedEntity>>, String>() {
                       @ProcessElement

core/src/test/java/google/registry/beam/initsql/LoadDatastoreSnapshotTest.java

@@ -0,0 +1,178 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.beam.initsql;
+
+import static google.registry.testing.DatastoreHelper.newContactResource;
+import static google.registry.testing.DatastoreHelper.newDomainBase;
+import static google.registry.testing.DatastoreHelper.newRegistry;
+
+import com.google.appengine.api.datastore.Entity;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.googlecode.objectify.Key;
+import google.registry.model.contact.ContactResource;
+import google.registry.model.domain.DomainAuthInfo;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
+import google.registry.model.ofy.Ofy;
+import google.registry.model.registry.Registry;
+import google.registry.testing.FakeClock;
+import google.registry.testing.InjectRule;
+import java.io.File;
+import org.apache.beam.sdk.testing.NeedsRunner;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.sdk.values.KV;
+import org.apache.beam.sdk.values.PCollectionTuple;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit test for {@link Transforms#loadDatastoreSnapshot}.
+ *
+ * <p>The test setup involves three entities, one Registry, one Domain, and two Contacts. Events
+ * happen in the following order:
+ *
+ * <ol>
+ *   <li>Registry and a filler Contact are inserted to Datastore.
+ *   <li>A CommitLog is persisted.
+ *   <li>Registry is updated.
+ *   <li>Another Contact and Domain are inserted into Datastore.
+ *   <li>Datastore is exported, but misses the newly inserted Contact.
+ *   <li>Filler Contact is deleted.
+ *   <li>A second CommitLog is persisted.
+ *   <li>Domain is updated in the Datastore.
+ *   <li>The third and last CommitLog is persisted.
+ * </ol>
+ *
+ * The final snapshot includes Registry, Domain, and Contact. This scenario verifies that:
+ *
+ * <ul>
+ *   <li>Incremental changes committed before an export does not override the exported valie.
+ *   <li>Entity missed by an export can be recovered from later CommitLogs.
+ *   <li>Multiple changes to an entity is applied in order.
+ *   <li>Deletes are properly handled.
+ * </ul>
+ */
+@RunWith(JUnit4.class)
+public class LoadDatastoreSnapshotTest {
+  private static final DateTime START_TIME = DateTime.parse("2000-01-01T00:00:00.0Z");
+
+  private static final ImmutableList<Class<?>> ALL_KINDS =
+      ImmutableList.of(Registry.class, ContactResource.class, DomainBase.class);
+  private static final ImmutableSet<String> ALL_KIND_STRS =
+      ALL_KINDS.stream().map(Key::getKind).collect(ImmutableSet.toImmutableSet());
+
+  @Rule public final transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Rule public final transient InjectRule injectRule = new InjectRule();
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(true);
+
+  private FakeClock fakeClock;
+  private File exportRootDir;
+  private File exportDir;
+  private File commitLogsDir;
+
+  // Canned data:
+  private transient Entity dsRegistry;
+  private transient Entity dsContact;
+  private transient Entity dsDomain;
+
+  private transient DateTime registryLastUpdateTime;
+  private transient DateTime contactLastUpdateTime;
+  private transient DateTime domainLastUpdateTime;
+
+  @Before
+  public void beforeEach() throws Exception {
+    fakeClock = new FakeClock(START_TIME);
+    try (BackupTestStore store = new BackupTestStore(fakeClock)) {
+      injectRule.setStaticField(Ofy.class, "clock", fakeClock);
+
+      exportRootDir = temporaryFolder.newFolder();
+      commitLogsDir = temporaryFolder.newFolder();
+
+      Registry registry = newRegistry("tld1", "TLD1");
+      ContactResource fillerContact = newContactResource("contact_filler");
+      store.insertOrUpdate(registry, fillerContact);
+      store.saveCommitLogs(commitLogsDir.getAbsolutePath());
+
+      registry =
+          registry
+              .asBuilder()
+              .setCreateBillingCost(registry.getStandardCreateCost().plus(1.0d))
+              .build();
+      registryLastUpdateTime = fakeClock.nowUtc();
+      store.insertOrUpdate(registry);
+
+      ContactResource contact = newContactResource("contact");
+      DomainBase domain = newDomainBase("domain1.tld1", contact);
+      contactLastUpdateTime = fakeClock.nowUtc();
+      store.insertOrUpdate(contact, domain);
+      exportDir =
+          store.export(
+              exportRootDir.getAbsolutePath(), ALL_KINDS, ImmutableSet.of(Key.create(contact)));
+
+      store.delete(fillerContact);
+      store.saveCommitLogs(commitLogsDir.getAbsolutePath());
+
+      domain =
+          domain
+              .asBuilder()
+              .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("NewPass")))
+              .build();
+      domainLastUpdateTime = fakeClock.nowUtc();
+      store.insertOrUpdate(domain);
+      store.saveCommitLogs(commitLogsDir.getAbsolutePath());
+
+      fakeClock.advanceOneMilli();
+
+      // Save persisted data for assertions.
+      dsRegistry = store.loadAsDatastoreEntity(registry);
+      dsContact = store.loadAsDatastoreEntity(contact);
+      dsDomain = store.loadAsDatastoreEntity(domain);
+    }
+  }
+
+  @Test
+  @Category(NeedsRunner.class)
+  public void loadDatastoreSnapshot() {
+    PCollectionTuple snapshot =
+        pipeline.apply(
+            Transforms.loadDatastoreSnapshot(
+                exportDir.getAbsolutePath(),
+                commitLogsDir.getAbsolutePath(),
+                START_TIME,
+                fakeClock.nowUtc(),
+                ALL_KIND_STRS));
+    InitSqlTestUtils.assertContainsExactlyElementsIn(
+        snapshot.get(Transforms.createTagForKind("DomainBase")),
+        KV.of(domainLastUpdateTime.getMillis(), dsDomain));
+    InitSqlTestUtils.assertContainsExactlyElementsIn(
+        snapshot.get(Transforms.createTagForKind("Registry")),
+        KV.of(registryLastUpdateTime.getMillis(), dsRegistry));
+    InitSqlTestUtils.assertContainsExactlyElementsIn(
+        snapshot.get(Transforms.createTagForKind("ContactResource")),
+        KV.of(contactLastUpdateTime.getMillis(), dsContact));
+    pipeline.run();
+  }
+}

core/src/test/java/google/registry/beam/initsql/WriteToSqlTest.java

@@ -0,0 +1,133 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.beam.initsql;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+
+import com.google.appengine.api.datastore.Entity;
+import com.google.common.collect.ImmutableList;
+import google.registry.backup.VersionedEntity;
+import google.registry.model.contact.ContactResource;
+import google.registry.model.ofy.Ofy;
+import google.registry.model.registrar.Registrar;
+import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestRule;
+import google.registry.testing.AppEngineRule;
+import google.registry.testing.DatastoreEntityExtension;
+import google.registry.testing.DatastoreHelper;
+import google.registry.testing.FakeClock;
+import google.registry.testing.InjectRule;
+import java.io.File;
+import java.io.PrintStream;
+import java.io.Serializable;
+import java.util.stream.Collectors;
+import org.apache.beam.sdk.testing.NeedsRunner;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.sdk.transforms.Create;
+import org.joda.time.DateTime;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit test for {@link Transforms#writeToSql}. */
+@RunWith(JUnit4.class)
+public class WriteToSqlTest implements Serializable {
+  private static final DateTime START_TIME = DateTime.parse("2000-01-01T00:00:00.0Z");
+
+  private final FakeClock fakeClock = new FakeClock(START_TIME);
+
+  @Rule public final transient InjectRule injectRule = new InjectRule();
+
+  // For use in the RuleChain below. Saves a reference to retrieve Database connection config.
+  public final transient JpaIntegrationTestRule database =
+      new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationTestRule();
+
+  @Rule
+  public final transient RuleChain jpaRules =
+      RuleChain.outerRule(new DatastoreEntityExtension()).around(database);
+
+  @Rule public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(true);
+
+  private ImmutableList<Entity> contacts;
+
+  private File credentialFile;
+
+  @Before
+  public void beforeEach() throws Exception {
+    try (BackupTestStore store = new BackupTestStore(fakeClock)) {
+      injectRule.setStaticField(Ofy.class, "clock", fakeClock);
+
+      // Required for contacts created below.
+      Registrar ofyRegistrar = AppEngineRule.makeRegistrar2();
+      store.insertOrUpdate(ofyRegistrar);
+      jpaTm().transact(() -> jpaTm().saveNewOrUpdate(store.loadAsOfyEntity(ofyRegistrar)));
+
+      ImmutableList.Builder<Entity> builder = new ImmutableList.Builder<>();
+
+      for (int i = 0; i < 3; i++) {
+        ContactResource contact = DatastoreHelper.newContactResource("contact_" + i);
+        store.insertOrUpdate(contact);
+        builder.add(store.loadAsDatastoreEntity(contact));
+      }
+      contacts = builder.build();
+    }
+    credentialFile = temporaryFolder.newFile();
+    new PrintStream(credentialFile)
+        .printf(
+            "%s %s %s",
+            database.getDatabaseUrl(),
+            database.getDatabaseUsername(),
+            database.getDatabasePassword())
+        .close();
+  }
+
+  @Test
+  @Category(NeedsRunner.class)
+  public void writeToSql_twoWriters() {
+    pipeline
+        .apply(
+            Create.of(
+                contacts.stream()
+                    .map(InitSqlTestUtils::entityToBytes)
+                    .map(bytes -> VersionedEntity.from(0L, bytes))
+                    .collect(Collectors.toList())))
+        .apply(
+            Transforms.writeToSql(
+                "ContactResource",
+                2,
+                4,
+                () ->
+                    DaggerBeamJpaModule_JpaTransactionManagerComponent.builder()
+                        .beamJpaModule(new BeamJpaModule(credentialFile.getAbsolutePath()))
+                        .build()
+                        .localDbJpaTransactionManager()));
+    pipeline.run().waitUntilFinish();
+
+    ImmutableList<?> sqlContacts = jpaTm().transact(() -> jpaTm().loadAll(ContactResource.class));
+    // TODO(weiminyu): compare load entities with originals. Note: lastUpdateTimes won't match by
+    // design. Need an elegant way to deal with this.bbq
+    assertThat(sqlContacts).hasSize(3);
+  }
+}

core/src/test/java/google/registry/beam/invoicing/BillingEventTest.java

@@ -132,8 +132,20 @@ public class BillingEventTest {
   public void testConvertBillingEvent_toCsv() {
     BillingEvent event = BillingEvent.parseFromRecord(schemaAndRecord);
     assertThat(event.toCsv())
-        .isEqualTo("1,2017-10-24 09:06:03 UTC,2017-01-19 23:59:43 UTC,myRegistrar,"
-            + "12345-CRRHELLO,test,RENEW,example.test,123456,5,USD,20.50,AUTO_RENEW");
+        .isEqualTo(
+            "1,2017-10-24 09:06:03 UTC,2017-01-19 23:59:43 UTC,myRegistrar,"
+                + "12345-CRRHELLO,,test,RENEW,example.test,123456,5,USD,20.50,AUTO_RENEW");
+  }
+
+  @Test
+  public void testConvertBillingEvent_nonNullPoNumber_toCsv() {
+    GenericRecord record = createRecord();
+    record.put("poNumber", "905610");
+    BillingEvent event = BillingEvent.parseFromRecord(new SchemaAndRecord(record, null));
+    assertThat(event.toCsv())
+        .isEqualTo(
+            "1,2017-10-24 09:06:03 UTC,2017-01-19 23:59:43 UTC,myRegistrar,"
+                + "12345-CRRHELLO,905610,test,RENEW,example.test,123456,5,USD,20.50,AUTO_RENEW");
   }
 
   @Test

core/src/test/java/google/registry/beam/invoicing/InvoicingPipelineTest.java

@@ -157,21 +157,21 @@ public class InvoicingPipelineTest {
     return ImmutableMap.of(
         "invoice_details_2017-10_theRegistrar_test.csv",
         ImmutableList.of(
-            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,"
+            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,,"
                 + "test,RENEW,mydomain2.test,REPO-ID,3,USD,20.50,",
-            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,"
+            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,,"
                 + "test,RENEW,mydomain.test,REPO-ID,3,USD,20.50,"),
         "invoice_details_2017-10_theRegistrar_hello.csv",
         ImmutableList.of(
-            "1,2017-10-02 00:00:00 UTC,2017-09-29 00:00:00 UTC,theRegistrar,234,"
+            "1,2017-10-02 00:00:00 UTC,2017-09-29 00:00:00 UTC,theRegistrar,234,,"
                 + "hello,CREATE,mydomain3.hello,REPO-ID,5,JPY,70.75,"),
         "invoice_details_2017-10_bestdomains_test.csv",
         ImmutableList.of(
-            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,bestdomains,456,"
+            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,bestdomains,456,116688,"
                 + "test,RENEW,mydomain4.test,REPO-ID,1,USD,20.50,"),
         "invoice_details_2017-10_anotherRegistrar_test.csv",
         ImmutableList.of(
-            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,anotherRegistrar,789,"
+            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,anotherRegistrar,789,,"
                 + "test,CREATE,mydomain5.test,REPO-ID,1,USD,0.00,SUNRISE ANCHOR_TENANT"));
   }
 

core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java

@@ -81,21 +81,21 @@ public class Spec11PipelineTest {
   @Rule public final transient TestPipeline p = TestPipeline.fromOptions(pipelineOptions);
   @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
 
-  private final Retrier retrier = new Retrier(
-      new FakeSleeper(new FakeClock(DateTime.parse("2019-07-15TZ"))), 1);
+  private final Retrier retrier =
+      new Retrier(new FakeSleeper(new FakeClock(DateTime.parse("2019-07-15TZ"))), 1);
   private Spec11Pipeline spec11Pipeline;
 
   @Before
   public void initializePipeline() throws IOException {
     File beamTempFolder = tempFolder.newFolder();
-    spec11Pipeline = new Spec11Pipeline(
-        "test-project",
-        beamTempFolder.getAbsolutePath() + "/staging",
-        beamTempFolder.getAbsolutePath() + "/templates/invoicing",
-        tempFolder.getRoot().getAbsolutePath(),
-        GoogleCredentialsBundle.create(GoogleCredentials.create(null)),
-        retrier
-    );
+    spec11Pipeline =
+        new Spec11Pipeline(
+            "test-project",
+            beamTempFolder.getAbsolutePath() + "/staging",
+            beamTempFolder.getAbsolutePath() + "/templates/invoicing",
+            tempFolder.getRoot().getAbsolutePath(),
+            GoogleCredentialsBundle.create(GoogleCredentials.create(null)),
+            retrier);
   }
 
   private static final ImmutableList<String> BAD_DOMAINS =
@@ -107,13 +107,15 @@ public class Spec11PipelineTest {
     // Put in half for theRegistrar and half for someRegistrar
     for (int i = 0; i < 255; i++) {
       subdomainsBuilder.add(
-          Subdomain.create(String.format("%s.com", i), "theRegistrar", "fake@theRegistrar.com"));
+          Subdomain.create(
+              String.format("%s.com", i), "theDomain", "theRegistrar", "fake@theRegistrar.com"));
     }
     for (int i = 255; i < 510; i++) {
       subdomainsBuilder.add(
-          Subdomain.create(String.format("%s.com", i), "someRegistrar", "fake@someRegistrar.com"));
+          Subdomain.create(
+              String.format("%s.com", i), "someDomain", "someRegistrar", "fake@someRegistrar.com"));
     }
-    subdomainsBuilder.add(Subdomain.create("no-email.com", "noEmailRegistrar", ""));
+    subdomainsBuilder.add(Subdomain.create("no-email.com", "fakeDomain", "noEmailRegistrar", ""));
     return subdomainsBuilder.build();
   }
 
@@ -165,8 +167,7 @@ public class Spec11PipelineTest {
     assertThat(noEmailThreatMatch.length()).isEqualTo(1);
     assertThat(noEmailThreatMatch.getJSONObject(0).get("fullyQualifiedDomainName"))
         .isEqualTo("no-email.com");
-    assertThat(noEmailThreatMatch.getJSONObject(0).get("threatType"))
-        .isEqualTo("MALWARE");
+    assertThat(noEmailThreatMatch.getJSONObject(0).get("threatType")).isEqualTo("MALWARE");
 
     JSONObject someRegistrarJSON = new JSONObject(sortedLines.get(1));
     assertThat(someRegistrarJSON.get("registrarEmailAddress")).isEqualTo("fake@someRegistrar.com");
@@ -176,8 +177,7 @@ public class Spec11PipelineTest {
     assertThat(someThreatMatch.length()).isEqualTo(1);
     assertThat(someThreatMatch.getJSONObject(0).get("fullyQualifiedDomainName"))
         .isEqualTo("444.com");
-    assertThat(someThreatMatch.getJSONObject(0).get("threatType"))
-        .isEqualTo("MALWARE");
+    assertThat(someThreatMatch.getJSONObject(0).get("threatType")).isEqualTo("MALWARE");
 
     // theRegistrar has two ThreatMatches, we have to parse it explicitly
     JSONObject theRegistrarJSON = new JSONObject(sortedLines.get(2));
@@ -228,10 +228,8 @@ public class Spec11PipelineTest {
     CloseableHttpResponse httpResponse =
         mock(CloseableHttpResponse.class, withSettings().serializable());
     when(httpResponse.getStatusLine())
-        .thenReturn(
-            new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "Done"));
-    when(httpResponse.getEntity())
-        .thenReturn(new FakeHttpEntity(getAPIResponse(badUrls)));
+        .thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "Done"));
+    when(httpResponse.getEntity()).thenReturn(new FakeHttpEntity(getAPIResponse(badUrls)));
     return httpResponse;
   }
 
@@ -298,5 +296,4 @@ public class Spec11PipelineTest {
     return ImmutableList.copyOf(
         ResourceUtils.readResourceUtf8(resultFile.toURI().toURL()).split("\n"));
   }
-
 }

core/src/test/java/google/registry/flows/EppCommitLogsTest.java

@@ -35,7 +35,6 @@ import google.registry.testing.EppLoader;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeHttpSession;
 import google.registry.testing.InjectRule;
-import google.registry.testing.ShardableTestCase;
 import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
@@ -45,7 +44,7 @@ import org.junit.runners.JUnit4;
 
 /** Test that domain flows create the commit logs needed to reload at points in the past. */
 @RunWith(JUnit4.class)
-public class EppCommitLogsTest extends ShardableTestCase {
+public class EppCommitLogsTest {
 
   @Rule
   public final AppEngineRule appEngine =

core/src/test/java/google/registry/flows/EppControllerTest.java

@@ -42,7 +42,6 @@ import google.registry.model.eppoutput.Result.Code;
 import google.registry.monitoring.whitebox.EppMetric;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
-import google.registry.testing.ShardableTestCase;
 import google.registry.util.Clock;
 import google.registry.xml.ValidationMode;
 import java.util.List;
@@ -64,7 +63,7 @@ import org.mockito.junit.MockitoRule;
 
 /** Unit tests for {@link EppController}. */
 @RunWith(JUnit4.class)
-public class EppControllerTest extends ShardableTestCase {
+public class EppControllerTest {
 
   @Rule
   public AppEngineRule appEngineRule =

core/src/test/java/google/registry/flows/EppTestCase.java

@@ -46,7 +46,6 @@ import google.registry.testing.FakeClock;
 import google.registry.testing.FakeHttpSession;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectRule;
-import google.registry.testing.ShardableTestCase;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -56,7 +55,7 @@ import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
 
-public class EppTestCase extends ShardableTestCase {
+public class EppTestCase {
 
   private static final MediaType APPLICATION_EPP_XML_UTF8 =
       MediaType.create("application", "epp+xml").withCharset(UTF_8);

core/src/test/java/google/registry/flows/EppTlsActionTest.java

@@ -23,7 +23,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 import google.registry.testing.FakeHttpSession;
-import google.registry.testing.ShardableTestCase;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -31,7 +30,7 @@ import org.mockito.ArgumentCaptor;
 
 /** Tests for {@link EppTlsAction}. */
 @RunWith(JUnit4.class)
-public class EppTlsActionTest extends ShardableTestCase {
+public class EppTlsActionTest {
 
   private static final byte[] INPUT_XML_BYTES = "<xml>".getBytes(UTF_8);
 

core/src/test/java/google/registry/flows/FlowReporterTest.java

@@ -29,7 +29,6 @@ import google.registry.model.eppinput.EppInput;
 import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting;
 import google.registry.model.eppoutput.EppResponse;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
-import google.registry.testing.ShardableTestCase;
 import java.util.Map;
 import java.util.Optional;
 import org.json.simple.JSONValue;
@@ -40,7 +39,7 @@ import org.junit.runners.JUnit4;
 
 /** Unit tests for {@link FlowReporter}. */
 @RunWith(JUnit4.class)
-public class FlowReporterTest extends ShardableTestCase {
+public class FlowReporterTest {
 
   static class TestCommandFlow implements Flow {
     @Override

core/src/test/java/google/registry/flows/FlowRunnerTest.java

@@ -36,7 +36,6 @@ import google.registry.monitoring.whitebox.EppMetric;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeHttpSession;
-import google.registry.testing.ShardableTestCase;
 import java.util.List;
 import java.util.Optional;
 import org.junit.Before;
@@ -48,7 +47,7 @@ import org.mockito.Mockito;
 
 /** Unit tests for {@link FlowRunner}. */
 @RunWith(JUnit4.class)
-public class FlowRunnerTest extends ShardableTestCase {
+public class FlowRunnerTest {
 
   @Rule
   public final AppEngineRule appEngineRule = new AppEngineRule.Builder().build();

core/src/test/java/google/registry/flows/FlowTestCase.java

@@ -52,7 +52,6 @@ import google.registry.testing.EppLoader;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeHttpSession;
 import google.registry.testing.InjectRule;
-import google.registry.testing.ShardableTestCase;
 import google.registry.testing.TestDataHelper;
 import google.registry.tmch.TmchCertificateAuthority;
 import google.registry.tmch.TmchXmlSignature;
@@ -63,18 +62,15 @@ import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.joda.time.DateTime;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /**
  * Base class for resource flow unit tests.
  *
  * @param <F> the flow type
  */
-@RunWith(JUnit4.class)
-public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
+public abstract class FlowTestCase<F extends Flow> {
 
   /** Whether to actually write to Datastore or just simulate. */
   public enum CommitMode { LIVE, DRY_RUN }
@@ -82,23 +78,22 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
   /** Whether to run in normal or superuser mode. */
   public enum UserPrivileges { NORMAL, SUPERUSER }
 
-  @Rule
+  @RegisterExtension
   public final AppEngineRule appEngine =
       AppEngineRule.builder().withDatastoreAndCloudSql().withTaskQueue().build();
 
-  @Rule
-  public final InjectRule inject = new InjectRule();
+  @RegisterExtension public final InjectRule inject = new InjectRule();
 
   protected EppLoader eppLoader;
   protected SessionMetadata sessionMetadata;
   protected FakeClock clock = new FakeClock(DateTime.now(UTC));
   protected TransportCredentials credentials = new PasswordOnlyTransportCredentials();
   protected EppRequestSource eppRequestSource = EppRequestSource.UNIT_TEST;
-  protected TmchXmlSignature testTmchXmlSignature = null;
+  private TmchXmlSignature testTmchXmlSignature = null;
 
   private EppMetric.Builder eppMetricBuilder;
 
-  @Before
+  @BeforeEach
   public void init() {
     sessionMetadata = new HttpSessionMetadata(new FakeHttpSession());
     sessionMetadata.setClientId("TheRegistrar");
@@ -204,29 +199,26 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
   }
 
   /**
-   * Assert that the actual grace periods and the corresponding billing events referenced from
-   * their keys match the expected map of grace periods to billing events.  For the expected map,
-   * the keys on the grace periods and IDs on the billing events are ignored.
+   * Assert that the actual grace periods and the corresponding billing events referenced from their
+   * keys match the expected map of grace periods to billing events. For the expected map, the keys
+   * on the grace periods and IDs on the billing events are ignored.
    */
-  public void assertGracePeriods(
-      Iterable<GracePeriod> actual,
-      ImmutableMap<GracePeriod, ? extends BillingEvent> expected) {
+  protected void assertGracePeriods(
+      Iterable<GracePeriod> actual, ImmutableMap<GracePeriod, ? extends BillingEvent> expected) {
     assertThat(canonicalizeGracePeriods(Maps.toMap(actual, FlowTestCase::expandGracePeriod)))
         .isEqualTo(canonicalizeGracePeriods(expected));
   }
 
-  public void assertPollMessages(
-      String clientId,
-      PollMessage... expected) {
+  protected void assertPollMessages(String clientId, PollMessage... expected) {
     assertPollMessagesHelper(getPollMessages(clientId), expected);
   }
 
-  public void assertPollMessages(PollMessage... expected) {
+  protected void assertPollMessages(PollMessage... expected) {
     assertPollMessagesHelper(getPollMessages(), expected);
   }
 
   /** Assert that the list matches all the poll messages in the fake Datastore. */
-  public void assertPollMessagesHelper(
+  private void assertPollMessagesHelper(
       Iterable<PollMessage> pollMessages, PollMessage... expected) {
     // Ordering is irrelevant but duplicates should be considered independently.
     assertThat(
@@ -279,7 +271,7 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
   }
 
   /** Shortcut to call {@link #runFlow(CommitMode, UserPrivileges)} as super user and live run. */
-  public EppOutput runFlowAsSuperuser() throws Exception {
+  protected EppOutput runFlowAsSuperuser() throws Exception {
     return runFlow(CommitMode.LIVE, UserPrivileges.SUPERUSER);
   }
 

core/src/test/java/google/registry/flows/ResourceFlowTestCase.java

@@ -46,8 +46,8 @@ import javax.annotation.Nullable;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.json.simple.JSONValue;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /**
  * Base class for resource flow unit tests.
@@ -60,8 +60,8 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
 
   private final TestLogHandler logHandler = new TestLogHandler();
 
-  @Before
-  public void beforeResourceFlowTestCase() {
+  @BeforeEach
+  void beforeResourceFlowTestCase() {
     // Attach TestLogHandler to the root logger so it has access to all log messages.
     // Note that in theory for assertIcannReportingActivityFieldLogged() below it would suffice to
     // attach it only to the FlowRunner logger, but for some reason this doesn't work for all flows.
@@ -89,7 +89,7 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
     return refreshedResource;
   }
 
-  protected ResourceCommand.SingleResourceCommand getResourceCommand() throws Exception {
+  private ResourceCommand.SingleResourceCommand getResourceCommand() throws Exception {
     return (ResourceCommand.SingleResourceCommand)
         ((ResourceCommandWrapper) eppLoader.getEpp().getCommandWrapper().getCommand())
             .getResourceCommand();
@@ -99,7 +99,7 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
     return getResourceCommand().getTargetId();
   }
 
-  protected Class<R> getResourceClass() {
+  private Class<R> getResourceClass() {
     return new TypeInstantiator<R>(getClass()){}.getExactType();
   }
 
@@ -119,7 +119,7 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
   }
 
   @Test
-  public void testRequiresLogin() {
+  void testRequiresLogin() {
     sessionMetadata.setClientId(null);
     EppException thrown = assertThrows(NotLoggedInException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();

core/src/test/java/google/registry/flows/TlsCredentialsTest.java

@@ -25,7 +25,6 @@ import static org.mockito.Mockito.when;
 import google.registry.model.registrar.Registrar;
 import google.registry.request.HttpException.BadRequestException;
 import google.registry.testing.AppEngineRule;
-import google.registry.testing.ShardableTestCase;
 import java.util.Optional;
 import javax.servlet.http.HttpServletRequest;
 import org.joda.time.DateTime;
@@ -36,7 +35,7 @@ import org.junit.runners.JUnit4;
 
 /** Unit tests for {@link TlsCredentials}. */
 @RunWith(JUnit4.class)
-public final class TlsCredentialsTest extends ShardableTestCase {
+public final class TlsCredentialsTest {
 
   @Rule
   public final AppEngineRule appEngine = AppEngineRule.builder().withDatastoreAndCloudSql().build();

core/src/test/java/google/registry/flows/contact/ContactCheckFlowTest.java

@@ -24,18 +24,17 @@ import google.registry.flows.EppException;
 import google.registry.flows.ResourceCheckFlowTestCase;
 import google.registry.flows.exceptions.TooManyResourceChecksException;
 import google.registry.model.contact.ContactResource;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactCheckFlow}. */
-public class ContactCheckFlowTest
-    extends ResourceCheckFlowTestCase<ContactCheckFlow, ContactResource> {
+class ContactCheckFlowTest extends ResourceCheckFlowTestCase<ContactCheckFlow, ContactResource> {
 
-  public ContactCheckFlowTest() {
+  ContactCheckFlowTest() {
     setEppInput("contact_check.xml");
   }
 
   @Test
-  public void testNothingExists() throws Exception {
+  void testNothingExists() throws Exception {
     // These ids come from the check xml.
     doCheckTest(
         create(true, "sh8013", null),
@@ -44,7 +43,7 @@ public class ContactCheckFlowTest
   }
 
   @Test
-  public void testOneExists() throws Exception {
+  void testOneExists() throws Exception {
     persistActiveContact("sh8013");
     // These ids come from the check xml.
     doCheckTest(
@@ -54,7 +53,7 @@ public class ContactCheckFlowTest
   }
 
   @Test
-  public void testOneExistsButWasDeleted() throws Exception {
+  void testOneExistsButWasDeleted() throws Exception {
     persistDeletedContact("sh8013", clock.nowUtc().minusDays(1));
     // These ids come from the check xml.
     doCheckTest(
@@ -64,27 +63,27 @@ public class ContactCheckFlowTest
   }
 
   @Test
-  public void testXmlMatches() throws Exception {
+  void testXmlMatches() throws Exception {
     persistActiveContact("sah8013");
     runFlowAssertResponse(loadFile("contact_check_response.xml"));
   }
 
   @Test
-  public void test50IdsAllowed() throws Exception {
+  void test50IdsAllowed() throws Exception {
     // Make sure we don't have a regression that reduces the number of allowed checks.
     setEppInput("contact_check_50.xml");
     runFlow();
   }
 
   @Test
-  public void testTooManyIds() {
+  void testTooManyIds() {
     setEppInput("contact_check_51.xml");
     EppException thrown = assertThrows(TooManyResourceChecksException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
   @Test
-  public void testIcannActivityReportField_getsLogged() throws Exception {
+  void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-check");
   }

core/src/test/java/google/registry/flows/contact/ContactCreateFlowTest.java

@@ -32,13 +32,12 @@ import google.registry.flows.exceptions.ResourceAlreadyExistsForThisClientExcept
 import google.registry.flows.exceptions.ResourceCreateContentionException;
 import google.registry.model.contact.ContactResource;
 import org.joda.time.DateTime;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactCreateFlow}. */
-public class ContactCreateFlowTest
-    extends ResourceFlowTestCase<ContactCreateFlow, ContactResource> {
+class ContactCreateFlowTest extends ResourceFlowTestCase<ContactCreateFlow, ContactResource> {
 
-  public ContactCreateFlowTest() {
+  ContactCreateFlowTest() {
     setEppInput("contact_create.xml");
     clock.setTo(DateTime.parse("1999-04-03T22:00:00.0Z"));
   }
@@ -56,24 +55,24 @@ public class ContactCreateFlowTest
   }
 
   @Test
-  public void testDryRun() throws Exception {
+  void testDryRun() throws Exception {
     dryRunFlowAssertResponse(loadFile("contact_create_response.xml"));
   }
 
   @Test
-  public void testSuccess_neverExisted() throws Exception {
+  void testSuccess_neverExisted() throws Exception {
     doSuccessfulTest();
   }
 
   @Test
-  public void testSuccess_existedButWasDeleted() throws Exception {
+  void testSuccess_existedButWasDeleted() throws Exception {
     persistDeletedContact(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1));
     clock.advanceOneMilli();
     doSuccessfulTest();
   }
 
   @Test
-  public void testFailure_alreadyExists() throws Exception {
+  void testFailure_alreadyExists() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     ResourceAlreadyExistsForThisClientException thrown =
         assertThrows(ResourceAlreadyExistsForThisClientException.class, this::runFlow);
@@ -85,7 +84,7 @@ public class ContactCreateFlowTest
   }
 
   @Test
-  public void testFailure_resourceContention() throws Exception {
+  void testFailure_resourceContention() throws Exception {
     String targetId = getUniqueIdFromCommand();
     persistResource(
         newContactResource(targetId)
@@ -101,13 +100,13 @@ public class ContactCreateFlowTest
   }
 
   @Test
-  public void testSuccess_nonAsciiInLocAddress() throws Exception {
+  void testSuccess_nonAsciiInLocAddress() throws Exception {
     setEppInput("contact_create_hebrew_loc.xml");
     doSuccessfulTest();
   }
 
   @Test
-  public void testFailure_nonAsciiInIntAddress() {
+  void testFailure_nonAsciiInIntAddress() {
     setEppInput("contact_create_hebrew_int.xml");
     EppException thrown =
         assertThrows(BadInternationalizedPostalInfoException.class, this::runFlow);
@@ -115,7 +114,7 @@ public class ContactCreateFlowTest
   }
 
   @Test
-  public void testFailure_declineDisclosure() {
+  void testFailure_declineDisclosure() {
     setEppInput("contact_create_decline_disclosure.xml");
     EppException thrown =
         assertThrows(DeclineContactDisclosureFieldDisallowedPolicyException.class, this::runFlow);
@@ -123,7 +122,7 @@ public class ContactCreateFlowTest
   }
 
   @Test
-  public void testIcannActivityReportField_getsLogged() throws Exception {
+  void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-create");
   }

core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java

@@ -37,26 +37,25 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.reporting.HistoryEntry;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactDeleteFlow}. */
-public class ContactDeleteFlowTest
-    extends ResourceFlowTestCase<ContactDeleteFlow, ContactResource> {
+class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, ContactResource> {
 
-  @Before
-  public void initFlowTest() {
+  @BeforeEach
+  void initFlowTest() {
     setEppInput("contact_delete.xml");
   }
 
   @Test
-  public void testDryRun() throws Exception {
+  void testDryRun() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     dryRunFlowAssertResponse(loadFile("contact_delete_response.xml"));
   }
 
   @Test
-  public void testSuccess() throws Exception {
+  void testSuccess() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
     assertTransactionalFlow(true);
@@ -73,7 +72,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testSuccess_clTridNotSpecified() throws Exception {
+  void testSuccess_clTridNotSpecified() throws Exception {
     setEppInput("contact_delete_no_cltrid.xml");
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
@@ -91,7 +90,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_neverExisted() throws Exception {
+  void testFailure_neverExisted() throws Exception {
     ResourceDoesNotExistException thrown =
         assertThrows(ResourceDoesNotExistException.class, this::runFlow);
     assertThat(thrown).hasMessageThat().contains(String.format("(%s)", getUniqueIdFromCommand()));
@@ -99,7 +98,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_existedButWasDeleted() throws Exception {
+  void testFailure_existedButWasDeleted() throws Exception {
     persistDeletedContact(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1));
     ResourceDoesNotExistException thrown =
         assertThrows(ResourceDoesNotExistException.class, this::runFlow);
@@ -108,19 +107,19 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_existedButWasClientDeleteProhibited() throws Exception {
+  void testFailure_existedButWasClientDeleteProhibited() throws Exception {
     doFailingStatusTest(
         StatusValue.CLIENT_DELETE_PROHIBITED, ResourceStatusProhibitsOperationException.class);
   }
 
   @Test
-  public void testFailure_existedButWasServerDeleteProhibited() throws Exception {
+  void testFailure_existedButWasServerDeleteProhibited() throws Exception {
     doFailingStatusTest(
         StatusValue.SERVER_DELETE_PROHIBITED, ResourceStatusProhibitsOperationException.class);
   }
 
   @Test
-  public void testFailure_existedButWasPendingDelete() throws Exception {
+  void testFailure_existedButWasPendingDelete() throws Exception {
     doFailingStatusTest(
         StatusValue.PENDING_DELETE, ResourceStatusProhibitsOperationException.class);
   }
@@ -137,7 +136,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_unauthorizedClient() throws Exception {
+  void testFailure_unauthorizedClient() throws Exception {
     sessionMetadata.setClientId("NewRegistrar");
     persistActiveContact(getUniqueIdFromCommand());
     EppException thrown = assertThrows(ResourceNotOwnedException.class, this::runFlow);
@@ -145,7 +144,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testSuccess_superuserUnauthorizedClient() throws Exception {
+  void testSuccess_superuserUnauthorizedClient() throws Exception {
     sessionMetadata.setClientId("NewRegistrar");
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
@@ -163,7 +162,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_failfastWhenLinkedToDomain() throws Exception {
+  void testFailure_failfastWhenLinkedToDomain() throws Exception {
     createTld("tld");
     persistResource(
         newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
@@ -172,7 +171,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testFailure_failfastWhenLinkedToApplication() throws Exception {
+  void testFailure_failfastWhenLinkedToApplication() throws Exception {
     createTld("tld");
     persistResource(
         newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
@@ -181,7 +180,7 @@ public class ContactDeleteFlowTest
   }
 
   @Test
-  public void testIcannActivityReportField_getsLogged() throws Exception {
+  void testIcannActivityReportField_getsLogged() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
     runFlow();