Make ICANN reporting not fail on success upload (#791)

* Make ICANN reporting not fail on success upload

According to the spec
(https://tools.ietf.org/html/draft-lozano-icann-registry-interfaces-13#page-16),
when an upload succeeds (HTTP response code 200), the result code
contained in the response message is always 1000 (success). So there is
no need to parse the response content and check the result code. Given
that we are having a problem parsing the response content due to encoding,
it is best that we don't check it so as to not get false negative
alerts when the upload is successful.

The current logic also has a bug: HttpRequest.execute() will by default
throw when the response code is non-20X. Therefore for a 400 response,
our parsing logic never runs on it. Coincidentally, this month when we
uploaded the July activity report (due to stale cursors), we get 400
responses (due to existing reports on the ICANN servers). The stack
trace printed for the thrown exceptions from the 400 responses contained
correctly parsed response contents. This lead us to believe that the issue with
encoding was transient last month. However when we tried again to upload this
month's report, our parser failed again (because the response code was 200 this
time, and our parser actually ran on the response contents).

This seems to suggest that ICANN is sending back readable response
contents, but our parser somehow failed to understand it, assuming that
ICANN is using the same encoding for 200 (which we tried and failed to
parse) and 400 response contents (which caused an exception and was printed
corrected in the stack trace).

This PR changed the transport behavior so that it doesn't throw
automatically for non-20X responses. We will print the content for both
200 and 400 responses, but only try to parse 400 response content. We
put the 400 response in an HttpResponseException and print stack trace
from it, which should display the content correctly so that we can
compare it with the result of our own parsing.

* Add tests

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/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/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/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,6 +42,8 @@ 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;
@@ -43,6 +51,15 @@ public class ContactHistory extends HistoryEntry {
   @Column(nullable = false)
   VKey<ContactResource> contactRepoId;
 
+  @Id
+  @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "HistorySequenceGenerator")
+  @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
@@ -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,43 @@ 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 = "HistorySequenceGenerator")
+  @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 +93,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 +156,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 +172,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/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 = "HistorySequenceGenerator")
+  @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/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/reporting/HistoryEntry.java

@@ -14,8 +14,10 @@
 
 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.ImmutableSet;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
@@ -28,21 +30,26 @@ 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 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;
 
@@ -51,6 +58,7 @@ import org.joda.time.DateTime;
 @Entity
 @MappedSuperclass
 @WithStringVKey // TODO(b/162229294): This should be resolved during the course of that bug
+@Access(AccessType.FIELD)
 public class HistoryEntry extends ImmutableObject implements Buildable {
 
   /** Represents the type of history entry. */
@@ -102,16 +110,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;
@@ -187,8 +192,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() {
@@ -259,6 +273,40 @@ 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;
+  }
+
   /** A builder for {@link HistoryEntry} since it is immutable */
   public static class Builder<T extends HistoryEntry, B extends Builder<?, ?>>
       extends GenericBuilder<T, B> {
@@ -268,11 +316,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/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/reporting/icann/IcannHttpReporter.java

@@ -24,6 +24,8 @@ 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;
@@ -80,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(
@@ -87,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());
@@ -94,13 +103,24 @@ public class IcannHttpReporter {
         response.getContent().close();
       }
       logger.atInfo().log(
-          "Received response code %d with content: %s\n\nResponse content in hex: %s",
+          "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));
-      XjcIirdeaResult result = parseResult(content);
-      if (result.getCode().getValue() != 1000) {
+      // 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/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/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,9 @@
       <basic name="amount" access="FIELD"/>
     </attributes>
   </embeddable>
+
+  <sequence-generator name="HistorySequenceGenerator" sequence-name="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>
@@ -34,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>
@@ -82,5 +101,12 @@
 
     <!-- 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/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/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,77 +121,217 @@ public class DomainBaseSqlTest {
 
   @Test
   void testDomainBasePersistence() {
-    jpaTm()
-        .transact(
-            () -> {
-              // Persist the contacts.  Note that these need to be persisted before the domain
-              // otherwise we get a foreign key constraint error.  If we ever decide to defer the
-              // relevant foreign key checks to commit time, then the order would not matter.
-              jpaTm().saveNew(contact);
-              jpaTm().saveNew(contact2);
-
-              // Persist the domain.
-              jpaTm().saveNew(domain);
-
-              // Persist the host.  This does _not_ need to be persisted before the domain,
-              // because only the row in the join table (DomainHost) is subject to foreign key
-              // constraints, and Hibernate knows to insert it after domain and host.
-              jpaTm().saveNew(host);
-            });
+    persistDomain();
 
     jpaTm()
         .transact(
             () -> {
-              // Load the domain in its entirety.
-              EntityManager em = jpaTm().getEntityManager();
-              DomainBase result = em.find(DomainBase.class, "4-COM");
-
-              // Fix DS data, since we can't persist it yet.
-              result =
-                  result
-                      .asBuilder()
-                      .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");
+              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);
-                  });
-        });
+        () ->
+            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);
-                  });
-        });
+        () ->
+            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
@@ -232,4 +374,42 @@ public class DomainBaseSqlTest {
         .setPersistedCurrentSponsorClientId("registrar1")
         .build();
   }
+
+  private void persistDomain() {
+    jpaTm()
+        .transact(
+            () -> {
+              // Persist the contacts.  Note that these need to be persisted before the domain
+              // otherwise we get a foreign key constraint error.  If we ever decide to defer the
+              // relevant foreign key checks to commit time, then the order would not matter.
+              jpaTm().saveNew(contact);
+              jpaTm().saveNew(contact2);
+
+              // Persist the domain.
+              jpaTm().saveNew(domain);
+
+              // Persist the host.  This does _not_ need to be persisted before the domain,
+              // because only the row in the join table (DomainHost) is subject to foreign key
+              // constraints, and Hibernate knows to insert it after domain and host.
+              jpaTm().saveNew(host);
+            });
+  }
+
+  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();
+
+    // Fix the original creation timestamp (this gets initialized on first write)
+    DomainBase org = domain.asBuilder().setCreationTime(thatDomain.getCreationTime()).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/token/AllocationTokenTest.java

@@ -224,11 +224,13 @@ public class AllocationTokenTest extends EntityTestCase {
 
   @Test
   void testBuild_redemptionHistoryEntryOnlyInSingleUse() {
+    DomainBase domain = persistActiveDomain("blahdomain.foo");
+    Key<HistoryEntry> historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1);
     AllocationToken.Builder builder =
         new AllocationToken.Builder()
             .setToken("foobar")
             .setTokenType(TokenType.UNLIMITED_USE)
-            .setRedemptionHistoryEntry(VKey.create(HistoryEntry.class, 1L));
+            .setRedemptionHistoryEntry(VKey.create(HistoryEntry.class, 1L, historyEntryKey));
     IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, builder::build);
     assertThat(thrown)
         .hasMessageThat()

core/src/test/java/google/registry/model/history/ContactHistoryTest.java

@@ -17,11 +17,14 @@ package google.registry.model.history;
 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.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatastoreHelper.newContactResourceWithRoid;
 import static google.registry.testing.SqlHelper.saveRegistrar;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.googlecode.objectify.Key;
 import google.registry.model.EntityTestCase;
+import google.registry.model.contact.ContactBase;
 import google.registry.model.contact.ContactHistory;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.eppcommon.Trid;
@@ -44,19 +47,8 @@ public class ContactHistoryTest extends EntityTestCase {
     jpaTm().transact(() -> jpaTm().saveNew(contact));
     VKey<ContactResource> contactVKey = contact.createVKey();
     ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().load(contactVKey));
-    ContactHistory contactHistory =
-        new ContactHistory.Builder()
-            .setType(HistoryEntry.Type.HOST_CREATE)
-            .setXmlBytes("<xml></xml>".getBytes(UTF_8))
-            .setModificationTime(fakeClock.nowUtc())
-            .setClientId("TheRegistrar")
-            .setTrid(Trid.create("ABC-123", "server-trid"))
-            .setBySuperuser(false)
-            .setReason("reason")
-            .setRequestedByRegistrar(true)
-            .setContactBase(contactFromDb)
-            .setContactRepoId(contactVKey)
-            .build();
+    ContactHistory contactHistory = createContactHistory(contactFromDb, contactVKey);
+    contactHistory.id = null;
     jpaTm().transact(() -> jpaTm().saveNew(contactHistory));
     jpaTm()
         .transact(
@@ -68,6 +60,47 @@ public class ContactHistoryTest extends EntityTestCase {
             });
   }
 
