From 4ba0f4a2cd5218e813e985c6af6c2f2affb08228 Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Fri, 20 Sep 2024 11:16:52 -0400 Subject: [PATCH] Change nested transact calls to retransact (#2563) --- .../token/AllocationTokenFlowUtils.java | 3 ++- .../registry/model/EppResourceUtils.java | 2 +- .../registry/model/common/FeatureFlag.java | 2 +- .../registry/model/domain/DomainBase.java | 2 +- .../model/registrar/RegistrarBase.java | 2 +- .../model/registrar/RegistrarPocBase.java | 21 ++++++++--------- .../model/reporting/HistoryEntryDao.java | 2 +- .../google/registry/model/tmch/TmchCrl.java | 3 ++- .../tools/RegistrarPocCommandTest.java | 23 +++++++++++-------- 9 files changed, 31 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java index 7ffd65fbf..288034e3f 100644 --- a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java @@ -177,8 +177,9 @@ public class AllocationTokenFlowUtils { return maybeTokenEntity.get(); } + // TODO(b/368069206): `reTransact` needed by tests only. maybeTokenEntity = - tm().transact(() -> tm().loadByKeyIfPresent(VKey.create(AllocationToken.class, token))); + tm().reTransact(() -> tm().loadByKeyIfPresent(VKey.create(AllocationToken.class, token))); if (maybeTokenEntity.isEmpty()) { throw new InvalidAllocationTokenException(); diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index 7751aff0e..de99b4bd0 100644 --- a/core/src/main/java/google/registry/model/EppResourceUtils.java +++ b/core/src/main/java/google/registry/model/EppResourceUtils.java @@ -345,7 +345,7 @@ public final class EppResourceUtils { "key must be either VKey or VKey, but it is %s", key); boolean isContactKey = key.getKind().equals(Contact.class); - return tm().transact( + return tm().reTransact( () -> { Query query; if (isContactKey) { diff --git a/core/src/main/java/google/registry/model/common/FeatureFlag.java b/core/src/main/java/google/registry/model/common/FeatureFlag.java index 77abde610..3386bf1d8 100644 --- a/core/src/main/java/google/registry/model/common/FeatureFlag.java +++ b/core/src/main/java/google/registry/model/common/FeatureFlag.java @@ -82,7 +82,7 @@ public class FeatureFlag extends ImmutableObject implements Buildable { TimedTransitionProperty.withInitialValue(FeatureStatus.INACTIVE); public static Optional getUncached(FeatureName featureName) { - return tm().transact(() -> tm().loadByKeyIfPresent(createVKey(featureName))); + return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(featureName))); } public static ImmutableList getAllUncached() { diff --git a/core/src/main/java/google/registry/model/domain/DomainBase.java b/core/src/main/java/google/registry/model/domain/DomainBase.java index f3c367d4a..1f95249f0 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -579,7 +579,7 @@ public class DomainBase extends EppResource /** Loads and returns the fully qualified host names of all linked nameservers. */ public ImmutableSortedSet loadNameserverHostNames() { - return tm().transact( + return tm().reTransact( () -> tm().loadByKeys(getNameservers()).values().stream() .map(Host::getHostName) diff --git a/core/src/main/java/google/registry/model/registrar/RegistrarBase.java b/core/src/main/java/google/registry/model/registrar/RegistrarBase.java index d3f79233d..ea91755a4 100644 --- a/core/src/main/java/google/registry/model/registrar/RegistrarBase.java +++ b/core/src/main/java/google/registry/model/registrar/RegistrarBase.java @@ -1014,7 +1014,7 @@ public class RegistrarBase extends UpdateAutoTimestampEntity implements Buildabl /** Loads and returns a registrar entity by its id directly from the database. */ public static Optional loadByRegistrarId(String registrarId) { checkArgument(!Strings.isNullOrEmpty(registrarId), "registrarId must be specified"); - return tm().transact(() -> tm().loadByKeyIfPresent(createVKey(registrarId))); + return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(registrarId))); } /** diff --git a/core/src/main/java/google/registry/model/registrar/RegistrarPocBase.java b/core/src/main/java/google/registry/model/registrar/RegistrarPocBase.java index eece3884d..f1e881134 100644 --- a/core/src/main/java/google/registry/model/registrar/RegistrarPocBase.java +++ b/core/src/main/java/google/registry/model/registrar/RegistrarPocBase.java @@ -172,19 +172,16 @@ public class RegistrarPocBase extends ImmutableObject implements Jsonifiable, Un */ public static void updateContacts( final Registrar registrar, final ImmutableSet contacts) { - tm().transact( - () -> { - ImmutableSet emailAddressesToKeep = - contacts.stream().map(RegistrarPoc::getEmailAddress).collect(toImmutableSet()); - tm().query( - "DELETE FROM RegistrarPoc WHERE registrarId = :registrarId AND " - + "emailAddress NOT IN :emailAddressesToKeep") - .setParameter("registrarId", registrar.getRegistrarId()) - .setParameter("emailAddressesToKeep", emailAddressesToKeep) - .executeUpdate(); + ImmutableSet emailAddressesToKeep = + contacts.stream().map(RegistrarPoc::getEmailAddress).collect(toImmutableSet()); + tm().query( + "DELETE FROM RegistrarPoc WHERE registrarId = :registrarId AND " + + "emailAddress NOT IN :emailAddressesToKeep") + .setParameter("registrarId", registrar.getRegistrarId()) + .setParameter("emailAddressesToKeep", emailAddressesToKeep) + .executeUpdate(); - tm().putAll(contacts); - }); + tm().putAll(contacts); } public String getName() { diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java index 65bb01dd1..51d604f62 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java @@ -118,7 +118,7 @@ public class HistoryEntryDao { /** Loads all history objects from all time from the given registrars. */ public static Iterable loadHistoryObjectsByRegistrars( ImmutableCollection registrarIds) { - return tm().transact( + return tm().reTransact( () -> Streams.concat( loadHistoryObjectByRegistrarsInternal(ContactHistory.class, registrarIds), 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 a5420b010..52d62e3ba 100644 --- a/core/src/main/java/google/registry/model/tmch/TmchCrl.java +++ b/core/src/main/java/google/registry/model/tmch/TmchCrl.java @@ -39,7 +39,8 @@ public final class TmchCrl extends CrossTldSingleton { /** Returns the singleton instance of this entity, without memoization. */ public static Optional get() { - return tm().transact(() -> tm().loadSingleton(TmchCrl.class)); + // TODO(b/368069206): move transaction up to `TmchCertificateAuthority#updateCrl` + return tm().reTransact(() -> tm().loadSingleton(TmchCrl.class)); } /** diff --git a/core/src/test/java/google/registry/tools/RegistrarPocCommandTest.java b/core/src/test/java/google/registry/tools/RegistrarPocCommandTest.java index e09446aff..989d4bb3c 100644 --- a/core/src/test/java/google/registry/tools/RegistrarPocCommandTest.java +++ b/core/src/test/java/google/registry/tools/RegistrarPocCommandTest.java @@ -19,6 +19,7 @@ import static google.registry.model.registrar.RegistrarPocBase.Type.ABUSE; import static google.registry.model.registrar.RegistrarPocBase.Type.ADMIN; import static google.registry.model.registrar.RegistrarPocBase.Type.TECH; import static google.registry.model.registrar.RegistrarPocBase.Type.WHOIS; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistSimpleResource; @@ -49,16 +50,18 @@ class RegistrarPocCommandTest extends CommandTestCase { @Test void testList() throws Exception { Registrar registrar = loadRegistrar("NewRegistrar"); - RegistrarPoc.updateContacts( - registrar, - ImmutableSet.of( - new RegistrarPoc.Builder() - .setRegistrar(registrar) - .setName("John Doe") - .setEmailAddress("john.doe@example.com") - .setTypes(ImmutableSet.of(ADMIN)) - .setVisibleInWhoisAsAdmin(true) - .build())); + tm().transact( + () -> + RegistrarPoc.updateContacts( + registrar, + ImmutableSet.of( + new RegistrarPoc.Builder() + .setRegistrar(registrar) + .setName("John Doe") + .setEmailAddress("john.doe@example.com") + .setTypes(ImmutableSet.of(ADMIN)) + .setVisibleInWhoisAsAdmin(true) + .build()))); runCommandForced("--mode=LIST", "--output=" + output, "NewRegistrar"); assertThat(Files.readAllLines(Paths.get(output), UTF_8)) .containsExactly(