Change transfer flow tests to assert on entire TransferData contents

This CL changes the domain and contact transfer flows to check the entire
TransferData on the post-transfer resource, rather than just spot-checking
certain fields.  This approach provides much better code coverage - in
particular, it checks that the non-request flows (approve, cancel, reject)
don't modify the fields that they shouldn't be modifying, and that they do
actually clear out the transfer server-approve entities fields written by
the transfer request flow.  It's slightly orthogonal, but I also added
testing that the server-approve entities fields are actually set in the
request flows, which was previously untested.

This is pre-work for introducing an exDate-storing field into TransferData,
by making it easier to test everywhere that exDate is set *and* unset only
in the correct places.

As part of this CL, I've introduced a TransferData.copyConstantFieldsToBuilder()
method that is like asBuilder() but instead of copying all the fields to the new
builder, it only copies the logically constant ones: losing/gaining client IDs,
the request time and TRID, and transferPeriod.  This is useful both in tests but
is also used in the resolvingPendingTransfer() helper that centralizes the core
transfer resolution logic (as of []  That method has its own tests,
and in the process I removed a bunch of crufty defunct TransferData tests.

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

java/google/registry/flows/ResourceFlowUtils.java

@@ -228,11 +228,7 @@ public final class ResourceFlowUtils {
     return builder
         .removeStatusValue(StatusValue.PENDING_TRANSFER)
         .setTransferData(
-            resource.getTransferData().asBuilder()
+            resource.getTransferData().copyConstantFieldsToBuilder()
-                .setServerApproveEntities(null)
-                .setServerApproveBillingEvent(null)
-                .setServerApproveAutorenewEvent(null)
-                .setServerApproveAutorenewPollMessage(null)
                 .setTransferStatus(transferStatus)
                 .setPendingTransferExpirationTime(checkNotNull(now))
                 .build());

java/google/registry/model/transfer/TransferData.java

@@ -120,6 +120,28 @@ public class TransferData extends BaseTransferObject implements Buildable {
     return new Builder(clone(this));
   }
 
+  /**
+   * Returns a fresh Builder populated only with the constant fields of this TransferData, i.e.
+   * those that are fixed and unchanging throughout the transfer process.
+   *
+   * <p>These fields are:
+   * <ul>
+   *   <li>transferRequestTrid
+   *   <li>transferRequestTime
+   *   <li>gainingClientId
+   *   <li>losingClientId
+   *   <li>transferPeriod
+   * </ul>
+   */
+  public Builder copyConstantFieldsToBuilder() {
+    return new Builder()
+        .setTransferRequestTrid(this.transferRequestTrid)
+        .setTransferRequestTime(this.transferRequestTime)
+        .setGainingClientId(this.gainingClientId)
+        .setLosingClientId(this.losingClientId)
+        .setTransferPeriod(this.transferPeriod);
+  }
+
   /** Builder for {@link TransferData} because it is immutable. */
   public static class Builder extends BaseTransferObject.Builder<TransferData, Builder> {
 

javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java

@@ -27,7 +27,6 @@ import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_DELETE_F
 import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_TRANSFER_REQUEST;
 import static google.registry.model.reporting.HistoryEntry.Type.HOST_DELETE;
 import static google.registry.model.reporting.HistoryEntry.Type.HOST_DELETE_FAILURE;
-import static google.registry.model.transfer.TransferStatus.SERVER_CANCELLED;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
 import static google.registry.testing.DatastoreHelper.createTld;
@@ -88,6 +87,7 @@ import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
+import google.registry.model.transfer.TransferStatus;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
@@ -274,8 +274,14 @@ public class DeleteContactsAndHostsActionTest
     assertAboutContacts()
         .that(softDeletedContact)
         .hasOneHistoryEntryEachOfTypes(CONTACT_TRANSFER_REQUEST, CONTACT_DELETE);
-    assertThat(softDeletedContact.getTransferData().getPendingTransferExpirationTime())
+    // Check that the transfer data reflects the cancelled transfer as we expect.
-        .isEqualTo(softDeletedContact.getDeletionTime());
+    TransferData oldTransferData = contact.getTransferData();
+    assertThat(softDeletedContact.getTransferData())
+        .isEqualTo(
+            oldTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.SERVER_CANCELLED)
+                .setPendingTransferExpirationTime(softDeletedContact.getDeletionTime())
+                .build());
     assertNoBillingEvents();
     PollMessage deletePollMessage =
         Iterables.getOnlyElement(getPollMessages("TheRegistrar", clock.nowUtc().plusMonths(1)));
@@ -290,7 +296,7 @@ public class DeleteContactsAndHostsActionTest
                     FluentIterable.from(gainingPollMessage.getResponseData())
                         .filter(TransferResponse.class))
                 .getTransferStatus())
-        .isEqualTo(SERVER_CANCELLED);
+        .isEqualTo(TransferStatus.SERVER_CANCELLED);
     PendingActionNotificationResponse panData =
         getOnlyElement(
             FluentIterable.from(gainingPollMessage.getResponseData())

javatests/google/registry/flows/contact/ContactTransferApproveFlowTest.java

@@ -36,6 +36,7 @@ import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import org.junit.Before;
@@ -64,6 +65,8 @@ public class ContactTransferApproveFlowTest
         .hasSize(1);
 
     // Setup done; run the test.
+    contact = reloadResourceByForeignKey();
+    TransferData originalTransferData = contact.getTransferData();
     assertTransactionalFlow(true);
     runFlowAssertResponse(readFile(expectedXmlFilename));
 
@@ -72,11 +75,15 @@ public class ContactTransferApproveFlowTest
     assertAboutContacts().that(contact)
         .hasCurrentSponsorClientId("NewRegistrar").and()
         .hasLastTransferTime(clock.nowUtc()).and()
-        .hasTransferStatus(TransferStatus.CLIENT_APPROVED).and()
-        .hasPendingTransferExpirationTime(clock.nowUtc()).and()
         .hasOneHistoryEntryEachOfTypes(
             HistoryEntry.Type.CONTACT_TRANSFER_REQUEST,
             HistoryEntry.Type.CONTACT_TRANSFER_APPROVE);
+    assertThat(contact.getTransferData())
+        .isEqualTo(
+            originalTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.CLIENT_APPROVED)
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
     assertNoBillingEvents();
     // The poll message (in the future) to the losing registrar for implicit ack should be gone.
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc().plusMonths(1))).isEmpty();

javatests/google/registry/flows/contact/ContactTransferCancelFlowTest.java

@@ -33,6 +33,7 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import org.junit.Before;
@@ -58,6 +59,8 @@ public class ContactTransferCancelFlowTest
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc().plusMonths(1))).hasSize(1);
 
     // Setup done; run the test.
+    contact = reloadResourceByForeignKey();
+    TransferData originalTransferData = contact.getTransferData();
     assertTransactionalFlow(true);
     runFlowAssertResponse(readFile(expectedXmlFilename));
 
