Fix slice support.

Fix slice support. Also discontinue use of deprecated class.

Closes #1323.

Author: mikereiche

src/main/java/org/springframework/data/couchbase/repository/query/AbstractCouchbaseQuery.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2021 the original author or authors
+ * Copyright 2020-2022 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.
@@ -22,6 +22,7 @@
 import org.springframework.data.couchbase.core.query.Query;
 import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.DeleteExecution;
 import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.PagedExecution;
+import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.SlicedExecution;
 import org.springframework.data.repository.core.EntityMetadata;
 import org.springframework.data.repository.query.ParameterAccessor;
 import org.springframework.data.repository.query.ParametersParameterAccessor;
@@ -116,7 +117,7 @@ private CouchbaseQueryExecution getExecutionToWrap(ParameterAccessor accessor, E
 			return new DeleteExecution(getOperations(), getQueryMethod());
 		} else if (isTailable(getQueryMethod())) {
 			return (q, t, r, c) -> operation.as(r).matching(q.with(accessor.getPageable())).all(); // s/b tail() instead of
-																																															// all()
+			// all()
 		} else if (getQueryMethod().isCollectionQuery()) {
 			return (q, t, r, c) -> operation.as(r).matching(q.with(accessor.getPageable())).all();
 		} else if (getQueryMethod().isStreamQuery()) {
@@ -127,6 +128,8 @@ private CouchbaseQueryExecution getExecutionToWrap(ParameterAccessor accessor, E
 			return (q, t, r, c) -> operation.as(r).matching(q).exists();
 		} else if (getQueryMethod().isPageQuery()) {
 			return new PagedExecution(operation, accessor.getPageable());
+		} else if (getQueryMethod().isSliceQuery()) {
+			return new SlicedExecution(operation, accessor.getPageable());
 		} else {
 			return (q, t, r, c) -> {
 				TerminatingFindByQuery<?> find = operation.as(r).matching(q);

src/main/java/org/springframework/data/couchbase/repository/query/CouchbaseQueryExecution.java

@@ -22,11 +22,11 @@
 import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.ExecutableFindByQuery;
 import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.TerminatingFindByQuery;
 import org.springframework.data.couchbase.core.query.Query;
+import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
 import org.springframework.data.domain.SliceImpl;
 import org.springframework.data.repository.query.QueryMethod;
-import org.springframework.data.repository.support.PageableExecutionUtils;
 import org.springframework.util.Assert;
 
 /**
@@ -93,13 +93,13 @@ public Object execute(Query query, Class<?> type, Class<?> returnType, String co
 	 */
 	final class SlicedExecution implements CouchbaseQueryExecution {
 
-		private final ExecutableFindByQuery<?> find;
+		private final ExecutableFindByQuery<?> operation;
 		private final Pageable pageable;
 
-		public SlicedExecution(ExecutableFindByQuery find, Pageable pageable) {
-			Assert.notNull(find, "Find must not be null!");
+		public SlicedExecution(ExecutableFindByQuery operation, Pageable pageable) {
+			Assert.notNull(operation, "Find must not be null!");
 			Assert.notNull(pageable, "Pageable must not be null!");
-			this.find = find;
+			this.operation = operation;
 			this.pageable = pageable;
 		}
 
@@ -110,12 +110,14 @@ public SlicedExecution(ExecutableFindByQuery find, Pageable pageable) {
 		@Override
 		@SuppressWarnings({ "unchecked", "rawtypes" })
 		public Object execute(Query query, Class<?> type, Class<?> returnType, String collection) {
-			int pageSize = pageable.getPageSize();
-			// Apply Pageable but tweak limit to peek into next page
-			Query modifiedQuery = query.skip(pageable.getOffset()).limit(pageSize + 1);
-			List result = find.as(returnType).matching(modifiedQuery).all();
-			boolean hasNext = result.size() > pageSize;
-			return new SliceImpl<Object>(hasNext ? result.subList(0, pageSize) : result, pageable, hasNext);
+			int overallLimit = 0; // query.getLimit();
+			TerminatingFindByQuery<?> matching = operation.as(returnType).matching(query);
+			// Adjust limit if page would exceed the overall limit
+			if (overallLimit != 0 && pageable.getOffset() + pageable.getPageSize() > overallLimit) {
+				query.limit((int) (overallLimit - pageable.getOffset()));
+			}
+			List<?> results = matching.all();
+			return new SliceImpl(results, pageable, results != null && !results.isEmpty());
 		}
 	}
 
@@ -146,10 +148,13 @@ public Object execute(Query query, Class<?> type, Class<?> returnType, String co
 			if (overallLimit != 0 && pageable.getOffset() + pageable.getPageSize() > overallLimit) {
 				query.limit((int) (overallLimit - pageable.getOffset()));
 			}
-			return PageableExecutionUtils.getPage(matching.all(), pageable, () -> {
-				long count = operation.matching(query.skip(-1).limit(-1).withoutSort()).count();
-				return overallLimit != 0 ? Math.min(count, overallLimit) : count;
-			});
+
+			List<?> result = matching.all(); // this needs to be done before count, as count clears the skip and limit
+
+			long count = operation.matching(query.skip(-1).limit(-1).withoutSort()).count();
+			count = overallLimit != 0 ? Math.min(count, overallLimit) : count;
+
+			return new PageImpl(result, pageable, count);
 		}
 	}
 

src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java

@@ -37,7 +37,6 @@
 import org.springframework.data.couchbase.core.query.N1QLExpression;
 import org.springframework.data.couchbase.repository.Query;
 import org.springframework.data.couchbase.repository.query.support.N1qlUtils;
-import org.springframework.data.domain.Pageable;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.mapping.PropertyHandler;
@@ -135,7 +134,7 @@ public StringBasedN1qlQueryParser(String statement, CouchbaseQueryMethod queryMe
 				false, null, null);
 		this.countContext = createN1qlSpelValues(bucketName, collection, queryMethod.getEntityInformation().getJavaType(),
 				queryMethod.getReturnedObjectType(), typeField, typeValue, true, null, null);
-		this.parsedExpression = getExpression(accessor, getParameters(accessor), null, parser, evaluationContextProvider);
+		this.parsedExpression = getExpression(accessor, null, parser, evaluationContextProvider);
 		checkPlaceholders(this.parsedExpression.toString());
 	}
 
@@ -188,7 +187,8 @@ public N1qlSpelValues createN1qlSpelValues(String bucketName, String collection,
 		return new N1qlSpelValues(selectEntity, entity, i(b).toString(), typeSelection, delete, returning);
 	}
 
-	private String getProjectedOrDistinctFields(String b, Class resultClass, String typeField, String[] fields, String[] distinctFields) {
+	private String getProjectedOrDistinctFields(String b, Class resultClass, String typeField, String[] fields,
+			String[] distinctFields) {
 		if (distinctFields != null && distinctFields.length != 0) {
 			return i(distinctFields).toString();
 		}
@@ -217,7 +217,7 @@ private void getProjectedFieldsInternal(String bucketName, CouchbasePersistentPr
 				if (prop == persistentEntity.getVersionProperty() && parent == null) {
 					return;
 				}
-				if( prop.getFieldName().equals(typeField)) // typeField already projected
+				if (prop.getFieldName().equals(typeField)) // typeField already projected
 					return;
 				// for distinct when no distinctFields were provided, do not include the expiration field.
 				if (forDistinct && prop.findAnnotation(Expiration.class) != null && parent == null) {
@@ -528,23 +528,17 @@ public N1qlSpelValues(String selectClause, String entityFields, String bucket, S
 	}
 
 	// copied from StringN1qlBasedQuery
-	private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtimeParameters,
+	private N1QLExpression getExpression(ParameterAccessor accessor,
 			ReturnedType returnedType, SpelExpressionParser parser,
 			QueryMethodEvaluationContextProvider evaluationContextProvider) {
 		boolean isCountQuery = queryMethod.isCountQuery();
+		Object[] runtimeParameters = getParameters(accessor);
 		EvaluationContext evaluationContext = evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(),
 				runtimeParameters);
 		N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery));
-
-		if (queryMethod.isSliceQuery()) {
-			Pageable pageable = accessor.getPageable();
-			Assert.notNull(pageable, "Pageable must not be null!");
-			parsedStatement = parsedStatement.limit(pageable.getPageSize() + 1).offset(Math.toIntExact(pageable.getOffset()));
-		}
 		return parsedStatement;
 	}
 
-	// getExpression() could do this itself, but pass as an arg to be consistent with StringN1qlBasedQuery
 	private static Object[] getParameters(ParameterAccessor accessor) {
 		ArrayList<Object> params = new ArrayList<>();
 		for (Object o : accessor) {

src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java

@@ -39,6 +39,7 @@
 import org.springframework.data.couchbase.repository.Scope;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Slice;
 import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Repository;
 
@@ -177,6 +178,12 @@ Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 	Airport findByKey(String id);
 
+	@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata  between $1 and $2")
+	Slice<Airport> fetchSlice(String startIata, String iata, Pageable pageable);
+
+	@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata  between $1 and $2")
+	Page<Airport> fetchPage(String startIata, String iata, Pageable pageable);
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target({ ElementType.METHOD, ElementType.TYPE })
 	// @Meta

src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java

@@ -91,6 +91,7 @@
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Slice;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
@@ -639,7 +640,7 @@ void sortedRepository() {
 	}
 
 	@Test
-	void count() {
+	void countSlicePage() {
 		airportRepository.withOptions(QueryOptions.queryOptions().scanConsistency(REQUEST_PLUS)).deleteAll();
 		String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
 
@@ -677,6 +678,18 @@ void count() {
 			airportCount = airportRepository.countByIataIn("XXX");
 			assertEquals(0, airportCount);
 
+			pageable = PageRequest.of(1, 2, Sort.by("iata"));
+			Slice<Airport> airportSlice = airportRepository.fetchSlice("AAA", "zzz", pageable);
+			assertEquals(2, airportSlice.getSize());
+			assertEquals("LAX", airportSlice.getContent().get(0).getIata());
+			assertEquals("PHX", airportSlice.getContent().get(1).getIata());
+
+			pageable = PageRequest.of(1, 2, Sort.by("iata"));
+			Page<Airport> airportPage = airportRepository.fetchPage("AAA", "zzz", pageable);
+			assertEquals(2, airportPage.getSize());
+			assertEquals("LAX", airportPage.getContent().get(0).getIata());
+			assertEquals("PHX", airportPage.getContent().get(1).getIata());
+
 		} finally {
 			airportRepository
 					.deleteAllById(Arrays.stream(iatas).map((iata) -> "airports::" + iata).collect(Collectors.toSet()));
@@ -833,7 +846,8 @@ void threadSafeStringParametersTest() throws Exception {
 		}
 	}
 
-	@Test // DATACOUCH-650
+	@Test
+	// DATACOUCH-650
 	void deleteAllById() {
 
 		Airport vienna = new Airport("airports::vie", "vie", "LOWW");