From 62c05c112d9a47a05c75dd53b41a2a007d6055a2 Mon Sep 17 00:00:00 2001 From: larryruili Date: Thu, 27 Apr 2017 15:19:29 -0700 Subject: [PATCH] Make TRID field in async host/contact deletion non-optional This cleans up a TODO introduced in the original bug, which allowed tasks to have optional Trids in the case it was an older enqueued task. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=154477042 --- .../batch/DeleteContactsAndHostsAction.java | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/java/google/registry/batch/DeleteContactsAndHostsAction.java b/java/google/registry/batch/DeleteContactsAndHostsAction.java index f4952d664..3b8cce54d 100644 --- a/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -361,28 +361,20 @@ public class DeleteContactsAndHostsAction implements Runnable { 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)); - } + String clientTransactionId = deletionRequest.clientTransactionId(); + String serverTransactionId = deletionRequest.serverTransactionId(); + Trid trid = Trid.create(clientTransactionId, serverTransactionId); + 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(); } /** @@ -442,10 +434,10 @@ public class DeleteContactsAndHostsAction implements Runnable { abstract String requestingClientId(); /** First half of TRID for the original request, split for serializability. */ - abstract Optional clientTransactionId(); + abstract String clientTransactionId(); /** Second half of TRID for the original request, split for serializability. */ - abstract Optional serverTransactionId(); + abstract String serverTransactionId(); abstract boolean isSuperuser(); abstract TaskHandle task(); @@ -455,8 +447,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 setClientTransactionId(String clientTransactionId); + abstract Builder setServerTransactionId(String serverTransactionId); abstract Builder setIsSuperuser(boolean isSuperuser); abstract Builder setTask(TaskHandle task); abstract DeletionRequest build(); @@ -485,11 +477,13 @@ public class DeleteContactsAndHostsAction implements Runnable { checkNotNull( params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified")) .setClientTransactionId( - Optional.fromNullable( - params.get(PARAM_CLIENT_TRANSACTION_ID))) + checkNotNull( + params.get(PARAM_CLIENT_TRANSACTION_ID), + "Client transaction id not specified")) .setServerTransactionId( - Optional.fromNullable( - params.get(PARAM_SERVER_TRANSACTION_ID))) + checkNotNull( + params.get(PARAM_SERVER_TRANSACTION_ID), + "Server transaction id not specified")) .setIsSuperuser( Boolean.valueOf( checkNotNull(params.get(PARAM_IS_SUPERUSER), "Is superuser not specified")))