From a5ea445c90465214d866451a51063d7e47423f8d Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Thu, 26 Mar 2020 17:13:30 -0400 Subject: [PATCH] Key to VKey conversion for Nameserver (#476) * Key to VKey conversion for Nameserver This change illustrates the conversion of a single key in the system (Key as used in the "nameservers" field of DomainBase) to a VKey. It currently builds, but had some curious (possibly unrelated?) test failures that I have not fully investigated. * Latest round of changes, all tests pass. * Changes requested in review. * Fix problems with null check in VKey accessors Add maybeGet versions of getSqlKey() and getOfyKey() that return Optional objects and make the nameserver management routines use those instead. --- .../registry/batch/AsyncTaskEnqueuer.java | 5 +- .../batch/DeleteContactsAndHostsAction.java | 5 +- .../batch/RefreshDnsOnHostRenameAction.java | 5 +- .../registry/flows/ResourceFlowUtils.java | 9 +- .../flows/domain/DomainCreateFlow.java | 9 +- .../registry/flows/domain/DomainInfoFlow.java | 7 +- .../flows/domain/DomainUpdateFlow.java | 13 ++- .../registry/flows/host/HostDeleteFlow.java | 14 +++- .../registry/model/domain/DomainBase.java | 69 ++++++++++++--- .../registry/model/host/HostResource.java | 5 ++ .../ofy/DatastoreTransactionManager.java | 18 +++- .../registry/model/ofy/ObjectifyService.java | 26 +++--- .../translators/VKeyTranslatorFactory.java | 84 +++++++++++++++++++ .../google/registry/persistence/VKey.java | 26 +++++- .../JpaTransactionManagerImpl.java | 20 +++++ .../transaction/TransactionManager.java | 7 ++ .../registry/rdap/RdapJsonFormatter.java | 8 +- .../tools/UniformRapidSuspensionCommand.java | 4 +- .../tools/server/GenerateZoneFilesAction.java | 6 +- .../DeleteContactsAndHostsActionTest.java | 6 +- .../writer/clouddns/CloudDnsWriterTest.java | 6 +- .../writer/dnsupdate/DnsUpdateWriterTest.java | 25 +++--- .../flows/domain/DomainDeleteFlowTest.java | 4 +- .../flows/domain/DomainInfoFlowTest.java | 8 +- .../flows/domain/DomainUpdateFlowTest.java | 53 ++++++------ .../flows/host/HostDeleteFlowTest.java | 2 +- .../registry/flows/host/HostInfoFlowTest.java | 2 +- .../flows/host/HostUpdateFlowTest.java | 2 +- .../registry/model/domain/DomainBaseTest.java | 74 ++++++++++++++-- .../rdap/RdapDomainSearchActionTest.java | 7 +- .../rde/DomainBaseToXjcConverterTest.java | 84 +++++++++---------- .../java/google/registry/rde/RdeFixtures.java | 76 ++++++++--------- .../spec11/Spec11EmailUtilsTest.java | 5 +- .../java/google/registry/server/Fixture.java | 44 +++++----- .../registry/testing/DatastoreHelper.java | 2 +- .../testing/FullFieldsTestEntityHelper.java | 7 +- .../tools/GenerateDnsReportCommandTest.java | 27 +++--- .../UniformRapidSuspensionCommandTest.java | 6 +- .../tools/UpdateDomainCommandTest.java | 13 +-- .../server/GenerateZoneFilesActionTest.java | 6 +- .../whois/DomainWhoisResponseTest.java | 5 +- 41 files changed, 555 insertions(+), 249 deletions(-) create mode 100644 core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index 7cd94e2b4..8de7d16c4 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -30,6 +30,7 @@ import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.util.AppEngineServiceUtils; import google.registry.util.Retrier; import javax.inject.Inject; @@ -148,12 +149,12 @@ public final class AsyncTaskEnqueuer { /** Enqueues a task to asynchronously refresh DNS for a renamed host. */ public void enqueueAsyncDnsRefresh(HostResource host, DateTime now) { - Key hostKey = Key.create(host); + VKey hostKey = host.createKey(); logger.atInfo().log("Enqueuing async DNS refresh for renamed host %s.", hostKey); addTaskToQueueWithRetry( asyncDnsRefreshPullQueue, TaskOptions.Builder.withMethod(Method.PULL) - .param(PARAM_HOST_KEY, hostKey.getString()) + .param(PARAM_HOST_KEY, hostKey.getOfyKey().getString()) .param(PARAM_REQUESTED_TIME, now.toString())); } diff --git a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java b/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java index 8e63deefc..7b91c32b7 100644 --- a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -85,6 +85,7 @@ import google.registry.model.poll.PendingActionNotificationResponse.HostPendingA import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry; import google.registry.model.server.Lock; +import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; @@ -284,7 +285,9 @@ public class DeleteContactsAndHostsAction implements Runnable { if (resourceKey.getKind().equals(KIND_CONTACT)) { return domain.getReferencedContacts().contains(resourceKey); } else if (resourceKey.getKind().equals(KIND_HOST)) { - return domain.getNameservers().contains(resourceKey); + return domain + .getNameservers() + .contains(VKey.createOfy(HostResource.class, (Key) resourceKey)); } else { throw new IllegalStateException("EPP resource key of unknown type: " + resourceKey); } diff --git a/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java b/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java index 488b304f1..2748cc29f 100644 --- a/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java +++ b/core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java @@ -52,6 +52,7 @@ import google.registry.mapreduce.inputs.NullInput; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.model.server.Lock; +import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; @@ -206,7 +207,9 @@ public class RefreshDnsOnHostRenameAction implements Runnable { Key referencingHostKey = null; for (DnsRefreshRequest request : refreshRequests) { if (isActive(domain, request.lastUpdateTime()) - && domain.getNameservers().contains(request.hostKey())) { + && domain + .getNameservers() + .contains(VKey.createOfy(HostResource.class, request.hostKey()))) { referencingHostKey = request.hostKey(); break; } diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 996510c4f..460a7c71b 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -80,11 +80,9 @@ public final class ResourceFlowUtils { final Function> getPotentialReferences) throws EppException { // Enter a transactionless context briefly. EppException failfastException = - tm() - .doTransactionless( + tm().doTransactionless( () -> { - final ForeignKeyIndex fki = - ForeignKeyIndex.load(resourceClass, targetId, now); + final ForeignKeyIndex fki = ForeignKeyIndex.load(resourceClass, targetId, now); if (fki == null) { return new ResourceDoesNotExistException(resourceClass, targetId); } @@ -99,8 +97,7 @@ public final class ResourceFlowUtils { .limit(FAILFAST_CHECK_COUNT) .keys(); Predicate predicate = - domain -> - getPotentialReferences.apply(domain).contains(fki.getResourceKey()); + domain -> getPotentialReferences.apply(domain).contains(fki.getResourceKey()); return ofy().load().keys(keys).values().stream().anyMatch(predicate) ? new ResourceToDeleteIsReferencedException() : null; diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index 053584093..0f6f29d71 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -14,6 +14,7 @@ package google.registry.flows.domain; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.verifyResourceDoesNotExist; @@ -95,6 +96,7 @@ import google.registry.model.eppinput.EppInput; import google.registry.model.eppinput.ResourceCommand; import google.registry.model.eppoutput.CreateData.DomainCreateData; import google.registry.model.eppoutput.EppResponse; +import google.registry.model.host.HostResource; import google.registry.model.index.EppResourceIndex; import google.registry.model.index.ForeignKeyIndex; import google.registry.model.ofy.ObjectifyService; @@ -108,6 +110,7 @@ import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; +import google.registry.persistence.VKey; import google.registry.tmch.LordnTaskUtils; import java.util.Optional; import javax.inject.Inject; @@ -352,7 +355,11 @@ public class DomainCreateFlow implements TransactionalFlow { .setRegistrant(command.getRegistrant()) .setAuthInfo(command.getAuthInfo()) .setFullyQualifiedDomainName(targetId) - .setNameservers(command.getNameservers()) + .setNameservers( + (ImmutableSet>) + command.getNameservers().stream() + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet())) .setStatusValues(statuses.build()) .setContacts(command.getContacts()) .addGracePeriod(GracePeriod.forBillingEvent(GracePeriodStatus.ADD, createBillingEvent)) diff --git a/core/src/main/java/google/registry/flows/domain/DomainInfoFlow.java b/core/src/main/java/google/registry/flows/domain/DomainInfoFlow.java index 3e20f9717..c58e11ee6 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainInfoFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainInfoFlow.java @@ -14,7 +14,6 @@ package google.registry.flows.domain; -import static com.google.common.collect.Sets.union; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.verifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; @@ -23,6 +22,7 @@ import static google.registry.flows.domain.DomainFlowUtils.handleFeeRequest; import static google.registry.flows.domain.DomainFlowUtils.loadForeignKeyedDesignatedContacts; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -102,8 +102,9 @@ public final class DomainInfoFlow implements Flow { flowCustomLogic.afterValidation( AfterValidationParameters.newBuilder().setDomain(domain).build()); // Prefetch all referenced resources. Calling values() blocks until loading is done. - ofy().load() - .values(union(domain.getNameservers(), domain.getReferencedContacts())).values(); + // We do nameservers separately since they've been converted to VKey. + tm().load(domain.getNameservers()); + ofy().load().values(domain.getReferencedContacts()).values(); // Registrars can only see a few fields on unauthorized domains. // This is a policy decision that is left up to us by the rfcs. DomainInfoData.Builder infoBuilder = DomainInfoData.newBuilder() diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java index 8090555d4..9c0fd648f 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -15,6 +15,7 @@ package google.registry.flows.domain; import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Sets.symmetricDifference; import static com.google.common.collect.Sets.union; import static google.registry.flows.FlowUtils.persistEntityChanges; @@ -71,9 +72,11 @@ import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppinput.EppInput; import google.registry.model.eppinput.ResourceCommand; import google.registry.model.eppoutput.EppResponse; +import google.registry.model.host.HostResource; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; +import google.registry.persistence.VKey; import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; @@ -243,8 +246,14 @@ public final class DomainUpdateFlow implements TransactionalFlow { .setLastEppUpdateClientId(clientId) .addStatusValues(add.getStatusValues()) .removeStatusValues(remove.getStatusValues()) - .addNameservers(add.getNameservers()) - .removeNameservers(remove.getNameservers()) + .addNameservers( + add.getNameservers().stream() + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet())) + .removeNameservers( + remove.getNameservers().stream() + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet())) .addContacts(add.getContacts()) .removeContacts(remove.getContacts()) .setRegistrant(firstNonNull(change.getRegistrant(), domain.getRegistrant())) diff --git a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java index 94d89c00b..b1f2a144a 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -14,6 +14,7 @@ package google.registry.flows.host; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.failfastForAsyncDelete; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -81,6 +82,17 @@ public final class HostDeleteFlow implements TransactionalFlow { @Inject EppResponse.Builder responseBuilder; @Inject HostDeleteFlow() {} + /** + * Hack to convert DomainBase's nameserver VKey's to Ofy Key's. + * + *

We currently need this because {@code failfastForAsyncDelete()} checks to see if a name is + * in the ofy keys and is used for both nameservers and contacts. When we convert contacts to + * VKey's, we can remove this and do the conversion in {@code failfastForAsyncDelete()}. + */ + private static ImmutableSet> getNameserverOfyKeys(DomainBase domain) { + return domain.getNameservers().stream().map(key -> key.getOfyKey()).collect(toImmutableSet()); + } + @Override public final EppResponse run() throws EppException { extensionManager.register(MetadataExtension.class); @@ -88,7 +100,7 @@ public final class HostDeleteFlow implements TransactionalFlow { validateClientIsLoggedIn(clientId); DateTime now = tm().getTransactionTime(); validateHostName(targetId); - failfastForAsyncDelete(targetId, now, HostResource.class, DomainBase::getNameservers); + failfastForAsyncDelete(targetId, now, HostResource.class, HostDeleteFlow::getNameserverOfyKeys); HostResource existingHost = loadAndVerifyExistence(HostResource.class, targetId, now); verifyNoDisallowedStatuses(existingHost, DISALLOWED_STATUSES); if (!isSuperuser) { 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 82d09c1b8..c0efa17bf 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -42,8 +42,10 @@ import com.google.common.collect.Ordering; import com.google.common.collect.Streams; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; +import com.googlecode.objectify.annotation.Ignore; import com.googlecode.objectify.annotation.IgnoreSave; import com.googlecode.objectify.annotation.Index; +import com.googlecode.objectify.annotation.OnLoad; import com.googlecode.objectify.condition.IfNull; import google.registry.flows.ResourceFlowUtils; import google.registry.model.EppResource; @@ -63,6 +65,7 @@ import google.registry.model.poll.PollMessage; import google.registry.model.registry.Registry; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; +import google.registry.persistence.VKey; import google.registry.util.CollectionUtils; import java.util.HashSet; import java.util.Objects; @@ -130,9 +133,16 @@ public class DomainBase extends EppResource @Index String tld; - /** References to hosts that are the nameservers for the domain. */ + /** + * References to hosts that are the nameservers for the domain. + * + *

This is a legacy field: we have to preserve it because it is still persisted and indexed in + * the datastore, but all external references go through nsHostVKeys. + */ @Index @ElementCollection @Transient Set> nsHosts; + @Ignore @Transient Set> nsHostVKeys; + /** * The union of the contacts visible via {@link #getContacts} and {@link #getRegistrant}. * @@ -240,6 +250,14 @@ public class DomainBase extends EppResource */ DateTime lastTransferTime; + @OnLoad + void load() { + nsHostVKeys = + nullToEmptyImmutableCopy(nsHosts).stream() + .map(hostKey -> VKey.createOfy(HostResource.class, hostKey)) + .collect(toImmutableSet()); + } + public ImmutableSet getSubordinateHosts() { return nullToEmptyImmutableCopy(subordinateHosts); } @@ -299,8 +317,10 @@ public class DomainBase extends EppResource return idnTableName; } - public ImmutableSet> getNameservers() { - return nullToEmptyImmutableCopy(nsHosts); + public ImmutableSet> getNameservers() { + // Since nsHostVKeys gets initialized both from setNameservers() and the OnLoad method, this + // should always be valid. + return nullToEmptyImmutableCopy(nsHostVKeys); } public final String getCurrentSponsorClientId() { @@ -482,7 +502,7 @@ public class DomainBase extends EppResource public ImmutableSortedSet loadNameserverFullyQualifiedHostNames() { return ofy() .load() - .keys(getNameservers()) + .keys(getNameservers().stream().map(VKey::getOfyKey).collect(toImmutableSet())) .values() .stream() .map(HostResource::getFullyQualifiedHostName) @@ -542,6 +562,14 @@ public class DomainBase extends EppResource Builder(DomainBase instance) { super(instance); + + // Convert nsHosts to nsHostVKeys. + if (instance.nsHosts != null) { + instance.nsHostVKeys = + instance.nsHosts.stream() + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet()); + } } @Override @@ -557,7 +585,7 @@ public class DomainBase extends EppResource } else { // There are nameservers, so make sure INACTIVE isn't there. removeStatusValue(StatusValue.INACTIVE); } - + checkArgumentNotNull( emptyToNull(instance.fullyQualifiedDomainName), "Missing fullyQualifiedDomainName"); checkArgument(instance.allContacts.stream().anyMatch(IS_REGISTRANT), "Missing registrant"); @@ -591,30 +619,45 @@ public class DomainBase extends EppResource return thisCastToDerived(); } - public Builder setNameservers(Key nameserver) { - getInstance().nsHosts = ImmutableSet.of(nameserver); + public Builder setNameservers(VKey nameserver) { + Optional> nsKey = nameserver.maybeGetOfyKey(); + if (nsKey.isPresent()) { + getInstance().nsHosts = ImmutableSet.of(nsKey.get()); + } else { + getInstance().nsHosts = null; + } + getInstance().nsHostVKeys = ImmutableSet.of(nameserver); return thisCastToDerived(); } - public Builder setNameservers(ImmutableSet> nameservers) { - getInstance().nsHosts = forceEmptyToNull(nameservers); + public Builder setNameservers(ImmutableSet> nameservers) { + // If we have all of the ofy keys, we can set nsHosts. Otherwise, make it null. + if (nameservers != null + && nameservers.stream().allMatch(key -> key.maybeGetOfyKey().isPresent())) { + getInstance().nsHosts = + nameservers.stream().map(key -> key.getOfyKey()).collect(toImmutableSet()); + } else { + getInstance().nsHosts = null; + } + + getInstance().nsHostVKeys = forceEmptyToNull(nameservers); return thisCastToDerived(); } - public Builder addNameserver(Key nameserver) { + public Builder addNameserver(VKey nameserver) { return addNameservers(ImmutableSet.of(nameserver)); } - public Builder addNameservers(ImmutableSet> nameservers) { + public Builder addNameservers(ImmutableSet> nameservers) { return setNameservers( ImmutableSet.copyOf(union(getInstance().getNameservers(), nameservers))); } - public Builder removeNameserver(Key nameserver) { + public Builder removeNameserver(VKey nameserver) { return removeNameservers(ImmutableSet.of(nameserver)); } - public Builder removeNameservers(ImmutableSet> nameservers) { + public Builder removeNameservers(ImmutableSet> nameservers) { return setNameservers( ImmutableSet.copyOf(difference(getInstance().getNameservers(), nameservers))); } diff --git a/core/src/main/java/google/registry/model/host/HostResource.java b/core/src/main/java/google/registry/model/host/HostResource.java index d00b94d7e..5674e2a91 100644 --- a/core/src/main/java/google/registry/model/host/HostResource.java +++ b/core/src/main/java/google/registry/model/host/HostResource.java @@ -33,6 +33,7 @@ import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.annotations.ReportedOn; import google.registry.model.domain.DomainBase; import google.registry.model.transfer.TransferData; +import google.registry.persistence.VKey; import java.net.InetAddress; import java.util.Optional; import java.util.Set; @@ -117,6 +118,10 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource return fullyQualifiedHostName; } + public VKey createKey() { + return VKey.createOfy(HostResource.class, Key.create(this)); + } + @Deprecated @Override public HostResource cloneProjectedAtTime(DateTime now) { diff --git a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java index cfeea11ec..a50f1714b 100644 --- a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java +++ b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java @@ -18,10 +18,13 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; +import com.googlecode.objectify.Key; import google.registry.persistence.VKey; import google.registry.persistence.transaction.TransactionManager; +import java.util.Iterator; import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.StreamSupport; import org.joda.time.DateTime; /** Datastore implementation of {@link TransactionManager}. */ @@ -128,9 +131,22 @@ public class DatastoreTransactionManager implements TransactionManager { throw new UnsupportedOperationException("Not available in the Datastore transaction manager"); } + // TODO: add tests for these methods. They currently have some degree of test coverage because + // they are used when retrieving the nameservers which require these, as they are now loaded by + // VKey instead of by ofy Key. But ideally, there should be one set of TransactionManager + // interface tests that are applied to both the datastore and SQL implementations. @Override public Optional load(VKey key) { - throw new UnsupportedOperationException("Not available in the Datastore transaction manager"); + return Optional.of(getOfy().load().key(key.getOfyKey()).now()); + } + + @Override + public ImmutableList load(Iterable> keys) { + Iterator> iter = + StreamSupport.stream(keys.spliterator(), false).map(key -> key.getOfyKey()).iterator(); + + // The lambda argument to keys() effectively converts Iterator -> Iterable. + return ImmutableList.copyOf(getOfy().load().keys(() -> iter).values()); } @Override diff --git a/core/src/main/java/google/registry/model/ofy/ObjectifyService.java b/core/src/main/java/google/registry/model/ofy/ObjectifyService.java index bd1995d73..b458e1f61 100644 --- a/core/src/main/java/google/registry/model/ofy/ObjectifyService.java +++ b/core/src/main/java/google/registry/model/ofy/ObjectifyService.java @@ -36,6 +36,7 @@ import com.googlecode.objectify.impl.translate.opt.joda.MoneyStringTranslatorFac import google.registry.config.RegistryEnvironment; import google.registry.model.EntityClasses; import google.registry.model.ImmutableObject; +import google.registry.model.host.HostResource; import google.registry.model.translators.BloomFilterOfStringTranslatorFactory; import google.registry.model.translators.CidrAddressBlockTranslatorFactory; import google.registry.model.translators.CommitLogRevisionsTranslatorFactory; @@ -45,6 +46,7 @@ import google.registry.model.translators.DurationTranslatorFactory; import google.registry.model.translators.InetAddressTranslatorFactory; import google.registry.model.translators.ReadableInstantUtcTranslatorFactory; import google.registry.model.translators.UpdateAutoTimestampTranslatorFactory; +import google.registry.model.translators.VKeyTranslatorFactory; import java.util.concurrent.atomic.AtomicLong; /** @@ -117,17 +119,19 @@ public class ObjectifyService { /** Register translators that allow less common types to be stored directly in Datastore. */ private static void registerTranslators() { - for (TranslatorFactory translatorFactory : ImmutableList.of( - new BloomFilterOfStringTranslatorFactory(), - new CidrAddressBlockTranslatorFactory(), - new CommitLogRevisionsTranslatorFactory(), - new CreateAutoTimestampTranslatorFactory(), - new CurrencyUnitTranslatorFactory(), - new DurationTranslatorFactory(), - new InetAddressTranslatorFactory(), - new MoneyStringTranslatorFactory(), - new ReadableInstantUtcTranslatorFactory(), - new UpdateAutoTimestampTranslatorFactory())) { + for (TranslatorFactory translatorFactory : + ImmutableList.of( + new BloomFilterOfStringTranslatorFactory(), + new CidrAddressBlockTranslatorFactory(), + new CommitLogRevisionsTranslatorFactory(), + new CreateAutoTimestampTranslatorFactory(), + new CurrencyUnitTranslatorFactory(), + new DurationTranslatorFactory(), + new InetAddressTranslatorFactory(), + new MoneyStringTranslatorFactory(), + new ReadableInstantUtcTranslatorFactory(), + new VKeyTranslatorFactory(HostResource.class), + new UpdateAutoTimestampTranslatorFactory())) { factory().getTranslators().add(translatorFactory); } } diff --git a/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java b/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java new file mode 100644 index 000000000..7b1cd2cc0 --- /dev/null +++ b/core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java @@ -0,0 +1,84 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model.translators; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.googlecode.objectify.Key; +import google.registry.persistence.VKey; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.net.URLEncoder; + +/** + * Translator factory for VKey. + * + *

These get translated to a string containing the URL safe encoding of the objectify key + * followed by a (url-unsafe) ampersand delimiter and the SQL key. + */ +public class VKeyTranslatorFactory extends AbstractSimpleTranslatorFactory { + private final Class refClass; + + public VKeyTranslatorFactory(Class refClass) { + super(VKey.class); + this.refClass = refClass; + } + + @Override + public SimpleTranslator createTranslator() { + return new SimpleTranslator() { + @Override + public VKey loadValue(String datastoreValue) { + int pos = datastoreValue.indexOf('&'); + Key ofyKey = null; + String sqlKey = null; + if (pos > 0) { + // We have an objectify key. + ofyKey = Key.create(datastoreValue.substring(0, pos)); + } + + if (pos < datastoreValue.length() - 1) { + // We have an SQL key. + sqlKey = decode(datastoreValue.substring(pos + 1)); + } + + return VKey.create(refClass, sqlKey, ofyKey); + } + + @Override + public String saveValue(VKey key) { + return ((key.getOfyKey() == null) ? "" : key.getOfyKey().getString()) + + "&" + + ((key.getSqlKey() == null) ? "" : encode(key.getSqlKey().toString())); + } + }; + } + + private static String encode(String val) { + try { + return URLEncoder.encode(val, UTF_8.toString()); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + + private static String decode(String encoded) { + try { + return URLDecoder.decode(encoded, UTF_8.toString()); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } +} diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index a9fc6d87f..18181d1f1 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -14,7 +14,10 @@ package google.registry.persistence; +import static com.google.common.base.Preconditions.checkState; + import google.registry.model.ImmutableObject; +import java.util.Optional; /** * VKey is an abstraction that encapsulates the key concept. @@ -25,12 +28,12 @@ import google.registry.model.ImmutableObject; public class VKey extends ImmutableObject { // The primary key for the referenced entity. - private Object primaryKey; + private final Object primaryKey; // The objectify key for the referenced entity. - private com.googlecode.objectify.Key ofyKey; + private final com.googlecode.objectify.Key ofyKey; - private Class kind; + private final Class kind; private VKey(Class kind, com.googlecode.objectify.Key ofyKey, Object primaryKey) { this.kind = kind; @@ -52,17 +55,34 @@ public class VKey extends ImmutableObject { return new VKey(kind, ofyKey, null); } + public static VKey create( + Class kind, Object primaryKey, com.googlecode.objectify.Key ofyKey) { + return new VKey(kind, ofyKey, primaryKey); + } + public Class getKind() { return this.kind; } /** Returns the SQL primary key. */ public Object getSqlKey() { + checkState(primaryKey != null, "Attempting obtain a null SQL key."); return this.primaryKey; } + /** Returns the SQL primary key if it exists. */ + public Optional maybeGetSqlKey() { + return Optional.of(this.primaryKey); + } + /** Returns the objectify key. */ public com.googlecode.objectify.Key getOfyKey() { + checkState(ofyKey != null, "Attempting obtain a null Objectify key."); return this.ofyKey; } + + /** Returns the objectify key if it exists. */ + public Optional> maybeGetOfyKey() { + return Optional.of(this.ofyKey); + } } diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index c46aa593f..f51dce6aa 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -15,6 +15,7 @@ package google.registry.persistence.transaction; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static java.util.stream.Collectors.joining; @@ -26,8 +27,10 @@ import com.google.common.flogger.FluentLogger; import google.registry.persistence.VKey; import google.registry.util.Clock; import java.lang.reflect.Field; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.StreamSupport; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.EntityTransaction; @@ -235,6 +238,23 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return Optional.ofNullable(getEntityManager().find(key.getKind(), key.getSqlKey())); } + @Override + public ImmutableList load(Iterable> keys) { + checkArgumentNotNull(keys, "keys must be specified"); + assertInTransaction(); + return StreamSupport.stream(keys.spliterator(), false) + .map( + key -> { + T entity = getEntityManager().find(key.getKind(), key.getSqlKey()); + if (entity == null) { + throw new NoSuchElementException( + key.getKind().getName() + " with key " + key.getSqlKey() + " not found."); + } + return entity; + }) + .collect(toImmutableList()); + } + @Override public ImmutableList loadAll(Class clazz) { checkArgumentNotNull(clazz, "clazz must be specified"); diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java index 3a634a8f6..fc28ec11f 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java @@ -110,6 +110,13 @@ public interface TransactionManager { /** Loads the entity by its id, returns empty if the entity doesn't exist. */ Optional load(VKey key); + /** + * Leads the set of entities by their key id. + * + * @throws NoSuchElementException if any of the keys are not found. + */ + ImmutableList load(Iterable> keys); + /** Loads all entities of the given type, returns empty if there is no such entity. */ ImmutableList loadAll(Class clazz); diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index dcd0aee16..362e13744 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -21,6 +21,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static google.registry.model.EppResourceUtils.isLinked; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.rdap.RdapIcannStandardInformation.CONTACT_REDACTED_VALUE; import static google.registry.util.CollectionUtils.union; @@ -341,8 +342,8 @@ public class RdapJsonFormatter { // Kick off the database loads of the nameservers that we will need, so it can load // asynchronously while we load and process the contacts. - Map, HostResource> loadedHosts = - ofy().load().keys(domainBase.getNameservers()); + ImmutableSet loadedHosts = + ImmutableSet.copyOf(tm().load(domainBase.getNameservers())); // Load the registrant and other contacts and add them to the data. Map, ContactResource> loadedContacts = ofy().load().keys(domainBase.getReferencedContacts()); @@ -378,8 +379,7 @@ public class RdapJsonFormatter { } // Add the nameservers to the data; the load was kicked off above for efficiency. // RDAP Response Profile 2.9: we MUST have the nameservers - for (HostResource hostResource : - HOST_RESOURCE_ORDERING.immutableSortedCopy(loadedHosts.values())) { + for (HostResource hostResource : HOST_RESOURCE_ORDERING.immutableSortedCopy(loadedHosts)) { builder.nameserversBuilder().add(createRdapNameserver(hostResource, OutputDataType.INTERNAL)); } diff --git a/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java b/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java index 56d436378..025e044c1 100644 --- a/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java +++ b/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java @@ -19,7 +19,7 @@ import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.Sets.difference; import static google.registry.model.EppResourceUtils.checkResourcesExist; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import static org.joda.time.DateTimeZone.UTC; @@ -149,7 +149,7 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand { private ImmutableSortedSet getExistingNameservers(DomainBase domain) { ImmutableSortedSet.Builder nameservers = ImmutableSortedSet.naturalOrder(); - for (HostResource host : ofy().load().keys(domain.getNameservers()).values()) { + for (HostResource host : tm().load(domain.getNameservers())) { nameservers.add(host.getForeignKey()); } return nameservers.build(); diff --git a/core/src/main/java/google/registry/tools/server/GenerateZoneFilesAction.java b/core/src/main/java/google/registry/tools/server/GenerateZoneFilesAction.java index 634c93dfb..a2dfab85c 100644 --- a/core/src/main/java/google/registry/tools/server/GenerateZoneFilesAction.java +++ b/core/src/main/java/google/registry/tools/server/GenerateZoneFilesAction.java @@ -20,7 +20,7 @@ import static com.google.common.collect.Iterators.filter; import static com.google.common.io.BaseEncoding.base16; import static google.registry.mapreduce.inputs.EppResourceInputs.createEntityInput; import static google.registry.model.EppResourceUtils.loadAtPointInTime; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.Action.Method.POST; import static java.nio.charset.StandardCharsets.UTF_8; import static org.joda.time.DateTimeZone.UTC; @@ -214,7 +214,7 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA private void emitForSubordinateHosts(DomainBase domain) { ImmutableSet subordinateHosts = domain.getSubordinateHosts(); if (!subordinateHosts.isEmpty()) { - for (HostResource unprojectedHost : ofy().load().keys(domain.getNameservers()).values()) { + for (HostResource unprojectedHost : tm().load(domain.getNameservers())) { HostResource host = loadAtPointInTime(unprojectedHost, exportTime).now(); // A null means the host was deleted (or not created) at this time. if ((host != null) && subordinateHosts.contains(host.getFullyQualifiedHostName())) { @@ -283,7 +283,7 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA Duration dnsDefaultDsTtl) { StringBuilder result = new StringBuilder(); String domainLabel = stripTld(domain.getFullyQualifiedDomainName(), domain.getTld()); - for (HostResource nameserver : ofy().load().keys(domain.getNameservers()).values()) { + for (HostResource nameserver : tm().load(domain.getNameservers())) { result.append(String.format( NS_FORMAT, domainLabel, diff --git a/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java b/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java index ede9dcc78..54641cbbd 100644 --- a/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java @@ -614,7 +614,7 @@ public class DeleteContactsAndHostsActionTest .hasDeletionTime(END_OF_TIME); DomainBase domain = loadByForeignKey(DomainBase.class, "example.tld", clock.nowUtc()).get(); - assertThat(domain.getNameservers()).contains(Key.create(hostAfter)); + assertThat(domain.getNameservers()).contains(hostAfter.createKey()); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostAfter, HOST_DELETE_FAILURE); assertPollMessageFor( historyEntry, @@ -684,7 +684,7 @@ public class DeleteContactsAndHostsActionTest persistResource( newDomainBase("example.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host))) + .setNameservers(ImmutableSet.of(host.createKey())) .setDeletionTime(clock.nowUtc().minusDays(5)) .build()); enqueuer.enqueueAsyncDelete( @@ -943,7 +943,7 @@ public class DeleteContactsAndHostsActionTest return persistResource( newDomainBase(domainName, contact) .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host))) + .setNameservers(ImmutableSet.of(host.createKey())) .build()); } diff --git a/core/src/test/java/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java b/core/src/test/java/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java index b14fefcc2..88ba262fb 100644 --- a/core/src/test/java/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java +++ b/core/src/test/java/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java @@ -38,12 +38,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.net.InetAddresses; import com.google.common.util.concurrent.RateLimiter; -import com.googlecode.objectify.Key; import google.registry.dns.writer.clouddns.CloudDnsWriter.ZoneStateException; import google.registry.model.domain.DomainBase; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.testing.AppEngineRule; import google.registry.util.Retrier; import google.registry.util.SystemClock; @@ -292,9 +292,9 @@ public class CloudDnsWriterTest { dsDataBuilder.add(DelegationSignerData.create(i, 3, 1, base16().decode("1234567890ABCDEF"))); } - ImmutableSet.Builder> hostResourceRefBuilder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> hostResourceRefBuilder = new ImmutableSet.Builder<>(); for (HostResource nameserver : nameservers) { - hostResourceRefBuilder.add(Key.create(nameserver)); + hostResourceRefBuilder.add(nameserver.createKey()); } return newDomainBase(domainName) diff --git a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java index 8298fc729..87e9de328 100644 --- a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java +++ b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java @@ -36,7 +36,6 @@ import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.net.InetAddresses; -import com.googlecode.objectify.Key; import google.registry.model.domain.DomainBase; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.StatusValue; @@ -106,7 +105,7 @@ public class DnsUpdateWriterTest { DomainBase domain = persistActiveDomain("example.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host1), Key.create(host2))) + .setNameservers(ImmutableSet.of(host1.createKey(), host2.createKey())) .build(); persistResource(domain); @@ -127,7 +126,7 @@ public class DnsUpdateWriterTest { DomainBase domain1 = persistActiveDomain("example1.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host1))) + .setNameservers(ImmutableSet.of(host1.createKey())) .build(); persistResource(domain1); @@ -135,7 +134,7 @@ public class DnsUpdateWriterTest { DomainBase domain2 = persistActiveDomain("example2.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host2))) + .setNameservers(ImmutableSet.of(host2.createKey())) .build(); persistResource(domain2); @@ -151,7 +150,7 @@ public class DnsUpdateWriterTest { DomainBase domain1 = persistActiveDomain("example1.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host1))) + .setNameservers(ImmutableSet.of(host1.createKey())) .build(); persistResource(domain1); @@ -159,7 +158,7 @@ public class DnsUpdateWriterTest { DomainBase domain2 = persistActiveDomain("example2.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host2))) + .setNameservers(ImmutableSet.of(host2.createKey())) .build(); persistResource(domain2); @@ -182,7 +181,7 @@ public class DnsUpdateWriterTest { DomainBase domain = persistActiveDomain("example.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(persistActiveHost("ns1.example.tld")))) + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createKey())) .setDsData( ImmutableSet.of( DelegationSignerData.create(1, 3, 1, base16().decode("0123456789ABCDEF")))) @@ -207,7 +206,7 @@ public class DnsUpdateWriterTest { persistActiveDomain("example.tld") .asBuilder() .addStatusValue(StatusValue.SERVER_HOLD) - .setNameservers(ImmutableSet.of(Key.create(persistActiveHost("ns1.example.tld")))) + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createKey())) .build(); persistResource(domain); @@ -251,7 +250,7 @@ public class DnsUpdateWriterTest { newDomainBase("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") - .addNameserver(Key.create(host)) + .addNameserver(host.createKey()) .build()); writer.publishHost("ns1.example.tld"); @@ -290,7 +289,7 @@ public class DnsUpdateWriterTest { persistResource( persistActiveDomain("example.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(persistActiveHost("ns1.example.com")))) + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.com").createKey())) .build()); writer.publishHost("ns1.example.tld"); @@ -324,7 +323,7 @@ public class DnsUpdateWriterTest { .asBuilder() .addSubordinateHost("ns1.example.tld") .addNameservers( - ImmutableSet.of(Key.create(externalNameserver), Key.create(inBailiwickNameserver))) + ImmutableSet.of(externalNameserver.createKey(), inBailiwickNameserver.createKey())) .build()); writer.publishDomain("example.tld"); @@ -359,7 +358,7 @@ public class DnsUpdateWriterTest { .asBuilder() .addSubordinateHost("ns1.example.tld") .addSubordinateHost("foo.example.tld") - .addNameserver(Key.create(inBailiwickNameserver)) + .addNameserver(inBailiwickNameserver.createKey()) .build()); writer.publishDomain("example.tld"); @@ -383,7 +382,7 @@ public class DnsUpdateWriterTest { DomainBase domain = persistActiveDomain("example.tld") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(persistActiveHost("ns1.example.tld")))) + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createKey())) .build(); persistResource(domain); when(mockResolver.send(any(Message.class))).thenReturn(messageWithResponseCode(Rcode.SERVFAIL)); diff --git a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java index 8d77dc4b1..514905866 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java @@ -693,7 +693,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase> nameservers = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> nameservers = new ImmutableSet.Builder<>(); for (int i = 1; i < 15; i++) { if (i != 2) { // Skip 2 since that's the one that the tests will add. nameservers.add( - Key.create( - loadByForeignKey( - HostResource.class, String.format("ns%d.example.foo", i), clock.nowUtc()) - .get())); + loadByForeignKey( + HostResource.class, String.format("ns%d.example.foo", i), clock.nowUtc()) + .get() + .createKey()); } } persistResource( @@ -285,11 +286,11 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase> nameservers = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> nameservers = new ImmutableSet.Builder<>(); for (int i = 0; i < 26; i++) { HostResource host = persistActiveHost(String.format("max_test_%d.example.tld", i)); if (i < 13) { - nameservers.add(Key.create(host)); + nameservers.add(host.createKey()); } } ImmutableList.Builder contactsBuilder = new ImmutableList.Builder<>(); @@ -376,15 +377,15 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase oneTimeBillKey; private Key recurringBillKey; + private Key domainKey; @Before public void setUp() { createTld("com"); - Key domainKey = Key.create(null, DomainBase.class, "4-COM"); - Key hostKey = - Key.create( - persistResource( + domainKey = Key.create(null, DomainBase.class, "4-COM"); + VKey hostKey = + persistResource( new HostResource.Builder() .setFullyQualifiedHostName("ns1.example.com") .setSuperordinateDomain(domainKey) .setRepoId("1-COM") - .build())); + .build()) + .createKey(); Key contact1Key = Key.create( persistResource( @@ -219,7 +221,7 @@ public class DomainBaseTest extends EntityTestCase { assertThat( newDomainBase("example.com") .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(newHostResource("foo.example.tld")))) + .setNameservers(ImmutableSet.of(newHostResource("foo.example.tld").createKey())) .build() .nsHosts) .isNotNull(); @@ -266,8 +268,8 @@ public class DomainBaseTest extends EntityTestCase { @Test public void testImplicitStatusValues() { - ImmutableSet> nameservers = - ImmutableSet.of(Key.create(newHostResource("foo.example.tld"))); + ImmutableSet> nameservers = + ImmutableSet.of(newHostResource("foo.example.tld").createKey()); StatusValue[] statuses = {StatusValue.OK}; // OK is implicit if there's no other statuses but there are nameservers. assertAboutDomains() @@ -769,4 +771,60 @@ public class DomainBaseTest extends EntityTestCase { assertThat(getOnlyElement(clone.getGracePeriods()).getType()) .isEqualTo(GracePeriodStatus.TRANSFER); } + + private static ImmutableSet> getOfyNameservers(DomainBase domain) { + return domain.getNameservers().stream().map(key -> key.getOfyKey()).collect(toImmutableSet()); + } + + @Test + public void testNameservers_nsHostsOfyKeys() { + assertThat(domain.nsHosts).isEqualTo(getOfyNameservers(domain)); + + // Test the setNameserver that functions on a function. + VKey host1Key = + persistResource( + new HostResource.Builder() + .setFullyQualifiedHostName("ns2.example.com") + .setSuperordinateDomain(domainKey) + .setRepoId("2-COM") + .build()) + .createKey(); + + DomainBase dom = new DomainBase.Builder(domain).setNameservers(host1Key).build(); + assertThat(dom.getNameservers()).isEqualTo(ImmutableSet.of(host1Key)); + assertThat(getOfyNameservers(dom)).isEqualTo(ImmutableSet.of(host1Key.getOfyKey())); + + // Test that setting to a single host of null throws an NPE. + assertThrows( + NullPointerException.class, + () -> new DomainBase.Builder(domain).setNameservers((VKey) null)); + + // Test that setting to a set of values works. + VKey host2Key = + persistResource( + new HostResource.Builder() + .setFullyQualifiedHostName("ns3.example.com") + .setSuperordinateDomain(domainKey) + .setRepoId("3-COM") + .build()) + .createKey(); + dom = + new DomainBase.Builder(domain).setNameservers(ImmutableSet.of(host1Key, host2Key)).build(); + assertThat(dom.getNameservers()).isEqualTo(ImmutableSet.of(host1Key, host2Key)); + assertThat(getOfyNameservers(dom)) + .isEqualTo(ImmutableSet.of(host1Key.getOfyKey(), host2Key.getOfyKey())); + + // Set of values, passing null. + dom = + new DomainBase.Builder(domain) + .setNameservers((ImmutableSet>) null) + .build(); + assertThat(dom.nsHostVKeys).isNull(); + assertThat(dom.nsHosts).isNull(); + + // Empty set of values gets translated to null. + dom = new DomainBase.Builder(domain).setNameservers(ImmutableSet.of()).build(); + assertThat(dom.nsHostVKeys).isNull(); + assertThat(dom.nsHosts).isNull(); + } } diff --git a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java index 7d1c47174..5b5e35e8c 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java @@ -45,6 +45,7 @@ import google.registry.model.host.HostResource; import google.registry.model.registrar.Registrar; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; +import google.registry.persistence.VKey; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapMetrics.WildcardType; @@ -409,7 +410,7 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase> hostKeysBuilder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> hostKeysBuilder = new ImmutableSet.Builder<>(); ImmutableSet.Builder subordinateHostnamesBuilder = new ImmutableSet.Builder<>(); String mainDomainName = String.format("domain%d.lol", numTotalDomainsPerActiveDomain); for (int i = numHosts; i >= 1; i--) { @@ -417,9 +418,9 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase> hostKeys = hostKeysBuilder.build(); + ImmutableSet> hostKeys = hostKeysBuilder.build(); // Create all the domains at once, then persist them in parallel, for increased efficiency. ImmutableList.Builder domainsBuilder = new ImmutableList.Builder<>(); for (int i = numActiveDomains * numTotalDomainsPerActiveDomain; i >= 1; i--) { diff --git a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java index 2b529eec7..8e05ad2d7 100644 --- a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java +++ b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java @@ -80,8 +80,8 @@ import org.junit.runners.JUnit4; /** * Unit tests for {@link DomainBaseToXjcConverter}. * - *

This tests the mapping between {@link DomainBase} and {@link XjcRdeDomain} as well as - * some exceptional conditions. + *

This tests the mapping between {@link DomainBase} and {@link XjcRdeDomain} as well as some + * exceptional conditions. */ @RunWith(JUnit4.class) public class DomainBaseToXjcConverterTest { @@ -99,14 +99,12 @@ public class DomainBaseToXjcConverterTest { @Test public void testConvertThick() { - XjcRdeDomain bean = - DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.FULL); + XjcRdeDomain bean = DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.FULL); assertThat(bean.getClID()).isEqualTo("GetTheeBack"); assertThat( - bean.getContacts() - .stream() + bean.getContacts().stream() .map(input -> String.format("%s %s", input.getType().toString(), input.getValue()))) .containsExactly("ADMIN 5372808-IRL", "TECH 5372808-TRL"); @@ -182,8 +180,7 @@ public class DomainBaseToXjcConverterTest { @Test public void testConvertThin() { - XjcRdeDomain bean = - DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.THIN); + XjcRdeDomain bean = DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.THIN); assertThat(bean.getRegistrant()).isNull(); assertThat(bean.getContacts()).isEmpty(); assertThat(bean.getSecDNS()).isNull(); @@ -191,15 +188,13 @@ public class DomainBaseToXjcConverterTest { @Test public void testMarshalThick() throws Exception { - XjcRdeDomain bean = - DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.FULL); + XjcRdeDomain bean = DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.FULL); wrapDeposit(bean).marshal(new ByteArrayOutputStream(), UTF_8); } @Test public void testMarshalThin() throws Exception { - XjcRdeDomain bean = - DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.THIN); + XjcRdeDomain bean = DomainBaseToXjcConverter.convertDomain(makeDomainBase(clock), RdeMode.THIN); wrapDeposit(bean).marshal(new ByteArrayOutputStream(), UTF_8); } @@ -225,17 +220,18 @@ public class DomainBaseToXjcConverterTest { newDomainBase("example.xn--q9jyb4c").asBuilder().setRepoId("2-Q9JYB4C").build(); HistoryEntry historyEntry = persistResource(new HistoryEntry.Builder().setParent(domain).build()); - BillingEvent.OneTime billingEvent = persistResource( - new BillingEvent.OneTime.Builder() - .setReason(Reason.CREATE) - .setTargetId("example.xn--q9jyb4c") - .setClientId("TheRegistrar") - .setCost(Money.of(USD, 26)) - .setPeriodYears(2) - .setEventTime(DateTime.parse("1910-01-01T00:00:00Z")) - .setBillingTime(DateTime.parse("1910-01-01T00:00:00Z")) - .setParent(historyEntry) - .build()); + BillingEvent.OneTime billingEvent = + persistResource( + new BillingEvent.OneTime.Builder() + .setReason(Reason.CREATE) + .setTargetId("example.xn--q9jyb4c") + .setClientId("TheRegistrar") + .setCost(Money.of(USD, 26)) + .setPeriodYears(2) + .setEventTime(DateTime.parse("1910-01-01T00:00:00Z")) + .setBillingTime(DateTime.parse("1910-01-01T00:00:00Z")) + .setParent(historyEntry) + .build()); domain = domain .asBuilder() @@ -272,11 +268,10 @@ public class DomainBaseToXjcConverterTest { .setLastEppUpdateTime(DateTime.parse("1920-01-01T00:00:00Z")) .setNameservers( ImmutableSet.of( - Key.create( - makeHostResource(clock, "3-Q9JYB4C", "bird.or.devil.みんな", "1.2.3.4")), - Key.create( - makeHostResource( - clock, "4-Q9JYB4C", "ns2.cat.みんな", "bad:f00d:cafe::15:beef")))) + makeHostResource(clock, "3-Q9JYB4C", "bird.or.devil.みんな", "1.2.3.4") + .createKey(), + makeHostResource(clock, "4-Q9JYB4C", "ns2.cat.みんな", "bad:f00d:cafe::15:beef") + .createKey())) .setRegistrant( Key.create( makeContactResource( @@ -383,27 +378,24 @@ public class DomainBaseToXjcConverterTest { .setPersistedCurrentSponsorClientId("GetTheeBack") .setCreationClientId("GetTheeBack") .setCreationTimeForTest(END_OF_TIME) - .setInternationalizedPostalInfo(new PostalInfo.Builder() - .setType(PostalInfo.Type.INTERNATIONALIZED) - .setName(name) - .setOrg("SINNERS INCORPORATED") - .setAddress(new ContactAddress.Builder() - .setStreet(ImmutableList.of("123 Example Boulevard")) - .setCity("KOKOMO") - .setState("BM") - .setZip("31337") - .setCountryCode("US") + .setInternationalizedPostalInfo( + new PostalInfo.Builder() + .setType(PostalInfo.Type.INTERNATIONALIZED) + .setName(name) + .setOrg("SINNERS INCORPORATED") + .setAddress( + new ContactAddress.Builder() + .setStreet(ImmutableList.of("123 Example Boulevard")) + .setCity("KOKOMO") + .setState("BM") + .setZip("31337") + .setCountryCode("US") + .build()) .build()) - .build()) .setRepoId(repoId) .setVoiceNumber( - new ContactPhoneNumber.Builder() - .setPhoneNumber("+1.2126660420") - .build()) - .setFaxNumber( - new ContactPhoneNumber.Builder() - .setPhoneNumber("+1.2126660421") - .build()) + new ContactPhoneNumber.Builder().setPhoneNumber("+1.2126660420").build()) + .setFaxNumber(new ContactPhoneNumber.Builder().setPhoneNumber("+1.2126660421").build()) .build()); } diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index 3aa2d6d0f..0ea2f5328 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -58,27 +58,30 @@ import org.joda.time.DateTime; final class RdeFixtures { static DomainBase makeDomainBase(FakeClock clock, String tld) { - DomainBase domain = new DomainBase.Builder() - .setFullyQualifiedDomainName("example." + tld) - .setRepoId(generateNewDomainRoid(tld)) - .setRegistrant(Key.create( - makeContactResource(clock, - "5372808-ERL", "(◕‿◕) nevermore", "prophet@evil.みんな"))) - .build(); + DomainBase domain = + new DomainBase.Builder() + .setFullyQualifiedDomainName("example." + tld) + .setRepoId(generateNewDomainRoid(tld)) + .setRegistrant( + Key.create( + makeContactResource( + clock, "5372808-ERL", "(◕‿◕) nevermore", "prophet@evil.みんな"))) + .build(); HistoryEntry historyEntry = persistResource(new HistoryEntry.Builder().setParent(domain).build()); clock.advanceOneMilli(); - BillingEvent.OneTime billingEvent = persistResourceWithCommitLog( - new BillingEvent.OneTime.Builder() - .setReason(Reason.CREATE) - .setTargetId("example." + tld) - .setClientId("TheRegistrar") - .setCost(Money.of(USD, 26)) - .setPeriodYears(2) - .setEventTime(DateTime.parse("1990-01-01T00:00:00Z")) - .setBillingTime(DateTime.parse("1990-01-01T00:00:00Z")) - .setParent(historyEntry) - .build()); + BillingEvent.OneTime billingEvent = + persistResourceWithCommitLog( + new BillingEvent.OneTime.Builder() + .setReason(Reason.CREATE) + .setTargetId("example." + tld) + .setClientId("TheRegistrar") + .setCost(Money.of(USD, 26)) + .setPeriodYears(2) + .setEventTime(DateTime.parse("1990-01-01T00:00:00Z")) + .setBillingTime(DateTime.parse("1990-01-01T00:00:00Z")) + .setParent(historyEntry) + .build()); domain = domain .asBuilder() @@ -114,8 +117,8 @@ final class RdeFixtures { .setIdnTableName("extended_latin") .setNameservers( ImmutableSet.of( - Key.create(makeHostResource(clock, "bird.or.devil.みんな", "1.2.3.4")), - Key.create(makeHostResource(clock, "ns2.cat.みんな", "bad:f00d:cafe::15:beef")))) + makeHostResource(clock, "bird.or.devil.みんな", "1.2.3.4").createKey(), + makeHostResource(clock, "ns2.cat.みんな", "bad:f00d:cafe::15:beef").createKey())) .setRegistrationExpirationTime(DateTime.parse("1994-01-01T00:00:00Z")) .setGracePeriods( ImmutableSet.of( @@ -220,26 +223,23 @@ final class RdeFixtures { .setPersistedCurrentSponsorClientId("GetTheeBack") .setCreationClientId("GetTheeBack") .setCreationTimeForTest(clock.nowUtc()) - .setInternationalizedPostalInfo(new PostalInfo.Builder() - .setType(PostalInfo.Type.INTERNATIONALIZED) - .setName(name) - .setOrg("DOGE INCORPORATED") - .setAddress(new ContactAddress.Builder() - .setStreet(ImmutableList.of("123 Example Boulevard")) - .setCity("KOKOMO") - .setState("BM") - .setZip("31337") - .setCountryCode("US") + .setInternationalizedPostalInfo( + new PostalInfo.Builder() + .setType(PostalInfo.Type.INTERNATIONALIZED) + .setName(name) + .setOrg("DOGE INCORPORATED") + .setAddress( + new ContactAddress.Builder() + .setStreet(ImmutableList.of("123 Example Boulevard")) + .setCity("KOKOMO") + .setState("BM") + .setZip("31337") + .setCountryCode("US") + .build()) .build()) - .build()) .setVoiceNumber( - new ContactPhoneNumber.Builder() - .setPhoneNumber("+1.5558675309") - .build()) - .setFaxNumber( - new ContactPhoneNumber.Builder() - .setPhoneNumber("+1.5558675310") - .build()) + new ContactPhoneNumber.Builder().setPhoneNumber("+1.5558675309").build()) + .setFaxNumber(new ContactPhoneNumber.Builder().setPhoneNumber("+1.5558675310").build()) .build()); } diff --git a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java index 4dfca835a..e5ca7492f 100644 --- a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java +++ b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java @@ -35,7 +35,6 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.net.MediaType; -import com.googlecode.objectify.Key; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; @@ -375,7 +374,9 @@ public class Spec11EmailUtilsTest { private static DomainBase persistDomainWithHost(String domainName, HostResource host) { return persistResource( - newDomainBase(domainName).asBuilder().setNameservers(ImmutableSet.of(Key.create(host))) + newDomainBase(domainName) + .asBuilder() + .setNameservers(ImmutableSet.of(host.createKey())) .build()); } } diff --git a/core/src/test/java/google/registry/server/Fixture.java b/core/src/test/java/google/registry/server/Fixture.java index 8cda8c67e..d9004b556 100644 --- a/core/src/test/java/google/registry/server/Fixture.java +++ b/core/src/test/java/google/registry/server/Fixture.java @@ -123,28 +123,34 @@ public enum Fixture { .build()); persistResource( - newDomainBase("love.xn--q9jyb4c", justine).asBuilder() - .setContacts(ImmutableSet.of( - DesignatedContact.create(ADMIN, Key.create(robert)), - DesignatedContact.create(BILLING, Key.create(google)), - DesignatedContact.create(TECH, Key.create(justine)))) - .setNameservers(ImmutableSet.of( - Key.create(persistActiveHost("ns1.love.xn--q9jyb4c")), - Key.create(persistActiveHost("ns2.love.xn--q9jyb4c")))) + newDomainBase("love.xn--q9jyb4c", justine) + .asBuilder() + .setContacts( + ImmutableSet.of( + DesignatedContact.create(ADMIN, Key.create(robert)), + DesignatedContact.create(BILLING, Key.create(google)), + DesignatedContact.create(TECH, Key.create(justine)))) + .setNameservers( + ImmutableSet.of( + persistActiveHost("ns1.love.xn--q9jyb4c").createKey(), + persistActiveHost("ns2.love.xn--q9jyb4c").createKey())) .build()); persistResource( - newDomainBase("moogle.example", justine).asBuilder() - .setContacts(ImmutableSet.of( - DesignatedContact.create(ADMIN, Key.create(robert)), - DesignatedContact.create(BILLING, Key.create(google)), - DesignatedContact.create(TECH, Key.create(justine)))) - .setNameservers(ImmutableSet.of( - Key.create(persistActiveHost("ns1.linode.com")), - Key.create(persistActiveHost("ns2.linode.com")), - Key.create(persistActiveHost("ns3.linode.com")), - Key.create(persistActiveHost("ns4.linode.com")), - Key.create(persistActiveHost("ns5.linode.com")))) + newDomainBase("moogle.example", justine) + .asBuilder() + .setContacts( + ImmutableSet.of( + DesignatedContact.create(ADMIN, Key.create(robert)), + DesignatedContact.create(BILLING, Key.create(google)), + DesignatedContact.create(TECH, Key.create(justine)))) + .setNameservers( + ImmutableSet.of( + persistActiveHost("ns1.linode.com").createKey(), + persistActiveHost("ns2.linode.com").createKey(), + persistActiveHost("ns3.linode.com").createKey(), + persistActiveHost("ns4.linode.com").createKey(), + persistActiveHost("ns5.linode.com").createKey())) .build()); persistResource( diff --git a/core/src/test/java/google/registry/testing/DatastoreHelper.java b/core/src/test/java/google/registry/testing/DatastoreHelper.java index 6515074c7..8f57e0488 100644 --- a/core/src/test/java/google/registry/testing/DatastoreHelper.java +++ b/core/src/test/java/google/registry/testing/DatastoreHelper.java @@ -138,7 +138,7 @@ public class DatastoreHelper { public static DomainBase newDomainBase(String domainName, HostResource host) { return newDomainBase(domainName) .asBuilder() - .setNameservers(ImmutableSet.of(Key.create(host))) + .setNameservers(ImmutableSet.of(host.createKey())) .build(); } diff --git a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java index 4cb3d88dc..6df7c35c8 100644 --- a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java +++ b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java @@ -40,6 +40,7 @@ import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; import google.registry.model.registrar.RegistrarContact; import google.registry.model.reporting.HistoryEntry; +import google.registry.persistence.VKey; import google.registry.util.Idn; import java.net.InetAddress; import java.util.List; @@ -374,12 +375,12 @@ public final class FullFieldsTestEntityHelper { builder.setContacts(contactsBuilder.build()); } if ((ns1 != null) || (ns2 != null)) { - ImmutableSet.Builder> nsBuilder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> nsBuilder = new ImmutableSet.Builder<>(); if (ns1 != null) { - nsBuilder.add(Key.create(ns1)); + nsBuilder.add(ns1.createKey()); } if (ns2 != null) { - nsBuilder.add(Key.create(ns2)); + nsBuilder.add(ns2.createKey()); } builder.setNameservers(nsBuilder.build()); } diff --git a/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java b/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java index f6b8cf75c..b8c0edd86 100644 --- a/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java +++ b/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java @@ -32,7 +32,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.net.InetAddresses; -import com.googlecode.objectify.Key; import google.registry.model.domain.DomainBase; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.StatusValue; @@ -144,15 +143,23 @@ public class GenerateDnsReportCommandTest extends CommandTestCase> hostRefs = new ImmutableSet.Builder<>(); + ImmutableSet.Builder> hostRefs = new ImmutableSet.Builder<>(); for (HostResource host : hosts) { - hostRefs.add(Key.create(host)); + hostRefs.add(host.createKey()); } persistResource(newDomainBase("evil.tld").asBuilder() .setNameservers(hostRefs.build()) diff --git a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java index c43557426..26514d2b7 100644 --- a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java @@ -31,6 +31,7 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DesignatedContact; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import org.junit.Test; /** Unit tests for {@link UpdateDomainCommand}. */ @@ -116,12 +117,12 @@ public class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = - ImmutableSet.of(Key.create(host1), Key.create(host2)); + ImmutableSet> nameservers = + ImmutableSet.of(host1.createKey(), host2.createKey()); persistResource( newDomainBase("example.tld").asBuilder().setNameservers(nameservers).build()); runCommandForced( @@ -213,7 +214,7 @@ public class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(Key.create(host)); + ImmutableSet> nameservers = ImmutableSet.of(host.createKey()); persistResource( newDomainBase("example.tld") .asBuilder() @@ -257,7 +258,7 @@ public class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(Key.create(host)); + ImmutableSet> nameservers = ImmutableSet.of(host.createKey()); persistResource( newDomainBase("example.tld") .asBuilder() diff --git a/core/src/test/java/google/registry/tools/server/GenerateZoneFilesActionTest.java b/core/src/test/java/google/registry/tools/server/GenerateZoneFilesActionTest.java index 2c7c8a04a..4828183e5 100644 --- a/core/src/test/java/google/registry/tools/server/GenerateZoneFilesActionTest.java +++ b/core/src/test/java/google/registry/tools/server/GenerateZoneFilesActionTest.java @@ -34,10 +34,10 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.testing.FakeClock; import google.registry.testing.mapreduce.MapreduceTestCase; import java.net.InetAddress; @@ -67,8 +67,8 @@ public class GenerateZoneFilesActionTest extends MapreduceTestCase> nameservers = - ImmutableSet.of(Key.create(host1), Key.create(host2)); + ImmutableSet> nameservers = + ImmutableSet.of(host1.createKey(), host2.createKey()); // This domain will have glue records, because it has a subordinate host which is its own // nameserver. None of the other domains should have glue records, because their nameservers are // subordinate to different domains. diff --git a/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java b/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java index 9792b285c..ad0d92b2a 100644 --- a/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java +++ b/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java @@ -37,6 +37,7 @@ import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; +import google.registry.persistence.VKey; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.whois.WhoisResponse.WhoisResponseResults; @@ -220,8 +221,8 @@ public class DomainWhoisResponseTest { .setEmailAddress("EMAIL@EXAMPLE.tld") .build()); - Key hostResource1Key = Key.create(hostResource1); - Key hostResource2Key = Key.create(hostResource2); + VKey hostResource1Key = hostResource1.createKey(); + VKey hostResource2Key = hostResource2.createKey(); Key registrantResourceKey = Key.create(registrant); Key adminResourceKey = Key.create(adminContact); Key techResourceKey = Key.create(techContact);