Convert DatastoreEntity/SqlEntity to return Optional, not List (#894 )

* Convert DatastoreEntity/SqlEntity to return Optional, not List We don't have any entities that convert to more than one entity, so we can use an Optional instead for clarity and simplicity.
Add an HTTP header to response from Nomulus after successful login (#879 )
2026-06-09 16:33:02 +00:00 · 2020-12-02 17:29:01 -05:00 · 2020-12-01 19:24:56 -05:00 · 2020-12-01 14:28:45 -05:00 · 2020-12-01 10:14:05 -05:00 · 2020-11-30 18:06:07 -05:00
80 changed files with 1164 additions and 774 deletions
@@ -74,6 +74,14 @@ public class EppRequestHandler {
          && eppOutput.getResponse().getResult().getCode() == SUCCESS_AND_CLOSE) {
        response.setHeader("Epp-Session", "close");
      }
+      // If a login request returns a success, a logged-in header is added to the response to inform
+      // the proxy that it is no longer necessary to send the full client certificate to the backend
+      // for this connection.
+      if (eppOutput.isResponse()
+          && eppOutput.getResponse().isLoginResponse()
+          && eppOutput.isSuccess()) {
+        response.setHeader("Logged-In", "true");
+      }
    } catch (Exception e) {
      logger.atWarning().withCause(e).log("handleEppCommand general exception");
      response.setStatus(SC_BAD_REQUEST);
@@ -141,7 +141,7 @@ public class LoginFlow implements Flow {
    sessionMetadata.resetFailedLoginAttempts();
    sessionMetadata.setClientId(login.getClientId());
    sessionMetadata.setServiceExtensionUris(serviceExtensionUrisBuilder.build());
-    return responseBuilder.build();
+    return responseBuilder.setIsLoginResponse().build();
  }

  /** Registrar with this client ID could not be found. */
@@ -87,7 +87,7 @@ public class ModelUtils {
              });

  /** Lists all instance fields on an object, including non-public and inherited fields. */
-  static Map<String, Field> getAllFields(Class<?> clazz) {
+  public static Map<String, Field> getAllFields(Class<?> clazz) {
    return ALL_FIELDS_CACHE.getUnchecked(clazz);
  }

@@ -211,11 +211,6 @@ public final class OteAccountBuilder {
    return transformRegistrars(builder -> builder.setPassword(password));
  }

-  /** Sets the client certificate hash to all the OT&amp;E Registrars. */
-  public OteAccountBuilder setCertificateHash(String certHash) {
-    return transformRegistrars(builder -> builder.setClientCertificateHash(certHash));
-  }
-
  /** Sets the client certificate to all the OT&amp;E Registrars. */
  public OteAccountBuilder setCertificate(String asciiCert, DateTime now) {
    return transformRegistrars(builder -> builder.setClientCertificate(asciiCert, now));
@@ -24,7 +24,6 @@ import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -50,8 +49,7 @@ import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithLongVKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -683,7 +681,7 @@ public abstract class BillingEvent extends ImmutableObject
  @ReportedOn
  @Entity
  @WithLongVKey
-  public static class Modification extends BillingEvent implements DatastoreEntity {
+  public static class Modification extends BillingEvent implements DatastoreOnlyEntity {

    /** The change in cost that should be applied to the original billing event. */
    Money cost;
@@ -745,11 +743,6 @@ public abstract class BillingEvent extends ImmutableObject
          .build();
    }

-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // not persisted in SQL
-    }
-
    /** A builder for {@link Modification} since it is immutable. */
    public static class Builder extends BillingEvent.Builder<Modification, Builder> {

@@ -21,7 +21,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;

 import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
@@ -29,8 +28,7 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
 import google.registry.model.UpdateAutoTimestamp;
 import google.registry.model.registry.Registry;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import java.util.List;
 import org.joda.time.DateTime;

@@ -40,7 +38,7 @@ import org.joda.time.DateTime;
 * scoped on {@link EntityGroupRoot}.
 */
@Entity
-public class Cursor extends ImmutableObject implements DatastoreEntity {
+public class Cursor extends ImmutableObject implements DatastoreOnlyEntity {

  /** The types of cursors, used as the string id field for each cursor in Datastore. */
  public enum CursorType {
@@ -137,11 +135,6 @@ public class Cursor extends ImmutableObject implements DatastoreEntity {
    return CursorType.valueOf(String.join("_", id.subList(1, id.size())));
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // Cursors are not converted since they are ephemeral
-  }
-
  /**
   * Checks that the type of the scoped object (or null) matches the required type for the specified
   * cursor (or null, if the cursor is a global cursor).
@@ -14,13 +14,11 @@

 package google.registry.model.common;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
 import google.registry.model.BackupGroupRoot;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;

 /**
 * The root key for the entity group which is known as the cross-tld entity group for historical
@@ -37,7 +35,7 @@ import google.registry.schema.replay.SqlEntity;
 * entity group for the single namespace where global data applicable for all TLDs lived.
 */
@Entity
-public class EntityGroupRoot extends BackupGroupRoot implements DatastoreEntity {
+public class EntityGroupRoot extends BackupGroupRoot implements DatastoreOnlyEntity {

  @SuppressWarnings("unused")
  @Id
@@ -47,9 +45,4 @@ public class EntityGroupRoot extends BackupGroupRoot implements DatastoreEntity
  public static Key<EntityGroupRoot> getCrossTldKey() {
    return Key.create(EntityGroupRoot.class, "cross-tld");
  }
-
-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
 }
@@ -14,7 +14,6 @@

 package google.registry.model.contact;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.EntitySubclass;
 import google.registry.model.ImmutableObject;
@@ -95,9 +94,9 @@ public class ContactHistory extends HistoryEntry implements SqlEntity {
  }

  /** Creates a {@link VKey} instance for this entity. */
+  @SuppressWarnings("unchecked")
  public VKey<ContactHistory> createVKey() {
-    return VKey.create(
-        ContactHistory.class, new ContactHistoryId(getContactRepoId(), getId()), Key.create(this));
+    return (VKey<ContactHistory>) createVKey(Key.create(this));
  }

  @PostLoad
@@ -111,12 +110,12 @@ public class ContactHistory extends HistoryEntry implements SqlEntity {

  // In Datastore, save as a HistoryEntry object regardless of this object's type
  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(asHistoryEntry());
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.of(asHistoryEntry());
  }

  /** Class to represent the composite primary key of {@link ContactHistory} entity. */
-  static class ContactHistoryId extends ImmutableObject implements Serializable {
+  public static class ContactHistoryId extends ImmutableObject implements Serializable {

    private String contactRepoId;

@@ -125,7 +124,7 @@ public class ContactHistory extends HistoryEntry implements SqlEntity {
    /** Hibernate requires this default constructor. */
    private ContactHistoryId() {}

-    ContactHistoryId(String contactRepoId, long id) {
+    public ContactHistoryId(String contactRepoId, long id) {
      this.contactRepoId = contactRepoId;
      this.id = id;
    }
@@ -311,6 +311,9 @@ public class DomainContent extends EppResource
        nullToEmptyImmutableCopy(gracePeriods).stream()
            .map(gracePeriod -> gracePeriod.cloneAfterOfyLoad(getRepoId()))
            .collect(toImmutableSet());
+    // TODO(b/169873747): Remove this method after explicitly re-saving all domain entities.
+    // See also: GradePeriod.onLoad.
+    gracePeriods.forEach(GracePeriod::onLoad);

    // Restore history record ids.
    autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage);
@@ -17,7 +17,6 @@ package google.registry.model.domain;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;

-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.EntitySubclass;
@@ -101,6 +100,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
  @Column(name = "host_repo_id")
  Set<VKey<HostResource>> nsHosts;

+  @Ignore
  @OneToMany(
      cascade = {CascadeType.ALL},
      fetch = FetchType.EAGER,
@@ -117,8 +117,9 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
        insertable = false,
        updatable = false)
  })
-  Set<DomainDsDataHistory> dsDataHistories;
+  Set<DomainDsDataHistory> dsDataHistories = ImmutableSet.of();

+  @Ignore
  @OneToMany(
      cascade = {CascadeType.ALL},
      fetch = FetchType.EAGER,
@@ -135,18 +136,14 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
        insertable = false,
        updatable = false)
  })
-  Set<GracePeriodHistory> gracePeriodHistories;
+  Set<GracePeriodHistory> gracePeriodHistories = ImmutableSet.of();

  @Override
  @Nullable
  @Access(AccessType.PROPERTY)
  @AttributeOverrides({
-      @AttributeOverride(
-          name = "unit",
-          column = @Column(name = "historyPeriodUnit")),
-      @AttributeOverride(
-          name = "value",
-          column = @Column(name = "historyPeriodValue"))
+    @AttributeOverride(name = "unit", column = @Column(name = "historyPeriodUnit")),
+    @AttributeOverride(name = "value", column = @Column(name = "historyPeriodValue"))
  })
  public Period getPeriod() {
    return super.getPeriod();
@@ -229,9 +226,9 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
  }

  /** Creates a {@link VKey} instance for this entity. */
+  @SuppressWarnings("unchecked")
  public VKey<DomainHistory> createVKey() {
-    return VKey.create(
-        DomainHistory.class, new DomainHistoryId(getDomainRepoId(), getId()), Key.create(this));
+    return (VKey<DomainHistory>) createVKey(Key.create(this));
  }

  @PostLoad
@@ -252,8 +249,8 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {

  // In Datastore, save as a HistoryEntry object regardless of this object's type
  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(asHistoryEntry());
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.of(asHistoryEntry());
  }

  /** Class to represent the composite primary key of {@link DomainHistory} entity. */
@@ -19,7 +19,6 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

 import com.google.common.annotations.VisibleForTesting;
 import com.googlecode.objectify.annotation.Embed;
-import com.googlecode.objectify.annotation.OnLoad;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Recurring;
 import google.registry.model.domain.rgp.GracePeriodStatus;
@@ -54,7 +53,10 @@ public class GracePeriod extends GracePeriodBase implements DatastoreAndSqlEntit
  }

  // TODO(b/169873747): Remove this method after explicitly re-saving all domain entities.
-  @OnLoad
+  // This method is invoked from DomainContent.load(): Objectify's @OnLoad annotation
+  // apparently does not work on embedded objects inside an entity.
+  // Changing signature to void onLoad(@AlsoLoad("gracePeriodId") Long gracePeriodId)
+  // would not work. Method is not called if gracePeriodId is null.
  void onLoad() {
    if (gracePeriodId == null) {
      gracePeriodId = ObjectifyService.allocateId();
@@ -14,11 +14,11 @@

 package google.registry.model.domain.secdns;

-import com.google.common.collect.ImmutableList;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.ofy.ObjectifyService;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
+import java.util.Optional;
 import javax.persistence.Access;
 import javax.persistence.AccessType;
 import javax.persistence.Column;
@@ -86,7 +86,7 @@ public class DomainDsDataHistory extends DomainDsDataBase implements SqlEntity {
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // not persisted in Datastore
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Not persisted in Datastore
  }
 }
@@ -65,6 +65,7 @@ import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlElementRef;
 import javax.xml.bind.annotation.XmlElementRefs;
 import javax.xml.bind.annotation.XmlElementWrapper;
+import javax.xml.bind.annotation.XmlTransient;
 import javax.xml.bind.annotation.XmlType;

 /**
@@ -87,6 +88,9 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
  /** The command result. The RFC allows multiple failure results, but we always return one. */
  Result result;

+  /** Indicates if this response is for a login request. */
+  @XmlTransient boolean isLoginResponse = false;
+
  /**
   * Information about messages queued for retrieval. This may appear in response to any EPP message
   * (if messages are queued), but in practice this will only be set in response to a poll request.
@@ -178,6 +182,10 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
    return result;
  }

+  public boolean isLoginResponse() {
+    return isLoginResponse;
+  }
+
  /** Marker interface for types that can go in the {@link #resData} field. */
  public interface ResponseData {}

@@ -222,5 +230,10 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
      getInstance().extensions = forceEmptyToNull(extensions);
      return this;
    }
+
+    public Builder setIsLoginResponse() {
+      getInstance().isLoginResponse = true;
+      return this;
+    }
  }
 }
@@ -14,7 +14,6 @@

 package google.registry.model.host;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.EntitySubclass;
 import google.registry.model.ImmutableObject;
@@ -96,9 +95,9 @@ public class HostHistory extends HistoryEntry implements SqlEntity {
  }

  /** Creates a {@link VKey} instance for this entity. */
+  @SuppressWarnings("unchecked")
  public VKey<HostHistory> createVKey() {
-    return VKey.create(
-        HostHistory.class, new HostHistoryId(getHostRepoId(), getId()), Key.create(this));
+    return (VKey<HostHistory>) createVKey(Key.create(this));
  }

  @PostLoad
@@ -112,12 +111,12 @@ public class HostHistory extends HistoryEntry implements SqlEntity {

  // In Datastore, save as a HistoryEntry object regardless of this object's type
  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(asHistoryEntry());
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.of(asHistoryEntry());
  }

  /** Class to represent the composite primary key of {@link HostHistory} entity. */
-  static class HostHistoryId extends ImmutableObject implements Serializable {
+  public static class HostHistoryId extends ImmutableObject implements Serializable {

    private String hostRepoId;

@@ -126,7 +125,7 @@ public class HostHistory extends HistoryEntry implements SqlEntity {
    /** Hibernate requires this default constructor. */
    private HostHistoryId() {}

-    HostHistoryId(String hostRepoId, long id) {
+    public HostHistoryId(String hostRepoId, long id) {
      this.hostRepoId = hostRepoId;
      this.id = id;
    }
@@ -17,7 +17,6 @@ package google.registry.model.index;
 import static google.registry.util.TypeUtils.instantiate;

 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
@@ -26,25 +25,21 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.BackupGroupRoot;
 import google.registry.model.EppResource;
 import google.registry.model.annotations.ReportedOn;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;

 /** An index that allows for quick enumeration of all EppResource entities (e.g. via map reduce). */
@ReportedOn
@Entity
-public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity {
+public class EppResourceIndex extends BackupGroupRoot implements DatastoreOnlyEntity {

-  @Id
-  String id;
+  @Id String id;

-  @Parent
-  Key<EppResourceIndexBucket> bucket;
+  @Parent Key<EppResourceIndexBucket> bucket;

  /** Although this field holds a {@link Key} it is named "reference" for historical reasons. */
  Key<? extends EppResource> reference;

-  @Index
-  String kind;
+  @Index String kind;

  public String getId() {
    return id;
@@ -69,7 +64,7 @@ public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity
    EppResourceIndex instance = instantiate(EppResourceIndex.class);
    instance.reference = resourceKey;
    instance.kind = resourceKey.getKind();
-    instance.id = resourceKey.getString();  // creates a web-safe key string
+    instance.id = resourceKey.getString(); // creates a web-safe key string
    instance.bucket = bucket;
    return instance;
  }
@@ -77,9 +72,4 @@ public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity
  public static <T extends EppResource> EppResourceIndex create(Key<T> resourceKey) {
    return create(EppResourceIndexBucket.getBucketKey(resourceKey), resourceKey);
  }
-
-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not relevant in SQL
-  }
 }
@@ -24,23 +24,17 @@ import com.googlecode.objectify.annotation.Id;
 import google.registry.model.EppResource;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.VirtualEntity;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;

 /** A virtual entity to represent buckets to which EppResourceIndex objects are randomly added. */
@Entity
@VirtualEntity
-public class EppResourceIndexBucket extends ImmutableObject implements DatastoreEntity {
+public class EppResourceIndexBucket extends ImmutableObject implements DatastoreOnlyEntity {

  @SuppressWarnings("unused")
  @Id
  private long bucketId;

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not relevant in SQL
-  }
-
  /**
   * Deterministic function that returns a bucket id based on the resource's roid.
   * NB: At the moment, nothing depends on this being deterministic, so we have the ability to
@@ -42,8 +42,7 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.host.HostResource;
 import google.registry.persistence.VKey;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import google.registry.util.NonFinalForTesting;
 import java.util.Map;
 import java.util.Optional;
@@ -63,34 +62,19 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
  @ReportedOn
  @Entity
  public static class ForeignKeyContactIndex extends ForeignKeyIndex<ContactResource>
-      implements DatastoreEntity {
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // not relevant in SQL
-    }
-  }
+      implements DatastoreOnlyEntity {}

  /** The {@link ForeignKeyIndex} type for {@link DomainBase} entities. */
  @ReportedOn
  @Entity
  public static class ForeignKeyDomainIndex extends ForeignKeyIndex<DomainBase>
-      implements DatastoreEntity {
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // not relevant in SQL
-    }
-  }
+      implements DatastoreOnlyEntity {}

  /** The {@link ForeignKeyIndex} type for {@link HostResource} entities. */
  @ReportedOn
  @Entity
  public static class ForeignKeyHostIndex extends ForeignKeyIndex<HostResource>
-      implements DatastoreEntity {
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // not relevant in SQL
-    }
-  }
+      implements DatastoreOnlyEntity {}

  static final ImmutableMap<Class<? extends EppResource>, Class<? extends ForeignKeyIndex<?>>>
      RESOURCE_CLASS_TO_FKI_CLASS =
@@ -22,7 +22,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;

 import com.google.common.collect.ContiguousSet;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Range;
@@ -34,8 +33,7 @@ import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import google.registry.util.NonFinalForTesting;
 import java.util.Random;
 import java.util.function.Supplier;
@@ -53,7 +51,7 @@ import org.joda.time.DateTime;
 */
@Entity
@NotBackedUp(reason = Reason.COMMIT_LOGS)
-public class CommitLogBucket extends ImmutableObject implements Buildable, DatastoreEntity {
+public class CommitLogBucket extends ImmutableObject implements Buildable, DatastoreOnlyEntity {

  /**
   * Ranges from 1 to {@link RegistryConfig#getCommitLogBucketCount()}, inclusive; starts at 1 since
@@ -72,11 +70,6 @@ public class CommitLogBucket extends ImmutableObject implements Buildable, Datas
    return lastWrittenTime;
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  /**
   * Returns the key for the specified bucket ID.
   *
@@ -27,8 +27,7 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -46,7 +45,7 @@ import org.joda.time.DateTime;
 */
@Entity
@NotBackedUp(reason = Reason.COMMIT_LOGS)
-public class CommitLogCheckpoint extends ImmutableObject implements DatastoreEntity {
+public class CommitLogCheckpoint extends ImmutableObject implements DatastoreOnlyEntity {

  /** Shared singleton parent entity for commit log checkpoints. */
  @Parent
@@ -73,11 +72,6 @@ public class CommitLogCheckpoint extends ImmutableObject implements DatastoreEnt
    return builder.build();
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  /**
   * Creates a CommitLogCheckpoint for the given wall time and bucket checkpoint times, specified as
   * a map from bucket ID to bucket commit timestamp.
@@ -17,21 +17,19 @@ package google.registry.model.ofy;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import org.joda.time.DateTime;

 /** Singleton parent entity for all commit log checkpoints. */
@Entity
@NotBackedUp(reason = Reason.COMMIT_LOGS)
-public class CommitLogCheckpointRoot extends ImmutableObject implements DatastoreEntity {
+public class CommitLogCheckpointRoot extends ImmutableObject implements DatastoreOnlyEntity {

  public static final long SINGLETON_ID = 1;  // There is always exactly one of these.

@@ -50,11 +48,6 @@ public class CommitLogCheckpointRoot extends ImmutableObject implements Datastor
    return lastWrittenTime;
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  public static CommitLogCheckpointRoot loadRoot() {
    CommitLogCheckpointRoot root = ofy().load().key(getKey()).now();
    return root == null ? new CommitLogCheckpointRoot() : root;
@@ -17,7 +17,6 @@ package google.registry.model.ofy;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 import static org.joda.time.DateTimeZone.UTC;

-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
@@ -26,8 +25,7 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import org.joda.time.DateTime;
@@ -41,7 +39,7 @@ import org.joda.time.DateTime;
 */
@Entity
@NotBackedUp(reason = Reason.COMMIT_LOGS)
-public class CommitLogManifest extends ImmutableObject implements DatastoreEntity {
+public class CommitLogManifest extends ImmutableObject implements DatastoreOnlyEntity {

  /** Commit log manifests are parented on a random bucket. */
  @Parent
@@ -70,11 +68,6 @@ public class CommitLogManifest extends ImmutableObject implements DatastoreEntit
    return nullToEmptyImmutableCopy(deletions);
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  public static CommitLogManifest create(
      Key<CommitLogBucket> parent, DateTime commitTime, Set<Key<?>> deletions) {
    CommitLogManifest instance = new CommitLogManifest();
@@ -21,7 +21,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy;

 import com.google.appengine.api.datastore.KeyFactory;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
@@ -29,13 +28,12 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;

 /** Representation of a saved entity in a {@link CommitLogManifest} (not deletes). */
@Entity
@NotBackedUp(reason = Reason.COMMIT_LOGS)
-public class CommitLogMutation extends ImmutableObject implements DatastoreEntity {
+public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyEntity {

  /** The manifest this belongs to. */
  @Parent
@@ -61,11 +59,6 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreEntit
    return createFromPbBytes(entityProtoBytes);
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  /**
   * Returns a new mutation entity created from an @Entity ImmutableObject instance.
   *
@@ -29,7 +29,6 @@ import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.persistence.VKey;
 import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
 import java.util.Map;
 import org.joda.time.DateTime;

@@ -137,10 +136,9 @@ class TransactionInfo {
                        if (entry.getValue().equals(Delete.SENTINEL)) {
                          jpaTm().delete(VKey.from(entry.getKey()));
                        } else {
-                          for (SqlEntity entity :
-                              ((DatastoreEntity) entry.getValue()).toSqlEntities()) {
-                            jpaTm().put(entity);
-                          }
+                          ((DatastoreEntity) entry.getValue())
+                              .toSqlEntity()
+                              .ifPresent(jpaTm()::put);
                        }
                      });
            });
@@ -121,33 +121,34 @@ public class Registrar extends ImmutableObject
    REAL(Objects::nonNull),

    /**
-     * A registrar account used by a real third-party registrar undergoing operational testing
-     * and evaluation. Should only be created in sandbox, and should have null IANA/billing IDs.
+     * A registrar account used by a real third-party registrar undergoing operational testing and
+     * evaluation. Should only be created in sandbox, and should have null IANA/billing IDs.
     */
    OTE(Objects::isNull),

    /**
-     * A registrar used for predelegation testing.  Should have a null billing ID.  The IANA ID
-     * should be either 9995 or 9996, which are reserved for predelegation testing.
+     * A registrar used for predelegation testing. Should have a null billing ID. The IANA ID should
+     * be either 9995 or 9996, which are reserved for predelegation testing.
     */
    PDT(n -> ImmutableSet.of(9995L, 9996L).contains(n)),

    /**
-     * A registrar used for external monitoring by ICANN.  Should have IANA ID 9997 and a null
+     * A registrar used for external monitoring by ICANN. Should have IANA ID 9997 and a null
     * billing ID.
     */
    EXTERNAL_MONITORING(isEqual(9997L)),

    /**
-     * A registrar used for when the registry acts as a registrar.  Must have either IANA ID
-     * 9998 (for billable transactions) or 9999 (for non-billable transactions). */
+     * A registrar used for when the registry acts as a registrar. Must have either IANA ID 9998
+     * (for billable transactions) or 9999 (for non-billable transactions).
+     */
    // TODO(b/13786188): determine what billing ID for this should be, if any.
    INTERNAL(n -> ImmutableSet.of(9998L, 9999L).contains(n)),

-    /** A registrar used for internal monitoring.  Should have null IANA/billing IDs. */
+    /** A registrar used for internal monitoring. Should have null IANA/billing IDs. */
    MONITORING(Objects::isNull),

-    /** A registrar used for internal testing.  Should have null IANA/billing IDs. */
+    /** A registrar used for internal testing. Should have null IANA/billing IDs. */
    TEST(Objects::isNull);

    /**
@@ -225,10 +226,7 @@ public class Registrar extends ImmutableObject
   */
  private static final Supplier<ImmutableMap<String, Registrar>> CACHE_BY_CLIENT_ID =
      memoizeWithShortExpiration(
-          () ->
-              tm()
-                  .doTransactionless(
-                      () -> Maps.uniqueIndex(loadAll(), Registrar::getClientId)));
+          () -> tm().doTransactionless(() -> Maps.uniqueIndex(loadAll(), Registrar::getClientId)));

  @Parent @Transient Key<EntityGroupRoot> parent = getCrossTldKey();

@@ -383,12 +381,10 @@ public class Registrar extends ImmutableObject
  @Index @Nullable Long ianaIdentifier;

  /** Identifier of registrar used in external billing system (e.g. Oracle). */
-  @Nullable
-  Long billingIdentifier;
+  @Nullable Long billingIdentifier;

  /** Purchase Order number used for invoices in external billing system, if applicable. */
-  @Nullable
-  String poNumber;
+  @Nullable String poNumber;

  /**
   * Map of currency-to-billing account for the registrar.
@@ -498,9 +494,7 @@ public class Registrar extends ImmutableObject
    if (billingAccountMap == null) {
      return ImmutableMap.of();
    }
-    return billingAccountMap
-        .entrySet()
-        .stream()
+    return billingAccountMap.entrySet().stream()
        .collect(toImmutableSortedMap(natural(), Map.Entry::getKey, v -> v.getValue().accountId));
  }

@@ -722,14 +716,18 @@ public class Registrar extends ImmutableObject

  /** Creates a {@link VKey} for this instance. */
  public VKey<Registrar> createVKey() {
-    return VKey.create(Registrar.class, clientIdentifier, Key.create(this));
+    return createVKey(Key.create(this));
  }

  /** Creates a {@link VKey} for the given {@code registrarId}. */
  public static VKey<Registrar> createVKey(String registrarId) {
    checkArgumentNotNull(registrarId, "registrarId must be specified");
-    return VKey.create(
-        Registrar.class, registrarId, Key.create(getCrossTldKey(), Registrar.class, registrarId));
+    return createVKey(Key.create(getCrossTldKey(), Registrar.class, registrarId));
+  }
+
+  /** Creates a {@link VKey} instance from a {@link Key} instance. */
+  public static VKey<Registrar> createVKey(Key<Registrar> key) {
+    return VKey.create(Registrar.class, key.getName(), key);
  }

  /** A builder for constructing {@link Registrar}, since it is immutable. */
@@ -744,21 +742,22 @@ public class Registrar extends ImmutableObject
      // Client id must be [3,16] chars long. See "clIDType" in the base EPP schema of RFC 5730.
      // (Need to validate this here as there's no matching EPP XSD for validation.)
      checkArgument(
-          Range.closed(3,  16).contains(clientId.length()),
+          Range.closed(3, 16).contains(clientId.length()),
          "Client identifier must be 3-16 characters long.");
      getInstance().clientIdentifier = clientId;
      return this;
    }

    public Builder setIanaIdentifier(@Nullable Long ianaIdentifier) {
-      checkArgument(ianaIdentifier == null || ianaIdentifier > 0,
-          "IANA ID must be a positive number");
+      checkArgument(
+          ianaIdentifier == null || ianaIdentifier > 0, "IANA ID must be a positive number");
      getInstance().ianaIdentifier = ianaIdentifier;
      return this;
    }

    public Builder setBillingIdentifier(@Nullable Long billingIdentifier) {
-      checkArgument(billingIdentifier == null || billingIdentifier > 0,
+      checkArgument(
+          billingIdentifier == null || billingIdentifier > 0,
          "Billing ID must be a positive number");
      getInstance().billingIdentifier = billingIdentifier;
      return this;
@@ -774,9 +773,7 @@ public class Registrar extends ImmutableObject
        getInstance().billingAccountMap = null;
      } else {
        getInstance().billingAccountMap =
-            billingAccountMap
-                .entrySet()
-                .stream()
+            billingAccountMap.entrySet().stream()
                .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new));
      }
      return this;
@@ -859,26 +856,6 @@ public class Registrar extends ImmutableObject
      }
    }

-    /**
-     * Sets client certificate hash, but not the certificate.
-     *
-     * <p><b>Warning:</b> {@link #setClientCertificate(String, DateTime)} sets the hash for you and
-     * is preferred. Calling this method will nullify the {@code clientCertificate} field.
-     */
-    public Builder setClientCertificateHash(String clientCertificateHash) {
-      if (clientCertificateHash != null) {
-        checkArgument(
-            Pattern.matches("[A-Za-z0-9+/]+", clientCertificateHash),
-            "--cert_hash not a valid base64 (no padding) value");
-        checkArgument(
-            base64().decode(clientCertificateHash).length == 256 / 8,
-            "--cert_hash base64 does not decode to 256 bits");
-      }
-      getInstance().clientCertificate = null;
-      getInstance().clientCertificateHash = clientCertificateHash;
-      return this;
-    }
-
    public Builder setContactsRequireSyncing(boolean contactsRequireSyncing) {
      getInstance().contactsRequireSyncing = contactsRequireSyncing;
      return this;
@@ -900,16 +877,12 @@ public class Registrar extends ImmutableObject
    }

    public Builder setPhoneNumber(String phoneNumber) {
-      getInstance().phoneNumber = (phoneNumber == null)
-          ? null
-          : checkValidPhoneNumber(phoneNumber);
+      getInstance().phoneNumber = (phoneNumber == null) ? null : checkValidPhoneNumber(phoneNumber);
      return this;
    }

    public Builder setFaxNumber(String faxNumber) {
-      getInstance().faxNumber = (faxNumber == null)
-          ? null
-          : checkValidPhoneNumber(faxNumber);
+      getInstance().faxNumber = (faxNumber == null) ? null : checkValidPhoneNumber(faxNumber);
      return this;
    }

@@ -944,7 +917,8 @@ public class Registrar extends ImmutableObject
    }

    public Builder setDriveFolderId(@Nullable String driveFolderId) {
-      checkArgument(driveFolderId == null || !driveFolderId.contains("/"),
+      checkArgument(
+          driveFolderId == null || !driveFolderId.contains("/"),
          "Drive folder ID must not be a full URL");
      getInstance().driveFolderId = driveFolderId;
      return this;
@@ -962,9 +936,10 @@ public class Registrar extends ImmutableObject

    /** @throws IllegalArgumentException if provided passcode is not 5-digit numeric */
    public Builder setPhonePasscode(String phonePasscode) {
-      checkArgument(phonePasscode == null
-          || PHONE_PASSCODE_PATTERN.matcher(phonePasscode).matches(),
-          "Not a valid telephone passcode (must be 5 digits long): %s", phonePasscode);
+      checkArgument(
+          phonePasscode == null || PHONE_PASSCODE_PATTERN.matcher(phonePasscode).matches(),
+          "Not a valid telephone passcode (must be 5 digits long): %s",
+          phonePasscode);
      getInstance().phonePasscode = phonePasscode;
      return this;
    }
@@ -982,9 +957,11 @@ public class Registrar extends ImmutableObject
      checkArgument(
          getInstance().localizedAddress != null || getInstance().internationalizedAddress != null,
          "Must specify at least one of localized or internationalized address");
-      checkArgument(getInstance().type.isValidIanaId(getInstance().ianaIdentifier),
-          String.format("Supplied IANA ID is not valid for %s registrar type: %s",
-            getInstance().type, getInstance().ianaIdentifier));
+      checkArgument(
+          getInstance().type.isValidIanaId(getInstance().ianaIdentifier),
+          String.format(
+              "Supplied IANA ID is not valid for %s registrar type: %s",
+              getInstance().type, getInstance().ianaIdentifier));
      return cloneEmptyToNull(super.build());
    }
  }
@@ -119,9 +119,7 @@ public class RegistrarContact extends ImmutableObject
  String name;

  /** The email address of the contact. */
-  @Id
-  @javax.persistence.Id
-  String emailAddress;
+  @Id @javax.persistence.Id String emailAddress;

  @Ignore @javax.persistence.Id String registrarId;

@@ -147,8 +145,7 @@ public class RegistrarContact extends ImmutableObject
   *
   * @see com.google.appengine.api.users.User#getUserId()
   */
-  @Index
-  String gaeUserId;
+  @Index String gaeUserId;

  /**
   * Whether this contact is publicly visible in WHOIS registrar query results as an Admin contact.
@@ -202,8 +199,7 @@ public class RegistrarContact extends ImmutableObject
   */
  public static void updateContacts(
      final Registrar registrar, final Set<RegistrarContact> contacts) {
-    tm()
-        .transact(
+    tm().transact(
            () -> {
              ofy()
                  .delete()
@@ -364,8 +360,15 @@ public class RegistrarContact extends ImmutableObject
  }

  public VKey<RegistrarContact> createVKey() {
-    return VKey.create(
-        RegistrarContact.class, new RegistrarPocId(emailAddress, registrarId), Key.create(this));
+    return createVKey(Key.create(this));
+  }
+
+  /** Creates a {@link VKey} instance from a {@link Key} instance. */
+  public static VKey<RegistrarContact> createVKey(Key<RegistrarContact> key) {
+    Key<Registrar> parent = key.getParent();
+    String registrarId = parent.getName();
+    String emailAddress = key.getName();
+    return VKey.create(RegistrarContact.class, new RegistrarPocId(emailAddress, registrarId), key);
  }

  /** Class to represent the composite primary key for {@link RegistrarContact} entity. */
@@ -32,7 +32,6 @@ import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.CacheLoader.InvalidCacheLoadException;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.hash.BloomFilter;
 import com.google.common.util.concurrent.UncheckedExecutionException;
@@ -45,9 +44,8 @@ import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ReportedOn;
 import google.registry.model.registry.Registry;
-import google.registry.schema.replay.DatastoreEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import google.registry.schema.replay.NonReplicatedEntity;
-import google.registry.schema.replay.SqlEntity;
 import google.registry.schema.tld.PremiumListDao;
 import google.registry.util.NonFinalForTesting;
 import java.io.ByteArrayOutputStream;
@@ -114,7 +112,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
  /** Virtual parent entity for premium list entry entities associated with a single revision. */
  @ReportedOn
  @Entity
-  public static class PremiumListRevision extends ImmutableObject implements DatastoreEntity {
+  public static class PremiumListRevision extends ImmutableObject implements DatastoreOnlyEntity {

    @Parent Key<PremiumList> parent;

@@ -171,11 +169,6 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
      }
      return revision;
    }
-
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // not persisted in SQL
-    }
  }

  /**
@@ -332,7 +325,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
  @ReportedOn
  @Entity
  public static class PremiumListEntry extends DomainLabelEntry<Money, PremiumListEntry>
-      implements Buildable, DatastoreEntity {
+      implements Buildable, DatastoreOnlyEntity {

    @Parent
    Key<PremiumListRevision> parent;
@@ -349,11 +342,6 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
      return new Builder(clone(this));
    }

-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of();
-    }
-
    /** A builder for constructing {@link PremiumListEntry} objects, since they are immutable. */
    public static class Builder extends DomainLabelEntry.Builder<PremiumListEntry, Builder> {

@@ -18,7 +18,6 @@ import static com.googlecode.objectify.Key.getKind;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;

 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
@@ -32,15 +31,20 @@ import google.registry.model.EppResource;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ReportedOn;
 import google.registry.model.contact.ContactHistory;
+import google.registry.model.contact.ContactHistory.ContactHistoryId;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.DomainHistory;
+import google.registry.model.domain.DomainHistory.DomainHistoryId;
 import google.registry.model.domain.Period;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostHistory;
+import google.registry.model.host.HostHistory.HostHistoryId;
 import google.registry.model.host.HostResource;
+import google.registry.persistence.VKey;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
+import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
 import javax.persistence.Access;
@@ -310,8 +314,35 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor

  // In SQL, save the child type
  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of((SqlEntity) toChildHistoryEntity());
+  public Optional<SqlEntity> toSqlEntity() {
+    return Optional.of((SqlEntity) toChildHistoryEntity());
+  }
+
+  /** Creates a {@link VKey} instance from a {@link Key} instance. */
+  public static VKey<? extends HistoryEntry> createVKey(Key<HistoryEntry> key) {
+    String repoId = key.getParent().getName();
+    long id = key.getId();
+    Key<EppResource> parent = key.getParent();
+    String parentKind = parent.getKind();
+    if (parentKind.equals(getKind(DomainBase.class))) {
+      return VKey.create(
+          DomainHistory.class,
+          new DomainHistoryId(repoId, id),
+          Key.create(parent, DomainHistory.class, id));
+    } else if (parentKind.equals(getKind(HostResource.class))) {
+      return VKey.create(
+          HostHistory.class,
+          new HostHistoryId(repoId, id),
+          Key.create(parent, HostHistory.class, id));
+    } else if (parentKind.equals(getKind(ContactResource.class))) {
+      return VKey.create(
+          ContactHistory.class,
+          new ContactHistoryId(repoId, id),
+          Key.create(parent, ContactHistory.class, id));
+    } else {
+      throw new IllegalStateException(
+          String.format("Unknown kind of HistoryEntry parent %s", parentKind));
+    }
  }

  /** A builder for {@link HistoryEntry} since it is immutable */
@@ -18,13 +18,13 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static google.registry.util.CollectionUtils.isNullOrEmpty;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
 import google.registry.util.DomainNameUtils;
+import java.util.Optional;
 import java.util.Set;
 import javax.persistence.Column;
 import javax.persistence.Entity;
@@ -111,8 +111,8 @@ public class Spec11ThreatMatch extends ImmutableObject implements Buildable, Sql
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // not stored in Datastore
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Not persisted in Datastore
  }

  @Override
@@ -16,7 +16,6 @@ package google.registry.model.server;

 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
@@ -24,13 +23,12 @@ import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ReportedOn;
 import google.registry.model.common.EntityGroupRoot;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;

 /** Pointer to the latest {@link KmsSecretRevision}. */
@Entity
@ReportedOn
-public class KmsSecret extends ImmutableObject implements DatastoreEntity {
+public class KmsSecret extends ImmutableObject implements DatastoreOnlyEntity {

  /** The unique name of this {@link KmsSecret}. */
  @Id String name;
@@ -48,11 +46,6 @@ public class KmsSecret extends ImmutableObject implements DatastoreEntity {
    return latestRevision;
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // not persisted in SQL
-  }
-
  public static KmsSecret create(String name, KmsSecretRevision latestRevision) {
    KmsSecret instance = new KmsSecret();
    instance.name = name;
@@ -22,15 +22,13 @@ import static google.registry.util.DateTimeUtils.isAtOrAfter;
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.flogger.FluentLogger;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
 import google.registry.util.RequestStatusChecker;
 import google.registry.util.RequestStatusCheckerImpl;
 import java.io.Serializable;
@@ -50,7 +48,7 @@ import org.joda.time.Duration;
 */
@Entity
@NotBackedUp(reason = Reason.TRANSIENT)
-public class Lock extends ImmutableObject implements DatastoreEntity, Serializable {
+public class Lock extends ImmutableObject implements DatastoreOnlyEntity, Serializable {

  private static final long serialVersionUID = 756397280691684645L;
  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -259,9 +257,4 @@ public class Lock extends ImmutableObject implements DatastoreEntity, Serializab
              }
            });
  }
-
-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // Locks are not converted since they are ephemeral
-  }
 }
@@ -23,7 +23,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableList;
 import com.google.common.primitives.Longs;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
@@ -34,8 +33,7 @@ import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
 import google.registry.model.common.CrossTldSingleton;
 import google.registry.persistence.VKey;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.NonReplicatedEntity;
 import java.nio.ByteBuffer;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
@@ -50,7 +48,7 @@ import javax.persistence.Transient;
@Unindex
@NotBackedUp(reason = Reason.AUTO_GENERATED)
 // TODO(b/27427316): Replace this with an entry in KMSKeyring
-public class ServerSecret extends CrossTldSingleton implements DatastoreEntity, SqlEntity {
+public class ServerSecret extends CrossTldSingleton implements NonReplicatedEntity {

  /**
   * Cache of the singleton ServerSecret instance that creates it if not present.
@@ -139,16 +137,6 @@ public class ServerSecret extends CrossTldSingleton implements DatastoreEntity,
        .array();
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // dually-written
-  }
-
-  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // dually-written
-  }
-
  @VisibleForTesting
  static void resetCache() {
    CACHE.invalidateAll();
@@ -26,7 +26,6 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;

 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Supplier;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.MapDifference;
 import com.google.common.collect.MapDifference.ValueDifference;
@@ -46,9 +45,8 @@ import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
 import google.registry.model.annotations.VirtualEntity;
 import google.registry.model.common.CrossTldSingleton;
-import google.registry.schema.replay.DatastoreAndSqlEntity;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.DatastoreOnlyEntity;
+import google.registry.schema.replay.NonReplicatedEntity;
 import google.registry.util.CollectionUtils;
 import google.registry.util.Concurrent;
 import google.registry.util.Retrier;
@@ -97,7 +95,7 @@ import org.joda.time.DateTime;
@NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
@javax.persistence.Entity(name = "ClaimsList")
@Table
-public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlEntity {
+public class ClaimsListShard extends ImmutableObject implements NonReplicatedEntity {

  private static final FluentLogger logger = FluentLogger.forEnclosingClass();

@@ -146,63 +144,62 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
  private static final Retrier LOADER_RETRIER = new Retrier(new SystemSleeper(), 2);

  private static ClaimsListShard loadClaimsListShard() {
-        // Find the most recent revision.
-        Key<ClaimsListRevision> revisionKey = getCurrentRevision();
+    // Find the most recent revision.
+    Key<ClaimsListRevision> revisionKey = getCurrentRevision();

-        Map<String, String> combinedLabelsToKeys = new HashMap<>();
-        DateTime creationTime = START_OF_TIME;
-        if (revisionKey != null) {
-          // Grab all of the keys for the shards that belong to the current revision.
-          final List<Key<ClaimsListShard>> shardKeys =
-              ofy().load().type(ClaimsListShard.class).ancestor(revisionKey).keys().list();
+    Map<String, String> combinedLabelsToKeys = new HashMap<>();
+    DateTime creationTime = START_OF_TIME;
+    if (revisionKey != null) {
+      // Grab all of the keys for the shards that belong to the current revision.
+      final List<Key<ClaimsListShard>> shardKeys =
+          ofy().load().type(ClaimsListShard.class).ancestor(revisionKey).keys().list();

-          List<ClaimsListShard> shards;
-          try {
-            // Load all of the shards concurrently, each in a separate transaction.
-            shards =
-                Concurrent.transform(
-                    shardKeys,
-                    key ->
-                        tm().transactNewReadOnly(
-                                () -> {
-                                  ClaimsListShard claimsListShard = ofy().load().key(key).now();
-                                  checkState(
-                                      claimsListShard != null,
-                                      "Key not found when loading claims list shards.");
-                                  return claimsListShard;
-                                }));
-          } catch (UncheckedExecutionException e) {
-            // We retry on IllegalStateException. However, there's a checkState inside the
-            // Concurrent.transform, so if it's thrown it'll be wrapped in an
-            // UncheckedExecutionException. We want to unwrap it so it's caught by the retrier.
-            if (e.getCause() != null) {
-              throwIfUnchecked(e.getCause());
-            }
-            throw e;
-          }
-
-          // Combine the shards together and return the concatenated ClaimsList.
-          if (!shards.isEmpty()) {
-            creationTime = shards.get(0).creationTime;
-            for (ClaimsListShard shard : shards) {
-              combinedLabelsToKeys.putAll(shard.labelsToKeys);
-              checkState(
-                  creationTime.equals(shard.creationTime),
-                  "Inconsistent claims list shard creation times.");
-            }
-          }
+      List<ClaimsListShard> shards;
+      try {
+        // Load all of the shards concurrently, each in a separate transaction.
+        shards =
+            Concurrent.transform(
+                shardKeys,
+                key ->
+                    tm().transactNewReadOnly(
+                            () -> {
+                              ClaimsListShard claimsListShard = ofy().load().key(key).now();
+                              checkState(
+                                  claimsListShard != null,
+                                  "Key not found when loading claims list shards.");
+                              return claimsListShard;
+                            }));
+      } catch (UncheckedExecutionException e) {
+        // We retry on IllegalStateException. However, there's a checkState inside the
+        // Concurrent.transform, so if it's thrown it'll be wrapped in an
+        // UncheckedExecutionException. We want to unwrap it so it's caught by the retrier.
+        if (e.getCause() != null) {
+          throwIfUnchecked(e.getCause());
        }
+        throw e;
+      }

-        ClaimsListShard datastoreList =
-            create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys));
-        // Also load the list from Cloud SQL, compare the two lists, and log if different.
-        try {
-          loadAndCompareCloudSqlList(datastoreList);
-        } catch (Throwable t) {
-          logger.atSevere().withCause(t).log("Error comparing claims lists.");
+      // Combine the shards together and return the concatenated ClaimsList.
+      if (!shards.isEmpty()) {
+        creationTime = shards.get(0).creationTime;
+        for (ClaimsListShard shard : shards) {
+          combinedLabelsToKeys.putAll(shard.labelsToKeys);
+          checkState(
+              creationTime.equals(shard.creationTime),
+              "Inconsistent claims list shard creation times.");
        }
-        return datastoreList;
-      };
+      }
+    }
+
+    ClaimsListShard datastoreList = create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys));
+    // Also load the list from Cloud SQL, compare the two lists, and log if different.
+    try {
+      loadAndCompareCloudSqlList(datastoreList);
+    } catch (Throwable t) {
+      logger.atSevere().withCause(t).log("Error comparing claims lists.");
+    }
+    return datastoreList;
+  };

  private static void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) {
    Optional<ClaimsListShard> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
@@ -304,8 +301,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
    Concurrent.transform(
        CollectionUtils.partitionMap(labelsToKeys, shardSize),
        (final ImmutableMap<String, String> labelsToKeysShard) ->
-            tm()
-                .transactNew(
+            tm().transactNew(
                    () -> {
                      ClaimsListShard shard = create(creationTime, labelsToKeysShard);
                      shard.isShard = true;
@@ -315,8 +311,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
                    }));

    // Persist the new revision, thus causing the newly created shards to go live.
-    tm()
-        .transactNew(
+    tm().transactNew(
            () -> {
              verify(
                  (getCurrentRevision() == null && oldRevision == null)
@@ -358,12 +353,10 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
  /** Virtual parent entity for claims list shards of a specific revision. */
  @Entity
  @VirtualEntity
-  public static class ClaimsListRevision extends ImmutableObject implements DatastoreEntity {
-    @Parent
-    Key<ClaimsListSingleton> parent;
+  public static class ClaimsListRevision extends ImmutableObject implements DatastoreOnlyEntity {
+    @Parent Key<ClaimsListSingleton> parent;

-    @Id
-    long versionId;
+    @Id long versionId;

    @VisibleForTesting
    public static Key<ClaimsListRevision> createKey(ClaimsListSingleton singleton) {
@@ -377,11 +370,6 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
    public static Key<ClaimsListRevision> createKey() {
      return createKey(new ClaimsListSingleton());
    }
-
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // ClaimsLists are dually written
-    }
  }

  /**
@@ -390,7 +378,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
   */
  @Entity
  @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
-  public static class ClaimsListSingleton extends CrossTldSingleton implements DatastoreEntity {
+  public static class ClaimsListSingleton extends CrossTldSingleton implements DatastoreOnlyEntity {
    Key<ClaimsListRevision> activeRevision;

    static ClaimsListSingleton create(Key<ClaimsListRevision> revision) {
@@ -403,11 +391,6 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
    public void setActiveRevision(Key<ClaimsListRevision> revision) {
      activeRevision = revision;
    }
-
-    @Override
-    public ImmutableList<SqlEntity> toSqlEntities() {
-      return ImmutableList.of(); // ClaimsLists are dually written
-    }
  }

  /**
@@ -20,7 +20,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;

-import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Entity;
 import google.registry.model.annotations.NotBackedUp;
@@ -28,8 +27,7 @@ import google.registry.model.annotations.NotBackedUp.Reason;
 import google.registry.model.common.CrossTldSingleton;
 import google.registry.model.tmch.TmchCrl.TmchCrlId;
 import google.registry.persistence.VKey;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
+import google.registry.schema.replay.NonReplicatedEntity;
 import java.io.Serializable;
 import java.util.Optional;
 import javax.annotation.concurrent.Immutable;
@@ -44,7 +42,7 @@ import org.joda.time.DateTime;
@Immutable
@NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
@IdClass(TmchCrlId.class)
-public final class TmchCrl extends CrossTldSingleton implements DatastoreEntity, SqlEntity {
+public final class TmchCrl extends CrossTldSingleton implements NonReplicatedEntity {

  @Id String crl;

@@ -105,16 +103,6 @@ public final class TmchCrl extends CrossTldSingleton implements DatastoreEntity,
    return updated;
  }

-  @Override
-  public ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(); // dually-written
-  }
-
-  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // dually-written
-  }
-
  static class TmchCrlId implements Serializable {

    @Column(name = "certificateRevocations")
@@ -14,9 +14,9 @@

 package google.registry.persistence.transaction;

-import com.google.common.collect.ImmutableList;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
+import java.util.Optional;
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
@@ -45,7 +45,7 @@ public class TransactionEntity implements SqlEntity {
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // not stored in Datastore per se
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Not persisted in Datastore per se
  }
 }
@@ -16,7 +16,6 @@ package google.registry.schema.cursor;

 import static com.google.appengine.api.search.checkers.Preconditions.checkNotNull;

-import com.google.common.collect.ImmutableList;
 import google.registry.model.ImmutableObject;
 import google.registry.model.UpdateAutoTimestamp;
 import google.registry.model.common.Cursor.CursorType;
@@ -26,6 +25,7 @@ import google.registry.schema.replay.SqlEntity;
 import google.registry.util.DateTimeUtils;
 import java.io.Serializable;
 import java.time.ZonedDateTime;
+import java.util.Optional;
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.EnumType;
@@ -104,8 +104,8 @@ public class Cursor implements SqlEntity {
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // Cursors are not converted since they are ephemeral
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Cursors are not converted since they are ephemeral
  }

  static class CursorId extends ImmutableObject implements Serializable {
@@ -19,7 +19,6 @@ import static google.registry.util.DateTimeUtils.isBeforeOrAt;
 import static google.registry.util.DateTimeUtils.toZonedDateTime;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

-import com.google.common.collect.ImmutableList;
 import google.registry.model.Buildable;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
@@ -234,8 +233,8 @@ public final class RegistryLock extends ImmutableObject implements Buildable, Sq
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // not stored in Datastore
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Not persisted in Datastore
  }

  /** Builder for {@link google.registry.schema.domain.RegistryLock}. */
@@ -14,18 +14,18 @@

 package google.registry.schema.replay;

-import com.google.common.collect.ImmutableList;
+import java.util.Optional;

 /** An entity that has the same Java object representation in SQL and Datastore. */
 public interface DatastoreAndSqlEntity extends DatastoreEntity, SqlEntity {

  @Override
-  default ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(this);
+  default Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.of(this);
  }

  @Override
-  default ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of(this);
+  default Optional<SqlEntity> toSqlEntity() {
+    return Optional.of(this);
  }
 }
@@ -14,7 +14,7 @@

 package google.registry.schema.replay;

-import com.google.common.collect.ImmutableList;
+import java.util.Optional;

 /**
 * An object that can be stored in Datastore and serialized using Objectify's {@link
@@ -26,5 +26,5 @@ import com.google.common.collect.ImmutableList;
 */
 public interface DatastoreEntity {

-  ImmutableList<SqlEntity> toSqlEntities();
+  Optional<SqlEntity> toSqlEntity();
 }
@@ -0,0 +1,25 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.schema.replay;
+
+import java.util.Optional;
+
+/** An entity that is only stored in Datastore, that should not be replayed to SQL. */
+public interface DatastoreOnlyEntity extends DatastoreEntity {
+  @Override
+  default Optional<SqlEntity> toSqlEntity() {
+    return Optional.empty();
+  }
+}
@@ -14,7 +14,7 @@

 package google.registry.schema.replay;

-import com.google.common.collect.ImmutableList;
+import java.util.Optional;

 /**
 * Represents an entity that should not participate in asynchronous replication.
@@ -24,12 +24,12 @@ import com.google.common.collect.ImmutableList;
 public interface NonReplicatedEntity extends DatastoreEntity, SqlEntity {

  @Override
-  default ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of();
+  default Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty();
  }

  @Override
-  default ImmutableList<SqlEntity> toSqlEntities() {
-    return ImmutableList.of();
+  default Optional<SqlEntity> toSqlEntity() {
+    return Optional.empty();
  }
 }
@@ -14,7 +14,7 @@

 package google.registry.schema.replay;

-import com.google.common.collect.ImmutableList;
+import java.util.Optional;

 /**
 * An object that can be stored in Cloud SQL using {@link
@@ -25,5 +25,5 @@ import com.google.common.collect.ImmutableList;
 */
 public interface SqlEntity {

-  ImmutableList<DatastoreEntity> toDatastoreEntities();
+  Optional<DatastoreEntity> toDatastoreEntity();
 }
@@ -18,7 +18,7 @@ import static google.registry.model.common.CrossTldSingleton.SINGLETON_ID;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;

-import com.google.common.collect.ImmutableList;
+import java.util.Optional;
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import org.joda.time.DateTime;
@@ -32,8 +32,8 @@ public class SqlReplayCheckpoint implements SqlEntity {
  private DateTime lastReplayTime;

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // not necessary to persist in Datastore
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Not necessary to persist in Datastore
  }

  public static DateTime get() {
@@ -16,7 +16,6 @@ package google.registry.schema.server;

 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

-import com.google.common.collect.ImmutableList;
 import google.registry.model.ImmutableObject;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
@@ -24,6 +23,7 @@ import google.registry.schema.server.Lock.LockId;
 import google.registry.util.DateTimeUtils;
 import java.io.Serializable;
 import java.time.ZonedDateTime;
+import java.util.Optional;
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.Id;
@@ -120,8 +120,8 @@ public class Lock implements SqlEntity {
  }

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // Locks are not converted since they are ephemeral
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // Locks are not converted since they are ephemeral
  }

  static class LockId extends ImmutableObject implements Serializable {
@@ -14,13 +14,13 @@

 package google.registry.schema.tld;

-import com.google.common.collect.ImmutableList;
 import google.registry.model.ImmutableObject;
 import google.registry.model.registry.label.PremiumList;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
 import java.io.Serializable;
 import java.math.BigDecimal;
+import java.util.Optional;
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.Id;
@@ -47,7 +47,7 @@ public class PremiumEntry extends ImmutableObject implements Serializable, SqlEn
  private PremiumEntry() {}

  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // PremiumList is dually-written
+  public Optional<DatastoreEntity> toDatastoreEntity() {
+    return Optional.empty(); // PremiumList is dually-written
  }
 }
@@ -138,15 +138,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
      validateWith = PathParameter.InputFile.class)
  Path clientCertificateFilename;

-  @Nullable
-  @Parameter(
-    names = "--cert_hash",
-    description =
-        "Hash of client certificate (SHA256 base64 no padding). Do not use this unless "
-            + "you want to store ONLY the hash and not the full certificate"
-  )
-  String clientCertificateHash;
-
  @Nullable
  @Parameter(
      names = "--failover_cert_file",
@@ -375,14 +366,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
        }
        builder.setFailoverClientCertificate(asciiCert, now);
      }
-      if (!isNullOrEmpty(clientCertificateHash)) {
-        checkArgument(clientCertificateFilename == null,
-            "Can't specify both --cert_hash and --cert_file");
-        if ("null".equals(clientCertificateHash)) {
-          clientCertificateHash = null;
-        }
-        builder.setClientCertificateHash(clientCertificateHash);
-      }
      if (ianaId != null) {
        builder.setIanaIdentifier(ianaId.orElse(null));
      }
@@ -65,13 +65,6 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo
      validateWith = PathParameter.InputFile.class)
  private Path certFile;

-  @Parameter(
-      names = {"-h", "--certhash"},
-      description =
-          "Hash of client certificate (SHA256 base64 no padding). Do not use this unless "
-              + "you want to store ONLY the hash and not the full certificate.")
-  private String certHash;
-
  @Parameter(
      names = {"--overwrite"},
      description = "Whether to replace existing entities if we encounter any, instead of failing.")
@@ -89,9 +82,7 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo
  /** Run any pre-execute command checks */
  @Override
  protected void init() throws Exception {
-    checkArgument(
-        certFile == null ^ certHash == null,
-        "Must specify exactly one of client certificate file or client certificate hash.");
+    checkArgument(certFile != null, "Must specify exactly one client certificate file.");

    password = passwordGenerator.createString(PASSWORD_LENGTH);
    oteAccountBuilder =
@@ -101,16 +92,10 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo
            .setIpAllowList(ipAllowList)
            .setReplaceExisting(overwrite);

-    if (certFile != null) {
      String asciiCert = MoreFiles.asCharSource(certFile, US_ASCII).read();
      // Don't wait for create_registrar to fail if it's a bad certificate file.
      loadCertificate(asciiCert);
      oteAccountBuilder.setCertificate(asciiCert, clock.nowUtc());
-    }
-
-    if (certHash != null) {
-      oteAccountBuilder.setCertificateHash(certHash);
-    }
  }

  @Override
@@ -95,7 +95,7 @@ public class DeleteOldCommitLogsActionTest
    // Before deleting the unneeded manifests - we have 11 of them (one for the first
    // creation, and 10 more for the mutateContacts)
    assertThat(ofy().load().type(CommitLogManifest.class).count()).isEqualTo(11);
-    // And each DatastoreHelper.persistResourceWithCommitLog creates 3 mutations
+    // And each DatabaseHelper.persistResourceWithCommitLog creates 3 mutations
    assertThat(ofy().load().type(CommitLogMutation.class).count()).isEqualTo(33);
  }

@@ -111,7 +111,7 @@ public class DeleteOldCommitLogsActionTest
    assertThat(ImmutableList.copyOf(ofy().load().type(CommitLogManifest.class).keys().iterable()))
        .containsExactlyElementsIn(contact.getRevisions().values());

-    // And each DatastoreHelper.persistResourceWithCommitLog creates 3 mutations
+    // And each DatabaseHelper.persistResourceWithCommitLog creates 3 mutations
    assertThat(ofyLoadType(CommitLogMutation.class)).hasSize(contact.getRevisions().size() * 3);
  }

@@ -29,8 +29,8 @@ import google.registry.model.registrar.Registrar;
 import google.registry.persistence.transaction.JpaTestRules;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension;
 import google.registry.testing.AppEngineExtension;
-import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.DatabaseHelper;
+import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectExtension;
 import java.io.Serializable;
@@ -501,7 +501,7 @@ class EppLifecycleDomainTest extends EppTestCase {

  @Test
  void testEapDomainDeletion_withinAddGracePeriod_eapFeeIsNotRefunded() throws Exception {
-    assertThatCommand("login_valid_fee_extension.xml").hasResponse("generic_success_response.xml");
+    assertThatCommand("login_valid_fee_extension.xml").hasSuccessfulLogin();
    createContacts(DateTime.parse("2000-06-01T00:00:00Z"));

    // Set the EAP schedule.
@@ -718,7 +718,7 @@ class EppLifecycleDomainTest extends EppTestCase {
            START_OF_TIME, PREDELEGATION,
            gaDate, GENERAL_AVAILABILITY));

-    assertThatCommand("login_valid_fee_extension.xml").hasResponse("generic_success_response.xml");
+    assertThatCommand("login_valid_fee_extension.xml").hasSuccessfulLogin();

    assertThatCommand("domain_check_fee_premium.xml")
        .atTime(gaDate.plusDays(1))
@@ -1196,7 +1196,7 @@ class EppLifecycleDomainTest extends EppTestCase {

    assertThatLogin("NewRegistrar", "foo-BAR2")
        .atTime(sunriseDate.minusDays(3))
-        .hasResponse("generic_success_response.xml");
+        .hasSuccessfulLogin();

    createContactsAndHosts();

@@ -1292,7 +1292,7 @@ class EppLifecycleDomainTest extends EppTestCase {

    assertThatLogin("NewRegistrar", "foo-BAR2")
        .atTime(sunriseDate.minusDays(3))
-        .hasResponse("generic_success_response.xml");
+        .hasSuccessfulLogin();

    createContactsAndHosts();

@@ -44,13 +44,13 @@ class EppLoginTlsTest extends EppTestCase {
    persistResource(
        loadRegistrar("NewRegistrar")
            .asBuilder()
-            .setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH)
+            .setClientCertificate(CertificateSamples.SAMPLE_CERT, DateTime.now(UTC))
            .build());
    // Set a cert for the second registrar, or else any cert will be allowed for login.
    persistResource(
        loadRegistrar("TheRegistrar")
            .asBuilder()
-            .setClientCertificateHash(CertificateSamples.SAMPLE_CERT2_HASH)
+            .setClientCertificate(CertificateSamples.SAMPLE_CERT2, DateTime.now(UTC))
            .build());
  }

@@ -121,6 +121,10 @@ public class EppTestCase {
      return assertCommandAndResponse(
          inputFilename, inputSubstitutions, outputFilename, outputSubstitutions, now);
    }
+
+    public String hasSuccessfulLogin() throws Exception {
+      return assertLoginCommandAndResponse(inputFilename, inputSubstitutions, null, now);
+    }
  }

  protected CommandAsserter assertThatCommand(String inputFilename) {
@@ -137,13 +141,33 @@ public class EppTestCase {
  }

  protected void assertThatLoginSucceeds(String clientId, String password) throws Exception {
-    assertThatLogin(clientId, password).hasResponse("generic_success_response.xml");
+    assertThatLogin(clientId, password).hasSuccessfulLogin();
  }

  protected void assertThatLogoutSucceeds() throws Exception {
    assertThatCommand("logout.xml").hasResponse("logout_response.xml");
  }

+  private String assertLoginCommandAndResponse(
+      String inputFilename,
+      @Nullable Map<String, String> inputSubstitutions,
+      @Nullable Map<String, String> outputSubstitutions,
+      DateTime now)
+      throws Exception {
+    String outputFilename = "generic_success_response.xml";
+    clock.setTo(now);
+    String input = loadFile(EppTestCase.class, inputFilename, inputSubstitutions);
+    String expectedOutput = loadFile(EppTestCase.class, outputFilename, outputSubstitutions);
+    setUpSession();
+    FakeResponse response = executeXmlCommand(input);
+
+    // Check that the logged-in header was added to the response
+    assertThat(response.getHeaders()).isEqualTo(ImmutableMap.of("Logged-In", "true"));
+
+    return verifyAndReturnOutput(
+        response.getPayload(), expectedOutput, inputFilename, outputFilename);
+  }
+
  private String assertCommandAndResponse(
      String inputFilename,
      @Nullable Map<String, String> inputSubstitutions,
@@ -154,6 +178,18 @@ public class EppTestCase {
    clock.setTo(now);
    String input = loadFile(EppTestCase.class, inputFilename, inputSubstitutions);
    String expectedOutput = loadFile(EppTestCase.class, outputFilename, outputSubstitutions);
+    setUpSession();
+    FakeResponse response = executeXmlCommand(input);
+
+    // Checks that the Logged-In header is not in the response. If testing the login command, use
+    // assertLoginCommandAndResponse instead of this method.
+    assertThat(response.getHeaders()).doesNotContainEntry("Logged-In", "true");
+
+    return verifyAndReturnOutput(
+        response.getPayload(), expectedOutput, inputFilename, outputFilename);
+  }
+
+  private void setUpSession() {
    if (sessionMetadata == null) {
      sessionMetadata =
          new HttpSessionMetadata(new FakeHttpSession()) {
@@ -165,7 +201,13 @@ public class EppTestCase {
            }
          };
    }
-    String actualOutput = executeXmlCommand(input);
+  }
+
+  private String verifyAndReturnOutput(
+      String actualOutput, String expectedOutput, String inputFilename, String outputFilename)
+      throws Exception {
+    // Run the resulting xml through the unmarshaller to verify that it was valid.
+    EppXmlTransformer.validateOutput(actualOutput);
    assertXmlEqualsWithMessage(
        expectedOutput,
        actualOutput,
@@ -176,7 +218,7 @@ public class EppTestCase {
    return actualOutput;
  }

-  private String executeXmlCommand(String inputXml) throws Exception {
+  private FakeResponse executeXmlCommand(String inputXml) throws Exception {
    EppRequestHandler handler = new EppRequestHandler();
    FakeResponse response = new FakeResponse();
    handler.response = response;
@@ -195,10 +237,7 @@ public class EppTestCase {
        inputXml.getBytes(UTF_8));
    assertThat(response.getStatus()).isEqualTo(SC_OK);
    assertThat(response.getContentType()).isEqualTo(APPLICATION_EPP_XML_UTF8);
-    String result = response.getPayload();
-    // Run the resulting xml through the unmarshaller to verify that it was valid.
-    EppXmlTransformer.validateOutput(result);
-    return result;
+    return response;
  }

  EppMetric getRecordedEppMetric() {
@@ -96,17 +96,24 @@ import google.registry.model.transfer.DomainTransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import google.registry.persistence.VKey;
+import google.registry.testing.ReplayExtension;
 import google.registry.testing.TaskQueueHelper.TaskMatcher;
 import java.util.Map;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 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;

 /** Unit tests for {@link DomainDeleteFlow}. */
 class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, DomainBase> {

+  @Order(value = Order.DEFAULT - 2)
+  @RegisterExtension
+  final ReplayExtension replayExtension = new ReplayExtension(clock);
+
  private DomainBase domain;
  private HistoryEntry earlierHistoryEntry;

@@ -137,6 +144,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
        persistResource(createAutorenewBillingEvent("TheRegistrar").build());
    PollMessage.Autorenew autorenewPollMessage =
        persistResource(createAutorenewPollMessage("TheRegistrar").build());
+
    domain =
        persistResource(
            domain
@@ -144,6 +152,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
                .setAutorenewBillingEvent(autorenewBillingEvent.createVKey())
                .setAutorenewPollMessage(autorenewPollMessage.createVKey())
                .build());
+
    assertTransactionalFlow(true);
  }

@@ -151,15 +160,20 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
    // Persist a linked contact.
    ContactResource contact = persistActiveContact("sh8013");
    domain =
-        newDomainBase(getUniqueIdFromCommand())
-            .asBuilder()
-            .setCreationTimeForTest(TIME_BEFORE_FLOW)
-            .setRegistrant(contact.createVKey())
-            .setRegistrationExpirationTime(expirationTime)
-            .build();
+        persistResource(
+            newDomainBase(getUniqueIdFromCommand())
+                .asBuilder()
+                .setCreationTimeForTest(TIME_BEFORE_FLOW)
+                .setRegistrant(contact.createVKey())
+                .setRegistrationExpirationTime(expirationTime)
+                .build());
    earlierHistoryEntry =
        persistResource(
-            new HistoryEntry.Builder().setType(DOMAIN_CREATE).setParent(domain).build());
+            new HistoryEntry.Builder()
+                .setType(DOMAIN_CREATE)
+                .setParent(domain)
+                .setModificationTime(clock.nowUtc())
+                .build());
  }

  private void setUpGracePeriods(GracePeriod... gracePeriods) {
@@ -208,7 +222,14 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain

  private void assertAutorenewClosedAndCancellationCreatedFor(
      BillingEvent.OneTime graceBillingEvent, HistoryEntry historyEntryDomainDelete) {
-    DateTime eventTime = clock.nowUtc();
+    assertAutorenewClosedAndCancellationCreatedFor(
+        graceBillingEvent, historyEntryDomainDelete, clock.nowUtc());
+  }
+
+  private void assertAutorenewClosedAndCancellationCreatedFor(
+      BillingEvent.OneTime graceBillingEvent,
+      HistoryEntry historyEntryDomainDelete,
+      DateTime eventTime) {
    assertBillingEvents(
        createAutorenewBillingEvent("TheRegistrar").setRecurrenceEndTime(eventTime).build(),
        graceBillingEvent,
@@ -293,7 +314,11 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
    setUpSuccessfulTest();
    setUpGracePeriods(
        GracePeriod.create(
-            GracePeriodStatus.ADD, domain.getRepoId(), TIME_BEFORE_FLOW.plusDays(1), "foo", null));
+            GracePeriodStatus.ADD,
+            domain.getRepoId(),
+            TIME_BEFORE_FLOW.plusDays(1),
+            "TheRegistrar",
+            null));
    dryRunFlowAssertResponse(loadFile("generic_success_response.xml"));
  }

@@ -398,7 +423,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
            GracePeriodStatus.TRANSFER,
            domain.getRepoId(),
            TIME_BEFORE_FLOW.plusDays(1),
-            "foo",
+            "NewRegistrar",
            null));
    // We should see exactly one poll message, which is for the autorenew 1 month in the future.
    assertPollMessages(createAutorenewPollMessage("TheRegistrar").build());
@@ -730,11 +755,11 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
            .setNameservers(ImmutableSet.of(host.createVKey()))
            .setDeletionTime(START_OF_TIME)
            .build());
-    clock.advanceOneMilli();
+    DateTime eventTime = clock.nowUtc();
    runFlowAssertResponse(loadFile("generic_success_response.xml"));
    assertDnsTasksEnqueued("example.tld");
    assertAutorenewClosedAndCancellationCreatedFor(
-        graceBillingEvent, getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE));
+        graceBillingEvent, getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE), eventTime);
  }

  @Test
@@ -14,6 +14,7 @@

 package google.registry.flows.session;

+import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatabaseHelper.deleteResource;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -32,6 +33,7 @@ import google.registry.flows.session.LoginFlow.PasswordChangesNotSupportedExcept
 import google.registry.flows.session.LoginFlow.RegistrarAccountNotActiveException;
 import google.registry.flows.session.LoginFlow.TooManyFailedLoginsException;
 import google.registry.flows.session.LoginFlow.UnsupportedLanguageException;
+import google.registry.model.eppoutput.EppOutput;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import org.junit.jupiter.api.BeforeEach;
@@ -74,6 +76,14 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
    doSuccessfulTest("login_valid.xml");
  }

+  @Test
+  void testSuccess_setsIsLoginResponse() throws Exception {
+    setEppInput("login_valid.xml");
+    assertTransactionalFlow(false);
+    EppOutput output = runFlow();
+    assertThat(output.getResponse().isLoginResponse()).isTrue();
+  }
+
  @Test
  void testSuccess_suspendedRegistrar() throws Exception {
    persistResource(getRegistrarBuilder().setState(State.SUSPENDED).build());
@@ -15,6 +15,7 @@
 package google.registry.flows.session;

 import static google.registry.testing.DatabaseHelper.persistResource;
+import static org.joda.time.DateTimeZone.UTC;

 import com.google.common.collect.ImmutableList;
 import com.google.common.net.InetAddresses;
@@ -26,6 +27,7 @@ import google.registry.model.registrar.Registrar;
 import google.registry.testing.CertificateSamples;
 import google.registry.util.CidrAddressBlock;
 import java.util.Optional;
+import org.joda.time.DateTime;
 import org.junit.jupiter.api.Test;

 /** Unit tests for {@link LoginFlow} when accessed via a TLS transport. */
@@ -41,7 +43,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase {
  @Override
  protected Registrar.Builder getRegistrarBuilder() {
    return super.getRegistrarBuilder()
-        .setClientCertificateHash(GOOD_CERT)
+        .setClientCertificate(CertificateSamples.SAMPLE_CERT, DateTime.now(UTC))
        .setIpAddressAllowList(
            ImmutableList.of(CidrAddressBlock.create(InetAddresses.forString(GOOD_IP.get()), 32)));
  }
@@ -156,16 +156,6 @@ public final class OteAccountBuilderTest {
        .isTrue();
  }

-  @Test
-  void testCreateOteEntities_setCertificateHash() {
-    OteAccountBuilder.forClientId("myclientid")
-        .setCertificateHash(SAMPLE_CERT_HASH)
-        .buildAndPersist();
-
-    assertThat(Registrar.loadByClientId("myclientid-3").get().getClientCertificateHash())
-        .isEqualTo(SAMPLE_CERT_HASH);
-  }
-
  @Test
  void testCreateOteEntities_setCertificate() {
    OteAccountBuilder.forClientId("myclientid")
@@ -19,6 +19,8 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.newDomainBase;
@@ -30,6 +32,7 @@ import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;

+import com.google.appengine.api.datastore.Entity;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
@@ -164,6 +167,15 @@ public class DomainBaseTest extends EntityTestCase {
                    .build()));
  }

+  @Test
+  void testGracePeriod_nullIdFromOfy() {
+    Entity entity = ofyTm().transact(() -> ofy().save().toEntity(domain));
+    entity.setUnindexedProperty("gracePeriods.gracePeriodId", null);
+    DomainBase domainFromEntity = ofyTm().transact(() -> ofy().load().fromEntity(entity));
+    GracePeriod gracePeriod = domainFromEntity.getGracePeriods().iterator().next();
+    assertThat(gracePeriod.gracePeriodId).isNotNull();
+  }
+
  @Test
  void testPersistence() {
    // Note that this only verifies that the value stored under the foreign key is the same as that
@@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
 import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence;
+import static google.registry.model.registry.Registry.TldState.GENERAL_AVAILABILITY;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.createTld;
@@ -25,9 +26,11 @@ import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid;
 import static google.registry.testing.DatabaseHelper.newDomainBase;
 import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static java.nio.charset.StandardCharsets.UTF_8;

 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
 import com.googlecode.objectify.Key;
 import google.registry.model.EntityTestCase;
 import google.registry.model.contact.ContactResource;
@@ -40,10 +43,14 @@ import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostResource;
+import google.registry.model.registrar.Registrar;
+import google.registry.model.registry.Registries;
+import google.registry.model.registry.Registry;
 import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
+import google.registry.testing.DatabaseHelper;
 import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.TestOfyOnly;
 import google.registry.testing.TestSqlOnly;
@@ -115,7 +122,8 @@ public class DomainHistoryTest extends EntityTestCase {
    DomainHistory domainHistory = createDomainHistory(domain);
    tm().transact(() -> tm().insert(domainHistory));

-    // retrieving a HistoryEntry or a DomainHistory with the same key should return the same object
+    // retrieving a HistoryEntry or a DomainHistory with the same key should return the same
+    // object
    // note: due to the @EntitySubclass annotation. all Keys for DomainHistory objects will have
    // type HistoryEntry
    VKey<DomainHistory> domainHistoryVKey = domainHistory.createVKey();
@@ -127,6 +135,68 @@ public class DomainHistoryTest extends EntityTestCase {
    assertThat(domainHistoryFromDb).isEqualTo(historyEntryFromDb);
  }

+  @TestOfyOnly
+  void testDoubleWriteOfOfyResource() {
+    // We have to add the registry to ofy, since we're currently loading the cache from ofy.  We
+    // also have to add it to SQL to satisfy the foreign key constraints of the registrar.
+    Registry registry =
+        DatabaseHelper.newRegistry(
+            "tld", "TLD", ImmutableSortedMap.of(START_OF_TIME, GENERAL_AVAILABILITY));
+    tm().transact(() -> tm().insert(registry));
+    Registries.resetCache();
+    jpaTm()
+        .transact(
+            () -> {
+              jpaTm().insert(registry);
+              Registrar registrar =
+                  appEngine
+                      .makeRegistrar2()
+                      .asBuilder()
+                      .setAllowedTlds(ImmutableSet.of("tld"))
+                      .build();
+              jpaTm().insert(registrar);
+            });
+
+    HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
+    ContactResource contact = newContactResourceWithRoid("contactId", "contact1");
+
+    // Set up the host and domain objects in both databases.
+    tm().transact(
+            () -> {
+              tm().insert(host);
+              tm().insert(contact);
+            });
+    jpaTm()
+        .transact(
+            () -> {
+              jpaTm().insert(host);
+              jpaTm().insert(contact);
+            });
+    fakeClock.advanceOneMilli();
+    DomainBase domain =
+        newDomainBase("example.tld", "domainRepoId", contact)
+            .asBuilder()
+            .setNameservers(host.createVKey())
+            .build();
+    tm().transact(() -> tm().insert(domain));
+    jpaTm().transact(() -> jpaTm().insert(domain));
+    fakeClock.advanceOneMilli();
+
+    DomainHistory domainHistory = createDomainHistory(domain);
+    tm().transact(() -> tm().insert(domainHistory));
+
+    // Load the DomainHistory object from the datastore.
+    VKey<DomainHistory> domainHistoryVKey = domainHistory.createVKey();
+    DomainHistory domainHistoryFromDb = tm().transact(() -> tm().load(domainHistoryVKey));
+
+    // attempt to write to SQL.
+    jpaTm().transact(() -> jpaTm().insert(domainHistoryFromDb));
+
+    // Reload and rewrite.
+    DomainHistory domainHistoryFromDb2 = tm().transact(() -> tm().load(domainHistoryVKey));
+    jpaTm().transact(() -> jpaTm().put(domainHistoryFromDb2));
+  }
+
  static DomainBase createDomainWithContactsAndHosts() {
    createTld("tld");
    HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
@@ -37,7 +37,7 @@ class EppResourceIndexTest extends EntityTestCase {
  @BeforeEach
  void setUp() {
    createTld("tld");
-    // The DatastoreHelper here creates the EppResourceIndex for us.
+    // The DatabaseHelper here creates the EppResourceIndex for us.
    contact = persistActiveContact("abcd1357");
  }

@@ -16,10 +16,16 @@ package google.registry.schema.replay;

 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;

 import com.google.common.collect.ImmutableSet;
+import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Embed;
+import com.googlecode.objectify.annotation.Parent;
+import google.registry.model.ModelUtils;
 import google.registry.model.common.GaeUserIdConverter;
+import google.registry.persistence.VKey;
+import google.registry.testing.DatastoreEntityExtension;
 import io.github.classgraph.ClassGraph;
 import io.github.classgraph.ClassInfo;
 import io.github.classgraph.ClassInfoList;
@@ -28,13 +34,18 @@ import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.lang.reflect.Method;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;

 /**
 * Test to verify classes implement {@link SqlEntity} and {@link DatastoreEntity} when they should.
 */
 public class EntityTest {

+  @RegisterExtension
+  final DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension();
+
  private static final ImmutableSet<Class<?>> NON_CONVERTED_CLASSES =
      ImmutableSet.of(GaeUserIdConverter.class);

@@ -59,6 +70,47 @@ public class EntityTest {
    }
  }

+  @Test
+  void testDatastoreEntityVKeyCreation() {
+    // For replication, we need to be able to convert from Key -> VKey for the relevant classes.
+    // This means that the relevant classes must have non-composite Objectify keys or must have a
+    // createVKey method
+    try (ScanResult scanResult =
+        new ClassGraph().enableAnnotationInfo().whitelistPackages("google.registry").scan()) {
+      ImmutableSet<Class<?>> datastoreEntityClasses =
+          getClasses(scanResult.getClassesImplementing(DatastoreEntity.class.getName()));
+      // some classes aren't converted so they aren't relevant
+      ImmutableSet<Class<?>> vkeyConversionNecessaryClasses =
+          datastoreEntityClasses.stream()
+              .filter(clazz -> !DatastoreOnlyEntity.class.isAssignableFrom(clazz))
+              .filter(clazz -> !NonReplicatedEntity.class.isAssignableFrom(clazz))
+              .collect(toImmutableSet());
+
+      ImmutableSet.Builder<Class<?>> failedClasses = new ImmutableSet.Builder<>();
+      for (Class<?> clazz : vkeyConversionNecessaryClasses) {
+        if (hasKeyWithParent(clazz)) {
+          try {
+            Method createVKeyMethod = clazz.getMethod("createVKey", Key.class);
+            if (!createVKeyMethod.getReturnType().equals(VKey.class)) {
+              failedClasses.add(clazz);
+            }
+          } catch (NoSuchMethodException e) {
+            failedClasses.add(clazz);
+          }
+        }
+      }
+      assertWithMessage(
+              "Some DatastoreEntity classes with parents were missing createVKey methods: ")
+          .that(failedClasses.build())
+          .isEmpty();
+    }
+  }
+
+  private boolean hasKeyWithParent(Class<?> clazz) {
+    return ModelUtils.getAllFields(clazz).values().stream()
+        .anyMatch(field -> field.getAnnotation(Parent.class) != null);
+  }
+
  private ImmutableSet<String> getAllClassesWithAnnotation(
      ScanResult scanResult, String annotation) {
    ImmutableSet.Builder<String> result = new ImmutableSet.Builder<>();
@@ -70,17 +122,20 @@ public class EntityTest {
    return result.build();
  }

-  private ImmutableSet<String> getClassNames(ClassInfoList classInfoList) {
+  private ImmutableSet<Class<?>> getClasses(ClassInfoList classInfoList) {
    return classInfoList.stream()
        .filter(ClassInfo::isStandardClass)
        .map(ClassInfo::loadClass)
        .filter(clazz -> !clazz.isAnnotationPresent(EntityForTesting.class))
        .filter(clazz -> !clazz.isAnnotationPresent(Embed.class))
        .filter(clazz -> !NON_CONVERTED_CLASSES.contains(clazz))
-        .map(Class::getName)
        .collect(toImmutableSet());
  }

+  private ImmutableSet<String> getClassNames(ClassInfoList classInfoList) {
+    return getClasses(classInfoList).stream().map(Class::getName).collect(toImmutableSet());
+  }
+
  /** Entities that are solely used for testing, to avoid scanning them in {@link EntityTest}. */
  @Target(ElementType.TYPE)
  @Retention(RetentionPolicy.RUNTIME)
@@ -638,11 +638,13 @@ public class DatabaseHelper {
      DateTime requestTime,
      DateTime expirationTime,
      DateTime extendedRegistrationExpirationTime) {
-    HistoryEntry historyEntryDomainTransfer = persistResource(
-        new HistoryEntry.Builder()
-            .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST)
-            .setParent(domain)
-            .build());
+    HistoryEntry historyEntryDomainTransfer =
+        persistResource(
+            new HistoryEntry.Builder()
+                .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST)
+                .setModificationTime(tm().transact(() -> tm().getTransactionTime()))
+                .setParent(domain)
+                .build());
    BillingEvent.OneTime transferBillingEvent = persistResource(createBillingEventForTransfer(
            domain,
            historyEntryDomainTransfer,
@@ -1184,6 +1186,7 @@ public class DatabaseHelper {
  public static void deleteResource(final Object resource) {
    if (alwaysSaveWithBackup) {
      tm().transact(() -> tm().delete(resource));
+      maybeAdvanceClock();
    } else {
      transactIfJpaTm(() -> tm().deleteWithoutBackup(resource));
    }
@@ -21,7 +21,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
-import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatabaseHelper.createTlds;
 import static google.registry.testing.DatabaseHelper.persistNewRegistrar;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
@@ -447,29 +446,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
    assertThat(registrar).isEmpty();
  }

-  @Test
-  void testSuccess_clientCertHashFlag() throws Exception {
-    runCommandForced(
-        "--name=blobio",
-        "--password=some_password",
-        "--registrar_type=REAL",
-        "--iana_id=8",
-        "--cert_hash=" + SAMPLE_CERT_HASH,
-        "--passcode=01234",
-        "--icann_referral_email=foo@bar.test",
-        "--street=\"123 Fake St\"",
-        "--city Fakington",
-        "--state MA",
-        "--zip 00351",
-        "--cc US",
-        "clientz");
-
-    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
-    assertThat(registrar).isPresent();
-    assertThat(registrar.get().getClientCertificate()).isNull();
-    assertThat(registrar.get().getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
-  }
-
  @Test
  void testSuccess_failoverClientCertFileFlag() throws Exception {
    fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z"));
@@ -1182,74 +1158,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
                "clientz"));
  }

-  @Test
-  void testFailure_certHashAndCertFile() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () ->
-            runCommandForced(
-                "--name=blobio",
-                "--password=some_password",
-                "--registrar_type=REAL",
-                "--iana_id=8",
-                "--cert_file=" + getCertFilename(),
-                "--cert_hash=ABCDEF",
-                "--passcode=01234",
-                "--icann_referral_email=foo@bar.test",
-                "--street=\"123 Fake St\"",
-                "--city Fakington",
-                "--state MA",
-                "--zip 00351",
-                "--cc US",
-                "clientz"));
-  }
-
-  @Test
-  void testFailure_certHashNotBase64() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                runCommandForced(
-                    "--name=blobio",
-                    "--password=some_password",
-                    "--registrar_type=REAL",
-                    "--iana_id=8",
-                    "--cert_hash=!",
-                    "--passcode=01234",
-                    "--icann_referral_email=foo@bar.test",
-                    "--street=\"123 Fake St\"",
-                    "--city Fakington",
-                    "--state MA",
-                    "--zip 00351",
-                    "--cc US",
-                    "clientz"));
-    assertThat(thrown).hasMessageThat().contains("base64");
-  }
-
-  @Test
-  void testFailure_certHashNotA256BitValue() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                runCommandForced(
-                    "--name=blobio",
-                    "--password=some_password",
-                    "--registrar_type=REAL",
-                    "--iana_id=8",
-                    "--cert_hash=abc",
-                    "--passcode=01234",
-                    "--icann_referral_email=foo@bar.test",
-                    "--street=\"123 Fake St\"",
-                    "--city Fakington",
-                    "--state MA",
-                    "--zip 00351",
-                    "--cc US",
-                    "clientz"));
-    assertThat(thrown).hasMessageThat().contains("256");
-  }
-
  @Test
  void testFailure_missingName() {
    IllegalArgumentException thrown =
@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.registrar.Registrar.State.ACTIVE;
 import static google.registry.model.registry.Registry.TldState.GENERAL_AVAILABILITY;
 import static google.registry.model.registry.Registry.TldState.START_DATE_SUNRISE;
-import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
@@ -98,8 +97,7 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
      String registrarName,
      String allowedTld,
      String password,
-      ImmutableList<CidrAddressBlock> ipAllowList,
-      boolean hashOnly) {
+      ImmutableList<CidrAddressBlock> ipAllowList) {
    Registrar registrar = loadRegistrar(registrarName);
    assertThat(registrar).isNotNull();
    assertThat(registrar.getAllowedTlds()).containsExactlyElementsIn(ImmutableSet.of(allowedTld));
@@ -108,18 +106,6 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
    assertThat(registrar.verifyPassword(password)).isTrue();
    assertThat(registrar.getIpAddressAllowList()).isEqualTo(ipAllowList);
    assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
-    // If certificate hash is provided, there's no certificate file stored with the registrar.
-    if (!hashOnly) {
-      assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);
-    }
-  }
-
-  private void verifyRegistrarCreation(
-      String registrarName,
-      String allowedTld,
-      String password,
-      ImmutableList<CidrAddressBlock> ipAllowList) {
-    verifyRegistrarCreation(registrarName, allowedTld, password, ipAllowList, false);
  }

  private void verifyRegistrarContactCreation(String registrarName, String email) {
@@ -184,24 +170,6 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
    verifyRegistrarContactCreation("abc-5", "abc@email.com");
  }

-  @Test
-  void testSuccess_certificateHash() throws Exception {
-    runCommandForced(
-        "--ip_allow_list=1.1.1.1",
-        "--registrar=blobio",
-        "--email=contact@email.com",
-        "--certhash=" + SAMPLE_CERT_HASH);
-
-    verifyTldCreation("blobio-eap", "BLOBIOE3", GENERAL_AVAILABILITY, true);
-
-    ImmutableList<CidrAddressBlock> ipAddress =
-        ImmutableList.of(CidrAddressBlock.create("1.1.1.1"));
-
-    verifyRegistrarCreation("blobio-5", "blobio-eap", PASSWORD, ipAddress, true);
-
-    verifyRegistrarContactCreation("blobio-5", "contact@email.com");
-  }
-
  @Test
  void testSuccess_multipleIps() throws Exception {
    runCommandForced(
@@ -256,7 +224,7 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
  }

  @Test
-  void testFailure_missingCertificateFileAndCertificateHash() {
+  void testFailure_missingCertificateFile() {
    IllegalArgumentException thrown =
        assertThrows(
            IllegalArgumentException.class,
@@ -265,26 +233,7 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
                    "--ip_allow_list=1.1.1.1", "--email=contact@email.com", "--registrar=blobio"));
    assertThat(thrown)
        .hasMessageThat()
-        .contains(
-            "Must specify exactly one of client certificate file or client certificate hash.");
-  }
-
-  @Test
-  void testFailure_suppliedCertificateFileAndCertificateHash() {
-    IllegalArgumentException thrown =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                runCommandForced(
-                    "--ip_allow_list=1.1.1.1",
-                    "--email=contact@email.com",
-                    "--registrar=blobio",
-                    "--certfile=" + getCertFilename(),
-                    "--certhash=" + SAMPLE_CERT_HASH));
-    assertThat(thrown)
-        .hasMessageThat()
-        .contains(
-            "Must specify exactly one of client certificate file or client certificate hash.");
+        .contains("Must specify exactly one client certificate file.");
  }

  @Test
@@ -21,7 +21,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
-import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatabaseHelper.createTlds;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -339,14 +338,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
    assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT3);
  }

-  @Test
-  void testSuccess_certHash() throws Exception {
-    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull();
-    runCommand("--cert_hash=" + SAMPLE_CERT_HASH, "--force", "NewRegistrar");
-    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash())
-        .isEqualTo(SAMPLE_CERT_HASH);
-  }
-
  @Test
  void testSuccess_clearCert() throws Exception {
    persistResource(
@@ -359,18 +350,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
    assertThat(loadRegistrar("NewRegistrar").getClientCertificate()).isNull();
  }

-  @Test
-  void testSuccess_clearCertHash() throws Exception {
-    persistResource(
-        loadRegistrar("NewRegistrar")
-            .asBuilder()
-            .setClientCertificateHash(SAMPLE_CERT_HASH)
-            .build());
-    assertThat(isNullOrEmpty(loadRegistrar("NewRegistrar").getClientCertificateHash())).isFalse();
-    runCommand("--cert_hash=null", "--force", "NewRegistrar");
-    assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull();
-  }
-
  @Test
  void testSuccess_ianaId() throws Exception {
    assertThat(loadRegistrar("NewRegistrar").getIanaIdentifier()).isEqualTo(8);
@@ -762,18 +741,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
        () -> runCommand("--cert_file=" + writeToTmpFile("ABCDEF"), "--force", "NewRegistrar"));
  }

-  @Test
-  void testFailure_certHashAndCertFile() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () ->
-            runCommand(
-                "--cert_file=" + getCertFilename(SAMPLE_CERT3),
-                "--cert_hash=ABCDEF",
-                "--force",
-                "NewRegistrar"));
-  }
-
  @Test
  void testFailure_missingClientId() {
    assertThrows(ParameterException.class, () -> runCommand("--force"));
@@ -20,6 +20,7 @@ import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
+import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;

 import com.beust.jcommander.ParameterException;
@@ -33,6 +34,7 @@ import google.registry.testing.CertificateSamples;
 import google.registry.util.CidrAddressBlock;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;

@@ -50,7 +52,7 @@ class ValidateLoginCredentialsCommandTest extends CommandTestCase<ValidateLoginC
        loadRegistrar("NewRegistrar")
            .asBuilder()
            .setPassword(PASSWORD)
-            .setClientCertificateHash(CERT_HASH)
+            .setClientCertificate(CertificateSamples.SAMPLE_CERT, DateTime.now(UTC))
            .setIpAddressAllowList(ImmutableList.of(new CidrAddressBlock(CLIENT_IP)))
            .setState(ACTIVE)
            .setAllowedTlds(ImmutableSet.of("tld"))
@@ -342,26 +342,6 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
    driver.diffPage("edit");
  }

-  @RetryingTest(3)
-  void settingsSecurityWithHashOnly() throws Throwable {
-    server.runInAppEngineEnvironment(
-        () -> {
-          persistResource(
-              loadRegistrar("TheRegistrar")
-                  .asBuilder()
-                  .setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH)
-                  .build());
-          return null;
-        });
-    driver.manage().window().setSize(new Dimension(1050, 2000));
-    driver.get(server.getUrl("/registrar#security-settings"));
-    driver.waitForDisplayedElement(By.tagName("h1"));
-    driver.diffPage("view");
-    driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click();
-    driver.waitForDisplayedElement(By.tagName("h1"));
-    driver.diffPage("edit");
-  }
-
  @RetryingTest(3)
  void index_registrarDisabled() throws Throwable {
    server.runInAppEngineEnvironment(
@@ -273,8 +273,6 @@ class google.registry.model.domain.DomainHistory {
  java.lang.String clientId;
  java.lang.String otherClientId;
  java.lang.String reason;
-  java.util.Set<google.registry.model.domain.GracePeriod$GracePeriodHistory> gracePeriodHistories;
-  java.util.Set<google.registry.model.domain.secdns.DomainDsDataHistory> dsDataHistories;
  java.util.Set<google.registry.model.reporting.DomainTransactionRecord> domainTransactionRecords;
  org.joda.time.DateTime modificationTime;
 }
@@ -286,16 +284,6 @@ class google.registry.model.domain.GracePeriod {
  java.lang.String clientId;
  org.joda.time.DateTime expirationTime;
 }
-class google.registry.model.domain.GracePeriod$GracePeriodHistory {
-  google.registry.model.domain.rgp.GracePeriodStatus type;
-  google.registry.persistence.VKey<google.registry.model.billing.BillingEvent$OneTime> billingEventOneTime;
-  google.registry.persistence.VKey<google.registry.model.billing.BillingEvent$Recurring> billingEventRecurring;
-  java.lang.Long domainHistoryRevisionId;
-  java.lang.Long gracePeriodHistoryRevisionId;
-  java.lang.Long gracePeriodId;
-  java.lang.String clientId;
-  org.joda.time.DateTime expirationTime;
-}
 class google.registry.model.domain.Period {
  google.registry.model.domain.Period$Unit unit;
  java.lang.Integer value;
@@ -328,14 +316,6 @@ class google.registry.model.domain.secdns.DelegationSignerData {
  int digestType;
  int keyTag;
 }
-class google.registry.model.domain.secdns.DomainDsDataHistory {
-  byte[] digest;
-  int algorithm;
-  int digestType;
-  int keyTag;
-  java.lang.Long domainHistoryRevisionId;
-  java.lang.Long dsDataHistoryRevisionId;
-}
 class google.registry.model.domain.token.AllocationToken {
  @Id java.lang.String token;
  boolean discountPremiums;
@@ -261,11 +261,11 @@ td.section {
    </tr> 
    <tr> 
     <td class="property_name">generated on</td> 
-     <td class="property_value">2020-11-18 21:42:47.876348</td> 
+     <td class="property_value">2020-11-30 21:59:21.319302</td> 
    </tr> 
    <tr>
     <td class="property_name">last flyway file</td>
-     <td id="lastFlywayFile" class="property_value">V79__drop_foreign_keys_on_pollmessage.sql</td>
+     <td id="lastFlywayFile" class="property_value">V80__defer_bill_event_key.sql</td>
    </tr>
   </tbody>
  </table> 
@@ -284,7 +284,7 @@ td.section {
     generated on
    </text> 
    <text text-anchor="start" x="4027.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
-     2020-11-18 21:42:47.876348
+     2020-11-30 21:59:21.319302
    </text> 
    <polygon fill="none" stroke="#888888" points="3940.44,-4 3940.44,-44 4205.44,-44 4205.44,-4 3940.44,-4" /> <!-- allocationtoken_a08ccbef --> 
    <g id="node1" class="node"> 
@@ -261,11 +261,11 @@ td.section {
    </tr> 
    <tr> 
     <td class="property_name">generated on</td> 
-     <td class="property_value">2020-11-18 21:42:46.037281</td> 
+     <td class="property_value">2020-11-30 21:59:18.813825</td> 
    </tr> 
    <tr>
     <td class="property_name">last flyway file</td>
-     <td id="lastFlywayFile" class="property_value">V79__drop_foreign_keys_on_pollmessage.sql</td>
+     <td id="lastFlywayFile" class="property_value">V80__defer_bill_event_key.sql</td>
    </tr>
   </tbody>
  </table> 
@@ -284,7 +284,7 @@ td.section {
     generated on
    </text> 
    <text text-anchor="start" x="4631.68" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
-     2020-11-18 21:42:46.037281
+     2020-11-30 21:59:18.813825
    </text> 
    <polygon fill="none" stroke="#888888" points="4544.18,-4 4544.18,-44 4809.18,-44 4809.18,-4 4544.18,-4" /> <!-- allocationtoken_a08ccbef --> 
    <g id="node1" class="node"> 
@@ -77,3 +77,4 @@ V76__change_history_nullability.sql
 V77__fixes_for_replay.sql
 V78__add_history_id_for_redemption_history_entry.sql
 V79__drop_foreign_keys_on_pollmessage.sql
+V80__defer_bill_event_key.sql
@@ -0,0 +1,27 @@
+-- Copyright 2020 The Nomulus Authors. All Rights Reserved.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+-- We have to make this foreign key initially deferred even though
+-- BillingEvent is saved before Domain (and deleted after).  If we don't, it
+-- appears that Hibernate can sporadically introduce FK constraint failures
+-- when updating a Domain to reference a new BillingEvent and then deleting
+-- the old BillingEvent.  This may be due to the fact that this FK
+-- relationships is not known to hibernate.
+ALTER TABLE "Domain" DROP CONSTRAINT fk_domain_transfer_billing_event_id;
+ALTER TABLE if exists "Domain"
+   ADD CONSTRAINT fk_domain_transfer_billing_event_id
+   FOREIGN KEY (transfer_billing_event_id)
+   REFERENCES "BillingEvent"
+   DEFERRABLE INITIALLY DEFERRED;
+
@@ -2064,7 +2064,7 @@ ALTER TABLE ONLY public."Domain"
 --

 ALTER TABLE ONLY public."Domain"
-    ADD CONSTRAINT fk_domain_transfer_billing_event_id FOREIGN KEY (transfer_billing_event_id) REFERENCES public."BillingEvent"(billing_event_id);
+    ADD CONSTRAINT fk_domain_transfer_billing_event_id FOREIGN KEY (transfer_billing_event_id) REFERENCES public."BillingEvent"(billing_event_id) DEFERRABLE INITIALLY DEFERRED;


 --
@@ -249,6 +249,20 @@ class EppServiceHandlerTest {
    assertThat(channel.isActive()).isFalse();
  }

+  @Test
+  void sendResponseToNextHandler_unrecognizedHeader() throws Exception {
+    setHandshakeSuccess();
+    String content = "<epp>stuff</epp>";
+    HttpResponse response = makeEppHttpResponse(content, HttpResponseStatus.OK);
+    response.headers().set("unrecognized-header", "test");
+    channel.writeOutbound(response);
+    ByteBuf expectedResponse = channel.readOutbound();
+    assertThat(Unpooled.wrappedBuffer(content.getBytes(UTF_8))).isEqualTo(expectedResponse);
+    // Nothing further to pass to the next handler.
+    assertThat((Object) channel.readOutbound()).isNull();
+    assertThat(channel.isActive()).isTrue();
+  }
+
  @Test
  void testFailure_disconnectOnNonOKResponseStatus() throws Exception {
    setHandshakeSuccess();
@@ -14,39 +14,20 @@
 """Helper for using the AppEngine Admin REST API."""

 import time
-from typing import Any, Dict, FrozenSet, Set
+from typing import FrozenSet, Optional, Set, Tuple

 from googleapiclient import discovery
-from googleapiclient import http

 import common

 # AppEngine services under management.
 SERVICES = frozenset(['backend', 'default', 'pubapi', 'tools'])
-# Forces 'list' calls (for services and versions) to return all
-# results in one shot, to avoid having to handle pagination. This values
-# should be greater than the maximum allowed services and versions in any
-# project (
-# https://cloud.google.com/appengine/docs/standard/python/an-overview-of-app-engine#limits).
-_PAGE_SIZE = 250
 # Number of times to check the status of an operation before timing out.
 _STATUS_CHECK_TIMES = 5
 # Delay between status checks of a long-running operation, in seconds
 _STATUS_CHECK_INTERVAL = 5


-class PagingError(Exception):
-    """Error for unexpected partial results.
-
-    List calls in this module do not handle pagination. This error is raised
-    when a partial result is received.
-    """
-    def __init__(self, uri: str):
-        super().__init__(
-            self, f'Received paged response unexpectedly when calling {uri}. '
-            'Consider increasing _PAGE_SIZE.')
-
-
 class AppEngineAdmin:
    """Wrapper around the AppEngine Admin REST API client.

@@ -55,9 +36,16 @@ class AppEngineAdmin:
    """
    def __init__(self,
                 project: str,
-                 service_lookup: discovery.Resource = None,
+                 service_lookup: Optional[discovery.Resource] = None,
                 status_check_interval: int = _STATUS_CHECK_INTERVAL) -> None:
-        """Initialize this instance for an AppEngine(GCP) project."""
+        """Initialize this instance for an AppEngine(GCP) project.
+
+        Args:
+            project: The GCP project name of this AppEngine instance.
+            service_lookup: The GCP discovery handle for service API lookup.
+            status_check_interval: The delay in seconds between status queries
+                when executing long running operations.
+        """
        self._project = project

        if service_lookup is not None:
@@ -66,6 +54,8 @@ class AppEngineAdmin:
            apps = discovery.build('appengine', 'v1beta').apps()

        self._services = apps.services()
+        self._versions = self._services.versions()
+        self._instances = self._versions.instances()
        self._operations = apps.operations()
        self._status_check_interval = status_check_interval

@@ -73,14 +63,6 @@ class AppEngineAdmin:
    def project(self):
        return self._project

-    def _checked_request(self, request: http.HttpRequest) -> Dict[str, Any]:
-        """Verifies that all results are returned for a request."""
-        response = request.execute()
-        if 'nextPageToken' in response:
-            raise PagingError(request.uri)
-
-        return response
-
    def get_serving_versions(self) -> FrozenSet[common.VersionKey]:
        """Returns the serving versions of every Nomulus service.

@@ -92,14 +74,15 @@ class AppEngineAdmin:
        Returns: An immutable collection of the serving versions grouped by
            service.
        """
-        response = self._checked_request(
-            self._services.list(appsId=self._project, pageSize=_PAGE_SIZE))
+        services = common.list_all_pages(self._services.list,
+                                         'services',
+                                         appsId=self._project)

        # Response format is specified at
-        # http://googleapis.github.io/google-api-python-client/docs/dyn/appengine_v1beta5.apps.services.html#list.
+        # http://googleapis.github.io/google-api-python-client/docs/dyn/appengine_v1beta.apps.services.html#list.

        versions = []
-        for service in response.get('services', []):
+        for service in services:
            if service['id'] in SERVICES:
                # yapf: disable
                versions_with_traffic = (
@@ -134,15 +117,15 @@ class AppEngineAdmin:
        # Sort the requested services for ease of testing. For now the mocked
        # AppEngine admin in appengine_test can only respond in a fixed order.
        for service_id in sorted(requested_services):
-            response = self._checked_request(self._services.versions().list(
-                appsId=self._project,
-                servicesId=service_id,
-                pageSize=_PAGE_SIZE))
+            response = common.list_all_pages(self._versions.list,
+                                             'versions',
+                                             appsId=self._project,
+                                             servicesId=service_id)

            # Format of version_list is defined at
-            # https://googleapis.github.io/google-api-python-client/docs/dyn/appengine_v1beta5.apps.services.versions.html#list.
+            # https://googleapis.github.io/google-api-python-client/docs/dyn/appengine_v1beta.apps.services.versions.html#list.

-            for version in response.get('versions', []):
+            for version in response:
                if common.VersionKey(service_id, version['id']) in versions:
                    scalings = [
                        s for s in list(common.AppEngineScaling)
@@ -165,16 +148,34 @@ class AppEngineAdmin:

        return frozenset(version_configs)

+    def list_instances(
+            self,
+            version: common.VersionKey) -> Tuple[common.VmInstanceInfo, ...]:
+        instances = common.list_all_pages(self._versions.instances().list,
+                                          'instances',
+                                          appsId=self._project,
+                                          servicesId=version.service_id,
+                                          versionsId=version.version_id)
+
+        # Format of version_list is defined at
+        # https://googleapis.github.io/google-api-python-client/docs/dyn/appengine_v1beta.apps.services.versions.instances.html#list
+
+        return tuple([
+            common.VmInstanceInfo(
+                inst['id'], common.parse_gcp_timestamp(inst['startTime']))
+            for inst in instances
+        ])
+
    def set_manual_scaling_num_instance(self, service_id: str, version_id: str,
                                        manual_instances: int) -> None:
        """Creates an request to change an AppEngine version's status."""
        update_mask = 'manualScaling.instances'
        body = {'manualScaling': {'instances': manual_instances}}
-        response = self._services.versions().patch(appsId=self._project,
-                                                   servicesId=service_id,
-                                                   versionsId=version_id,
-                                                   updateMask=update_mask,
-                                                   body=body).execute()
+        response = self._versions.patch(appsId=self._project,
+                                        servicesId=service_id,
+                                        versionsId=version_id,
+                                        updateMask=update_mask,
+                                        body=body).execute()

        operation_id = response.get('name').split('operations/')[1]
        for _ in range(_STATUS_CHECK_TIMES):
@@ -17,11 +17,14 @@ import unittest
 from unittest import mock
 from unittest.mock import patch

+from googleapiclient import http
+
 import appengine
 import common


-def setup_appengine_admin() -> Tuple[object, object]:
+def setup_appengine_admin(
+) -> Tuple[appengine.AppEngineAdmin, http.HttpRequest]:
    """Helper for setting up a mocked AppEngineAdmin instance.

    Returns:
@@ -32,7 +35,7 @@ def setup_appengine_admin() -> Tuple[object, object]:
    # Assign mocked API response to mock_request.execute.
    mock_request = mock.MagicMock()
    mock_request.uri.return_value = 'myuri'
-    # Mocked resource shared by services, versions, and operations.
+    # Mocked resource shared by services, versions, instances, and operations.
    resource = mock.MagicMock()
    resource.list.return_value = mock_request
    resource.get.return_value = mock_request
@@ -41,6 +44,7 @@ def setup_appengine_admin() -> Tuple[object, object]:
    apps = mock.MagicMock()
    apps.services.return_value = resource
    resource.versions.return_value = resource
+    resource.instances.return_value = resource
    apps.operations.return_value = resource
    service_lookup = mock.MagicMock()
    service_lookup.apps.return_value = apps
@@ -66,11 +70,6 @@ class AppEngineTestCase(unittest.TestCase):
        else:
            self._mock_request.execute.return_value = responses

-    def test_checked_request_multipage_raises(self) -> None:
-        self._set_mocked_response({'nextPageToken': ''})
-        self.assertRaises(appengine.PagingError,
-                          self._client.get_serving_versions)
-
    def test_get_serving_versions(self) -> None:
        self._set_mocked_response({
            'services': [{
@@ -11,13 +11,16 @@
 # 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.
-"""Declares data types that describe AppEngine services and versions."""
+"""Data types and utilities common to the other modules in this package."""

 import dataclasses
+import datetime
 import enum
 import pathlib
 import re
-from typing import Optional
+from typing import Any, Optional, Tuple
+
+from google.protobuf import timestamp_pb2


 class CannotRollbackError(Exception):
@@ -91,6 +94,13 @@ class VersionConfig(VersionKey):
    manual_scaling_instances: Optional[int] = None


+@dataclasses.dataclass(frozen=True)
+class VmInstanceInfo:
+    """Information about an AppEngine VM instance."""
+    instance_name: str
+    start_time: datetime.datetime
+
+
 def get_nomulus_root() -> str:
    """Finds the current Nomulus root directory.

@@ -109,3 +119,63 @@ def get_nomulus_root() -> str:

    raise RuntimeError(
        'Do not move this file out of the Nomulus directory tree.')
+
+
+def list_all_pages(func, data_field: str, *args, **kwargs) -> Tuple[Any, ...]:
+    """Collects all data items from a paginator-based 'List' API.
+
+    Args:
+        func: The GCP API method that supports paged responses.
+        data_field: The field in a response object containing the data
+            items to be returned. This is guaranteed to be an Iterable
+            type.
+        *args: Positional arguments passed to func.
+        *kwargs: Keyword arguments passed to func.
+
+    Returns: An immutable collection of data items assembled from the
+        paged responses.
+    """
+    result_collector = []
+    page_token = None
+    while True:
+        request = func(*args, pageToken=page_token, **kwargs)
+        response = request.execute()
+        result_collector.extend(response.get(data_field, []))
+        page_token = response.get('nextPageToken')
+        if not page_token:
+            return tuple(result_collector)
+
+
+def parse_gcp_timestamp(timestamp: str) -> datetime.datetime:
+    """Parses a timestamp string in GCP API to datetime.
+
+    This method uses protobuf's Timestamp class to parse timestamp strings.
+    This class is used by GCP APIs to parse timestamp strings, and is tolerant
+    to certain cases which can break datetime as of Python 3.8, e.g., the
+    trailing 'Z' as timezone, and fractional seconds with number of digits
+    other than 3 or 6.
+
+    Args:
+        timestamp: A string in RFC 3339 format.
+
+    Returns: A datetime instance.
+    """
+    ts = timestamp_pb2.Timestamp()
+    ts.FromJsonString(timestamp)
+    return ts.ToDatetime()
+
+
+def to_gcp_timestamp(timestamp: datetime.datetime) -> str:
+    """Converts a datetime to string.
+
+    This method uses protobuf's Timestamp class to parse timestamp strings.
+    This class is used by GCP APIs to parse timestamp strings.
+
+    Args:
+        timestamp: The datetime instance to be converted.
+
+    Returns: A string in RFC 3339 format.
+    """
+    ts = timestamp_pb2.Timestamp()
+    ts.FromDatetime(timestamp)
+    return ts.ToJsonString()
@@ -0,0 +1,70 @@
+# Copyright 2020 The Nomulus Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Unit tests for the common module."""
+import datetime
+import unittest
+from unittest import mock
+from unittest.mock import call, patch
+
+import common
+
+
+class CommonTestCase(unittest.TestCase):
+    """Unit tests for the common module."""
+    def setUp(self) -> None:
+        self._mock_request = mock.MagicMock()
+        self._mock_api = mock.MagicMock()
+        self._mock_api.list.return_value = self._mock_request
+        self.addCleanup(patch.stopall)
+
+    def test_list_all_pages_single_page(self):
+        self._mock_request.execute.return_value = {'data': [1]}
+        response = common.list_all_pages(self._mock_api.list,
+                                         'data',
+                                         appsId='project')
+        self.assertSequenceEqual(response, [1])
+        self._mock_api.list.assert_called_once_with(pageToken=None,
+                                                    appsId='project')
+
+    def test_list_all_pages_multi_page(self):
+        self._mock_request.execute.side_effect = [{
+            'data': [1],
+            'nextPageToken': 'token'
+        }, {
+            'data': [2]
+        }]
+        response = common.list_all_pages(self._mock_api.list,
+                                         'data',
+                                         appsId='project')
+        self.assertSequenceEqual(response, [1, 2])
+        self.assertSequenceEqual(self._mock_api.list.call_args_list, [
+            call(pageToken=None, appsId='project'),
+            call(pageToken='token', appsId='project')
+        ])
+
+    def test_parse_timestamp(self):
+        self.assertEqual(common.parse_gcp_timestamp('2020-01-01T00:00:00Z'),
+                         datetime.datetime(2020, 1, 1))
+
+    def test_parse_timestamp_irregular_nano_digits(self):
+        # datetime only accepts 3 or 6 digits in fractional second.
+        self.assertRaises(
+            ValueError,
+            lambda: datetime.datetime.fromisoformat('2020-01-01T00:00:00.9'))
+        self.assertEqual(common.parse_gcp_timestamp('2020-01-01T00:00:00.9Z'),
+                         datetime.datetime(2020, 1, 1, microsecond=900000))
+
+
+if __name__ == '__main__':
+    unittest.main()
@@ -51,7 +51,7 @@ class ServiceRollback:
 def _get_service_rollback_plan(
        target_configs: FrozenSet[common.VersionConfig],
        serving_configs: FrozenSet[common.VersionConfig]
-) -> Tuple[ServiceRollback]:
+) -> Tuple[ServiceRollback, ...]:
    # yapf: enable
    """Determines the versions to bring up/down in each service.

@@ -111,7 +111,7 @@ def _generate_steps(
        appengine_admin: appengine.AppEngineAdmin,
        env: str,
        target_release: str,
-        rollback_plan: Tuple[ServiceRollback]
+        rollback_plan: Tuple[ServiceRollback, ...]
 ) -> Tuple[steps.RollbackStep, ...]:
    # yapf: enable
    """Generates the sequence of operations for execution.
@@ -158,11 +158,11 @@ def _generate_steps(

    for plan in rollback_plan:
        for version in plan.serving_versions:
-            if plan.target_version.scaling != common.AppEngineScaling.AUTOMATIC:
+            if version.scaling != common.AppEngineScaling.AUTOMATIC:
                rollback_steps.append(
                    steps.start_or_stop_version(appengine_admin.project,
                                                'stop', version))
-            if plan.target_version.scaling == common.AppEngineScaling.MANUAL:
+            if version.scaling == common.AppEngineScaling.MANUAL:
                # Release all but one instances. Cannot set num_instances to 0
                # with this api.
                rollback_steps.append(
@@ -180,7 +180,7 @@ def _generate_steps(

 def get_rollback_plan(gcs_client: gcs.GcsClient,
                      appengine_admin: appengine.AppEngineAdmin, env: str,
-                      target_release: str) -> Tuple[steps.RollbackStep]:
+                      target_release: str) -> Tuple[steps.RollbackStep, ...]:
    """Generates the sequence of rollback operations for execution."""
    target_versions = gcs_client.get_versions_by_release(env, target_release)
    serving_versions = appengine_admin.get_serving_versions()
@@ -0,0 +1,186 @@
+# Copyright 2020 The Nomulus Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Script to rolling-restart the Nomulus server on AppEngine.
+
+This script effects a rolling restart of the Nomulus server by deleting VM
+instances at a controlled pace and leave it to the AppEngine scaling policy
+to bring up new VM instances.
+
+For each service, this script gets a list of VM instances and sequentially
+handles each instance as follows:
+1. Issue a gcloud delete command for this instance.
+2. Poll the AppEngine at fixed intervals until this instance no longer exists.
+
+Instance deletion is not instantaneous. An instance actively processing
+requests takes time to shutdown, and its replacement almost always comes
+up immediately after the shutdown. For this reason, we believe that our current
+implementation is sufficient safe, and will not pursue more sophisticated
+algorithms.
+
+Note that for backend instances that may handle large queries, it may take tens
+of seconds, even minutes, to shut down one of them.
+
+This script also accepts an optional start_time parameter that serves as a
+filter of instances to delete: only those instances that started before this
+time will be deleted. This parameter makes error handling easy. When this
+script fails, simply rerun with the same start_time until it succeeds.
+"""
+import argparse
+import datetime
+import sys
+import time
+from typing import Iterable, Optional, Tuple
+
+import appengine
+import common
+import steps
+
+HELP_MAIN = 'Script to rolling-restart the Nomulus server on AppEngine'
+HELP_MIN_DELAY = 'Minimum delay in seconds between instance deletions.'
+HELP_MIN_LIVE_INSTANCE_PERCENT = (
+    'Minimum number of instances to keep, as a percentage '
+    'of the total at the beginning of the restart process.')
+
+
+# yapf: disable
+def generate_steps(
+        appengine_admin: appengine.AppEngineAdmin,
+        version: common.VersionKey,
+        started_before: datetime.datetime
+) -> Tuple[steps.KillNomulusInstance, ...]:
+    # yapf: enable
+    instances = appengine_admin.list_instances(version)
+    return tuple([
+        steps.kill_nomulus_instance(appengine_admin.project, version,
+                                    inst.instance_name) for inst in instances
+        if inst.start_time <= started_before
+    ])
+
+
+def execute_steps(appengine_admin: appengine.AppEngineAdmin,
+                  version: common.VersionKey,
+                  cmds: Tuple[steps.KillNomulusInstance, ...], min_delay: int,
+                  configured_num_instances: Optional[int]) -> None:
+    print(f'Restarting {len(cmds)} instances in {version.service_id}')
+    for cmd in cmds:
+        print(cmd.info())
+        cmd.execute()
+
+        while True:
+            time.sleep(min_delay)
+            running_instances = [
+                inst.instance_name
+                for inst in appengine_admin.list_instances(version)
+            ]
+            if cmd.instance_name in running_instances:
+                print('Waiting for VM to shut down...')
+                continue
+            if (configured_num_instances is not None
+                    and len(running_instances) < configured_num_instances):
+                print('Waiting for new VM to come up...')
+                continue
+            break
+        print('VM instance has shut down.\n')
+
+    print(f'Done: {len(cmds)} instances in {version.service_id}\n')
+
+
+# yapf: disable
+def restart_one_service(appengine_admin: appengine.AppEngineAdmin,
+                        version: common.VersionKey,
+                        min_delay: int,
+                        started_before: datetime.datetime,
+                        configured_num_instances: Optional[int]) -> None:
+    # yapf: enable
+    """Restart VM instances in one service according to their start time.
+
+    Args:
+        appengine_admin: The client of AppEngine Admin API.
+        version: The Nomulus version to restart. This must be the currently
+            serving version.
+        min_delay: The minimum delay between successive deletions.
+        started_before: Only VM instances started before this time are to be
+            deleted.
+        configured_num_instances: When present, the constant number of instances
+            this version is configured with.
+    """
+    cmds = generate_steps(appengine_admin, version, started_before)
+    # yapf: disable
+    execute_steps(
+        appengine_admin, version, cmds, min_delay, configured_num_instances)
+    # yapf: enable
+
+
+# yapf: disable
+def rolling_restart(project: str,
+                    services: Iterable[str],
+                    min_delay: int,
+                    started_before: datetime.datetime):
+    # yapf: enable
+    print(f'Rolling restart {project} at '
+          f'{common.to_gcp_timestamp(started_before)}\n')
+    appengine_admin = appengine.AppEngineAdmin(project)
+    version_configs = appengine_admin.get_version_configs(
+        set(appengine_admin.get_serving_versions()))
+    restart_versions = [
+        version for version in version_configs
+        if version.service_id in services
+    ]
+    # yapf: disable
+    for version in restart_versions:
+        restart_one_service(appengine_admin,
+                            version,
+                            min_delay,
+                            started_before,
+                            version.manual_scaling_instances)
+    # yapf: enable
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(prog='rolling_restart',
+                                     description=HELP_MAIN)
+    parser.add_argument('--project',
+                        '-p',
+                        required=True,
+                        help='The GCP project of the Nomulus server.')
+    parser.add_argument('--services',
+                        '-s',
+                        nargs='+',
+                        choices=appengine.SERVICES,
+                        default=appengine.SERVICES,
+                        help='The services to rollback.')
+    parser.add_argument('--min_delay',
+                        '-d',
+                        type=int,
+                        default=5,
+                        choices=range(1, 100),
+                        help=HELP_MIN_DELAY)
+    parser.add_argument(
+        '--started_before',
+        '-b',
+        type=common.parse_gcp_timestamp,
+        default=datetime.datetime.utcnow(),
+        help='Only kill VM instances started before this time.')
+
+    args = parser.parse_args()
+    rolling_restart(**vars(args))
+    return 0
+
+
+if __name__ == '__main__':
+    try:
+        sys.exit(main())
+    except Exception as ex:  # pylint: disable=broad-except
+        print(ex)
+        sys.exit(1)
@@ -0,0 +1,149 @@
+# Copyright 2020 The Nomulus Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Unit tests of rolling_restart."""
+
+import datetime
+import unittest
+from unittest import mock
+
+import common
+import rolling_restart
+import steps
+
+import appengine_test
+
+
+class RollingRestartTestCase(unittest.TestCase):
+    """Tests for rolling_restart."""
+    def setUp(self) -> None:
+        self._appengine_admin, self._appengine_request = (
+            appengine_test.setup_appengine_admin())
+        self._version = common.VersionKey('my_service', 'my_version')
+        self.addCleanup(mock.patch.stopall)
+
+    def _setup_execute_steps_tests(self):
+        self._appengine_request.execute.side_effect = [
+            # First list_instance response.
+            {
+                'instances': [{
+                    'id': 'vm_to_delete',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }, {
+                    'id': 'vm_to_stay',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }]
+            },
+            # Second list_instance response
+            {
+                'instances': [{
+                    'id': 'vm_to_stay',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }]
+            },
+            # Third list_instance response
+            {
+                'instances': [{
+                    'id': 'vm_to_stay',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }, {
+                    'id': 'vm_new',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }]
+            }
+        ]
+
+    def _setup_generate_steps_tests(self):
+        self._appengine_request.execute.side_effect = [
+            # First page of list_instance response.
+            {
+                'instances': [{
+                    'id': 'vm_2019',
+                    'startTime': '2019-01-01T00:00:00Z'
+                }],
+                'nextPageToken':
+                'token'
+            },
+            # Second and final page of list_instance response
+            {
+                'instances': [{
+                    'id': 'vm_2020',
+                    'startTime': '2020-01-01T00:00:00Z'
+                }]
+            }
+        ]
+
+    def test_kill_vm_command(self) -> None:
+        cmd = steps.kill_nomulus_instance(
+            'my_project', common.VersionKey('my_service', 'my_version'),
+            'my_inst')
+        self.assertEqual(cmd.instance_name, 'my_inst')
+        self.assertIn(('gcloud app instances delete my_inst --quiet '
+                       '--user-output-enabled=false --service my_service '
+                       '--version my_version --project my_project'),
+                      cmd.info())
+
+    def _generate_kill_vm_command(self, version: common.VersionKey,
+                                  instance_name: str):
+        return steps.kill_nomulus_instance(self._appengine_admin.project,
+                                           version, instance_name)
+
+    def test_generate_commands(self):
+        self._setup_generate_steps_tests()
+        commands = rolling_restart.generate_steps(self._appengine_admin,
+                                                  self._version,
+                                                  datetime.datetime.utcnow())
+        self.assertSequenceEqual(commands, [
+            self._generate_kill_vm_command(self._version, 'vm_2019'),
+            self._generate_kill_vm_command(self._version, 'vm_2020')
+        ])
+
+    def test_generate_commands_older_vm(self):
+        self._setup_generate_steps_tests()
+        version = common.VersionKey('my_service', 'my_version')
+        # yapf: disable
+        commands = rolling_restart.generate_steps(
+            self._appengine_admin,
+            version,
+            common.parse_gcp_timestamp('2019-12-01T00:00:00Z'))
+        # yapf: enable
+        self.assertSequenceEqual(
+            commands, [self._generate_kill_vm_command(version, 'vm_2019')])
+
+    def test_execute_steps_variable_instances(self):
+        self._setup_execute_steps_tests()
+        cmd = mock.MagicMock()
+        cmd.instance_name = 'vm_to_delete'
+        cmds = tuple([cmd])  # yapf does not format (cmd,) correctly.
+        rolling_restart.execute_steps(appengine_admin=self._appengine_admin,
+                                      version=self._version,
+                                      cmds=cmds,
+                                      min_delay=0,
+                                      configured_num_instances=None)
+        self.assertEqual(self._appengine_request.execute.call_count, 2)
+
+    def test_execute_steps_fixed_instances(self):
+        self._setup_execute_steps_tests()
+        cmd = mock.MagicMock()
+        cmd.instance_name = 'vm_to_delete'
+        cmds = tuple([cmd])  # yapf does not format (cmd,) correctly.
+        rolling_restart.execute_steps(appengine_admin=self._appengine_admin,
+                                      version=self._version,
+                                      cmds=cmds,
+                                      min_delay=0,
+                                      configured_num_instances=2)
+        self.assertEqual(self._appengine_request.execute.call_count, 3)
+
+
+if __name__ == '__main__':
+    unittest.main()
@@ -122,6 +122,24 @@ def direct_service_traffic_to_version(
         '--quiet', f'--splits={version.version_id}=1', '--project', project))


+@dataclasses.dataclass(frozen=True)
+class KillNomulusInstance(RollbackStep):
+    """Step that kills a Nomulus VM instance."""
+    instance_name: str
+
+
+# yapf: disable
+def kill_nomulus_instance(project: str,
+                          version: common.VersionKey,
+                          instance_name: str) -> KillNomulusInstance:
+    # yapf: enable
+    return KillNomulusInstance(
+        'Delete one VM instance.',
+        ('gcloud', 'app', 'instances', 'delete', instance_name, '--quiet',
+         '--user-output-enabled=false', '--service', version.service_id,
+         '--version', version.version_id, '--project', project), instance_name)
+
+
@dataclasses.dataclass(frozen=True)
 class _UpdateDeployTag(RollbackStep):
    """Updates the deployment tag on GCS."""
Author	SHA1	Message	Date
gbrodman	4f0189c162	Convert DatastoreEntity/SqlEntity to return Optional, not List (#894 ) * Convert DatastoreEntity/SqlEntity to return Optional, not List We don't have any entities that convert to more than one entity, so we can use an Optional instead for clarity and simplicity.	2020-12-02 17:29:01 -05:00
sarahcaseybot	59c852d812	Add an HTTP header to response from Nomulus after successful login (#879 ) * Add a logged-in response header * small fixes * Refactor EPP test cases to check for headers * small change	2020-12-01 19:24:56 -05:00
sarahcaseybot	2621448f5e	Remove ability to set only the certificate hash for a registrar (#891 )	2020-12-01 14:28:45 -05:00
Weimin Yu	94ef81dca4	Script to rolling-start Nomulus (#888 ) * Script to rolling-start Nomulus Add a script to restart Nomulus non-disruptively. This can be used after a configuration change to external resources (e.g., Cloud SQL credential) to make Nomulus pick up the latest config. Also added proper support to paging based List api methods, replacing the current hack that forces the server to return everything in one response. The List method for instances has a lower limit on page size than others which is not sufficient for our project.	2020-12-01 10:14:05 -05:00
Michael Muller	64e1a4b345	Make Domain -> BillingEvent FK deferred (#890 ) * Make Domain -> BillingEvent FK deferred It appears that Hibernate can sporadically introduce FK constraint failures when updating a Domain to reference a new BillingEvent and then deleting the old BillingEvent, causing a flakey test failure in DomainDeleteFlowTest. This may be due to the fact that this FK relationships is not known to hibernate. An alternate solution appears to be to flush after every update, but that likely has some pretty serious performance implications.	2020-11-30 18:06:07 -05:00
Michael Muller	cde1c78f5e	Add replay-testing to DomainDeleteFlowTest (#886 ) * Minor fixes: - Initialize "requestedByRegistrar" to false (it's non-nullable). - Store test entities (registrar, hosts and contacts) in JPA. * Flyway changes * Add ReplayExtension to DomainDeleteFlowTest * Check in latest ER diagrams	2020-11-25 11:15:10 -05:00
gbrodman	3b1c198c11	Ensure that all relevant Keys can be converted to VKeys (#883 ) * Ensure that all relevant Keys can be converted to VKeys When replaying commit logs to SQL (specifically deletes) we will need to convert Datastore Keys to SQL VKeys in order to know what (if anything) to delete. The test added to EntityTest (and the associated code changes) mean that for every relevant object we'll be able to call VKeyTranslatorFactory.createVKey(Key<?>) for all possible keys that we care about. Note that we do not care about entities that only exist in Datastore or entities that are non-replicated -- by their nature, deletes for those types of objects in Datastore are not relevant in SQL. * Responses to code review - changing comments / method names - using ModelUtils	2020-11-24 14:33:06 -05:00
Michael Muller	3afcc0dcb4	Fix DatastoreHelper -> DatabaseHelper in comments (#885 ) * Fix DatastoreHelper -> DatabaseHelper in comments Fix a few comments that still make reference to DatastoreHelper.	2020-11-24 14:32:15 -05:00
Michael Muller	67c6d73a18	Fix DomainHistory merge issues (#884 ) * Reproduce DomainHistory double write failure * Add fix for cascade sets and clean up hacks * Fix DatastoreHelper to work with name change. * Remove Ignored entities from ofy schema	2020-11-24 11:42:00 -05:00
Weimin Yu	33499aaf9e	Make sure post load work happens in GracePeriod (#878 ) * Make sure post load work happens in GracePeriod The GracePeriod method with ofy @OnLoad annotation is not called. Apparently Ofy only checks for @OnLoad on first-class entities, not embedded ones. Added a call to this method from DomainContent's OnLoad method. Reproduced issue with a test and verified that the fix works.	2020-11-23 13:47:27 -05:00