From 55f1131b2fe6cebc9f37b716d3bc2358700d0637 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 7 Jul 2014 16:17:27 +0200 Subject: [PATCH] DATAJPA-564 - Allow SpEL expressions to be the only consumer of query method parameters. We now allow the parameters of query methods to also be exclusively consumed by expressions within a given query string. Previously this wasn't possible due to to strict checks for parameter usage in JpaQueryMethod. Relax check for query param usage in JpaQueryMethod. --- .../jpa/repository/query/JpaQueryMethod.java | 3 +- .../jpa/repository/query/ParameterBinder.java | 12 +++++++- .../jpa/repository/query/StringQuery.java | 6 ++-- .../query/StringQueryParameterBinder.java | 30 +++++++++++++++++++ .../jpa/repository/UserRepositoryTests.java | 28 ++++++++++++++++- .../jpa/repository/sample/UserRepository.java | 12 ++++++++ 6 files changed, 85 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java index 25bf77fe9..b41f29848 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java @@ -107,7 +107,8 @@ public class JpaQueryMethod extends QueryMethod { continue; } - if (!annotatedQuery.contains(String.format(":%s", parameter.getName()))) { + if (!annotatedQuery.contains(String.format(":%s", parameter.getName())) + && !annotatedQuery.contains(String.format("#%s", parameter.getName()))) { throw new IllegalStateException(String.format( "Using named parameters for method %s but parameter '%s' not found in annotated query '%s'!", method, parameter.getName(), annotatedQuery)); diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java index 4f9575568..370d215aa 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java @@ -104,7 +104,7 @@ public class ParameterBinder { for (JpaParameter parameter : parameters) { - if (parameter.isBindable()) { + if (canBindParameter(parameter)) { Object value = computeParameterValue(parameter, values[methodParameterPosition], values); @@ -117,6 +117,16 @@ public class ParameterBinder { return query; } + /** + * Returns {@literal true} if the given parameter can be bound. + * + * @param parameter + * @return + */ + protected boolean canBindParameter(JpaParameter parameter) { + return parameter.isBindable(); + } + /** * Computes the value to bind for the given {@link JpaParameter} and the given {@code value} by potentially using the * other {@code values}. This is intended to be customized in sub-classes. diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index c897cac68..24603753e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -138,11 +138,11 @@ class StringQuery { * * @author Thomas Darimont */ - private static enum ParameterBindingParser { + public static enum ParameterBindingParser { INSTANCE; - private static final String EXPRESSION_PREFIX = "__$synthetic$__"; + static final String EXPRESSION_PARAMETER_PREFIX = "__$synthetic$__"; private static final Pattern PARAMETER_BINDING_BY_INDEX = Pattern.compile("\\?(\\d+)"); 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! " @@ -223,7 +223,7 @@ class StringQuery { parameterIndex = expressionParameterIndex; replacement = "?" + parameterIndex; } else { - parameterName = EXPRESSION_PREFIX + expressionParameterIndex; + parameterName = EXPRESSION_PARAMETER_PREFIX + expressionParameterIndex; replacement = ":" + parameterName; } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQueryParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQueryParameterBinder.java index 621ef9489..f78421900 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQueryParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQueryParameterBinder.java @@ -15,6 +15,8 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.List; + import javax.persistence.Query; import org.springframework.data.jpa.repository.query.JpaParameters.JpaParameter; @@ -64,6 +66,34 @@ public class StringQueryParameterBinder extends ExpressionAwareParameterBinder { super.bind(jpaQuery, methodParameter, binding.prepare(value), position); } + /* (non-Javadoc) + * @see org.springframework.data.jpa.repository.query.ParameterBinder#canBindParameter(org.springframework.data.jpa.repository.query.JpaParameters.JpaParameter) + */ + @Override + protected boolean canBindParameter(JpaParameter parameter) { + + List parameterBindings = query.getParameterBindings(); + + // if no parameter bindings are present, we simply rely on the check in super. + if (parameterBindings.isEmpty()) { + return super.canBindParameter(parameter); + } + + // otherwise determine whether there are any non expression parameters left to be bound. + int expressionParameterCount = 0; + for (ParameterBinding binding : parameterBindings) { + + if (binding.isExpression()) { + expressionParameterCount++; + } + } + + boolean allParametersAreUsedInExpressions = parameterBindings.size() - expressionParameterCount == 0; + + // if all parameters are used in expressions, then we can skip their bindings now, since they'll get bound later. + return !allParametersAreUsedInExpressions && super.canBindParameter(parameter); + } + /** * Finds the {@link LikeParameterBinding} to be applied before binding a parameter value to the query. * diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index c4644dd2a..8f9a1e35d 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -58,8 +58,8 @@ import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.Role; import org.springframework.data.jpa.domain.sample.SpecialUser; import org.springframework.data.jpa.domain.sample.User; -import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.data.jpa.repository.sample.SampleSecurity.SampleSecurityContextHolder; +import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.annotation.Transactional; @@ -1706,6 +1706,32 @@ public class UserRepositoryTests { assertThat(users.get(0), is(secondUser)); } + /** + * @see DATAJPA-564 + */ + @Test + public void shouldfindUsersByFirstnameForSpELExpressionOnlyWithParameterNameVariableReference() { + + flushTestUsers(); + List users = repository.findUsersByFirstnameForSpELExpressionWithParameterVariableOnly("Joachim"); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(secondUser)); + } + + /** + * @see DATAJPA-564 + */ + @Test + public void shouldfindUsersByFirstnameForSpELExpressionOnlyWithParameterIndexReference() { + + flushTestUsers(); + List users = repository.findUsersByFirstnameForSpELExpressionWithParameterIndexOnly("Joachim"); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(secondUser)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index b1b2295c5..e5519f2a4 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -507,4 +507,16 @@ public interface UserRepository extends JpaRepository, JpaSpecifi */ @Query("select u from User u where u.firstname = ?1 and u.firstname=?#{[0]} and u.emailAddress = ?#{principal.emailAddress}") List findByFirstnameAndCurrentUserWithCustomQuery(String firstname); + + /** + * @see DATAJPA-564 + */ + @Query("select u from User u where u.firstname = :#{#firstname}") + List findUsersByFirstnameForSpELExpressionWithParameterVariableOnly(@Param("firstname") String firstname); + + /** + * @see DATAJPA-564 + */ + @Query("select u from User u where u.firstname = ?#{[0]}") + List findUsersByFirstnameForSpELExpressionWithParameterIndexOnly(String firstname); }