diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index a06ced5ea..d12bfdd9d 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -14,10 +14,12 @@ package google.registry.persistence; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.googlecode.objectify.Key; +import google.registry.model.BackupGroupRoot; import google.registry.model.ImmutableObject; import google.registry.model.translators.VKeyTranslatorFactory; import java.io.Serializable; @@ -75,11 +77,42 @@ 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}. */ + /** + * Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code id}. + * + *

IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those + * that do not have a null parent in their Objectify keys) require the full entity group + * inheritance chain to be specified and thus cannot use this create method. You need to use + * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the + * Datastore key. + */ public static VKey create(Class kind, long id) { + checkArgument( + kind.isAssignableFrom(BackupGroupRoot.class), + "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain" + + " specified in a parent", + kind.getCanonicalName()); return new VKey(kind, Key.create(kind, id), id); } + /** + * Creates a symmetric {@link VKey} in which both sql and ofy keys are {@code name}. + * + *

IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those + * that do not have a null parent in their Objectify keys) require the full entity group + * inheritance chain to be specified and thus cannot use this create method. You need to use + * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the + * Datastore key. + */ + public static VKey create(Class kind, String name) { + checkArgument( + kind.isAssignableFrom(BackupGroupRoot.class), + "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain" + + " specified in a parent", + kind.getCanonicalName()); + return new VKey(kind, Key.create(kind, name), name); + } + /** Returns the type of the entity. */ public Class getKind() { return this.kind; 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 8979cfa00..f1037754e 100644 --- a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java +++ b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java @@ -31,7 +31,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.model.EntityTestCase; import google.registry.model.ImmutableObjectSubject; -import google.registry.model.billing.BillingEvent; import google.registry.model.contact.Disclose.PostalInfoChoice; import google.registry.model.contact.PostalInfo.Type; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; @@ -46,6 +45,7 @@ import org.junit.jupiter.api.Test; /** Unit tests for {@link ContactResource}. */ public class ContactResourceTest extends EntityTestCase { + private ContactResource originalContact; private ContactResource contactResource; @@ -113,8 +113,6 @@ public class ContactResourceTest extends EntityTestCase { .setGainingClientId("gaining") .setLosingClientId("losing") .setPendingTransferExpirationTime(fakeClock.nowUtc()) - .setServerApproveEntities( - 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/GracePeriodTest.java b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java index a4eb11971..3ae94f976 100644 --- a/core/src/test/java/google/registry/model/domain/GracePeriodTest.java +++ b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java @@ -98,6 +98,11 @@ public class GracePeriodTest { @Test void testFailure_createForRecurring_notAutoRenew() { + Key recurringKey = + Key.create( + Key.create(Key.create(DomainBase.class, "1-TEST"), HistoryEntry.class, 343L), + Recurring.class, + 12345); IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, @@ -107,7 +112,7 @@ public class GracePeriodTest { "1-TEST", now.plusDays(1), "TheRegistrar", - VKey.create(Recurring.class, 12345))); + VKey.create(Recurring.class, 12345, recurringKey))); assertThat(thrown).hasMessageThat().contains("autorenew"); } } 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 78e49cfb9..9fac4423d 100644 --- a/core/src/test/java/google/registry/model/host/HostResourceTest.java +++ b/core/src/test/java/google/registry/model/host/HostResourceTest.java @@ -27,13 +27,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.common.net.InetAddresses; import google.registry.model.EntityTestCase; -import google.registry.model.billing.BillingEvent; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferStatus; -import google.registry.persistence.VKey; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -62,8 +60,6 @@ class HostResourceTest extends EntityTestCase { .setGainingClientId("gaining") .setLosingClientId("losing") .setPendingTransferExpirationTime(fakeClock.nowUtc()) - .setServerApproveEntities( - 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 2ea01fd4b..d5a62cad9 100644 --- a/core/src/test/java/google/registry/model/transfer/TransferDataTest.java +++ b/core/src/test/java/google/registry/model/transfer/TransferDataTest.java @@ -18,10 +18,13 @@ import static com.google.common.truth.Truth.assertThat; import static org.joda.time.DateTimeZone.UTC; import com.google.common.collect.ImmutableSet; +import com.googlecode.objectify.Key; import google.registry.model.billing.BillingEvent; +import google.registry.model.domain.DomainBase; import google.registry.model.domain.Period; import google.registry.model.eppcommon.Trid; import google.registry.model.poll.PollMessage; +import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; import org.joda.time.DateTime; @@ -46,11 +49,33 @@ public class TransferDataTest { @BeforeEach void beforeEach() { - 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); + Key historyEntryKey = + Key.create(Key.create(DomainBase.class, "4-TLD"), HistoryEntry.class, 1356L); + transferBillingEventKey = + VKey.create( + BillingEvent.OneTime.class, + 12345, + Key.create(historyEntryKey, BillingEvent.OneTime.class, 12345)); + otherServerApproveBillingEventKey = + VKey.create( + BillingEvent.Cancellation.class, + 2468, + Key.create(historyEntryKey, BillingEvent.Cancellation.class, 2468)); + recurringBillingEventKey = + VKey.create( + BillingEvent.Recurring.class, + 13579, + Key.create(historyEntryKey, BillingEvent.Recurring.class, 13579)); + autorenewPollMessageKey = + VKey.create( + PollMessage.Autorenew.class, + 67890, + Key.create(historyEntryKey, PollMessage.Autorenew.class, 67890)); + otherServerApprovePollMessageKey = + VKey.create( + PollMessage.OneTime.class, + 314159, + Key.create(historyEntryKey, PollMessage.OneTime.class, 314159)); } @Test diff --git a/core/src/test/java/google/registry/persistence/VKeyTest.java b/core/src/test/java/google/registry/persistence/VKeyTest.java index 9ec3f55bd..ca392cda1 100644 --- a/core/src/test/java/google/registry/persistence/VKeyTest.java +++ b/core/src/test/java/google/registry/persistence/VKeyTest.java @@ -14,8 +14,12 @@ package google.registry.persistence; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import com.googlecode.objectify.Key; +import google.registry.model.billing.BillingEvent.OneTime; +import google.registry.model.registrar.RegistrarContact; import google.registry.testing.AppEngineExtension; import google.registry.testing.TestObject; import org.junit.jupiter.api.Test; @@ -37,7 +41,22 @@ class VKeyTest { VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))); assertThat(key.maybeGetSqlKey().isPresent()).isTrue(); assertThat(key.maybeGetOfyKey().isPresent()).isTrue(); + assertThat(VKey.createSql(TestObject.class, "foo").maybeGetSqlKey()).hasValue("foo"); + } - assertThat(VKey.createSql(TestObject.class, "foo").maybeGetSqlKey().get()).isEqualTo("foo"); + @Test + void testCreateById_failsWhenParentIsNullButShouldntBe() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> VKey.create(OneTime.class, 134L)); + assertThat(thrown).hasMessageThat().contains("BackupGroupRoot"); + } + + @Test + void testCreateByName_failsWhenParentIsNullButShouldntBe() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> VKey.create(RegistrarContact.class, "fake@example.com")); + assertThat(thrown).hasMessageThat().contains("BackupGroupRoot"); } }