From f95f27ed723977f8df8a21dfe478ecdc23ba01fc Mon Sep 17 00:00:00 2001 From: mcilwain Date: Wed, 2 Nov 2016 08:55:46 -0700 Subject: [PATCH] Switch over to non-ReferenceUnion fields on DomainBase This is the second phase of a three phase migration to remove ReferenceUnions. As of the end of this phase, ReferenceUnions are no longer read from in any active code paths, but are still written to in case a rollback to the previous version is necessary. The third and final phase will remove the ReferenceUnions entirely. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=137951076 --- .../registry/model/EppResourceUtils.java | 10 ++++----- .../model/domain/DesignatedContact.java | 2 +- .../registry/model/domain/DomainBase.java | 22 ++++++------------- .../registry/rdap/RdapDomainSearchAction.java | 2 +- .../model/domain/DomainApplicationTest.java | 6 ++--- .../model/domain/DomainResourceTest.java | 17 +++++++------- 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/java/google/registry/model/EppResourceUtils.java b/java/google/registry/model/EppResourceUtils.java index 24a8cf477..65a5a891e 100644 --- a/java/google/registry/model/EppResourceUtils.java +++ b/java/google/registry/model/EppResourceUtils.java @@ -347,12 +347,10 @@ public final class EppResourceUtils { public static List> queryDomainsUsingResource( Class clazz, Key key, DateTime now, int limit) { checkArgument(ContactResource.class.equals(clazz) || HostResource.class.equals(clazz)); - return ofy().load().type(DomainBase.class) - .filter( - clazz.equals(ContactResource.class) - ? "allContacts.contactId.linked" - : "nameservers.linked", - key) + return ofy() + .load() + .type(DomainBase.class) + .filter(clazz.equals(ContactResource.class) ? "allContacts.contact" : "nsHosts", key) .filter("deletionTime >", now) .limit(limit) .keys() diff --git a/java/google/registry/model/domain/DesignatedContact.java b/java/google/registry/model/domain/DesignatedContact.java index 55699d155..cb176d4be 100644 --- a/java/google/registry/model/domain/DesignatedContact.java +++ b/java/google/registry/model/domain/DesignatedContact.java @@ -73,6 +73,6 @@ public class DesignatedContact extends ImmutableObject { } public Key getContactKey() { - return contactId.getLinked(); + return contact; } } diff --git a/java/google/registry/model/domain/DomainBase.java b/java/google/registry/model/domain/DomainBase.java index 102dd7360..bf1d43bfa 100644 --- a/java/google/registry/model/domain/DomainBase.java +++ b/java/google/registry/model/domain/DomainBase.java @@ -182,11 +182,7 @@ public abstract class DomainBase extends EppResource { } public ImmutableSet> getNameservers() { - ImmutableSet.Builder> builder = new ImmutableSet.Builder<>(); - for (ReferenceUnion union : nullToEmptyImmutableCopy(nameservers)) { - builder.add(union.getLinked()); - } - return builder.build(); + return nullToEmptyImmutableCopy(nsHosts); } /** Loads and returns the fully qualified host names of all linked nameservers. */ @@ -241,13 +237,13 @@ public abstract class DomainBase extends EppResource { @OnSave void dualSaveReferenceUnions() { for (DesignatedContact contact : nullToEmptyImmutableCopy(allContacts)) { - contact.contact = contact.contactId.getLinked(); + contact.contactId = ReferenceUnion.create(contact.contact); } - ImmutableSet.Builder> hostKeys = new ImmutableSet.Builder<>(); - for (ReferenceUnion refUnion : nullToEmptyImmutableCopy(nameservers)) { - hostKeys.add(refUnion.getLinked()); + ImmutableSet.Builder> hosts = new ImmutableSet.Builder<>(); + for (Key hostKey : nullToEmptyImmutableCopy(nsHosts)) { + hosts.add(ReferenceUnion.create(hostKey)); } - nsHosts = hostKeys.build(); + nameservers = hosts.build(); } /** Predicate to determine if a given {@link DesignatedContact} is the registrant. */ @@ -306,11 +302,7 @@ public abstract class DomainBase extends EppResource { } public B setNameservers(ImmutableSet> nameservers) { - ImmutableSet.Builder> builder = new ImmutableSet.Builder<>(); - for (Key key : nullToEmpty(nameservers)) { - builder.add(ReferenceUnion.create(key)); - } - getInstance().nameservers = builder.build(); + getInstance().nsHosts = nameservers; return thisCastToDerived(); } diff --git a/java/google/registry/rdap/RdapDomainSearchAction.java b/java/google/registry/rdap/RdapDomainSearchAction.java index 15b2144bd..fb8912c38 100644 --- a/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/java/google/registry/rdap/RdapDomainSearchAction.java @@ -269,7 +269,7 @@ public class RdapDomainSearchAction extends RdapActionBase { for (List> chunk : Iterables.partition(hostKeys, 30)) { for (DomainResource domain : ofy().load() .type(DomainResource.class) - .filter("nameservers.linked in", chunk) + .filter("nsHosts in", chunk) .filter("deletionTime >", now) .limit(rdapResultSetMaxSize + 1)) { if (!domains.contains(domain)) { diff --git a/javatests/google/registry/model/domain/DomainApplicationTest.java b/javatests/google/registry/model/domain/DomainApplicationTest.java index 364ee21f7..988cca4d4 100644 --- a/javatests/google/registry/model/domain/DomainApplicationTest.java +++ b/javatests/google/registry/model/domain/DomainApplicationTest.java @@ -156,16 +156,16 @@ public class DomainApplicationTest extends EntityTestCase { @Test public void testEmptySetsAndArraysBecomeNull() { - assertThat(emptyBuilder().setNameservers(null).build().nameservers).isNull(); + assertThat(emptyBuilder().setNameservers(null).build().nsHosts).isNull(); assertThat(emptyBuilder() .setNameservers(ImmutableSet.>of()) .build() - .nameservers) + .nsHosts) .isNull(); assertThat(emptyBuilder() .setNameservers(ImmutableSet.of(Key.create(newHostResource("foo.example.tld")))) .build() - .nameservers) + .nsHosts) .isNotNull(); // This behavior should also hold true for ImmutableObjects nested in collections. assertThat(emptyBuilder() diff --git a/javatests/google/registry/model/domain/DomainResourceTest.java b/javatests/google/registry/model/domain/DomainResourceTest.java index 89d3d8549..a624ed03b 100644 --- a/javatests/google/registry/model/domain/DomainResourceTest.java +++ b/javatests/google/registry/model/domain/DomainResourceTest.java @@ -206,13 +206,13 @@ public class DomainResourceTest extends EntityTestCase { @Test public void testEmptySetsAndArraysBecomeNull() { assertThat(newDomainResource("example.com").asBuilder() - .setNameservers(null).build().nameservers).isNull(); + .setNameservers(null).build().nsHosts).isNull(); assertThat(newDomainResource("example.com").asBuilder() - .setNameservers(ImmutableSet.>of()).build().nameservers) + .setNameservers(ImmutableSet.>of()).build().nsHosts) .isNull(); assertThat(newDomainResource("example.com").asBuilder() .setNameservers(ImmutableSet.of(Key.create(newHostResource("foo.example.tld")))) - .build().nameservers) + .build().nsHosts) .isNotNull(); // This behavior should also hold true for ImmutableObjects nested in collections. assertThat(newDomainResource("example.com").asBuilder() @@ -461,21 +461,22 @@ public class DomainResourceTest extends EntityTestCase { public void testDualSavingOfDesignatedContact() { ContactResource contact = persistActiveContact("time1006"); DesignatedContact designatedContact = new DesignatedContact(); - designatedContact.contactId = ReferenceUnion.create(Key.create(contact)); + designatedContact.contact = Key.create(contact); designatedContact.type = Type.ADMIN; DomainResource domainWithContact = domain.asBuilder().setContacts(ImmutableSet.of(designatedContact)).build(); - assertThat(getOnlyElement(domainWithContact.getContacts()).contact).isNull(); + assertThat(getOnlyElement(domainWithContact.getContacts()).contactId).isNull(); DomainResource reloadedDomain = persistResource(domainWithContact); - assertThat(getOnlyElement(reloadedDomain.getContacts()).contact).isEqualTo(Key.create(contact)); + assertThat(getOnlyElement(reloadedDomain.getContacts()).contactId) + .isEqualTo(ReferenceUnion.create(Key.create(contact))); } @Test public void testDualSavingOfNameservers() { HostResource host = persistActiveHost("zzz.xxx.yyy"); DomainResource domain = newDomainResource("python-django-unchained.com", host); - assertThat(domain.nsHosts).isNull(); + assertThat(domain.nameservers).isNull(); DomainResource djangoReloaded = persistResource(domain); - assertThat(djangoReloaded.nsHosts).containsExactly(Key.create(host)); + assertThat(djangoReloaded.nameservers).containsExactly(ReferenceUnion.create(Key.create(host))); } }