Use the save API version for the HPA controller and the deployment (#807)

This supposedly can fix our deployment problem. Tested on alpha.

Also updated the deployment script to replace the service object as
well.

core/build.gradle

@@ -63,8 +63,6 @@ def dockerIncompatibleTestPatterns = [
     // methods, so we exclude the whole test class.
     "google/registry/tools/params/PathParameterTest.*",
     "google/registry/persistence/PersistenceModuleTest.*",
-    // This test is failing in docker when using Java 11. The cause is unclear.
-    "google/registry/tools/DomainLockUtilsTest.*",
 ]
 
 // Tests that conflict with members of both the main test suite and the
@@ -635,131 +633,6 @@ artifacts {
   testRuntime testJar
 }
 
-/**
- * We have to break out the test suites because some of the tests conflict
- * with one another, but unfortunately this breaks the "--tests" flag.  The
- * --tests flag only applies to the task named on the command line (usually
- * just "test"), not for all tasks of type "Test".
- *
- * As a better solution, FilteringTest sets testNameIncludePatterns (the
- * internal property that --tests sets) from the value of the "testFilter"
- * property, allowing us to filter across all the tests in core without
- * explicitly specifying a test task or causing errors because there are no
- * matching tests in the main task.
- *
- * To use it, define "testFilter" to be a comma-separated collection of class
- * names (wildcards are allowed):
- *
- *   ./gradlew test -P testFilter=*.FooBar,google.registry.tools.ShellCommandTest
- */
-class FilteringTest extends Test {
-
-  FilteringTest() {
-    useJUnitPlatform();
-  }
-
-  private void applyTestFilter() {
-    if (project.testFilter) {
-      testNameIncludePatterns = project.testFilter.split(',')
-
-      // By default, gradle test tasks will produce a failure if no tests
-      // match the include/exclude/filter rules.  Since test filtering allows us
-      // to select a set of tests from a particular task, we don't want this
-      // behavior.
-      filter.failOnNoMatchingTests = false
-    }
-  }
-
-  /**
-   * Set to false if you also want to include TestCase and TestSuite classes.
-   *
-   * <p>Must be defined before "test", if at all.
-   */
-  boolean excludeTestCases = true
-
-  void setTests(List<String> tests) {
-    // Common exclude pattern. See README in parent directory for explanation.
-    if (excludeTestCases) {
-      exclude "**/*TestCase.*", "**/*TestSuite.*"
-    }
-    include tests
-    applyTestFilter()
-  }
-
-  /**
-   * Include all of the tests (except Test{Case,TestSuite}).  This actually
-   * doesn't explicitly "include" anything, in which cast the Test class tries
-   * to include everything that is not explicitly excluded.
-   */
-  void includeAllTests() {
-    exclude "**/*TestCase.*", "**/*TestSuite.*"
-    applyTestFilter()
-  }
-}
-
-task fragileTest(type: FilteringTest) {
-  // Common exclude pattern. See README in parent directory for explanation.
-  tests = fragileTestPatterns
-
-  if (rootProject.findProperty("skipDockerIncompatibleTests") == "true") {
-    exclude dockerIncompatibleTestPatterns
-  }
-
-  // Run every test class in a freshly started process.
-  forkEvery 1
-
-  doFirst {
-    new File(screenshotsDir).deleteDir()
-  }
-}
-
-task outcastTest(type: FilteringTest) {
-  tests = outcastTestPatterns
-
-  // Sets the maximum number of test executors that may exist at the same time.
-  // Note that this number appears to contribute to NoClassDefFoundError
-  // exceptions on certain machines and distros. The root cause is unclear.
-  // Try reducing this number if you experience similar problems.
-  maxParallelForks 3
-}
-
-// Whitebox test verifying that RegistryTool can be instantiated. Note the
-// use of runtimeClasspath. This test emulates the logic in RegistryCli#run.
-// A to-do is added there to refactor.
-// TODO(weiminyu): Need a similar test for Registry server.
-task registryToolIntegrationTest {
-  dependsOn compileJava
-  doLast {
-    def classLoader =
-        new URLClassLoader(sourceSets.main.runtimeClasspath.collect {
-            it.toURI().toURL()
-        } as URL[])
-    def commandClasses =
-        (classLoader.loadClass('google.registry.tools.RegistryTool')
-            .getDeclaredField('COMMAND_MAP').get(null) as Map).values()
-
-    commandClasses.each {
-      try {
-        Constructor<?> c = ((Class<?>) it).getDeclaredConstructor()
-        c.setAccessible(true)
-        c.newInstance()
-      } catch (Throwable e) {
-        throw new RuntimeException("Failed to instantiate ${it}:\n ${e}")
-      }
-    }
-  }
-}
-
-// Dedicated test suite for schema-dependent tests.
-task sqlIntegrationTest(type: FilteringTest) {
-  // TestSuite still requires a JUnit 4 runner, which knows how to handle JUnit 5 tests.
-  // Here we need to override parent's choice of JUnit 5. If changing this, remember to
-  // change :integration:sqlIntegrationTest too.
-  useJUnit()
-  excludeTestCases = false
-  tests = ['google/registry/schema/integration/SqlIntegrationTestSuite.*']
-}
-
 task findGoldenImages(type: JavaExec) {
   classpath = sourceSets.test.runtimeClasspath
   main = 'google.registry.webdriver.GoldenImageFinder'
@@ -879,39 +752,6 @@ task generateGoldenImages(type: FilteringTest) {
 }
 generateGoldenImages.finalizedBy(findGoldenImages)
 
-task standardTest(type: FilteringTest) {
-  includeAllTests()
-  exclude fragileTestPatterns
-  exclude outcastTestPatterns
-  // See SqlIntegrationTestSuite.java
-  exclude '**/*BeforeSuiteTest.*', '**/*AfterSuiteTest.*'
-
-  if (rootProject.findProperty("skipDockerIncompatibleTests") == "true") {
-    exclude dockerIncompatibleTestPatterns
-  }
-
-  // Run every test class in its own process.
-  // Uncomment to unblock build while troubleshooting inexplicable test errors.
-  // This setting makes the build take 35 minutes, without it it takes about 10.
-  // forkEvery 1
-
-  // Sets the maximum number of test executors that may exist at the same time.
-  // Also, Gradle executes tests in 1 thread and some of our test infrastructures
-  // depend on that, e.g. DualDatabaseTestInvocationContextProvider injects
-  // different implementation of TransactionManager into TransactionManagerFactory.
-  maxParallelForks 5
-
-  systemProperty 'test.projectRoot', rootProject.projectRootDir
-  systemProperty 'test.resourcesDir', resourcesDir
-}
-
-test {
-  // Don't run any tests from this task, all testing gets done in the
-  // FilteringTest tasks.
-  exclude "**"
-  // TODO(weiminyu): Remove dependency on sqlIntegrationTest
-}.dependsOn(fragileTest, outcastTest, standardTest, registryToolIntegrationTest, sqlIntegrationTest)
-
 createUberJar('nomulus', 'nomulus', 'google.registry.tools.RegistryTool')
 
 // A jar with classes and resources from main sourceSet, excluding internal
@@ -1046,6 +886,147 @@ task runTestServer(dependsOn: copyJsFilesForTestServer, type: JavaExec) {
   classpath = sourceSets.test.runtimeClasspath
 }
 
+/**
+ * We have to break out the test suites because some of the tests conflict
+ * with one another, but unfortunately this breaks the "--tests" flag.  The
+ * --tests flag only applies to the task named on the command line (usually
+ * just "test"), not for all tasks of type "Test".
+ *
+ * As a better solution, FilteringTest sets testNameIncludePatterns (the
+ * internal property that --tests sets) from the value of the "testFilter"
+ * property, allowing us to filter across all the tests in core without
+ * explicitly specifying a test task or causing errors because there are no
+ * matching tests in the main task.
+ *
+ * To use it, define "testFilter" to be a comma-separated collection of class
+ * names (wildcards are allowed):
+ *
+ *   ./gradlew test -P testFilter=*.FooBar,google.registry.tools.ShellCommandTest
+ */
+class FilteringTest extends Test {
+
+  FilteringTest() {
+    useJUnitPlatform();
+  }
+
+  private void applyTestFilter() {
+    if (project.testFilter) {
+      testNameIncludePatterns = project.testFilter.split(',')
+
+      // By default, gradle test tasks will produce a failure if no tests
+      // match the include/exclude/filter rules.  Since test filtering allows us
+      // to select a set of tests from a particular task, we don't want this
+      // behavior.
+      filter.failOnNoMatchingTests = false
+    }
+  }
+
+  /**
+   * Set to false if you also want to include TestCase and TestSuite classes.
+   *
+   * <p>Must be defined before "test", if at all.
+   */
+  boolean excludeTestCases = true
+
+  void setTests(List<String> tests) {
+    // Common exclude pattern. See README in parent directory for explanation.
+    if (excludeTestCases) {
+      exclude "**/*TestCase.*", "**/*TestSuite.*"
+    }
+    include tests
+    applyTestFilter()
+  }
+
+  /**
+   * Include all of the tests (except Test{Case,TestSuite}).  This actually
+   * doesn't explicitly "include" anything, in which cast the Test class tries
+   * to include everything that is not explicitly excluded.
+   */
+  void includeAllTests() {
+    exclude "**/*TestCase.*", "**/*TestSuite.*"
+    applyTestFilter()
+  }
+}
+
+task fragileTest(type: FilteringTest) {
+  // Common exclude pattern. See README in parent directory for explanation.
+  tests = fragileTestPatterns
+
+  if (rootProject.findProperty("skipDockerIncompatibleTests") == "true") {
+    exclude dockerIncompatibleTestPatterns
+  }
+
+  // Run every test class in a freshly started process.
+  forkEvery 1
+
+  doFirst {
+    new File(screenshotsDir).deleteDir()
+  }
+}
+
+task outcastTest(type: FilteringTest) {
+  tests = outcastTestPatterns
+
+  // Sets the maximum number of test executors that may exist at the same time.
+  // Note that this number appears to contribute to NoClassDefFoundError
+  // exceptions on certain machines and distros. The root cause is unclear.
+  // Try reducing this number if you experience similar problems.
+  maxParallelForks 3
+}
+
+// Dedicated test suite for schema-dependent tests.
+task sqlIntegrationTest(type: FilteringTest) {
+  // TestSuite still requires a JUnit 4 runner, which knows how to handle JUnit 5 tests.
+  // Here we need to override parent's choice of JUnit 5. If changing this, remember to
+  // change :integration:sqlIntegrationTest too.
+  useJUnit()
+  excludeTestCases = false
+  tests = ['google/registry/schema/integration/SqlIntegrationTestSuite.*']
+}
+
+// Verifies that RegistryTool can be instantiated:
+// - All dependencies are packaged in nomulus.jar
+// - JPA setup succeeds.
+task registryToolIntegrationTest(dependsOn: nomulus, type: FilteringTest) {
+  tests = ['google/registry/tools/RegistryToolTest.*']
+  testClassesDirs = sourceSets.test.output.classesDirs
+  classpath = nomulus.outputs.files.plus(configurations.testRuntimeClasspath)
+      .plus(files(testClassesDirs))
+}
+
+task standardTest(type: FilteringTest) {
+  includeAllTests()
+  exclude fragileTestPatterns
+  exclude outcastTestPatterns
+  // See SqlIntegrationTestSuite.java
+  exclude '**/*BeforeSuiteTest.*', '**/*AfterSuiteTest.*'
+
+  if (rootProject.findProperty("skipDockerIncompatibleTests") == "true") {
+    exclude dockerIncompatibleTestPatterns
+  }
+
+  // Run every test class in its own process.
+  // Uncomment to unblock build while troubleshooting inexplicable test errors.
+  // This setting makes the build take 35 minutes, without it it takes about 10.
+  // forkEvery 1
+
+  // Sets the maximum number of test executors that may exist at the same time.
+  // Also, Gradle executes tests in 1 thread and some of our test infrastructures
+  // depend on that, e.g. DualDatabaseTestInvocationContextProvider injects
+  // different implementation of TransactionManager into TransactionManagerFactory.
+  maxParallelForks 5
+
+  systemProperty 'test.projectRoot', rootProject.projectRootDir
+  systemProperty 'test.resourcesDir', resourcesDir
+}
+
+test {
+  // Don't run any tests from this task, all testing gets done in the
+  // FilteringTest tasks.
+  exclude "**"
+  // TODO(weiminyu): Remove dependency on sqlIntegrationTest
+}.dependsOn(fragileTest, outcastTest, standardTest, registryToolIntegrationTest, sqlIntegrationTest)
+
 project.build.dependsOn devtool
 project.build.dependsOn buildToolImage
 project.build.dependsOn ':stage'

core/gradle/dependency-locks/testCompile.lockfile

@@ -6,9 +6,9 @@ aopalliance:aopalliance:1.0
 args4j:args4j:2.33
 cglib:cglib-nodep:2.2
 com.beust:jcommander:1.60
-com.fasterxml.jackson.core:jackson-annotations:2.10.2
-com.fasterxml.jackson.core:jackson-core:2.10.2
-com.fasterxml.jackson.core:jackson-databind:2.10.2
+com.fasterxml.jackson.core:jackson-annotations:2.11.2
+com.fasterxml.jackson.core:jackson-core:2.11.2
+com.fasterxml.jackson.core:jackson-databind:2.11.2
 com.fasterxml:classmate:1.5.1
 com.github.jnr:jffi:1.2.23
 com.github.jnr:jnr-a64asm:1.0.0

core/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -6,9 +6,9 @@ aopalliance:aopalliance:1.0
 args4j:args4j:2.33
 cglib:cglib-nodep:2.2
 com.beust:jcommander:1.60
-com.fasterxml.jackson.core:jackson-annotations:2.10.2
-com.fasterxml.jackson.core:jackson-core:2.10.2
-com.fasterxml.jackson.core:jackson-databind:2.10.2
+com.fasterxml.jackson.core:jackson-annotations:2.11.2
+com.fasterxml.jackson.core:jackson-core:2.11.2
+com.fasterxml.jackson.core:jackson-databind:2.11.2
 com.fasterxml:classmate:1.5.1
 com.github.jnr:jffi:1.2.23
 com.github.jnr:jnr-a64asm:1.0.0

core/gradle/dependency-locks/testRuntime.lockfile

@@ -6,9 +6,9 @@ aopalliance:aopalliance:1.0
 args4j:args4j:2.33
 cglib:cglib-nodep:2.2
 com.beust:jcommander:1.60
-com.fasterxml.jackson.core:jackson-annotations:2.10.2
-com.fasterxml.jackson.core:jackson-core:2.10.2
-com.fasterxml.jackson.core:jackson-databind:2.10.2
+com.fasterxml.jackson.core:jackson-annotations:2.11.2
+com.fasterxml.jackson.core:jackson-core:2.11.2
+com.fasterxml.jackson.core:jackson-databind:2.11.2
 com.fasterxml:classmate:1.5.1
 com.github.jnr:jffi:1.2.23
 com.github.jnr:jnr-a64asm:1.0.0

core/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -6,9 +6,9 @@ aopalliance:aopalliance:1.0
 args4j:args4j:2.33
 cglib:cglib-nodep:2.2
 com.beust:jcommander:1.60
-com.fasterxml.jackson.core:jackson-annotations:2.10.2
-com.fasterxml.jackson.core:jackson-core:2.10.2
-com.fasterxml.jackson.core:jackson-databind:2.10.2
+com.fasterxml.jackson.core:jackson-annotations:2.11.2
+com.fasterxml.jackson.core:jackson-core:2.11.2
+com.fasterxml.jackson.core:jackson-databind:2.11.2
 com.fasterxml:classmate:1.5.1
 com.github.jnr:jffi:1.2.23
 com.github.jnr:jnr-a64asm:1.0.0

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

@@ -169,16 +169,20 @@ public final class AsyncTaskEnqueuer {
         lock.getRelockDuration().isPresent(),
         "Lock with ID %s not configured for relock",
         lock.getRevisionId());
+    enqueueDomainRelock(lock.getRelockDuration().get(), lock.getRevisionId(), 0);
+  }
+
+  /** Enqueues a task to asynchronously re-lock a registry-locked domain after it was unlocked. */
+  void enqueueDomainRelock(Duration countdown, long lockRevisionId, int previousAttempts) {
     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()))
-            .countdownMillis(lock.getRelockDuration().get().getMillis()));
+            .param(RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, String.valueOf(lockRevisionId))
+            .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, String.valueOf(previousAttempts))
+            .countdownMillis(countdown.getMillis()));
   }
 
   /**

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

@@ -21,6 +21,7 @@ import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY;
 import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS;
 import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE;
 import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME;
+import static google.registry.request.RequestParameters.extractIntParameter;
 import static google.registry.request.RequestParameters.extractLongParameter;
 import static google.registry.request.RequestParameters.extractOptionalBooleanParameter;
 import static google.registry.request.RequestParameters.extractOptionalIntParameter;
@@ -94,9 +95,15 @@ public class BatchModule {
   }
 
   @Provides
-  @Parameter("oldUnlockRevisionId")
+  @Parameter(RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM)
   static long provideOldUnlockRevisionId(HttpServletRequest req) {
-    return extractLongParameter(req, "oldUnlockRevisionId");
+    return extractLongParameter(req, RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM);
+  }
+
+  @Provides
+  @Parameter(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM)
+  static int providePreviousAttempts(HttpServletRequest req) {
+    return extractIntParameter(req, RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM);
   }
 
   @Provides

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

@@ -15,19 +15,23 @@
 package google.registry.batch;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.request.Action.Method.POST;
 import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.net.MediaType;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.registrar.Registrar;
+import google.registry.model.registrar.RegistrarContact;
 import google.registry.model.registry.RegistryLockDao;
 import google.registry.request.Action;
 import google.registry.request.Parameter;
@@ -36,11 +40,15 @@ import google.registry.request.auth.Auth;
 import google.registry.schema.domain.RegistryLock;
 import google.registry.tools.DomainLockUtils;
 import google.registry.util.DateTimeUtils;
+import google.registry.util.EmailMessage;
+import google.registry.util.SendEmailService;
+import java.util.Optional;
 import javax.inject.Inject;
+import javax.mail.internet.AddressException;
+import javax.mail.internet.InternetAddress;
+import org.joda.time.Duration;
 
-/**
- * Task that relocks a previously-Registry-Locked domain after some predetermined period of time.
- */
+/** Task that re-locks a previously-Registry-Locked domain after a predetermined period of time. */
 @Action(
     service = Action.Service.BACKEND,
     path = RelockDomainAction.PATH,
@@ -51,30 +59,78 @@ public class RelockDomainAction implements Runnable {
 
   public static final String PATH = "/_dr/task/relockDomain";
   public static final String OLD_UNLOCK_REVISION_ID_PARAM = "oldUnlockRevisionId";
+  public static final String PREVIOUS_ATTEMPTS_PARAM = "previousAttempts";
+
+  static final int ATTEMPTS_BEFORE_SLOWDOWN = 36; // every ten minutes for six hours then every hour
+  static final int FAILURES_BEFORE_EMAIL = 2; // email after three failures, one half hour
+  private static final Duration TEN_MINUTES = Duration.standardMinutes(10);
+  private static final Duration ONE_HOUR = Duration.standardHours(1);
+
+  private static final String RELOCK_SUCCESS_EMAIL_TEMPLATE =
+      "The domain %s was successfully re-locked.\n\nPlease contact support at %s if you have any "
+          + "questions.";
+  private static final String RELOCK_NON_RETRYABLE_FAILURE_EMAIL_TEMPLATE =
+      "There was an error when automatically re-locking %s. Error message: %s\n\nPlease contact "
+          + "support at %s if you have any questions.";
+  private static final String RELOCK_TRANSIENT_FAILURE_EMAIL_TEMPLATE =
+      "There was an unexpected error when automatically re-locking %s. We will continue retrying "
+          + "the lock for five hours. Please contact support at %s if you have any questions";
+  private static final String RELOCK_UNKNOWN_ID_FAILURE_EMAIL_TEMPLATE =
+      "The old lock with revision ID %d is not present or is not accessible";
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   private final long oldUnlockRevisionId;
+  private final int previousAttempts;
+  private final InternetAddress alertRecipientAddress;
+  private final InternetAddress gSuiteOutgoingEmailAddress;
+  private final String supportEmail;
+  private final SendEmailService sendEmailService;
   private final DomainLockUtils domainLockUtils;
   private final Response response;
+  private final AsyncTaskEnqueuer asyncTaskEnqueuer;
 
   @Inject
   public RelockDomainAction(
       @Parameter(OLD_UNLOCK_REVISION_ID_PARAM) long oldUnlockRevisionId,
+      @Parameter(PREVIOUS_ATTEMPTS_PARAM) int previousAttempts,
+      @Config("alertRecipientEmailAddress") InternetAddress alertRecipientAddress,
+      @Config("gSuiteOutgoingEmailAddress") InternetAddress gSuiteOutgoingEmailAddress,
+      @Config("supportEmail") String supportEmail,
+      SendEmailService sendEmailService,
       DomainLockUtils domainLockUtils,
-      Response response) {
+      Response response,
+      AsyncTaskEnqueuer asyncTaskEnqueuer) {
     this.oldUnlockRevisionId = oldUnlockRevisionId;
+    this.previousAttempts = previousAttempts;
+    this.alertRecipientAddress = alertRecipientAddress;
+    this.gSuiteOutgoingEmailAddress = gSuiteOutgoingEmailAddress;
+    this.supportEmail = supportEmail;
+    this.sendEmailService = sendEmailService;
     this.domainLockUtils = domainLockUtils;
     this.response = response;
+    this.asyncTaskEnqueuer = asyncTaskEnqueuer;
   }
 
   @Override
   public void run() {
-    jpaTm().transact(this::relockDomain);
+    /* We wish to manually control our retry behavior, in order to limit the number of retries
+     * and/or notify registrars / support only after a certain number of retries, or only
+     * with a certain type of failure. AppEngine will automatically retry on any non-2xx status
+     * code, so return SC_NO_CONTENT (204) by default to avoid this auto-retry.
+     *
+     * See https://cloud.google.com/appengine/docs/standard/java/taskqueue/push/retrying-tasks
+     * for more details on retry behavior. */
+    response.setStatus(SC_NO_CONTENT);
+    response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
+
+    // nb: DomainLockUtils relies on the JPA transaction being the outermost transaction
+    jpaTm().transact(() -> tm().transact(this::relockDomain));
   }
 
   private void relockDomain() {
-    RegistryLock oldLock;
+    RegistryLock oldLock = null;
+    DomainBase domain;
     try {
       oldLock =
           RegistryLockDao.getByRevisionId(oldUnlockRevisionId)
@@ -82,87 +138,187 @@ public class RelockDomainAction implements Runnable {
                   () ->
                       new IllegalArgumentException(
                           String.format("Unknown revision ID %d", oldUnlockRevisionId)));
-      DomainBase domain =
+      domain =
           ofy()
               .load()
               .type(DomainBase.class)
               .id(oldLock.getRepoId())
               .now()
               .cloneProjectedAtTime(jpaTm().getTransactionTime());
-
-      if (domain.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES)
-          || oldLock.getRelock() != null) {
-        // The domain was manually locked, so we shouldn't worry about relocking
-        String message =
-            String.format(
-                "Domain %s is already manually relocked, skipping automated relock.",
-                domain.getDomainName());
-        logger.atInfo().log(message);
-        // SC_NO_CONTENT (204) skips retry -- see the comment below
-        response.setStatus(SC_NO_CONTENT);
-        response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
-        response.setPayload(message);
-        return;
-      }
-      verifyDomainAndLockState(oldLock, domain);
     } catch (Throwable t) {
-      /* If there's a bad verification code or the domain is in a bad state, we won't want to retry.
-       * AppEngine will retry on non-2xx error codes, so we return SC_NO_CONTENT (204) to avoid it.
-       *
-       * See https://cloud.google.com/appengine/docs/standard/java/taskqueue/push/retrying-tasks
-       * for more details on retry behavior. */
-      logger.atWarning().withCause(t).log(
-          "Exception when attempting to relock domain with old revision ID %d.",
-          oldUnlockRevisionId);
-      response.setStatus(SC_NO_CONTENT);
-      response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
-      response.setPayload(String.format("Relock failed: %s", t.getMessage()));
+      handleTransientFailure(Optional.ofNullable(oldLock), t);
       return;
     }
-    applyRelock(oldLock);
+
+    if (domain.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES)
+        || oldLock.getRelock() != null) {
+      // The domain was manually locked, so we shouldn't worry about re-locking
+      String message =
+          String.format(
+              "Domain %s is already manually re-locked, skipping automated re-lock.",
+              domain.getDomainName());
+      logger.atInfo().log(message);
+      response.setPayload(message);
+      return;
+    }
+    try {
+      verifyDomainAndLockState(oldLock, domain);
+    } catch (Throwable t) {
+      // If the domain was, for example, transferred, then notify the old registrar and don't retry.
+      handleNonRetryableFailure(oldLock, t);
+      return;
+    }
+    try {
+      applyRelock(oldLock);
+    } catch (Throwable t) {
+      handleTransientFailure(Optional.of(oldLock), t);
+    }
   }
 
   private void applyRelock(RegistryLock oldLock) {
-    try {
-      domainLockUtils.administrativelyApplyLock(
-          oldLock.getDomainName(),
-          oldLock.getRegistrarId(),
-          oldLock.getRegistrarPocId(),
-          oldLock.isSuperuser());
-      logger.atInfo().log("Relocked domain %s.", oldLock.getDomainName());
-      response.setStatus(SC_OK);
-    } catch (Throwable t) {
-      // Any errors that occur here are unexpected, so we should retry. Return a non-2xx
-      // error code to get AppEngine to retry
-      logger.atSevere().withCause(t).log(
-          "Exception when attempting to relock domain %s.", oldLock.getDomainName());
-      response.setStatus(SC_INTERNAL_SERVER_ERROR);
-      response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
-      response.setPayload(String.format("Relock failed: %s", t.getMessage()));
+    domainLockUtils.administrativelyApplyLock(
+        oldLock.getDomainName(),
+        oldLock.getRegistrarId(),
+        oldLock.getRegistrarPocId(),
+        oldLock.isSuperuser());
+    logger.atInfo().log("Re-locked domain %s.", oldLock.getDomainName());
+    response.setStatus(SC_OK);
+    // Only send a success email if we previously sent a failure email
+    if (previousAttempts > FAILURES_BEFORE_EMAIL) {
+      sendSuccessEmail(oldLock);
     }
   }
 
   private void verifyDomainAndLockState(RegistryLock oldLock, DomainBase domain) {
     // Domain shouldn't be deleted or have a pending transfer/delete
     String domainName = domain.getDomainName();
-    checkArgument(
-        !DateTimeUtils.isAtOrAfter(jpaTm().getTransactionTime(), domain.getDeletionTime()),
-        "Domain %s has been deleted",
-        domainName);
     ImmutableSet<StatusValue> statusValues = domain.getStatusValues();
     checkArgument(
         !statusValues.contains(StatusValue.PENDING_DELETE),
-        "Domain %s has a pending delete",
+        "Domain %s has a pending delete.",
+        domainName);
+    checkArgument(
+        !DateTimeUtils.isAtOrAfter(jpaTm().getTransactionTime(), domain.getDeletionTime()),
+        "Domain %s has been deleted.",
         domainName);
     checkArgument(
         !statusValues.contains(StatusValue.PENDING_TRANSFER),
-        "Domain %s has a pending transfer",
+        "Domain %s has a pending transfer.",
         domainName);
     checkArgument(
         domain.getCurrentSponsorClientId().equals(oldLock.getRegistrarId()),
-        "Domain %s has been transferred from registrar %s to registrar %s since the unlock",
+        "Domain %s has been transferred from registrar %s to registrar %s since the unlock.",
         domainName,
         oldLock.getRegistrarId(),
         domain.getCurrentSponsorClientId());
   }
+
+  private void handleNonRetryableFailure(RegistryLock oldLock, Throwable t) {
+    logger.atWarning().withCause(t).log(
+        "Exception thrown when attempting to re-lock domain with old revision ID %d.",
+        oldUnlockRevisionId);
+    response.setPayload(String.format("Re-lock failed: %s", t.getMessage()));
+
+    String body =
+        String.format(
+            RELOCK_NON_RETRYABLE_FAILURE_EMAIL_TEMPLATE,
+            oldLock.getDomainName(),
+            t.getMessage(),
+            supportEmail);
+    sendEmailService.sendEmail(
+        EmailMessage.newBuilder()
+            .setFrom(gSuiteOutgoingEmailAddress)
+            .setBody(body)
+            .setSubject(String.format("Error re-locking domain %s", oldLock.getDomainName()))
+            .setRecipients(getEmailRecipients(oldLock.getRegistrarId()))
+            .build());
+  }
+
+  private void handleTransientFailure(Optional<RegistryLock> oldLock, Throwable t) {
+    String message = String.format("Re-lock failed: %s", t.getMessage());
+    logger.atSevere().withCause(t).log(message);
+    response.setPayload(message);
+
+    if (previousAttempts == FAILURES_BEFORE_EMAIL) {
+      if (oldLock.isPresent()) {
+        sendGenericTransientFailureEmail(oldLock.get());
+      } else {
+        // if the old lock isn't present, something has gone horribly wrong
+        sendUnknownRevisionIdAlertEmail();
+      }
+    }
+    Duration timeBeforeRetry = previousAttempts < ATTEMPTS_BEFORE_SLOWDOWN ? TEN_MINUTES : ONE_HOUR;
+    asyncTaskEnqueuer.enqueueDomainRelock(
+        timeBeforeRetry, oldUnlockRevisionId, previousAttempts + 1);
+  }
+
+  private void sendSuccessEmail(RegistryLock oldLock) {
+    String body =
+        String.format(RELOCK_SUCCESS_EMAIL_TEMPLATE, oldLock.getDomainName(), supportEmail);
+
+    sendEmailService.sendEmail(
+        EmailMessage.newBuilder()
+            .setFrom(gSuiteOutgoingEmailAddress)
+            .setBody(body)
+            .setSubject(String.format("Successful re-lock of domain %s", oldLock.getDomainName()))
+            .setRecipients(getEmailRecipients(oldLock.getRegistrarId()))
+            .build());
+  }
+
+  private void sendGenericTransientFailureEmail(RegistryLock oldLock) {
+    String body =
+        String.format(
+            RELOCK_TRANSIENT_FAILURE_EMAIL_TEMPLATE, oldLock.getDomainName(), supportEmail);
+    // For an unexpected failure, notify both the lock-enabled contacts and our alerting email
+    ImmutableSet<InternetAddress> allRecipients =
+        new ImmutableSet.Builder<InternetAddress>()
+            .addAll(getEmailRecipients(oldLock.getRegistrarId()))
+            .add(alertRecipientAddress)
+            .build();
+    sendEmailService.sendEmail(
+        EmailMessage.newBuilder()
+            .setFrom(gSuiteOutgoingEmailAddress)
+            .setBody(body)
+            .setSubject(String.format("Error re-locking domain %s", oldLock.getDomainName()))
+            .setRecipients(allRecipients)
+            .build());
+  }
+
+  private void sendUnknownRevisionIdAlertEmail() {
+    sendEmailService.sendEmail(
+        EmailMessage.newBuilder()
+            .setFrom(gSuiteOutgoingEmailAddress)
+            .setBody(String.format(RELOCK_UNKNOWN_ID_FAILURE_EMAIL_TEMPLATE, oldUnlockRevisionId))
+            .setSubject("Error re-locking domain")
+            .setRecipients(ImmutableSet.of(alertRecipientAddress))
+            .build());
+  }
+
+  private ImmutableSet<InternetAddress> getEmailRecipients(String registrarId) {
+    Registrar registrar =
+        Registrar.loadByClientIdCached(registrarId)
+            .orElseThrow(
+                () ->
+                    new IllegalStateException(String.format("Unknown registrar %s", registrarId)));
+
+    ImmutableSet<String> registryLockEmailAddresses =
+        registrar.getContacts().stream()
+            .filter(RegistrarContact::isRegistryLockAllowed)
+            .map(RegistrarContact::getRegistryLockEmailAddress)
+            .filter(Optional::isPresent)
+            .map(Optional::get)
+            .collect(toImmutableSet());
+
+    ImmutableSet.Builder<InternetAddress> builder = new ImmutableSet.Builder<>();
+    // can't use streams due to the 'throws' in the InternetAddress constructor
+    for (String registryLockEmailAddress : registryLockEmailAddresses) {
+      try {
+        builder.add(new InternetAddress(registryLockEmailAddress));
+      } catch (AddressException e) {
+        // This shouldn't stop any other emails going out, so swallow it
+        logger.atWarning().log("Invalid email address %s", registryLockEmailAddress);
+      }
+    }
+    return builder.build();
+  }
 }

