From 3ae2eac08e6b69b8f5455ac193d19dcc202574fb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 27 Feb 2018 10:20:27 +0100 Subject: [PATCH] DATAJPA-1273 - Disable usage of Tuple for projections when running on EclipseLink. As EclipseLink doesn't support the execution of Tuple-based queries currently [0], we now fall back to a plan object array based execution for projecting queries. This implies the need for developers to make sure the columns returned are declared in the order the accessor methods are defined in projection interfaces. We now register a dedicated QueryExecutionListener that reports all methods that need that extra care on application startup into INFO level. [0] https://bugs.eclipse.org/bugs/show_bug.cgi?id=289141 --- .../repository/query/AbstractJpaQuery.java | 7 ++ .../support/JpaRepositoryFactory.java | 65 +++++++++++++++++++ ...ctStringBasedJpaQueryIntegrationTests.java | 1 + .../repository/query/NamedQueryUnitTests.java | 1 + .../query/SimpleJpaQueryUnitTests.java | 1 + 5 files changed, 75 insertions(+) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 31476f6e2..a5e6ac9fa 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -33,6 +33,7 @@ import javax.persistence.TupleElement; import javax.persistence.TypedQuery; import org.springframework.core.convert.converter.Converter; +import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.query.JpaQueryExecution.CollectionExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.ModifyingExecution; @@ -66,6 +67,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { private final JpaQueryMethod method; private final EntityManager em; private final JpaMetamodel metamodel; + private final PersistenceProvider provider; Lazy parameterBinder = new Lazy<>(this::createBinder); @@ -83,6 +85,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { this.method = method; this.em = em; this.metamodel = new JpaMetamodel(em.getMetamodel()); + this.provider = PersistenceProvider.fromEntityManager(em); } /* @@ -243,6 +246,10 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { */ protected Optional> getTypeToRead() { + if (PersistenceProvider.ECLIPSELINK.equals(provider)) { + return Optional.empty(); + } + ResultProcessor resultFactory = getQueryMethod().getResultProcessor(); ReturnedType returnedType = resultFactory.getReturnedType(); diff --git a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java index 0c25eab04..d270ae9d0 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java @@ -17,10 +17,13 @@ package org.springframework.data.jpa.repository.support; import static org.springframework.data.querydsl.QuerydslUtils.*; +import lombok.extern.slf4j.Slf4j; + import java.io.Serializable; import java.util.Optional; import javax.persistence.EntityManager; +import javax.persistence.Tuple; import org.springframework.beans.factory.BeanFactory; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -28,19 +31,24 @@ import org.springframework.data.jpa.projection.CollectionAwareProjectionFactory; import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.provider.QueryExtractor; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.query.AbstractJpaQuery; import org.springframework.data.jpa.repository.query.JpaQueryLookupStrategy; +import org.springframework.data.jpa.repository.query.JpaQueryMethod; +import org.springframework.data.jpa.util.JpaMetamodel; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.querydsl.QuerydslPredicateExecutor; import org.springframework.data.querydsl.SimpleEntityPathResolver; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.core.support.QueryCreationListener; import org.springframework.data.repository.core.support.RepositoryComposition; import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.core.support.RepositoryFragment; import org.springframework.data.repository.query.EvaluationContextProvider; import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryLookupStrategy.Key; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -76,6 +84,10 @@ public class JpaRepositoryFactory extends RepositoryFactorySupport { this.entityPathResolver = SimpleEntityPathResolver.INSTANCE; addRepositoryProxyPostProcessor(crudMethodMetadataPostProcessor); + + if (extractor.equals(PersistenceProvider.ECLIPSELINK)) { + addQueryCreationListener(new EclipseLinkProjectionQueryCreationListener(entityManager)); + } } /* @@ -205,4 +217,57 @@ public class JpaRepositoryFactory extends RepositoryFactorySupport { return fragments; } + + /** + * Query creation listener that informs EclipseLink users that they have to be extra careful when defining repository + * query methods using projections as we have to rely on the declaration order of the accessors in projection + * interfaces matching the order in columns. Alias-based mapping doesn't work with EclipseLink as it doesn't support + * {@link Tuple} based queries yet. + * + * @author Oliver Gierke + * @since 2.0.5 + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=289141 + */ + @Slf4j + private static class EclipseLinkProjectionQueryCreationListener implements QueryCreationListener { + + private static final String ECLIPSELINK_PROJECTIONS = "Usage of Spring Data projections detected on persistence provider EclipseLink. Make sure the following query methods declare result columns in exactly the order the accessors are declared in the projecting interface or the order of parameters for DTOs:"; + + private final JpaMetamodel metamodel; + + private boolean warningLogged = false; + + /** + * Creates a new {@link EclipseLinkProjectionQueryCreationListener} for the given {@link EntityManager}. + * + * @param em must not be {@literal null}. + */ + public EclipseLinkProjectionQueryCreationListener(EntityManager em) { + + Assert.notNull(em, "EntityManager must not be null!"); + + this.metamodel = new JpaMetamodel(em.getMetamodel()); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.QueryCreationListener#onCreation(org.springframework.data.repository.query.RepositoryQuery) + */ + @Override + public void onCreation(AbstractJpaQuery query) { + + JpaQueryMethod queryMethod = query.getQueryMethod(); + ReturnedType type = queryMethod.getResultProcessor().getReturnedType(); + + if (type.isProjecting() && !metamodel.isJpaManaged(type.getReturnedType())) { + + if (!warningLogged) { + log.info(ECLIPSELINK_PROJECTIONS); + this.warningLogged = true; + } + + log.info(" - {}", queryMethod); + } + } + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryIntegrationTests.java index 63e358a87..9a78fc96d 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryIntegrationTests.java @@ -54,6 +54,7 @@ public class AbstractStringBasedJpaQueryIntegrationTests { public void createsNormalQueryForJpaManagedReturnTypes() throws Exception { EntityManager mock = mock(EntityManager.class); + when(mock.getDelegate()).thenReturn(mock); when(mock.getEntityManagerFactory()).thenReturn(em.getEntityManagerFactory()); when(mock.getMetamodel()).thenReturn(em.getMetamodel()); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java index b04ac8bf5..0d217c5ea 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java @@ -67,6 +67,7 @@ public class NamedQueryUnitTests { when(em.getMetamodel()).thenReturn(metamodel); when(em.getEntityManagerFactory()).thenReturn(emf); + when(em.getDelegate()).thenReturn(em); when(emf.createEntityManager()).thenReturn(em); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java index 7c311de12..06b3c1a1c 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java @@ -91,6 +91,7 @@ public class SimpleJpaQueryUnitTests { when(em.createQuery(anyString())).thenReturn(query); when(em.createQuery(anyString(), eq(Long.class))).thenReturn(typedQuery); when(em.getEntityManagerFactory()).thenReturn(emf); + when(em.getDelegate()).thenReturn(em); when(emf.createEntityManager()).thenReturn(em); when(metadata.getDomainType()).thenReturn((Class) User.class); when(metadata.getReturnedDomainClass(Mockito.any(Method.class))).thenReturn((Class) User.class);