diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index 1fb3aa457..afe8b3750 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -266,7 +266,7 @@ public class DeleteProberDataAction implements Runnable { // Note that we don't bother handling grace periods, billing events, pending transfers, poll // messages, or auto-renews because those will all be hard-deleted the next time the job runs // anyway. - tm().putAllWithoutBackup(ImmutableList.of(deletedDomain, historyEntry)); + tm().putAll(ImmutableList.of(deletedDomain, historyEntry)); // updating foreign keys is a no-op in SQL updateForeignKeyIndexDeletionTime(deletedDomain); dnsQueue.addDomainRefreshTask(deletedDomain.getDomainName()); diff --git a/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java b/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java index 95d1a4419..d9b7fc847 100644 --- a/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java +++ b/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java @@ -197,7 +197,7 @@ public class DatabaseMigrationStateSchedule extends CrossTldSingleton { MigrationState.DATASTORE_ONLY, "migrationTransitionMap must start with DATASTORE_ONLY"); validateTransitionAtCurrentTime(transitions); - jpaTm().putWithoutBackup(new DatabaseMigrationStateSchedule(transitions)); + jpaTm().put(new DatabaseMigrationStateSchedule(transitions)); CACHE.invalidateAll(); } diff --git a/core/src/main/java/google/registry/model/server/ServerSecret.java b/core/src/main/java/google/registry/model/server/ServerSecret.java index ef965e0f5..61a06a55e 100644 --- a/core/src/main/java/google/registry/model/server/ServerSecret.java +++ b/core/src/main/java/google/registry/model/server/ServerSecret.java @@ -66,7 +66,7 @@ public class ServerSecret extends CrossTldSingleton { Optional secret = tm().loadSingleton(ServerSecret.class); if (!secret.isPresent()) { secret = Optional.of(create(UUID.randomUUID())); - tm().insertWithoutBackup(secret.get()); + tm().insert(secret.get()); } return secret.get(); }); diff --git a/core/src/main/java/google/registry/model/tmch/TmchCrl.java b/core/src/main/java/google/registry/model/tmch/TmchCrl.java index 5f2fb6b16..a8b903765 100644 --- a/core/src/main/java/google/registry/model/tmch/TmchCrl.java +++ b/core/src/main/java/google/registry/model/tmch/TmchCrl.java @@ -56,7 +56,7 @@ public final class TmchCrl extends CrossTldSingleton { tmchCrl.updated = jpaTm().getTransactionTime(); tmchCrl.crl = checkNotNull(crl, "crl"); tmchCrl.url = checkNotNull(url, "url"); - jpaTm().transactNew(() -> jpaTm().putWithoutBackup(tmchCrl)); + jpaTm().put(tmchCrl); }); } 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 5b4a0677e..7888a054b 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -293,16 +293,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { insertAll(ImmutableSet.copyOf(entities)); } - @Override - public void insertWithoutBackup(ImmutableObject entity) { - insert(entity); - } - - @Override - public void insertAllWithoutBackup(ImmutableCollection entities) { - insertAll(entities); - } - @Override public void put(Object entity) { checkArgumentNotNull(entity, "entity must be specified"); @@ -329,16 +319,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { entities.forEach(this::put); } - @Override - public void putWithoutBackup(ImmutableObject entity) { - put(entity); - } - - @Override - public void putAllWithoutBackup(ImmutableCollection entities) { - putAll(entities); - } - @Override public void update(Object entity) { checkArgumentNotNull(entity, "entity must be specified"); @@ -362,16 +342,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { updateAll(ImmutableList.copyOf(entities)); } - @Override - public void updateWithoutBackup(ImmutableObject entity) { - update(entity); - } - - @Override - public void updateAllWithoutBackup(ImmutableCollection entities) { - updateAll(entities); - } - @Override public boolean exists(VKey key) { checkArgumentNotNull(key, "key must be specified"); @@ -549,21 +519,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return managedEntity; } - @Override - public void deleteWithoutBackup(VKey key) { - delete(key); - } - - @Override - public void deleteWithoutBackup(Iterable> keys) { - delete(keys); - } - - @Override - public void deleteWithoutBackup(Object entity) { - delete(entity); - } - @Override public QueryComposer createQueryComposer(Class entity) { return new JpaQueryComposerImpl<>(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 2cdcc5ed8..698218de9 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java @@ -101,30 +101,6 @@ public interface TransactionManager { /** Persists all new entities in the database, throws exception if any entity already exists. */ void insertAll(ImmutableObject... entities); - /** - * Persists a new entity in the database without writing any backup if the underlying database is - * Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy() with tm() in - * the application code. When the method is invoked with Datastore, it won't write the commit log - * backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void insertWithoutBackup(ImmutableObject entity); - - /** - * Persists all new entities in the database without writing any backup if the underlying database - * is Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy() with tm() in - * the application code. When the method is invoked with Datastore, it won't write the commit log - * backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void insertAllWithoutBackup(ImmutableCollection entities); - /** Persists a new entity or update the existing entity in the database. */ void put(Object entity); @@ -134,30 +110,6 @@ public interface TransactionManager { /** Persists all new entities or updates the existing entities in the database. */ void putAll(ImmutableCollection entities); - /** - * Persists a new entity or update the existing entity in the database without writing commit logs - * if the underlying database is Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy() with tm() in - * the application code. When the method is invoked with Datastore, it won't write the commit log - * backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void putWithoutBackup(ImmutableObject entity); - - /** - * Persists all new entities or update the existing entities in the database without writing - * commit logs if the underlying database is Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy() with tm() in - * the application code. When the method is invoked with Datastore, it won't write the commit log - * backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void putAllWithoutBackup(ImmutableCollection entities); - /** Updates an entity in the database, throws exception if the entity does not exist. */ void update(Object entity); @@ -167,30 +119,6 @@ public interface TransactionManager { /** Updates all entities in the database, throws exception if any entity does not exist. */ void updateAll(ImmutableObject... entities); - /** - * Updates an entity in the database without writing commit logs if the underlying database is - * Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy calls with tm() - * in the application code. When the method is invoked with Datastore, it won't write the commit - * log backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void updateWithoutBackup(ImmutableObject entity); - - /** - * Updates all entities in the database without writing any backup if the underlying database is - * Datastore. - * - *

This method is for the sake of keeping a single code path when replacing ofy calls with tm() - * in the application code. When the method is invoked with Datastore, it won't write the commit - * log backup; when invoked with Cloud SQL, it behaves the same as the method which doesn't have - * "WithoutBackup" in its method name because it is not necessary to have the backup mechanism in - * SQL. - */ - void updateAllWithoutBackup(ImmutableCollection entities); - /** Returns whether the given entity with same ID exists. */ boolean exists(Object entity); @@ -285,24 +213,6 @@ public interface TransactionManager { */ T delete(T entity); - /** - * Deletes the entity by its id without writing commit logs if the underlying database is - * Datastore. - */ - void deleteWithoutBackup(VKey key); - - /** - * Deletes the set of entities by their key id without writing commit logs if the underlying - * database is Datastore. - */ - void deleteWithoutBackup(Iterable> keys); - - /** - * Deletes the given entity from the database without writing commit logs if the underlying - * database is Datastore. - */ - void deleteWithoutBackup(Object entity); - /** Returns a QueryComposer which can be used to perform queries against the current database. */ QueryComposer createQueryComposer(Class entity); diff --git a/core/src/test/java/google/registry/export/SyncGroupMembersActionTest.java b/core/src/test/java/google/registry/export/SyncGroupMembersActionTest.java index c9358c493..01a783333 100644 --- a/core/src/test/java/google/registry/export/SyncGroupMembersActionTest.java +++ b/core/src/test/java/google/registry/export/SyncGroupMembersActionTest.java @@ -130,8 +130,7 @@ public class SyncGroupMembersActionTest { void test_doPost_removesAllContactsFromGroup() throws Exception { when(connection.getMembersOfGroup("newregistrar-primary-contacts@domain-registry.example")) .thenReturn(ImmutableSet.of("defunct@example.com", "janedoe@theregistrar.com")); - tm().transact( - () -> loadRegistrar("NewRegistrar").getContacts().forEach(tm()::deleteWithoutBackup)); + tm().transact(() -> loadRegistrar("NewRegistrar").getContacts().forEach(tm()::delete)); runAction(); verify(connection).removeMemberFromGroup( "newregistrar-primary-contacts@domain-registry.example", "defunct@example.com"); diff --git a/core/src/test/java/google/registry/persistence/transaction/ReplicaSimulatingJpaTransactionManager.java b/core/src/test/java/google/registry/persistence/transaction/ReplicaSimulatingJpaTransactionManager.java index 2506c922d..c2db90024 100644 --- a/core/src/test/java/google/registry/persistence/transaction/ReplicaSimulatingJpaTransactionManager.java +++ b/core/src/test/java/google/registry/persistence/transaction/ReplicaSimulatingJpaTransactionManager.java @@ -173,16 +173,6 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan delegate.insertAll(entities); } - @Override - public void insertWithoutBackup(ImmutableObject entity) { - delegate.insertWithoutBackup(entity); - } - - @Override - public void insertAllWithoutBackup(ImmutableCollection entities) { - delegate.insertAllWithoutBackup(entities); - } - @Override public void put(Object entity) { delegate.put(entity); @@ -198,16 +188,6 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan delegate.putAll(entities); } - @Override - public void putWithoutBackup(ImmutableObject entity) { - delegate.putWithoutBackup(entity); - } - - @Override - public void putAllWithoutBackup(ImmutableCollection entities) { - delegate.putAllWithoutBackup(entities); - } - @Override public void update(Object entity) { delegate.update(entity); @@ -223,16 +203,6 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan delegate.updateAll(entities); } - @Override - public void updateWithoutBackup(ImmutableObject entity) { - delegate.updateWithoutBackup(entity); - } - - @Override - public void updateAllWithoutBackup(ImmutableCollection entities) { - delegate.updateAllWithoutBackup(entities); - } - @Override public boolean exists(VKey key) { return delegate.exists(key); @@ -310,21 +280,6 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan return delegate.delete(entity); } - @Override - public void deleteWithoutBackup(VKey key) { - delegate.deleteWithoutBackup(key); - } - - @Override - public void deleteWithoutBackup(Iterable> keys) { - delegate.deleteWithoutBackup(keys); - } - - @Override - public void deleteWithoutBackup(Object entity) { - delegate.deleteWithoutBackup(entity); - } - @Override public QueryComposer createQueryComposer(Class entity) { return delegate.createQueryComposer(entity); diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index 7bd3629c8..4a1400a03 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -18,7 +18,6 @@ import static com.google.common.io.BaseEncoding.base16; import static google.registry.testing.DatabaseHelper.generateNewContactHostRoid; import static google.registry.testing.DatabaseHelper.generateNewDomainRoid; import static google.registry.testing.DatabaseHelper.persistResource; -import static google.registry.testing.DatabaseHelper.persistResourceWithBackup; import static google.registry.testing.DatabaseHelper.persistSimpleResource; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static org.joda.money.CurrencyUnit.USD; @@ -76,7 +75,7 @@ final class RdeFixtures { .build()); clock.advanceOneMilli(); BillingEvent.OneTime billingEvent = - persistResourceWithBackup( + persistResource( new BillingEvent.OneTime.Builder() .setReason(Reason.CREATE) .setTargetId("example." + tld) @@ -219,13 +218,13 @@ final class RdeFixtures { .build()) .build(); clock.advanceOneMilli(); - return persistResourceWithBackup(domain); + return persistResource(domain); } static ContactResource makeContactResource( FakeClock clock, String id, String name, String email) { clock.advanceOneMilli(); - return persistResourceWithBackup( + return persistResource( new ContactResource.Builder() .setContactId(id) .setRepoId(generateNewContactHostRoid()) @@ -256,7 +255,7 @@ final class RdeFixtures { static HostResource makeHostResource(FakeClock clock, String fqhn, String ip) { clock.advanceOneMilli(); - return persistResourceWithBackup( + return persistResource( new HostResource.Builder() .setRepoId(generateNewContactHostRoid()) .setCreationRegistrarId("LawyerCat") diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index fcece0542..0f53b54ab 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -999,31 +999,9 @@ public class DatabaseHelper { return createRepoId(allocateId(), getContactAndHostRoidSuffix()); } - /** - * Persists a test resource to Datastore and returns it. - * - *

Tests should always use this method (or the shortcut persist methods in this class) to - * persist test data, to avoid potentially subtle bugs related to race conditions and a stale ofy - * session cache. Specifically, this method calls .now() on the save to force the write to - * actually get sent to Datastore (although it does not force it to be applied) and clears the - * session cache. If necessary, this method also updates the relevant {@link EppResourceIndex}, - * {@link ForeignKeyIndex}. - * - *

Note: Your resource will not be enrolled in a commit log. If you want backups, use - * {@link #persistResourceWithBackup(Object)}. - */ - public static R persistResource(final R resource) { - return persistResource(resource, false); - } - - /** Same as {@link #persistResource(Object)} with backups enabled. */ - public static R persistResourceWithBackup(final R resource) { - return persistResource(resource, true); - } - - private static void saveResource(R resource, boolean wantBackup) { + private static void saveResource(R resource) { if (tm().isOfy()) { - Consumer saver = wantBackup ? tm()::put : tm()::putWithoutBackup; + Consumer saver = tm()::put; saver.accept(resource); if (resource instanceof EppResource) { EppResource eppResource = (EppResource) resource; @@ -1046,12 +1024,12 @@ public class DatabaseHelper { } } - private static R persistResource( - final R resource, final boolean wantBackup) { + /** Persists an object in the DB for tests. */ + public static R persistResource(final R resource) { assertWithMessage("Attempting to persist a Builder is almost certainly an error in test code") .that(resource) .isNotInstanceOf(Buildable.Builder.class); - tm().transact(() -> saveResource(resource, wantBackup)); + tm().transact(() -> saveResource(resource)); maybeAdvanceClock(); // Force the session cache to be cleared so that when we read the resource back, we read from // Datastore and not from the session cache. This is needed to trigger Objectify's load process @@ -1080,12 +1058,8 @@ public class DatabaseHelper { return tm().transact(() -> tm().loadByEntity(resource)); } + /** Persists the specified resources to the DB. */ public static void persistResources(final Iterable resources) { - persistResources(resources, false); - } - - 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) @@ -1093,7 +1067,7 @@ public class DatabaseHelper { } // Persist domains ten at a time, to avoid exceeding the entity group limit. for (final List chunk : Iterables.partition(resources, 10)) { - tm().transact(() -> chunk.forEach(resource -> saveResource(resource, wantBackup))); + tm().transact(() -> chunk.forEach(DatabaseHelper::saveResource)); maybeAdvanceClock(); } // Force the session to be cleared so that when we read it back, we read from Datastore @@ -1248,7 +1222,7 @@ public class DatabaseHelper { * entities. */ public static void insertSimpleResources(final Iterable resources) { - tm().transact(() -> tm().putAllWithoutBackup(ImmutableList.copyOf(resources))); + tm().transact(() -> tm().putAll(ImmutableList.copyOf(resources))); maybeAdvanceClock(); // Force the session to be cleared so that when we read it back, we read from Datastore // and not from the transaction's session cache. @@ -1256,7 +1230,7 @@ public class DatabaseHelper { } public static void deleteResource(final Object resource) { - tm().transact(() -> tm().deleteWithoutBackup(resource)); + tm().transact(() -> tm().delete(resource)); // Force the session to be cleared so that when we read it back, we read from Datastore and // not from the transaction's session cache. tm().clearSessionCache(); diff --git a/core/src/test/java/google/registry/tools/MutatingCommandTest.java b/core/src/test/java/google/registry/tools/MutatingCommandTest.java index eec1cd53b..99d6636c0 100644 --- a/core/src/test/java/google/registry/tools/MutatingCommandTest.java +++ b/core/src/test/java/google/registry/tools/MutatingCommandTest.java @@ -122,7 +122,7 @@ public class MutatingCommandTest { Arrays.asList(host1, host2, registrar1, registrar2).stream() .map(entity -> VKey.from(Key.create(entity))) .collect(toImmutableList()); - tm().transact(() -> tm().deleteWithoutBackup(keys)); + tm().transact(() -> tm().delete(keys)); MutatingCommand command = new MutatingCommand() { @Override protected void init() {