@@ -66,11 +69,15 @@ public class ContactTransferCancelFlowTest
     assertAboutContacts().that(contact)
         .hasCurrentSponsorClientId("TheRegistrar").and()
         .hasLastTransferTimeNotEqualTo(clock.nowUtc()).and()
-        .hasTransferStatus(TransferStatus.CLIENT_CANCELLED).and()
-        .hasPendingTransferExpirationTime(clock.nowUtc()).and()
         .hasOneHistoryEntryEachOfTypes(
             HistoryEntry.Type.CONTACT_TRANSFER_REQUEST,
             HistoryEntry.Type.CONTACT_TRANSFER_CANCEL);
+    assertThat(contact.getTransferData())
+        .isEqualTo(
+            originalTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.CLIENT_CANCELLED)
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
     assertNoBillingEvents();
     // The poll message (in the future) to the gaining registrar for implicit ack should be gone.
     assertThat(getPollMessages("NewRegistrar", clock.nowUtc().plusMonths(1))).isEmpty();

javatests/google/registry/flows/contact/ContactTransferRejectFlowTest.java

@@ -35,6 +35,7 @@ import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import org.junit.Before;
@@ -62,6 +63,8 @@ public class ContactTransferRejectFlowTest
         .hasSize(1);
 
     // Setup done; run the test.
+    contact = reloadResourceByForeignKey();
+    TransferData originalTransferData = contact.getTransferData();
     assertTransactionalFlow(true);
     runFlowAssertResponse(readFile(expectedXmlFilename));
 
@@ -70,10 +73,15 @@ public class ContactTransferRejectFlowTest
     assertAboutContacts().that(contact)
         .hasCurrentSponsorClientId("TheRegistrar").and()
         .hasLastTransferTimeNotEqualTo(clock.nowUtc()).and()
-        .hasTransferStatus(TransferStatus.CLIENT_REJECTED).and()
         .hasOneHistoryEntryEachOfTypes(
             HistoryEntry.Type.CONTACT_TRANSFER_REQUEST,
             HistoryEntry.Type.CONTACT_TRANSFER_REJECT);
+    assertThat(contact.getTransferData())
+        .isEqualTo(
+            originalTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.CLIENT_REJECTED)
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
     // The poll message (in the future) to the losing registrar for implicit ack should be gone.
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc().plusMonths(1)))
         .isEmpty();

javatests/google/registry/flows/contact/ContactTransferRequestFlowTest.java

@@ -14,15 +14,23 @@
 
 package google.registry.flows.contact;
 
+import static com.google.common.base.Predicates.equalTo;
+import static com.google.common.base.Predicates.not;
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.config.RegistryConfig.getContactAutomaticTransferLength;
+import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
+import static google.registry.testing.DatastoreHelper.assertPollMessagesEqual;
 import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.getPollMessages;
 import static google.registry.testing.DatastoreHelper.persistActiveContact;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.googlecode.objectify.Key;
 import google.registry.flows.ResourceFlowUtils.BadAuthInfoForResourceException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.exceptions.AlreadyPendingTransferException;
@@ -31,9 +39,14 @@ import google.registry.flows.exceptions.ObjectAlreadySponsoredException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
 import google.registry.model.contact.ContactAuthInfo;
 import google.registry.model.contact.ContactResource;
+import google.registry.model.domain.Period;
+import google.registry.model.domain.Period.Unit;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppcommon.Trid;
+import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferStatus;
 import org.joda.time.DateTime;
 import org.junit.Before;
@@ -68,22 +81,56 @@ public class ContactTransferRequestFlowTest
     // Transfer should have been requested. Verify correct fields were set.
     contact = reloadResourceByForeignKey();
     assertAboutContacts().that(contact)
-        .hasTransferStatus(TransferStatus.PENDING).and()
-        .hasTransferGainingClientId("NewRegistrar").and()
-        .hasTransferLosingClientId("TheRegistrar").and()
-        .hasTransferRequestClientTrid(getClientTrid()).and()
         .hasCurrentSponsorClientId("TheRegistrar").and()
-        .hasPendingTransferExpirationTime(afterTransfer).and()
         .hasOnlyOneHistoryEntryWhich()
         .hasType(HistoryEntry.Type.CONTACT_TRANSFER_REQUEST);
+    Trid expectedTrid =
+        Trid.create(
+            getClientTrid(),
+            contact.getTransferData().getTransferRequestTrid().getServerTransactionId());
+    assertThat(contact.getTransferData())
+        .isEqualTo(
+            new TransferData.Builder()
+                .setTransferRequestTrid(expectedTrid)
+                .setTransferRequestTime(clock.nowUtc())
+                .setGainingClientId("NewRegistrar")
+                .setLosingClientId("TheRegistrar")
+                // Period is meaningless for contact transfers, but this is the default.
+                .setTransferPeriod(Period.create(1, Unit.YEARS))
+                .setTransferStatus(TransferStatus.PENDING)
+                .setPendingTransferExpirationTime(afterTransfer)
+                // Make the server-approve entities field a no-op comparison; it's easier to
+                // do this comparison separately below.
+                .setServerApproveEntities(contact.getTransferData().getServerApproveEntities())
+                .build());
     assertNoBillingEvents();
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc())).hasSize(1);
+    PollMessage losingRequestMessage =
+        getOnlyElement(getPollMessages("TheRegistrar", clock.nowUtc()));
 
     // If we fast forward AUTOMATIC_TRANSFER_DAYS the transfer should have happened.
     assertAboutContacts().that(contact.cloneProjectedAtTime(afterTransfer))
         .hasCurrentSponsorClientId("NewRegistrar");
     assertThat(getPollMessages("NewRegistrar", afterTransfer)).hasSize(1);
     assertThat(getPollMessages("TheRegistrar", afterTransfer)).hasSize(2);
