Convert all VKeys to symmetric VKeys (#641)

* Convert hosts to symmetric VKey

* Convert ContactResource to symmetric VKeys

* Convert BillingEvents to symmetric VKeys

* Converted PollMessage to symmetric VKeys

* Convert AllocationToken to symmetric VKeys

* Remove static methods, get everything working

* Changes requested in review.

* Removed newly introduced createOfy() calls

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

@@ -285,11 +285,9 @@ public class DeleteContactsAndHostsAction implements Runnable {
       if (resourceKey.getKind().equals(KIND_CONTACT)) {
         return domain
             .getReferencedContacts()
-            .contains(VKey.createOfy(ContactResource.class, (Key<ContactResource>) resourceKey));
+            .contains(VKey.from((Key<ContactResource>) resourceKey));
       } else if (resourceKey.getKind().equals(KIND_HOST)) {
-        return domain
+        return domain.getNameservers().contains(VKey.from((Key<HostResource>) resourceKey));
-            .getNameservers()
-            .contains(VKey.createOfy(HostResource.class, (Key<HostResource>) resourceKey));
       } else {
         throw new IllegalStateException("EPP resource key of unknown type: " + resourceKey);
       }

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

@@ -207,9 +207,7 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
       Key<HostResource> referencingHostKey = null;
       for (DnsRefreshRequest request : refreshRequests) {
         if (isActive(domain, request.lastUpdateTime())
-            && domain
+            && domain.getNameservers().contains(VKey.from(request.hostKey()))) {
-                .getNameservers()
-                .contains(VKey.createOfy(HostResource.class, request.hostKey()))) {
           referencingHostKey = request.hostKey();
           break;
         }

core/src/main/java/google/registry/model/billing/BillingEvent.java

@@ -347,7 +347,27 @@ public abstract class BillingEvent extends ImmutableObject
 
     @Override
     public VKey<OneTime> createVKey() {
-      return VKey.createOfy(getClass(), Key.create(this));
+      return VKey.create(
+          getClass(),
+          parent.getParent().getName() + "/" + parent.getId() + "/" + getId(),
+          Key.create(this));
+    }
+
+    public static VKey<OneTime> createVKey(Key<OneTime> key) {
+      // TODO(b/159207551): As it stands, the SQL key generated here doesn't mesh with the primary
+      // key type for the table, which is a single long integer.
+      if (key == null) {
+        return null;
+      }
+      Key parent = key.getParent();
+      Key grandparent = (parent != null) ? parent.getParent() : null;
+      String path =
+          (grandparent != null ? grandparent.getName() : "")
+              + "/"
+              + (parent != null ? parent.getId() : "")
+              + "/"
+              + key.getId();
+      return VKey.create(OneTime.class, path, key);
     }
 
     @Override
@@ -485,7 +505,21 @@ public abstract class BillingEvent extends ImmutableObject
 
     @Override
     public VKey<Recurring> createVKey() {
-      return VKey.createOfy(getClass(), Key.create(this));
+      return VKey.create(
+          getClass(),
+          parent.getParent().getName() + "/" + parent.getId() + "/" + getId(),
+          Key.create(this));
+    }
+
+    public static VKey<Recurring> createVKey(Key<Recurring> key) {
+      // TODO(b/159207551): As it stands, the SQL key generated here doesn't mesh with the primary
+      // key type for the table, which is a single long integer.
+      if (key == null) {
+        return null;
+      }
+      String path =
+          key.getParent().getParent().getName() + "/" + key.getParent().getId() + "/" + key.getId();
+      return VKey.create(Recurring.class, path, key);
     }
 
     @Override
@@ -596,18 +630,30 @@ public abstract class BillingEvent extends ImmutableObject
           .setParent(historyEntry);
       // Set the grace period's billing event using the appropriate Cancellation builder method.
       if (gracePeriod.getOneTimeBillingEvent() != null) {
-        builder.setOneTimeEventKey(
+        builder.setOneTimeEventKey(VKey.from(gracePeriod.getOneTimeBillingEvent()));
-            VKey.createOfy(BillingEvent.OneTime.class, gracePeriod.getOneTimeBillingEvent()));
       } else if (gracePeriod.getRecurringBillingEvent() != null) {
-        builder.setRecurringEventKey(
+        builder.setRecurringEventKey(VKey.from(gracePeriod.getRecurringBillingEvent()));
-            VKey.createOfy(BillingEvent.Recurring.class, gracePeriod.getRecurringBillingEvent()));
       }
       return builder.build();
     }
 
     @Override
     public VKey<Cancellation> createVKey() {
-      return VKey.createOfy(getClass(), Key.create(this));
+      return VKey.create(
+          getClass(),
+          parent.getParent().getName() + "/" + parent.getId() + "/" + getId(),
+          Key.create(this));
+    }
+
+    public static VKey<Cancellation> createVKey(Key<Cancellation> key) {
+      // TODO(b/159207551): As it stands, the SQL key generated here doesn't mesh with the primary
+      // key type for the table, which is a single long integer.
+      if (key == null) {
+        return null;
+      }
+      String path =
+          key.getParent().getParent().getName() + "/" + key.getParent().getId() + "/" + key.getId();
+      return VKey.create(Cancellation.class, path, key);
     }
 
     @Override
@@ -687,7 +733,21 @@ public abstract class BillingEvent extends ImmutableObject
 
     @Override
     public VKey<Modification> createVKey() {
-      return VKey.createOfy(this.getClass(), Key.create(this));
+      return VKey.create(
+          getClass(),
+          parent.getParent().getName() + "/" + parent.getId() + "/" + getId(),
+          Key.create(this));
+    }
+
+    public static VKey<Modification> createVKey(Key<Modification> key) {
+      // TODO(b/159207551): As it stands, the SQL key generated here doesn't mesh with the primary
+      // key type for the table, which is a single long integer.
+      if (key == null) {
+        return null;
+      }
+      String path =
+          key.getParent().getParent().getName() + "/" + key.getParent().getId() + "/" + key.getId();
+      return VKey.create(Modification.class, path, key);
     }
 
     /**

core/src/main/java/google/registry/model/contact/ContactResource.java

@@ -200,8 +200,7 @@ public class ContactResource extends EppResource
 
   @Override
   public VKey<ContactResource> createVKey() {
-    // TODO(mmuller): create symmetric keys if we can ever reload both sides.
+    return VKey.create(ContactResource.class, getRepoId(), Key.create(this));
-    return VKey.createOfy(ContactResource.class, Key.create(this));
   }
 
   @Override

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

@@ -85,6 +85,6 @@ public class DesignatedContact extends ImmutableObject {
   }
 
   public DesignatedContact reconstitute() {
-    return create(type, VKey.createOfy(ContactResource.class, contact));
+    return create(type, VKey.from(contact));
   }
 }

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

@@ -643,10 +643,6 @@ public class DomainBase extends EppResource
     return VKey.create(DomainBase.class, getRepoId(), Key.create(this));
   }
 
-  public static VKey<DomainBase> createVKey(Key key) {
-    return VKey.create(DomainBase.class, key.getName(), key);
-  }
-
   /** Predicate to determine if a given {@link DesignatedContact} is the registrant. */
   private static final Predicate<DesignatedContact> IS_REGISTRANT =
       (DesignatedContact contact) -> DesignatedContact.Type.REGISTRANT.equals(contact.type);

core/src/main/java/google/registry/model/domain/token/AllocationToken.java

@@ -183,7 +183,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable {
   }
 
   public VKey<AllocationToken> createVKey() {
-    return VKey.createOfy(getClass(), Key.create(this));
+    return VKey.create(getClass(), getToken(), Key.create(this));
   }
 
   @Override

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

@@ -126,7 +126,7 @@ public class HostBase extends EppResource {
 
   @Override
   public VKey<? extends EppResource> createVKey() {
-    return VKey.createOfy(HostBase.class, Key.create(this));
+    return VKey.create(HostBase.class, getRepoId(), Key.create(this));
   }
 
   @Deprecated

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

@@ -83,7 +83,8 @@ public class HostHistory extends HistoryEntry {
     @Override
     public Builder setParent(Key<? extends EppResource> parent) {
       super.setParent(parent);
-      getInstance().hostRepoId = VKey.createOfy(HostResource.class, (Key<HostResource>) parent);
+      getInstance().hostRepoId =
+          VKey.create(HostResource.class, parent.getName(), (Key<HostResource>) parent);
       return this;
     }
   }

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

@@ -48,8 +48,7 @@ public class HostResource extends HostBase
 
   @Override
   public VKey<HostResource> createVKey() {
-    // TODO(mmuller): create symmetric keys if we can ever reload both sides.
+    return VKey.create(HostResource.class, getRepoId(), Key.create(this));
-    return VKey.createOfy(HostResource.class, Key.create(this));
   }
 
   @Override

core/src/main/java/google/registry/model/poll/PollMessage.java

@@ -154,6 +154,18 @@ public abstract class PollMessage extends ImmutableObject
   @Override
   public abstract VKey<? extends PollMessage> createVKey();
 
+  public static VKey<PollMessage> createVKey(Key<PollMessage> key) {
+    // TODO(b/159207551): As it stands, the SQL key generated here doesn't mesh with the primary key
+    // type for the table, which is a single long integer.  Also note that the class id is not
+    // correct here and as such the resulting key will not be loadable from SQL.
+    if (key == null) {
+      return null;
+    }
+    String path =
+        key.getParent().getParent().getName() + "/" + key.getParent().getId() + key.getId();
+    return VKey.create(PollMessage.class, path, key);
+  }
+
   /** Override Buildable.asBuilder() to give this method stronger typing. */
   @Override
   public abstract Builder<?, ?> asBuilder();
@@ -297,7 +309,7 @@ public abstract class PollMessage extends ImmutableObject
 
     @Override
     public VKey<OneTime> createVKey() {
-      return VKey.createOfy(this.getClass(), Key.create(this));
+      return VKey.create(this.getClass(), getId(), Key.create(this));
     }
 
     @Override
@@ -398,7 +410,7 @@ public abstract class PollMessage extends ImmutableObject
 
     @Override
     public VKey<Autorenew> createVKey() {
-      return VKey.createOfy(this.getClass(), Key.create(this));
+      return VKey.create(this.getClass(), getId(), Key.create(this));
     }
 
     @Override

core/src/main/java/google/registry/model/reporting/HistoryEntry.java

@@ -30,6 +30,7 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ReportedOn;
 import google.registry.model.domain.Period;
 import google.registry.model.eppcommon.Trid;
+import google.registry.persistence.VKey;
 import java.util.Set;
 import javax.annotation.Nullable;
 import javax.persistence.AttributeOverride;
@@ -234,6 +235,19 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
     return nullToEmptyImmutableCopy(domainTransactionRecords);
   }
 
+  public static VKey<HistoryEntry> createVKey(Key<HistoryEntry> key) {
+    // TODO(b/159207551): This will likely need some revision.  As it stands, this method was
+    // introduced purely to facilitate testing of VKey specialization in VKeyTranslatorFactory.
+    // This class will likely require that functionality, though perhaps not this implementation of
+    // it.
+    // For now, just assume that the primary key of a history entry is comprised of the parent
+    // type, key and the object identifer.
+    return VKey.create(
+        HistoryEntry.class,
+        key.getParent().getKind() + "/" + key.getParent().getName() + "/" + key.getId(),
+        key);
+  }
+
   @Override
   public Builder asBuilder() {
     return new Builder(clone(this));

core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java

@@ -67,9 +67,17 @@ public class VKeyTranslatorFactory extends AbstractSimpleTranslatorFactory<VKey,
           clazz.getDeclaredMethod("createVKey", com.googlecode.objectify.Key.class);
       return (VKey<T>) createVKeyMethod.invoke(null, new Object[] {key});
     } catch (NoSuchMethodException e) {
-      // Revert to an ofy vkey for now.  TODO(mmuller): remove this when all classes with VKeys have
+      checkArgument(
-      // converters.
+          key.getParent() == null,
-      return VKey.createOfy(clazz, key);
+          "Cannot auto-convert key %s of kind %s because it has a parent.  Add a createVKey(Key) "
+              + "method for it.",
+          key,
+          key.getKind());
+      if (key.getName() != null) {
+        return VKey.create(clazz, key.getName(), key);
+      } else {
+        return VKey.create(clazz, key.getId(), key);
+      }
     } catch (IllegalAccessException | InvocationTargetException e) {
       // If we have a createVKey(Key) method with incorrect permissions or that is non-static, this
       // is probably an error so let's reported.

core/src/main/java/google/registry/persistence/VKey.java

@@ -19,6 +19,7 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.googlecode.objectify.Key;
 import google.registry.model.ImmutableObject;
+import google.registry.model.translators.VKeyTranslatorFactory;
 import java.io.Serializable;
 import java.util.Optional;
 
@@ -46,38 +47,17 @@ public class VKey<T> extends ImmutableObject implements Serializable {
     this.primaryKey = primaryKey;
   }
 
-  /** Creates a {@link VKey} which only contains the sql primary key. */
+  /**
+   * Creates a {@link VKey} which only contains the sql primary key.
+   *
+   * <p>Deprecated. Create symmetric keys with create() instead.
+   */
   public static <T> VKey<T> createSql(Class<? extends T> kind, Object sqlKey) {
     checkArgumentNotNull(kind, "kind must not be null");
     checkArgumentNotNull(sqlKey, "sqlKey must not be null");
     return new VKey(kind, null, sqlKey);
   }
 
-  /** Creates a {@link VKey} which only contains the ofy primary key. */
-  public static <T> VKey<T> createOfy(
-      Class<? extends T> kind, com.googlecode.objectify.Key<? extends T> ofyKey) {
-    checkArgumentNotNull(kind, "kind must not be null");
-    checkArgumentNotNull(ofyKey, "ofyKey must not be null");
-    return new VKey(kind, ofyKey, null);
-  }
-
-  /**
-   * Creates a {@link VKey} which only contains the ofy primary key by specifying the id of the
-   * {@link Key}.
-   */
-  public static <T> VKey<T> createOfy(Class<? extends T> kind, long id) {
-    return createOfy(kind, Key.create(kind, id));
-  }
-
-  /**
-   * Creates a {@link VKey} which only contains the ofy primary key by specifying the name of the
-   * {@link Key}.
-   */
-  public static <T> VKey<T> createOfy(Class<? extends T> kind, String name) {
-    checkArgumentNotNull(kind, "name must not be null");
-    return createOfy(kind, Key.create(kind, name));
-  }
-
   /** Creates a {@link VKey} which only contains both sql and ofy primary key. */
   public static <T> VKey<T> create(
       Class<? extends T> kind, Object sqlKey, com.googlecode.objectify.Key ofyKey) {
@@ -87,6 +67,11 @@ public class VKey<T> extends ImmutableObject implements Serializable {
     return new VKey(kind, ofyKey, sqlKey);
   }
 
+  /** Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code id}. */
+  public static <T> VKey<T> create(Class<? extends T> kind, long id) {
+    return new VKey(kind, Key.create(kind, id), id);
+  }
+
   /** Returns the type of the entity. */
   public Class<? extends T> getKind() {
     return this.kind;
@@ -113,4 +98,9 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   public Optional<com.googlecode.objectify.Key<T>> maybeGetOfyKey() {
     return Optional.ofNullable(this.ofyKey);
   }
+
+  /** Convenience method to construct a VKey from an objectify Key. */
+  public static <T> VKey<T> from(Key<T> key) {
+    return VKeyTranslatorFactory.createVKey(key);
+  }
 }

core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java

@@ -295,7 +295,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
       query = query.filter("currentSponsorClientId", desiredRegistrar.get());
     }
     return StreamSupport.stream(query.keys().spliterator(), false)
-        .map(key -> VKey.createOfy(HostResource.class, key))
+        .map(key -> VKey.from(key))
         .collect(toImmutableSet());
   }
 
@@ -406,7 +406,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
     }
     return searchByNameserverRefs(
         StreamSupport.stream(query.keys().spliterator(), false)
-            .map(key -> VKey.createOfy(HostResource.class, key))
+            .map(key -> VKey.from(key))
             .collect(toImmutableSet()));
   }
 

