From 47a38e8309b842f526a0a8e9dbcc0e26c179047c Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Tue, 28 Sep 2021 17:16:54 -0400 Subject: [PATCH] Add/use more DatabaseHelper convenience methods (#1327) * Add/use more DatabaseHelper convenience methods This also fixes up some existing uses of "put" in test code that should be inserts or updates (depending on which is intended). Doing an insert/update makes stronger guarantees about an entity either not existing or existing, depending on what you're doing. * Convert more Object -> ImmutableObject * Merge branch 'master' into tx-manager-sigs * Revert breaking PremiumListDao change * Refactor more insertInDb() * Fight more testing errors * Merge branch 'master' into tx-manager-sigs * Merge branch 'master' into tx-manager-sigs * Merge branch 'master' into tx-manager-sigs * Merge branch 'master' into tx-manager-sigs * Add removeTmOverrideForTest() calls * Merge branch 'master' into tx-manager-sigs --- .../ofy/DatastoreTransactionManager.java | 10 +- .../JpaTransactionManagerImpl.java | 10 +- .../transaction/TransactionManager.java | 8 +- .../TransactionManagerFactory.java | 4 +- .../ReplayCommitLogsToSqlActionTest.java | 42 +- .../beam/common/RegistryJpaReadTest.java | 5 +- .../beam/common/RegistryJpaWriteTest.java | 10 +- .../beam/initsql/BackupTestStore.java | 3 +- .../google/registry/model/EntityTestCase.java | 4 +- .../model/billing/BillingEventTest.java | 10 +- .../registry/model/common/CursorTest.java | 2 +- .../model/contact/ContactResourceTest.java | 79 ++- .../model/domain/DomainBaseSqlTest.java | 487 ++++++++---------- .../registry/model/domain/DomainBaseTest.java | 2 +- .../domain/token/AllocationTokenTest.java | 2 +- .../model/history/ContactHistoryTest.java | 28 +- .../model/history/DomainHistoryTest.java | 20 +- .../model/history/HostHistoryTest.java | 24 +- .../history/LegacyHistoryObjectTest.java | 21 +- .../registry/model/host/HostResourceTest.java | 2 +- .../model/index/EppResourceIndexTest.java | 2 +- .../model/index/ForeignKeyIndexTest.java | 2 +- .../registry/model/poll/PollMessageTest.java | 12 +- .../model/registrar/RegistrarTest.java | 2 +- .../ReplicateToDatastoreActionTest.java | 9 +- .../model/reporting/HistoryEntryTest.java | 2 +- .../reporting/Spec11ThreatMatchTest.java | 50 +- .../registry/model/tld/RegistryTest.java | 2 +- .../transaction/CriteriaQueryBuilderTest.java | 3 +- .../JpaTransactionManagerImplTest.java | 52 +- .../registrar/RegistrarContactTest.java | 7 +- .../schema/registrar/RegistrarDaoTest.java | 38 +- .../registry/testing/DatabaseHelper.java | 46 +- ...ackfillSpec11ThreatMatchesCommandTest.java | 7 +- 34 files changed, 449 insertions(+), 558 deletions(-) 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