From e0d0dee9534bbd116c828d8f70ad7b8109d63bce Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Tue, 2 Jun 2020 13:17:16 -0400 Subject: [PATCH] Use TransactionManager for hosts and contacts (#603) * Use TransactionManager for hosts and contacts Replace Ofy calls with TransactionManager for most interactions involving hosts and contacts. In the course of this, also convert ForeignKeyIndex and the EppResourceCache. * Minor formatting fix --- .../registry/flows/ResourceFlowUtils.java | 8 +-- .../flows/domain/DomainCreateFlow.java | 7 +-- .../flows/domain/DomainFlowUtils.java | 13 ++-- .../flows/domain/DomainUpdateFlow.java | 22 ++----- .../google/registry/model/EppResource.java | 61 +++++++++---------- .../registry/model/EppResourceUtils.java | 3 +- .../registry/model/domain/DomainCommand.java | 28 ++++----- .../registry/model/index/ForeignKeyIndex.java | 19 +++--- .../registry/rdap/RdapDomainSearchAction.java | 43 +++++++------ .../registry/tools/CommandUtilities.java | 4 +- .../tools/GetHistoryEntriesCommand.java | 14 ++--- .../tools/ResaveEppResourceCommand.java | 15 +++-- .../registry/whois/DomainWhoisResponse.java | 2 +- .../flows/host/HostUpdateFlowTest.java | 2 +- .../registry/model/EppResourceTest.java | 17 +++--- .../model/index/ForeignKeyIndexTest.java | 7 +-- .../google/registry/model/schema.txt | 6 +- 17 files changed, 127 insertions(+), 144 deletions(-) diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 11f7f4f7f..5076c4f0d 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -94,10 +94,10 @@ public final class ResourceFlowUtils { * trust the query and need to do the full mapreduce. */ Iterable> keys = - queryForLinkedDomains(fki.getResourceKey(), now) + queryForLinkedDomains(fki.getResourceKey().getOfyKey(), now) .limit(FAILFAST_CHECK_COUNT) .keys(); - VKey resourceVKey = VKey.createOfy(resourceClass, fki.getResourceKey()); + VKey resourceVKey = fki.getResourceKey(); Predicate predicate = domain -> getPotentialReferences.apply(domain).contains(resourceVKey); return ofy().load().keys(keys).values().stream().anyMatch(predicate) @@ -136,9 +136,9 @@ public final class ResourceFlowUtils { public static void verifyResourceDoesNotExist( Class clazz, String targetId, DateTime now, String clientId) throws EppException { - Key key = loadAndGetKey(clazz, targetId, now); + VKey key = loadAndGetKey(clazz, targetId, now); if (key != null) { - R resource = ofy().load().key(key).now(); + R resource = tm().load(key); // These are similar exceptions, but we can track them internally as log-based metrics. if (Objects.equals(clientId, resource.getPersistedCurrentSponsorClientId())) { throw new ResourceAlreadyExistsForThisClientException(targetId); 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 17fda5853..40019b615 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -77,7 +77,6 @@ import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.billing.BillingEvent.Recurring; -import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand; import google.registry.model.domain.DomainCommand.Create; @@ -353,14 +352,12 @@ public class DomainCreateFlow implements TransactionalFlow { .setLaunchNotice(hasClaimsNotice ? launchCreate.get().getNotice() : null) .setSmdId(signedMarkId) .setDsData(secDnsCreate.isPresent() ? secDnsCreate.get().getDsData() : null) - .setRegistrant(VKey.createOfy(ContactResource.class, command.getRegistrant())) + .setRegistrant(command.getRegistrant()) .setAuthInfo(command.getAuthInfo()) .setFullyQualifiedDomainName(targetId) .setNameservers( (ImmutableSet>) - command.getNameservers().stream() - .map(key -> VKey.createOfy(HostResource.class, key)) - .collect(toImmutableSet())) + command.getNameservers().stream().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/DomainFlowUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java index feaaf95e7..db176f37b 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -307,14 +307,11 @@ public class DomainFlowUtils { /** Verify that no linked resources have disallowed statuses. */ static void verifyNotInPendingDelete( Set contacts, - Key registrant, - Set> nameservers) + VKey registrant, + Set> nameservers) throws EppException { - ImmutableList.Builder> keysToLoad = new ImmutableList.Builder<>(); - contacts.stream() - .map(DesignatedContact::getContactKey) - .map(VKey::getOfyKey) - .forEach(keysToLoad::add); + ImmutableList.Builder> keysToLoad = new ImmutableList.Builder<>(); + contacts.stream().map(DesignatedContact::getContactKey).forEach(keysToLoad::add); Optional.ofNullable(registrant).ifPresent(keysToLoad::add); keysToLoad.addAll(nameservers); verifyNotInPendingDelete(EppResource.loadCached(keysToLoad.build()).values()); @@ -381,7 +378,7 @@ public class DomainFlowUtils { } static void validateRequiredContactsPresent( - @Nullable Key registrant, Set contacts) + @Nullable VKey registrant, Set contacts) throws RequiredParameterMissingException { if (registrant == null) { throw new MissingRegistrantException(); 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 127a1b8ff..fd43d9f7c 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -60,7 +60,6 @@ import google.registry.flows.domain.DomainFlowUtils.MissingRegistrantException; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; -import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand.Update; import google.registry.model.domain.DomainCommand.Update.AddRemove; @@ -73,11 +72,9 @@ 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; @@ -247,22 +244,11 @@ public final class DomainUpdateFlow implements TransactionalFlow { .setLastEppUpdateClientId(clientId) .addStatusValues(add.getStatusValues()) .removeStatusValues(remove.getStatusValues()) - .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())) + .addNameservers(add.getNameservers().stream().collect(toImmutableSet())) + .removeNameservers(remove.getNameservers().stream().collect(toImmutableSet())) .addContacts(add.getContacts()) .removeContacts(remove.getContacts()) - .setRegistrant( - firstNonNull( - change.getRegistrant() != null - ? VKey.createOfy(ContactResource.class, change.getRegistrant()) - : null, - domain.getRegistrant())) + .setRegistrant(firstNonNull(change.getRegistrant(), domain.getRegistrant())) .setAuthInfo(firstNonNull(change.getAuthInfo(), domain.getAuthInfo())); return domainBuilder.build(); } @@ -275,7 +261,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { private void validateNewState(DomainBase newDomain) throws EppException { validateNoDuplicateContacts(newDomain.getContacts()); - validateRequiredContactsPresent(newDomain.getRegistrant().getOfyKey(), newDomain.getContacts()); + validateRequiredContactsPresent(newDomain.getRegistrant(), newDomain.getContacts()); validateDsData(newDomain.getDsData()); validateNameserversCountForTld( newDomain.getTld(), diff --git a/core/src/main/java/google/registry/model/EppResource.java b/core/src/main/java/google/registry/model/EppResource.java index f9c4622a4..c6112e612 100644 --- a/core/src/main/java/google/registry/model/EppResource.java +++ b/core/src/main/java/google/registry/model/EppResource.java @@ -16,12 +16,11 @@ package google.registry.model; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.union; import static google.registry.config.RegistryConfig.getEppResourceCachingDuration; import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntries; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.nullToEmpty; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; @@ -32,11 +31,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Streams; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Id; import com.googlecode.objectify.annotation.Index; @@ -47,10 +44,10 @@ import google.registry.model.transfer.TransferData; import google.registry.persistence.VKey; import google.registry.util.NonFinalForTesting; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.stream.StreamSupport; import javax.persistence.Column; import javax.persistence.MappedSuperclass; import javax.persistence.Transient; @@ -333,18 +330,18 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { } } - static final CacheLoader, EppResource> CACHE_LOADER = - new CacheLoader, EppResource>() { + static final CacheLoader, EppResource> CACHE_LOADER = + new CacheLoader, EppResource>() { @Override - public EppResource load(Key key) { - return tm().doTransactionless(() -> ofy().load().key(key).now()); + public EppResource load(VKey key) { + return tm().doTransactionless(() -> tm().load(key)); } @Override - public Map, EppResource> loadAll( - Iterable> keys) { - return tm().doTransactionless(() -> loadMultiple(keys)); + public Map, EppResource> loadAll( + Iterable> keys) { + return tm().doTransactionless(() -> loadAsMap(keys)); } }; @@ -356,10 +353,10 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { * directly should of course never use the cache. */ @NonFinalForTesting - private static LoadingCache, EppResource> cacheEppResources = + private static LoadingCache, EppResource> cacheEppResources = createEppResourcesCache(getEppResourceCachingDuration()); - private static LoadingCache, EppResource> createEppResourcesCache( + private static LoadingCache, EppResource> createEppResourcesCache( Duration expiry) { return CacheBuilder.newBuilder() .expireAfterWrite(expiry.getMillis(), MILLISECONDS) @@ -373,29 +370,16 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { cacheEppResources = createEppResourcesCache(effectiveExpiry); } - private static ImmutableMap, EppResource> loadMultiple( - Iterable> keys) { - // This cast is safe because, in Objectify, Key can also be - // treated as a Key. - @SuppressWarnings("unchecked") - ImmutableList> typedKeys = - Streams.stream(keys).map(key -> (Key) key).collect(toImmutableList()); - - // Typing shenanigans are required to return the map with the correct required generic type. - return ofy().load().keys(typedKeys).entrySet().stream() - .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); - } - /** * Loads the given EppResources by their keys using the cache (if enabled). * *

Don't use this unless you really need it for performance reasons, and be sure that you are * OK with the trade-offs in loss of transactional consistency. */ - public static ImmutableMap, EppResource> loadCached( - Iterable> keys) { + public static ImmutableMap, EppResource> loadCached( + Iterable> keys) { if (!RegistryConfig.isEppResourceCachingEnabled()) { - return loadMultiple(keys); + return loadAsMap(keys); } try { return cacheEppResources.getAll(keys); @@ -410,9 +394,9 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { *

Don't use this unless you really need it for performance reasons, and be sure that you are * OK with the trade-offs in loss of transactional consistency. */ - public static T loadCached(Key key) { + public static T loadCached(VKey key) { if (!RegistryConfig.isEppResourceCachingEnabled()) { - return ofy().load().key(key).now(); + return tm().load(key); } try { // Safe to cast because loading a Key returns an entity of type T. @@ -423,4 +407,17 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { throw new RuntimeException("Error loading cached EppResources", e.getCause()); } } + + private static ImmutableMap, EppResource> loadAsMap( + Iterable> keys) { + return StreamSupport.stream(keys.spliterator(), false) + // It's possible for us to receive the same key more than once which causes + // the immutable map build to break with a duplicate key, so we have to ensure key + // uniqueness. + .distinct() + // We have to use "key -> key" here instead of the identity() function, because + // the latter breaks the fairly complicated generic type checking required by the + // caching interface. + .collect(toImmutableMap(key -> key, key -> tm().load(key))); + } } diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index fa4faeae4..678021648 100644 --- a/core/src/main/java/google/registry/model/EppResourceUtils.java +++ b/core/src/main/java/google/registry/model/EppResourceUtils.java @@ -17,6 +17,7 @@ package google.registry.model; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.DateTimeUtils.isAtOrAfter; import static google.registry.util.DateTimeUtils.isBeforeOrAt; import static google.registry.util.DateTimeUtils.latestOf; @@ -141,7 +142,7 @@ public final class EppResourceUtils { T resource = useCache ? EppResource.loadCached(fki.getResourceKey()) - : ofy().load().key(fki.getResourceKey()).now(); + : tm().maybeLoad(fki.getResourceKey()).orElse(null); if (resource == null || isAtOrAfter(now, resource.getDeletionTime())) { return Optional.empty(); } diff --git a/core/src/main/java/google/registry/model/domain/DomainCommand.java b/core/src/main/java/google/registry/model/domain/DomainCommand.java index 766fec521..6584f0bb8 100644 --- a/core/src/main/java/google/registry/model/domain/DomainCommand.java +++ b/core/src/main/java/google/registry/model/domain/DomainCommand.java @@ -30,7 +30,6 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.contact.ContactResource; @@ -82,8 +81,7 @@ public class DomainCommand { String registrantContactId; /** A resolved key to the registrant who registered this domain. */ - @XmlTransient - Key registrant; + @XmlTransient VKey registrant; /** Authorization info (aka transfer secret) of the domain. */ DomainAuthInfo authInfo; @@ -93,7 +91,7 @@ public class DomainCommand { } @Nullable - public Key getRegistrant() { + public VKey getRegistrant() { return registrant; } @@ -129,8 +127,7 @@ public class DomainCommand { Set nameserverFullyQualifiedHostNames; /** Resolved keys to hosts that are the nameservers for the domain. */ - @XmlTransient - Set> nameservers; + @XmlTransient Set> nameservers; /** Foreign keyed associated contacts for the domain (other than registrant). */ @XmlElement(name = "contact") @@ -160,7 +157,7 @@ public class DomainCommand { return nullToEmptyImmutableCopy(nameserverFullyQualifiedHostNames); } - public ImmutableSet> getNameservers() { + public ImmutableSet> getNameservers() { return nullToEmptyImmutableCopy(nameservers); } @@ -190,7 +187,7 @@ public class DomainCommand { now); for (DesignatedContact contact : contacts) { if (DesignatedContact.Type.REGISTRANT.equals(contact.getType())) { - clone.registrant = contact.getContactKey().getOfyKey(); + clone.registrant = contact.getContactKey(); clone.contacts = forceEmptyToNull(difference(contacts, contact)); break; } @@ -354,8 +351,7 @@ public class DomainCommand { Set nameserverFullyQualifiedHostNames; /** Resolved keys to hosts that are the nameservers for the domain. */ - @XmlTransient - Set> nameservers; + @XmlTransient Set> nameservers; /** Foreign keyed associated contacts for the domain (other than registrant). */ @XmlElement(name = "contact") @@ -369,7 +365,7 @@ public class DomainCommand { return nullSafeImmutableCopy(nameserverFullyQualifiedHostNames); } - public ImmutableSet> getNameservers() { + public ImmutableSet> getNameservers() { return nullToEmptyImmutableCopy(nameservers); } @@ -419,7 +415,7 @@ public class DomainCommand { } } - private static Set> linkHosts( + private static Set> linkHosts( Set fullyQualifiedHostNames, DateTime now) throws InvalidReferencesException { if (fullyQualifiedHostNames == null) { return null; @@ -437,20 +433,18 @@ public class DomainCommand { for (ForeignKeyedDesignatedContact contact : contacts) { foreignKeys.add(contact.contactId); } - ImmutableMap> loadedContacts = + ImmutableMap> loadedContacts = loadByForeignKeysCached(foreignKeys.build(), ContactResource.class, now); ImmutableSet.Builder linkedContacts = new ImmutableSet.Builder<>(); for (ForeignKeyedDesignatedContact contact : contacts) { linkedContacts.add( - DesignatedContact.create( - contact.type, - VKey.createOfy(ContactResource.class, loadedContacts.get(contact.contactId)))); + DesignatedContact.create(contact.type, loadedContacts.get(contact.contactId))); } return linkedContacts.build(); } /** Loads keys to cached EPP resources by their foreign keys. */ - private static ImmutableMap> loadByForeignKeysCached( + private static ImmutableMap> loadByForeignKeysCached( final Set foreignKeys, final Class clazz, final DateTime now) throws InvalidReferencesException { Map> fkis = ForeignKeyIndex.loadCached(clazz, foreignKeys, now); diff --git a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java index f94adc2b2..cf37ab17a 100644 --- a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java +++ b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java @@ -43,6 +43,7 @@ import google.registry.model.annotations.ReportedOn; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.util.NonFinalForTesting; import java.util.Map; import java.util.Optional; @@ -99,7 +100,7 @@ public abstract class ForeignKeyIndex extends BackupGroup *

This field holds a key to the only referenced resource. It is named "topReference" for * historical reasons. */ - Key topReference; + VKey topReference; public String getForeignKey() { return foreignKey; @@ -109,7 +110,7 @@ public abstract class ForeignKeyIndex extends BackupGroup return deletionTime; } - public Key getResourceKey() { + public VKey getResourceKey() { return topReference; } @@ -125,7 +126,7 @@ public abstract class ForeignKeyIndex extends BackupGroup @SuppressWarnings("unchecked") Class resourceClass = (Class) resource.getClass(); ForeignKeyIndex instance = instantiate(mapToFkiClass(resourceClass)); - instance.topReference = Key.create(resource); + instance.topReference = (VKey) resource.createVKey(); instance.foreignKey = resource.getForeignKey(); instance.deletionTime = deletionTime; return instance; @@ -141,18 +142,18 @@ public abstract class ForeignKeyIndex extends BackupGroup /** * Loads a {@link Key} to an {@link EppResource} from Datastore by foreign key. * - *

Returns null if no foreign key index with this foreign key was ever created, or if the - * most recently created foreign key index was deleted before time "now". This method does not - * actually check that the referenced resource actually exists. However, for normal epp resources, - * it is safe to assume that the referenced resource exists if the foreign key index does. + *

Returns null if no foreign key index with this foreign key was ever created, or if the most + * recently created foreign key index was deleted before time "now". This method does not actually + * check that the referenced resource actually exists. However, for normal epp resources, it is + * safe to assume that the referenced resource exists if the foreign key index does. * * @param clazz the resource type to load * @param foreignKey id to match * @param now the current logical time to use when checking for soft deletion of the foreign key - * index + * index */ @Nullable - public static Key loadAndGetKey( + public static VKey loadAndGetKey( Class clazz, String foreignKey, DateTime now) { ForeignKeyIndex index = load(clazz, foreignKey, now); return (index == null) ? null : index.getResourceKey(); diff --git a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java index 39c1ba946..5f917a67e 100644 --- a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java @@ -14,6 +14,7 @@ package google.registry.rdap; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.ofy.ObjectifyService.ofy; @@ -28,10 +29,10 @@ import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import com.google.common.net.InetAddresses; import com.google.common.primitives.Booleans; -import com.googlecode.objectify.Key; import com.googlecode.objectify.cmd.Query; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.rdap.RdapJsonFormatter.OutputDataType; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; @@ -50,6 +51,7 @@ import java.util.Comparator; import java.util.List; import java.util.Optional; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import javax.inject.Inject; /** @@ -243,7 +245,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { */ private DomainSearchResponse searchByNameserverLdhName( final RdapSearchPattern partialStringQuery) { - Iterable> hostKeys = getNameserverRefsByLdhName(partialStringQuery); + Iterable> hostKeys = getNameserverRefsByLdhName(partialStringQuery); if (Iterables.isEmpty(hostKeys)) { metricInformationBuilder.setNumHostsRetrieved(0); throw new NotFoundException("No matching nameservers found"); @@ -261,7 +263,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { * initial string is not required (e.g. "*.example.tld" is valid), because we can look up the * domain and just list all of its subordinate hosts. */ - private Iterable> getNameserverRefsByLdhName( + private Iterable> getNameserverRefsByLdhName( final RdapSearchPattern partialStringQuery) { // Handle queries without a wildcard. if (!partialStringQuery.getHasWildcard()) { @@ -292,11 +294,13 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { if (desiredRegistrar.isPresent()) { query = query.filter("currentSponsorClientId", desiredRegistrar.get()); } - return query.keys(); + return StreamSupport.stream(query.keys().spliterator(), false) + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet()); } /** Assembles a list of {@link HostResource} keys by name when the pattern has no wildcard. */ - private Iterable> getNameserverRefsByLdhNameWithoutWildcard( + private Iterable> getNameserverRefsByLdhNameWithoutWildcard( final RdapSearchPattern partialStringQuery) { // If we need to check the sponsoring registrar, we need to load the resource rather than just // the key. @@ -310,9 +314,9 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { return (!host.isPresent() || !desiredRegistrar.get().equals(host.get().getPersistedCurrentSponsorClientId())) ? ImmutableList.of() - : ImmutableList.of(Key.create(host.get())); + : ImmutableList.of(host.get().createVKey()); } else { - Key hostKey = + VKey hostKey = loadAndGetKey( HostResource.class, partialStringQuery.getInitialString(), @@ -322,7 +326,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { } /** Assembles a list of {@link HostResource} keys by name using a superordinate domain suffix. */ - private Iterable> getNameserverRefsByLdhNameWithSuffix( + private Iterable> getNameserverRefsByLdhNameWithSuffix( final RdapSearchPattern partialStringQuery) { // The suffix must be a domain that we manage. That way, we can look up the domain and search // through the subordinate hosts. This is more efficient, and lets us permit wildcard searches @@ -338,7 +342,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { "A suffix in a lookup by nameserver name " + "must be a domain defined in the system")); Optional desiredRegistrar = getDesiredRegistrar(); - ImmutableList.Builder> builder = new ImmutableList.Builder<>(); + ImmutableList.Builder> builder = new ImmutableList.Builder<>(); for (String fqhn : ImmutableSortedSet.copyOf(domainBase.getSubordinateHosts())) { // We can't just check that the host name starts with the initial query string, because // then the query ns.exam*.example.com would match against nameserver ns.example.com. @@ -351,10 +355,10 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { shouldIncludeDeleted() ? START_OF_TIME : getRequestTime()); if (host.isPresent() && desiredRegistrar.get().equals(host.get().getPersistedCurrentSponsorClientId())) { - builder.add(Key.create(host.get())); + builder.add(host.get().createVKey()); } } else { - Key hostKey = + VKey hostKey = loadAndGetKey( HostResource.class, fqhn, @@ -400,7 +404,10 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { if (desiredRegistrar.isPresent()) { query = query.filter("currentSponsorClientId", desiredRegistrar.get()); } - return searchByNameserverRefs(query.keys()); + return searchByNameserverRefs( + StreamSupport.stream(query.keys().spliterator(), false) + .map(key -> VKey.createOfy(HostResource.class, key)) + .collect(toImmutableSet())); } /** @@ -409,7 +416,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { *

This method is called by {@link #searchByNameserverLdhName} and {@link * #searchByNameserverIp} after they assemble the relevant host keys. */ - private DomainSearchResponse searchByNameserverRefs(final Iterable> hostKeys) { + private DomainSearchResponse searchByNameserverRefs(final Iterable> hostKeys) { // We must break the query up into chunks, because the in operator is limited to 30 subqueries. // Since it is possible for the same domain to show up more than once in our result list (if // we do a wildcard nameserver search that returns multiple nameservers used by the same @@ -420,11 +427,13 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { ImmutableSortedSet.orderedBy( Comparator.comparing(DomainBase::getFullyQualifiedDomainName)); int numHostKeysSearched = 0; - for (List> chunk : Iterables.partition(hostKeys, 30)) { + for (List> chunk : Iterables.partition(hostKeys, 30)) { numHostKeysSearched += chunk.size(); - Query query = ofy().load() - .type(DomainBase.class) - .filter("nsHosts in", chunk); + Query query = + ofy() + .load() + .type(DomainBase.class) + .filter("nsHosts in", chunk.stream().map(VKey::getOfyKey).collect(toImmutableSet())); if (!shouldIncludeDeleted()) { query = query.filter("deletionTime >", getRequestTime()); // If we are not performing an inequality query, we can filter on the cursor in the query. diff --git a/core/src/main/java/google/registry/tools/CommandUtilities.java b/core/src/main/java/google/registry/tools/CommandUtilities.java index 90e474455..d53878a67 100644 --- a/core/src/main/java/google/registry/tools/CommandUtilities.java +++ b/core/src/main/java/google/registry/tools/CommandUtilities.java @@ -18,12 +18,12 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Ascii; import com.google.common.base.Strings; -import com.googlecode.objectify.Key; import google.registry.model.EppResource; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.model.index.ForeignKeyIndex; +import google.registry.persistence.VKey; import org.joda.time.DateTime; /** Container class for static utility methods. */ @@ -41,7 +41,7 @@ class CommandUtilities { this.clazz = clazz; } - public Key getKey(String uniqueId, DateTime now) { + public VKey getKey(String uniqueId, DateTime now) { return ForeignKeyIndex.loadAndGetKey(clazz, uniqueId, now); } } diff --git a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java index 0642e6c9d..4c527c3cf 100644 --- a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java +++ b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java @@ -23,9 +23,9 @@ import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; -import com.googlecode.objectify.Key; import google.registry.model.EppResource; import google.registry.model.reporting.HistoryEntry; +import google.registry.persistence.VKey; import google.registry.tools.CommandUtilities.ResourceType; import google.registry.xml.XmlTransformer; import org.joda.time.DateTime; @@ -57,7 +57,7 @@ final class GetHistoryEntriesCommand implements CommandWithRemoteApi { @Override public void run() { - Key parentKey = null; + VKey parentKey = null; if (type != null || uniqueId != null) { checkArgument( type != null && uniqueId != null, @@ -66,12 +66,12 @@ final class GetHistoryEntriesCommand implements CommandWithRemoteApi { checkArgumentNotNull(parentKey, "Invalid resource ID"); } for (HistoryEntry entry : - (parentKey == null + (parentKey == null ? ofy().load().type(HistoryEntry.class) - : ofy().load().type(HistoryEntry.class).ancestor(parentKey)) - .order("modificationTime") - .filter("modificationTime >=", after) - .filter("modificationTime <=", before)) { + : ofy().load().type(HistoryEntry.class).ancestor(parentKey.getOfyKey())) + .order("modificationTime") + .filter("modificationTime >=", after) + .filter("modificationTime <=", before)) { System.out.printf( "Client: %s\nTime: %s\nClient TRID: %s\nServer TRID: %s\n%s\n", entry.getClientId(), diff --git a/core/src/main/java/google/registry/tools/ResaveEppResourceCommand.java b/core/src/main/java/google/registry/tools/ResaveEppResourceCommand.java index 758666832..000b3f7e3 100644 --- a/core/src/main/java/google/registry/tools/ResaveEppResourceCommand.java +++ b/core/src/main/java/google/registry/tools/ResaveEppResourceCommand.java @@ -14,14 +14,14 @@ package google.registry.tools; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; -import com.googlecode.objectify.Key; import google.registry.model.EppResource; +import google.registry.persistence.VKey; import google.registry.tools.CommandUtilities.ResourceType; import org.joda.time.DateTime; @@ -48,12 +48,15 @@ public final class ResaveEppResourceCommand extends MutatingCommand { @Override protected void init() { - Key resourceKey = checkArgumentNotNull( - type.getKey(uniqueId, DateTime.now(UTC)), - "Could not find active resource of type %s: %s", type, uniqueId); + VKey resourceKey = + checkArgumentNotNull( + type.getKey(uniqueId, DateTime.now(UTC)), + "Could not find active resource of type %s: %s", + type, + uniqueId); // Load the resource directly to bypass running cloneProjectedAtTime() automatically, which can // cause stageEntityChange() to fail due to implicit projection changes. - EppResource resource = ofy().load().key(resourceKey).now(); + EppResource resource = tm().load(resourceKey); stageEntityChange(resource, resource); } } diff --git a/core/src/main/java/google/registry/whois/DomainWhoisResponse.java b/core/src/main/java/google/registry/whois/DomainWhoisResponse.java index a718c04fd..133d2ae5c 100644 --- a/core/src/main/java/google/registry/whois/DomainWhoisResponse.java +++ b/core/src/main/java/google/registry/whois/DomainWhoisResponse.java @@ -156,7 +156,7 @@ final class DomainWhoisResponse extends WhoisResponseImpl { // If we refer to a contact that doesn't exist, that's a bug. It means referential integrity // has somehow been broken. We skip the rest of this contact, but log it to hopefully bring it // someone's attention. - ContactResource contactResource = EppResource.loadCached(contact.get().getOfyKey()); + ContactResource contactResource = EppResource.loadCached(contact.get()); if (contactResource == null) { logger.atSevere().log( "(BUG) Broken reference found from domain %s to contact %s", diff --git a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java index 7a0a52e1e..b845cdcc3 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -178,7 +178,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase oldFkiBeforeRename = ForeignKeyIndex.load(HostResource.class, oldHostName(), clock.nowUtc().minusMillis(1)); - assertThat(oldFkiBeforeRename.getResourceKey()).isEqualTo(Key.create(renamedHost)); + assertThat(oldFkiBeforeRename.getResourceKey()).isEqualTo(renamedHost.createVKey()); assertThat(oldFkiBeforeRename.getDeletionTime()).isEqualTo(clock.nowUtc()); ForeignKeyIndex oldFkiAfterRename = ForeignKeyIndex.load(HostResource.class, oldHostName(), clock.nowUtc()); diff --git a/core/src/test/java/google/registry/model/EppResourceTest.java b/core/src/test/java/google/registry/model/EppResourceTest.java index bc9719ce1..1c223dca1 100644 --- a/core/src/test/java/google/registry/model/EppResourceTest.java +++ b/core/src/test/java/google/registry/model/EppResourceTest.java @@ -22,7 +22,6 @@ import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistResource; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; import google.registry.model.contact.ContactResource; import google.registry.model.host.HostResource; import google.registry.testing.TestCacheRule; @@ -40,12 +39,12 @@ public class EppResourceTest extends EntityTestCase { @Test public void test_loadCached_ignoresContactChange() { ContactResource originalContact = persistActiveContact("contact123"); - assertThat(EppResource.loadCached(ImmutableList.of(Key.create(originalContact)))) - .containsExactly(Key.create(originalContact), originalContact); + assertThat(EppResource.loadCached(ImmutableList.of(originalContact.createVKey()))) + .containsExactly(originalContact.createVKey(), originalContact); ContactResource modifiedContact = persistResource(originalContact.asBuilder().setEmailAddress("different@fake.lol").build()); - assertThat(EppResource.loadCached(ImmutableList.of(Key.create(originalContact)))) - .containsExactly(Key.create(originalContact), originalContact); + assertThat(EppResource.loadCached(ImmutableList.of(originalContact.createVKey()))) + .containsExactly(originalContact.createVKey(), originalContact); assertThat(loadByForeignKey(ContactResource.class, "contact123", fakeClock.nowUtc())) .hasValue(modifiedContact); } @@ -53,13 +52,13 @@ public class EppResourceTest extends EntityTestCase { @Test public void test_loadCached_ignoresHostChange() { HostResource originalHost = persistActiveHost("ns1.example.com"); - assertThat(EppResource.loadCached(ImmutableList.of(Key.create(originalHost)))) - .containsExactly(Key.create(originalHost), originalHost); + assertThat(EppResource.loadCached(ImmutableList.of(originalHost.createVKey()))) + .containsExactly(originalHost.createVKey(), originalHost); HostResource modifiedHost = persistResource( originalHost.asBuilder().setLastTransferTime(fakeClock.nowUtc().minusDays(60)).build()); - assertThat(EppResource.loadCached(ImmutableList.of(Key.create(originalHost)))) - .containsExactly(Key.create(originalHost), originalHost); + assertThat(EppResource.loadCached(ImmutableList.of(originalHost.createVKey()))) + .containsExactly(originalHost.createVKey(), originalHost); assertThat(loadByForeignKey(HostResource.class, "ns1.example.com", fakeClock.nowUtc())) .hasValue(modifiedHost); } diff --git a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java index b27569987..372d273fe 100644 --- a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java +++ b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java @@ -17,7 +17,7 @@ package google.registry.model.index; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; 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.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.deleteResource; import static google.registry.testing.DatastoreHelper.persistActiveContact; @@ -27,7 +27,6 @@ import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; import google.registry.model.EntityTestCase; import google.registry.model.contact.ContactResource; import google.registry.model.host.HostResource; @@ -56,7 +55,7 @@ public class ForeignKeyIndexTest extends EntityTestCase { HostResource host = persistActiveHost("ns1.example.com"); ForeignKeyIndex fki = ForeignKeyIndex.load(HostResource.class, "ns1.example.com", fakeClock.nowUtc()); - assertThat(ofy().load().key(fki.getResourceKey()).now()).isEqualTo(host); + assertThat(tm().load(fki.getResourceKey())).isEqualTo(host); assertThat(fki.getDeletionTime()).isEqualTo(END_OF_TIME); } @@ -89,7 +88,7 @@ public class ForeignKeyIndexTest extends EntityTestCase { fakeClock.advanceOneMilli(); ForeignKeyHostIndex fki = new ForeignKeyHostIndex(); fki.foreignKey = "ns1.example.com"; - fki.topReference = Key.create(host1); + fki.topReference = host1.createVKey(); fki.deletionTime = fakeClock.nowUtc(); persistResource(fki); assertThat(ForeignKeyIndex.load(HostResource.class, "ns1.example.com", fakeClock.nowUtc())) diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index 52562bc8d..ff833117a 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -314,20 +314,20 @@ class google.registry.model.index.EppResourceIndexBucket { } class google.registry.model.index.ForeignKeyIndex$ForeignKeyContactIndex { @Id java.lang.String foreignKey; - com.googlecode.objectify.Key topReference; google.registry.model.UpdateAutoTimestamp updateTimestamp; + google.registry.persistence.VKey topReference; org.joda.time.DateTime deletionTime; } class google.registry.model.index.ForeignKeyIndex$ForeignKeyDomainIndex { @Id java.lang.String foreignKey; - com.googlecode.objectify.Key topReference; google.registry.model.UpdateAutoTimestamp updateTimestamp; + google.registry.persistence.VKey topReference; org.joda.time.DateTime deletionTime; } class google.registry.model.index.ForeignKeyIndex$ForeignKeyHostIndex { @Id java.lang.String foreignKey; - com.googlecode.objectify.Key topReference; google.registry.model.UpdateAutoTimestamp updateTimestamp; + google.registry.persistence.VKey topReference; org.joda.time.DateTime deletionTime; } class google.registry.model.ofy.CommitLogBucket {