core/src/test/java/google/registry/flows/EppTestCase.java

@@ -340,8 +340,7 @@ public class EppTestCase extends ShardableTestCase {
         .setTargetId(domain.getDomainName())
         .setClientId(domain.getCurrentSponsorClientId())
         .setEventTime(deleteTime)
-        .setOneTimeEventKey(
+        .setOneTimeEventKey(VKey.from(findKeyToActualOneTimeBillingEvent(billingEventToCancel)))
-            VKey.createOfy(OneTime.class, findKeyToActualOneTimeBillingEvent(billingEventToCancel)))
         .setBillingTime(createTime.plus(Registry.get(domain.getTld()).getAddGracePeriodLength()))
         .setReason(Reason.CREATE)
         .setParent(getOnlyHistoryEntryOfType(domain, Type.DOMAIN_DELETE))
@@ -355,8 +354,7 @@ public class EppTestCase extends ShardableTestCase {
         .setTargetId(domain.getDomainName())
         .setClientId(domain.getCurrentSponsorClientId())
         .setEventTime(deleteTime)
-        .setOneTimeEventKey(
+        .setOneTimeEventKey(VKey.from(findKeyToActualOneTimeBillingEvent(billingEventToCancel)))
-            VKey.createOfy(OneTime.class, findKeyToActualOneTimeBillingEvent(billingEventToCancel)))
         .setBillingTime(renewTime.plus(Registry.get(domain.getTld()).getRenewGracePeriodLength()))
         .setReason(Reason.RENEW)
         .setParent(getOnlyHistoryEntryOfType(domain, Type.DOMAIN_DELETE))

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

@@ -403,8 +403,7 @@ public class DomainTransferApproveFlowTest
             .setEventTime(clock.nowUtc()) // The cancellation happens at the moment of transfer.
             .setBillingTime(
                 oldExpirationTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()))
-            .setRecurringEventKey(
+            .setRecurringEventKey(VKey.from(domain.getAutorenewBillingEvent())));
-                VKey.createOfy(BillingEvent.Recurring.class, domain.getAutorenewBillingEvent())));
   }
 
   @Test

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

