Use the built-in replicaJpaTm() in RDAP (#1506)

* Use the built-in replicaJpaTm() in RDAP

This includes a test for the replica-simulating transaction manager and
removal of any replica-specific code in RDAP tests, because it's
unnecessary due to the existing tests.

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

@@ -18,7 +18,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 
 import com.google.common.collect.ImmutableList;
 import java.util.Collection;
-import javax.persistence.EntityManager;
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.CriteriaQuery;
 import javax.persistence.criteria.Expression;
@@ -42,12 +41,14 @@ public class CriteriaQueryBuilder<T> {
 
   private final CriteriaQuery<T> query;
   private final Root<?> root;
+  private final JpaTransactionManager jpaTm;
   private final ImmutableList.Builder<Predicate> predicates = new ImmutableList.Builder<>();
   private final ImmutableList.Builder<Order> orders = new ImmutableList.Builder<>();
 
-  private CriteriaQueryBuilder(CriteriaQuery<T> query, Root<?> root) {
+  private CriteriaQueryBuilder(CriteriaQuery<T> query, Root<?> root, JpaTransactionManager jpaTm) {
     this.query = query;
     this.root = root;
+    this.jpaTm = jpaTm;
   }
 
   /** Adds a WHERE clause to the query, given the specified operation, field, and value. */
@@ -75,18 +76,18 @@ public class CriteriaQueryBuilder<T> {
    */
   public <V> CriteriaQueryBuilder<T> whereFieldContains(String fieldName, Object value) {
     return where(
-        jpaTm().getEntityManager().getCriteriaBuilder().isMember(value, root.get(fieldName)));
+        jpaTm.getEntityManager().getCriteriaBuilder().isMember(value, root.get(fieldName)));
   }
 
   /** Orders the result by the given field ascending. */
   public CriteriaQueryBuilder<T> orderByAsc(String fieldName) {
-    orders.add(jpaTm().getEntityManager().getCriteriaBuilder().asc(root.get(fieldName)));
+    orders.add(jpaTm.getEntityManager().getCriteriaBuilder().asc(root.get(fieldName)));
     return this;
   }
 
   /** Orders the result by the given field descending. */
   public CriteriaQueryBuilder<T> orderByDesc(String fieldName) {
-    orders.add(jpaTm().getEntityManager().getCriteriaBuilder().desc(root.get(fieldName)));
+    orders.add(jpaTm.getEntityManager().getCriteriaBuilder().desc(root.get(fieldName)));
     return this;
   }
 
@@ -103,23 +104,24 @@ public class CriteriaQueryBuilder<T> {
 
   /** Creates a query builder that will SELECT from the given class. */
   public static <T> CriteriaQueryBuilder<T> create(Class<T> clazz) {
-    return create(jpaTm().getEntityManager(), clazz);
+    return create(jpaTm(), clazz);
   }
 
   /** Creates a query builder for the given entity manager. */
-  public static <T> CriteriaQueryBuilder<T> create(EntityManager em, Class<T> clazz) {
+  public static <T> CriteriaQueryBuilder<T> create(JpaTransactionManager jpaTm, Class<T> clazz) {
-    CriteriaQuery<T> query = em.getCriteriaBuilder().createQuery(clazz);
+    CriteriaQuery<T> query = jpaTm.getEntityManager().getCriteriaBuilder().createQuery(clazz);
     Root<T> root = query.from(clazz);
     query = query.select(root);
-    return new CriteriaQueryBuilder<>(query, root);
+    return new CriteriaQueryBuilder<>(query, root, jpaTm);
   }
 
   /** Creates a "count" query for the table for the class. */
-  public static <T> CriteriaQueryBuilder<Long> createCount(EntityManager em, Class<T> clazz) {
+  public static <T> CriteriaQueryBuilder<Long> createCount(
-    CriteriaBuilder builder = em.getCriteriaBuilder();
+      JpaTransactionManager jpaTm, Class<T> clazz) {
+    CriteriaBuilder builder = jpaTm.getEntityManager().getCriteriaBuilder();
     CriteriaQuery<Long> query = builder.createQuery(Long.class);
     Root<T> root = query.from(clazz);
     query = query.select(builder.count(root));
-    return new CriteriaQueryBuilder<>(query, root);
+    return new CriteriaQueryBuilder<>(query, root, jpaTm);
   }
 }

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

@@ -1131,7 +1131,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
 
     private TypedQuery<T> buildQuery() {
       CriteriaQueryBuilder<T> queryBuilder =
-          CriteriaQueryBuilder.create(getEntityManager(), entityClass);
+          CriteriaQueryBuilder.create(JpaTransactionManagerImpl.this, entityClass);
       return addCriteria(queryBuilder);
     }
 
@@ -1178,7 +1178,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
     @Override
     public long count() {
       CriteriaQueryBuilder<Long> queryBuilder =
-          CriteriaQueryBuilder.createCount(getEntityManager(), entityClass);
+          CriteriaQueryBuilder.createCount(JpaTransactionManagerImpl.this, entityClass);
       return addCriteria(queryBuilder).getSingleResult();
     }
 

core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java

@@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey;
 import static google.registry.model.ofy.ObjectifyService.auditedOfy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.request.Action.Method.GET;
 import static google.registry.request.Action.Method.HEAD;
@@ -37,10 +38,8 @@ import com.google.common.primitives.Booleans;
 import com.googlecode.objectify.cmd.Query;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.host.HostResource;
-import google.registry.persistence.PersistenceModule.ReadOnlyReplicaJpaTm;
 import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.CriteriaQueryBuilder;
-import google.registry.persistence.transaction.JpaTransactionManager;
 import google.registry.rdap.RdapJsonFormatter.OutputDataType;
 import google.registry.rdap.RdapMetrics.EndpointType;
 import google.registry.rdap.RdapMetrics.SearchType;
@@ -93,8 +92,6 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
   @Inject @Parameter("nsLdhName") Optional<String> nsLdhNameParam;
   @Inject @Parameter("nsIp") Optional<String> nsIpParam;
 
-  @Inject @ReadOnlyReplicaJpaTm JpaTransactionManager readOnlyJpaTm;
-
   @Inject
   public RdapDomainSearchAction() {
     super("domain search", EndpointType.DOMAINS);
@@ -228,12 +225,13 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
       resultSet = getMatchingResources(query, true, querySizeLimit);
     } else {
       resultSet =
-          readOnlyJpaTm.transact(
+          replicaJpaTm()
+              .transact(
                   () -> {
                     CriteriaBuilder criteriaBuilder =
-                    readOnlyJpaTm.getEntityManager().getCriteriaBuilder();
+                        replicaJpaTm().getEntityManager().getCriteriaBuilder();
                     CriteriaQueryBuilder<DomainBase> queryBuilder =
-                    CriteriaQueryBuilder.create(DomainBase.class)
+                        CriteriaQueryBuilder.create(replicaJpaTm(), DomainBase.class)
                             .where(
                                 "fullyQualifiedDomainName",
                                 criteriaBuilder::like,
@@ -274,7 +272,8 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
       resultSet = getMatchingResources(query, true, querySizeLimit);
     } else {
       resultSet =
-          readOnlyJpaTm.transact(
+          replicaJpaTm()
+              .transact(
                   () -> {
                     CriteriaQueryBuilder<DomainBase> builder =
                         queryItemsSql(
@@ -357,7 +356,8 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
           .map(VKey::from)
           .collect(toImmutableSet());
     } else {
-      return readOnlyJpaTm.transact(
+      return replicaJpaTm()
+          .transact(
               () -> {
                 CriteriaQueryBuilder<HostResource> builder =
                     queryItemsSql(
@@ -370,7 +370,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
                   builder =
                       builder.where(
                           "currentSponsorClientId",
-                      readOnlyJpaTm.getEntityManager().getCriteriaBuilder()::equal,
+                          replicaJpaTm().getEntityManager().getCriteriaBuilder()::equal,
                           desiredRegistrar.get());
                 }
                 return getMatchingResourcesSql(builder, true, maxNameserversInFirstStage)
@@ -512,10 +512,11 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
         parameters.put("desiredRegistrar", desiredRegistrar.get());
       }
       hostKeys =
-          readOnlyJpaTm.transact(
+          replicaJpaTm()
+              .transact(
                   () -> {
                     javax.persistence.Query query =
-                    readOnlyJpaTm
+                        replicaJpaTm()
                             .getEntityManager()
                             .createNativeQuery(queryBuilder.toString())
                             .setMaxResults(maxNameserversInFirstStage);
@@ -570,15 +571,16 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
         }
         stream.forEach(domainSetBuilder::add);
       } else {
-        readOnlyJpaTm.transact(
+        replicaJpaTm()
+            .transact(
                 () -> {
                   for (VKey<HostResource> hostKey : hostKeys) {
                     CriteriaQueryBuilder<DomainBase> queryBuilder =
-                    CriteriaQueryBuilder.create(DomainBase.class)
+                        CriteriaQueryBuilder.create(replicaJpaTm(), DomainBase.class)
                             .whereFieldContains("nsHosts", hostKey)
                             .orderByAsc("fullyQualifiedDomainName");
                     CriteriaBuilder criteriaBuilder =
-                    readOnlyJpaTm.getEntityManager().getCriteriaBuilder();
+                        replicaJpaTm().getEntityManager().getCriteriaBuilder();
                     if (!shouldIncludeDeleted()) {
                       queryBuilder =
                           queryBuilder.where(
@@ -591,7 +593,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase {
                               criteriaBuilder::greaterThan,
                               cursorString.get());
                     }
-                readOnlyJpaTm
+                    replicaJpaTm()
                         .criteriaQuery(queryBuilder.build())
                         .getResultStream()
                         .filter(this::isAuthorized)

core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java

@@ -15,7 +15,7 @@
 package google.registry.rdap;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
 import static google.registry.rdap.RdapUtils.getRegistrarByIanaIdentifier;
@@ -277,7 +277,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase {
           resultSet = getMatchingResources(query, false, rdapResultSetMaxSize + 1);
         } else {
           resultSet =
-              jpaTm()
+              replicaJpaTm()
                   .transact(
                       () -> {
                         CriteriaQueryBuilder<ContactResource> builder =
@@ -399,7 +399,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase {
                   querySizeLimit);
         } else {
           contactResultSet =
-              jpaTm()
+              replicaJpaTm()
                   .transact(
                       () ->
                           getMatchingResourcesSql(

core/src/main/java/google/registry/rdap/RdapNameserverSearchAction.java

@@ -15,7 +15,7 @@
 package google.registry.rdap;
 
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.request.Action.Method.GET;
 import static google.registry.request.Action.Method.HEAD;
@@ -233,7 +233,7 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase {
       return makeSearchResults(
           getMatchingResources(query, shouldIncludeDeleted(), querySizeLimit), CursorType.NAME);
     } else {
-      return jpaTm()
+      return replicaJpaTm()
           .transact(
               () -> {
                 CriteriaQueryBuilder<HostResource> queryBuilder =
@@ -290,11 +290,11 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase {
       }
       queryBuilder.append(" ORDER BY repo_id ASC");
       rdapResultSet =
-          jpaTm()
+          replicaJpaTm()
               .transact(
                   () -> {
                     javax.persistence.Query query =
-                        jpaTm()
+                        replicaJpaTm()
                             .getEntityManager()
                             .createNativeQuery(queryBuilder.toString(), HostResource.class)
                             .setMaxResults(querySizeLimit);

core/src/main/java/google/registry/rdap/RdapSearchActionBase.java

@@ -16,7 +16,7 @@ package google.registry.rdap;
 
 import static com.google.common.base.Charsets.UTF_8;
 import static google.registry.model.ofy.ObjectifyService.auditedOfy;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 
 import com.google.common.collect.ImmutableList;
@@ -193,16 +193,17 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
    */
   <T extends EppResource> RdapResultSet<T> getMatchingResourcesSql(
       CriteriaQueryBuilder<T> builder, boolean checkForVisibility, int querySizeLimit) {
-    jpaTm().assertInTransaction();
+    replicaJpaTm().assertInTransaction();
     Optional<String> desiredRegistrar = getDesiredRegistrar();
     if (desiredRegistrar.isPresent()) {
       builder =
           builder.where(
-              "currentSponsorClientId", jpaTm().getEntityManager().getCriteriaBuilder()::equal,
+              "currentSponsorClientId",
+              replicaJpaTm().getEntityManager().getCriteriaBuilder()::equal,
               desiredRegistrar.get());
     }
     List<T> queryResult =
-        jpaTm().criteriaQuery(builder.build()).setMaxResults(querySizeLimit).getResultList();
+        replicaJpaTm().criteriaQuery(builder.build()).setMaxResults(querySizeLimit).getResultList();
     if (checkForVisibility) {
       return filterResourcesByVisibility(queryResult, querySizeLimit);
     } else {
@@ -395,7 +396,7 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
       RdapSearchPattern partialStringQuery,
       Optional<String> cursorString,
       DeletedItemHandling deletedItemHandling) {
-    jpaTm().assertInTransaction();
+    replicaJpaTm().assertInTransaction();
     if (partialStringQuery.getInitialString().length()
         < RdapSearchPattern.MIN_INITIAL_STRING_LENGTH) {
       throw new UnprocessableEntityException(
@@ -403,8 +404,8 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
               "Initial search string must be at least %d characters",
               RdapSearchPattern.MIN_INITIAL_STRING_LENGTH));
     }
-    CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder();
+    CriteriaBuilder criteriaBuilder = replicaJpaTm().getEntityManager().getCriteriaBuilder();
-    CriteriaQueryBuilder<T> builder = CriteriaQueryBuilder.create(clazz);
+    CriteriaQueryBuilder<T> builder = CriteriaQueryBuilder.create(replicaJpaTm(), clazz);
     if (partialStringQuery.getHasWildcard()) {
       builder =
           builder.where(
@@ -493,9 +494,9 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
               "Initial search string must be at least %d characters",
               RdapSearchPattern.MIN_INITIAL_STRING_LENGTH));
     }
-    jpaTm().assertInTransaction();
+    replicaJpaTm().assertInTransaction();
-    CriteriaQueryBuilder<T> builder = CriteriaQueryBuilder.create(clazz);
+    CriteriaQueryBuilder<T> builder = CriteriaQueryBuilder.create(replicaJpaTm(), clazz);
-    CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder();
+    CriteriaBuilder criteriaBuilder = replicaJpaTm().getEntityManager().getCriteriaBuilder();
     builder = builder.where(filterField, criteriaBuilder::equal, queryString);
     if (cursorString.isPresent()) {
       if (cursorField.isPresent()) {
@@ -544,7 +545,7 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
       RdapSearchPattern partialStringQuery,
       Optional<String> cursorString,
       DeletedItemHandling deletedItemHandling) {
-    jpaTm().assertInTransaction();
+    replicaJpaTm().assertInTransaction();
     return queryItemsSql(clazz, "repoId", partialStringQuery, cursorString, deletedItemHandling);
   }
 
@@ -553,7 +554,9 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
     if (!Objects.equals(deletedItemHandling, DeletedItemHandling.INCLUDE)) {
       builder =
           builder.where(
-              "deletionTime", jpaTm().getEntityManager().getCriteriaBuilder()::equal, END_OF_TIME);
+              "deletionTime",
+              replicaJpaTm().getEntityManager().getCriteriaBuilder()::equal,
+              END_OF_TIME);
     }
     return builder;
   }

core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerExtensionTest.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.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
 import static google.registry.testing.DatabaseHelper.insertInDb;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -62,6 +63,17 @@ public class JpaTransactionManagerExtensionTest {
             });
   }
 
+  @Test
+  void testReplicaJpaTm() {
+    TestEntity testEntity = new TestEntity("foo", "bar");
+    assertThat(
+            assertThrows(
+                PersistenceException.class,
+                () -> replicaJpaTm().transact(() -> replicaJpaTm().put(testEntity))))
+        .hasMessageThat()
+        .isEqualTo("Error while committing the transaction");
+  }
+
   @Test
   void testExtraParameters() {
     // This test verifies that 1) withEntityClass() has registered TestEntity and 2) The table

core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java

@@ -15,7 +15,6 @@
 package google.registry.rdap;
 
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.rdap.RdapTestHelper.assertThat;
 import static google.registry.rdap.RdapTestHelper.parseJsonObject;
 import static google.registry.request.Action.Method.POST;
@@ -46,7 +45,6 @@ import google.registry.model.registrar.Registrar;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.tld.Registry;
 import google.registry.persistence.VKey;
-import google.registry.persistence.transaction.ReplicaSimulatingJpaTransactionManager;
 import google.registry.rdap.RdapMetrics.EndpointType;
 import google.registry.rdap.RdapMetrics.SearchType;
 import google.registry.rdap.RdapMetrics.WildcardType;
@@ -376,7 +374,6 @@ class RdapDomainSearchActionTest extends RdapSearchActionTestCase<RdapDomainSear
     action.nsIpParam = Optional.empty();
     action.cursorTokenParam = Optional.empty();
     action.requestPath = actionPath;
-    action.readOnlyJpaTm = jpaTm();
   }
 
   private JsonObject generateExpectedJsonForTwoDomainsNsReply() {
@@ -724,18 +721,6 @@ class RdapDomainSearchActionTest extends RdapSearchActionTestCase<RdapDomainSear
     verifyMetrics(SearchType.BY_DOMAIN_NAME, Optional.of(1L));
   }
 
-  @TestSqlOnly
-  void testDomainMatch_readOnlyReplica() {
-    login("evilregistrar");
-    rememberWildcardType("cat.lol");
-    action.readOnlyJpaTm = new ReplicaSimulatingJpaTransactionManager(jpaTm());
-    action.nameParam = Optional.of("cat.lol");
-    action.parameterMap = ImmutableListMultimap.of("name", "cat.lol");
-    action.run();
-    assertThat(response.getPayload()).contains("Yes Virginia <script>");
-    assertThat(response.getStatus()).isEqualTo(200);
-  }
-
   @TestOfyAndSql
   void testDomainMatch_foundWithUpperCase() {
     login("evilregistrar");