diff --git a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java b/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java index 48408fdcb..421575fb6 100644 --- a/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -109,6 +109,7 @@ import org.joda.time.Duration; * A mapreduce that processes batch asynchronous deletions of contact and host resources by mapping * over all domains and checking for any references to the contacts/hosts in pending deletion. */ +@Deprecated @Action( service = Action.Service.BACKEND, path = "/_dr/task/deleteContactsAndHosts", diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 5e8f93fb8..f277ec902 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -16,6 +16,7 @@ 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.isLinked; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -62,7 +63,10 @@ public final class ResourceFlowUtils { private ResourceFlowUtils() {} - /** In {@link #failfastForAsyncDelete}, check this (arbitrary) number of query results. */ + /** + * In {@link #checkLinkedDomains(String, DateTime, Class, Function)}, check this (arbitrary) + * number of query results. + */ private static final int FAILFAST_CHECK_COUNT = 5; /** Check that the given clientId corresponds to the owner of given resource. */ @@ -73,36 +77,54 @@ public final class ResourceFlowUtils { } } - /** Check whether an asynchronous delete would obviously fail, and throw an exception if so. */ - public static void failfastForAsyncDelete( + /** + * Check whether if there are domains linked to the resource to be deleted. Throws an exception if + * so. + * + *

Note that in datastore this is a smoke test as the query for linked domains is eventually + * consistent, so we only check a few domains to fail fast. + */ + public static void checkLinkedDomains( final String targetId, final DateTime now, final Class resourceClass, - final Function> getPotentialReferences) throws EppException { - // Enter a transactionless context briefly. + final Function> getPotentialReferences) + throws EppException { EppException failfastException = - tm().doTransactionless( - () -> { - final ForeignKeyIndex fki = ForeignKeyIndex.load(resourceClass, targetId, now); - if (fki == null) { - return new ResourceDoesNotExistException(resourceClass, targetId); - } - /* Query for the first few linked domains, and if found, actually load them. The - * query is eventually consistent and so might be very stale, but the direct - * load will not be stale, just non-transactional. If we find at least one - * 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 = - getLinkedDomainKeys(fki.getResourceKey(), now, FAILFAST_CHECK_COUNT); + tm().isOfy() + ? tm().doTransactionless( + () -> { + final ForeignKeyIndex fki = + ForeignKeyIndex.load(resourceClass, targetId, now); + if (fki == null) { + return new ResourceDoesNotExistException(resourceClass, targetId); + } + // Query for the first few linked domains, and if found, actually load them. + // The query is eventually consistent and so might be very stale, but the + // direct load will not be stale, just non-transactional. If we find at least + // one 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 = + getLinkedDomainKeys(fki.getResourceKey(), now, FAILFAST_CHECK_COUNT); - VKey resourceVKey = fki.getResourceKey(); - Predicate predicate = - domain -> getPotentialReferences.apply(domain).contains(resourceVKey); - return tm().loadByKeys(keys).values().stream().anyMatch(predicate) - ? new ResourceToDeleteIsReferencedException() - : null; - }); + VKey resourceVKey = fki.getResourceKey(); + Predicate predicate = + domain -> getPotentialReferences.apply(domain).contains(resourceVKey); + return tm().loadByKeys(keys).values().stream().anyMatch(predicate) + ? new ResourceToDeleteIsReferencedException() + : null; + }) + : tm().transact( + () -> { + final ForeignKeyIndex fki = + ForeignKeyIndex.load(resourceClass, targetId, now); + if (fki == null) { + return new ResourceDoesNotExistException(resourceClass, targetId); + } + return isLinked(fki.getResourceKey(), now) + ? new ResourceToDeleteIsReferencedException() + : null; + }); if (failfastException != null) { throw failfastException; } @@ -123,8 +145,7 @@ public final class ResourceFlowUtils { } public static R loadAndVerifyExistence( - Class clazz, String targetId, DateTime now) - throws ResourceDoesNotExistException { + Class clazz, String targetId, DateTime now) throws ResourceDoesNotExistException { return verifyExistence(clazz, targetId, loadByForeignKey(clazz, targetId, now)); } @@ -156,16 +177,16 @@ public final class ResourceFlowUtils { } /** Check that the given AuthInfo is either missing or else is valid for the given resource. */ - public static void verifyOptionalAuthInfo( - Optional authInfo, ContactResource contact) throws EppException { + public static void verifyOptionalAuthInfo(Optional authInfo, ContactResource contact) + throws EppException { if (authInfo.isPresent()) { verifyAuthInfo(authInfo.get(), contact); } } /** Check that the given AuthInfo is either missing or else is valid for the given resource. */ - public static void verifyOptionalAuthInfo( - Optional authInfo, DomainBase domain) throws EppException { + public static void verifyOptionalAuthInfo(Optional authInfo, DomainBase domain) + throws EppException { if (authInfo.isPresent()) { verifyAuthInfo(authInfo.get(), domain); } @@ -229,7 +250,7 @@ public final class ResourceFlowUtils { /** Check that the same values aren't being added and removed in an update command. */ public static void checkSameValuesNotAddedAndRemoved( ImmutableSet fieldsToAdd, ImmutableSet fieldsToRemove) - throws AddRemoveSameValueException { + throws AddRemoveSameValueException { if (!intersection(fieldsToAdd, fieldsToRemove).isEmpty()) { throw new AddRemoveSameValueException(); } 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 21ceb5345..3fe179340 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java @@ -15,12 +15,16 @@ package google.registry.flows.contact; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; -import static google.registry.flows.ResourceFlowUtils.failfastForAsyncDelete; +import static google.registry.flows.ResourceFlowUtils.checkLinkedDomains; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; 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.ResourceTransferUtils.denyPendingTransfer; +import static google.registry.model.ResourceTransferUtils.handlePendingTransferOnDelete; +import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; +import static google.registry.model.transfer.TransferStatus.SERVER_CANCELLED; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -40,7 +44,8 @@ import google.registry.model.eppcommon.AuthInfo; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; import google.registry.model.eppoutput.EppResponse; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.eppoutput.Result.Code; +import google.registry.model.reporting.HistoryEntry.Type; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import java.util.Optional; import javax.inject.Inject; @@ -63,10 +68,11 @@ import org.joda.time.DateTime; @ReportingSpec(ActivityReportField.CONTACT_DELETE) public final class ContactDeleteFlow implements TransactionalFlow { - private static final ImmutableSet DISALLOWED_STATUSES = ImmutableSet.of( - StatusValue.CLIENT_DELETE_PROHIBITED, - StatusValue.PENDING_DELETE, - StatusValue.SERVER_DELETE_PROHIBITED); + private static final ImmutableSet DISALLOWED_STATUSES = + ImmutableSet.of( + StatusValue.CLIENT_DELETE_PROHIBITED, + StatusValue.PENDING_DELETE, + StatusValue.SERVER_DELETE_PROHIBITED); @Inject ExtensionManager extensionManager; @Inject @ClientId String clientId; @@ -77,7 +83,9 @@ public final class ContactDeleteFlow implements TransactionalFlow { @Inject ContactHistory.Builder historyBuilder; @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject EppResponse.Builder responseBuilder; - @Inject ContactDeleteFlow() {} + + @Inject + ContactDeleteFlow() {} @Override public final EppResponse run() throws EppException { @@ -85,23 +93,45 @@ public final class ContactDeleteFlow implements TransactionalFlow { extensionManager.validate(); validateClientIsLoggedIn(clientId); DateTime now = tm().getTransactionTime(); - failfastForAsyncDelete(targetId, now, ContactResource.class, DomainBase::getReferencedContacts); + checkLinkedDomains(targetId, now, ContactResource.class, DomainBase::getReferencedContacts); ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now); verifyNoDisallowedStatuses(existingContact, DISALLOWED_STATUSES); verifyOptionalAuthInfo(authInfo, existingContact); if (!isSuperuser) { verifyResourceOwnership(clientId, existingContact); } - asyncTaskEnqueuer.enqueueAsyncDelete( - existingContact, tm().getTransactionTime(), clientId, trid, isSuperuser); - ContactResource newContact = - existingContact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build(); - historyBuilder - .setType(HistoryEntry.Type.CONTACT_PENDING_DELETE) - .setModificationTime(now) - .setContactBase(newContact); - tm().insert(historyBuilder.build()); + Type historyEntryType; + Code resultCode; + ContactResource newContact; + if (tm().isOfy()) { + asyncTaskEnqueuer.enqueueAsyncDelete( + existingContact, tm().getTransactionTime(), clientId, trid, isSuperuser); + newContact = existingContact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build(); + historyEntryType = Type.CONTACT_PENDING_DELETE; + resultCode = SUCCESS_WITH_ACTION_PENDING; + } else { + // Handle pending transfers on contact deletion. + newContact = + existingContact.getStatusValues().contains(StatusValue.PENDING_TRANSFER) + ? denyPendingTransfer(existingContact, SERVER_CANCELLED, now, clientId) + : existingContact; + // Wipe out PII on contact deletion. + newContact = + newContact.asBuilder().wipeOut().setStatusValues(null).setDeletionTime(now).build(); + historyEntryType = Type.CONTACT_DELETE; + resultCode = SUCCESS; + } + ContactHistory contactHistory = + historyBuilder + .setType(historyEntryType) + .setModificationTime(now) + .setContactBase(newContact) + .build(); + if (!tm().isOfy()) { + handlePendingTransferOnDelete(existingContact, newContact, now, contactHistory); + } + tm().insert(contactHistory); tm().update(newContact); - return responseBuilder.setResultFromCode(SUCCESS_WITH_ACTION_PENDING).build(); + return responseBuilder.setResultFromCode(resultCode).build(); } } 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 f39cd39d7..b276c608a 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -15,7 +15,7 @@ package google.registry.flows.host; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; -import static google.registry.flows.ResourceFlowUtils.failfastForAsyncDelete; +import static google.registry.flows.ResourceFlowUtils.checkLinkedDomains; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses; import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; @@ -65,10 +65,11 @@ import org.joda.time.DateTime; @ReportingSpec(ActivityReportField.HOST_DELETE) public final class HostDeleteFlow implements TransactionalFlow { - private static final ImmutableSet DISALLOWED_STATUSES = ImmutableSet.of( - StatusValue.CLIENT_DELETE_PROHIBITED, - StatusValue.PENDING_DELETE, - StatusValue.SERVER_DELETE_PROHIBITED); + private static final ImmutableSet DISALLOWED_STATUSES = + ImmutableSet.of( + StatusValue.CLIENT_DELETE_PROHIBITED, + StatusValue.PENDING_DELETE, + StatusValue.SERVER_DELETE_PROHIBITED); @Inject ExtensionManager extensionManager; @Inject @ClientId String clientId; @@ -78,7 +79,9 @@ public final class HostDeleteFlow implements TransactionalFlow { @Inject HistoryEntry.Builder historyBuilder; @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject EppResponse.Builder responseBuilder; - @Inject HostDeleteFlow() {} + + @Inject + HostDeleteFlow() {} @Override public final EppResponse run() throws EppException { @@ -87,7 +90,7 @@ public final class HostDeleteFlow implements TransactionalFlow { validateClientIsLoggedIn(clientId); DateTime now = tm().getTransactionTime(); validateHostName(targetId); - failfastForAsyncDelete(targetId, now, HostResource.class, DomainBase::getNameservers); + checkLinkedDomains(targetId, now, HostResource.class, DomainBase::getNameservers); HostResource existingHost = loadAndVerifyExistence(HostResource.class, targetId, now); verifyNoDisallowedStatuses(existingHost, DISALLOWED_STATUSES); if (!isSuperuser) { diff --git a/core/src/test/java/google/registry/flows/EppLifecycleContactTest.java b/core/src/test/java/google/registry/flows/EppLifecycleContactTest.java index c825f90b0..3691c7004 100644 --- a/core/src/test/java/google/registry/flows/EppLifecycleContactTest.java +++ b/core/src/test/java/google/registry/flows/EppLifecycleContactTest.java @@ -18,9 +18,11 @@ import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACK_MESSAGE; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_NO_MESSAGES; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.EppMetricSubject.assertThat; import com.google.common.collect.ImmutableMap; +import google.registry.model.eppoutput.Result; import google.registry.testing.AppEngineExtension; import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestOfyAndSql; @@ -63,14 +65,22 @@ class EppLifecycleContactTest extends EppTestCase { .hasCommandName("ContactInfo") .and() .hasStatus(SUCCESS); - assertThatCommand("contact_delete_sh8013.xml") - .hasResponse("contact_delete_response_sh8013.xml"); + Result.Code resultCode; + if (tm().isOfy()) { + assertThatCommand("contact_delete_sh8013.xml") + .hasResponse("contact_delete_response_sh8013_pending.xml"); + resultCode = SUCCESS_WITH_ACTION_PENDING; + } else { + assertThatCommand("contact_delete_sh8013.xml") + .hasResponse("contact_delete_response_sh8013.xml"); + resultCode = SUCCESS; + } assertThat(getRecordedEppMetric()) .hasClientId("NewRegistrar") .and() .hasCommandName("ContactDelete") .and() - .hasStatus(SUCCESS_WITH_ACTION_PENDING); + .hasStatus(resultCode); assertThatLogoutSucceeds(); } 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 46ee7b059..81e43159e 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -14,19 +14,26 @@ package google.registry.flows.contact; +import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; +import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; +import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_PENDING_DELETE; import static google.registry.testing.ContactResourceSubject.assertAboutContacts; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.getPollMessages; import static google.registry.testing.DatabaseHelper.newContactResource; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveContact; +import static google.registry.testing.DatabaseHelper.persistContactWithPendingTransfer; import static google.registry.testing.DatabaseHelper.persistDeletedContact; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions; +import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import google.registry.flows.EppException; import google.registry.flows.ResourceFlowTestCase; import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException; @@ -36,10 +43,19 @@ import google.registry.flows.exceptions.ResourceToDeleteIsReferencedException; 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.model.poll.PendingActionNotificationResponse; +import google.registry.model.poll.PollMessage; +import google.registry.model.registry.Registry; +import google.registry.model.reporting.HistoryEntry.Type; +import google.registry.model.transfer.TransferData; +import google.registry.model.transfer.TransferResponse; +import google.registry.model.transfer.TransferStatus; import google.registry.testing.DualDatabaseTest; import google.registry.testing.ReplayExtension; import google.registry.testing.TestOfyAndSql; +import google.registry.testing.TestOfyOnly; +import google.registry.testing.TestSqlOnly; +import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.extension.RegisterExtension; @@ -57,18 +73,24 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase exception) throws Exception { persistResource( - newContactResource(getUniqueIdFromCommand()).asBuilder() + newContactResource(getUniqueIdFromCommand()) + .asBuilder() .setStatusValues(ImmutableSet.of(statusValue)) .build()); EppException thrown = assertThrows(exception, this::runFlow); @@ -153,13 +290,13 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase - - Command completed successfully; action pending + + Command completed successfully ABC-12345 diff --git a/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid.xml b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid.xml index 7d1721ff9..32dd8b167 100644 --- a/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid.xml +++ b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid.xml @@ -1,7 +1,7 @@ - - Command completed successfully; action pending + + Command completed successfully server-trid diff --git a/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid_pending.xml b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid_pending.xml new file mode 100644 index 000000000..7d1721ff9 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid_pending.xml @@ -0,0 +1,10 @@ + + + + Command completed successfully; action pending + + + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/contact/contact_delete_response_pending.xml b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_pending.xml new file mode 100644 index 000000000..91fe71a83 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/contact/contact_delete_response_pending.xml @@ -0,0 +1,11 @@ + + + + Command completed successfully; action pending + + + ABC-12345 + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013.xml b/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013.xml index 91fe71a83..ab613d34f 100644 --- a/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013.xml +++ b/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013.xml @@ -1,7 +1,7 @@ - - Command completed successfully; action pending + + Command completed successfully ABC-12345 diff --git a/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013_pending.xml b/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013_pending.xml new file mode 100644 index 000000000..91fe71a83 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/contact_delete_response_sh8013_pending.xml @@ -0,0 +1,11 @@ + + + + Command completed successfully; action pending + + + ABC-12345 + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/session/contact_delete_response_sh8013_pending.xml b/core/src/test/resources/google/registry/flows/session/contact_delete_response_sh8013_pending.xml new file mode 100644 index 000000000..91fe71a83 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/session/contact_delete_response_sh8013_pending.xml @@ -0,0 +1,11 @@ + + + + Command completed successfully; action pending + + + ABC-12345 + server-trid + + +