diff --git a/java/google/registry/flows/FlowModule.java b/java/google/registry/flows/FlowModule.java index ac16c18fe..057524e85 100644 --- a/java/google/registry/flows/FlowModule.java +++ b/java/google/registry/flows/FlowModule.java @@ -156,7 +156,8 @@ public class FlowModule { @FlowScope Trid provideTrid(EppInput eppInput, ServerTridProvider serverTridProvider) { return Trid.create( - eppInput.getCommandWrapper().getClTrid(), serverTridProvider.createServerTrid()); + eppInput.getCommandWrapper().getClTrid().orElse(null), + serverTridProvider.createServerTrid()); } @Provides diff --git a/java/google/registry/flows/async/AsyncFlowEnqueuer.java b/java/google/registry/flows/async/AsyncFlowEnqueuer.java index 93e9d1aae..c311d421b 100644 --- a/java/google/registry/flows/async/AsyncFlowEnqueuer.java +++ b/java/google/registry/flows/async/AsyncFlowEnqueuer.java @@ -124,9 +124,8 @@ public final class AsyncFlowEnqueuer { .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()); - } + trid.getClientTransactionId() + .ifPresent(clTrid -> task.param(PARAM_CLIENT_TRANSACTION_ID, clTrid)); addTaskToQueueWithRetry(asyncDeletePullQueue, task); } diff --git a/java/google/registry/model/eppcommon/Trid.java b/java/google/registry/model/eppcommon/Trid.java index ba3a0a175..0dc6d17db 100644 --- a/java/google/registry/model/eppcommon/Trid.java +++ b/java/google/registry/model/eppcommon/Trid.java @@ -16,7 +16,6 @@ package google.registry.model.eppcommon; 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; @@ -40,6 +39,7 @@ public class Trid extends ImmutableObject { /** The client transaction id, if provided by the client, otherwise null. */ @XmlElement(name = "clTRID", namespace = "urn:ietf:params:xml:ns:epp-1.0") + @Nullable String clientTransactionId; public String getServerTransactionId() { @@ -50,7 +50,6 @@ public class Trid extends ImmutableObject { return Optional.ofNullable(clientTransactionId); } - @VisibleForTesting public static Trid create(@Nullable String clientTransactionId, String serverTransactionId) { checkArgumentNotNull(serverTransactionId, "serverTransactionId cannot be null"); Trid instance = new Trid(); diff --git a/java/google/registry/model/eppinput/EppInput.java b/java/google/registry/model/eppinput/EppInput.java index bb9924fa7..5d127462d 100644 --- a/java/google/registry/model/eppinput/EppInput.java +++ b/java/google/registry/model/eppinput/EppInput.java @@ -366,10 +366,15 @@ public class EppInput extends ImmutableObject { @XmlElementWrapper List extension; - String clTRID; + @Nullable String clTRID; - public String getClTrid() { - return clTRID; + /** + * Returns the client transaction ID. + * + *

This is optional (i.e. it may not be specified) per RFC 5730. + */ + public Optional getClTrid() { + return Optional.ofNullable(clTRID); } public InnerCommand getCommand() { diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 409af27f1..e1a5c0df6 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -63,6 +63,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.function.Function; +import javax.annotation.Nullable; import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; @@ -141,8 +142,9 @@ public abstract class FlowTestCase extends ShardableTestCase { return TestDataHelper.loadFile(getClass(), filename, substitutions); } + @Nullable protected String getClientTrid() throws Exception { - return eppLoader.getEpp().getCommandWrapper().getClTrid(); + return eppLoader.getEpp().getCommandWrapper().getClTrid().orElse(null); } /** Gets the client ID that the flow will run as. */ diff --git a/javatests/google/registry/flows/ResourceFlowTestCase.java b/javatests/google/registry/flows/ResourceFlowTestCase.java index 488973f89..9a0f19478 100644 --- a/javatests/google/registry/flows/ResourceFlowTestCase.java +++ b/javatests/google/registry/flows/ResourceFlowTestCase.java @@ -170,9 +170,8 @@ public abstract class ResourceFlowTestCase expected.param("clientTransactionId", clTrid)); assertTasksEnqueued("async-delete-pull", expected); } diff --git a/javatests/google/registry/flows/domain/DomainCheckFlowTest.java b/javatests/google/registry/flows/domain/DomainCheckFlowTest.java index 5e3b00255..7bd06e453 100644 --- a/javatests/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCheckFlowTest.java @@ -114,6 +114,16 @@ public class DomainCheckFlowTest create(true, "example3.tld", null)); } + @Test + public void testSuccess_clTridNotSpecified() throws Exception { + setEppInput("domain_check_no_cltrid.xml"); + persistActiveDomain("example1.tld"); + doCheckTest( + create(false, "example1.tld", "In use"), + create(true, "example2.tld", null), + create(true, "example3.tld", null)); + } + @Test public void testSuccess_oneExists_allocationTokenIsInvalid() throws Exception { setEppInput("domain_check_allocationtoken.xml"); diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 111c3b5aa..e438fe9a5 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -393,6 +393,13 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase + + + + example1.tld + example2.tld + example3.tld + + + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_create_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/domain_create_no_cltrid.xml new file mode 100644 index 000000000..3c3e0516d --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_create_no_cltrid.xml @@ -0,0 +1,21 @@ + + + + + example.tld + 2 + + ns1.example.net + ns2.example.net + + jd1234 + sh8013 + sh8013 + + 2fooBAR + + + + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_create_response_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/domain_create_response_no_cltrid.xml new file mode 100644 index 000000000..2b6b3c65a --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_create_response_no_cltrid.xml @@ -0,0 +1,18 @@ + + + + Command completed successfully + + + + example.tld + 1999-04-03T22:00:00.0Z + 2001-04-03T22:00:00.0Z + + + + server-trid + + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_info_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/domain_info_no_cltrid.xml new file mode 100644 index 000000000..74ed06c2c --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_info_no_cltrid.xml @@ -0,0 +1,10 @@ + + + + + example.tld + + + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_info_response_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/domain_info_response_no_cltrid.xml new file mode 100644 index 000000000..79e4b4497 --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_info_response_no_cltrid.xml @@ -0,0 +1,37 @@ + + + + Command completed successfully + + + + example.tld + %ROID% + + jd1234 + sh8013 + sh8013 + + ns1.example.tld + ns1.example.net + + ns1.example.tld + ns2.example.tld + NewRegistrar + TheRegistrar + 1999-04-03T22:00:00.0Z + NewRegistrar + 1999-12-03T09:00:00.0Z + 2005-04-03T22:00:00.0Z + 2000-04-08T09:00:00.0Z + + 2fooBAR + + + + + server-trid + + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_update_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/domain_update_no_cltrid.xml new file mode 100644 index 000000000..f66cc8e5b --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_update_no_cltrid.xml @@ -0,0 +1,31 @@ + + + + + example.tld + + + ns2.example.foo + + mak21 + Payment overdue. + + + + ns1.example.foo + + sh8013 + + + + sh8013 + + 2BARfoo + + + + + + diff --git a/javatests/google/registry/flows/domain/testdata/generic_success_response_no_cltrid.xml b/javatests/google/registry/flows/domain/testdata/generic_success_response_no_cltrid.xml new file mode 100644 index 000000000..32dd8b167 --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/generic_success_response_no_cltrid.xml @@ -0,0 +1,10 @@ + + + + Command completed successfully + + + server-trid + + + diff --git a/javatests/google/registry/flows/poll/PollRequestFlowTest.java b/javatests/google/registry/flows/poll/PollRequestFlowTest.java index 5e820ecae..bad4886ad 100644 --- a/javatests/google/registry/flows/poll/PollRequestFlowTest.java +++ b/javatests/google/registry/flows/poll/PollRequestFlowTest.java @@ -59,28 +59,42 @@ public class PollRequestFlowTest extends FlowTestCase { host = persistActiveHost("ns1.test.example"); } - @Test - public void testSuccess_domainTransferApproved() throws Exception { + private void persistPendingTransferPollMessage() { persistResource( new PollMessage.OneTime.Builder() .setClientId(getClientIdForFlow()) .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Transfer approved.") - .setResponseData(ImmutableList.of(new DomainTransferResponse.Builder() - .setFullyQualifiedDomainName("test.example") - .setTransferStatus(TransferStatus.SERVER_APPROVED) - .setGainingClientId(getClientIdForFlow()) - .setTransferRequestTime(clock.nowUtc().minusDays(5)) - .setLosingClientId("TheRegistrar") - .setPendingTransferExpirationTime(clock.nowUtc().minusDays(1)) - .setExtendedRegistrationExpirationTime(clock.nowUtc().plusYears(1)) - .build())) + .setResponseData( + ImmutableList.of( + new DomainTransferResponse.Builder() + .setFullyQualifiedDomainName("test.example") + .setTransferStatus(TransferStatus.SERVER_APPROVED) + .setGainingClientId(getClientIdForFlow()) + .setTransferRequestTime(clock.nowUtc().minusDays(5)) + .setLosingClientId("TheRegistrar") + .setPendingTransferExpirationTime(clock.nowUtc().minusDays(1)) + .setExtendedRegistrationExpirationTime(clock.nowUtc().plusYears(1)) + .build())) .setParent(createHistoryEntryForEppResource(domain)) .build()); + } + + @Test + public void testSuccess_domainTransferApproved() throws Exception { + persistPendingTransferPollMessage(); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_response_domain_transfer.xml")); } + @Test + public void testSuccess_clTridNotSpecified() throws Exception { + setEppInput("poll_no_cltrid.xml"); + persistPendingTransferPollMessage(); + assertTransactionalFlow(false); + runFlowAssertResponse(loadFile("poll_response_domain_transfer_no_cltrid.xml")); + } + @Test public void testSuccess_contactTransferPending() throws Exception { clock.setTo(DateTime.parse("2000-06-13T22:00:00.0Z")); diff --git a/javatests/google/registry/flows/poll/testdata/poll_no_cltrid.xml b/javatests/google/registry/flows/poll/testdata/poll_no_cltrid.xml new file mode 100644 index 000000000..d00ad8534 --- /dev/null +++ b/javatests/google/registry/flows/poll/testdata/poll_no_cltrid.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer_no_cltrid.xml b/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer_no_cltrid.xml new file mode 100644 index 000000000..0525a5c8a --- /dev/null +++ b/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer_no_cltrid.xml @@ -0,0 +1,25 @@ + + + + Command completed successfully; ack to dequeue + + + 2011-01-01T01:01:01Z + Transfer approved. + + + + test.example + serverApproved + NewRegistrar + 2010-12-28T01:01:01Z + TheRegistrar + 2011-01-01T01:01:01Z + 2012-01-02T01:01:01Z + + + + server-trid + + + diff --git a/javatests/google/registry/model/eppinput/EppInputTest.java b/javatests/google/registry/model/eppinput/EppInputTest.java index f7bc46b3e..6a3084783 100644 --- a/javatests/google/registry/model/eppinput/EppInputTest.java +++ b/javatests/google/registry/model/eppinput/EppInputTest.java @@ -35,7 +35,7 @@ public class EppInputTest { public void testUnmarshalling_contactInfo() throws Exception { EppInput input = unmarshal(EppInput.class, loadBytes(ContactResourceTest.class, "contact_info.xml").read()); - assertThat(input.getCommandWrapper().getClTrid()).isEqualTo("ABC-12345"); + assertThat(input.getCommandWrapper().getClTrid()).hasValue("ABC-12345"); assertThat(input.getCommandType()).isEqualTo("info"); assertThat(input.getResourceType()).hasValue("contact"); assertThat(input.getSingleTargetId()).hasValue("sh8013"); @@ -46,7 +46,7 @@ public class EppInputTest { public void testUnmarshalling_domainCheck() throws Exception { EppInput input = unmarshal(EppInput.class, loadBytes(DomainResourceTest.class, "domain_check.xml").read()); - assertThat(input.getCommandWrapper().getClTrid()).isEqualTo("ABC-12345"); + assertThat(input.getCommandWrapper().getClTrid()).hasValue("ABC-12345"); assertThat(input.getCommandType()).isEqualTo("check"); assertThat(input.getResourceType()).hasValue("domain"); assertThat(input.getSingleTargetId()).isEmpty(); @@ -56,7 +56,7 @@ public class EppInputTest { @Test public void testUnmarshalling_login() throws Exception { EppInput input = unmarshal(EppInput.class, loadBytes(getClass(), "login_valid.xml").read()); - assertThat(input.getCommandWrapper().getClTrid()).isEqualTo("ABC-12345"); + assertThat(input.getCommandWrapper().getClTrid()).hasValue("ABC-12345"); assertThat(input.getCommandType()).isEqualTo("login"); assertThat(input.getResourceType()).isEmpty(); assertThat(input.getSingleTargetId()).isEmpty();