@@ -1144,8 +1144,7 @@ public class DomainTransferRequestFlowTest
             .setEventTime(clock.nowUtc().plus(Registry.get("tld").getAutomaticTransferLength()))
             .setBillingTime(autorenewTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()))
             // The cancellation should refer to the old autorenew billing event.
-            .setRecurringEventKey(
+            .setRecurringEventKey(VKey.from(existingAutorenewEvent)));
-                VKey.createOfy(BillingEvent.Recurring.class, existingAutorenewEvent)));
   }
 
   @Test
@@ -1173,8 +1172,7 @@ public class DomainTransferRequestFlowTest
             .setBillingTime(
                 expirationTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()))
             // The cancellation should refer to the old autorenew billing event.
-            .setRecurringEventKey(
+            .setRecurringEventKey(VKey.from(existingAutorenewEvent)));
-                VKey.createOfy(BillingEvent.Recurring.class, existingAutorenewEvent)));
   }
 
   @Test

core/src/test/java/google/registry/model/contact/ContactResourceTest.java

@@ -113,7 +113,7 @@ public class ContactResourceTest extends EntityTestCase {
                     .setLosingClientId("losing")
                     .setPendingTransferExpirationTime(fakeClock.nowUtc())
                     .setServerApproveEntities(
-                        ImmutableSet.of(VKey.createOfy(BillingEvent.OneTime.class, 1)))
+                        ImmutableSet.of(VKey.create(BillingEvent.OneTime.class, 1)))
                     .setTransferRequestTime(fakeClock.nowUtc())
                     .setTransferStatus(TransferStatus.SERVER_APPROVED)
                     .setTransferRequestTrid(Trid.create("client-trid", "server-trid"))

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

