diff --git a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java index 89e0e17c7..b5cd691cd 100644 --- a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java +++ b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java @@ -134,7 +134,7 @@ public class DatastoreTransactionManager implements TransactionManager { } @Override - public void insertWithoutBackup(Object entity) { + public void insertWithoutBackup(ImmutableObject entity) { putWithoutBackup(entity); } @@ -160,7 +160,7 @@ public class DatastoreTransactionManager implements TransactionManager { } @Override - public void putWithoutBackup(Object entity) { + public void putWithoutBackup(ImmutableObject entity) { syncIfTransactionless(getOfy().saveWithoutBackup().entities(toDatastoreEntity(entity))); } @@ -180,12 +180,12 @@ public class DatastoreTransactionManager implements TransactionManager { } @Override - public void updateAll(Object... entities) { - updateAll(ImmutableList.of(entities)); + public void updateAll(ImmutableObject... entities) { + updateAll(ImmutableList.copyOf(entities)); } @Override - public void updateWithoutBackup(Object entity) { + public void updateWithoutBackup(ImmutableObject entity) { putWithoutBackup(entity); } diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index 145e3f524..f156f6d7d 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -318,7 +318,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } @Override - public void insertWithoutBackup(Object entity) { + public void insertWithoutBackup(ImmutableObject entity) { insert(entity); } @@ -356,7 +356,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } @Override - public void putWithoutBackup(Object entity) { + public void putWithoutBackup(ImmutableObject entity) { put(entity); } @@ -386,12 +386,12 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } @Override - public void updateAll(Object... entities) { - updateAll(ImmutableList.of(entities)); + public void updateAll(ImmutableObject... entities) { + updateAll(ImmutableList.copyOf(entities)); } @Override - public void updateWithoutBackup(Object entity) { + public void updateWithoutBackup(ImmutableObject entity) { update(entity); } diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java index 9f5b0c98a..94cee5920 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java @@ -111,7 +111,7 @@ public interface TransactionManager { * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in * SQL. */ - void insertWithoutBackup(Object entity); + void insertWithoutBackup(ImmutableObject entity); /** * Persists all new entities in the database without writing any backup if the underlying database @@ -144,7 +144,7 @@ public interface TransactionManager { * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in * SQL. */ - void putWithoutBackup(Object entity); + void putWithoutBackup(ImmutableObject entity); /** * Persists all new entities or update the existing entities in the database without writing @@ -165,7 +165,7 @@ public interface TransactionManager { void updateAll(ImmutableCollection entities); /** Updates all entities in the database, throws exception if any entity does not exist. */ - void updateAll(Object... entities); + void updateAll(ImmutableObject... entities); /** * Updates an entity in the database without writing commit logs if the underlying database is @@ -177,7 +177,7 @@ public interface TransactionManager { * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in * SQL. */ - void updateWithoutBackup(Object entity); + void updateWithoutBackup(ImmutableObject entity); /** * Updates all entities in the database without writing any backup if the underlying database is diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java index e957cf147..8799353dc 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java @@ -132,7 +132,9 @@ public class TransactionManagerFactory { /** * Sets the return of {@link #tm()} to the given instance of {@link TransactionManager}. * - *

Used when overriding the per-test transaction manager for dual-database tests. + *

Used when overriding the per-test transaction manager for dual-database tests. Should be + * matched with a corresponding invocation of {@link #removeTmOverrideForTest()} either at the end + * of the test or in an @AfterEach handler. */ @VisibleForTesting public static void setTmForTest(TransactionManager newTm) { diff --git a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java index c8ecfd222..28544ea80 100644 --- a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java +++ b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java @@ -27,6 +27,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.insertInDb; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.util.DateTimeUtils.START_OF_TIME; @@ -163,12 +164,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_multipleDiffFiles() throws Exception { - jpaTm() - .transact( - () -> { - jpaTm().insertWithoutBackup(TestObject.create("previous to keep")); - jpaTm().insertWithoutBackup(TestObject.create("previous to delete")); - }); + insertInDb(TestObject.create("previous to keep"), TestObject.create("previous to delete")); DateTime now = fakeClock.nowUtc(); // Create 3 transactions, across two diff files. // Before: {"previous to keep", "previous to delete"} @@ -216,7 +212,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_noManifests() throws Exception { DateTime now = fakeClock.nowUtc(); - jpaTm().transact(() -> jpaTm().insertWithoutBackup(TestObject.create("previous to keep"))); + insertInDb(TestObject.create("previous to keep")); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(1)); saveDiffFile(gcsUtils, createCheckpoint(now.minusMillis(2))); jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMillis(1))); @@ -228,7 +224,7 @@ public class ReplayCommitLogsToSqlActionTest { void testReplay_dryRun() throws Exception { action.dryRun = true; DateTime now = fakeClock.nowUtc(); - jpaTm().transact(() -> jpaTm().insertWithoutBackup(TestObject.create("previous to keep"))); + insertInDb(TestObject.create("previous to keep")); Key bucketKey = getBucketKey(1); Key manifestKey = CommitLogManifest.createKey(bucketKey, now); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(2)); @@ -253,7 +249,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_manifestWithNoDeletions() throws Exception { DateTime now = fakeClock.nowUtc(); - jpaTm().transact(() -> jpaTm().insertWithoutBackup(TestObject.create("previous to keep"))); + insertInDb(TestObject.create("previous to keep")); Key bucketKey = getBucketKey(1); Key manifestKey = CommitLogManifest.createKey(bucketKey, now); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(2)); @@ -271,12 +267,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_manifestWithNoMutations() throws Exception { DateTime now = fakeClock.nowUtc(); - jpaTm() - .transact( - () -> { - jpaTm().insertWithoutBackup(TestObject.create("previous to keep")); - jpaTm().insertWithoutBackup(TestObject.create("previous to delete")); - }); + insertInDb(TestObject.create("previous to keep"), TestObject.create("previous to delete")); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(2)); jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMinutes(1).minusMillis(1))); saveDiffFile( @@ -293,7 +284,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_mutateExistingEntity() throws Exception { DateTime now = fakeClock.nowUtc(); - jpaTm().transact(() -> jpaTm().put(TestObject.create("existing", "a"))); + insertInDb(TestObject.create("existing", "a")); Key manifestKey = CommitLogManifest.createKey(getBucketKey(1), now); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(1).minusMillis(1)); jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMinutes(1))); @@ -312,7 +303,7 @@ public class ReplayCommitLogsToSqlActionTest { @Test void testReplay_deleteMissingEntity() throws Exception { DateTime now = fakeClock.nowUtc(); - jpaTm().transact(() -> jpaTm().put(TestObject.create("previous to keep", "a"))); + insertInDb(TestObject.create("previous to keep", "a")); saveDiffFileNotToRestore(gcsUtils, now.minusMinutes(1).minusMillis(1)); jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMinutes(1))); saveDiffFile( @@ -368,7 +359,7 @@ public class ReplayCommitLogsToSqlActionTest { // Create and save to SQL a registrar contact that we will delete RegistrarContact toDelete = AppEngineExtension.makeRegistrarContact1(); - jpaTm().transact(() -> jpaTm().put(toDelete)); + insertInDb(toDelete); jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMinutes(1).minusMillis(1))); // spy the txn manager so we can see what order things were inserted/removed @@ -569,8 +560,7 @@ public class ReplayCommitLogsToSqlActionTest { .asBuilder() .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}))) .build(); - jpaTm().transact(() -> jpaTm().put(domain)); - + insertInDb(domain); assertThat(jpaTm().transact(() -> jpaTm().loadAllOf(DelegationSignerData.class))).isNotEmpty(); saveDiffFile( @@ -580,12 +570,8 @@ public class ReplayCommitLogsToSqlActionTest { getBucketKey(1), now.minusMinutes(3), ImmutableSet.of(Key.create(domain)))); runAndAssertSuccess(now.minusMinutes(1), 1, 1); - jpaTm() - .transact( - () -> { - assertThat(jpaTm().loadAllOf(DomainBase.class)).isEmpty(); - assertThat(jpaTm().loadAllOf(DelegationSignerData.class)).isEmpty(); - }); + assertThat(jpaTm().transact(() -> jpaTm().loadAllOf(DomainBase.class))).isEmpty(); + assertThat(jpaTm().transact(() -> jpaTm().loadAllOf(DelegationSignerData.class))).isEmpty(); } @Test @@ -627,7 +613,7 @@ public class ReplayCommitLogsToSqlActionTest { ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {4, 5, 6})); DomainBase domainWithDsData = newDomainBase("example.tld").asBuilder().setDsData(dsData).build(); - jpaTm().transact(() -> jpaTm().put(domainWithDsData)); + insertInDb(domainWithDsData); // Replay a version of that domain without the dsData Key manifestKeyOne = @@ -639,7 +625,7 @@ public class ReplayCommitLogsToSqlActionTest { // Create an object (any object) to delete via replay to trigger Hibernate flush events TestObject testObject = TestObject.create("foo", "bar"); - jpaTm().transact(() -> jpaTm().put(testObject)); + insertInDb(testObject); // Replay the original domain, with the original dsData Key manifestKeyTwo = diff --git a/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java b/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java index 2041079c8..eb4a8e1f4 100644 --- a/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java +++ b/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java @@ -14,7 +14,6 @@ package google.registry.beam.common; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.AppEngineExtension.makeRegistrar1; import static google.registry.testing.DatabaseHelper.insertInDb; import static google.registry.testing.DatabaseHelper.newRegistry; @@ -78,7 +77,7 @@ public class RegistryJpaReadTest { @BeforeEach void beforeEach() { Registrar ofyRegistrar = AppEngineExtension.makeRegistrar2(); - jpaTm().transact(() -> jpaTm().put(ofyRegistrar)); + insertInDb(ofyRegistrar); ImmutableList.Builder builder = new ImmutableList.Builder<>(); @@ -87,7 +86,7 @@ public class RegistryJpaReadTest { builder.add(contact); } contacts = builder.build(); - jpaTm().transact(() -> jpaTm().putAll(contacts)); + insertInDb(contacts); } @Test diff --git a/core/src/test/java/google/registry/beam/common/RegistryJpaWriteTest.java b/core/src/test/java/google/registry/beam/common/RegistryJpaWriteTest.java index a9afc5ed8..499fa2f71 100644 --- a/core/src/test/java/google/registry/beam/common/RegistryJpaWriteTest.java +++ b/core/src/test/java/google/registry/beam/common/RegistryJpaWriteTest.java @@ -17,6 +17,8 @@ package google.registry.beam.common; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.testing.DatabaseHelper.newContactResource; +import static google.registry.testing.DatabaseHelper.putInDb; import com.google.appengine.api.datastore.Entity; import com.google.common.collect.ImmutableList; @@ -32,7 +34,6 @@ import google.registry.model.registrar.Registrar; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.AppEngineExtension; -import google.registry.testing.DatabaseHelper; import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; @@ -76,12 +77,12 @@ class RegistryJpaWriteTest implements Serializable { // Required for contacts created below. Registrar ofyRegistrar = AppEngineExtension.makeRegistrar2(); store.insertOrUpdate(ofyRegistrar); - jpaTm().transact(() -> jpaTm().put(store.loadAsOfyEntity(ofyRegistrar))); + putInDb(store.loadAsOfyEntity(ofyRegistrar)); ImmutableList.Builder builder = new ImmutableList.Builder<>(); for (int i = 0; i < 3; i++) { - ContactResource contact = DatabaseHelper.newContactResource("contact_" + i); + ContactResource contact = newContactResource("contact_" + i); store.insertOrUpdate(contact); builder.add(store.loadAsDatastoreEntity(contact)); } @@ -106,8 +107,7 @@ class RegistryJpaWriteTest implements Serializable { .withJpaConverter(Transforms::convertVersionedEntityToSqlEntity)); testPipeline.run().waitUntilFinish(); - ImmutableList sqlContacts = jpaTm().transact(() -> jpaTm().loadAllOf(ContactResource.class)); - assertThat(sqlContacts) + assertThat(jpaTm().transact(() -> jpaTm().loadAllOf(ContactResource.class))) .comparingElementsUsing(immutableObjectCorrespondence("revisions", "updateTimestamp")) .containsExactlyElementsIn( contacts.stream() diff --git a/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java b/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java index 8aaa67184..313c8f461 100644 --- a/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java +++ b/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java @@ -25,6 +25,7 @@ import com.google.appengine.api.datastore.EntityNotFoundException; import com.googlecode.objectify.Key; import google.registry.backup.CommitLogExports; import google.registry.backup.VersionedEntity; +import google.registry.model.ImmutableObject; import google.registry.model.ofy.CommitLogCheckpoint; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeClock; @@ -124,7 +125,7 @@ public final class BackupTestStore implements AutoCloseable { * *

See {@link #loadAsDatastoreEntity} and {@link VersionedEntity} for more information. */ - public Object loadAsOfyEntity(Object ofyEntity) { + public ImmutableObject loadAsOfyEntity(ImmutableObject ofyEntity) { try { return auditedOfy().load().fromEntity(datastoreService.get(Key.create(ofyEntity).getRaw())); } catch (EntityNotFoundException e) { diff --git a/core/src/test/java/google/registry/model/EntityTestCase.java b/core/src/test/java/google/registry/model/EntityTestCase.java index 74b110542..bf0e8c34b 100644 --- a/core/src/test/java/google/registry/model/EntityTestCase.java +++ b/core/src/test/java/google/registry/model/EntityTestCase.java @@ -189,8 +189,8 @@ public abstract class EntityTestCase { return fields; } - /** Verify indexing for an entity. */ - public void verifyIndexing(Object obj, String... indexed) throws Exception { + /** Verify Datastore indexing for an entity. */ + public void verifyDatastoreIndexing(Object obj, String... indexed) throws Exception { Set indexedSet = ImmutableSet.copyOf(indexed); Set allSet = getAllPotentiallyIndexedFieldPaths(obj.getClass()); // Sanity test that the indexed fields we listed were found. diff --git a/core/src/test/java/google/registry/model/billing/BillingEventTest.java b/core/src/test/java/google/registry/model/billing/BillingEventTest.java index 610224ffe..6337207d2 100644 --- a/core/src/test/java/google/registry/model/billing/BillingEventTest.java +++ b/core/src/test/java/google/registry/model/billing/BillingEventTest.java @@ -236,24 +236,24 @@ public class BillingEventTest extends EntityTestCase { @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing( + verifyDatastoreIndexing( oneTime, "clientId", "eventTime", "billingTime", "syntheticCreationTime", "allocationToken"); - verifyIndexing( + verifyDatastoreIndexing( oneTimeSynthetic, "clientId", "eventTime", "billingTime", "syntheticCreationTime", "allocationToken"); - verifyIndexing( + verifyDatastoreIndexing( recurring, "clientId", "eventTime", "recurrenceEndTime", "recurrenceTimeOfYear.timeString"); - verifyIndexing(cancellationOneTime, "clientId", "eventTime", "billingTime"); - verifyIndexing(modification, "clientId", "eventTime"); + verifyDatastoreIndexing(cancellationOneTime, "clientId", "eventTime", "billingTime"); + verifyDatastoreIndexing(modification, "clientId", "eventTime"); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/model/common/CursorTest.java b/core/src/test/java/google/registry/model/common/CursorTest.java index f4f6c2109..40f38472f 100644 --- a/core/src/test/java/google/registry/model/common/CursorTest.java +++ b/core/src/test/java/google/registry/model/common/CursorTest.java @@ -75,7 +75,7 @@ public class CursorTest extends EntityTestCase { final DateTime time = DateTime.parse("2012-07-12T03:30:00.000Z"); tm().transact(() -> tm().put(Cursor.createGlobal(RECURRING_BILLING, time))); Cursor cursor = tm().transact(() -> tm().loadByKey(Cursor.createGlobalVKey(RECURRING_BILLING))); - verifyIndexing(cursor); + verifyDatastoreIndexing(cursor); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java index fb5b12b72..ad6e59c18 100644 --- a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java +++ b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java @@ -17,15 +17,15 @@ package google.registry.model.contact; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; import static google.registry.testing.ContactResourceSubject.assertAboutContacts; import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation; -import static google.registry.testing.SqlHelper.saveRegistrar; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -33,7 +33,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import google.registry.model.EntityTestCase; -import google.registry.model.ImmutableObjectSubject; import google.registry.model.contact.Disclose.PostalInfoChoice; import google.registry.model.contact.PostalInfo.Type; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; @@ -45,11 +44,14 @@ import google.registry.model.index.ForeignKeyIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyContactIndex; 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 google.registry.testing.TestOfyOnly; +import google.registry.testing.TestSqlOnly; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link ContactResource}. */ +@DualDatabaseTest public class ContactResourceTest extends EntityTestCase { private ContactResource originalContact; @@ -66,11 +68,11 @@ public class ContactResourceTest extends EntityTestCase { new ContactResource.Builder() .setContactId("contact_id") .setRepoId("1-FOOBAR") - .setCreationRegistrarId("registrar1") + .setCreationRegistrarId("TheRegistrar") .setLastEppUpdateTime(fakeClock.nowUtc()) - .setLastEppUpdateRegistrarId("registrar2") + .setLastEppUpdateRegistrarId("NewRegistrar") .setLastTransferTime(fakeClock.nowUtc()) - .setPersistedCurrentSponsorRegistrarId("registrar3") + .setPersistedCurrentSponsorRegistrarId("NewRegistrar") .setLocalizedPostalInfo( new PostalInfo.Builder() .setType(Type.LOCALIZED) @@ -116,8 +118,8 @@ public class ContactResourceTest extends EntityTestCase { .setStatusValues(ImmutableSet.of(StatusValue.OK)) .setTransferData( new ContactTransferData.Builder() - .setGainingRegistrarId("gaining") - .setLosingRegistrarId("losing") + .setGainingRegistrarId("TheRegistrar") + .setLosingRegistrarId("NewRegistrar") .setPendingTransferExpirationTime(fakeClock.nowUtc()) .setTransferRequestTime(fakeClock.nowUtc()) .setTransferStatus(TransferStatus.SERVER_APPROVED) @@ -128,34 +130,28 @@ public class ContactResourceTest extends EntityTestCase { contactResource = persistResource(cloneAndSetAutoTimestamps(originalContact)); } - @Test + @TestOfyAndSql void testContactBaseToContactResource() { - ImmutableObjectSubject.assertAboutImmutableObjects() + assertAboutImmutableObjects() .that(new ContactResource.Builder().copyFrom(contactResource).build()) .isEqualExceptFields(contactResource, "updateTimestamp", "revisions"); } - @Test + @TestSqlOnly void testCloudSqlPersistence_failWhenViolateForeignKeyConstraint() { - assertThrowForeignKeyViolation(() -> insertInDb(originalContact)); + assertThrowForeignKeyViolation( + () -> + insertInDb( + originalContact + .asBuilder() + .setRepoId("2-FOOBAR") + .setCreationRegistrarId("nonexistent-registrar") + .build())); } - @Test + @TestSqlOnly void testCloudSqlPersistence_succeed() { - saveRegistrar("registrar1"); - saveRegistrar("registrar2"); - saveRegistrar("registrar3"); - saveRegistrar("gaining"); - saveRegistrar("losing"); - insertInDb(originalContact); - ContactResource persisted = - jpaTm() - .transact( - () -> - jpaTm() - .loadByKey( - VKey.createSql(ContactResource.class, originalContact.getRepoId()))); - + ContactResource persisted = loadByEntity(originalContact); ContactResource fixed = originalContact .asBuilder() @@ -167,12 +163,10 @@ public class ContactResourceTest extends EntityTestCase { .setServerApproveEntities(null) .build()) .build(); - ImmutableObjectSubject.assertAboutImmutableObjects() - .that(persisted) - .isEqualExceptFields(fixed, "updateTimestamp"); + assertAboutImmutableObjects().that(persisted).isEqualExceptFields(fixed, "updateTimestamp"); } - @Test + @TestOfyAndSql void testPersistence() { assertThat( loadByForeignKey( @@ -180,12 +174,13 @@ public class ContactResourceTest extends EntityTestCase { .hasValue(contactResource); } - @Test + @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing(contactResource, "deletionTime", "currentSponsorClientId", "searchName"); + verifyDatastoreIndexing( + contactResource, "deletionTime", "currentSponsorClientId", "searchName"); } - @Test + @TestOfyAndSql void testEmptyStringsBecomeNull() { assertThat(new ContactResource.Builder().setContactId(null).build().getContactId()).isNull(); assertThat(new ContactResource.Builder().setContactId("").build().getContactId()).isNull(); @@ -217,7 +212,7 @@ public class ContactResourceTest extends EntityTestCase { .isNotNull(); } - @Test + @TestOfyAndSql void testEmptyTransferDataBecomesNull() { ContactResource withNull = new ContactResource.Builder().setTransferData(null).build(); ContactResource withEmpty = @@ -226,7 +221,7 @@ public class ContactResourceTest extends EntityTestCase { assertThat(withEmpty.transferData).isNull(); } - @Test + @TestOfyAndSql void testImplicitStatusValues() { // OK is implicit if there's no other statuses. assertAboutContacts() @@ -248,7 +243,7 @@ public class ContactResourceTest extends EntityTestCase { .hasExactlyStatusValues(StatusValue.CLIENT_HOLD); } - @Test + @TestOfyAndSql void testExpiredTransfer() { ContactResource afterTransfer = contactResource @@ -269,7 +264,7 @@ public class ContactResourceTest extends EntityTestCase { assertThat(afterTransfer.getLastTransferTime()).isEqualTo(fakeClock.nowUtc().plusDays(1)); } - @Test + @TestOfyAndSql void testSetCreationTime_cantBeCalledTwice() { IllegalStateException thrown = assertThrows( @@ -278,13 +273,13 @@ public class ContactResourceTest extends EntityTestCase { assertThat(thrown).hasMessageThat().contains("creationTime can only be set once"); } - @Test + @TestOfyAndSql void testToHydratedString_notCircular() { // If there are circular references, this will overflow the stack. contactResource.toHydratedString(); } - @Test + @TestOfyAndSql void testBeforeDatastoreSaveOnReplay_indexes() { ImmutableList foreignKeyIndexes = ofyTm().loadAllOf(ForeignKeyContactIndex.class); diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index f38eb6b50..f5cb14e7b 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -19,6 +19,10 @@ import static google.registry.flows.domain.DomainTransferUtils.createPendingTran import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; +import static google.registry.testing.DatabaseHelper.loadByKey; +import static google.registry.testing.DatabaseHelper.updateInDb; import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation; import static google.registry.testing.SqlHelper.saveRegistrar; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -139,40 +143,19 @@ public class DomainBaseSqlTest { @TestSqlOnly void testDomainBasePersistence() { persistDomain(); - - jpaTm() - .transact( - () -> { - DomainBase result = jpaTm().loadByKey(domain.createVKey()); - assertEqualDomainExcept(result); - }); + assertEqualDomainExcept(loadByKey(domain.createVKey())); } @TestSqlOnly void testHostForeignKeyConstraints() { - assertThrowForeignKeyViolation( - () -> - jpaTm() - .transact( - () -> { - // Persist the domain without the associated host object. - jpaTm().insert(contact); - jpaTm().insert(contact2); - jpaTm().insert(domain); - })); + // Persist the domain without the associated host object. + assertThrowForeignKeyViolation(() -> insertInDb(contact, contact2, domain)); } @TestSqlOnly void testContactForeignKeyConstraints() { - assertThrowForeignKeyViolation( - () -> - jpaTm() - .transact( - () -> { - // Persist the domain without the associated contact objects. - jpaTm().insert(domain); - jpaTm().insert(host); - })); + // Persist the domain without the associated contact objects. + assertThrowForeignKeyViolation(() -> insertInDb(domain, host)); } @TestSqlOnly @@ -184,13 +167,8 @@ public class DomainBaseSqlTest { DomainBase persisted = jpaTm().loadByKey(domain.createVKey()); jpaTm().put(persisted.asBuilder().build()); }); - jpaTm() - .transact( - () -> { - // Load the domain in its entirety. - DomainBase result = jpaTm().loadByKey(domain.createVKey()); - assertEqualDomainExcept(result); - }); + // Load the domain in its entirety. + assertEqualDomainExcept(loadByKey(domain.createVKey())); } @TestSqlOnly @@ -379,24 +357,12 @@ public class DomainBaseSqlTest { @TestSqlOnly void testUpdates() { createTld("com"); - jpaTm() - .transact( - () -> { - jpaTm().insert(contact); - jpaTm().insert(contact2); - jpaTm().insert(domain); - jpaTm().insert(host); - }); + insertInDb(contact, contact2, domain, host); domain = domain.asBuilder().setNameservers(ImmutableSet.of()).build(); - jpaTm().transact(() -> jpaTm().put(domain)); - jpaTm() - .transact( - () -> { - DomainBase result = jpaTm().loadByKey(domain.createVKey()); - assertAboutImmutableObjects() - .that(result) - .isEqualExceptFields(domain, "updateTimestamp", "creationTime"); - }); + updateInDb(domain); + assertAboutImmutableObjects() + .that(loadByEntity(domain)) + .isEqualExceptFields(domain, "updateTimestamp", "creationTime"); } static ContactResource makeContact(String repoId) { @@ -410,128 +376,109 @@ public class DomainBaseSqlTest { private void persistDomain() { createTld("com"); - jpaTm() - .transact( - () -> { - // Persist the contacts. Note that these need to be persisted before the domain - // otherwise we get a foreign key constraint error. If we ever decide to defer the - // relevant foreign key checks to commit time, then the order would not matter. - jpaTm().insert(contact); - jpaTm().insert(contact2); - - // Persist the domain. - jpaTm().insert(domain); - - // Persist the host. This does _not_ need to be persisted before the domain, - // because only the row in the join table (DomainHost) is subject to foreign key - // constraints, and Hibernate knows to insert it after domain and host. - jpaTm().insert(host); - }); + insertInDb(contact, contact2, domain, host); } @TestSqlOnly void persistDomainWithCompositeVKeys() { createTld("com"); - jpaTm() - .transact( - () -> { - historyEntry = - new DomainHistory.Builder() - .setId(100L) - .setType(HistoryEntry.Type.DOMAIN_CREATE) - .setPeriod(Period.create(1, Period.Unit.YEARS)) - .setModificationTime(DateTime.now(UTC)) - .setDomainRepoId("4-COM") - .setRegistrarId("registrar1") - // These are non-null, but I don't think some tests set them. - .setReason("felt like it") - .setRequestedByRegistrar(false) - .setXmlBytes(new byte[0]) - .build(); - BillingEvent.Recurring billEvent = - new BillingEvent.Recurring.Builder() - .setId(200L) - .setReason(Reason.RENEW) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setTargetId("example.com") - .setRegistrarId("registrar1") - .setDomainRepoId("4-COM") - .setDomainHistoryRevisionId(1L) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setParent(historyEntry) - .build(); - PollMessage.Autorenew autorenewPollMessage = - new PollMessage.Autorenew.Builder() - .setId(300L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - PollMessage.OneTime deletePollMessage = - new PollMessage.OneTime.Builder() - .setId(400L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - BillingEvent.OneTime oneTimeBillingEvent = - new BillingEvent.OneTime.Builder() - .setId(500L) - // Use SERVER_STATUS so we don't have to add a period. - .setReason(Reason.SERVER_STATUS) - .setTargetId("example.com") - .setRegistrarId("registrar1") - .setDomainRepoId("4-COM") - .setBillingTime(DateTime.now(UTC)) - .setCost(Money.of(USD, 100)) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - DomainTransferData transferData = - new DomainTransferData.Builder() - .setServerApproveBillingEvent(oneTimeBillingEvent.createVKey()) - .setServerApproveAutorenewEvent(billEvent.createVKey()) - .setServerApproveAutorenewPollMessage(autorenewPollMessage.createVKey()) - .build(); - gracePeriods = - ImmutableSet.of( - GracePeriod.create( - GracePeriodStatus.ADD, - "4-COM", - END_OF_TIME, - "registrar1", - oneTimeBillingEvent.createVKey()), - GracePeriod.createForRecurring( - GracePeriodStatus.AUTO_RENEW, - "4-COM", - END_OF_TIME, - "registrar1", - billEvent.createVKey())); + historyEntry = + new DomainHistory.Builder() + .setId(100L) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setPeriod(Period.create(1, Period.Unit.YEARS)) + .setModificationTime(DateTime.now(UTC)) + .setDomainRepoId("4-COM") + .setRegistrarId("registrar1") + // These are non-null, but I don't think some tests set them. + .setReason("felt like it") + .setRequestedByRegistrar(false) + .setXmlBytes(new byte[0]) + .build(); + BillingEvent.Recurring billEvent = + new BillingEvent.Recurring.Builder() + .setId(200L) + .setReason(Reason.RENEW) + .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) + .setTargetId("example.com") + .setRegistrarId("registrar1") + .setDomainRepoId("4-COM") + .setDomainHistoryRevisionId(1L) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setRecurrenceEndTime(END_OF_TIME) + .setParent(historyEntry) + .build(); + PollMessage.Autorenew autorenewPollMessage = + new PollMessage.Autorenew.Builder() + .setId(300L) + .setRegistrarId("registrar1") + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + PollMessage.OneTime deletePollMessage = + new PollMessage.OneTime.Builder() + .setId(400L) + .setRegistrarId("registrar1") + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + BillingEvent.OneTime oneTimeBillingEvent = + new BillingEvent.OneTime.Builder() + .setId(500L) + // Use SERVER_STATUS so we don't have to add a period. + .setReason(Reason.SERVER_STATUS) + .setTargetId("example.com") + .setRegistrarId("registrar1") + .setDomainRepoId("4-COM") + .setBillingTime(DateTime.now(UTC)) + .setCost(Money.of(USD, 100)) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + DomainTransferData transferData = + new DomainTransferData.Builder() + .setServerApproveBillingEvent(oneTimeBillingEvent.createVKey()) + .setServerApproveAutorenewEvent(billEvent.createVKey()) + .setServerApproveAutorenewPollMessage(autorenewPollMessage.createVKey()) + .build(); + gracePeriods = + ImmutableSet.of( + GracePeriod.create( + GracePeriodStatus.ADD, + "4-COM", + END_OF_TIME, + "registrar1", + oneTimeBillingEvent.createVKey()), + GracePeriod.createForRecurring( + GracePeriodStatus.AUTO_RENEW, + "4-COM", + END_OF_TIME, + "registrar1", + billEvent.createVKey())); - jpaTm().insert(contact); - jpaTm().insert(contact2); - jpaTm().insert(host); - domain = - domain - .asBuilder() - .setAutorenewBillingEvent(billEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) - .setDeletePollMessage(deletePollMessage.createVKey()) - .setTransferData(transferData) - .setGracePeriods(gracePeriods) - .build(); - historyEntry = historyEntry.asBuilder().setDomain(domain).build(); - jpaTm().insert(historyEntry); - jpaTm().insert(autorenewPollMessage); - jpaTm().insert(billEvent); - jpaTm().insert(deletePollMessage); - jpaTm().insert(oneTimeBillingEvent); - jpaTm().insert(domain); - }); + domain = + domain + .asBuilder() + .setAutorenewBillingEvent(billEvent.createVKey()) + .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setDeletePollMessage(deletePollMessage.createVKey()) + .setTransferData(transferData) + .setGracePeriods(gracePeriods) + .build(); + historyEntry = historyEntry.asBuilder().setDomain(domain).build(); + insertInDb( + contact, + contact2, + host, + historyEntry, + autorenewPollMessage, + billEvent, + deletePollMessage, + oneTimeBillingEvent, + domain); // Store the existing BillingRecurrence VKey. This happens after the event has been persisted. - DomainBase persisted = jpaTm().transact(() -> jpaTm().loadByKey(domain.createVKey())); + DomainBase persisted = loadByKey(domain.createVKey()); // Verify that the domain data has been persisted. // dsData still isn't persisted. gracePeriods appears to have the same values but for some @@ -539,8 +486,7 @@ public class DomainBaseSqlTest { assertEqualDomainExcept(persisted, "creationTime", "dsData", "gracePeriods"); // Verify that the DomainContent object from the history record sets the fields correctly. - DomainHistory persistedHistoryEntry = - jpaTm().transact(() -> jpaTm().loadByKey(historyEntry.createVKey())); + DomainHistory persistedHistoryEntry = loadByKey(historyEntry.createVKey()); assertThat(persistedHistoryEntry.getDomainContent().get().getAutorenewPollMessage()) .isEqualTo(domain.getAutorenewPollMessage()); assertThat(persistedHistoryEntry.getDomainContent().get().getAutorenewBillingEvent()) @@ -562,114 +508,110 @@ public class DomainBaseSqlTest { @TestSqlOnly void persistDomainWithLegacyVKeys() { createTld("com"); - jpaTm() - .transact( - () -> { - historyEntry = - new DomainHistory.Builder() - .setId(100L) - .setType(HistoryEntry.Type.DOMAIN_CREATE) - .setPeriod(Period.create(1, Period.Unit.YEARS)) - .setModificationTime(DateTime.now(UTC)) - .setDomainRepoId("4-COM") - .setRegistrarId("registrar1") - // These are non-null, but I don't think some tests set them. - .setReason("felt like it") - .setRequestedByRegistrar(false) - .setXmlBytes(new byte[0]) - .build(); - BillingEvent.Recurring billEvent = - new BillingEvent.Recurring.Builder() - .setId(200L) - .setReason(Reason.RENEW) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setTargetId("example.com") - .setRegistrarId("registrar1") - .setDomainRepoId("4-COM") - .setDomainHistoryRevisionId(1L) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setParent(historyEntry) - .build(); - PollMessage.Autorenew autorenewPollMessage = - new PollMessage.Autorenew.Builder() - .setId(300L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - PollMessage.OneTime deletePollMessage = - new PollMessage.OneTime.Builder() - .setId(400L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - BillingEvent.OneTime oneTimeBillingEvent = - new BillingEvent.OneTime.Builder() - .setId(500L) - // Use SERVER_STATUS so we don't have to add a period. - .setReason(Reason.SERVER_STATUS) - .setTargetId("example.com") - .setRegistrarId("registrar1") - .setDomainRepoId("4-COM") - .setBillingTime(DateTime.now(UTC)) - .setCost(Money.of(USD, 100)) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) - .build(); - DomainTransferData transferData = - createPendingTransferData( - new DomainTransferData.Builder() - .setTransferRequestTrid(Trid.create("foo", "bar")) - .setTransferRequestTime(fakeClock.nowUtc()) - .setGainingRegistrarId("registrar2") - .setLosingRegistrarId("registrar1") - .setPendingTransferExpirationTime(fakeClock.nowUtc().plusDays(1)), - ImmutableSet.of(oneTimeBillingEvent, billEvent, autorenewPollMessage), - Period.create(0, Unit.YEARS)); - gracePeriods = - ImmutableSet.of( - GracePeriod.create( - GracePeriodStatus.ADD, - "4-COM", - END_OF_TIME, - "registrar1", - oneTimeBillingEvent.createVKey()), - GracePeriod.createForRecurring( - GracePeriodStatus.AUTO_RENEW, - "4-COM", - END_OF_TIME, - "registrar1", - billEvent.createVKey())); + historyEntry = + new DomainHistory.Builder() + .setId(100L) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setPeriod(Period.create(1, Period.Unit.YEARS)) + .setModificationTime(DateTime.now(UTC)) + .setDomainRepoId("4-COM") + .setRegistrarId("registrar1") + // These are non-null, but I don't think some tests set them. + .setReason("felt like it") + .setRequestedByRegistrar(false) + .setXmlBytes(new byte[0]) + .build(); + BillingEvent.Recurring billEvent = + new BillingEvent.Recurring.Builder() + .setId(200L) + .setReason(Reason.RENEW) + .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) + .setTargetId("example.com") + .setRegistrarId("registrar1") + .setDomainRepoId("4-COM") + .setDomainHistoryRevisionId(1L) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setRecurrenceEndTime(END_OF_TIME) + .setParent(historyEntry) + .build(); + PollMessage.Autorenew autorenewPollMessage = + new PollMessage.Autorenew.Builder() + .setId(300L) + .setRegistrarId("registrar1") + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + PollMessage.OneTime deletePollMessage = + new PollMessage.OneTime.Builder() + .setId(400L) + .setRegistrarId("registrar1") + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + BillingEvent.OneTime oneTimeBillingEvent = + new BillingEvent.OneTime.Builder() + .setId(500L) + // Use SERVER_STATUS so we don't have to add a period. + .setReason(Reason.SERVER_STATUS) + .setTargetId("example.com") + .setRegistrarId("registrar1") + .setDomainRepoId("4-COM") + .setBillingTime(DateTime.now(UTC)) + .setCost(Money.of(USD, 100)) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + DomainTransferData transferData = + createPendingTransferData( + new DomainTransferData.Builder() + .setTransferRequestTrid(Trid.create("foo", "bar")) + .setTransferRequestTime(fakeClock.nowUtc()) + .setGainingRegistrarId("registrar2") + .setLosingRegistrarId("registrar1") + .setPendingTransferExpirationTime(fakeClock.nowUtc().plusDays(1)), + ImmutableSet.of(oneTimeBillingEvent, billEvent, autorenewPollMessage), + Period.create(0, Unit.YEARS)); + gracePeriods = + ImmutableSet.of( + GracePeriod.create( + GracePeriodStatus.ADD, + "4-COM", + END_OF_TIME, + "registrar1", + oneTimeBillingEvent.createVKey()), + GracePeriod.createForRecurring( + GracePeriodStatus.AUTO_RENEW, + "4-COM", + END_OF_TIME, + "registrar1", + billEvent.createVKey())); - jpaTm().insert(contact); - jpaTm().insert(contact2); - jpaTm().insert(host); - domain = - domain - .asBuilder() - .setAutorenewBillingEvent( - createLegacyVKey(BillingEvent.Recurring.class, billEvent.getId())) - .setAutorenewPollMessage( - createLegacyVKey( - PollMessage.Autorenew.class, autorenewPollMessage.getId())) - .setDeletePollMessage( - createLegacyVKey(PollMessage.OneTime.class, deletePollMessage.getId())) - .setTransferData(transferData) - .setGracePeriods(gracePeriods) - .build(); - historyEntry = historyEntry.asBuilder().setDomain(domain).build(); - jpaTm().insert(historyEntry); - jpaTm().insert(autorenewPollMessage); - jpaTm().insert(billEvent); - jpaTm().insert(deletePollMessage); - jpaTm().insert(oneTimeBillingEvent); - jpaTm().insert(domain); - }); + domain = + domain + .asBuilder() + .setAutorenewBillingEvent( + createLegacyVKey(BillingEvent.Recurring.class, billEvent.getId())) + .setAutorenewPollMessage( + createLegacyVKey(PollMessage.Autorenew.class, autorenewPollMessage.getId())) + .setDeletePollMessage( + createLegacyVKey(PollMessage.OneTime.class, deletePollMessage.getId())) + .setTransferData(transferData) + .setGracePeriods(gracePeriods) + .build(); + historyEntry = historyEntry.asBuilder().setDomain(domain).build(); + insertInDb( + contact, + contact2, + host, + historyEntry, + autorenewPollMessage, + billEvent, + deletePollMessage, + oneTimeBillingEvent, + domain); // Store the existing BillingRecurrence VKey. This happens after the event has been persisted. - DomainBase persisted = jpaTm().transact(() -> jpaTm().loadByKey(domain.createVKey())); + DomainBase persisted = loadByKey(domain.createVKey()); // Verify that the domain data has been persisted. // dsData still isn't persisted. gracePeriods appears to have the same values but for some @@ -677,8 +619,7 @@ public class DomainBaseSqlTest { assertEqualDomainExcept(persisted, "creationTime", "dsData", "gracePeriods"); // Verify that the DomainContent object from the history record sets the fields correctly. - DomainHistory persistedHistoryEntry = - jpaTm().transact(() -> jpaTm().loadByKey(historyEntry.createVKey())); + DomainHistory persistedHistoryEntry = loadByKey(historyEntry.createVKey()); assertThat(persistedHistoryEntry.getDomainContent().get().getAutorenewPollMessage()) .isEqualTo(domain.getAutorenewPollMessage()); assertThat(persistedHistoryEntry.getDomainContent().get().getAutorenewBillingEvent()) diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java index ce371779a..69e284cfc 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java @@ -208,7 +208,7 @@ public class DomainBaseTest extends EntityTestCase { @Test void testIndexing() throws Exception { - verifyIndexing( + verifyDatastoreIndexing( domain, "allContacts.contact", "fullyQualifiedDomainName", diff --git a/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java b/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java index 5d1af30e4..120cd8fdf 100644 --- a/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java +++ b/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java @@ -97,7 +97,7 @@ public class AllocationTokenTest extends EntityTestCase { void testIndexing() throws Exception { DomainBase domain = persistActiveDomain("blahdomain.foo"); Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1); - verifyIndexing( + verifyDatastoreIndexing( persistResource( new AllocationToken.Builder() .setToken("abc123") diff --git a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java index abeade002..8e1326569 100644 --- a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java @@ -19,9 +19,9 @@ import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableO import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newContactResource; import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid; -import static google.registry.testing.SqlHelper.saveRegistrar; import static java.nio.charset.StandardCharsets.UTF_8; import com.googlecode.objectify.Key; @@ -32,23 +32,23 @@ import google.registry.model.contact.ContactResource; import google.registry.model.eppcommon.Trid; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; -import org.junit.jupiter.api.Test; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyOnly; +import google.registry.testing.TestSqlOnly; /** Tests for {@link ContactHistory}. */ +@DualDatabaseTest public class ContactHistoryTest extends EntityTestCase { ContactHistoryTest() { super(JpaEntityCoverageCheck.ENABLED); } - @Test + @TestSqlOnly void testPersistence() { - saveRegistrar("TheRegistrar"); - ContactResource contact = newContactResourceWithRoid("contactId", "contact1"); insertInDb(contact); - VKey contactVKey = contact.createVKey(); - ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().loadByKey(contactVKey)); + ContactResource contactFromDb = loadByEntity(contact); ContactHistory contactHistory = createContactHistory(contactFromDb); insertInDb(contactHistory); jpaTm() @@ -60,14 +60,11 @@ public class ContactHistoryTest extends EntityTestCase { }); } - @Test + @TestSqlOnly void testLegacyPersistence_nullContactBase() { - saveRegistrar("TheRegistrar"); - ContactResource contact = newContactResourceWithRoid("contactId", "contact1"); insertInDb(contact); - VKey contactVKey = contact.createVKey(); - ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().loadByKey(contactVKey)); + ContactResource contactFromDb = loadByEntity(contact); ContactHistory contactHistory = createContactHistory(contactFromDb).asBuilder().setContact(null).build(); insertInDb(contactHistory); @@ -81,10 +78,8 @@ public class ContactHistoryTest extends EntityTestCase { }); } - @Test + @TestOfyOnly void testOfyPersistence() { - saveRegistrar("TheRegistrar"); - ContactResource contact = newContactResourceWithRoid("contactId", "contact1"); tm().transact(() -> tm().insert(contact)); VKey contactVKey = contact.createVKey(); @@ -105,9 +100,8 @@ public class ContactHistoryTest extends EntityTestCase { assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb); } - @Test + @TestSqlOnly void testBeforeSqlSave_afterContactPersisted() { - saveRegistrar("TheRegistrar"); ContactResource contactResource = newContactResource("contactId"); ContactHistory contactHistory = new ContactHistory.Builder() diff --git a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java index d185fa1b5..7bc2ce962 100644 --- a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java @@ -26,6 +26,7 @@ import static google.registry.testing.DatabaseHelper.insertInDb; import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid; +import static google.registry.testing.DatabaseHelper.putInDb; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static java.nio.charset.StandardCharsets.UTF_8; @@ -146,14 +147,8 @@ public class DomainHistoryTest extends EntityTestCase { "tld", "TLD", ImmutableSortedMap.of(START_OF_TIME, GENERAL_AVAILABILITY)); tm().transact(() -> tm().insert(registry)); Registries.resetCache(); - jpaTm() - .transact( - () -> { - jpaTm().insert(registry); - jpaTm() - .insert( - makeRegistrar2().asBuilder().setAllowedTlds(ImmutableSet.of("tld")).build()); - }); + insertInDb( + registry, makeRegistrar2().asBuilder().setAllowedTlds(ImmutableSet.of("tld")).build()); HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); ContactResource contact = newContactResourceWithRoid("contactId", "contact1"); @@ -164,12 +159,7 @@ public class DomainHistoryTest extends EntityTestCase { tm().insert(host); tm().insert(contact); }); - jpaTm() - .transact( - () -> { - jpaTm().insert(host); - jpaTm().insert(contact); - }); + insertInDb(host, contact); DomainBase domain = newDomainBase("example.tld", "domainRepoId", contact) .asBuilder() @@ -190,7 +180,7 @@ public class DomainHistoryTest extends EntityTestCase { // Reload and rewrite. DomainHistory domainHistoryFromDb2 = tm().transact(() -> tm().loadByKey(domainHistoryVKey)); - jpaTm().transact(() -> jpaTm().put(domainHistoryFromDb2)); + putInDb(domainHistoryFromDb2); } @TestSqlOnly diff --git a/core/src/test/java/google/registry/model/history/HostHistoryTest.java b/core/src/test/java/google/registry/model/history/HostHistoryTest.java index 44b844d41..a45431f1f 100644 --- a/core/src/test/java/google/registry/model/history/HostHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/HostHistoryTest.java @@ -19,6 +19,7 @@ import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableO import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newHostResource; import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid; import static google.registry.testing.SqlHelper.saveRegistrar; @@ -32,24 +33,23 @@ import google.registry.model.host.HostHistory; import google.registry.model.host.HostResource; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; -import org.junit.jupiter.api.Test; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyOnly; +import google.registry.testing.TestSqlOnly; /** Tests for {@link HostHistory}. */ +@DualDatabaseTest public class HostHistoryTest extends EntityTestCase { HostHistoryTest() { super(JpaEntityCoverageCheck.ENABLED); } - @Test + @TestSqlOnly void testPersistence() { - saveRegistrar("TheRegistrar"); - HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); insertInDb(host); - VKey hostVKey = - VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1")); - HostResource hostFromDb = jpaTm().transact(() -> jpaTm().loadByKey(hostVKey)); + HostResource hostFromDb = loadByEntity(host); HostHistory hostHistory = createHostHistory(hostFromDb); insertInDb(hostHistory); jpaTm() @@ -61,13 +61,12 @@ public class HostHistoryTest extends EntityTestCase { }); } - @Test + @TestSqlOnly void testLegacyPersistence_nullHostBase() { - saveRegistrar("TheRegistrar"); HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); insertInDb(host); - HostResource hostFromDb = jpaTm().transact(() -> jpaTm().loadByKey(host.createVKey())); + HostResource hostFromDb = loadByEntity(host); HostHistory hostHistory = createHostHistory(hostFromDb).asBuilder().setHost(null).build(); insertInDb(hostHistory); @@ -80,7 +79,7 @@ public class HostHistoryTest extends EntityTestCase { }); } - @Test + @TestOfyOnly void testOfySave() { saveRegistrar("registrar1"); @@ -105,9 +104,8 @@ public class HostHistoryTest extends EntityTestCase { assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb); } - @Test + @TestSqlOnly void testBeforeSqlSave_afterHostPersisted() { - saveRegistrar("TheRegistrar"); HostResource hostResource = newHostResource("ns1.example.tld"); HostHistory hostHistory = new HostHistory.Builder() diff --git a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java index 4aed091f7..e4487919f 100644 --- a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java +++ b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java @@ -20,6 +20,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByKey; import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid; import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid; import static google.registry.util.CollectionUtils.nullToEmpty; @@ -79,14 +80,8 @@ public class LegacyHistoryObjectTest extends EntityTestCase { ContactHistory legacyContactHistory = (ContactHistory) fromObjectify; // Next, save that from-Datastore object in SQL and verify we can load it back in - jpaTm() - .transact( - () -> { - jpaTm().insert(contact); - jpaTm().insert(legacyContactHistory); - }); - ContactHistory legacyHistoryFromSql = - jpaTm().transact(() -> jpaTm().loadByKey(legacyContactHistory.createVKey())); + insertInDb(contact, legacyContactHistory); + ContactHistory legacyHistoryFromSql = loadByKey(legacyContactHistory.createVKey()); assertAboutImmutableObjects() .that(legacyContactHistory) .isEqualExceptFields(legacyHistoryFromSql); @@ -171,14 +166,8 @@ public class LegacyHistoryObjectTest extends EntityTestCase { HostHistory legacyHostHistory = (HostHistory) fromObjectify; // Next, save that from-Datastore object in SQL and verify we can load it back in - jpaTm() - .transact( - () -> { - jpaTm().insert(host); - jpaTm().insert(legacyHostHistory); - }); - HostHistory legacyHistoryFromSql = - jpaTm().transact(() -> jpaTm().loadByKey(legacyHostHistory.createVKey())); + insertInDb(host, legacyHostHistory); + HostHistory legacyHistoryFromSql = loadByKey(legacyHostHistory.createVKey()); assertAboutImmutableObjects().that(legacyHostHistory).isEqualExceptFields(legacyHistoryFromSql); // can't compare hostRepoId directly since it doesn't save the ofy key in SQL assertThat(legacyHostHistory.getParentVKey().getSqlKey()) diff --git a/core/src/test/java/google/registry/model/host/HostResourceTest.java b/core/src/test/java/google/registry/model/host/HostResourceTest.java index eee2ed4e0..6c46e6d47 100644 --- a/core/src/test/java/google/registry/model/host/HostResourceTest.java +++ b/core/src/test/java/google/registry/model/host/HostResourceTest.java @@ -121,7 +121,7 @@ class HostResourceTest extends EntityTestCase { void testIndexing() throws Exception { // Clone it and save it before running the indexing test so that its transferData fields are // populated from the superordinate domain. - verifyIndexing( + verifyDatastoreIndexing( persistResource(host), "deletionTime", "fullyQualifiedHostName", diff --git a/core/src/test/java/google/registry/model/index/EppResourceIndexTest.java b/core/src/test/java/google/registry/model/index/EppResourceIndexTest.java index 48e446930..c7da11258 100644 --- a/core/src/test/java/google/registry/model/index/EppResourceIndexTest.java +++ b/core/src/test/java/google/registry/model/index/EppResourceIndexTest.java @@ -49,7 +49,7 @@ class EppResourceIndexTest extends EntityTestCase { @Test void testIndexing() throws Exception { - verifyIndexing(Iterables.getOnlyElement(getEppResourceIndexObjects()), "kind"); + verifyDatastoreIndexing(Iterables.getOnlyElement(getEppResourceIndexObjects()), "kind"); } @Test diff --git a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java index b7b15ba6c..bce4a2306 100644 --- a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java +++ b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java @@ -79,7 +79,7 @@ class ForeignKeyIndexTest extends EntityTestCase { void testIndexing() throws Exception { // Persist a host and implicitly persist a ForeignKeyIndex for it. persistActiveHost("ns1.example.com"); - verifyIndexing( + verifyDatastoreIndexing( ForeignKeyIndex.load(HostResource.class, "ns1.example.com", fakeClock.nowUtc()), "deletionTime"); } diff --git a/core/src/test/java/google/registry/model/poll/PollMessageTest.java b/core/src/test/java/google/registry/model/poll/PollMessageTest.java index 704aa0fd3..76beca234 100644 --- a/core/src/test/java/google/registry/model/poll/PollMessageTest.java +++ b/core/src/test/java/google/registry/model/poll/PollMessageTest.java @@ -15,10 +15,10 @@ package google.registry.model.poll; import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByKey; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistResource; @@ -94,17 +94,13 @@ public class PollMessageTest extends EntityTestCase { @TestSqlOnly void testCloudSqlSupportForPolymorphicVKey() { insertInDb(oneTime); - PollMessage persistedOneTime = - jpaTm() - .transact(() -> jpaTm().loadByKey(VKey.createSql(PollMessage.class, oneTime.getId()))); + PollMessage persistedOneTime = loadByKey(VKey.createSql(PollMessage.class, oneTime.getId())); assertThat(persistedOneTime).isInstanceOf(PollMessage.OneTime.class); assertThat(persistedOneTime).isEqualTo(oneTime); insertInDb(autoRenew); PollMessage persistedAutoRenew = - jpaTm() - .transact( - () -> jpaTm().loadByKey(VKey.createSql(PollMessage.class, autoRenew.getId()))); + loadByKey(VKey.createSql(PollMessage.class, autoRenew.getId())); assertThat(persistedAutoRenew).isInstanceOf(PollMessage.Autorenew.class); assertThat(persistedAutoRenew).isEqualTo(autoRenew); } @@ -149,6 +145,6 @@ public class PollMessageTest extends EntityTestCase { .setAutorenewEndTime(fakeClock.nowUtc().plusDays(365)) .setTargetId("foobar.foo") .build()); - verifyIndexing(pollMessage); + verifyDatastoreIndexing(pollMessage); } } diff --git a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java index 0fb78f682..2f34418da 100644 --- a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java +++ b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java @@ -137,7 +137,7 @@ class RegistrarTest extends EntityTestCase { @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing(registrar, "registrarName", "ianaIdentifier"); + verifyDatastoreIndexing(registrar, "registrarName", "ianaIdentifier"); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java index ccbc12896..b162b0369 100644 --- a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java +++ b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java @@ -100,12 +100,7 @@ public class ReplicateToDatastoreActionTest { TestObject bar = TestObject.create("bar"); TestObject baz = TestObject.create("baz"); - jpaTm() - .transact( - () -> { - jpaTm().insert(foo); - jpaTm().insert(bar); - }); + insertInDb(foo, bar); runAndVerifySuccess(); assertThat(ofyTm().transact(() -> ofyTm().loadByKey(foo.key()))).isEqualTo(foo); @@ -219,7 +214,7 @@ public class ReplicateToDatastoreActionTest { @Test void testBeforeDatastoreSaveCallback() { TestObject testObject = TestObject.create("foo"); - jpaTm().transact(() -> jpaTm().put(testObject)); + insertInDb(testObject); action.run(); assertThat(ofyTm().loadAllOf(TestObject.class)).containsExactly(testObject); assertThat(TestObject.beforeDatastoreSaveCallCount).isEqualTo(1); diff --git a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java index e161bc28c..f3e2bf5e5 100644 --- a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java +++ b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java @@ -157,6 +157,6 @@ class HistoryEntryTest extends EntityTestCase { @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing(domainHistory.asHistoryEntry(), "modificationTime", "clientId"); + verifyDatastoreIndexing(domainHistory.asHistoryEntry(), "modificationTime", "clientId"); } } diff --git a/core/src/test/java/google/registry/model/reporting/Spec11ThreatMatchTest.java b/core/src/test/java/google/registry/model/reporting/Spec11ThreatMatchTest.java index 2aad2f643..f76fd8f88 100644 --- a/core/src/test/java/google/registry/model/reporting/Spec11ThreatMatchTest.java +++ b/core/src/test/java/google/registry/model/reporting/Spec11ThreatMatchTest.java @@ -14,11 +14,12 @@ package google.registry.model.reporting; -import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.model.reporting.Spec11ThreatMatch.ThreatType.MALWARE; import static google.registry.model.reporting.Spec11ThreatMatch.ThreatType.UNWANTED_SOFTWARE; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation; import static google.registry.testing.SqlHelper.saveRegistrar; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -107,53 +108,20 @@ public final class Spec11ThreatMatchTest extends EntityTestCase { void testPersistence() { createTld("tld"); saveRegistrar(REGISTRAR_ID); - - jpaTm() - .transact( - () -> { - jpaTm().insert(registrantContact); - jpaTm().insert(domain); - jpaTm().insert(host); - jpaTm().insert(threat); - }); - - VKey threatVKey = VKey.createSql(Spec11ThreatMatch.class, threat.getId()); - Spec11ThreatMatch persistedThreat = jpaTm().transact(() -> jpaTm().loadByKey(threatVKey)); - - // Threat object saved for the first time doesn't have an ID; it is generated by SQL - threat.id = persistedThreat.id; - assertThat(threat).isEqualTo(persistedThreat); + insertInDb(registrantContact, domain, host, threat); + assertAboutImmutableObjects().that(loadByEntity(threat)).isEqualExceptFields(threat, "id"); } @TestSqlOnly @Disabled("We can't rely on foreign keys until we've migrated to SQL") void testThreatForeignKeyConstraints() { - assertThrowForeignKeyViolation( - () -> { - jpaTm() - .transact( - () -> { - // Persist the threat without the associated registrar. - jpaTm().insert(host); - jpaTm().insert(registrantContact); - jpaTm().insert(domain); - jpaTm().insert(threat); - }); - }); + // Persist the threat without the associated registrar. + assertThrowForeignKeyViolation(() -> insertInDb(host, registrantContact, domain, threat)); saveRegistrar(REGISTRAR_ID); - assertThrowForeignKeyViolation( - () -> { - jpaTm() - .transact( - () -> { - // Persist the threat without the associated domain. - jpaTm().insert(registrantContact); - jpaTm().insert(host); - jpaTm().insert(threat); - }); - }); + // Persist the threat without the associated domain. + assertThrowForeignKeyViolation(() -> insertInDb(registrantContact, host, threat)); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/model/tld/RegistryTest.java b/core/src/test/java/google/registry/model/tld/RegistryTest.java index c8185eed3..a4fef56a3 100644 --- a/core/src/test/java/google/registry/model/tld/RegistryTest.java +++ b/core/src/test/java/google/registry/model/tld/RegistryTest.java @@ -95,7 +95,7 @@ public final class RegistryTest extends EntityTestCase { @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing(Registry.get("tld")); + verifyDatastoreIndexing(Registry.get("tld")); } @TestOfyAndSql diff --git a/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java index 9fd3597bb..60fcf6ff6 100644 --- a/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java @@ -16,6 +16,7 @@ package google.registry.persistence.transaction; import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.testing.DatabaseHelper.insertInDb; import com.google.common.collect.ImmutableList; import google.registry.model.ImmutableObject; @@ -49,7 +50,7 @@ class CriteriaQueryBuilderTest { @BeforeEach void beforeEach() { - jpaTm().transact(() -> jpaTm().putAll(ImmutableList.of(entity1, entity2, entity3))); + insertInDb(entity1, entity2, entity3); } @Test diff --git a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java index bf715c0ad..1ceb3c729 100644 --- a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java @@ -18,7 +18,9 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.DatabaseHelper.assertDetachedFromEntityManager; +import static google.registry.testing.DatabaseHelper.existsInDb; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByKey; import static google.registry.testing.TestDataHelper.fileClassPath; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -46,6 +48,8 @@ import javax.persistence.IdClass; import javax.persistence.OptimisticLockException; import javax.persistence.RollbackException; import org.hibernate.exception.JDBCConnectionException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -80,6 +84,16 @@ class JpaTransactionManagerImplTest { TestEntity.class, TestCompoundIdEntity.class, TestNamedCompoundIdEntity.class) .buildUnitTestExtension(); + @BeforeEach + void beforeEach() { + TransactionManagerFactory.setTmForTest(jpaTm()); + } + + @AfterEach + void afterEach() { + TransactionManagerFactory.removeTmOverrideForTest(); + } + @Test void transact_succeeds() { assertPersonEmpty(); @@ -139,10 +153,10 @@ class JpaTransactionManagerImplTest { @Test void insert_succeeds() { - assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse(); - insertInDb(theEntity); - assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isTrue(); - assertThat(jpaTm().transact(() -> jpaTm().loadByKey(theEntityKey))).isEqualTo(theEntity); + assertThat(existsInDb(theEntity)).isFalse(); + jpaTm().transact(() -> jpaTm().insert(theEntity)); + assertThat(existsInDb(theEntity)).isTrue(); + assertThat(loadByKey(theEntityKey)).isEqualTo(theEntity); } @Test @@ -258,17 +272,17 @@ class JpaTransactionManagerImplTest { @Test void insert_throwsExceptionIfEntityExists() { - assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse(); - insertInDb(theEntity); - assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isTrue(); - assertThat(jpaTm().transact(() -> jpaTm().loadByKey(theEntityKey))).isEqualTo(theEntity); - assertThrows(RollbackException.class, () -> insertInDb(theEntity)); + assertThat(existsInDb(theEntity)).isFalse(); + jpaTm().transact(() -> jpaTm().insert(theEntity)); + assertThat(existsInDb(theEntity)).isTrue(); + assertThat(loadByKey(theEntityKey)).isEqualTo(theEntity); + assertThrows(RollbackException.class, () -> jpaTm().transact(() -> jpaTm().insert(theEntity))); } @Test void createCompoundIdEntity_succeeds() { assertThat(jpaTm().transact(() -> jpaTm().exists(compoundIdEntity))).isFalse(); - insertInDb(compoundIdEntity); + jpaTm().transact(() -> jpaTm().insert(compoundIdEntity)); assertThat(jpaTm().transact(() -> jpaTm().exists(compoundIdEntity))).isTrue(); assertThat(jpaTm().transact(() -> jpaTm().loadByKey(compoundIdEntityKey))) .isEqualTo(compoundIdEntity); @@ -278,18 +292,12 @@ class JpaTransactionManagerImplTest { void createNamedCompoundIdEntity_succeeds() { // Compound IDs should also work even if the field names don't match up exactly TestNamedCompoundIdEntity entity = new TestNamedCompoundIdEntity("foo", 1); - insertInDb(entity); - jpaTm() - .transact( - () -> { - assertThat(jpaTm().exists(entity)).isTrue(); - assertThat( - jpaTm() - .loadByKey( - VKey.createSql( - TestNamedCompoundIdEntity.class, new NamedCompoundId("foo", 1)))) - .isEqualTo(entity); - }); + jpaTm().transact(() -> jpaTm().insert(entity)); + assertThat(existsInDb(entity)).isTrue(); + assertThat( + loadByKey( + VKey.createSql(TestNamedCompoundIdEntity.class, new NamedCompoundId("foo", 1)))) + .isEqualTo(entity); } @Test diff --git a/core/src/test/java/google/registry/schema/registrar/RegistrarContactTest.java b/core/src/test/java/google/registry/schema/registrar/RegistrarContactTest.java index a76113813..d2489a92d 100644 --- a/core/src/test/java/google/registry/schema/registrar/RegistrarContactTest.java +++ b/core/src/test/java/google/registry/schema/registrar/RegistrarContactTest.java @@ -17,7 +17,9 @@ package google.registry.schema.registrar; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.registrar.RegistrarContact.Type.WHOIS; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.setTmForTest; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.SqlHelper.saveRegistrar; import com.google.common.collect.ImmutableSet; @@ -48,6 +50,7 @@ class RegistrarContactTest { @BeforeEach public void beforeEach() { + setTmForTest(jpaTm()); testRegistrar = saveRegistrar("registrarId"); testRegistrarPoc = new RegistrarContact.Builder() @@ -67,8 +70,6 @@ class RegistrarContactTest { @Test void testPersistence_succeeds() { insertInDb(testRegistrarPoc); - RegistrarContact persisted = - jpaTm().transact(() -> jpaTm().loadByKey(testRegistrarPoc.createVKey())); - assertThat(persisted).isEqualTo(testRegistrarPoc); + assertThat(loadByEntity(testRegistrarPoc)).isEqualTo(testRegistrarPoc); } } diff --git a/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java b/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java index e62c4372f..edf52a19f 100644 --- a/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java +++ b/core/src/test/java/google/registry/schema/registrar/RegistrarDaoTest.java @@ -16,7 +16,10 @@ package google.registry.schema.registrar; import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.testing.DatabaseHelper.existsInDb; import static google.registry.testing.DatabaseHelper.insertInDb; +import static google.registry.testing.DatabaseHelper.loadByKey; +import static google.registry.testing.DatabaseHelper.updateInDb; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -26,9 +29,11 @@ import google.registry.model.registrar.RegistrarAddress; import google.registry.persistence.VKey; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; +import google.registry.persistence.transaction.TransactionManagerFactory; import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import org.joda.time.DateTime; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -52,7 +57,8 @@ public class RegistrarDaoTest { private Registrar testRegistrar; @BeforeEach - void setUp() { + void beforeEach() { + TransactionManagerFactory.setTmForTest(jpaTm()); testRegistrar = new Registrar.Builder() .setType(Registrar.Type.TEST) @@ -69,41 +75,39 @@ public class RegistrarDaoTest { .build(); } + @AfterEach + void afterEach() { + TransactionManagerFactory.removeTmOverrideForTest(); + } + @Test void saveNew_worksSuccessfully() { - assertThat(jpaTm().transact(() -> jpaTm().exists(testRegistrar))).isFalse(); + assertThat(existsInDb(testRegistrar)).isFalse(); insertInDb(testRegistrar); - assertThat(jpaTm().transact(() -> jpaTm().exists(testRegistrar))).isTrue(); + assertThat(existsInDb(testRegistrar)).isTrue(); } @Test void update_worksSuccessfully() { insertInDb(testRegistrar); - Registrar persisted = jpaTm().transact(() -> jpaTm().loadByKey(registrarKey)); + Registrar persisted = loadByKey(registrarKey); assertThat(persisted.getRegistrarName()).isEqualTo("registrarName"); - jpaTm() - .transact( - () -> - jpaTm() - .update( - persisted.asBuilder().setRegistrarName("changedRegistrarName").build())); - Registrar updated = jpaTm().transact(() -> jpaTm().loadByKey(registrarKey)); + updateInDb(persisted.asBuilder().setRegistrarName("changedRegistrarName").build()); + Registrar updated = loadByKey(registrarKey); assertThat(updated.getRegistrarName()).isEqualTo("changedRegistrarName"); } @Test void update_throwsExceptionWhenEntityDoesNotExist() { - assertThat(jpaTm().transact(() -> jpaTm().exists(testRegistrar))).isFalse(); - assertThrows( - IllegalArgumentException.class, - () -> jpaTm().transact(() -> jpaTm().update(testRegistrar))); + assertThat(existsInDb(testRegistrar)).isFalse(); + assertThrows(IllegalArgumentException.class, () -> updateInDb(testRegistrar)); } @Test void load_worksSuccessfully() { - assertThat(jpaTm().transact(() -> jpaTm().exists(testRegistrar))).isFalse(); + assertThat(existsInDb(testRegistrar)).isFalse(); insertInDb(testRegistrar); - Registrar persisted = jpaTm().transact(() -> jpaTm().loadByKey(registrarKey)); + Registrar persisted = loadByKey(registrarKey); assertThat(persisted.getRegistrarId()).isEqualTo("registrarId"); assertThat(persisted.getRegistrarName()).isEqualTo("registrarName"); diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 3b613a1cb..13095db80 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -986,18 +986,18 @@ public class DatabaseHelper { *

Note: Your resource will not be enrolled in a commit log. If you want backups, use * {@link #persistResourceWithCommitLog(Object)}. */ - public static R persistResource(final R resource) { + public static R persistResource(final R resource) { return persistResource(resource, false); } /** Same as {@link #persistResource(Object)} with backups enabled. */ - public static R persistResourceWithCommitLog(final R resource) { + public static R persistResourceWithCommitLog(final R resource) { return persistResource(resource, true); } - private static void saveResource(R resource, boolean wantBackup) { + private static void saveResource(R resource, boolean wantBackup) { if (tm().isOfy()) { - Consumer saver = + Consumer saver = wantBackup || alwaysSaveWithBackup ? tm()::put : tm()::putWithoutBackup; saver.accept(resource); if (resource instanceof EppResource) { @@ -1011,7 +1011,7 @@ public class DatabaseHelper { } private static void persistEppResourceExtras( - R resource, EppResourceIndex index, Consumer saver) { + R resource, EppResourceIndex index, Consumer saver) { assertWithMessage("Cannot persist an EppResource with a missing repoId in tests") .that(resource.getRepoId()) .isNotEmpty(); @@ -1021,7 +1021,8 @@ public class DatabaseHelper { } } - private static R persistResource(final R resource, final boolean wantBackup) { + private static R persistResource( + final R resource, final boolean wantBackup) { assertWithMessage("Attempting to persist a Builder is almost certainly an error in test code") .that(resource) .isNotInstanceOf(Buildable.Builder.class); @@ -1042,7 +1043,7 @@ public class DatabaseHelper { tm().transact( () -> { if (tm().isOfy()) { - Consumer saver = tm()::put; + Consumer saver = tm()::put; saver.accept(resource); persistEppResourceExtras(resource, eppResourceIndex, saver); } else { @@ -1054,11 +1055,12 @@ public class DatabaseHelper { return transactIfJpaTm(() -> tm().loadByEntity(resource)); } - public static void persistResources(final Iterable resources) { + public static void persistResources(final Iterable resources) { persistResources(resources, false); } - private static void persistResources(final Iterable resources, final boolean wantBackup) { + private static void persistResources( + final Iterable resources, final boolean wantBackup) { for (R resource : resources) { assertWithMessage("Attempting to persist a Builder is almost certainly an error in test code") .that(resource) @@ -1379,9 +1381,9 @@ public class DatabaseHelper { return transactIfJpaTm(() -> tm().loadByEntitiesIfPresent(entities)); } - /** Returns whether or not the given entity exists in the database. */ - public static boolean existsInDatabase(Object object) { - return transactIfJpaTm(() -> tm().exists(object)); + /** Returns whether or not the given entity exists in Cloud SQL. */ + public static boolean existsInDb(ImmutableObject object) { + return jpaTm().transact(() -> jpaTm().exists(object)); } /** Inserts the given entity/entities into Cloud SQL in a single transaction. */ @@ -1394,6 +1396,26 @@ public class DatabaseHelper { jpaTm().transact(() -> jpaTm().insertAll(entities)); } + /** Puts the given entity/entities into Cloud SQL in a single transaction. */ + public static void putInDb(T... entities) { + jpaTm().transact(() -> jpaTm().putAll(entities)); + } + + /** Puts the given entities into Cloud SQL in a single transaction. */ + public static void putInDb(ImmutableCollection entities) { + jpaTm().transact(() -> jpaTm().putAll(entities)); + } + + /** Updates the given entities in Cloud SQL in a single transaction. */ + public static void updateInDb(T... entities) { + jpaTm().transact(() -> jpaTm().updateAll(entities)); + } + + /** Updates the given entities in Cloud SQL in a single transaction. */ + public static void updateInDb(ImmutableCollection entities) { + jpaTm().transact(() -> jpaTm().updateAll(entities)); + } + /** * In JPA mode, asserts that the given entity is detached from the current entity manager. * diff --git a/core/src/test/java/google/registry/tools/javascrap/BackfillSpec11ThreatMatchesCommandTest.java b/core/src/test/java/google/registry/tools/javascrap/BackfillSpec11ThreatMatchesCommandTest.java index e488db031..4d6e39807 100644 --- a/core/src/test/java/google/registry/tools/javascrap/BackfillSpec11ThreatMatchesCommandTest.java +++ b/core/src/test/java/google/registry/tools/javascrap/BackfillSpec11ThreatMatchesCommandTest.java @@ -22,6 +22,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.deleteResource; +import static google.registry.testing.DatabaseHelper.insertInDb; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; @@ -175,7 +176,7 @@ public class BackfillSpec11ThreatMatchesCommandTest .setRegistrarId("TheRegistrar") .setThreatTypes(ImmutableSet.of(ThreatType.MALWARE)) .build(); - jpaTm().transact(() -> jpaTm().put(previous)); + insertInDb(previous); runCommandForced(); ImmutableList threatMatches = @@ -197,7 +198,7 @@ public class BackfillSpec11ThreatMatchesCommandTest .setRegistrarId("TheRegistrar") .setThreatTypes(ImmutableSet.of(ThreatType.MALWARE)) .build(); - jpaTm().transact(() -> jpaTm().put(previous)); + insertInDb(previous); runCommandForced("--overwrite_existing_dates"); verifyExactlyThreeEntriesInDbFromLastDay(); @@ -214,7 +215,7 @@ public class BackfillSpec11ThreatMatchesCommandTest assertThrows(RuntimeException.class, this::runCommandForced); assertThat(runtimeException.getCause().getClass()).isEqualTo(IOException.class); assertThat(runtimeException).hasCauseThat().hasMessageThat().isEqualTo("hi"); - jpaTm().transact(() -> assertThat(jpaTm().loadAllOf(Spec11ThreatMatch.class)).isEmpty()); + assertThat(jpaTm().transact(() -> jpaTm().loadAllOf(Spec11ThreatMatch.class))).isEmpty(); } @TestOfyAndSql