diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java index e1773ee32..d330bb828 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java @@ -49,23 +49,20 @@ interface QueryParameterSetter { private final Function valueExtractor; private final Parameter parameter; private final @Nullable TemporalType temporalType; - private final boolean lenient; /** * @param valueExtractor must not be {@literal null}. * @param parameter must not be {@literal null}. * @param temporalType may be {@literal null}. - * @param lenient must not be {@literal null}. */ NamedOrIndexedQueryParameterSetter(Function valueExtractor, Parameter parameter, - @Nullable TemporalType temporalType, boolean lenient) { + @Nullable TemporalType temporalType) { Assert.notNull(valueExtractor, "ValueExtractor must not be null!"); this.valueExtractor = valueExtractor; this.parameter = parameter; this.temporalType = temporalType; - this.lenient = lenient; } /* @@ -77,44 +74,47 @@ interface QueryParameterSetter { Object value = valueExtractor.apply(values); - try { + if (temporalType != null) { - if (temporalType != null) { + // One would think we can simply use parameter to identify the parameter we want to set. + // But that does not work with list valued parameters. At least Hibernate tries to bind them by name. + // TODO: move to using setParameter(Parameter, value) when https://hibernate.atlassian.net/browse/HHH-11870 is + // fixed. - // One would think we can simply use parameter to identify the parameter we want to set. - // But that does not work with list valued parameters. At least Hibernate tries to bind them by name. - // TODO: move to using setParameter(Parameter, value) when https://hibernate.atlassian.net/browse/HHH-11870 is - // fixed. - - if (parameter instanceof ParameterExpression) { - query.setParameter((Parameter) parameter, (Date) value, temporalType); - } else if (parameter.getName() != null && QueryUtils.hasNamedParameter(query)) { - query.setParameter(parameter.getName(), (Date) value, temporalType); - } else { + if (parameter instanceof ParameterExpression) { + query.setParameter((Parameter) parameter, (Date) value, temporalType); + } else if (parameter.getName() != null && QueryUtils.hasNamedParameter(query)) { + query.setParameter(parameter.getName(), (Date) value, temporalType); + } else { + if (query.getParameters().size() >= parameter.getPosition() || registerExcessParameters(query)) { query.setParameter(parameter.getPosition(), (Date) value, temporalType); } + } + } else { + + if (parameter instanceof ParameterExpression) { + query.setParameter((Parameter) parameter, value); + } else if (parameter.getName() != null && QueryUtils.hasNamedParameter(query)) { + query.setParameter(parameter.getName(), value); } else { - - if (parameter instanceof ParameterExpression) { - query.setParameter((Parameter) parameter, value); - } else if (parameter.getName() != null && QueryUtils.hasNamedParameter(query)) { - query.setParameter(parameter.getName(), value); - } else { + if (query.getParameters().size() >= parameter.getPosition() || registerExcessParameters(query)) { query.setParameter(parameter.getPosition(), value); } } - } catch (IllegalArgumentException o_O) { - - if (!lenient) { - throw o_O; - } - - // Since EclipseLink doesn't reliably report whether a query has parameters - // we simply try to set the parameters and ignore possible failures. - // this is relevant for queries with SpEL expressions, where the method parameters don't have to match the - // parameters in the query. } } + + private boolean registerExcessParameters(Query query) { + + // DATAJPA-1172 + // Since EclipseLink doesn't reliably report whether a query has parameters + // we simply try to set the parameters and ignore possible failures. + // this is relevant for native queries with SpEL expressions, where the method parameters don't have to match the + // parameters in the query. + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=521915 + + return query.getParameters().size() == 0 && query.getClass().getName().startsWith("org.eclipse"); + } } } 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 6b02a50ea..75b9e8f60 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 @@ -114,7 +114,7 @@ abstract class QueryParameterSetterFactory { : null; return new NamedOrIndexedQueryParameterSetter(valueExtractor.andThen(binding::prepare), - ParameterImpl.of(parameter, binding), temporalType, lenient); + ParameterImpl.of(parameter, binding), temporalType); } /** @@ -279,7 +279,7 @@ abstract class QueryParameterSetterFactory { TemporalType temporalType = parameter.isTemporalParameter() ? parameter.getRequiredTemporalType() : null; return new NamedOrIndexedQueryParameterSetter(values -> getAndPrepare(parameter, metadata, values), - metadata.getExpression(), temporalType, false); + metadata.getExpression(), temporalType); } @Nullable diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index 30792c8f8..670c757be 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -15,6 +15,11 @@ */ package org.springframework.data.jpa.repository; +import static org.assertj.core.api.Assertions.*; + +import javax.persistence.Query; + +import org.junit.Test; import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.test.context.ContextConfiguration; @@ -23,6 +28,7 @@ import org.springframework.test.context.ContextConfiguration; * * @author Oliver Gierke * @author Thomas Darimont + * @author Jens Schauder */ @ContextConfiguration(value = "classpath:eclipselink.xml") public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserRepositoryTests { @@ -66,4 +72,17 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi */ @Override public void findByElementCollectionAttribute() {} + + /** + * This test will fail once https://bugs.eclipse.org/bugs/show_bug.cgi?id=521915 is fixed. + */ + @Override + @Test // DATAJPA-1172 + public void queryProvidesCorrectNumberOfParametersForNativeQuery() { + + Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); + assertThat(query.getParameters()).describedAs( + "Due to a bug eclipse has size 0. If this is no longer the case the special code path triggered in NamedOrIndexedQueryParameterSetter.registerExcessParameters can be removed") + .hasSize(0); + } } 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 8a83fa826..f08a6d636 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -20,6 +20,7 @@ import static org.springframework.data.domain.Example.*; import static org.springframework.data.domain.ExampleMatcher.*; import static org.springframework.data.domain.Sort.Direction.*; import static org.springframework.data.jpa.domain.Specification.*; +import static org.springframework.data.jpa.domain.Specification.not; import static org.springframework.data.jpa.domain.sample.UserSpecifications.*; import java.util.ArrayList; @@ -50,8 +51,6 @@ import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Example; import org.springframework.data.domain.ExampleMatcher; -import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher; -import org.springframework.data.domain.ExampleMatcher.StringMatcher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -60,6 +59,7 @@ import org.springframework.data.domain.Slice; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.domain.ExampleMatcher.*; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.Role; @@ -85,6 +85,7 @@ import com.google.common.base.Optional; * @author Christoph Strobl * @author Mark Paluch * @author Kevin Peters + * @author Jens Schauder */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:application-context.xml") @@ -2048,6 +2049,21 @@ public class UserRepositoryTests { assertThat(result.getContent().get(0)).isEqualTo(thirdUser); } + @Test // DATAJPA-1172 + public void exceptionsDuringParameterSettingGetThrown() { + + assertThatExceptionOfType(InvalidDataAccessApiUsageException.class) // + .isThrownBy(() -> repository.findByStringAge("twelve")) // + .matches(e -> !e.getMessage().contains("Named parameter [age] not set")); + } + + @Test // DATAJPA-1172 + public void queryProvidesCorrectNumberOfParametersForNativeQuery() { + + Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); + assertThat(query.getParameters()).hasSize(2); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java index 26fefe61a..94f016e06 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java @@ -34,6 +34,7 @@ import javax.persistence.TemporalType; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Answers; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.data.domain.Pageable; @@ -51,9 +52,10 @@ import org.springframework.data.repository.query.Param; @RunWith(MockitoJUnitRunner.class) public class ParameterBinderUnitTests { + public static final int MAX_PARAMETERS = 1; private Method valid; - @Mock private Query query; + @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Query query; private Method useIndexedParameters; private Method indexedParametersWithSort; @@ -64,6 +66,8 @@ public class ParameterBinderUnitTests { useIndexedParameters = SampleRepository.class.getMethod("useIndexedParameters", String.class); indexedParametersWithSort = SampleRepository.class.getMethod("indexedParameterWithSort", String.class, Sort.class); + + when(query.getParameters().size()).thenReturn(MAX_PARAMETERS); } static class User { @@ -91,6 +95,9 @@ public class ParameterBinderUnitTests { List validWithVarArgs(Integer... ids); User optionalParameter(Optional name); + + @org.springframework.data.jpa.repository.Query("select x from User where name = :name") + User withQuery(String name, String other); } @Test @@ -209,6 +216,18 @@ public class ParameterBinderUnitTests { verify(query).setParameter(eq(1), eq("Foo")); } + @Test // DATAJPA-1172 + public void doesNotBindExcessParameters() throws Exception { + + Method method = SampleRepository.class.getMethod("withQuery", String.class, String.class); + + Object[] values = { "foo", "superfluous" }; + ParameterBinderFactory.createBinder(new JpaParameters(method)).bind(query, values); + + verify(query).setParameter(eq(1), any()); + verify(query, never()).setParameter(eq(2), any()); + } + public SampleEntity findByEmbeddable(SampleEmbeddable embeddable) { return null; 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 f00a249a5..e841189d7 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 @@ -491,6 +491,10 @@ public interface UserRepository List findRolesAndFirstnameBy(); + // DATAJPA-1172 + @Query("select u from User u where u.age = :age") + List findByStringAge(@Param("age") String age); + static interface RolesAndFirstname { String getFirstname();