diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 9a0996d1f..2ea750f3d 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -15,8 +15,8 @@ package google.registry.flows; import static com.google.common.collect.Sets.intersection; +import static google.registry.model.EppResourceUtils.getLinkedDomainKeys; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.model.EppResourceUtils.queryForLinkedDomains; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -94,14 +94,13 @@ public final class ResourceFlowUtils { * actual reference then we can reliably fail. If we don't find any, we can't * trust the query and need to do the full mapreduce. */ - Iterable> keys = - queryForLinkedDomains(fki.getResourceKey().getOfyKey(), now) - .limit(FAILFAST_CHECK_COUNT) - .keys(); + Iterable> keys = + getLinkedDomainKeys(fki.getResourceKey(), now, FAILFAST_CHECK_COUNT); + VKey resourceVKey = fki.getResourceKey(); Predicate predicate = domain -> getPotentialReferences.apply(domain).contains(resourceVKey); - return ofy().load().keys(keys).values().stream().anyMatch(predicate) + return tm().loadByKeys(keys).values().stream().anyMatch(predicate) ? new ResourceToDeleteIsReferencedException() : null; }); diff --git a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java index b8f6a2953..bc686f185 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java @@ -21,7 +21,6 @@ import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -101,7 +100,8 @@ public final class ContactDeleteFlow implements TransactionalFlow { .setType(HistoryEntry.Type.CONTACT_PENDING_DELETE) .setModificationTime(now) .setParent(Key.create(existingContact)); - ofy().save().entities(newContact, historyBuilder.build()); + tm().insert(historyBuilder.build().toChildHistoryEntity()); + tm().update(newContact); return responseBuilder.setResultFromCode(SUCCESS_WITH_ACTION_PENDING).build(); } } diff --git a/core/src/main/java/google/registry/flows/contact/ContactInfoFlow.java b/core/src/main/java/google/registry/flows/contact/ContactInfoFlow.java index 9a3e43299..2364564b3 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactInfoFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactInfoFlow.java @@ -20,7 +20,6 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.model.EppResourceUtils.isLinked; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.Flow; @@ -77,7 +76,7 @@ public final class ContactInfoFlow implements Flow { clientId.equals(contact.getCurrentSponsorClientId()) || authInfo.isPresent(); ImmutableSet.Builder statusValues = new ImmutableSet.Builder<>(); statusValues.addAll(contact.getStatusValues()); - if (isLinked(Key.create(contact), now)) { + if (isLinked(contact.createVKey(), now)) { statusValues.add(StatusValue.LINKED); } return responseBuilder 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 cc8d52bc1..f39cd39d7 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -21,7 +21,6 @@ import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.flows.host.HostFlowUtils.validateHostName; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -108,7 +107,8 @@ public final class HostDeleteFlow implements TransactionalFlow { .setType(HistoryEntry.Type.HOST_PENDING_DELETE) .setModificationTime(now) .setParent(Key.create(existingHost)); - ofy().save().entities(newHost, historyBuilder.build()); + tm().insert(historyBuilder.build().toChildHistoryEntity()); + tm().update(newHost); return responseBuilder.setResultFromCode(SUCCESS_WITH_ACTION_PENDING).build(); } } diff --git a/core/src/main/java/google/registry/flows/host/HostInfoFlow.java b/core/src/main/java/google/registry/flows/host/HostInfoFlow.java index 3442b25a2..809b5e83c 100644 --- a/core/src/main/java/google/registry/flows/host/HostInfoFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostInfoFlow.java @@ -21,7 +21,6 @@ import static google.registry.model.EppResourceUtils.isLinked; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.Flow; @@ -68,7 +67,7 @@ public final class HostInfoFlow implements Flow { HostResource host = loadAndVerifyExistence(HostResource.class, targetId, now); ImmutableSet.Builder statusValues = new ImmutableSet.Builder<>(); statusValues.addAll(host.getStatusValues()); - if (isLinked(Key.create(host), now)) { + if (isLinked(host.createVKey(), now)) { statusValues.add(StatusValue.LINKED); } HostInfoData.Builder hostInfoDataBuilder = HostInfoData.newBuilder(); diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index f6692182f..7995cf260 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.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.util.DateTimeUtils.isAtOrAfter; @@ -28,7 +29,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.googlecode.objectify.Key; import com.googlecode.objectify.Result; -import com.googlecode.objectify.cmd.Query; import com.googlecode.objectify.util.ResultNow; import google.registry.config.RegistryConfig; import google.registry.model.EppResource.BuilderWithTransferData; @@ -37,6 +37,7 @@ import google.registry.model.EppResource.ResourceWithTransferData; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.StatusValue; +import google.registry.model.host.HostResource; import google.registry.model.index.ForeignKeyIndex; import google.registry.model.ofy.CommitLogManifest; import google.registry.model.ofy.CommitLogMutation; @@ -44,11 +45,13 @@ import google.registry.model.registry.Registry; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; +import google.registry.persistence.VKey; import java.util.List; import java.util.Map.Entry; import java.util.Optional; import java.util.function.Function; import javax.annotation.Nullable; +import javax.persistence.Query; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -57,6 +60,22 @@ public final class EppResourceUtils { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final String CONTACT_LINKED_DOMAIN_QUERY = + "SELECT repoId FROM Domain " + + "WHERE (adminContact = :fkRepoId " + + "OR billingContact = :fkRepoId " + + "OR techContact = :fkRepoId " + + "OR registrantContact = :fkRepoId) " + + "AND deletionTime > :now"; + + // We have to use the native SQL query here because DomainHost table doesn't have its entity + // class so we cannot reference its property like domainHost.hostRepoId in a JPQL query. + private static final String HOST_LINKED_DOMAIN_QUERY = + "SELECT d.repo_id FROM \"Domain\" d " + + "JOIN \"DomainHost\" dh ON dh.domain_repo_id = d.repo_id " + + "WHERE d.deletion_time > :now " + + "AND dh.host_repo_id = :fkRepoId"; + /** Returns the full domain repoId in the format HEX-TLD for the specified long id and tld. */ public static String createDomainRepoId(long repoId, String tld) { return createRepoId(repoId, Registry.get(tld).getRoidSuffix()); @@ -365,21 +384,63 @@ public final class EppResourceUtils { } /** - * Returns a query for domains or applications that reference a specified contact or host. + * Returns a set of {@link VKey} for domains that reference a specified contact or host. * - *

This is an eventually consistent query. + *

This is an eventually consistent query if used for Datastore. * * @param key the referent key * @param now the logical time of the check + * @param limit the maximum number of returned keys */ - public static Query queryForLinkedDomains( - Key key, DateTime now) { - boolean isContactKey = key.getKind().equals(Key.getKind(ContactResource.class)); - return ofy() - .load() - .type(DomainBase.class) - .filter(isContactKey ? "allContacts.contact" : "nsHosts", key) - .filter("deletionTime >", now); + public static ImmutableSet> getLinkedDomainKeys( + VKey key, DateTime now, int limit) { + checkArgument( + key.getKind().equals(ContactResource.class) || key.getKind().equals(HostResource.class), + "key must be either VKey or VKey, but it is %s", + key); + boolean isContactKey = key.getKind().equals(ContactResource.class); + if (tm().isOfy()) { + return ofy() + .load() + .type(DomainBase.class) + .filter(isContactKey ? "allContacts.contact" : "nsHosts", key.getOfyKey()) + .filter("deletionTime >", now) + .limit(limit) + .keys() + .list() + .stream() + .map(DomainBase::createVKey) + .collect(toImmutableSet()); + } else { + return tm().transact( + () -> { + Query query; + if (isContactKey) { + query = + jpaTm() + .getEntityManager() + .createQuery(CONTACT_LINKED_DOMAIN_QUERY, String.class) + .setParameter("fkRepoId", key) + .setParameter("now", now); + } else { + query = + jpaTm() + .getEntityManager() + .createNativeQuery(HOST_LINKED_DOMAIN_QUERY) + .setParameter("fkRepoId", key.getSqlKey()) + .setParameter("now", now.toDate()); + } + return (ImmutableSet>) + query + .setMaxResults(limit) + .getResultStream() + .map( + repoId -> + DomainBase.createVKey( + Key.create(DomainBase.class, (String) repoId))) + .collect(toImmutableSet()); + }); + } } /** @@ -390,8 +451,8 @@ public final class EppResourceUtils { * @param key the referent key * @param now the logical time of the check */ - public static boolean isLinked(Key key, DateTime now) { - return queryForLinkedDomains(key, now).limit(1).count() > 0; + public static boolean isLinked(VKey key, DateTime now) { + return getLinkedDomainKeys(key, now, 1).size() > 0; } private EppResourceUtils() {} diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index 6691588cf..8bdeb1589 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -425,7 +425,7 @@ public class RdapJsonFormatter { if (outputDataType == OutputDataType.FULL) { ImmutableSet.Builder statuses = new ImmutableSet.Builder<>(); statuses.addAll(hostResource.getStatusValues()); - if (isLinked(Key.create(hostResource), getRequestTime())) { + if (isLinked(hostResource.createVKey(), getRequestTime())) { statuses.add(StatusValue.LINKED); } if (hostResource.isSubordinate() @@ -562,7 +562,7 @@ public class RdapJsonFormatter { .statusBuilder() .addAll( makeStatusValueList( - isLinked(Key.create(contactResource), getRequestTime()) + isLinked(contactResource.createVKey(), getRequestTime()) ? union(contactResource.getStatusValues(), StatusValue.LINKED) : contactResource.getStatusValues(), false, diff --git a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java index ee147ad46..bec6d1198 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -37,10 +37,12 @@ import google.registry.model.contact.ContactResource; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; import google.registry.model.reporting.HistoryEntry; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyAndSql; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link ContactDeleteFlow}. */ +@DualDatabaseTest class ContactDeleteFlowTest extends ResourceFlowTestCase { @BeforeEach @@ -48,13 +50,13 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase { ContactInfoFlowTest() { @@ -94,7 +96,7 @@ class ContactInfoFlowTest extends ResourceFlowTestCase { @BeforeEach @@ -58,13 +60,13 @@ class HostDeleteFlowTest extends ResourceFlowTestCase