From c9d4ffb2336725905fa7324834c993911a67b81c Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Wed, 24 Jun 2020 08:02:11 -0400 Subject: [PATCH] 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 --- .../batch/DeleteContactsAndHostsAction.java | 6 +- .../batch/RefreshDnsOnHostRenameAction.java | 4 +- .../registry/model/billing/BillingEvent.java | 76 +++++++++++++++++-- .../model/contact/ContactResource.java | 3 +- .../model/domain/DesignatedContact.java | 2 +- .../registry/model/domain/DomainBase.java | 4 - .../model/domain/token/AllocationToken.java | 2 +- .../google/registry/model/host/HostBase.java | 2 +- .../registry/model/host/HostHistory.java | 3 +- .../registry/model/host/HostResource.java | 3 +- .../registry/model/poll/PollMessage.java | 16 +++- .../model/reporting/HistoryEntry.java | 14 ++++ .../translators/VKeyTranslatorFactory.java | 14 +++- .../google/registry/persistence/VKey.java | 42 ++++------ .../registry/rdap/RdapDomainSearchAction.java | 4 +- .../google/registry/flows/EppTestCase.java | 6 +- .../domain/DomainTransferApproveFlowTest.java | 3 +- .../domain/DomainTransferRequestFlowTest.java | 6 +- .../model/contact/ContactResourceTest.java | 2 +- .../registry/model/domain/DomainBaseTest.java | 23 +++--- .../registry/model/host/HostResourceTest.java | 2 +- .../model/transfer/TransferDataTest.java | 10 +-- .../VKeyTranslatorFactoryTest.java | 33 ++++++-- .../google/registry/persistence/VKeyTest.java | 2 - 24 files changed, 181 insertions(+), 101 deletions(-) diff --git a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java b/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java index 54d15cbd5..94450ef09 100644 --- a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/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) resourceKey)); + .contains(VKey.from((Key) resourceKey)); } else if (resourceKey.getKind().equals(KIND_HOST)) { - return domain - .getNameservers() - .contains(VKey.createOfy(HostResource.class, (Key) resourceKey)); + return domain.getNameservers().contains(VKey.from((Key) resourceKey)); } else { throw new IllegalStateException("EPP resource key of unknown type: " + resourceKey); } diff --git a/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java b/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java index aafa6304e..79b33ced5 100644 --- a/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java +++ b/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java @@ -207,9 +207,7 @@ public class RefreshDnsOnHostRenameAction implements Runnable { Key referencingHostKey = null; for (DnsRefreshRequest request : refreshRequests) { if (isActive(domain, request.lastUpdateTime()) - && domain - .getNameservers() - .contains(VKey.createOfy(HostResource.class, request.hostKey()))) { + && domain.getNameservers().contains(VKey.from(request.hostKey()))) { referencingHostKey = request.hostKey(); break; } diff --git a/core/src/main/java/google/registry/model/billing/BillingEvent.java b/core/src/main/java/google/registry/model/billing/BillingEvent.java index 8f811678b..40f43d037 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -347,7 +347,27 @@ public abstract class BillingEvent extends ImmutableObject @Override public VKey createVKey() { - return VKey.createOfy(getClass(), Key.create(this)); + return VKey.create( + getClass(), + parent.getParent().getName() + "/" + parent.getId() + "/" + getId(), + Key.create(this)); + } + + public static VKey createVKey(Key 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 createVKey() { - return VKey.createOfy(getClass(), Key.create(this)); + return VKey.create( + getClass(), + parent.getParent().getName() + "/" + parent.getId() + "/" + getId(), + Key.create(this)); + } + + public static VKey createVKey(Key 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( - VKey.createOfy(BillingEvent.OneTime.class, gracePeriod.getOneTimeBillingEvent())); + builder.setOneTimeEventKey(VKey.from(gracePeriod.getOneTimeBillingEvent())); } else if (gracePeriod.getRecurringBillingEvent() != null) { - builder.setRecurringEventKey( - VKey.createOfy(BillingEvent.Recurring.class, gracePeriod.getRecurringBillingEvent())); + builder.setRecurringEventKey(VKey.from(gracePeriod.getRecurringBillingEvent())); } return builder.build(); } @Override public VKey createVKey() { - return VKey.createOfy(getClass(), Key.create(this)); + return VKey.create( + getClass(), + parent.getParent().getName() + "/" + parent.getId() + "/" + getId(), + Key.create(this)); + } + + public static VKey createVKey(Key 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 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 createVKey(Key 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); } /** diff --git a/core/src/main/java/google/registry/model/contact/ContactResource.java b/core/src/main/java/google/registry/model/contact/ContactResource.java index aabbb883a..4d1bf745e 100644 --- a/core/src/main/java/google/registry/model/contact/ContactResource.java +++ b/core/src/main/java/google/registry/model/contact/ContactResource.java @@ -200,8 +200,7 @@ public class ContactResource extends EppResource @Override public VKey createVKey() { - // TODO(mmuller): create symmetric keys if we can ever reload both sides. - return VKey.createOfy(ContactResource.class, Key.create(this)); + return VKey.create(ContactResource.class, getRepoId(), Key.create(this)); } @Override diff --git a/core/src/main/java/google/registry/model/domain/DesignatedContact.java b/core/src/main/java/google/registry/model/domain/DesignatedContact.java index 1868f9c21..e38f2a3c6 100644 --- a/core/src/main/java/google/registry/model/domain/DesignatedContact.java +++ b/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)); } } diff --git a/core/src/main/java/google/registry/model/domain/DomainBase.java b/core/src/main/java/google/registry/model/domain/DomainBase.java index deaceac7b..3f7d82ad4 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/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 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 IS_REGISTRANT = (DesignatedContact contact) -> DesignatedContact.Type.REGISTRANT.equals(contact.type); diff --git a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java index f40ac4d90..40f9ef4f7 100644 --- a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java +++ b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java @@ -183,7 +183,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable { } public VKey createVKey() { - return VKey.createOfy(getClass(), Key.create(this)); + return VKey.create(getClass(), getToken(), Key.create(this)); } @Override diff --git a/core/src/main/java/google/registry/model/host/HostBase.java b/core/src/main/java/google/registry/model/host/HostBase.java index 6dae739a6..5f10d95d9 100644 --- a/core/src/main/java/google/registry/model/host/HostBase.java +++ b/core/src/main/java/google/registry/model/host/HostBase.java @@ -126,7 +126,7 @@ public class HostBase extends EppResource { @Override public VKey createVKey() { - return VKey.createOfy(HostBase.class, Key.create(this)); + return VKey.create(HostBase.class, getRepoId(), Key.create(this)); } @Deprecated diff --git a/core/src/main/java/google/registry/model/host/HostHistory.java b/core/src/main/java/google/registry/model/host/HostHistory.java index b21082c4d..3507bc247 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -83,7 +83,8 @@ public class HostHistory extends HistoryEntry { @Override public Builder setParent(Key parent) { super.setParent(parent); - getInstance().hostRepoId = VKey.createOfy(HostResource.class, (Key) parent); + getInstance().hostRepoId = + VKey.create(HostResource.class, parent.getName(), (Key) parent); return this; } } diff --git a/core/src/main/java/google/registry/model/host/HostResource.java b/core/src/main/java/google/registry/model/host/HostResource.java index 57e6bfbda..fb03e5c37 100644 --- a/core/src/main/java/google/registry/model/host/HostResource.java +++ b/core/src/main/java/google/registry/model/host/HostResource.java @@ -48,8 +48,7 @@ public class HostResource extends HostBase @Override public VKey createVKey() { - // TODO(mmuller): create symmetric keys if we can ever reload both sides. - return VKey.createOfy(HostResource.class, Key.create(this)); + return VKey.create(HostResource.class, getRepoId(), Key.create(this)); } @Override diff --git a/core/src/main/java/google/registry/model/poll/PollMessage.java b/core/src/main/java/google/registry/model/poll/PollMessage.java index c26a09302..7094f8623 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessage.java +++ b/core/src/main/java/google/registry/model/poll/PollMessage.java @@ -154,6 +154,18 @@ public abstract class PollMessage extends ImmutableObject @Override public abstract VKey createVKey(); + public static VKey createVKey(Key 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 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 createVKey() { - return VKey.createOfy(this.getClass(), Key.create(this)); + return VKey.create(this.getClass(), getId(), Key.create(this)); } @Override diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java index af9bd54ca..d096733cb 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/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 createVKey(Key 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)); diff --git a/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java b/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java index 9a3e11873..4213b239a 100644 --- a/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java +++ b/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java @@ -67,9 +67,17 @@ public class VKeyTranslatorFactory extends AbstractSimpleTranslatorFactory) 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 - // converters. - return VKey.createOfy(clazz, key); + checkArgument( + key.getParent() == null, + "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. diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index 62624fc17..b464035ac 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/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 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. + * + *

Deprecated. Create symmetric keys with create() instead. + */ public static VKey createSql(Class 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 VKey createOfy( - Class kind, com.googlecode.objectify.Key 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 VKey createOfy(Class 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 VKey createOfy(Class 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 VKey create( Class kind, Object sqlKey, com.googlecode.objectify.Key ofyKey) { @@ -87,6 +67,11 @@ public class VKey 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 VKey create(Class kind, long id) { + return new VKey(kind, Key.create(kind, id), id); + } + /** Returns the type of the entity. */ public Class getKind() { return this.kind; @@ -113,4 +98,9 @@ public class VKey extends ImmutableObject implements Serializable { public Optional> maybeGetOfyKey() { return Optional.ofNullable(this.ofyKey); } + + /** Convenience method to construct a VKey from an objectify Key. */ + public static VKey from(Key key) { + return VKeyTranslatorFactory.createVKey(key); + } } diff --git a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java index aae5d869a..a6231a921 100644 --- a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/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())); } diff --git a/core/src/test/java/google/registry/flows/EppTestCase.java b/core/src/test/java/google/registry/flows/EppTestCase.java index 5b34e01f2..0851fca90 100644 --- a/core/src/test/java/google/registry/flows/EppTestCase.java +++ b/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( - VKey.createOfy(OneTime.class, findKeyToActualOneTimeBillingEvent(billingEventToCancel))) + .setOneTimeEventKey(VKey.from(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( - VKey.createOfy(OneTime.class, findKeyToActualOneTimeBillingEvent(billingEventToCancel))) + .setOneTimeEventKey(VKey.from(findKeyToActualOneTimeBillingEvent(billingEventToCancel))) .setBillingTime(renewTime.plus(Registry.get(domain.getTld()).getRenewGracePeriodLength())) .setReason(Reason.RENEW) .setParent(getOnlyHistoryEntryOfType(domain, Type.DOMAIN_DELETE)) diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java index 45b8c8ba1..ada58dcae 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java +++ b/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( - VKey.createOfy(BillingEvent.Recurring.class, domain.getAutorenewBillingEvent()))); + .setRecurringEventKey(VKey.from(domain.getAutorenewBillingEvent()))); } @Test diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java index 5a3d2d44b..2feba3d19 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/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( - VKey.createOfy(BillingEvent.Recurring.class, existingAutorenewEvent))); + .setRecurringEventKey(VKey.from(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( - VKey.createOfy(BillingEvent.Recurring.class, existingAutorenewEvent))); + .setRecurringEventKey(VKey.from(existingAutorenewEvent))); } @Test diff --git a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java index c30dcf9b0..c90c2c0fd 100644 --- a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java +++ b/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")) diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java index dfb492da0..578541df9 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java +++ b/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.createOfy(BillingEvent.Recurring.class, recurringBillKey), - VKey.createOfy(PollMessage.Autorenew.class, autorenewPollKey))) - .setServerApproveBillingEvent( - VKey.createOfy(BillingEvent.OneTime.class, oneTimeBillKey)) - .setServerApproveAutorenewEvent( - VKey.createOfy(BillingEvent.Recurring.class, recurringBillKey)) - .setServerApproveAutorenewPollMessage( - VKey.createOfy(PollMessage.Autorenew.class, autorenewPollKey)) + VKey.from(oneTimeBillKey), + VKey.from(recurringBillKey), + VKey.from(autorenewPollKey))) + .setServerApproveBillingEvent(VKey.from(oneTimeBillKey)) + .setServerApproveAutorenewEvent(VKey.from(recurringBillKey)) + .setServerApproveAutorenewPollMessage(VKey.from(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( - VKey.createOfy(BillingEvent.Recurring.class, recurringBillKey)) - .setServerApproveBillingEvent( - VKey.createOfy(BillingEvent.OneTime.class, oneTimeBillKey)) + .setServerApproveAutorenewEvent(VKey.from(recurringBillKey)) + .setServerApproveBillingEvent(VKey.from(oneTimeBillKey)) .build(); domain = persistResource( diff --git a/core/src/test/java/google/registry/model/host/HostResourceTest.java b/core/src/test/java/google/registry/model/host/HostResourceTest.java index 7891de0b5..ae4d62164 100644 --- a/core/src/test/java/google/registry/model/host/HostResourceTest.java +++ b/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")) diff --git a/core/src/test/java/google/registry/model/transfer/TransferDataTest.java b/core/src/test/java/google/registry/model/transfer/TransferDataTest.java index af540f278..e3d7a83f0 100644 --- a/core/src/test/java/google/registry/model/transfer/TransferDataTest.java +++ b/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); - otherServerApproveBillingEventKey = VKey.createOfy(BillingEvent.Cancellation.class, 2468); - recurringBillingEventKey = VKey.createOfy(BillingEvent.Recurring.class, 13579); - autorenewPollMessageKey = VKey.createOfy(PollMessage.Autorenew.class, 67890); - otherServerApprovePollMessageKey = VKey.createOfy(PollMessage.OneTime.class, 314159); + transferBillingEventKey = VKey.create(BillingEvent.OneTime.class, 12345); + otherServerApproveBillingEventKey = VKey.create(BillingEvent.Cancellation.class, 2468); + recurringBillingEventKey = VKey.create(BillingEvent.Recurring.class, 13579); + autorenewPollMessageKey = VKey.create(PollMessage.Autorenew.class, 67890); + otherServerApprovePollMessageKey = VKey.create(PollMessage.OneTime.class, 314159); } @Test diff --git a/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java b/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java index d9dc30fe5..7ab68c9b7 100644 --- a/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java +++ b/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() { - CommitLogBucket bucket = new CommitLogBucket.Builder().build(); - Key key = Key.create(bucket); - VKey vkey = VKeyTranslatorFactory.createVKey(key); - assertThat(vkey.getKind()).isEqualTo(CommitLogBucket.class); + void testKeyWithParent() { + Key parent = Key.create(CommitLogCheckpointRoot.class, "parent"); + Key key = Key.create(parent, CommitLogCheckpoint.class, "foo"); + IllegalArgumentException e = + 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 key = + Key.create(Key.create(DomainBase.class, "ROID-1"), HistoryEntry.class, 101); + VKey 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 diff --git a/core/src/test/java/google/registry/persistence/VKeyTest.java b/core/src/test/java/google/registry/persistence/VKeyTest.java index 84b741b08..713b5b3ff 100644 --- a/core/src/test/java/google/registry/persistence/VKeyTest.java +++ b/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 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"); } }