core/src/main/java/google/registry/beam/invoicing/sql/billing_events.sql

@@ -55,9 +55,7 @@ FROM (
     FROM
       `%PROJECT_ID%.%DATASTORE_EXPORT_DATA_SET%.%REGISTRY_TABLE%`
     WHERE
-    -- TODO(b/18092292): Add a filter for tldState (not PDT/PREDELEGATION)
-      tldType = 'REAL'
-    AND disableInvoicing is not TRUE) ) AS BillingEvent
+      enableInvoicing IS TRUE) ) AS BillingEvent
   -- Gather billing ID from registrar table
   -- This is a 'JOIN' as opposed to 'LEFT JOIN' to filter out
   -- non-billable registrars

core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java

@@ -370,7 +370,8 @@ public class DomainCreateFlow implements TransactionalFlow {
     if (allocationToken.isPresent()
         && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) {
       entitiesToSave.add(
-          allocationTokenFlowUtils.redeemToken(allocationToken.get(), Key.create(historyEntry)));
+          allocationTokenFlowUtils.redeemToken(
+              allocationToken.get(), HistoryEntry.createVKey(Key.create(historyEntry))));
     }
     enqueueTasks(newDomain, hasSignedMarks, hasClaimsNotice);
 

core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java

@@ -68,6 +68,7 @@ import google.registry.model.domain.DomainCommand.Update.Change;
 import google.registry.model.domain.fee.FeeUpdateCommandExtension;
 import google.registry.model.domain.metadata.MetadataExtension;
 import google.registry.model.domain.secdns.SecDnsUpdateExtension;
+import google.registry.model.domain.superuser.DomainUpdateSuperuserExtension;
 import google.registry.model.eppcommon.AuthInfo;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppinput.EppInput;
@@ -152,7 +153,8 @@ public final class DomainUpdateFlow implements TransactionalFlow {
     extensionManager.register(
         FeeUpdateCommandExtension.class,
         MetadataExtension.class,
-        SecDnsUpdateExtension.class);
+        SecDnsUpdateExtension.class,
+        DomainUpdateSuperuserExtension.class);
     flowCustomLogic.beforeValidation();
     extensionManager.validate();
     validateClientIsLoggedIn(clientId);
@@ -251,6 +253,15 @@ public final class DomainUpdateFlow implements TransactionalFlow {
             .removeContacts(remove.getContacts())
             .setRegistrant(firstNonNull(change.getRegistrant(), domain.getRegistrant()))
             .setAuthInfo(firstNonNull(change.getAuthInfo(), domain.getAuthInfo()));
+    Optional<DomainUpdateSuperuserExtension> superuserExt =
+        eppInput.getSingleExtension(DomainUpdateSuperuserExtension.class);
+    if (superuserExt.isPresent()) {
+      if (superuserExt.get().getAutorenews().isPresent()) {
+        boolean autorenews = superuserExt.get().getAutorenews().get();
+        domainBuilder.setAutorenewEndTime(
+            Optional.ofNullable(autorenews ? null : domain.getRegistrationExpirationTime()));
+      }
+    }
     return domainBuilder.build();
   }
 

core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java

