From 33a7b9d4a8b15ffe082aba904541fed0c7b0d9fe Mon Sep 17 00:00:00 2001 From: mikereiche Date: Fri, 4 Feb 2022 11:24:29 -0800 Subject: [PATCH] Fix slice support. Fix slice support. Also discontinue use of deprecated class. Closes #1323. --- .../query/AbstractCouchbaseQuery.java | 7 ++-- .../query/CouchbaseQueryExecution.java | 35 +++++++++++-------- .../query/StringBasedN1qlQueryParser.java | 18 ++++------ .../couchbase/domain/AirportRepository.java | 7 ++++ ...chbaseRepositoryQueryIntegrationTests.java | 18 ++++++++-- 5 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/AbstractCouchbaseQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/AbstractCouchbaseQuery.java index 115d1872b..4174eef53 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/AbstractCouchbaseQuery.java +++ b/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); diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/CouchbaseQueryExecution.java b/src/main/java/org/springframework/data/couchbase/repository/query/CouchbaseQueryExecution.java index 9a98018fd..a2204879e 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/CouchbaseQueryExecution.java +++ b/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(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); } } diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java b/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java index 81492eb44..e9790c77b 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java +++ b/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 params = new ArrayList<>(); for (Object o : accessor) { diff --git a/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java b/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java index e5f460182..9a66f5efa 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java +++ b/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 projectIds, @Param(" @ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS) Airport findByKey(String id); + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata between $1 and $2") + Slice fetchSlice(String startIata, String iata, Pageable pageable); + + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata between $1 and $2") + Page fetchPage(String startIata, String iata, Pageable pageable); + @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.METHOD, ElementType.TYPE }) // @Meta diff --git a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java index a954494a0..d1e1d4480 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java +++ b/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 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 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");