@@ -76,7 +76,7 @@ public class DomainBaseTest extends EntityTestCase {
         persistResource(
                 new HostResource.Builder()
                     .setHostName("ns1.example.com")
-                    .setSuperordinateDomain(VKey.createOfy(DomainBase.class, domainKey))
+                    .setSuperordinateDomain(VKey.from(domainKey))
                     .setRepoId("1-COM")
                     .build())
             .createVKey();
@@ -139,15 +139,12 @@ public class DomainBaseTest extends EntityTestCase {
                             .setPendingTransferExpirationTime(fakeClock.nowUtc())
                             .setServerApproveEntities(
                                 ImmutableSet.of(
-                                    VKey.createOfy(BillingEvent.OneTime.class, oneTimeBillKey),
+                                    VKey.from(oneTimeBillKey),
-                                    VKey.createOfy(BillingEvent.Recurring.class, recurringBillKey),
+                                    VKey.from(recurringBillKey),
-                                    VKey.createOfy(PollMessage.Autorenew.class, autorenewPollKey)))
+                                    VKey.from(autorenewPollKey)))
-                            .setServerApproveBillingEvent(
+                            .setServerApproveBillingEvent(VKey.from(oneTimeBillKey))
-                                VKey.createOfy(BillingEvent.OneTime.class, oneTimeBillKey))
+                            .setServerApproveAutorenewEvent(VKey.from(recurringBillKey))
-                            .setServerApproveAutorenewEvent(
+                            .setServerApproveAutorenewPollMessage(VKey.from(autorenewPollKey))
-                                VKey.createOfy(BillingEvent.Recurring.class, recurringBillKey))
-                            .setServerApproveAutorenewPollMessage(
-                                VKey.createOfy(PollMessage.Autorenew.class, autorenewPollKey))
                             .setTransferRequestTime(fakeClock.nowUtc().plusDays(1))
                             .setTransferStatus(TransferStatus.SERVER_APPROVED)
                             .setTransferRequestTrid(Trid.create("client-trid", "server-trid"))