+    PollMessage gainingApproveMessage =
+        getOnlyElement(getPollMessages("NewRegistrar", afterTransfer));
+    PollMessage losingApproveMessage =
+        getOnlyElement(
+            Iterables.filter(
+                getPollMessages("TheRegistrar", afterTransfer),
+                not(equalTo(losingRequestMessage))));
+
+    // Check for TransferData server-approve entities containing what we expect: only
+    // poll messages, the approval notice ones for gaining and losing registrars.
+    assertPollMessagesEqual(
+        Iterables.filter(
+            ofy().load()
+                // Use toArray() to coerce the type to something keys() will accept.
+                .keys(contact.getTransferData().getServerApproveEntities().toArray(new Key<?>[]{}))
+                .values(),
+            PollMessage.class),
+        ImmutableList.of(gainingApproveMessage, losingApproveMessage));
   }
 
   private void doFailingTest(String commandFilename) throws Exception {

javatests/google/registry/flows/domain/DomainDeleteFlowTest.java

@@ -560,13 +560,15 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
         .isEmpty();
     assertThat(getPollMessages("TheRegistrar", deletionTime)).hasSize(1);
     assertOnlyBillingEventIsClosedAutorenew("TheRegistrar");
-    // The keys written for server approve should be null, and the entities should all be deleted.
+    // The domain TransferData should reflect the cancelled transfer as we expect, with
-    assertThat(domain.getTransferData().getServerApproveEntities()).isEmpty();
+    // all the speculative server-approve fields nulled out.
-    assertThat(domain.getTransferData().getServerApproveBillingEvent()).isNull();
+    assertThat(domain.getTransferData())
-    assertThat(domain.getTransferData().getServerApproveAutorenewEvent()).isNull();
+        .isEqualTo(
-    assertThat(domain.getTransferData().getServerApproveAutorenewPollMessage()).isNull();
+            oldTransferData.copyConstantFieldsToBuilder()
-    assertThat(domain.getTransferData().getPendingTransferExpirationTime())
+                .setTransferStatus(TransferStatus.SERVER_CANCELLED)
-        .isEqualTo(clock.nowUtc());
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
+    // The server-approve entities should all be deleted.
     assertThat(ofy().load().key(oldTransferData.getServerApproveBillingEvent()).now()).isNull();
     assertThat(ofy().load().key(oldTransferData.getServerApproveAutorenewEvent()).now()).isNull();
     assertThat(ofy().load().key(oldTransferData.getServerApproveAutorenewPollMessage()).now())

javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java

@@ -107,13 +107,19 @@ public class DomainTransferApproveFlowTest
     clock.advanceOneMilli();
   }
 
-  private void assertTransferApproved(DomainResource domain) {
+  private void assertTransferApproved(DomainResource domain, TransferData oldTransferData) {
     assertAboutDomains().that(domain)
-        .hasTransferStatus(TransferStatus.CLIENT_APPROVED).and()
         .hasCurrentSponsorClientId("NewRegistrar").and()
         .hasLastTransferTime(clock.nowUtc()).and()
-        .hasPendingTransferExpirationTime(clock.nowUtc()).and()
         .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER);
+    // The domain TransferData should reflect the approved transfer as we expect, with
+    // all the speculative server-approve fields nulled out.
+    assertThat(domain.getTransferData())
+        .isEqualTo(
+            oldTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(TransferStatus.CLIENT_APPROVED)
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
   }
 
   private void setEppLoader(String commandFilename) {
@@ -150,6 +156,7 @@ public class DomainTransferApproveFlowTest
       DateTime expectedExpirationTime) throws Exception {
     setEppLoader(commandFilename);
     Registry registry = Registry.get(tld);
+    domain = reloadResourceByForeignKey();
     // Make sure the implicit billing event is there; it will be deleted by the flow.
     // We also expect to see autorenew events for the gaining and losing registrars.
     assertBillingEventsForResource(
@@ -161,6 +168,7 @@ public class DomainTransferApproveFlowTest
     assertThat(getPollMessages(domain, "NewRegistrar", clock.nowUtc().plusMonths(1))).hasSize(1);
     assertThat(getPollMessages(domain, "TheRegistrar", clock.nowUtc().plusMonths(1))).hasSize(1);
     // Setup done; run the test.
+    TransferData originalTransferData = domain.getTransferData();
     assertTransactionalFlow(true);
     runFlowAssertResponse(readFile(expectedXmlFilename));
     // Transfer should have succeeded. Verify correct fields were set.
@@ -173,7 +181,7 @@ public class DomainTransferApproveFlowTest
         getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_APPROVE);
     assertAboutHistoryEntries().that(historyEntryTransferApproved)
         .hasOtherClientId("NewRegistrar");
-    assertTransferApproved(domain);
+    assertTransferApproved(domain, originalTransferData);
     assertAboutDomains().that(domain).hasRegistrationExpirationTime(expectedExpirationTime);
     assertThat(ofy().load().key(domain.getAutorenewBillingEvent()).now().getEventTime())
         .isEqualTo(expectedExpirationTime);
@@ -369,7 +377,7 @@ public class DomainTransferApproveFlowTest
 
   @Test
   public void testSuccess_autorenewBeforeTransfer() throws Exception {
-    DomainResource domain = reloadResourceByForeignKey();
+    domain = reloadResourceByForeignKey();
     DateTime oldExpirationTime = clock.nowUtc().minusDays(1);
     persistResource(domain.asBuilder()
         .setRegistrationExpirationTime(oldExpirationTime)
@@ -407,7 +415,7 @@ public class DomainTransferApproveFlowTest
   @Test
   public void testFailure_badDomainPassword() throws Exception {
     // Change the domain's password so it does not match the password in the file.
-    domain = persistResource(domain.asBuilder()
+    persistResource(domain.asBuilder()
         .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("badpassword")))
         .build());
     thrown.expect(BadAuthInfoForResourceException.class);
@@ -472,7 +480,7 @@ public class DomainTransferApproveFlowTest
 
   @Test
   public void testFailure_deletedDomain() throws Exception {
-    domain = persistResource(
+    persistResource(
         domain.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     thrown.expect(ResourceDoesNotExistException.class,
         String.format("(%s)", getUniqueIdFromCommand()));
@@ -574,7 +582,7 @@ public class DomainTransferApproveFlowTest
 
   @Test
   public void testSuccess_superuserExtension_transferPeriodZero() throws Exception {
-    DomainResource domain = reloadResourceByForeignKey();
+    domain = reloadResourceByForeignKey();
     TransferData.Builder transferDataBuilder = domain.getTransferData().asBuilder();
     persistResource(
         domain

javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java

@@ -47,6 +47,7 @@ import google.registry.model.poll.PollMessage;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import org.joda.time.DateTime;
@@ -113,11 +114,12 @@ public class DomainTransferCancelFlowTest
     assertTransactionalFlow(true);
     DateTime originalExpirationTime = domain.getRegistrationExpirationTime();
     ImmutableSet<GracePeriod> originalGracePeriods = domain.getGracePeriods();
+    TransferData originalTransferData = domain.getTransferData();
     runFlowAssertResponse(readFile(expectedXmlFilename));
 
     // Transfer should have been cancelled. Verify correct fields were set.
     domain = reloadResourceByForeignKey();
-    assertTransferFailed(domain, TransferStatus.CLIENT_CANCELLED);
+    assertTransferFailed(domain, TransferStatus.CLIENT_CANCELLED, originalTransferData);
     assertAboutDomains().that(domain)
         .hasRegistrationExpirationTime(originalExpirationTime).and()
         .hasLastTransferTimeNotEqualTo(clock.nowUtc());

javatests/google/registry/flows/domain/DomainTransferFlowTestCase.java

@@ -204,17 +204,19 @@ public class DomainTransferFlowTestCase<F extends Flow, R extends EppResource>
         .build();
   }
 
-  protected void assertTransferFailed(DomainResource domain, TransferStatus status) {
+  protected void assertTransferFailed(
+      DomainResource domain, TransferStatus status, TransferData oldTransferData) {
     assertAboutDomains().that(domain)
-        .hasTransferStatus(status).and()
-        .hasPendingTransferExpirationTime(clock.nowUtc()).and()
         .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER).and()
         .hasCurrentSponsorClientId("TheRegistrar");
-    TransferData transferData = domain.getTransferData();
+    // The domain TransferData should reflect the failed transfer as we expect, with
-    assertThat(transferData.getServerApproveBillingEvent()).isNull();
+    // all the speculative server-approve fields nulled out.
-    assertThat(transferData.getServerApproveAutorenewEvent()).isNull();
+    assertThat(domain.getTransferData())
-    assertThat(transferData.getServerApproveAutorenewPollMessage()).isNull();
+        .isEqualTo(
-    assertThat(transferData.getServerApproveEntities()).isEmpty();
+            oldTransferData.copyConstantFieldsToBuilder()
+                .setTransferStatus(status)
+                .setPendingTransferExpirationTime(clock.nowUtc())
+                .build());
   }
 
   /** Adds a domain that has a pending transfer on it from TheRegistrar to NewRegistrar. */

javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java

@@ -51,6 +51,7 @@ import google.registry.model.poll.PollMessage;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import org.joda.time.DateTime;
@@ -89,10 +90,11 @@ public class DomainTransferRejectFlowTest
     assertTransactionalFlow(true);
     DateTime originalExpirationTime = domain.getRegistrationExpirationTime();
     ImmutableSet<GracePeriod> originalGracePeriods = domain.getGracePeriods();
+    TransferData originalTransferData = domain.getTransferData();
     runFlowAssertResponse(readFile(expectedXmlFilename));
     // Transfer should have been rejected. Verify correct fields were set.
     domain = reloadResourceByForeignKey();
-    assertTransferFailed(domain, TransferStatus.CLIENT_REJECTED);
+    assertTransferFailed(domain, TransferStatus.CLIENT_REJECTED, originalTransferData);
     assertAboutDomains().that(domain)
         .hasRegistrationExpirationTime(originalExpirationTime).and()
         .hasLastTransferTimeNotEqualTo(clock.nowUtc()).and()

javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java

@@ -20,6 +20,8 @@ import static google.registry.model.reporting.DomainTransactionRecord.Transactio
 import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
 import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST;
 import static google.registry.testing.DatastoreHelper.assertBillingEvents;
+import static google.registry.testing.DatastoreHelper.assertBillingEventsEqual;
+import static google.registry.testing.DatastoreHelper.assertPollMessagesEqual;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
 import static google.registry.testing.DatastoreHelper.getOnlyPollMessage;
@@ -39,6 +41,7 @@ import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
@@ -74,11 +77,13 @@ import google.registry.model.domain.Period.Unit;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppcommon.Trid;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferResponse;
 import google.registry.model.transfer.TransferStatus;
 import java.util.Map;
@@ -124,33 +129,56 @@ public class DomainTransferRequestFlowTest
   }
 
   private void assertTransferRequested(
-      DomainResource domain, Optional<Duration> expectedAutomaticTransferLength) throws Exception {
+      DomainResource domain, DateTime automaticTransferTime, Period expectedPeriod)
-    DateTime afterAutoAckTime =
+      throws Exception {
-        (expectedAutomaticTransferLength.isPresent())
-            ? clock.nowUtc().plus(expectedAutomaticTransferLength.get())
-            : clock.nowUtc().plus(Registry.get(domain.getTld()).getAutomaticTransferLength());
     assertAboutDomains().that(domain)
-        .hasTransferStatus(TransferStatus.PENDING).and()
-        .hasTransferGainingClientId("NewRegistrar").and()
-        .hasTransferLosingClientId("TheRegistrar").and()
-        .hasTransferRequestClientTrid(getClientTrid()).and()
         .hasCurrentSponsorClientId("TheRegistrar").and()
-        .hasPendingTransferExpirationTime(afterAutoAckTime).and()
         .hasStatusValue(StatusValue.PENDING_TRANSFER);
+    Trid expectedTrid =
+        Trid.create(
+            getClientTrid(),
+            domain.getTransferData().getTransferRequestTrid().getServerTransactionId());
+    assertThat(domain.getTransferData())
+        .isEqualTo(
+            // Compare against only the following fields by rebuilding the existing TransferData.
+            // Equivalent to assertThat(transferData.getGainingClientId()).isEqualTo("NewReg")
+            // and similar individual assertions, but produces a nicer error message this way.
+            domain.getTransferData().asBuilder()
+                .setGainingClientId("NewRegistrar")
+                .setLosingClientId("TheRegistrar")
+                .setTransferRequestTrid(expectedTrid)
+                .setTransferRequestTime(clock.nowUtc())
+                .setTransferPeriod(expectedPeriod)
+                .setTransferStatus(TransferStatus.PENDING)
+                .setPendingTransferExpirationTime(automaticTransferTime)
+                // Don't compare the server-approve entity fields; they're hard to reconstruct
+                // and logic later will check them.
+                .build());
   }
 
   private void assertTransferApproved(
-      DomainResource domain, Optional<Duration> expectedAutomaticTransferLength) {
+      DomainResource domain, DateTime automaticTransferTime, Period expectedPeriod)
-    DateTime afterAutoAckTime =
+      throws Exception {
-        (expectedAutomaticTransferLength.isPresent())
-            ? clock.nowUtc().plus(expectedAutomaticTransferLength.get())
-            : clock.nowUtc().plus(Registry.get(domain.getTld()).getAutomaticTransferLength());
     assertAboutDomains().that(domain)
-        .hasTransferStatus(TransferStatus.SERVER_APPROVED).and()
         .hasCurrentSponsorClientId("NewRegistrar").and()
-        .hasLastTransferTime(afterAutoAckTime).and()
+        .hasLastTransferTime(automaticTransferTime).and()
-        .hasPendingTransferExpirationTime(afterAutoAckTime).and()
         .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER);
+    Trid expectedTrid =
+        Trid.create(
+            getClientTrid(),
+            domain.getTransferData().getTransferRequestTrid().getServerTransactionId());
+    assertThat(domain.getTransferData())
+        .isEqualTo(
+            new TransferData.Builder()
+                .setGainingClientId("NewRegistrar")
+                .setLosingClientId("TheRegistrar")
+                .setTransferRequestTrid(expectedTrid)
+                .setTransferRequestTime(clock.nowUtc())
+                .setTransferPeriod(expectedPeriod)
+                .setTransferStatus(TransferStatus.SERVER_APPROVED)
+                .setPendingTransferExpirationTime(automaticTransferTime)
+                // Server-approve entity fields should all be nulled out.
+                .build());
   }
 
   /**
@@ -177,20 +205,22 @@ public class DomainTransferRequestFlowTest
     // Setup done; run the test.
     assertTransactionalFlow(true);
     runFlowAssertResponse(readFile(expectedXmlFilename, substitutions));
-    // Transfer should have been requested. Verify correct fields were set.
+    // Transfer should have been requested.
     domain = reloadResourceByForeignKey();
+    // Verify that HistoryEntry was created.
+    assertAboutDomains().that(domain)
+        .hasOneHistoryEntryEachOfTypes(DOMAIN_CREATE, DOMAIN_TRANSFER_REQUEST);
     final HistoryEntry historyEntryTransferRequest =
         getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST);
-    subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
-    assertTransferRequested(domain, Optional.<Duration>absent());
-    assertAboutDomains().that(domain)
-        .hasPendingTransferExpirationTime(implicitTransferTime).and()
-        .hasOneHistoryEntryEachOfTypes(DOMAIN_CREATE, DOMAIN_TRANSFER_REQUEST);
     assertAboutHistoryEntries()
         .that(historyEntryTransferRequest)
         .hasPeriodYears(1)
         .and()
         .hasOtherClientId("TheRegistrar");
+    // Verify correct fields were set.
+    assertTransferRequested(domain, implicitTransferTime, Period.create(1, Unit.YEARS));
+
+    subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
     assertAboutHosts().that(subordinateHost).hasNoHistoryEntries();
 
     assertHistoryEntriesContainBillingEventsAndGracePeriods(
@@ -201,11 +231,9 @@ public class DomainTransferRequestFlowTest
         /* expectTransferBillingEvent = */ true,
         extraExpectedBillingEvents);
 
-    assertPollMessagesEmitted(
+    assertPollMessagesEmitted(expectedExpirationTime, implicitTransferTime);
-        expectedExpirationTime, implicitTransferTime, Optional.<Duration>absent());
-
     assertAboutDomainAfterAutomaticTransfer(
-        expectedExpirationTime, implicitTransferTime, Optional.<Duration>absent());
+        expectedExpirationTime, implicitTransferTime, Period.create(1, Unit.YEARS));
   }
 
   private void assertHistoryEntriesContainBillingEventsAndGracePeriods(
@@ -220,6 +248,8 @@ public class DomainTransferRequestFlowTest
     final HistoryEntry historyEntryTransferRequest =
         getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST);
 
+    // Construct the billing events we expect to exist, starting with the (optional) billing
+    // event for the transfer itself.
     Optional<BillingEvent.OneTime> optionalTransferBillingEvent;
     if (expectTransferBillingEvent) {
       // For normal transfers, a BillingEvent should be created AUTOMATIC_TRANSFER_DAYS in the
@@ -241,28 +271,57 @@ public class DomainTransferRequestFlowTest
       // Superuser transfers with no bundled renewal have no transfer billing event.
       optionalTransferBillingEvent = Optional.<BillingEvent.OneTime>absent();
     }
-    // Assert that the billing events we expect are present - any extra cancellations, then the
+    // Construct the autorenew events for the losing/existing client and the gaining one. Note that
-    // transfer billing event if there is one, plus two autorenew billing events, one for the losing
+    // all of the other transfer flow tests happen on day 3 of the transfer, but the initial
-    // client ending at the transfer time, and one for the gaining client starting at the domain's
+    // request by definition takes place on day 1, so we need to edit the times in the
-    // post-transfer expiration time.
+    // autorenew events from the base test case.
-    assertBillingEvents(FluentIterable.from(extraExpectedBillingEvents)
+    BillingEvent.Recurring losingClientAutorenew =
-        .transform(new Function<BillingEvent.Cancellation.Builder, BillingEvent>() {
+        getLosingClientAutorenewEvent().asBuilder()
-          @Override
+            .setRecurrenceEndTime(implicitTransferTime)
-          public BillingEvent apply(Builder builder) {
+            .build();
-            return builder.setParent(historyEntryTransferRequest).build();
+    BillingEvent.Recurring gainingClientAutorenew =
-          }})
+        getGainingClientAutorenewEvent().asBuilder()
+            .setEventTime(expectedExpirationTime)
+            .build();
+    // Construct extra billing events expected by the specific test.
+    ImmutableList<BillingEvent> extraBillingEvents =
+        FluentIterable.from(extraExpectedBillingEvents)
+            .transform(
+                new Function<BillingEvent.Cancellation.Builder, BillingEvent>() {
+                  @Override
+                  public BillingEvent apply(Builder builder) {
+                    return builder.setParent(historyEntryTransferRequest).build();
+                  }})
+            .toList();
+    // Assert that the billing events we constructed above actually exist in datastore.
+    assertBillingEvents(FluentIterable.from(extraBillingEvents)
         .append(optionalTransferBillingEvent.asSet())
-        .append(
+        .append(losingClientAutorenew)
-            // All of the other transfer flow tests happen on day 3 of the transfer, but the initial
+        .append(gainingClientAutorenew)
-            // request by definition takes place on day 1, so we need to edit the times in the
-            // autorenew events from the base test case.
-            getLosingClientAutorenewEvent().asBuilder()
-                .setRecurrenceEndTime(implicitTransferTime)
-                .build(),
-            getGainingClientAutorenewEvent().asBuilder()
-                .setEventTime(expectedExpirationTime)
-                .build())
         .toArray(BillingEvent.class));
+    // Assert that the domain's TransferData server-approve billing events match the above.
+    if (expectTransferBillingEvent) {
+      assertBillingEventsEqual(
+          ofy().load().key(domain.getTransferData().getServerApproveBillingEvent()).now(),
+          optionalTransferBillingEvent.get());
+    } else {
+      assertThat(domain.getTransferData().getServerApproveBillingEvent()).isNull();
+    }
+    assertBillingEventsEqual(
+        ofy().load().key(domain.getTransferData().getServerApproveAutorenewEvent()).now(),
+        gainingClientAutorenew);
+    // Assert that the full set of server-approve billing events is exactly the extra ones plus
+    // the transfer billing event (if present) and the gaining client autorenew.
+    assertBillingEventsEqual(
+        Iterables.filter(
+            ofy().load()
+                // Use toArray() to coerce the type to something keys() will accept.
+                .keys(domain.getTransferData().getServerApproveEntities().toArray(new Key<?>[]{}))
+                .values(),
+            BillingEvent.class),
+        FluentIterable.from(extraBillingEvents)
+            .append(optionalTransferBillingEvent.asSet())
+            .append(gainingClientAutorenew));
     // The domain's autorenew billing event should still point to the losing client's event.
     BillingEvent.Recurring domainAutorenewEvent =
         ofy().load().key(domain.getAutorenewBillingEvent()).now();
@@ -291,18 +350,12 @@ public class DomainTransferRequestFlowTest
   }
 
   private void assertPollMessagesEmitted(
-      DateTime expectedExpirationTime,
+      DateTime expectedExpirationTime, DateTime implicitTransferTime) {
-      DateTime implicitTransferTime,
-      Optional<Duration> expectedAutomaticTransferLength) {
     // Assert that there exists a poll message to notify the losing registrar that a transfer was
-    // requested. If the expected automatic transfer length is zero, then also expect a server
+    // requested. If the implicit transfer time is now (i.e. the automatic transfer length is zero)
-    // approved poll message.
+    // then also expect a server approved poll message.
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc()))
-        .hasSize(
+        .hasSize(implicitTransferTime.equals(clock.nowUtc()) ? 2 : 1);
-            (expectedAutomaticTransferLength.isPresent()
-                    && expectedAutomaticTransferLength.get().equals(Duration.ZERO))
-                ? 2
-                : 1);
 
     // Two poll messages on the gaining registrar's side at the expected expiration time: a
     // (OneTime) transfer approved message, and an Autorenew poll message.
@@ -354,15 +407,32 @@ public class DomainTransferRequestFlowTest
             .filter(TransferResponse.class))
                 .getTransferStatus())
                 .isEqualTo(TransferStatus.SERVER_APPROVED);
+
+    // Assert that the poll messages show up in the TransferData server approve entities.
+    assertPollMessagesEqual(
+        ofy().load().key(domain.getTransferData().getServerApproveAutorenewPollMessage()).now(),
+        autorenewPollMessage);
+    // Assert that the full set of server-approve poll messages is exactly the server approve
+    // OneTime messages to gaining and losing registrars plus the gaining client autorenew.
+    assertPollMessagesEqual(
+        Iterables.filter(
+            ofy().load()
+                // Use toArray() to coerce the type to something keys() will accept.
+                .keys(domain.getTransferData().getServerApproveEntities().toArray(new Key<?>[]{}))
+                .values(),
+            PollMessage.class),
+        ImmutableList.of(
+            transferApprovedPollMessage,
+            losingTransferApprovedPollMessage,
+            autorenewPollMessage));
   }
 
   private void assertAboutDomainAfterAutomaticTransfer(
-      DateTime expectedExpirationTime,
+      DateTime expectedExpirationTime, DateTime implicitTransferTime, Period expectedPeriod)
-      DateTime implicitTransferTime,
+      throws Exception {
-      Optional<Duration> expectedAutomaticTransferLength) {
     Registry registry = Registry.get(domain.getTld());
     DomainResource domainAfterAutomaticTransfer = domain.cloneProjectedAtTime(implicitTransferTime);
-    assertTransferApproved(domainAfterAutomaticTransfer, expectedAutomaticTransferLength);
+    assertTransferApproved(domainAfterAutomaticTransfer, implicitTransferTime, expectedPeriod);
     assertAboutDomains().that(domainAfterAutomaticTransfer)
         .hasRegistrationExpirationTime(expectedExpirationTime);
     assertThat(ofy().load().key(domainAfterAutomaticTransfer.getAutorenewBillingEvent()).now()
@@ -440,11 +510,8 @@ public class DomainTransferRequestFlowTest
       // Transfer should have been requested.
       domain = reloadResourceByForeignKey();
     }
-    // Verify correct fields were set.
+    // Verify that HistoryEntry was created.
-    subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
-    assertTransferRequested(domain, Optional.of(expectedAutomaticTransferLength));
     assertAboutDomains().that(domain)
-        .hasPendingTransferExpirationTime(implicitTransferTime).and()
         .hasOneHistoryEntryEachOfTypes(DOMAIN_CREATE, DOMAIN_TRANSFER_REQUEST);
     final HistoryEntry historyEntryTransferRequest =
         getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST);
