From 964b3f83d1b226102b2b011456b32170d8fb72ad Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Tue, 9 May 2023 15:48:07 -0500 Subject: [PATCH] Rewrite LIKE clauses with wildcards as CONCAT functions. We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). Resolves #2939. Related: #2760. --- .../jpa/repository/query/StringQuery.java | 66 +++++++++++++------ .../query/LikeBindingUnitTests.java | 10 --- .../QueryWithNullLikeIntegrationTests.java | 39 ++++++++++- .../query/StringQueryUnitTests.java | 9 +-- 4 files changed, 90 insertions(+), 34 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index d52447ae36..a761ab2247 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -27,6 +27,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.repository.query.SpelQueryContext; import org.springframework.data.repository.query.SpelQueryContext.SpelExtractor; @@ -53,6 +55,8 @@ */ class StringQuery implements DeclaredQuery { + private static final Log LOGGER = LogFactory.getLog(StringQuery.class); + private final String query; private final List bindings; private final @Nullable String alias; @@ -334,7 +338,47 @@ private static String replaceFirst(String text, String substring, String replace return text; } - return text.substring(0, index) + replacement + text.substring(index + substring.length()); + return text.substring(0, index) + potentiallyWrapWithWildcards(replacement, substring) + + text.substring(index + substring.length()); + } + + /** + * If there are any pre- or post-wildcards ({@literal %}), replace them with a {@literal CONCAT} function and proper + * wildcards as string literals. NOTE: {@literal CONCAT} appears to be a standard function across relational + * databases as well as JPA providers. + * + * @param replacement + * @param substring + * @return the replacement string properly wrapped in a {@literal CONCAT} function with wildcards applied. + * @since 3.1 + */ + private static String potentiallyWrapWithWildcards(String replacement, String substring) { + + boolean wildcards = substring.startsWith("%") || substring.endsWith("%"); + + if (!wildcards) { + return replacement; + } + + StringBuilder concatWrapper = new StringBuilder("CONCAT("); + + if (substring.startsWith("%")) { + concatWrapper.append("'%',"); + } + + concatWrapper.append(replacement); + + if (substring.endsWith("%")) { + concatWrapper.append(",'%'"); + } + + concatWrapper.append(")"); + + LOGGER.warn( + "You are using a non-standard query feature that may not be supported in the future (LIKE with '%' wildcards). " + + "We suggest you rewrite [" + substring + "] as [" + concatWrapper + "]"); + + return concatWrapper.toString(); } @Nullable @@ -708,28 +752,12 @@ public Type getType() { } /** - * Prepares the given raw keyword according to the like type. + * Extracts the raw value properly. */ @Nullable @Override public Object prepare(@Nullable Object value) { - - Object unwrapped = PersistenceProvider.unwrapTypedParameterValue(value); - if (unwrapped == null) { - return null; - } - - switch (type) { - case STARTING_WITH: - return String.format("%s%%", unwrapped); - case ENDING_WITH: - return String.format("%%%s", unwrapped); - case CONTAINING: - return String.format("%%%s%%", unwrapped); - case LIKE: - default: - return unwrapped; - } + return PersistenceProvider.unwrapTypedParameterValue(value); } @Override diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/LikeBindingUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/LikeBindingUnitTests.java index 555aa23d96..bcd0556bff 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/LikeBindingUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/LikeBindingUnitTests.java @@ -84,14 +84,4 @@ void setsUpInstanceForIndex() { assertThat(binding.hasPosition(1)).isTrue(); assertThat(binding.getType()).isEqualTo(Type.CONTAINING); } - - @Test - void augmentsValueCorrectly() { - - assertAugmentedValue(Type.CONTAINING, "%value%"); - assertAugmentedValue(Type.ENDING_WITH, "%value"); - assertAugmentedValue(Type.STARTING_WITH, "value%"); - - assertThat(new LikeParameterBinding(1, Type.CONTAINING).prepare(null)).isNull(); - } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java index 9afd65db6b..58417e882f 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import jakarta.persistence.EntityManagerFactory; @@ -102,6 +102,40 @@ void customQueryWithNullMatch() { assertThat(Employees).extracting(EmployeeWithName::getName).isEmpty(); } + @Test + void customQueryWithMultipleMatchInNative() { + + List Employees = repository.customQueryWithNullableParamInNative("Baggins"); + + assertThat(Employees).extracting(EmployeeWithName::getName).containsExactlyInAnyOrder("Frodo Baggins", + "Bilbo Baggins"); + } + + @Test + void customQueryWithSingleMatchInNative() { + + List Employees = repository.customQueryWithNullableParamInNative("Frodo"); + + assertThat(Employees).extracting(EmployeeWithName::getName).containsExactlyInAnyOrder("Frodo Baggins"); + } + + @Test + void customQueryWithEmptyStringMatchInNative() { + + List Employees = repository.customQueryWithNullableParamInNative(""); + + assertThat(Employees).extracting(EmployeeWithName::getName).containsExactlyInAnyOrder("Frodo Baggins", + "Bilbo Baggins"); + } + + @Test + void customQueryWithNullMatchInNative() { + + List Employees = repository.customQueryWithNullableParamInNative(null); + + assertThat(Employees).extracting(EmployeeWithName::getName).isEmpty(); + } + @Test void derivedQueryStartsWithSingleMatch() { @@ -235,6 +269,9 @@ public interface EmployeeWithNullLikeRepository extends JpaRepository customQueryWithNullableParam(@Nullable @Param("partialName") String partialName); + @Query(value = "select * from EmployeeWithName as e where e.name like %:partialName%", nativeQuery = true) + List customQueryWithNullableParamInNative(@Nullable @Param("partialName") String partialName); + List findByNameStartsWith(@Nullable String partialName); List findByNameEndsWith(@Nullable String partialName); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java index 78427ece9a..3c7ece5c4b 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java @@ -65,7 +65,7 @@ void detectsPositionalLikeBindings() { assertThat(query.hasParameterBindings()).isTrue(); assertThat(query.getQueryString()) - .isEqualTo("select u from User u where u.firstname like ?1 or u.lastname like ?2"); + .isEqualTo("select u from User u where u.firstname like CONCAT('%',?1,'%') or u.lastname like CONCAT('%',?2)"); List bindings = query.getParameterBindings(); assertThat(bindings).hasSize(2); @@ -87,7 +87,7 @@ void detectsNamedLikeBindings() { StringQuery query = new StringQuery("select u from User u where u.firstname like %:firstname", true); assertThat(query.hasParameterBindings()).isTrue(); - assertThat(query.getQueryString()).isEqualTo("select u from User u where u.firstname like :firstname"); + assertThat(query.getQueryString()).isEqualTo("select u from User u where u.firstname like CONCAT('%',:firstname)"); List bindings = query.getParameterBindings(); assertThat(bindings).hasSize(1); @@ -199,8 +199,9 @@ void removesLikeBindingsFromQueryIfQueryContainsSimpleBinding() { assertNamedBinding(LikeParameterBinding.class, "escapedWord", bindings.get(0)); assertNamedBinding(ParameterBinding.class, "word", bindings.get(1)); - assertThat(query.getQueryString()).isEqualTo("SELECT a FROM Article a WHERE a.overview LIKE :escapedWord ESCAPE '~'" - + " OR a.content LIKE :escapedWord ESCAPE '~' OR a.title = :word ORDER BY a.articleId DESC"); + assertThat(query.getQueryString()) + .isEqualTo("SELECT a FROM Article a WHERE a.overview LIKE CONCAT('%',:escapedWord,'%') ESCAPE '~'" + + " OR a.content LIKE CONCAT('%',:escapedWord,'%') ESCAPE '~' OR a.title = :word ORDER BY a.articleId DESC"); } @Test // DATAJPA-483