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

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<PollMessage> 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.

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())

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<Key<PollMessage>, String> EXTERNAL_KEY_CONVERTER =
-      new PollMessageExternalKeyConverter();
-
   /** Entity id. */
   @Id
   long id;

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.
  *
- * <p>The format of the key string is A-B-C-D-E as follows:
+ * <p>The format of the key string is A-B-C-D-E-F as follows:
  *
  * <pre>
  *   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)
  * </pre>
+ *
+ * <p>A typical poll message ID might thus look like: 1-FE0F22-TLD-10071463070-10072612074-2018
  */
-public class PollMessageExternalKeyConverter extends Converter<Key<PollMessage>, 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<Key<PollMessage>,
           ContactResource.class, 2L,
           HostResource.class, 3L);
 
-  @Override
+  /** Returns an external poll message ID for the given poll message. */
-  protected String doForward(Key<PollMessage> key) {
+  public static String makePollMessageExternalId(PollMessage pollMessage) {
     @SuppressWarnings("unchecked")
     Key<EppResource> ancestorResource =
-        (Key<EppResource>) (Key<?>) key.getParent().getParent();
+        (Key<EppResource>) (Key<?>) pollMessage.getParentKey().getParent();
-    long externalKeyClassId = EXTERNAL_KEY_CLASS_ID_MAP.get(
+    long externalKeyClassId =
-        ofy().factory().getMetadata(ancestorResource.getKind()).getEntityClass());
+        EXTERNAL_KEY_CLASS_ID_MAP.get(
-    return String.format("%d-%s-%d-%d",
+            ofy().factory().getMetadata(ancestorResource.getKind()).getEntityClass());
+    return String.format(
+        "%d-%s-%d-%d-%d",
         externalKeyClassId,
         ancestorResource.getName(),
-        key.getParent().getId(),
+        pollMessage.getParentKey().getId(),
-        key.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.
+   *
+   * <p>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
+  // TODO(b/68953444): Make the year field mandatory once sufficient time has elapsed and backwards
-  protected Key<PollMessage> doBackward(String externalKey) {
+  //                   compatibility is no longer necessary.
+  public static Key<PollMessage> parsePollMessageExternalId(String externalKey) {
     List<String> 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<Key<PollMessage>,
               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() {}
 }
 

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"));

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"));

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<PollAckFlow> {
 
-  /** 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<PollAckFlow> {
 
   @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<PollAckFlow> {
 
   @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<PollAckFlow> {
   @Test
   public void testFailure_noSuchMessage() throws Exception {
     assertTransactionalFlow(true);
-    thrown.expect(
+    Exception e = expectThrows(MessageDoesNotExistException.class, this::runFlow);
-        MessageDoesNotExistException.class,
+    assertThat(e)
-        String.format("(1-3-EXAMPLE-4-%d)", MESSAGE_ID));
+        .hasMessageThat()
-    runFlow();
+        .containsMatch(String.format("(1-3-EXAMPLE-4-%d-2011)", MESSAGE_ID));
   }
 
   @Test
-  public void testFailure_invalidId_wrongNumberOfComponents() throws Exception {
+  public void testFailure_invalidId_tooFewComponents() throws Exception {
-    setEppInput("poll_ack_invalid_id.xml");
+    setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-2-3"));
     assertTransactionalFlow(true);
-    thrown.expect(InvalidMessageIdException.class);
+    assertThrows(InvalidMessageIdException.class, this::runFlow);
-    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);
+    assertThrows(InvalidMessageIdException.class, this::runFlow);
-    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);
+    assertThrows(InvalidMessageIdException.class, this::runFlow);
-    runFlow();
   }
 
   @Test
   public void testFailure_missingId() throws Exception {
     setEppInput("poll_ack_missing_id.xml");
     assertTransactionalFlow(true);
-    thrown.expect(MissingMessageIdException.class);
+    assertThrows(MissingMessageIdException.class, this::runFlow);
-    runFlow();
   }
 
   @Test
@@ -195,8 +233,7 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
             .setParent(createHistoryEntryForEppResource(domain))
             .build());
     assertTransactionalFlow(true);
-    thrown.expect(NotAuthorizedToAckMessageException.class);
+    assertThrows(NotAuthorizedToAckMessageException.class, this::runFlow);
-    runFlow();
   }
 
   @Test
@@ -210,9 +247,9 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
             .setParent(createHistoryEntryForEppResource(domain))
             .build());
     assertTransactionalFlow(true);
-    thrown.expect(
+    Exception e = expectThrows(MessageDoesNotExistException.class, this::runFlow);
-        MessageDoesNotExistException.class,
+    assertThat(e)
-        String.format("(1-3-EXAMPLE-4-%d)", MESSAGE_ID));
+        .hasMessageThat()
-    runFlow();
+        .containsMatch(String.format("(1-3-EXAMPLE-4-%d-2011)", MESSAGE_ID));
   }
 }

javatests/google/registry/flows/poll/testdata/poll_ack.xml

@@ -1,6 +1,6 @@
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
   <command>
-    <poll op="ack" msgID="1-3-EXAMPLE-4-3"/>
+    <poll op="ack" msgID="%MSGID%"/>
     <clTRID>ABC-12346</clTRID>
   </command>
 </epp>

javatests/google/registry/flows/poll/testdata/poll_ack_contact.xml

@@ -1,6 +0,0 @@
-<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
-  <command>
-    <poll op="ack" msgID="2-2-ROID-4-3"/>
-    <clTRID>ABC-12346</clTRID>
-  </command>
-</epp>

javatests/google/registry/flows/poll/testdata/poll_ack_invalid_eppresource_id.xml

@@ -1,6 +0,0 @@
-<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
-  <command>
-    <poll op="ack" msgID="999-1-1-1"/>
-    <clTRID>ABC-12346</clTRID>
-  </command>
-</epp>

javatests/google/registry/flows/poll/testdata/poll_ack_invalid_id.xml

@@ -1,6 +0,0 @@
-<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
-  <command>
-    <poll op="ack" msgID="1-2-3"/>
-    <clTRID>ABC-12346</clTRID>
-  </command>
-</epp>

javatests/google/registry/flows/poll/testdata/poll_ack_invalid_string_id.xml

@@ -1,6 +0,0 @@
-<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
-  <command>
-    <poll op="ack" msgID="ABC-12345"/>
-    <clTRID>ABC-12346</clTRID>
-  </command>
-</epp>

javatests/google/registry/flows/poll/testdata/poll_ack_response.xml

@@ -3,7 +3,7 @@
     <result code="1000">
       <msg>Command completed successfully</msg>
     </result>
-    <msgQ count="4" id="1-3-EXAMPLE-4-3"/>
+    <msgQ count="4" id="1-3-EXAMPLE-4-3-2011"/>
     <trID>
       <clTRID>ABC-12346</clTRID>
       <svTRID>server-trid</svTRID>

javatests/google/registry/flows/poll/testdata/poll_response_autorenew.xml

@@ -3,7 +3,7 @@
     <result code="1301">
       <msg>Command completed successfully; ack to dequeue</msg>
     </result>
-    <msgQ count="1" id="1-3-EXAMPLE-5-6">
+    <msgQ count="1" id="1-3-EXAMPLE-5-6-2011">
       <qDate>2011-01-01T01:01:01Z</qDate>
       <msg>Domain was auto-renewed.</msg>
     </msgQ>

javatests/google/registry/flows/poll/testdata/poll_response_contact_delete.xml

@@ -3,7 +3,7 @@
         <result code="1301">
             <msg>Command completed successfully; ack to dequeue</msg>
         </result>
-        <msgQ count="1" id="2-2-ROID-5-6">
+        <msgQ count="1" id="2-2-ROID-5-6-2011">
             <qDate>2011-01-01T01:01:01Z</qDate>
             <msg>Deleted contact jd1234</msg>
         </msgQ>

javatests/google/registry/flows/poll/testdata/poll_response_contact_transfer.xml

@@ -3,7 +3,7 @@
     <result code="1301">
       <msg>Command completed successfully; ack to dequeue</msg>
     </result>
-    <msgQ count="1" id="2-2-ROID-5-3">
+    <msgQ count="1" id="2-2-ROID-5-3-2000">
       <qDate>2000-06-08T22:00:00Z</qDate>
       <msg>Transfer requested.</msg>
     </msgQ>

javatests/google/registry/flows/poll/testdata/poll_response_domain_pending_notification.xml

@@ -3,7 +3,7 @@
     <result code="1301">
       <msg>Command completed successfully; ack to dequeue</msg>
     </result>
-    <msgQ count="1" id="1-3-EXAMPLE-5-6">
+    <msgQ count="1" id="1-3-EXAMPLE-5-6-2011">
       <qDate>2011-01-01T01:01:01Z</qDate>
       <msg>Domain deleted.</msg>
     </msgQ>

javatests/google/registry/flows/poll/testdata/poll_response_domain_transfer.xml

@@ -3,7 +3,7 @@
     <result code="1301">
       <msg>Command completed successfully; ack to dequeue</msg>
     </result>
-    <msgQ count="1" id="1-3-EXAMPLE-5-6">
+    <msgQ count="1" id="1-3-EXAMPLE-5-6-2011">
       <qDate>2011-01-01T01:01:01Z</qDate>
       <msg>Transfer approved.</msg>
     </msgQ>

javatests/google/registry/flows/poll/testdata/poll_response_host_delete.xml

@@ -3,7 +3,7 @@
         <result code="1301">
             <msg>Command completed successfully; ack to dequeue</msg>
         </result>
-        <msgQ count="1" id="3-4-ROID-5-6">
+        <msgQ count="1" id="3-4-ROID-5-6-2011">
             <qDate>2011-01-01T01:01:01Z</qDate>
             <msg>Deleted host ns1.test.example</msg>
         </msgQ>

javatests/google/registry/flows/testdata/poll_response_autorenew.xml

@@ -0,0 +1,22 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1301">
+      <msg>Command completed successfully; ack to dequeue</msg>
+    </result>
+    <msgQ count="1" id="%ID%">
+      <qDate>%QDATE%</qDate>
+      <msg>Domain was auto-renewed.</msg>
+    </msgQ>
+    <resData>
+      <domain:renData
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:exDate>%EXDATE%</domain:exDate>
+      </domain:renData>
+    </resData>
+    <trID>
+      <clTRID>ABC-12345</clTRID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>

javatests/google/registry/flows/testdata/poll_response_contact_transfer.xml

@@ -3,7 +3,7 @@
     <result code="1301">
       <msg>Command completed successfully; ack to dequeue</msg>
     </result>
-    <msgQ count="1" id="2-1-ROID-3-6">
+    <msgQ count="1" id="2-1-ROID-3-6-2000">
       <qDate>2000-06-08T22:00:00Z</qDate>
       <msg>Transfer requested.</msg>
     </msgQ>

javatests/google/registry/flows/testdata/poll_response_domain_transfer_request.xml

@@ -3,7 +3,7 @@
         <result code="1301">
             <msg>Command completed successfully; ack to dequeue</msg>
         </result>
-        <msgQ count="1" id="1-C-EXAMPLE-17-23">
+        <msgQ count="1" id="1-C-EXAMPLE-17-23-2001">
             <qDate>2001-01-01T00:00:00Z</qDate>
             <msg>Transfer requested.</msg>
         </msgQ>

javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_loser.xml

@@ -3,7 +3,7 @@
         <result code="1301">
             <msg>Command completed successfully; ack to dequeue</msg>
         </result>
-        <msgQ count="1" id="1-C-EXAMPLE-17-22">
+        <msgQ count="1" id="1-C-EXAMPLE-17-22-2001">
             <qDate>2001-01-06T00:00:00Z</qDate>
             <msg>Transfer approved.</msg>
         </msgQ>

javatests/google/registry/flows/testdata/poll_response_domain_transfer_server_approve_winner.xml

@@ -4,7 +4,7 @@
         <result code="1301">
             <msg>Command completed successfully; ack to dequeue</msg>
         </result>
-        <msgQ count="1" id="1-C-EXAMPLE-17-21">
+        <msgQ count="1" id="1-C-EXAMPLE-17-21-2001">
             <qDate>2001-01-06T00:00:00Z</qDate>
             <msg>Transfer approved.</msg>
         </msgQ>

javatests/google/registry/flows/testdata/poll_response_empty.xml

@@ -0,0 +1,11 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <response>
+    <result code="1300">
+      <msg>Command completed successfully; no messages</msg>
+    </result>
+    <trID>
+      <clTRID>ABC-12345</clTRID>
+      <svTRID>server-trid</svTRID>
+    </trID>
+  </response>
+</epp>

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));
+  FakeClock clock = new FakeClock(DateTime.parse("2007-07-07T01:01:01Z"));
-  PollMessageExternalKeyConverter converter = new PollMessageExternalKeyConverter();
 
   @Before
   public void setUp() throws Exception {
@@ -79,48 +80,65 @@ public class PollMessageExternalKeyConverterTest {
 
   @Test
   public void testSuccess_domain() {
-    PollMessage.OneTime pollMessage = persistResource(
+    PollMessage.OneTime pollMessage =
-        new PollMessage.OneTime.Builder()
+        persistResource(
-            .setClientId("TheRegistrar")
+            new PollMessage.OneTime.Builder()
-            .setEventTime(clock.nowUtc())
+                .setClientId("TheRegistrar")
-            .setMsg("Test poll message")
+                .setEventTime(clock.nowUtc())
-            .setParent(historyEntry)
+                .setMsg("Test poll message")
-            .build());
+                .setParent(historyEntry)
-    Key<PollMessage> key = Key.<PollMessage>create(pollMessage);
+                .build());
-    assertThat(converter.convert(key)).isEqualTo("1-2-FOOBAR-4-5");
+    assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("1-2-FOOBAR-4-5-2007");
-    assertThat(converter.reverse().convert("1-2-FOOBAR-4-5")).isEqualTo(key);
+    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(
+    PollMessage.OneTime pollMessage =
-        new PollMessage.OneTime.Builder()
+        persistResource(
-            .setClientId("TheRegistrar")
+            new PollMessage.OneTime.Builder()
-            .setEventTime(clock.nowUtc())
+                .setClientId("TheRegistrar")
-            .setMsg("Test poll message")
+                .setEventTime(clock.nowUtc())
-            .setParent(historyEntry)
+                .setMsg("Test poll message")
-            .build());
+                .setParent(historyEntry)
-    Key<PollMessage> key = Key.<PollMessage>create(pollMessage);
+                .build());
-    assertThat(converter.convert(key)).isEqualTo("2-5-ROID-4-6");
+    assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("2-5-ROID-4-6-2007");
-    assertThat(converter.reverse().convert("2-5-ROID-4-6")).isEqualTo(key);
+    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(
+    PollMessage.OneTime pollMessage =
-        new PollMessage.OneTime.Builder()
+        persistResource(
-            .setClientId("TheRegistrar")
+            new PollMessage.OneTime.Builder()
-            .setEventTime(clock.nowUtc())
+                .setClientId("TheRegistrar")
-            .setMsg("Test poll message")
+                .setEventTime(clock.nowUtc())
-            .setParent(historyEntry)
+                .setMsg("Test poll message")
-            .build());
+                .setParent(historyEntry)
-    Key<PollMessage> key = Key.<PollMessage>create(pollMessage);
+                .build());
-    assertThat(converter.convert(key)).isEqualTo("3-5-ROID-4-6");
+    assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("3-5-ROID-4-6-2007");
-    assertThat(converter.reverse().convert("3-5-ROID-4-6")).isEqualTo(key);
+    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);
+    assertThrows(
-    converter.reverse().convert("4-2-FOOBAR-4-5");
+        PollMessageExternalKeyParseException.class,
+        () -> parsePollMessageExternalId("4-2-FOOBAR-4-5-2007"));
   }
 
   @Test
   public void testFailure_tooFewComponentParts() throws Exception {
-    thrown.expect(PollMessageExternalKeyParseException.class);
+    assertThrows(
-    converter.reverse().convert("1-3-EXAMPLE");
+        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);
+    assertThrows(
-    converter.reverse().convert("A-B-FOOBAR-D-E");
+        PollMessageExternalKeyParseException.class,
+        () -> parsePollMessageExternalId("A-B-FOOBAR-D-E-F"));
   }
 }