Consolidate conversion from Duration to Period in DurationConverter (#786)

* Consolidate conversion from Duration to Period in DurationConverter

* Resolve comment

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/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,6 +15,7 @@
 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;
@@ -36,6 +37,7 @@ import javax.persistence.Entity;
       @javax.persistence.Index(columnList = "historyType"),
       @javax.persistence.Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
 public class ContactHistory extends HistoryEntry {
   // Store ContactBase instead of ContactResource so we don't pick up its @Id
   ContactBase contactBase;

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

@@ -15,6 +15,7 @@
 package google.registry.model.domain;
 
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.EntitySubclass;
 import google.registry.model.EppResource;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.host.HostResource;
@@ -43,6 +44,7 @@ import javax.persistence.JoinTable;
       @javax.persistence.Index(columnList = "historyType"),
       @javax.persistence.Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
 public class DomainHistory extends HistoryEntry {
   // Store DomainContent instead of DomainBase so we don't pick up its @Id
   DomainContent domainContent;

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,6 +15,7 @@
 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;
@@ -37,6 +38,7 @@ import javax.persistence.Entity;
       @javax.persistence.Index(columnList = "historyType"),
       @javax.persistence.Index(columnList = "historyModificationTime")
     })
+@EntitySubclass
 public class HostHistory extends HistoryEntry {
 
   // Store HostBase instead of HostResource so we don't pick up its @Id

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,8 +30,14 @@ 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;
@@ -111,7 +119,8 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
   @Id
   @javax.persistence.Id
   @Column(name = "historyRevisionId")
-  Long id;
+  @VisibleForTesting
+  public Long id;
 
   /** The resource this event mutated. */
   @Parent @Transient protected Key<? extends EppResource> parent;
@@ -259,6 +268,44 @@ 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(
+                  VKey.create(DomainBase.class, parent.getName(), (Key<DomainBase>) parent))
+              .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,17 +315,40 @@ 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();
     }
 
-    // Until we move completely to SQL, override this in subclasses (e.g. HostHistory) to set VKeys
     public B setParent(Key<? extends EppResource> parent) {
       getInstance().parent = parent;
       return thisCastToDerived();

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/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/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/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>
@@ -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/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

@@ -17,15 +17,18 @@ 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.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 +48,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,19 +64,8 @@ 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()
@@ -82,9 +79,62 @@ public class DomainHistoryTest extends EntityTestCase {
             });
   }
 
+  @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");
   }
+
+  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.createVKey())
+        .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/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/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/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/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;
+  google.registry.persistence.VKey<google.registry.model.domain.DomainBase> domainRepoId;
+  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.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>

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);