Add tests for obscure hostname canonicalization rule (#1388)

Also correctly configures Gradle for the util subproject (it wasn't possible to
run tests in IntelliJ without these changes).

build.gradle

@@ -209,19 +209,22 @@ rootProject.ext {
 
 task runPresubmits(type: Exec) {
 
-  // Find a python version greater than 3.7.3 (this is somewhat arbitrary, we
-  // know we'd like at least 3.6, but 3.7.3 is the latest that ships with
-  // Debian so it seems like that should be available anywhere).
-  def MIN_PY_VER = 0x3070300
-  if (pyver('python') >= MIN_PY_VER) {
-    executable 'python'
-  } else if (pyver('/usr/bin/python3') >= MIN_PY_VER) {
-    executable '/usr/bin/python3'
-  } else {
-    throw new GradleException("No usable Python version found (build " +
-                              "requires at least python 3.7.3)");
-  }
   args('config/presubmits.py')
+
+  doFirst {
+    // Find a python version greater than 3.7.3 (this is somewhat arbitrary, we
+    // know we'd like at least 3.6, but 3.7.3 is the latest that ships with
+    // Debian so it seems like that should be available anywhere).
+    def MIN_PY_VER = 0x3070300
+    if (pyver('python') >= MIN_PY_VER) {
+      executable 'python'
+    } else if (pyver('/usr/bin/python3') >= MIN_PY_VER) {
+      executable '/usr/bin/python3'
+    } else {
+      throw new GradleException("No usable Python version found (build " +
+                                "requires at least python 3.7.3)");
+    }
+  }
 }
 
 def javadocSource = []

config/presubmits.py

@@ -213,6 +213,7 @@ PRESUBMITS = {
         {"src/test", "ActivityReportingQueryBuilder.java",
          # This class contains helper method to make queries in Beam.
          "RegistryJpaIO.java",
+         "CreateSyntheticHistoryEntriesAction.java",
          # TODO(b/179158393): Remove everything below, which should be done
          # using Criteria
          "JpaTransactionManager.java",

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

@@ -55,7 +55,7 @@ public final class CommitLogImports {
    * represents the changes in one transaction. The {@code CommitLogManifest} contains deleted
    * entity keys, whereas each {@code CommitLogMutation} contains one whole entity.
    */
-  public static ImmutableList<ImmutableList<VersionedEntity>> loadEntitiesByTransaction(
+  static ImmutableList<ImmutableList<VersionedEntity>> loadEntitiesByTransaction(
       InputStream inputStream) {
     try (InputStream input = new BufferedInputStream(inputStream)) {
       Iterator<ImmutableObject> commitLogs = createDeserializingIterator(input, false);
@@ -104,7 +104,7 @@ public final class CommitLogImports {
    * represents the changes in one transaction. The {@code CommitLogManifest} contains deleted
    * entity keys, whereas each {@code CommitLogMutation} contains one whole entity.
    */
-  public static ImmutableList<VersionedEntity> loadEntities(InputStream inputStream) {
+  static ImmutableList<VersionedEntity> loadEntities(InputStream inputStream) {
     return loadEntitiesByTransaction(inputStream).stream()
         .flatMap(ImmutableList::stream)
         .collect(toImmutableList());

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

@@ -105,29 +105,29 @@ public abstract class VersionedEntity implements Serializable {
    * VersionedEntity VersionedEntities}. See {@link CommitLogImports#loadEntities} for more
    * information.
    */
-  public static Stream<VersionedEntity> fromManifest(CommitLogManifest manifest) {
+  static Stream<VersionedEntity> fromManifest(CommitLogManifest manifest) {
     long commitTimeMillis = manifest.getCommitTime().getMillis();
     return manifest.getDeletions().stream()
         .map(com.googlecode.objectify.Key::getRaw)
-        .map(key -> builder().commitTimeMills(commitTimeMillis).key(key).build());
+        .map(key -> newBuilder().commitTimeMills(commitTimeMillis).key(key).build());
   }
 
   /* Converts a {@link CommitLogMutation} to a {@link VersionedEntity}. */
-  public static VersionedEntity fromMutation(CommitLogMutation mutation) {
+  static VersionedEntity fromMutation(CommitLogMutation mutation) {
     return from(
         com.googlecode.objectify.Key.create(mutation).getParent().getId(),
         mutation.getEntityProtoBytes());
   }
 
   public static VersionedEntity from(long commitTimeMillis, byte[] entityProtoBytes) {
-    return builder()
+    return newBuilder()
         .entityProtoBytes(entityProtoBytes)
         .key(EntityTranslator.createFromPbBytes(entityProtoBytes).getKey())
         .commitTimeMills(commitTimeMillis)
         .build();
   }
 
-  static Builder builder() {
+  private static Builder newBuilder() {
     return new AutoValue_VersionedEntity.Builder();
   }
 
@@ -142,7 +142,7 @@ public abstract class VersionedEntity implements Serializable {
 
     public abstract VersionedEntity build();
 
-    public Builder entityProtoBytes(byte[] bytes) {
+    Builder entityProtoBytes(byte[] bytes) {
       return entityProtoBytes(new ImmutableBytes(bytes));
     }
   }

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

@@ -55,6 +55,7 @@ import org.joda.time.format.DateTimeFormatter;
     path = SendExpiringCertificateNotificationEmailAction.PATH,
     auth = Auth.AUTH_INTERNAL_OR_ADMIN)
 public class SendExpiringCertificateNotificationEmailAction implements Runnable {
+
   public static final String PATH = "/_dr/task/sendExpiringCertificateNotificationEmail";
   /**
    * Used as an offset when storing the last notification email sent date.
@@ -96,8 +97,13 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable
   public void run() {
     response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
     try {
-      sendNotificationEmails();
+      int numEmailsSent = sendNotificationEmails();
+      String message =
+          String.format(
+              "Done. Sent %d expiring certificate notification emails in total.", numEmailsSent);
+      logger.atInfo().log(message);
       response.setStatus(SC_OK);
+      response.setPayload(message);
     } catch (Exception e) {
       logger.atWarning().withCause(e).log(
           "Exception thrown when sending expiring certificate notification emails.");
@@ -263,8 +269,6 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable
         emailsSent++;
       }
     }
-    logger.atInfo().log(
-        "Attempted to send %d expiring certificate notification emails.", emailsSent);
     return emailsSent;
   }
 
@@ -327,6 +331,7 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable
 
   @AutoValue
   public abstract static class RegistrarInfo {
+
     static RegistrarInfo create(
         Registrar registrar, boolean isCertExpiring, boolean isFailOverCertExpiring) {
       return new AutoValue_SendExpiringCertificateNotificationEmailAction_RegistrarInfo(

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

@@ -84,8 +84,12 @@ public class WipeOutContactHistoryPiiAction implements Runnable {
                             getNextContactHistoryEntitiesWithPiiBatch(wipeOutTime)));
         totalNumOfWipedEntities += numOfWipedEntities;
       } while (numOfWipedEntities > 0);
-      logger.atInfo().log(
-          "Wiped out PII of %d ContactHistory entities in total.", totalNumOfWipedEntities);
+      String msg =
+          String.format(
+              "Done. Wiped out PII of %d ContactHistory entities in total.",
+              totalNumOfWipedEntities);
+      logger.atInfo().log(msg);
+      response.setPayload(msg);
       response.setStatus(SC_OK);
 
     } catch (Exception e) {

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

@@ -92,7 +92,12 @@ public class WipeoutDatastoreAction implements Runnable {
               .setJobName(createJobName("bulk-delete-datastore-", clock))
               .setContainerSpecGcsPath(
                   String.format("%s/%s_metadata.json", stagingBucketUrl, PIPELINE_NAME))
-              .setParameters(ImmutableMap.of("kindsToDelete", "*"));
+              .setParameters(
+                  ImmutableMap.of(
+                      "kindsToDelete",
+                      "*",
+                      "registryEnvironment",
+                      RegistryEnvironment.get().name()));
       LaunchFlexTemplateResponse launchResponse =
           dataflow
               .projects()

core/src/main/java/google/registry/beam/common/RegistryPipelineOptions.java

@@ -34,7 +34,6 @@ import org.apache.beam.sdk.options.Description;
 public interface RegistryPipelineOptions extends GcpOptions {
 
   @Description("The Registry environment.")
-  @Nullable
   RegistryEnvironment getRegistryEnvironment();
 
   void setRegistryEnvironment(RegistryEnvironment environment);

core/src/main/java/google/registry/beam/datastore/BulkDeleteDatastorePipeline.java

@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.datastore.v1.Entity;
+import google.registry.config.RegistryEnvironment;
 import java.util.Iterator;
 import java.util.Map;
 import org.apache.beam.sdk.Pipeline;
@@ -308,6 +309,11 @@ public class BulkDeleteDatastorePipeline {
 
   public interface BulkDeletePipelineOptions extends GcpOptions {
 
+    @Description("The Registry environment.")
+    RegistryEnvironment getRegistryEnvironment();
+
+    void setRegistryEnvironment(RegistryEnvironment environment);
+
     @Description(
         "The Datastore KINDs to be deleted. The format may be:\n"
             + "\t- The list of kinds to be deleted as a comma-separated string, or\n"

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

@@ -22,6 +22,7 @@ import static google.registry.beam.initsql.BackupPaths.getExportFilePatterns;
 import static google.registry.model.ofy.ObjectifyService.auditedOfy;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static google.registry.util.DateTimeUtils.isBeforeOrAt;
+import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
 import static java.util.Comparator.comparing;
 import static org.apache.beam.sdk.values.TypeDescriptors.kvs;
 import static org.apache.beam.sdk.values.TypeDescriptors.strings;
@@ -277,7 +278,7 @@ public final class Transforms {
 
   // Prober contacts referencing phantom registrars. They and their associated history entries can
   // be safely ignored.
-  private static final ImmutableSet IGNORED_CONTACTS =
+  private static final ImmutableSet<String> IGNORED_CONTACTS =
       ImmutableSet.of(
           "1_WJ0TEST-GOOGLE", "1_WJ1TEST-GOOGLE", "1_WJ2TEST-GOOGLE", "1_WJ3TEST-GOOGLE");
 
@@ -320,7 +321,8 @@ public final class Transforms {
     return true;
   }
 
-  private static Entity repairBadData(Entity entity) {
+  @VisibleForTesting
+  static Entity repairBadData(Entity entity) {
     if (entity.getKind().equals("Cancellation")
         && Objects.equals(entity.getProperty("reason"), "AUTO_RENEW")) {
       // AUTO_RENEW has been moved from 'reason' to flags. Change reason to RENEW and add the
@@ -328,6 +330,15 @@ public final class Transforms {
       // instead of append. See b/185954992.
       entity.setUnindexedProperty("reason", Reason.RENEW.name());
       entity.setUnindexedProperty("flags", ImmutableList.of(Flag.AUTO_RENEW.name()));
+    } else if (entity.getKind().equals("DomainBase")) {
+      // Canonicalize old domain/host names from 2016 and earlier before we were enforcing this.
+      entity.setIndexedProperty(
+          "fullyQualifiedDomainName",
+          canonicalizeDomainName((String) entity.getProperty("fullyQualifiedDomainName")));
+    } else if (entity.getKind().equals("HostResource")) {
+      entity.setIndexedProperty(
+          "fullyQualifiedHostName",
+          canonicalizeDomainName((String) entity.getProperty("fullyQualifiedHostName")));
     }
     return entity;
   }
@@ -365,7 +376,8 @@ public final class Transforms {
    * Returns a {@link PTransform} that produces a {@link PCollection} containing all elements in the
    * given {@link Iterable}.
    */
-  static PTransform<PBegin, PCollection<String>> toStringPCollection(Iterable<String> strings) {
+  private static PTransform<PBegin, PCollection<String>> toStringPCollection(
+      Iterable<String> strings) {
     return Create.of(strings).withCoder(StringUtf8Coder.of());
   }
 
@@ -373,7 +385,7 @@ public final class Transforms {
    * Returns a {@link PTransform} from file {@link Metadata} to {@link VersionedEntity} using
    * caller-provided {@code transformer}.
    */
-  static PTransform<PCollection<Metadata>, PCollection<VersionedEntity>> processFiles(
+  private static PTransform<PCollection<Metadata>, PCollection<VersionedEntity>> processFiles(
       DoFn<ReadableFile, VersionedEntity> transformer) {
     return new PTransform<PCollection<Metadata>, PCollection<VersionedEntity>>() {
       @Override
@@ -389,7 +401,7 @@ public final class Transforms {
     private final DateTime fromTime;
     private final DateTime toTime;
 
-    public FilterCommitLogFileByTime(DateTime fromTime, DateTime toTime) {
+    FilterCommitLogFileByTime(DateTime fromTime, DateTime toTime) {
       checkNotNull(fromTime, "fromTime");
       checkNotNull(toTime, "toTime");
       checkArgument(

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

@@ -348,4 +348,14 @@
     <schedule>every 3 minutes</schedule>
     <target>backend</target>
   </cron>
+
+  <cron>
+    <url><![CDATA[/_dr/task/wipeOutContactHistoryPii]]></url>
+    <description>
+      This job runs weekly to wipe out PII fields of ContactHistory entities
+      that have been in the database for a certain period of time.
+    </description>
+    <schedule>every monday 15:00</schedule>
+    <target>backend</target>
+  </cron>
 </cronentries>

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

@@ -229,6 +229,15 @@ public final class DomainRenewFlow implements TransactionalFlow {
 
   private DomainHistory buildDomainHistory(
       DomainBase newDomain, DateTime now, Period period, Duration renewGracePeriod) {
+    Optional<MetadataExtension> metadataExtensionOpt =
+        eppInput.getSingleExtension(MetadataExtension.class);
+    if (metadataExtensionOpt.isPresent()) {
+      MetadataExtension metadataExtension = metadataExtensionOpt.get();
+      if (metadataExtension.getReason() != null) {
+        historyBuilder.setReason(metadataExtension.getReason());
+      }
+      historyBuilder.setRequestedByRegistrar(metadataExtension.getRequestedByRegistrar());
+    }
     return historyBuilder
         .setType(DOMAIN_RENEW)
         .setPeriod(period)

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

@@ -15,12 +15,8 @@
 package google.registry.model;
 
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
-import static org.joda.time.DateTimeZone.UTC;
 
-import com.google.common.flogger.FluentLogger;
-import com.google.common.flogger.StackSize;
 import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.OnLoad;
 import google.registry.model.translators.UpdateAutoTimestampTranslatorFactory;
@@ -44,12 +40,10 @@ import org.joda.time.DateTime;
 @Embeddable
 public class UpdateAutoTimestamp extends ImmutableObject {
 
-  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
   // When set to true, database converters/translators should do the auto update.  When set to
   // false, auto update should be suspended (this exists to allow us to preserve the original value
   // during a replay).
-  private static ThreadLocal<Boolean> autoUpdateEnabled = ThreadLocal.withInitial(() -> true);
+  private static final ThreadLocal<Boolean> autoUpdateEnabled = ThreadLocal.withInitial(() -> true);
 
   @Transient DateTime timestamp;
 
@@ -63,16 +57,7 @@ public class UpdateAutoTimestamp extends ImmutableObject {
   @PrePersist
   @PreUpdate
   void setTimestamp() {
-    // On the off chance that this is called outside of a transaction, log it instead of failing
-    // with an exception from attempting to call jpaTm().getTransactionTime(), and then fall back
-    // to DateTime.now(UTC).
-    if (!jpaTm().inTransaction()) {
-      logger.atSevere().withStackTrace(StackSize.MEDIUM).log(
-          "Failed to update automatic timestamp because this wasn't called in a JPA transaction%s.",
-          ofyTm().inTransaction() ? " (but there is an open Ofy transaction)" : "");
-      timestamp = DateTime.now(UTC);
-      lastUpdateTime = DateTimeUtils.toZonedDateTime(timestamp);
-    } else if (autoUpdateEnabled() || lastUpdateTime == null) {
+    if (autoUpdateEnabled() || lastUpdateTime == null) {
       timestamp = jpaTm().getTransactionTime();
       lastUpdateTime = DateTimeUtils.toZonedDateTime(timestamp);
     }

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

@@ -865,7 +865,8 @@ public class DomainContent extends EppResource
     public B setDomainName(String domainName) {
       checkArgument(
           domainName.equals(canonicalizeDomainName(domainName)),
-          "Domain name must be in puny-coded, lower-case form");
+          "Domain name %s not in puny-coded, lower-case form",
+          domainName);
       getInstance().fullyQualifiedDomainName = domainName;
       return thisCastToDerived();
     }

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

@@ -196,7 +196,8 @@ public class HostBase extends EppResource {
     public B setHostName(String hostName) {
       checkArgument(
           hostName.equals(canonicalizeDomainName(hostName)),
-          "Host name must be in puny-coded, lower-case form");
+          "Host name %s not in puny-coded, lower-case form",
+          hostName);
       getInstance().fullyQualifiedHostName = hostName;
       return thisCastToDerived();
     }

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

@@ -132,13 +132,16 @@ public class TransactionManagerFactory {
   /**
    * Sets the return of {@link #tm()} to the given instance of {@link TransactionManager}.
    *
+   * <p>DO NOT CALL THIS DIRECTLY IF POSSIBLE. Strongly prefer the use of <code>TmOverrideExtension
+   * </code> in test code instead.
+   *
    * <p>Used when overriding the per-test transaction manager for dual-database tests. Should be
    * matched with a corresponding invocation of {@link #removeTmOverrideForTest()} either at the end
    * of the test or in an <code>@AfterEach</code> handler.
    */
   @VisibleForTesting
-  public static void setTmForTest(TransactionManager newTm) {
-    tmForTest = Optional.of(newTm);
+  public static void setTmOverrideForTest(TransactionManager newTmOverride) {
+    tmForTest = Optional.of(newTmOverride);
   }
 
   /** Resets the overridden transaction manager post-test. */
@@ -152,10 +155,10 @@ public class TransactionManagerFactory {
     }
   }
 
-  /** Thrown when a write is attempted when the DB is in read-only mode. */
+  /** Registry is currently undergoing maintenance and is in read-only mode. */
   public static class ReadOnlyModeException extends IllegalStateException {
-    public ReadOnlyModeException() {
-      super("Registry is currently in read-only mode");
+    ReadOnlyModeException() {
+      super("Registry is currently undergoing maintenance and is in read-only mode");
     }
   }
 }

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

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.util.CollectionUtils.findDuplicates;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameter;
@@ -53,6 +54,17 @@ final class RenewDomainCommand extends MutatingEppToolCommand {
   @Parameter(description = "Names of the domains to renew.", required = true)
   private List<String> mainParameters;
 
+  @Parameter(
+      names = {"--reason"},
+      description = "Reason for the change.")
+  String reason;
+
+  @Parameter(
+      names = {"--registrar_request"},
+      description = "Whether the change was requested by a registrar.",
+      arity = 1)
+  Boolean requestedByRegistrar;
+
   @Inject
   Clock clock;
 
@@ -70,12 +82,23 @@ final class RenewDomainCommand extends MutatingEppToolCommand {
       checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domainName);
       setSoyTemplate(DomainRenewSoyInfo.getInstance(), DomainRenewSoyInfo.RENEWDOMAIN);
       DomainBase domain = domainOptional.get();
-      addSoyRecord(
-          isNullOrEmpty(clientId) ? domain.getCurrentSponsorRegistrarId() : clientId,
+
+      SoyMapData soyMapData =
           new SoyMapData(
               "domainName", domain.getDomainName(),
               "expirationDate", domain.getRegistrationExpirationTime().toString(DATE_FORMATTER),
-              "period", String.valueOf(period)));
+              "period", String.valueOf(period));
+
+      if (requestedByRegistrar != null) {
+        soyMapData.put("requestedByRegistrar", requestedByRegistrar.toString());
+      }
+      if (reason != null) {
+        checkArgumentNotNull(
+            requestedByRegistrar, "--registrar_request is required when --reason is specified");
+        soyMapData.put("reason", reason);
+      }
+      addSoyRecord(
+          isNullOrEmpty(clientId) ? domain.getCurrentSponsorRegistrarId() : clientId, soyMapData);
     }
   }
 }

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

@@ -36,6 +36,7 @@ import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
+import google.registry.tools.soy.DomainRenewSoyInfo;
 import google.registry.tools.soy.UniformRapidSuspensionSoyInfo;
 import java.util.ArrayList;
 import java.util.List;
@@ -43,6 +44,7 @@ import java.util.Optional;
 import java.util.Set;
 import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
 import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
 
 /** A command to suspend a domain for the Uniform Rapid Suspension process. */
 @Parameters(separators = " =",
@@ -97,6 +99,13 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
       description = "Flag indicating that is is an undo command, which removes locks.")
   private boolean undo;
 
+  @Parameter(
+      names = {"--renew_one_year"},
+      required = true,
+      description = "Flag indicating whether or not the domain will be renewed for a year.",
+      arity = 1)
+  private boolean renewOneYear;
+
   /** Set of existing locks that need to be preserved during undo, sorted for nicer output. */
   ImmutableSortedSet<String> existingLocks;
 
@@ -114,19 +123,20 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
     superuser = true;
     DateTime now = DateTime.now(UTC);
     ImmutableSet<String> newHostsSet = ImmutableSet.copyOf(newHosts);
-    Optional<DomainBase> domain = loadByForeignKey(DomainBase.class, domainName, now);
-    checkArgumentPresent(domain, "Domain '%s' does not exist or is deleted", domainName);
+    Optional<DomainBase> domainOpt = loadByForeignKey(DomainBase.class, domainName, now);
+    checkArgumentPresent(domainOpt, "Domain '%s' does not exist or is deleted", domainName);
+    DomainBase domain = domainOpt.get();
     Set<String> missingHosts =
         difference(newHostsSet, checkResourcesExist(HostResource.class, newHosts, now));
     checkArgument(missingHosts.isEmpty(), "Hosts do not exist: %s", missingHosts);
     checkArgument(
         locksToPreserve.isEmpty() || undo,
         "Locks can only be preserved when running with --undo");
-    existingNameservers = getExistingNameservers(domain.get());
-    existingLocks = getExistingLocks(domain.get());
-    existingDsData = getExistingDsData(domain.get());
+    existingNameservers = getExistingNameservers(domain);
+    existingLocks = getExistingLocks(domain);
+    existingDsData = getExistingDsData(domain);
     removeStatuses =
-        (hasClientHold(domain.get()) && !undo)
+        (hasClientHold(domain) && !undo)
             ? ImmutableSet.of(StatusValue.CLIENT_HOLD.getXmlName())
             : ImmutableSet.of();
     ImmutableSet<String> statusesToApply;
@@ -138,6 +148,30 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
     } else {
       statusesToApply = URS_LOCKS;
     }
+
+    // trigger renew flow
+    if (renewOneYear) {
+      setSoyTemplate(DomainRenewSoyInfo.getInstance(), DomainRenewSoyInfo.RENEWDOMAIN);
+      addSoyRecord(
+          CLIENT_ID,
+          new SoyMapData(
+              "domainName",
+              domain.getDomainName(),
+              "expirationDate",
+              domain
+                  .getRegistrationExpirationTime()
+                  .toString(DateTimeFormat.forPattern("YYYY-MM-dd")),
+              // period is the number of years to renew the registration for
+              "period",
+              String.valueOf(1),
+              // use the same values for reason and requestedByRegistrar from update flow
+              "reason",
+              (undo ? "Undo " : "") + "Uniform Rapid Suspension",
+              "requestedByRegistrar",
+              Boolean.toString(false)));
+    }
+
+    // trigger update flow
     setSoyTemplate(
         UniformRapidSuspensionSoyInfo.getInstance(),
         UniformRapidSuspensionSoyInfo.UNIFORMRAPIDSUSPENSION);

core/src/main/java/google/registry/tools/javascrap/CreateSyntheticHistoryEntriesAction.java

@@ -21,6 +21,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 
 import com.google.appengine.tools.mapreduce.Mapper;
+import com.google.common.base.CaseFormat;
 import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import google.registry.config.RegistryConfig.Config;
@@ -29,16 +30,12 @@ import google.registry.mapreduce.inputs.EppResourceInputs;
 import google.registry.model.EppResource;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.reporting.HistoryEntry;
-import google.registry.persistence.transaction.CriteriaQueryBuilder;
 import google.registry.rde.RdeStagingAction;
 import google.registry.request.Action;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.tools.server.GenerateZoneFilesAction;
-import java.util.Optional;
 import javax.inject.Inject;
-import javax.persistence.criteria.CriteriaBuilder;
-import javax.persistence.criteria.CriteriaQuery;
 
 /**
  * A mapreduce that creates synthetic history objects in SQL for all {@link EppResource} objects.
@@ -86,12 +83,12 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
   }
 
   /**
-   * The number of shards to run the map-only mapreduce on.
+   * The default number of shards to run the map-only mapreduce on.
    *
-   * <p>This is much lower than the default of 100, or even 10, because we can afford it being slow
-   * and we want to avoid overloading SQL.
+   * <p>This is much lower than the default of 100 because we can afford it being slow and we want
+   * to avoid overloading SQL.
    */
-  private static final int NUM_SHARDS = 3;
+  private static final int NUM_SHARDS = 10;
 
   @Override
   public void run() {
@@ -105,8 +102,9 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
         .sendLinkToMapreduceConsole(response);
   }
 
-  // Lifted from HistoryEntryDao
-  private static Optional<? extends HistoryEntry> mostRecentHistoryFromSql(EppResource resource) {
+  // Returns true iff any of the *History objects in SQL contain a representation of this resource
+  // at the point in time that the *History object was created.
+  private static boolean hasHistoryContainingResource(EppResource resource) {
     return jpaTm()
         .transact(
             () -> {
@@ -114,18 +112,24 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
               Class<? extends HistoryEntry> historyClass =
                   getHistoryClassFromParent(resource.getClass());
               // The field representing repo ID unfortunately varies by history class
-              String repoIdFieldName = getRepoIdFieldNameFromHistoryClass(historyClass);
-              CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder();
-              CriteriaQuery<? extends HistoryEntry> criteriaQuery =
-                  CriteriaQueryBuilder.create(historyClass)
-                      .where(repoIdFieldName, criteriaBuilder::equal, resource.getRepoId())
-                      .orderByDesc("modificationTime")
-                      .build();
-              return jpaTm()
-                  .criteriaQuery(criteriaQuery)
-                  .setMaxResults(1)
-                  .getResultStream()
-                  .findFirst();
+              String repoIdFieldName =
+                  CaseFormat.LOWER_CAMEL.to(
+                      CaseFormat.LOWER_UNDERSCORE,
+                      getRepoIdFieldNameFromHistoryClass(historyClass));
+              // The "history" fields in the *History objects are all prefixed with "history_". If
+              // any of the non-"history_" fields are non-null, that means that that row contains
+              // a representation of that EppResource at that point in time. We use creation_time as
+              // a marker since it's the simplest field and all EppResources will have it.
+              return (boolean)
+                  jpaTm()
+                      .getEntityManager()
+                      .createNativeQuery(
+                          String.format(
+                              "SELECT EXISTS (SELECT 1 FROM \"%s\" WHERE %s = :repoId AND"
+                                  + " creation_time IS NOT NULL)",
+                              historyClass.getSimpleName(), repoIdFieldName))
+                      .setParameter("repoId", resource.getRepoId())
+                      .getSingleResult();
             });
   }
 
@@ -164,10 +168,7 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
       EppResource eppResource = auditedOfy().load().key(resourceKey).now();
       // Only save new history entries if the most recent history for this object in SQL does not
       // have the resource at that point in time already
-      Optional<? extends HistoryEntry> maybeHistory = mostRecentHistoryFromSql(eppResource);
-      if (maybeHistory
-          .map(history -> !history.getResourceAtPointInTime().isPresent())
-          .orElse(true)) {
+      if (!hasHistoryContainingResource(eppResource)) {
         ofyTm()
             .transact(
                 () ->

core/src/main/resources/google/registry/beam/bulk_delete_datastore_pipeline_metadata.json

@@ -2,6 +2,15 @@
   "name": "Bulk Delete Cloud Datastore",
   "description": "An Apache Beam batch pipeline that deletes Cloud Datastore in bulk. This is easier to use than the GCP-provided template.",
   "parameters": [
+    {
+      "name": "registryEnvironment",
+      "label": "The Registry environment.",
+      "helpText": "The Registry environment, required only because the worker initializer demands it.",
+      "is_optional": false,
+      "regexes": [
+        "^PRODUCTION|SANDBOX|CRASH|QA|ALPHA$"
+      ]
+    },
     {
       "name": "kindsToDelete",
       "label": "The data KINDs to delete.",

core/src/main/resources/google/registry/beam/init_sql_pipeline_metadata.json

@@ -5,10 +5,10 @@
     {
       "name": "registryEnvironment",
       "label": "The Registry environment.",
-      "helpText": "The Registry environment, required if environment-specific initialization (such as JPA) is needed on worker VMs.",
-      "is_optional": true,
+      "helpText": "The Registry environment.",
+      "is_optional": false,
       "regexes": [
-        "^[0-9A-Z_]+$"
+        "^PRODUCTION|SANDBOX|CRASH|QA|ALPHA$"
       ]
     },
     {

core/src/main/resources/google/registry/beam/invoicing_pipeline_metadata.json

@@ -5,7 +5,7 @@
     {
       "name": "registryEnvironment",
       "label": "The Registry environment.",
-      "helpText": "The Registry environment, required if environment-specific initialization (such as JPA) is needed on worker VMs.",
+      "helpText": "The Registry environment.",
       "is_optional": false,
       "regexes": [
         "^PRODUCTION|SANDBOX|CRASH|QA|ALPHA$"

core/src/main/resources/google/registry/beam/rde_pipeline_metadata.json

@@ -5,7 +5,7 @@
     {
       "name": "registryEnvironment",
       "label": "The Registry environment.",
-      "helpText": "The Registry environment, required if environment-specific initialization (such as JPA) is needed on worker VMs.",
+      "helpText": "The Registry environment.",
       "is_optional": false,
       "regexes": [
         "^PRODUCTION|SANDBOX|CRASH|QA|ALPHA$"

core/src/main/resources/google/registry/beam/spec11_pipeline_metadata.json

@@ -5,7 +5,7 @@
     {
       "name": "registryEnvironment",
       "label": "The Registry environment.",
-      "helpText": "The Registry environment, required if environment-specific initialization (such as JPA) is needed on worker VMs.",
+      "helpText": "The Registry environment.",
       "is_optional": false,
       "regexes": [
         "^PRODUCTION|SANDBOX|CRASH|QA|ALPHA$"

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

@@ -21,6 +21,8 @@
 {@param domainName: string}
 {@param expirationDate: string}
 {@param period: string}
+{@param? reason: string}
+{@param? requestedByRegistrar: string}
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
   <command>
@@ -32,6 +34,18 @@
         <domain:period unit="y">{$period}</domain:period>
       </domain:renew>
     </renew>
+    {if $reason or $requestedByRegistrar}
+      <extension>
+          <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+            {if $reason}
+              <metadata:reason>{$reason}</metadata:reason>
+            {/if}
+            {if $requestedByRegistrar}
+              <metadata:requestedByRegistrar>{$requestedByRegistrar}</metadata:requestedByRegistrar>
+            {/if}
+          </metadata:metadata>
+      </extension>
+    {/if}
     <clTRID>RegistryTool</clTRID>
   </command>
 </epp>

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

@@ -639,9 +639,32 @@ class SendExpiringCertificateNotificationEmailActionTest {
   }
 
   @TestOfyAndSql
-  void run_responseStatusIs200() {
+  void run_sentZeroEmail_responseStatusIs200() {
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_OK);
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Sent 0 expiring certificate notification emails in total.");
+  }
+
+  @TestOfyAndSql
+  void run_sentEmails_responseStatusIs200() throws Exception {
+    for (int i = 1; i <= 5; i++) {
+      persistResource(
+          createRegistrar(
+                  "id_" + i,
+                  "name" + i,
+                  SelfSignedCaCertificate.create(
+                          "www.example.tld",
+                          DateTime.parse("2020-09-02T00:00:00Z"),
+                          DateTime.parse("2021-06-01T00:00:00Z"))
+                      .cert(),
+                  null)
+              .build());
+    }
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(SC_OK);
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Sent 5 expiring certificate notification emails in total.");
   }
 
   /** Returns a sample registrar with a customized registrar name, client id and certificate* */

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

@@ -23,7 +23,6 @@ import static org.apache.http.HttpStatus.SC_OK;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.truth.Truth8;
 import google.registry.model.contact.ContactAddress;
 import google.registry.model.contact.ContactAuthInfo;
 import google.registry.model.contact.ContactBase;
@@ -50,8 +49,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 @DualDatabaseTest
 class WipeOutContactHistoryPiiActionTest {
 
+  private static final int TEST_BATCH_SIZE = 20;
   private static final int MIN_MONTHS_BEFORE_WIPE_OUT = 18;
-  private static final int BATCH_SIZE = 500;
   private static final ContactResource defaultContactResource =
       new ContactResource.Builder()
           .setContactId("sh8013")
@@ -115,7 +114,8 @@ class WipeOutContactHistoryPiiActionTest {
   void beforeEach() {
     response = new FakeResponse();
     action =
-        new WipeOutContactHistoryPiiAction(clock, MIN_MONTHS_BEFORE_WIPE_OUT, BATCH_SIZE, response);
+        new WipeOutContactHistoryPiiAction(
+            clock, MIN_MONTHS_BEFORE_WIPE_OUT, TEST_BATCH_SIZE, response);
   }
 
   @TestSqlOnly
@@ -133,10 +133,10 @@ class WipeOutContactHistoryPiiActionTest {
   }
 
   @TestSqlOnly
-  void getAllHistoryEntitiesOlderThan_returnsOnlyPartOfThePersistedEntities() {
+  void getAllHistoryEntitiesOlderThan_returnsOnlyOldEnoughPersistedEntities() {
     ImmutableList<ContactHistory> expectedToBeWipedOut =
         persistLotsOfContactHistoryEntities(
-            40, MIN_MONTHS_BEFORE_WIPE_OUT + 2, 0, defaultContactResource);
+            19, MIN_MONTHS_BEFORE_WIPE_OUT + 2, 0, defaultContactResource);
 
     // persisted entities that should not be part of the actual result
     persistLotsOfContactHistoryEntities(
@@ -145,7 +145,7 @@ class WipeOutContactHistoryPiiActionTest {
     jpaTm()
         .transact(
             () ->
-                Truth8.assertThat(
+                assertThat(
                         action.getNextContactHistoryEntitiesWithPiiBatch(
                             clock.nowUtc().minusMonths(MIN_MONTHS_BEFORE_WIPE_OUT)))
                     .containsExactlyElementsIn(expectedToBeWipedOut));
@@ -175,6 +175,8 @@ class WipeOutContactHistoryPiiActionTest {
         .isEqualTo(0);
 
     assertThat(response.getStatus()).isEqualTo(SC_OK);
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Wiped out PII of 0 ContactHistory entities in total.");
   }
 
   @TestSqlOnly
@@ -197,6 +199,8 @@ class WipeOutContactHistoryPiiActionTest {
     assertAllEntitiesContainPii(DatabaseHelper.loadByEntitiesIfPresent(expectedToBeWipedOut));
 
     action.run();
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Wiped out PII of 20 ContactHistory entities in total.");
 
     // The query should return an empty stream after the wipe out action.
     assertThat(
@@ -216,7 +220,7 @@ class WipeOutContactHistoryPiiActionTest {
   void run_withMultipleBatches_numOfEntitiesAsNonMultipleOfBatchSize_success() {
     int numOfMonthsFromNow = MIN_MONTHS_BEFORE_WIPE_OUT + 2;
     ImmutableList<ContactHistory> expectedToBeWipedOut =
-        persistLotsOfContactHistoryEntities(1234, numOfMonthsFromNow, 0, defaultContactResource);
+        persistLotsOfContactHistoryEntities(56, numOfMonthsFromNow, 0, defaultContactResource);
 
     // The query should return a subset of all persisted data.
     assertThat(
@@ -227,10 +231,12 @@ class WipeOutContactHistoryPiiActionTest {
                             .getNextContactHistoryEntitiesWithPiiBatch(
                                 clock.nowUtc().minusMonths(MIN_MONTHS_BEFORE_WIPE_OUT))
                             .count()))
-        .isEqualTo(BATCH_SIZE);
+        .isEqualTo(TEST_BATCH_SIZE);
 
     assertAllEntitiesContainPii(DatabaseHelper.loadByEntitiesIfPresent(expectedToBeWipedOut));
     action.run();
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Wiped out PII of 56 ContactHistory entities in total.");
 
     // The query should return an empty stream after the wipe out action.
     assertThat(
@@ -250,7 +256,8 @@ class WipeOutContactHistoryPiiActionTest {
   void run_withMultipleBatches_numOfEntitiesAsMultiplesOfBatchSize_success() {
     int numOfMonthsFromNow = MIN_MONTHS_BEFORE_WIPE_OUT + 2;
     ImmutableList<ContactHistory> expectedToBeWipedOut =
-        persistLotsOfContactHistoryEntities(2000, numOfMonthsFromNow, 0, defaultContactResource);
+        persistLotsOfContactHistoryEntities(
+            TEST_BATCH_SIZE * 2, numOfMonthsFromNow, 0, defaultContactResource);
 
     // The query should return a subset of all persisted data.
     assertThat(
@@ -261,10 +268,12 @@ class WipeOutContactHistoryPiiActionTest {
                             .getNextContactHistoryEntitiesWithPiiBatch(
                                 clock.nowUtc().minusMonths(MIN_MONTHS_BEFORE_WIPE_OUT))
                             .count()))
-        .isEqualTo(BATCH_SIZE);
+        .isEqualTo(TEST_BATCH_SIZE);
 
     assertAllEntitiesContainPii(DatabaseHelper.loadByEntitiesIfPresent(expectedToBeWipedOut));
     action.run();
+    assertThat(response.getPayload())
+        .isEqualTo("Done. Wiped out PII of 40 ContactHistory entities in total.");
 
     // The query should return an empty stream after the wipe out action.
     assertThat(

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

@@ -0,0 +1,64 @@
+// Copyright 2021 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.beam.initsql;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.beam.initsql.Transforms.repairBadData;
+import static google.registry.model.ofy.ObjectifyService.auditedOfy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
+import static google.registry.testing.DatabaseHelper.createTld;
+import static google.registry.testing.DatabaseHelper.newDomainBase;
+import static google.registry.testing.DatabaseHelper.newHostResource;
+
+import com.google.appengine.api.datastore.Entity;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.host.HostResource;
+import google.registry.testing.AppEngineExtension;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+/** Unit tests for {@link Transforms}. */
+public class TransformsTest {
+
+  @RegisterExtension
+  public final AppEngineExtension appEngine =
+      AppEngineExtension.builder().withDatastoreAndCloudSql().build();
+
+  @BeforeEach
+  void beforeEach() {
+    createTld("tld");
+  }
+
+  @Test
+  void testRepairBadData_canonicalizesDomainName() {
+    DomainBase domain = newDomainBase("foobar.tld");
+    Entity entity = ofyTm().transact(() -> auditedOfy().toEntity(domain));
+    entity.setIndexedProperty("fullyQualifiedDomainName", "FOOBäR.TLD");
+    assertThat(((DomainBase) auditedOfy().toPojo(repairBadData(entity))).getDomainName())
+        .isEqualTo("xn--foobr-jra.tld");
+  }
+
+  @Test
+  void testRepairBadData_canonicalizesHostName() {
+    HostResource host = newHostResource("baz.foobar.tld");
+    Entity entity = ofyTm().transact(() -> auditedOfy().toEntity(host));
+    entity.setIndexedProperty(
+        "fullyQualifiedHostName", "b̴̹͔͓̣̭̫͇͕̻̬̱͇͗͌́̆̋͒a̶̬̖͚̋̈́̽̇͝͠z̵͠.FOOBäR.TLD");
+    assertThat(((HostResource) auditedOfy().toPojo(repairBadData(entity))).getHostName())
+        .isEqualTo(
+            "xn--baz-kdcb2ajgzb4jtg6doej4e6b9am7c7b6c5nd4k7gpa2a9a7dufyewec.xn--foobr-jra.tld");
+  }
+}

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

@@ -17,9 +17,6 @@ package google.registry.beam.invoicing;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.tld.Registry.TldState.GENERAL_AVAILABILITY;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.removeTmOverrideForTest;
-import static google.registry.persistence.transaction.TransactionManagerFactory.setTmForTest;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.newRegistry;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
@@ -48,6 +45,7 @@ import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationT
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
 import google.registry.testing.TestDataHelper;
+import google.registry.testing.TmOverrideExtension;
 import google.registry.util.ResourceUtils;
 import java.io.File;
 import java.nio.file.Files;
@@ -77,6 +75,25 @@ import org.junit.jupiter.api.io.TempDir;
 /** Unit tests for {@link InvoicingPipeline}. */
 class InvoicingPipelineTest {
 
+  @RegisterExtension
+  @Order(Order.DEFAULT - 1)
+  final transient DatastoreEntityExtension datastore =
+      new DatastoreEntityExtension().allThreads(true);
+
+  @RegisterExtension
+  final TestPipelineExtension pipeline =
+      TestPipelineExtension.create().enableAbandonedNodeEnforcement(true);
+
+  @RegisterExtension
+  final JpaIntegrationTestExtension database =
+      new JpaTestExtensions.Builder().withClock(new FakeClock()).buildIntegrationTestExtension();
+
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
+  @TempDir Path tmpDir;
+
   private static final String BILLING_BUCKET_URL = "billing_bucket";
   private static final String YEAR_MONTH = "2017-10";
   private static final String INVOICE_FILE_PREFIX = "REG-INV";
@@ -225,21 +242,6 @@ class InvoicingPipelineTest {
           "2017-10-01,2018-09-30,456,20.50,USD,10125,1,PURCHASE,bestdomains - test,1,"
               + "RENEW | TLD: test | TERM: 1-year,20.50,USD,116688");
 
-  @RegisterExtension
-  @Order(Order.DEFAULT - 1)
-  final transient DatastoreEntityExtension datastore =
-      new DatastoreEntityExtension().allThreads(true);
-
-  @RegisterExtension
-  final TestPipelineExtension pipeline =
-      TestPipelineExtension.create().enableAbandonedNodeEnforcement(true);
-
-  @RegisterExtension
-  final JpaIntegrationTestExtension database =
-      new JpaTestExtensions.Builder().withClock(new FakeClock()).buildIntegrationTestExtension();
-
-  @TempDir Path tmpDir;
-
   private final InvoicingPipelineOptions options =
       PipelineOptionsFactory.create().as(InvoicingPipelineOptions.class);
 
@@ -261,13 +263,12 @@ class InvoicingPipelineTest {
     String query = InvoicingPipeline.makeQuery("2017-10", "my-project-id");
     assertThat(query)
         .isEqualTo(TestDataHelper.loadFile(this.getClass(), "billing_events_test.sql"));
-    // This is necessary because the TestPipelineExtension verifies that the pipelien is run.
+    // This is necessary because the TestPipelineExtension verifies that the pipeline is run.
     pipeline.run();
   }
 
   @Test
   void testSuccess_fullSqlPipeline() throws Exception {
-    setTmForTest(jpaTm());
     setupCloudSql();
     options.setDatabase("CLOUD_SQL");
     InvoicingPipeline invoicingPipeline = new InvoicingPipeline(options);
@@ -282,18 +283,15 @@ class InvoicingPipelineTest {
                 + "UnitPriceCurrency,PONumber");
     assertThat(overallInvoice.subList(1, overallInvoice.size()))
         .containsExactlyElementsIn(EXPECTED_INVOICE_OUTPUT);
-    removeTmOverrideForTest();
   }
 
   @Test
   void testSuccess_readFromCloudSql() throws Exception {
-    setTmForTest(jpaTm());
     setupCloudSql();
     PCollection<BillingEvent> billingEvents = InvoicingPipeline.readFromCloudSql(options, pipeline);
     billingEvents = billingEvents.apply(new ChangeDomainRepo());
     PAssert.that(billingEvents).containsInAnyOrder(INPUT_EVENTS);
     pipeline.run().waitUntilFinish();
-    removeTmOverrideForTest();
   }
 
   @Test

core/src/test/java/google/registry/beam/rde/RdePipelineTest.java

@@ -22,7 +22,6 @@ import static google.registry.beam.rde.RdePipeline.encodePendings;
 import static google.registry.model.common.Cursor.CursorType.RDE_STAGING;
 import static google.registry.model.rde.RdeMode.FULL;
 import static google.registry.model.rde.RdeMode.THIN;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.rde.RdeResourceType.CONTACT;
 import static google.registry.rde.RdeResourceType.DOMAIN;
@@ -64,7 +63,6 @@ import google.registry.model.tld.Registry;
 import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
-import google.registry.persistence.transaction.TransactionManagerFactory;
 import google.registry.rde.DepositFragment;
 import google.registry.rde.Ghostryde;
 import google.registry.rde.PendingDeposit;
@@ -74,6 +72,7 @@ import google.registry.testing.CloudTasksHelper.TaskMatcher;
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeKeyringModule;
+import google.registry.testing.TmOverrideExtension;
 import java.io.IOException;
 import java.util.function.Function;
 import java.util.regex.Matcher;
@@ -88,7 +87,6 @@ import org.bouncycastle.openpgp.PGPPrivateKey;
 import org.bouncycastle.openpgp.PGPPublicKey;
 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.Order;
 import org.junit.jupiter.api.Test;
@@ -156,6 +154,10 @@ public class RdePipelineTest {
   final JpaIntegrationTestExtension database =
       new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension();
 
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
   @RegisterExtension
   final TestPipelineExtension pipeline =
       TestPipelineExtension.fromOptions(options).enableAbandonedNodeEnforcement(true);
@@ -164,7 +166,6 @@ public class RdePipelineTest {
 
   @BeforeEach
   void beforeEach() throws Exception {
-    TransactionManagerFactory.setTmForTest(jpaTm());
     loadInitialData();
 
     // Two real registrars have been created by loadInitialData(), named "New Registrar" and "The
@@ -221,11 +222,6 @@ public class RdePipelineTest {
     rdePipeline = new RdePipeline(options, gcsUtils, cloudTasksHelper.getTestCloudTasksUtils());
   }
 
-  @AfterEach
-  void afterEach() {
-    TransactionManagerFactory.removeTmOverrideForTest();
-  }
-
   @Test
   void testSuccess_encodeAndDecodePendingsMap() throws Exception {
     String encodedString = encodePendings(pendings);

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

@@ -18,8 +18,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.removeTmOverrideForTest;
-import static google.registry.persistence.transaction.TransactionManagerFactory.setTmForTest;
 import static google.registry.testing.AppEngineExtension.makeRegistrar1;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistActiveContact;
@@ -52,6 +50,7 @@ import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationT
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeSleeper;
+import google.registry.testing.TmOverrideExtension;
 import google.registry.util.ResourceUtils;
 import google.registry.util.Retrier;
 import java.io.File;
@@ -129,6 +128,10 @@ class Spec11PipelineTest {
   final JpaIntegrationTestExtension database =
       new JpaTestExtensions.Builder().withClock(new FakeClock()).buildIntegrationTestExtension();
 
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
   private final Spec11PipelineOptions options =
       PipelineOptionsFactory.create().as(Spec11PipelineOptions.class);
 
@@ -233,7 +236,6 @@ class Spec11PipelineTest {
   }
 
   private void setupCloudSql() {
-    setTmForTest(jpaTm());
     persistNewRegistrar("TheRegistrar");
     persistNewRegistrar("NewRegistrar");
     Registrar registrar1 =
@@ -273,7 +275,6 @@ class Spec11PipelineTest {
     persistResource(createDomain("no-email.com", "2A4BA9BBC-COM", registrar2, contact2));
     persistResource(
         createDomain("anti-anti-anti-virus.dev", "555666888-DEV", registrar3, contact3));
-    removeTmOverrideForTest();
   }
 
   private void verifySaveToGcs() throws Exception {

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

@@ -42,6 +42,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import google.registry.flows.EppException;
+import google.registry.flows.EppRequestSource;
 import google.registry.flows.FlowUtils.UnknownCurrencyEppException;
 import google.registry.flows.ResourceFlowTestCase;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
@@ -533,6 +534,55 @@ class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, DomainBa
             .build());
   }
 
+  @TestOfyAndSql
+  void testSuccess_metaData_withReasonAndRequestedByRegistrar() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput(
+        "domain_renew_metadata_with_reason_and_requestedByRegistrar.xml",
+        ImmutableMap.of(
+            "DOMAIN",
+            "example.tld",
+            "EXPDATE",
+            "2000-04-03",
+            "YEARS",
+            "1",
+            "REASON",
+            "domain-renew-test",
+            "REQUESTED",
+            "false"));
+    persistDomain();
+    runFlow();
+    DomainBase domain = reloadResourceByForeignKey();
+    assertAboutDomains()
+        .that(domain)
+        .hasOneHistoryEntryEachOfTypes(
+            HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_RENEW);
+    assertAboutHistoryEntries()
+        .that(getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_RENEW))
+        .hasMetadataReason("domain-renew-test")
+        .and()
+        .hasMetadataRequestedByRegistrar(false);
+  }
+
+  @TestOfyAndSql
+  void testSuccess_metaData_withRequestedByRegistrarOnly() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput("domain_renew_metadata_with_requestedByRegistrar_only.xml");
+
+    persistDomain();
+    runFlow();
+    DomainBase domain1 = reloadResourceByForeignKey();
+    assertAboutDomains()
+        .that(domain1)
+        .hasOneHistoryEntryEachOfTypes(
+            HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_RENEW);
+    assertAboutHistoryEntries()
+        .that(getOnlyHistoryEntryOfType(domain1, HistoryEntry.Type.DOMAIN_RENEW))
+        .hasMetadataReason(null)
+        .and()
+        .hasMetadataRequestedByRegistrar(true);
+  }
+
   @TestOfyAndSql
   void testFailure_neverExisted() throws Exception {
     ResourceDoesNotExistException thrown =

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

@@ -696,7 +696,7 @@ public class DomainBaseTest extends EntityTestCase {
             IllegalArgumentException.class, () -> domain.asBuilder().setDomainName("AAA.BBB"));
     assertThat(thrown)
         .hasMessageThat()
-        .contains("Domain name must be in puny-coded, lower-case form");
+        .isEqualTo("Domain name AAA.BBB not in puny-coded, lower-case form");
   }
 
   @Test
@@ -706,7 +706,7 @@ public class DomainBaseTest extends EntityTestCase {
             IllegalArgumentException.class, () -> domain.asBuilder().setDomainName("みんな.みんな"));
     assertThat(thrown)
         .hasMessageThat()
-        .contains("Domain name must be in puny-coded, lower-case form");
+        .isEqualTo("Domain name みんな.みんな not in puny-coded, lower-case form");
   }
 
   @Test

core/src/test/java/google/registry/model/host/HostResourceTest.java

@@ -200,7 +200,7 @@ class HostResourceTest extends EntityTestCase {
             IllegalArgumentException.class, () -> host.asBuilder().setHostName("AAA.BBB.CCC"));
     assertThat(thrown)
         .hasMessageThat()
-        .contains("Host name must be in puny-coded, lower-case form");
+        .isEqualTo("Host name AAA.BBB.CCC not in puny-coded, lower-case form");
   }
 
   @TestOfyAndSql
@@ -210,7 +210,7 @@ class HostResourceTest extends EntityTestCase {
             IllegalArgumentException.class, () -> host.asBuilder().setHostName("みんな.みんな.みんな"));
     assertThat(thrown)
         .hasMessageThat()
-        .contains("Host name must be in puny-coded, lower-case form");
+        .isEqualTo("Host name みんな.みんな.みんな not in puny-coded, lower-case form");
   }
 
   @TestOfyAndSql

core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java

@@ -49,8 +49,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 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.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junitpioneer.jupiter.RetryingTest;
@@ -74,7 +73,8 @@ public class ReplicateToDatastoreActionTest {
   private ReplicateToDatastoreAction action;
   private FakeResponse response;
 
-  @BeforeEach
+  // TODO(b/197534789): fix these tests and re-add the @BeforeEach
+  // @BeforeEach
   void setUp() {
     resetAction();
     injectExtension.setStaticField(Ofy.class, "clock", fakeClock);
@@ -88,13 +88,15 @@ public class ReplicateToDatastoreActionTest {
     TestObject.beforeDatastoreSaveCallCount = 0;
   }
 
-  @AfterEach
+  // TODO(b/197534789): fix these tests and re-add the @AfterEach
+  // @AfterEach
   void tearDown() {
     DatabaseHelper.removeDatabaseMigrationSchedule();
     fakeClock.disableAutoIncrement();
   }
 
   @RetryingTest(4)
+  @Disabled("b/197534789")
   void testReplication() {
     TestObject foo = TestObject.create("foo");
     TestObject bar = TestObject.create("bar");
@@ -120,6 +122,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @RetryingTest(4)
+  @Disabled("b/197534789")
   void testReplayFromLastTxn() {
     TestObject foo = TestObject.create("foo");
     TestObject bar = TestObject.create("bar");
@@ -142,6 +145,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @RetryingTest(4)
+  @Disabled("b/197534789")
   void testUnintentionalConcurrency() {
     TestObject foo = TestObject.create("foo");
     TestObject bar = TestObject.create("bar");
@@ -177,6 +181,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @RetryingTest(4)
+  @Disabled("b/197534789")
   void testMissingTransactions() {
     // Write a transaction (should have a transaction id of 1).
     TestObject foo = TestObject.create("foo");
@@ -194,6 +199,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @Test
+  @Disabled("b/197534789")
   void testMissingTransactions_fullTask() {
     // Write a transaction (should have a transaction id of 1).
     TestObject foo = TestObject.create("foo");
@@ -212,6 +218,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @Test
+  @Disabled("b/197534789")
   void testBeforeDatastoreSaveCallback() {
     TestObject testObject = TestObject.create("foo");
     insertInDb(testObject);
@@ -221,6 +228,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @Test
+  @Disabled("b/197534789")
   void testNotInMigrationState_doesNothing() {
     // set a schedule that backtracks the current status to DATASTORE_PRIMARY
     DateTime now = fakeClock.nowUtc();
@@ -257,6 +265,7 @@ public class ReplicateToDatastoreActionTest {
   }
 
   @Test
+  @Disabled("b/197534789")
   void testFailure_cannotAcquireLock() {
     RequestStatusChecker requestStatusChecker = mock(RequestStatusChecker.class);
     when(requestStatusChecker.getLogId()).thenReturn("logId");

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

@@ -36,6 +36,7 @@ import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import google.registry.testing.DatabaseHelper;
 import google.registry.testing.FakeClock;
+import google.registry.testing.TmOverrideExtension;
 import java.io.Serializable;
 import java.math.BigInteger;
 import java.sql.SQLException;
@@ -48,8 +49,7 @@ import javax.persistence.IdClass;
 import javax.persistence.OptimisticLockException;
 import javax.persistence.RollbackException;
 import org.hibernate.exception.JDBCConnectionException;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
@@ -84,15 +84,9 @@ class JpaTransactionManagerImplTest {
               TestEntity.class, TestCompoundIdEntity.class, TestNamedCompoundIdEntity.class)
           .buildUnitTestExtension();
 
-  @BeforeEach
-  void beforeEach() {
-    TransactionManagerFactory.setTmForTest(jpaTm());
-  }
-
-  @AfterEach
-  void afterEach() {
-    TransactionManagerFactory.removeTmOverrideForTest();
-  }
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
 
   @Test
   void transact_succeeds() {

core/src/test/java/google/registry/rdap/RdapDomainActionTest.java

@@ -45,7 +45,12 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
-/** Unit tests for {@link RdapDomainAction}. */
+/**
+ * Unit tests for {@link RdapDomainAction}.
+ *
+ * <p>TODO(b/26872828): The next time we do any work on RDAP, consider adding the APNIC RDAP
+ * conformance checker to the unit test suite.
+ */
 class RdapDomainActionTest extends RdapActionBaseTestCase<RdapDomainAction> {
 
   RdapDomainActionTest() {

core/src/test/java/google/registry/schema/registrar/RegistrarContactTest.java

@@ -16,8 +16,6 @@ package google.registry.schema.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.registrar.RegistrarContact.Type.WHOIS;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.setTmForTest;
 import static google.registry.testing.DatabaseHelper.insertInDb;
 import static google.registry.testing.DatabaseHelper.loadByEntity;
 import static google.registry.testing.SqlHelper.saveRegistrar;
@@ -28,6 +26,7 @@ import google.registry.model.registrar.RegistrarContact;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.DatastoreEntityExtension;
+import google.registry.testing.TmOverrideExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -44,13 +43,16 @@ class RegistrarContactTest {
   JpaIntegrationWithCoverageExtension jpa =
       new JpaTestExtensions.Builder().buildIntegrationWithCoverageExtension();
 
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
   private Registrar testRegistrar;
 
   private RegistrarContact testRegistrarPoc;
 
   @BeforeEach
   public void beforeEach() {
-    setTmForTest(jpaTm());
     testRegistrar = saveRegistrar("registrarId");
     testRegistrarPoc =
         new RegistrarContact.Builder()

core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java

@@ -15,7 +15,6 @@
 package google.registry.schema.registrar;
 
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.DatabaseHelper.existsInDb;
 import static google.registry.testing.DatabaseHelper.insertInDb;
 import static google.registry.testing.DatabaseHelper.loadByKey;
@@ -29,11 +28,10 @@ import google.registry.model.registrar.RegistrarAddress;
 import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension;
-import google.registry.persistence.transaction.TransactionManagerFactory;
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
+import google.registry.testing.TmOverrideExtension;
 import org.joda.time.DateTime;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -52,13 +50,16 @@ public class RegistrarDaoTest {
   JpaIntegrationWithCoverageExtension jpa =
       new JpaTestExtensions.Builder().withClock(fakeClock).buildIntegrationWithCoverageExtension();
 
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
   private final VKey<Registrar> registrarKey = VKey.createSql(Registrar.class, "registrarId");
 
   private Registrar testRegistrar;
 
   @BeforeEach
   void beforeEach() {
-    TransactionManagerFactory.setTmForTest(jpaTm());
     testRegistrar =
         new Registrar.Builder()
             .setType(Registrar.Type.TEST)
@@ -75,11 +76,6 @@ public class RegistrarDaoTest {
             .build();
   }
 
-  @AfterEach
-  void afterEach() {
-    TransactionManagerFactory.removeTmOverrideForTest();
-  }
-
   @Test
   void saveNew_worksSuccessfully() {
     assertThat(existsInDb(testRegistrar)).isFalse();

core/src/test/java/google/registry/schema/replay/SqlEntityTest.java

@@ -21,10 +21,9 @@ import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.model.registrar.RegistrarContact.RegistrarPocId;
 import google.registry.persistence.VKey;
-import google.registry.persistence.transaction.TransactionManagerFactory;
 import google.registry.testing.AppEngineExtension;
 import google.registry.testing.DatastoreEntityExtension;
-import org.junit.jupiter.api.AfterEach;
+import google.registry.testing.TmOverrideExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -41,17 +40,15 @@ public class SqlEntityTest {
   final AppEngineExtension database =
       new AppEngineExtension.Builder().withCloudSql().withoutCannedData().build();
 
+  @RegisterExtension
+  @Order(Order.DEFAULT + 1)
+  TmOverrideExtension tmOverrideExtension = TmOverrideExtension.withJpa();
+
   @BeforeEach
   void setup() throws Exception {
-    TransactionManagerFactory.setTmForTest(TransactionManagerFactory.jpaTm());
     AppEngineExtension.loadInitialData();
   }
 
-  @AfterEach
-  void teardown() {
-    TransactionManagerFactory.removeTmOverrideForTest();
-  }
-
   @Test
   void getPrimaryKeyString_oneIdColumn() {
     // AppEngineExtension canned data: Registrar1

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

@@ -487,20 +487,22 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
       if (replayer != null) {
         replayer.replay();
       }
-
-      if (withCloudSql) {
-        if (enableJpaEntityCoverageCheck) {
-          jpaIntegrationWithCoverageExtension.afterEach(context);
-        } else if (withJpaUnitTest) {
-          jpaUnitTestExtension.afterEach(context);
-        } else {
-          jpaIntegrationTestExtension.afterEach(context);
-        }
-      }
-      tearDown();
     } finally {
-      if (isWithDatastoreAndCloudSql()) {
-        restoreTmAfterDualDatabaseTest(context);
+      try {
+        if (withCloudSql) {
+          if (enableJpaEntityCoverageCheck) {
+            jpaIntegrationWithCoverageExtension.afterEach(context);
+          } else if (withJpaUnitTest) {
+            jpaUnitTestExtension.afterEach(context);
+          } else {
+            jpaIntegrationTestExtension.afterEach(context);
+          }
+        }
+        tearDown();
+      } finally {
+        if (isWithDatastoreAndCloudSql()) {
+          restoreTmAfterDualDatabaseTest(context);
+        }
       }
     }
   }

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

@@ -154,7 +154,7 @@ class DualDatabaseTestInvocationContextProvider implements TestTemplateInvocatio
       context.getStore(NAMESPACE).put(ORIGINAL_TM_KEY, tm());
       DatabaseType databaseType =
           (DatabaseType) context.getStore(NAMESPACE).get(INJECTED_TM_SUPPLIER_KEY);
-      TransactionManagerFactory.setTmForTest(databaseType.getTm());
+      TransactionManagerFactory.setTmOverrideForTest(databaseType.getTm());
     }
   }
 

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

@@ -38,7 +38,6 @@ import google.registry.persistence.transaction.Transaction.Delete;
 import google.registry.persistence.transaction.Transaction.Mutation;
 import google.registry.persistence.transaction.Transaction.Update;
 import google.registry.persistence.transaction.TransactionEntity;
-import google.registry.util.RequestStatusChecker;
 import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
@@ -46,7 +45,6 @@ import javax.annotation.Nullable;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.mockito.Mockito;
 
 /**
  * A JUnit extension that replays datastore transactions against postgresql.
@@ -83,11 +81,13 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
    * Create a replay extension that replays from SQL to cloud datastore when running in SQL mode.
    */
   public static ReplayExtension createWithDoubleReplay(FakeClock clock) {
-    return new ReplayExtension(
-        clock,
-        true,
-        new ReplicateToDatastoreAction(
-            clock, Mockito.mock(RequestStatusChecker.class), new FakeResponse()));
+    // TODO: use the proper double-replay extension when the tests are not flaky
+    // return new ReplayExtension(
+    //     clock,
+    //     true,
+    //     new ReplicateToDatastoreAction(
+    //         clock, Mockito.mock(RequestStatusChecker.class), new FakeResponse()));
+    return createWithCompare(clock);
   }
 
   @Override

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

@@ -43,6 +43,15 @@ public final class TestLogHandlerUtils {
         handler.getStoredLogRecords(), logRecord -> logRecord.getMessage().startsWith(prefix));
   }
 
+  /** Assert that the specified log message is <em>not</em> found. */
+  public static void assertNoLogMessage(CapturingLogHandler handler, Level level, String message) {
+    for (LogRecord logRecord : handler.getRecords()) {
+      if (logRecord.getLevel().equals(level) && logRecord.getMessage().contains(message)) {
+        assertWithMessage("Log message \"%s\" found: %s", message, logRecord.getMessage()).fail();
+      }
+    }
+  }
+
   public static void assertLogMessage(CapturingLogHandler handler, Level level, String message) {
     for (LogRecord logRecord : handler.getRecords()) {
       if (logRecord.getLevel().equals(level) && logRecord.getMessage().contains(message)) {

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

@@ -0,0 +1,73 @@
+// Copyright 2021 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.testing;
+
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
+
+import google.registry.persistence.transaction.TransactionManager;
+import google.registry.persistence.transaction.TransactionManagerFactory;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+/**
+ * JUnit extension for overriding the {@link TransactionManager} in tests.
+ *
+ * <p>You will typically want to run this at <code>@Order(Order.DEFAULT + 1)</code> alongside a
+ * {@link google.registry.persistence.transaction.JpaTransactionManagerExtension} or {@link
+ * DatastoreEntityExtension} with default {@link org.junit.jupiter.api.Order}. The transaction
+ * manager extension needs to run first so that when this override is called it's not trying to use
+ * the default dummy one.
+ *
+ * <p>This extension is incompatible with {@link DualDatabaseTest}. Use either that or this, but not
+ * both.
+ */
+public final class TmOverrideExtension implements BeforeEachCallback, AfterEachCallback {
+
+  private static enum TmOverride {
+    OFY,
+    JPA;
+  }
+
+  private final TmOverride tmOverride;
+
+  private TmOverrideExtension(TmOverride tmOverride) {
+    this.tmOverride = tmOverride;
+  }
+
+  /** Use the {@link google.registry.model.ofy.DatastoreTransactionManager} for all tests. */
+  public static TmOverrideExtension withOfy() {
+    return new TmOverrideExtension(TmOverride.OFY);
+  }
+
+  /**
+   * Use the {@link google.registry.persistence.transaction.JpaTransactionManager} for all tests.
+   */
+  public static TmOverrideExtension withJpa() {
+    return new TmOverrideExtension(TmOverride.JPA);
+  }
+
+  @Override
+  public void beforeEach(ExtensionContext context) {
+    TransactionManagerFactory.setTmOverrideForTest(
+        tmOverride == TmOverride.OFY ? ofyTm() : jpaTm());
+  }
+
+  @Override
+  public void afterEach(ExtensionContext context) {
+    TransactionManagerFactory.removeTmOverrideForTest();
+  }
+}

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

@@ -42,7 +42,6 @@ import java.io.OutputStream;
 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;
@@ -59,10 +58,6 @@ import org.mockito.junit.jupiter.MockitoExtension;
 @ExtendWith(MockitoExtension.class)
 public abstract class CommandTestCase<C extends Command> {
 
-  // Lock for stdout/stderr.  Note that this is static: since we're dealing with globals, we need
-  // to lock for the entire JVM.
-  private static final ReentrantLock streamsLock = new ReentrantLock();
-
   private final ByteArrayOutputStream stdout = new ByteArrayOutputStream();
   private final ByteArrayOutputStream stderr = new ByteArrayOutputStream();
   private PrintStream oldStdout, oldStderr;
@@ -90,10 +85,7 @@ public abstract class CommandTestCase<C extends Command> {
     RegistryToolEnvironment.UNITTEST.setup(systemPropertyExtension);
     command = newCommandInstance();
 
-    // Capture standard output/error. This is problematic because gradle tests run in parallel in
-    // the same JVM.  So first lock out any other tests in this JVM that are trying to do this
-    // trick.
-    streamsLock.lock();
+    // Capture standard output/error.
     oldStdout = System.out;
     System.setOut(new PrintStream(new OutputSplitter(System.out, stdout)));
     oldStderr = System.err;
@@ -104,7 +96,6 @@ public abstract class CommandTestCase<C extends Command> {
   public final void afterEachCommandTestCase() {
     System.setOut(oldStdout);
     System.setErr(oldStderr);
-    streamsLock.unlock();
   }
 
   void runCommandInEnvironment(RegistryToolEnvironment env, String... args) throws Exception {

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

@@ -132,6 +132,55 @@ public class RenewDomainCommandTest extends EppToolCommandTestCase<RenewDomainCo
         .verifyNoMoreSent();
   }
 
+  @Test
+  void testSuccess_withReasonAndRegistrarRequest() throws Exception {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    runCommandForced(
+        "domain.tld", "--period=1", "--reason=Renewing test domain", "--registrar_request=true");
+
+    eppVerifier
+        .expectRegistrarId("TheRegistrar")
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "domain.tld",
+                "EXPDATE",
+                "2015-09-05",
+                "YEARS",
+                "1",
+                "REASON",
+                "Renewing test domain",
+                "REQUESTED",
+                "true"));
+  }
+
+  @Test
+  void testSuccess_withReqistrarRequestOnly() throws Exception {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    runCommandForced("domain.tld", "--period=1", "--registrar_request=true");
+
+    eppVerifier
+        .expectRegistrarId("TheRegistrar")
+        .verifySent(
+            "domain_renew_with_metadata_requestedByRegistrar_only.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "domain.tld",
+                "EXPDATE",
+                "2015-09-05",
+                "YEARS",
+                "1",
+                "REQUESTED",
+                "true"));
+  }
+
   @Test
   void testFailure_domainDoesntExist() {
     IllegalArgumentException e =
@@ -173,4 +222,19 @@ public class RenewDomainCommandTest extends EppToolCommandTestCase<RenewDomainCo
   void testFailure_missingDomainNames() {
     assertThrows(ParameterException.class, () -> runCommand("--period 4"));
   }
+
+  @Test
+  void testFailure_registrarRequestIsRequiredWhenReasonIsPresent() {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    IllegalArgumentException e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> runCommandForced("domain.tld", "--period=1", "--reason=testing_only"));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo("--registrar_request is required when --reason is specified");
+  }
 }

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

@@ -23,12 +23,15 @@ import static google.registry.testing.DatabaseHelper.persistResource;
 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.domain.DomainBase;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
 import google.registry.persistence.VKey;
 import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
+import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -40,6 +43,8 @@ class UniformRapidSuspensionCommandTest
   private HostResource ns2;
   private HostResource urs1;
   private HostResource urs2;
+  private DomainBase defaultDomainBase;
+  private ImmutableSet<DelegationSignerData> defaultDsData;
 
   @BeforeEach
   void beforeEach() {
@@ -50,32 +55,36 @@ class UniformRapidSuspensionCommandTest
     ns2 = persistActiveHost("ns2.example.com");
     urs1 = persistActiveHost("urs1.example.com");
     urs2 = persistActiveHost("urs2.example.com");
+    defaultDomainBase = newDomainBase("evil.tld");
+    defaultDsData =
+        ImmutableSet.of(
+            DelegationSignerData.create(1, 2, 3, new HexBinaryAdapter().unmarshal("dead")),
+            DelegationSignerData.create(4, 5, 6, new HexBinaryAdapter().unmarshal("beef")));
   }
 
-  private void persistDomainWithHosts(HostResource... hosts) {
+  private void persistDomainWithHosts(
+      DomainBase domainBase, ImmutableSet<DelegationSignerData> dsData, HostResource... hosts) {
     ImmutableSet.Builder<VKey<HostResource>> hostRefs = new ImmutableSet.Builder<>();
     for (HostResource host : hosts) {
       hostRefs.add(host.createVKey());
     }
-    persistResource(newDomainBase("evil.tld").asBuilder()
-        .setNameservers(hostRefs.build())
-        .setDsData(ImmutableSet.of(
-            DelegationSignerData.create(1, 2, 3, new HexBinaryAdapter().unmarshal("dead")),
-            DelegationSignerData.create(4, 5, 6, new HexBinaryAdapter().unmarshal("beef"))))
-        .build());
+    persistResource(
+        domainBase.asBuilder().setNameservers(hostRefs.build()).setDsData(dsData).build());
   }
 
   @Test
   void testCommand_addsLocksReplacesHostsAndDsDataPrintsUndo() throws Exception {
-    persistDomainWithHosts(ns1, ns2);
+    persistDomainWithHosts(defaultDomainBase, defaultDsData, ns1, ns2);
     runCommandForced(
         "--domain_name=evil.tld",
         "--hosts=urs1.example.com,urs2.example.com",
-        "--dsdata=1 1 1 abcd");
+        "--dsdata=1 1 1 abcd",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension.xml");
+        .verifySent("uniform_rapid_suspension.xml")
+        .verifyNoMoreSent();
     assertInStdout("uniform_rapid_suspension --undo");
     assertInStdout("--domain_name evil.tld");
     assertInStdout("--hosts ns1.example.com,ns2.example.com");
@@ -86,12 +95,16 @@ class UniformRapidSuspensionCommandTest
 
   @Test
   void testCommand_respectsExistingHost() throws Exception {
-    persistDomainWithHosts(urs2, ns1);
-    runCommandForced("--domain_name=evil.tld", "--hosts=urs1.example.com,urs2.example.com");
+    persistDomainWithHosts(defaultDomainBase, defaultDsData, urs2, ns1);
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--hosts=urs1.example.com,urs2.example.com",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_existing_host.xml");
+        .verifySent("uniform_rapid_suspension_existing_host.xml")
+        .verifyNoMoreSent();
     assertInStdout("uniform_rapid_suspension --undo ");
     assertInStdout("--domain_name evil.tld");
     assertInStdout("--hosts ns1.example.com,urs2.example.com");
@@ -101,8 +114,11 @@ class UniformRapidSuspensionCommandTest
   @Test
   void testCommand_generatesUndoForUndelegatedDomain() throws Exception {
     persistActiveDomain("evil.tld");
-    runCommandForced("--domain_name=evil.tld", "--hosts=urs1.example.com,urs2.example.com");
-    eppVerifier.verifySentAny();
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--hosts=urs1.example.com,urs2.example.com",
+        "--renew_one_year=false");
+    eppVerifier.verifySentAny().verifyNoMoreSent();
     assertInStdout("uniform_rapid_suspension --undo");
     assertInStdout("--domain_name evil.tld");
     assertNotInStdout("--locks_to_preserve");
@@ -114,8 +130,8 @@ class UniformRapidSuspensionCommandTest
         newDomainBase("evil.tld").asBuilder()
           .addStatusValue(StatusValue.SERVER_DELETE_PROHIBITED)
           .build());
-    runCommandForced("--domain_name=evil.tld");
-    eppVerifier.verifySentAny();
+    runCommandForced("--domain_name=evil.tld", "--renew_one_year=false");
+    eppVerifier.verifySentAny().verifyNoMoreSent();
     assertInStdout("uniform_rapid_suspension --undo");
     assertInStdout("--domain_name evil.tld");
     assertInStdout("--locks_to_preserve serverDeleteProhibited");
@@ -133,11 +149,13 @@ class UniformRapidSuspensionCommandTest
     runCommandForced(
         "--domain_name=evil.tld",
         "--hosts=urs1.example.com,urs2.example.com",
-        "--dsdata=1 1 1 abcd");
+        "--dsdata=1 1 1 abcd",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_with_client_hold.xml");
+        .verifySent("uniform_rapid_suspension_with_client_hold.xml")
+        .verifyNoMoreSent();
     assertInStdout("uniform_rapid_suspension --undo");
     assertInStdout("--domain_name evil.tld");
     assertInStdout("--hosts ns1.example.com,ns2.example.com");
@@ -146,54 +164,62 @@ class UniformRapidSuspensionCommandTest
 
   @Test
   void testUndo_removesLocksReplacesHostsAndDsData() throws Exception {
-    persistDomainWithHosts(urs1, urs2);
+    persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
     runCommandForced(
-        "--domain_name=evil.tld", "--undo", "--hosts=ns1.example.com,ns2.example.com");
+        "--domain_name=evil.tld",
+        "--undo",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_undo.xml");
+        .verifySent("uniform_rapid_suspension_undo.xml")
+        .verifyNoMoreSent();
     assertNotInStdout("--undo");  // Undo shouldn't print a new undo command.
   }
 
   @Test
   void testUndo_respectsLocksToPreserveFlag() throws Exception {
-    persistDomainWithHosts(urs1, urs2);
+    persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
     runCommandForced(
         "--domain_name=evil.tld",
         "--undo",
         "--locks_to_preserve=serverDeleteProhibited",
-        "--hosts=ns1.example.com,ns2.example.com");
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_undo_preserve.xml");
+        .verifySent("uniform_rapid_suspension_undo_preserve.xml")
+        .verifyNoMoreSent();
     assertNotInStdout("--undo");  // Undo shouldn't print a new undo command.
   }
 
   @Test
   void testUndo_restoresClientHolds() throws Exception {
-    persistDomainWithHosts(urs1, urs2);
+    persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
     runCommandForced(
         "--domain_name=evil.tld",
         "--undo",
         "--hosts=ns1.example.com,ns2.example.com",
-        "--restore_client_hold");
+        "--restore_client_hold",
+        "--renew_one_year=false");
     eppVerifier
         .expectRegistrarId("CharlestonRoad")
         .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_undo_client_hold.xml");
+        .verifySent("uniform_rapid_suspension_undo_client_hold.xml")
+        .verifyNoMoreSent();
     assertNotInStdout("--undo"); // Undo shouldn't print a new undo command.
   }
 
   @Test
-  void testAutorenews_setToFalsebyDefault() throws Exception {
+  void testAutorenews_setToFalseByDefault() throws Exception {
     persistResource(
         newDomainBase("evil.tld")
             .asBuilder()
             .addStatusValue(StatusValue.SERVER_DELETE_PROHIBITED)
             .build());
-    runCommandForced("--domain_name=evil.tld");
+    runCommandForced("--domain_name=evil.tld", "--renew_one_year=false");
     eppVerifier.verifySentAny();
     assertInStdout("<superuser:autorenews>false</superuser:autorenews>");
   }
@@ -209,11 +235,132 @@ class UniformRapidSuspensionCommandTest
         "--domain_name=evil.tld",
         "--undo",
         "--hosts=ns1.example.com,ns2.example.com",
-        "--restore_client_hold");
+        "--restore_client_hold",
+        "--renew_one_year=false");
     eppVerifier.verifySentAny();
     assertInStdout("<superuser:autorenews>true</superuser:autorenews>");
   }
 
+  @Test
+  void testRenewOneYearWithoutUndo_verifyReasonWithoutUndo() throws Exception {
+    persistDomainWithHosts(
+        newDomainBase("evil.tld")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.parse("2021-10-01T05:01:11Z"))
+            .setRegistrationExpirationTime(DateTime.parse("2022-10-01T05:01:11Z"))
+            .setPersistedCurrentSponsorRegistrarId("CharlestonRoad")
+            .build(),
+        defaultDsData,
+        urs1,
+        urs2);
+
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=true");
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"))
+        .verifySentAny();
+  }
+
+  @Test
+  void testRenewOneYearWithUndo_verifyReasonWithUndo() throws Exception {
+    persistDomainWithHosts(
+        newDomainBase("evil.tld")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.parse("2021-10-01T05:01:11Z"))
+            .setRegistrationExpirationTime(DateTime.parse("2022-10-01T05:01:11Z"))
+            .setPersistedCurrentSponsorRegistrarId("CharlestonRoad")
+            .build(),
+        defaultDsData,
+        urs1,
+        urs2);
+
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--undo",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=true");
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Undo Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"))
+        .verifySentAny();
+  }
+
+  @Test
+  void testRenewOneYear_verifyBothRenewAndUpdateFlowsAreTriggered() throws Exception {
+    persistDomainWithHosts(
+        newDomainBase("evil.tld")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.parse("2021-10-01T05:01:11Z"))
+            .setRegistrationExpirationTime(DateTime.parse("2022-10-01T05:01:11Z"))
+            .setPersistedCurrentSponsorRegistrarId("CharlestonRoad")
+            .build(),
+        defaultDsData,
+        urs1,
+        urs2);
+
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--undo",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=true");
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Undo Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"));
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent("uniform_rapid_suspension_undo.xml");
+
+    // verify that no other flows are triggered after the renew and update flows
+    eppVerifier.verifyNoMoreSent();
+  }
+
   @Test
   void testFailure_locksToPreserveWithoutUndo() {
     persistActiveDomain("evil.tld");
@@ -222,7 +369,9 @@ class UniformRapidSuspensionCommandTest
             IllegalArgumentException.class,
             () ->
                 runCommandForced(
-                    "--domain_name=evil.tld", "--locks_to_preserve=serverDeleteProhibited"));
+                    "--domain_name=evil.tld",
+                    "--locks_to_preserve=serverDeleteProhibited",
+                    "--renew_one_year=false"));
     assertThat(thrown).hasMessageThat().contains("--undo");
   }
 
@@ -232,17 +381,29 @@ class UniformRapidSuspensionCommandTest
     ParameterException thrown =
         assertThrows(
             ParameterException.class,
-            () -> runCommandForced("--hosts=urs1.example.com,urs2.example.com"));
+            () ->
+                runCommandForced(
+                    "--hosts=urs1.example.com,urs2.example.com", "--renew_one_year=false"));
     assertThat(thrown).hasMessageThat().contains("--domain_name");
   }
 
+  @Test
+  void testFailure_renewOneYearRequired() {
+    persistActiveDomain("evil.tld");
+    ParameterException thrown =
+        assertThrows(ParameterException.class, () -> runCommandForced("--domain_name=evil.tld"));
+    assertThat(thrown).hasMessageThat().contains("--renew_one_year");
+  }
+
   @Test
   void testFailure_extraFieldInDsData() {
     persistActiveDomain("evil.tld");
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
-            () -> runCommandForced("--domain_name=evil.tld", "--dsdata=1 1 1 abc 1"));
+            () ->
+                runCommandForced(
+                    "--domain_name=evil.tld", "--dsdata=1 1 1 abc 1", "--renew_one_year=false"));
     assertThat(thrown)
         .hasMessageThat()
         .contains("dsRecord 1 1 1 abc 1 should have 4 parts, but has 5");
@@ -254,7 +415,9 @@ class UniformRapidSuspensionCommandTest
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
-            () -> runCommandForced("--domain_name=evil.tld", "--dsdata=1 1 1"));
+            () ->
+                runCommandForced(
+                    "--domain_name=evil.tld", "--dsdata=1 1 1", "--renew_one_year=false"));
     assertThat(thrown).hasMessageThat().contains("dsRecord 1 1 1 should have 4 parts, but has 3");
   }
 
@@ -264,7 +427,9 @@ class UniformRapidSuspensionCommandTest
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
-            () -> runCommandForced("--domain_name=evil.tld", "--dsdata=1,2,3"));
+            () ->
+                runCommandForced(
+                    "--domain_name=evil.tld", "--dsdata=1,2,3", "--renew_one_year=false"));
     assertThat(thrown).hasMessageThat().contains("dsRecord 1 should have 4 parts, but has 1");
   }
 }

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

@@ -25,12 +25,15 @@ import static google.registry.testing.DatabaseHelper.newDomainBase;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
 import static google.registry.testing.DatabaseHelper.persistActiveHost;
 import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.testing.TestLogHandlerUtils.assertLogMessage;
+import static google.registry.testing.TestLogHandlerUtils.assertNoLogMessage;
 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 com.google.common.flogger.LoggerConfig;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Flag;
 import google.registry.model.billing.BillingEvent.Reason;
@@ -46,6 +49,9 @@ import google.registry.persistence.VKey;
 import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.InjectExtension;
 import google.registry.testing.TestOfyAndSql;
+import google.registry.util.CapturingLogHandler;
+import java.util.logging.Level;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
@@ -53,6 +59,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 @DualDatabaseTest
 class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand> {
 
+  private final CapturingLogHandler logHandler = new CapturingLogHandler();
+
   private DomainBase domain;
 
   @RegisterExtension public final InjectExtension inject = new InjectExtension();
@@ -62,6 +70,12 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
     inject.setStaticField(Ofy.class, "clock", fakeClock);
     command.clock = fakeClock;
     domain = persistActiveDomain("example.tld");
+    LoggerConfig.getConfig(UpdateDomainCommand.class).addHandler(logHandler);
+  }
+
+  @AfterEach
+  void afterEach() {
+    LoggerConfig.getConfig(UpdateDomainCommand.class).removeHandler(logHandler);
   }
 
   @TestOfyAndSql
@@ -299,7 +313,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
     runCommandForced("--client=NewRegistrar", "--autorenews=false", "example.tld");
     eppVerifier.verifySent(
         "domain_update_set_autorenew.xml", ImmutableMap.of("AUTORENEWS", "false"));
-    assertThat(getStderrAsString()).doesNotContain("autorenew grace period");
+    assertNoLogMessage(logHandler, Level.WARNING, "autorenew grace period");
   }
 
   @TestOfyAndSql
@@ -340,9 +354,9 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
     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");
+    assertLogMessage(
+        logHandler, Level.WARNING, "The following domains are in autorenew grace periods.");
+    assertLogMessage(logHandler, Level.WARNING, "example.tld");
   }
 
   @TestOfyAndSql

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

@@ -0,0 +1,19 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:reason>%REASON%</metadata:reason>
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>

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

@@ -0,0 +1,18 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+        <domain:curExpDate>2000-04-03</domain:curExpDate>
+        <domain:period unit="y">1</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:requestedByRegistrar>true</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>

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

@@ -0,0 +1,19 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:reason>%REASON%</metadata:reason>
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>RegistryTool</clTRID>
+  </command>
+</epp>

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

@@ -0,0 +1,18 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>RegistryTool</clTRID>
+  </command>
+</epp>

util/build.gradle

@@ -43,12 +43,16 @@ dependencies {
   testCompile deps['com.google.appengine:appengine-api-stubs']
   testCompile deps['com.google.guava:guava-testlib']
   testCompile deps['com.google.truth:truth']
+  testCompile deps['junit:junit']
   testCompile deps['org.junit.jupiter:junit-jupiter-api']
   testCompile deps['org.junit.jupiter:junit-jupiter-engine']
+  testCompile deps['org.junit.platform:junit-platform-runner']
+  testCompile deps['org.junit.platform:junit-platform-suite-api']
   testCompile deps['org.hamcrest:hamcrest']
   testCompile deps['org.hamcrest:hamcrest-core']
   testCompile deps['org.mockito:mockito-core']
   testCompile deps['org.mockito:mockito-junit-jupiter']
+  testCompile deps['org.testcontainers:junit-jupiter']
   testCompile files("${rootDir}/third_party/objectify/v4_1/objectify-4.1.3.jar")
   testCompile project(path: ':common', configuration: 'testing')
   testRuntime deps['com.google.flogger:flogger-system-backend']
@@ -57,3 +61,7 @@ dependencies {
   testAnnotationProcessor deps['com.google.auto.value:auto-value']
   testAnnotationProcessor deps['com.google.dagger:dagger-compiler']
 }
+
+test {
+  useJUnitPlatform()
+}

util/gradle/dependency-locks/testCompile.lockfile

@@ -1,6 +1,10 @@
 # This is a Gradle generated file for dependency locking.
 # Manual edits can break the build and are not advised.
 # This file is expected to be part of source control.
+com.fasterxml.jackson.core:jackson-annotations:2.10.3
+com.github.docker-java:docker-java-api:3.2.7
+com.github.docker-java:docker-java-transport-zerodep:3.2.7
+com.github.docker-java:docker-java-transport:3.2.7
 com.google.android:annotations:4.1.1.4
 com.google.api-client:google-api-client:1.31.3
 com.google.api.grpc:proto-google-cloud-tasks-v2:1.33.2
@@ -61,6 +65,8 @@ joda-time:joda-time:2.9.2
 junit:junit:4.13.2
 net.bytebuddy:byte-buddy-agent:1.10.19
 net.bytebuddy:byte-buddy:1.10.19
+net.java.dev.jna:jna:5.5.0
+org.apache.commons:commons-compress:1.20
 org.apache.httpcomponents:httpclient:4.5.13
 org.apache.httpcomponents:httpcore:4.4.14
 org.apiguardian:apiguardian-api:1.1.0
@@ -76,11 +82,19 @@ org.junit.jupiter:junit-jupiter-api:5.7.0
 org.junit.jupiter:junit-jupiter-engine:5.7.0
 org.junit.platform:junit-platform-commons:1.7.0
 org.junit.platform:junit-platform-engine:1.7.0
+org.junit.platform:junit-platform-launcher:1.7.0
+org.junit.platform:junit-platform-runner:1.7.0
+org.junit.platform:junit-platform-suite-api:1.7.0
 org.junit:junit-bom:5.7.0
 org.mockito:mockito-core:3.7.7
 org.mockito:mockito-junit-jupiter:3.7.7
 org.objenesis:objenesis:3.1
 org.opentest4j:opentest4j:1.2.0
 org.ow2.asm:asm:9.0
+org.rnorth.duct-tape:duct-tape:1.0.8
+org.rnorth.visible-assertions:visible-assertions:2.1.2
+org.slf4j:slf4j-api:1.7.30
+org.testcontainers:junit-jupiter:1.15.2
+org.testcontainers:testcontainers:1.15.2
 org.threeten:threetenbp:1.5.1
 org.yaml:snakeyaml:1.17

util/gradle/dependency-locks/testCompileClasspath.lockfile

@@ -1,6 +1,10 @@
 # This is a Gradle generated file for dependency locking.
 # Manual edits can break the build and are not advised.
 # This file is expected to be part of source control.
+com.fasterxml.jackson.core:jackson-annotations:2.10.3
+com.github.docker-java:docker-java-api:3.2.7
+com.github.docker-java:docker-java-transport-zerodep:3.2.7
+com.github.docker-java:docker-java-transport:3.2.7
 com.google.api-client:google-api-client:1.31.3
 com.google.api.grpc:proto-google-cloud-tasks-v2:1.33.2
 com.google.api.grpc:proto-google-cloud-tasks-v2beta2:0.89.2
@@ -58,6 +62,8 @@ joda-time:joda-time:2.9.2
 junit:junit:4.13.2
 net.bytebuddy:byte-buddy-agent:1.10.19
 net.bytebuddy:byte-buddy:1.10.19
+net.java.dev.jna:jna:5.5.0
+org.apache.commons:commons-compress:1.20
 org.apache.httpcomponents:httpclient:4.5.13
 org.apache.httpcomponents:httpcore:4.4.14
 org.apiguardian:apiguardian-api:1.1.0
@@ -68,15 +74,23 @@ org.checkerframework:checker-qual:3.9.1
 org.conscrypt:conscrypt-openjdk-uber:2.5.1
 org.hamcrest:hamcrest-core:2.2
 org.hamcrest:hamcrest:2.2
-org.junit.jupiter:junit-jupiter-api:5.6.2
-org.junit.jupiter:junit-jupiter-engine:5.6.2
-org.junit.platform:junit-platform-commons:1.6.2
-org.junit.platform:junit-platform-engine:1.6.2
-org.junit:junit-bom:5.6.2
+org.junit.jupiter:junit-jupiter-api:5.7.0
+org.junit.jupiter:junit-jupiter-engine:5.7.0
+org.junit.platform:junit-platform-commons:1.7.0
+org.junit.platform:junit-platform-engine:1.7.0
+org.junit.platform:junit-platform-launcher:1.7.0
+org.junit.platform:junit-platform-runner:1.7.0
+org.junit.platform:junit-platform-suite-api:1.7.0
+org.junit:junit-bom:5.7.0
 org.mockito:mockito-core:3.7.7
 org.mockito:mockito-junit-jupiter:3.7.7
 org.objenesis:objenesis:3.1
 org.opentest4j:opentest4j:1.2.0
 org.ow2.asm:asm:9.0
+org.rnorth.duct-tape:duct-tape:1.0.8
+org.rnorth.visible-assertions:visible-assertions:2.1.2
+org.slf4j:slf4j-api:1.7.30
+org.testcontainers:junit-jupiter:1.15.2
+org.testcontainers:testcontainers:1.15.2
 org.threeten:threetenbp:1.5.1
 org.yaml:snakeyaml:1.17

util/gradle/dependency-locks/testRuntime.lockfile

@@ -1,6 +1,10 @@
 # This is a Gradle generated file for dependency locking.
 # Manual edits can break the build and are not advised.
 # This file is expected to be part of source control.
+com.fasterxml.jackson.core:jackson-annotations:2.10.3
+com.github.docker-java:docker-java-api:3.2.7
+com.github.docker-java:docker-java-transport-zerodep:3.2.7
+com.github.docker-java:docker-java-transport:3.2.7
 com.google.android:annotations:4.1.1.4
 com.google.api-client:google-api-client:1.31.3
 com.google.api.grpc:proto-google-cloud-tasks-v2:1.33.2
@@ -63,6 +67,8 @@ joda-time:joda-time:2.9.2
 junit:junit:4.13.2
 net.bytebuddy:byte-buddy-agent:1.10.19
 net.bytebuddy:byte-buddy:1.10.19
+net.java.dev.jna:jna:5.5.0
+org.apache.commons:commons-compress:1.20
 org.apache.httpcomponents:httpclient:4.5.13
 org.apache.httpcomponents:httpcore:4.4.14
 org.apiguardian:apiguardian-api:1.1.0
@@ -78,11 +84,19 @@ org.junit.jupiter:junit-jupiter-api:5.7.0
 org.junit.jupiter:junit-jupiter-engine:5.7.0
 org.junit.platform:junit-platform-commons:1.7.0
 org.junit.platform:junit-platform-engine:1.7.0
+org.junit.platform:junit-platform-launcher:1.7.0
+org.junit.platform:junit-platform-runner:1.7.0
+org.junit.platform:junit-platform-suite-api:1.7.0
 org.junit:junit-bom:5.7.0
 org.mockito:mockito-core:3.7.7
 org.mockito:mockito-junit-jupiter:3.7.7
 org.objenesis:objenesis:3.1
 org.opentest4j:opentest4j:1.2.0
 org.ow2.asm:asm:9.0
+org.rnorth.duct-tape:duct-tape:1.0.8
+org.rnorth.visible-assertions:visible-assertions:2.1.2
+org.slf4j:slf4j-api:1.7.30
+org.testcontainers:junit-jupiter:1.15.2
+org.testcontainers:testcontainers:1.15.2
 org.threeten:threetenbp:1.5.1
 org.yaml:snakeyaml:1.17

util/gradle/dependency-locks/testRuntimeClasspath.lockfile

@@ -1,6 +1,10 @@
 # This is a Gradle generated file for dependency locking.
 # Manual edits can break the build and are not advised.
 # This file is expected to be part of source control.
+com.fasterxml.jackson.core:jackson-annotations:2.10.3
+com.github.docker-java:docker-java-api:3.2.7
+com.github.docker-java:docker-java-transport-zerodep:3.2.7
+com.github.docker-java:docker-java-transport:3.2.7
 com.google.android:annotations:4.1.1.4
 com.google.api-client:google-api-client:1.31.3
 com.google.api.grpc:proto-google-cloud-tasks-v2:1.33.2
@@ -63,6 +67,8 @@ joda-time:joda-time:2.9.2
 junit:junit:4.13.2
 net.bytebuddy:byte-buddy-agent:1.10.19
 net.bytebuddy:byte-buddy:1.10.19
+net.java.dev.jna:jna:5.5.0
+org.apache.commons:commons-compress:1.20
 org.apache.httpcomponents:httpclient:4.5.13
 org.apache.httpcomponents:httpcore:4.4.14
 org.apiguardian:apiguardian-api:1.1.0
@@ -78,11 +84,19 @@ org.junit.jupiter:junit-jupiter-api:5.7.0
 org.junit.jupiter:junit-jupiter-engine:5.7.0
 org.junit.platform:junit-platform-commons:1.7.0
 org.junit.platform:junit-platform-engine:1.7.0
+org.junit.platform:junit-platform-launcher:1.7.0
+org.junit.platform:junit-platform-runner:1.7.0
+org.junit.platform:junit-platform-suite-api:1.7.0
 org.junit:junit-bom:5.7.0
 org.mockito:mockito-core:3.7.7
 org.mockito:mockito-junit-jupiter:3.7.7
 org.objenesis:objenesis:3.1
 org.opentest4j:opentest4j:1.2.0
 org.ow2.asm:asm:9.0
+org.rnorth.duct-tape:duct-tape:1.0.8
+org.rnorth.visible-assertions:visible-assertions:2.1.2
+org.slf4j:slf4j-api:1.7.30
+org.testcontainers:junit-jupiter:1.15.2
+org.testcontainers:testcontainers:1.15.2
 org.threeten:threetenbp:1.5.1
 org.yaml:snakeyaml:1.17

util/src/test/java/google/registry/util/DomainNameUtilsTest.java

@@ -25,7 +25,7 @@ import org.junit.jupiter.api.Test;
 class DomainNameUtilsTest {
 
   @Test
-  void testCanonicalizeDomainName() {
+  void testCanonicalizeDomainName_succeeds() {
     assertThat(canonicalizeDomainName("foo")).isEqualTo("foo");
     assertThat(canonicalizeDomainName("FOO")).isEqualTo("foo");
     assertThat(canonicalizeDomainName("foo.tld")).isEqualTo("foo.tld");
@@ -38,6 +38,21 @@ class DomainNameUtilsTest {
     assertThat(canonicalizeDomainName("ħ")).isEqualTo("xn--1ea");
   }
 
+  @Test
+  void testCanonicalizeDomainName_allowsRdnsNames() {
+    assertThat(canonicalizeDomainName("119.63.227.45-ns1.jhz-tt.uk"))
+        .isEqualTo("119.63.227.45-ns1.jhz-tt.uk");
+  }
+
+  @Test
+  void testCanonicalizeDomainName_throwsOn34HyphenRule() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> canonicalizeDomainName("119.63.227.45--ns1.jhz-tt.uk"));
+    assertThat(thrown).hasCauseThat().hasMessageThat().contains("HYPHEN_3_4");
+  }
+
   @Test
   void testCanonicalizeDomainName_acePrefixUnicodeChars() {
     assertThrows(IllegalArgumentException.class, () -> canonicalizeDomainName("xn--みんな"));