@@ -750,10 +747,8 @@ public class DomainBaseTest extends EntityTestCase {
             .setPendingTransferExpirationTime(transferExpirationTime)
             .setTransferStatus(TransferStatus.PENDING)
             .setGainingClientId("TheRegistrar")
-            .setServerApproveAutorenewEvent(
+            .setServerApproveAutorenewEvent(VKey.from(recurringBillKey))
-                VKey.createOfy(BillingEvent.Recurring.class, recurringBillKey))
+            .setServerApproveBillingEvent(VKey.from(oneTimeBillKey))
-            .setServerApproveBillingEvent(
-                VKey.createOfy(BillingEvent.OneTime.class, oneTimeBillKey))
             .build();
     domain =
         persistResource(

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

@@ -63,7 +63,7 @@ public class HostResourceTest extends EntityTestCase {
                         .setLosingClientId("losing")
                         .setPendingTransferExpirationTime(fakeClock.nowUtc())
                         .setServerApproveEntities(
-                            ImmutableSet.of(VKey.createOfy(BillingEvent.OneTime.class, 1)))
+                            ImmutableSet.of(VKey.create(BillingEvent.OneTime.class, 1)))
                         .setTransferRequestTime(fakeClock.nowUtc())
                         .setTransferStatus(TransferStatus.SERVER_APPROVED)
                         .setTransferRequestTrid(Trid.create("client-trid", "server-trid"))

core/src/test/java/google/registry/model/transfer/TransferDataTest.java

@@ -48,11 +48,11 @@ public class TransferDataTest {
 
   @Before
   public void setUp() {
-    transferBillingEventKey = VKey.createOfy(BillingEvent.OneTime.class, 12345);
+    transferBillingEventKey = VKey.create(BillingEvent.OneTime.class, 12345);
-    otherServerApproveBillingEventKey = VKey.createOfy(BillingEvent.Cancellation.class, 2468);
+    otherServerApproveBillingEventKey = VKey.create(BillingEvent.Cancellation.class, 2468);
-    recurringBillingEventKey = VKey.createOfy(BillingEvent.Recurring.class, 13579);
+    recurringBillingEventKey = VKey.create(BillingEvent.Recurring.class, 13579);
-    autorenewPollMessageKey = VKey.createOfy(PollMessage.Autorenew.class, 67890);
+    autorenewPollMessageKey = VKey.create(PollMessage.Autorenew.class, 67890);
-    otherServerApprovePollMessageKey = VKey.createOfy(PollMessage.OneTime.class, 314159);
+    otherServerApprovePollMessageKey = VKey.create(PollMessage.OneTime.class, 314159);
   }
 
   @Test

core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java

@@ -17,10 +17,13 @@ package google.registry.model.translators;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.newDomainBase;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
+import static org.junit.Assert.assertThrows;
 
 import com.googlecode.objectify.Key;
 import google.registry.model.domain.DomainBase;
-import google.registry.model.ofy.CommitLogBucket;
+import google.registry.model.ofy.CommitLogCheckpoint;
+import google.registry.model.ofy.CommitLogCheckpointRoot;
+import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
 import google.registry.testing.AppEngineRule;
 import org.junit.jupiter.api.Test;
@@ -34,7 +37,7 @@ public class VKeyTranslatorFactoryTest {
   public VKeyTranslatorFactoryTest() {}
 
   @Test
-  void testEntityWithVKeyCreate() {
+  void testEntityWithFlatKey() {
     // Creating an objectify key instead of a datastore key as this should get a correctly formatted
     // key path.
     DomainBase domain = newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1"));
@@ -46,13 +49,27 @@ public class VKeyTranslatorFactoryTest {
   }
 
   @Test
-  void testEntityWithoutVKeyCreate() {
+  void testKeyWithParent() {
-    CommitLogBucket bucket = new CommitLogBucket.Builder().build();
+    Key<CommitLogCheckpointRoot> parent = Key.create(CommitLogCheckpointRoot.class, "parent");
-    Key<CommitLogBucket> key = Key.create(bucket);
+    Key<CommitLogCheckpoint> key = Key.create(parent, CommitLogCheckpoint.class, "foo");
-    VKey<CommitLogBucket> vkey = VKeyTranslatorFactory.createVKey(key);
+    IllegalArgumentException e =
-    assertThat(vkey.getKind()).isEqualTo(CommitLogBucket.class);
+        assertThrows(IllegalArgumentException.class, () -> VKeyTranslatorFactory.createVKey(key));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo(
+            "Cannot auto-convert key Key<?>(CommitLogCheckpointRoot(\"parent\")/"
+                + "CommitLogCheckpoint(\"foo\")) of kind CommitLogCheckpoint because it has a "
+                + "parent.  Add a createVKey(Key) method for it.");
+  }
+
+  @Test
+  void testEntityWithAncestor() {
+    Key<HistoryEntry> key =
+        Key.create(Key.create(DomainBase.class, "ROID-1"), HistoryEntry.class, 101);
+    VKey<HistoryEntry> vkey = VKeyTranslatorFactory.createVKey(key);
+    assertThat(vkey.getKind()).isEqualTo(HistoryEntry.class);
     assertThat(vkey.getOfyKey()).isEqualTo(key);
-    assertThat(vkey.maybeGetSqlKey().isPresent()).isFalse();
+    assertThat(vkey.getSqlKey()).isEqualTo("DomainBase/ROID-1/101");
   }
 
   @Test

core/src/test/java/google/registry/persistence/VKeyTest.java

@@ -42,8 +42,6 @@ public class VKeyTest {
     assertThat(key.maybeGetSqlKey().isPresent()).isTrue();
     assertThat(key.maybeGetOfyKey().isPresent()).isTrue();
 
-    Key<TestObject> ofyKey = Key.create(TestObject.create("foo"));
-    assertThat(VKey.createOfy(TestObject.class, ofyKey).maybeGetOfyKey().get()).isEqualTo(ofyKey);
     assertThat(VKey.createSql(TestObject.class, "foo").maybeGetSqlKey().get()).isEqualTo("foo");
   }
 }