Convert ofy() to tm() for all contact transfer flows (#937)

* Convert ofy() to tm() for all contact transfer flows

* Resolve comments

core/src/main/java/google/registry/flows/contact/ContactTransferApproveFlow.java

@@ -22,9 +22,9 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
 import static google.registry.flows.contact.ContactFlowUtils.createGainingTransferPollMessage;
 import static google.registry.flows.contact.ContactFlowUtils.createTransferResponse;
 import static google.registry.model.ResourceTransferUtils.approvePendingTransfer;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
+import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException;
 import google.registry.flows.ExtensionManager;
@@ -94,7 +94,8 @@ public final class ContactTransferApproveFlow implements TransactionalFlow {
     // Create a poll message for the gaining client.
     PollMessage gainingPollMessage =
         createGainingTransferPollMessage(targetId, newContact.getTransferData(), historyEntry);
-    ofy().save().<Object>entities(newContact, historyEntry, gainingPollMessage);
+    tm().insertAll(ImmutableSet.of(historyEntry.toChildHistoryEntity(), gainingPollMessage));
+    tm().update(newContact);
     // Delete the billing event and poll messages that were written in case the transfer would have
     // been implicitly server approved.
     tm().delete(existingContact.getTransferData().getServerApproveEntities());

core/src/main/java/google/registry/flows/contact/ContactTransferCancelFlow.java

@@ -22,9 +22,9 @@ import static google.registry.flows.ResourceFlowUtils.verifyTransferInitiator;
 import static google.registry.flows.contact.ContactFlowUtils.createLosingTransferPollMessage;
 import static google.registry.flows.contact.ContactFlowUtils.createTransferResponse;
 import static google.registry.model.ResourceTransferUtils.denyPendingTransfer;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
+import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException;
 import google.registry.flows.ExtensionManager;
@@ -90,7 +90,8 @@ public final class ContactTransferCancelFlow implements TransactionalFlow {
     // Create a poll message for the losing client.
     PollMessage losingPollMessage =
         createLosingTransferPollMessage(targetId, newContact.getTransferData(), historyEntry);
-    ofy().save().<Object>entities(newContact, historyEntry, losingPollMessage);
+    tm().insertAll(ImmutableSet.of(historyEntry.toChildHistoryEntity(), losingPollMessage));
+    tm().update(newContact);
     // Delete the billing event and poll messages that were written in case the transfer would have
     // been implicitly server approved.
     tm().delete(existingContact.getTransferData().getServerApproveEntities());

core/src/main/java/google/registry/flows/contact/ContactTransferRejectFlow.java

@@ -22,9 +22,9 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
 import static google.registry.flows.contact.ContactFlowUtils.createGainingTransferPollMessage;
 import static google.registry.flows.contact.ContactFlowUtils.createTransferResponse;
 import static google.registry.model.ResourceTransferUtils.denyPendingTransfer;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
+import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException;
 import google.registry.flows.ExtensionManager;
@@ -87,7 +87,8 @@ public final class ContactTransferRejectFlow implements TransactionalFlow {
         .build();
     PollMessage gainingPollMessage =
         createGainingTransferPollMessage(targetId, newContact.getTransferData(), historyEntry);
-    ofy().save().<Object>entities(newContact, historyEntry, gainingPollMessage);
+    tm().insertAll(ImmutableSet.of(historyEntry.toChildHistoryEntity(), gainingPollMessage));
+    tm().update(newContact);
     // Delete the billing event and poll messages that were written in case the transfer would have
     // been implicitly server approved.
     tm().delete(existingContact.getTransferData().getServerApproveEntities());

core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java

@@ -23,7 +23,6 @@ import static google.registry.flows.contact.ContactFlowUtils.createGainingTransf
 import static google.registry.flows.contact.ContactFlowUtils.createLosingTransferPollMessage;
 import static google.registry.flows.contact.ContactFlowUtils.createTransferResponse;
 import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.common.collect.ImmutableSet;
@@ -145,12 +144,13 @@ public final class ContactTransferRequestFlow implements TransactionalFlow {
         .setTransferData(pendingTransferData)
         .addStatusValue(StatusValue.PENDING_TRANSFER)
         .build();
-    ofy().save().<Object>entities(
+    tm().update(newContact);
-        newContact,
+    tm().insertAll(
-        historyEntry,
+            ImmutableSet.of(
+                historyEntry.toChildHistoryEntity(),
                 requestPollMessage,
                 serverApproveGainingPollMessage,
-        serverApproveLosingPollMessage);
+                serverApproveLosingPollMessage));
     return responseBuilder
         .setResultFromCode(SUCCESS_WITH_ACTION_PENDING)
         .setResData(createTransferResponse(targetId, newContact.getTransferData()))

core/src/main/java/google/registry/model/poll/PollMessage.java

@@ -16,6 +16,7 @@ package google.registry.model.poll;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.util.CollectionUtils.forceEmptyToNull;
+import static google.registry.util.CollectionUtils.isNullOrEmpty;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
@@ -378,6 +379,47 @@ public abstract class PollMessage extends ImmutableObject
           .build();
     }
 
+    @Override
+    @OnLoad
+    void onLoad() {
+      super.onLoad();
+      if (!isNullOrEmpty(contactPendingActionNotificationResponses)) {
+        pendingActionNotificationResponse = contactPendingActionNotificationResponses.get(0);
+      }
+      if (!isNullOrEmpty(contactTransferResponses)) {
+        contactId = contactTransferResponses.get(0).getContactId();
+        transferResponse = contactTransferResponses.get(0);
+      }
+    }
+
+    @Override
+    @PostLoad
+    void postLoad() {
+      super.postLoad();
+      if (pendingActionNotificationResponse != null) {
+        contactPendingActionNotificationResponses =
+            ImmutableList.of(
+                ContactPendingActionNotificationResponse.create(
+                    pendingActionNotificationResponse.nameOrId.value,
+                    pendingActionNotificationResponse.getActionResult(),
+                    pendingActionNotificationResponse.getTrid(),
+                    pendingActionNotificationResponse.processedDate));
+      }
+      if (contactId != null && transferResponse != null) {
+        contactTransferResponses =
+            ImmutableList.of(
+                new ContactTransferResponse.Builder()
+                    .setContactId(contactId)
+                    .setGainingClientId(transferResponse.getGainingClientId())
+                    .setLosingClientId(transferResponse.getLosingClientId())
+                    .setTransferStatus(transferResponse.getTransferStatus())
+                    .setTransferRequestTime(transferResponse.getTransferRequestTime())
+                    .setPendingTransferExpirationTime(
+                        transferResponse.getPendingTransferExpirationTime())
+                    .build());
+      }
+    }
+
     /** A builder for {@link OneTime} since it is immutable. */
     public static class Builder extends PollMessage.Builder<OneTime, Builder> {
 
@@ -396,6 +438,10 @@ public abstract class PollMessage extends ImmutableObject
                     .filter(ContactPendingActionNotificationResponse.class::isInstance)
                     .map(ContactPendingActionNotificationResponse.class::cast)
                     .collect(toImmutableList()));
+        if (getInstance().contactPendingActionNotificationResponses != null) {
+          getInstance().pendingActionNotificationResponse =
+              getInstance().contactPendingActionNotificationResponses.get(0);
+        }
         getInstance().contactTransferResponses =
             forceEmptyToNull(
                 responseData
@@ -403,6 +449,11 @@ public abstract class PollMessage extends ImmutableObject
                     .filter(ContactTransferResponse.class::isInstance)
                     .map(ContactTransferResponse.class::cast)
                     .collect(toImmutableList()));
+        if (getInstance().contactTransferResponses != null) {
+          getInstance().contactId = getInstance().contactTransferResponses.get(0).getContactId();
+          getInstance().transferResponse = getInstance().contactTransferResponses.get(0);
+        }
+
         getInstance().domainPendingActionNotificationResponses =
             forceEmptyToNull(
                 responseData

core/src/main/java/google/registry/model/transfer/TransferData.java

@@ -14,15 +14,25 @@
 
 package google.registry.model.transfer;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static google.registry.util.CollectionUtils.isNullOrEmpty;
+import static google.registry.util.CollectionUtils.nullToEmpty;
 import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.googlecode.objectify.Key;
+import com.googlecode.objectify.annotation.AlsoLoad;
 import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.IgnoreSave;
 import com.googlecode.objectify.condition.IfNull;
 import google.registry.model.Buildable;
 import google.registry.model.EppResource;
+import google.registry.model.contact.ContactResource;
+import google.registry.model.domain.DomainBase;
 import google.registry.model.eppcommon.Trid;
+import google.registry.model.poll.PollMessage;
+import google.registry.model.reporting.HistoryEntry;
 import google.registry.persistence.VKey;
 import google.registry.util.TypeUtils.TypeInstantiator;
 import java.util.Set;
@@ -32,6 +42,7 @@ import javax.persistence.AttributeOverrides;
 import javax.persistence.Column;
 import javax.persistence.Embedded;
 import javax.persistence.MappedSuperclass;
+import javax.persistence.PostLoad;
 import javax.persistence.Transient;
 
 /**
@@ -68,15 +79,27 @@ public abstract class TransferData<
   @IgnoreSave(IfNull.class)
   Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities;
 
-  // The following 3 fields are the replacement for serverApproveEntities in Cloud SQL.
-  // TODO(b/177589157): Make transfer flows work with Cloud SQL.
   @Ignore
-  @Column(name = "transfer_gaining_poll_message_id")
+  @Column(name = "transfer_repo_id")
-  Long gainingTransferPollMessageId;
+  String repoId;
 
   @Ignore
-  @Column(name = "transfer_losing_poll_message_id")
+  @Column(name = "transfer_history_entry_id")
-  Long losingTransferPollMessageId;
+  Long historyEntryId;
+
+  // The pollMessageId1 and pollMessageId2 are used to store the IDs for gaining and losing poll
+  // messages in Cloud SQL, and they are added to replace the VKeys in serverApproveEntities.
+  // Although we can distinguish which is which when we construct the TransferData instance from
+  // the transfer request flow, when the instance is loaded from Datastore, we cannot make this
+  // distinction because they are just VKeys. Also, the only way we use serverApproveEntities is to
+  // just delete all the entities referenced by the VKeys, so we don't need to make the distinction.
+  @Ignore
+  @Column(name = "transfer_poll_message_id_1")
+  Long pollMessageId1;
+
+  @Ignore
+  @Column(name = "transfer_poll_message_id_2")
+  Long pollMessageId2;
 
   public abstract boolean isEmpty();
 
@@ -116,6 +139,83 @@ public abstract class TransferData<
     return newBuilder;
   }
 
+  void onLoad(
+      @AlsoLoad("serverApproveEntities")
+          Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities) {
+    mapServerApproveEntitiesToFields(serverApproveEntities, this);
+  }
+
+  @PostLoad
+  void postLoad() {
+    mapFieldsToServerApproveEntities();
+  }
+
+  /**
+   * Reconstructs serverApproveEntities set from the individual fields, e.g. repoId, historyEntryId,
+   * pollMessageId1.
+   */
+  void mapFieldsToServerApproveEntities() {
+    if (repoId == null) {
+      return;
+    }
+    Key<? extends EppResource> eppKey;
+    if (getClass().equals(DomainBase.class)) {
+      eppKey = Key.create(DomainBase.class, repoId);
+    } else {
+      eppKey = Key.create(ContactResource.class, repoId);
+    }
+    Key<HistoryEntry> historyEntryKey = Key.create(eppKey, HistoryEntry.class, historyEntryId);
+    ImmutableSet.Builder<VKey<? extends TransferServerApproveEntity>> entityKeysBuilder =
+        new ImmutableSet.Builder<>();
+    if (pollMessageId1 != null) {
+      Key<PollMessage> ofyKey = Key.create(historyEntryKey, PollMessage.class, pollMessageId1);
+      entityKeysBuilder.add(PollMessage.createVKey(ofyKey));
+    }
+    if (pollMessageId2 != null) {
+      Key<PollMessage> ofyKey = Key.create(historyEntryKey, PollMessage.class, pollMessageId2);
+      entityKeysBuilder.add(PollMessage.createVKey(ofyKey));
+    }
+    serverApproveEntities = entityKeysBuilder.build();
+  }
+
+  /** Maps serverApproveEntities set to the individual fields. */
+  static void mapServerApproveEntitiesToFields(
+      Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities,
+      TransferData transferData) {
+    if (isNullOrEmpty(serverApproveEntities)) {
+      transferData.historyEntryId = null;
+      transferData.repoId = null;
+      transferData.pollMessageId1 = null;
+      transferData.pollMessageId2 = null;
+      return;
+    }
+    // Each element in serverApproveEntities should have the exact same Key<HistoryEntry> as its
+    // parent. So, we can use any to set historyEntryId and repoId.
+    Key<?> key = serverApproveEntities.iterator().next().getOfyKey();
+    transferData.historyEntryId = key.getParent().getId();
+    transferData.repoId = key.getParent().getParent().getName();
+
+    ImmutableList<Long> sortedPollMessageIds = getSortedPollMessageIds(serverApproveEntities);
+    if (sortedPollMessageIds.size() >= 1) {
+      transferData.pollMessageId1 = sortedPollMessageIds.get(0);
+    }
+    if (sortedPollMessageIds.size() >= 2) {
+      transferData.pollMessageId2 = sortedPollMessageIds.get(1);
+    }
+  }
+
+  /**
+   * Gets poll message IDs from the given serverApproveEntities and sorted the IDs in natural order.
+   */
+  private static ImmutableList<Long> getSortedPollMessageIds(
+      Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities) {
+    return nullToEmpty(serverApproveEntities).stream()
+        .filter(vKey -> PollMessage.class.isAssignableFrom(vKey.getKind()))
+        .map(vKey -> (long) vKey.getSqlKey())
+        .sorted()
+        .collect(toImmutableList());
+  }
+
   /** Builder for {@link TransferData} because it is immutable. */
   public abstract static class Builder<T extends TransferData, B extends Builder<T, B>>
       extends BaseTransferObject.Builder<T, B> {
@@ -141,6 +241,7 @@ public abstract class TransferData<
 
     @Override
     public T build() {
+      mapServerApproveEntitiesToFields(getInstance().serverApproveEntities, getInstance());
       return super.build();
     }
   }

core/src/test/java/google/registry/flows/contact/ContactTransferApproveFlowTest.java

@@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatabaseHelper.assertNoBillingEvents;
 import static google.registry.testing.DatabaseHelper.createTld;
-import static google.registry.testing.DatabaseHelper.deleteResource;
 import static google.registry.testing.DatabaseHelper.getOnlyPollMessage;
 import static google.registry.testing.DatabaseHelper.getPollMessages;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -41,10 +40,12 @@ 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.DualDatabaseTest;
+import google.registry.testing.TestOfyAndSql;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactTransferApproveFlow}. */
+@DualDatabaseTest
 class ContactTransferApproveFlowTest
     extends ContactTransferFlowTestCase<ContactTransferApproveFlow, ContactResource> {
 
@@ -121,24 +122,24 @@ class ContactTransferApproveFlowTest
     runFlow();
   }
 
-  @Test
+  @TestOfyAndSql
   void testDryRun() throws Exception {
     setEppInput("contact_transfer_approve.xml");
     dryRunFlowAssertResponse(loadFile("contact_transfer_approve_response.xml"));
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess() throws Exception {
     doSuccessfulTest("contact_transfer_approve.xml", "contact_transfer_approve_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_withAuthinfo() throws Exception {
     doSuccessfulTest("contact_transfer_approve_with_authinfo.xml",
         "contact_transfer_approve_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badContactPassword() {
     // Change the contact's password so it does not match the password in the file.
     contact = persistResource(
@@ -152,7 +153,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_neverBeenTransferred() {
     changeTransferStatus(null);
     EppException thrown =
@@ -161,7 +162,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientApproved() {
     changeTransferStatus(TransferStatus.CLIENT_APPROVED);
     EppException thrown =
@@ -170,7 +171,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientRejected() {
     changeTransferStatus(TransferStatus.CLIENT_REJECTED);
     EppException thrown =
@@ -179,7 +180,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientCancelled() {
     changeTransferStatus(TransferStatus.CLIENT_CANCELLED);
     EppException thrown =
@@ -188,7 +189,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverApproved() {
     changeTransferStatus(TransferStatus.SERVER_APPROVED);
     EppException thrown =
@@ -197,7 +198,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverCancelled() {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     EppException thrown =
@@ -206,7 +207,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_gainingClient() {
     setClientIdForFlow("NewRegistrar");
     EppException thrown =
@@ -215,7 +216,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_unrelatedClient() {
     setClientIdForFlow("ClientZ");
     EppException thrown =
@@ -224,7 +225,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_deletedContact() throws Exception {
     contact = persistResource(
         contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
@@ -236,9 +237,9 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_nonexistentContact() throws Exception {
-    deleteResource(contact);
+    persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     contact = persistResource(
         contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     ResourceDoesNotExistException thrown =
@@ -249,7 +250,7 @@ class ContactTransferApproveFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-transfer-approve");

core/src/test/java/google/registry/flows/contact/ContactTransferCancelFlowTest.java

@@ -18,7 +18,6 @@ import static com.google.common.collect.MoreCollectors.onlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatabaseHelper.assertNoBillingEvents;
-import static google.registry.testing.DatabaseHelper.deleteResource;
 import static google.registry.testing.DatabaseHelper.getOnlyPollMessage;
 import static google.registry.testing.DatabaseHelper.getPollMessages;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -38,10 +37,12 @@ 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.DualDatabaseTest;
+import google.registry.testing.TestOfyAndSql;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactTransferCancelFlow}. */
+@DualDatabaseTest
 class ContactTransferCancelFlowTest
     extends ContactTransferFlowTestCase<ContactTransferCancelFlow, ContactResource> {
 
@@ -105,24 +106,24 @@ class ContactTransferCancelFlowTest
     runFlow();
   }
 
-  @Test
+  @TestOfyAndSql
   void testDryRun() throws Exception {
     setEppInput("contact_transfer_cancel.xml");
     dryRunFlowAssertResponse(loadFile("contact_transfer_cancel_response.xml"));
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess() throws Exception {
     doSuccessfulTest("contact_transfer_cancel.xml", "contact_transfer_cancel_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_withAuthinfo() throws Exception {
     doSuccessfulTest("contact_transfer_cancel_with_authinfo.xml",
         "contact_transfer_cancel_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badContactPassword() {
     // Change the contact's password so it does not match the password in the file.
     contact =
@@ -138,7 +139,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_neverBeenTransferred() {
     changeTransferStatus(null);
     EppException thrown =
@@ -147,7 +148,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientApproved() {
     changeTransferStatus(TransferStatus.CLIENT_APPROVED);
     EppException thrown =
@@ -156,7 +157,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientRejected() {
     changeTransferStatus(TransferStatus.CLIENT_REJECTED);
     EppException thrown =
@@ -165,7 +166,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientCancelled() {
     changeTransferStatus(TransferStatus.CLIENT_CANCELLED);
     EppException thrown =
@@ -174,7 +175,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverApproved() {
     changeTransferStatus(TransferStatus.SERVER_APPROVED);
     EppException thrown =
@@ -183,7 +184,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverCancelled() {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     EppException thrown =
@@ -192,7 +193,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_sponsoringClient() {
     setClientIdForFlow("TheRegistrar");
     EppException thrown =
@@ -202,7 +203,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_unrelatedClient() {
     setClientIdForFlow("ClientZ");
     EppException thrown =
@@ -212,7 +213,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_deletedContact() throws Exception {
     contact =
         persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
@@ -224,9 +225,9 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_nonexistentContact() throws Exception {
-    deleteResource(contact);
+    persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     ResourceDoesNotExistException thrown =
         assertThrows(
             ResourceDoesNotExistException.class,
@@ -235,7 +236,7 @@ class ContactTransferCancelFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-transfer-cancel");

core/src/test/java/google/registry/flows/contact/ContactTransferQueryFlowTest.java

@@ -17,7 +17,6 @@ package google.registry.flows.contact;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatabaseHelper.assertNoBillingEvents;
-import static google.registry.testing.DatabaseHelper.deleteResource;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -32,11 +31,13 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.TransferStatus;
+import google.registry.testing.DualDatabaseTest;
+import google.registry.testing.TestOfyAndSql;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactTransferQueryFlow}. */
+@DualDatabaseTest
 class ContactTransferQueryFlowTest
     extends ContactTransferFlowTestCase<ContactTransferQueryFlow, ContactResource> {
 
@@ -68,65 +69,65 @@ class ContactTransferQueryFlowTest
     runFlow();
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess() throws Exception {
     doSuccessfulTest("contact_transfer_query.xml", "contact_transfer_query_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_withContactRoid() throws Exception {
     doSuccessfulTest("contact_transfer_query_with_roid.xml", "contact_transfer_query_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_sponsoringClient() throws Exception {
     setClientIdForFlow("TheRegistrar");
     doSuccessfulTest("contact_transfer_query.xml", "contact_transfer_query_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_withAuthinfo() throws Exception {
     setClientIdForFlow("ClientZ");
     doSuccessfulTest("contact_transfer_query_with_authinfo.xml",
         "contact_transfer_query_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientApproved() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_APPROVED);
     doSuccessfulTest("contact_transfer_query.xml",
         "contact_transfer_query_response_client_approved.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientRejected() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_REJECTED);
     doSuccessfulTest("contact_transfer_query.xml",
         "contact_transfer_query_response_client_rejected.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientCancelled() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_CANCELLED);
     doSuccessfulTest("contact_transfer_query.xml",
         "contact_transfer_query_response_client_cancelled.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_serverApproved() throws Exception {
     changeTransferStatus(TransferStatus.SERVER_APPROVED);
     doSuccessfulTest("contact_transfer_query.xml",
         "contact_transfer_query_response_server_approved.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_serverCancelled() throws Exception {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     doSuccessfulTest("contact_transfer_query.xml",
         "contact_transfer_query_response_server_cancelled.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_pendingDeleteContact() throws Exception {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     contact = persistResource(
@@ -135,7 +136,7 @@ class ContactTransferQueryFlowTest
         "contact_transfer_query_response_server_cancelled.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badContactPassword() {
     // Change the contact's password so it does not match the password in the file.
     contact =
@@ -151,7 +152,7 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badContactRoid() {
     // Set the contact to a different ROID, but don't persist it; this is just so the substitution
     // code above will write the wrong ROID into the file.
@@ -163,7 +164,7 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_neverBeenTransferred() {
     changeTransferStatus(null);
     EppException thrown =
@@ -173,7 +174,7 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_unrelatedClient() {
     setClientIdForFlow("ClientZ");
     EppException thrown =
@@ -183,7 +184,7 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_deletedContact() throws Exception {
     contact =
         persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
@@ -194,9 +195,9 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_nonexistentContact() throws Exception {
-    deleteResource(contact);
+    persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     ResourceDoesNotExistException thrown =
         assertThrows(
             ResourceDoesNotExistException.class, () -> doFailingTest("contact_transfer_query.xml"));
@@ -204,7 +205,7 @@ class ContactTransferQueryFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-transfer-query");

core/src/test/java/google/registry/flows/contact/ContactTransferRejectFlowTest.java

@@ -18,7 +18,6 @@ import static com.google.common.collect.MoreCollectors.onlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatabaseHelper.assertNoBillingEvents;
-import static google.registry.testing.DatabaseHelper.deleteResource;
 import static google.registry.testing.DatabaseHelper.getOnlyPollMessage;
 import static google.registry.testing.DatabaseHelper.getPollMessages;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -40,10 +39,12 @@ 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.DualDatabaseTest;
+import google.registry.testing.TestOfyAndSql;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactTransferRejectFlow}. */
+@DualDatabaseTest
 class ContactTransferRejectFlowTest
     extends ContactTransferFlowTestCase<ContactTransferRejectFlow, ContactResource> {
 
@@ -120,24 +121,24 @@ class ContactTransferRejectFlowTest
     runFlow();
   }
 
-  @Test
+  @TestOfyAndSql
   void testDryRun() throws Exception {
     setEppInput("contact_transfer_reject.xml");
     dryRunFlowAssertResponse(loadFile("contact_transfer_reject_response.xml"));
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess() throws Exception {
     doSuccessfulTest("contact_transfer_reject.xml", "contact_transfer_reject_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_domainAuthInfo() throws Exception {
     doSuccessfulTest("contact_transfer_reject_with_authinfo.xml",
         "contact_transfer_reject_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badPassword() {
     // Change the contact's password so it does not match the password in the file.
     contact =
@@ -153,7 +154,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_neverBeenTransferred() {
     changeTransferStatus(null);
     EppException thrown =
@@ -162,7 +163,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientApproved() {
     changeTransferStatus(TransferStatus.CLIENT_APPROVED);
     EppException thrown =
@@ -171,7 +172,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientRejected() {
     changeTransferStatus(TransferStatus.CLIENT_REJECTED);
     EppException thrown =
@@ -180,7 +181,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientCancelled() {
     changeTransferStatus(TransferStatus.CLIENT_CANCELLED);
     EppException thrown =
@@ -189,7 +190,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverApproved() {
     changeTransferStatus(TransferStatus.SERVER_APPROVED);
     EppException thrown =
@@ -198,7 +199,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverCancelled() {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     EppException thrown =
@@ -207,7 +208,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_gainingClient() {
     setClientIdForFlow("NewRegistrar");
     EppException thrown =
@@ -216,7 +217,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_unrelatedClient() {
     setClientIdForFlow("ClientZ");
     EppException thrown =
@@ -225,7 +226,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_deletedContact() throws Exception {
     contact =
         persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
@@ -237,9 +238,9 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_nonexistentContact() throws Exception {
-    deleteResource(contact);
+    persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
     ResourceDoesNotExistException thrown =
         assertThrows(
             ResourceDoesNotExistException.class,
@@ -248,7 +249,7 @@ class ContactTransferRejectFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-transfer-reject");

core/src/test/java/google/registry/flows/contact/ContactTransferRequestFlowTest.java

@@ -20,7 +20,8 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.collect.MoreCollectors.onlyElement;
 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.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatabaseHelper.assertNoBillingEvents;
 import static google.registry.testing.DatabaseHelper.assertPollMessagesEqual;
@@ -29,11 +30,11 @@ import static google.registry.testing.DatabaseHelper.getPollMessages;
 import static google.registry.testing.DatabaseHelper.persistActiveContact;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
+import static google.registry.util.CollectionUtils.forceEmptyToNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.googlecode.objectify.Key;
 import google.registry.flows.EppException;
 import google.registry.flows.ResourceFlowUtils.BadAuthInfoForResourceException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
@@ -50,12 +51,13 @@ import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.transfer.ContactTransferData;
 import google.registry.model.transfer.TransferStatus;
-import google.registry.persistence.VKey;
+import google.registry.testing.DualDatabaseTest;
+import google.registry.testing.TestOfyAndSql;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactTransferRequestFlow}. */
+@DualDatabaseTest
 class ContactTransferRequestFlowTest
     extends ContactTransferFlowTestCase<ContactTransferRequestFlow, ContactResource> {
 
@@ -102,7 +104,8 @@ class ContactTransferRequestFlowTest
                 .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())
+                .setServerApproveEntities(
+                    forceEmptyToNull(contact.getTransferData().getServerApproveEntities()))
                 .build());
     assertNoBillingEvents();
     assertThat(getPollMessages("TheRegistrar", clock.nowUtc())).hasSize(1);
@@ -126,13 +129,8 @@ class ContactTransferRequestFlowTest
     // poll messages, the approval notice ones for gaining and losing registrars.
     assertPollMessagesEqual(
         Iterables.filter(
-            ofy()
+            transactIfJpaTm(
-                .load()
+                    () -> tm().loadByKeys(contact.getTransferData().getServerApproveEntities()))
-                // Use toArray() to coerce the type to something keys() will accept.
-                .keys(
-                    contact.getTransferData().getServerApproveEntities().stream()
-                        .map(VKey::getOfyKey)
-                        .toArray(Key[]::new))
                 .values(),
             PollMessage.class),
         ImmutableList.of(gainingApproveMessage, losingApproveMessage));
@@ -145,18 +143,18 @@ class ContactTransferRequestFlowTest
     runFlow();
   }
 
-  @Test
+  @TestOfyAndSql
   void testDryRun() throws Exception {
     setEppInput("contact_transfer_request.xml");
     dryRunFlowAssertResponse(loadFile("contact_transfer_request_response.xml"));
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess() throws Exception {
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_noAuthInfo() {
     EppException thrown =
         assertThrows(
@@ -165,7 +163,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_badPassword() {
     // Change the contact's password so it does not match the password in the file.
     contact =
@@ -181,37 +179,37 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientApproved() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_APPROVED);
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientRejected() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_REJECTED);
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_clientCancelled() throws Exception {
     changeTransferStatus(TransferStatus.CLIENT_CANCELLED);
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_serverApproved() throws Exception {
     changeTransferStatus(TransferStatus.SERVER_APPROVED);
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testSuccess_serverCancelled() throws Exception {
     changeTransferStatus(TransferStatus.SERVER_CANCELLED);
     doSuccessfulTest("contact_transfer_request.xml", "contact_transfer_request_response.xml");
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_pending() {
     contact =
         persistResource(
@@ -232,7 +230,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_sponsoringClient() {
     setClientIdForFlow("TheRegistrar");
     EppException thrown =
@@ -242,7 +240,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_deletedContact() throws Exception {
     contact =
         persistResource(contact.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
@@ -254,7 +252,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_nonexistentContact() throws Exception {
     deleteResource(contact);
     ResourceDoesNotExistException thrown =
@@ -265,7 +263,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_clientTransferProhibited() {
     contact =
         persistResource(
@@ -278,7 +276,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_serverTransferProhibited() {
     contact =
         persistResource(
@@ -291,7 +289,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testFailure_pendingDelete() {
     contact =
         persistResource(contact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build());
@@ -303,7 +301,7 @@ class ContactTransferRequestFlowTest
     assertAboutEppExceptions().that(thrown).marshalsToXml();
   }
 
-  @Test
+  @TestOfyAndSql
   void testIcannActivityReportField_getsLogged() throws Exception {
     runFlow();
     assertIcannReportingActivityFieldLogged("srs-cont-transfer-request");

core/src/test/java/google/registry/model/transfer/TransferDataTest.java

@@ -54,28 +54,28 @@ public class TransferDataTest {
     transferBillingEventKey =
         VKey.create(
             BillingEvent.OneTime.class,
-            12345,
+            12345L,
-            Key.create(historyEntryKey, BillingEvent.OneTime.class, 12345));
+            Key.create(historyEntryKey, BillingEvent.OneTime.class, 12345L));
     otherServerApproveBillingEventKey =
         VKey.create(
             BillingEvent.Cancellation.class,
-            2468,
+            2468L,
-            Key.create(historyEntryKey, BillingEvent.Cancellation.class, 2468));
+            Key.create(historyEntryKey, BillingEvent.Cancellation.class, 2468L));
     recurringBillingEventKey =
         VKey.create(
             BillingEvent.Recurring.class,
-            13579,
+            13579L,
-            Key.create(historyEntryKey, BillingEvent.Recurring.class, 13579));
+            Key.create(historyEntryKey, BillingEvent.Recurring.class, 13579L));
     autorenewPollMessageKey =
         VKey.create(
             PollMessage.Autorenew.class,
-            67890,
+            67890L,
-            Key.create(historyEntryKey, PollMessage.Autorenew.class, 67890));
+            Key.create(historyEntryKey, PollMessage.Autorenew.class, 67890L));
     otherServerApprovePollMessageKey =
         VKey.create(
             PollMessage.OneTime.class,
-            314159,
+            314159L,
-            Key.create(historyEntryKey, PollMessage.OneTime.class, 314159));
+            Key.create(historyEntryKey, PollMessage.OneTime.class, 314159L));
   }
 
   @Test

core/src/test/java/google/registry/testing/DatabaseHelper.java

@@ -109,6 +109,7 @@ import google.registry.model.transfer.TransferStatus;
 import google.registry.persistence.VKey;
 import google.registry.tmch.LordnTaskUtils;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -116,7 +117,6 @@ import javax.annotation.Nullable;
 import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
-import org.joda.time.DateTimeComparator;
 import org.joda.time.DateTimeZone;
 
 /** Static utils for setting up test resources. */
@@ -532,11 +532,14 @@ public class DatabaseHelper {
       DateTime requestTime,
       DateTime expirationTime,
       DateTime now) {
-    HistoryEntry historyEntryContactTransfer = persistResource(
+    HistoryEntry historyEntryContactTransfer =
+        persistResource(
             new HistoryEntry.Builder()
                 .setType(HistoryEntry.Type.CONTACT_TRANSFER_REQUEST)
-            .setParent(contact)
+                .setParent(persistResource(contact))
-            .build());
+                .setModificationTime(now)
+                .build()
+                .toChildHistoryEntity());
     return persistResource(
         contact
             .asBuilder()
@@ -1115,7 +1118,8 @@ public class DatabaseHelper {
                               historyEntry ->
                                   historyEntry.getParent().getName().equals(resource.getRepoId()))
                           .collect(toImmutableList());
-                  return ImmutableList.sortedCopyOf(DateTimeComparator.getInstance(), filtered);
+                  return ImmutableList.sortedCopyOf(
+                      Comparator.comparing(HistoryEntry::getModificationTime), filtered);
                 });
   }
 

db/src/main/resources/sql/er_diagram/brief_er_diagram.html

@@ -261,11 +261,11 @@ td.section {
     </tr> 
     <tr> 
      <td class="property_name">generated on</td> 
-     <td class="property_value">2021-01-14 16:15:22.842637</td> 
+     <td class="property_value">2021-01-21 00:11:27.19594</td> 
     </tr> 
     <tr>
      <td class="property_name">last flyway file</td>
-     <td id="lastFlywayFile" class="property_value">V84__add_vkey_columns_in_billing_cancellation.sql</td>
+     <td id="lastFlywayFile" class="property_value">V85__add_required_columns_in_transfer_data.sql</td>
     </tr>
    </tbody>
   </table> 
@@ -274,19 +274,19 @@ td.section {
   <svg viewbox="0.00 0.00 4221.44 2624.18" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" id="erDiagram" style="overflow: hidden; width: 100%; height: 800px"> <g id="graph0" class="graph" transform="scale(1 1) rotate(0) translate(4 2620.18)"> 
     <title>SchemaCrawler_Diagram</title> 
     <polygon fill="white" stroke="transparent" points="-4,4 -4,-2620.18 4217.44,-2620.18 4217.44,4 -4,4" /> 
-    <text text-anchor="start" x="3944.94" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="3952.94" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      generated by
     </text> 
-    <text text-anchor="start" x="4027.94" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="4035.94" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      SchemaCrawler 16.10.1
     </text> 
-    <text text-anchor="start" x="3943.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="3951.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
      generated on
     </text> 
-    <text text-anchor="start" x="4027.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
+    <text text-anchor="start" x="4035.94" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">
-     2021-01-14 16:15:22.842637
+     2021-01-21 00:11:27.19594
     </text> 
-    <polygon fill="none" stroke="#888888" points="3940.44,-4 3940.44,-44 4205.44,-44 4205.44,-4 3940.44,-4" /> <!-- allocationtoken_a08ccbef --> 
+    <polygon fill="none" stroke="#888888" points="3948.44,-4 3948.44,-44 4205.44,-44 4205.44,-4 3948.44,-4" /> <!-- allocationtoken_a08ccbef --> 
     <g id="node1" class="node"> 
      <title>allocationtoken_a08ccbef</title> 
      <polygon fill="#ebcef2" stroke="transparent" points="2538.5,-1071.68 2538.5,-1090.68 2691.5,-1090.68 2691.5,-1071.68 2538.5,-1071.68" /> 

db/src/main/resources/sql/flyway.txt

@@ -82,3 +82,4 @@ V81__drop_spec11_fkeys.sql
 V82__add_columns_to_restore_symmetric_billing_vkey.sql
 V83__add_indexes_on_domainhost.sql
 V84__add_vkey_columns_in_billing_cancellation.sql
+V85__add_required_columns_in_transfer_data.sql

db/src/main/resources/sql/flyway/V85__add_required_columns_in_transfer_data.sql

@@ -0,0 +1,57 @@
+-- Copyright 2021 The Nomulus Authors. All Rights Reserved.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+alter table "Contact"
+    add column if not exists "transfer_history_entry_id" int8;
+alter table "Contact"
+    add column if not exists "transfer_repo_id" text;
+alter table "Contact"
+    rename column "transfer_gaining_poll_message_id"
+    to "transfer_poll_message_id_1";
+alter table "Contact"
+    rename column "transfer_losing_poll_message_id"
+    to "transfer_poll_message_id_2";
+
+alter table "ContactHistory"
+    add column if not exists "transfer_history_entry_id" int8;
+alter table "ContactHistory"
+    add column if not exists "transfer_repo_id" text;
+alter table "ContactHistory"
+    rename column "transfer_gaining_poll_message_id"
+    to "transfer_poll_message_id_1";
+alter table "ContactHistory"
+    rename column "transfer_losing_poll_message_id"
+    to "transfer_poll_message_id_2";
+
+alter table "Domain"
+    add column if not exists "transfer_history_entry_id" int8;
+alter table "Domain"
+    add column if not exists "transfer_repo_id" text;
+alter table "Domain"
+    rename column "transfer_gaining_poll_message_id"
+    to "transfer_poll_message_id_1";
+alter table "Domain"
+    rename column "transfer_losing_poll_message_id"
+    to "transfer_poll_message_id_2";
+
+alter table "DomainHistory"
+    add column if not exists "transfer_history_entry_id" int8;
+alter table "DomainHistory"
+    add column if not exists "transfer_repo_id" text;
+alter table "DomainHistory"
+    rename column "transfer_gaining_poll_message_id"
+    to "transfer_poll_message_id_1";
+alter table "DomainHistory"
+    rename column "transfer_losing_poll_message_id"
+    to "transfer_poll_message_id_2";

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -140,8 +140,10 @@
         addr_local_org text,
         addr_local_type text,
         search_name text,
-        transfer_gaining_poll_message_id int8,
+        transfer_history_entry_id int8,
-        transfer_losing_poll_message_id int8,
+        transfer_poll_message_id_1 int8,
+        transfer_poll_message_id_2 int8,
+        transfer_repo_id text,
         transfer_client_txn_id text,
         transfer_server_txn_id text,
         transfer_gaining_registrar_id text,
@@ -201,8 +203,10 @@
         addr_local_org text,
         addr_local_type text,
         search_name text,
-        transfer_gaining_poll_message_id int8,
+        transfer_history_entry_id int8,
-        transfer_losing_poll_message_id int8,
+        transfer_poll_message_id_1 int8,
+        transfer_poll_message_id_2 int8,
+        transfer_repo_id text,
         transfer_client_txn_id text,
         transfer_server_txn_id text,
         transfer_gaining_registrar_id text,
@@ -284,8 +288,10 @@
         transfer_renew_period_unit text,
         transfer_renew_period_value int4,
         transfer_registration_expiration_time timestamptz,
-        transfer_gaining_poll_message_id int8,
+        transfer_history_entry_id int8,
-        transfer_losing_poll_message_id int8,
+        transfer_poll_message_id_1 int8,
+        transfer_poll_message_id_2 int8,
+        transfer_repo_id text,
         transfer_client_txn_id text,
         transfer_server_txn_id text,
         transfer_gaining_registrar_id text,
@@ -353,8 +359,10 @@
         transfer_renew_period_unit text,
         transfer_renew_period_value int4,
         transfer_registration_expiration_time timestamptz,
-        transfer_gaining_poll_message_id int8,
+        transfer_history_entry_id int8,
-        transfer_losing_poll_message_id int8,
+        transfer_poll_message_id_1 int8,
+        transfer_poll_message_id_2 int8,
+        transfer_repo_id text,
         transfer_client_txn_id text,
         transfer_server_txn_id text,
         transfer_gaining_registrar_id text,

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -210,8 +210,8 @@ CREATE TABLE public."Contact" (
     search_name text,
     voice_phone_extension text,
     voice_phone_number text,
-    transfer_gaining_poll_message_id bigint,
+    transfer_poll_message_id_1 bigint,
-    transfer_losing_poll_message_id bigint,
+    transfer_poll_message_id_2 bigint,
     transfer_client_txn_id text,
     transfer_server_txn_id text,
     transfer_gaining_registrar_id text,
@@ -219,7 +219,9 @@ CREATE TABLE public."Contact" (
     transfer_pending_expiration_time timestamp with time zone,
     transfer_request_time timestamp with time zone,
     transfer_status text,
-    update_timestamp timestamp with time zone
+    update_timestamp timestamp with time zone,
+    transfer_history_entry_id bigint,
+    transfer_repo_id text
 );
 
 
@@ -273,8 +275,8 @@ CREATE TABLE public."ContactHistory" (
     addr_local_org text,
     addr_local_type text,
     search_name text,
-    transfer_gaining_poll_message_id bigint,
+    transfer_poll_message_id_1 bigint,
-    transfer_losing_poll_message_id bigint,
+    transfer_poll_message_id_2 bigint,
     transfer_client_txn_id text,
     transfer_server_txn_id text,
     transfer_gaining_registrar_id text,
@@ -292,7 +294,9 @@ CREATE TABLE public."ContactHistory" (
     last_epp_update_time timestamp with time zone,
     statuses text[],
     contact_repo_id text NOT NULL,
-    update_timestamp timestamp with time zone
+    update_timestamp timestamp with time zone,
+    transfer_history_entry_id bigint,
+    transfer_repo_id text
 );
 
 
@@ -351,8 +355,8 @@ CREATE TABLE public."Domain" (
     billing_contact text,
     registrant_contact text,
     tech_contact text,
-    transfer_gaining_poll_message_id bigint,
+    transfer_poll_message_id_1 bigint,
-    transfer_losing_poll_message_id bigint,
+    transfer_poll_message_id_2 bigint,
     transfer_billing_cancellation_id bigint,
     transfer_billing_event_id bigint,
     transfer_billing_recurrence_id bigint,
@@ -377,7 +381,9 @@ CREATE TABLE public."Domain" (
     deletion_poll_message_history_id bigint,
     transfer_billing_recurrence_history_id bigint,
     transfer_autorenew_poll_message_history_id bigint,
-    transfer_billing_event_history_id bigint
+    transfer_billing_event_history_id bigint,
+    transfer_history_entry_id bigint,
+    transfer_repo_id text
 );
 
 
@@ -438,8 +444,8 @@ CREATE TABLE public."DomainHistory" (
     transfer_renew_period_unit text,
     transfer_renew_period_value integer,
     transfer_registration_expiration_time timestamp with time zone,
-    transfer_gaining_poll_message_id bigint,
+    transfer_poll_message_id_1 bigint,
-    transfer_losing_poll_message_id bigint,
+    transfer_poll_message_id_2 bigint,
     transfer_client_txn_id text,
     transfer_server_txn_id text,
     transfer_gaining_registrar_id text,
@@ -465,7 +471,9 @@ CREATE TABLE public."DomainHistory" (
     deletion_poll_message_history_id bigint,
     transfer_billing_recurrence_history_id bigint,
     transfer_autorenew_poll_message_history_id bigint,
-    transfer_billing_event_history_id bigint
+    transfer_billing_event_history_id bigint,
+    transfer_history_entry_id bigint,
+    transfer_repo_id text
 );