From ec2f21f1ea69369e161857e71ef324243dfc7182 Mon Sep 17 00:00:00 2001 From: mikereiche Date: Thu, 29 Jun 2023 11:15:11 -0700 Subject: [PATCH] Restore StringQuery constructor that takes only string. Closes #1769. --- .../couchbase/core/query/StringQuery.java | 4 ++ .../query/StringBasedN1qlQueryParser.java | 70 +++++++++++++++---- .../query/StringN1qlQueryCreatorTests.java | 55 +++++++++++++++ 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java b/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java index 92ff962e1..264a9a2cb 100644 --- a/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java +++ b/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java @@ -64,6 +64,10 @@ public StringQuery(CouchbaseQueryMethod queryMethod, String n1qlString, this.spelExpressionParser = spelExpressionParser; } + public StringQuery(String n1qlString) { + this(null,n1qlString, null, null, null); + } + @Override public String toN1qlSelectString(CouchbaseConverter converter, String bucketName, String scope, String collection, Class domainClass, Class resultClass, boolean isCount, String[] distinctFields, String[] fields) { 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 082e12766..ebb11a29d 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 @@ -38,6 +38,7 @@ import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; import org.springframework.data.couchbase.core.mapping.Expiration; import org.springframework.data.couchbase.core.query.N1QLExpression; +import org.springframework.data.couchbase.core.query.StringQuery; import org.springframework.data.couchbase.repository.Query; import org.springframework.data.couchbase.repository.query.support.N1qlUtils; import org.springframework.data.mapping.PersistentEntity; @@ -122,6 +123,12 @@ public class StringBasedN1qlQueryParser { * regexp that detect positional placeholder ($ followed by digits only) */ public static final Pattern POSITIONAL_PLACEHOLDER_PATTERN = Pattern.compile("\\W(\\$\\p{Digit}+)\\b"); + + /** + * regexp that detect SPEL Expression (#{..}) + */ + public static final Pattern SPEL_EXPRESSION_PATTERN = Pattern.compile("(#\\{[^\\}]*\\})"); + /** * regexp that detects " and ' quote boundaries, ignoring escaped quotes */ @@ -157,7 +164,8 @@ public StringBasedN1qlQueryParser(String statement, CouchbaseQueryMethod queryMe this.statement = statement; this.queryMethod = queryMethod; this.couchbaseConverter = couchbaseConverter; - this.statementContext = createN1qlSpelValues(collection != null ? collection : bucketName, scope, collection, + this.statementContext = queryMethod == null ? null + : createN1qlSpelValues(collection != null ? collection : bucketName, scope, collection, queryMethod.getEntityInformation().getJavaType(), typeField, typeValue, queryMethod.isCountQuery(), null, null); this.parsedExpression = getExpression(statement, queryMethod, accessor, spelExpressionParser, evaluationContextProvider); @@ -371,6 +379,9 @@ private void checkPlaceholders(String statement) { Matcher quoteMatcher = QUOTE_DETECTION_PATTERN.matcher(statement); Matcher positionMatcher = POSITIONAL_PLACEHOLDER_PATTERN.matcher(statement); Matcher namedMatcher = NAMED_PLACEHOLDER_PATTERN.matcher(statement); + String queryIdentifier = (this.queryMethod != null ? queryMethod.getClass().getName() + : StringQuery.class.getName()) + "." + + (this.queryMethod != null ? queryMethod.getName() : this.statement); List quotes = new ArrayList(); while (quoteMatcher.find()) { @@ -383,8 +394,14 @@ private void checkPlaceholders(String statement) { 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 {}", this.queryMethod.getName(), placeholder); + if (checkNotQuoted(placeholder, positionMatcher.start(), positionMatcher.end(), quotes, queryIdentifier)) { + if (this.queryMethod == null) { + throw new IllegalArgumentException( + "StringQuery created from StringQuery(String) cannot have parameters. " + + "They cannot be processed. " + + "Use an @Query annotated method and the SPEL Expression #{[]} : " + statement); + } + LOGGER.trace("{}: Found positional placeholder {}", queryIdentifier, placeholder); posCount++; parameterNames.add(placeholder.substring(1)); // save without the leading $ } @@ -393,8 +410,13 @@ private void checkPlaceholders(String statement) { 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 {}", this.queryMethod.getName(), placeholder); + if (checkNotQuoted(placeholder, namedMatcher.start(), namedMatcher.end(), quotes, queryIdentifier)) { + if (this.queryMethod == null) { + throw new IllegalArgumentException( + "StringQuery created from StringQuery(String) cannot have parameters. " + + "Use an @Query annotated method and the SPEL Expression #{[]} : " + statement); + } + LOGGER.trace("{}: Found named placeholder {}", queryIdentifier, placeholder); namedCount++; parameterNames.add(placeholder.substring(1));// save without the leading $ } @@ -402,8 +424,7 @@ private void checkPlaceholders(String statement) { if (posCount > 0 && namedCount > 0) { // actual values from parameterNames might be more useful throw new IllegalArgumentException("Using both named (" + namedCount + ") and positional (" + posCount - + ") placeholders is not supported, please choose one over the other in " + queryMethod.getClass().getName() - + "." + this.queryMethod.getName() + "()"); + + ") placeholders is not supported, please choose one over the other in " + queryIdentifier + "()"); } if (posCount > 0) { @@ -413,12 +434,30 @@ private void checkPlaceholders(String statement) { } else { placeHolderType = PlaceholderType.NONE; } + + if (this.queryMethod == null) { + Matcher spelMatcher = SPEL_EXPRESSION_PATTERN.matcher(statement); + while (spelMatcher.find()) { + String placeholder = spelMatcher.group(1); + // check not in quoted + if (checkNotQuoted(placeholder, spelMatcher.start(), spelMatcher.end(), quotes, queryIdentifier)) { + if (this.queryMethod == null) { + throw new IllegalArgumentException( + "StringQuery created from StringQuery(String) cannot SPEL expressions. " + + "Use an @Query annotated method and the SPEL Expression #{[]} : " + + statement); + } + LOGGER.trace("{}: Found SPEL Experssion {}", queryIdentifier, placeholder); + } + } + } + } - private boolean checkNotQuoted(String item, int start, int end, List quotes) { + private boolean checkNotQuoted(String item, int start, int end, List quotes, String queryIdentifier) { for (int[] quote : quotes) { if (quote[0] <= start && quote[1] >= end) { - LOGGER.trace("{}: potential placeholder {} is inside quotes, ignored", this.queryMethod.getName(), item); + LOGGER.trace("{}: potential placeholder {} is inside quotes, ignored", queryIdentifier, item); return false; } } @@ -647,10 +686,15 @@ public N1qlSpelValues(String selectClause, String entityFields, String bucket, S public N1QLExpression getExpression(String statement, CouchbaseQueryMethod queryMethod, ParameterAccessor accessor, SpelExpressionParser parser, QueryMethodEvaluationContextProvider evaluationContextProvider) { - Object[] runtimeParameters = getParameters(accessor); - EvaluationContext evaluationContext = evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(), - runtimeParameters); - N1QLExpression parsedStatement = x(doParse(statement, parser, evaluationContext, this.getStatementContext())); + N1QLExpression parsedStatement; + if (accessor != null && queryMethod != null && parser != null) { + Object[] runtimeParameters = getParameters(accessor); + EvaluationContext evaluationContext = evaluationContextProvider + .getEvaluationContext(queryMethod.getParameters(), runtimeParameters); + parsedStatement = x(doParse(statement, parser, evaluationContext, this.getStatementContext())); + } else { + parsedStatement = x(statement); + } checkPlaceholders(parsedStatement.toString()); return parsedStatement; } diff --git a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java index 34f70fa66..663b90bf2 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java @@ -16,6 +16,7 @@ package org.springframework.data.couchbase.repository.query; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import java.lang.reflect.Method; @@ -29,6 +30,7 @@ import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity; import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; import org.springframework.data.couchbase.core.query.Query; +import org.springframework.data.couchbase.core.query.StringQuery; import org.springframework.data.couchbase.domain.User; import org.springframework.data.couchbase.domain.UserRepository; import org.springframework.data.mapping.context.MappingContext; @@ -138,6 +140,59 @@ void createsQueryCorrectly2() throws Exception { query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, false, null, null)); } + @Test + void stringQuerycreatesQueryCorrectly() throws Exception { + String queryString = "a b c"; + Query query = new StringQuery(queryString); + assertEquals(queryString, query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, + false, null, null)); + } + + @Test + void stringQueryNoPositionalParameters() { + String queryString = " $1"; + Query query = new StringQuery(queryString); + assertThrows(IllegalArgumentException.class, () -> query.toN1qlSelectString(converter, bucketName(), null, null, + User.class, User.class, false, null, null)); + } + + @Test + void stringQueryNoNamedParameters() { + String queryString = " $george"; + Query query = new StringQuery(queryString); + assertThrows(IllegalArgumentException.class, () -> query.toN1qlSelectString(converter, bucketName(), null, null, + User.class, User.class, false, null, null)); + } + + @Test + void stringQueryNoSpelExpressions() { + String queryString = "#{#n1ql.filter}"; + Query query = new StringQuery(queryString); + assertThrows(IllegalArgumentException.class, () -> query.toN1qlSelectString(converter, bucketName(), null, null, + User.class, User.class, false, null, null)); + } + + @Test + void stringQueryNoPositionalParametersQuotes() { + String queryString = " '$1'"; + Query query = new StringQuery(queryString); + query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, false, null, null); + } + + @Test + void stringQueryNoNamedParametersQuotes() { + String queryString = " '$george'"; + Query query = new StringQuery(queryString); + query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, false, null, null); + } + + @Test + void stringQueryNoSpelExpressionsQuotes() { + String queryString = "'#{#n1ql.filter}'"; + Query query = new StringQuery(queryString); + query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, false, null, null); + } + @Test void spelTests() throws Exception { String input = "spelTests";