Perform synchronous contact delete in SQL (#1137)

In SQL the contact of a domain is an indexed field and therefore we can
find linked domains synchronously, without the need for MapReduce.

The delete logic is mostly lifted from DeleteContactsAndHostsAction, but
because everything happens in a transaction we do not need to recheck a
lot of the preconditions that were necessary to ensure that the async
delete request still meets the conditions that when the request was
enqueued.

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",

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,26 +77,33 @@ public final class ResourceFlowUtils {
     }
   }
 
-  /** Check whether an asynchronous delete would obviously fail, and throw an exception if so. */
+  /**
-  public static <R extends EppResource> void failfastForAsyncDelete(
+   * Check whether if there are domains linked to the resource to be deleted. Throws an exception if
+   * so.
+   *
+   * <p>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 <R extends EppResource> void checkLinkedDomains(
       final String targetId,
       final DateTime now,
       final Class<R> resourceClass,
-      final Function<DomainBase, ImmutableSet<?>> getPotentialReferences) throws EppException {
+      final Function<DomainBase, ImmutableSet<?>> getPotentialReferences)
-    // Enter a transactionless context briefly.
+      throws EppException {
     EppException failfastException =
-        tm().doTransactionless(
+        tm().isOfy()
+            ? tm().doTransactionless(
                     () -> {
-                  final ForeignKeyIndex<R> fki = ForeignKeyIndex.load(resourceClass, targetId, now);
+                      final ForeignKeyIndex<R> 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 for the first few linked domains, and if found, actually load them.
-                   * query is eventually consistent and so might be very stale, but the direct
+                      // The query is eventually consistent and so might be very stale, but the
-                   * load will not be stale, just non-transactional. If we find at least one
+                      // direct load will not be stale, just non-transactional. If we find at least
-                   * actual reference then we can reliably fail. If we don't find any, we can't
+                      // one  actual reference then we can reliably fail. If we don't find any,
-                   * trust the query and need to do the full mapreduce.
+                      // we can't  trust the query and need to do the full mapreduce.
-                   */
                       Iterable<VKey<DomainBase>> keys =
                           getLinkedDomainKeys(fki.getResourceKey(), now, FAILFAST_CHECK_COUNT);
 
