From d5a8c15d915f1240b4cae0758ec82b9e5271c52a Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 23 Jul 2018 15:44:59 +0200 Subject: [PATCH] DATAJPA-1281 - Polishing. Improve error message for BasicQueryParameterSetterFactory as index out of bounds access can happen here as well. Add unit test. Original pull request: #255. --- .../query/QueryParameterSetterFactory.java | 26 +++++++++++++++---- .../QueryParameterSetterFactoryUnitTests.java | 16 +++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java index f585a97d4..187e781f2 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java @@ -208,9 +208,26 @@ abstract class QueryParameterSetterFactory { Assert.notNull(binding, "Binding must not be null."); - JpaParameter parameter = declaredQuery.hasNamedParameter() // - ? findParameterForBinding(binding) // - : parameters.getBindableParameter(binding.getRequiredPosition() - 1); + JpaParameter parameter; + + if (declaredQuery.hasNamedParameter()) { + parameter = findParameterForBinding(binding); + } else { + + int parameterIndex = binding.getRequiredPosition() - 1; + JpaParameters bindableParameters = parameters.getBindableParameters(); + + Assert.isTrue( // + parameterIndex < bindableParameters.getNumberOfParameters(), // + () -> String.format( // + "At least %s parameter(s) provided but only %s parameter(s) present in query.", // + binding.getRequiredPosition(), // + bindableParameters.getNumberOfParameters() // + ) // + ); + + parameter = bindableParameters.getParameter(binding.getRequiredPosition() - 1); + } return parameter == null // ? QueryParameterSetter.NOOP // @@ -235,10 +252,9 @@ abstract class QueryParameterSetterFactory { } /** - * {@link QueryParameterSetterFactory} - * * @author Jens Schauder * @author Oliver Gierke + * @see QueryParameterSetterFactory */ private static class CriteriaQueryParameterSetterFactory extends QueryParameterSetterFactory { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java index e40b785cf..4376d6c1f 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactoryUnitTests.java @@ -32,6 +32,7 @@ import org.springframework.data.jpa.repository.query.StringQuery.ParameterBindin * Unit tests for {@link QueryParameterSetterFactory}. * * @author Jens Schauder + * @author Mark Paluch */ public class QueryParameterSetterFactoryUnitTests { @@ -65,7 +66,7 @@ public class QueryParameterSetterFactoryUnitTests { } @Test // DATAJPA-1281 - public void exceptionWhenQueryContainsInsufficientAmountOfParameters() { + public void exceptionWhenCriteriaQueryContainsInsufficientAmountOfParameters() { // no parameter present in the criteria query List> metadata = Collections.emptyList(); @@ -77,6 +78,19 @@ public class QueryParameterSetterFactoryUnitTests { Assertions.assertThatExceptionOfType(IllegalArgumentException.class) // .isThrownBy(() -> setterFactory.create(binding, DeclaredQuery.of("QueryStringWith :NamedParameter"))) // .withMessage("At least 1 parameter(s) provided but only 0 parameter(s) present in query."); + } + @Test // DATAJPA-1281 + public void exceptionWhenBasicQueryContainsInsufficientAmountOfParameters() { + + // no parameter present in the criteria query + QueryParameterSetterFactory setterFactory = QueryParameterSetterFactory.basic(parameters); + + // one argument present in the method signature + when(binding.getRequiredPosition()).thenReturn(1); + + Assertions.assertThatExceptionOfType(IllegalArgumentException.class) // + .isThrownBy(() -> setterFactory.create(binding, DeclaredQuery.of("QueryStringWith ?1"))) // + .withMessage("At least 1 parameter(s) provided but only 0 parameter(s) present in query."); } }