diff --git a/java/google/registry/flows/poll/PollAckFlow.java b/java/google/registry/flows/poll/PollAckFlow.java index a1fd6ceae..90cd3bc74 100644 --- a/java/google/registry/flows/poll/PollAckFlow.java +++ b/java/google/registry/flows/poll/PollAckFlow.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkState; 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.allocateId; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.isBeforeOrAt; @@ -32,7 +31,6 @@ import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.PollMessageId; import google.registry.flows.TransactionalFlow; -import google.registry.model.domain.DomainResource; import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.MessageQueueInfo; import google.registry.model.poll.PollMessage; @@ -107,23 +105,14 @@ public class PollAckFlow implements TransactionalFlow { // Move the eventTime of this autorenew poll message forward by a year. DateTime nextEventTime = autorenewPollMessage.getEventTime().plusYears(1); - // Delete the existing poll message, then optionally create a new one (with a different ID, to - // preserve global uniqueness of poll messages across all time) if there are more events to be - // delivered. - ofy().delete().entity(autorenewPollMessage); + // If the next event falls within the bounds of the end time, then just update the eventTime + // and re-save it for future autorenew poll messages to be delivered. Otherwise, this + // autorenew poll message has no more events to deliver and should be deleted. if (nextEventTime.isBefore(autorenewPollMessage.getAutorenewEndTime())) { - PollMessage.Autorenew newAutorenewPollMessage = - autorenewPollMessage - .asBuilder() - .setId(allocateId()) - .setEventTime(nextEventTime) - .build(); - Key domainKey = autorenewPollMessage.getParentKey().getParent(); - DomainResource domain = ofy().load().key(domainKey).now(); - DomainResource updatedDomain = - domain.asBuilder().setAutorenewPollMessage(Key.create(newAutorenewPollMessage)).build(); - ofy().save().entities(newAutorenewPollMessage, updatedDomain); + ofy().save().entity(autorenewPollMessage.asBuilder().setEventTime(nextEventTime).build()); includeAckedMessageInCount = isBeforeOrAt(nextEventTime, now); + } else { + ofy().delete().entity(autorenewPollMessage); } } // We need to return the new queue length. If this was the last message in the queue being diff --git a/javatests/google/registry/flows/EppLifecycleDomainTest.java b/javatests/google/registry/flows/EppLifecycleDomainTest.java index e3d76755f..8d0ce7904 100644 --- a/javatests/google/registry/flows/EppLifecycleDomainTest.java +++ b/javatests/google/registry/flows/EppLifecycleDomainTest.java @@ -364,61 +364,6 @@ public class EppLifecycleDomainTest extends EppTestCase { assertCommandAndResponse("logout.xml", "logout_response.xml"); } - @Test - public void testDomainCreate_annualAutoRenewPollMessagesAreSent() 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", - "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"), - "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-17", // Note -- different than the 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-17"), - "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. diff --git a/javatests/google/registry/flows/poll/PollAckFlowTest.java b/javatests/google/registry/flows/poll/PollAckFlowTest.java index c3eb0d253..5c0330b22 100644 --- a/javatests/google/registry/flows/poll/PollAckFlowTest.java +++ b/javatests/google/registry/flows/poll/PollAckFlowTest.java @@ -14,17 +14,13 @@ package google.registry.flows.poll; -import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.ofy.ObjectifyService.ofy; 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.expectThrows; import static google.registry.util.DateTimeUtils.END_OF_TIME; -import com.googlecode.objectify.Key; import google.registry.flows.FlowTestCase; import google.registry.flows.poll.PollAckFlow.InvalidMessageIdException; import google.registry.flows.poll.PollAckFlow.MessageDoesNotExistException; @@ -33,7 +29,6 @@ import google.registry.flows.poll.PollAckFlow.NotAuthorizedToAckMessageException import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainResource; import google.registry.model.poll.PollMessage; -import google.registry.model.poll.PollMessage.Autorenew; import org.joda.time.DateTime; import org.junit.Before; import org.junit.Test; @@ -71,8 +66,8 @@ public class PollAckFlowTest extends FlowTestCase { .build()); } - private PollMessage.Autorenew persistAutorenewPollMessage(DateTime eventTime, DateTime endTime) { - return persistResource( + private void persistAutorenewPollMessage(DateTime eventTime, DateTime endTime) { + persistResource( new PollMessage.Autorenew.Builder() .setId(MESSAGE_ID) .setClientId(getClientIdForFlow()) @@ -114,25 +109,14 @@ public class PollAckFlowTest extends FlowTestCase { @Test public void testSuccess_recentActiveAutorenew() throws Exception { - PollMessage.Autorenew autorenew = - persistAutorenewPollMessage(clock.nowUtc().plusMonths(6), END_OF_TIME); + persistAutorenewPollMessage(clock.nowUtc().minusMonths(6), END_OF_TIME); assertTransactionalFlow(true); - MessageDoesNotExistException e = - expectThrows( - MessageDoesNotExistException.class, - () -> runFlowAssertResponse(readFile("poll_ack_response_empty.xml"))); - assertThat(e).hasMessageThat().contains("given ID (1-3-EXAMPLE-4-3) doesn't exist"); - // No changes should have been made to the autorenew since it wasn't delivered yet. - assertThat(ofy().load().entity(autorenew).now()).isEqualTo(autorenew); + runFlowAssertResponse(readFile("poll_ack_response_empty.xml")); } @Test public void testSuccess_oldActiveAutorenew() throws Exception { - DateTime autorenewDate = clock.nowUtc().minusYears(2); - PollMessage.Autorenew autorenew = - persistAutorenewPollMessage(autorenewDate, END_OF_TIME); - Long autorenewId = autorenew.getId(); - assertThat(ofy().load().entity(autorenew).now().getEventTime()).isEqualTo(autorenewDate); + 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. for (int i = 1; i < 4; i++) { @@ -140,14 +124,6 @@ public class PollAckFlowTest extends FlowTestCase { } assertTransactionalFlow(true); runFlowAssertResponse(readFile("poll_ack_response.xml")); - // The previous autorenew should have been deleted. - assertThat(ofy().load().entity(autorenew).now()).isNull(); - // Check that the domain now points to a new autorenew, with a different id, that will be - // delivered one year after the previous one. - Key newPollMessageKey = ofy().load().entity(domain).now().getAutorenewPollMessage(); - assertThat(ofy().load().key(newPollMessageKey).now().getEventTime()) - .isEqualTo(autorenewDate.plusYears(1)); - assertThat(newPollMessageKey.getId()).isNotEqualTo(autorenewId); } @Test diff --git a/javatests/google/registry/flows/testdata/poll_response_autorenew.xml b/javatests/google/registry/flows/testdata/poll_response_autorenew.xml deleted file mode 100644 index d0e371056..000000000 --- a/javatests/google/registry/flows/testdata/poll_response_autorenew.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - - 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_empty.xml b/javatests/google/registry/flows/testdata/poll_response_empty.xml deleted file mode 100644 index 85911436a..000000000 --- a/javatests/google/registry/flows/testdata/poll_response_empty.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - Command completed successfully; no messages - - - ABC-12345 - server-trid - - -