From 6d747e97f5067c728fbc185d451520fcd9163e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Wed, 27 Jan 2016 17:31:27 +0100 Subject: [PATCH] DATACOUCH-180 - Allow named parameters in inline N1QL queries Choice of placeholder "mode" is determined by the syntax used inside the query, early during repository instantiation. An IllegalArgumentException is thrown if both named and positional parameters are used. --- .../repository/N1qlPlaceholderTests.java | 123 ++++++++++++++++++ .../couchbase/repository/PartyRepository.java | 16 +++ src/main/asciidoc/repository.adoc | 4 +- .../query/AbstractN1qlBasedQuery.java | 24 ++-- .../query/PartTreeN1qlBasedQuery.java | 7 +- .../query/StringN1qlBasedQuery.java | 115 +++++++++++++++- src/test/resources/logback.xml | 2 +- 7 files changed, 271 insertions(+), 20 deletions(-) create mode 100644 src/integration/java/org/springframework/data/couchbase/repository/N1qlPlaceholderTests.java diff --git a/src/integration/java/org/springframework/data/couchbase/repository/N1qlPlaceholderTests.java b/src/integration/java/org/springframework/data/couchbase/repository/N1qlPlaceholderTests.java new file mode 100644 index 00000000..79aa9b5f --- /dev/null +++ b/src/integration/java/org/springframework/data/couchbase/repository/N1qlPlaceholderTests.java @@ -0,0 +1,123 @@ +/* + * Copyright 2013-2015 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 + * + * http://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; + +import static org.junit.Assert.*; + +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.couchbase.IntegrationTestApplicationConfig; +import org.springframework.data.couchbase.core.query.Query; +import org.springframework.data.couchbase.repository.config.RepositoryOperationsMapping; +import org.springframework.data.couchbase.repository.support.CouchbaseRepositoryFactory; +import org.springframework.data.couchbase.repository.support.IndexManager; +import org.springframework.data.repository.CrudRepository; +import org.springframework.data.repository.query.Param; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestExecutionListeners; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Simon Baslé + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(classes = IntegrationTestApplicationConfig.class) +@TestExecutionListeners(PartyPopulatorListener.class) +public class N1qlPlaceholderTests { + + @Autowired + private RepositoryOperationsMapping operationsMapping; + + @Autowired + private IndexManager indexManager; + + private CouchbaseRepositoryFactory factory; + private PartyRepository partyRepository; + + @Before + public void setup() throws Exception { + factory = new CouchbaseRepositoryFactory(operationsMapping, indexManager); + partyRepository = factory.getRepository(PartyRepository.class); + } + + @Test + public void shouldFindUsingNamedParameters() { + String included = "90"; + String excluded = "New Year"; + int min = 200; + List result = partyRepository.findAllWithNamedParams(excluded, included, min); + + assertEquals(2, result.size()); + for (Party party : result) { + assertTrue(party.getDescription().contains(included)); + assertFalse(party.getDescription().contains(excluded)); + assertTrue(party.getAttendees() >= min); + } + } + + @Test + public void shouldFindUsingPositionalParameters() { + String included = "90"; + String excluded = "New Year"; + int min = 200; + List result = partyRepository.findAllWithPositionalParams(excluded, included, min); + + assertEquals(2, result.size()); + for (Party party : result) { + assertTrue(party.getDescription().contains(included)); + assertFalse(party.getDescription().contains(excluded)); + assertTrue(party.getAttendees() >= min); + } + } + + @Test + public void shouldIgnoreQuotedNamedParamsAndParamAnnotationIfPosUsed() { + String included = "90"; + String excluded = "New Year"; + int min = 200; + List result = partyRepository.findAllWithPositionalParamsAndQuotedNamedParams(excluded, included, min); + + assertEquals(2, result.size()); + for (Party party : result) { + assertTrue(party.getDescription().contains(included)); + assertFalse(party.getDescription().contains(excluded)); + assertTrue(party.getAttendees() >= min); + } + } + + private interface BadRepository extends CrudRepository { + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND `desc` LIKE '%' || $2 || '%' AND attendees >= $3" + + " AND `desc` NOT LIKE '%' || $included || '%'") + List findAllWithMixedParamsInQuery(@Param("excluded") String ex, @Param("included") String inc, @Param("min") long min); + } + + @Test + public void shouldFailUsingMixedParameters() { + try { + factory.getRepository(BadRepository.class); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals(e.toString(), "Using both named (1) and positional (2) placeholders is not supported, please choose " + + "one over the other in findAllWithMixedParamsInQuery", e.getMessage()); + } + } +} diff --git a/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java b/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java index 83eb14b2..082b1ef2 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java @@ -3,14 +3,17 @@ package org.springframework.data.couchbase.repository; import java.util.Date; import java.util.List; +import org.springframework.data.couchbase.core.query.N1qlSecondaryIndexed; import org.springframework.data.couchbase.core.query.Query; import org.springframework.data.couchbase.core.query.View; import org.springframework.data.couchbase.core.query.ViewIndexed; +import org.springframework.data.repository.query.Param; /** * @author Simon Baslé */ @ViewIndexed(designDoc = "party", viewName = "all") +@N1qlSecondaryIndexed(indexName = "party") public interface PartyRepository extends CouchbaseRepository { List findByAttendeesGreaterThanEqual(int minAttendees); @@ -38,4 +41,17 @@ public interface PartyRepository extends CouchbaseRepository { @Query("SELECT 1 = 1") boolean justABoolean(); + + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND `desc` LIKE '%' || $included || '%' AND attendees >= $min" + + " AND `desc` NOT LIKE '%' || $excluded || '%'") + List findAllWithNamedParams(@Param("excluded") String ex, @Param("included") String inc, @Param("min") long minimumAttendees); + + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND `desc` LIKE '%' || $2 || '%' AND attendees >= $3" + + " AND `desc` NOT LIKE '%' || $1 || '%'") + List findAllWithPositionalParams(String ex, String inc, long minimumAttendees); + + @Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND `desc` LIKE '%' || $2 || '%' AND attendees >= $3" + + " AND `desc` NOT LIKE '%' || $1 || '%' AND `desc` != \"this is \\\"$excluded\\\"\"") + List findAllWithPositionalParamsAndQuotedNamedParams(@Param("excluded") String ex, @Param("included") String inc, @Param("min") long min); + } diff --git a/src/main/asciidoc/repository.adoc b/src/main/asciidoc/repository.adoc index f0f27be0..de149f70 100644 --- a/src/main/asciidoc/repository.adoc +++ b/src/main/asciidoc/repository.adoc @@ -135,7 +135,7 @@ public interface UserRepository extends CrudRepository { Here we see two N1QL-backed ways of querying. -The first method uses the `Query` annotation to provide a N1QL statement inline. SpEL (Spring Expression Language) is supported by surrounding SpEL expression blocks between `#{` and `}`. You can also include N1QL positional placeholders (eg. `$1`, `$2`). +The first method uses the `Query` annotation to provide a N1QL statement inline. SpEL (Spring Expression Language) is supported by surrounding SpEL expression blocks between `#{` and `}`. A few N1QL-specific values are provided through SpEL: - `#n1ql.selectEntity` allows to easily make sure the statement will select all the fields necessary to build the full entity (including document ID and CAS value). @@ -145,6 +145,8 @@ A few N1QL-specific values are provided through SpEL: IMPORTANT: We recommend that you always use the `selectEntity` SpEL and a WHERE clause with a `filter` SpEL (since otherwise your query could be impacted by entities from other repositories). +String-based queries support parametrized queries. You can either use positional placeholders like "`$1`", in which case each of the method parameters will map, in order, to `$1`, `$2`, `$3`... Alternatively, you can use named placeholders using the "`$someString`" syntax. Method parameters will be matched with their corresponding placeholder using the parameter's name, which can be overridden by annotating each parameter (except a `Pageable` or `Sort`) with `@Param` (eg. `@Param("someString")`). You cannot mix the two approaches in your query and will get an `IllegalArgumentException` if you do. + You can also do single projections in your N1QL queries (provided it selects only one field and returns only one result, usually an aggregation like `COUNT`, `AVG`, `MAX`...). Such projection would have a simple return type like `long`, `boolean` or `String`. This is *NOT* intended for projections to DTOs. Another example: "`#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND test = $1`", which is equivalent to diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/AbstractN1qlBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/AbstractN1qlBasedQuery.java index e3cfc2e9..42749bed 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/AbstractN1qlBasedQuery.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/AbstractN1qlBasedQuery.java @@ -20,10 +20,12 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import com.couchbase.client.core.lang.Tuple2; import com.couchbase.client.java.document.json.JsonArray; -import com.couchbase.client.java.error.QueryExecutionException; -import com.couchbase.client.java.query.N1qlQuery; +import com.couchbase.client.java.document.json.JsonObject; +import com.couchbase.client.java.document.json.JsonValue; import com.couchbase.client.java.query.N1qlParams; +import com.couchbase.client.java.query.N1qlQuery; import com.couchbase.client.java.query.Statement; import com.couchbase.client.java.query.consistency.ScanConsistency; import org.slf4j.Logger; @@ -34,10 +36,8 @@ import org.springframework.data.couchbase.core.CouchbaseQueryExecutionException; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.SliceImpl; -import org.springframework.data.repository.core.NamedQueries; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.ParametersParameterAccessor; -import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.util.StreamUtils; @@ -73,13 +73,13 @@ public abstract class AbstractN1qlBasedQuery implements RepositoryQuery { protected abstract Statement getStatement(ParameterAccessor accessor, Object[] runtimeParameters); - protected abstract JsonArray getPlaceholderValues(ParameterAccessor accessor); + protected abstract JsonValue getPlaceholderValues(ParameterAccessor accessor); @Override public Object execute(Object[] parameters) { ParametersParameterAccessor accessor = new ParametersParameterAccessor(queryMethod.getParameters(), parameters); Statement statement = getStatement(accessor, parameters); - JsonArray queryPlaceholderValues = getPlaceholderValues(accessor); + JsonValue queryPlaceholderValues = getPlaceholderValues(accessor); //prepare the final query N1qlQuery query = buildQuery(statement, queryPlaceholderValues, @@ -94,13 +94,15 @@ public abstract class AbstractN1qlBasedQuery implements RepositoryQuery { queryMethod.isPageQuery(), queryMethod.isSliceQuery(), queryMethod.isModifyingQuery()); } - protected static N1qlQuery buildQuery(Statement statement, JsonArray queryPlaceholderValues, ScanConsistency scanConsistency) { + protected static N1qlQuery buildQuery(Statement statement, JsonValue queryPlaceholderValues, ScanConsistency scanConsistency) { N1qlParams n1qlParams = N1qlParams.build().consistency(scanConsistency); N1qlQuery query; - if (!queryPlaceholderValues.isEmpty()) { - query = N1qlQuery.parameterized(statement, queryPlaceholderValues, n1qlParams); - } - else { + + if (queryPlaceholderValues instanceof JsonObject && !((JsonObject) queryPlaceholderValues).isEmpty()) { + query = N1qlQuery.parameterized(statement, (JsonObject) queryPlaceholderValues, n1qlParams); + } else if (queryPlaceholderValues instanceof JsonArray && !((JsonArray) queryPlaceholderValues).isEmpty()) { + query = N1qlQuery.parameterized(statement, (JsonArray) queryPlaceholderValues, n1qlParams); + } else { query = N1qlQuery.simple(statement, n1qlParams); } return query; diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java index c7021a78..f57c1b28 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java @@ -18,11 +18,14 @@ package org.springframework.data.couchbase.repository.query; import static com.couchbase.client.java.query.Select.select; import static com.couchbase.client.java.query.dsl.Expression.i; -import static com.couchbase.client.java.query.dsl.Expression.path; import static com.couchbase.client.java.query.dsl.functions.AggregateFunctions.count; import static com.couchbase.client.java.query.dsl.functions.MetaFunctions.meta; +import com.couchbase.client.core.lang.Tuple; +import com.couchbase.client.core.lang.Tuple2; import com.couchbase.client.java.document.json.JsonArray; +import com.couchbase.client.java.document.json.JsonObject; +import com.couchbase.client.java.document.json.JsonValue; import com.couchbase.client.java.query.Statement; import com.couchbase.client.java.query.dsl.Expression; import com.couchbase.client.java.query.dsl.path.FromPath; @@ -46,7 +49,7 @@ public class PartTreeN1qlBasedQuery extends AbstractN1qlBasedQuery { } @Override - protected JsonArray getPlaceholderValues(ParameterAccessor accessor) { + protected JsonValue getPlaceholderValues(ParameterAccessor accessor) { return JsonArray.empty(); } 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 index 42301f53..6cbdde31 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/StringN1qlBasedQuery.java @@ -16,7 +16,14 @@ package org.springframework.data.couchbase.repository.query; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import com.couchbase.client.java.document.json.JsonArray; +import com.couchbase.client.java.document.json.JsonObject; +import com.couchbase.client.java.document.json.JsonValue; import com.couchbase.client.java.query.N1qlQuery; import com.couchbase.client.java.query.Statement; import org.slf4j.Logger; @@ -24,6 +31,7 @@ import org.slf4j.LoggerFactory; import org.springframework.data.couchbase.core.CouchbaseOperations; import org.springframework.data.repository.query.EvaluationContextProvider; +import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.expression.EvaluationContext; @@ -82,7 +90,20 @@ public class StringN1qlBasedQuery extends AbstractN1qlBasedQuery { */ public static final String SPEL_FILTER = "#" + SPEL_PREFIX + ".filter"; + /** regexp that detect $named placeholder (starts with a letter, then alphanum chars) */ + private static final Pattern NAMED_PLACEHOLDER_PATTERN = Pattern.compile("\\W(\\$\\p{Alpha}\\p{Alnum}*)\\b"); + /** regexp that detect positional placeholder ($ followed by digits only) */ + private static final Pattern POSITIONAL_PLACEHOLDER_PATTERN = Pattern.compile("\\W(\\$\\p{Digit}+)\\b"); + /** regexp that detects " and ' quote boundaries, ignoring escaped quotes */ + private static final Pattern QUOTE_DETECTION_PATTERN = Pattern.compile("[\"'](?:[^\"'\\\\]*(?:\\\\.)?)*[\"']"); + + /** enumeration of all the combinations of placeholder types that could be found in a N1QL statement */ + private enum PlaceholderType { + NAMED, POSITIONAL, NONE + } + private final String originalStatement; + private final PlaceholderType placeHolderType; private final SpelExpressionParser parser; private final EvaluationContextProvider evaluationContextProvider; private final N1qlSpelValues countContext; @@ -101,6 +122,8 @@ public class StringN1qlBasedQuery extends AbstractN1qlBasedQuery { super(queryMethod, couchbaseOperations); this.originalStatement = statement; + this.placeHolderType = checkPlaceholders(statement); + this.parser = spelParser; this.evaluationContextProvider = evaluationContextProvider; @@ -108,6 +131,59 @@ public class StringN1qlBasedQuery extends AbstractN1qlBasedQuery { this.countContext = createN1qlSpelValues(getCouchbaseOperations().getCouchbaseBucket().name(), getTypeField(), getTypeValue(), true); } + private PlaceholderType checkPlaceholders(String statement) { + Matcher quoteMatcher = QUOTE_DETECTION_PATTERN.matcher(statement); + Matcher positionMatcher = POSITIONAL_PLACEHOLDER_PATTERN.matcher(statement); + Matcher namedMatcher = NAMED_PLACEHOLDER_PATTERN.matcher(statement); + + List quotes = new ArrayList(); + while(quoteMatcher.find()) { + quotes.add(new int[] { quoteMatcher.start(), quoteMatcher.end() }); + } + + int posCount = 0; + int namedCount = 0; + + while(positionMatcher.find()) { + String placeholder = positionMatcher.group(1); + //check not in quoted + if (checkNotQuoted(placeholder, positionMatcher.start(), positionMatcher.end(), quotes)) { + LOGGER.trace("{}: Found positional placeholder {}", getQueryMethod().getName(), placeholder); + posCount++; + } + } + + while(namedMatcher.find()) { + String placeholder = namedMatcher.group(1); + //check not in quoted + if (checkNotQuoted(placeholder, namedMatcher.start(), namedMatcher.end(), quotes)) { + LOGGER.trace("{}: Found named placeholder {}", getQueryMethod().getName(), placeholder); + namedCount++; + } + } + + if (posCount > 0 && namedCount > 0) { + throw new IllegalArgumentException("Using both named (" + namedCount + ") and positional (" + posCount + + ") placeholders is not supported, please choose one over the other in " + queryMethod.getName()); + } else if (posCount > 0) { + return PlaceholderType.POSITIONAL; + } else if (namedCount > 0) { + return PlaceholderType.NAMED; + } else { + return PlaceholderType.NONE; + } + } + + private boolean checkNotQuoted(String item, int start, int end, List quotes) { + for (int[] quote : quotes) { + if (quote[0] <= start && quote[1] >= end) { + LOGGER.trace("{}: potential placeholder {} is inside quotes, ignored", queryMethod.getName(), item); + return false; + } + } + return true; + } + public static N1qlSpelValues createN1qlSpelValues(String bucketName, String typeField, Class typeValue, boolean isCount) { String b = "`" + bucketName + "`"; String entity = "META(" + b + ").id AS " + CouchbaseOperations.SELECT_ID + @@ -150,12 +226,41 @@ public class StringN1qlBasedQuery extends AbstractN1qlBasedQuery { } @Override - protected JsonArray getPlaceholderValues(ParameterAccessor accessor) { - JsonArray values = JsonArray.create(); - for (Object value : accessor) { - values.add(value); + protected JsonValue getPlaceholderValues(ParameterAccessor accessor) { + switch (this.placeHolderType) { + case NAMED: + return getNamedPlaceholderValues(accessor); + case POSITIONAL: + return getPositionalPlaceholderValues(accessor); + case NONE: + default: + return JsonArray.empty(); } - return values; + } + + private JsonValue getPositionalPlaceholderValues(ParameterAccessor accessor) { + JsonArray posValues = JsonArray.create(); + for (Parameter parameter : getQueryMethod().getParameters().getBindableParameters()) { + posValues.add(accessor.getBindableValue(parameter.getIndex())); + } + return posValues; + } + + private JsonObject getNamedPlaceholderValues(ParameterAccessor accessor) { + JsonObject namedValues = JsonObject.create(); + + for (Parameter parameter : getQueryMethod().getParameters().getBindableParameters()) { + String placeholder = parameter.getPlaceholder(); + Object value = accessor.getBindableValue(parameter.getIndex()); + + if (placeholder != null && placeholder.charAt(0) == ':') { + placeholder = placeholder.replaceFirst(":", ""); + namedValues.put(placeholder, value); + } else { + namedValues.put(parameter.getName(), value); + } + } + return namedValues; } @Override diff --git a/src/test/resources/logback.xml b/src/test/resources/logback.xml index 04281627..c67b1d6a 100644 --- a/src/test/resources/logback.xml +++ b/src/test/resources/logback.xml @@ -18,7 +18,7 @@