+  @Test
+  void testOfyPersistence() {
+    saveRegistrar("TheRegistrar");
+
+    ContactResource contact = newContactResourceWithRoid("contactId", "contact1");
+    tm().transact(() -> tm().saveNew(contact));
+    VKey<ContactResource> contactVKey = contact.createVKey();
+    ContactResource contactFromDb = tm().transact(() -> tm().load(contactVKey));
+    fakeClock.advanceOneMilli();
+    ContactHistory contactHistory = createContactHistory(contactFromDb, contactVKey);
+    tm().transact(() -> tm().saveNew(contactHistory));
+
+    // retrieving a HistoryEntry or a ContactHistory with the same key should return the same object
+    // note: due to the @EntitySubclass annotation. all Keys for ContactHistory objects will have
+    // type HistoryEntry
+    VKey<ContactHistory> contactHistoryVKey =
+        VKey.createOfy(ContactHistory.class, Key.create(contactHistory));
+    VKey<HistoryEntry> historyEntryVKey =
+        VKey.createOfy(HistoryEntry.class, Key.create(contactHistory.asHistoryEntry()));
+    ContactHistory hostHistoryFromDb = tm().transact(() -> tm().load(contactHistoryVKey));
+    HistoryEntry historyEntryFromDb = tm().transact(() -> tm().load(historyEntryVKey));
+
+    assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb);
+  }
+
+  private ContactHistory createContactHistory(
+      ContactBase contact, VKey<ContactResource> contactVKey) {
+    return new ContactHistory.Builder()
+        .setType(HistoryEntry.Type.HOST_CREATE)
+        .setXmlBytes("<xml></xml>".getBytes(UTF_8))
+        .setModificationTime(fakeClock.nowUtc())
+        .setClientId("TheRegistrar")
+        .setTrid(Trid.create("ABC-123", "server-trid"))
+        .setBySuperuser(false)
+        .setReason("reason")
+        .setRequestedByRegistrar(true)
+        .setContactBase(contact)
+        .setContactRepoId(contactVKey)
+        .build();
+  }
+
   static void assertContactHistoriesEqual(ContactHistory one, ContactHistory two) {
     assertAboutImmutableObjects()
         .that(one)

core/src/test/java/google/registry/model/history/DomainHistoryTest.java

@@ -14,18 +14,22 @@
 
 package google.registry.model.history;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 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.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatastoreHelper.newContactResourceWithRoid;
 import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.newHostResourceWithRoid;
 import static google.registry.testing.SqlHelper.saveRegistrar;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.googlecode.objectify.Key;
 import google.registry.model.EntityTestCase;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainContent;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostResource;
@@ -45,9 +49,14 @@ public class DomainHistoryTest extends EntityTestCase {
     saveRegistrar("TheRegistrar");
 
     HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
-    jpaTm().transact(() -> jpaTm().saveNew(host));
     ContactResource contact = newContactResourceWithRoid("contactId", "contact1");
-    jpaTm().transact(() -> jpaTm().saveNew(contact));
+
+    jpaTm()
+        .transact(
+            () -> {
+              jpaTm().saveNew(host);
+              jpaTm().saveNew(contact);
+            });
 
     DomainBase domain =
         newDomainBase("example.tld", "domainRepoId", contact)
@@ -56,35 +65,81 @@ public class DomainHistoryTest extends EntityTestCase {
             .build();
     jpaTm().transact(() -> jpaTm().saveNew(domain));
 
-    DomainHistory domainHistory =
-        new DomainHistory.Builder()
-            .setType(HistoryEntry.Type.DOMAIN_CREATE)
-            .setXmlBytes("<xml></xml>".getBytes(UTF_8))
-            .setModificationTime(fakeClock.nowUtc())
-            .setClientId("TheRegistrar")
-            .setTrid(Trid.create("ABC-123", "server-trid"))
-            .setBySuperuser(false)
-            .setReason("reason")
-            .setRequestedByRegistrar(true)
-            .setDomainContent(domain)
-            .setDomainRepoId(domain.createVKey())
-            .build();
+    DomainHistory domainHistory = createDomainHistory(domain);
+    domainHistory.id = null;
     jpaTm().transact(() -> jpaTm().saveNew(domainHistory));
 
     jpaTm()
         .transact(
             () -> {
-              DomainHistory fromDatabase =
-                  jpaTm().load(VKey.createSql(DomainHistory.class, domainHistory.getId()));
+              DomainHistory fromDatabase = jpaTm().load(domainHistory.createVKey());
               assertDomainHistoriesEqual(fromDatabase, domainHistory);
               assertThat(fromDatabase.getDomainRepoId().getSqlKey())
                   .isEqualTo(domainHistory.getDomainRepoId().getSqlKey());
+              assertThat(fromDatabase.getNsHosts())
+                  .containsExactlyElementsIn(
+                      domainHistory.getNsHosts().stream()
+                          .map(key -> VKey.createSql(HostResource.class, key.getSqlKey()))
+                          .collect(toImmutableSet()));
             });
   }
 
+  @Test
+  void testOfyPersistence() {
+    saveRegistrar("TheRegistrar");
+
+    HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
+    ContactResource contact = newContactResourceWithRoid("contactId", "contact1");
+
+    tm().transact(
+            () -> {
+              tm().saveNew(host);
+              tm().saveNew(contact);
+            });
+    fakeClock.advanceOneMilli();
+
+    DomainBase domain =
+        newDomainBase("example.tld", "domainRepoId", contact)
+            .asBuilder()
+            .setNameservers(host.createVKey())
+            .build();
+    tm().transact(() -> tm().saveNew(domain));
+
+    fakeClock.advanceOneMilli();
+    DomainHistory domainHistory = createDomainHistory(domain);
+    tm().transact(() -> tm().saveNew(domainHistory));
+
+    // retrieving a HistoryEntry or a DomainHistory with the same key should return the same object
+    // note: due to the @EntitySubclass annotation. all Keys for ContactHistory objects will have
+    // type HistoryEntry
+    VKey<DomainHistory> domainHistoryVKey =
+        VKey.createOfy(DomainHistory.class, Key.create(domainHistory));
+    VKey<HistoryEntry> historyEntryVKey =
+        VKey.createOfy(HistoryEntry.class, Key.create(domainHistory.asHistoryEntry()));
+    DomainHistory domainHistoryFromDb = tm().transact(() -> tm().load(domainHistoryVKey));
+    HistoryEntry historyEntryFromDb = tm().transact(() -> tm().load(historyEntryVKey));
+
+    assertThat(domainHistoryFromDb).isEqualTo(historyEntryFromDb);
+  }
+
   static void assertDomainHistoriesEqual(DomainHistory one, DomainHistory two) {
     assertAboutImmutableObjects()
         .that(one)
-        .isEqualExceptFields(two, "domainContent", "domainRepoId", "parent");
+        .isEqualExceptFields(two, "domainContent", "domainRepoId", "parent", "nsHosts");
+  }
+
+  private DomainHistory createDomainHistory(DomainContent domain) {
+    return new DomainHistory.Builder()
+        .setType(HistoryEntry.Type.DOMAIN_CREATE)
+        .setXmlBytes("<xml></xml>".getBytes(UTF_8))
+        .setModificationTime(fakeClock.nowUtc())
+        .setClientId("TheRegistrar")
+        .setTrid(Trid.create("ABC-123", "server-trid"))
+        .setBySuperuser(false)
+        .setReason("reason")
+        .setRequestedByRegistrar(true)
+        .setDomainContent(domain)
+        .setDomainRepoId(domain.getRepoId())
+        .build();
   }
 }

core/src/test/java/google/registry/model/history/HostHistoryTest.java

@@ -17,12 +17,15 @@ package google.registry.model.history;
 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.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatastoreHelper.newHostResourceWithRoid;
 import static google.registry.testing.SqlHelper.saveRegistrar;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.googlecode.objectify.Key;
 import google.registry.model.EntityTestCase;
 import google.registry.model.eppcommon.Trid;
+import google.registry.model.host.HostBase;
 import google.registry.model.host.HostHistory;
 import google.registry.model.host.HostResource;
 import google.registry.model.reporting.HistoryEntry;
@@ -44,19 +47,8 @@ public class HostHistoryTest extends EntityTestCase {
     jpaTm().transact(() -> jpaTm().saveNew(host));
     VKey<HostResource> hostVKey = VKey.createSql(HostResource.class, "host1");
     HostResource hostFromDb = jpaTm().transact(() -> jpaTm().load(hostVKey));
-    HostHistory hostHistory =
-        new HostHistory.Builder()
-            .setType(HistoryEntry.Type.HOST_CREATE)
-            .setXmlBytes("<xml></xml>".getBytes(UTF_8))
-            .setModificationTime(fakeClock.nowUtc())
-            .setClientId("TheRegistrar")
-            .setTrid(Trid.create("ABC-123", "server-trid"))
-            .setBySuperuser(false)
-            .setReason("reason")
-            .setRequestedByRegistrar(true)
-            .setHostBase(hostFromDb)
-            .setHostRepoId(hostVKey)
-            .build();
+    HostHistory hostHistory = createHostHistory(hostFromDb, hostVKey);
+    hostHistory.id = null;
     jpaTm().transact(() -> jpaTm().saveNew(hostHistory));
     jpaTm()
         .transact(
@@ -69,10 +61,49 @@ public class HostHistoryTest extends EntityTestCase {
             });
   }
 
+  @Test
+  public void testOfySave() {
+    saveRegistrar("registrar1");
+
+    HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
+    tm().transact(() -> tm().saveNew(host));
+    VKey<HostResource> hostVKey = VKey.create(HostResource.class, "host1", Key.create(host));
+    HostResource hostFromDb = tm().transact(() -> tm().load(hostVKey));
+    HostHistory hostHistory = createHostHistory(hostFromDb, hostVKey);
+    fakeClock.advanceOneMilli();
+    tm().transact(() -> tm().saveNew(hostHistory));
+
+    // retrieving a HistoryEntry or a HostHistory with the same key should return the same object
+    // note: due to the @EntitySubclass annotation. all Keys for ContactHistory objects will have
+    // type HistoryEntry
+    VKey<HostHistory> hostHistoryVKey = VKey.createOfy(HostHistory.class, Key.create(hostHistory));
+    VKey<HistoryEntry> historyEntryVKey =
+        VKey.createOfy(HistoryEntry.class, Key.create(hostHistory.asHistoryEntry()));
+    HostHistory hostHistoryFromDb = tm().transact(() -> tm().load(hostHistoryVKey));
+    HistoryEntry historyEntryFromDb = tm().transact(() -> tm().load(historyEntryVKey));
+
+    assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb);
+  }
+
   private void assertHostHistoriesEqual(HostHistory one, HostHistory two) {
     assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "hostBase");
     assertAboutImmutableObjects()
         .that(one.getHostBase())
         .isEqualExceptFields(two.getHostBase(), "repoId");
   }
+
+  private HostHistory createHostHistory(HostBase hostBase, VKey<HostResource> hostVKey) {
+    return new HostHistory.Builder()
+        .setType(HistoryEntry.Type.HOST_CREATE)
+        .setXmlBytes("<xml></xml>".getBytes(UTF_8))
+        .setModificationTime(fakeClock.nowUtc())
+        .setClientId("TheRegistrar")
+        .setTrid(Trid.create("ABC-123", "server-trid"))
+        .setBySuperuser(false)
+        .setReason("reason")
+        .setRequestedByRegistrar(true)
+        .setHostBase(hostBase)
+        .setHostRepoId(hostVKey)
+        .build();
+  }
 }

core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.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 com.google.common.truth.Truth.assertThat;
 
