Deprecate methods taking a Query and a Pageable returning a Page.

Since both the Query and the Pageable have limit, offset and sorting information, these methods are confusing and the strategy to resolve the conflict is not clear.

Existing code using the methods got replaced.

Users of these methods should use the various findAll and findBy methods to construct Page objects as desired.

Closes #2138

Fixed corner case unpaged

Author: schauder

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java

@@ -230,7 +230,9 @@ public interface JdbcAggregateOperations {
 	 * @param pageable the pagination information. Must not be {@code null}.
 	 * @return Guaranteed to be not {@code null}.
 	 * @since 2.0
+	 * @deprecated use a combination of other methods of this class to construct results of type {@link Page}.
 	 */
+	@Deprecated(since = "4.0")
 	<T> Page<T> findAll(Class<T> domainType, Pageable pageable);
 
 	/**
@@ -275,7 +277,9 @@ public interface JdbcAggregateOperations {
 	 * @param pageable can be null.
 	 * @return a {@link Page} of entities matching the given {@link Example}.
 	 * @since 3.0
+	 * @deprecated use a combination of other methods of this class to construct results of type {@link Page}.
 	 */
+	@Deprecated(since = "4.0")
 	<T> Page<T> findAll(Query query, Class<T> domainType, Pageable pageable);
 
 	/**

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java

@@ -359,6 +359,7 @@ public <T> Stream<T> streamAll(Class<T> domainType, Sort sort) {
 		return allStreamable.map(this::triggerAfterConvert);
 	}
 
+	@Deprecated
 	@Override
 	public <T> Page<T> findAll(Class<T> domainType, Pageable pageable) {
 
@@ -388,6 +389,7 @@ public <T> Stream<T> streamAll(Query query, Class<T> domainType) {
 		return accessStrategy.streamAll(query, domainType).map(this::triggerAfterConvert);
 	}
 
+	@Deprecated
 	@Override
 	public <T> Page<T> findAll(Query query, Class<T> domainType, Pageable pageable) {
 

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/FetchableFluentQueryByExample.java

@@ -35,6 +35,7 @@
 import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.relational.repository.query.RelationalExampleMapper;
+import org.springframework.data.support.PageableExecutionUtils;
 import org.springframework.util.Assert;
 
 /**
@@ -126,8 +127,15 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
 	@Override
 	public Page<R> page(Pageable pageable) {
 
-		return this.entityOperations.findAll(createQuery(p -> p.with(pageable)), getExampleType(), pageable)
-				.map(item -> this.getConversionFunction().apply(item));
+		Query contentQuery = createQuery(p -> p.with(pageable));
+		List<S> content = this.entityOperations.findAll(contentQuery, getExampleType());
+
+		List<R> result = new ArrayList<>(content.size());
+		for (S s : content) {
+			result.add(getConversionFunction().apply(s));
+		}
+
+		return PageableExecutionUtils.getPage(result, pageable, () -> this.entityOperations.count(createQuery(), getExampleType()));
 	}
 
 	@Override

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/SimpleJdbcRepository.java

@@ -27,11 +27,14 @@
 import org.springframework.data.jdbc.core.convert.JdbcConverter;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
+import org.springframework.data.relational.core.query.CriteriaDefinition;
+import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.relational.repository.query.RelationalExampleMapper;
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.PagingAndSortingRepository;
 import org.springframework.data.repository.query.FluentQuery;
 import org.springframework.data.repository.query.QueryByExampleExecutor;
+import org.springframework.data.support.PageableExecutionUtils;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.Assert;
 
@@ -139,7 +142,13 @@ public List<T> findAll(Sort sort) {
 
 	@Override
 	public Page<T> findAll(Pageable pageable) {
-		return entityOperations.findAll(entity.getType(), pageable);
+
+		Assert.notNull(pageable, "Pageable must not be null");
+
+		Query query = applyPageable(Query.query(CriteriaDefinition.empty()), pageable);
+		List<T> content = entityOperations.findAll(query, entity.getType());
+
+		return PageableExecutionUtils.getPage(content, pageable, () -> entityOperations.count(entity.getType()));
 	}
 
 	@Override
@@ -172,9 +181,15 @@ public <S extends T> List<S> findAll(Example<S> example, Sort sort) {
 	public <S extends T> Page<S> findAll(Example<S> example, Pageable pageable) {
 
 		Assert.notNull(example, "Example must not be null");
+		Assert.notNull(pageable, "Pageable must not be null");
+
+		Query mappedQuery = this.exampleMapper.getMappedExample(example);
+		Query contentQuery = applyPageable(mappedQuery, pageable);
+
+		List<S> content = this.entityOperations.findAll(contentQuery, example.getProbeType());
 
-		return this.entityOperations.findAll(this.exampleMapper.getMappedExample(example), example.getProbeType(),
-				pageable);
+		return PageableExecutionUtils.getPage(content, pageable,
+				() -> this.entityOperations.count(mappedQuery, example.getProbeType()));
 	}
 
 	@Override
@@ -203,4 +218,13 @@ public <S extends T, R> R findBy(Example<S> example, Function<FluentQuery.Fetcha
 
 		return queryFunction.apply(fluentQuery);
 	}
+
+	private Query applyPageable(Query query, Pageable pageable) {
+
+		if (pageable.isPaged()) {
+			query = query.limit(pageable.getPageSize()).offset(pageable.getOffset());
+		}
+
+		return query.sort(pageable.getSort());
+	}
 }

spring-data-jdbc/src/main/kotlin/org/springframework/data/jdbc/core/JdbcAggregateOperationsExtensions.kt

@@ -80,6 +80,7 @@ inline fun <reified T : Any> JdbcAggregateOperations.findAll(sort: Sort): List<T
 /**
  * Extension for [JdbcAggregateOperations.findAll] with pagination.
  */
+@Deprecated("Use a combination of operations of this class to construct results of type Page")
 inline fun <reified T : Any> JdbcAggregateOperations.findAll(pageable: Pageable): Page<T> =
     findAll(T::class.java, pageable)
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java

@@ -119,14 +119,16 @@ public class JdbcRepositoryIntegrationTests {
 
 	public static Stream<Arguments> findAllByExamplePageableSource() {
 
+		// Pageable pageRequest, int size, int totalPages, List<String> notContains
 		return Stream.of( //
 				Arguments.of(PageRequest.of(0, 3), 3, 34, Arrays.asList("3", "4", "100")), //
 				Arguments.of(PageRequest.of(1, 10), 10, 10, Arrays.asList("9", "20", "30")), //
 				Arguments.of(PageRequest.of(2, 10), 10, 10, Arrays.asList("1", "2", "3")), //
 				Arguments.of(PageRequest.of(33, 3), 1, 34, Collections.emptyList()), //
 				Arguments.of(PageRequest.of(36, 3), 0, 34, Collections.emptyList()), //
 				Arguments.of(PageRequest.of(0, 10000), 100, 1, Collections.emptyList()), //
-				Arguments.of(PageRequest.of(100, 10000), 0, 1, Collections.emptyList()) //
+				Arguments.of(PageRequest.of(100, 10000), 0, 1, Collections.emptyList()), //
+				Arguments.of(Pageable.unpaged(), 100, 1, Collections.emptyList()) //
 		);
 	}
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java

@@ -51,6 +51,7 @@
 import org.springframework.data.relational.core.mapping.event.Identifier;
 import org.springframework.data.relational.core.mapping.event.RelationalEvent;
 import org.springframework.data.relational.core.mapping.event.WithId;
+import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.PagingAndSortingRepository;
 import org.springframework.jdbc.core.JdbcOperations;
@@ -264,7 +265,7 @@ void publishesEventsOnFindAllPaged() {
 		DummyEntity entity1 = new DummyEntity(42L);
 		DummyEntity entity2 = new DummyEntity(23L);
 
-		doReturn(asList(entity1, entity2)).when(dataAccessStrategy).findAll(any(), any(Pageable.class));
+		doReturn(asList(entity1, entity2)).when(dataAccessStrategy).findAll(any(Query.class), any(Class.class));
 		doReturn(2L).when(dataAccessStrategy).count(any());
 
 		repository.findAll(PageRequest.of(0, 20));