From 4f0429a70476b14b6d35b185e76afc6502d618e4 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 26 Feb 2018 15:46:16 +0100 Subject: [PATCH] DATAJPA-1273 - Improve projected named query execution to use Tuples. NamedQuery now inspects the ReturnedType of the query method to potentially create a JPA Query instance typed to Tuple for projections onto non JPA managed types. This will cause Tuple instances being returned that allow binding to projection accessors by the aliases defined in the query instead of a plain Object array that will rely on the declaration order of the columns to be returned being in sync with the declaration order of the accessors in the projection interface. Disabled integration tests for EclipseLink as it apparently doesn't support Tuple as result type properly. Added a standalone test case to showcase the issue. --- .../repository/query/AbstractJpaQuery.java | 20 ++++++++++++ .../query/AbstractStringBasedJpaQuery.java | 10 ++---- .../data/jpa/repository/query/NamedQuery.java | 7 ++-- .../jpa/repository/query/NativeJpaQuery.java | 3 -- .../data/jpa/domain/sample/User.java | 9 +++++- .../EclipseLinkMetamodelIntegrationTests.java | 8 +++++ .../MetamodelIntegrationTests.java | 32 ++++++++++++++++++- .../OpenJpaMetamodelIntegrationTests.java | 7 +++- ...lipseLinkNamespaceUserRepositoryTests.java | 7 ++++ .../jpa/repository/UserRepositoryTests.java | 20 +++++++++++- .../jpa/repository/sample/UserRepository.java | 3 ++ 11 files changed, 109 insertions(+), 17 deletions(-) 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 8c58114e1..c00e84414 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 @@ -32,6 +32,7 @@ import javax.persistence.TupleElement; import javax.persistence.TypedQuery; import org.springframework.core.convert.converter.Converter; +import org.springframework.data.jpa.provider.HibernateUtils; 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; @@ -45,6 +46,7 @@ import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; +import org.springframework.data.util.Version; import org.springframework.util.Assert; /** @@ -57,6 +59,8 @@ import org.springframework.util.Assert; */ public abstract class AbstractJpaQuery implements RepositoryQuery { + protected static final Version HIBERNATE_VERSION_SUPPORTING_TUPLES = new Version(5, 2, 11); + private final JpaQueryMethod method; private final EntityManager em; private final JpaMetamodel metamodel; @@ -223,6 +227,22 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { return method.applyHintsToCountQuery() ? applyHints(countQuery, method) : countQuery; } + /** + * Returns the type to be used when creating the JPA query. + * + * @return + * @since 2.0.5 + */ + protected Class getTypeToRead() { + + ResultProcessor resultFactory = getQueryMethod().getResultProcessor(); + ReturnedType returnedType = resultFactory.getReturnedType(); + + return returnedType.isProjecting() && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) // + ? HibernateUtils.isVersionOrBetter(HIBERNATE_VERSION_SUPPORTING_TUPLES) ? Tuple.class : null // + : null; + } + /** * Creates a {@link Query} instance for the given values. * diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index a737193c6..94eba6a92 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -17,13 +17,10 @@ package org.springframework.data.jpa.repository.query; import javax.persistence.EntityManager; import javax.persistence.Query; -import javax.persistence.Tuple; import org.springframework.data.repository.query.EvaluationContextProvider; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.ParametersParameterAccessor; -import org.springframework.data.repository.query.ResultProcessor; -import org.springframework.data.repository.query.ReturnedType; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; @@ -134,11 +131,8 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { return em.createQuery(queryString); } - ResultProcessor resultFactory = getQueryMethod().getResultProcessor(); - ReturnedType returnedType = resultFactory.getReturnedType(); + Class typeToRead = getTypeToRead(); - return returnedType.isProjecting() && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) // - ? em.createQuery(queryString, Tuple.class) // - : em.createQuery(queryString); + return typeToRead == null ? em.createQuery(queryString) : em.createQuery(queryString, typeToRead); } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java index 230e52fa1..299ee2b37 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java @@ -21,7 +21,6 @@ import javax.persistence.TypedQuery; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.springframework.data.jpa.provider.QueryExtractor; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.QueryCreationException; @@ -139,7 +138,11 @@ final class NamedQuery extends AbstractJpaQuery { @Override protected Query doCreateQuery(Object[] values) { - Query query = getEntityManager().createNamedQuery(queryName); + EntityManager em = getEntityManager(); + + Class typeToRead = getTypeToRead(); + Query query = typeToRead == null ? em.createNamedQuery(queryName) : em.createNamedQuery(queryName, typeToRead); + return createBinder(values).bindAndPrepare(query); } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java index 4ed902638..d312ed970 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java @@ -25,7 +25,6 @@ import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; -import org.springframework.data.util.Version; import org.springframework.expression.spel.standard.SpelExpressionParser; /** @@ -38,8 +37,6 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; */ final class NativeJpaQuery extends AbstractStringBasedJpaQuery { - private static final Version HIBERNATE_VERSION_SUPPORTING_TUPLES = new Version(5, 2, 11); - private final Class resultType; /** diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/User.java b/src/test/java/org/springframework/data/jpa/domain/sample/User.java index 1fa4065fb..b5558d14d 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/User.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/User.java @@ -34,6 +34,9 @@ import javax.persistence.ManyToOne; import javax.persistence.NamedAttributeNode; import javax.persistence.NamedEntityGraph; import javax.persistence.NamedEntityGraphs; +import javax.persistence.NamedNativeQueries; +import javax.persistence.NamedNativeQuery; +import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; import javax.persistence.NamedStoredProcedureQueries; import javax.persistence.NamedStoredProcedureQuery; @@ -75,7 +78,11 @@ import javax.persistence.TemporalType; @NamedAttributeNode(value = "colleagues", subgraph = "User.colleaguesOfColleagues") }), @NamedSubgraph(name = "User.colleaguesOfColleagues", attributeNodes = { @NamedAttributeNode("roles"), }) }) }) -@NamedQuery(name = "User.findByEmailAddress", query = "SELECT u FROM User u WHERE u.emailAddress = ?1") +@NamedQueries({ // + @NamedQuery(name = "User.findByEmailAddress", // + query = "SELECT u FROM User u WHERE u.emailAddress = ?1"), // + @NamedQuery(name = "User.findByNamedQueryWithAliasInInvertedOrder", // + query = "SELECT u.lastname AS lastname, u.firstname AS firstname FROM User u ORDER BY u.lastname ASC") }) @NamedStoredProcedureQueries({ // @NamedStoredProcedureQuery(name = "User.plus1", procedureName = "plus1inout", parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class), diff --git a/src/test/java/org/springframework/data/jpa/infrastructure/EclipseLinkMetamodelIntegrationTests.java b/src/test/java/org/springframework/data/jpa/infrastructure/EclipseLinkMetamodelIntegrationTests.java index 44d03c1e0..4888198ce 100644 --- a/src/test/java/org/springframework/data/jpa/infrastructure/EclipseLinkMetamodelIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/infrastructure/EclipseLinkMetamodelIntegrationTests.java @@ -50,4 +50,12 @@ public class EclipseLinkMetamodelIntegrationTests extends MetamodelIntegrationTe @Ignore @Override public void doesNotExposeAliasForTupleIfNoneDefined() {} + + /** + * TODO: Remove, once https://bugs.eclipse.org/bugs/show_bug.cgi?id=289141 is fixed. + */ + @Test + @Ignore + @Override + public void returnsAliasesInTuple() {} } diff --git a/src/test/java/org/springframework/data/jpa/infrastructure/MetamodelIntegrationTests.java b/src/test/java/org/springframework/data/jpa/infrastructure/MetamodelIntegrationTests.java index ec4ddb9b7..ecc9c6c46 100644 --- a/src/test/java/org/springframework/data/jpa/infrastructure/MetamodelIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/infrastructure/MetamodelIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2013-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.data.jpa.infrastructure; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.util.ArrayList; import java.util.List; import javax.persistence.EntityManager; @@ -86,6 +87,7 @@ public abstract class MetamodelIntegrationTests { public void doesNotExposeAliasForTupleIfNoneDefined() { User user = new User(); + user.setFirstname("Dave"); user.setEmailAddress("email"); @@ -99,4 +101,32 @@ public abstract class MetamodelIntegrationTests { assertThat(elements, hasSize(1)); assertThat(elements.get(0).getAlias(), is(nullValue())); } + + @Test + @Transactional + public void returnsAliasesInTuple() { + + User user = new User(); + user.setFirstname("Dave"); + user.setLastname("Matthews"); + user.setEmailAddress("email"); + + em.persist(user); + + TypedQuery query = em.createQuery( + "SELECT u.lastname AS lastname, u.firstname AS firstname FROM User u ORDER BY u.lastname ASC", Tuple.class); + + List resultList = query.getResultList(); + List> elements = resultList.get(0).getElements(); + + assertThat(elements, hasSize(2)); + + List aliases = new ArrayList(elements.size()); + + for (TupleElement element : elements) { + aliases.add(element.getAlias()); + } + + assertThat(aliases, containsInAnyOrder("firstname", "lastname")); + } } diff --git a/src/test/java/org/springframework/data/jpa/infrastructure/OpenJpaMetamodelIntegrationTests.java b/src/test/java/org/springframework/data/jpa/infrastructure/OpenJpaMetamodelIntegrationTests.java index 739653ec6..6cb903593 100644 --- a/src/test/java/org/springframework/data/jpa/infrastructure/OpenJpaMetamodelIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/infrastructure/OpenJpaMetamodelIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,4 +39,9 @@ public class OpenJpaMetamodelIntegrationTests extends MetamodelIntegrationTests @Ignore @Override public void doesNotExposeAliasForTupleIfNoneDefined() {} + + @Ignore + @Test + @Override + public void returnsAliasesInTuple() {} } diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index 489618d65..8c56c78ff 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -98,4 +98,11 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi */ @Override public void supportsProjectionsWithNativeQueriesAndCamelCaseProperty() {} + + /** + * TODO: Remove, once https://bugs.eclipse.org/bugs/show_bug.cgi?id=289141 is fixed. + */ + @Override + @Test + public void bindsNativeQueryResultsToProjectionByName() {} } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 01bc9c92f..badf9a166 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2017 the original author or authors. + * Copyright 2008-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -2217,6 +2217,24 @@ public class UserRepositoryTests { assertThat(emailAddress, is(user.getEmailAddress())); } + @Test // DATAJPA-1273 + public void bindsNativeQueryResultsToProjectionByName() { + + Assume + .assumeTrue(getHibernateVersion().isGreaterThanOrEqualTo(HIBERNATE_VERSION_SUPPORTING_TUPLE_ON_NATIVE_QUERIES)); + + flushTestUsers(); + + List result = repository.findByNamedQueryWithAliasInInvertedOrder(); + + assertThat(result, is(not(empty()))); + + NameOnly element = result.get(0); + + assertThat(element.getFirstname(), is("Joachim")); + assertThat(element.getLastname(), is("Arrasz")); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index dfb7cd2ed..5c7e57579 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -486,6 +486,9 @@ public interface UserRepository EmailOnly findEmailOnlyByNativeQuery(Integer id); + // DATAJPA-1273 + List findByNamedQueryWithAliasInInvertedOrder(); + interface RolesAndFirstname { String getFirstname();