@@ -102,6 +113,17 @@ public final class ResourceFlowUtils {
                       return tm().loadByKeys(keys).values().stream().anyMatch(predicate)
                           ? new ResourceToDeleteIsReferencedException()
                           : null;
+                    })
+            : tm().transact(
+                    () -> {
+                      final ForeignKeyIndex<R> 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 extends EppResource & ForeignKeyedEppResource> R loadAndVerifyExistence(
-      Class<R> clazz, String targetId, DateTime now)
+      Class<R> clazz, String targetId, DateTime now) throws ResourceDoesNotExistException {
-          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(
+  public static void verifyOptionalAuthInfo(Optional<AuthInfo> authInfo, ContactResource contact)
-      Optional<AuthInfo> authInfo, ContactResource contact) throws EppException {
+      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(
+  public static void verifyOptionalAuthInfo(Optional<AuthInfo> authInfo, DomainBase domain)
-      Optional<AuthInfo> authInfo, DomainBase domain) throws EppException {
+      throws EppException {
     if (authInfo.isPresent()) {
       verifyAuthInfo(authInfo.get(), domain);
     }

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,7 +68,8 @@ import org.joda.time.DateTime;
 @ReportingSpec(ActivityReportField.CONTACT_DELETE)
 public final class ContactDeleteFlow implements TransactionalFlow {
 
-  private static final ImmutableSet<StatusValue> DISALLOWED_STATUSES = ImmutableSet.of(
+  private static final ImmutableSet<StatusValue> DISALLOWED_STATUSES =
+      ImmutableSet.of(
           StatusValue.CLIENT_DELETE_PROHIBITED,
           StatusValue.PENDING_DELETE,
           StatusValue.SERVER_DELETE_PROHIBITED);
@@ -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);
     }
+    Type historyEntryType;
+    Code resultCode;
+    ContactResource newContact;
+    if (tm().isOfy()) {
       asyncTaskEnqueuer.enqueueAsyncDelete(
           existingContact, tm().getTransactionTime(), clientId, trid, isSuperuser);
-    ContactResource newContact =
+      newContact = existingContact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build();
-        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(HistoryEntry.Type.CONTACT_PENDING_DELETE)
+            .setType(historyEntryType)
             .setModificationTime(now)
-        .setContactBase(newContact);
+            .setContactBase(newContact)
-    tm().insert(historyBuilder.build());
+            .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();
   }
 }

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,7 +65,8 @@ import org.joda.time.DateTime;
 @ReportingSpec(ActivityReportField.HOST_DELETE)
 public final class HostDeleteFlow implements TransactionalFlow {
 
-  private static final ImmutableSet<StatusValue> DISALLOWED_STATUSES = ImmutableSet.of(
+  private static final ImmutableSet<StatusValue> DISALLOWED_STATUSES =
+      ImmutableSet.of(
           StatusValue.CLIENT_DELETE_PROHIBITED,
           StatusValue.PENDING_DELETE,
           StatusValue.SERVER_DELETE_PROHIBITED);
@@ -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) {

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);
+    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();
   }
 

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<ContactDeleteFlow, Cont
     setEppInput("contact_delete.xml");
   }
 
-  @TestOfyAndSql
+  @TestOfyOnly
-  void testDryRun() throws Exception {
+  void testDryRun_ofy() throws Exception {
+    persistActiveContact(getUniqueIdFromCommand());
+    dryRunFlowAssertResponse(loadFile("contact_delete_response_pending.xml"));
+  }
+
+  @TestSqlOnly
+  void testDryRun_sql() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     dryRunFlowAssertResponse(loadFile("contact_delete_response.xml"));
   }
 
-  @TestOfyAndSql
+  @TestOfyOnly
-  void testSuccess() throws Exception {
+  void testSuccess_ofy() throws Exception {
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
     assertTransactionalFlow(true);
-    runFlowAssertResponse(loadFile("contact_delete_response.xml"));
+    runFlowAssertResponse(loadFile("contact_delete_response_pending.xml"));
     ContactResource deletedContact = reloadResourceByForeignKey();
     assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE);
     assertAsyncDeletionTaskEnqueued(
@@ -76,18 +98,104 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, Cont
     assertAboutContacts()
         .that(deletedContact)
         .hasOnlyOneHistoryEntryWhich()
-        .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE);
+        .hasType(Type.CONTACT_PENDING_DELETE);
     assertNoBillingEvents();
     assertLastHistoryContainsResource(deletedContact);
   }
 
-  @TestOfyAndSql
+  @TestSqlOnly
-  void testSuccess_clTridNotSpecified() throws Exception {
+  void testSuccess_sql() throws Exception {
+    persistActiveContact(getUniqueIdFromCommand());
+    clock.advanceOneMilli();
+    assertTransactionalFlow(true);
+    runFlowAssertResponse(loadFile("contact_delete_response.xml"));
+    assertThat(reloadResourceByForeignKey()).isNull();
+    assertAboutContacts()
+        .that(reloadResourceByForeignKey(clock.nowUtc().minusMillis(1)))
+        .isNotActiveAt(clock.nowUtc())
+        .and()
+        .hasNullLocalizedPostalInfo()
+        .and()
+        .hasNullInternationalizedPostalInfo()
+        .and()
+        .hasNullEmailAddress()
+        .and()
+        .hasNullVoiceNumber()
+        .and()
+        .hasNullFaxNumber()
+        .and()
+        .hasOnlyOneHistoryEntryWhich()
+        .hasType(Type.CONTACT_DELETE);
+    assertNoTasksEnqueued(QUEUE_ASYNC_DELETE);
+    assertNoBillingEvents();
+  }
+
+  @TestSqlOnly
+  void testSuccess_pendingTransfer_sql() throws Exception {
+    DateTime transferRequestTime = clock.nowUtc().minusDays(3);
+    TransferData oldTransferData =
+        persistContactWithPendingTransfer(
+                persistActiveContact(getUniqueIdFromCommand()),
+                transferRequestTime,
+                transferRequestTime.plus(Registry.DEFAULT_TRANSFER_GRACE_PERIOD),
+                clock.nowUtc())
+            .getTransferData();
+    clock.advanceOneMilli();
+    assertTransactionalFlow(true);
+    runFlowAssertResponse(loadFile("contact_delete_response.xml"));
+    assertThat(reloadResourceByForeignKey()).isNull();
+    ContactResource softDeletedContact = reloadResourceByForeignKey(clock.nowUtc().minusMillis(1));
+    assertAboutContacts()
+        .that(softDeletedContact)
+        .isNotActiveAt(clock.nowUtc())
+        .and()
+        .hasNullLocalizedPostalInfo()
+        .and()
+        .hasNullInternationalizedPostalInfo()
+        .and()
+        .hasNullEmailAddress()
+        .and()
+        .hasNullVoiceNumber()
+        .and()
+        .hasNullFaxNumber()
+        .and()
+        .hasOneHistoryEntryEachOfTypes(Type.CONTACT_DELETE, Type.CONTACT_TRANSFER_REQUEST);
+    assertThat(softDeletedContact.getTransferData())
+        .isEqualTo(
+            oldTransferData
+                .copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.SERVER_CANCELLED)
+                .setPendingTransferExpirationTime(softDeletedContact.getDeletionTime())
+                .build());
+    PollMessage gainingPollMessage =
+        Iterables.getOnlyElement(getPollMessages("NewRegistrar", clock.nowUtc()));
+    assertThat(gainingPollMessage.getEventTime()).isEqualTo(clock.nowUtc());
+    assertThat(
+            gainingPollMessage.getResponseData().stream()
+                .filter(TransferResponse.class::isInstance)
+                .map(TransferResponse.class::cast)
+                .collect(onlyElement())
+                .getTransferStatus())
+        .isEqualTo(TransferStatus.SERVER_CANCELLED);
+    PendingActionNotificationResponse panData =
+        gainingPollMessage.getResponseData().stream()
+            .filter(PendingActionNotificationResponse.class::isInstance)
+            .map(PendingActionNotificationResponse.class::cast)
+            .collect(onlyElement());
+    assertThat(panData.getTrid())
+        .isEqualTo(Trid.create("transferClient-trid", "transferServer-trid"));
+    assertThat(panData.getActionResult()).isFalse();
+    assertNoTasksEnqueued(QUEUE_ASYNC_DELETE);
+    assertNoBillingEvents();
+  }
+
+  @TestOfyOnly
+  void testSuccess_clTridNotSpecified_ofy() throws Exception {
     setEppInput("contact_delete_no_cltrid.xml");
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
     assertTransactionalFlow(true);
-    runFlowAssertResponse(loadFile("contact_delete_response_no_cltrid.xml"));
+    runFlowAssertResponse(loadFile("contact_delete_response_no_cltrid_pending.xml"));
     ContactResource deletedContact = reloadResourceByForeignKey();
     assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE);
     assertAsyncDeletionTaskEnqueued(
@@ -95,7 +203,35 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, Cont
     assertAboutContacts()
         .that(deletedContact)
         .hasOnlyOneHistoryEntryWhich()
-        .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE);
+        .hasType(CONTACT_PENDING_DELETE);
+    assertNoBillingEvents();
+  }
+
+  @TestSqlOnly
+  void testSuccess_clTridNotSpecified_sql() throws Exception {
+    setEppInput("contact_delete_no_cltrid.xml");
+    persistActiveContact(getUniqueIdFromCommand());
+    clock.advanceOneMilli();
+    assertTransactionalFlow(true);
+    runFlowAssertResponse(loadFile("contact_delete_response_no_cltrid.xml"));
+    assertThat(reloadResourceByForeignKey()).isNull();
+    assertAboutContacts()
+        .that(reloadResourceByForeignKey(clock.nowUtc().minusMillis(1)))
+        .isNotActiveAt(clock.nowUtc())
+        .and()
+        .hasNullLocalizedPostalInfo()
+        .and()
+        .hasNullInternationalizedPostalInfo()
+        .and()
+        .hasNullEmailAddress()
+        .and()
+        .hasNullVoiceNumber()
+        .and()
+        .hasNullFaxNumber()
+        .and()
+        .hasOnlyOneHistoryEntryWhich()
+        .hasType(Type.CONTACT_DELETE);
+    assertNoTasksEnqueued(QUEUE_ASYNC_DELETE);
     assertNoBillingEvents();
   }
 
@@ -137,7 +273,8 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, Cont
   private void doFailingStatusTest(StatusValue statusValue, Class<? extends EppException> 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<ContactDeleteFlow, Cont
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @TestOfyAndSql
+  @TestOfyOnly
-  void testSuccess_superuserUnauthorizedClient() throws Exception {
+  void testSuccess_superuserUnauthorizedClient_ofy() throws Exception {
     sessionMetadata.setClientId("NewRegistrar");
     persistActiveContact(getUniqueIdFromCommand());
     clock.advanceOneMilli();
     runFlowAssertResponse(
-        CommitMode.LIVE, UserPrivileges.SUPERUSER, loadFile("contact_delete_response.xml"));
+        CommitMode.LIVE, UserPrivileges.SUPERUSER, loadFile("contact_delete_response_pending.xml"));
     ContactResource deletedContact = reloadResourceByForeignKey();
     assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE);
     assertAsyncDeletionTaskEnqueued(
@@ -167,15 +304,42 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, Cont
     assertAboutContacts()
         .that(deletedContact)
         .hasOnlyOneHistoryEntryWhich()
-        .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE);
+        .hasType(CONTACT_PENDING_DELETE);
+    assertNoBillingEvents();
+  }
+
+  @TestSqlOnly
+  void testSuccess_superuserUnauthorizedClient_sql() throws Exception {
+    sessionMetadata.setClientId("NewRegistrar");
+    persistActiveContact(getUniqueIdFromCommand());
+    clock.advanceOneMilli();
+    runFlowAssertResponse(
+        CommitMode.LIVE, UserPrivileges.SUPERUSER, loadFile("contact_delete_response.xml"));
+    assertThat(reloadResourceByForeignKey()).isNull();
+    assertAboutContacts()
+        .that(reloadResourceByForeignKey(clock.nowUtc().minusMillis(1)))
+        .isNotActiveAt(clock.nowUtc())
+        .and()
+        .hasNullLocalizedPostalInfo()
+        .and()
+        .hasNullInternationalizedPostalInfo()
+        .and()
+        .hasNullEmailAddress()
+        .and()
+        .hasNullVoiceNumber()
+        .and()
+        .hasNullFaxNumber()
+        .and()
+        .hasOnlyOneHistoryEntryWhich()
+        .hasType(Type.CONTACT_DELETE);
+    assertNoTasksEnqueued(QUEUE_ASYNC_DELETE);
     assertNoBillingEvents();
   }
 
   @TestOfyAndSql
   void testFailure_failfastWhenLinkedToDomain() throws Exception {
     createTld("tld");
-    persistResource(
+    persistResource(newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
-        newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
     EppException thrown = assertThrows(ResourceToDeleteIsReferencedException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
@@ -183,8 +347,7 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase<ContactDeleteFlow, Cont
   @TestOfyAndSql
   void testFailure_failfastWhenLinkedToApplication() throws Exception {
     createTld("tld");
-    persistResource(
+    persistResource(newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
-        newDomainBase("example.tld", persistActiveContact(getUniqueIdFromCommand())));
     EppException thrown = assertThrows(ResourceToDeleteIsReferencedException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }

core/src/test/resources/google/registry/flows/contact/contact_delete_response.xml

@@ -1,7 +1,7 @@
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
   <response>
-    <result code="1001">
+    <result code="1000">
-      <msg>Command completed successfully; action pending</msg>
+      <msg>Command completed successfully</msg>
     </result>
     <trID>
       <clTRID>ABC-12345</clTRID>

core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid.xml

@@ -1,7 +1,7 @@
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
   <response>
-    <result code="1001">
+    <result code="1000">
-      <msg>Command completed successfully; action pending</msg>
+      <msg>Command completed successfully</msg>
     </result>
     <trID>
       <svTRID>server-trid</svTRID>

core/src/test/resources/google/registry/flows/contact/contact_delete_response_no_cltrid_pending.xml

@@ -0,0 +1,10 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1001">
+      <msg>Command completed successfully; action pending</msg>
+    </result>
+    <trID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>

core/src/test/resources/google/registry/flows/contact/contact_delete_response_pending.xml

@@ -0,0 +1,11 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1001">
+      <msg>Command completed successfully; action pending</msg>
+    </result>
+    <trID>
+      <clTRID>ABC-12345</clTRID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>

core/src/test/resources/google/registry/flows/contact_delete_response_sh8013.xml

@@ -1,7 +1,7 @@
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
   <response>
-    <result code="1001">
+    <result code="1000">
-      <msg>Command completed successfully; action pending</msg>
+      <msg>Command completed successfully</msg>
     </result>
     <trID>
       <clTRID>ABC-12345</clTRID>

core/src/test/resources/google/registry/flows/contact_delete_response_sh8013_pending.xml

@@ -0,0 +1,11 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1001">
+      <msg>Command completed successfully; action pending</msg>
+    </result>
+    <trID>
+      <clTRID>ABC-12345</clTRID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>

core/src/test/resources/google/registry/flows/session/contact_delete_response_sh8013_pending.xml

@@ -0,0 +1,11 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1001">
+      <msg>Command completed successfully; action pending</msg>
+    </result>
+    <trID>
+      <clTRID>ABC-12345</clTRID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>