@@ -40,20 +40,22 @@ public class ClaimsListDaoTest {
 
   @Test
   void trySave_insertsClaimsListSuccessfully() {
-    ClaimsList claimsList =
-        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListShard claimsList =
+        ClaimsListShard.create(
+            fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
     ClaimsListDao.trySave(claimsList);
-    ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     assertThat(insertedClaimsList.getCreationTimestamp()).isEqualTo(fakeClock.nowUtc());
   }
 
   @Test
   void trySave_noExceptionThrownWhenSaveFail() {
-    ClaimsList claimsList =
-        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListShard claimsList =
+        ClaimsListShard.create(
+            fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
     ClaimsListDao.trySave(claimsList);
-    ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     // Save ClaimsList with existing revisionId should fail because revisionId is the primary key.
     ClaimsListDao.trySave(insertedClaimsList);
@@ -61,9 +63,9 @@ public class ClaimsListDaoTest {
 
   @Test
   void trySave_claimsListWithNoEntries() {
-    ClaimsList claimsList = ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of());
+    ClaimsListShard claimsList = ClaimsListShard.create(fakeClock.nowUtc(), ImmutableMap.of());
     ClaimsListDao.trySave(claimsList);
-    ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get();
+    ClaimsListShard insertedClaimsList = ClaimsListDao.getLatestRevision().get();
     assertClaimsListEquals(claimsList, insertedClaimsList);
     assertThat(insertedClaimsList.getLabelsToKeys()).isEmpty();
   }
@@ -75,16 +77,18 @@ public class ClaimsListDaoTest {
 
   @Test
   void getCurrent_returnsLatestClaims() {
-    ClaimsList oldClaimsList =
-        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
-    ClaimsList newClaimsList =
-        ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4"));
+    ClaimsListShard oldClaimsList =
+        ClaimsListShard.create(
+            fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2"));
+    ClaimsListShard newClaimsList =
+        ClaimsListShard.create(
+            fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4"));
     ClaimsListDao.trySave(oldClaimsList);
     ClaimsListDao.trySave(newClaimsList);
     assertClaimsListEquals(newClaimsList, ClaimsListDao.getLatestRevision().get());
   }
 
-  private void assertClaimsListEquals(ClaimsList left, ClaimsList right) {
+  private void assertClaimsListEquals(ClaimsListShard left, ClaimsListShard right) {
     assertThat(left.getRevisionId()).isEqualTo(right.getRevisionId());
     assertThat(left.getTmdbGenerationTime()).isEqualTo(right.getTmdbGenerationTime());
     assertThat(left.getLabelsToKeys()).isEqualTo(right.getLabelsToKeys());

core/src/test/java/google/registry/model/tmch/ClaimsListShardTest.java

@@ -76,7 +76,7 @@ public class ClaimsListShardTest {
     DateTime now = DateTime.now(UTC);
     // Save it with sharding, and make sure that reloading it works.
     ClaimsListShard unsharded = ClaimsListShard.create(now, ImmutableMap.copyOf(labelsToKeys));
-    unsharded.save(shardSize);
+    unsharded.saveToDatastore(shardSize);
     assertThat(ClaimsListShard.get().labelsToKeys).isEqualTo(unsharded.labelsToKeys);
     List<ClaimsListShard> shards1 = ofy().load().type(ClaimsListShard.class).list();
     assertThat(shards1).hasSize(4);
@@ -90,7 +90,7 @@ public class ClaimsListShardTest {
       labelsToKeys.put(Integer.toString(i), Integer.toString(i));
     }
     unsharded = ClaimsListShard.create(now.plusDays(1), ImmutableMap.copyOf(labelsToKeys));
-    unsharded.save(shardSize);
+    unsharded.saveToDatastore(shardSize);
     ofy().clearSessionCache();
     assertThat(ClaimsListShard.get().labelsToKeys).hasSize(unsharded.labelsToKeys.size());
     assertThat(ClaimsListShard.get().labelsToKeys).isEqualTo(unsharded.labelsToKeys);

core/src/test/java/google/registry/persistence/converter/DurationConverterTest.java

@@ -51,22 +51,37 @@ public class DurationConverterTest {
             .plus(Duration.standardMinutes(30))
             .plus(Duration.standardSeconds(15))
             .plus(Duration.millis(7));
-    DurationTestEntity entity = new DurationTestEntity(testDuration);
-    jpaTm().transact(() -> jpaTm().getEntityManager().persist(entity));
-    DurationTestEntity persisted =
-        jpaTm().transact(() -> jpaTm().getEntityManager().find(DurationTestEntity.class, "id"));
-    assertThat(persisted.duration.getMillis()).isEqualTo(testDuration.getMillis());
+    assertPersistedEntityHasSameDuration(testDuration);
   }
 
   @Test
   void testRoundTripLargeNumberOfDays() {
     Duration testDuration =
         Duration.standardDays(10001).plus(Duration.standardHours(100)).plus(Duration.millis(790));
-    DurationTestEntity entity = new DurationTestEntity(testDuration);
-    jpaTm().transact(() -> jpaTm().getEntityManager().persist(entity));
+    assertPersistedEntityHasSameDuration(testDuration);
+  }
+
+  @Test
+  void testRoundTripLessThanOneDay() {
+    Duration testDuration =
+        Duration.standardHours(15)
+            .plus(Duration.standardMinutes(40))
+            .plus(Duration.standardSeconds(50));
+    assertPersistedEntityHasSameDuration(testDuration);
+  }
+
+  @Test
+  void testRoundTripExactOneDay() {
+    Duration testDuration = Duration.standardDays(1);
+    assertPersistedEntityHasSameDuration(testDuration);
+  }
+
+  private void assertPersistedEntityHasSameDuration(Duration duration) {
+    DurationTestEntity entity = new DurationTestEntity(duration);
+    jpaTm().transact(() -> jpaTm().saveNew(entity));
     DurationTestEntity persisted =
         jpaTm().transact(() -> jpaTm().getEntityManager().find(DurationTestEntity.class, "id"));
-    assertThat(persisted.duration.getMillis()).isEqualTo(testDuration.getMillis());
+    assertThat(persisted.duration.getMillis()).isEqualTo(duration.getMillis());
   }
 
   @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference.

core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRuleTest.java

@@ -20,7 +20,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import google.registry.model.ImmutableObject;
 import google.registry.persistence.transaction.JpaTestRules.JpaUnitTestExtension;
-import google.registry.schema.tmch.ClaimsList;
 import java.util.List;
 import javax.persistence.Entity;
 import javax.persistence.Id;
@@ -36,9 +35,7 @@ public class JpaTransactionManagerRuleTest {
 
   @RegisterExtension
   public final JpaUnitTestExtension jpaExtension =
-      new JpaTestRules.Builder()
-          .withEntityClass(ClaimsList.class, TestEntity.class)
-          .buildUnitTestRule();
+      new JpaTestRules.Builder().withEntityClass(TestEntity.class).buildUnitTestRule();
 
   @Test
   void verifiesRuleWorks() {
@@ -58,7 +55,7 @@ public class JpaTransactionManagerRuleTest {
               List results =
                   jpaTm()
                       .getEntityManager()
-                      .createNativeQuery("SELECT * FROM \"ClaimsList\"")
+                      .createNativeQuery("SELECT * FROM \"TestEntity\"")
                       .getResultList();
               assertThat(results).isEmpty();
             });

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

@@ -21,6 +21,8 @@ import static google.registry.testing.DatastoreHelper.createTld;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import com.google.api.client.http.HttpResponseException;
+import com.google.api.client.http.HttpStatusCodes;
 import com.google.api.client.http.LowLevelHttpRequest;
 import com.google.api.client.http.LowLevelHttpResponse;
 import com.google.api.client.testing.http.MockHttpTransport;
@@ -50,7 +52,8 @@ class IcannHttpReporterTest {
   AppEngineExtension appEngineRule =
       new AppEngineExtension.Builder().withDatastoreAndCloudSql().build();
 
-  private MockHttpTransport createMockTransport(final ByteSource iirdeaResponse) {
+  private MockHttpTransport createMockTransport(
+      int statusCode, final ByteSource iirdeaResponse) {
     return new MockHttpTransport() {
       @Override
       public LowLevelHttpRequest buildRequest(String method, String url) {
@@ -59,7 +62,7 @@ class IcannHttpReporterTest {
               @Override
               public LowLevelHttpResponse execute() throws IOException {
                 MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
-                response.setStatusCode(200);
+                response.setStatusCode(statusCode);
                 response.setContentType(PLAIN_TEXT_UTF_8.toString());
                 response.setContent(iirdeaResponse.read());
                 return response;
@@ -71,6 +74,10 @@ class IcannHttpReporterTest {
     };
   }
 
+  private MockHttpTransport createMockTransport(final ByteSource iirdeaResponse) {
+    return createMockTransport(HttpStatusCodes.STATUS_CODE_OK, iirdeaResponse);
+  }
+
   @BeforeEach
   void beforeEach() {
     createTld("test");
@@ -117,10 +124,21 @@ class IcannHttpReporterTest {
   @Test
   void testFail_BadIirdeaResponse() throws Exception {
     IcannHttpReporter reporter = createReporter();
-    reporter.httpTransport = createMockTransport(IIRDEA_BAD_XML);
+    reporter.httpTransport =
+        createMockTransport(HttpStatusCodes.STATUS_CODE_BAD_REQUEST, IIRDEA_BAD_XML);
     assertThat(reporter.send(FAKE_PAYLOAD, "test-transactions-201706.csv")).isFalse();
   }
 
+  @Test
+  void testFail_transportException() throws Exception {
+    IcannHttpReporter reporter = createReporter();
+    reporter.httpTransport =
+        createMockTransport(HttpStatusCodes.STATUS_CODE_FORBIDDEN, ByteSource.empty());
+    assertThrows(
+        HttpResponseException.class,
+        () -> reporter.send(FAKE_PAYLOAD, "test-transactions-201706.csv"));
+  }
+
   @Test
   void testFail_invalidFilename_nonSixDigitYearMonth() {
     IcannHttpReporter reporter = createReporter();

core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java

@@ -27,6 +27,7 @@ import google.registry.model.poll.PollMessageTest;
 import google.registry.model.registry.RegistryLockDaoTest;
 import google.registry.model.registry.label.ReservedListSqlDaoTest;
 import google.registry.model.reporting.Spec11ThreatMatchTest;
+import google.registry.model.tmch.ClaimsListDaoTest;
 import google.registry.persistence.transaction.JpaEntityCoverageExtension;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
 import google.registry.schema.cursor.CursorDaoTest;
@@ -35,7 +36,6 @@ import google.registry.schema.integration.SqlIntegrationTestSuite.BeforeSuiteTes
 import google.registry.schema.registrar.RegistrarDaoTest;
 import google.registry.schema.server.LockDaoTest;
 import google.registry.schema.tld.PremiumListDaoTest;
-import google.registry.schema.tmch.ClaimsListDaoTest;
 import google.registry.testing.AppEngineExtension;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;

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

@@ -17,6 +17,7 @@ package google.registry.testing;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Fact.fact;
 import static com.google.common.truth.Fact.simpleFact;
+import static com.google.common.truth.OptionalSubject.optionals;
 import static google.registry.model.EppResourceUtils.isActive;
 import static google.registry.testing.DatastoreHelper.getHistoryEntriesOfType;
 import static google.registry.testing.HistoryEntrySubject.historyEntries;
@@ -32,6 +33,7 @@ import google.registry.model.reporting.HistoryEntry;
 import google.registry.testing.TruthChainer.And;
 import google.registry.testing.TruthChainer.Which;
 import java.util.List;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import org.joda.time.DateTime;
 
@@ -190,6 +192,16 @@ abstract class AbstractEppResourceSubject<
     return andChainer();
   }
 
+  protected <E> And<S> hasValue(E expected, Optional<E> actual, String name) {
+    check(name).about(optionals()).that(actual).hasValue(expected);
+    return andChainer();
+  }
+
+  protected <E> And<S> hasNoValue(Optional<E> actual, String name) {
+    check(name).about(optionals()).that(actual).isEmpty();
+    return andChainer();
+  }
+
   protected <E> And<S> doesNotHaveValue(E badValue, E actual, String name) {
     check(name).that(actual).isNotEqualTo(badValue);
     return andChainer();

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

@@ -17,6 +17,7 @@ package google.registry.testing;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Fact.simpleFact;
 import static com.google.common.truth.Truth.assertAbout;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.truth.FailureMetadata;
@@ -102,6 +103,15 @@ public final class DomainBaseSubject
     return hasValue(smdId, actual.getSmdId(), "getSmdId()");
   }
 
+  public And<DomainBaseSubject> hasAutorenewEndTime(DateTime autorenewEndTime) {
+    checkArgumentNotNull(autorenewEndTime, "Use hasNoAutorenewEndTime() instead");
+    return hasValue(autorenewEndTime, actual.getAutorenewEndTime(), "getAutorenewEndTime()");
+  }
+
+  public And<DomainBaseSubject> hasNoAutorenewEndTime() {
+    return hasNoValue(actual.getAutorenewEndTime(), "getAutorenewEndTime()");
+  }
+
   public static SimpleSubjectBuilder<DomainBaseSubject, DomainBase> assertAboutDomains() {
     return assertAbout(DomainBaseSubject::new);
   }

core/src/test/java/google/registry/tmch/TmchDnlActionTest.java

@@ -50,7 +50,8 @@ class TmchDnlActionTest extends TmchActionTestCase {
 
     // Make sure the contents of testdata/dnl-latest.csv got inserted into the database.
     ClaimsListShard claimsList = ClaimsListShard.get();
-    assertThat(claimsList.getCreationTime()).isEqualTo(DateTime.parse("2013-11-24T23:15:37.4Z"));
+    assertThat(claimsList.getTmdbGenerationTime())
+        .isEqualTo(DateTime.parse("2013-11-24T23:15:37.4Z"));
     assertThat(claimsList.getClaimKey("xn----7sbejwbn3axu3d"))
         .hasValue("2013112500/7/4/8/dIHW0DiuybvhdP8kIz");
     assertThat(claimsList.getClaimKey("lolcat")).isEmpty();

core/src/test/java/google/registry/tools/CommandTestCase.java

@@ -19,6 +19,7 @@ import static com.google.common.collect.Iterables.toArray;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.JCommander;
 import com.google.common.base.Joiner;
@@ -41,6 +42,7 @@ import java.io.PrintStream;
 import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.locks.ReentrantLock;
+import org.joda.time.DateTime;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -66,7 +68,7 @@ public abstract class CommandTestCase<C extends Command> {
 
   protected C command;
 
-  public final FakeClock fakeClock = new FakeClock();
+  protected final FakeClock fakeClock = new FakeClock(DateTime.now(UTC));
 
   @RegisterExtension
   public final AppEngineExtension appEngine =

core/src/test/java/google/registry/tools/CountDomainsCommandTest.java

@@ -17,12 +17,9 @@ package google.registry.tools;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
-import static org.joda.time.DateTimeZone.UTC;
 
 import google.registry.model.ofy.Ofy;
-import google.registry.testing.FakeClock;
 import google.registry.testing.InjectExtension;
-import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -30,14 +27,12 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 /** Unit tests for {@link CountDomainsCommand}. */
 public class CountDomainsCommandTest extends CommandTestCase<CountDomainsCommand> {
 
-  protected FakeClock clock = new FakeClock(DateTime.now(UTC));
-
   @RegisterExtension public final InjectExtension inject = new InjectExtension();
 
   @BeforeEach
   final void beforeEach() {
-    inject.setStaticField(Ofy.class, "clock", clock);
-    command.clock = clock;
+    inject.setStaticField(Ofy.class, "clock", fakeClock);
+    command.clock = fakeClock;
     createTlds("foo", "bar", "baz", "qux");
   }
 
@@ -55,13 +50,12 @@ public class CountDomainsCommandTest extends CommandTestCase<CountDomainsCommand
 
   @Test
   void testSuccess_multipleTlds() throws Exception {
-    command.clock = clock;
     for (int i = 0; i < 29; i++) {
       persistActiveDomain(String.format("test-%d.foo", i));
     }
     for (int j = 0; j < 17; j++) {
       persistActiveDomain(String.format("test-%d.baz", j));
-      persistDeletedDomain(String.format("del-%d.foo", j), clock.nowUtc().minusYears(1));
+      persistDeletedDomain(String.format("del-%d.foo", j), fakeClock.nowUtc().minusYears(1));
     }
     persistActiveDomain("not-counted.qux");
     runCommand("--tlds=foo,bar,baz");

core/src/test/java/google/registry/tools/DomainLockUtilsTest.java

@@ -271,6 +271,7 @@ public final class DomainLockUtilsTest {
             .param(
                 RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
                 String.valueOf(lock.getRevisionId()))
+            .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0")
             .etaDelta(
                 standardHours(6).minus(standardSeconds(30)),
                 standardDays(6).plus(standardSeconds(30))));

core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java

@@ -15,27 +15,52 @@
 package google.registry.tools;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
 import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
-import static google.registry.testing.DatastoreHelper.createTlds;
+import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
+import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.newContactResource;
 import static google.registry.testing.DatastoreHelper.newDomainBase;
+import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
 import static google.registry.testing.DatastoreHelper.persistResource;
+import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.beust.jcommander.ParameterException;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import google.registry.model.billing.BillingEvent;
+import google.registry.model.billing.BillingEvent.Flag;
+import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DesignatedContact;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.GracePeriod;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
+import google.registry.model.ofy.Ofy;
+import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import google.registry.testing.InjectExtension;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /** Unit tests for {@link UpdateDomainCommand}. */
 class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand> {
 
+  private DomainBase domain;
+
+  @RegisterExtension public final InjectExtension inject = new InjectExtension();
+
+  @BeforeEach
+  void beforeEach() {
+    inject.setStaticField(Ofy.class, "clock", fakeClock);
+    command.clock = fakeClock;
+    domain = persistActiveDomain("example.tld");
+  }
+
   @Test
   void testSuccess_complete() throws Exception {
     runCommandForced(
@@ -78,6 +103,8 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
 
   @Test
   void testSuccess_multipleDomains() throws Exception {
+    createTld("abc");
+    persistActiveDomain("example.abc");
     runCommandForced(
         "--client=NewRegistrar",
         "--add_nameservers=ns1.zdns.google,ns2.zdns.google",
@@ -110,7 +137,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
   }
 
   private void runTest_multipleDomains_setNameservers(String nsParam) throws Exception {
-    createTlds("abc", "tld");
+    createTld("abc");
     HostResource host1 = persistActiveHost("foo.bar.tld");
     HostResource host2 = persistActiveHost("baz.bar.tld");
     persistResource(
@@ -254,6 +281,59 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
     eppVerifier.verifySent("domain_update_clear_ds_records.xml");
   }
 
+  @Test
+  void testSuccess_enableAutorenew() throws Exception {
+    runCommandForced("--client=NewRegistrar", "--autorenews=true", "example.tld");
+    eppVerifier.verifySent(
+        "domain_update_set_autorenew.xml", ImmutableMap.of("AUTORENEWS", "true"));
+  }
+
+  @Test
+  void testSuccess_disableAutorenew() throws Exception {
+    runCommandForced("--client=NewRegistrar", "--autorenews=false", "example.tld");
+    eppVerifier.verifySent(
+        "domain_update_set_autorenew.xml", ImmutableMap.of("AUTORENEWS", "false"));
+    assertThat(getStderrAsString()).doesNotContain("autorenew grace period");
+  }
+
+  @Test
+  void testSuccess_disableAutorenew_inAutorenewGracePeriod() throws Exception {
+    HistoryEntry createHistoryEntry =
+        persistResource(
+            new HistoryEntry.Builder().setType(DOMAIN_CREATE).setParent(domain).build());
+    BillingEvent.Recurring autorenewBillingEvent =
+        persistResource(
+            new BillingEvent.Recurring.Builder()
+                .setReason(Reason.RENEW)
+                .setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
+                .setTargetId("example.tld")
+                .setClientId("NewRegistrar")
+                .setEventTime(fakeClock.nowUtc().minusDays(5))
+                .setRecurrenceEndTime(END_OF_TIME)
+                .setParent(createHistoryEntry)
+                .build());
+    persistResource(
+        domain
+            .asBuilder()
+            .setRegistrationExpirationTime(fakeClock.nowUtc().plusDays(360))
+            .setAutorenewBillingEvent(autorenewBillingEvent.createVKey())
+            .setGracePeriods(
+                ImmutableSet.of(
+                    GracePeriod.createForRecurring(
+                        AUTO_RENEW,
+                        domain.getRepoId(),
+                        fakeClock.nowUtc().plusDays(40),
+                        "NewRegistrar",
+                        autorenewBillingEvent.createVKey())))
+            .build());
+    runCommandForced("--client=NewRegistrar", "--autorenews=false", "example.tld");
+    eppVerifier.verifySent(
+        "domain_update_set_autorenew.xml", ImmutableMap.of("AUTORENEWS", "false"));
+    String stdErr = getStderrAsString();
+    assertThat(stdErr).contains("The following domains are in autorenew grace periods.");
+    assertThat(stdErr).contains("example.tld");
+  }
+
   @Test
   void testFailure_cantUpdateRegistryLockedDomainEvenAsSuperuser() {
     HostResource host = persistActiveHost("ns1.zdns.google");

core/src/test/java/google/registry/tools/UploadClaimsListCommandTest.java

@@ -37,7 +37,8 @@ class UploadClaimsListCommandTest extends CommandTestCase<UploadClaimsListComman
     runCommand("--force", filename);
 
     ClaimsListShard claimsList = ClaimsListShard.get();
-    assertThat(claimsList.getCreationTime()).isEqualTo(DateTime.parse("2012-08-16T00:00:00.0Z"));
+    assertThat(claimsList.getTmdbGenerationTime())
+        .isEqualTo(DateTime.parse("2012-08-16T00:00:00.0Z"));
     assertThat(claimsList.getClaimKey("example"))
         .hasValue("2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001");
     assertThat(claimsList.getClaimKey("another-example"))

core/src/test/java/google/registry/tools/javascrap/BackfillRegistryLocksCommandTest.java

@@ -15,6 +15,7 @@
 package google.registry.tools.javascrap;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
@@ -144,15 +145,15 @@ class BackfillRegistryLocksCommandTest extends CommandTestCase<BackfillRegistryL
         "--domain_roids", String.format("%s,%s", ursDomain.getRepoId(), nonUrsDomain.getRepoId()));
 
     RegistryLock ursLock = getMostRecentVerifiedRegistryLockByRepoId(ursDomain.getRepoId()).get();
-    assertThat(ursLock.getLockCompletionTimestamp().get()).isEqualTo(ursTime);
+    assertThat(ursLock.getLockCompletionTimestamp()).hasValue(ursTime);
     RegistryLock nonUrsLock =
         getMostRecentVerifiedRegistryLockByRepoId(nonUrsDomain.getRepoId()).get();
-    assertThat(nonUrsLock.getLockCompletionTimestamp().get()).isEqualTo(fakeClock.nowUtc());
+    assertThat(nonUrsLock.getLockCompletionTimestamp()).hasValue(fakeClock.nowUtc());
   }
 
   @Test
   void testFailure_mustProvideDomainRoids() {
-    assertThat(assertThrows(IllegalArgumentException.class, () -> runCommandForced()))
+    assertThat(assertThrows(IllegalArgumentException.class, this::runCommandForced))
         .hasMessageThat()
         .isEqualTo("Must provide non-empty domain_roids argument");
   }

core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java

@@ -41,7 +41,7 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase {
   @RetryingTest(3)
   void get_owner_fails() throws Throwable {
     driver.get(server.getUrl("/registrar-ote-setup"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("unauthorized");
   }
 
@@ -49,14 +49,14 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase {
   void get_admin_succeeds() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar-ote-setup"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("formEmpty");
     driver.findElement(By.id("clientId")).sendKeys("acmereg");
     driver.findElement(By.id("email")).sendKeys("acmereg@registry.example");
     driver.findElement(By.id("password")).sendKeys("StRoNgPaSsWoRd");
     driver.diffPage("formFilled");
     driver.findElement(By.id("submit-button")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("oteResult");
   }
 
@@ -64,11 +64,11 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase {
   void get_admin_fails_badEmail() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar-ote-setup"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.findElement(By.id("clientId")).sendKeys("acmereg");
     driver.findElement(By.id("email")).sendKeys("bad email");
     driver.findElement(By.id("submit-button")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("oteResultFailed");
   }
 }

core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java

@@ -68,7 +68,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   @RetryingTest(3)
   void index_owner() throws Throwable {
     driver.get(server.getUrl("/registrar"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -77,7 +77,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void index_adminAndOwner() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -88,22 +88,21 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     // To make sure we're only ADMIN (and not also "OWNER"), we switch to the NewRegistrar we
     // aren't in the contacts of
     driver.get(server.getUrl("/registrar?clientId=NewRegistrar"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
   @RetryingTest(3)
   void contactUs() throws Throwable {
     driver.get(server.getUrl("/registrar#contact-us"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
   @RetryingTest(3)
   void settingsContact() throws Throwable {
     driver.get(server.getUrl("/registrar#contact-settings"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -112,16 +111,14 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsContact_asAdmin() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar?clientId=NewRegistrar#contact-settings"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
   @RetryingTest(3)
   void settingsContactItem() throws Throwable {
     driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -132,8 +129,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     driver.get(
         server.getUrl(
             "/registrar?clientId=NewRegistrar#contact-settings/janedoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -141,9 +137,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsContactEdit() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     driver.diffPage("page");
   }
 
@@ -162,9 +157,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
         });
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     // The password should show as dots when the user types it in
     driver.findElement(By.id("contacts[1].registryLockPassword")).sendKeys("password");
     driver.diffPage("page_with_password");
@@ -179,9 +173,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     Thread.sleep(5);
     driver.diffPage("page_with_password_after_hide");
 
-    // now actually set the password
-    driver.waitForElement(By.id("reg-app-btn-save")).click();
-    Thread.sleep(500);
+    // Now click the Save button and wait for another Edit button to show up
+    driver.waitForRefreshedElementAfterAction(
+        () -> driver.waitForDisplayedElement(By.id("reg-app-btn-save")).click(),
+        By.id("reg-app-btn-edit"));
     driver.diffPage("contact_view");
 
     server.runInAppEngineEnvironment(
@@ -213,9 +208,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
         });
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     driver.diffPage("page");
   }
 
@@ -225,9 +219,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
         () -> persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(true).build()));
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     driver.diffPage("page");
   }
 
@@ -235,9 +228,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsContactAdd() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#contact-settings"));
-    Thread.sleep(1000);
-    driver.waitForElement(By.tagName("h1"));
-    driver.waitForElement(By.id("reg-app-btn-add")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-add")).click();
     // Attempt to fix flaky tests. The going theory is that the click button CSS animation needs to
     // finish before the screenshot is captured.
     Thread.sleep(250);
@@ -251,10 +243,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     // To make sure we're only ADMIN (and not also "OWNER"), we switch to the NewRegistrar we
     // aren't in the contacts of
     driver.get(server.getUrl("/registrar?clientId=NewRegistrar#admin-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("edit");
   }
 
@@ -272,7 +264,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsAdmin_whenNotAdmin_showsHome() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#admin-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
   }
 
@@ -280,7 +272,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void getOteStatus_noButtonWhenReal() throws Exception {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar#admin-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("result");
   }
 
@@ -299,7 +291,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void getOteStatus_completed() throws Exception {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar?clientId=otefinished-1#admin-settings"));
-    driver.waitForElement(By.id("btn-ote-status"));
+    driver.waitForDisplayedElement(By.id("btn-ote-status"));
     driver.diffPage("before_click");
     driver.findElement(By.id("btn-ote-status")).click();
     driver.findElement(By.id("ote-results-table")).click();
@@ -312,10 +304,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsSecurity() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#security-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("edit");
   }
 
@@ -325,7 +317,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     server.setIsAdmin(true);
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar?clientId=NewRegistrar#security-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
   }
 
@@ -343,10 +335,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
         });
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#security-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("edit");
   }
 
@@ -363,10 +355,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
         });
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#security-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("edit");
   }
 
@@ -377,14 +369,14 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
             persistResource(
                 loadRegistrar("TheRegistrar").asBuilder().setState(State.DISABLED).build()));
     driver.get(server.getUrl("/registrar"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("view");
   }
 
   @RetryingTest(3)
   void settingsWhois() throws Throwable {
     driver.get(server.getUrl("/registrar#whois-settings"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("page");
   }
 
@@ -392,8 +384,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsWhoisEdit() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#whois-settings"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
-    Thread.sleep(1000);
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.id("reg-app-btn-save"));
     driver.diffPage("page");
   }
 
@@ -401,10 +393,12 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void settingsWhoisEditError() throws Throwable {
     driver.manage().window().setSize(new Dimension(1050, 2000));
     driver.get(server.getUrl("/registrar#whois-settings"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     driver.setFormFieldsById(ImmutableMap.of("faxNumber", "cat"));
-    driver.waitForElement(By.id("reg-app-btn-save")).click();
-    Thread.sleep(1000);
+    driver.waitForDisplayedElement(By.id("reg-app-btn-save")).click();
+    // After the click, a div element without id would show up with an error message.
+    driver.waitForElementWithCondition(
+        By.tagName("div"), e -> e.getText().startsWith("Must be a valid +E.164 phone number,"));
     driver.diffPage("page");
   }
 
@@ -412,7 +406,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
   void indexPage_smallScrolledDown() throws Throwable {
     driver.manage().window().setSize(new Dimension(600, 300));
     driver.get(server.getUrl("/registrar"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.executeScript("document.getElementById('reg-content-and-footer').scrollTop = 200");
     Thread.sleep(500);
     driver.diffPage("page");
@@ -439,21 +433,21 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
     driver.get(
         server.getUrl(
             "/registry-lock-verify?isLock=true&lockVerificationCode=" + lockVerificationCode));
-    driver.waitForElement(By.id("reg-content"));
+    driver.waitForDisplayedElement(By.id("reg-content"));
     driver.diffPage("page");
   }
 
   @RetryingTest(3)
   void registryLockVerify_unknownLock() throws Throwable {
     driver.get(server.getUrl("/registry-lock-verify?isLock=true&lockVerificationCode=asdfasdf"));
-    driver.waitForElement(By.id("reg-content"));
+    driver.waitForDisplayedElement(By.id("reg-content"));
     driver.diffPage("page");
   }
 
   @RetryingTest(3)
   void registryLock_empty() throws Throwable {
     driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.diffPage("page");
   }
 
@@ -465,7 +459,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.diffPage("page");
   }
 
@@ -477,7 +471,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.diffPage("page");
   }
 
@@ -530,7 +524,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.diffPage("page");
   }
 
@@ -542,9 +536,9 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.findElement(By.id("button-unlock-example.tld")).click();
-    driver.waitForElement(By.className("modal-content"));
+    driver.waitForDisplayedElement(By.className("modal-content"));
     driver.findElement(By.id("domain-lock-password")).sendKeys("password");
     driver.diffPage("page");
   }
@@ -559,9 +553,9 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.findElement(By.id("button-lock-domain")).click();
-    driver.waitForElement(By.className("modal-content"));
+    driver.waitForDisplayedElement(By.className("modal-content"));
     driver.findElement(By.id("domain-lock-input-value")).sendKeys("somedomain.tld");
     driver.diffPage("page");
   }
@@ -578,7 +572,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           return null;
         });
     driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock"));
-    driver.waitForElement(By.tagName("h2"));
+    driver.waitForDisplayedElement(By.tagName("h2"));
     driver.diffPage("page");
   }
 

core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java

@@ -55,7 +55,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
 
   /** Checks that an element is visible. */
   void assertEltVisible(String eltId) throws Throwable {
-    assertThat(driver.waitForElement(By.id(eltId)).isDisplayed()).isTrue();
+    assertThat(driver.waitForDisplayedElement(By.id(eltId)).isDisplayed()).isTrue();
   }
 
   /** Checks that an element is invisible. */
@@ -141,7 +141,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
   @RetryingTest(3)
   void testWhoisSettingsEdit() throws Throwable {
     driver.get(server.getUrl("/registrar#whois-settings"));
-    driver.waitForElement(By.id("reg-app-btn-edit")).click();
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
     driver.setFormFieldsById(
         new ImmutableMap.Builder<String, String>()
             .put("emailAddress", "test1@example.com")
@@ -178,7 +178,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
   @RetryingTest(3)
   void testContactSettingsView() throws Throwable {
     driver.get(server.getUrl("/registrar#contact-settings"));
-    driver.waitForElement(By.id("reg-app-btn-add"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-add"));
     ImmutableList<RegistrarContact> contacts =
         server.runInAppEngineEnvironment(
             () -> loadRegistrar("TheRegistrar").getContacts().asList());
@@ -192,7 +192,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
   @RetryingTest(3)
   void testSecuritySettingsView() throws Throwable {
     driver.get(server.getUrl("/registrar#security-settings"));
-    driver.waitForElement(By.id("reg-app-btn-edit"));
+    driver.waitForDisplayedElement(By.id("reg-app-btn-edit"));
     Registrar registrar = server.runInAppEngineEnvironment(() -> loadRegistrar("TheRegistrar"));
     assertThat(driver.findElement(By.id("phonePasscode")).getAttribute("value"))
         .isEqualTo(registrar.getPhonePasscode());

core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java

@@ -41,7 +41,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
   @RetryingTest(3)
   void get_owner_fails() throws Throwable {
     driver.get(server.getUrl("/registrar-create"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("unauthorized");
   }
 
@@ -49,7 +49,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
   void get_admin_succeeds() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar-create"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("formEmpty");
     driver.findElement(By.id("clientId")).sendKeys("my-name");
     driver.findElement(By.id("name")).sendKeys("registrar name");
@@ -69,7 +69,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
     driver.findElement(By.id("passcode")).sendKeys("01234");
     driver.diffPage("formFilled");
     driver.findElement(By.id("submit-button")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("createResult");
   }
 
@@ -77,7 +77,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
   void get_admin_fails_badEmail() throws Throwable {
     server.setIsAdmin(true);
     driver.get(server.getUrl("/registrar-create"));
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.findElement(By.id("clientId")).sendKeys("my-name");
     driver.findElement(By.id("name")).sendKeys("registrar name");
     driver
@@ -93,7 +93,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
     driver.findElement(By.id("city")).sendKeys("Citysville");
     driver.findElement(By.id("countryCode")).sendKeys("fr");
     driver.findElement(By.id("submit-button")).click();
-    driver.waitForElement(By.tagName("h1"));
+    driver.waitForDisplayedElement(By.tagName("h1"));
     driver.diffPage("createResultFailed");
   }
 }

core/src/test/java/google/registry/webdriver/WebDriverPlusScreenDifferExtension.java

@@ -19,9 +19,11 @@ import static java.util.stream.Collectors.joining;
 import static org.apache.commons.text.StringEscapeUtils.escapeEcmaScript;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicates;
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
@@ -34,6 +36,7 @@ import org.openqa.selenium.Dimension;
 import org.openqa.selenium.HasCapabilities;
 import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.OutputType;
+import org.openqa.selenium.StaleElementReferenceException;
 import org.openqa.selenium.TakesScreenshot;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebDriverException;
@@ -55,7 +58,6 @@ public final class WebDriverPlusScreenDifferExtension
         HasCapabilities {
 
   private static final int WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS = 10;
-  private static final int WAIT_FOR_ELEMENTS_BONUS_DELAY_MS = 150;
 
   // The maximum difference between pixels that would be considered as "identical". Calculated as
   // the sum of the absolute difference between the values of the RGB channels. So a 120,30,200
@@ -121,28 +123,40 @@ public final class WebDriverPlusScreenDifferExtension
     driver.get(url.toString());
   }
 
-  /** Waits indefinitely for an element to appear on the page, then returns it. */
-  WebElement waitForElement(By by) throws InterruptedException {
-    while (true) {
-      List<WebElement> elements = findElements(by);
-      if (!elements.isEmpty()) {
-        Thread.sleep(WAIT_FOR_ELEMENTS_BONUS_DELAY_MS);
-        return elements.get(0);
-      }
-      Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS);
-    }
+  /** Waits indefinitely for a <em>visible</em> element matching {@code by}, then returns it. */
+  WebElement waitForDisplayedElement(By by) throws InterruptedException {
+    return waitForElementWithCondition(by, WebElement::isDisplayed);
   }
 
-  /** Waits for element matching {@code by} whose {@code attribute} satisfies {@code predicate}. */
-  public WebElement waitForAttribute(
-      By by, String attribute, Predicate</*@Nullable*/ ? super CharSequence> predicate)
+  /** Waits indefinitely for an element matching {@code by}, then returns it. */
+  WebElement waitForElement(By by) throws InterruptedException {
+    return waitForElementWithCondition(by, Predicates.alwaysTrue());
+  }
+
+  /**
+   * Executes an action and waits indefinitely for the replacement of an <em>existing</em> element.
+   */
+  WebElement waitForRefreshedElementAfterAction(ThrowingRunnable action, By by) throws Exception {
+    WebElement element = findElement(by);
+    action.run();
+    while (true) {
+      try {
+        element.isDisplayed(); // Eventually triggers StaleElementReferenceException
+        Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS);
+      } catch (StaleElementReferenceException e) {
+        break;
+      }
+    }
+    return waitForDisplayedElement(by);
+  }
+
+  /** Waits for an element matching {@code by} and satisfying {@code predicate}, then returns it. */
+  WebElement waitForElementWithCondition(By by, Predicate<WebElement> predicate)
       throws InterruptedException {
     while (true) {
-      for (WebElement element : findElements(by)) {
-        if (predicate.test(element.getAttribute(attribute))) {
-          Thread.sleep(WAIT_FOR_ELEMENTS_BONUS_DELAY_MS);
-          return element;
-        }
+      Optional<WebElement> firstMatch = findElements(by).stream().filter(predicate).findFirst();
+      if (firstMatch.isPresent()) {
+        return firstMatch.get();
       }
       Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS);
     }
@@ -303,4 +317,9 @@ public final class WebDriverPlusScreenDifferExtension
     Preconditions.checkNotNull(imageNamePrefix);
     return imageNamePrefix + "_" + imageKey;
   }
+
+  public interface ThrowingRunnable {
+
+    void run() throws Exception;
+  }
 }

core/src/test/resources/google/registry/flows/domain/domain_update_superuser_extension.xml

@@ -0,0 +1,17 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <update>
+      <domain:update
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+        <domain:chg/>
+      </domain:update>
+    </update>
+    <extension>
+      <superuser:domainUpdate xmlns:superuser="urn:google:params:xml:ns:superuser-1.0">
+        <superuser:autorenews>%AUTORENEWS%</superuser:autorenews>
+      </superuser:domainUpdate>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>

core/src/test/resources/google/registry/model/schema.txt

@@ -99,6 +99,46 @@ class google.registry.model.contact.ContactAddress {
 class google.registry.model.contact.ContactAuthInfo {
   google.registry.model.eppcommon.AuthInfo$PasswordAuth pw;
 }
+class google.registry.model.contact.ContactBase {
+  @Id java.lang.String repoId;
+  com.google.common.collect.ImmutableSortedMap<org.joda.time.DateTime, com.googlecode.objectify.Key<google.registry.model.ofy.CommitLogManifest>> revisions;
+  google.registry.model.CreateAutoTimestamp creationTime;
+  google.registry.model.UpdateAutoTimestamp updateTimestamp;
+  google.registry.model.contact.ContactAuthInfo authInfo;
+  google.registry.model.contact.ContactPhoneNumber fax;
+  google.registry.model.contact.ContactPhoneNumber voice;
+  google.registry.model.contact.Disclose disclose;
+  google.registry.model.contact.PostalInfo internationalizedPostalInfo;
+  google.registry.model.contact.PostalInfo localizedPostalInfo;
+  google.registry.model.transfer.ContactTransferData transferData;
+  java.lang.String contactId;
+  java.lang.String creationClientId;
+  java.lang.String currentSponsorClientId;
+  java.lang.String email;
+  java.lang.String lastEppUpdateClientId;
+  java.lang.String searchName;
+  java.util.Set<google.registry.model.eppcommon.StatusValue> status;
+  org.joda.time.DateTime deletionTime;
+  org.joda.time.DateTime lastEppUpdateTime;
+  org.joda.time.DateTime lastTransferTime;
+}
+class google.registry.model.contact.ContactHistory {
+  @Id java.lang.Long id;
+  @Parent com.googlecode.objectify.Key<? extends google.registry.model.EppResource> parent;
+  boolean bySuperuser;
+  byte[] xmlBytes;
+  google.registry.model.contact.ContactBase contactBase;
+  google.registry.model.domain.Period period;
+  google.registry.model.eppcommon.Trid trid;
+  google.registry.model.reporting.HistoryEntry$Type type;
+  google.registry.persistence.VKey<google.registry.model.contact.ContactResource> contactRepoId;
+  java.lang.Boolean requestedByRegistrar;
+  java.lang.String clientId;
+  java.lang.String otherClientId;
+  java.lang.String reason;
+  java.util.Set<google.registry.model.reporting.DomainTransactionRecord> domainTransactionRecords;
+  org.joda.time.DateTime modificationTime;
+}
 class google.registry.model.contact.ContactPhoneNumber {
   java.lang.String extension;
   java.lang.String phoneNumber;
@@ -191,6 +231,53 @@ class google.registry.model.domain.DomainBase {
   org.joda.time.DateTime lastTransferTime;
   org.joda.time.DateTime registrationExpirationTime;
 }
+class google.registry.model.domain.DomainContent {
+  @Id java.lang.String repoId;
+  com.google.common.collect.ImmutableSortedMap<org.joda.time.DateTime, com.googlecode.objectify.Key<google.registry.model.ofy.CommitLogManifest>> revisions;
+  google.registry.model.CreateAutoTimestamp creationTime;
+  google.registry.model.UpdateAutoTimestamp updateTimestamp;
+  google.registry.model.domain.DomainAuthInfo authInfo;
+  google.registry.model.domain.launch.LaunchNotice launchNotice;
+  google.registry.model.transfer.DomainTransferData transferData;
+  google.registry.persistence.VKey<google.registry.model.billing.BillingEvent$Recurring> autorenewBillingEvent;
+  google.registry.persistence.VKey<google.registry.model.poll.PollMessage$Autorenew> autorenewPollMessage;
+  google.registry.persistence.VKey<google.registry.model.poll.PollMessage$OneTime> deletePollMessage;
+  java.lang.String creationClientId;
+  java.lang.String currentSponsorClientId;
+  java.lang.String fullyQualifiedDomainName;
+  java.lang.String idnTableName;
+  java.lang.String lastEppUpdateClientId;
+  java.lang.String smdId;
+  java.lang.String tld;
+  java.util.Set<google.registry.model.domain.DesignatedContact> allContacts;
+  java.util.Set<google.registry.model.domain.GracePeriod> gracePeriods;
+  java.util.Set<google.registry.model.domain.secdns.DelegationSignerData> dsData;
+  java.util.Set<google.registry.model.eppcommon.StatusValue> status;
+  java.util.Set<google.registry.persistence.VKey<google.registry.model.host.HostResource>> nsHosts;
+  java.util.Set<java.lang.String> subordinateHosts;
+  org.joda.time.DateTime autorenewEndTime;
+  org.joda.time.DateTime deletionTime;
+  org.joda.time.DateTime lastEppUpdateTime;
+  org.joda.time.DateTime lastTransferTime;
+  org.joda.time.DateTime registrationExpirationTime;
+}
+class google.registry.model.domain.DomainHistory {
+  @Id java.lang.Long id;
+  @Parent com.googlecode.objectify.Key<? extends google.registry.model.EppResource> parent;
+  boolean bySuperuser;
+  byte[] xmlBytes;
+  google.registry.model.domain.DomainContent domainContent;
+  google.registry.model.domain.Period period;
+  google.registry.model.eppcommon.Trid trid;
+  google.registry.model.reporting.HistoryEntry$Type type;
+  java.lang.Boolean requestedByRegistrar;
+  java.lang.String clientId;
+  java.lang.String domainRepoId;
+  java.lang.String otherClientId;
+  java.lang.String reason;
+  java.util.Set<google.registry.model.reporting.DomainTransactionRecord> domainTransactionRecords;
+  org.joda.time.DateTime modificationTime;
+}
 class google.registry.model.domain.GracePeriod {
   google.registry.model.domain.rgp.GracePeriodStatus type;
   google.registry.persistence.VKey<google.registry.model.billing.BillingEvent$OneTime> billingEventOneTime;
@@ -288,6 +375,40 @@ class google.registry.model.eppcommon.Trid {
   java.lang.String clientTransactionId;
   java.lang.String serverTransactionId;
 }
+class google.registry.model.host.HostBase {
+  @Id java.lang.String repoId;
+  com.google.common.collect.ImmutableSortedMap<org.joda.time.DateTime, com.googlecode.objectify.Key<google.registry.model.ofy.CommitLogManifest>> revisions;
+  google.registry.model.CreateAutoTimestamp creationTime;
+  google.registry.model.UpdateAutoTimestamp updateTimestamp;
+  google.registry.persistence.VKey<google.registry.model.domain.DomainBase> superordinateDomain;
+  java.lang.String creationClientId;
+  java.lang.String currentSponsorClientId;
+  java.lang.String fullyQualifiedHostName;
+  java.lang.String lastEppUpdateClientId;
+  java.util.Set<google.registry.model.eppcommon.StatusValue> status;
+  java.util.Set<java.net.InetAddress> inetAddresses;
+  org.joda.time.DateTime deletionTime;
+  org.joda.time.DateTime lastEppUpdateTime;
+  org.joda.time.DateTime lastSuperordinateChange;
+  org.joda.time.DateTime lastTransferTime;
+}
+class google.registry.model.host.HostHistory {
+  @Id java.lang.Long id;
+  @Parent com.googlecode.objectify.Key<? extends google.registry.model.EppResource> parent;
+  boolean bySuperuser;
+  byte[] xmlBytes;
+  google.registry.model.domain.Period period;
+  google.registry.model.eppcommon.Trid trid;
+  google.registry.model.host.HostBase hostBase;
+  google.registry.model.reporting.HistoryEntry$Type type;
+  google.registry.persistence.VKey<google.registry.model.host.HostResource> hostRepoId;
+  java.lang.Boolean requestedByRegistrar;
+  java.lang.String clientId;
+  java.lang.String otherClientId;
+  java.lang.String reason;
+  java.util.Set<google.registry.model.reporting.DomainTransactionRecord> domainTransactionRecords;
+  org.joda.time.DateTime modificationTime;
+}
 class google.registry.model.host.HostResource {
   @Id java.lang.String repoId;
   com.google.common.collect.ImmutableSortedMap<org.joda.time.DateTime, com.googlecode.objectify.Key<google.registry.model.ofy.CommitLogManifest>> revisions;
@@ -766,4 +887,4 @@ enum google.registry.model.transfer.TransferStatus {
   PENDING;
   SERVER_APPROVED;
   SERVER_CANCELLED;
-}
+}

core/src/test/resources/google/registry/tools/server/domain_update_set_autorenew.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <update>
+      <domain:update
+       xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+      </domain:update>
+    </update>
+    <extension>
+      <superuser:domainUpdate xmlns:superuser="urn:google:params:xml:ns:superuser-1.0">
+        <superuser:autorenews>%AUTORENEWS%</superuser:autorenews>
+      </superuser:domainUpdate>
+    </extension>
+    <clTRID>RegistryTool</clTRID>
+  </command>
+</epp>

db/src/main/resources/sql/flyway/V51__use_composite_primary_key_for_domain_history_table.sql

@@ -0,0 +1,34 @@
+-- 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.
+
+alter sequence "history_id_sequence" increment 50;
+
+alter table "DomainHistory" alter column history_revision_id drop default;
+alter table "ContactHistory" alter column history_revision_id drop default;
+alter table "HostHistory" alter column history_revision_id drop default;
+
+alter table if exists "DomainHistoryHost"
+    drop constraint fk6b8eqdxwe3guc56tgpm89atx;
+
+alter table "DomainHistory" drop constraint "DomainHistory_pkey";
+
+alter table "DomainHistory"
+    add constraint "DomainHistory_pkey" primary key (domain_repo_id, history_revision_id);
+
+alter table "DomainHistoryHost" add column domain_history_domain_repo_id text not null;
+
+alter table if exists "DomainHistoryHost"
+    add constraint FKa9woh3hu8gx5x0vly6bai327n
+    foreign key (domain_history_domain_repo_id, domain_history_history_revision_id)
+    references "DomainHistory";

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -11,7 +11,7 @@
 -- 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.
-create sequence history_id_sequence start 1 increment 1;
+create sequence history_id_sequence start 1 increment 50;
 
     create table "AllocationToken" (
        token text not null,
@@ -86,8 +86,8 @@ create sequence history_id_sequence start 1 increment 1;
 
     create table "ClaimsList" (
        revision_id  bigserial not null,
-        creation_timestamp timestamptz not null,
         tmdb_generation_time timestamptz not null,
+        creation_timestamp timestamptz not null,
         primary key (revision_id)
     );
 
@@ -286,7 +286,8 @@ create sequence history_id_sequence start 1 increment 1;
     );
 
     create table "DomainHistory" (
-       history_revision_id int8 not null,
+       domain_repo_id text not null,
+        history_revision_id int8 not null,
         history_by_superuser boolean not null,
         history_registrar_id text,
         history_modification_time timestamptz not null,
@@ -341,12 +342,12 @@ create sequence history_id_sequence start 1 increment 1;
         last_epp_update_time timestamptz,
         statuses text[],
         update_timestamp timestamptz,
-        domain_repo_id text not null,
-        primary key (history_revision_id)
+        primary key (domain_repo_id, history_revision_id)
     );
 
     create table "DomainHistoryHost" (
-       domain_history_history_revision_id int8 not null,
+       domain_history_domain_repo_id text not null,
+        domain_history_history_revision_id int8 not null,
         host_repo_id text
     );
 
@@ -644,8 +645,8 @@ create index spec11threatmatch_check_date_idx on "Spec11ThreatMatch" (check_date
        references "ClaimsList";
 
     alter table if exists "DomainHistoryHost" 
-       add constraint FK378h8v3j8qd8xtjn2e0bcmrtj 
-       foreign key (domain_history_history_revision_id) 
+       add constraint FKa9woh3hu8gx5x0vly6bai327n 
+       foreign key (domain_history_domain_repo_id, domain_history_history_revision_id) 
        references "DomainHistory";
 
     alter table if exists "DomainHost" 

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -275,24 +275,12 @@ CREATE TABLE public."Contact" (
 );
 
 
---
--- Name: history_id_sequence; Type: SEQUENCE; Schema: public; Owner: -
---
-
-CREATE SEQUENCE public.history_id_sequence
-    START WITH 1
-    INCREMENT BY 1
-    NO MINVALUE
-    NO MAXVALUE
-    CACHE 1;
-
-
 --
 -- Name: ContactHistory; Type: TABLE; Schema: public; Owner: -
 --
 
 CREATE TABLE public."ContactHistory" (
-    history_revision_id bigint DEFAULT nextval('public.history_id_sequence'::regclass) NOT NULL,
+    history_revision_id bigint NOT NULL,
     history_by_superuser boolean NOT NULL,
     history_registrar_id text,
     history_modification_time timestamp with time zone NOT NULL,
@@ -431,7 +419,7 @@ CREATE TABLE public."Domain" (
 --
 
 CREATE TABLE public."DomainHistory" (
-    history_revision_id bigint DEFAULT nextval('public.history_id_sequence'::regclass) NOT NULL,
+    history_revision_id bigint NOT NULL,
     history_by_superuser boolean NOT NULL,
     history_registrar_id text,
     history_modification_time timestamp with time zone NOT NULL,
@@ -496,7 +484,8 @@ CREATE TABLE public."DomainHistory" (
 
 CREATE TABLE public."DomainHistoryHost" (
     domain_history_history_revision_id bigint NOT NULL,
-    host_repo_id text
+    host_repo_id text,
+    domain_history_domain_repo_id text NOT NULL
 );
 
 
@@ -549,7 +538,7 @@ ALTER SEQUENCE public."GracePeriod_id_seq" OWNED BY public."GracePeriod".id;
 --
 
 CREATE TABLE public."HostHistory" (
-    history_revision_id bigint DEFAULT nextval('public.history_id_sequence'::regclass) NOT NULL,
+    history_revision_id bigint NOT NULL,
     history_by_superuser boolean NOT NULL,
     history_registrar_id text NOT NULL,
     history_modification_time timestamp with time zone NOT NULL,
@@ -929,6 +918,18 @@ CREATE SEQUENCE public."Transaction_id_seq"
 ALTER SEQUENCE public."Transaction_id_seq" OWNED BY public."Transaction".id;
 
 
+--
+-- Name: history_id_sequence; Type: SEQUENCE; Schema: public; Owner: -
+--
+
+CREATE SEQUENCE public.history_id_sequence
+    START WITH 1
+    INCREMENT BY 50
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+
 --
 -- Name: BillingCancellation billing_cancellation_id; Type: DEFAULT; Schema: public; Owner: -
 --
@@ -1083,7 +1084,7 @@ ALTER TABLE ONLY public."Cursor"
 --
 
 ALTER TABLE ONLY public."DomainHistory"
-    ADD CONSTRAINT "DomainHistory_pkey" PRIMARY KEY (history_revision_id);
+    ADD CONSTRAINT "DomainHistory_pkey" PRIMARY KEY (domain_repo_id, history_revision_id);
 
 
 --
@@ -1612,14 +1613,6 @@ ALTER TABLE ONLY public."HostHistory"
     ADD CONSTRAINT fk3d09knnmxrt6iniwnp8j2ykga FOREIGN KEY (history_registrar_id) REFERENCES public."Registrar"(registrar_id);
 
 
---
--- Name: DomainHistoryHost fk6b8eqdxwe3guc56tgpm89atx; Type: FK CONSTRAINT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public."DomainHistoryHost"
-    ADD CONSTRAINT fk6b8eqdxwe3guc56tgpm89atx FOREIGN KEY (domain_history_history_revision_id) REFERENCES public."DomainHistory"(history_revision_id);
-
-
 --
 -- Name: ClaimsEntry fk6sc6at5hedffc0nhdcab6ivuq; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -1916,6 +1909,14 @@ ALTER TABLE ONLY public."PollMessage"
     ADD CONSTRAINT fk_poll_message_transfer_response_losing_registrar_id FOREIGN KEY (transfer_response_losing_registrar_id) REFERENCES public."Registrar"(registrar_id);
 
 
+--
+-- Name: DomainHistoryHost fka9woh3hu8gx5x0vly6bai327n; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."DomainHistoryHost"
+    ADD CONSTRAINT fka9woh3hu8gx5x0vly6bai327n FOREIGN KEY (domain_history_domain_repo_id, domain_history_history_revision_id) REFERENCES public."DomainHistory"(domain_repo_id, history_revision_id);
+
+
 --
 -- Name: DomainHost fkfmi7bdink53swivs390m2btxg; Type: FK CONSTRAINT; Schema: public; Owner: -
 --

dependencies.gradle

@@ -131,7 +131,7 @@ ext {
       'org.bouncycastle:bcpg-jdk15on:1.61',
       'org.bouncycastle:bcpkix-jdk15on:1.61',
       'org.bouncycastle:bcprov-jdk15on:1.61',
-      'com.fasterxml.jackson.core:jackson-databind:2.9.10',
+      'com.fasterxml.jackson.core:jackson-databind:2.11.2',
       'org.flywaydb:flyway-core:5.2.4',
       'org.glassfish.jaxb:jaxb-runtime:2.3.0',
       'org.hamcrest:hamcrest-all:1.3',

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-6.6-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-6.6.1-all.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

gradlew

@@ -130,7 +130,7 @@ fi
 if [ "$cygwin" = "true" -o "$msys" = "true" ] ; then
     APP_HOME=`cygpath --path --mixed "$APP_HOME"`
     CLASSPATH=`cygpath --path --mixed "$CLASSPATH"`
-    
+
     JAVACMD=`cygpath --unix "$JAVACMD"`
 
     # We build the pattern for arguments to be converted via cygpath

gradlew.bat

@@ -40,7 +40,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome
 
 set JAVA_EXE=java.exe
 %JAVA_EXE% -version >NUL 2>&1
-if "%ERRORLEVEL%" == "0" goto init
+if "%ERRORLEVEL%" == "0" goto execute
 
 echo.
 echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
@@ -54,7 +54,7 @@ goto fail
 set JAVA_HOME=%JAVA_HOME:"=%
 set JAVA_EXE=%JAVA_HOME%/bin/java.exe
 
-if exist "%JAVA_EXE%" goto init
+if exist "%JAVA_EXE%" goto execute
 
 echo.
 echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME%
@@ -64,21 +64,6 @@ echo location of your Java installation.
 
 goto fail
 
-:init
-@rem Get command-line arguments, handling Windows variants
-
-if not "%OS%" == "Windows_NT" goto win9xME_args
-
-:win9xME_args
-@rem Slurp the command line arguments.
-set CMD_LINE_ARGS=
-set _SKIP=2
-
-:win9xME_args_slurp
-if "x%~1" == "x" goto execute
-
-set CMD_LINE_ARGS=%*
-
 :execute
 @rem Setup the command line
 
@@ -86,7 +71,7 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar
 
 
 @rem Execute Gradle
-"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %CMD_LINE_ARGS%
+"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*
 
 :end
 @rem End local scope for the variables with windows NT shell

integration/run_schema_check.sh

@@ -26,9 +26,14 @@ Checks for post-deployment change to Flyway scripts.
 
 With Flyway, once an incremental change script is deployed, it must not be
 changed. Even changes to comments or whitespaces would cause validation
-failures during future deployment. This script checks for changes to scripts
-that have already been deployed to Sandbox. The assumption is that the schema
-in Sandbox is always newer than that in production.
+failures during future deployment. This script checks for changes (including
+removal and renaming which may happen due to incorrect merge conflict
+resolution) to scripts that have already been deployed to Sandbox. The
+assumption is that the schema in Sandbox is always newer than that in
+production.
+
+A side-effect of this check is that old branches missing recently deployed
+scripts must update first.
 
 Options:
     -h, --help  show this help text
@@ -61,7 +66,7 @@ fi
 sandbox_tag=$(fetchVersion sql sandbox ${DEV_PROJECT})
 echo "Checking Flyway scripts against schema in Sandbox (${sandbox_tag})."
 modified_sqls=$(git diff --name-status ${sandbox_tag} \
-    db/src/main/resources/sql/flyway | grep ^M | grep \.sql$ | wc -l)
+    db/src/main/resources/sql/flyway | grep "^M\|^D\|^R" | grep \.sql$ | wc -l)
 
 if [[ ${modified_sqls} = 0 ]]; then
   echo "No illegal change to deployed schema scripts."
@@ -69,6 +74,7 @@ if [[ ${modified_sqls} = 0 ]]; then
 else
   echo "Changes to the following files are not allowed:"
   echo $(git diff --name-status ${sandbox_tag} \
-      db/src/main/resources/sql/flyway | grep ^M | grep \.sql$)
+      db/src/main/resources/sql/flyway | grep "^M\|^D\|^R" | grep \.sql$)
+  echo "Make sure your branch is up to date with HEAD of master."
   exit 1
 fi

integration/testutils_bashrc

@@ -15,7 +15,8 @@
 # This file declares shell functions used by other scripts in this folder.
 
 # Fetch the tag of the currently deployed release of Nomulus server
-# or SQL schema.
+# or SQL schema. The caller is responsible for fetching relevant git
+# tags to the local repo.
 function fetchVersion() {
   local deployed_system=${1}
   local env=${2}

util/src/main/java/google/registry/util/EmailMessage.java

@@ -16,6 +16,7 @@ package google.registry.util;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.net.MediaType;
 import java.util.Collection;
 import java.util.Optional;
@@ -27,10 +28,10 @@ public abstract class EmailMessage {
 
   public abstract String subject();
   public abstract String body();
-  public abstract ImmutableList<InternetAddress> recipients();
+  public abstract ImmutableSet<InternetAddress> recipients();
   public abstract InternetAddress from();
 
-  public abstract ImmutableList<InternetAddress> bccs();
+  public abstract ImmutableSet<InternetAddress> bccs();
 
   public abstract Optional<MediaType> contentType();
   public abstract Optional<Attachment> attachment();
@@ -63,9 +64,9 @@ public abstract class EmailMessage {
     public abstract Builder setContentType(MediaType contentType);
     public abstract Builder setAttachment(Attachment attachment);
 
-    abstract ImmutableList.Builder<InternetAddress> recipientsBuilder();
+    abstract ImmutableSet.Builder<InternetAddress> recipientsBuilder();
 
-    abstract ImmutableList.Builder<InternetAddress> bccsBuilder();
+    abstract ImmutableSet.Builder<InternetAddress> bccsBuilder();
 
     public Builder addRecipient(InternetAddress value) {
       recipientsBuilder().add(value);