From 5047d568de1369e563c870cfaa7b0ed32dc23d61 Mon Sep 17 00:00:00 2001 From: larryruili Date: Thu, 13 Apr 2017 11:32:11 -0700 Subject: [PATCH] Notify registrars of async contact/host deletions We now send PendingActionNotificationResponses in our poll messages upon completion of an asynchronous contact or host deletion. This is part 1 of 2, which begins logging Trid in all enqueued Host/Contact deletion flows for use in batch deletions, and optionally consuming the resultant Trid info to emit a Host/ContactPendingActionNotifcationResponse. Part 2 will make this response emission non-optional, which will happen once the queue is cleared of all non-Trid containing tasks. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=153084197 --- .../batch/DeleteContactsAndHostsAction.java | 62 +++++++- .../flows/async/AsyncFlowEnqueuer.java | 10 +- .../flows/contact/ContactDeleteFlow.java | 4 +- .../registry/flows/host/HostDeleteFlow.java | 4 +- .../registry/model/eppoutput/EppResponse.java | 31 ++-- .../PendingActionNotificationResponse.java | 26 ++++ .../registry/model/poll/PollMessage.java | 5 + .../DeleteContactsAndHostsActionTest.java | 132 +++++++++++++----- .../registry/flows/ResourceFlowTestCase.java | 12 +- .../flows/contact/ContactDeleteFlowTest.java | 7 +- .../flows/host/HostDeleteFlowTest.java | 7 +- javatests/google/registry/model/schema.txt | 6 + 12 files changed, 240 insertions(+), 66 deletions(-) diff --git a/java/google/registry/batch/DeleteContactsAndHostsAction.java b/java/google/registry/batch/DeleteContactsAndHostsAction.java index ffb5b47fb..f4952d664 100644 --- a/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -23,9 +23,11 @@ import static com.googlecode.objectify.Key.getKind; import static google.registry.flows.ResourceFlowUtils.createResolvedTransferData; import static google.registry.flows.ResourceFlowUtils.handlePendingTransferOnDelete; import static google.registry.flows.ResourceFlowUtils.updateForeignKeyIndexDeletionTime; +import static google.registry.flows.async.AsyncFlowEnqueuer.PARAM_CLIENT_TRANSACTION_ID; import static google.registry.flows.async.AsyncFlowEnqueuer.PARAM_IS_SUPERUSER; import static google.registry.flows.async.AsyncFlowEnqueuer.PARAM_REQUESTING_CLIENT_ID; import static google.registry.flows.async.AsyncFlowEnqueuer.PARAM_RESOURCE_KEY; +import static google.registry.flows.async.AsyncFlowEnqueuer.PARAM_SERVER_TRANSACTION_ID; import static google.registry.flows.async.AsyncFlowEnqueuer.QUEUE_ASYNC_DELETE; import static google.registry.model.EppResourceUtils.isActive; import static google.registry.model.EppResourceUtils.isDeleted; @@ -68,7 +70,11 @@ 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.eppcommon.Trid; +import google.registry.model.eppoutput.EppResponse.ResponseData; import google.registry.model.host.HostResource; +import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; +import google.registry.model.poll.PendingActionNotificationResponse.HostPendingActionNotificationResponse; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.TransferStatus; @@ -318,6 +324,8 @@ public class DeleteContactsAndHostsAction implements Runnable { .setMsg(pollMessageText) .setParent(historyEntry) .setEventTime(now) + .setResponseData( + getPollMessageResponseData(deletionRequest, resource, deleteAllowed, now)) .build(); EppResource resourceToSave; @@ -348,6 +356,35 @@ public class DeleteContactsAndHostsAction implements Runnable { deleteAllowed ? Type.DELETED : Type.NOT_DELETED, pollMessageText); } + private static ImmutableList getPollMessageResponseData( + DeletionRequest deletionRequest, + EppResource resource, + boolean deleteAllowed, + DateTime now) { + Optional clientTransactionId = deletionRequest.clientTransactionId(); + Optional serverTransactionId = deletionRequest.serverTransactionId(); + // TODO(b/36402020): Make this unconditional, once older tasks enqueued without Trid data + // have been processed out of the queue. + checkState( + clientTransactionId.isPresent() == serverTransactionId.isPresent(), + "Found one part of TRID without the other!"); + if (clientTransactionId.isPresent() && serverTransactionId.isPresent()) { + Trid trid = Trid.create(clientTransactionId.get(), serverTransactionId.get()); + if (resource instanceof HostResource) { + return ImmutableList.of( + HostPendingActionNotificationResponse.create( + ((HostResource) resource).getFullyQualifiedHostName(), deleteAllowed, trid, now)); + } else if (resource instanceof ContactResource) { + return ImmutableList.of( + ContactPendingActionNotificationResponse.create( + ((ContactResource) resource).getContactId(), deleteAllowed, trid, now)); + } else { + throw new IllegalStateException("EPP resource of unknown type " + Key.create(resource)); + } + } + return ImmutableList.of(); + } + /** * Determine the proper history entry type for the delete operation, as a function of * whether or not the delete was successful. @@ -403,6 +440,13 @@ public class DeleteContactsAndHostsAction implements Runnable { * the actual current owner of the resource). */ abstract String requestingClientId(); + + /** First half of TRID for the original request, split for serializability. */ + abstract Optional clientTransactionId(); + + /** Second half of TRID for the original request, split for serializability. */ + abstract Optional serverTransactionId(); + abstract boolean isSuperuser(); abstract TaskHandle task(); @@ -411,6 +455,8 @@ public class DeleteContactsAndHostsAction implements Runnable { abstract Builder setKey(Key key); abstract Builder setLastUpdateTime(DateTime lastUpdateTime); abstract Builder setRequestingClientId(String requestingClientId); + abstract Builder setClientTransactionId(Optional clientTransactionId); + abstract Builder setServerTransactionId(Optional serverTransactionId); abstract Builder setIsSuperuser(boolean isSuperuser); abstract Builder setTask(TaskHandle task); abstract DeletionRequest build(); @@ -435,10 +481,18 @@ public class DeleteContactsAndHostsAction implements Runnable { new AutoValue_DeleteContactsAndHostsAction_DeletionRequest.Builder() .setKey(resourceKey) .setLastUpdateTime(resource.getUpdateAutoTimestamp().getTimestamp()) - .setRequestingClientId(checkNotNull( - params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified")) - .setIsSuperuser(Boolean.valueOf( - checkNotNull(params.get(PARAM_IS_SUPERUSER), "Is superuser not specified"))) + .setRequestingClientId( + checkNotNull( + params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified")) + .setClientTransactionId( + Optional.fromNullable( + params.get(PARAM_CLIENT_TRANSACTION_ID))) + .setServerTransactionId( + Optional.fromNullable( + params.get(PARAM_SERVER_TRANSACTION_ID))) + .setIsSuperuser( + Boolean.valueOf( + checkNotNull(params.get(PARAM_IS_SUPERUSER), "Is superuser not specified"))) .setTask(task) .build()); } diff --git a/java/google/registry/flows/async/AsyncFlowEnqueuer.java b/java/google/registry/flows/async/AsyncFlowEnqueuer.java index 0a0fe4c2e..6f5540fde 100644 --- a/java/google/registry/flows/async/AsyncFlowEnqueuer.java +++ b/java/google/registry/flows/async/AsyncFlowEnqueuer.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.model.EppResource; +import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostResource; import google.registry.util.FormattingLogger; import google.registry.util.Retrier; @@ -36,6 +37,8 @@ public final class AsyncFlowEnqueuer { /** The HTTP parameter names used by async flows. */ public static final String PARAM_RESOURCE_KEY = "resourceKey"; public static final String PARAM_REQUESTING_CLIENT_ID = "requestingClientId"; + public static final String PARAM_CLIENT_TRANSACTION_ID = "clientTransactionId"; + public static final String PARAM_SERVER_TRANSACTION_ID = "serverTransactionId"; public static final String PARAM_IS_SUPERUSER = "isSuperuser"; public static final String PARAM_HOST_KEY = "hostKey"; @@ -70,17 +73,18 @@ public final class AsyncFlowEnqueuer { /** Enqueues a task to asynchronously delete a contact or host, by key. */ public void enqueueAsyncDelete( - EppResource resourceToDelete, String requestingClientId, boolean isSuperuser) { + EppResource resourceToDelete, String requestingClientId, Trid trid, boolean isSuperuser) { Key resourceKey = Key.create(resourceToDelete); logger.infofmt( "Enqueuing async deletion of %s on behalf of registrar %s.", resourceKey, requestingClientId); TaskOptions task = - TaskOptions.Builder - .withMethod(Method.PULL) + TaskOptions.Builder.withMethod(Method.PULL) .countdownMillis(asyncDeleteDelay.getMillis()) .param(PARAM_RESOURCE_KEY, resourceKey.getString()) .param(PARAM_REQUESTING_CLIENT_ID, requestingClientId) + .param(PARAM_CLIENT_TRANSACTION_ID, trid.getClientTransactionId()) + .param(PARAM_SERVER_TRANSACTION_ID, trid.getServerTransactionId()) .param(PARAM_IS_SUPERUSER, Boolean.toString(isSuperuser)); addTaskToQueueWithRetry(asyncDeletePullQueue, task); } diff --git a/java/google/registry/flows/contact/ContactDeleteFlow.java b/java/google/registry/flows/contact/ContactDeleteFlow.java index ef9f25e57..c8e8a02c6 100644 --- a/java/google/registry/flows/contact/ContactDeleteFlow.java +++ b/java/google/registry/flows/contact/ContactDeleteFlow.java @@ -40,6 +40,7 @@ import google.registry.model.domain.DomainBase; import google.registry.model.domain.metadata.MetadataExtension; 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.reporting.IcannReportingTypes.ActivityReportField; @@ -78,6 +79,7 @@ public final class ContactDeleteFlow implements TransactionalFlow { @Inject ExtensionManager extensionManager; @Inject @ClientId String clientId; @Inject @TargetId String targetId; + @Inject Trid trid; @Inject @Superuser boolean isSuperuser; @Inject Optional authInfo; @Inject HistoryEntry.Builder historyBuilder; @@ -98,7 +100,7 @@ public final class ContactDeleteFlow implements TransactionalFlow { if (!isSuperuser) { verifyResourceOwnership(clientId, existingContact); } - asyncFlowEnqueuer.enqueueAsyncDelete(existingContact, clientId, isSuperuser); + asyncFlowEnqueuer.enqueueAsyncDelete(existingContact, clientId, trid, isSuperuser); ContactResource newContact = existingContact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build(); historyBuilder diff --git a/java/google/registry/flows/host/HostDeleteFlow.java b/java/google/registry/flows/host/HostDeleteFlow.java index 20abd05c7..e303cfe7f 100644 --- a/java/google/registry/flows/host/HostDeleteFlow.java +++ b/java/google/registry/flows/host/HostDeleteFlow.java @@ -38,6 +38,7 @@ import google.registry.model.EppResource; import google.registry.model.domain.DomainBase; import google.registry.model.domain.metadata.MetadataExtension; import google.registry.model.eppcommon.StatusValue; +import google.registry.model.eppcommon.Trid; import google.registry.model.eppoutput.EppResponse; import google.registry.model.host.HostResource; import google.registry.model.reporting.HistoryEntry; @@ -80,6 +81,7 @@ public final class HostDeleteFlow implements TransactionalFlow { @Inject ExtensionManager extensionManager; @Inject @ClientId String clientId; @Inject @TargetId String targetId; + @Inject Trid trid; @Inject @Superuser boolean isSuperuser; @Inject HistoryEntry.Builder historyBuilder; @Inject AsyncFlowEnqueuer asyncFlowEnqueuer; @@ -106,7 +108,7 @@ public final class HostDeleteFlow implements TransactionalFlow { : existingHost; verifyResourceOwnership(clientId, owningResource); } - asyncFlowEnqueuer.enqueueAsyncDelete(existingHost, clientId, isSuperuser); + asyncFlowEnqueuer.enqueueAsyncDelete(existingHost, clientId, trid, isSuperuser); HostResource newHost = existingHost.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build(); historyBuilder diff --git a/java/google/registry/model/eppoutput/EppResponse.java b/java/google/registry/model/eppoutput/EppResponse.java index 2f88291a6..2d0c38530 100644 --- a/java/google/registry/model/eppoutput/EppResponse.java +++ b/java/google/registry/model/eppoutput/EppResponse.java @@ -60,6 +60,7 @@ import google.registry.model.host.HostInfoData; import google.registry.model.poll.MessageQueueInfo; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; +import google.registry.model.poll.PendingActionNotificationResponse.HostPendingActionNotificationResponse; import google.registry.model.transfer.TransferResponse.ContactTransferResponse; import google.registry.model.transfer.TransferResponse.DomainTransferResponse; import javax.annotation.Nullable; @@ -98,20 +99,22 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting { /** Zero or more response "resData" results. */ @XmlElementRefs({ - @XmlElementRef(type = ContactCheckData.class), - @XmlElementRef(type = ContactCreateData.class), - @XmlElementRef(type = ContactInfoData.class), - @XmlElementRef(type = ContactPendingActionNotificationResponse.class), - @XmlElementRef(type = ContactTransferResponse.class), - @XmlElementRef(type = DomainCheckData.class), - @XmlElementRef(type = DomainCreateData.class), - @XmlElementRef(type = DomainInfoData.class), - @XmlElementRef(type = DomainPendingActionNotificationResponse.class), - @XmlElementRef(type = DomainRenewData.class), - @XmlElementRef(type = DomainTransferResponse.class), - @XmlElementRef(type = HostCheckData.class), - @XmlElementRef(type = HostCreateData.class), - @XmlElementRef(type = HostInfoData.class)}) + @XmlElementRef(type = ContactCheckData.class), + @XmlElementRef(type = ContactCreateData.class), + @XmlElementRef(type = ContactInfoData.class), + @XmlElementRef(type = ContactPendingActionNotificationResponse.class), + @XmlElementRef(type = ContactTransferResponse.class), + @XmlElementRef(type = DomainCheckData.class), + @XmlElementRef(type = DomainCreateData.class), + @XmlElementRef(type = DomainInfoData.class), + @XmlElementRef(type = DomainPendingActionNotificationResponse.class), + @XmlElementRef(type = DomainRenewData.class), + @XmlElementRef(type = DomainTransferResponse.class), + @XmlElementRef(type = HostCheckData.class), + @XmlElementRef(type = HostCreateData.class), + @XmlElementRef(type = HostInfoData.class), + @XmlElementRef(type = HostPendingActionNotificationResponse.class) + }) @XmlElementWrapper ImmutableList resData; diff --git a/java/google/registry/model/poll/PendingActionNotificationResponse.java b/java/google/registry/model/poll/PendingActionNotificationResponse.java index 240e8db75..049408758 100644 --- a/java/google/registry/model/poll/PendingActionNotificationResponse.java +++ b/java/google/registry/model/poll/PendingActionNotificationResponse.java @@ -125,4 +125,30 @@ public abstract class PendingActionNotificationResponse processedDate); } } + + /** An adapter to output the XML in response to resolving a pending command on a host. */ + @Embed + @XmlRootElement(name = "panData", namespace = "urn:ietf:params:xml:ns:domain-1.0") + @XmlType( + propOrder = {"name", "trid", "processedDate"}, + namespace = "urn:ietf:params:xml:ns:domain-1.0" + ) + public static class HostPendingActionNotificationResponse + extends PendingActionNotificationResponse { + + @XmlElement + NameOrId getName() { + return nameOrId; + } + + public static HostPendingActionNotificationResponse create( + String fullyQualifiedHostName, boolean actionResult, Trid trid, DateTime processedDate) { + return init( + new HostPendingActionNotificationResponse(), + fullyQualifiedHostName, + actionResult, + trid, + processedDate); + } + } } diff --git a/java/google/registry/model/poll/PollMessage.java b/java/google/registry/model/poll/PollMessage.java index 2896393e4..2e0782a2f 100644 --- a/java/google/registry/model/poll/PollMessage.java +++ b/java/google/registry/model/poll/PollMessage.java @@ -39,6 +39,7 @@ import google.registry.model.eppoutput.EppResponse.ResponseData; import google.registry.model.eppoutput.EppResponse.ResponseExtension; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; +import google.registry.model.poll.PendingActionNotificationResponse.HostPendingActionNotificationResponse; import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import google.registry.model.transfer.TransferResponse.ContactTransferResponse; @@ -189,6 +190,7 @@ public abstract class PollMessage extends ImmutableObject List contactTransferResponses; List domainPendingActionNotificationResponses; List domainTransferResponses; + List hostPendingActionNotificationResponses; // Extensions. Objectify cannot persist a base class type, so we must have a separate field // to hold every possible derived type of ResponseExtensions that we might store. @@ -211,6 +213,7 @@ public abstract class PollMessage extends ImmutableObject .addAll(nullToEmpty(contactTransferResponses)) .addAll(nullToEmpty(domainPendingActionNotificationResponses)) .addAll(nullToEmpty(domainTransferResponses)) + .addAll(nullToEmpty(hostPendingActionNotificationResponses)) .build(); } @@ -240,6 +243,8 @@ public abstract class PollMessage extends ImmutableObject iterable.filter(DomainPendingActionNotificationResponse.class).toList()); getInstance().domainTransferResponses = forceEmptyToNull( iterable.filter(DomainTransferResponse.class).toList()); + getInstance().hostPendingActionNotificationResponses = forceEmptyToNull( + iterable.filter(HostPendingActionNotificationResponse.class).toList()); return this; } diff --git a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java index 2731f7cc7..e04a269c0 100644 --- a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java +++ b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java @@ -69,9 +69,12 @@ import google.registry.model.contact.PostalInfo; import google.registry.model.domain.DomainResource; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; +import google.registry.model.eppoutput.EppResponse.ResponseData; import google.registry.model.host.HostResource; import google.registry.model.ofy.Ofy; import google.registry.model.poll.PendingActionNotificationResponse; +import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; +import google.registry.model.poll.PendingActionNotificationResponse.HostPendingActionNotificationResponse; import google.registry.model.poll.PollMessage; import google.registry.model.poll.PollMessage.OneTime; import google.registry.model.registry.Registry; @@ -147,7 +150,8 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_contact_referencedByActiveDomain_doesNotGetDeleted() throws Exception { ContactResource contact = persistContactPendingDelete("blah8221"); persistResource(newDomainResource("example.tld", contact)); - enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); ContactResource contactUpdated = loadByForeignKey(ContactResource.class, "blah8221", clock.nowUtc()); @@ -164,14 +168,17 @@ public class DeleteContactsAndHostsActionTest assertPollMessageFor( historyEntry, "TheRegistrar", - "Can't delete contact blah8221 because it is referenced by a domain."); + "Can't delete contact blah8221 because it is referenced by a domain.", + false, + contact); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_contact_notReferenced_getsDeleted_andPiiWipedOut() throws Exception { ContactResource contact = persistContactWithPii("jim919"); - enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(ContactResource.class, "jim919", clock.nowUtc())).isNull(); ContactResource contactAfterDeletion = ofy().load().entity(contact).now(); @@ -195,7 +202,7 @@ public class DeleteContactsAndHostsActionTest .and() .hasNullFaxNumber(); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactAfterDeletion, CONTACT_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted contact jim919."); + assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted contact jim919.", true, contact); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -203,7 +210,8 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_contactWithoutPendingTransfer_isDeletedAndHasNoTransferData() throws Exception { ContactResource contact = persistContactPendingDelete("blah8221"); - enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); ContactResource contactAfterDeletion = ofy().load().entity(contact).now(); assertThat(contactAfterDeletion.getTransferData()).isEqualTo(TransferData.EMPTY); @@ -218,7 +226,8 @@ public class DeleteContactsAndHostsActionTest transferRequestTime, transferRequestTime.plus(Registry.DEFAULT_TRANSFER_GRACE_PERIOD), clock.nowUtc()); - enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); // Check that the contact is deleted as of now. assertThat(loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())).isNull(); @@ -263,7 +272,8 @@ public class DeleteContactsAndHostsActionTest .asBuilder() .setDeletionTime(clock.nowUtc().minusDays(3)) .build()); - enqueuer.enqueueAsyncDelete(contactUsed, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contactUsed, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(ContactResource.class, "blah1234", clock.nowUtc())).isNull(); ContactResource contactBeforeDeletion = @@ -279,14 +289,16 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(CONTACT_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactBeforeDeletion, CONTACT_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted contact blah1234."); + assertPollMessageFor( + historyEntry, "TheRegistrar", "Deleted contact blah1234.", true, contactUsed); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_contact_notRequestedByOwner_doesNotGetDeleted() throws Exception { ContactResource contact = persistContactPendingDelete("jane0991"); - enqueuer.enqueueAsyncDelete(contact, "OtherRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "OtherRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); ContactResource contactAfter = loadByForeignKey(ContactResource.class, "jane0991", clock.nowUtc()); @@ -299,14 +311,17 @@ public class DeleteContactsAndHostsActionTest assertPollMessageFor( historyEntry, "OtherRegistrar", - "Can't delete contact jane0991 because it was transferred prior to deletion."); + "Can't delete contact jane0991 because it was transferred prior to deletion.", + false, + contact); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_contact_notRequestedByOwner_isSuperuser_getsDeleted() throws Exception { ContactResource contact = persistContactWithPii("nate007"); - enqueuer.enqueueAsyncDelete(contact, "OtherRegistrar", true); + enqueuer.enqueueAsyncDelete( + contact, "OtherRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), true); runMapreduce(); assertThat(loadByForeignKey(ContactResource.class, "nate007", clock.nowUtc())).isNull(); ContactResource contactAfterDeletion = ofy().load().entity(contact).now(); @@ -330,7 +345,7 @@ public class DeleteContactsAndHostsActionTest .and() .hasNullFaxNumber(); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactAfterDeletion, CONTACT_DELETE); - assertPollMessageFor(historyEntry, "OtherRegistrar", "Deleted contact nate007."); + assertPollMessageFor(historyEntry, "OtherRegistrar", "Deleted contact nate007.", true, contact); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -338,10 +353,14 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_targetResourcesDontExist_areDelayedForADay() throws Exception { ContactResource contactNotSaved = newContactResource("somecontact"); HostResource hostNotSaved = newHostResource("a11.blah.foo"); - enqueuer.enqueueAsyncDelete(contactNotSaved, "TheRegistrar", false); - enqueuer.enqueueAsyncDelete(hostNotSaved, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contactNotSaved, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); + enqueuer.enqueueAsyncDelete( + hostNotSaved, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); - String payloadFormat = "resourceKey=%s&requestingClientId=TheRegistrar&isSuperuser=false"; + String payloadFormat = + "resourceKey=%s&requestingClientId=TheRegistrar&" + + "clientTransactionId=fakeClientTrid&serverTransactionId=fakeServerTrid&isSuperuser=false"; assertTasksEnqueued( QUEUE_ASYNC_DELETE, new TaskMatcher() @@ -369,8 +388,10 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_resourcesNotInPendingDelete_areSkipped() throws Exception { ContactResource contact = persistActiveContact("blah2222"); HostResource host = persistActiveHost("rustles.your.jimmies"); - enqueuer.enqueueAsyncDelete(contact, "TheRegistrar", false); - enqueuer.enqueueAsyncDelete(host, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contact, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); + enqueuer.enqueueAsyncDelete( + host, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(ContactResource.class, "blah2222", clock.nowUtc())) .isEqualTo(contact); @@ -383,8 +404,10 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_alreadyDeletedResources_areSkipped() throws Exception { ContactResource contactDeleted = persistDeletedContact("blah1236", clock.nowUtc().minusDays(2)); HostResource hostDeleted = persistDeletedHost("a.lim.lop", clock.nowUtc().minusDays(3)); - enqueuer.enqueueAsyncDelete(contactDeleted, "TheRegistrar", false); - enqueuer.enqueueAsyncDelete(hostDeleted, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + contactDeleted, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); + enqueuer.enqueueAsyncDelete( + hostDeleted, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(ofy().load().entity(contactDeleted).now()).isEqualTo(contactDeleted); assertThat(ofy().load().entity(hostDeleted).now()).isEqualTo(hostDeleted); @@ -395,7 +418,8 @@ public class DeleteContactsAndHostsActionTest public void testSuccess_host_referencedByActiveDomain_doesNotGetDeleted() throws Exception { HostResource host = persistHostPendingDelete("ns1.example.tld"); persistUsedDomain("example.tld", persistActiveContact("abc456"), host); - enqueuer.enqueueAsyncDelete(host, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + host, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); HostResource hostAfter = loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()); @@ -410,14 +434,17 @@ public class DeleteContactsAndHostsActionTest assertPollMessageFor( historyEntry, "TheRegistrar", - "Can't delete host ns1.example.tld because it is referenced by a domain."); + "Can't delete host ns1.example.tld because it is referenced by a domain.", + false, + host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_host_notReferenced_getsDeleted() throws Exception { HostResource host = persistHostPendingDelete("ns2.example.tld"); - enqueuer.enqueueAsyncDelete(host, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + host, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isNull(); HostResource hostBeforeDeletion = @@ -433,7 +460,7 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld."); + assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld.", true, host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -446,7 +473,8 @@ public class DeleteContactsAndHostsActionTest .setNameservers(ImmutableSet.of(Key.create(host))) .setDeletionTime(clock.nowUtc().minusDays(5)) .build()); - enqueuer.enqueueAsyncDelete(host, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + host, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc())).isNull(); HostResource hostBeforeDeletion = @@ -462,7 +490,7 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns1.example.tld."); + assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns1.example.tld.", true, host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -480,7 +508,8 @@ public class DeleteContactsAndHostsActionTest .asBuilder() .setSuperordinateDomain(Key.create(domain)) .build()); - enqueuer.enqueueAsyncDelete(host, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + host, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); // Check that the host is deleted as of now. assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isNull(); @@ -501,14 +530,15 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld."); + assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld.", true, host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_host_notRequestedByOwner_doesNotGetDeleted() throws Exception { HostResource host = persistHostPendingDelete("ns2.example.tld"); - enqueuer.enqueueAsyncDelete(host, "OtherRegistrar", false); + enqueuer.enqueueAsyncDelete( + host, "OtherRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); HostResource hostAfter = loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc()); @@ -521,14 +551,17 @@ public class DeleteContactsAndHostsActionTest assertPollMessageFor( historyEntry, "OtherRegistrar", - "Can't delete host ns2.example.tld because it was transferred prior to deletion."); + "Can't delete host ns2.example.tld because it was transferred prior to deletion.", + false, + host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @Test public void testSuccess_host_notRequestedByOwner_isSuperuser_getsDeleted() throws Exception { HostResource host = persistHostPendingDelete("ns66.example.tld"); - enqueuer.enqueueAsyncDelete(host, "OtherRegistrar", true); + enqueuer.enqueueAsyncDelete( + host, "OtherRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), true); runMapreduce(); assertThat(loadByForeignKey(HostResource.class, "ns66.example.tld", clock.nowUtc())).isNull(); HostResource hostBeforeDeletion = @@ -544,7 +577,8 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "OtherRegistrar", "Deleted host ns66.example.tld."); + assertPollMessageFor( + historyEntry, "OtherRegistrar", "Deleted host ns66.example.tld.", true, host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -560,7 +594,8 @@ public class DeleteContactsAndHostsActionTest HostResource h4 = persistHostPendingDelete("used.host.com"); persistUsedDomain("usescontactandhost.tld", c4, h4); for (EppResource resource : ImmutableList.of(c1, c2, c3, c4, h1, h2, h3, h4)) { - enqueuer.enqueueAsyncDelete(resource, "TheRegistrar", false); + enqueuer.enqueueAsyncDelete( + resource, "TheRegistrar", Trid.create("fakeClientTrid", "fakeServerTrid"), false); } runMapreduce(); for (EppResource resource : ImmutableList.of(c1, c2, c3, h1, h2, h3)) { @@ -605,13 +640,38 @@ public class DeleteContactsAndHostsActionTest /** * Helper method to check that one poll message exists with a given history entry, resource, - * client id, and message. + * client id, and message. Also checks that the only resulting async response matches the resource + * type, and has the appropriate actionResult, nameOrId, and Trid. */ - private static void assertPollMessageFor(HistoryEntry historyEntry, String clientId, String msg) { + private static void assertPollMessageFor( + HistoryEntry historyEntry, + String clientId, + String msg, + boolean expectedActionResult, + EppResource resource) { PollMessage.OneTime pollMessage = (OneTime) getOnlyPollMessageForHistoryEntry(historyEntry); - assertThat(msg).isEqualTo(pollMessage.getMsg()); - assertThat(clientId).isEqualTo(pollMessage.getClientId()); + assertThat(pollMessage.getMsg()).isEqualTo(msg); assertThat(pollMessage.getClientId()).isEqualTo(clientId); + + ImmutableList pollResponses = pollMessage.getResponseData(); + assertThat(pollResponses).hasSize(1); + ResponseData responseData = pollMessage.getResponseData().get(0); + + String expectedResourceName; + if (resource instanceof HostResource) { + assertThat(responseData).isInstanceOf(HostPendingActionNotificationResponse.class); + expectedResourceName = ((HostResource) resource).getFullyQualifiedHostName(); + } else { + assertThat(responseData).isInstanceOf(ContactPendingActionNotificationResponse.class); + expectedResourceName = ((ContactResource) resource).getContactId(); + } + PendingActionNotificationResponse pendingResponse = + (PendingActionNotificationResponse) responseData; + assertThat(pendingResponse.getActionResult()).isEqualTo(expectedActionResult); + assertThat(pendingResponse.getNameAsString()).isEqualTo(expectedResourceName); + Trid trid = pendingResponse.getTrid(); + assertThat(trid.getClientTransactionId()).isEqualTo("fakeClientTrid"); + assertThat(trid.getServerTransactionId()).isEqualTo("fakeServerTrid"); } private static ContactResource persistContactPendingDelete(String contactId) { diff --git a/javatests/google/registry/flows/ResourceFlowTestCase.java b/javatests/google/registry/flows/ResourceFlowTestCase.java index 7b8b0748d..b7d602f2e 100644 --- a/javatests/google/registry/flows/ResourceFlowTestCase.java +++ b/javatests/google/registry/flows/ResourceFlowTestCase.java @@ -32,6 +32,7 @@ import google.registry.model.EppResource; import google.registry.model.EppResourceUtils; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.launch.ApplicationIdTargetExtension; +import google.registry.model.eppcommon.Trid; import google.registry.model.eppinput.EppInput.ResourceCommandWrapper; import google.registry.model.eppinput.ResourceCommand; import google.registry.model.index.EppResourceIndex; @@ -155,11 +156,16 @@ public abstract class ResourceFlowTestCase void assertAsyncDeletionTaskEnqueued( - T resource, String requestingClientId, boolean isSuperuser) throws Exception { + T resource, String requestingClientId, Trid trid, boolean isSuperuser) throws Exception { String expectedPayload = String.format( - "resourceKey=%s&requestingClientId=%s&isSuperuser=%s", - Key.create(resource).getString(), requestingClientId, Boolean.toString(isSuperuser)); + "resourceKey=%s&requestingClientId=%s&clientTransactionId=%s&" + + "serverTransactionId=%s&isSuperuser=%s", + Key.create(resource).getString(), + requestingClientId, + trid.getClientTransactionId(), + trid.getServerTransactionId(), + Boolean.toString(isSuperuser)); assertTasksEnqueued( "async-delete-pull", new TaskMatcher() diff --git a/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java b/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java index 97b4ad9d7..57722346e 100644 --- a/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -31,6 +31,7 @@ import google.registry.flows.exceptions.ResourceStatusProhibitsOperationExceptio 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 org.junit.Before; import org.junit.Test; @@ -58,7 +59,8 @@ public class ContactDeleteFlowTest runFlowAssertResponse(readFile("contact_delete_response.xml")); ContactResource deletedContact = reloadResourceByForeignKey(); assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE); - assertAsyncDeletionTaskEnqueued(deletedContact, "TheRegistrar", false); + assertAsyncDeletionTaskEnqueued( + deletedContact, "TheRegistrar", Trid.create("ABC-12345", "server-trid"), false); assertAboutContacts().that(deletedContact) .hasOnlyOneHistoryEntryWhich() .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE); @@ -127,7 +129,8 @@ public class ContactDeleteFlowTest CommitMode.LIVE, UserPrivileges.SUPERUSER, readFile("contact_delete_response.xml")); ContactResource deletedContact = reloadResourceByForeignKey(); assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE); - assertAsyncDeletionTaskEnqueued(deletedContact, "NewRegistrar", true); + assertAsyncDeletionTaskEnqueued( + deletedContact, "NewRegistrar", Trid.create("ABC-12345", "server-trid"), true); assertAboutContacts().that(deletedContact) .hasOnlyOneHistoryEntryWhich() .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE); diff --git a/javatests/google/registry/flows/host/HostDeleteFlowTest.java b/javatests/google/registry/flows/host/HostDeleteFlowTest.java index b52a4e484..c542517f1 100644 --- a/javatests/google/registry/flows/host/HostDeleteFlowTest.java +++ b/javatests/google/registry/flows/host/HostDeleteFlowTest.java @@ -37,6 +37,7 @@ import google.registry.flows.host.HostFlowUtils.HostNameNotNormalizedException; import google.registry.flows.host.HostFlowUtils.HostNameNotPunyCodedException; import google.registry.model.domain.DomainResource; import google.registry.model.eppcommon.StatusValue; +import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostResource; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; @@ -68,7 +69,8 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase contactPendingActionNotificationResponses; java.util.List domainPendingActionNotificationResponses; + java.util.List hostPendingActionNotificationResponses; java.util.List contactTransferResponses; java.util.List domainTransferResponses; org.joda.time.DateTime eventTime;