From edf63705c93dda4a24191885e9d25638995f3c20 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 29 Jun 2016 09:38:49 +0200 Subject: [PATCH] DATAJPA-912 - Optimize paged query execution. We execute paged queries now in an optimized way. The data is obtained for each paged execution but the count query is deferred. We determine the total from the pageable and the results in which we don't hit the page size bounds (i.e. results are less than a full page without offset or results are greater 0 and less than a full page with offset). In all other cases we issue an additional count query. --- .../repository/query/JpaQueryExecution.java | 32 +++---- .../support/QueryDslJpaRepository.java | 18 ++-- .../support/SimpleJpaRepository.java | 15 ++-- .../jpa/repository/UserRepositoryTests.java | 39 +++++++- .../query/JpaQueryExecutionUnitTests.java | 89 +++++++++++++++++-- .../support/QueryDslJpaRepositoryTests.java | 33 ++++++- .../support/SimpleJpaRepositoryUnitTests.java | 45 ++++++++-- 7 files changed, 219 insertions(+), 52 deletions(-) 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);