From d83bfac77eb7caad05cbaff76c1db4dbc7442b5a 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 | 8 +- .../query/StringQueryUnitTests.java | 102 ++++++++++++++++++ 3 files changed, 122 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 efa1b8b1e..4c2a1e06c 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 @@ -155,6 +155,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; @@ -502,6 +506,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 1a26f381f..05ccf64c2 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; @@ -121,8 +122,11 @@ 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 a44267a7b..7052c1eb8 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 @@ -160,6 +160,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()).isEqualTo(ParameterOrigin.ofExpression("firstname")); + assertThat(binding.getName()).isEqualTo("__$synthetic$__1"); + assertThat(binding.getType()).isEqualTo(Type.ENDING_WITH); + + binding = (LikeParameterBinding) bindings.get(1); + assertThat(binding).isNotNull(); + assertThat(binding.getOrigin()).isEqualTo(ParameterOrigin.ofExpression("firstname")); + assertThat(binding.getName()).isEqualTo("__$synthetic$__2"); + assertThat(binding.getType()).isEqualTo(Type.STARTING_WITH); + } + @Test // GH-3041 void rewritesPositionalLikeToUniqueParametersIfNecessary() { @@ -263,6 +323,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() {