From 361a53a3c985c14539e5ec1a31cf4ad192f67a5d Mon Sep 17 00:00:00 2001 From: mcilwain Date: Mon, 17 Oct 2016 13:26:31 -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=136388057 --- .../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 0712f42cf..96c3e8003 100644 --- a/java/google/registry/model/domain/DesignatedContact.java +++ b/java/google/registry/model/domain/DesignatedContact.java @@ -72,6 +72,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 9c37eebda..987618974 100644 --- a/java/google/registry/model/domain/DomainBase.java +++ b/java/google/registry/model/domain/DomainBase.java @@ -183,11 +183,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. */ @@ -242,13 +238,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. */ @@ -307,11 +303,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 c76997ff2..cbe4d2c0c 100644 --- a/javatests/google/registry/model/domain/DomainApplicationTest.java +++ b/javatests/google/registry/model/domain/DomainApplicationTest.java @@ -145,16 +145,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 3d7bfd4bd..e15f8d8d3 100644 --- a/javatests/google/registry/model/domain/DomainResourceTest.java +++ b/javatests/google/registry/model/domain/DomainResourceTest.java @@ -207,13 +207,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() @@ -462,21 +462,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))); } }