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 0796d93d9..56e0e830e 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -17,7 +17,6 @@ package google.registry.model.contact; import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; -import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.reporting.HistoryEntry; @@ -58,7 +57,17 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { // Store ContactBase instead of ContactResource so we don't pick up its @Id @Nullable ContactBase contactBase; - @Id String contactRepoId; + @Id + @Access(AccessType.PROPERTY) + public String getContactRepoId() { + return parent.getName(); + } + + /** This method is private because it is only used by Hibernate. */ + @SuppressWarnings("unused") + private void setContactRepoId(String contactRepoId) { + parent = Key.create(ContactResource.class, contactRepoId); + } @Id @Column(name = "historyRevisionId") @@ -79,15 +88,14 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { } /** The key to the {@link ContactResource} this is based off of. */ - public VKey getContactRepoId() { - return VKey.create( - ContactResource.class, contactRepoId, Key.create(ContactResource.class, contactRepoId)); + public VKey getParentVKey() { + return VKey.create(ContactResource.class, getContactRepoId()); } /** Creates a {@link VKey} instance for this entity. */ public VKey createVKey() { return VKey.create( - ContactHistory.class, new ContactHistoryId(contactRepoId, getId()), Key.create(this)); + ContactHistory.class, new ContactHistoryId(getContactRepoId(), getId()), Key.create(this)); } @PostLoad @@ -97,8 +105,6 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { if (contactBase != null && contactBase.getContactId() == null) { contactBase = null; } - // Fill in the full, symmetric, parent repo ID key - parent = Key.create(ContactResource.class, contactRepoId); } // In Datastore, save as a HistoryEntry object regardless of this object's type @@ -184,17 +190,8 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { } public Builder setContactRepoId(String contactRepoId) { - getInstance().contactRepoId = contactRepoId; getInstance().parent = Key.create(ContactResource.class, contactRepoId); return this; } - - // We can remove this once all HistoryEntries are converted to History objects - @Override - public Builder setParent(Key parent) { - super.setParent(parent); - getInstance().contactRepoId = parent.getName(); - return this; - } } } 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 67778758b..aa1d77a70 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -36,7 +36,6 @@ import javax.persistence.Index; import javax.persistence.JoinColumn; import javax.persistence.JoinTable; import javax.persistence.OneToMany; -import javax.persistence.PostLoad; import javax.persistence.Table; import org.joda.time.DateTime; @@ -108,12 +107,6 @@ public class DomainBase extends DomainContent return gracePeriods; } - @PostLoad - @SuppressWarnings("UnusedMethod") - private final void postLoad() { - restoreOfyKeys(getRepoId()); - } - /** * Returns the set of {@link DelegationSignerData} associated with the domain. * diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java index a726e1222..ab39f073f 100644 --- a/core/src/main/java/google/registry/model/domain/DomainContent.java +++ b/core/src/main/java/google/registry/model/domain/DomainContent.java @@ -35,6 +35,7 @@ import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static google.registry.util.DomainNameUtils.getTldFromDomainName; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; +import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Ordering; @@ -68,7 +69,6 @@ import java.util.HashSet; import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; import javax.annotation.Nullable; import javax.persistence.Access; import javax.persistence.AccessType; @@ -323,15 +323,9 @@ public class DomainContent extends EppResource .collect(toImmutableSet()); } - /** - * The {@link javax.persistence.PostLoad} method for {@link DomainContent}. - * - *

We name this domainContentPostLoad to distinguish it from the {@link PostLoad} method in - * DomainBase (if they share the same name, this one is never called). - */ @PostLoad @SuppressWarnings("UnusedMethod") - private final void domainContentPostLoad() { + private final void postLoad() { // Reconstitute the contact list. ImmutableSet.Builder contactsBuilder = new ImmutableSet.Builder<>(); @@ -350,34 +344,9 @@ public class DomainContent extends EppResource } allContacts = contactsBuilder.build(); - } - /** - * Restores the composite ofy keys from SQL data. - * - *

MUST ONLY BE CALLED FROM A PostLoad method. This is a package-visible method that - * effectively mutates an immutable object. - * - *

We have to do this because: - * - *

    - *
  • We've changed the {@link PostLoad} method behavior to make all {@link PostLoad} calls in - * the class hierarchy (and not merely the most specific one) be called after an object is - * loaded. - *
  • When restoring a {@link DomainBase} object (which is a subclass) the repo id is not - * populated until after our {@link PostLoad} method is called. Therefore, we need to - * restore these ofy keys (which depend on the repo id) from {@link DomainBase}'s {@link - * PostLoad} method. - *
  • When restoring a {@link DomainHistory} object, hibernate restores a {@link DomainContent} - * instance, therefore we need our own {@link PostLoad} method to restore the other fields. - * In order to restore the ofy keys, we need to invoke this method separately from {@link - * DomainHistory}'s {@link PostLoad} method and pass in the repo id, which is stored in a - * different field in {@link DomainHistory}. - *
- */ - void restoreOfyKeys(String repoId) { - // Reconstitute the ofy keys. - Key myKey = Key.create(DomainBase.class, repoId); + // Reconstitute the composite ofy keys from the SQL data. + Key myKey = Key.create(DomainBase.class, getRepoId()); deletePollMessage = restoreOfyFrom(myKey, deletePollMessage, deletePollMessageHistoryId); autorenewBillingEvent = restoreOfyFrom(myKey, autorenewBillingEvent, autorenewBillingEventHistoryId); @@ -764,7 +733,7 @@ public class DomainContent extends EppResource /** An override of {@link EppResource#asBuilder} with tighter typing. */ @Override - public Builder asBuilder() { + public Builder asBuilder() { return new Builder<>(clone(this)); } 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 0c1cfc1fd..228937834 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; import com.googlecode.objectify.annotation.Ignore; -import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.host.HostResource; @@ -73,7 +72,17 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { // Store DomainContent instead of DomainBase so we don't pick up its @Id @Nullable DomainContent domainContent; - @Id String domainRepoId; + @Id + @Access(AccessType.PROPERTY) + public String getDomainRepoId() { + return parent.getName(); + } + + /** This method is private because it is only used by Hibernate. */ + @SuppressWarnings("unused") + private void setDomainRepoId(String domainRepoId) { + parent = Key.create(DomainBase.class, domainRepoId); + } // We could have reused domainContent.nsHosts here, but Hibernate throws a weird exception after // we change to use a composite primary key. @@ -152,14 +161,14 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { } /** The key to the {@link DomainBase} this is based off of. */ - public VKey getDomainRepoId() { - return VKey.create(DomainBase.class, domainRepoId, Key.create(DomainBase.class, domainRepoId)); + public VKey getParentVKey() { + return VKey.create(DomainBase.class, getDomainRepoId()); } /** Creates a {@link VKey} instance for this entity. */ public VKey createVKey() { return VKey.create( - DomainHistory.class, new DomainHistoryId(domainRepoId, getId()), Key.create(this)); + DomainHistory.class, new DomainHistoryId(getDomainRepoId(), getId()), Key.create(this)); } @PostLoad @@ -171,10 +180,11 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { if (domainContent.getDomainName() == null) { domainContent = null; } else { - domainContent.restoreOfyKeys(domainRepoId); + if (domainContent.getRepoId() == null) { + domainContent = domainContent.asBuilder().setRepoId(parent.getName()).build(); + } } } - parent = Key.create(DomainBase.class, domainRepoId); } // In Datastore, save as a HistoryEntry object regardless of this object's type @@ -263,17 +273,8 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { } public Builder setDomainRepoId(String domainRepoId) { - getInstance().domainRepoId = domainRepoId; getInstance().parent = Key.create(DomainBase.class, domainRepoId); return this; } - - // We can remove this once all HistoryEntries are converted to History objects - @Override - public Builder setParent(Key parent) { - super.setParent(parent); - getInstance().domainRepoId = parent.getName(); - return this; - } } } diff --git a/core/src/main/java/google/registry/model/eppcommon/StatusValue.java b/core/src/main/java/google/registry/model/eppcommon/StatusValue.java index 9911b065e..5aa27deee 100644 --- a/core/src/main/java/google/registry/model/eppcommon/StatusValue.java +++ b/core/src/main/java/google/registry/model/eppcommon/StatusValue.java @@ -20,8 +20,10 @@ import static com.google.common.base.Strings.nullToEmpty; import com.google.common.collect.ImmutableSet; import google.registry.model.EppResource; +import google.registry.model.contact.ContactBase; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; +import google.registry.model.domain.DomainContent; import google.registry.model.host.HostBase; import google.registry.model.host.HostResource; import google.registry.model.translators.EnumToAttributeAdapter.EppEnum; @@ -128,9 +130,15 @@ public enum StatusValue implements EppEnum { /** Enum to help clearly list which resource types a status value is allowed to be present on. */ private enum AllowedOn { - ALL(ContactResource.class, DomainBase.class, HostBase.class, HostResource.class), + ALL( + ContactBase.class, + ContactResource.class, + DomainContent.class, + DomainBase.class, + HostBase.class, + HostResource.class), NONE, - DOMAINS(DomainBase.class); + DOMAINS(DomainContent.class, DomainBase.class); private final ImmutableSet> classes; 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 af9e2f34e..078020037 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -14,11 +14,9 @@ package google.registry.model.host; - import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; -import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.reporting.HistoryEntry; @@ -60,7 +58,17 @@ public class HostHistory extends HistoryEntry implements SqlEntity { // Store HostBase instead of HostResource so we don't pick up its @Id @Nullable HostBase hostBase; - @Id String hostRepoId; + @Id + @Access(AccessType.PROPERTY) + public String getHostRepoId() { + return parent.getName(); + } + + /** This method is private because it is only used by Hibernate. */ + @SuppressWarnings("unused") + private void setHostRepoId(String hostRepoId) { + parent = Key.create(HostResource.class, hostRepoId); + } @Id @Column(name = "historyRevisionId") @@ -81,13 +89,14 @@ public class HostHistory extends HistoryEntry implements SqlEntity { } /** The key to the {@link google.registry.model.host.HostResource} this is based off of. */ - public VKey getHostRepoId() { - return VKey.create(HostResource.class, hostRepoId, Key.create(HostResource.class, hostRepoId)); + public VKey getParentVKey() { + return VKey.create(HostResource.class, getHostRepoId()); } /** Creates a {@link VKey} instance for this entity. */ public VKey createVKey() { - return VKey.create(HostHistory.class, new HostHistoryId(hostRepoId, getId()), Key.create(this)); + return VKey.create( + HostHistory.class, new HostHistoryId(getHostRepoId(), getId()), Key.create(this)); } @PostLoad @@ -97,8 +106,6 @@ public class HostHistory extends HistoryEntry implements SqlEntity { if (hostBase != null && hostBase.getHostName() == null) { hostBase = null; } - // Fill in the full, symmetric, parent repo ID key - parent = Key.create(HostResource.class, hostRepoId); } // In Datastore, save as a HistoryEntry object regardless of this object's type @@ -184,17 +191,8 @@ public class HostHistory extends HistoryEntry implements SqlEntity { } public Builder setHostRepoId(String hostRepoId) { - getInstance().hostRepoId = hostRepoId; getInstance().parent = Key.create(HostResource.class, hostRepoId); return this; } - - // We can remove this once all HistoryEntries are converted to History objects - @Override - public Builder setParent(Key parent) { - super.setParent(parent); - getInstance().hostRepoId = parent.getName(); - return this; - } } } diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index b8fda844f..edce5a70b 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -88,7 +88,7 @@ public class VKey extends ImmutableObject implements Serializable { */ public static VKey create(Class kind, long id) { checkArgument( - kind.isAssignableFrom(BackupGroupRoot.class), + BackupGroupRoot.class.isAssignableFrom(kind), "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain" + " specified in a parent", kind.getCanonicalName()); @@ -106,7 +106,7 @@ public class VKey extends ImmutableObject implements Serializable { */ public static VKey create(Class kind, String name) { checkArgument( - kind.isAssignableFrom(BackupGroupRoot.class), + BackupGroupRoot.class.isAssignableFrom(kind), "The kind %s is not a BackupGroupRoot and thus needs its entire entity group chain" + " specified in a parent", kind.getCanonicalName()); diff --git a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java index 62428abe6..879276623 100644 --- a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java @@ -54,8 +54,7 @@ public class ContactHistoryTest extends EntityTestCase { () -> { ContactHistory fromDatabase = jpaTm().load(contactHistory.createVKey()); assertContactHistoriesEqual(fromDatabase, contactHistory); - assertThat(fromDatabase.getContactRepoId().getSqlKey()) - .isEqualTo(contactHistory.getContactRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(contactHistory.getParentVKey()); }); } @@ -79,8 +78,7 @@ public class ContactHistoryTest extends EntityTestCase { () -> { ContactHistory fromDatabase = jpaTm().load(contactHistory.createVKey()); assertContactHistoriesEqual(fromDatabase, contactHistory); - assertThat(fromDatabase.getContactRepoId().getSqlKey()) - .isEqualTo(contactHistory.getContactRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(contactHistory.getParentVKey()); }); } diff --git a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java index 0d2f930a4..cbeb24876 100644 --- a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java @@ -66,8 +66,7 @@ public class DomainHistoryTest extends EntityTestCase { () -> { DomainHistory fromDatabase = jpaTm().load(domainHistory.createVKey()); assertDomainHistoriesEqual(fromDatabase, domainHistory); - assertThat(fromDatabase.getDomainRepoId().getSqlKey()) - .isEqualTo(domainHistory.getDomainRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(domainHistory.getParentVKey()); }); } @@ -83,8 +82,7 @@ public class DomainHistoryTest extends EntityTestCase { () -> { DomainHistory fromDatabase = jpaTm().load(domainHistory.createVKey()); assertDomainHistoriesEqual(fromDatabase, domainHistory); - assertThat(fromDatabase.getDomainRepoId().getSqlKey()) - .isEqualTo(domainHistory.getDomainRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(domainHistory.getParentVKey()); assertThat(fromDatabase.getNsHosts()) .containsExactlyElementsIn( domainHistory.getNsHosts().stream() diff --git a/core/src/test/java/google/registry/model/history/HostHistoryTest.java b/core/src/test/java/google/registry/model/history/HostHistoryTest.java index 1604ef98d..11e037f30 100644 --- a/core/src/test/java/google/registry/model/history/HostHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/HostHistoryTest.java @@ -55,8 +55,7 @@ public class HostHistoryTest extends EntityTestCase { () -> { HostHistory fromDatabase = jpaTm().load(hostHistory.createVKey()); assertHostHistoriesEqual(fromDatabase, hostHistory); - assertThat(fromDatabase.getHostRepoId().getSqlKey()) - .isEqualTo(hostHistory.getHostRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(hostHistory.getParentVKey()); }); } @@ -76,8 +75,7 @@ public class HostHistoryTest extends EntityTestCase { () -> { HostHistory fromDatabase = jpaTm().load(hostHistory.createVKey()); assertHostHistoriesEqual(fromDatabase, hostHistory); - assertThat(fromDatabase.getHostRepoId().getSqlKey()) - .isEqualTo(hostHistory.getHostRepoId().getSqlKey()); + assertThat(fromDatabase.getParentVKey()).isEqualTo(hostHistory.getParentVKey()); }); } diff --git a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java index ce454c00b..5169c0d6e 100644 --- a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java +++ b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java @@ -92,8 +92,8 @@ public class LegacyHistoryObjectTest extends EntityTestCase { .that(legacyContactHistory) .isEqualExceptFields(legacyHistoryFromSql); // can't compare contactRepoId directly since it doesn't save the ofy key - assertThat(legacyContactHistory.getContactRepoId().getSqlKey()) - .isEqualTo(legacyHistoryFromSql.getContactRepoId().getSqlKey()); + assertThat(legacyContactHistory.getParentVKey().getSqlKey()) + .isEqualTo(legacyHistoryFromSql.getParentVKey().getSqlKey()); } @Test @@ -169,8 +169,8 @@ public class LegacyHistoryObjectTest extends EntityTestCase { jpaTm().transact(() -> jpaTm().load(legacyHostHistory.createVKey())); assertAboutImmutableObjects().that(legacyHostHistory).isEqualExceptFields(legacyHistoryFromSql); // can't compare hostRepoId directly since it doesn't save the ofy key in SQL - assertThat(legacyHostHistory.getHostRepoId().getSqlKey()) - .isEqualTo(legacyHistoryFromSql.getHostRepoId().getSqlKey()); + assertThat(legacyHostHistory.getParentVKey().getSqlKey()) + .isEqualTo(legacyHistoryFromSql.getParentVKey().getSqlKey()); } private HistoryEntry historyEntryForDomain(DomainBase domain) { diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index 076502d99..eaae1ad5c 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -133,7 +133,6 @@ class google.registry.model.contact.ContactHistory { google.registry.model.reporting.HistoryEntry$Type type; java.lang.Boolean requestedByRegistrar; java.lang.String clientId; - java.lang.String contactRepoId; java.lang.String otherClientId; java.lang.String reason; java.util.Set domainTransactionRecords; @@ -272,7 +271,6 @@ class google.registry.model.domain.DomainHistory { google.registry.model.reporting.HistoryEntry$Type type; java.lang.Boolean requestedByRegistrar; java.lang.String clientId; - java.lang.String domainRepoId; java.lang.String otherClientId; java.lang.String reason; java.util.Set domainTransactionRecords; @@ -403,7 +401,6 @@ class google.registry.model.host.HostHistory { google.registry.model.reporting.HistoryEntry$Type type; java.lang.Boolean requestedByRegistrar; java.lang.String clientId; - java.lang.String hostRepoId; java.lang.String otherClientId; java.lang.String reason; java.util.Set domainTransactionRecords;