From 931156fdd7e72db5cc4036d03ef56dabaf5ffe68 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Tue, 5 Dec 2017 14:35:37 -0800 Subject: [PATCH] Append event year to poll message external IDs This solves the problem of external poll message IDs not being globally unique by simply appending the event year. This means that autorenew poll messages will increment by one every year, so they will always be unique. This also requires no data schema changes, and thus most importantly, no data migration. Incoming requests lacking this new year field will continue to work for now for backwards compatibility reasons. This is possible because we don't actually use the year for anything. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=178012685 --- .../registry/flows/poll/PollAckFlow.java | 6 +- .../registry/flows/poll/PollRequestFlow.java | 4 +- .../registry/model/poll/PollMessage.java | 5 - .../poll/PollMessageExternalKeyConverter.java | 44 ++++--- .../flows/EppLifecycleContactTest.java | 2 +- .../flows/EppLifecycleDomainTest.java | 61 +++++++++- .../registry/flows/poll/PollAckFlowTest.java | 87 ++++++++++---- .../registry/flows/poll/testdata/poll_ack.xml | 2 +- .../flows/poll/testdata/poll_ack_contact.xml | 6 - .../poll_ack_invalid_eppresource_id.xml | 6 - .../poll/testdata/poll_ack_invalid_id.xml | 6 - .../testdata/poll_ack_invalid_string_id.xml | 6 - .../flows/poll/testdata/poll_ack_response.xml | 2 +- .../poll/testdata/poll_response_autorenew.xml | 2 +- .../testdata/poll_response_contact_delete.xml | 2 +- .../poll_response_contact_transfer.xml | 2 +- ...l_response_domain_pending_notification.xml | 2 +- .../poll_response_domain_transfer.xml | 2 +- .../testdata/poll_response_host_delete.xml | 2 +- .../testdata/poll_response_autorenew.xml | 22 ++++ .../poll_response_contact_transfer.xml | 2 +- .../poll_response_domain_transfer_request.xml | 2 +- ...e_domain_transfer_server_approve_loser.xml | 2 +- ..._domain_transfer_server_approve_winner.xml | 2 +- .../flows/testdata/poll_response_empty.xml | 11 ++ .../PollMessageExternalKeyConverterTest.java | 107 +++++++++++------- 26 files changed, 270 insertions(+), 127 deletions(-) delete mode 100644 javatests/google/registry/flows/poll/testdata/poll_ack_contact.xml delete mode 100644 javatests/google/registry/flows/poll/testdata/poll_ack_invalid_eppresource_id.xml delete mode 100644 javatests/google/registry/flows/poll/testdata/poll_ack_invalid_id.xml delete mode 100644 javatests/google/registry/flows/poll/testdata/poll_ack_invalid_string_id.xml create mode 100644 javatests/google/registry/flows/testdata/poll_response_autorenew.xml create mode 100644 javatests/google/registry/flows/testdata/poll_response_empty.xml diff --git a/java/google/registry/flows/poll/PollAckFlow.java b/java/google/registry/flows/poll/PollAckFlow.java index 90cd3bc74..991d1990f 100644 --- a/java/google/registry/flows/poll/PollAckFlow.java +++ b/java/google/registry/flows/poll/PollAckFlow.java @@ -19,6 +19,7 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.poll.PollFlowUtils.getPollMessagesQuery; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_NO_MESSAGES; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.model.poll.PollMessageExternalKeyConverter.parsePollMessageExternalId; import static google.registry.util.DateTimeUtils.isBeforeOrAt; import com.googlecode.objectify.Key; @@ -71,7 +72,7 @@ public class PollAckFlow implements TransactionalFlow { Key pollMessageKey; // Try parsing the messageId, and throw an exception if it's invalid. try { - pollMessageKey = PollMessage.EXTERNAL_KEY_CONVERTER.reverse().convert(messageId); + pollMessageKey = parsePollMessageExternalId(messageId); } catch (PollMessageExternalKeyParseException e) { throw new InvalidMessageIdException(messageId); } @@ -84,6 +85,9 @@ public class PollAckFlow implements TransactionalFlow { if (pollMessage == null || !isBeforeOrAt(pollMessage.getEventTime(), now)) { throw new MessageDoesNotExistException(messageId); } + // TODO(b/68953444): Once the year field on the external poll message ID becomes mandatory, add + // a check that the value of the year field is correct, by checking that + // makePollMessageExternalId(pollMessage) equals messageId. // Make sure this client is authorized to ack this message. It could be that the message is // supposed to go to a different registrar. diff --git a/java/google/registry/flows/poll/PollRequestFlow.java b/java/google/registry/flows/poll/PollRequestFlow.java index 78b047c0b..970395d29 100644 --- a/java/google/registry/flows/poll/PollRequestFlow.java +++ b/java/google/registry/flows/poll/PollRequestFlow.java @@ -18,8 +18,8 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.poll.PollFlowUtils.getPollMessagesQuery; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACK_MESSAGE; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_NO_MESSAGES; +import static google.registry.model.poll.PollMessageExternalKeyConverter.makePollMessageExternalId; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.EppException.ParameterValueSyntaxErrorException; import google.registry.flows.ExtensionManager; @@ -73,7 +73,7 @@ public class PollRequestFlow implements Flow { .setQueueDate(pollMessage.getEventTime()) .setMsg(pollMessage.getMsg()) .setQueueLength(getPollMessagesQuery(clientId, now).count()) - .setMessageId(PollMessage.EXTERNAL_KEY_CONVERTER.convert(Key.create(pollMessage))) + .setMessageId(makePollMessageExternalId(pollMessage)) .build()) .setMultipleResData(pollMessage.getResponseData()) .setExtensions(pollMessage.getResponseExtensions()) diff --git a/java/google/registry/model/poll/PollMessage.java b/java/google/registry/model/poll/PollMessage.java index 9211c57d3..85232805b 100644 --- a/java/google/registry/model/poll/PollMessage.java +++ b/java/google/registry/model/poll/PollMessage.java @@ -21,7 +21,6 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Converter; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.Streams; @@ -75,10 +74,6 @@ import org.joda.time.DateTime; public abstract class PollMessage extends ImmutableObject implements Buildable, TransferServerApproveEntity { - - public static final Converter, String> EXTERNAL_KEY_CONVERTER = - new PollMessageExternalKeyConverter(); - /** Entity id. */ @Id long id; diff --git a/java/google/registry/model/poll/PollMessageExternalKeyConverter.java b/java/google/registry/model/poll/PollMessageExternalKeyConverter.java index d800d7644..b2b4ec3bd 100644 --- a/java/google/registry/model/poll/PollMessageExternalKeyConverter.java +++ b/java/google/registry/model/poll/PollMessageExternalKeyConverter.java @@ -16,7 +16,6 @@ package google.registry.model.poll; import static google.registry.model.ofy.ObjectifyService.ofy; -import com.google.common.base.Converter; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableBiMap; import com.googlecode.objectify.Key; @@ -31,7 +30,7 @@ import java.util.List; * A converter between external key strings for {@link PollMessage}s (i.e. what registrars use to * identify and ACK them) and Datastore keys to the resource. * - *

The format of the key string is A-B-C-D-E as follows: + *

The format of the key string is A-B-C-D-E-F as follows: * * 

  *   A = EppResource.typeId (decimal)
@@ -39,9 +38,12 @@ import java.util.List;
  *   C = EppResource.repoId suffix (STRING)
  *   D = HistoryEntry.id (decimal)
  *   E = PollMessage.id (decimal)
+ *   F = PollMessage.eventTime (decimal, year only)
  *
+ * + *

A typical poll message ID might thus look like: 1-FE0F22-TLD-10071463070-10072612074-2018 */ -public class PollMessageExternalKeyConverter extends Converter, String> { +public class PollMessageExternalKeyConverter { /** An exception thrown when an external key cannot be parsed. */ public static class PollMessageExternalKeyParseException extends RuntimeException {} @@ -56,29 +58,38 @@ public class PollMessageExternalKeyConverter extends Converter, ContactResource.class, 2L, HostResource.class, 3L); - @Override - protected String doForward(Key key) { + /** Returns an external poll message ID for the given poll message. */ + public static String makePollMessageExternalId(PollMessage pollMessage) { @SuppressWarnings("unchecked") Key ancestorResource = - (Key) (Key) key.getParent().getParent(); - long externalKeyClassId = EXTERNAL_KEY_CLASS_ID_MAP.get( - ofy().factory().getMetadata(ancestorResource.getKind()).getEntityClass()); - return String.format("%d-%s-%d-%d", + (Key) (Key) pollMessage.getParentKey().getParent(); + long externalKeyClassId = + EXTERNAL_KEY_CLASS_ID_MAP.get( + ofy().factory().getMetadata(ancestorResource.getKind()).getEntityClass()); + return String.format( + "%d-%s-%d-%d-%d", externalKeyClassId, ancestorResource.getName(), - key.getParent().getId(), - key.getId()); + pollMessage.getParentKey().getId(), + pollMessage.getId(), + pollMessage.getEventTime().getYear()); } /** - * Returns an Objectify Key to a PollMessage corresponding with the external key string. + * Returns an Objectify Key to a PollMessage corresponding with the external ID. + * + *

Note that the year field that is included at the end of the poll message isn't actually + * used for anything; it exists solely to create unique externally visible IDs for autorenews. We + * thus ignore it (for now) for backwards compatibility reasons, so that registrars can still ACK + * existing poll message IDs they may have lying around. * * @throws PollMessageExternalKeyParseException if the external key has an invalid format. */ - @Override - protected Key doBackward(String externalKey) { + // TODO(b/68953444): Make the year field mandatory once sufficient time has elapsed and backwards + // compatibility is no longer necessary. + public static Key parsePollMessageExternalId(String externalKey) { List idComponents = Splitter.on('-').splitToList(externalKey); - if (idComponents.size() != 5) { + if (idComponents.size() != 5 && idComponents.size() != 6) { throw new PollMessageExternalKeyParseException(); } try { @@ -97,9 +108,12 @@ public class PollMessageExternalKeyConverter extends Converter, Long.parseLong(idComponents.get(3))), PollMessage.class, Long.parseLong(idComponents.get(4))); + // Note that idComponents.get(5) is entirely ignored; we never use the year field internally. } catch (NumberFormatException e) { throw new PollMessageExternalKeyParseException(); } } + + private PollMessageExternalKeyConverter() {} } diff --git a/javatests/google/registry/flows/EppLifecycleContactTest.java b/javatests/google/registry/flows/EppLifecycleContactTest.java index eda75cf00..73f801e44 100644 --- a/javatests/google/registry/flows/EppLifecycleContactTest.java +++ b/javatests/google/registry/flows/EppLifecycleContactTest.java @@ -114,7 +114,7 @@ public class EppLifecycleContactTest extends EppTestCase { .hasStatus(SUCCESS_WITH_ACK_MESSAGE); assertCommandAndResponse( "poll_ack.xml", - ImmutableMap.of("ID", "2-1-ROID-3-6"), + ImmutableMap.of("ID", "2-1-ROID-3-6-2000"), "poll_ack_response_empty.xml", null, DateTime.parse("2000-06-08T22:02:00Z")); diff --git a/javatests/google/registry/flows/EppLifecycleDomainTest.java b/javatests/google/registry/flows/EppLifecycleDomainTest.java index 8d0ce7904..c5085f77a 100644 --- a/javatests/google/registry/flows/EppLifecycleDomainTest.java +++ b/javatests/google/registry/flows/EppLifecycleDomainTest.java @@ -364,6 +364,61 @@ public class EppLifecycleDomainTest extends EppTestCase { assertCommandAndResponse("logout.xml", "logout_response.xml"); } + @Test + public void testDomainCreate_annualAutoRenewPollMessages_haveUniqueIds() throws Exception { + assertCommandAndResponse("login_valid.xml", "login_response.xml"); + // Create the domain. + createFakesite(); + + // The first autorenew poll message isn't seen until after the initial two years of registration + // are up. + assertCommandAndResponse( + "poll.xml", "poll_response_empty.xml", DateTime.parse("2001-01-01T00:01:00Z")); + assertCommandAndResponse( + "poll.xml", + ImmutableMap.of(), + "poll_response_autorenew.xml", + ImmutableMap.of( + "ID", "1-C-EXAMPLE-13-16-2002", + "QDATE", "2002-06-01T00:04:00Z", + "DOMAIN", "fakesite.example", + "EXDATE", "2003-06-01T00:04:00Z"), + DateTime.parse("2002-07-01T00:01:00Z")); + assertCommandAndResponse( + "poll_ack.xml", + ImmutableMap.of("ID", "1-C-EXAMPLE-13-16-2002"), + "poll_ack_response_empty.xml", + null, + DateTime.parse("2002-07-01T00:02:00Z")); + + // The second autorenew poll message isn't seen until after another year, and it should have a + // different ID. + assertCommandAndResponse( + "poll.xml", "poll_response_empty.xml", DateTime.parse("2002-07-01T00:05:00Z")); + assertCommandAndResponse( + "poll.xml", + ImmutableMap.of(), + "poll_response_autorenew.xml", + ImmutableMap.of( + "ID", "1-C-EXAMPLE-13-16-2003", // Note -- Year is different from previous ID. + "QDATE", "2003-06-01T00:04:00Z", + "DOMAIN", "fakesite.example", + "EXDATE", "2004-06-01T00:04:00Z"), + DateTime.parse("2003-07-01T00:05:00Z")); + + // Ack the second poll message and verify that none remain. + assertCommandAndResponse( + "poll_ack.xml", + ImmutableMap.of("ID", "1-C-EXAMPLE-13-16-2003"), + "poll_ack_response_empty.xml", + null, + DateTime.parse("2003-07-01T00:05:05Z")); + assertCommandAndResponse( + "poll.xml", "poll_response_empty.xml", DateTime.parse("2003-07-01T00:05:10Z")); + + assertCommandAndResponse("logout.xml", "logout_response.xml"); + } + @Test public void testDomainTransferPollMessage_serverApproved() throws Exception { // As the losing registrar, create the domain. @@ -391,7 +446,7 @@ public class EppLifecycleDomainTest extends EppTestCase { DateTime.parse("2001-01-01T00:01:00Z")); assertCommandAndResponse( "poll_ack.xml", - ImmutableMap.of("ID", "1-C-EXAMPLE-17-23"), + ImmutableMap.of("ID", "1-C-EXAMPLE-17-23-2001"), "poll_ack_response_empty.xml", null, DateTime.parse("2001-01-01T00:01:00Z")); @@ -403,7 +458,7 @@ public class EppLifecycleDomainTest extends EppTestCase { DateTime.parse("2001-01-06T00:01:00Z")); assertCommandAndResponse( "poll_ack.xml", - ImmutableMap.of("ID", "1-C-EXAMPLE-17-22"), + ImmutableMap.of("ID", "1-C-EXAMPLE-17-22-2001"), "poll_ack_response_empty.xml", null, DateTime.parse("2001-01-06T00:01:00Z")); @@ -419,7 +474,7 @@ public class EppLifecycleDomainTest extends EppTestCase { DateTime.parse("2001-01-06T00:02:00Z")); assertCommandAndResponse( "poll_ack.xml", - ImmutableMap.of("ID", "1-C-EXAMPLE-17-21"), + ImmutableMap.of("ID", "1-C-EXAMPLE-17-21-2001"), "poll_ack_response_empty.xml", null, DateTime.parse("2001-01-06T00:02:00Z")); diff --git a/javatests/google/registry/flows/poll/PollAckFlowTest.java b/javatests/google/registry/flows/poll/PollAckFlowTest.java index 34547a287..ab30213ea 100644 --- a/javatests/google/registry/flows/poll/PollAckFlowTest.java +++ b/javatests/google/registry/flows/poll/PollAckFlowTest.java @@ -14,13 +14,17 @@ package google.registry.flows.poll; +import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatastoreHelper.createHistoryEntryForEppResource; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.newDomainResource; import static google.registry.testing.DatastoreHelper.persistActiveContact; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.JUnitBackports.assertThrows; +import static google.registry.testing.JUnitBackports.expectThrows; import static google.registry.util.DateTimeUtils.END_OF_TIME; +import com.google.common.collect.ImmutableMap; import google.registry.flows.FlowTestCase; import google.registry.flows.poll.PollAckFlow.InvalidMessageIdException; import google.registry.flows.poll.PollAckFlow.MessageDoesNotExistException; @@ -36,7 +40,7 @@ import org.junit.Test; /** Unit tests for {@link PollAckFlow}. */ public class PollAckFlowTest extends FlowTestCase { - /** This is the message id contained in the "poll_ack.xml" test data file. */ + /** This is the message id being sent in the ACK request. */ private static final long MESSAGE_ID = 3; private DomainResource domain; @@ -44,7 +48,7 @@ public class PollAckFlowTest extends FlowTestCase { @Before public void setUp() { - setEppInput("poll_ack.xml"); + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2011")); setClientIdForFlow("NewRegistrar"); clock.setTo(DateTime.parse("2011-01-02T01:01:01Z")); createTld("example"); @@ -87,7 +91,38 @@ public class PollAckFlowTest extends FlowTestCase { @Test public void testSuccess_contactPollMessage() throws Exception { - setEppInput("poll_ack_contact.xml"); + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "2-2-ROID-4-3-2011")); + persistResource( + new PollMessage.OneTime.Builder() + .setId(MESSAGE_ID) + .setClientId(getClientIdForFlow()) + .setEventTime(clock.nowUtc().minusDays(1)) + .setMsg("Some poll message.") + .setParent(createHistoryEntryForEppResource(contact)) + .build()); + assertTransactionalFlow(true); + runFlowAssertResponse(loadFile("poll_ack_response_empty.xml")); + } + + // TODO(b/68953444): Remove test when missing year field backwards compatibility no longer needed. + @Test + public void testSuccess_contactPollMessage_withMissingYearField() throws Exception { + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "2-2-ROID-4-3")); + persistResource( + new PollMessage.OneTime.Builder() + .setId(MESSAGE_ID) + .setClientId(getClientIdForFlow()) + .setEventTime(clock.nowUtc().minusDays(1)) + .setMsg("Some poll message.") + .setParent(createHistoryEntryForEppResource(contact)) + .build()); + assertTransactionalFlow(true); + runFlowAssertResponse(loadFile("poll_ack_response_empty.xml")); + } + + @Test + public void testSuccess_contactPollMessage_withIncorrectYearField() throws Exception { + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "2-2-ROID-4-3-1999")); persistResource( new PollMessage.OneTime.Builder() .setId(MESSAGE_ID) @@ -146,42 +181,45 @@ public class PollAckFlowTest extends FlowTestCase { @Test public void testFailure_noSuchMessage() throws Exception { assertTransactionalFlow(true); - thrown.expect( - MessageDoesNotExistException.class, - String.format("(1-3-EXAMPLE-4-%d)", MESSAGE_ID)); - runFlow(); + Exception e = expectThrows(MessageDoesNotExistException.class, this::runFlow); + assertThat(e) + .hasMessageThat() + .containsMatch(String.format("(1-3-EXAMPLE-4-%d-2011)", MESSAGE_ID)); } @Test - public void testFailure_invalidId_wrongNumberOfComponents() throws Exception { - setEppInput("poll_ack_invalid_id.xml"); + public void testFailure_invalidId_tooFewComponents() throws Exception { + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-2-3")); assertTransactionalFlow(true); - thrown.expect(InvalidMessageIdException.class); - runFlow(); + assertThrows(InvalidMessageIdException.class, this::runFlow); + } + + @Test + public void testFailure_invalidId_tooManyComponents() throws Exception { + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "2-2-ROID-4-3-1999-2007")); + assertTransactionalFlow(true); + assertThrows(InvalidMessageIdException.class, this::runFlow); } @Test public void testFailure_invalidId_stringInsteadOfNumeric() throws Exception { - setEppInput("poll_ack_invalid_string_id.xml"); + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "ABC-12345")); assertTransactionalFlow(true); - thrown.expect(InvalidMessageIdException.class); - runFlow(); + assertThrows(InvalidMessageIdException.class, this::runFlow); } @Test public void testFailure_invalidEppResourceClassId() throws Exception { - setEppInput("poll_ack_invalid_eppresource_id.xml"); + setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "999-1-1-1")); assertTransactionalFlow(true); - thrown.expect(InvalidMessageIdException.class); - runFlow(); + assertThrows(InvalidMessageIdException.class, this::runFlow); } @Test public void testFailure_missingId() throws Exception { setEppInput("poll_ack_missing_id.xml"); assertTransactionalFlow(true); - thrown.expect(MissingMessageIdException.class); - runFlow(); + assertThrows(MissingMessageIdException.class, this::runFlow); } @Test @@ -195,8 +233,7 @@ public class PollAckFlowTest extends FlowTestCase { .setParent(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(true); - thrown.expect(NotAuthorizedToAckMessageException.class); - runFlow(); + assertThrows(NotAuthorizedToAckMessageException.class, this::runFlow); } @Test @@ -210,9 +247,9 @@ public class PollAckFlowTest extends FlowTestCase { .setParent(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(true); - thrown.expect( - MessageDoesNotExistException.class, - String.format("(1-3-EXAMPLE-4-%d)", MESSAGE_ID)); - runFlow(); + Exception e = expectThrows(MessageDoesNotExistException.class, this::runFlow); + assertThat(e) + .hasMessageThat() + .containsMatch(String.format("(1-3-EXAMPLE-4-%d-2011)", MESSAGE_ID)); } } diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack.xml b/javatests/google/registry/flows/poll/testdata/poll_ack.xml index 29eca3a8a..d7fbb2bcb 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_ack.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_ack.xml @@ -1,6 +1,6 @@ - + ABC-12346 diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack_contact.xml b/javatests/google/registry/flows/poll/testdata/poll_ack_contact.xml deleted file mode 100644 index 43dd05a26..000000000 --- a/javatests/google/registry/flows/poll/testdata/poll_ack_contact.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - ABC-12346 - - diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_eppresource_id.xml b/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_eppresource_id.xml deleted file mode 100644 index 9573736fa..000000000 --- a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_eppresource_id.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - ABC-12346 - - diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_id.xml b/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_id.xml deleted file mode 100644 index f962b1d1a..000000000 --- a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_id.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - ABC-12346 - - diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_string_id.xml b/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_string_id.xml deleted file mode 100644 index fbe9ddf5c..000000000 --- a/javatests/google/registry/flows/poll/testdata/poll_ack_invalid_string_id.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - ABC-12346 - - diff --git a/javatests/google/registry/flows/poll/testdata/poll_ack_response.xml b/javatests/google/registry/flows/poll/testdata/poll_ack_response.xml index 58910dfd3..69183a40a 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_ack_response.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_ack_response.xml @@ -3,7 +3,7 @@ Command completed successfully - + ABC-12346 server-trid diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_autorenew.xml b/javatests/google/registry/flows/poll/testdata/poll_response_autorenew.xml index d3226de88..b5f056fbd 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_autorenew.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_autorenew.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2011-01-01T01:01:01Z Domain was auto-renewed. diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_contact_delete.xml b/javatests/google/registry/flows/poll/testdata/poll_response_contact_delete.xml index 5cdaf200e..43c908ceb 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_contact_delete.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_contact_delete.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2011-01-01T01:01:01Z Deleted contact jd1234 diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_contact_transfer.xml b/javatests/google/registry/flows/poll/testdata/poll_response_contact_transfer.xml index be7ded758..7dfdb112f 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_contact_transfer.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_contact_transfer.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2000-06-08T22:00:00Z Transfer requested. diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_domain_pending_notification.xml b/javatests/google/registry/flows/poll/testdata/poll_response_domain_pending_notification.xml index 14345ff4d..ca1bac781 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_domain_pending_notification.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_domain_pending_notification.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2011-01-01T01:01:01Z Domain deleted. diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer.xml b/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer.xml index f4ede359c..f60f65892 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2011-01-01T01:01:01Z Transfer approved. diff --git a/javatests/google/registry/flows/poll/testdata/poll_response_host_delete.xml b/javatests/google/registry/flows/poll/testdata/poll_response_host_delete.xml index 20263d1fe..a5ef2e8f2 100644 --- a/javatests/google/registry/flows/poll/testdata/poll_response_host_delete.xml +++ b/javatests/google/registry/flows/poll/testdata/poll_response_host_delete.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2011-01-01T01:01:01Z Deleted host ns1.test.example diff --git a/javatests/google/registry/flows/testdata/poll_response_autorenew.xml b/javatests/google/registry/flows/testdata/poll_response_autorenew.xml new file mode 100644 index 000000000..174ac9a53 --- /dev/null +++ b/javatests/google/registry/flows/testdata/poll_response_autorenew.xml @@ -0,0 +1,22 @@ + + + + Command completed successfully; ack to dequeue + + + %QDATE% + Domain was auto-renewed. + + + + %DOMAIN% + %EXDATE% + + + + ABC-12345 + server-trid + + + diff --git a/javatests/google/registry/flows/testdata/poll_response_contact_transfer.xml b/javatests/google/registry/flows/testdata/poll_response_contact_transfer.xml index 641961da3..a9ce57551 100644 --- a/javatests/google/registry/flows/testdata/poll_response_contact_transfer.xml +++ b/javatests/google/registry/flows/testdata/poll_response_contact_transfer.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2000-06-08T22:00:00Z Transfer requested. diff --git a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_request.xml b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_request.xml index d833b884b..0f19e13b6 100644 --- a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_request.xml +++ b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_request.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2001-01-01T00:00:00Z Transfer requested. diff --git a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_loser.xml b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_loser.xml index 173a77dda..a12a093f3 100644 --- a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_loser.xml +++ b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_loser.xml @@ -3,7 +3,7 @@ Command completed successfully; ack to dequeue - + 2001-01-06T00:00:00Z Transfer approved. diff --git a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_winner.xml b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_winner.xml index 270222758..bf4411bff 100644 --- a/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_winner.xml +++ b/javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_winner.xml @@ -4,7 +4,7 @@ Command completed successfully; ack to dequeue - + 2001-01-06T00:00:00Z Transfer approved. diff --git a/javatests/google/registry/flows/testdata/poll_response_empty.xml b/javatests/google/registry/flows/testdata/poll_response_empty.xml new file mode 100644 index 000000000..85911436a --- /dev/null +++ b/javatests/google/registry/flows/testdata/poll_response_empty.xml @@ -0,0 +1,11 @@ + + + + Command completed successfully; no messages + + + ABC-12345 + server-trid + + + diff --git a/javatests/google/registry/model/poll/PollMessageExternalKeyConverterTest.java b/javatests/google/registry/model/poll/PollMessageExternalKeyConverterTest.java index 86b6cd80c..e293aea6d 100644 --- a/javatests/google/registry/model/poll/PollMessageExternalKeyConverterTest.java +++ b/javatests/google/registry/model/poll/PollMessageExternalKeyConverterTest.java @@ -15,13 +15,15 @@ package google.registry.model.poll; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.poll.PollMessageExternalKeyConverter.makePollMessageExternalId; +import static google.registry.model.poll.PollMessageExternalKeyConverter.parsePollMessageExternalId; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.persistActiveContact; import static google.registry.testing.DatastoreHelper.persistActiveDomain; import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.JUnitBackports.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.joda.time.DateTimeZone.UTC; import com.googlecode.objectify.Key; import google.registry.model.domain.Period; @@ -56,8 +58,7 @@ public class PollMessageExternalKeyConverterTest { public final ExceptionRule thrown = new ExceptionRule(); HistoryEntry historyEntry; - FakeClock clock = new FakeClock(DateTime.now(UTC)); - PollMessageExternalKeyConverter converter = new PollMessageExternalKeyConverter(); + FakeClock clock = new FakeClock(DateTime.parse("2007-07-07T01:01:01Z")); @Before public void setUp() throws Exception { @@ -79,48 +80,65 @@ public class PollMessageExternalKeyConverterTest { @Test public void testSuccess_domain() { - PollMessage.OneTime pollMessage = persistResource( - new PollMessage.OneTime.Builder() - .setClientId("TheRegistrar") - .setEventTime(clock.nowUtc()) - .setMsg("Test poll message") - .setParent(historyEntry) - .build()); - Key key = Key.create(pollMessage); - assertThat(converter.convert(key)).isEqualTo("1-2-FOOBAR-4-5"); - assertThat(converter.reverse().convert("1-2-FOOBAR-4-5")).isEqualTo(key); + PollMessage.OneTime pollMessage = + persistResource( + new PollMessage.OneTime.Builder() + .setClientId("TheRegistrar") + .setEventTime(clock.nowUtc()) + .setMsg("Test poll message") + .setParent(historyEntry) + .build()); + assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("1-2-FOOBAR-4-5-2007"); + assertThat(parsePollMessageExternalId("1-2-FOOBAR-4-5-2007")) + .isEqualTo(Key.create(pollMessage)); } @Test public void testSuccess_contact() { historyEntry = persistResource(historyEntry.asBuilder().setParent(persistActiveContact("tim")).build()); - PollMessage.OneTime pollMessage = persistResource( - new PollMessage.OneTime.Builder() - .setClientId("TheRegistrar") - .setEventTime(clock.nowUtc()) - .setMsg("Test poll message") - .setParent(historyEntry) - .build()); - Key key = Key.create(pollMessage); - assertThat(converter.convert(key)).isEqualTo("2-5-ROID-4-6"); - assertThat(converter.reverse().convert("2-5-ROID-4-6")).isEqualTo(key); + PollMessage.OneTime pollMessage = + persistResource( + new PollMessage.OneTime.Builder() + .setClientId("TheRegistrar") + .setEventTime(clock.nowUtc()) + .setMsg("Test poll message") + .setParent(historyEntry) + .build()); + assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("2-5-ROID-4-6-2007"); + assertThat(parsePollMessageExternalId("2-5-ROID-4-6-2007")).isEqualTo(Key.create(pollMessage)); } @Test public void testSuccess_host() { historyEntry = persistResource(historyEntry.asBuilder().setParent(persistActiveHost("time.zyx")).build()); - PollMessage.OneTime pollMessage = persistResource( - new PollMessage.OneTime.Builder() - .setClientId("TheRegistrar") - .setEventTime(clock.nowUtc()) - .setMsg("Test poll message") - .setParent(historyEntry) - .build()); - Key key = Key.create(pollMessage); - assertThat(converter.convert(key)).isEqualTo("3-5-ROID-4-6"); - assertThat(converter.reverse().convert("3-5-ROID-4-6")).isEqualTo(key); + PollMessage.OneTime pollMessage = + persistResource( + new PollMessage.OneTime.Builder() + .setClientId("TheRegistrar") + .setEventTime(clock.nowUtc()) + .setMsg("Test poll message") + .setParent(historyEntry) + .build()); + assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("3-5-ROID-4-6-2007"); + assertThat(parsePollMessageExternalId("3-5-ROID-4-6-2007")).isEqualTo(Key.create(pollMessage)); + } + + @Test + // TODO(b/68953444): Remove leniency when backwards compatibility with missing field is no longer + // required. + public void testSuccess_stillParsesWithMissingYearField() { + PollMessage.OneTime pollMessage = + persistResource( + new PollMessage.OneTime.Builder() + .setClientId("TheRegistrar") + .setEventTime(clock.nowUtc()) + .setMsg("Test poll message") + .setParent(historyEntry) + .build()); + assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("1-2-FOOBAR-4-5-2007"); + assertThat(parsePollMessageExternalId("1-2-FOOBAR-4-5")).isEqualTo(Key.create(pollMessage)); } @Test @@ -128,19 +146,30 @@ public class PollMessageExternalKeyConverterTest { // Populate the testdata correctly as for 1-2-FOOBAR-4-5 so we know that the only thing that // is wrong here is the EppResourceTypeId. testSuccess_domain(); - thrown.expect(PollMessageExternalKeyParseException.class); - converter.reverse().convert("4-2-FOOBAR-4-5"); + assertThrows( + PollMessageExternalKeyParseException.class, + () -> parsePollMessageExternalId("4-2-FOOBAR-4-5-2007")); } @Test public void testFailure_tooFewComponentParts() throws Exception { - thrown.expect(PollMessageExternalKeyParseException.class); - converter.reverse().convert("1-3-EXAMPLE"); + assertThrows( + PollMessageExternalKeyParseException.class, + () -> parsePollMessageExternalId("1-3-EXAMPLE")); } + @Test + public void testFailure_tooManyComponentParts() throws Exception { + assertThrows( + PollMessageExternalKeyParseException.class, + () -> parsePollMessageExternalId("1-3-EXAMPLE-4-5-2007-2009")); + } + + @Test public void testFailure_nonNumericIds() throws Exception { - thrown.expect(PollMessageExternalKeyParseException.class); - converter.reverse().convert("A-B-FOOBAR-D-E"); + assertThrows( + PollMessageExternalKeyParseException.class, + () -> parsePollMessageExternalId("A-B-FOOBAR-D-E-F")); } }