@@ -33,6 +33,7 @@ import google.registry.model.domain.token.AllocationToken.TokenStatus;
 import google.registry.model.domain.token.AllocationToken.TokenType;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.persistence.VKey;
 import java.util.List;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -107,7 +108,7 @@ public class AllocationTokenFlowUtils {
 
   /** Redeems a SINGLE_USE {@link AllocationToken}, returning the redeemed copy. */
   public AllocationToken redeemToken(
-      AllocationToken token, Key<HistoryEntry> redemptionHistoryEntry) {
+      AllocationToken token, VKey<HistoryEntry> redemptionHistoryEntry) {
     checkArgument(
         TokenType.SINGLE_USE.equals(token.getTokenType()),
         "Only SINGLE_USE tokens can be marked as redeemed");
@@ -124,7 +125,8 @@ public class AllocationTokenFlowUtils {
   private void validateToken(
       InternetDomainName domainName, AllocationToken token, String clientId, DateTime now)
       throws EppException {
-    if (!token.getAllowedClientIds().isEmpty() && !token.getAllowedClientIds().contains(clientId)) {
+    if (!token.getAllowedRegistrarIds().isEmpty()
+        && !token.getAllowedRegistrarIds().contains(clientId)) {
       throw new AllocationTokenNotValidForRegistrarException();
     }
     if (!token.getAllowedTlds().isEmpty()

core/src/main/java/google/registry/idn/Arabic-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Arab
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Armenian-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Armn
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Bengali-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Beng
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Chinese-IDN.txt

@@ -1,24 +1,24 @@
 # Registry: Charleston Road Registry Inc.
-# Script: Chinese
+# Language: Chinese
 # Version: 1.0
 # Effective Date: 04-12-2012
 #
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com
 # Notes: This table describes codepoints allowed for the Chinese script.
 
-Reference 0 Unicode 3.2
-Reference 1 A Complete Set of Simplified Chinese Characters
-Reference 2 Chinese Variants Collation Table
-Reference 3 Chinese Big Dictionary
-Reference 4 Chinese Relationship Table for Unihan Project
-Reference 5 GB2312
-Reference 6 General Table for Modern Chinese
-Reference 7 International Chinese Standard Big Dictionary
-Reference 8 Unihan Database
-Reference 9 BIG5
+# Reference 0 Unicode 3.2
+# Reference 1 A Complete Set of Simplified Chinese Characters
+# Reference 2 Chinese Variants Collation Table
+# Reference 3 Chinese Big Dictionary
+# Reference 4 Chinese Relationship Table for Unihan Project
+# Reference 5 GB2312
+# Reference 6 General Table for Modern Chinese
+# Reference 7 International Chinese Standard Big Dictionary
+# Reference 8 Unihan Database
+# Reference 9 BIG5
 
 U+002D(0);U+002D(0);
 U+0030(0);U+0030(0);

core/src/main/java/google/registry/idn/Cyrillic-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Cyrl
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Devanagari-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Deva
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Ethiopic-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Ethi
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Georgian-IDN.txt

@@ -3,7 +3,7 @@
 #
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Greek-IDN.txt

@@ -3,7 +3,7 @@
 # Script: Grek
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Gurmukhi-IDN.txt

@@ -3,7 +3,7 @@
 # Script: Guru
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Hebrew-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Hebr
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Kannada-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Knda
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Khmer-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Khmr
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 #
 # Telephone: +1 (650) 253-0000

core/src/main/java/google/registry/idn/Korean-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Kore
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Malayalam-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Mlym
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Myanmar-IDN.txt

@@ -3,7 +3,7 @@
 # Version: 1.0
 # Effective Date: 04-12-2012
 #
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Oriya-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Orya
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Sinhala-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Sinh
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Tamil-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Taml
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Telugu-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Telu
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Thai-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Thai
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Tibetan-IDN.txt

@@ -2,7 +2,7 @@
 # Script: Tibt
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com

core/src/main/java/google/registry/idn/Traditional-Chinese-IDN.txt

@@ -1,23 +1,23 @@
 # Registry: Charleston Road Registry Inc.
-# Script: Traditional Chinese
+# Language: Traditional Chinese
 # Version: 1.0
 # Effective Date: 04-12-2012
-# Contact: tas-contact.google.com
+# Contact: iana-contact@google.com
 # Address: 1600 Amphitheatre Parkway Mountain View, CA 94043, USA
 # Telephone: +1 (650) 253-0000
 # Website: www.google.com
 # Notes: This table describes codepoints allowed for the Traditional Chinese script.
 
-Reference 0 Unicode 3.2
-Reference 1 A Complete Set of Simplified Chinese Characters
-Reference 2 Chinese Variants Collation Table
-Reference 3 Chinese Big Dictionary
-Reference 4 Chinese Relationship Table for Unihan Project
-Reference 5 GB2312
-Reference 6 General Table for Modern Chinese
-Reference 7 International Chinese Standard Big Dictionary
-Reference 8 Unihan Database
-Reference 9 BIG5
+# Reference 0 Unicode 3.2
+# Reference 1 A Complete Set of Simplified Chinese Characters
+# Reference 2 Chinese Variants Collation Table
+# Reference 3 Chinese Big Dictionary
+# Reference 4 Chinese Relationship Table for Unihan Project
+# Reference 5 GB2312
+# Reference 6 General Table for Modern Chinese
+# Reference 7 International Chinese Standard Big Dictionary
+# Reference 8 Unihan Database
+# Reference 9 BIG5
 
 U+002D(0);U+002D(0);
 U+0030(0);U+0030(0);

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

@@ -19,9 +19,12 @@ import google.registry.model.billing.BillingEvent;
 import google.registry.model.common.Cursor;
 import google.registry.model.common.EntityGroupRoot;
 import google.registry.model.common.GaeUserIdConverter;
+import google.registry.model.contact.ContactHistory;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainHistory;
 import google.registry.model.domain.token.AllocationToken;
+import google.registry.model.host.HostHistory;
 import google.registry.model.host.HostResource;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
@@ -68,9 +71,11 @@ public final class EntityClasses {
           CommitLogCheckpointRoot.class,
           CommitLogManifest.class,
           CommitLogMutation.class,
+          ContactHistory.class,
           ContactResource.class,
           Cursor.class,
           DomainBase.class,
+          DomainHistory.class,
           EntityGroupRoot.class,
           EppResourceIndex.class,
           EppResourceIndexBucket.class,
@@ -79,6 +84,7 @@ public final class EntityClasses {
           ForeignKeyIndex.ForeignKeyHostIndex.class,
           GaeUserIdConverter.class,
           HistoryEntry.class,
+          HostHistory.class,
           HostResource.class,
           KmsSecret.class,
           KmsSecretRevision.class,

core/src/main/java/google/registry/model/billing/BillingEvent.java

@@ -46,6 +46,7 @@ import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithLongVKey;
+import google.registry.schema.replay.DatastoreAndSqlEntity;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -274,7 +275,7 @@ public abstract class BillingEvent extends ImmutableObject
         @javax.persistence.Index(columnList = "allocation_token_id")
       })
   @AttributeOverride(name = "id", column = @Column(name = "billing_event_id"))
-  public static class OneTime extends BillingEvent {
+  public static class OneTime extends BillingEvent implements DatastoreAndSqlEntity {
 
     /** The billable value. */
     @AttributeOverrides({
@@ -450,7 +451,7 @@ public abstract class BillingEvent extends ImmutableObject
         @javax.persistence.Index(columnList = "recurrence_time_of_year")
       })
   @AttributeOverride(name = "id", column = @Column(name = "billing_recurrence_id"))
-  public static class Recurring extends BillingEvent {
+  public static class Recurring extends BillingEvent implements DatastoreAndSqlEntity {
 
     /**
      * The billing event recurs every year between {@link #eventTime} and this time on the
@@ -544,7 +545,7 @@ public abstract class BillingEvent extends ImmutableObject
         @javax.persistence.Index(columnList = "billingTime")
       })
   @AttributeOverride(name = "id", column = @Column(name = "billing_cancellation_id"))
-  public static class Cancellation extends BillingEvent {
+  public static class Cancellation extends BillingEvent implements DatastoreAndSqlEntity {
 
     /** The billing time of the charge that is being cancelled. */
     @Index
@@ -664,7 +665,7 @@ public abstract class BillingEvent extends ImmutableObject
   /** An event representing a modification of an existing one-time billing event. */
   @ReportedOn
   @Entity
-  public static class Modification extends BillingEvent {
+  public static class Modification extends BillingEvent implements DatastoreAndSqlEntity {
 
     /** The change in cost that should be applied to the original billing event. */
     Money cost;

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

@@ -15,11 +15,17 @@
 package google.registry.model.contact;
 
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.EntitySubclass;
 import google.registry.model.EppResource;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
 import javax.persistence.Column;
 import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
 
 /**
  * A persisted history entry representing an EPP modification to a contact.
@@ -36,13 +42,25 @@ import javax.persistence.Entity;
       @javax.persistence.Index(columnList = "historyType"),
       @javax.persistence.Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
+@Access(AccessType.FIELD)
 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;
 
+  @Id
+  @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "TempHistorySequenceGenerator")
+  @Column(name = "historyRevisionId")
+  @Access(AccessType.PROPERTY)
+  @Override
+  public long getId() {
+    return super.getId();
+  }
+
   /** The state of the {@link ContactBase} object at this point in time. */
   public ContactBase getContactBase() {
     return contactBase;

core/src/main/java/google/registry/model/domain/DomainBase.java

@@ -14,7 +14,6 @@
 
 package google.registry.model.domain;
 
-
 import com.googlecode.objectify.Key;
 import google.registry.model.EppResource;
 import google.registry.model.EppResource.ForeignKeyedEppResource;
@@ -82,12 +81,26 @@ public class DomainBase extends DomainContent
     return super.nsHosts;
   }
 
+  /**
+   * Returns the set of {@link GracePeriod} associated with the domain.
+   *
+   * <p>This is the getter method specific for Hibernate to access the field so it is set to
+   * private. The caller can use the public {@link #getGracePeriods()} to get the grace periods.
+   *
+   * <p>Note that we need to set `insertable = false, updatable = false` for @JoinColumn, otherwise
+   * Hibernate would try to set the foreign key to null(through an UPDATE TABLE sql) instead of
+   * deleting the whole entry from the table when the {@link GracePeriod} is removed from the set.
+   */
   @Access(AccessType.PROPERTY)
   @OneToMany(
       cascade = {CascadeType.ALL},
       fetch = FetchType.EAGER,
       orphanRemoval = true)
-  @JoinColumn(name = "domainRepoId", referencedColumnName = "repoId")
+  @JoinColumn(
+      name = "domainRepoId",
+      referencedColumnName = "repoId",
+      insertable = false,
+      updatable = false)
   @SuppressWarnings("UnusedMethod")
   private Set<GracePeriod> getInternalGracePeriods() {
     return gracePeriods;

core/src/main/java/google/registry/model/domain/DomainContent.java

@@ -280,12 +280,10 @@ public class DomainContent extends EppResource
     // object will have a null hashcode so that it can get a recalculated hashcode
     // when its hashCode() is invoked.
     // TODO(b/162739503): Remove this after fully migrating to Cloud SQL.
-    if (gracePeriods != null) {
-      gracePeriods =
-          gracePeriods.stream()
-              .map(gracePeriod -> gracePeriod.cloneWithDomainRepoId(getRepoId()))
-              .collect(toImmutableSet());
-    }
+    gracePeriods =
+        nullToEmptyImmutableCopy(gracePeriods).stream()
+            .map(gracePeriod -> gracePeriod.cloneWithDomainRepoId(getRepoId()))
+            .collect(toImmutableSet());
   }
 
   @PostLoad
@@ -380,7 +378,7 @@ public class DomainContent extends EppResource
   // Hibernate needs this in order to populate nsHosts but no one else should ever use it
   @SuppressWarnings("UnusedMethod")
   private void setNsHosts(Set<VKey<HostResource>> nsHosts) {
-    this.nsHosts = nsHosts;
+    this.nsHosts = forceEmptyToNull(nsHosts);
   }
 
   // Hibernate needs this in order to populate gracePeriods but no one else should ever use it
@@ -698,7 +696,13 @@ public class DomainContent extends EppResource
       }
       checkArgumentNotNull(instance.getRegistrant(), "Missing registrant");
       instance.tld = getTldFromDomainName(instance.fullyQualifiedDomainName);
-      return super.build();
+
+      T newDomain = super.build();
+      // Hibernate throws exception if gracePeriods is null because we enabled all cascadable
+      // operations and orphan removal.
+      newDomain.gracePeriods =
+          newDomain.gracePeriods == null ? ImmutableSet.of() : newDomain.gracePeriods;
+      return newDomain;
     }
 
     public B setDomainName(String domainName) {

core/src/main/java/google/registry/model/domain/DomainHistory.java

@@ -14,19 +14,32 @@
 
 package google.registry.model.domain;
 
+import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
+
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.EntitySubclass;
+import com.googlecode.objectify.annotation.Ignore;
 import google.registry.model.EppResource;
-import google.registry.model.contact.ContactResource;
+import google.registry.model.ImmutableObject;
+import google.registry.model.domain.DomainHistory.DomainHistoryId;
 import google.registry.model.host.HostResource;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import java.io.Serializable;
 import java.util.Set;
 import javax.persistence.Access;
 import javax.persistence.AccessType;
 import javax.persistence.Column;
 import javax.persistence.ElementCollection;
 import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.IdClass;
+import javax.persistence.Index;
 import javax.persistence.JoinTable;
+import javax.persistence.PostLoad;
+import javax.persistence.Table;
 
 /**
  * A persisted history entry representing an EPP modification to a domain.
@@ -36,26 +49,44 @@ import javax.persistence.JoinTable;
  * the foreign-keyed fields in that class can refer to this object.
  */
 @Entity
-@javax.persistence.Table(
+@Table(
     indexes = {
-      @javax.persistence.Index(columnList = "creationTime"),
-      @javax.persistence.Index(columnList = "historyRegistrarId"),
-      @javax.persistence.Index(columnList = "historyType"),
-      @javax.persistence.Index(columnList = "historyModificationTime")
+      @Index(columnList = "creationTime"),
+      @Index(columnList = "historyRegistrarId"),
+      @Index(columnList = "historyType"),
+      @Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
+@Access(AccessType.FIELD)
+@IdClass(DomainHistoryId.class)
 public class DomainHistory extends HistoryEntry {
+
   // Store DomainContent instead of DomainBase so we don't pick up its @Id
   DomainContent domainContent;
 
-  @Column(nullable = false)
-  VKey<DomainBase> domainRepoId;
+  @Id String domainRepoId;
 
+  // We could have reused domainContent.nsHosts here, but Hibernate throws a weird exception after
+  // we change to use a composite primary key.
+  // TODO(b/166776754): Investigate if we can reuse domainContent.nsHosts for storing host keys.
+  @Ignore
   @ElementCollection
   @JoinTable(name = "DomainHistoryHost")
-  @Access(AccessType.PROPERTY)
   @Column(name = "host_repo_id")
+  Set<VKey<HostResource>> nsHosts;
+
+  @Id
+  @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "TempHistorySequenceGenerator")
+  @Column(name = "historyRevisionId")
+  @Access(AccessType.PROPERTY)
+  @Override
+  public long getId() {
+    return super.getId();
+  }
+
+  /** Returns keys to the {@link HostResource} that are the nameservers for the domain. */
   public Set<VKey<HostResource>> getNsHosts() {
-    return domainContent.nsHosts;
+    return nsHosts;
   }
 
   /** The state of the {@link DomainContent} object at this point in time. */
@@ -63,16 +94,51 @@ public class DomainHistory extends HistoryEntry {
     return domainContent;
   }
 
-  /** The key to the {@link ContactResource} this is based off of. */
+  /** The key to the {@link DomainBase} this is based off of. */
   public VKey<DomainBase> getDomainRepoId() {
-    return domainRepoId;
+    return VKey.create(DomainBase.class, domainRepoId, Key.create(DomainBase.class, domainRepoId));
   }
 
-  // Hibernate needs this in order to populate nsHosts but no one else should ever use it
-  @SuppressWarnings("UnusedMethod")
-  private void setNsHosts(Set<VKey<HostResource>> nsHosts) {
+  public VKey<DomainHistory> createVKey() {
+    return VKey.createSql(DomainHistory.class, new DomainHistoryId(domainRepoId, getId()));
+  }
+
+  @PostLoad
+  void postLoad() {
     if (domainContent != null) {
-      domainContent.nsHosts = nsHosts;
+      domainContent.nsHosts = nullToEmptyImmutableCopy(nsHosts);
+    }
+  }
+
+  /** Class to represent the composite primary key of {@link DomainHistory} entity. */
+  static class DomainHistoryId extends ImmutableObject implements Serializable {
+
+    private String domainRepoId;
+
+    private Long id;
+
+    /** Hibernate requires this default constructor. */
+    private DomainHistoryId() {}
+
+    DomainHistoryId(String domainRepoId, long id) {
+      this.domainRepoId = domainRepoId;
+      this.id = id;
+    }
+
+    String getDomainRepoId() {
+      return domainRepoId;
+    }
+
+    void setDomainRepoId(String domainRepoId) {
+      this.domainRepoId = domainRepoId;
+    }
+
+    long getId() {
+      return id;
+    }
+
+    void setId(long id) {
+      this.id = id;
     }
   }
 
@@ -91,12 +157,15 @@ public class DomainHistory extends HistoryEntry {
 
     public Builder setDomainContent(DomainContent domainContent) {
       getInstance().domainContent = domainContent;
+      if (domainContent != null) {
+        getInstance().nsHosts = nullToEmptyImmutableCopy(domainContent.nsHosts);
+      }
       return this;
     }
 
-    public Builder setDomainRepoId(VKey<DomainBase> domainRepoId) {
+    public Builder setDomainRepoId(String domainRepoId) {
       getInstance().domainRepoId = domainRepoId;
-      domainRepoId.maybeGetOfyKey().ifPresent(parent -> getInstance().parent = parent);
+      getInstance().parent = Key.create(DomainBase.class, domainRepoId);
       return this;
     }
 
@@ -104,8 +173,7 @@ public class DomainHistory extends HistoryEntry {
     @Override
     public Builder setParent(Key<? extends EppResource> parent) {
       super.setParent(parent);
-      getInstance().domainRepoId =
-          VKey.create(DomainBase.class, parent.getName(), (Key<DomainBase>) parent);
+      getInstance().domainRepoId = parent.getName();
       return this;
     }
   }

core/src/main/java/google/registry/model/domain/superuser/DomainDeleteSuperuserExtension.java

@@ -20,6 +20,7 @@ import javax.xml.bind.annotation.XmlRootElement;
 /** A superuser extension that may be present on domain delete commands. */
 @XmlRootElement(name = "domainDelete")
 public class DomainDeleteSuperuserExtension extends SuperuserExtension {
+
   @XmlElement(name = "redemptionGracePeriodDays")
   int redemptionGracePeriodDays;
 

core/src/main/java/google/registry/model/domain/superuser/DomainTransferRequestSuperuserExtension.java

@@ -21,6 +21,7 @@ import javax.xml.bind.annotation.XmlRootElement;
 /** A superuser extension that may be present on domain transfer request commands. */
 @XmlRootElement(name = "domainTransferRequest")
 public class DomainTransferRequestSuperuserExtension extends SuperuserExtension {
+
   // We need to specify the period here because the transfer object's period cannot be set to zero.
   @XmlElement(name = "renewalPeriod")
   Period renewalPeriod;

core/src/main/java/google/registry/model/domain/superuser/DomainUpdateSuperuserExtension.java

@@ -0,0 +1,35 @@
+// 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.domain.superuser;
+
+import static com.google.common.base.Strings.isNullOrEmpty;
+
+import java.util.Optional;
+import javax.annotation.Nullable;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+
+/** A superuser extension that may be present on domain update commands. */
+@XmlRootElement(name = "domainUpdate")
+public class DomainUpdateSuperuserExtension extends SuperuserExtension {
+
+  @XmlElement(name = "autorenews")
+  @Nullable
+  String autorenews;
+
+  public Optional<Boolean> getAutorenews() {
+    return Optional.ofNullable(isNullOrEmpty(autorenews) ? null : Boolean.valueOf(autorenews));
+  }
+}

core/src/main/java/google/registry/model/domain/token/AllocationToken.java

@@ -49,16 +49,32 @@ import google.registry.model.common.TimedTransitionProperty.TimedTransition;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithStringVKey;
+import google.registry.schema.replay.DatastoreAndSqlEntity;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
+import javax.persistence.Column;
+import javax.persistence.EnumType;
+import javax.persistence.Enumerated;
+import javax.persistence.Table;
 import org.joda.time.DateTime;
 
 /** An entity representing an allocation token. */
 @ReportedOn
 @Entity
 @WithStringVKey
-public class AllocationToken extends BackupGroupRoot implements Buildable {
+@javax.persistence.Entity
+@Table(
+    indexes = {
+      @javax.persistence.Index(
+          columnList = "token",
+          name = "allocation_token_token_idx",
+          unique = true),
+      @javax.persistence.Index(
+          columnList = "domainName",
+          name = "allocation_token_domain_name_idx"),
+    })
+public class AllocationToken extends BackupGroupRoot implements Buildable, DatastoreAndSqlEntity {
 
   // Promotions should only move forward, and ENDED / CANCELLED are terminal states.
   private static final ImmutableMultimap<TokenStatus, TokenStatus> VALID_TOKEN_STATUS_TRANSITIONS =
@@ -86,19 +102,22 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
   }
 
   /** The allocation token string. */
-  @Id String token;
+  @javax.persistence.Id @Id String token;
 
   /** The key of the history entry for which the token was used. Null if not yet used. */
-  @Nullable @Index Key<HistoryEntry> redemptionHistoryEntry;
+  @Nullable @Index VKey<HistoryEntry> redemptionHistoryEntry;
 
   /** The fully-qualified domain name that this token is limited to, if any. */
   @Nullable @Index String domainName;
 
   /** When this token was created. */
+  @Column(nullable = false)
   CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null);
 
   /** Allowed registrar client IDs for this token, or null if all registrars are allowed. */
-  @Nullable Set<String> allowedClientIds;
+  @Column(name = "allowedRegistrarIds")
+  @Nullable
+  Set<String> allowedClientIds;
 
   /** Allowed TLDs for this token, or null if all TLDs are allowed. */
   @Nullable Set<String> allowedTlds;
@@ -117,6 +136,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
   int discountYears = 1;
 
   /** The type of the token, either single-use or unlimited-use. */
+  @Enumerated(EnumType.STRING)
   TokenType tokenType;
 
   // TODO: Remove onLoad once all allocation tokens are migrated to have a discountYears of 1.
@@ -161,7 +181,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
     return token;
   }
 
-  public Optional<Key<HistoryEntry>> getRedemptionHistoryEntry() {
+  public Optional<VKey<HistoryEntry>> getRedemptionHistoryEntry() {
     return Optional.ofNullable(redemptionHistoryEntry);
   }
 
@@ -177,7 +197,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
     return Optional.ofNullable(creationTime.getTimestamp());
   }
 
-  public ImmutableSet<String> getAllowedClientIds() {
+  public ImmutableSet<String> getAllowedRegistrarIds() {
     return nullToEmptyImmutableCopy(allowedClientIds);
   }
 
@@ -260,7 +280,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
       return this;
     }
 
-    public Builder setRedemptionHistoryEntry(Key<HistoryEntry> redemptionHistoryEntry) {
+    public Builder setRedemptionHistoryEntry(VKey<HistoryEntry> redemptionHistoryEntry) {
       getInstance().redemptionHistoryEntry =
           checkArgumentNotNull(redemptionHistoryEntry, "Redemption history entry must not be null");
       return this;
@@ -279,8 +299,8 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
       return this;
     }
 
-    public Builder setAllowedClientIds(Set<String> allowedClientIds) {
-      getInstance().allowedClientIds = forceEmptyToNull(allowedClientIds);
+    public Builder setAllowedRegistrarIds(Set<String> allowedRegistrarIds) {
+      getInstance().allowedClientIds = forceEmptyToNull(allowedRegistrarIds);
       return this;
     }
 

core/src/main/java/google/registry/model/eppinput/EppInput.java

@@ -50,6 +50,7 @@ import google.registry.model.domain.secdns.SecDnsCreateExtension;
 import google.registry.model.domain.secdns.SecDnsUpdateExtension;
 import google.registry.model.domain.superuser.DomainDeleteSuperuserExtension;
 import google.registry.model.domain.superuser.DomainTransferRequestSuperuserExtension;
+import google.registry.model.domain.superuser.DomainUpdateSuperuserExtension;
 import google.registry.model.domain.token.AllocationTokenExtension;
 import google.registry.model.eppinput.ResourceCommand.ResourceCheck;
 import google.registry.model.eppinput.ResourceCommand.SingleResourceCommand;
@@ -309,53 +310,62 @@ public class EppInput extends ImmutableObject {
   @XmlType(propOrder = {"command", "extension", "clTRID"})
   public static class CommandWrapper extends ImmutableObject {
     @XmlElements({
-        @XmlElement(name = "check", type = Check.class),
-        @XmlElement(name = "create", type = Create.class),
-        @XmlElement(name = "delete", type = Delete.class),
-        @XmlElement(name = "info", type = Info.class),
-        @XmlElement(name = "login", type = Login.class),
-        @XmlElement(name = "logout", type = Logout.class),
-        @XmlElement(name = "poll", type = Poll.class),
-        @XmlElement(name = "renew", type = Renew.class),
-        @XmlElement(name = "transfer", type = Transfer.class),
-        @XmlElement(name = "update", type = Update.class) })
+      @XmlElement(name = "check", type = Check.class),
+      @XmlElement(name = "create", type = Create.class),
+      @XmlElement(name = "delete", type = Delete.class),
+      @XmlElement(name = "info", type = Info.class),
+      @XmlElement(name = "login", type = Login.class),
+      @XmlElement(name = "logout", type = Logout.class),
+      @XmlElement(name = "poll", type = Poll.class),
+      @XmlElement(name = "renew", type = Renew.class),
+      @XmlElement(name = "transfer", type = Transfer.class),
+      @XmlElement(name = "update", type = Update.class)
+    })
     InnerCommand command;
 
     /** Zero or more command extensions. */
     @XmlElementRefs({
-        // allocation token extension
-        @XmlElementRef(type = AllocationTokenExtension.class),
-        // fee extension version 0.6
-        @XmlElementRef(type = FeeCheckCommandExtensionV06.class),
-        @XmlElementRef(type = FeeInfoCommandExtensionV06.class),
-        @XmlElementRef(type = FeeCreateCommandExtensionV06.class),
-        @XmlElementRef(type = FeeRenewCommandExtensionV06.class),
-        @XmlElementRef(type = FeeTransferCommandExtensionV06.class),
-        @XmlElementRef(type = FeeUpdateCommandExtensionV06.class),
-        // fee extension version 0.11
-        @XmlElementRef(type = FeeCheckCommandExtensionV11.class),
-        @XmlElementRef(type = FeeCreateCommandExtensionV11.class),
-        @XmlElementRef(type = FeeRenewCommandExtensionV11.class),
-        @XmlElementRef(type = FeeTransferCommandExtensionV11.class),
-        @XmlElementRef(type = FeeUpdateCommandExtensionV11.class),
-        // fee extension version 0.12
-        @XmlElementRef(type = FeeCheckCommandExtensionV12.class),
-        @XmlElementRef(type = FeeCreateCommandExtensionV12.class),
-        @XmlElementRef(type = FeeRenewCommandExtensionV12.class),
-        @XmlElementRef(type = FeeTransferCommandExtensionV12.class),
-        @XmlElementRef(type = FeeUpdateCommandExtensionV12.class),
-        // other extensions
-        @XmlElementRef(type = LaunchCheckExtension.class),
-        @XmlElementRef(type = LaunchCreateExtension.class),
-        @XmlElementRef(type = LaunchDeleteExtension.class),
-        @XmlElementRef(type = LaunchInfoExtension.class),
-        @XmlElementRef(type = LaunchUpdateExtension.class),
-        @XmlElementRef(type = MetadataExtension.class),
-        @XmlElementRef(type = RgpUpdateExtension.class),
-        @XmlElementRef(type = SecDnsCreateExtension.class),
-        @XmlElementRef(type = SecDnsUpdateExtension.class),
-        @XmlElementRef(type = DomainTransferRequestSuperuserExtension.class),
-        @XmlElementRef(type = DomainDeleteSuperuserExtension.class) })
+      // Fee extension version 0.6
+      @XmlElementRef(type = FeeCheckCommandExtensionV06.class),
+      @XmlElementRef(type = FeeInfoCommandExtensionV06.class),
+      @XmlElementRef(type = FeeCreateCommandExtensionV06.class),
+      @XmlElementRef(type = FeeRenewCommandExtensionV06.class),
+      @XmlElementRef(type = FeeTransferCommandExtensionV06.class),
+      @XmlElementRef(type = FeeUpdateCommandExtensionV06.class),
+
+      // Fee extension version 0.11
+      @XmlElementRef(type = FeeCheckCommandExtensionV11.class),
+      @XmlElementRef(type = FeeCreateCommandExtensionV11.class),
+      @XmlElementRef(type = FeeRenewCommandExtensionV11.class),
+      @XmlElementRef(type = FeeTransferCommandExtensionV11.class),
+      @XmlElementRef(type = FeeUpdateCommandExtensionV11.class),
+
+      // Fee extension version 0.12
+      @XmlElementRef(type = FeeCheckCommandExtensionV12.class),
+      @XmlElementRef(type = FeeCreateCommandExtensionV12.class),
+      @XmlElementRef(type = FeeRenewCommandExtensionV12.class),
+      @XmlElementRef(type = FeeTransferCommandExtensionV12.class),
+      @XmlElementRef(type = FeeUpdateCommandExtensionV12.class),
+
+      // Launch phase extensions
+      @XmlElementRef(type = LaunchCheckExtension.class),
+      @XmlElementRef(type = LaunchCreateExtension.class),
+      @XmlElementRef(type = LaunchDeleteExtension.class),
+      @XmlElementRef(type = LaunchInfoExtension.class),
+      @XmlElementRef(type = LaunchUpdateExtension.class),
+
+      // Superuser extensions
+      @XmlElementRef(type = DomainDeleteSuperuserExtension.class),
+      @XmlElementRef(type = DomainTransferRequestSuperuserExtension.class),
+      @XmlElementRef(type = DomainUpdateSuperuserExtension.class),
+
+      // Other extensions
+      @XmlElementRef(type = AllocationTokenExtension.class),
+      @XmlElementRef(type = MetadataExtension.class),
+      @XmlElementRef(type = RgpUpdateExtension.class),
+      @XmlElementRef(type = SecDnsCreateExtension.class),
+      @XmlElementRef(type = SecDnsUpdateExtension.class)
+    })
     @XmlElementWrapper
     List<CommandExtension> extension;
 

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

@@ -15,11 +15,17 @@
 package google.registry.model.host;
 
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.EntitySubclass;
 import google.registry.model.EppResource;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
 import javax.persistence.Column;
 import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
 
 /**
  * A persisted history entry representing an EPP modification to a host.
@@ -37,6 +43,8 @@ import javax.persistence.Entity;
       @javax.persistence.Index(columnList = "historyType"),
       @javax.persistence.Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
+@Access(AccessType.FIELD)
 public class HostHistory extends HistoryEntry {
 
   // Store HostBase instead of HostResource so we don't pick up its @Id
@@ -45,6 +53,15 @@ public class HostHistory extends HistoryEntry {
   @Column(nullable = false)
   VKey<HostResource> hostRepoId;
 
+  @Id
+  @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "TempHistorySequenceGenerator")
+  @Column(name = "historyRevisionId")
+  @Access(AccessType.PROPERTY)
+  @Override
+  public long getId() {
+    return super.getId();
+  }
+
   /** The state of the {@link HostBase} object at this point in time. */
   public HostBase getHostBase() {
     return hostBase;

core/src/main/java/google/registry/model/index/EppResourceIndex.java

@@ -17,6 +17,7 @@ package google.registry.model.index;
 import static google.registry.util.TypeUtils.instantiate;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
@@ -25,11 +26,13 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.BackupGroupRoot;
 import google.registry.model.EppResource;
 import google.registry.model.annotations.ReportedOn;
+import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.SqlEntity;
 
 /** An index that allows for quick enumeration of all EppResource entities (e.g. via map reduce). */
 @ReportedOn
 @Entity
-public class EppResourceIndex extends BackupGroupRoot {
+public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity {
 
   @Id
   String id;
@@ -74,4 +77,9 @@ public class EppResourceIndex extends BackupGroupRoot {
   public static <T extends EppResource> EppResourceIndex create(Key<T> resourceKey) {
     return create(EppResourceIndexBucket.getBucketKey(resourceKey), resourceKey);
   }
+
+  @Override
+  public ImmutableList<SqlEntity> toSqlEntities() {
+    return ImmutableList.of(); // not relevant in SQL
+  }
 }

core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java

@@ -24,16 +24,23 @@ import com.googlecode.objectify.annotation.Id;
 import google.registry.model.EppResource;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.VirtualEntity;
+import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.SqlEntity;
 
 /** A virtual entity to represent buckets to which EppResourceIndex objects are randomly added. */
 @Entity
 @VirtualEntity
-public class EppResourceIndexBucket extends ImmutableObject {
+public class EppResourceIndexBucket extends ImmutableObject implements DatastoreEntity {
 
   @SuppressWarnings("unused")
   @Id
   private long bucketId;
 
+  @Override
+  public ImmutableList<SqlEntity> toSqlEntities() {
+    return ImmutableList.of(); // not relevant in SQL
+  }
+
   /**
    * Deterministic function that returns a bucket id based on the resource's roid.
    * NB: At the moment, nothing depends on this being deterministic, so we have the ability to

core/src/main/java/google/registry/model/index/ForeignKeyIndex.java

@@ -43,6 +43,8 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.host.HostResource;
 import google.registry.persistence.VKey;
+import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.SqlEntity;
 import google.registry.util.NonFinalForTesting;
 import java.util.Map;
 import java.util.Optional;
@@ -61,28 +63,44 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   /** The {@link ForeignKeyIndex} type for {@link ContactResource} entities. */
   @ReportedOn
   @Entity
-  public static class ForeignKeyContactIndex extends ForeignKeyIndex<ContactResource> {}
+  public static class ForeignKeyContactIndex extends ForeignKeyIndex<ContactResource>
+      implements DatastoreEntity {
+    @Override
+    public ImmutableList<SqlEntity> toSqlEntities() {
+      return ImmutableList.of(); // not relevant in SQL
+    }
+  }
 
   /** The {@link ForeignKeyIndex} type for {@link DomainBase} entities. */
   @ReportedOn
   @Entity
-  public static class ForeignKeyDomainIndex extends ForeignKeyIndex<DomainBase> {}
+  public static class ForeignKeyDomainIndex extends ForeignKeyIndex<DomainBase>
+      implements DatastoreEntity {
+    @Override
+    public ImmutableList<SqlEntity> toSqlEntities() {
+      return ImmutableList.of(); // not relevant in SQL
+    }
+  }
 
   /** The {@link ForeignKeyIndex} type for {@link HostResource} entities. */
   @ReportedOn
   @Entity
-  public static class ForeignKeyHostIndex extends ForeignKeyIndex<HostResource> {}
+  public static class ForeignKeyHostIndex extends ForeignKeyIndex<HostResource>
+      implements DatastoreEntity {
+    @Override
+    public ImmutableList<SqlEntity> toSqlEntities() {
+      return ImmutableList.of(); // not relevant in SQL
+    }
+  }
 
-  static final ImmutableMap<
-          Class<? extends EppResource>, Class<? extends ForeignKeyIndex<?>>>
+  static final ImmutableMap<Class<? extends EppResource>, Class<? extends ForeignKeyIndex<?>>>
       RESOURCE_CLASS_TO_FKI_CLASS =
           ImmutableMap.of(
               ContactResource.class, ForeignKeyContactIndex.class,
               DomainBase.class, ForeignKeyDomainIndex.class,
               HostResource.class, ForeignKeyHostIndex.class);
 
-  @Id
-  String foreignKey;
+  @Id String foreignKey;
 
   /**
    * The deletion time of this {@link ForeignKeyIndex}.
@@ -90,8 +108,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
    * <p>This will generally be equal to the deletion time of {@link #topReference}. However, in the
    * case of a {@link HostResource} that was renamed, this field will hold the time of the rename.
    */
-  @Index
-  DateTime deletionTime;
+  @Index DateTime deletionTime;
 
   /**
    * The referenced resource.

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

@@ -24,13 +24,16 @@ import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.Key;
+import google.registry.model.contact.ContactHistory;
+import google.registry.model.host.HostHistory;
+import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.TransactionManager;
-import java.util.Map.Entry;
 import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.function.Supplier;
 import java.util.stream.StreamSupport;
+import javax.annotation.Nullable;
 import org.joda.time.DateTime;
 
 /** Datastore implementation of {@link TransactionManager}. */
@@ -99,8 +102,7 @@ public class DatastoreTransactionManager implements TransactionManager {
 
   @Override
   public void saveNew(Object entity) {
-    checkArgumentNotNull(entity, "entity must be specified");
-    getOfy().save().entity(entity);
+    saveEntity(entity);
   }
 
   @Override
@@ -110,8 +112,7 @@ public class DatastoreTransactionManager implements TransactionManager {
 
   @Override
   public void saveNewOrUpdate(Object entity) {
-    checkArgumentNotNull(entity, "entity must be specified");
-    getOfy().save().entity(entity);
+    saveEntity(entity);
   }
 
   @Override
@@ -121,8 +122,7 @@ public class DatastoreTransactionManager implements TransactionManager {
 
   @Override
   public void update(Object entity) {
-    checkArgumentNotNull(entity, "entity must be specified");
-    getOfy().save().entity(entity);
+    saveEntity(entity);
   }
 
   @Override
@@ -137,7 +137,7 @@ public class DatastoreTransactionManager implements TransactionManager {
 
   @Override
   public <T> boolean checkExists(VKey<T> key) {
-    return getOfy().load().key(key.getOfyKey()).now() != null;
+    return loadNullable(key) != null;
   }
 
   // TODO: add tests for these methods.  They currently have some degree of test coverage because
@@ -146,12 +146,12 @@ public class DatastoreTransactionManager implements TransactionManager {
   // interface tests that are applied to both the datastore and SQL implementations.
   @Override
   public <T> Optional<T> maybeLoad(VKey<T> key) {
-    return Optional.ofNullable(getOfy().load().key(key.getOfyKey()).now());
+    return Optional.ofNullable(loadNullable(key));
   }
 
   @Override
   public <T> T load(VKey<T> key) {
-    T result = getOfy().load().key(key.getOfyKey()).now();
+    T result = loadNullable(key);
     if (result == null) {
       throw new NoSuchElementException(key.toString());
     }
@@ -167,7 +167,10 @@ public class DatastoreTransactionManager implements TransactionManager {
             .collect(toImmutableMap(key -> (Key<T>) key.getOfyKey(), Functions.identity()));
 
     return getOfy().load().keys(keyMap.keySet()).entrySet().stream()
-        .collect(ImmutableMap.toImmutableMap(entry -> keyMap.get(entry.getKey()), Entry::getValue));
+        .collect(
+            toImmutableMap(
+                entry -> keyMap.get(entry.getKey()),
+                entry -> toChildHistoryEntryIfPossible(entry.getValue())));
   }
 
   @Override
@@ -191,4 +194,37 @@ public class DatastoreTransactionManager implements TransactionManager {
             .collect(toImmutableList());
     getOfy().delete().keys(list).now();
   }
+
+  /**
+   * The following three methods exist due to the migration to Cloud SQL.
+   *
+   * <p>In Cloud SQL, {@link HistoryEntry} objects are represented instead as {@link DomainHistory},
+   * {@link ContactHistory}, and {@link HostHistory} objects. During the migration, we do not wish
+   * to change the Datastore schema so all of these objects are stored in Datastore as HistoryEntry
+   * objects. They are converted to/from the appropriate classes upon retrieval, and converted to
+   * HistoryEntry on save. See go/r3.0-history-objects for more details.
+   */
+  private void saveEntity(Object entity) {
+    checkArgumentNotNull(entity, "entity must be specified");
+    if (entity instanceof HistoryEntry) {
+      entity = ((HistoryEntry) entity).asHistoryEntry();
+    }
+    getOfy().save().entity(entity);
+  }
+
+  @SuppressWarnings("unchecked")
+  private <T> T toChildHistoryEntryIfPossible(@Nullable T obj) {
+    // NB: The Key of the object in question may not necessarily be the resulting class that we
+    // wish to have. Because all *History classes are @EntitySubclasses, their Keys will have type
+    // HistoryEntry -- even if you create them based off the *History class.
+    if (obj != null && HistoryEntry.class.isAssignableFrom(obj.getClass())) {
+      return (T) ((HistoryEntry) obj).toChildHistoryEntity();
+    }
+    return obj;
+  }
+
+  @Nullable
+  private <T> T loadNullable(VKey<T> key) {
+    return toChildHistoryEntryIfPossible(getOfy().load().key(key.getOfyKey()).now());
+  }
 }

core/src/main/java/google/registry/model/poll/PollMessage.java

@@ -45,6 +45,7 @@ import google.registry.model.transfer.TransferResponse.ContactTransferResponse;
 import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithLongVKey;
+import google.registry.schema.replay.DatastoreAndSqlEntity;
 import java.util.List;
 import java.util.Optional;
 import javax.persistence.AttributeOverride;
@@ -92,7 +93,7 @@ import org.joda.time.DateTime;
       @javax.persistence.Index(columnList = "eventTime")
     })
 public abstract class PollMessage extends ImmutableObject
-    implements Buildable, TransferServerApproveEntity {
+    implements Buildable, DatastoreAndSqlEntity, TransferServerApproveEntity {
 
   /** Entity id. */
   @Id

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

@@ -73,6 +73,7 @@ import google.registry.model.annotations.ReportedOn;
 import google.registry.model.common.EntityGroupRoot;
 import google.registry.model.registrar.Registrar.BillingAccountEntry.CurrencyMapper;
 import google.registry.model.registry.Registry;
+import google.registry.persistence.VKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
 import google.registry.util.CidrAddressBlock;
 import java.security.cert.CertificateParsingException;
@@ -703,6 +704,10 @@ public class Registrar extends ImmutableObject
     return new Builder(clone(this));
   }
 
+  public VKey<Registrar> createVKey() {
+    return VKey.create(Registrar.class, clientIdentifier, Key.create(this));
+  }
+
   /** A builder for constructing {@link Registrar}, since it is immutable. */
   public static class Builder extends Buildable.Builder<Registrar> {
     public Builder() {}

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

@@ -20,6 +20,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.Sets.difference;
 import static com.google.common.io.BaseEncoding.base64;
+import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registrar.Registrar.checkValidEmail;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@@ -35,20 +36,26 @@ import com.google.common.collect.Streams;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
+import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.Index;
+import com.googlecode.objectify.annotation.OnLoad;
 import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.model.JsonMapBuilder;
 import google.registry.model.Jsonifiable;
 import google.registry.model.annotations.ReportedOn;
+import google.registry.model.registrar.RegistrarContact.RegistrarPocId;
+import google.registry.persistence.VKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
+import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
-import javax.persistence.Column;
+import javax.persistence.IdClass;
+import javax.persistence.PostLoad;
 import javax.persistence.Table;
 import javax.persistence.Transient;
 
@@ -59,16 +66,17 @@ import javax.persistence.Transient;
  * <p>IMPORTANT NOTE: Any time that you change, update, or delete RegistrarContact entities, you
  * *MUST* also modify the persisted Registrar entity with {@link Registrar#contactsRequireSyncing}
  * set to true.
+ *
+ * <p>TODO(b/163366543): Rename the class name to RegistrarPoc after database migration
  */
 @ReportedOn
 @Entity
-@javax.persistence.Entity
+@javax.persistence.Entity(name = "RegistrarPoc")
 @Table(
-    name = "RegistrarPoc",
     indexes = {
       @javax.persistence.Index(columnList = "gaeUserId", name = "registrarpoc_gae_user_id_idx")
     })
-// TODO(shicong): Rename the class name to RegistrarPoc after database migration
+@IdClass(RegistrarPocId.class)
 public class RegistrarContact extends ImmutableObject
     implements DatastoreAndSqlEntity, Jsonifiable {
 
@@ -113,9 +121,10 @@ public class RegistrarContact extends ImmutableObject
   /** The email address of the contact. */
   @Id
   @javax.persistence.Id
-  @Column(nullable = false)
   String emailAddress;
 
+  @Ignore @javax.persistence.Id String registrarId;
+
   /** External email address of this contact used for registry lock confirmations. */
   String registryLockEmailAddress;
 
@@ -342,6 +351,39 @@ public class RegistrarContact extends ImmutableObject
         .build();
   }
 
+  /** Sets Cloud SQL specific fields when the entity is loaded from Datastore. */
+  @OnLoad
+  void onLoad() {
+    registrarId = parent.getName();
+  }
+
+  /** Sets Datastore specific fields when the entity is loaded from Cloud SQL. */
+  @PostLoad
+  void postLoad() {
+    parent = Key.create(getCrossTldKey(), Registrar.class, registrarId);
+  }
+
+  public VKey<RegistrarContact> createVKey() {
+    return VKey.create(
+        RegistrarContact.class, new RegistrarPocId(emailAddress, registrarId), Key.create(this));
+  }
+
+  /** Class to represent the composite primary key for {@link RegistrarContact} entity. */
+  static class RegistrarPocId extends ImmutableObject implements Serializable {
+
+    String emailAddress;
+
+    String registrarId;
+
+    // Hibernate requires this default constructor.
+    private RegistrarPocId() {}
+
+    RegistrarPocId(String emailAddress, String registrarId) {
+      this.emailAddress = emailAddress;
+      this.registrarId = registrarId;
+    }
+  }
+
   /** A builder for constructing a {@link RegistrarContact}, since it is immutable. */
   public static class Builder extends Buildable.Builder<RegistrarContact> {
     public Builder() {}
@@ -372,6 +414,7 @@ public class RegistrarContact extends ImmutableObject
             !isNullOrEmpty(getInstance().registryLockEmailAddress),
             "Registry lock email must not be null if allowing registry lock access");
       }
+      getInstance().registrarId = getInstance().parent.getName();
       return cloneEmptyToNull(super.build());
     }
 

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

@@ -340,12 +340,12 @@ public class Registry extends ImmutableObject implements Buildable {
   TldType tldType = TldType.REAL;
 
   /**
-   * Whether to disable invoicing for a {@link TldType#REAL} TLD.
+   * Whether to enable invoicing for this TLD.
    *
-   * <p>Note that invoicing is always disabled for {@link TldType#TEST} TLDs. Setting this field has
-   * no effect for {@link TldType#TEST} TLDs.
+   * <p>Note that this boolean is the sole determiner on whether invoices should be generated for a
+   * TLD. This applies to {@link TldType#TEST} TLDs as well.
    */
-  boolean disableInvoicing = false;
+  boolean invoicingEnabled = false;
 
   /**
    * A property that transitions to different TldStates at different times. Stored as a list of
@@ -646,8 +646,8 @@ public class Registry extends ImmutableObject implements Buildable {
       return this;
     }
 
-    public Builder setDisableInvoicing(boolean disableInvoicing) {
-      getInstance().disableInvoicing = disableInvoicing;
+    public Builder setInvoicingEnabled(boolean invoicingEnabled) {
+      getInstance().invoicingEnabled = invoicingEnabled;
       return this;
     }
 

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

@@ -114,7 +114,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
   /** Virtual parent entity for premium list entry entities associated with a single revision. */
   @ReportedOn
   @Entity
-  public static class PremiumListRevision extends ImmutableObject {
+  public static class PremiumListRevision extends ImmutableObject implements DatastoreEntity {
 
     @Parent Key<PremiumList> parent;
 
@@ -171,6 +171,11 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
       }
       return revision;
     }
+
+    @Override
+    public ImmutableList<SqlEntity> toSqlEntities() {
+      return ImmutableList.of(); // not persisted in SQL
+    }
   }
 
   /**

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

@@ -14,8 +14,11 @@
 
 package google.registry.model.reporting;
 
+import static com.googlecode.objectify.Key.getKind;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
@@ -28,20 +31,28 @@ import google.registry.model.Buildable;
 import google.registry.model.EppResource;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ReportedOn;
+import google.registry.model.contact.ContactHistory;
+import google.registry.model.contact.ContactResource;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainHistory;
 import google.registry.model.domain.Period;
 import google.registry.model.eppcommon.Trid;
+import google.registry.model.host.HostHistory;
+import google.registry.model.host.HostResource;
 import google.registry.persistence.VKey;
+import google.registry.persistence.WithStringVKey;
+import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.SqlEntity;
 import java.util.Set;
 import javax.annotation.Nullable;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
 import javax.persistence.AttributeOverride;
 import javax.persistence.AttributeOverrides;
 import javax.persistence.Column;
 import javax.persistence.EnumType;
 import javax.persistence.Enumerated;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
 import javax.persistence.MappedSuperclass;
-import javax.persistence.SequenceGenerator;
 import javax.persistence.Transient;
 import org.joda.time.DateTime;
 
@@ -49,7 +60,9 @@ import org.joda.time.DateTime;
 @ReportedOn
 @Entity
 @MappedSuperclass
-public class HistoryEntry extends ImmutableObject implements Buildable {
+@WithStringVKey // TODO(b/162229294): This should be resolved during the course of that bug
+@Access(AccessType.FIELD)
+public class HistoryEntry extends ImmutableObject implements Buildable, DatastoreEntity, SqlEntity {
 
   /** Represents the type of history entry. */
   public enum Type {
@@ -100,16 +113,13 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
     SYNTHETIC
   }
 
-  /** The autogenerated id of this event. */
-  @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "HistorySequenceGenerator")
-  @SequenceGenerator(
-      name = "HistorySequenceGenerator",
-      sequenceName = "history_id_sequence",
-      allocationSize = 1)
-  @Id
-  @javax.persistence.Id
-  @Column(name = "historyRevisionId")
-  Long id;
+  /**
+   * The autogenerated id of this event. Note that, this field is marked as {@link Transient} in the
+   * SQL schema, this is because the child class of {@link HistoryEntry}, e.g. {@link
+   * DomainHistory}, uses a composite primary key which the id is part of, and Hibernate requires
+   * that all the {@link javax.persistence.Id} fields must be put in the exact same class.
+   */
+  @Id @Transient @VisibleForTesting public Long id;
 
   /** The resource this event mutated. */
   @Parent @Transient protected Key<? extends EppResource> parent;
@@ -185,8 +195,17 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
   @Transient // domain-specific
   Set<DomainTransactionRecord> domainTransactionRecords;
 
-  public Long getId() {
-    return id;
+  public long getId() {
+    // For some reason, Hibernate throws NPE during some initialization phase if we don't deal with
+    // the null case. Setting the id to 0L when it is null should be fine because 0L for primitive
+    // type is considered as null for wrapper class in the Hibernate context.
+    return id == null ? 0L : id;
+  }
+
+  // This method is required by Hibernate.
+  @SuppressWarnings("UnusedMethod")
+  private void setId(long id) {
+    this.id = id;
   }
 
   public Key<? extends EppResource> getParent() {
@@ -257,6 +276,52 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
     return new Builder(clone(this));
   }
 
+  public HistoryEntry asHistoryEntry() {
+    return new Builder().copyFrom(this).build();
+  }
+
+  @SuppressWarnings("unchecked")
+  public HistoryEntry toChildHistoryEntity() {
+    String parentKind = getParent().getKind();
+    final HistoryEntry resultEntity;
+    // can't use a switch statement since we're calling getKind()
+    if (parentKind.equals(getKind(DomainBase.class))) {
+      resultEntity =
+          new DomainHistory.Builder().copyFrom(this).setDomainRepoId(parent.getName()).build();
+    } else if (parentKind.equals(getKind(HostResource.class))) {
+      resultEntity =
+          new HostHistory.Builder()
+              .copyFrom(this)
+              .setHostRepoId(
+                  VKey.create(HostResource.class, parent.getName(), (Key<HostResource>) parent))
+              .build();
+    } else if (parentKind.equals(getKind(ContactResource.class))) {
+      resultEntity =
+          new ContactHistory.Builder()
+              .copyFrom(this)
+              .setContactRepoId(
+                  VKey.create(
+                      ContactResource.class, parent.getName(), (Key<ContactResource>) parent))
+              .build();
+    } else {
+      throw new IllegalStateException(
+          String.format("Unknown kind of HistoryEntry parent %s", parentKind));
+    }
+    return resultEntity;
+  }
+
+  // In SQL, save the child type
+  @Override
+  public ImmutableList<SqlEntity> toSqlEntities() {
+    return ImmutableList.of(toChildHistoryEntity());
+  }
+
+  // In Datastore, save as a HistoryEntry object regardless of this object's type
+  @Override
+  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
+    return ImmutableList.of(asHistoryEntry());
+  }
+
   /** A builder for {@link HistoryEntry} since it is immutable */
   public static class Builder<T extends HistoryEntry, B extends Builder<?, ?>>
       extends GenericBuilder<T, B> {
@@ -266,11 +331,35 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
       super(instance);
     }
 
+    // Used to fill out the fields in this object from an object which may not be exactly the same
+    // as the class T, where both classes still subclass HistoryEntry
+    public B copyFrom(HistoryEntry historyEntry) {
+      setId(historyEntry.id);
+      setParent(historyEntry.parent);
+      setType(historyEntry.type);
+      setPeriod(historyEntry.period);
+      setXmlBytes(historyEntry.xmlBytes);
+      setModificationTime(historyEntry.modificationTime);
+      setClientId(historyEntry.clientId);
+      setOtherClientId(historyEntry.otherClientId);
+      setTrid(historyEntry.trid);
+      setBySuperuser(historyEntry.bySuperuser);
+      setReason(historyEntry.reason);
+      setRequestedByRegistrar(historyEntry.requestedByRegistrar);
+      setDomainTransactionRecords(nullToEmptyImmutableCopy(historyEntry.domainTransactionRecords));
+      return thisCastToDerived();
+    }
+
     @Override
     public T build() {
       return super.build();
     }
 
+    public B setId(long id) {
+      getInstance().id = id;
+      return thisCastToDerived();
+    }
+
     public B setParent(EppResource parent) {
       getInstance().parent = Key.create(parent);
       return thisCastToDerived();

core/src/main/java/google/registry/model/tmch/ClaimsListDao.java

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package google.registry.schema.tmch;
+package google.registry.model.tmch;
 
 import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
 import static google.registry.model.CacheUtils.tryMemoizeWithExpiration;
@@ -24,28 +24,28 @@ import google.registry.util.NonFinalForTesting;
 import java.util.Optional;
 import javax.persistence.EntityManager;
 
-/** Data access object for {@link ClaimsList}. */
+/** Data access object for {@link ClaimsListShard}. */
 public class ClaimsListDao {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   /** In-memory cache for claims list. */
   @NonFinalForTesting
-  private static Supplier<Optional<ClaimsList>> cacheClaimsList =
+  private static Supplier<Optional<ClaimsListShard>> cacheClaimsList =
       tryMemoizeWithExpiration(getDomainLabelListCacheDuration(), ClaimsListDao::getLatestRevision);
 
-  private static void save(ClaimsList claimsList) {
+  private static void save(ClaimsListShard claimsList) {
     jpaTm().transact(() -> jpaTm().getEntityManager().persist(claimsList));
   }
 
   /**
-   * Try to save the given {@link ClaimsList} into Cloud SQL. If the save fails, the error will be
-   * logged but no exception will be thrown.
+   * Try to save the given {@link ClaimsListShard} into Cloud SQL. If the save fails, the error will
+   * be logged but no exception will be thrown.
    *
    * <p>This method is used during the dual-write phase of database migration as Datastore is still
    * the authoritative database.
    */
-  public static void trySave(ClaimsList claimsList) {
+  static void trySave(ClaimsListShard claimsList) {
     try {
       ClaimsListDao.save(claimsList);
       logger.atInfo().log(
@@ -57,12 +57,12 @@ public class ClaimsListDao {
   }
 
   /**
-   * Returns the most recent revision of the {@link ClaimsList} in Cloud SQL, if it exists.
+   * Returns the most recent revision of the {@link ClaimsListShard} in Cloud SQL, if it exists.
    * TODO(shicong): Change this method to package level access after dual-read phase.
    * ClaimsListShard uses this method to retrieve claims list in Cloud SQL for the comparison, and
    * ClaimsListShard is not in this package.
    */
-  public static Optional<ClaimsList> getLatestRevision() {
+  public static Optional<ClaimsListShard> getLatestRevision() {
     return jpaTm()
         .transact(
             () -> {
@@ -73,15 +73,15 @@ public class ClaimsListDao {
               return em.createQuery(
                       "FROM ClaimsList cl LEFT JOIN FETCH cl.labelsToKeys WHERE cl.revisionId ="
                           + " :revisionId",
-                      ClaimsList.class)
+                      ClaimsListShard.class)
                   .setParameter("revisionId", revisionId)
                   .getResultStream()
                   .findFirst();
             });
   }
 
-  /** Returns the most recent revision of the {@link ClaimsList}, from cache. */
-  public static Optional<ClaimsList> getLatestRevisionCached() {
+  /** Returns the most recent revision of the {@link ClaimsListShard}, from cache. */
+  public static Optional<ClaimsListShard> getLatestRevisionCached() {
     return cacheClaimsList.get();
   }
 

core/src/main/java/google/registry/model/tmch/ClaimsListShard.java

@@ -40,6 +40,7 @@ import com.googlecode.objectify.annotation.Id;
 import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.OnSave;
 import com.googlecode.objectify.annotation.Parent;
+import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
@@ -47,8 +48,6 @@ import google.registry.model.annotations.VirtualEntity;
 import google.registry.model.common.CrossTldSingleton;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import google.registry.util.CollectionUtils;
 import google.registry.util.Concurrent;
 import google.registry.util.Retrier;
@@ -59,6 +58,15 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.Callable;
 import javax.annotation.Nullable;
+import javax.persistence.CollectionTable;
+import javax.persistence.Column;
+import javax.persistence.ElementCollection;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.JoinColumn;
+import javax.persistence.MapKeyColumn;
+import javax.persistence.Table;
+import javax.persistence.Transient;
 import org.joda.time.DateTime;
 
 /**
@@ -74,10 +82,21 @@ import org.joda.time.DateTime;
  * 10MB per transaction limit.
  *
  * <p>Therefore, it is never OK to save an instance of this class directly to Datastore. Instead you
- * must use the {@link #save} method to do it for you.
+ * must use the {@link #saveToDatastore} method to do it for you.
+ *
+ * <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 claims list with only different {@link
+ * #revisionId}. However, this is not an actual problem because we only use the claims list with
+ * highest {@link #revisionId}.
+ *
+ * <p>TODO(b/162007765): Rename the class to ClaimsList and remove Datastore related fields and
+ * methods.
  */
 @Entity
 @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
+@javax.persistence.Entity(name = "ClaimsList")
+@Table
 public class ClaimsListShard extends ImmutableObject implements DatastoreEntity {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -85,22 +104,44 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
   /** The number of claims list entries to store per shard. */
   private static final int SHARD_SIZE = 10000;
 
-  @Id
-  long id;
+  @Transient @Id long id;
 
-  @Parent
-  Key<ClaimsListRevision> parent;
+  @Transient @Parent Key<ClaimsListRevision> parent;
 
-  /** When the claims list was last updated. */
+  @Ignore
+  @javax.persistence.Id
+  @GeneratedValue(strategy = GenerationType.IDENTITY)
+  Long revisionId;
+
+  @Ignore
+  @Column(nullable = false)
+  CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
+
+  /**
+   * When the claims list was last updated.
+   *
+   * <p>Note that the value of this field is parsed from the claims list file(See this <a
+   * href="https://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.1">RFC</>), it is
+   * the DNL List creation datetime from the rfc. Since this field has been used by Datastore, we
+   * cannot change its name until we finish the migration.
+   *
+   * <p>TODO(b/166784536): Rename this field to tmdbGenerationTime.
+   */
+  @Column(name = "tmdb_generation_time", nullable = false)
   DateTime creationTime;
 
   /** A map from labels to claims keys. */
   @EmbedMap
+  @ElementCollection
+  @CollectionTable(
+      name = "ClaimsEntry",
+      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
+  @MapKeyColumn(name = "domainLabel", nullable = false)
+  @Column(name = "claimKey", nullable = false)
   Map<String, String> labelsToKeys;
 
   /** Indicates that this is a shard rather than a "full" list. */
-  @Ignore
-  boolean isShard = false;
+  @Ignore @Transient boolean isShard = false;
 
   private static final Retrier LOADER_RETRIER = new Retrier(new SystemSleeper(), 2);
 
@@ -164,10 +205,10 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
         return datastoreList;
       };
 
-  private static final void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) {
-    Optional<ClaimsList> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
+  private static void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) {
+    Optional<ClaimsListShard> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
     if (maybeCloudSqlList.isPresent()) {
-      ClaimsList cloudSqlList = maybeCloudSqlList.get();
+      ClaimsListShard cloudSqlList = maybeCloudSqlList.get();
       MapDifference<String, String> diff =
           Maps.difference(datastoreList.labelsToKeys, cloudSqlList.getLabelsToKeys());
       if (!diff.areEqual()) {
@@ -206,15 +247,34 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
       memoizeWithShortExpiration(
           () -> LOADER_RETRIER.callWithRetry(LOADER_CALLABLE, IllegalStateException.class));
 
-  public DateTime getCreationTime() {
+  /** Returns the revision id of this claims list, or throws exception if it is null. */
+  public Long getRevisionId() {
+    checkState(
+        revisionId != null, "revisionId is null because it is not persisted in the database");
+    return revisionId;
+  }
+
+  /**
+   * Returns the time when the external TMDB service generated this revision of the claims list.
+   *
+   * @see <a href="https://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.1">DNL List
+   *     creation datetime</a>
+   */
+  public DateTime getTmdbGenerationTime() {
     return creationTime;
   }
 
+  /** Returns the creation time of this claims list. */
+  public DateTime getCreationTimestamp() {
+    return creationTimestamp.getTimestamp();
+  }
+
   /** Returns the claim key for a given domain if there is one, empty otherwise. */
   public Optional<String> getClaimKey(String label) {
     return Optional.ofNullable(labelsToKeys.get(label));
   }
 
+  /** Returns an {@link Map} mapping domain label to its lookup key. */
   public ImmutableMap<String, String> getLabelsToKeys() {
     return ImmutableMap.copyOf(labelsToKeys);
   }
@@ -229,11 +289,12 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
    * switching over to using them atomically, then deleting the old ones.
    */
   public void save() {
-    save(SHARD_SIZE);
+    saveToDatastore(SHARD_SIZE);
+    ClaimsListDao.trySave(this);
   }
 
   @VisibleForTesting
-  void save(int shardSize) {
+  void saveToDatastore(int shardSize) {
     // Figure out what the next versionId should be based on which ones already exist.
     final Key<ClaimsListRevision> oldRevision = getCurrentRevision();
     final Key<ClaimsListRevision> parentKey = ClaimsListRevision.createKey();
@@ -270,10 +331,11 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
             });
   }
 
-  public static ClaimsListShard create(DateTime creationTime, Map<String, String> labelsToKeys) {
+  public static ClaimsListShard create(
+      DateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
     ClaimsListShard instance = new ClaimsListShard();
     instance.id = allocateId();
-    instance.creationTime = checkNotNull(creationTime);
+    instance.creationTime = checkNotNull(tmdbGenerationTime);
     instance.labelsToKeys = checkNotNull(labelsToKeys);
     return instance;
   }

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

@@ -59,6 +59,9 @@ public class HibernateSchemaExporter {
     settings.put(Environment.USER, username);
     settings.put(Environment.PASS, password);
     settings.put(Environment.HBM2DDL_AUTO, "none");
+    // Register driver explicitly to work around ServiceLoader change after Java 8.
+    // Driver self-registration only works if driver is declared in a module.
+    settings.put(Environment.DRIVER, "org.postgresql.Driver");
     settings.put(Environment.SHOW_SQL, "true");
     settings.put(
         Environment.PHYSICAL_NAMING_STRATEGY, NomulusNamingStrategy.class.getCanonicalName());

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

@@ -0,0 +1,45 @@
+// 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.persistence;
+
+import com.google.common.collect.ImmutableSet;
+import org.hibernate.boot.archive.internal.StandardArchiveDescriptorFactory;
+import org.hibernate.boot.archive.scan.internal.ScanResultImpl;
+import org.hibernate.boot.archive.scan.internal.StandardScanner;
+import org.hibernate.boot.archive.scan.spi.ScanEnvironment;
+import org.hibernate.boot.archive.scan.spi.ScanOptions;
+import org.hibernate.boot.archive.scan.spi.ScanParameters;
+import org.hibernate.boot.archive.scan.spi.ScanResult;
+import org.hibernate.boot.archive.scan.spi.Scanner;
+
+/**
+ * A do-nothing {@link Scanner} for Hibernate that works around bugs in Hibernate's default
+ * implementation. This is required for the Nomulus tool.
+ *
+ * <p>Please refer to <a href="../../../../resources/META-INF/persistence.xml">persistence.xml</a>
+ * for more information.
+ */
+public class NoopJpaEntityScanner extends StandardScanner {
+
+  public NoopJpaEntityScanner() {
+    super(StandardArchiveDescriptorFactory.INSTANCE);
+  }
+
+  @Override
+  public ScanResult scan(
+      ScanEnvironment environment, ScanOptions options, ScanParameters parameters) {
+    return new ScanResultImpl(ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of());
+  }
+}

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

@@ -14,10 +14,12 @@
 
 package google.registry.persistence;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.googlecode.objectify.Key;
+import google.registry.model.BackupGroupRoot;
 import google.registry.model.ImmutableObject;
 import google.registry.model.translators.VKeyTranslatorFactory;
 import java.io.Serializable;
@@ -75,11 +77,42 @@ public class VKey<T> extends ImmutableObject implements Serializable {
     return new VKey<T>(kind, ofyKey, sqlKey);
   }
 
-  /** Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code id}. */
+  /**
+   * Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code id}.
+   *
+   * <p>IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those
+   * that do not have a null parent in their Objectify keys) require the full entity group
+   * inheritance chain to be specified and thus cannot use this create method. You need to use
+   * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the
+   * Datastore key.
+   */
   public static <T> VKey<T> create(Class<T> kind, long id) {
+    checkArgument(
+        kind.isAssignableFrom(BackupGroupRoot.class),
+        "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain"
+            + " specified in a parent",
+        kind.getCanonicalName());
     return new VKey<T>(kind, Key.create(kind, id), id);
   }
 
+  /**
+   * Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code name}.
+   *
+   * <p>IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those
+   * that do not have a null parent in their Objectify keys) require the full entity group
+   * inheritance chain to be specified and thus cannot use this create method. You need to use
+   * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the
+   * Datastore key.
+   */
+  public static <T> VKey<T> create(Class<T> kind, String name) {
+    checkArgument(
+        kind.isAssignableFrom(BackupGroupRoot.class),
+        "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain"
+            + " specified in a parent",
+        kind.getCanonicalName());
+    return new VKey<T>(kind, Key.create(kind, name), name);
+  }
+
   /** Returns the type of the entity. */
   public Class<? extends T> getKind() {
     return this.kind;

core/src/main/java/google/registry/persistence/converter/AllocationTokenStatusTransitionConverter.java

@@ -0,0 +1,49 @@
+// 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.persistence.converter;
+
+import com.google.common.collect.Maps;
+import google.registry.model.common.TimedTransitionProperty;
+import google.registry.model.domain.token.AllocationToken.TokenStatus;
+import google.registry.model.domain.token.AllocationToken.TokenStatusTransition;
+import java.util.Map;
+import javax.persistence.Converter;
+import org.joda.time.DateTime;
+
+/**
+ * JPA converter for storing/retrieving {@link TimedTransitionProperty} maps referring to {@link
+ * TokenStatus}es.
+ */
+@Converter(autoApply = true)
+public class AllocationTokenStatusTransitionConverter
+    extends TimedTransitionPropertyConverterBase<TokenStatus, TokenStatusTransition> {
+
+  @Override
+  Map.Entry<String, String> convertToDatabaseMapEntry(
+      Map.Entry<DateTime, TokenStatusTransition> entry) {
+    return Maps.immutableEntry(entry.getKey().toString(), entry.getValue().getValue().name());
+  }
+
+  @Override
+  Map.Entry<DateTime, TokenStatus> convertToEntityMapEntry(Map.Entry<String, String> entry) {
+    return Maps.immutableEntry(
+        DateTime.parse(entry.getKey()), TokenStatus.valueOf(entry.getValue()));
+  }
+
+  @Override
+  Class<TokenStatusTransition> getTimedTransitionSubclass() {
+    return TokenStatusTransition.class;
+  }
+}

core/src/main/java/google/registry/persistence/converter/DurationConverter.java

@@ -40,11 +40,17 @@ public class DurationConverter implements AttributeConverter<Duration, PGInterva
     if (duration == null) {
       return new PGInterval();
     }
+    // When the period is created from duration by calling duration.toPeriod(), only precise fields
+    // in the period type will be used. Thus, only the hour, minute, second and millisecond fields
+    // on the period will be used. The year, month, week and day fields will not be populated:
+    //   1. If the duration is small, less than one day, then this method will just set
+    //      hours/minutes/seconds correctly.
+    //   2. If the duration is larger than one day then all the remaining duration will
+    //      be stored in the largest available field, hours in this case.
+    // So, when we convert the period to a PGInterval instance, we set the days field by extracting
+    // it from period's hours field.
+    Period period = duration.toPeriod();
     PGInterval interval = new PGInterval();
-    Period period = new Period(duration);
-    // For some reason when the period is created from the duration, it does not set days, but
-    // instead just a total number of hours. Years and months are not created because those can
-    // differ in length of milliseconds.
     interval.setDays(period.getHours() / 24);
     interval.setHours(period.getHours() % 24);
     interval.setMinutes(period.getMinutes());

core/src/main/java/google/registry/persistence/converter/PremiumListKeyConverter.java

@@ -0,0 +1,37 @@
+// 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.persistence.converter;
+
+import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
+
+import com.googlecode.objectify.Key;
+import google.registry.model.registry.label.PremiumList;
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+/** JPA converter for a {@link Key} containing a {@link PremiumList} */
+@Converter(autoApply = true)
+public class PremiumListKeyConverter implements AttributeConverter<Key<PremiumList>, String> {
+
+  @Override
+  public String convertToDatabaseColumn(Key<PremiumList> attribute) {
+    return (attribute == null) ? null : attribute.getName();
+  }
+
+  @Override
+  public Key<PremiumList> convertToEntityAttribute(String dbData) {
+    return (dbData == null) ? null : Key.create(getCrossTldKey(), PremiumList.class, dbData);
+  }
+}

core/src/main/java/google/registry/persistence/converter/ReservedListKeyConverter.java

@@ -0,0 +1,37 @@
+// 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.persistence.converter;
+
+import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
+
+import com.googlecode.objectify.Key;
+import google.registry.model.registry.label.ReservedList;
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+/** JPA converter for a {@link Key} containing a {@link ReservedList} */
+@Converter(autoApply = true)
+public class ReservedListKeyConverter implements AttributeConverter<Key<ReservedList>, String> {
+
+  @Override
+  public String convertToDatabaseColumn(Key<ReservedList> attribute) {
+    return (attribute == null) ? null : attribute.getName();
+  }
+
+  @Override
+  public Key<ReservedList> convertToEntityAttribute(String dbData) {
+    return (dbData == null) ? null : Key.create(getCrossTldKey(), ReservedList.class, dbData);
+  }
+}

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

@@ -391,7 +391,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
   private static ImmutableSet<EntityId> getEntityIdsFromEntity(
       EntityType<?> entityType, Object entity) {
     if (entityType.hasSingleIdAttribute()) {
-      String idName = entityType.getDeclaredId(entityType.getIdType().getJavaType()).getName();
+      String idName = entityType.getId(entityType.getIdType().getJavaType()).getName();
       Object idValue = getFieldValue(entity, idName);
       return ImmutableSet.of(new EntityId(idName, idValue));
     } else {
@@ -402,7 +402,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
   private static ImmutableSet<EntityId> getEntityIdsFromSqlKey(
       EntityType<?> entityType, Object sqlKey) {
     if (entityType.hasSingleIdAttribute()) {
-      String idName = entityType.getDeclaredId(entityType.getIdType().getJavaType()).getName();
+      String idName = entityType.getId(entityType.getIdType().getJavaType()).getName();
       return ImmutableSet.of(new EntityId(idName, sqlKey));
     } else {
       return getEntityIdsFromIdContainer(entityType, sqlKey);
@@ -429,7 +429,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
 
   private static Object getFieldValue(Object object, String fieldName) {
     try {
-      Field field = object.getClass().getDeclaredField(fieldName);
+      Field field = getField(object.getClass(), fieldName);
       field.setAccessible(true);
       return field.get(object);
     } catch (NoSuchFieldException | IllegalAccessException e) {
@@ -437,6 +437,21 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
     }
   }
 
+  /** Gets the field definition from clazz or any superclass. */
+  private static Field getField(Class clazz, String fieldName) throws NoSuchFieldException {
+    try {
+      // Note that we have to use getDeclaredField() for this, getField() just finds public fields.
+      return clazz.getDeclaredField(fieldName);
+    } catch (NoSuchFieldException e) {
+      Class base = clazz.getSuperclass();
+      if (base != null) {
+        return getField(base, fieldName);
+      } else {
+        throw e;
+      }
+    }
+  }
+
   private static class TransactionInfo {
     EntityManager entityManager;
     boolean inTransaction = false;

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

@@ -14,6 +14,9 @@
 
 package google.registry.persistence.transaction;
 
+import com.google.common.collect.ImmutableList;
+import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.SqlEntity;
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
@@ -27,7 +30,7 @@ import javax.persistence.Table;
  */
 @Entity
 @Table(name = "Transaction")
-public class TransactionEntity {
+public class TransactionEntity implements SqlEntity {
 
   @Id
   @GeneratedValue(strategy = GenerationType.IDENTITY)
@@ -40,4 +43,9 @@ public class TransactionEntity {
   TransactionEntity(byte[] contents) {
     this.contents = contents;
   }
+
+  @Override
+  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
+    return ImmutableList.of(); // not stored in Datastore per se
+  }
 }

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

@@ -24,10 +24,13 @@ import com.google.api.client.http.GenericUrl;
 import com.google.api.client.http.HttpHeaders;
 import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpResponse;
+import com.google.api.client.http.HttpResponseException;
+import com.google.api.client.http.HttpStatusCodes;
 import com.google.api.client.http.HttpTransport;
 import com.google.common.base.Ascii;
 import com.google.common.base.Splitter;
 import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
 import com.google.common.io.ByteStreams;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.keyring.api.KeyModule.Key;
@@ -79,6 +82,7 @@ public class IcannHttpReporter {
     headers.setContentType(CSV_UTF_8.toString());
     request.setHeaders(headers);
     request.setFollowRedirects(false);
+    request.setThrowExceptionOnExecuteError(false);
 
     HttpResponse response = null;
     logger.atInfo().log(
@@ -86,6 +90,12 @@ public class IcannHttpReporter {
     boolean success = true;
     try {
       response = request.execute();
+      // Only responses with a 200 or 400 status have a body. For everything else, throw so that
+      // the caller catches it and prints the stack trace.
+      if (response.getStatusCode() != HttpStatusCodes.STATUS_CODE_OK
+          && response.getStatusCode() != HttpStatusCodes.STATUS_CODE_BAD_REQUEST) {
+        throw new HttpResponseException(response);
+      }
       byte[] content;
       try {
         content = ByteStreams.toByteArray(response.getContent());
@@ -93,11 +103,24 @@ public class IcannHttpReporter {
         response.getContent().close();
       }
       logger.atInfo().log(
-          "Received response code %d with content %s",
-          response.getStatusCode(), new String(content, UTF_8));
-      XjcIirdeaResult result = parseResult(content);
-      if (result.getCode().getValue() != 1000) {
+          "Received response code %d\n\n"
+              + "Response headers: %s\n\n"
+              + "Response content in UTF-8: %s\n\n"
+              + "Response content in HEX: %s",
+          response.getStatusCode(),
+          response.getHeaders(),
+          new String(content, UTF_8),
+          BaseEncoding.base16().encode(content));
+      // For reasons unclear at the moment, when we parse the response content using UTF-8 we get
+      // garbled texts. Since we know that an HTTP 200 response can only contain a result code of
+      // 1000 (i. e. success), there is no need to parse it.
+      if (response.getStatusCode() == HttpStatusCodes.STATUS_CODE_BAD_REQUEST) {
         success = false;
+        // To debug if there is a problem with our parsing, we wrap the response and print the stack
+        // trace of it. As far as we can tell, the stack trace for such an exception contains the
+        // response content that is decoded correctly using the expected charset.
+        new HttpResponseException(response).printStackTrace();
+        XjcIirdeaResult result = parseResult(content);
         logger.atWarning().log(
             "PUT rejected, status code %s:\n%s\n%s",
             result.getCode(), result.getMsg(), result.getDescription());

core/src/main/java/google/registry/schema/tmch/ClaimsList.java

@@ -1,116 +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.tmch;
-
-import static com.google.common.base.Preconditions.checkState;
-import static google.registry.util.DateTimeUtils.toJodaDateTime;
-import static google.registry.util.DateTimeUtils.toZonedDateTime;
-
-import com.google.common.collect.ImmutableList;
-import google.registry.model.CreateAutoTimestamp;
-import google.registry.model.ImmutableObject;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
-import java.time.ZonedDateTime;
-import java.util.Map;
-import java.util.Optional;
-import javax.persistence.CollectionTable;
-import javax.persistence.Column;
-import javax.persistence.ElementCollection;
-import javax.persistence.Entity;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
-import javax.persistence.Id;
-import javax.persistence.JoinColumn;
-import javax.persistence.MapKeyColumn;
-import javax.persistence.Table;
-import org.joda.time.DateTime;
-
-/**
- * A list of TMCH claims labels and their associated claims keys.
- *
- * <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 claims list with only different {@link
- * #revisionId}. However, this is not an actual problem because we only use the claims list with
- * highest {@link #revisionId}.
- */
-@Entity
-@Table
-public class ClaimsList extends ImmutableObject implements SqlEntity {
-  @Id
-  @GeneratedValue(strategy = GenerationType.IDENTITY)
-  @Column
-  private Long revisionId;
-
-  @Column(nullable = false)
-  private CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
-
-  @Column(nullable = false)
-  private ZonedDateTime tmdbGenerationTime;
-
-  @ElementCollection
-  @CollectionTable(
-      name = "ClaimsEntry",
-      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
-  @MapKeyColumn(name = "domainLabel", nullable = false)
-  @Column(name = "claimKey", nullable = false)
-  private Map<String, String> labelsToKeys;
-
-  private ClaimsList(ZonedDateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
-    this.tmdbGenerationTime = tmdbGenerationTime;
-    this.labelsToKeys = labelsToKeys;
-  }
-
-  // Hibernate requires this default constructor.
-  private ClaimsList() {}
-
-  /** Constructs a {@link ClaimsList} object. */
-  public static ClaimsList create(DateTime creationTimestamp, Map<String, String> labelsToKeys) {
-    return new ClaimsList(toZonedDateTime(creationTimestamp), labelsToKeys);
-  }
-
-  /** Returns the revision id of this claims list, or throws exception if it is null. */
-  public Long getRevisionId() {
-    checkState(
-        revisionId != null, "revisionId is null because it is not persisted in the database");
-    return revisionId;
-  }
-
-  /** Returns the TMDB generation time of this claims list. */
-  public DateTime getTmdbGenerationTime() {
-    return toJodaDateTime(tmdbGenerationTime);
-  }
-
-  /** Returns the creation time of this claims list. */
-  public DateTime getCreationTimestamp() {
-    return creationTimestamp.getTimestamp();
-  }
-
-  /** Returns an {@link Map} mapping domain label to its lookup key. */
-  public Map<String, String> getLabelsToKeys() {
-    return labelsToKeys;
-  }
-
-  /** Returns the claim key for a given domain if there is one, empty otherwise. */
-  public Optional<String> getClaimKey(String label) {
-    return Optional.ofNullable(labelsToKeys.get(label));
-  }
-
-  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // ClaimsList is dual-written
-  }
-}

core/src/main/java/google/registry/tmch/ClaimsListParser.java

@@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
-import google.registry.schema.tmch.ClaimsList;
+import google.registry.model.tmch.ClaimsListShard;
 import java.util.List;
 import org.joda.time.DateTime;
 
@@ -34,11 +34,11 @@ import org.joda.time.DateTime;
 public class ClaimsListParser {
 
   /**
-   * Converts the lines from the DNL CSV file into a {@link ClaimsList} object.
+   * Converts the lines from the DNL CSV file into a {@link ClaimsListShard} object.
    *
    * <p>Please note that this does <b>not</b> insert the object into Datastore.
    */
-  public static ClaimsList parse(List<String> lines) {
+  public static ClaimsListShard parse(List<String> lines) {
     ImmutableMap.Builder<String, String> builder = new ImmutableMap.Builder<>();
 
     // First line: <version>,<DNL List creation datetime>
@@ -74,6 +74,6 @@ public class ClaimsListParser {
       builder.put(label, lookupKey);
     }
 
-    return ClaimsList.create(creationTime, builder.build());
+    return ClaimsListShard.create(creationTime, builder.build());
   }
 }

core/src/main/java/google/registry/tmch/TmchDnlAction.java

@@ -21,8 +21,6 @@ import google.registry.keyring.api.KeyModule.Key;
 import google.registry.model.tmch.ClaimsListShard;
 import google.registry.request.Action;
 import google.registry.request.auth.Auth;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import java.io.IOException;
 import java.security.SignatureException;
 import java.util.List;
@@ -56,14 +54,10 @@ public final class TmchDnlAction implements Runnable {
     } catch (SignatureException | IOException | PGPException e) {
       throw new RuntimeException(e);
     }
-    ClaimsList claims = ClaimsListParser.parse(lines);
-    ClaimsListShard claimsListShard =
-        ClaimsListShard.create(claims.getTmdbGenerationTime(), claims.getLabelsToKeys());
-    claimsListShard.save();
+    ClaimsListShard claims = ClaimsListParser.parse(lines);
+    claims.save();
     logger.atInfo().log(
         "Inserted %,d claims into Datastore, created at %s",
-        claimsListShard.size(), claimsListShard.getCreationTime());
-
-    ClaimsListDao.trySave(claims);
+        claims.size(), claims.getTmdbGenerationTime());
   }
 }

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

@@ -118,10 +118,10 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand {
 
   @Nullable
   @Parameter(
-      names = "--disable_invoicing",
-      description = "Whether invoicing is disabled for a REAL tld.",
+      names = "--invoicing_enabled",
+      description = "Whether invoicing is enabled for this tld.",
       arity = 1)
-  private Boolean disableInvoicing;
+  private Boolean invoicingEnabled;
 
   @Nullable
   @Parameter(
@@ -327,7 +327,7 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand {
       Optional.ofNullable(serverStatusChangeCost)
           .ifPresent(builder::setServerStatusChangeBillingCost);
       Optional.ofNullable(tldType).ifPresent(builder::setTldType);
-      Optional.ofNullable(disableInvoicing).ifPresent(builder::setDisableInvoicing);
+      Optional.ofNullable(invoicingEnabled).ifPresent(builder::setInvoicingEnabled);
       Optional.ofNullable(lordnUsername).ifPresent(u -> builder.setLordnUsername(u.orElse(null)));
       Optional.ofNullable(claimsPeriodEnd).ifPresent(builder::setClaimsPeriodEnd);
       Optional.ofNullable(numDnsPublishShards).ifPresent(builder::setNumDnsPublishLocks);

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

@@ -182,7 +182,8 @@ class GenerateAllocationTokensCommand implements CommandWithRemoteApi {
                         new AllocationToken.Builder()
                             .setToken(t)
                             .setTokenType(tokenType == null ? SINGLE_USE : tokenType)
-                            .setAllowedClientIds(ImmutableSet.copyOf(nullToEmpty(allowedClientIds)))
+                            .setAllowedRegistrarIds(
+                                ImmutableSet.copyOf(nullToEmpty(allowedClientIds)))
                             .setAllowedTlds(ImmutableSet.copyOf(nullToEmpty(allowedTlds)));
                     Optional.ofNullable(discountFraction).ifPresent(token::setDiscountFraction);
                     Optional.ofNullable(discountPremiums).ifPresent(token::setDiscountPremiums);

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

@@ -16,6 +16,7 @@ package google.registry.tools;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -59,7 +60,7 @@ final class GetAllocationTokenCommand implements CommandWithRemoteApi {
           System.out.printf("Token %s was not redeemed.\n", token);
         } else {
           DomainBase domain =
-              domains.get(loadedToken.getRedemptionHistoryEntry().get().<DomainBase>getParent());
+              domains.get(loadedToken.getRedemptionHistoryEntry().get().getOfyKey().getParent());
           if (domain == null) {
             System.out.printf("ERROR: Token %s was redeemed but domain can't be loaded.\n", token);
           } else {
@@ -82,7 +83,8 @@ final class GetAllocationTokenCommand implements CommandWithRemoteApi {
             .map(AllocationToken::getRedemptionHistoryEntry)
             .filter(Optional::isPresent)
             .map(Optional::get)
-            .map(Key::<DomainBase>getParent)
+            .map(key -> tm().load(key))
+            .map(he -> (Key<DomainBase>) he.getParent())
             .collect(toImmutableList());
     ImmutableMap.Builder<Key<DomainBase>, DomainBase> domainsBuilder = new ImmutableMap.Builder<>();
     for (List<Key<DomainBase>> keys : Lists.partition(domainKeys, BATCH_SIZE)) {

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

@@ -112,8 +112,6 @@ final class RegistryCli implements AutoCloseable, CommandRunner {
     // Create all command instances. It would be preferrable to do this in the constructor, but
     // JCommander mutates the command instances and doesn't reset them so we have to do it for every
     // run.
-    // TODO(weiminyu): extract this into a standalone static method to simplify
-    //   :core:registryToolIntegrationTest
     try {
       for (Map.Entry<String, ? extends Class<? extends Command>> entry : commands.entrySet()) {
         Command command = entry.getValue().getDeclaredConstructor().newInstance();

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

@@ -131,7 +131,7 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
   private AllocationToken updateToken(AllocationToken original) {
     AllocationToken.Builder builder = original.asBuilder();
     Optional.ofNullable(allowedClientIds)
-        .ifPresent(clientIds -> builder.setAllowedClientIds(ImmutableSet.copyOf(clientIds)));
+        .ifPresent(clientIds -> builder.setAllowedRegistrarIds(ImmutableSet.copyOf(clientIds)));
     Optional.ofNullable(allowedTlds)
         .ifPresent(tlds -> builder.setAllowedTlds(ImmutableSet.copyOf(tlds)));
     Optional.ofNullable(discountFraction).ifPresent(builder::setDiscountFraction);

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

@@ -17,10 +17,11 @@ package google.registry.tools;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
 import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
-import static org.joda.time.DateTimeZone.UTC;
+import static java.util.function.Predicate.isEqual;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -31,15 +32,19 @@ import com.google.common.flogger.FluentLogger;
 import com.google.template.soy.data.SoyMapData;
 import google.registry.model.domain.DesignatedContact;
 import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.GracePeriodBase;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.tools.params.NameserversParameter;
 import google.registry.tools.soy.DomainUpdateSoyInfo;
+import google.registry.util.Clock;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
+import javax.annotation.Nullable;
+import javax.inject.Inject;
 import org.joda.time.DateTime;
 
 /** A command to update a new domain via EPP. */
@@ -48,6 +53,8 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
+  @Inject Clock clock;
+
   @Parameter(names = "--statuses", description = "Comma-separated list of statuses to set.")
   private List<String> statuses = new ArrayList<>();
 
@@ -123,6 +130,15 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
   )
   boolean clearDsRecords = false;
 
+  @Nullable
+  @Parameter(
+      names = "--autorenews",
+      arity = 1,
+      description =
+          "Whether the domain autorenews. If false, the domain will automatically be"
+              + " deleted at the end of its current registration period.")
+  Boolean autorenews;
+
   @Override
   protected void initMutatingEppToolCommand() {
     if (!nameservers.isEmpty()) {
@@ -159,7 +175,18 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
       clearDsRecords = true;
     }
 
+    ImmutableSet.Builder<String> autorenewGracePeriodWarningDomains = new ImmutableSet.Builder<>();
+    DateTime now = clock.nowUtc();
     for (String domain : domains) {
+      Optional<DomainBase> domainOptional = loadByForeignKey(DomainBase.class, domain, now);
+      checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domain);
+      DomainBase domainBase = domainOptional.get();
+      checkArgument(
+          !domainBase.getStatusValues().contains(SERVER_UPDATE_PROHIBITED),
+          "The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed "
+              + "to make updates, and if so, use the domain_unlock command to enable updates.",
+          domain);
+
       // Use TreeSets so that the results are always in the same order (this makes testing easier).
       Set<String> addAdminsThisDomain = new TreeSet<>(addAdmins);
       Set<String> removeAdminsThisDomain = new TreeSet<>(removeAdmins);
@@ -171,16 +198,6 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
       Set<String> removeStatusesThisDomain = new TreeSet<>(removeStatuses);
 
       if (!nameservers.isEmpty() || !admins.isEmpty() || !techs.isEmpty() || !statuses.isEmpty()) {
-        DateTime now = DateTime.now(UTC);
-        Optional<DomainBase> domainOptional =
-            loadByForeignKey(DomainBase.class, domain, now);
-        checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domain);
-        DomainBase domainBase = domainOptional.get();
-        checkArgument(
-            !domainBase.getStatusValues().contains(SERVER_UPDATE_PROHIBITED),
-            "The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed "
-                + "to make updates, and if so, use the domain_unlock command to enable updates.",
-            domain);
         if (!nameservers.isEmpty()) {
           ImmutableSortedSet<String> existingNameservers = domainBase.loadNameserverHostNames();
           populateAddRemoveLists(
@@ -232,33 +249,41 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
       }
 
       boolean add =
-          !addNameserversThisDomain.isEmpty()
+          (!addNameserversThisDomain.isEmpty()
               || !addAdminsThisDomain.isEmpty()
               || !addTechsThisDomain.isEmpty()
-              || !addStatusesThisDomain.isEmpty();
+              || !addStatusesThisDomain.isEmpty());
 
       boolean remove =
-          !removeNameserversThisDomain.isEmpty()
+          (!removeNameserversThisDomain.isEmpty()
               || !removeAdminsThisDomain.isEmpty()
               || !removeTechsThisDomain.isEmpty()
-              || !removeStatusesThisDomain.isEmpty();
+              || !removeStatusesThisDomain.isEmpty());
 
-      boolean change = registrant != null || password != null;
-
-      boolean secdns =
-          !addDsRecords.isEmpty()
+      boolean change = (registrant != null || password != null);
+      boolean secDns =
+          (!addDsRecords.isEmpty()
               || !removeDsRecords.isEmpty()
               || !dsRecords.isEmpty()
-              || clearDsRecords;
+              || clearDsRecords);
 
-      if (!add && !remove && !change && !secdns) {
+      if (!add && !remove && !change && !secDns && autorenews == null) {
         logger.atInfo().log("No changes need to be made to domain %s", domain);
         continue;
       }
 
+      // If autorenew is being turned off and this domain is already in the autorenew grace period,
+      // then we want to warn the user that they might want to delete it instead.
+      if (Boolean.FALSE.equals(autorenews)) {
+        if (domainBase.getGracePeriods().stream()
+            .map(GracePeriodBase::getType)
+            .anyMatch(isEqual(AUTO_RENEW))) {
+          autorenewGracePeriodWarningDomains.add(domain);
+        }
+      }
+
       setSoyTemplate(DomainUpdateSoyInfo.getInstance(), DomainUpdateSoyInfo.DOMAINUPDATE);
-      addSoyRecord(
-          clientId,
+      SoyMapData soyMapData =
           new SoyMapData(
               "domain", domain,
               "add", add,
@@ -274,14 +299,27 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
               "change", change,
               "registrant", registrant,
               "password", password,
-              "secdns", secdns,
+              "secdns", secDns,
               "addDsRecords", DsRecord.convertToSoy(addDsRecords),
               "removeDsRecords", DsRecord.convertToSoy(removeDsRecords),
-              "removeAllDsRecords", clearDsRecords));
+              "removeAllDsRecords", clearDsRecords);
+      if (autorenews != null) {
+        soyMapData.put("autorenews", autorenews.toString());
+      }
+      addSoyRecord(clientId, soyMapData);
+    }
+
+    ImmutableSet<String> domainsToWarn = autorenewGracePeriodWarningDomains.build();
+    if (!domainsToWarn.isEmpty()) {
+      logger.atWarning().log(
+          "The following domains are in autorenew grace periods. Consider aborting this command"
+              + " and running `nomulus delete_domain` instead to terminate autorenewal immediately"
+              + " rather than in one year, if desired:\n%s",
+          String.join(", ", domainsToWarn));
     }
   }
 
-  protected void populateAddRemoveLists(
+  private void populateAddRemoveLists(
       Set<String> targetSet, Set<String> oldSet, Set<String> addSet, Set<String> removeSet) {
     addSet.addAll(Sets.difference(targetSet, oldSet));
     removeSet.addAll(Sets.difference(oldSet, targetSet));

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

@@ -22,8 +22,6 @@ import com.beust.jcommander.Parameters;
 import com.google.common.base.Joiner;
 import com.google.common.io.Files;
 import google.registry.model.tmch.ClaimsListShard;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import google.registry.tmch.ClaimsListParser;
 import java.io.File;
 import java.io.IOException;
@@ -39,7 +37,7 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
 
   private String claimsListFilename;
 
-  private ClaimsList claimsList;
+  private ClaimsListShard claimsList;
 
   @Override
   protected void init() throws IOException {
@@ -58,8 +56,7 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
 
   @Override
   public String execute() {
-    ClaimsListShard.create(claimsList.getTmdbGenerationTime(), claimsList.getLabelsToKeys()).save();
-    ClaimsListDao.trySave(claimsList);
+    claimsList.save();
     return String.format("Successfully uploaded claims list %s", claimsListFilename);
   }
 }

core/src/main/java/google/registry/xjc/XjcObject.java

@@ -42,10 +42,6 @@ public abstract class XjcObject {
     XjcXmlTransformer.marshalStrict(this, out, encoding);
   }
 
-  public void marshalLenient(OutputStream out, Charset encoding) throws XmlException {
-    XjcXmlTransformer.marshalLenient(this, out, encoding);
-  }
-
   /**
    * Turns object into a formatted XML string <i>by any means necessary</i>.
    *

core/src/main/java/google/registry/xml/xsd/superuser.xsd

@@ -47,4 +47,12 @@
     </all>
   </complexType>
 
+  <element name="domainUpdate" type="superuser:domainUpdateType" />
+
+  <complexType name="domainUpdateType">
+    <all>
+      <element name="autorenews" minOccurs="0" type="boolean" />
+    </all>
+  </complexType>
+
 </schema>

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

@@ -56,7 +56,8 @@ registry.registrar.RegistryLock.prototype.runAfterRender = function(objArgs) {
   } else {
     goog.soy.renderElement(
         goog.dom.getRequiredElement('locks-content'),
-        registry.soy.registrar.registrylock.lockNotAllowedOnRegistrar);
+        registry.soy.registrar.registrylock.lockNotAllowedOnRegistrar,
+        {supportEmail: objArgs.supportEmail});
   }
 };
 

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

@@ -10,6 +10,12 @@
       <basic name="amount" access="FIELD"/>
     </attributes>
   </embeddable>
+
+  <sequence-generator name="HistorySequenceGenerator" sequence-name="history_id_sequence"/>
+
+  <!-- TODO(shicong): Drop this sequence and change all history tables to use the above one. -->
+  <sequence-generator name="TempHistorySequenceGenerator" sequence-name="temp_history_id_sequence"/>
+
   <persistence-unit-metadata>
     <persistence-unit-defaults>
       <entity-listeners>

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

@@ -11,14 +11,33 @@
     <provider>org.hibernate.jpa.HibernatePersistenceProvider</provider>
 
     <!--
-      All JPA entities must be enumerated here. JPA does not support auto detection.
+      All JPA entity-mapping files and annotated classes must be enumerated
+      here. Automatic entity detection is not part of the JPA spec. Explicit
+      declaration makes it easier to migrate to another provider.
 
-      Note that Hibernate's auto detection functionality (hibernate.archive.autodection)
-      does not meet our needs. It only scans archives, not the 'classes' folders. So we
-      are left with two options:
-      *  Move tests to another (sub)project. This is not a big problem, but feels unnatural.
-      *  Use Hibernate's ServiceRegistry for bootstrapping (not JPA-compliant)
+      Although Hibernate provides the auto detection functionality (configured by
+      the hibernate.archive.autodetection property), it relies on a fragile
+      scanner that can be broken by certain classes. For example, in the uber jar
+      for the Nomulus tool, a repackaged Guava class ( {@code
+      com.google.appengine.repackaged.com.google.common.html.LinkDetector})
+      from appengine-api-1.0-sdk:1.9.81 can break the scanner in
+      hibernate-core:5.4.17.Final. The large number of third-party classes also
+      makes JPA setup noticeably slower in the tool.
+
+      When auto detection is enabled in Hibernate, we also need a separate
+      persistence.xml for tests. See <a
+      href="https://stackoverflow.com/questions/61127082/hibernate-doesnt-find-entities-in-test">
+      this webpage</a> for an example.
+
+      Because of the reasons above, we disable auto detection in Hibernate.
+
+      When auto detection is disabled, Hibernate still invokes the scanner which always
+      goes over the archive that has this file. We need to override the default scanner
+      with an NOOP one for Nomulus tool.
     -->
+
+    <mapping-file>META-INF/orm.xml</mapping-file>
+
     <class>google.registry.model.billing.BillingEvent$Cancellation</class>
     <class>google.registry.model.billing.BillingEvent$OneTime</class>
     <class>google.registry.model.billing.BillingEvent$Recurring</class>
@@ -26,6 +45,7 @@
     <class>google.registry.model.contact.ContactResource</class>
     <class>google.registry.model.domain.DomainBase</class>
     <class>google.registry.model.domain.DomainHistory</class>
+    <class>google.registry.model.domain.token.AllocationToken</class>
     <class>google.registry.model.host.HostHistory</class>
     <class>google.registry.model.host.HostResource</class>
     <class>google.registry.model.registrar.Registrar</class>
@@ -33,8 +53,8 @@
     <class>google.registry.model.registry.label.PremiumList</class>
     <class>google.registry.model.reporting.Spec11ThreatMatch</class>
     <class>google.registry.persistence.transaction.TransactionEntity</class>
+    <class>google.registry.model.tmch.ClaimsListShard</class>
     <class>google.registry.schema.domain.RegistryLock</class>
-    <class>google.registry.schema.tmch.ClaimsList</class>
     <class>google.registry.schema.cursor.Cursor</class>
     <class>google.registry.schema.server.Lock</class>
     <class>google.registry.schema.tld.PremiumEntry</class>
@@ -46,6 +66,7 @@
     <class>google.registry.model.registry.label.ReservedList</class>
 
     <!-- Customized type converters -->
+    <class>google.registry.persistence.converter.AllocationTokenStatusTransitionConverter</class>
     <class>google.registry.persistence.converter.BillingCostTransitionConverter</class>
     <class>google.registry.persistence.converter.BillingEventFlagSetConverter</class>
     <class>google.registry.persistence.converter.BloomFilterConverter</class>
@@ -58,7 +79,9 @@
     <class>google.registry.persistence.converter.InetAddressSetConverter</class>
     <class>google.registry.persistence.converter.LocalDateConverter</class>
     <class>google.registry.persistence.converter.PostalInfoChoiceListConverter</class>
+    <class>google.registry.persistence.converter.PremiumListKeyConverter</class>
     <class>google.registry.persistence.converter.RegistrarPocSetConverter</class>
+    <class>google.registry.persistence.converter.ReservedListKeyConverter</class>
     <class>google.registry.persistence.converter.Spec11ThreatMatchThreatTypeSetConverter</class>
     <class>google.registry.persistence.converter.StatusValueSetConverter</class>
     <class>google.registry.persistence.converter.StringListConverter</class>
@@ -76,8 +99,16 @@
     <class>google.registry.model.host.VKeyConverter_HostResource</class>
     <class>google.registry.model.poll.VKeyConverter_Autorenew</class>
     <class>google.registry.model.poll.VKeyConverter_OneTime</class>
+    <class>google.registry.model.reporting.VKeyConverter_HistoryEntry</class>
 
     <!-- TODO(weiminyu): check out application-layer validation. -->
     <validation-mode>NONE</validation-mode>
+    <properties>
+      <!-- Disables auto detection. -->
+      <property name="hibernate.archive.autodetection" value=""/>
+      <!-- NOOP scanner needed for Nomulus tool. -->
+      <property name="hibernate.archive.scanner"
+          value="google.registry.persistence.NoopJpaEntityScanner"/>
+    </properties>
   </persistence-unit>
 </persistence>

core/src/main/resources/google/registry/tools/soy/DomainUpdate.soy

@@ -35,6 +35,7 @@
   {@param addDsRecords: list<[keyTag:int, alg:int, digestType:int, digest:string]>}
   {@param removeDsRecords: list<[keyTag:int, alg:int, digestType:int, digest:string]>}
   {@param removeAllDsRecords: bool}
+  {@param? autorenews: string}
 
   <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
   <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
@@ -96,39 +97,46 @@
           {/if}
         </domain:update>
       </update>
-      {if $secdns}
+      {if $secdns or $autorenews}
         <extension>
-          <secDNS:update xmlns:secDNS="urn:ietf:params:xml:ns:secDNS-1.1">
-            {if $removeAllDsRecords}
-              <secDNS:rem>
-                <secDNS:all>true</secDNS:all>
-              </secDNS:rem>
-            {/if}
-            {if length($removeDsRecords) > 0}
-              <secDNS:rem>
-                {for $dsRecord in $removeDsRecords}
-                  <secDNS:dsData>
-                    <secDNS:keyTag>{$dsRecord.keyTag}</secDNS:keyTag>
-                    <secDNS:alg>{$dsRecord.alg}</secDNS:alg>
-                    <secDNS:digestType>{$dsRecord.digestType}</secDNS:digestType>
-                    <secDNS:digest>{$dsRecord.digest}</secDNS:digest>
-                  </secDNS:dsData>
-                {/for}
-              </secDNS:rem>
-            {/if}
-            {if length($addDsRecords) > 0}
-              <secDNS:add>
-                {for $dsRecord in $addDsRecords}
-                  <secDNS:dsData>
-                    <secDNS:keyTag>{$dsRecord.keyTag}</secDNS:keyTag>
-                    <secDNS:alg>{$dsRecord.alg}</secDNS:alg>
-                    <secDNS:digestType>{$dsRecord.digestType}</secDNS:digestType>
-                    <secDNS:digest>{$dsRecord.digest}</secDNS:digest>
-                  </secDNS:dsData>
-                {/for}
-              </secDNS:add>
-            {/if}
-          </secDNS:update>
+          {if $secdns}
+            <secDNS:update xmlns:secDNS="urn:ietf:params:xml:ns:secDNS-1.1">
+              {if $removeAllDsRecords}
+                <secDNS:rem>
+                  <secDNS:all>true</secDNS:all>
+                </secDNS:rem>
+              {/if}
+              {if length($removeDsRecords) > 0}
+                <secDNS:rem>
+                  {for $dsRecord in $removeDsRecords}
+                    <secDNS:dsData>
+                      <secDNS:keyTag>{$dsRecord.keyTag}</secDNS:keyTag>
+                      <secDNS:alg>{$dsRecord.alg}</secDNS:alg>
+                      <secDNS:digestType>{$dsRecord.digestType}</secDNS:digestType>
+                      <secDNS:digest>{$dsRecord.digest}</secDNS:digest>
+                    </secDNS:dsData>
+                  {/for}
+                </secDNS:rem>
+              {/if}
+              {if length($addDsRecords) > 0}
+                <secDNS:add>
+                  {for $dsRecord in $addDsRecords}
+                    <secDNS:dsData>
+                      <secDNS:keyTag>{$dsRecord.keyTag}</secDNS:keyTag>
+                      <secDNS:alg>{$dsRecord.alg}</secDNS:alg>
+                      <secDNS:digestType>{$dsRecord.digestType}</secDNS:digestType>
+                      <secDNS:digest>{$dsRecord.digest}</secDNS:digest>
+                    </secDNS:dsData>
+                  {/for}
+                </secDNS:add>
+              {/if}
+            </secDNS:update>
+          {/if}
+          {if $autorenews}
+            <superuser:domainUpdate xmlns:superuser="urn:google:params:xml:ns:superuser-1.0">
+              <superuser:autorenews>{$autorenews}</superuser:autorenews>
+            </superuser:domainUpdate>
+          {/if}
         </extension>
       {/if}
       <clTRID>RegistryTool</clTRID>

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

@@ -163,5 +163,7 @@
 
 /** Content if the registrar is not allowed to use registry lock. */
 {template .lockNotAllowedOnRegistrar}
-  <h2>Registry Lock is coming soon; please stay tuned for updates.</h2>
+  {@param supportEmail: string}
+  <h2>Sorry, your registrar hasn't enrolled in registry lock yet. To do so, please
+    contact {$supportEmail}.</h2>
 {/template}

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

@@ -31,7 +31,7 @@ import static google.registry.testing.TestLogHandlerUtils.assertLogMessage;
 import static org.joda.time.Duration.standardDays;
 import static org.joda.time.Duration.standardHours;
 import static org.joda.time.Duration.standardSeconds;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableSortedSet;
@@ -159,7 +159,7 @@ public class AsyncTaskEnqueuerTest {
                 .setRegistrarPocId("someone@example.com")
                 .setVerificationCode("hi")
                 .build());
-    asyncTaskEnqueuer.enqueueDomainRelock(lock);
+    asyncTaskEnqueuer.enqueueDomainRelock(lock.getRelockDuration().get(), lock.getRevisionId(), 0);
     assertTasksEnqueued(
         QUEUE_ASYNC_ACTIONS,
         new TaskMatcher()
@@ -169,6 +169,7 @@ public class AsyncTaskEnqueuerTest {
             .param(
                 RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
                 String.valueOf(lock.getRevisionId()))
+            .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0")
             .etaDelta(
                 standardHours(6).minus(standardSeconds(30)),
                 standardHours(6).plus(standardSeconds(30))));
@@ -188,9 +189,9 @@ public class AsyncTaskEnqueuerTest {
                 .setVerificationCode("hi")
                 .build());
     assertThat(
-            assertThrows(
-                IllegalArgumentException.class,
-                () -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration)))
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration)))
         .hasMessageThat()
         .isEqualTo(
             String.format(

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

@@ -15,10 +15,12 @@
 package google.registry.batch;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS;
 import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE;
 import static google.registry.model.eppcommon.StatusValue.PENDING_TRANSFER;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
 import static google.registry.testing.DatastoreHelper.persistDomainAsDeleted;
@@ -26,9 +28,16 @@ import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.SqlHelper.getMostRecentVerifiedRegistryLockByRepoId;
 import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode;
 import static google.registry.testing.SqlHelper.saveRegistryLock;
+import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
+import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
 import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
 import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static org.joda.time.Duration.standardSeconds;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.domain.DomainBase;
@@ -38,17 +47,27 @@ import google.registry.testing.AppEngineExtension;
 import google.registry.testing.DeterministicStringGenerator;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
+import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import google.registry.testing.UserInfo;
 import google.registry.tools.DomainLockUtils;
 import google.registry.util.AppEngineServiceUtils;
+import google.registry.util.EmailMessage;
+import google.registry.util.SendEmailService;
 import google.registry.util.StringGenerator.Alphabets;
 import java.util.Optional;
+import javax.mail.internet.InternetAddress;
+import org.joda.time.DateTime;
 import org.joda.time.Duration;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 /** Unit tests for {@link RelockDomainAction}. */
+@ExtendWith(MockitoExtension.class)
 public class RelockDomainActionTest {
 
   private static final String DOMAIN_NAME = "example.tld";
@@ -56,7 +75,7 @@ public class RelockDomainActionTest {
   private static final String POC_ID = "marla.singer@example.com";
 
   private final FakeResponse response = new FakeResponse();
-  private final FakeClock clock = new FakeClock();
+  private final FakeClock clock = new FakeClock(DateTime.parse("2015-05-18T12:34:56Z"));
   private final DomainLockUtils domainLockUtils =
       new DomainLockUtils(
           new DeterministicStringGenerator(Alphabets.BASE_58),
@@ -68,15 +87,18 @@ public class RelockDomainActionTest {
   public final AppEngineExtension appEngineRule =
       AppEngineExtension.builder()
           .withDatastoreAndCloudSql()
+          .withTaskQueue()
           .withUserService(UserInfo.create(POC_ID, "12345"))
           .build();
 
   private DomainBase domain;
   private RegistryLock oldLock;
+  @Mock private SendEmailService sendEmailService;
+  private AsyncTaskEnqueuer asyncTaskEnqueuer;
   private RelockDomainAction action;
 
   @BeforeEach
-  void beforeEach() {
+  void beforeEach() throws Exception {
     createTlds("tld", "net");
     HostResource host = persistActiveHost("ns1.example.net");
     domain = persistResource(newDomainBase(DOMAIN_NAME, host));
@@ -88,9 +110,22 @@ public class RelockDomainActionTest {
         domainLockUtils.administrativelyApplyUnlock(
             DOMAIN_NAME, CLIENT_ID, false, Optional.empty());
     assertThat(reloadDomain(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES);
+
+    AppEngineServiceUtils appEngineServiceUtils = mock(AppEngineServiceUtils.class);
+    lenient()
+        .when(appEngineServiceUtils.getServiceHostname("backend"))
+        .thenReturn("backend.hostname.fake");
+
+    asyncTaskEnqueuer =
+        AsyncTaskEnqueuerTest.createForTesting(appEngineServiceUtils, clock, Duration.ZERO);
     action = createAction(oldLock.getRevisionId());
   }
 
+  @AfterEach
+  void afterEach() {
+    verifyNoMoreInteractions(sendEmailService);
+  }
+
   @Test
   void testLock() {
     action.run();
@@ -104,29 +139,36 @@ public class RelockDomainActionTest {
   }
 
   @Test
-  void testFailure_unknownCode() {
+  void testFailure_unknownCode() throws Exception {
     action = createAction(12128675309L);
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
-    assertThat(response.getPayload()).isEqualTo("Relock failed: Unknown revision ID 12128675309");
+    assertThat(response.getPayload()).isEqualTo("Re-lock failed: Unknown revision ID 12128675309");
+    assertTaskEnqueued(1, 12128675309L, Duration.standardMinutes(10)); // should retry, transient
   }
 
   @Test
-  void testFailure_pendingDelete() {
+  void testFailure_pendingDelete() throws Exception {
     persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_DELETE)).build());
     action.run();
+    String expectedFailureMessage = "Domain example.tld has a pending delete.";
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo(String.format("Relock failed: Domain %s has a pending delete", DOMAIN_NAME));
+        .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
+    assertNonTransientFailureEmail(expectedFailureMessage);
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
   }
 
   @Test
-  void testFailure_pendingTransfer() {
+  void testFailure_pendingTransfer() throws Exception {
     persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_TRANSFER)).build());
     action.run();
+    String expectedFailureMessage = "Domain example.tld has a pending transfer.";
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo(String.format("Relock failed: Domain %s has a pending transfer", DOMAIN_NAME));
+        .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
+    assertNonTransientFailureEmail(expectedFailureMessage);
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
   }
 
   @Test
@@ -135,29 +177,64 @@ public class RelockDomainActionTest {
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock.");
+        .isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock.");
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
   }
 
   @Test
-  void testFailure_domainDeleted() {
+  void testFailure_domainDeleted() throws Exception {
     persistDomainAsDeleted(domain, clock.nowUtc());
     action.run();
+    String expectedFailureMessage = "Domain example.tld has been deleted.";
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo(String.format("Relock failed: Domain %s has been deleted", DOMAIN_NAME));
+        .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
+    assertNonTransientFailureEmail(expectedFailureMessage);
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
   }
 
   @Test
-  void testFailure_domainTransferred() {
+  void testFailure_domainTransferred() throws Exception {
     persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build());
     action.run();
+    String expectedFailureMessage =
+        "Domain example.tld has been transferred from registrar TheRegistrar to registrar "
+            + "NewRegistrar since the unlock.";
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo(
-            String.format(
-                "Relock failed: Domain %s has been transferred from registrar %s to registrar "
-                    + "%s since the unlock",
-                DOMAIN_NAME, CLIENT_ID, "NewRegistrar"));
+        .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
+    assertNonTransientFailureEmail(expectedFailureMessage);
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
+  }
+
+  @Test
+  public void testFailure_transientFailure_enqueuesTask() {
+    // Hard-delete the domain to simulate a DB failure
+    deleteResource(domain);
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
+    assertThat(response.getPayload()).isEqualTo("Re-lock failed: null");
+    assertTaskEnqueued(1);
+  }
+
+  @Test
+  void testFailure_sufficientTransientFailures_sendsEmail() throws Exception {
+    // Hard-delete the domain to simulate a DB failure
+    deleteResource(domain);
+    action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL);
+    action.run();
+    assertTaskEnqueued(RelockDomainAction.FAILURES_BEFORE_EMAIL + 1);
+    assertTransientFailureEmail();
+    assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
+    assertThat(response.getPayload()).isEqualTo("Re-lock failed: null");
+  }
+
+  @Test
+  void testSuccess_afterSufficientFailures_sendsEmail() throws Exception {
+    action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL + 1);
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(SC_OK);
+    assertSuccessEmailSent();
   }
 
   @Test
@@ -170,14 +247,108 @@ public class RelockDomainActionTest {
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
     assertThat(response.getPayload())
-        .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock.");
+        .isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock.");
+    assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
+  }
+
+  @Test
+  void testFailure_slowsDown() throws Exception {
+    deleteResource(domain);
+    action = createAction(oldLock.getRevisionId(), RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN);
+    action.run();
+    assertTaskEnqueued(
+        RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN + 1,
+        oldLock.getRevisionId(),
+        Duration.standardHours(1));
+  }
+
+  private void assertSuccessEmailSent() throws Exception {
+    EmailMessage expectedEmail =
+        EmailMessage.newBuilder()
+            .setSubject("Successful re-lock of domain example.tld")
+            .setBody(
+                "The domain example.tld was successfully re-locked.\n\nPlease "
+                    + "contact support at support@example.com if you have any questions.")
+            .setRecipients(
+                ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com")))
+            .setFrom(new InternetAddress("outgoing@example.com"))
+            .build();
+    verify(sendEmailService).sendEmail(expectedEmail);
+  }
+
+  private void assertNonTransientFailureEmail(String exceptionMessage) throws Exception {
+    String expectedBody =
+        String.format(
+            "There was an error when automatically re-locking example.tld. Error message: %s\n\n"
+                + "Please contact support at support@example.com if you have any questions.",
+            exceptionMessage);
+    assertFailureEmailWithBody(
+        expectedBody, ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com")));
+  }
+
+  private void assertTransientFailureEmail() throws Exception {
+    String expectedBody =
+        "There was an unexpected error when automatically re-locking example.tld. We will continue "
+            + "retrying the lock for five hours. Please contact support at support@example.com if "
+            + "you have any questions";
+    assertFailureEmailWithBody(
+        expectedBody,
+        ImmutableSet.of(
+            new InternetAddress("Marla.Singer.RegistryLock@crr.com"),
+            new InternetAddress("alerts@example.com")));
+  }
+
+  private void assertFailureEmailWithBody(String body, ImmutableSet<InternetAddress> recipients)
+      throws Exception {
+    EmailMessage expectedEmail =
+        EmailMessage.newBuilder()
+            .setSubject("Error re-locking domain example.tld")
+            .setBody(body)
+            .setRecipients(recipients)
+            .setFrom(new InternetAddress("outgoing@example.com"))
+            .build();
+    verify(sendEmailService).sendEmail(expectedEmail);
+  }
+
+  private void assertTaskEnqueued(int numAttempts) {
+    assertTaskEnqueued(numAttempts, oldLock.getRevisionId(), Duration.standardMinutes(10));
+  }
+
+  private void assertTaskEnqueued(int numAttempts, long oldUnlockRevisionId, Duration duration) {
+    assertTasksEnqueued(
+        QUEUE_ASYNC_ACTIONS,
+        new TaskMatcher()
+            .url(RelockDomainAction.PATH)
+            .method("POST")
+            .header("Host", "backend.hostname.fake")
+            .param(
+                RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
+                String.valueOf(oldUnlockRevisionId))
+            .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, String.valueOf(numAttempts))
+            .etaDelta(duration.minus(standardSeconds(30)), duration.plus(standardSeconds(30))));
   }
 
   private DomainBase reloadDomain(DomainBase domain) {
     return ofy().load().entity(domain).now();
   }
 
-  private RelockDomainAction createAction(Long oldUnlockRevisionId) {
-    return new RelockDomainAction(oldUnlockRevisionId, domainLockUtils, response);
+  private RelockDomainAction createAction(Long oldUnlockRevisionId) throws Exception {
+    return createAction(oldUnlockRevisionId, 0);
+  }
+
+  private RelockDomainAction createAction(Long oldUnlockRevisionId, int previousAttempts)
+      throws Exception {
+    InternetAddress alertRecipientAddress = new InternetAddress("alerts@example.com");
+    InternetAddress gSuiteOutgoingAddress = new InternetAddress("outgoing@example.com");
+    return new RelockDomainAction(
+        oldUnlockRevisionId,
+        previousAttempts,
+        alertRecipientAddress,
+        gSuiteOutgoingAddress,
+        "support@example.com",
+        sendEmailService,
+        domainLockUtils,
+        response,
+        asyncTaskEnqueuer);
   }
 }

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

@@ -51,7 +51,7 @@ class InitSqlPipelineGraphTest {
       TestPipelineExtension.create().enableAbandonedNodeEnforcement(false);
 
   @Test
-  public void createPipeline_compareGraph() throws IOException {
+  void createPipeline_compareGraph() throws IOException {
     new InitSqlPipeline(options, testPipeline).setupPipeline();
     String dotString = PipelineDotRenderer.toDotString(testPipeline);
     URL goldenDotUrl = Resources.getResource(InitSqlPipelineGraphTest.class, GOLDEN_DOT_FILE);

core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java

@@ -71,6 +71,7 @@ import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.registry.label.ReservedList;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.persistence.VKey;
 import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -162,12 +163,13 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase<DomainCheckFlow, Dom
   @Test
   void testSuccess_oneExists_allocationTokenIsRedeemed() throws Exception {
     setEppInput("domain_check_allocationtoken.xml");
-    persistActiveDomain("example1.tld");
+    DomainBase domain = persistActiveDomain("example1.tld");
+    Key<HistoryEntry> historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1L);
     persistResource(
         new AllocationToken.Builder()
             .setToken("abc123")
             .setTokenType(SINGLE_USE)
-            .setRedemptionHistoryEntry(Key.create(HistoryEntry.class, 1L))
+            .setRedemptionHistoryEntry(VKey.create(HistoryEntry.class, 1L, historyEntryKey))
             .build());
     doCheckTest(
         create(false, "example1.tld", "In use"),
@@ -417,7 +419,7 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase<DomainCheckFlow, Dom
             .setToken("abc123")
             .setTokenType(UNLIMITED_USE)
             .setDiscountFraction(0.5)
-            .setAllowedClientIds(ImmutableSet.of("someOtherClient"))
+            .setAllowedRegistrarIds(ImmutableSet.of("someOtherClient"))
             .setTokenStatusTransitions(
                 ImmutableSortedMap.<DateTime, TokenStatus>naturalOrder()
                     .put(START_OF_TIME, TokenStatus.NOT_STARTED)

core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java

@@ -162,6 +162,7 @@ import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.monitoring.whitebox.EppMetric;
+import google.registry.persistence.VKey;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import java.math.BigDecimal;
 import java.util.Map;
@@ -492,11 +493,13 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
         "domain_create_allocationtoken.xml",
         ImmutableMap.of("DOMAIN", "example.tld", "YEARS", "2"));
     persistContactsAndHosts();
+    DomainBase domain = persistActiveDomain("foo.tld");
+    Key<HistoryEntry> historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 505L);
     persistResource(
         new AllocationToken.Builder()
             .setToken("abc123")
             .setTokenType(SINGLE_USE)
-            .setRedemptionHistoryEntry(Key.create(HistoryEntry.class, 505L))
+            .setRedemptionHistoryEntry(VKey.create(HistoryEntry.class, 505L, historyEntryKey))
             .build());
     clock.advanceOneMilli();
     EppException thrown =
@@ -519,7 +522,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
     HistoryEntry historyEntry =
         ofy().load().type(HistoryEntry.class).ancestor(reloadResourceByForeignKey()).first().now();
     assertThat(ofy().load().entity(token).now().getRedemptionHistoryEntry())
-        .hasValue(Key.create(historyEntry));
+        .hasValue(HistoryEntry.createVKey(Key.create(historyEntry)));
   }
 
   @Test
@@ -1263,7 +1266,9 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
         ofy().load().key(Key.create(AllocationToken.class, token)).now();
     assertThat(reloadedToken.isRedeemed()).isTrue();
     assertThat(reloadedToken.getRedemptionHistoryEntry())
-        .hasValue(Key.create(getHistoryEntries(reloadResourceByForeignKey()).get(0)));
+        .hasValue(
+            HistoryEntry.createVKey(
+                Key.create(getHistoryEntries(reloadResourceByForeignKey()).get(0))));
   }
 
   private void assertAllocationTokenWasNotRedeemed(String token) {
@@ -1498,7 +1503,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
         new AllocationToken.Builder()
             .setToken("abc123")
             .setTokenType(UNLIMITED_USE)
-            .setAllowedClientIds(ImmutableSet.of("someClientId"))
+            .setAllowedRegistrarIds(ImmutableSet.of("someClientId"))
             .setDiscountFraction(0.5)
             .setTokenStatusTransitions(
                 ImmutableSortedMap.<DateTime, TokenStatus>naturalOrder()

core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java

@@ -572,6 +572,7 @@ class DomainTransferRequestFlowTest
       Duration expectedAutomaticTransferLength,
       BillingEvent.Cancellation.Builder... extraExpectedBillingEvents)
       throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
     setEppInput(commandFilename, substitutions);
     ImmutableSet<GracePeriod> originalGracePeriods = domain.getGracePeriods();
     // Replace the ROID in the xml file with the one generated in our test.
@@ -903,7 +904,6 @@ class DomainTransferRequestFlowTest
   @Test
   void testSuccess_superuserExtension_zeroPeriod_nonZeroAutomaticTransferLength() throws Exception {
     setupDomain("example", "tld");
-    eppRequestSource = EppRequestSource.TOOL;
     clock.advanceOneMilli();
     doSuccessfulSuperuserExtensionTest(
         "domain_transfer_request_superuser_extension.xml",
@@ -918,7 +918,6 @@ class DomainTransferRequestFlowTest
   @Test
   void testSuccess_superuserExtension_zeroPeriod_zeroAutomaticTransferLength() throws Exception {
     setupDomain("example", "tld");
-    eppRequestSource = EppRequestSource.TOOL;
     clock.advanceOneMilli();
     doSuccessfulSuperuserExtensionTest(
         "domain_transfer_request_superuser_extension.xml",
@@ -934,7 +933,6 @@ class DomainTransferRequestFlowTest
   void testSuccess_superuserExtension_nonZeroPeriod_nonZeroAutomaticTransferLength()
       throws Exception {
     setupDomain("example", "tld");
-    eppRequestSource = EppRequestSource.TOOL;
     clock.advanceOneMilli();
     doSuccessfulSuperuserExtensionTest(
         "domain_transfer_request_superuser_extension.xml",
@@ -948,7 +946,6 @@ class DomainTransferRequestFlowTest
 
   @Test
   void testSuccess_superuserExtension_zeroPeriod_autorenewGraceActive() throws Exception {
-    eppRequestSource = EppRequestSource.TOOL;
     setupDomain("example", "tld");
     VKey<BillingEvent.Recurring> existingAutorenewEvent = domain.getAutorenewBillingEvent();
     // Set domain to have auto-renewed just before the transfer request, so that it will have an

core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -89,12 +89,14 @@ import google.registry.model.host.HostResource;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import java.util.Optional;
 import org.joda.money.Money;
+import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link DomainUpdateFlow}. */
-public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, DomainBase> {
+class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, DomainBase> {
 
   private static final DelegationSignerData SOME_DSDATA =
       DelegationSignerData.create(1, 2, 3, base16().decode("0123"));
@@ -105,13 +107,12 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
           "DIGEST_TYPE", "1",
           "DIGEST", "38EC35D5B3A34B44C39B");
 
-  ContactResource sh8013Contact;
-  ContactResource mak21Contact;
-  ContactResource unusedContact;
-  HistoryEntry historyEntryDomainCreate;
+  private ContactResource sh8013Contact;
+  private ContactResource mak21Contact;
+  private ContactResource unusedContact;
 
   @BeforeEach
-  public void initDomainTest() {
+  void initDomainTest() {
     createTld("tld");
     // Note that "domain_update.xml" tests adding and removing the same contact type.
     setEppInput("domain_update.xml");
@@ -141,12 +142,11 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
                 .setRegistrant(mak21Contact.createVKey())
                 .setNameservers(ImmutableSet.of(host.createVKey()))
                 .build());
-    historyEntryDomainCreate =
-        persistResource(
-            new HistoryEntry.Builder()
-                .setType(HistoryEntry.Type.DOMAIN_CREATE)
-                .setParent(domain)
-                .build());
+    persistResource(
+        new HistoryEntry.Builder()
+            .setType(HistoryEntry.Type.DOMAIN_CREATE)
+            .setParent(domain)
+            .build());
     clock.advanceOneMilli();
     return domain;
   }
@@ -164,12 +164,11 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
                         DesignatedContact.create(Type.ADMIN, unusedContact.createVKey())))
                 .setNameservers(ImmutableSet.of(host.createVKey()))
                 .build());
-    historyEntryDomainCreate =
-        persistResource(
-            new HistoryEntry.Builder()
-                .setType(HistoryEntry.Type.DOMAIN_CREATE)
-                .setParent(domain)
-                .build());
+    persistResource(
+        new HistoryEntry.Builder()
+            .setType(HistoryEntry.Type.DOMAIN_CREATE)
+            .setParent(domain)
+            .build());
     clock.advanceOneMilli();
     return domain;
   }
@@ -193,27 +192,29 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
         .and()
         .hasLastEppUpdateTime(clock.nowUtc())
         .and()
-        .hasLastEppUpdateClientId("TheRegistrar");
+        .hasLastEppUpdateClientId("TheRegistrar")
+        .and()
+        .hasNoAutorenewEndTime();
     assertNoBillingEvents();
     assertDnsTasksEnqueued("example.tld");
   }
 
   @Test
-  public void testDryRun() throws Exception {
+  void testDryRun() throws Exception {
     persistReferencedEntities();
     persistDomain();
     dryRunFlowAssertResponse(loadFile("generic_success_response.xml"));
   }
 
   @Test
-  public void testSuccess() throws Exception {
+  void testSuccess() throws Exception {
     persistReferencedEntities();
     persistDomain();
     doSuccessfulTest();
   }
 
   @Test
-  public void testSuccess_clTridNotSpecified() throws Exception {
+  void testSuccess_clTridNotSpecified() throws Exception {
     setEppInput("domain_update_no_cltrid.xml");
     persistReferencedEntities();
     persistDomain();
@@ -221,7 +222,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_cachingDisabled() throws Exception {
+  void testSuccess_cachingDisabled() throws Exception {
     boolean origIsCachingEnabled = RegistryConfig.isEppResourceCachingEnabled();
     try {
       RegistryConfig.overrideIsEppResourceCachingEnabledForTesting(false);
@@ -234,7 +235,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_inQuietPeriod() throws Exception {
+  void testSuccess_inQuietPeriod() throws Exception {
     persistResource(
         Registry.get("tld")
             .asBuilder()
@@ -246,7 +247,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_emptyRegistrant() throws Exception {
+  void testFailure_emptyRegistrant() throws Exception {
     setEppInput("domain_update_empty_registrant.xml");
     persistReferencedEntities();
     persistDomain();
@@ -272,7 +273,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_maxNumberOfNameservers() throws Exception {
+  void testSuccess_maxNumberOfNameservers() throws Exception {
     persistReferencedEntities();
     persistDomain();
     // Modify domain to have 13 nameservers. We will then remove one and add one in the test.
@@ -281,7 +282,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_addAndRemoveLargeNumberOfNameserversAndContacts() throws Exception {
+  void testSuccess_addAndRemoveLargeNumberOfNameserversAndContacts() throws Exception {
     persistReferencedEntities();
     persistDomain();
     setEppInput("domain_update_max_everything.xml");
@@ -325,7 +326,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_metadata() throws Exception {
+  void testSuccess_metadata() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     setEppInput("domain_update_metadata.xml");
     persistReferencedEntities();
@@ -344,7 +345,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_metadataNotFromTool() throws Exception {
+  void testSuccess_metadataNotFromTool() throws Exception {
     setEppInput("domain_update_metadata.xml");
     persistReferencedEntities();
     persistDomain();
@@ -353,7 +354,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_removeContact() throws Exception {
+  void testSuccess_removeContact() throws Exception {
     setEppInput("domain_update_remove_contact.xml");
     persistReferencedEntities();
     persistDomain();
@@ -361,7 +362,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_addAndRemoveSubordinateHostNameservers() throws Exception {
+  void testSuccess_addAndRemoveSubordinateHostNameservers() throws Exception {
     // Test that operations involving subordinate hosts as nameservers do not change the subordinate
     // host relationship itself.
     setEppInput("domain_update_subordinate_hosts.xml");
@@ -394,7 +395,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_registrantMovedToTechContact() throws Exception {
+  void testSuccess_registrantMovedToTechContact() throws Exception {
     setEppInput("domain_update_registrant_to_tech.xml");
     persistReferencedEntities();
     ContactResource sh8013 =
@@ -409,7 +410,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_multipleReferencesToSameContactRemoved() throws Exception {
+  void testSuccess_multipleReferencesToSameContactRemoved() throws Exception {
     setEppInput("domain_update_remove_multiple_contacts.xml");
     persistReferencedEntities();
     ContactResource sh8013 =
@@ -430,7 +431,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_removeClientUpdateProhibited() throws Exception {
+  void testSuccess_removeClientUpdateProhibited() throws Exception {
     persistReferencedEntities();
     persistResource(
         persistDomain()
@@ -474,7 +475,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAdd() throws Exception {
+  void testSuccess_secDnsAdd() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         null,
@@ -485,7 +486,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddPreservesExisting() throws Exception {
+  void testSuccess_secDnsAddPreservesExisting() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -497,7 +498,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddSameDoesNothing() throws Exception {
+  void testSuccess_secDnsAddSameDoesNothing() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -506,7 +507,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddOnlyKeyTagRemainsSame() throws Exception {
+  void testSuccess_secDnsAddOnlyKeyTagRemainsSame() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -517,7 +518,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
 
   // Changing any of the four fields in DelegationSignerData should result in a new object
   @Test
-  public void testSuccess_secDnsAddOnlyChangeKeyTag() throws Exception {
+  void testSuccess_secDnsAddOnlyChangeKeyTag() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -527,7 +528,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddOnlyChangeAlgorithm() throws Exception {
+  void testSuccess_secDnsAddOnlyChangeAlgorithm() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -536,7 +537,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddOnlyChangeDigestType() throws Exception {
+  void testSuccess_secDnsAddOnlyChangeDigestType() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -545,7 +546,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddOnlyChangeDigest() throws Exception {
+  void testSuccess_secDnsAddOnlyChangeDigest() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add.xml",
         ImmutableSet.of(SOME_DSDATA),
@@ -554,7 +555,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddToMaxRecords() throws Exception {
+  void testSuccess_secDnsAddToMaxRecords() throws Exception {
     ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
     for (int i = 0; i < 7; ++i) {
       builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
@@ -573,7 +574,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsRemove() throws Exception {
+  void testSuccess_secDnsRemove() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_rem.xml",
         ImmutableSet.of(
@@ -583,7 +584,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsRemoveAll() throws Exception {
+  void testSuccess_secDnsRemoveAll() throws Exception {
     // As an aside, this test also validates that it's ok to set the 'urgent' attribute to false.
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_rem_all.xml",
@@ -594,7 +595,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddRemove() throws Exception {
+  void testSuccess_secDnsAddRemove() throws Exception {
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add_rem.xml",
         ImmutableSet.of(
@@ -606,7 +607,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddRemoveToMaxRecords() throws Exception {
+  void testSuccess_secDnsAddRemoveToMaxRecords() throws Exception {
     ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
     for (int i = 0; i < 7; ++i) {
       builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
@@ -630,7 +631,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsAddRemoveSame() throws Exception {
+  void testSuccess_secDnsAddRemoveSame() throws Exception {
     // Adding and removing the same dsData is a no-op because removes are processed first.
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_add_rem_same.xml",
@@ -643,13 +644,13 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_secDnsRemoveAlreadyNotThere() throws Exception {
+  void testSuccess_secDnsRemoveAlreadyNotThere() throws Exception {
     // Removing a dsData that isn't there is a no-op.
     doSecDnsSuccessfulTest(
         "domain_update_dsdata_rem.xml", ImmutableSet.of(SOME_DSDATA), ImmutableSet.of(SOME_DSDATA));
   }
 
-  public void doServerStatusBillingTest(String xmlFilename, boolean isBillable) throws Exception {
+  void doServerStatusBillingTest(String xmlFilename, boolean isBillable) throws Exception {
     setEppInput(xmlFilename);
     clock.advanceOneMilli();
     runFlowAssertResponse(
@@ -674,7 +675,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_addServerStatusBillingEvent() throws Exception {
+  void testSuccess_addServerStatusBillingEvent() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     persistReferencedEntities();
     persistDomain();
@@ -682,7 +683,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_noBillingOnPreExistingServerStatus() throws Exception {
+  void testSuccess_noBillingOnPreExistingServerStatus() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     DomainBase addStatusDomain = persistActiveDomain(getUniqueIdFromCommand());
     persistResource(
@@ -691,7 +692,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_removeServerStatusBillingEvent() throws Exception {
+  void testSuccess_removeServerStatusBillingEvent() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     persistReferencedEntities();
     DomainBase removeStatusDomain = persistDomain();
@@ -701,7 +702,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_changeServerStatusBillingEvent() throws Exception {
+  void testSuccess_changeServerStatusBillingEvent() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     persistReferencedEntities();
     DomainBase changeStatusDomain = persistDomain();
@@ -711,26 +712,26 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_noBillingEventOnNonServerStatusChange() throws Exception {
+  void testSuccess_noBillingEventOnNonServerStatusChange() throws Exception {
     persistActiveDomain(getUniqueIdFromCommand());
     doServerStatusBillingTest("domain_update_add_non_server_status.xml", false);
   }
 
   @Test
-  public void testSuccess_noBillingEventOnServerHoldStatusChange() throws Exception {
+  void testSuccess_noBillingEventOnServerHoldStatusChange() throws Exception {
     persistActiveDomain(getUniqueIdFromCommand());
     doServerStatusBillingTest("domain_update_add_server_hold_status.xml", false);
   }
 
   @Test
-  public void testSuccess_noBillingEventOnServerStatusChangeNotFromRegistrar() throws Exception {
+  void testSuccess_noBillingEventOnServerStatusChangeNotFromRegistrar() throws Exception {
     eppRequestSource = EppRequestSource.TOOL;
     persistActiveDomain(getUniqueIdFromCommand());
     doServerStatusBillingTest("domain_update_add_server_status_non_registrar.xml", false);
   }
 
   @Test
-  public void testSuccess_superuserClientUpdateProhibited() throws Exception {
+  void testSuccess_superuserClientUpdateProhibited() throws Exception {
     setEppInput("domain_update_add_server_hold_status.xml");
     persistReferencedEntities();
     persistResource(
@@ -758,29 +759,29 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_secDnsAllCannotBeFalse() throws Exception {
+  void testFailure_secDnsAllCannotBeFalse() throws Exception {
     doSecDnsFailingTest(SecDnsAllUsageException.class, "domain_update_dsdata_rem_all_false.xml");
   }
 
   @Test
-  public void testFailure_secDnsEmptyNotAllowed() throws Exception {
+  void testFailure_secDnsEmptyNotAllowed() throws Exception {
     doSecDnsFailingTest(EmptySecDnsUpdateException.class, "domain_update_dsdata_empty.xml");
   }
 
   @Test
-  public void testFailure_secDnsUrgentNotSupported() throws Exception {
+  void testFailure_secDnsUrgentNotSupported() throws Exception {
     doSecDnsFailingTest(
         UrgentAttributeNotSupportedException.class, "domain_update_dsdata_urgent.xml");
   }
 
   @Test
-  public void testFailure_secDnsChangeNotSupported() throws Exception {
+  void testFailure_secDnsChangeNotSupported() throws Exception {
     doSecDnsFailingTest(
         MaxSigLifeChangeNotSupportedException.class, "domain_update_maxsiglife.xml");
   }
 
   @Test
-  public void testFailure_secDnsTooManyDsRecords() throws Exception {
+  void testFailure_secDnsTooManyDsRecords() throws Exception {
     ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
     for (int i = 0; i < 8; ++i) {
       builder.add(DelegationSignerData.create(i, 2, 3, new byte[] {0, 1, 2}));
@@ -794,7 +795,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_tooManyNameservers() throws Exception {
+  void testFailure_tooManyNameservers() throws Exception {
     setEppInput("domain_update_add_nameserver.xml");
     persistReferencedEntities();
     persistDomain();
@@ -805,7 +806,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_wrongExtension() throws Exception {
+  void testFailure_wrongExtension() throws Exception {
     setEppInput("domain_update_wrong_extension.xml");
     persistReferencedEntities();
     persistDomain();
@@ -814,7 +815,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_neverExisted() throws Exception {
+  void testFailure_neverExisted() throws Exception {
     persistReferencedEntities();
     ResourceDoesNotExistException thrown =
         assertThrows(ResourceDoesNotExistException.class, this::runFlow);
@@ -822,7 +823,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_existedButWasDeleted() throws Exception {
+  void testFailure_existedButWasDeleted() throws Exception {
     persistReferencedEntities();
     persistDeletedDomain(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1));
     ResourceDoesNotExistException thrown =
@@ -831,7 +832,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_missingHost() throws Exception {
+  void testFailure_missingHost() throws Exception {
     persistActiveHost("ns1.example.foo");
     persistActiveContact("sh8013");
     persistActiveContact("mak21");
@@ -842,7 +843,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_missingContact() throws Exception {
+  void testFailure_missingContact() throws Exception {
     persistActiveHost("ns1.example.foo");
     persistActiveHost("ns2.example.foo");
     persistActiveContact("mak21");
@@ -853,7 +854,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_addingDuplicateContact() throws Exception {
+  void testFailure_addingDuplicateContact() throws Exception {
     persistReferencedEntities();
     persistActiveContact("foo");
     persistDomain();
@@ -878,7 +879,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_statusValueNotClientSettable() throws Exception {
+  void testFailure_statusValueNotClientSettable() throws Exception {
     setEppInput("domain_update_prohibited_status.xml");
     persistReferencedEntities();
     persistDomain();
@@ -887,7 +888,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_superuserStatusValueNotClientSettable() throws Exception {
+  void testSuccess_superuserStatusValueNotClientSettable() throws Exception {
     setEppInput("domain_update_prohibited_status.xml");
     persistReferencedEntities();
     persistDomain();
@@ -896,7 +897,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_serverUpdateProhibited_prohibitsNonSuperuserUpdates() throws Exception {
+  void testFailure_serverUpdateProhibited_prohibitsNonSuperuserUpdates() throws Exception {
     persistReferencedEntities();
     persistResource(
         newDomainBase(getUniqueIdFromCommand())
@@ -908,7 +909,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_serverUpdateProhibited_allowsSuperuserUpdates() throws Exception {
+  void testSuccess_serverUpdateProhibited_allowsSuperuserUpdates() throws Exception {
     persistReferencedEntities();
     persistResource(persistDomain().asBuilder().addStatusValue(SERVER_UPDATE_PROHIBITED).build());
     clock.advanceOneMilli();
@@ -917,7 +918,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_serverUpdateProhibited_notSettableWithoutSuperuser() throws Exception {
+  void testFailure_serverUpdateProhibited_notSettableWithoutSuperuser() throws Exception {
     setEppInput("domain_update_add_registry_lock.xml");
     persistReferencedEntities();
     persistDomain();
@@ -926,7 +927,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_serverUpdateProhibited_isSettableWithSuperuser() throws Exception {
+  void testSuccess_serverUpdateProhibited_isSettableWithSuperuser() throws Exception {
     setEppInput("domain_update_add_registry_lock.xml");
     persistReferencedEntities();
     persistDomain();
@@ -935,7 +936,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_clientUpdateProhibited() throws Exception {
+  void testFailure_clientUpdateProhibited() throws Exception {
     createTld("com");
     setEppInput("domain_update_authinfo.xml");
     persistReferencedEntities();
@@ -950,7 +951,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_serverUpdateProhibited() throws Exception {
+  void testFailure_serverUpdateProhibited() throws Exception {
     persistReferencedEntities();
     persistResource(
         newDomainBase(getUniqueIdFromCommand())
@@ -963,7 +964,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_pendingDelete() throws Exception {
+  void testFailure_pendingDelete() throws Exception {
     persistReferencedEntities();
     persistResource(
         newDomainBase(getUniqueIdFromCommand())
@@ -977,7 +978,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_duplicateContactInCommand() throws Exception {
+  void testFailure_duplicateContactInCommand() throws Exception {
     setEppInput("domain_update_duplicate_contact.xml");
     persistReferencedEntities();
     persistDomain();
@@ -986,7 +987,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_multipleDuplicateContactInCommand() throws Exception {
+  void testFailure_multipleDuplicateContactInCommand() throws Exception {
     setEppInput("domain_update_multiple_duplicate_contacts.xml");
     persistReferencedEntities();
     persistDomain();
@@ -1001,7 +1002,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_missingContactType() throws Exception {
+  void testFailure_missingContactType() throws Exception {
     // We need to test for missing type, but not for invalid - the schema enforces that for us.
     setEppInput("domain_update_missing_contact_type.xml");
     persistReferencedEntities();
@@ -1011,7 +1012,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_unauthorizedClient() throws Exception {
+  void testFailure_unauthorizedClient() throws Exception {
     sessionMetadata.setClientId("NewRegistrar");
     persistReferencedEntities();
     persistDomain();
@@ -1020,7 +1021,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_superuserUnauthorizedClient() throws Exception {
+  void testSuccess_superuserUnauthorizedClient() throws Exception {
     sessionMetadata.setClientId("NewRegistrar");
     persistReferencedEntities();
     persistDomain();
@@ -1030,7 +1031,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_notAuthorizedForTld() throws Exception {
+  void testFailure_notAuthorizedForTld() throws Exception {
     persistResource(
         loadRegistrar("TheRegistrar").asBuilder().setAllowedTlds(ImmutableSet.of()).build());
     persistReferencedEntities();
@@ -1040,7 +1041,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_superuserNotAuthorizedForTld() throws Exception {
+  void testSuccess_superuserNotAuthorizedForTld() throws Exception {
     persistResource(
         loadRegistrar("TheRegistrar").asBuilder().setAllowedTlds(ImmutableSet.of()).build());
     persistReferencedEntities();
@@ -1051,7 +1052,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_sameNameserverAddedAndRemoved() throws Exception {
+  void testFailure_sameNameserverAddedAndRemoved() throws Exception {
     setEppInput("domain_update_add_remove_same_host.xml");
     persistReferencedEntities();
     persistResource(
@@ -1068,7 +1069,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_sameContactAddedAndRemoved() throws Exception {
+  void testFailure_sameContactAddedAndRemoved() throws Exception {
     setEppInput("domain_update_add_remove_same_contact.xml");
     persistReferencedEntities();
     persistResource(
@@ -1086,7 +1087,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_removeAdmin() throws Exception {
+  void testFailure_removeAdmin() throws Exception {
     setEppInput("domain_update_remove_admin.xml");
     persistReferencedEntities();
     persistResource(
@@ -1102,7 +1103,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_removeTech() throws Exception {
+  void testFailure_removeTech() throws Exception {
     setEppInput("domain_update_remove_tech.xml");
     persistReferencedEntities();
     persistResource(
@@ -1118,7 +1119,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_addPendingDeleteContact() throws Exception {
+  void testFailure_addPendingDeleteContact() throws Exception {
     persistReferencedEntities();
     persistDomain();
     persistActiveHost("ns1.example.foo");
@@ -1137,7 +1138,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_addPendingDeleteHost() throws Exception {
+  void testFailure_addPendingDeleteHost() throws Exception {
     persistReferencedEntities();
     persistDomain();
     persistActiveHost("ns1.example.foo");
@@ -1156,7 +1157,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_newRegistrantNotAllowListed() throws Exception {
+  void testFailure_newRegistrantNotAllowListed() throws Exception {
     persistReferencedEntities();
     persistDomain();
     persistResource(
@@ -1170,8 +1171,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_addedNameserverDisallowedInTld()
-      throws Exception {
+  void testFailure_addedNameserverDisallowedInTld() throws Exception {
     persistReferencedEntities();
     persistDomain();
     persistResource(
@@ -1186,7 +1186,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_newNameserverAllowListed() throws Exception {
+  void testSuccess_newNameserverAllowListed() throws Exception {
     setEppInput("domain_update_add_nameserver.xml");
     persistReferencedEntities();
     persistDomain();
@@ -1212,7 +1212,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_changeRegistrantAllowListed() throws Exception {
+  void testSuccess_changeRegistrantAllowListed() throws Exception {
     setEppInput("domain_update_registrant.xml");
     persistReferencedEntities();
     persistDomain();
@@ -1229,7 +1229,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_changeContactsAndRegistrant() throws Exception {
+  void testSuccess_changeContactsAndRegistrant() throws Exception {
     setEppInput("domain_update_contacts_and_registrant.xml");
     persistReferencedEntities();
     persistDomainWithRegistrant();
@@ -1256,7 +1256,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_nameserverAndRegistrantAllowListed() throws Exception {
+  void testSuccess_nameserverAndRegistrantAllowListed() throws Exception {
     persistReferencedEntities();
     persistDomain();
     persistResource(
@@ -1269,7 +1269,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_tldWithNameserverAllowList_removeNameserver() throws Exception {
+  void testSuccess_tldWithNameserverAllowList_removeNameserver() throws Exception {
     setEppInput("domain_update_remove_nameserver.xml");
     persistReferencedEntities();
     persistDomain();
@@ -1301,7 +1301,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_tldWithNameserverAllowList_removeLastNameserver() throws Exception {
+  void testFailure_tldWithNameserverAllowList_removeLastNameserver() throws Exception {
     persistReferencedEntities();
     persistDomain();
     setEppInput("domain_update_remove_nameserver.xml");
@@ -1317,7 +1317,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testSuccess_domainCreateNotRestricted_doNotApplyServerProhibitedStatusCodes()
+  void testSuccess_domainCreateNotRestricted_doNotApplyServerProhibitedStatusCodes()
       throws Exception {
     persistReferencedEntities();
     persistDomain();
@@ -1328,7 +1328,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testFailure_freePremium_wrongFee() throws Exception {
+  void testFailure_freePremium_wrongFee() throws Exception {
     setEppInput("domain_update_fee.xml", ImmutableMap.of("FEE_VERSION", "0.11"));
     persistReferencedEntities();
     persistDomain();
@@ -1339,7 +1339,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   // This test should throw an exception, because the fee extension is required when the fee is not
   // zero.
   @Test
-  public void testFailure_missingFeeOnNonFreeUpdate() throws Exception {
+  void testFailure_missingFeeOnNonFreeUpdate() throws Exception {
     setEppInput("domain_update_wildcard.xml", ImmutableMap.of("DOMAIN", "non-free-update.tld"));
     persistReferencedEntities();
     persistDomain();
@@ -1349,11 +1349,41 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   }
 
   @Test
-  public void testIcannActivityReportField_getsLogged() throws Exception {
+  void testIcannActivityReportField_getsLogged() throws Exception {
     persistReferencedEntities();
     persistDomain();
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-dom-update");
     assertTldsFieldLogged("tld");
   }
+
+  @Test
+  void testSuperuserExtension_turnsOffAutorenew() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput("domain_update_superuser_extension.xml", ImmutableMap.of("AUTORENEWS", "false"));
+    DateTime expirationTime = clock.nowUtc().plusYears(3);
+    persistReferencedEntities();
+    persistResource(
+        persistDomain().asBuilder().setRegistrationExpirationTime(expirationTime).build());
+    clock.advanceOneMilli();
+    runFlowAsSuperuser();
+    assertAboutDomains().that(reloadResourceByForeignKey()).hasAutorenewEndTime(expirationTime);
+  }
+
+  @Test
+  void testSuperuserExtension_turnsOnAutorenew() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput("domain_update_superuser_extension.xml", ImmutableMap.of("AUTORENEWS", "true"));
+    DateTime expirationTime = clock.nowUtc().plusYears(3);
+    persistReferencedEntities();
+    persistResource(
+        persistDomain()
+            .asBuilder()
+            .setAutorenewEndTime(Optional.of(expirationTime))
+            .setRegistrationExpirationTime(expirationTime)
+            .build());
+    clock.advanceOneMilli();
+    runFlowAsSuperuser();
+    assertAboutDomains().that(reloadResourceByForeignKey()).hasNoAutorenewEndTime();
+  }
 }

core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java

@@ -22,6 +22,7 @@ import static google.registry.model.domain.token.AllocationToken.TokenStatus.VAL
 import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE;
 import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
@@ -42,11 +43,13 @@ import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTok
 import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotValidForRegistrarException;
 import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotValidForTldException;
 import google.registry.flows.domain.token.AllocationTokenFlowUtils.InvalidAllocationTokenException;
+import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.DomainCommand;
 import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.persistence.VKey;
 import google.registry.testing.AppEngineExtension;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
@@ -127,7 +130,7 @@ class AllocationTokenFlowUtilsTest {
   void test_validateToken_invalidForClientId() {
     persistResource(
         createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusDays(1))
-            .setAllowedClientIds(ImmutableSet.of("NewRegistrar"))
+            .setAllowedRegistrarIds(ImmutableSet.of("NewRegistrar"))
             .build());
     assertValidateThrowsEppException(AllocationTokenNotValidForRegistrarException.class);
   }
@@ -189,11 +192,13 @@ class AllocationTokenFlowUtilsTest {
 
   @Test
   void test_checkDomainsWithToken_showsFailureMessageForRedeemedToken() {
+    DomainBase domain = persistActiveDomain("example.tld");
+    Key<HistoryEntry> historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1051L);
     persistResource(
         new AllocationToken.Builder()
             .setToken("tokeN")
             .setTokenType(SINGLE_USE)
-            .setRedemptionHistoryEntry(Key.create(HistoryEntry.class, 101L))
+            .setRedemptionHistoryEntry(VKey.create(HistoryEntry.class, 101L, historyEntryKey))
             .build());
     assertThat(
             flowUtils

core/src/test/java/google/registry/model/CreateAutoTimestampTest.java

@@ -21,6 +21,7 @@ import static org.joda.time.DateTimeZone.UTC;
 
 import com.googlecode.objectify.annotation.Entity;
 import google.registry.model.common.CrossTldSingleton;
+import google.registry.schema.replay.EntityTest.EntityForTesting;
 import google.registry.testing.AppEngineExtension;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.Test;
@@ -38,6 +39,7 @@ public class CreateAutoTimestampTest {
 
   /** Timestamped class. */
   @Entity(name = "CatTestEntity")
+  @EntityForTesting
   public static class TestObject extends CrossTldSingleton {
     CreateAutoTimestamp createTime = CreateAutoTimestamp.create(null);
   }

core/src/test/java/google/registry/model/ImmutableObjectTest.java

@@ -29,6 +29,7 @@ import com.google.common.collect.Iterables;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
+import google.registry.schema.replay.EntityTest.EntityForTesting;
 import google.registry.testing.AppEngineExtension;
 import google.registry.util.CidrAddressBlock;
 import java.util.ArrayDeque;
@@ -279,6 +280,7 @@ public class ImmutableObjectTest {
 
   /** Simple subclass of ImmutableObject. */
   @Entity
+  @EntityForTesting
   public static class ValueObject extends ImmutableObject {
     @Id
     long id;

core/src/test/java/google/registry/model/UpdateAutoTimestampTest.java

@@ -21,6 +21,7 @@ import static org.joda.time.DateTimeZone.UTC;
 
 import com.googlecode.objectify.annotation.Entity;
 import google.registry.model.common.CrossTldSingleton;
+import google.registry.schema.replay.EntityTest.EntityForTesting;
 import google.registry.testing.AppEngineExtension;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.Test;
@@ -38,6 +39,7 @@ public class UpdateAutoTimestampTest {
 
   /** Timestamped class. */
   @Entity(name = "UatTestEntity")
+  @EntityForTesting
   public static class UpdateAutoTimestampTestObject extends CrossTldSingleton {
     UpdateAutoTimestamp updateTime = UpdateAutoTimestamp.create(null);
   }

core/src/test/java/google/registry/model/contact/ContactResourceTest.java

@@ -31,7 +31,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.EntityTestCase;
 import google.registry.model.ImmutableObjectSubject;
-import google.registry.model.billing.BillingEvent;
 import google.registry.model.contact.Disclose.PostalInfoChoice;
 import google.registry.model.contact.PostalInfo.Type;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
@@ -46,6 +45,7 @@ import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactResource}. */
 public class ContactResourceTest extends EntityTestCase {
+
   private ContactResource originalContact;
   private ContactResource contactResource;
 
@@ -113,8 +113,6 @@ public class ContactResourceTest extends EntityTestCase {
                     .setGainingClientId("gaining")
                     .setLosingClientId("losing")
                     .setPendingTransferExpirationTime(fakeClock.nowUtc())
-                    .setServerApproveEntities(
-                        ImmutableSet.of(VKey.create(BillingEvent.OneTime.class, 1)))
                     .setTransferRequestTime(fakeClock.nowUtc())
                     .setTransferStatus(TransferStatus.SERVER_APPROVED)
                     .setTransferRequestTrid(Trid.create("client-trid", "server-trid"))

core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java

@@ -14,6 +14,7 @@
 
 package google.registry.model.domain;
 
+import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation;
@@ -21,6 +22,7 @@ import static google.registry.testing.SqlHelper.saveRegistrar;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.DateTimeZone.UTC;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.contact.ContactResource;
@@ -37,7 +39,7 @@ import google.registry.persistence.transaction.JpaTestRules;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
-import javax.persistence.EntityManager;
+import java.util.Arrays;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
@@ -119,6 +121,261 @@ public class DomainBaseSqlTest {
 
   @Test
   void testDomainBasePersistence() {
+    persistDomain();
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase result = jpaTm().load(domain.createVKey());
+              assertEqualDomainExcept(result);
+            });
+  }
+
+  @Test
+  void testHostForeignKeyConstraints() {
+    assertThrowForeignKeyViolation(
+        () ->
+            jpaTm()
+                .transact(
+                    () -> {
+                      // Persist the domain without the associated host object.
+                      jpaTm().saveNew(contact);
+                      jpaTm().saveNew(contact2);
+                      jpaTm().saveNew(domain);
+                    }));
+  }
+
+  @Test
+  void testContactForeignKeyConstraints() {
+    assertThrowForeignKeyViolation(
+        () ->
+            jpaTm()
+                .transact(
+                    () -> {
+                      // Persist the domain without the associated contact objects.
+                      jpaTm().saveNew(domain);
+                      jpaTm().saveNew(host);
+                    }));
+  }
+
+  @Test
+  void testResaveDomain_succeeds() {
+    persistDomain();
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              jpaTm().saveNewOrUpdate(persisted.asBuilder().build());
+            });
+    jpaTm()
+        .transact(
+            () -> {
+              // Load the domain in its entirety.
+              DomainBase result = jpaTm().load(domain.createVKey());
+              assertEqualDomainExcept(result);
+            });
+  }
+
+  @Test
+  void testModifyGracePeriod_setEmptyCollectionSuccessfully() {
+    persistDomain();
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              DomainBase modified =
+                  persisted.asBuilder().setGracePeriods(ImmutableSet.of()).build();
+              jpaTm().saveNewOrUpdate(modified);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertThat(persisted.getGracePeriods()).isEmpty();
+            });
+  }
+
+  @Test
+  void testModifyGracePeriod_setNullCollectionSuccessfully() {
+    persistDomain();
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              DomainBase modified = persisted.asBuilder().setGracePeriods(null).build();
+              jpaTm().saveNewOrUpdate(modified);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertThat(persisted.getGracePeriods()).isEmpty();
+            });
+  }
+
+  @Test
+  void testModifyGracePeriod_addThenRemoveSuccessfully() {
+    persistDomain();
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              DomainBase modified =
+                  persisted
+                      .asBuilder()
+                      .addGracePeriod(
+                          GracePeriod.create(
+                              GracePeriodStatus.RENEW, "4-COM", END_OF_TIME, "registrar1", null))
+                      .build();
+              jpaTm().saveNewOrUpdate(modified);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertThat(persisted.getGracePeriods().size()).isEqualTo(2);
+              persisted
+                  .getGracePeriods()
+                  .forEach(
+                      gracePeriod -> {
+                        assertThat(gracePeriod.id).isNotNull();
+                        if (gracePeriod.getType() == GracePeriodStatus.ADD) {
+                          assertAboutImmutableObjects()
+                              .that(gracePeriod)
+                              .isEqualExceptFields(
+                                  GracePeriod.create(
+                                      GracePeriodStatus.ADD,
+                                      "4-COM",
+                                      END_OF_TIME,
+                                      "registrar1",
+                                      null),
+                                  "id");
+                        } else if (gracePeriod.getType() == GracePeriodStatus.RENEW) {
+                          assertAboutImmutableObjects()
+                              .that(gracePeriod)
+                              .isEqualExceptFields(
+                                  GracePeriod.create(
+                                      GracePeriodStatus.RENEW,
+                                      "4-COM",
+                                      END_OF_TIME,
+                                      "registrar1",
+                                      null),
+                                  "id");
+                        } else {
+                          fail("Unexpected GracePeriod: " + gracePeriod);
+                        }
+                      });
+              assertEqualDomainExcept(persisted, "gracePeriods");
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              DomainBase.Builder builder = persisted.asBuilder();
+              for (GracePeriod gracePeriod : persisted.getGracePeriods()) {
+                if (gracePeriod.getType() == GracePeriodStatus.RENEW) {
+                  builder.removeGracePeriod(gracePeriod);
+                }
+              }
+              jpaTm().saveNewOrUpdate(builder.build());
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertEqualDomainExcept(persisted);
+            });
+  }
+
+  @Test
+  void testModifyGracePeriod_removeThenAddSuccessfully() {
+    persistDomain();
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              DomainBase modified =
+                  persisted.asBuilder().setGracePeriods(ImmutableSet.of()).build();
+              jpaTm().saveNewOrUpdate(modified);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertThat(persisted.getGracePeriods()).isEmpty();
+              DomainBase modified =
+                  persisted
+                      .asBuilder()
+                      .addGracePeriod(
+                          GracePeriod.create(
+                              GracePeriodStatus.ADD, "4-COM", END_OF_TIME, "registrar1", null))
+                      .build();
+              jpaTm().saveNewOrUpdate(modified);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase persisted = jpaTm().load(domain.createVKey());
+              assertThat(persisted.getGracePeriods().size()).isEqualTo(1);
+              assertAboutImmutableObjects()
+                  .that(persisted.getGracePeriods().iterator().next())
+                  .isEqualExceptFields(
+                      GracePeriod.create(
+                          GracePeriodStatus.ADD, "4-COM", END_OF_TIME, "registrar1", null),
+                      "id");
+              assertEqualDomainExcept(persisted, "gracePeriods");
+            });
+  }
+
+  @Test
+  void testUpdates() {
+    jpaTm()
+        .transact(
+            () -> {
+              jpaTm().saveNew(contact);
+              jpaTm().saveNew(contact2);
+              jpaTm().saveNew(domain);
+              jpaTm().saveNew(host);
+            });
+    domain = domain.asBuilder().setNameservers(ImmutableSet.of()).build();
+    jpaTm().transact(() -> jpaTm().saveNewOrUpdate(domain));
+    jpaTm()
+        .transact(
+            () -> {
+              DomainBase result = jpaTm().load(domain.createVKey());
+
+              // Fix DS data, since we can't persist that yet.
+              result =
+                  result
+                      .asBuilder()
+                      .setDsData(
+                          ImmutableSet.of(
+                              DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+                      .build();
+
+              assertAboutImmutableObjects()
+                  .that(result)
+                  .isEqualExceptFields(domain, "updateTimestamp", "creationTime");
+            });
+  }
+
+  static ContactResource makeContact(String repoId) {
+    return new ContactResource.Builder()
+        .setRepoId(repoId)
+        .setCreationClientId("registrar1")
+        .setTransferData(new ContactTransferData.Builder().build())
+        .setPersistedCurrentSponsorClientId("registrar1")
+        .build();
+  }
+
+  private void persistDomain() {
     jpaTm()
         .transact(
             () -> {
@@ -136,69 +393,23 @@ public class DomainBaseSqlTest {
               // constraints, and Hibernate knows to insert it after domain and host.
               jpaTm().saveNew(host);
             });
-
-    jpaTm()
-        .transact(
-            () -> {
-              // Load the domain in its entirety.
-              EntityManager em = jpaTm().getEntityManager();
-              DomainBase result = em.find(DomainBase.class, "4-COM");
-
-              // Fix grace period and DS data, since we can't persist them yet.
-              result =
-                  result
-                      .asBuilder()
-                      .setRegistrant(contactKey)
-                      .setDsData(
-                          ImmutableSet.of(
-                              DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
-                      .build();
-
-              // Fix the original creation timestamp (this gets initialized on first write)
-              DomainBase org = domain.asBuilder().setCreationTime(result.getCreationTime()).build();
-
-              // Note that the equality comparison forces a lazy load of all fields.
-              assertAboutImmutableObjects()
-                  .that(result)
-                  .isEqualExceptFields(org, "updateTimestamp");
-            });
   }
 
-  @Test
-  void testHostForeignKeyConstraints() {
-    assertThrowForeignKeyViolation(
-        () -> {
-          jpaTm()
-              .transact(
-                  () -> {
-                    // Persist the domain without the associated host object.
-                    jpaTm().saveNew(contact);
-                    jpaTm().saveNew(contact2);
-                    jpaTm().saveNew(domain);
-                  });
-        });
-  }
+  private void assertEqualDomainExcept(DomainBase thatDomain, String... excepts) {
+    // Fix DS data, since we can't persist it yet.
+    thatDomain =
+        thatDomain
+            .asBuilder()
+            .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+            .build();
 
-  @Test
-  void testContactForeignKeyConstraints() {
-    assertThrowForeignKeyViolation(
-        () -> {
-          jpaTm()
-              .transact(
-                  () -> {
-                    // Persist the domain without the associated contact objects.
-                    jpaTm().saveNew(domain);
-                    jpaTm().saveNew(host);
-                  });
-        });
-  }
+    // Fix the original creation timestamp (this gets initialized on first write)
+    DomainBase org = domain.asBuilder().setCreationTime(thatDomain.getCreationTime()).build();
 
-  static ContactResource makeContact(String repoId) {
-    return new ContactResource.Builder()
-        .setRepoId(repoId)
-        .setCreationClientId("registrar1")
-        .setTransferData(new ContactTransferData.Builder().build())
-        .setPersistedCurrentSponsorClientId("registrar1")
-        .build();
+    String[] moreExcepts = Arrays.copyOf(excepts, excepts.length + 1);
+    moreExcepts[moreExcepts.length - 1] = "updateTimestamp";
+
+    // Note that the equality comparison forces a lazy load of all fields.
+    assertAboutImmutableObjects().that(thatDomain).isEqualExceptFields(org, moreExcepts);
   }
 }

core/src/test/java/google/registry/model/domain/GracePeriodTest.java

@@ -98,6 +98,11 @@ public class GracePeriodTest {
 
   @Test
   void testFailure_createForRecurring_notAutoRenew() {
+    Key<Recurring> recurringKey =
+        Key.create(
+            Key.create(Key.create(DomainBase.class, "1-TEST"), HistoryEntry.class, 343L),
+            Recurring.class,
+            12345);
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
@@ -107,7 +112,7 @@ public class GracePeriodTest {
                     "1-TEST",
                     now.plusDays(1),
                     "TheRegistrar",
-                    VKey.create(Recurring.class, 12345)));
+                    VKey.create(Recurring.class, 12345, recurringKey)));
     assertThat(thrown).hasMessageThat().contains("autorenew");
   }
 }