diff --git a/java/google/registry/batch/DeleteContactsAndHostsAction.java b/java/google/registry/batch/DeleteContactsAndHostsAction.java index 7ba336454..1e070eb28 100644 --- a/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -92,6 +92,7 @@ import google.registry.util.SystemClock; import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Named; import org.joda.time.DateTime; @@ -379,7 +380,7 @@ public class DeleteContactsAndHostsAction implements Runnable { EppResource resource, boolean deleteAllowed, DateTime now) { - String clientTransactionId = deletionRequest.clientTransactionId(); + @Nullable String clientTransactionId = deletionRequest.clientTransactionId(); String serverTransactionId = deletionRequest.serverTransactionId(); Trid trid = Trid.create(clientTransactionId, serverTransactionId); if (resource instanceof HostResource) { @@ -445,6 +446,7 @@ public class DeleteContactsAndHostsAction implements Runnable { abstract Key key(); abstract DateTime lastUpdateTime(); + /** * The client id of the registrar that requested this deletion (which might NOT be the same as * the actual current owner of the resource). @@ -452,6 +454,7 @@ public class DeleteContactsAndHostsAction implements Runnable { abstract String requestingClientId(); /** First half of TRID for the original request, split for serializability. */ + @Nullable abstract String clientTransactionId(); /** Second half of TRID for the original request, split for serializability. */ @@ -467,7 +470,7 @@ public class DeleteContactsAndHostsAction implements Runnable { abstract Builder setKey(Key key); abstract Builder setLastUpdateTime(DateTime lastUpdateTime); abstract Builder setRequestingClientId(String requestingClientId); - abstract Builder setClientTransactionId(String clientTransactionId); + abstract Builder setClientTransactionId(@Nullable String clientTransactionId); abstract Builder setServerTransactionId(String serverTransactionId); abstract Builder setIsSuperuser(boolean isSuperuser); abstract Builder setRequestedTime(DateTime requestedTime); @@ -494,9 +497,8 @@ public class DeleteContactsAndHostsAction implements Runnable { .setRequestingClientId( checkNotNull( params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified")) - .setClientTransactionId( - checkNotNull( - params.get(PARAM_CLIENT_TRANSACTION_ID), "Client transaction id not specified")) + // Note that client transaction ID is optional, in which case this sets it to null. + .setClientTransactionId(params.get(PARAM_CLIENT_TRANSACTION_ID)) .setServerTransactionId( checkNotNull( params.get(PARAM_SERVER_TRANSACTION_ID), "Server transaction id not specified")) diff --git a/java/google/registry/flows/async/AsyncFlowEnqueuer.java b/java/google/registry/flows/async/AsyncFlowEnqueuer.java index 1d30ba42b..ce186ff3f 100644 --- a/java/google/registry/flows/async/AsyncFlowEnqueuer.java +++ b/java/google/registry/flows/async/AsyncFlowEnqueuer.java @@ -83,10 +83,12 @@ public final class AsyncFlowEnqueuer { .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)) .param(PARAM_REQUESTED_TIME, now.toString()); + if (trid.getClientTransactionId().isPresent()) { + task.param(PARAM_CLIENT_TRANSACTION_ID, trid.getClientTransactionId().get()); + } addTaskToQueueWithRetry(asyncDeletePullQueue, task); } diff --git a/java/google/registry/model/eppcommon/Trid.java b/java/google/registry/model/eppcommon/Trid.java index cd3b2d307..ba3a0a175 100644 --- a/java/google/registry/model/eppcommon/Trid.java +++ b/java/google/registry/model/eppcommon/Trid.java @@ -19,6 +19,7 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.annotations.VisibleForTesting; import com.googlecode.objectify.annotation.Embed; import google.registry.model.ImmutableObject; +import java.util.Optional; import javax.annotation.Nullable; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; @@ -45,8 +46,8 @@ public class Trid extends ImmutableObject { return serverTransactionId; } - public String getClientTransactionId() { - return clientTransactionId; + public Optional getClientTransactionId() { + return Optional.ofNullable(clientTransactionId); } @VisibleForTesting diff --git a/java/google/registry/tools/AllocateDomainCommand.java b/java/google/registry/tools/AllocateDomainCommand.java index c90b36b6c..ed5a4a124 100644 --- a/java/google/registry/tools/AllocateDomainCommand.java +++ b/java/google/registry/tools/AllocateDomainCommand.java @@ -134,7 +134,7 @@ final class AllocateDomainCommand extends MutatingEppToolCommand { "Could not find any history entries for domain application %s", application.getRepoId()); String clientTransactionId = - emptyToNull(history.getTrid().getClientTransactionId()); + emptyToNull(history.getTrid().getClientTransactionId().orElse(null)); Period period = checkNotNull(extractPeriodFromXml(history.getXmlBytes())); checkArgument(period.getUnit() == Period.Unit.YEARS); ImmutableMap.Builder contactsMapBuilder = diff --git a/java/google/registry/tools/GetHistoryEntriesCommand.java b/java/google/registry/tools/GetHistoryEntriesCommand.java index 9dbcd26e8..c15d14877 100644 --- a/java/google/registry/tools/GetHistoryEntriesCommand.java +++ b/java/google/registry/tools/GetHistoryEntriesCommand.java @@ -77,7 +77,7 @@ final class GetHistoryEntriesCommand implements RemoteApiCommand { "Client: %s\nTime: %s\nClient TRID: %s\nServer TRID: %s\n%s\n", entry.getClientId(), entry.getModificationTime(), - (entry.getTrid() == null) ? null : entry.getTrid().getClientTransactionId(), + (entry.getTrid() == null) ? null : entry.getTrid().getClientTransactionId().orElse(null), (entry.getTrid() == null) ? null : entry.getTrid().getServerTransactionId(), entry.getXmlBytes() == null ? String.format("[no XML stored for %s]\n", entry.getType()) diff --git a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java index 118e761f5..712c0e06d 100644 --- a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java +++ b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java @@ -17,6 +17,7 @@ package google.registry.batch; import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.flows.async.AsyncFlowEnqueuer.QUEUE_ASYNC_DELETE; import static google.registry.flows.async.AsyncFlowEnqueuer.QUEUE_ASYNC_HOST_RENAME; import static google.registry.flows.async.AsyncFlowMetrics.OperationResult.STALE; @@ -97,6 +98,7 @@ import google.registry.testing.mapreduce.MapreduceTestCase; import google.registry.util.Retrier; import google.registry.util.Sleeper; import google.registry.util.SystemSleeper; +import java.util.Optional; import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.Before; @@ -182,7 +184,8 @@ public class DeleteContactsAndHostsActionTest "TheRegistrar", "Can't delete contact blah8221 because it is referenced by a domain.", false, - contact); + contact, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); verify(action.asyncFlowMetrics).recordContactHostDeletionBatchSize(1L); verify(action.asyncFlowMetrics) @@ -192,13 +195,23 @@ public class DeleteContactsAndHostsActionTest @Test public void testSuccess_contact_notReferenced_getsDeleted_andPiiWipedOut() throws Exception { + runSuccessfulContactDeletionTest(Optional.of("fakeClientTrid")); + } + + @Test + public void testSuccess_contact_andNoClientTrid_deletesSuccessfully() throws Exception { + runSuccessfulContactDeletionTest(Optional.empty()); + } + + + private void runSuccessfulContactDeletionTest(Optional clientTrid) throws Exception { ContactResource contact = persistContactWithPii("jim919"); DateTime timeEnqueued = clock.nowUtc(); enqueuer.enqueueAsyncDelete( contact, timeEnqueued, "TheRegistrar", - Trid.create("fakeClientTrid", "fakeServerTrid"), + Trid.create(clientTrid.orElse(null), "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(ContactResource.class, "jim919", clock.nowUtc())).isNull(); @@ -223,12 +236,14 @@ public class DeleteContactsAndHostsActionTest .and() .hasNullFaxNumber(); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactAfterDeletion, CONTACT_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted contact jim919.", true, contact); + assertPollMessageFor( + historyEntry, "TheRegistrar", "Deleted contact jim919.", true, contact, clientTrid); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); verify(action.asyncFlowMetrics).recordContactHostDeletionBatchSize(1L); verify(action.asyncFlowMetrics) .recordAsyncFlowResult(OperationType.CONTACT_DELETE, OperationResult.SUCCESS, timeEnqueued); verifyNoMoreInteractions(action.asyncFlowMetrics); + } @Test @@ -339,7 +354,12 @@ public class DeleteContactsAndHostsActionTest .hasType(CONTACT_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactBeforeDeletion, CONTACT_DELETE); assertPollMessageFor( - historyEntry, "TheRegistrar", "Deleted contact blah1234.", true, contactUsed); + historyEntry, + "TheRegistrar", + "Deleted contact blah1234.", + true, + contactUsed, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -366,7 +386,8 @@ public class DeleteContactsAndHostsActionTest "OtherRegistrar", "Can't delete contact jane0991 because it was transferred prior to deletion.", false, - contact); + contact, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -402,7 +423,13 @@ public class DeleteContactsAndHostsActionTest .and() .hasNullFaxNumber(); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactAfterDeletion, CONTACT_DELETE); - assertPollMessageFor(historyEntry, "OtherRegistrar", "Deleted contact nate007.", true, contact); + assertPollMessageFor( + historyEntry, + "OtherRegistrar", + "Deleted contact nate007.", + true, + contact, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -539,7 +566,8 @@ public class DeleteContactsAndHostsActionTest "TheRegistrar", "Can't delete host ns1.example.tld because it is referenced by a domain.", false, - host); + host, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); verify(action.asyncFlowMetrics).recordContactHostDeletionBatchSize(1L); verify(action.asyncFlowMetrics) @@ -549,13 +577,22 @@ public class DeleteContactsAndHostsActionTest @Test public void testSuccess_host_notReferenced_getsDeleted() throws Exception { + runSuccessfulHostDeletionTest(Optional.of("fakeClientTrid")); + } + + @Test + public void testSuccess_host_andNoClientTrid_deletesSuccessfully() throws Exception { + runSuccessfulHostDeletionTest(Optional.empty()); + } + + private void runSuccessfulHostDeletionTest(Optional clientTrid) throws Exception { HostResource host = persistHostPendingDelete("ns2.example.tld"); DateTime timeEnqueued = clock.nowUtc(); enqueuer.enqueueAsyncDelete( host, timeEnqueued, "TheRegistrar", - Trid.create("fakeClientTrid", "fakeServerTrid"), + Trid.create(clientTrid.orElse(null), "fakeServerTrid"), false); runMapreduce(); assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isNull(); @@ -572,7 +609,13 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld.", true, host); + assertPollMessageFor( + historyEntry, + "TheRegistrar", + "Deleted host ns2.example.tld.", + true, + host, + clientTrid); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); verify(action.asyncFlowMetrics).recordContactHostDeletionBatchSize(1L); verify(action.asyncFlowMetrics) @@ -610,7 +653,13 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns1.example.tld.", true, host); + assertPollMessageFor( + historyEntry, + "TheRegistrar", + "Deleted host ns1.example.tld.", + true, + host, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -654,7 +703,13 @@ public class DeleteContactsAndHostsActionTest .hasOnlyOneHistoryEntryWhich() .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); - assertPollMessageFor(historyEntry, "TheRegistrar", "Deleted host ns2.example.tld.", true, host); + assertPollMessageFor( + historyEntry, + "TheRegistrar", + "Deleted host ns2.example.tld.", + true, + host, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -681,7 +736,8 @@ public class DeleteContactsAndHostsActionTest "OtherRegistrar", "Can't delete host ns2.example.tld because it was transferred prior to deletion.", false, - host); + host, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -710,7 +766,12 @@ public class DeleteContactsAndHostsActionTest .hasType(HOST_DELETE); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostBeforeDeletion, HOST_DELETE); assertPollMessageFor( - historyEntry, "OtherRegistrar", "Deleted host ns66.example.tld.", true, host); + historyEntry, + "OtherRegistrar", + "Deleted host ns66.example.tld.", + true, + host, + Optional.of("fakeClientTrid")); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } @@ -784,7 +845,8 @@ public class DeleteContactsAndHostsActionTest String clientId, String msg, boolean expectedActionResult, - EppResource resource) { + EppResource resource, + Optional clientTrid) { PollMessage.OneTime pollMessage = (OneTime) getOnlyPollMessageForHistoryEntry(historyEntry); assertThat(pollMessage.getMsg()).isEqualTo(msg); assertThat(pollMessage.getClientId()).isEqualTo(clientId); @@ -806,7 +868,7 @@ public class DeleteContactsAndHostsActionTest assertThat(pendingResponse.getActionResult()).isEqualTo(expectedActionResult); assertThat(pendingResponse.getNameAsString()).isEqualTo(expectedResourceName); Trid trid = pendingResponse.getTrid(); - assertThat(trid.getClientTransactionId()).isEqualTo("fakeClientTrid"); + assertThat(trid.getClientTransactionId()).isEqualTo(clientTrid); assertThat(trid.getServerTransactionId()).isEqualTo("fakeServerTrid"); } diff --git a/javatests/google/registry/flows/ResourceFlowTestCase.java b/javatests/google/registry/flows/ResourceFlowTestCase.java index 856fcdbd8..b5ccee7ca 100644 --- a/javatests/google/registry/flows/ResourceFlowTestCase.java +++ b/javatests/google/registry/flows/ResourceFlowTestCase.java @@ -163,16 +163,17 @@ public abstract class ResourceFlowTestCase void assertAsyncDeletionTaskEnqueued( T resource, String requestingClientId, Trid trid, boolean isSuperuser) throws Exception { - assertTasksEnqueued( - "async-delete-pull", - new TaskMatcher() - .etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90 - .param("resourceKey", Key.create(resource).getString()) - .param("requestingClientId", requestingClientId) - .param("clientTransactionId", trid.getClientTransactionId()) - .param("serverTransactionId", trid.getServerTransactionId()) - .param("isSuperuser", Boolean.toString(isSuperuser)) - .param("requestedTime", clock.nowUtc().toString())); + TaskMatcher expected = new TaskMatcher() + .etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90 + .param("resourceKey", Key.create(resource).getString()) + .param("requestingClientId", requestingClientId) + .param("serverTransactionId", trid.getServerTransactionId()) + .param("isSuperuser", Boolean.toString(isSuperuser)) + .param("requestedTime", clock.nowUtc().toString()); + if (trid.getClientTransactionId().isPresent()) { + expected.param("clientTransactionId", trid.getClientTransactionId().get()); + } + assertTasksEnqueued("async-delete-pull", expected); } diff --git a/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java b/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java index a3c2ea04b..0e6c12e0b 100644 --- a/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/javatests/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -65,7 +65,26 @@ public class ContactDeleteFlowTest assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE); assertAsyncDeletionTaskEnqueued( deletedContact, "TheRegistrar", Trid.create("ABC-12345", "server-trid"), false); - assertAboutContacts().that(deletedContact) + assertAboutContacts() + .that(deletedContact) + .hasOnlyOneHistoryEntryWhich() + .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE); + assertNoBillingEvents(); + } + + @Test + public void testSuccess_clTridNotSpecified() throws Exception { + setEppInput("contact_delete_no_cltrid.xml"); + persistActiveContact(getUniqueIdFromCommand()); + clock.advanceOneMilli(); + assertTransactionalFlow(true); + runFlowAssertResponse(loadFile("contact_delete_response_no_cltrid.xml")); + ContactResource deletedContact = reloadResourceByForeignKey(); + assertAboutContacts().that(deletedContact).hasStatusValue(StatusValue.PENDING_DELETE); + assertAsyncDeletionTaskEnqueued( + deletedContact, "TheRegistrar", Trid.create(null, "server-trid"), false); + assertAboutContacts() + .that(deletedContact) .hasOnlyOneHistoryEntryWhich() .hasType(HistoryEntry.Type.CONTACT_PENDING_DELETE); assertNoBillingEvents(); diff --git a/javatests/google/registry/flows/contact/testdata/contact_delete_no_cltrid.xml b/javatests/google/registry/flows/contact/testdata/contact_delete_no_cltrid.xml new file mode 100644 index 000000000..2c735b416 --- /dev/null +++ b/javatests/google/registry/flows/contact/testdata/contact_delete_no_cltrid.xml @@ -0,0 +1,10 @@ + + + + + sh8013 + + + + diff --git a/javatests/google/registry/flows/contact/testdata/contact_delete_response_no_cltrid.xml b/javatests/google/registry/flows/contact/testdata/contact_delete_response_no_cltrid.xml new file mode 100644 index 000000000..7d1721ff9 --- /dev/null +++ b/javatests/google/registry/flows/contact/testdata/contact_delete_response_no_cltrid.xml @@ -0,0 +1,10 @@ + + + + Command completed successfully; action pending + + + server-trid + + + diff --git a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java index 48c5a5171..c458ff291 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -17,6 +17,7 @@ package google.registry.flows.domain; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE; @@ -344,7 +345,7 @@ public class DomainTransferRequestFlowTest .filter(PendingActionNotificationResponse.class::isInstance) .map(PendingActionNotificationResponse.class::cast) .collect(onlyElement()); - assertThat(panData.getTrid().getClientTransactionId()).isEqualTo("ABC-12345"); + assertThat(panData.getTrid().getClientTransactionId()).hasValue("ABC-12345"); assertThat(panData.getActionResult()).isTrue(); // Two poll messages on the losing registrar's side at the implicit transfer time: a diff --git a/javatests/google/registry/flows/host/HostDeleteFlowTest.java b/javatests/google/registry/flows/host/HostDeleteFlowTest.java index 5ef1f674d..762d08ac3 100644 --- a/javatests/google/registry/flows/host/HostDeleteFlowTest.java +++ b/javatests/google/registry/flows/host/HostDeleteFlowTest.java @@ -84,6 +84,25 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase + + + + %HOSTNAME% + + + + diff --git a/javatests/google/registry/flows/host/testdata/host_delete_response_no_cltrid.xml b/javatests/google/registry/flows/host/testdata/host_delete_response_no_cltrid.xml new file mode 100644 index 000000000..7d1721ff9 --- /dev/null +++ b/javatests/google/registry/flows/host/testdata/host_delete_response_no_cltrid.xml @@ -0,0 +1,10 @@ + + + + Command completed successfully; action pending + + + server-trid + + + diff --git a/javatests/google/registry/rde/imports/RdeImportTestUtils.java b/javatests/google/registry/rde/imports/RdeImportTestUtils.java index de5887b1c..c3316f9be 100644 --- a/javatests/google/registry/rde/imports/RdeImportTestUtils.java +++ b/javatests/google/registry/rde/imports/RdeImportTestUtils.java @@ -15,6 +15,7 @@ package google.registry.rde.imports; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import google.registry.model.eppcommon.Trid; @@ -34,9 +35,10 @@ public class RdeImportTestUtils { */ public static void checkTrid(Trid trid) { assertThat(trid).isNotNull(); - assertThat(trid.getClientTransactionId()).isNotNull(); - assertThat(trid.getClientTransactionId().length()).isAtLeast(3); - assertThat(trid.getClientTransactionId().length()).isAtMost(64); - assertThat(trid.getClientTransactionId()).startsWith("Import_"); + assertThat(trid.getClientTransactionId()).isPresent(); + String clTrid = trid.getClientTransactionId().get(); + assertThat(clTrid.length()).isAtLeast(3); + assertThat(clTrid.length()).isAtMost(64); + assertThat(clTrid).startsWith("Import_"); } } diff --git a/javatests/google/registry/testing/TaskQueueHelper.java b/javatests/google/registry/testing/TaskQueueHelper.java index 8d3b4d89f..8aad0059c 100644 --- a/javatests/google/registry/testing/TaskQueueHelper.java +++ b/javatests/google/registry/testing/TaskQueueHelper.java @@ -15,6 +15,7 @@ package google.registry.testing; import static com.google.appengine.tools.development.testing.LocalTaskQueueTestConfig.getLocalTaskQueue; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Predicates.in; import static com.google.common.base.Predicates.not; @@ -104,8 +105,7 @@ public class TaskQueueHelper { } public TaskMatcher payload(String payload) { - checkState( - expected.params.isEmpty(), "Cannot add a payload to a TaskMatcher with params."); + checkState(expected.params.isEmpty(), "Cannot add a payload to a TaskMatcher with params"); expected.payload = payload; return this; } @@ -122,16 +122,16 @@ public class TaskQueueHelper { } public TaskMatcher param(String key, String value) { - checkState( - expected.payload == null, "Cannot add params to a TaskMatcher with a payload."); + checkState(expected.payload == null, "Cannot add params to a TaskMatcher with a payload"); + checkNotNull(value, "Test error: A task can never have a null value, so don't assert it"); expected.params.put(key, value); return this; } public TaskMatcher etaDelta(Duration lowerBound, Duration upperBound) { - checkState(!lowerBound.isShorterThan(Duration.ZERO), "lowerBound must be non-negative."); + checkState(!lowerBound.isShorterThan(Duration.ZERO), "lowerBound must be non-negative"); checkState( - upperBound.isLongerThan(lowerBound), "upperBound must be greater than lowerBound."); + upperBound.isLongerThan(lowerBound), "upperBound must be greater than lowerBound"); expected.etaDeltaLowerBound = lowerBound.getStandardSeconds(); expected.etaDeltaUpperBound = upperBound.getStandardSeconds(); return this;