Refactor some code related to the transaction manager (#2366)

Removed the deprecation mark as it is natural to expose methods related
to a transaction like getting the entity manager or checking if one is
in a transaction through the transaction manager interface.
This commit is contained in:
Lai Jiang
2024-03-14 14:37:44 +00:00
committed by GitHub
parent 9af006836c
commit b9cfa65546
5 changed files with 26 additions and 56 deletions
@@ -16,7 +16,6 @@ package google.registry.bsa;
import static com.google.common.base.Verify.verify;
import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_REPEATABLE_READ;
import static google.registry.persistence.transaction.JpaTransactionManagerImpl.isInTransaction;
import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@@ -36,18 +35,18 @@ public final class BsaTransactions {
@CanIgnoreReturnValue
public static <T> T bsaTransact(Callable<T> work) {
verify(!isInTransaction(), "May only be used for top-level transactions.");
verify(!tm().inTransaction(), "May only be used for top-level transactions.");
return tm().transact(TRANSACTION_REPEATABLE_READ, work);
}
public static void bsaTransact(ThrowingRunnable work) {
verify(!isInTransaction(), "May only be used for top-level transactions.");
verify(!tm().inTransaction(), "May only be used for top-level transactions.");
tm().transact(TRANSACTION_REPEATABLE_READ, work);
}
@CanIgnoreReturnValue
public static <T> T bsaQuery(Callable<T> work) {
verify(!isInTransaction(), "May only be used for top-level transactions.");
verify(!tm().inTransaction(), "May only be used for top-level transactions.");
// TRANSACTION_REPEATABLE_READ is default on replica.
return replicaTm().transact(work);
}
@@ -17,7 +17,7 @@ package google.registry.bsa.persistence;
import static com.google.common.base.Verify.verify;
import static google.registry.bsa.BsaStringUtils.DOMAIN_SPLITTER;
import static google.registry.bsa.BsaTransactions.bsaQuery;
import static google.registry.persistence.transaction.JpaTransactionManagerImpl.em;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -39,7 +39,7 @@ public final class Queries {
* JpaTransactionManagerImpl}.
*/
private static Object detach(Object obj) {
em().detach(obj);
tm().getEntityManager().detach(obj);
return obj;
}
@@ -49,7 +49,8 @@ public final class Queries {
return ImmutableList.copyOf(
bsaQuery(
() ->
em().createQuery(
tm().getEntityManager()
.createQuery(
"SELECT b.label FROM BsaLabel b WHERE b.label > :lastRead ORDER BY b.label",
String.class)
.setParameter("lastRead", lastRead.orElse(""))
@@ -60,7 +61,8 @@ public final class Queries {
static Stream<BsaUnblockableDomain> queryBsaUnblockableDomainByLabels(
ImmutableCollection<String> labels) {
return ((Stream<?>)
em().createQuery("FROM BsaUnblockableDomain WHERE label in (:labels)")
tm().getEntityManager()
.createQuery("FROM BsaUnblockableDomain WHERE label in (:labels)")
.setParameter("labels", labels)
.getResultStream())
.map(Queries::detach)
@@ -69,7 +71,8 @@ public final class Queries {
static Stream<BsaLabel> queryBsaLabelByLabels(ImmutableCollection<String> labels) {
return ((Stream<?>)
em().createQuery("FROM BsaLabel where label in (:labels)")
tm().getEntityManager()
.createQuery("FROM BsaLabel where label in (:labels)")
.setParameter("labels", labels)
.getResultStream())
.map(Queries::detach)
@@ -77,7 +80,8 @@ public final class Queries {
}
static int deleteBsaLabelByLabels(ImmutableCollection<String> labels) {
return em().createQuery("DELETE FROM BsaLabel where label IN (:deleted_labels)")
return tm().getEntityManager()
.createQuery("DELETE FROM BsaLabel where label IN (:deleted_labels)")
.setParameter("deleted_labels", labels)
.executeUpdate();
}
@@ -87,7 +91,8 @@ public final class Queries {
return ImmutableList.copyOf(
bsaQuery(
() ->
em().createQuery(
tm().getEntityManager()
.createQuery(
"FROM BsaUnblockableDomain d WHERE d.label > :label OR (d.label = :label"
+ " AND d.tld > :tld) ORDER BY d.tld, d.label ")
.setParameter("label", lastRead.map(d -> d.label).orElse(""))
@@ -111,13 +116,14 @@ public final class Queries {
"SELECT CONCAT(d.label, '.', d.tld) FROM \"BsaUnblockableDomain\" d "
+ "WHERE (d.label, d.tld) IN (%s)",
labelTldParis);
return ImmutableSet.copyOf(em().createNativeQuery(sql).getResultList());
return ImmutableSet.copyOf(tm().getEntityManager().createNativeQuery(sql).getResultList());
}
static ImmutableSet<String> queryNewlyCreatedDomains(
ImmutableCollection<String> tlds, DateTime minCreationTime, DateTime now) {
return ImmutableSet.copyOf(
em().createQuery(
tm().getEntityManager()
.createQuery(
"SELECT domainName FROM Domain WHERE creationTime >= :minCreationTime "
+ "AND deletionTime > :now "
+ "AND tld in (:tlds)",
@@ -36,12 +36,10 @@ public interface JpaTransactionManager extends TransactionManager {
*
* <p>The returned instance is closed when the current transaction completes.
*
* @deprecated Use the static {@link JpaTransactionManagerImpl#em} method for now. In current
* implementation the entity manager is obtained from a static {@code ThreadLocal} object that
* is set up by the outermost {@link #transact} call. As an instance method, this method gives
* the illusion that the call site has control over which database instance to use.
* <p>Note that in the current implementation the entity manager is obtained from a static {@code
* ThreadLocal} object that is set up by the outermost {@link #transact} call. Nested call sites
* have no control over which database instance to use.
*/
@Deprecated // See Javadoc above.
EntityManager getEntityManager();
/**
@@ -91,26 +91,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
private static final ThreadLocal<TransactionInfo> transactionInfo =
ThreadLocal.withInitial(TransactionInfo::new);
/** Returns true if inside a transaction; returns false otherwise. */
public static boolean isInTransaction() {
return transactionInfo.get().inTransaction;
}
/**
* Returns the {@link EntityManager} for the current database transaction.
*
* <p>This method must be call from inside a transaction.
*/
public static EntityManager em() {
EntityManager entityManager = transactionInfo.get().entityManager;
if (entityManager == null) {
throw new PersistenceException(
"No EntityManager has been initialized. getEntityManager() must be invoked in the scope"
+ " of a transaction");
}
return entityManager;
}
public JpaTransactionManagerImpl(EntityManagerFactory emf, Clock clock, boolean readOnly) {
this.emf = emf;
this.clock = clock;
@@ -131,16 +111,10 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
return emf.createEntityManager();
}
@Deprecated // See Javadoc of interface method.
@Override
public EntityManager getEntityManager() {
EntityManager entityManager = transactionInfo.get().entityManager;
if (entityManager == null) {
throw new PersistenceException(
"No EntityManager has been initialized. getEntityManager() must be invoked in the scope"
+ " of a transaction");
}
return entityManager;
assertInTransaction();
return transactionInfo.get().entityManager;
}
@Override
@@ -158,7 +132,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
return getEntityManager().createQuery(sqlString);
}
@Deprecated // See Javadoc of instance method.
@Override
public boolean inTransaction() {
return transactionInfo.get().inTransaction;
@@ -34,16 +34,10 @@ public interface TransactionManager {
/**
* Returns {@code true} if the caller is in a transaction.
*
* <p>Note that this function is kept for backward compatibility. We will review the use case
* later when adding the cloud sql implementation.
*
* @deprecated Use the static {@link JpaTransactionManagerImpl#isInTransaction()} method for now.
* In current implementation the entity manager is obtained from a static {@code ThreadLocal}
* object that is set up by the outermost {@link #transact} call. As an instance method, this
* method gives the illusion that the call site has control over which database instance to
* use.
* <p>Note that in the current implementation the entity manager is obtained from a static {@code
* ThreadLocal} object that is set up by the outermost {@link #transact} call. Nested call sites
* have no control over which database instance to use.
*/
@Deprecated // See Javadoc above.
boolean inTransaction();
/**