From c07c5fb2530a9f809532008b2f58f543e8e9a774 Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Tue, 3 Nov 2026 13:42:58 -0500 Subject: [PATCH] Use detach() instead of clear() in RegistryQuery Switch RegistryQuery typed and criteria queries to tm().query() and tm().criteriaQuery(), and replace entityManager.clear() with entityManager.detach(object) in RegistryQuery.detachObject now that HHH-14735 is fixed in Hibernate. --- .../registry/beam/common/RegistryQuery.java | 27 +++++---------- .../beam/common/RegistryJpaReadTest.java | 33 +++++++++++++++++-- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/google/registry/beam/common/RegistryQuery.java b/core/src/main/java/google/registry/beam/common/RegistryQuery.java index 1893830924e..fe820ae1a4a 100644 --- a/core/src/main/java/google/registry/beam/common/RegistryQuery.java +++ b/core/src/main/java/google/registry/beam/common/RegistryQuery.java @@ -57,10 +57,7 @@ static RegistryQuery createQuery( Query query = nativeQuery ? entityManager.createNativeQuery(sql) : entityManager.createQuery(sql); if (parameters != null) { - parameters.forEach( - (key, value) -> { - query.setParameter(key, value); - }); + parameters.forEach(query::setParameter); } JpaTransactionManager.setQueryFetchSize(query, QUERY_FETCH_SIZE); @SuppressWarnings("unchecked") @@ -78,14 +75,12 @@ static RegistryQuery createQuery( static RegistryQuery createQuery( String jpql, @Nullable Map parameters, Class clazz) { return () -> { - // TODO(b/193662898): switch to jpaTm().query() when it can properly detach loaded entities. - EntityManager entityManager = tm().getEntityManager(); - TypedQuery query = entityManager.createQuery(jpql, clazz); + TypedQuery query = tm().query(jpql, clazz); if (parameters != null) { parameters.forEach(query::setParameter); } JpaTransactionManager.setQueryFetchSize(query, QUERY_FETCH_SIZE); - return query.getResultStream().map(e -> detach(entityManager, e)); + return query.getResultStream(); }; } @@ -100,11 +95,9 @@ static RegistryQuery createQuery( */ static RegistryQuery createQuery(CriteriaQuerySupplier criteriaQuery) { return () -> { - // TODO(b/193662898): switch to jpaTm().query() when it can properly detach loaded entities. - EntityManager entityManager = tm().getEntityManager(); - TypedQuery query = entityManager.createQuery(criteriaQuery.get()); + TypedQuery query = tm().criteriaQuery(criteriaQuery.get()); JpaTransactionManager.setQueryFetchSize(query, QUERY_FETCH_SIZE); - return query.getResultStream().map(e -> detach(entityManager, e)); + return query.getResultStream(); }; } @@ -125,16 +118,12 @@ static T detach(EntityManager entityManager, T object) { return object; } - static void detachObject(EntityManager entityManager, Object object) { - Class objectClass = object.getClass(); - if (objectClass.isPrimitive() || objectClass == String.class) { + private static void detachObject(EntityManager entityManager, @Nullable Object object) { + if (object == null || object instanceof String) { return; } try { - // TODO(b/193662898): choose detach() or clear() based on the type of transaction. - // For context, EntityManager.detach() does not remove all metadata about loaded entities. - // See b/193925312 or https://hibernate.atlassian.net/browse/HHH-14735 for details. - entityManager.clear(); + entityManager.detach(object); } catch (IllegalArgumentException e) { // Not an entity. Do nothing. } diff --git a/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java b/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java index 281aa4ff608..2797d1ebcd1 100644 --- a/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java +++ b/core/src/test/java/google/registry/beam/common/RegistryJpaReadTest.java @@ -14,7 +14,9 @@ package google.registry.beam.common; +import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.JpaTransactionManagerExtension.makeRegistrar1; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.newHost; import static google.registry.testing.DatabaseHelper.newTld; import static google.registry.testing.DatabaseHelper.persistResource; @@ -37,13 +39,13 @@ import google.registry.model.eppcommon.AuthInfo.PasswordAuth; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.Host; -import google.registry.model.host.HostBase; import google.registry.model.registrar.Registrar; import google.registry.model.tld.Tld; import google.registry.persistence.transaction.CriteriaQueryBuilder; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.FakeClock; +import jakarta.persistence.EntityManager; import java.time.Instant; import org.apache.beam.sdk.coders.StringUtf8Coder; import org.apache.beam.sdk.testing.PAssert; @@ -91,7 +93,11 @@ void beforeEach() { void readWithCriteriaQuery() { Read read = RegistryJpaIO.read( - () -> CriteriaQueryBuilder.create(Host.class).build(), HostBase::getHostName) + () -> CriteriaQueryBuilder.create(Host.class).build(), + host -> { + assertThat(tm().getEntityManager().contains(host)).isFalse(); + return host.getHostName(); + }) .withCoder(StringUtf8Coder.of()); PCollection repoIds = testPipeline.apply(read); @@ -112,6 +118,7 @@ void readWithStringQuery() { false, (Object[] row) -> { Domain domain = (Domain) row[0]; + assertThat(tm().getEntityManager().contains(domain)).isFalse(); String emailAddress = (String) row[1]; return domain.getRepoId() + "-" + emailAddress; }) @@ -154,7 +161,10 @@ void readWithStringTypedQuery() { + " and d.deletionTime > CAST(now() AS timestamp)", ImmutableMap.of("type", Registrar.Type.REAL), Domain.class, - Domain::getRepoId) + domain -> { + assertThat(tm().getEntityManager().contains(domain)).isFalse(); + return domain.getRepoId(); + }) .withCoder(StringUtf8Coder.of()); PCollection repoIds = testPipeline.apply(read); @@ -162,6 +172,23 @@ void readWithStringTypedQuery() { testPipeline.run(); } + @Test + void detach_detachesOnlySpecifiedEntity() { + tm().transact( + () -> { + EntityManager entityManager = tm().getEntityManager(); + Host host0 = entityManager.find(Host.class, hosts.get(0).getRepoId()); + Host host1 = entityManager.find(Host.class, hosts.get(1).getRepoId()); + assertThat(entityManager.contains(host0)).isTrue(); + assertThat(entityManager.contains(host1)).isTrue(); + + RegistryQuery.detach(entityManager, host0); + + assertThat(entityManager.contains(host0)).isFalse(); + assertThat(entityManager.contains(host1)).isTrue(); + }); + } + private void setupForJoinQuery() { Tld registry = newTld("com", "ABCDAPP"); Registrar registrar =