From 7f170ea408e31e94517e7a9db805cabac4041a20 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 24 Feb 2025 11:57:43 +0100 Subject: [PATCH] Refine count-query derivation parameter post-processing. We've now expanded parameter post-processing for derived count queries to consider binding types (in, like) and to correctly retain invocation parameter redirects instead of assuming an exact mapping of parameter positions in the final query to the actual invocation argument names/indices. Closes #3784 --- .../repository/query/ParameterBinding.java | 14 +++ .../jpa/repository/query/StringQuery.java | 9 +- .../query/StringQueryUnitTests.java | 102 ++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinding.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinding.java index 65fa814d4..e5cffccaf 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinding.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinding.java @@ -156,6 +156,10 @@ class ParameterBinding { */ public boolean bindsTo(ParameterBinding other) { + if (getIdentifier().equals(other.getIdentifier())) { + return true; + } + if (identifier.hasName() && other.identifier.hasName()) { if (identifier.getName().equals(other.identifier.getName())) { return true; @@ -503,6 +507,16 @@ class ParameterBinding { return new Expression(expression); } + /** + * Creates a {@link MethodInvocationArgument} object for {@code name} + * + * @param name the parameter name from the method invocation. + * @return {@link MethodInvocationArgument} object for {@code name}. + */ + static MethodInvocationArgument ofParameter(String name) { + return ofParameter(name, null); + } + /** * Creates a {@link MethodInvocationArgument} object for {@code name} and {@code position}. Either the name or the * position must be given. 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 8bc8dff0a..4e794dee5 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 @@ -23,6 +23,7 @@ import java.util.List; import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -141,8 +142,12 @@ class StringQuery implements DeclaredQuery { for (ParameterBinding binding : bindings) { - if (binding.getOrigin().isExpression() && derivedBindings.removeIf( - it -> !it.getOrigin().isExpression() && it.getIdentifier().equals(binding.getIdentifier()))) { + Predicate identifier = binding::bindsTo; + Predicate notCompatible = Predicate.not(binding::isCompatibleWith); + + // replace incompatible bindings + if ( derivedBindings.removeIf( + it -> identifier.test(it) && notCompatible.test(it))) { derivedBindings.add(binding); } } 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 04bf714ae..4c0bd3376 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 @@ -161,6 +161,66 @@ class StringQueryUnitTests { assertThat(((MethodInvocationArgument) parameterBinding.getOrigin()).identifier().getName()).isEqualTo("firstname"); } + @Test // GH-3784 + void rewritesNamedLikeToUniqueParametersRetainingCountQuery() { + + DeclaredQuery query = new StringQuery( + "select u from User u where u.firstname like %:firstname or u.firstname like :firstname% or u.firstname = :firstname", + false).deriveCountQuery(null); + + assertThat(query.getQueryString()) // + .isEqualTo( + "select count(u) from User u where u.firstname like :firstname or u.firstname like :firstname_1 or u.firstname = :firstname_2"); + + List bindings = query.getParameterBindings(); + assertThat(bindings).hasSize(3); + + LikeParameterBinding binding = (LikeParameterBinding) bindings.get(0); + assertThat(binding).isNotNull(); + assertThat(binding.getOrigin()).isEqualTo(ParameterOrigin.ofParameter("firstname")); + assertThat(binding.getName()).isEqualTo("firstname"); + assertThat(binding.getType()).isEqualTo(Type.ENDING_WITH); + + binding = (LikeParameterBinding) bindings.get(1); + assertThat(binding).isNotNull(); + assertThat(binding.getOrigin()).isEqualTo(ParameterOrigin.ofParameter("firstname")); + assertThat(binding.getName()).isEqualTo("firstname_1"); + assertThat(binding.getType()).isEqualTo(Type.STARTING_WITH); + + ParameterBinding parameterBinding = bindings.get(2); + assertThat(parameterBinding).isNotNull(); + assertThat(parameterBinding.getOrigin()).isEqualTo(ParameterOrigin.ofParameter("firstname")); + assertThat(parameterBinding.getName()).isEqualTo("firstname_2"); + assertThat(((MethodInvocationArgument) parameterBinding.getOrigin()).identifier().getName()).isEqualTo("firstname"); + } + + @Test // GH-3784 + void rewritesExpressionsLikeToUniqueParametersRetainingCountQuery() { + + DeclaredQuery query = new StringQuery( + "select u from User u where u.firstname like %:#{firstname} or u.firstname like :#{firstname}%", false) + .deriveCountQuery(null); + + assertThat(query.getQueryString()) // + .isEqualTo( + "select count(u) from User u where u.firstname like :__$synthetic$__1 or u.firstname like :__$synthetic$__2"); + + List bindings = query.getParameterBindings(); + assertThat(bindings).hasSize(2); + + LikeParameterBinding binding = (LikeParameterBinding) bindings.get(0); + assertThat(binding).isNotNull(); + assertThat(binding.getOrigin().isExpression()).isTrue(); + assertThat(binding.getName()).isEqualTo("__$synthetic$__1"); + assertThat(binding.getType()).isEqualTo(Type.ENDING_WITH); + + binding = (LikeParameterBinding) bindings.get(1); + assertThat(binding).isNotNull(); + assertThat(binding.getOrigin().isExpression()).isTrue(); + assertThat(binding.getName()).isEqualTo("__$synthetic$__2"); + assertThat(binding.getType()).isEqualTo(Type.STARTING_WITH); + } + @Test // GH-3041 void rewritesPositionalLikeToUniqueParametersIfNecessary() { @@ -264,6 +324,48 @@ class StringQueryUnitTests { assertNamedBinding(ParameterBinding.class, "bar", bindings.get(2)); } + @Test // GH-3784 + void deriveCountQueryWithNamedInRetainsOrigin() { + + String queryString = "select u from User u where (:logins) IS NULL OR LOWER(u.login) IN (:logins)"; + DeclaredQuery query = new StringQuery(queryString, false).deriveCountQuery(null); + + assertThat(query.getQueryString()) + .isEqualTo("select count(u) from User u where (:logins) IS NULL OR LOWER(u.login) IN (:logins_1)"); + + List bindings = query.getParameterBindings(); + assertThat(bindings).hasSize(2); + + assertNamedBinding(ParameterBinding.class, "logins", bindings.get(0)); + assertThat((MethodInvocationArgument) bindings.get(0).getOrigin()).extracting(MethodInvocationArgument::identifier) + .extracting(BindingIdentifier::getName).isEqualTo("logins"); + + assertNamedBinding(InParameterBinding.class, "logins_1", bindings.get(1)); + assertThat((MethodInvocationArgument) bindings.get(1).getOrigin()).extracting(MethodInvocationArgument::identifier) + .extracting(BindingIdentifier::getName).isEqualTo("logins"); + } + + @Test // GH-3784 + void deriveCountQueryWithPositionalInRetainsOrigin() { + + String queryString = "select u from User u where (?1) IS NULL OR LOWER(u.login) IN (?1)"; + DeclaredQuery query = new StringQuery(queryString, false).deriveCountQuery(null); + + assertThat(query.getQueryString()) + .isEqualTo("select count(u) from User u where (?1) IS NULL OR LOWER(u.login) IN (?2)"); + + List bindings = query.getParameterBindings(); + assertThat(bindings).hasSize(2); + + assertPositionalBinding(ParameterBinding.class, 1, bindings.get(0)); + assertThat((MethodInvocationArgument) bindings.get(0).getOrigin()).extracting(MethodInvocationArgument::identifier) + .extracting(BindingIdentifier::getPosition).isEqualTo(1); + + assertPositionalBinding(InParameterBinding.class, 2, bindings.get(1)); + assertThat((MethodInvocationArgument) bindings.get(1).getOrigin()).extracting(MethodInvocationArgument::identifier) + .extracting(BindingIdentifier::getPosition).isEqualTo(1); + } + @Test // DATAJPA-461 void detectsPositionalInParameterBindings() {