From e0aefcbeb98e1a1283f142ed8eb1cff071c0e5c9 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 19 Jan 2018 15:11:45 +0100 Subject: [PATCH] DATAJPA-1235 - Quoted sections in manually declared queries are now detected correctly. Unified the query parsing logic into a new abstraction DeclaredQuery with StringQuery being it's primary implementation. A QuotationMap value object now detects quotes in the query to then allow StringQuery to decide whether a named parameter candidate is in a quoted range to drop it if that's the case. Original pull request: #244. --- .../query/AbstractStringBasedJpaQuery.java | 16 +-- .../jpa/repository/query/DeclaredQuery.java | 87 ++++++++++++ .../repository/query/EmptyDeclaredQuery.java | 73 +++++++++++ .../jpa/repository/query/JpaQueryMethod.java | 2 +- .../query/ParameterBinderFactory.java | 18 ++- .../query/QueryParameterSetterFactory.java | 17 ++- .../data/jpa/repository/query/QueryUtils.java | 58 ++++---- .../jpa/repository/query/QuotationMap.java | 76 +++++++++++ .../jpa/repository/query/StringQuery.java | 45 ++++++- .../jpa/repository/UserRepositoryTests.java | 18 +++ .../QueryParameterSetterFactoryUnitTests.java | 4 +- .../repository/query/QueryUtilsUnitTests.java | 44 +------ .../query/QuotationMapUnitTests.java | 124 ++++++++++++++++++ .../query/StringQueryUnitTests.java | 109 +++++++++++++++ .../jpa/repository/sample/UserRepository.java | 4 + 15 files changed, 590 insertions(+), 105 deletions(-) create mode 100644 src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java create mode 100644 src/main/java/org/springframework/data/jpa/repository/query/EmptyDeclaredQuery.java create mode 100644 src/main/java/org/springframework/data/jpa/repository/query/QuotationMap.java create mode 100644 src/test/java/org/springframework/data/jpa/repository/query/QuotationMapUnitTests.java diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 184815a45..b46f2c7e0 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -36,8 +36,8 @@ import org.springframework.util.Assert; */ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { - private final StringQuery query; - private final StringQuery countQuery; + private final DeclaredQuery query; + private final DeclaredQuery countQuery; private final EvaluationContextProvider evaluationContextProvider; private final SpelExpressionParser parser; @@ -62,8 +62,8 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { this.evaluationContextProvider = evaluationContextProvider; this.query = new ExpressionBasedStringQuery(queryString, method.getEntityInformation(), parser); - this.countQuery = new StringQuery(method.getCountQuery() != null ? method.getCountQuery() - : QueryUtils.createCountQueryFor(this.query.getQueryString(), method.getCountQueryProjection())); + this.countQuery = query.deriveCountQuery(method.getCountQuery(), method.getCountQueryProjection()); + this.parser = parser; } @@ -91,8 +91,8 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { @Override protected ParameterBinder createBinder() { - return ParameterBinderFactory.createQueryAwareBinder(getQueryMethod().getParameters(), query, - parser, evaluationContextProvider); + return ParameterBinderFactory.createQueryAwareBinder(getQueryMethod().getParameters(), query, parser, + evaluationContextProvider); } /* @@ -113,14 +113,14 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { /** * @return the query */ - public StringQuery getQuery() { + public DeclaredQuery getQuery() { return query; } /** * @return the countQuery */ - public StringQuery getCountQuery() { + public DeclaredQuery getCountQuery() { return countQuery; } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java new file mode 100644 index 000000000..b67f4b9c9 --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java @@ -0,0 +1,87 @@ +/* + * Copyright 2018 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.jpa.repository.query; + +import java.util.List; + +import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; + +/** + * A wrapper for a String representation of a query offering information about the query. + * + * @author Jens Schauder + */ +interface DeclaredQuery { + + /** + * Creates a {@literal DeclaredQuery} from a query {@literal String}. + * + * @param query might be {@literal null} or empty. + * + * @return a {@literal DeclaredQuery} instance even for a {@literal null} or empty argument. + */ + static DeclaredQuery of(@Nullable String query) { + return StringUtils.isEmpty(query) ? EmptyDeclaredQuery.EMPTY_QUERY : new StringQuery(query); + } + + /** + * @return whether the underlying query has at least one named parameter. + */ + boolean hasNamedParameter(); + + /** + * Returns the query string. + */ + String getQueryString(); + + /** + * Returns the main alias used in the query. + * + * @return the alias + */ + @Nullable + String getAlias(); + + /** + * Returns whether the query is using a constructor expression. + * + * @since 1.10 + */ + boolean hasConstructorExpression(); + + /** + * Returns whether the query uses the default projection, i.e. returns the main alias defined for the query. + */ + boolean isDefaultProjection(); + + /** + * Returns the {@link StringQuery.ParameterBinding}s registered. + */ + List getParameterBindings(); + + /** + * Creates a new {@literal DeclaredQuery} representing a count query, i.e. a query returning the number of rows to be + * expected from the original query, either derived from the query wrapped by this instance or from the information + * passed as arguments. + * + * @param countQuery an optional query string to be used if present. + * @param countQueryProjection an optional return type for the query. + * @return A new {@literal DeclaredQuery} instance. + */ + DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection); + +} diff --git a/src/main/java/org/springframework/data/jpa/repository/query/EmptyDeclaredQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/EmptyDeclaredQuery.java new file mode 100644 index 000000000..e8a8d495a --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/repository/query/EmptyDeclaredQuery.java @@ -0,0 +1,73 @@ +/* + * Copyright 2018 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.jpa.repository.query; + +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +import java.util.Collections; +import java.util.List; + +/** + * NULL-Object pattern implementation. + * + * @author Jens Schauder + */ +class EmptyDeclaredQuery implements DeclaredQuery { + + /** + * An implementation implementing the NULL-Object pattern for situations where there is no query. + */ + static final DeclaredQuery EMPTY_QUERY = new EmptyDeclaredQuery(); + + @Override + public boolean hasNamedParameter() { + return false; + } + + @Override + public String getQueryString() { + return ""; + } + + @Override + public String getAlias() { + return null; + } + + @Override + public boolean hasConstructorExpression() { + return false; + } + + @Override + public boolean isDefaultProjection() { + return false; + } + + @Override + public List getParameterBindings() { + return Collections.emptyList(); + } + + @Override + public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection) { + + Assert.hasText(countQuery, "CountQuery must not be empty!"); + + return DeclaredQuery.of(countQuery); + } +} diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java index 7534bd1ba..0db7162a4 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java @@ -107,7 +107,7 @@ public class JpaQueryMethod extends QueryMethod { String annotatedQuery = getAnnotatedQuery(); - if (!QueryUtils.hasNamedParameter(annotatedQuery)) { + if (!DeclaredQuery.of(annotatedQuery).hasNamedParameter()) { return; } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinderFactory.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinderFactory.java index 9d175eabc..4e16d3371 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinderFactory.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinderFactory.java @@ -26,7 +26,6 @@ import org.springframework.data.jpa.repository.query.StringQuery.ParameterBindin import org.springframework.data.repository.query.EvaluationContextProvider; import org.springframework.data.util.StreamUtils; import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -88,7 +87,7 @@ class ParameterBinderFactory { * @return a {@link ParameterBinder} that can assign values for the method parameters to query parameters of a * {@link javax.persistence.Query} while processing SpEL expressions where applicable. */ - static ParameterBinder createQueryAwareBinder(JpaParameters parameters, StringQuery query, + static ParameterBinder createQueryAwareBinder(JpaParameters parameters, DeclaredQuery query, SpelExpressionParser parser, EvaluationContextProvider evaluationContextProvider) { Assert.notNull(parameters, "JpaParameters must not be null!"); @@ -101,8 +100,7 @@ class ParameterBinderFactory { evaluationContextProvider, parameters); QueryParameterSetterFactory basicSetterFactory = QueryParameterSetterFactory.basic(parameters); - return new ParameterBinder(parameters, - createSetters(query.getQueryString(), bindings, expressionSetterFactory, basicSetterFactory)); + return new ParameterBinder(parameters, createSetters(bindings, query, expressionSetterFactory, basicSetterFactory)); } private static List getBindings(JpaParameters parameters) { @@ -122,22 +120,22 @@ class ParameterBinderFactory { private static Iterable createSetters(List parameterBindings, QueryParameterSetterFactory... factories) { - return createSetters(null, parameterBindings, factories); + return createSetters(parameterBindings, EmptyDeclaredQuery.EMPTY_QUERY, factories); } - private static Iterable createSetters(@Nullable String queryString, - List parameterBindings, QueryParameterSetterFactory... strategies) { + private static Iterable createSetters(List parameterBindings, + DeclaredQuery declaredQuery, QueryParameterSetterFactory... strategies) { return parameterBindings.stream() // - .map(it -> createQueryParameterSetter(it, strategies, queryString)) // + .map(it -> createQueryParameterSetter(it, strategies, declaredQuery)) // .collect(StreamUtils.toUnmodifiableList()); } private static QueryParameterSetter createQueryParameterSetter(ParameterBinding binding, - QueryParameterSetterFactory[] strategies, @Nullable String queryString) { + QueryParameterSetterFactory[] strategies, DeclaredQuery declaredQuery) { return Arrays.stream(strategies)// - .map(it -> it.create(binding, queryString))// + .map(it -> it.create(binding, declaredQuery))// .filter(Objects::nonNull)// .findFirst()// .orElse(QueryParameterSetter.NOOP); diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java index b0a4ef203..14ff20dab 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java @@ -46,7 +46,7 @@ import org.springframework.util.Assert; abstract class QueryParameterSetterFactory { @Nullable - abstract QueryParameterSetter create(ParameterBinding binding, @Nullable String queryString); + abstract QueryParameterSetter create(ParameterBinding binding, DeclaredQuery declaredQuery); /** * Creates a new {@link QueryParameterSetterFactory} for the given {@link JpaParameters}. @@ -104,10 +104,9 @@ abstract class QueryParameterSetterFactory { * @param valueExtractor extracts the relevant value from an array of method parameter values. * @param binding the binding of the query parameter to be set. * @param parameter the method parameter to bind. - * @param lenient when true certain exceptions thrown when setting the query parameters get ignored. */ private static QueryParameterSetter createSetter(Function valueExtractor, ParameterBinding binding, - @Nullable JpaParameter parameter, boolean lenient) { + @Nullable JpaParameter parameter) { TemporalType temporalType = parameter != null && parameter.isTemporalParameter() // ? parameter.getRequiredTemporalType() // @@ -153,7 +152,7 @@ abstract class QueryParameterSetterFactory { */ @Nullable @Override - public QueryParameterSetter create(ParameterBinding binding, @Nullable String queryString) { + public QueryParameterSetter create(ParameterBinding binding, DeclaredQuery declaredQuery) { if (!binding.isExpression()) { return null; @@ -161,7 +160,7 @@ abstract class QueryParameterSetterFactory { Expression expression = parser.parseExpression(binding.getExpression()); - return createSetter(values -> evaluateExpression(expression, values), binding, null, true); + return createSetter(values -> evaluateExpression(expression, values), binding, null); } /** @@ -205,17 +204,17 @@ abstract class QueryParameterSetterFactory { * @see org.springframework.data.jpa.repository.query.QueryParameterSetterFactory#create(org.springframework.data.jpa.repository.query.StringQuery.ParameterBinding, java.lang.String) */ @Override - public QueryParameterSetter create(ParameterBinding binding, @Nullable String queryString) { + public QueryParameterSetter create(ParameterBinding binding, DeclaredQuery declaredQuery) { Assert.notNull(binding, "Binding must not be null."); - JpaParameter parameter = QueryUtils.hasNamedParameter(queryString) // + JpaParameter parameter = declaredQuery.hasNamedParameter() // ? findParameterForBinding(binding) // : parameters.getBindableParameter(binding.getRequiredPosition() - 1); return parameter == null // ? QueryParameterSetter.NOOP // - : createSetter(values -> getValue(values, parameter), binding, parameter, false); + : createSetter(values -> getValue(values, parameter), binding, parameter); } @Nullable @@ -267,7 +266,7 @@ abstract class QueryParameterSetterFactory { * @see org.springframework.data.jpa.repository.query.QueryParameterSetterFactory#create(org.springframework.data.jpa.repository.query.StringQuery.ParameterBinding, java.lang.String) */ @Override - public QueryParameterSetter create(ParameterBinding binding, @Nullable String queryString) { + public QueryParameterSetter create(ParameterBinding binding, DeclaredQuery declaredQuery) { ParameterMetadata metadata = expressions.get(binding.getRequiredPosition() - 1); diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index ed8911797..e46dde11e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -73,6 +73,7 @@ import org.springframework.util.StringUtils; * @author Christoph Strobl * @author Mark Paluch * @author Sébastien Péralta + * @author Jens Schauder */ public abstract class QueryUtils { @@ -84,7 +85,7 @@ public abstract class QueryUtils { // Cc Control // Cf Format // P Punctuation - static final String IDENTIFIER = "[._[\\P{Z}&&\\P{Cc}&&\\P{Cf}&&\\P{P}]]+"; + private static final String IDENTIFIER = "[._[\\P{Z}&&\\P{Cc}&&\\P{Cf}&&\\P{P}]]+"; static final String COLON_NO_DOUBLE_COLON = "(? idAttributes) { @@ -202,9 +202,9 @@ public abstract class QueryUtils { /** * Returns the query string for the given class name. * - * @param template - * @param entityName - * @return + * @param template must not be {@literal null}. + * @param entityName must not be {@literal null}. + * @return the template with placeholders replaced by the {@literal entityName}. Guaranteed to be not {@literal null}. */ public static String getQueryString(String template, String entityName) { @@ -264,10 +264,10 @@ public abstract class QueryUtils { * Returns the order clause for the given {@link Order}. Will prefix the clause with the given alias if the referenced * property refers to a join alias, i.e. starts with {@code $alias.}. * - * @param joinAliases the join aliases of the original query. - * @param alias the alias for the root entity. - * @param order the order object to build the clause for. - * @return + * @param joinAliases the join aliases of the original query. Must not be {@literal null}. + * @param alias the alias for the root entity. May be {@literal null}. + * @param order the order object to build the clause for. Must not be {@literal null}. + * @return a String containing a order clause. Guaranteed to be not {@literal null}. */ private static String getOrderClause(Set joinAliases, Set functionAlias, @Nullable String alias, Order order) { @@ -299,8 +299,8 @@ public abstract class QueryUtils { /** * Returns the aliases used for {@code left (outer) join}s. * - * @param query - * @return + * @param query a query string to extract the aliases of joins from. Must not be {@literal null}. + * @return a {@literal Set} of aliases used in the query. Guaranteed to be not {@literal null}. */ static Set getOuterJoinAliases(String query) { @@ -321,12 +321,12 @@ public abstract class QueryUtils { /** * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}. * - * @param query - * @return + * @param query a {@literal String} containing a query. Must not be {@literal null}. + * @return a {@literal Set} containing all found aliases. Guaranteed to be not {@literal null}. */ private static Set getFunctionAliases(String query) { - Set result = new HashSet(); + Set result = new HashSet<>(); Matcher matcher = FUNCTION_PATTERN.matcher(query); while (matcher.find()) { @@ -348,10 +348,12 @@ public abstract class QueryUtils { /** * Resolves the alias for the entity to be retrieved from the given JPA query. * - * @param query - * @return + * @param query must not be {@literal null}. + * @return Might return {@literal null}. + * @deprecated use {@link DeclaredQuery#getAlias()} instead. */ @Nullable + @Deprecated public static String detectAlias(String query) { Matcher matcher = ALIAS_MATCH.matcher(query); @@ -363,12 +365,13 @@ public abstract class QueryUtils { * Creates a where-clause referencing the given entities and appends it to the given query string. Binds the given * entities to the query. * - * @param + * @param type of the entities. * @param queryString must not be {@literal null}. * @param entities must not be {@literal null}. * @param entityManager must not be {@literal null}. - * @return + * @return Guaranteed to be not {@literal null}. */ + public static Query applyAndBind(String queryString, Iterable entities, EntityManager entityManager) { Assert.notNull(queryString, "Querystring must not be null!"); @@ -414,8 +417,10 @@ public abstract class QueryUtils { * Creates a count projected query from the given original query. * * @param originalQuery must not be {@literal null} or empty. - * @return + * @return Guaranteed to be not {@literal null}. + * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead. */ + @Deprecated public static String createCountQueryFor(String originalQuery) { return createCountQueryFor(originalQuery, null); } @@ -425,9 +430,11 @@ public abstract class QueryUtils { * * @param originalQuery must not be {@literal null}. * @param countProjection may be {@literal null}. - * @return + * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}. * @since 1.6 + * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead. */ + @Deprecated public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) { Assert.hasText(originalQuery, "OriginalQuery must not be null or empty!"); @@ -453,8 +460,8 @@ public abstract class QueryUtils { /** * Returns whether the given {@link Query} contains named parameters. * - * @param query - * @return + * @param query Must not be {@literal null}. + * @return whether the given {@link Query} contains named parameters. */ public static boolean hasNamedParameter(Query query) { @@ -477,9 +484,10 @@ public abstract class QueryUtils { * Returns whether the given query contains named parameters. * * @param query can be {@literal null} or empty. - * @return + * @return whether the given query contains named parameters. */ - public static boolean hasNamedParameter(@Nullable String query) { + @Deprecated + static boolean hasNamedParameter(@Nullable String query) { return StringUtils.hasText(query) && NAMED_PARAMETER.matcher(query).find(); } @@ -493,7 +501,7 @@ public abstract class QueryUtils { */ public static List toOrders(Sort sort, From from, CriteriaBuilder cb) { - List orders = new ArrayList(); + List orders = new ArrayList<>(); if (sort.isUnsorted()) { return orders; diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QuotationMap.java b/src/main/java/org/springframework/data/jpa/repository/query/QuotationMap.java new file mode 100644 index 000000000..5830815dc --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/repository/query/QuotationMap.java @@ -0,0 +1,76 @@ +/* + * Copyright 2018 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.jpa.repository.query; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.springframework.data.domain.Range; +import org.springframework.lang.Nullable; + +/** + * Datastructure that analyses a String to determine the parts of the String that are quoted and offers an API to query + * that information. + * + * @author Jens Schauder + */ +class QuotationMap { + + private static final Set QUOTING_CHARACTERS = new HashSet<>(Arrays.asList('"', '\'')); + private List> quotedRanges = new ArrayList<>(); + + QuotationMap(@Nullable String query) { + + if (query == null) + return; + + Character inQuotation = null; + int start = 0; + + for (int i = 0; i < query.length(); i++) { + + char currentChar = query.charAt(i); + if (QUOTING_CHARACTERS.contains(currentChar)) { + + if (inQuotation == null) { + + inQuotation = currentChar; + start = i; + } else if (currentChar == inQuotation) { + + inQuotation = null; + quotedRanges.add(Range.of(Range.Bound.inclusive(start), Range.Bound.inclusive(i))); + } + } + } + + if (inQuotation != null) { + throw new IllegalArgumentException( + String.format("The string <%s> starts a quoted range at %d, but never ends it.", query, start)); + } + } + + /** + * @param index to check if it is part of a quoted range. + * @return whether the query contains a quoted range at {@literal index}. + */ + public boolean isQuoted(int index) { + return quotedRanges.stream().anyMatch(r -> r.contains(index)); + } +} diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index e65bfd0de..5c2f4e6f8 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -44,7 +44,7 @@ import org.springframework.util.StringUtils; * @author Mark Paluch * @author Jens Schauder */ -class StringQuery { +class StringQuery implements DeclaredQuery { private final String query; private final List bindings; @@ -63,6 +63,7 @@ class StringQuery { this.bindings = new ArrayList<>(); this.query = ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query, this.bindings); + this.alias = QueryUtils.detectAlias(query); this.hasConstructorExpression = QueryUtils.hasConstructorExpression(query); } @@ -77,14 +78,22 @@ class StringQuery { /** * Returns the {@link ParameterBinding}s registered. */ - List getParameterBindings() { + public List getParameterBindings() { return bindings; } + @Override + public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection) { + + return DeclaredQuery + .of(countQuery != null ? countQuery : QueryUtils.createCountQueryFor(query, countQueryProjection)); + } + /** * Returns the query string. */ - String getQueryString() { + @Override + public String getQueryString() { return query; } @@ -93,8 +102,9 @@ class StringQuery { * * @return the alias */ + @Override @Nullable - String getAlias() { + public String getAlias() { return alias; } @@ -103,15 +113,28 @@ class StringQuery { * * @since 1.10 */ - boolean hasConstructorExpression() { + @Override + public boolean hasConstructorExpression() { return hasConstructorExpression; } /** * Returns whether the query uses the default projection, i.e. returns the main alias defined for the query. */ - boolean isDefaultProjection() { - return QueryUtils.getProjection(query).equals(alias); + @Override + public boolean isDefaultProjection() { + return getProjection().equals(alias); + } + + public String getProjection() { + return QueryUtils.getProjection(query); + } + + @Override + public boolean hasNamedParameter() { + + return bindings.stream() // + .anyMatch(b -> b.getName() != null); } /** @@ -191,8 +214,14 @@ class StringQuery { */ int expressionParameterIndex = parametersShouldBeAccessedByIndex ? greatestParameterIndex : 0; + QuotationMap quotationMap = new QuotationMap(query); + while (matcher.find()) { + if (quotationMap.isQuoted(matcher.start())) { + continue; + } + String parameterIndexString = matcher.group(INDEXED_PARAMETER_GROUP); String parameterName = parameterIndexString != null ? null : matcher.group(NAMED_PARAMETER_GROUP); Integer parameterIndex = parameterIndexString == null ? null : Integer.valueOf(parameterIndexString); @@ -341,6 +370,7 @@ class StringQuery { throw new IllegalArgumentException(String.format("Unsupported parameter binding type %s!", typeSource)); } } + } /** @@ -729,4 +759,5 @@ class StringQuery { return Type.LIKE; } } + } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index f10d814a6..a2543a023 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -2059,6 +2059,24 @@ public class UserRepositoryTests { assertThat(result.getLastname()).isEqualTo(user.getLastname()); } + @Test //DATAJPA-1235 + public void handlesColonsFollowedByIntegerInStringLiteral(){ + + String firstName = "aFirstName"; + + User expected = new User(firstName, "000:1", "something@something"); + User notExpected = new User(firstName, "000\\:1", "something@something.else"); + + repository.save(expected); + repository.save(notExpected); + + assertThat(repository.findAll()).hasSize(2); + + List users = repository.queryWithIndexedParameterAndColonFollowedByIntegerInString(firstName); + + assertThat(users).extracting(User::getId).containsExactly(expected.getId()); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java index e33ae434e..ebd0812eb 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java @@ -49,14 +49,14 @@ public class QueryParameterSetterFactoryUnitTests { @Test // DATAJPA-1058 public void noExceptionWhenQueryDoesNotContainNamedParameters() { - setterFactory.create(binding, "QueryStringWithOutNamedParameter"); + setterFactory.create(binding, DeclaredQuery.of("QueryStringWithOutNamedParameter")); } @Test // DATAJPA-1058 public void exceptionWhenQueryContainNamedParametersAndMethodParametersAreNotNamed() { Assertions.assertThatExceptionOfType(IllegalStateException.class) // - .isThrownBy(() -> setterFactory.create(binding, "QueryStringWith :NamedParameter")) // + .isThrownBy(() -> setterFactory.create(binding, DeclaredQuery.of("QueryStringWith :NamedParameter"))) // .withMessageContaining("Java 8") // .withMessageContaining("@Param") // .withMessageContaining("-parameters"); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java index 5a833ada4..647e88c3f 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java @@ -23,7 +23,6 @@ import static org.springframework.data.jpa.repository.query.QueryUtils.*; import java.util.Collections; import java.util.Set; -import org.assertj.core.api.SoftAssertions; import org.hamcrest.Matcher; import org.junit.Test; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -38,6 +37,7 @@ import org.springframework.data.jpa.domain.JpaSort; * @author Thomas Darimont * @author Komi Innocent * @author Christoph Strobl + * @author Jens Schauder */ public class QueryUtilsUnitTests { @@ -395,48 +395,6 @@ public class QueryUtilsUnitTests { .endsWith("WHERE x.id = :id"); } - @Test // DATAJPA-1200 - public void testHasNamedParameter() { - - SoftAssertions softly = new SoftAssertions(); - - checkHasNamedParameter(softly, "select something from x where id = :id", true, "named parameter"); - checkHasNamedParameter(softly, "in the :id middle", true, "middle"); - checkHasNamedParameter(softly, ":id start", true, "beginning"); - checkHasNamedParameter(softly, ":id", true, "alone"); - checkHasNamedParameter(softly, "select something from x where id = :id", true, "named parameter"); - checkHasNamedParameter(softly, "select something from x where id = #something", true, "hash"); - checkHasNamedParameter(softly, ":UPPERCASE", true, "uppercase"); - checkHasNamedParameter(softly, ":lowercase", true, "lowercase"); - checkHasNamedParameter(softly, ":2something", true, "beginning digit"); - checkHasNamedParameter(softly, ":2", true, "only digit"); - checkHasNamedParameter(softly, ":.something", true, "dot"); - checkHasNamedParameter(softly, ":_something", true, "underscore"); - checkHasNamedParameter(softly, ":$something", true, "dollar"); - checkHasNamedParameter(softly, ":\uFE0F", true, "non basic latin emoji"); // - checkHasNamedParameter(softly, ":\u4E01", true, "chinese japanese korean"); - - checkHasNamedParameter(softly, "no bind variable", false, "no bind variable"); - checkHasNamedParameter(softly, ":\u2004whitespace", false, "non basic latin whitespace"); - checkHasNamedParameter(softly, "select something from x where id = ?1", false, "indexed parameter"); - checkHasNamedParameter(softly, "::", false, "double colon"); - checkHasNamedParameter(softly, ":", false, "end of query"); - checkHasNamedParameter(softly, ":\u0003", false, "non-printable"); - checkHasNamedParameter(softly, ":*", false, "basic latin emoji"); - checkHasNamedParameter(softly, "\\:", false, "escaped colon"); - checkHasNamedParameter(softly, "::id", false, "double colon with identifier"); - checkHasNamedParameter(softly, "\\:id", false, "escaped colon with identifier"); - - softly.assertAll(); - } - - private static void checkHasNamedParameter(SoftAssertions softly, String query, boolean expected, String label) { - - softly.assertThat(QueryUtils.hasNamedParameter(query)) // - .describedAs(String.format("<%s> (%s)", query, label)) // - .isEqualTo(expected); - } - private static void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery), is(countQuery)); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QuotationMapUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QuotationMapUnitTests.java new file mode 100644 index 000000000..b5ed12079 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/QuotationMapUnitTests.java @@ -0,0 +1,124 @@ +/* + * Copyright 2018 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.jpa.repository.query; + +import static org.assertj.core.api.Assertions.*; + +import org.assertj.core.api.SoftAssertions; +import org.junit.Test; + +/** + * Unit tests for {@link QuotationMap}. + * + * @author Jens Schauder + */ +public class QuotationMapUnitTests { + + SoftAssertions softly = new SoftAssertions(); + + @Test // DATAJPA-1235 + public void emptyStringDoesNotContainQuotes() { + isNotQuoted("", "empty String", -1, 0, 1); + } + + @Test // DATAJPA-1235 + public void nullStringDoesNotContainQuotes() { + isNotQuoted(null, "null String", -1, 0, 1); + } + + @Test // DATAJPA-1235 + public void simpleStringDoesNotContainQuotes() { + String query = "something"; + isNotQuoted(query, "simple String", -1, 0, query.length() - 1, query.length(), query.length() + 1); + } + + @Test // DATAJPA-1235 + public void fullySingleQuotedStringDoesContainQuotes() { + + String query = "'something'"; + isNotQuoted(query, "quoted String", -1, query.length()); + isQuoted(query, "quoted String", 0, 1, 5, query.length() - 1); + } + + @Test // DATAJPA-1235 + public void fullyDoubleQuotedStringDoesContainQuotes() { + + String query = "\"something\""; + isNotQuoted(query, "double quoted String", -1, query.length()); + isQuoted(query, "double quoted String", 0, 1, 5, query.length() - 1); + } + + @Test // DATAJPA-1235 + public void stringWithEmptyQuotes() { + + String query = "abc''def"; + isNotQuoted(query, "zero length quote", -1, 0, 1, 2, 5, 6, 7); + isQuoted(query, "zero length quote", 3, 4); + } + + @Test // DATAJPA-1235 + public void doubleInSingleQuotes() { + + String query = "abc'\"'def"; + isNotQuoted(query, "double inside single quote", -1, 0, 1, 2, 6, 7, 8); + isQuoted(query, "double inside single quote", 3, 4, 5); + } + + @Test // DATAJPA-1235 + public void singleQuotesInDoubleQuotes() { + + String query = "abc\"'\"def"; + isNotQuoted(query, "single inside double quote", -1, 0, 1, 2, 6, 7, 8); + isQuoted(query, "single inside double quote", 3, 4, 5); + } + + @Test // DATAJPA-1235 + public void escapedQuotes() { + + String query = "a'b''cd''e'f"; + isNotQuoted(query, "escaped quote", -1, 0, 11, 12); + isQuoted(query, "escaped quote", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); + } + + @Test // DATAJPA-1235 + public void openEndedQuoteThrowsException() { + + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> new QuotationMap("a'b")); + } + + public void isNotQuoted(String query, Object label, int... indexes) { + + QuotationMap quotationMap = new QuotationMap(query); + + for (int index : indexes) { + + assertThat(quotationMap.isQuoted(index)) + .describedAs(String.format("(%s) %s does not contain a quote at %s", label, query, index)) // + .isFalse(); + } + } + + public void isQuoted(String query, Object label, int... indexes) { + + QuotationMap quotationMap = new QuotationMap(query); + + for (int index : indexes) { + + assertThat(quotationMap.isQuoted(index)) + .describedAs(String.format("(%s) %s does contain a quote at %s", label, query, index)).isTrue(); + } + } +} diff --git a/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java index 8d7871323..2f5233f50 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java @@ -20,6 +20,7 @@ import static org.junit.Assert.*; import java.util.List; +import org.assertj.core.api.SoftAssertions; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -39,6 +40,8 @@ public class StringQueryUnitTests { public @Rule ExpectedException exception = ExpectedException.none(); + SoftAssertions softly = new SoftAssertions(); + @Test // DATAJPA-341 public void doesNotConsiderPlainLikeABinding() { @@ -306,6 +309,112 @@ public class StringQueryUnitTests { } } + @Test // DATAJPA-1235 + public void getProjection() { + + checkProjection("SELECT something FROM", "", "only lowercase is supported (!?)"); + checkProjection("select something from", "something", "single expression"); + checkProjection("select x, y, z from", "x, y, z", "tuple"); + checkProjection("sect x, y, z from", "", "missing select"); + checkProjection("select x, y, z fron", "", "missing from"); + + softly.assertAll(); + } + + void checkProjection(String query, String expected, String description) { + + softly.assertThat(new StringQuery(query).getProjection()) // + .as("%s (%s)", description, query) // + .isEqualTo(expected); + } + + @Test // DATAJPA-1235 + public void getAlias() { + + checkAlias("from User u", "u", "simple query"); + checkAlias("select count(u) from User u", "u", "count query"); + checkAlias("select u from User as u where u.username = ?", "u", "with as"); + checkAlias("SELECT FROM USER U", "U", "uppercase"); + checkAlias("select u from User u", "u", "simple query"); + checkAlias("select u from com.acme.User u", "u", "fully qualified package name"); + checkAlias("select u from T05User u", "u", "intersting entity name"); + + softly.assertAll(); + } + + private void checkAlias(String query, String expected, String description) { + + softly.assertThat(new StringQuery(query).getAlias()) // + .as("%s (%s)", description, query) // + .isEqualTo(expected); + } + + @Test // DATAJPA-1200 + public void testHasNamedParameter() { + + SoftAssertions softly = new SoftAssertions(); + + checkHasNamedParameter("select something from x where id = :id", true, "named parameter"); + checkHasNamedParameter("in the :id middle", true, "middle"); + checkHasNamedParameter(":id start", true, "beginning"); + checkHasNamedParameter(":id", true, "alone"); + checkHasNamedParameter("select something from x where id = :id", true, "named parameter"); + checkHasNamedParameter("select something from x where id = #something", true, "hash"); + checkHasNamedParameter(":UPPERCASE", true, "uppercase"); + checkHasNamedParameter(":lowercase", true, "lowercase"); + checkHasNamedParameter(":2something", true, "beginning digit"); + checkHasNamedParameter(":2", true, "only digit"); + checkHasNamedParameter(":.something", true, "dot"); + checkHasNamedParameter(":_something", true, "underscore"); + checkHasNamedParameter(":$something", true, "dollar"); + checkHasNamedParameter(":\uFE0F", true, "non basic latin emoji"); // + checkHasNamedParameter(":\u4E01", true, "chinese japanese korean"); + + checkHasNamedParameter("no bind variable", false, "no bind variable"); + checkHasNamedParameter(":\u2004whitespace", false, "non basic latin whitespace"); + checkHasNamedParameter("select something from x where id = ?1", false, "indexed parameter"); + checkHasNamedParameter("::", false, "double colon"); + checkHasNamedParameter(":", false, "end of query"); + checkHasNamedParameter(":\u0003", false, "non-printable"); + checkHasNamedParameter(":*", false, "basic latin emoji"); + checkHasNamedParameter("\\:", false, "escaped colon"); + checkHasNamedParameter("::id", false, "double colon with identifier"); + checkHasNamedParameter("\\:id", false, "escaped colon with identifier"); + + softly.assertAll(); + } + + @Test // DATAJPA-1235 + public void ignoresQuotedNamedParameterLookAlike() { + + checkNumberOfNamedParameters("select something from blah where x = '0:name'", 0, "single quoted"); + checkNumberOfNamedParameters("select something from blah where x = \"0:name\"", 0, "double quoted"); + checkNumberOfNamedParameters("select something from blah where x = '\"0':name", 1, "double quote in single quotes"); + checkNumberOfNamedParameters("select something from blah where x = \"'0\":name", 1, + "single quote in double quotes"); + + softly.assertAll(); + } + + public void checkNumberOfNamedParameters(String query, int expectedSize, String label) { + + DeclaredQuery declaredQuery = DeclaredQuery.of(query); + + softly.assertThat(declaredQuery.hasNamedParameter()) // + .describedAs("hasNamed Parameter " + label) // + .isEqualTo(expectedSize > 0); + softly.assertThat(declaredQuery.getParameterBindings()) // + .describedAs("parameterBindings " + label) // + .hasSize(expectedSize); + } + + private void checkHasNamedParameter(String query, boolean expected, String label) { + + softly.assertThat(new StringQuery(query).hasNamedParameter()) // + .describedAs(String.format("<%s> (%s)", query, label)) // + .isEqualTo(expected); + } + private void assertPositionalBinding(Class bindingType, Integer position, ParameterBinding expectedBinding) { diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index 2d54d31cb..66944af65 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -509,6 +509,10 @@ public interface UserRepository @Query(value = "SELECT firstname, lastname FROM SD_User WHERE id = ?1", nativeQuery = true) NameOnly findByNativeQuery(Integer id); + // DATAJPA-1235 + @Query("SELECT u FROM User u where u.firstname >= ?1 and u.lastname = '000:1'") + List queryWithIndexedParameterAndColonFollowedByIntegerInString(String firstname); + static interface RolesAndFirstname { String getFirstname();