Remove leniency on poll message ID format without years in them

It's been long enough since the format change adding in years that all
registrars should no longer have any IDs in the old format lying around
that they're still attempting to ACK. All poll messages have already been
coming back to registrars with the new format for months now.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184714735

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.makePollMessageExternalId;
 import static google.registry.model.poll.PollMessageExternalKeyConverter.parsePollMessageExternalId;
 import static google.registry.util.DateTimeUtils.isBeforeOrAt;
 
@@ -80,14 +81,14 @@ public class PollAckFlow implements TransactionalFlow {
     final DateTime now = ofy().getTransactionTime();
 
     // Load the message to be acked. If a message is queued to be delivered in the future, we treat
-    // it as if it doesn't exist yet.
+    // it as if it doesn't exist yet. Same for if the message ID year isn't the same as the actual
+    // poll message's event time (that means they're passing in an old already-acked ID).
     PollMessage pollMessage = ofy().load().key(pollMessageKey).now();
-    if (pollMessage == null || !isBeforeOrAt(pollMessage.getEventTime(), now)) {
+    if (pollMessage == null
+        || !isBeforeOrAt(pollMessage.getEventTime(), now)
+        || !makePollMessageExternalId(pollMessage).equals(messageId)) {
       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/model/poll/PollMessageExternalKeyConverter.java

@@ -85,11 +85,9 @@ public class PollMessageExternalKeyConverter {
    *
    * @throws PollMessageExternalKeyParseException if the external key has an invalid format.
    */
-  // TODO(b/68953444): Make the year field mandatory once sufficient time has elapsed and backwards
-  //                   compatibility is no longer necessary.
   public static Key<PollMessage> parsePollMessageExternalId(String externalKey) {
     List<String> idComponents = Splitter.on('-').splitToList(externalKey);
-    if (idComponents.size() != 5 && idComponents.size() != 6) {
+    if (idComponents.size() != 6) {
       throw new PollMessageExternalKeyParseException();
     }
     try {

javatests/google/registry/flows/poll/PollAckFlowTest.java

@@ -104,24 +104,8 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
     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 {
+  public void testFailure_contactPollMessage_withIncorrectYearField() 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()
@@ -132,7 +116,7 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
             .setParent(createHistoryEntryForEppResource(contact))
             .build());
     assertTransactionalFlow(true);
-    runFlowAssertResponse(loadFile("poll_ack_response_empty.xml"));
+    assertThrows(MessageDoesNotExistException.class, this::runFlow);
   }
 
   @Test
@@ -144,6 +128,7 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
 
   @Test
   public void testSuccess_recentActiveAutorenew() throws Exception {
+    setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2010"));
     persistAutorenewPollMessage(clock.nowUtc().minusMonths(6), END_OF_TIME);
     assertTransactionalFlow(true);
     runFlowAssertResponse(loadFile("poll_ack_response_empty.xml"));
@@ -151,6 +136,7 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
 
   @Test
   public void testSuccess_oldActiveAutorenew() throws Exception {
+    setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2009"));
     persistAutorenewPollMessage(clock.nowUtc().minusYears(2), END_OF_TIME);
     // Create three other messages to be queued for retrieval to get our count right, since the poll
     // ack response wants there to be 4 messages in the queue when the ack comes back.
@@ -158,11 +144,15 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
       persistOneTimePollMessage(MESSAGE_ID + i);
     }
     assertTransactionalFlow(true);
-    runFlowAssertResponse(loadFile("poll_ack_response.xml"));
+    runFlowAssertResponse(
+        loadFile(
+            "poll_ack_response.xml",
+            ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2009", "COUNT", "4")));
   }
 
   @Test
   public void testSuccess_oldInactiveAutorenew() throws Exception {
+    setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2010"));
     persistAutorenewPollMessage(clock.nowUtc().minusMonths(6), clock.nowUtc());
     assertTransactionalFlow(true);
     runFlowAssertResponse(loadFile("poll_ack_response_empty.xml"));
@@ -175,7 +165,10 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
       persistOneTimePollMessage(MESSAGE_ID + i);
     }
     assertTransactionalFlow(true);
-    runFlowAssertResponse(loadFile("poll_ack_response.xml"));
+    runFlowAssertResponse(
+        loadFile(
+            "poll_ack_response.xml",
+            ImmutableMap.of("MSGID", "1-3-EXAMPLE-4-3-2011", "COUNT", "4")));
   }
 
   @Test
@@ -201,6 +194,21 @@ public class PollAckFlowTest extends FlowTestCase<PollAckFlow> {
     assertThrows(InvalidMessageIdException.class, this::runFlow);
   }
 
+  @Test
+  public void testFailure_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);
+    assertThrows(InvalidMessageIdException.class, this::runFlow);
+  }
+
   @Test
   public void testFailure_invalidId_stringInsteadOfNumeric() throws Exception {
     setEppInput("poll_ack.xml", ImmutableMap.of("MSGID", "ABC-12345"));

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-2011"/>
+    <msgQ count="%COUNT%" id="%MSGID%"/>
     <trID>
       <clTRID>ABC-12346</clTRID>
       <svTRID>server-trid</svTRID>

javatests/google/registry/model/poll/PollMessageExternalKeyConverterTest.java

@@ -122,19 +122,10 @@ public class PollMessageExternalKeyConverterTest {
   }
 
   @Test
-  // TODO(b/68953444): Remove leniency when backwards compatibility with missing field is no longer
+  public void testFailure_missingYearField() {
-  //                   required.
+    assertThrows(
-  public void testSuccess_stillParsesWithMissingYearField() {
+        PollMessageExternalKeyParseException.class,
-    PollMessage.OneTime pollMessage =
+        () -> parsePollMessageExternalId("1-2-FOOBAR-4-5"));
-        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