Skip to content

Commit c88a35c

Browse files
mikereichemikereiche
authored
Fix slice support. (#1324)
Fix slice support. Also discontinue use of deprecated class. Closes #1323.
1 parent 1438bf9 commit c88a35c

File tree

5 files changed

+54
-31
lines changed

5 files changed

+54
-31
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2021 the original author or authors
2+
* Copyright 2020-2022 the original author or authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import org.springframework.data.couchbase.core.query.Query;
2323
import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.DeleteExecution;
2424
import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.PagedExecution;
25+
import org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution.SlicedExecution;
2526
import org.springframework.data.repository.core.EntityMetadata;
2627
import org.springframework.data.repository.query.ParameterAccessor;
2728
import org.springframework.data.repository.query.ParametersParameterAccessor;
@@ -116,7 +117,7 @@ private CouchbaseQueryExecution getExecutionToWrap(ParameterAccessor accessor, E
116117
return new DeleteExecution(getOperations(), getQueryMethod());
117118
} else if (isTailable(getQueryMethod())) {
118119
return (q, t, r, c) -> operation.as(r).matching(q.with(accessor.getPageable())).all(); // s/b tail() instead of
119-
// all()
120+
// all()
120121
} else if (getQueryMethod().isCollectionQuery()) {
121122
return (q, t, r, c) -> operation.as(r).matching(q.with(accessor.getPageable())).all();
122123
} else if (getQueryMethod().isStreamQuery()) {
@@ -127,6 +128,8 @@ private CouchbaseQueryExecution getExecutionToWrap(ParameterAccessor accessor, E
127128
return (q, t, r, c) -> operation.as(r).matching(q).exists();
128129
} else if (getQueryMethod().isPageQuery()) {
129130
return new PagedExecution(operation, accessor.getPageable());
131+
} else if (getQueryMethod().isSliceQuery()) {
132+
return new SlicedExecution(operation, accessor.getPageable());
130133
} else {
131134
return (q, t, r, c) -> {
132135
TerminatingFindByQuery<?> find = operation.as(r).matching(q);

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.ExecutableFindByQuery;
2323
import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation.TerminatingFindByQuery;
2424
import org.springframework.data.couchbase.core.query.Query;
25+
import org.springframework.data.domain.PageImpl;
2526
import org.springframework.data.domain.Pageable;
2627
import org.springframework.data.domain.Slice;
2728
import org.springframework.data.domain.SliceImpl;
2829
import org.springframework.data.repository.query.QueryMethod;
29-
import org.springframework.data.repository.support.PageableExecutionUtils;
3030
import org.springframework.util.Assert;
3131

3232
/**
@@ -93,13 +93,13 @@ public Object execute(Query query, Class<?> type, Class<?> returnType, String co
9393
*/
9494
final class SlicedExecution implements CouchbaseQueryExecution {
9595

96-
private final ExecutableFindByQuery<?> find;
96+
private final ExecutableFindByQuery<?> operation;
9797
private final Pageable pageable;
9898

99-
public SlicedExecution(ExecutableFindByQuery find, Pageable pageable) {
100-
Assert.notNull(find, "Find must not be null!");
99+
public SlicedExecution(ExecutableFindByQuery operation, Pageable pageable) {
100+
Assert.notNull(operation, "Find must not be null!");
101101
Assert.notNull(pageable, "Pageable must not be null!");
102-
this.find = find;
102+
this.operation = operation;
103103
this.pageable = pageable;
104104
}
105105

@@ -110,12 +110,14 @@ public SlicedExecution(ExecutableFindByQuery find, Pageable pageable) {
110110
@Override
111111
@SuppressWarnings({ "unchecked", "rawtypes" })
112112
public Object execute(Query query, Class<?> type, Class<?> returnType, String collection) {
113-
int pageSize = pageable.getPageSize();
114-
// Apply Pageable but tweak limit to peek into next page
115-
Query modifiedQuery = query.skip(pageable.getOffset()).limit(pageSize + 1);
116-
List result = find.as(returnType).matching(modifiedQuery).all();
117-
boolean hasNext = result.size() > pageSize;
118-
return new SliceImpl<Object>(hasNext ? result.subList(0, pageSize) : result, pageable, hasNext);
113+
int overallLimit = 0; // query.getLimit();
114+
TerminatingFindByQuery<?> matching = operation.as(returnType).matching(query);
115+
// Adjust limit if page would exceed the overall limit
116+
if (overallLimit != 0 && pageable.getOffset() + pageable.getPageSize() > overallLimit) {
117+
query.limit((int) (overallLimit - pageable.getOffset()));
118+
}
119+
List<?> results = matching.all();
120+
return new SliceImpl(results, pageable, results != null && !results.isEmpty());
119121
}
120122
}
121123

@@ -146,10 +148,13 @@ public Object execute(Query query, Class<?> type, Class<?> returnType, String co
146148
if (overallLimit != 0 && pageable.getOffset() + pageable.getPageSize() > overallLimit) {
147149
query.limit((int) (overallLimit - pageable.getOffset()));
148150
}
149-
return PageableExecutionUtils.getPage(matching.all(), pageable, () -> {
150-
long count = operation.matching(query.skip(-1).limit(-1).withoutSort()).count();
151-
return overallLimit != 0 ? Math.min(count, overallLimit) : count;
152-
});
151+
152+
List<?> result = matching.all(); // this needs to be done before count, as count clears the skip and limit
153+
154+
long count = operation.matching(query.skip(-1).limit(-1).withoutSort()).count();
155+
count = overallLimit != 0 ? Math.min(count, overallLimit) : count;
156+
157+
return new PageImpl(result, pageable, count);
153158
}
154159
}
155160

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.springframework.data.couchbase.core.query.N1QLExpression;
3838
import org.springframework.data.couchbase.repository.Query;
3939
import org.springframework.data.couchbase.repository.query.support.N1qlUtils;
40-
import org.springframework.data.domain.Pageable;
4140
import org.springframework.data.mapping.PersistentEntity;
4241
import org.springframework.data.mapping.PersistentPropertyPath;
4342
import org.springframework.data.mapping.PropertyHandler;
@@ -135,7 +134,7 @@ public StringBasedN1qlQueryParser(String statement, CouchbaseQueryMethod queryMe
135134
false, null, null);
136135
this.countContext = createN1qlSpelValues(bucketName, collection, queryMethod.getEntityInformation().getJavaType(),
137136
queryMethod.getReturnedObjectType(), typeField, typeValue, true, null, null);
138-
this.parsedExpression = getExpression(accessor, getParameters(accessor), null, parser, evaluationContextProvider);
137+
this.parsedExpression = getExpression(accessor, null, parser, evaluationContextProvider);
139138
checkPlaceholders(this.parsedExpression.toString());
140139
}
141140

@@ -188,7 +187,8 @@ public N1qlSpelValues createN1qlSpelValues(String bucketName, String collection,
188187
return new N1qlSpelValues(selectEntity, entity, i(b).toString(), typeSelection, delete, returning);
189188
}
190189

191-
private String getProjectedOrDistinctFields(String b, Class resultClass, String typeField, String[] fields, String[] distinctFields) {
190+
private String getProjectedOrDistinctFields(String b, Class resultClass, String typeField, String[] fields,
191+
String[] distinctFields) {
192192
if (distinctFields != null && distinctFields.length != 0) {
193193
return i(distinctFields).toString();
194194
}
@@ -217,7 +217,7 @@ private void getProjectedFieldsInternal(String bucketName, CouchbasePersistentPr
217217
if (prop == persistentEntity.getVersionProperty() && parent == null) {
218218
return;
219219
}
220-
if( prop.getFieldName().equals(typeField)) // typeField already projected
220+
if (prop.getFieldName().equals(typeField)) // typeField already projected
221221
return;
222222
// for distinct when no distinctFields were provided, do not include the expiration field.
223223
if (forDistinct && prop.findAnnotation(Expiration.class) != null && parent == null) {
@@ -528,23 +528,17 @@ public N1qlSpelValues(String selectClause, String entityFields, String bucket, S
528528
}
529529

530530
// copied from StringN1qlBasedQuery
531-
private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtimeParameters,
531+
private N1QLExpression getExpression(ParameterAccessor accessor,
532532
ReturnedType returnedType, SpelExpressionParser parser,
533533
QueryMethodEvaluationContextProvider evaluationContextProvider) {
534534
boolean isCountQuery = queryMethod.isCountQuery();
535+
Object[] runtimeParameters = getParameters(accessor);
535536
EvaluationContext evaluationContext = evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(),
536537
runtimeParameters);
537538
N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery));
538-
539-
if (queryMethod.isSliceQuery()) {
540-
Pageable pageable = accessor.getPageable();
541-
Assert.notNull(pageable, "Pageable must not be null!");
542-
parsedStatement = parsedStatement.limit(pageable.getPageSize() + 1).offset(Math.toIntExact(pageable.getOffset()));
543-
}
544539
return parsedStatement;
545540
}
546541

547-
// getExpression() could do this itself, but pass as an arg to be consistent with StringN1qlBasedQuery
548542
private static Object[] getParameters(ParameterAccessor accessor) {
549543
ArrayList<Object> params = new ArrayList<>();
550544
for (Object o : accessor) {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.springframework.data.couchbase.repository.Scope;
4040
import org.springframework.data.domain.Page;
4141
import org.springframework.data.domain.Pageable;
42+
import org.springframework.data.domain.Slice;
4243
import org.springframework.data.repository.query.Param;
4344
import org.springframework.stereotype.Repository;
4445

@@ -177,6 +178,12 @@ Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("
177178
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
178179
Airport findByKey(String id);
179180

181+
@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata between $1 and $2")
182+
Slice<Airport> fetchSlice(String startIata, String iata, Pageable pageable);
183+
184+
@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata between $1 and $2")
185+
Page<Airport> fetchPage(String startIata, String iata, Pageable pageable);
186+
180187
@Retention(RetentionPolicy.RUNTIME)
181188
@Target({ ElementType.METHOD, ElementType.TYPE })
182189
// @Meta

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
import org.springframework.data.domain.Page;
9292
import org.springframework.data.domain.PageRequest;
9393
import org.springframework.data.domain.Pageable;
94+
import org.springframework.data.domain.Slice;
9495
import org.springframework.data.domain.Sort;
9596
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
9697
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
@@ -639,7 +640,7 @@ void sortedRepository() {
639640
}
640641

641642
@Test
642-
void count() {
643+
void countSlicePage() {
643644
airportRepository.withOptions(QueryOptions.queryOptions().scanConsistency(REQUEST_PLUS)).deleteAll();
644645
String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
645646

@@ -677,6 +678,18 @@ void count() {
677678
airportCount = airportRepository.countByIataIn("XXX");
678679
assertEquals(0, airportCount);
679680

681+
pageable = PageRequest.of(1, 2, Sort.by("iata"));
682+
Slice<Airport> airportSlice = airportRepository.fetchSlice("AAA", "zzz", pageable);
683+
assertEquals(2, airportSlice.getSize());
684+
assertEquals("LAX", airportSlice.getContent().get(0).getIata());
685+
assertEquals("PHX", airportSlice.getContent().get(1).getIata());
686+
687+
pageable = PageRequest.of(1, 2, Sort.by("iata"));
688+
Page<Airport> airportPage = airportRepository.fetchPage("AAA", "zzz", pageable);
689+
assertEquals(2, airportPage.getSize());
690+
assertEquals("LAX", airportPage.getContent().get(0).getIata());
691+
assertEquals("PHX", airportPage.getContent().get(1).getIata());
692+
680693
} finally {
681694
airportRepository
682695
.deleteAllById(Arrays.stream(iatas).map((iata) -> "airports::" + iata).collect(Collectors.toSet()));
@@ -833,7 +846,8 @@ void threadSafeStringParametersTest() throws Exception {
833846
}
834847
}
835848

836-
@Test // DATACOUCH-650
849+
@Test
850+
// DATACOUCH-650
837851
void deleteAllById() {
838852

839853
Airport vienna = new Airport("airports::vie", "vie", "LOWW");

0 commit comments

Comments
 (0)