From 4f750778de490b4a6d48d5ec4958d2edda56ef86 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Wed, 9 Sep 2020 13:56:59 -0400 Subject: [PATCH] Implement DatastoreEntity/SqlEntity for many more classes (#788) * Implement DatastoreEntity/SqlEntity for many more classes We still have many more classes to go, but this gets us closer to guaranteeing that we can convert from Datastore to SQL objects and back again. * Shift SqlEntity impl to HistoryEntry --- .../registry/model/billing/BillingEvent.java | 9 +- .../model/contact/ContactHistory.java | 1 + .../registry/model/domain/DomainHistory.java | 1 + .../model/index/EppResourceIndex.java | 10 ++- .../model/index/EppResourceIndexBucket.java | 9 +- .../registry/model/index/ForeignKeyIndex.java | 35 ++++++-- .../registry/model/poll/PollMessage.java | 3 +- .../model/registry/label/PremiumList.java | 7 +- .../model/reporting/HistoryEntry.java | 17 +++- .../transaction/TransactionEntity.java | 10 ++- .../model/CreateAutoTimestampTest.java | 2 + .../registry/model/ImmutableObjectTest.java | 2 + .../model/UpdateAutoTimestampTest.java | 2 + .../google/registry/model/ofy/OfyTest.java | 86 +++++++++---------- ...mmitLogRevisionsTranslatorFactoryTest.java | 18 ++-- .../registry/schema/replay/EntityTest.java | 26 ++++-- .../google/registry/testing/TestObject.java | 7 +- 17 files changed, 164 insertions(+), 81 deletions(-) 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 d96ae2657..73767c351 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -46,6 +46,7 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import google.registry.persistence.VKey; import google.registry.persistence.WithLongVKey; +import google.registry.schema.replay.DatastoreAndSqlEntity; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -274,7 +275,7 @@ public abstract class BillingEvent extends ImmutableObject @javax.persistence.Index(columnList = "allocation_token_id") }) @AttributeOverride(name = "id", column = @Column(name = "billing_event_id")) - public static class OneTime extends BillingEvent { + public static class OneTime extends BillingEvent implements DatastoreAndSqlEntity { /** The billable value. */ @AttributeOverrides({ @@ -450,7 +451,7 @@ public abstract class BillingEvent extends ImmutableObject @javax.persistence.Index(columnList = "recurrence_time_of_year") }) @AttributeOverride(name = "id", column = @Column(name = "billing_recurrence_id")) - public static class Recurring extends BillingEvent { + public static class Recurring extends BillingEvent implements DatastoreAndSqlEntity { /** * The billing event recurs every year between {@link #eventTime} and this time on the @@ -544,7 +545,7 @@ public abstract class BillingEvent extends ImmutableObject @javax.persistence.Index(columnList = "billingTime") }) @AttributeOverride(name = "id", column = @Column(name = "billing_cancellation_id")) - public static class Cancellation extends BillingEvent { + public static class Cancellation extends BillingEvent implements DatastoreAndSqlEntity { /** The billing time of the charge that is being cancelled. */ @Index @@ -664,7 +665,7 @@ public abstract class BillingEvent extends ImmutableObject /** An event representing a modification of an existing one-time billing event. */ @ReportedOn @Entity - public static class Modification extends BillingEvent { + public static class Modification extends BillingEvent implements DatastoreAndSqlEntity { /** The change in cost that should be applied to the original billing event. */ Money cost; diff --git a/core/src/main/java/google/registry/model/contact/ContactHistory.java b/core/src/main/java/google/registry/model/contact/ContactHistory.java index 3fae69b76..fe66c3d1e 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -45,6 +45,7 @@ import javax.persistence.Id; @EntitySubclass @Access(AccessType.FIELD) public class ContactHistory extends HistoryEntry { + // Store ContactBase instead of ContactResource so we don't pick up its @Id ContactBase contactBase; diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index 0821ccceb..ef547df9c 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -60,6 +60,7 @@ import javax.persistence.Table; @Access(AccessType.FIELD) @IdClass(DomainHistoryId.class) public class DomainHistory extends HistoryEntry { + // Store DomainContent instead of DomainBase so we don't pick up its @Id DomainContent domainContent; diff --git a/core/src/main/java/google/registry/model/index/EppResourceIndex.java b/core/src/main/java/google/registry/model/index/EppResourceIndex.java index ee5cd84d9..b9f21a6a6 100644 --- a/core/src/main/java/google/registry/model/index/EppResourceIndex.java +++ b/core/src/main/java/google/registry/model/index/EppResourceIndex.java @@ -17,6 +17,7 @@ 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; @@ -25,11 +26,13 @@ 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; /** An index that allows for quick enumeration of all EppResource entities (e.g. via map reduce). */ @ReportedOn @Entity -public class EppResourceIndex extends BackupGroupRoot { +public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity { @Id String id; @@ -74,4 +77,9 @@ public class EppResourceIndex extends BackupGroupRoot { public static EppResourceIndex create(Key resourceKey) { return create(EppResourceIndexBucket.getBucketKey(resourceKey), resourceKey); } + + @Override + public ImmutableList toSqlEntities() { + return ImmutableList.of(); // not relevant in SQL + } } diff --git a/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java b/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java index 78722acc0..c2fc76bde 100644 --- a/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java +++ b/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java @@ -24,16 +24,23 @@ 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; /** A virtual entity to represent buckets to which EppResourceIndex objects are randomly added. */ @Entity @VirtualEntity -public class EppResourceIndexBucket extends ImmutableObject { +public class EppResourceIndexBucket extends ImmutableObject implements DatastoreEntity { @SuppressWarnings("unused") @Id private long bucketId; + @Override + public ImmutableList 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 diff --git a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java index 1f6a495de..1bfbe99c7 100644 --- a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java +++ b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java @@ -43,6 +43,8 @@ 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.util.NonFinalForTesting; import java.util.Map; import java.util.Optional; @@ -61,28 +63,44 @@ public abstract class ForeignKeyIndex extends BackupGroup /** The {@link ForeignKeyIndex} type for {@link ContactResource} entities. */ @ReportedOn @Entity - public static class ForeignKeyContactIndex extends ForeignKeyIndex {} + public static class ForeignKeyContactIndex extends ForeignKeyIndex + implements DatastoreEntity { + @Override + public ImmutableList toSqlEntities() { + return ImmutableList.of(); // not relevant in SQL + } + } /** The {@link ForeignKeyIndex} type for {@link DomainBase} entities. */ @ReportedOn @Entity - public static class ForeignKeyDomainIndex extends ForeignKeyIndex {} + public static class ForeignKeyDomainIndex extends ForeignKeyIndex + implements DatastoreEntity { + @Override + public ImmutableList toSqlEntities() { + return ImmutableList.of(); // not relevant in SQL + } + } /** The {@link ForeignKeyIndex} type for {@link HostResource} entities. */ @ReportedOn @Entity - public static class ForeignKeyHostIndex extends ForeignKeyIndex {} + public static class ForeignKeyHostIndex extends ForeignKeyIndex + implements DatastoreEntity { + @Override + public ImmutableList toSqlEntities() { + return ImmutableList.of(); // not relevant in SQL + } + } - static final ImmutableMap< - Class, Class>> + static final ImmutableMap, Class>> RESOURCE_CLASS_TO_FKI_CLASS = ImmutableMap.of( ContactResource.class, ForeignKeyContactIndex.class, DomainBase.class, ForeignKeyDomainIndex.class, HostResource.class, ForeignKeyHostIndex.class); - @Id - String foreignKey; + @Id String foreignKey; /** * The deletion time of this {@link ForeignKeyIndex}. @@ -90,8 +108,7 @@ public abstract class ForeignKeyIndex extends BackupGroup *

This will generally be equal to the deletion time of {@link #topReference}. However, in the * case of a {@link HostResource} that was renamed, this field will hold the time of the rename. */ - @Index - DateTime deletionTime; + @Index DateTime deletionTime; /** * The referenced resource. 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 b53769f70..f076a08b4 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessage.java +++ b/core/src/main/java/google/registry/model/poll/PollMessage.java @@ -45,6 +45,7 @@ import google.registry.model.transfer.TransferResponse.ContactTransferResponse; import google.registry.model.transfer.TransferResponse.DomainTransferResponse; import google.registry.persistence.VKey; import google.registry.persistence.WithLongVKey; +import google.registry.schema.replay.DatastoreAndSqlEntity; import java.util.List; import java.util.Optional; import javax.persistence.AttributeOverride; @@ -92,7 +93,7 @@ import org.joda.time.DateTime; @javax.persistence.Index(columnList = "eventTime") }) public abstract class PollMessage extends ImmutableObject - implements Buildable, TransferServerApproveEntity { + implements Buildable, DatastoreAndSqlEntity, TransferServerApproveEntity { /** Entity id. */ @Id diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumList.java b/core/src/main/java/google/registry/model/registry/label/PremiumList.java index fd53e7e8f..3f00be202 100644 --- a/core/src/main/java/google/registry/model/registry/label/PremiumList.java +++ b/core/src/main/java/google/registry/model/registry/label/PremiumList.java @@ -114,7 +114,7 @@ public final class PremiumList extends BaseDomainLabelList parent; @@ -171,6 +171,11 @@ public final class PremiumList extends BaseDomainLabelList toSqlEntities() { + return ImmutableList.of(); // not persisted in SQL + } } /** 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 10a2986f4..326d25d13 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -18,6 +18,7 @@ import static com.googlecode.objectify.Key.getKind; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; @@ -40,6 +41,8 @@ import google.registry.model.host.HostHistory; import google.registry.model.host.HostResource; import google.registry.persistence.VKey; import google.registry.persistence.WithStringVKey; +import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.SqlEntity; import java.util.Set; import javax.annotation.Nullable; import javax.persistence.Access; @@ -59,7 +62,7 @@ import org.joda.time.DateTime; @MappedSuperclass @WithStringVKey // TODO(b/162229294): This should be resolved during the course of that bug @Access(AccessType.FIELD) -public class HistoryEntry extends ImmutableObject implements Buildable { +public class HistoryEntry extends ImmutableObject implements Buildable, DatastoreEntity, SqlEntity { /** Represents the type of history entry. */ public enum Type { @@ -307,6 +310,18 @@ public class HistoryEntry extends ImmutableObject implements Buildable { return resultEntity; } + // In SQL, save the child type + @Override + public ImmutableList toSqlEntities() { + return ImmutableList.of(toChildHistoryEntity()); + } + + // In Datastore, save as a HistoryEntry object regardless of this object's type + @Override + public ImmutableList toDatastoreEntities() { + return ImmutableList.of(asHistoryEntry()); + } + /** A builder for {@link HistoryEntry} since it is immutable */ public static class Builder> extends GenericBuilder { diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionEntity.java b/core/src/main/java/google/registry/persistence/transaction/TransactionEntity.java index 5fdf47c20..c295c56a1 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionEntity.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionEntity.java @@ -14,6 +14,9 @@ package google.registry.persistence.transaction; +import com.google.common.collect.ImmutableList; +import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.SqlEntity; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; @@ -27,7 +30,7 @@ import javax.persistence.Table; */ @Entity @Table(name = "Transaction") -public class TransactionEntity { +public class TransactionEntity implements SqlEntity { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @@ -40,4 +43,9 @@ public class TransactionEntity { TransactionEntity(byte[] contents) { this.contents = contents; } + + @Override + public ImmutableList toDatastoreEntities() { + return ImmutableList.of(); // not stored in Datastore per se + } } diff --git a/core/src/test/java/google/registry/model/CreateAutoTimestampTest.java b/core/src/test/java/google/registry/model/CreateAutoTimestampTest.java index 0db3b73e2..c3bd3c7fe 100644 --- a/core/src/test/java/google/registry/model/CreateAutoTimestampTest.java +++ b/core/src/test/java/google/registry/model/CreateAutoTimestampTest.java @@ -21,6 +21,7 @@ import static org.joda.time.DateTimeZone.UTC; import com.googlecode.objectify.annotation.Entity; import google.registry.model.common.CrossTldSingleton; +import google.registry.schema.replay.EntityTest.EntityForTesting; import google.registry.testing.AppEngineExtension; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; @@ -38,6 +39,7 @@ public class CreateAutoTimestampTest { /** Timestamped class. */ @Entity(name = "CatTestEntity") + @EntityForTesting public static class TestObject extends CrossTldSingleton { CreateAutoTimestamp createTime = CreateAutoTimestamp.create(null); } diff --git a/core/src/test/java/google/registry/model/ImmutableObjectTest.java b/core/src/test/java/google/registry/model/ImmutableObjectTest.java index 3cc1974a9..12b8197fc 100644 --- a/core/src/test/java/google/registry/model/ImmutableObjectTest.java +++ b/core/src/test/java/google/registry/model/ImmutableObjectTest.java @@ -29,6 +29,7 @@ import com.google.common.collect.Iterables; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; +import google.registry.schema.replay.EntityTest.EntityForTesting; import google.registry.testing.AppEngineExtension; import google.registry.util.CidrAddressBlock; import java.util.ArrayDeque; @@ -279,6 +280,7 @@ public class ImmutableObjectTest { /** Simple subclass of ImmutableObject. */ @Entity + @EntityForTesting public static class ValueObject extends ImmutableObject { @Id long id; diff --git a/core/src/test/java/google/registry/model/UpdateAutoTimestampTest.java b/core/src/test/java/google/registry/model/UpdateAutoTimestampTest.java index d8636b3b1..cfd49a987 100644 --- a/core/src/test/java/google/registry/model/UpdateAutoTimestampTest.java +++ b/core/src/test/java/google/registry/model/UpdateAutoTimestampTest.java @@ -21,6 +21,7 @@ import static org.joda.time.DateTimeZone.UTC; import com.googlecode.objectify.annotation.Entity; import google.registry.model.common.CrossTldSingleton; +import google.registry.schema.replay.EntityTest.EntityForTesting; import google.registry.testing.AppEngineExtension; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; @@ -38,6 +39,7 @@ public class UpdateAutoTimestampTest { /** Timestamped class. */ @Entity(name = "UatTestEntity") + @EntityForTesting public static class UpdateAutoTimestampTestObject extends CrossTldSingleton { UpdateAutoTimestamp updateTime = UpdateAutoTimestamp.create(null); } diff --git a/core/src/test/java/google/registry/model/ofy/OfyTest.java b/core/src/test/java/google/registry/model/ofy/OfyTest.java index 9cf8bd430..761699860 100644 --- a/core/src/test/java/google/registry/model/ofy/OfyTest.java +++ b/core/src/test/java/google/registry/model/ofy/OfyTest.java @@ -45,6 +45,7 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.Trid; import google.registry.model.reporting.HistoryEntry; +import google.registry.schema.replay.EntityTest.EntityForTesting; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatastoreHelper; import google.registry.testing.FakeClock; @@ -69,13 +70,14 @@ public class OfyTest { @BeforeEach void beforeEach() { createTld("tld"); - someObject = new HistoryEntry.Builder() - .setClientId("client id") - .setModificationTime(START_OF_TIME) - .setParent(persistActiveContact("parentContact")) - .setTrid(Trid.create("client", "server")) - .setXmlBytes("".getBytes(UTF_8)) - .build(); + someObject = + new HistoryEntry.Builder() + .setClientId("client id") + .setModificationTime(START_OF_TIME) + .setParent(persistActiveContact("parentContact")) + .setTrid(Trid.create("client", "server")) + .setXmlBytes("".getBytes(UTF_8)) + .build(); // This can't be initialized earlier because namespaces need the AppEngineRule to work. } @@ -111,8 +113,7 @@ public class OfyTest { assertThrows( IllegalArgumentException.class, () -> - tm() - .transact( + tm().transact( () -> { ofy().save().entity(someObject); ofy().save().entity(someObject); @@ -126,8 +127,7 @@ public class OfyTest { assertThrows( IllegalArgumentException.class, () -> - tm() - .transact( + tm().transact( () -> { ofy().delete().entity(someObject); ofy().delete().entity(someObject); @@ -141,8 +141,7 @@ public class OfyTest { assertThrows( IllegalArgumentException.class, () -> - tm() - .transact( + tm().transact( () -> { ofy().save().entity(someObject); ofy().delete().entity(someObject); @@ -156,8 +155,7 @@ public class OfyTest { assertThrows( IllegalArgumentException.class, () -> - tm() - .transact( + tm().transact( () -> { ofy().delete().entity(someObject); ofy().save().entity(someObject); @@ -174,13 +172,12 @@ public class OfyTest { /** Simple entity class with lifecycle callbacks. */ @com.googlecode.objectify.annotation.Entity + @EntityForTesting public static class LifecycleObject extends ImmutableObject { - @Parent - Key parent = getCrossTldKey(); + @Parent Key parent = getCrossTldKey(); - @Id - long id = 1; + @Id long id = 1; boolean onLoadCalled; boolean onSaveCalled; @@ -218,20 +215,22 @@ public class OfyTest { /** Avoid regressions of b/21309102 where transaction time did not change on each retry. */ @Test void testTransact_getsNewTimestampOnEachTry() { - tm().transact(new Runnable() { + tm().transact( + new Runnable() { - DateTime firstAttemptTime; + DateTime firstAttemptTime; - @Override - public void run() { - if (firstAttemptTime == null) { - // Sleep a bit to ensure that the next attempt is at a new millisecond. - firstAttemptTime = tm().getTransactionTime(); - sleepUninterruptibly(10, MILLISECONDS); - throw new ConcurrentModificationException(); - } - assertThat(tm().getTransactionTime()).isGreaterThan(firstAttemptTime); - }}); + @Override + public void run() { + if (firstAttemptTime == null) { + // Sleep a bit to ensure that the next attempt is at a new millisecond. + firstAttemptTime = tm().getTransactionTime(); + sleepUninterruptibly(10, MILLISECONDS); + throw new ConcurrentModificationException(); + } + assertThat(tm().getTransactionTime()).isGreaterThan(firstAttemptTime); + } + }); } @Test @@ -319,17 +318,19 @@ public class OfyTest { } }; // A commit logged work that throws on the first attempt to get its result. - CommitLoggedWork commitLoggedWork = new CommitLoggedWork(work, new SystemClock()) { - boolean firstCallToGetResult = true; + CommitLoggedWork commitLoggedWork = + new CommitLoggedWork(work, new SystemClock()) { + boolean firstCallToGetResult = true; - @Override - public Void getResult() { - if (firstCallToGetResult) { - firstCallToGetResult = false; - throw new DatastoreTimeoutException(""); - } - return null; - }}; + @Override + public Void getResult() { + if (firstCallToGetResult) { + firstCallToGetResult = false; + throw new DatastoreTimeoutException(""); + } + return null; + } + }; // Despite the DatastoreTimeoutException in the first call to getResult(), this should succeed // without retrying. If a retry is triggered, the test should fail due to the call to fail(). ofy().transactCommitLoggedWork(commitLoggedWork); @@ -381,8 +382,7 @@ public class OfyTest { void test_getBaseEntityClassFromEntityOrKey_subclassEntity() { DomainBase domain = DatastoreHelper.newDomainBase("test.tld"); assertThat(getBaseEntityClassFromEntityOrKey(domain)).isEqualTo(DomainBase.class); - assertThat(getBaseEntityClassFromEntityOrKey(Key.create(domain))) - .isEqualTo(DomainBase.class); + assertThat(getBaseEntityClassFromEntityOrKey(Key.create(domain))).isEqualTo(DomainBase.class); } @Test diff --git a/core/src/test/java/google/registry/model/translators/CommitLogRevisionsTranslatorFactoryTest.java b/core/src/test/java/google/registry/model/translators/CommitLogRevisionsTranslatorFactoryTest.java index 311fe9946..11d5c70ee 100644 --- a/core/src/test/java/google/registry/model/translators/CommitLogRevisionsTranslatorFactoryTest.java +++ b/core/src/test/java/google/registry/model/translators/CommitLogRevisionsTranslatorFactoryTest.java @@ -27,6 +27,7 @@ import com.googlecode.objectify.annotation.Entity; import google.registry.model.common.CrossTldSingleton; import google.registry.model.ofy.CommitLogManifest; import google.registry.model.ofy.Ofy; +import google.registry.schema.replay.EntityTest.EntityForTesting; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; @@ -42,6 +43,7 @@ public class CommitLogRevisionsTranslatorFactoryTest { private static final DateTime START_TIME = DateTime.parse("2000-01-01TZ"); @Entity(name = "ClrtfTestEntity") + @EntityForTesting public static class TestObject extends CrossTldSingleton { ImmutableSortedMap> revisions = ImmutableSortedMap.of(); } @@ -73,11 +75,11 @@ public class CommitLogRevisionsTranslatorFactoryTest { @Test void testSave_doesNotMutateOriginalResource() { - TestObject object = new TestObject(); - save(object); - assertThat(object.revisions).isEmpty(); - assertThat(reload().revisions).isNotEmpty(); - } + TestObject object = new TestObject(); + save(object); + assertThat(object.revisions).isEmpty(); + assertThat(reload().revisions).isNotEmpty(); + } @Test void testSave_translatorAddsKeyToCommitLogToField() { @@ -149,8 +151,10 @@ public class CommitLogRevisionsTranslatorFactoryTest { com.google.appengine.api.datastore.Entity entity = tm().transactNewReadOnly(() -> ofy().save().toEntity(reload())); assertThat(entity.getProperties().keySet()).containsExactly("revisions.key", "revisions.value"); - assertThat(entity.getProperties()).containsEntry( - "revisions.key", ImmutableList.of(START_TIME.toDate(), START_TIME.plusDays(1).toDate())); + assertThat(entity.getProperties()) + .containsEntry( + "revisions.key", + ImmutableList.of(START_TIME.toDate(), START_TIME.plusDays(1).toDate())); assertThat(entity.getProperty("revisions.value")).isInstanceOf(List.class); assertThat(((List) entity.getProperty("revisions.value")).get(0)) .isInstanceOf(com.google.appengine.api.datastore.Key.class); diff --git a/core/src/test/java/google/registry/schema/replay/EntityTest.java b/core/src/test/java/google/registry/schema/replay/EntityTest.java index bc6c88a5f..51463d102 100644 --- a/core/src/test/java/google/registry/schema/replay/EntityTest.java +++ b/core/src/test/java/google/registry/schema/replay/EntityTest.java @@ -41,24 +41,32 @@ public class EntityTest { new ClassGraph().enableAnnotationInfo().whitelistPackages("google.registry").scan()) { // All javax.persistence entities must implement SqlEntity and vice versa ImmutableSet javaxPersistenceClasses = - getClassNames( - scanResult.getClassesWithAnnotation(javax.persistence.Entity.class.getName())); + getAllClassesWithAnnotation(scanResult, javax.persistence.Entity.class.getName()); ImmutableSet sqlEntityClasses = getClassNames(scanResult.getClassesImplementing(SqlEntity.class.getName())); - assertThat(javaxPersistenceClasses).isEqualTo(sqlEntityClasses); + assertThat(sqlEntityClasses).containsExactlyElementsIn(javaxPersistenceClasses); - // All com.googlecode.objectify.annotation.Entity classes must implement DatastoreEntity and - // vice versa + // All com.googlecode.objectify entities must implement DatastoreEntity and vice versa ImmutableSet objectifyClasses = - getClassNames( - scanResult.getClassesWithAnnotation( - com.googlecode.objectify.annotation.Entity.class.getName())); + getAllClassesWithAnnotation( + scanResult, com.googlecode.objectify.annotation.Entity.class.getName()); ImmutableSet datastoreEntityClasses = getClassNames(scanResult.getClassesImplementing(DatastoreEntity.class.getName())); - assertThat(objectifyClasses).isEqualTo(datastoreEntityClasses); + assertThat(datastoreEntityClasses).containsExactlyElementsIn(objectifyClasses); } } + private ImmutableSet getAllClassesWithAnnotation( + ScanResult scanResult, String annotation) { + ImmutableSet.Builder result = new ImmutableSet.Builder<>(); + ClassInfoList classesWithAnnotation = scanResult.getClassesWithAnnotation(annotation); + result.addAll(getClassNames(classesWithAnnotation)); + classesWithAnnotation.stream() + .map(ClassInfo::getSubclasses) + .forEach(classInfoList -> result.addAll(getClassNames(classInfoList))); + return result.build(); + } + private ImmutableSet getClassNames(ClassInfoList classInfoList) { return classInfoList.stream() .filter(ClassInfo::isStandardClass) diff --git a/core/src/test/java/google/registry/testing/TestObject.java b/core/src/test/java/google/registry/testing/TestObject.java index 3c726b73c..7392ccf9a 100644 --- a/core/src/test/java/google/registry/testing/TestObject.java +++ b/core/src/test/java/google/registry/testing/TestObject.java @@ -23,11 +23,11 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.annotations.VirtualEntity; import google.registry.model.common.EntityGroupRoot; +import google.registry.schema.replay.EntityTest.EntityForTesting; -/** - * A test model object that can be persisted in any entity group. - */ +/** A test model object that can be persisted in any entity group. */ @Entity +@EntityForTesting public class TestObject extends ImmutableObject { @Parent @@ -65,6 +65,7 @@ public class TestObject extends ImmutableObject { /** A test @VirtualEntity model object, which should not be persisted. */ @Entity @VirtualEntity + @EntityForTesting public static class TestVirtualObject extends ImmutableObject { @Id