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.

core/src/main/java/google/registry/bsa/BsaTransactions.java

@@ -16,7 +16,6 @@
 
 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);
   }

core/src/main/java/google/registry/bsa/persistence/Queries.java

@@ -17,7 +17,7 @@
 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 @@ private Queries() {}
    * JpaTransactionManagerImpl}.
    */
   private static Object detach(Object obj) {
-    em().detach(obj);
+    tm().getEntityManager().detach(obj);
     return obj;
   }
 
@@ -49,7 +49,8 @@ public static ImmutableList<String> batchReadBsaLabelText(
     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 static ImmutableList<String> batchReadBsaLabelText(
   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,15 +71,17 @@ static Stream<BsaUnblockableDomain> queryBsaUnblockableDomainByLabels(
 
   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)
         .map(BsaLabel.class::cast);
   }
 
   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 @@ static ImmutableList<BsaUnblockableDomain> batchReadUnblockables(
     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 @@ static ImmutableSet<String> queryUnblockablesByNames(ImmutableSet<String> domain
             "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)",

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java

@@ -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();
 
   /**

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java

@@ -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 EntityManager getStandaloneEntityManager() {
     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 Query query(String sqlString) {
     return getEntityManager().createQuery(sqlString);
   }
 
-  @Deprecated // See Javadoc of instance method.
   @Override
   public boolean inTransaction() {
     return transactionInfo.get().inTransaction;

core/src/main/java/google/registry/persistence/transaction/TransactionManager.java

@@ -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();
 
   /**