From 6a58d84eac9a2b19bbd8e6104922925df06f2c83 Mon Sep 17 00:00:00 2001 From: mikereiche Date: Tue, 22 Sep 2020 16:39:26 -0700 Subject: [PATCH] DATACOUCH-616 - Fix parsing of String Query containing conditionals in quotes For a query as below that has conditional portions, the parsing for parameters was being done before the conditional portions were being resolved, which left the parameters between quotes where they wre not being recognized as query parameters. @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} " + " #{#projectIds != null ? 'AND iata IN $1' : ''} ") Long count(@Param("projectIds") List projectIds) --- .../ReactiveInsertByIdOperationSupport.java | 3 +- .../ReactiveUpsertByIdOperationSupport.java | 2 +- .../data/couchbase/repository/Query.java | 1 - .../query/ReactiveStringN1qlBasedQuery.java | 81 ------------ .../query/StringBasedN1qlQueryParser.java | 56 +++++++-- .../query/StringN1qlBasedQuery.java | 117 ------------------ .../query/StringN1qlQueryCreator.java | 41 +----- .../couchbase/domain/AirportRepository.java | 11 +- .../data/couchbase/domain/UserAnnotated.java | 2 +- ...chbaseRepositoryQueryIntegrationTests.java | 6 +- 10 files changed, 67 insertions(+), 253 deletions(-) delete mode 100644 src/main/java/org/springframework/data/couchbase/repository/query/ReactiveStringN1qlBasedQuery.java delete mode 100644 src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java index c977511f4..c76acf371 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java +++ b/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java @@ -15,7 +15,6 @@ */ package org.springframework.data.couchbase.core; -import com.couchbase.client.java.kv.UpsertOptions; import org.springframework.data.couchbase.core.mapping.Document; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -98,7 +97,7 @@ private InsertOptions buildInsertOptions() { } else if (durabilityLevel != DurabilityLevel.NONE) { options.durability(durabilityLevel); } - if (expiry != null) { + if (expiry != null && ! expiry.isZero()) { options.expiry(expiry); } else if (domainType.isAnnotationPresent(Document.class)) { Document documentAnn = domainType.getAnnotation(Document.class); diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java index bb07b2c77..48eb5c796 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java +++ b/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java @@ -98,7 +98,7 @@ private UpsertOptions buildUpsertOptions() { } else if (durabilityLevel != DurabilityLevel.NONE) { options.durability(durabilityLevel); } - if (expiry != null) { + if (expiry != null && ! expiry.isZero()) { options.expiry(expiry); } else if (domainType.isAnnotationPresent(Document.class)) { Document documentAnn = domainType.getAnnotation(Document.class); diff --git a/src/main/java/org/springframework/data/couchbase/repository/Query.java b/src/main/java/org/springframework/data/couchbase/repository/Query.java index d18f90496..21562a787 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/Query.java +++ b/src/main/java/org/springframework/data/couchbase/repository/Query.java @@ -23,7 +23,6 @@ import org.springframework.data.annotation.QueryAnnotation; import org.springframework.data.couchbase.core.CouchbaseTemplate; -import org.springframework.data.couchbase.repository.query.StringN1qlBasedQuery; /** * Annotation to support the use of N1QL queries with Couchbase. diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/ReactiveStringN1qlBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/ReactiveStringN1qlBasedQuery.java deleted file mode 100644 index 5a1058a3d..000000000 --- a/src/main/java/org/springframework/data/couchbase/repository/query/ReactiveStringN1qlBasedQuery.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright 2017-2020 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.couchbase.repository.query; - -import static org.springframework.data.couchbase.core.query.N1QLExpression.*; - -import org.springframework.data.couchbase.core.ReactiveCouchbaseOperations; -import org.springframework.data.couchbase.core.query.N1QLExpression; -import org.springframework.data.repository.query.ParameterAccessor; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; -import org.springframework.data.repository.query.RepositoryQuery; -import org.springframework.data.repository.query.ReturnedType; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.spel.standard.SpelExpressionParser; - -import com.couchbase.client.java.json.JsonValue; - -/** - * A reactive StringN1qlBasedQuery {@link RepositoryQuery} for Couchbase, based on N1QL and a String statement. - *

