Do not cancel pending transfers unless there is one to cancel

A previous CL inadvertently caused the system to always set the transfer status to SERVER_CANCELLED when deleting a resource, even if there was no transfer. This led to RDE problems.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=140890919

java/google/registry/flows/ResourceFlowUtils.java

@@ -14,6 +14,7 @@
 
 package google.registry.flows;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.tryFind;
@@ -214,6 +215,7 @@ public final class ResourceFlowUtils {
    */
   public static TransferData createResolvedTransferData(
       TransferData oldTransferData, TransferStatus transferStatus, DateTime now) {
+    checkArgument(!oldTransferData.equals(TransferData.EMPTY), "No old transfer to resolve.");
     return oldTransferData.asBuilder()
         .setExtendedRegistrationYears(null)
         .setServerApproveEntities(null)
@@ -238,6 +240,9 @@ public final class ResourceFlowUtils {
       R extends EppResource & ResourceWithTransferData,
       B extends EppResource.Builder<R, B> & BuilderWithTransferData<B>> B resolvePendingTransfer(
           R resource, TransferStatus transferStatus, DateTime now) {
+    checkState(
+        resource.getStatusValues().contains(StatusValue.PENDING_TRANSFER),
+        "Resource is not in pending transfer status.");
     return ((B) resource.asBuilder())
         .removeStatusValue(StatusValue.PENDING_TRANSFER)
         .setTransferData(

java/google/registry/flows/async/DeleteContactsAndHostsAction.java

@@ -63,6 +63,7 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.ExternalMessagingName;
 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.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
@@ -317,10 +318,12 @@ public class DeleteContactsAndHostsAction implements Runnable {
         EppResource.Builder<?, ?> resourceToSaveBuilder;
         if (resource instanceof ContactResource) {
           ContactResource contact = (ContactResource) resource;
-          resourceToSaveBuilder = contact.asBuilder()
+          ContactResource.Builder contactToSaveBuilder = contact.asBuilder();
-              .setTransferData(createResolvedTransferData(
+          if (contact.getStatusValues().contains(StatusValue.PENDING_TRANSFER)) {
-                  contact.getTransferData(), TransferStatus.SERVER_CANCELLED, now))
+            contactToSaveBuilder = contactToSaveBuilder.setTransferData(createResolvedTransferData(
-              .wipeOut();
+                contact.getTransferData(), TransferStatus.SERVER_CANCELLED, now));
+          }
+          resourceToSaveBuilder = contactToSaveBuilder.wipeOut();
         } else {
           resourceToSaveBuilder = resource.asBuilder();
         }

java/google/registry/flows/domain/DomainDeleteFlow.java

@@ -131,9 +131,10 @@ public final class DomainDeleteFlow implements TransactionalFlow {
         AfterValidationParameters.newBuilder().setExistingDomain(existingDomain).build());
     ImmutableSet.Builder<ImmutableObject> entitiesToSave = new ImmutableSet.Builder<>();
     HistoryEntry historyEntry = buildHistoryEntry(existingDomain, now);
-    Builder builder =
+    Builder builder = existingDomain.getStatusValues().contains(StatusValue.PENDING_TRANSFER)
-        ResourceFlowUtils.<DomainResource, DomainResource.Builder>resolvePendingTransfer(
+        ? ResourceFlowUtils.<DomainResource, DomainResource.Builder>resolvePendingTransfer(
-            existingDomain, TransferStatus.SERVER_CANCELLED, now);
+            existingDomain, TransferStatus.SERVER_CANCELLED, now)
+        : existingDomain.asBuilder();
     builder.setDeletionTime(now).setStatusValues(null);
     // If the domain is in the Add Grace Period, we delete it immediately, which is already
     // reflected in the builder we just prepared. Otherwise we give it a PENDING_DELETE status.

javatests/google/registry/flows/async/DeleteContactsAndHostsActionTest.java

@@ -75,6 +75,7 @@ import google.registry.model.poll.PollMessage;
 import google.registry.model.poll.PollMessage.OneTime;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
@@ -196,6 +197,16 @@ public class DeleteContactsAndHostsActionTest
     assertNoTasksEnqueued(QUEUE_ASYNC_DELETE);
   }
 
+  @Test
+  public void testSuccess_contactWithoutPendingTransfer_isDeletedAndHasNoTransferData()
+      throws Exception {
+    ContactResource contact = persistContactPendingDelete("blah8221");
+    enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false);
+    runMapreduce();
+    ContactResource contactAfterDeletion = ofy().load().entity(contact).now();
+    assertThat(contactAfterDeletion.getTransferData()).isEqualTo(TransferData.EMPTY);
+  }
+
   @Test
   public void testSuccess_contactWithPendingTransfer_getsDeleted() throws Exception {
     DateTime transferRequestTime = clock.nowUtc().minusDays(3);

javatests/google/registry/flows/domain/DomainDeleteFlowTest.java

@@ -480,6 +480,16 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
     runFlowAssertResponse(readFile("domain_delete_response_autorenew_fee.xml", FEE_12_MAP));
   }
 
+  @Test
+  public void testSuccess_noPendingTransfer_deletedAndHasNoTransferData() throws Exception {
+    setClientIdForFlow("TheRegistrar");
+    setupSuccessfulTest();
+    clock.advanceOneMilli();
+    runFlowAssertResponse(readFile("domain_delete_response_pending.xml"));
+    DomainResource domain = reloadResourceByForeignKey();
+    assertThat(domain.getTransferData()).isEqualTo(TransferData.EMPTY);
+  }
+
   @Test
   public void testSuccess_pendingTransfer() throws Exception {
     setClientIdForFlow("TheRegistrar");