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.
Add replay-testing to DomainDeleteFlowTest (#886 )
2026-06-09 16:33:02 +00:00 · 2020-11-30 18:06:07 -05:00 · 2020-11-25 11:15:10 -05:00 · 2020-11-24 14:33:06 -05:00 · 2020-11-24 14:32:15 -05:00 · 2020-11-24 11:42:00 -05:00
41 changed files with 454 additions and 364 deletions
@@ -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);
  }

@@ -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
-  }
 }
@@ -95,9 +95,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
@@ -116,7 +116,7 @@ public class ContactHistory extends HistoryEntry implements SqlEntity {
  }

  /** 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 +125,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);
@@ -101,6 +101,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 +118,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 +137,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 +227,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
@@ -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();
@@ -96,9 +96,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
@@ -117,7 +117,7 @@ public class HostHistory extends HistoryEntry implements SqlEntity {
  }

  /** 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 +126,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.
   *
@@ -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;
@@ -900,16 +897,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 +937,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 +956,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 +977,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> {

@@ -32,13 +32,17 @@ 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.Set;
@@ -314,6 +318,33 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor
    return ImmutableList.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 */
  public static class Builder<T extends HistoryEntry, B extends Builder<?, ?>>
      extends GenericBuilder<T, B> {
@@ -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
-    }
  }

  /**
@@ -29,6 +29,7 @@ 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.NonReplicatedEntity;
 import google.registry.schema.replay.SqlEntity;
 import java.io.Serializable;
 import java.util.Optional;
@@ -44,7 +45,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;

@@ -0,0 +1,24 @@
+// 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 com.google.common.collect.ImmutableList;
+
+public interface DatastoreOnlyEntity extends DatastoreEntity {
+  @Override
+  default ImmutableList<SqlEntity> toSqlEntities() {
+    return ImmutableList.of();
+  }
+}
@@ -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;
@@ -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
@@ -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));
    }
@@ -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;


 --
Author	SHA1	Message	Date
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