- * The statement can contain positional placeholders (eg. name = $1) that will map to the method's - * parameters, in the same order. - *

- * The statement can also contain SpEL expressions enclosed in #{ and }. - *

- * - * @author Subhashni Balakrishnan - * @since 3.0 - */ -public class ReactiveStringN1qlBasedQuery extends ReactiveAbstractN1qlBasedQuery { - - private final StringBasedN1qlQueryParser queryParser; - private final SpelExpressionParser parser; - private final QueryMethodEvaluationContextProvider evaluationContextProvider; - - public ReactiveStringN1qlBasedQuery(String statement, CouchbaseQueryMethod queryMethod, - ReactiveCouchbaseOperations couchbaseOperations, SpelExpressionParser spelParser, - QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(queryMethod, couchbaseOperations); - - this.queryParser = new StringBasedN1qlQueryParser(statement, queryMethod, getCouchbaseOperations().getBucketName(), - getCouchbaseOperations().getConverter(), getTypeField(), getTypeValue()); - this.parser = spelParser; - this.evaluationContextProvider = evaluationContextProvider; - } - - protected String getTypeField() { - return getCouchbaseOperations().getConverter().getTypeKey(); - } - - protected String getTypeValue() { - return getQueryMethod().getEntityInformation().getJavaType().getName(); - } - - @Override - protected JsonValue getPlaceholderValues(ParameterAccessor accessor) { - return this.queryParser.getPlaceholderValues(accessor); - } - - @Override - public N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtimeParameters, - ReturnedType returnedType) { - EvaluationContext evaluationContext = evaluationContextProvider - .getEvaluationContext(getQueryMethod().getParameters(), runtimeParameters); - return x(queryParser.doParse(parser, evaluationContext, false)); - } - -} 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 aea20d75e..b1e9e4956 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 @@ -32,9 +32,13 @@ 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.domain.Sort; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.QueryMethod; +import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.expression.EvaluationContext; import org.springframework.expression.common.TemplateParserContext; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -42,6 +46,7 @@ import com.couchbase.client.java.json.JsonArray; import com.couchbase.client.java.json.JsonObject; import com.couchbase.client.java.json.JsonValue; +import org.springframework.util.Assert; /** * @author Subhashni Balakrishnan @@ -103,20 +108,23 @@ public class StringBasedN1qlQueryParser { private static final Logger LOGGER = LoggerFactory.getLogger(StringBasedN1qlQueryParser.class); private final String statement; private final QueryMethod queryMethod; - private final PlaceholderType placeHolderType; + private PlaceholderType placeHolderType; private final N1qlSpelValues statementContext; private final N1qlSpelValues countContext; private final CouchbaseConverter couchbaseConverter; private final Collection parameterNames = new HashSet(); + public final N1QLExpression parsedExpression; public StringBasedN1qlQueryParser(String statement, QueryMethod queryMethod, String bucketName, - CouchbaseConverter couchbaseConverter, String typeField, String typeValue) { + CouchbaseConverter couchbaseConverter, String typeField, String typeValue, ParameterAccessor accessor, + SpelExpressionParser parser, QueryMethodEvaluationContextProvider evaluationContextProvider) { this.statement = statement; this.queryMethod = queryMethod; - this.placeHolderType = checkPlaceholders(statement); this.statementContext = createN1qlSpelValues(bucketName, typeField, typeValue, false); this.countContext = createN1qlSpelValues(bucketName, typeField, typeValue, true); this.couchbaseConverter = couchbaseConverter; + this.parsedExpression = getExpression(accessor, getParameters(accessor), null, parser, evaluationContextProvider); + checkPlaceholders( this.parsedExpression.toString() ); } public static N1qlSpelValues createN1qlSpelValues(String bucketName, String typeField, String typeValue, @@ -151,7 +159,7 @@ public String doParse(SpelExpressionParser parser, EvaluationContext evaluationC return parsedExpression.getValue(evaluationContext, String.class); } - private PlaceholderType checkPlaceholders(String statement) { + private void checkPlaceholders(String statement) { Matcher quoteMatcher = QUOTE_DETECTION_PATTERN.matcher(statement); Matcher positionMatcher = POSITIONAL_PLACEHOLDER_PATTERN.matcher(statement); @@ -192,11 +200,11 @@ private PlaceholderType checkPlaceholders(String statement) { } if (posCount > 0) { - return PlaceholderType.POSITIONAL; + placeHolderType = PlaceholderType.POSITIONAL; } else if (namedCount > 0) { - return PlaceholderType.NAMED; + placeHolderType = PlaceholderType.NAMED; } else { - return PlaceholderType.NONE; + placeHolderType = PlaceholderType.NONE; } } @@ -402,5 +410,39 @@ public N1qlSpelValues(String selectClause, String entityFields, String bucket, S this.returning = returning; } } + // copied from StringN1qlBasedQuery + private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtimeParameters, + ReturnedType returnedType, SpelExpressionParser parser, QueryMethodEvaluationContextProvider evaluationContextProvider) { + boolean isCountQuery = queryMethod.getName().toLowerCase().startsWith("count"); // should be queryMethod.isCountQuery() + EvaluationContext evaluationContext = evaluationContextProvider + .getEvaluationContext(queryMethod.getParameters(), runtimeParameters); + N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery)); + + Sort sort = accessor.getSort(); + if (sort.isSorted()) { + N1QLExpression[] cbSorts = N1qlUtils.createSort(sort); + parsedStatement = parsedStatement.orderBy(cbSorts); + } + if (queryMethod.isPageQuery()) { + Pageable pageable = accessor.getPageable(); + Assert.notNull(pageable, "Pageable must not be null!"); + parsedStatement = parsedStatement.limit(pageable.getPageSize()).offset( + Math.toIntExact(pageable.getOffset())); + } else 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) { + params.add(o); + } + return params.toArray(); + } } diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java deleted file mode 100644 index 15456d569..000000000 --- a/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright 2012-2020 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.data.couchbase.repository.query; - -import static org.springframework.data.couchbase.core.query.N1QLExpression.*; - -import org.springframework.data.couchbase.core.CouchbaseOperations; -import org.springframework.data.couchbase.core.query.N1QLExpression; -import org.springframework.data.couchbase.repository.query.support.N1qlUtils; -import org.springframework.data.domain.Pageable; -import org.springframework.data.domain.Sort; -import org.springframework.data.repository.query.ParameterAccessor; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; -import org.springframework.data.repository.query.RepositoryQuery; -import org.springframework.data.repository.query.ReturnedType; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.util.Assert; - -import com.couchbase.client.java.json.JsonValue; - -/** - * A {@link RepositoryQuery} for Couchbase, based on N1QL and a String statement. - *

- * The statement can contain positional placeholders (eg. name = $1) that will map to the method's - * parameters, in the same order. - *

- * The statement can also contain SpEL expressions enclosed in #{ and }. - *

- * There are couchbase-provided variables included for the {@link StringBasedN1qlQueryParser#SPEL_BUCKET bucket - * namespace}, the {@link StringBasedN1qlQueryParser#SPEL_ENTITY ID and CAS fields} necessary for entity reconstruction - * or a shortcut that covers {@link StringBasedN1qlQueryParser#SPEL_SELECT_FROM_CLAUSE SELECT AND FROM clauses}, along - * with a variable for {@link StringBasedN1qlQueryParser#SPEL_FILTER WHERE clause filtering} of the correct entity. - * - * @author Simon Baslé - * @author Subhashni Balakrishnan - * @author Mark Paluch - */ -public class StringN1qlBasedQuery extends AbstractN1qlBasedQuery { - private final SpelExpressionParser parser; - private final QueryMethodEvaluationContextProvider evaluationContextProvider; - private final StringBasedN1qlQueryParser queryParser; - - public StringN1qlBasedQuery(String statement, CouchbaseQueryMethod queryMethod, - CouchbaseOperations couchbaseOperations, SpelExpressionParser spelParser, - QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(queryMethod, couchbaseOperations); - this.queryParser = new StringBasedN1qlQueryParser(statement, queryMethod, getCouchbaseOperations().getBucketName(), - getCouchbaseOperations().getConverter(), getTypeField(), getTypeValue().getName()); - this.parser = spelParser; - this.evaluationContextProvider = evaluationContextProvider; - } - - protected String getTypeField() { - return getCouchbaseOperations().getConverter().getTypeKey(); - } - - protected Class getTypeValue() { - return getQueryMethod().getEntityInformation().getJavaType(); - } - - @Override - protected JsonValue getPlaceholderValues(ParameterAccessor accessor) { - return this.queryParser.getPlaceholderValues(accessor); - } - - @Override - public N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtimeParameters, - ReturnedType returnedType) { - EvaluationContext evaluationContext = evaluationContextProvider - .getEvaluationContext(getQueryMethod().getParameters(), runtimeParameters); - N1QLExpression parsedStatement = x(this.queryParser.doParse(parser, evaluationContext, false)); - - Sort sort = accessor.getSort(); - if (sort.isSorted()) { - N1QLExpression[] cbSorts = N1qlUtils.createSort(sort); - parsedStatement = parsedStatement.orderBy(cbSorts); - } - if (queryMethod.isPageQuery()) { - Pageable pageable = accessor.getPageable(); - Assert.notNull(pageable, "Pageable must not be null!"); - parsedStatement = parsedStatement.limit(pageable.getPageSize()).offset(Math.toIntExact(pageable.getOffset())); - } else 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; - } - - @Override - protected N1QLExpression getCount(ParameterAccessor accessor, Object[] runtimeParameters) { - EvaluationContext evaluationContext = evaluationContextProvider - .getEvaluationContext(getQueryMethod().getParameters(), runtimeParameters); - return x(this.queryParser.doParse(parser, evaluationContext, true)); - } - - @Override - protected boolean useGeneratedCountQuery() { - return this.queryParser.useGeneratedCountQuery(); - } - -} diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreator.java index 05213c457..9183c1162 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreator.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreator.java @@ -57,6 +57,7 @@ public class StringN1qlQueryCreator extends AbstractQueryCreator params = new ArrayList<>(); - for (Object o : accessor) { - params.add(o); - } - return params.toArray(); - } } - - 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 06ce1fec1..2254d81e2 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java +++ b/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java @@ -18,14 +18,13 @@ import java.util.List; -import org.jetbrains.annotations.NotNull; +import com.couchbase.client.java.query.QueryScanConsistency; import org.springframework.data.couchbase.repository.Query; import org.springframework.data.couchbase.repository.ScanConsistency; import org.springframework.data.repository.PagingAndSortingRepository; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; -import com.couchbase.client.java.query.QueryScanConsistency; - /** * template class for Reactive Couchbase operations * @@ -61,4 +60,10 @@ public interface AirportRepository extends PagingAndSortingRepository projectIds, @Param("planIds") List planIds, @Param("active") Boolean active); + } diff --git a/src/test/java/org/springframework/data/couchbase/domain/UserAnnotated.java b/src/test/java/org/springframework/data/couchbase/domain/UserAnnotated.java index 32bc7dfa0..30c3b06c6 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/UserAnnotated.java +++ b/src/test/java/org/springframework/data/couchbase/domain/UserAnnotated.java @@ -33,7 +33,7 @@ * @author Michael Reiche */ -@Document(expiry = 5) +@Document(expiry = 1) public class UserAnnotated extends User { public UserAnnotated(String id, String firstname, String lastname) { 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 c4ea6cd88..e0d048b22 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java @@ -19,14 +19,13 @@ import static org.junit.jupiter.api.Assertions.*; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.stream.Collectors; -import java.util.stream.StreamSupport; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -135,6 +134,9 @@ void count() { airportRepository.save(airport); } + Long count = airportRepository.countFancyExpression( Arrays.asList("JFK"), Arrays.asList("jfk"), false); + assertEquals( 1, count); + long airportCount = airportRepository.count(); assertEquals(7, airportCount);