DATAJPA-1200 - Tweaked parameter name parsing to not consider :: and \: starts of a named parameter.
Unified the query parsing in QueryUtils and StringQuery to use the same regex for matching named parameters. Added look behind to not consider :: and \: start of a named parameter. Unicode whitespace, control, formatting and punctuation characters except '.' and '_' are no longer considered legal parameter names. Note: QueryUtils accepts # as start of a named parameter. StringQuery doesn't. Original pull request: #231.
This commit is contained in:
Jens Schauder
committed by
Oliver Gierke
Oliver Gierke
parent
336afdb5cf
commit
f155e522c5
@@ -79,6 +79,14 @@ public abstract class QueryUtils {
|
||||
public static final String COUNT_QUERY_STRING = "select count(%s) from %s x";
|
||||
public static final String DELETE_ALL_QUERY_STRING = "delete from %s x";
|
||||
|
||||
// Used Regex/Unicode categories (see http://www.unicode.org/reports/tr18/#General_Category_Property):
|
||||
// Z Separator
|
||||
// Cc Control
|
||||
// Cf Format
|
||||
// P Punctuation
|
||||
static final String IDENTIFIER = "[._[\\P{Z}&&\\P{Cc}&&\\P{Cf}&&\\P{P}]]+";
|
||||
static final String COLON_NO_DOUBLE_COLON = "(?<![:\\\\]):";
|
||||
|
||||
private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7";
|
||||
private static final String SIMPLE_COUNT_VALUE = "$2";
|
||||
private static final String COMPLEX_COUNT_VALUE = "$3$6";
|
||||
@@ -89,7 +97,7 @@ public abstract class QueryUtils {
|
||||
private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(.+)\\s+from");
|
||||
|
||||
private static final Pattern NO_DIGITS = Pattern.compile("\\D+");
|
||||
private static final String IDENTIFIER = "[\\p{Lu}\\P{InBASIC_LATIN}\\p{Alnum}._$]+";
|
||||
|
||||
private static final String IDENTIFIER_GROUP = String.format("(%s)", IDENTIFIER);
|
||||
|
||||
private static final String JOIN = "join\\s+(fetch\\s+)?" + IDENTIFIER + "\\s+(as\\s+)?" + IDENTIFIER_GROUP;
|
||||
@@ -98,8 +106,8 @@ public abstract class QueryUtils {
|
||||
private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s";
|
||||
private static final Pattern ORDER_BY = Pattern.compile(".*order\\s+by\\s+.*", CASE_INSENSITIVE);
|
||||
|
||||
private static final Pattern NAMED_PARAMETER = Pattern.compile(":" + IDENTIFIER + "|\\#" + IDENTIFIER,
|
||||
CASE_INSENSITIVE);
|
||||
private static final Pattern NAMED_PARAMETER = Pattern
|
||||
.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|\\#" + IDENTIFIER, CASE_INSENSITIVE);
|
||||
|
||||
private static final Pattern CONSTRUCTOR_EXPRESSION;
|
||||
|
||||
|
||||
@@ -128,6 +128,10 @@ class StringQuery {
|
||||
private static final Pattern PARAMETER_BINDING_PATTERN;
|
||||
private static final String MESSAGE = "Already found parameter binding with same index / parameter name but differing binding type! "
|
||||
+ "Already have: %s, found %s! If you bind a parameter multiple times make sure they use the same binding.";
|
||||
public static final int INDEXED_PARAMETER_GROUP = 4;
|
||||
public static final int NAMED_PARAMETER_GROUP = 6;
|
||||
public static final int COMPARISION_TYPE_GROUP = 1;
|
||||
public static final int EXPRESSION_GROUP = 9;
|
||||
|
||||
static {
|
||||
|
||||
@@ -148,7 +152,7 @@ class StringQuery {
|
||||
builder.append("(");
|
||||
builder.append("%?(\\?(\\d+))%?"); // position parameter and parameter index
|
||||
builder.append("|"); // or
|
||||
builder.append("%?(:([\\p{L}\\w]+))%?"); // named parameter and the parameter name
|
||||
builder.append("%?("+QueryUtils.COLON_NO_DOUBLE_COLON + "(" + QueryUtils.IDENTIFIER + "+))%?"); // named parameter and the parameter name
|
||||
builder.append("|"); // or
|
||||
builder.append("%?((:|\\?)#\\{([^}]+)\\})%?"); // expression parameter and expression
|
||||
builder.append(")");
|
||||
@@ -161,7 +165,7 @@ class StringQuery {
|
||||
* Parses {@link ParameterBinding} instances from the given query and adds them to the registered bindings. Returns
|
||||
* the cleaned up query.
|
||||
*/
|
||||
private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(String query,
|
||||
String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(String query,
|
||||
List<ParameterBinding> bindings) {
|
||||
|
||||
String result = query;
|
||||
@@ -187,10 +191,10 @@ class StringQuery {
|
||||
|
||||
while (matcher.find()) {
|
||||
|
||||
String parameterIndexString = matcher.group(4);
|
||||
String parameterName = parameterIndexString != null ? null : matcher.group(6);
|
||||
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);
|
||||
String typeSource = matcher.group(1);
|
||||
String typeSource = matcher.group(COMPARISION_TYPE_GROUP);
|
||||
String expression = null;
|
||||
String replacement = null;
|
||||
|
||||
@@ -205,7 +209,7 @@ class StringQuery {
|
||||
replacement = ":" + parameterName;
|
||||
}
|
||||
|
||||
expression = matcher.group(9);
|
||||
expression = matcher.group(EXPRESSION_GROUP);
|
||||
}
|
||||
|
||||
switch (ParameterBindingType.of(typeSource)) {
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
/*
|
||||
* Copyright 2017 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.assertThat;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.assertj.core.api.SoftAssertions;
|
||||
import org.junit.Test;
|
||||
import org.springframework.data.jpa.repository.query.StringQuery.ParameterBinding;
|
||||
import org.springframework.data.jpa.repository.query.StringQuery.ParameterBindingParser;
|
||||
|
||||
/**
|
||||
* Unit tests for the {@link ParameterBindingParser}.
|
||||
*
|
||||
* @author Jens Schauder
|
||||
*/
|
||||
public class ParameterBindingParserUnitTests {
|
||||
|
||||
@Test // DATAJPA-1200
|
||||
public void idenficationOfParameters(){
|
||||
|
||||
SoftAssertions softly = new SoftAssertions();
|
||||
|
||||
checkHasParameter(softly, "select something from x where id = :id", true, "named parameter");
|
||||
checkHasParameter(softly, "in the :id middle", true, "middle");
|
||||
checkHasParameter(softly, ":id start", true, "beginning");
|
||||
checkHasParameter(softly, ":id", true, "alone");
|
||||
checkHasParameter(softly, "select something from x where id = :id", true, "named parameter");
|
||||
checkHasParameter(softly, ":UPPERCASE", true, "uppercase");
|
||||
checkHasParameter(softly, ":lowercase", true, "lowercase");
|
||||
checkHasParameter(softly, ":2something", true, "beginning digit");
|
||||
checkHasParameter(softly, ":2", true, "only digit");
|
||||
checkHasParameter(softly, ":.something", true, "dot"); // <--
|
||||
checkHasParameter(softly, ":_something", true, "underscore");
|
||||
checkHasParameter(softly, ":$something", true, "dollar"); // <--
|
||||
checkHasParameter(softly, ":\uFE0F", true, "non basic latin emoji"); // <--
|
||||
checkHasParameter(softly, ":\u4E01", true, "chinese japanese korean");
|
||||
checkHasParameter(softly, "select something from x where id = ?1", true, "indexed parameter");
|
||||
|
||||
checkHasParameter(softly, "select something from x where id = #something", false, "hash"); // <-- should we accept hash as named parameter start?
|
||||
|
||||
checkHasParameter(softly, "no bind variable", false, "no bind variable");
|
||||
checkHasParameter(softly, ":\u2004whitespace", false, "non basic latin whitespace"); // <--
|
||||
checkHasParameter(softly, "::", false, "double colon");
|
||||
checkHasParameter(softly, ":", false, "end of query");
|
||||
checkHasParameter(softly, ":\u0003", false, "non-printable");
|
||||
checkHasParameter(softly, ":\u002A", false, "basic latin emoji");
|
||||
checkHasParameter(softly, "\\:", false, "escaped colon");
|
||||
checkHasParameter(softly, "::id", false, "double colon with identifier");
|
||||
checkHasParameter(softly, "\\:id", false, "escaped colon with identifier");
|
||||
|
||||
softly.assertAll();
|
||||
}
|
||||
|
||||
public void checkHasParameter(SoftAssertions softly, String query, boolean containsParameter, String label) {
|
||||
|
||||
List<ParameterBinding> bindings = new ArrayList();
|
||||
ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query, bindings);
|
||||
softly.assertThat(bindings.size()) //
|
||||
.describedAs(String.format("<%s> (%s)", query, label)) //
|
||||
.isEqualTo(containsParameter ? 1 : 0);
|
||||
}
|
||||
}
|
||||
@@ -23,6 +23,7 @@ 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;
|
||||
@@ -394,6 +395,48 @@ 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));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user