@@ -453,6 +520,10 @@ public class DomainTransferRequestFlowTest
         .hasPeriodYears(expectedPeriod.getValue())
         .and()
         .hasOtherClientId("TheRegistrar");
+    // Verify correct fields were set.
+    assertTransferRequested(domain, implicitTransferTime, expectedPeriod);
+
+    subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
     assertAboutHosts().that(subordinateHost).hasNoHistoryEntries();
 
     boolean expectTransferBillingEvent = expectedPeriod.getValue() != 0;
@@ -464,13 +535,9 @@ public class DomainTransferRequestFlowTest
         expectTransferBillingEvent,
         extraExpectedBillingEvents);
 
-    assertPollMessagesEmitted(
+    assertPollMessagesEmitted(expectedExpirationTime, implicitTransferTime);
-        expectedExpirationTime,
-        implicitTransferTime,
-        Optional.of(expectedAutomaticTransferLength));
-
     assertAboutDomainAfterAutomaticTransfer(
-        expectedExpirationTime, implicitTransferTime, Optional.of(expectedAutomaticTransferLength));
+        expectedExpirationTime, implicitTransferTime, expectedPeriod);
   }
 
   private void runTest(

javatests/google/registry/model/transfer/TransferDataTest.java

@@ -15,25 +15,16 @@
 package google.registry.model.transfer;
 
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.ofy.ObjectifyService.ofy;
-import static google.registry.testing.DatastoreHelper.createTld;
-import static google.registry.testing.DatastoreHelper.persistResource;
-import static google.registry.util.DateTimeUtils.END_OF_TIME;
-import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.model.billing.BillingEvent;
-import google.registry.model.billing.BillingEvent.Flag;
+import google.registry.model.domain.Period;
-import google.registry.model.billing.BillingEvent.Reason;
+import google.registry.model.eppcommon.Trid;
-import google.registry.model.domain.DomainResource;
 import google.registry.model.poll.PollMessage;
-import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
 import google.registry.testing.AppEngineRule;
-import google.registry.testing.DatastoreHelper;
-import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
@@ -50,102 +41,53 @@ public class TransferDataTest {
       .withDatastore()
       .build();
 
-  protected final DateTime now = DateTime.now(UTC);
+  private final DateTime now = DateTime.now(UTC);
 
-  HistoryEntry historyEntry;
+  private Key<BillingEvent.OneTime> transferBillingEventKey;
-  TransferData transferData;
+  private Key<BillingEvent.Cancellation> otherServerApproveBillingEventKey;
-  BillingEvent.OneTime transferBillingEvent;
+  private Key<BillingEvent.Recurring> recurringBillingEventKey;
-  BillingEvent.OneTime nonTransferBillingEvent;
+  private Key<PollMessage.Autorenew> autorenewPollMessageKey;
-  BillingEvent.OneTime otherTransferBillingEvent;
+  private Key<PollMessage.OneTime> otherServerApprovePollMessageKey;
-  BillingEvent.Recurring recurringBillingEvent;
 
   @Before
   public void setUp() {
-    createTld("tld");
+    transferBillingEventKey = Key.create(BillingEvent.OneTime.class, 12345);
-    DomainResource domain = DatastoreHelper.persistActiveDomain("tat.tld");
+    otherServerApproveBillingEventKey = Key.create(BillingEvent.Cancellation.class, 2468);
-    historyEntry = persistResource(new HistoryEntry.Builder().setParent(domain).build());
+    recurringBillingEventKey = Key.create(BillingEvent.Recurring.class, 13579);
-    transferBillingEvent = persistResource(makeBillingEvent());
+    autorenewPollMessageKey = Key.create(PollMessage.Autorenew.class, 67890);
-
+    otherServerApprovePollMessageKey = Key.create(PollMessage.OneTime.class, 314159);
-    nonTransferBillingEvent = persistResource(
-        makeBillingEvent().asBuilder().setReason(Reason.CREATE).build());
-
-    otherTransferBillingEvent = persistResource(
-        makeBillingEvent().asBuilder().setCost(Money.of(USD, 33)).build());
-
-    recurringBillingEvent = persistResource(
-        new BillingEvent.Recurring.Builder()
-            .setReason(Reason.RENEW)
-            .setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
-            .setClientId("TheRegistrar")
-            .setTargetId("foo.tld")
-            .setEventTime(now)
-            .setRecurrenceEndTime(END_OF_TIME)
-            .setParent(historyEntry)
-            .build());
-  }
-
-  private BillingEvent.OneTime makeBillingEvent() {
-    return new BillingEvent.OneTime.Builder()
-        .setReason(Reason.TRANSFER)
-        .setClientId("TheRegistrar")
-        .setTargetId("foo.tld")
-        .setEventTime(now)
-        .setBillingTime(now.plusDays(5))
-        .setCost(Money.of(USD, 42))
-        .setPeriodYears(3)
-        .setParent(historyEntry)
-        .build();
-  }
-
-  @SafeVarargs
-  private static TransferData makeTransferDataWithEntities(
-      Key<? extends TransferServerApproveEntity>... entityKeys) {
-    ImmutableSet<Key<? extends TransferServerApproveEntity>> entityKeysSet =
-        ImmutableSet.copyOf(entityKeys);
-    return new TransferData.Builder().setServerApproveEntities(entityKeysSet).build();
   }
 
   @Test
-  public void testSuccess_FindBillingEventNoEntities() throws Exception {
+  public void test_copyConstantFieldsToBuilder() throws Exception {
-    transferData = makeTransferDataWithEntities();
+    TransferData constantTransferData =
-    assertThat(transferData.serverApproveBillingEvent).isNull();
+        new TransferData.Builder()
-    assertThat(transferData.getServerApproveBillingEvent()).isNull();
+            .setTransferRequestTrid(Trid.create("server-trid", "client-trid"))
-  }
+            .setTransferRequestTime(now)
-
+            .setGainingClientId("NewRegistrar")
-  @Test
+            .setLosingClientId("TheRegistrar")
-  public void testSuccess_FindBillingEventOtherEntities() throws Exception {
+            // Test must use a non-1-year period, since that's the default value.
-    transferData = makeTransferDataWithEntities(
+            .setTransferPeriod(Period.create(5, Period.Unit.YEARS))
-        Key.create(nonTransferBillingEvent),
+            .build();
-        Key.create(recurringBillingEvent),
+    TransferData fullTransferData =
-        Key.create(PollMessage.OneTime.class, 1));
+        constantTransferData.asBuilder()
-    assertThat(transferData.serverApproveBillingEvent).isNull();
+            .setPendingTransferExpirationTime(now)
-    assertThat(transferData.getServerApproveBillingEvent()).isNull();
+            .setTransferStatus(TransferStatus.PENDING)
-  }
+            .setServerApproveEntities(
-
+                ImmutableSet.<Key<? extends TransferServerApproveEntity>>of(
-  @Test
+                    transferBillingEventKey,
-  public void testSuccess_GetStoredBillingEventNoEntities() throws Exception {
+                    otherServerApproveBillingEventKey,
-    transferData = new TransferData.Builder()
+                    recurringBillingEventKey,
-        .setServerApproveBillingEvent(Key.create(transferBillingEvent))
+                    autorenewPollMessageKey,
-        .build();
+                    otherServerApprovePollMessageKey))
-    assertThat(ofy().load().key(transferData.serverApproveBillingEvent).now())
+            .setServerApproveBillingEvent(transferBillingEventKey)
-        .isEqualTo(transferBillingEvent);
+            .setServerApproveAutorenewEvent(recurringBillingEventKey)
-    assertThat(ofy().load().key(transferData.getServerApproveBillingEvent()).now())
+            .setServerApproveAutorenewPollMessage(autorenewPollMessageKey)
-        .isEqualTo(transferBillingEvent);
+            .build();
-  }
+    // asBuilder() copies over all fields
-
+    assertThat(fullTransferData.asBuilder().build()).isEqualTo(fullTransferData);
-  @Test
+    // copyConstantFieldsToBuilder() copies only constant fields
-  public void testSuccess_GetStoredBillingEventMultipleEntities() throws Exception {
+    assertThat(fullTransferData.copyConstantFieldsToBuilder().build())
-    transferData = makeTransferDataWithEntities(
+        .isEqualTo(constantTransferData);
-        Key.create(otherTransferBillingEvent),
-        Key.create(nonTransferBillingEvent),
-        Key.create(recurringBillingEvent),
-        Key.create(PollMessage.OneTime.class, 1));
-    transferData = transferData.asBuilder()
-        .setServerApproveBillingEvent(Key.create(transferBillingEvent))
-        .build();
-    assertThat(ofy().load().key(transferData.serverApproveBillingEvent).now())
-        .isEqualTo(transferBillingEvent);
-    assertThat(ofy().load().key(transferData.getServerApproveBillingEvent()).now())
-        .isEqualTo(transferBillingEvent);
   }
 }

javatests/google/registry/testing/ContactResourceSubject.java

@@ -22,7 +22,6 @@ import com.google.common.truth.SimpleSubjectBuilder;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.contact.PostalInfo;
 import google.registry.model.eppcommon.AuthInfo;
-import google.registry.model.transfer.TransferStatus;
 import google.registry.testing.TruthChainer.And;
 import org.joda.time.DateTime;
 
@@ -126,42 +125,6 @@ public final class ContactResourceSubject
     return hasValue(pw, authInfo == null ? null : authInfo.getPw().getValue(), "has auth info pw");
   }
 
-  public And<ContactResourceSubject> hasTransferStatus(TransferStatus transferStatus) {
-    return hasValue(
-        transferStatus,
-        actual().getTransferData().getTransferStatus(),
-        "has transferStatus");
-  }
-
-  public And<ContactResourceSubject> hasTransferRequestClientTrid(String clTrid) {
-    return hasValue(
-        clTrid,
-        actual().getTransferData().getTransferRequestTrid().getClientTransactionId(),
-        "has trid");
-  }
-
-  public And<ContactResourceSubject> hasPendingTransferExpirationTime(
-      DateTime pendingTransferExpirationTime) {
-    return hasValue(
-        pendingTransferExpirationTime,
-        actual().getTransferData().getPendingTransferExpirationTime(),
-        "has pendingTransferExpirationTime");
-  }
-
-  public And<ContactResourceSubject> hasTransferGainingClientId(String gainingClientId) {
-    return hasValue(
-        gainingClientId,
-        actual().getTransferData().getGainingClientId(),
-        "has transfer ga");
-  }
-
-  public And<ContactResourceSubject> hasTransferLosingClientId(String losingClientId) {
-    return hasValue(
-        losingClientId,
-        actual().getTransferData().getLosingClientId(),
-        "has transfer losingClientId");
-  }
-
   public And<ContactResourceSubject> hasLastTransferTime(DateTime lastTransferTime) {
     return hasValue(
         lastTransferTime,

javatests/google/registry/testing/DatastoreHelper.java

@@ -99,6 +99,7 @@ import google.registry.model.transfer.TransferData.Builder;
 import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
 import google.registry.model.transfer.TransferStatus;
 import google.registry.tmch.LordnTask;
+import java.util.Arrays;
 import java.util.List;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -645,19 +646,27 @@ public class DatastoreHelper {
         ofy().load().type(BillingEvent.Cancellation.class).ancestor(resource));
   }
 
+  /** Assert that the actual billing event matches the expected one, ignoring IDs. */
+  public static void assertBillingEventsEqual(BillingEvent actual, BillingEvent expected) {
+    assertThat(BILLING_EVENT_ID_STRIPPER.apply(actual))
+        .isEqualTo(BILLING_EVENT_ID_STRIPPER.apply(expected));
+  }
+
+  /** Assert that the actual billing events match the expected ones, ignoring IDs and order. */
+  public static void assertBillingEventsEqual(
+      Iterable<BillingEvent> actual, Iterable<BillingEvent> expected) {
+    assertThat(Iterables.transform(actual, BILLING_EVENT_ID_STRIPPER))
+        .containsExactlyElementsIn(Iterables.transform(expected, BILLING_EVENT_ID_STRIPPER));
+  }
 
   /** Assert that the expected billing events are exactly the ones found in the fake Datastore. */
   public static void assertBillingEvents(BillingEvent... expected) throws Exception {
-    assertThat(FluentIterable.from(getBillingEvents()).transform(BILLING_EVENT_ID_STRIPPER))
+    assertBillingEventsEqual(getBillingEvents(), Arrays.asList(expected));
-        .containsExactlyElementsIn(
-            FluentIterable.from(expected).transform(BILLING_EVENT_ID_STRIPPER));
   }
 
   /** Assert that the expected billing events set is exactly the one found in the fake Datastore. */
   public static void assertBillingEvents(ImmutableSet<BillingEvent> expected) throws Exception {
-    assertThat(FluentIterable.from(getBillingEvents()).transform(BILLING_EVENT_ID_STRIPPER))
+    assertBillingEventsEqual(getBillingEvents(), expected);
-        .containsExactlyElementsIn(
-            FluentIterable.from(expected.asList()).transform(BILLING_EVENT_ID_STRIPPER));
   }
 
   /**
@@ -684,6 +693,19 @@ public class DatastoreHelper {
           return billingEvent.asBuilder().setId(1L).build();
         }};
 
+  /** Assert that the actual poll message matches the expected one, ignoring IDs. */
+  public static void assertPollMessagesEqual(PollMessage actual, PollMessage expected) {
+    assertThat(POLL_MESSAGE_ID_STRIPPER.apply(actual))
+        .isEqualTo(POLL_MESSAGE_ID_STRIPPER.apply(expected));
+  }
+
+  /** Assert that the actual poll messages match the expected ones, ignoring IDs and order. */
+  public static void assertPollMessagesEqual(
+      Iterable<PollMessage> actual, Iterable<PollMessage> expected) {
+    assertThat(Iterables.transform(actual, POLL_MESSAGE_ID_STRIPPER))
+        .containsExactlyElementsIn(Iterables.transform(expected, POLL_MESSAGE_ID_STRIPPER));
+  }
+
   public static void assertPollMessagesForResource(EppResource resource, PollMessage... expected)
       throws Exception {
     assertThat(FluentIterable.from(getPollMessages(resource)).transform(POLL_MESSAGE_ID_STRIPPER))

javatests/google/registry/testing/DomainResourceSubject.java

@@ -20,7 +20,6 @@ import static com.google.common.truth.Truth.assertAbout;
 import com.google.common.truth.FailureStrategy;
 import com.google.common.truth.SimpleSubjectBuilder;
 import google.registry.model.domain.DomainResource;
-import google.registry.model.transfer.TransferStatus;
 import google.registry.testing.TruthChainer.And;
 import java.util.Objects;
 import org.joda.time.DateTime;
@@ -33,42 +32,6 @@ public final class DomainResourceSubject
   private static class SubjectFactory
       extends ReflectiveSubjectFactory<DomainResource, DomainResourceSubject>{}
 
-  public And<DomainResourceSubject> hasTransferStatus(TransferStatus transferStatus) {
-    return hasValue(
-        transferStatus,
-        actual().getTransferData().getTransferStatus(),
-        "has transferStatus");
-  }
-
-  public And<DomainResourceSubject> hasTransferRequestClientTrid(String clTrid) {
-    return hasValue(
-        clTrid,
-        actual().getTransferData().getTransferRequestTrid().getClientTransactionId(),
-        "has trid");
-  }
-
-  public And<DomainResourceSubject> hasPendingTransferExpirationTime(
-      DateTime pendingTransferExpirationTime) {
-    return hasValue(
-        pendingTransferExpirationTime,
-        actual().getTransferData().getPendingTransferExpirationTime(),
-        "has pendingTransferExpirationTime");
-  }
-
-  public And<DomainResourceSubject> hasTransferGainingClientId(String gainingClientId) {
-    return hasValue(
-        gainingClientId,
-        actual().getTransferData().getGainingClientId(),
-        "has transfer ga");
-  }
-
-  public And<DomainResourceSubject> hasTransferLosingClientId(String losingClientId) {
-    return hasValue(
-        losingClientId,
-        actual().getTransferData().getLosingClientId(),
-        "has transfer losingClientId");
-  }
-
   public And<DomainResourceSubject> hasRegistrationExpirationTime(DateTime expiration) {
     if (!Objects.equals(actual().getRegistrationExpirationTime(), expiration)) {
       failWithBadResults(