From e7474ae2d4f33d9293696b1c999fd20c48aa935c Mon Sep 17 00:00:00 2001 From: Michael Reiche <48999328+mikereiche@users.noreply.github.com> Date: Thu, 29 Jun 2023 11:55:48 -0700 Subject: [PATCH] Restore StringQuery constructor that takes only string. (#1770) Closes #1769. --- .../data/couchbase/core/query/Query.java | 34 ++++- .../couchbase/core/query/StringQuery.java | 24 ++-- .../query/StringBasedN1qlQueryParser.java | 70 ++++++++-- .../StringN1qlQueryCreatorMockedTests.java | 121 ++++++++++++++++++ 4 files changed, 223 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/springframework/data/couchbase/core/query/Query.java b/src/main/java/org/springframework/data/couchbase/core/query/Query.java index 37438b1f..e0ef4670 100644 --- a/src/main/java/org/springframework/data/couchbase/core/query/Query.java +++ b/src/main/java/org/springframework/data/couchbase/core/query/Query.java @@ -361,6 +361,22 @@ public class Query { return statement.toString(); } + public String toN1qlSelectString(CouchbaseConverter converter, String bucketName, String scopeName, + String collectionName, Class domainClass, Class returnClass, boolean isCount, String[] distinctFields, + String[] fields) { + StringBasedN1qlQueryParser.N1qlSpelValues n1ql = getN1qlSpelValues(converter, bucketName, scopeName, + collectionName, domainClass, returnClass, isCount, distinctFields, fields); + final StringBuilder statement = new StringBuilder(); + appendString(statement, n1ql.selectEntity); // select ... + appendWhereString(statement, n1ql.filter); // typeKey = typeValue + appendWhere(statement, new int[] { 0 }, converter); // criteria on this Query + if (!isCount) { + appendSort(statement); + appendSkipAndLimit(statement); + } + return statement.toString(); + } + public String toN1qlRemoveString(ReactiveCouchbaseTemplate template, String scopeName, String collectionName, Class domainClass) { StringBasedN1qlQueryParser.N1qlSpelValues n1ql = getN1qlSpelValues(template, scopeName, collectionName, domainClass, @@ -376,19 +392,27 @@ public class Query { public static StringBasedN1qlQueryParser.N1qlSpelValues getN1qlSpelValues(ReactiveCouchbaseTemplate template, String scopeName, String collectionName, Class domainClass, Class returnClass, boolean isCount, String[] distinctFields, String[] fields) { - String typeKey = template.getConverter().getTypeKey(); - final CouchbasePersistentEntity persistentEntity = template.getConverter().getMappingContext() + return getN1qlSpelValues(template.getConverter(), template.getBucketName(), scopeName, collectionName, + domainClass, returnClass, isCount, distinctFields, fields); + } + + public static StringBasedN1qlQueryParser.N1qlSpelValues getN1qlSpelValues(CouchbaseConverter converter, + String bucketName, + String scopeName, String collectionName, Class domainClass, Class returnClass, boolean isCount, + String[] distinctFields, String[] fields) { + String typeKey = converter.getTypeKey(); + final CouchbasePersistentEntity persistentEntity = converter.getMappingContext() .getRequiredPersistentEntity(domainClass); MappingCouchbaseEntityInformation info = new MappingCouchbaseEntityInformation<>(persistentEntity); String typeValue = info.getJavaType().getName(); TypeInformation typeInfo = ClassTypeInformation.from(info.getJavaType()); - Alias alias = template.getConverter().getTypeAlias(typeInfo); + Alias alias = converter.getTypeAlias(typeInfo); if (alias != null && alias.isPresent()) { typeValue = alias.toString(); } - StringBasedN1qlQueryParser sbnqp = new StringBasedN1qlQueryParser(template.getBucketName(), scopeName, - collectionName, template.getConverter(), domainClass, returnClass, typeKey, typeValue, isCount, distinctFields, + StringBasedN1qlQueryParser sbnqp = new StringBasedN1qlQueryParser(bucketName, scopeName, collectionName, + converter, domainClass, returnClass, typeKey, typeValue, isCount, distinctFields, fields); return sbnqp.getStatementContext(); } 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 4baf1289..45bbebd8 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 @@ -18,6 +18,7 @@ package org.springframework.data.couchbase.core.query; import java.util.Locale; import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate; +import org.springframework.data.couchbase.core.convert.CouchbaseConverter; import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity; import org.springframework.data.couchbase.core.support.TemplateUtils; import org.springframework.data.couchbase.repository.query.CouchbaseQueryMethod; @@ -65,11 +66,18 @@ public class StringQuery extends Query { this.spelExpressionParser = spelExpressionParser; } + public StringQuery(String n1qlString) { + this(null,n1qlString, null, null, null); + } + @Override public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String scope, String collection, Class domainClass, Class resultClass, boolean isCount, String[] distinctFields, String[] fields) { - - StringBasedN1qlQueryParser parser = getStringN1qlQueryParser(template, scope, collection, domainClass, + return toN1qlSelectString(template.getConverter(), template.getBucketName(), scope, collection, domainClass, resultClass, isCount, distinctFields, fields ); + } + public String toN1qlSelectString(CouchbaseConverter converter, String bucketName, String scope, String collection, + Class domainClass, Class resultClass, boolean isCount, String[] distinctFields, String[] fields) { + StringBasedN1qlQueryParser parser = getStringN1qlQueryParser(converter, bucketName, scope, collection, domainClass, distinctFields, fields); N1QLExpression parsedExpression = parser.getExpression(inlineN1qlQuery, queryMethod, parameterAccessor, @@ -100,7 +108,7 @@ public class StringQuery extends Query { } else { // named parameters or no parameters, no index required paramIndexPtr = new int[] { -1 }; } - appendWhere(statement, paramIndexPtr, template.getConverter()); // criteria on this Query - should be empty for + appendWhere(statement, paramIndexPtr, converter); // criteria on this Query - should be empty for if (!isCount) { appendSort(statement); appendSkipAndLimit(statement); @@ -111,21 +119,21 @@ public class StringQuery extends Query { return statement.toString(); } - private StringBasedN1qlQueryParser getStringN1qlQueryParser(ReactiveCouchbaseTemplate template, String scopeName, + private StringBasedN1qlQueryParser getStringN1qlQueryParser(CouchbaseConverter converter, String bucketName, String scopeName, String collectionName, Class domainClass, String[] distinctFields, String[] fields) { - String typeKey = template.getConverter().getTypeKey(); - final CouchbasePersistentEntity persistentEntity = template.getConverter().getMappingContext() + String typeKey = converter.getTypeKey(); + final CouchbasePersistentEntity persistentEntity = converter.getMappingContext() .getRequiredPersistentEntity(domainClass); MappingCouchbaseEntityInformation info = new MappingCouchbaseEntityInformation<>(persistentEntity); String typeValue = info.getJavaType().getName(); TypeInformation typeInfo = ClassTypeInformation.from(info.getJavaType()); - Alias alias = template.getConverter().getTypeAlias(typeInfo); + Alias alias = converter.getTypeAlias(typeInfo); if (alias != null && alias.isPresent()) { typeValue = alias.toString(); } // there are no options for distinct and fields for @Query StringBasedN1qlQueryParser sbnqp = new StringBasedN1qlQueryParser(inlineN1qlQuery, queryMethod, - template.getBucketName(), scopeName, collectionName, template.getConverter(), typeKey, typeValue, + bucketName, scopeName, collectionName, converter, typeKey, typeValue, parameterAccessor, new SpelExpressionParser(), evaluationContextProvider); return sbnqp; 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 d758cbbe..214b652b 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 @@ -36,6 +36,7 @@ import org.springframework.data.couchbase.core.convert.CouchbaseConverter; 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; @@ -120,6 +121,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 */ @@ -155,7 +162,8 @@ public class StringBasedN1qlQueryParser { 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); @@ -369,6 +377,9 @@ public class StringBasedN1qlQueryParser { 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()) { @@ -381,8 +392,14 @@ public class StringBasedN1qlQueryParser { 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 $ } @@ -391,8 +408,13 @@ public class StringBasedN1qlQueryParser { 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 $ } @@ -400,8 +422,7 @@ public class StringBasedN1qlQueryParser { 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) { @@ -411,12 +432,30 @@ public class StringBasedN1qlQueryParser { } 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; } } @@ -634,10 +673,15 @@ public class StringBasedN1qlQueryParser { 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/StringN1qlQueryCreatorMockedTests.java b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java index 6df5b249..6b8355cd 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.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 static org.springframework.data.couchbase.config.BeanNames.COUCHBASE_TEMPLATE; @@ -35,6 +36,7 @@ import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext; 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.couchbase.repository.config.EnableCouchbaseRepositories; @@ -108,6 +110,125 @@ class StringN1qlQueryCreatorMockedTests { fail("should have failed with IllegalArgumentException: query has no inline Query or named Query not found"); } + @Test + void createsQueryCorrectly() throws Exception { + String input = "getByFirstnameAndLastname"; + Method method = UserRepository.class.getMethod(input, String.class, String.class); + + CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, + new DefaultRepositoryMetadata(UserRepository.class), new SpelAwareProxyProjectionFactory(), + converter.getMappingContext()); + + StringN1qlQueryCreator creator = new StringN1qlQueryCreator(getAccessor(getParameters(method), "Oliver", "Twist"), + queryMethod, converter, new SpelExpressionParser(), QueryMethodEvaluationContextProvider.DEFAULT, namedQueries); + + Query query = creator.createQuery(); + assertEquals( + "SELECT `_class`, META(`" + bucketName() + + "`).`cas` AS __cas, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, META(`" + + bucketName() + "`).`id` AS __id, `firstname`, `lastname`, `subtype` FROM `" + bucketName() + + "` where `_class` = \"abstractuser\" and firstname = $1 and lastname = $2", + query.toN1qlSelectString(converter, bucketName(), null, null, User.class, User.class, false, null, null)); + } + + @Test + void createsQueryCorrectly2() throws Exception { + String input = "getByFirstnameOrLastname"; + Method method = UserRepository.class.getMethod(input, String.class, String.class); + + CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, + new DefaultRepositoryMetadata(UserRepository.class), new SpelAwareProxyProjectionFactory(), + converter.getMappingContext()); + + StringN1qlQueryCreator creator = new StringN1qlQueryCreator(getAccessor(getParameters(method), "Oliver", "Twist"), + queryMethod, converter, new SpelExpressionParser(), QueryMethodEvaluationContextProvider.DEFAULT, namedQueries); + + Query query = creator.createQuery(); + assertEquals( + "SELECT `_class`, META(`" + bucketName() + + "`).`cas` AS __cas, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, META(`" + + bucketName() + "`).`id` AS __id, `firstname`, `lastname`, `subtype` FROM `" + bucketName() + + "` where `_class` = \"abstractuser\" and (firstname = $first or lastname = $last)", + 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"; + Method method = UserRepository.class.getMethod(input); + CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, + new DefaultRepositoryMetadata(UserRepository.class), new SpelAwareProxyProjectionFactory(), + converter.getMappingContext()); + + StringN1qlQueryCreator creator = new StringN1qlQueryCreator(getAccessor(getParameters(method)), queryMethod, + converter, new SpelExpressionParser(), QueryMethodEvaluationContextProvider.DEFAULT, namedQueries); + + Query query = creator.createQuery(); + + assertEquals("SELECT `_class`, META(`myCollection`).`cas`" + + " AS __cas, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, META(`myCollection`).`id`" + + " AS __id, `firstname`, `lastname`, `subtype` FROM `myCollection`|`_class` = \"abstractuser\"|`myCollection`|`myScope`|`myCollection`", + query.toN1qlSelectString(converter, bucketName(), "myScope", "myCollection", User.class, null, false, null, + null)); + } + + private String bucketName() { + return "some_bucket"; + } + private ParameterAccessor getAccessor(Parameters params, Object... values) { return new ParametersParameterAccessor(params, values); }