From 04349d25dc737c5d04d8b0784c67d6497ef4fe19 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 4 Sep 2011 16:56:50 +0200 Subject: [PATCH] DATAJPA-86 - Pagination over queries using group-by works correctly. Added handling of count methods for queries using group by. In case the count query returns multiple results we use the number of results instead of failing. If the result contains one result we use this one. Moved hint handling to AbstractJpaQuery. --- .../repository/query/AbstractJpaQuery.java | 45 ++++++++++++++++--- .../repository/query/JpaQueryExecution.java | 11 ++--- .../data/jpa/repository/query/NamedQuery.java | 21 +++------ .../jpa/repository/query/ParameterBinder.java | 2 +- .../repository/query/PartTreeJpaQuery.java | 17 +++---- .../jpa/repository/query/SimpleJpaQuery.java | 40 +++-------------- .../jpa/repository/UserRepositoryTests.java | 12 +++++ .../query/JpaQueryExecutionUnitTests.java | 2 +- .../query/SimpleJpaQueryUnitTests.java | 19 ++------ .../jpa/repository/sample/UserRepository.java | 5 +++ 10 files changed, 89 insertions(+), 85 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 bee2a61e2..8b124a1f4 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 @@ -17,12 +17,13 @@ package org.springframework.data.jpa.repository.query; import javax.persistence.EntityManager; import javax.persistence.Query; +import javax.persistence.QueryHint; +import javax.persistence.TypedQuery; import org.springframework.data.jpa.repository.query.JpaQueryExecution.CollectionExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.ModifyingExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.PagedExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.SingleEntityExecution; -import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.util.Assert; @@ -58,7 +59,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { * org.springframework.data.repository.query.RepositoryQuery#getQueryMethod * () */ - public QueryMethod getQueryMethod() { + public JpaQueryMethod getQueryMethod() { return method; } @@ -106,12 +107,46 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { } } - protected ParameterBinder createBinder(Object[] values) { + /** + * Applies the declared query hints to the given query. + * + * @param query + * @return + */ + private T applyHints(T query, JpaQueryMethod method) { + for (QueryHint hint : method.getHints()) { + query.setHint(hint.name(), hint.value()); + } + + return query; + } + + protected ParameterBinder createBinder(Object[] values) { return new ParameterBinder(getQueryMethod().getParameters(), values); } - protected abstract Query createQuery(Object[] values); + protected Query createQuery(Object[] values) { + return applyHints(doCreateQuery(values), method); + } - protected abstract Query createCountQuery(Object[] values); + protected TypedQuery createCountQuery(Object[] values) { + return applyHints(doCreateCountQuery(values), method); + } + + /** + * Creates a {@link Query} instance for the given values. + * + * @param values must not be {@literal null}. + * @return + */ + protected abstract Query doCreateQuery(Object[] values); + + /** + * Creates a {@link TypedQuery} for counting using the given values. + * + * @param values must not be {@literal null}. + * @return + */ + protected abstract TypedQuery doCreateCountQuery(Object[] values); } \ No newline at end of file diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java index 33d362483..3125e8430 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java @@ -15,9 +15,12 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.List; + import javax.persistence.EntityManager; import javax.persistence.NoResultException; import javax.persistence.Query; +import javax.persistence.TypedQuery; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -42,7 +45,6 @@ public abstract class JpaQueryExecution { * @param binder * @return */ - public Object execute(AbstractJpaQuery query, Object[] values) { Assert.notNull(query); @@ -71,7 +73,6 @@ public abstract class JpaQueryExecution { @Override protected Object doExecute(AbstractJpaQuery query, Object[] values) { - return query.createQuery(values).getResultList(); } } @@ -93,11 +94,11 @@ public abstract class JpaQueryExecution { protected Object doExecute(AbstractJpaQuery repositoryQuery, Object[] values) { // Execute query to compute total - Query projection = repositoryQuery.createCountQuery(values); - Long total = (Long) projection.getSingleResult(); + TypedQuery projection = repositoryQuery.createCountQuery(values); + List totals = projection.getResultList(); + Long total = totals.size() == 1 ? totals.get(0) : totals.size(); Query query = repositoryQuery.createQuery(values); - ParameterAccessor accessor = new ParametersParameterAccessor(parameters, values); return new PageImpl(query.getResultList(), accessor.getPageable(), total); 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 f327074e0..5a8269efa 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 @@ -17,6 +17,7 @@ package org.springframework.data.jpa.repository.query; import javax.persistence.EntityManager; import javax.persistence.Query; +import javax.persistence.TypedQuery; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -101,15 +102,10 @@ final class NamedQuery extends AbstractJpaQuery { /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.query.AbstractStringBasedJpaQuery - * # - * createQuery(org.springframework.data.jpa.repository.query.ParameterBinder - * ) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#doCreateQuery(java.lang.Object[]) */ @Override - protected Query createQuery(Object[] values) { + protected Query doCreateQuery(Object[] values) { Query query = getEntityManager().createNamedQuery(queryName); return createBinder(values).bindAndPrepare(query); @@ -117,18 +113,15 @@ final class NamedQuery extends AbstractJpaQuery { /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.query.AbstractStringBasedJpaQuery - * #createCountQuery(org.springframework.data.jpa.repository.query. - * ParameterBinder) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#doCreateCountQuery(java.lang.Object[]) */ @Override - protected Query createCountQuery(Object[] values) { + protected TypedQuery doCreateCountQuery(Object[] values) { Query query = createQuery(values); String queryString = extractor.extractQueryString(query); - return createBinder(values).bind(getEntityManager().createQuery(QueryUtils.createCountQueryFor(queryString))); + return createBinder(values).bind( + getEntityManager().createQuery(QueryUtils.createCountQueryFor(queryString), Long.class)); } } \ No newline at end of file diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java index b8b7154e5..b6d473ffd 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java @@ -95,7 +95,7 @@ public class ParameterBinder { * @param query * @return */ - public Query bind(Query query) { + public T bind(T query) { int methodParameterPosition = 0; int queryParameterPosition = 1; diff --git a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java index 7da5d921d..bbcea7ab4 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java @@ -62,28 +62,23 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.query.AbstractJpaQuery#createQuery - * (javax.persistence.EntityManager, - * org.springframework.data.jpa.repository.query.ParameterBinder) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#doCreateQuery(java.lang.Object[]) */ @Override - public Query createQuery(Object[] values) { + public Query doCreateQuery(Object[] values) { return query.createQuery(values); } /* * (non-Javadoc) - * - * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery# - * createCountQuery(javax.persistence.EntityManager) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#doCreateCountQuery(java.lang.Object[]) */ @Override - public Query createCountQuery(Object[] values) { + @SuppressWarnings("unchecked") + public TypedQuery doCreateCountQuery(Object[] values) { - return countQuery.createQuery(values); + return (TypedQuery) countQuery.createQuery(values); } /** diff --git a/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java index 1157eb7d6..f44ea9918 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java @@ -15,11 +15,9 @@ */ package org.springframework.data.jpa.repository.query; -import java.util.List; - import javax.persistence.EntityManager; import javax.persistence.Query; -import javax.persistence.QueryHint; +import javax.persistence.TypedQuery; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,7 +40,6 @@ final class SimpleJpaQuery extends AbstractJpaQuery { private final String queryString; private final String countQuery; private final String alias; - private final List hints; private final Parameters parameters; /** @@ -53,7 +50,6 @@ final class SimpleJpaQuery extends AbstractJpaQuery { super(method, em); this.queryString = queryString; this.alias = QueryUtils.detectAlias(queryString); - this.hints = method.getHints(); this.parameters = method.getParameters(); this.countQuery = method.getCountQuery() == null ? QueryUtils.createCountQueryFor(queryString) : method .getCountQuery(); @@ -64,48 +60,26 @@ final class SimpleJpaQuery extends AbstractJpaQuery { /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.query.AbstractStringBasedJpaQuery - * # - * createQuery(org.springframework.data.jpa.repository.query.ParameterBinder - * ) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#createQuery(java.lang.Object[]) */ @Override - public Query createQuery(Object[] values) { + public Query doCreateQuery(Object[] values) { ParameterAccessor accessor = new ParametersParameterAccessor(parameters, values); String sortedQueryString = QueryUtils.applySorting(queryString, accessor.getSort(), alias); Query query = getEntityManager().createQuery(sortedQueryString); - return createBinder(values).bindAndPrepare(applyHints(query)); + return createBinder(values).bindAndPrepare(query); } /* * (non-Javadoc) - * - * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery# - * createCountQuery(java.lang.Object[]) + * @see org.springframework.data.jpa.repository.query.AbstractJpaQuery#doCreateCountQuery(java.lang.Object[]) */ @Override - protected Query createCountQuery(Object[] values) { + protected TypedQuery doCreateCountQuery(Object[] values) { - return createBinder(values).bind(applyHints(getEntityManager().createQuery(countQuery))); - } - - /** - * Applies the declared query hints to the given query. - * - * @param query - * @return - */ - private Query applyHints(Query query) { - - for (QueryHint hint : hints) { - query.setHint(hint.name(), hint.value()); - } - - return query; + return createBinder(values).bind(getEntityManager().createQuery(countQuery, Long.class)); } /** 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 af579d7f9..580c86c2b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -725,6 +725,18 @@ public class UserRepositoryTests { assertThat(result, hasItem(firstUser)); } + /** + * @see DATADOC-86 + */ + @Test + public void readsPageWithGroupByClauseCorrectly() { + + flushTestUsers(); + + Page result = repository.findByLastnameGrouped(new PageRequest(0, 10)); + assertThat(result.getTotalPages(), is(1)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java index 7de43cc7d..df37a3d57 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java @@ -30,7 +30,7 @@ import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.jpa.repository.query.JpaQueryExecution.ModifyingExecution; /** - * Unit test for {@link QueryExecution}. + * Unit test for {@link JpaQueryExecution}. * * @author Oliver Gierke */ 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 87ad4b90c..6b6c5fee8 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 @@ -23,8 +23,7 @@ import static org.mockito.Mockito.*; import java.lang.reflect.Method; import javax.persistence.EntityManager; -import javax.persistence.Query; -import javax.persistence.QueryHint; +import javax.persistence.TypedQuery; import org.junit.Before; import org.junit.Test; @@ -34,7 +33,6 @@ import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; -import org.springframework.data.jpa.repository.QueryHints; import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.Parameters; @@ -54,31 +52,22 @@ public class SimpleJpaQueryUnitTests { @Mock QueryExtractor extractor; @Mock - Query query; + TypedQuery query; @Mock RepositoryMetadata metadata; @Mock ParameterBinder binder; @Before - @QueryHints(@QueryHint(name = "foo", value = "bar")) public void setUp() throws SecurityException, NoSuchMethodException { when(em.createQuery(anyString())).thenReturn(query); + when(em.createQuery(anyString(), eq(Long.class))).thenReturn(query); Method setUp = UserRepository.class.getMethod("findByLastname", String.class); method = new JpaQueryMethod(setUp, metadata, extractor); } - @Test - public void appliesHintsCorrectly() throws Exception { - - SimpleJpaQuery jpaQuery = new SimpleJpaQuery(method, em, "foobar"); - jpaQuery.createQuery(new Object[] { "gierke" }); - - verify(query).setHint("foo", "bar"); - } - @Test public void prefersDeclaredCountQueryOverCreatingOne() throws Exception { @@ -86,7 +75,7 @@ public class SimpleJpaQueryUnitTests { when(method.getCountQuery()).thenReturn("foo"); when(method.getParameters()).thenReturn( new Parameters(SimpleJpaQueryUnitTests.class.getMethod("prefersDeclaredCountQueryOverCreatingOne"))); - when(em.createQuery("foo")).thenReturn(query); + when(em.createQuery("foo", Long.class)).thenReturn(query); SimpleJpaQuery jpaQuery = new SimpleJpaQuery(method, em, "select u from User u"); 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 6dcd8d00b..77b1da59f 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 @@ -185,4 +185,9 @@ public interface UserRepository extends JpaRepository, JpaSpecifi List findByLastnameAndFirstnameAllIgnoringCase(String lastname, String firstname); + /** + * @see DATADOC-86 - Count execution for group by + */ + @Query("select u.lastname from User u group by u.lastname") + Page findByLastnameGrouped(Pageable pageable); }