From b09588f9220d22a9ecac6d477f896f438d366176 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 26 Jun 2014 12:46:08 +0200 Subject: [PATCH] DATAJPA-564 - More robust handling of SpEL expressions in String queries. We now treat the result of the SpEL expression evaluation as Object instead of just String in ExpressionAwareParameterBinder. We now prefer index based parameter binding over named parameter binding iff only SpEL expression parameters are present. We now make named parameters available as variables in SpEL expressions. --- .../query/ExpressionAwareParameterBinder.java | 13 ++++- .../jpa/repository/query/ParameterBinder.java | 9 ++++ .../jpa/repository/query/StringQuery.java | 12 ++++- .../jpa/repository/UserRepositoryTests.java | 52 +++++++++++++++++++ .../jpa/repository/sample/UserRepository.java | 24 +++++++++ 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ExpressionAwareParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/ExpressionAwareParameterBinder.java index 24bd60a44..b370666c0 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ExpressionAwareParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ExpressionAwareParameterBinder.java @@ -118,7 +118,7 @@ class ExpressionAwareParameterBinder extends ParameterBinder { * @return */ protected Object evaluateExpression(Expression expr) { - return expr.getValue(getEvaluationContext(), String.class); + return expr.getValue(getEvaluationContext(), Object.class); } /** @@ -131,9 +131,20 @@ class ExpressionAwareParameterBinder extends ParameterBinder { EvaluationContext delegatee = evaluationContextProvider.getEvaluationContext(); StandardEvaluationContext evalContext = new DelegatingStandardEvaluationContext(getValues(), delegatee); + populateParameterVariables(evalContext); + return evalContext; } + private void populateParameterVariables(StandardEvaluationContext evalContext) { + + for (JpaParameter param : getParameters()) { + if (param.isNamedParameter()) { + evalContext.setVariable(param.getName(), getValues()[param.getIndex()]); + } + } + } + /** * A {@link StandardEvaluationContext} that delegates to the given {@link EvaluationContext}. Variables are first * looked-up locally and if not the lookup is performed against the delegatee. 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 f63b7a895..29f56b867 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 @@ -192,4 +192,13 @@ public class ParameterBinder { Object[] getValues() { return values; } + + /** + * Returns the parameters. + * + * @return + */ + JpaParameters getParameters() { + return parameters; + } } 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 8a9a4815d..c897cac68 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 @@ -189,10 +189,18 @@ class StringQuery { String result = query; Matcher matcher = PARAMETER_BINDING_PATTERN.matcher(query); - int greatestParameterIndex = determineGreatestParameterIndexIfPresent(query); + int greatestParameterIndex = tryFindGreatestParameterIndexIn(query); boolean parametersShouldBeAccessedByIndex = greatestParameterIndex != -1; + /* + * Prefer indexed access over named parameters if only SpEL Expression parameters are present. + */ + if (!parametersShouldBeAccessedByIndex && query.contains("?#{")) { + parametersShouldBeAccessedByIndex = true; + greatestParameterIndex = 0; + } + /* * If parameters need to be bound by index, we bind the synthetic expression parameters starting from position of the greatest discovered index parameter in order to * not mix-up with the actual parameter indices. @@ -266,7 +274,7 @@ class StringQuery { return result; } - private int determineGreatestParameterIndexIfPresent(String query) { + private int tryFindGreatestParameterIndexIn(String query) { Matcher parameterIndexMatcher = PARAMETER_BINDING_BY_INDEX.matcher(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 9e7f8e1f3..03cd91848 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1617,6 +1617,58 @@ public class UserRepositoryTests { assertThat(users.get(0), is(firstUser)); } + /** + * @see DATAJPA-XXX + */ + @Test + public void shouldFindBySpELExpressionWithoutArgumentsWithQuestionmark() { + + flushTestUsers(); + List users = repository.findOliverBySpELExpressionWithoutArgumentsWithQuestionmark(); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(firstUser)); + } + + /** + * @see DATAJPA-XXX + */ + @Test + public void shouldFindBySpELExpressionWithoutArgumentsWithColon() { + + flushTestUsers(); + List users = repository.findOliverBySpELExpressionWithoutArgumentsWithColon(); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(firstUser)); + } + + /** + * @see DATAJPA-XXX + */ + @Test + public void shouldFindUsersByAgeForSpELExpression() { + + flushTestUsers(); + List users = repository.findUsersByAgeForSpELExpressionByIndexedParameter(35); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(secondUser)); + } + + /** + * @see DATAJPA-XXX + */ + @Test + public void shouldfindUsersByFirstnameForSpELExpressionWithParameterNameVariableReference() { + + flushTestUsers(); + List users = repository.findUsersByFirstnameForSpELExpression("Joachim"); + + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(secondUser)); + } + /** * @see DATAJPA-XXX */ 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 a6b19a50b..eb7909090 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 @@ -477,4 +477,28 @@ public interface UserRepository extends JpaRepository, JpaSpecifi * @see DATAJPA-XXX */ List queryByLastname(Expression lastname); + + /** + * @see DATAJPA-XXX + */ + @Query("select u from User u where u.firstname = ?#{'Oliver'}") + List findOliverBySpELExpressionWithoutArgumentsWithQuestionmark(); + + /** + * @see DATAJPA-XXX + */ + @Query("select u from User u where u.firstname = :#{'Oliver'}") + List findOliverBySpELExpressionWithoutArgumentsWithColon(); + + /** + * @see DATAJPA-XXX + */ + @Query("select u from User u where u.age = ?#{[0]}") + List findUsersByAgeForSpELExpressionByIndexedParameter(int age); + + /** + * @see DATAJPA-XXX + */ + @Query("select u from User u where u.firstname = :firstname and u.firstname = :#{#firstname}") + List findUsersByFirstnameForSpELExpression(@Param("firstname") String firstname); }