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 5ebcac88d..ada670440 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 @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -16,7 +16,6 @@ package org.springframework.data.jpa.repository.query; import java.util.Collection; -import java.util.Collections; import java.util.List; import javax.persistence.EntityManager; @@ -27,7 +26,6 @@ import javax.persistence.StoredProcedureQuery; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.domain.SliceImpl; @@ -35,6 +33,8 @@ import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; +import org.springframework.data.repository.support.PageableExecutionUtils; +import org.springframework.data.repository.support.PageableExecutionUtils.TotalSupplier; import org.springframework.data.util.CloseableIterator; import org.springframework.data.util.StreamUtils; import org.springframework.util.Assert; @@ -46,6 +46,7 @@ import org.springframework.util.Assert; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ public abstract class JpaQueryExecution { @@ -171,27 +172,20 @@ public abstract class JpaQueryExecution { @Override @SuppressWarnings("unchecked") - protected Object doExecute(AbstractJpaQuery repositoryQuery, Object[] values) { - - // Execute query to compute total - Query projection = repositoryQuery.createCountQuery(values); - - List totals = projection.getResultList(); - Long total = totals.size() == 1 ? CONVERSION_SERVICE.convert(totals.get(0), Long.class) : totals.size(); + protected Object doExecute(final AbstractJpaQuery repositoryQuery, final Object[] values) { ParameterAccessor accessor = new ParametersParameterAccessor(parameters, values); - Pageable pageable = accessor.getPageable(); - - if (total.equals(0L)) { - return new PageImpl(Collections.emptyList(), pageable, total); - } - Query query = repositoryQuery.createQuery(values); - List content = pageable == null || total > pageable.getOffset() ? query.getResultList() - : Collections.emptyList(); + return PageableExecutionUtils.getPage(query.getResultList(), accessor.getPageable(), new TotalSupplier() { - return new PageImpl(content, pageable, total); + @Override + public long get() { + + List totals = repositoryQuery.createCountQuery(values).getResultList(); + return (totals.size() == 1 ? CONVERSION_SERVICE.convert(totals.get(0), Long.class) : totals.size()); + } + }); } } diff --git a/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java index c7cc18d79..950caa32e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -16,7 +16,6 @@ package org.springframework.data.jpa.repository.support; import java.io.Serializable; -import java.util.Collections; import java.util.List; import java.util.Map.Entry; @@ -24,13 +23,14 @@ import javax.persistence.EntityManager; import javax.persistence.LockModeType; import org.springframework.data.domain.Page; -import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.querydsl.QSort; import org.springframework.data.querydsl.QueryDslPredicateExecutor; import org.springframework.data.querydsl.SimpleEntityPathResolver; +import org.springframework.data.repository.support.PageableExecutionUtils; +import org.springframework.data.repository.support.PageableExecutionUtils.TotalSupplier; import com.querydsl.core.types.EntityPath; import com.querydsl.core.types.OrderSpecifier; @@ -45,6 +45,7 @@ import com.querydsl.jpa.impl.AbstractJPAQuery; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ public class QueryDslJpaRepository extends SimpleJpaRepository implements QueryDslPredicateExecutor { @@ -136,13 +137,16 @@ public class QueryDslJpaRepository extends SimpleJpa @Override public Page findAll(Predicate predicate, Pageable pageable) { - JPQLQuery countQuery = createQuery(predicate); + final JPQLQuery countQuery = createQuery(predicate); JPQLQuery query = querydsl.applyPagination(pageable, createQuery(predicate).select(path)); - long total = countQuery.fetchCount(); - List content = pageable == null || total > pageable.getOffset() ? query.fetch() : Collections. emptyList(); + return PageableExecutionUtils.getPage(query.fetch(), pageable, new TotalSupplier() { - return new PageImpl(content, pageable, total); + @Override + public long get() { + return countQuery.fetchCount(); + } + }); } /* diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index b1e4c1958..6e4c7efb8 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -54,6 +54,8 @@ import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.query.Jpa21Utils; import org.springframework.data.jpa.repository.query.JpaEntityGraph; import org.springframework.data.jpa.repository.query.QueryUtils; +import org.springframework.data.repository.support.PageableExecutionUtils; +import org.springframework.data.repository.support.PageableExecutionUtils.TotalSupplier; import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; @@ -578,16 +580,19 @@ public class SimpleJpaRepository * @param pageable can be {@literal null}. * @return */ - protected Page readPage(TypedQuery query, Class domainClass, Pageable pageable, - Specification spec) { + protected Page readPage(TypedQuery query, final Class domainClass, Pageable pageable, + final Specification spec) { query.setFirstResult(pageable.getOffset()); query.setMaxResults(pageable.getPageSize()); - Long total = executeCountQuery(getCountQuery(spec, domainClass)); - List content = total > pageable.getOffset() ? query.getResultList() : Collections. emptyList(); + return PageableExecutionUtils.getPage(query.getResultList(), pageable, new TotalSupplier() { - return new PageImpl(content, pageable, total); + @Override + public long get() { + return executeCountQuery(getCountQuery(spec, domainClass)); + } + }); } /** 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 44a3d5798..da24758bd 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -21,8 +21,8 @@ import static org.junit.Assert.*; import static org.springframework.data.domain.Example.*; import static org.springframework.data.domain.ExampleMatcher.*; import static org.springframework.data.domain.Sort.Direction.*; -import static org.springframework.data.jpa.domain.Specifications.*; import static org.springframework.data.jpa.domain.Specifications.not; +import static org.springframework.data.jpa.domain.Specifications.*; import static org.springframework.data.jpa.domain.sample.UserSpecifications.*; import java.util.ArrayList; @@ -55,8 +55,7 @@ import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Example; import org.springframework.data.domain.ExampleMatcher; -import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher; -import org.springframework.data.domain.ExampleMatcher.StringMatcher; +import org.springframework.data.domain.ExampleMatcher.*; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -1653,6 +1652,40 @@ public class UserRepositoryTests { assertThat(secondPage.getContent(), hasItems(youngest3)); } + /** + * @see DATAJPA-912 + */ + @Test + public void pageableQueryReportsTotalFromResult() { + + flushTestUsers(); + + Page firstPage = repository.findAll(new PageRequest(0, 10)); + assertThat(firstPage.getContent(), hasSize(4)); + assertThat(firstPage.getTotalElements(), is(4L)); + + Page secondPage = repository.findAll(new PageRequest(1, 3)); + assertThat(secondPage.getContent(), hasSize(1)); + assertThat(secondPage.getTotalElements(), is(4L)); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pageableQueryReportsTotalFromCount() { + + flushTestUsers(); + + Page firstPage = repository.findAll(new PageRequest(0, 4)); + assertThat(firstPage.getContent(), hasSize(4)); + assertThat(firstPage.getTotalElements(), is(4L)); + + Page secondPage = repository.findAll(new PageRequest(10, 10)); + assertThat(secondPage.getContent(), hasSize(0)); + assertThat(secondPage.getTotalElements(), is(4L)); + } + /** * @see DATAJPA-506 */ 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 dc6864124..25f3da18e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-2016 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. @@ -21,6 +21,7 @@ import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import java.util.Arrays; +import java.util.Collections; import javax.persistence.EntityManager; import javax.persistence.Query; @@ -43,6 +44,7 @@ import org.springframework.data.repository.query.Parameters; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class JpaQueryExecutionUnitTests { @@ -115,9 +117,10 @@ public class JpaQueryExecutionUnitTests { /** * @see DATAJPA-124 + * @see DATAJPA-912 */ @Test - public void pagedExecutionDoesNotRetrieveObjectsForPageableOutOfRange() throws Exception { + public void pagedExecutionRetrievesObjectsForPageableOutOfRange() throws Exception { Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); when(jpaQuery.createCountQuery(Mockito.any(Object[].class))).thenReturn(countQuery); @@ -127,24 +130,94 @@ public class JpaQueryExecutionUnitTests { PagedExecution execution = new PagedExecution(parameters); execution.doExecute(jpaQuery, new Object[] { new PageRequest(2, 10) }); - verify(query, times(0)).getResultList(); + verify(query).getResultList(); + verify(countQuery).getResultList(); } /** * @see DATAJPA-477 + * @see DATAJPA-912 */ @Test - public void pagedExecutionShouldNotGenerateUnecessaryQueryIfCountReportedNoResults() throws Exception { + public void pagedExecutionShouldNotGenerateCountQueryIfQueryReportedNoResults() throws Exception { Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); - when(jpaQuery.createCountQuery(Mockito.any(Object[].class))).thenReturn(countQuery); - when(countQuery.getResultList()).thenReturn(Arrays.asList(0L)); + when(jpaQuery.createQuery(Mockito.any(Object[].class))).thenReturn(query); + when(query.getResultList()).thenReturn(Arrays.asList(0L)); PagedExecution execution = new PagedExecution(parameters); execution.doExecute(jpaQuery, new Object[] { new PageRequest(0, 10) }); - verify(query, times(0)).getResultList(); - verify(jpaQuery, times(0)).createQuery((Object[]) any()); + verify(countQuery, times(0)).getResultList(); + verify(jpaQuery, times(0)).createCountQuery((Object[]) any()); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pagedExecutionShouldUseCountFromResultIfOffsetIsZeroAndResultsWithinPageSize() throws Exception { + + Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); + when(jpaQuery.createQuery(Mockito.any(Object[].class))).thenReturn(query); + when(query.getResultList()).thenReturn(Arrays.asList(new Object(), new Object(), new Object(), new Object())); + + PagedExecution execution = new PagedExecution(parameters); + execution.doExecute(jpaQuery, new Object[] { new PageRequest(0, 10) }); + + verify(jpaQuery, times(0)).createCountQuery((Object[]) any()); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pagedExecutionShouldUseCountFromResultWithOffsetAndResultsWithinPageSize() throws Exception { + + Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); + when(jpaQuery.createQuery(Mockito.any(Object[].class))).thenReturn(query); + when(query.getResultList()).thenReturn(Arrays.asList(new Object(), new Object(), new Object(), new Object())); + + PagedExecution execution = new PagedExecution(parameters); + execution.doExecute(jpaQuery, new Object[] { new PageRequest(5, 10) }); + + verify(jpaQuery, times(0)).createCountQuery((Object[]) any()); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pagedExecutionShouldUseRequestCountFromResultWithOffsetAndResultsHitLowerPageSizeBounds() throws Exception { + + Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); + when(jpaQuery.createQuery(Mockito.any(Object[].class))).thenReturn(query); + when(query.getResultList()).thenReturn(Collections.emptyList()); + when(jpaQuery.createCountQuery(Mockito.any(Object[].class))).thenReturn(query); + when(countQuery.getResultList()).thenReturn(Arrays.asList(20L)); + + PagedExecution execution = new PagedExecution(parameters); + execution.doExecute(jpaQuery, new Object[] { new PageRequest(4, 4) }); + + verify(jpaQuery).createCountQuery((Object[]) any()); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pagedExecutionShouldUseRequestCountFromResultWithOffsetAndResultsHitUpperPageSizeBounds() throws Exception { + + Parameters parameters = new DefaultParameters(getClass().getMethod("sampleMethod", Pageable.class)); + when(jpaQuery.createQuery(Mockito.any(Object[].class))).thenReturn(query); + when(query.getResultList()).thenReturn(Arrays.asList(new Object(), new Object(), new Object(), new Object())); + when(jpaQuery.createCountQuery(Mockito.any(Object[].class))).thenReturn(query); + when(countQuery.getResultList()).thenReturn(Arrays.asList(20L)); + + PagedExecution execution = new PagedExecution(parameters); + execution.doExecute(jpaQuery, new Object[] { new PageRequest(4, 4) }); + + verify(jpaQuery).createCountQuery((Object[]) any()); } public static void sampleMethod(Pageable pageable) { diff --git a/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java index 4c8fd455d..63f06c56b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -53,6 +53,7 @@ import com.querydsl.core.types.dsl.PathBuilderFactory; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration({ "classpath:infrastructure.xml" }) @@ -359,4 +360,34 @@ public class QueryDslJpaRepositoryTests { public void worksWithNullPageable() { assertThat(repository.findAll(user.dateOfBirth.isNull(), (Pageable) null).getContent(), hasSize(3)); } + + /** + * @see DATAJPA-912 + */ + @Test + public void pageableQueryReportsTotalFromResult() { + + Page firstPage = repository.findAll(user.dateOfBirth.isNull(), new PageRequest(0, 10)); + assertThat(firstPage.getContent(), hasSize(3)); + assertThat(firstPage.getTotalElements(), is(3L)); + + Page secondPage = repository.findAll(user.dateOfBirth.isNull(), new PageRequest(1, 2)); + assertThat(secondPage.getContent(), hasSize(1)); + assertThat(secondPage.getTotalElements(), is(3L)); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void pageableQueryReportsTotalFromCount() { + + Page firstPage = repository.findAll(user.dateOfBirth.isNull(), new PageRequest(0, 3)); + assertThat(firstPage.getContent(), hasSize(3)); + assertThat(firstPage.getTotalElements(), is(3L)); + + Page secondPage = repository.findAll(user.dateOfBirth.isNull(), new PageRequest(10, 10)); + assertThat(secondPage.getContent(), hasSize(0)); + assertThat(secondPage.getTotalElements(), is(3L)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java index 2536fb363..b4924f570 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2015 the original author or authors. + * Copyright 2011-2016 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. @@ -15,12 +15,11 @@ */ package org.springframework.data.jpa.repository.support; -import static java.util.Collections.singletonMap; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static java.util.Collections.*; +import static org.mockito.Mockito.*; import java.io.Serializable; +import java.util.Arrays; import javax.persistence.EntityGraph; import javax.persistence.EntityManager; @@ -44,6 +43,7 @@ import org.springframework.data.repository.CrudRepository; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class SimpleJpaRepositoryUnitTests { @@ -81,14 +81,41 @@ public class SimpleJpaRepositoryUnitTests { /** * @see DATAJPA-124 + * @see DATAJPA-912 */ @Test - public void doesNotActuallyRetrieveObjectsForPageableOutOfRange() { + public void retrieveObjectsForPageableOutOfRange() { when(countQuery.getSingleResult()).thenReturn(20L); repo.findAll(new PageRequest(2, 10)); - verify(query, times(0)).getResultList(); + verify(query).getResultList(); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void doesNotRetrieveCountWithoutOffsetAndResultsWithinPageSize() { + + when(query.getResultList()).thenReturn(Arrays.asList(new User(), new User())); + + repo.findAll(new PageRequest(0, 10)); + + verify(countQuery, never()).getSingleResult(); + } + + /** + * @see DATAJPA-912 + */ + @Test + public void doesNotRetrieveCountWithOffsetAndResultsWithinPageSize() { + + when(query.getResultList()).thenReturn(Arrays.asList(new User(), new User())); + + repo.findAll(new PageRequest(2, 10)); + + verify(countQuery, never()).getSingleResult(); } /** @@ -106,7 +133,7 @@ public class SimpleJpaRepositoryUnitTests { */ @Test @SuppressWarnings({ "rawtypes", "unchecked" }) - public void shouldPropagateConfiguredEntityGraphToFindOne() throws Exception{ + public void shouldPropagateConfiguredEntityGraphToFindOne() throws Exception { String entityGraphName = "User.detail"; when(entityGraphAnnotation.value()).thenReturn(entityGraphName); @@ -115,7 +142,7 @@ public class SimpleJpaRepositoryUnitTests { when(em.getEntityGraph(entityGraphName)).thenReturn((EntityGraph) entityGraph); when(information.getEntityName()).thenReturn("User"); when(metadata.getMethod()).thenReturn(CrudRepository.class.getMethod("findOne", Serializable.class)); - + Integer id = 0; repo.findOne(id);