From f155e522c5ed43bcf1dea31380e1a682d04e86f6 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 24 Oct 2017 12:28:09 +0200 Subject: [PATCH] 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. --- .../data/jpa/repository/query/QueryUtils.java | 14 +++- .../jpa/repository/query/StringQuery.java | 16 ++-- .../ParameterBindingParserUnitTests.java | 79 +++++++++++++++++++ .../repository/query/QueryUtilsUnitTests.java | 43 ++++++++++ 4 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/repository/query/ParameterBindingParserUnitTests.java 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 7873c6d12..63d7e22a1 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 @@ -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 = "(? 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)) { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterBindingParserUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterBindingParserUnitTests.java new file mode 100644 index 000000000..d38228c61 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterBindingParserUnitTests.java @@ -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 bindings = new ArrayList(); + ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query, bindings); + softly.assertThat(bindings.size()) // + .describedAs(String.format("<%s> (%s)", query, label)) // + .isEqualTo(containsParameter ? 1 : 0); + } +} \ No newline at end of file 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 3a479882b..28882b4cf 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,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)); }