diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java index 596052681..bace8c141 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java @@ -35,10 +35,14 @@ public interface JpaTransactionManager extends TransactionManager { TypedQuery query(String sqlString, Class resultClass); /** - * Creates a JPA SQL query for the given query string (which does not return results). + * Creates a JPA SQL query for the given query string. * *

This is a convenience method for the longer * jpaTm().getEntityManager().createQuery(...). + * + *

Note that while this method can legally be used for queries that return results, it + * should not be, as it does not correctly detach entities as must be done for nomulus model + * objects. */ Query query(String sqlString); diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index 573805fbe..db6b88516 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -23,6 +23,7 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static java.util.AbstractMap.SimpleEntry; import static java.util.stream.Collectors.joining; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -43,8 +44,11 @@ import google.registry.persistence.VKey; import google.registry.util.Clock; import google.registry.util.Retrier; import google.registry.util.SystemSleeper; +import java.lang.reflect.Array; import java.lang.reflect.Field; +import java.util.Calendar; import java.util.Collections; +import java.util.Date; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; @@ -58,8 +62,12 @@ import javax.annotation.Nullable; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.EntityTransaction; +import javax.persistence.FlushModeType; +import javax.persistence.LockModeType; +import javax.persistence.Parameter; import javax.persistence.PersistenceException; import javax.persistence.Query; +import javax.persistence.TemporalType; import javax.persistence.TypedQuery; import javax.persistence.metamodel.EntityType; import javax.persistence.metamodel.SingularAttribute; @@ -116,7 +124,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { @Override public TypedQuery query(String sqlString, Class resultClass) { - return getEntityManager().createQuery(sqlString, resultClass); + return new DetachingTypedQuery(getEntityManager().createQuery(sqlString, resultClass)); } @Override @@ -665,6 +673,33 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } } + @Nullable + private T detachIfEntity(@Nullable T object) { + if (object == null) { + return null; + } + + // Check if the object is an array, if so we'll want to recurse through the elements. + if (object.getClass().isArray()) { + for (int i = 0; i < Array.getLength(object); ++i) { + detachIfEntity(Array.get(object, i)); + } + return object; + } + + // Check to see if it is an entity (queries can return raw column values or counts, so this + // could be String, Long, ...). + try { + getEntityManager().getMetamodel().entity(object.getClass()); + } catch (IllegalArgumentException e) { + // The object is not an entity. Return without detaching. + return object; + } + + // At this point, object must be an entity. + return detach(object); + } + /** Detach the entity, suitable for use in Optional.map(). */ @Nullable private T detach(@Nullable T entity) { @@ -758,6 +793,207 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } } + /** + * Typed query wrapper that applies a transform to all result objects. + * + *

This is used to detach objects upon load. + */ + @VisibleForTesting + class DetachingTypedQuery implements TypedQuery { + + TypedQuery delegate; + + public DetachingTypedQuery(TypedQuery delegate) { + this.delegate = delegate; + } + + @Override + public List getResultList() { + return delegate + .getResultStream() + .map(JpaTransactionManagerImpl.this::detachIfEntity) + .collect(toImmutableList()); + } + + @Override + public Stream getResultStream() { + return delegate.getResultStream().map(JpaTransactionManagerImpl.this::detachIfEntity); + } + + @Override + public T getSingleResult() { + return detachIfEntity(delegate.getSingleResult()); + } + + @Override + public TypedQuery setMaxResults(int maxResults) { + delegate.setMaxResults(maxResults); + return this; + } + + @Override + public TypedQuery setFirstResult(int startPosition) { + delegate.setFirstResult(startPosition); + return this; + } + + @Override + public TypedQuery setHint(String hintName, Object value) { + delegate.setHint(hintName, value); + return this; + } + + @Override + public TypedQuery setParameter(Parameter param, U value) { + delegate.setParameter(param, value); + return this; + } + + @Override + public TypedQuery setParameter( + Parameter param, Calendar value, TemporalType temporalType) { + delegate.setParameter(param, value, temporalType); + return this; + } + + @Override + public TypedQuery setParameter( + Parameter param, Date value, TemporalType temporalType) { + delegate.setParameter(param, value, temporalType); + return this; + } + + @Override + public TypedQuery setParameter(String name, Object value) { + delegate.setParameter(name, value); + return this; + } + + @Override + public TypedQuery setParameter(String name, Calendar value, TemporalType temporalType) { + delegate.setParameter(name, value, temporalType); + return this; + } + + @Override + public TypedQuery setParameter(String name, Date value, TemporalType temporalType) { + delegate.setParameter(name, value, temporalType); + return this; + } + + @Override + public TypedQuery setParameter(int position, Object value) { + delegate.setParameter(position, value); + return this; + } + + @Override + public TypedQuery setParameter(int position, Calendar value, TemporalType temporalType) { + delegate.setParameter(position, value, temporalType); + return this; + } + + @Override + public TypedQuery setParameter(int position, Date value, TemporalType temporalType) { + delegate.setParameter(position, value, temporalType); + return this; + } + + @Override + public TypedQuery setFlushMode(FlushModeType flushMode) { + delegate.setFlushMode(flushMode); + return this; + } + + @Override + public TypedQuery setLockMode(LockModeType lockMode) { + delegate.setLockMode(lockMode); + return this; + } + + // Query interface + + @Override + public int executeUpdate() { + return delegate.executeUpdate(); + } + + @Override + public int getMaxResults() { + return delegate.getMaxResults(); + } + + @Override + public int getFirstResult() { + return delegate.getFirstResult(); + } + + @Override + public Map getHints() { + return delegate.getHints(); + } + + @Override + public Set> getParameters() { + return delegate.getParameters(); + } + + @Override + public Parameter getParameter(String name) { + return delegate.getParameter(name); + } + + @Override + public Parameter getParameter(String name, Class type) { + return delegate.getParameter(name, type); + } + + @Override + public Parameter getParameter(int position) { + return delegate.getParameter(position); + } + + @Override + public Parameter getParameter(int position, Class type) { + return delegate.getParameter(position, type); + } + + @Override + public boolean isBound(Parameter param) { + return delegate.isBound(param); + } + + @Override + public U getParameterValue(Parameter param) { + return delegate.getParameterValue(param); + } + + @Override + public Object getParameterValue(String name) { + return delegate.getParameterValue(name); + } + + @Override + public Object getParameterValue(int position) { + return delegate.getParameterValue(position); + } + + @Override + public FlushModeType getFlushMode() { + return delegate.getFlushMode(); + } + + @Override + public LockModeType getLockMode() { + return delegate.getLockMode(); + } + + @Override + public U unwrap(Class cls) { + return delegate.unwrap(cls); + } + } + private class JpaQueryComposerImpl extends QueryComposer { private static final int DEFAULT_FETCH_SIZE = 1000; diff --git a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java index 88432b17c..e119e9c70 100644 --- a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java @@ -588,6 +588,36 @@ class JpaTransactionManagerImplTest { .contains("Inserted/updated object reloaded: "); } + @Test + void query_detachesResults() { + jpaTm().transact(() -> jpaTm().insertAll(moreEntities)); + jpaTm() + .transact( + () -> + jpaTm().query("FROM TestEntity", TestEntity.class).getResultList().stream() + .forEach(e -> assertThat(jpaTm().getEntityManager().contains(e)).isFalse())); + jpaTm() + .transact( + () -> + jpaTm() + .query("FROM TestEntity", TestEntity.class) + .getResultStream() + .forEach(e -> assertThat(jpaTm().getEntityManager().contains(e)).isFalse())); + + jpaTm() + .transact( + () -> + assertThat( + jpaTm() + .getEntityManager() + .contains( + jpaTm() + .query( + "FROM TestEntity WHERE name = 'entity1'", TestEntity.class) + .getSingleResult())) + .isFalse()); + } + private void insertPerson(int age) { jpaTm() .getEntityManager()