diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java index 01e331d9f..2245edb23 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java @@ -98,13 +98,7 @@ public final class JpaQueryLookupStrategy { @Override protected RepositoryQuery resolveQuery(JpaQueryMethod method, EntityManager em, NamedQueries namedQueries) { - - try { - return new PartTreeJpaQuery(method, em, persistenceProvider); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException( - String.format("Could not create query metamodel for method %s!", method.toString()), e); - } + return new PartTreeJpaQuery(method, em, persistenceProvider); } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java index 781db7462..3633262b2 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java @@ -119,6 +119,8 @@ class ParameterMetadataProvider { @SuppressWarnings("unchecked") public ParameterMetadata next(Part part) { + Assert.isTrue(parameters.hasNext(), String.format("No parameter available for part %s.", part)); + Parameter parameter = parameters.next(); return (ParameterMetadata) next(part, parameter.getType(), parameter); } @@ -159,7 +161,8 @@ class ParameterMetadataProvider { Class reifiedType = Expression.class.equals(type) ? (Class) Object.class : type; ParameterExpression expression = parameter.isExplicitlyNamed() - ? builder.parameter(reifiedType, parameter.getName()) : builder.parameter(reifiedType); + ? builder.parameter(reifiedType, parameter.getName()) + : builder.parameter(reifiedType); ParameterMetadata value = new ParameterMetadata(expression, part.getType(), bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next(), this.persistenceProvider); @@ -242,7 +245,8 @@ class ParameterMetadataProvider { } return Collection.class.isAssignableFrom(expressionType) - ? persistenceProvider.potentiallyConvertEmptyCollection(toCollection(value)) : value; + ? persistenceProvider.potentiallyConvertEmptyCollection(toCollection(value)) + : value; } /** diff --git a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java index 707b0a51c..20cbad6f9 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java @@ -61,13 +61,20 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { this.em = em; this.domainClass = method.getEntityInformation().getJavaType(); - this.tree = new PartTree(method.getName(), domainClass); this.parameters = method.getParameters(); boolean recreationRequired = parameters.hasDynamicProjection() || parameters.potentiallySortsDynamically(); - this.countQuery = new CountQueryPreparer(persistenceProvider, recreationRequired); - this.query = tree.isCountProjection() ? countQuery : new QueryPreparer(persistenceProvider, recreationRequired); + try { + + this.tree = new PartTree(method.getName(), domainClass); + this.countQuery = new CountQueryPreparer(persistenceProvider, recreationRequired); + this.query = tree.isCountProjection() ? countQuery : new QueryPreparer(persistenceProvider, recreationRequired); + + } catch (Exception o_O) { + throw new IllegalArgumentException( + String.format("Failed to create query method %s! %s", method, o_O.getMessage()), o_O); + } } /* @@ -96,9 +103,9 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { @Override protected JpaQueryExecution getExecution() { - if(this.tree.isDelete()) { + if (this.tree.isDelete()) { return new DeleteExecution(em); - } else if(this.tree.isExistsProjection()) { + } else if (this.tree.isExistsProjection()) { return new ExistsExecution(); } @@ -176,7 +183,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { query.setMaxResults(tree.getMaxResults()); } - if(tree.isExistsProjection()) { + if (tree.isExistsProjection()) { query.setMaxResults(1); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java new file mode 100644 index 000000000..cb2c3c752 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import static org.mockito.Mockito.*; + +import javax.persistence.criteria.CriteriaBuilder; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.repository.query.Parameters; +import org.springframework.data.repository.query.parser.Part; + +/** + * Unit tests for {@link ParameterMetadataProvider}. + * + * @author Jens Schauder + * @author Oliver Gierke + */ +public class ParameterMetadataProviderUnitTests { + + public @Rule ExpectedException exception = ExpectedException.none(); + + @Test // DATAJPA-863 + public void errorMessageMentionesParametersWhenParametersAreExhausted() { + + PersistenceProvider persistenceProvider = mock(PersistenceProvider.class); + CriteriaBuilder builder = mock(CriteriaBuilder.class); + + Parameters parameters = mock(Parameters.class, RETURNS_DEEP_STUBS); + ParameterMetadataProvider metadataProvider = new ParameterMetadataProvider(builder, parameters, + persistenceProvider); + + exception.expect(IllegalArgumentException.class); + exception.expectMessage("parameter"); + + metadataProvider.next(mock(Part.class)); + } +} diff --git a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java index 669083ee7..55d05cfbe 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java @@ -88,7 +88,7 @@ public class PartTreeJpaQueryIntegrationTests { @Test public void cannotIgnoreCaseIfNotString() throws Exception { - thrown.expect(IllegalStateException.class); + thrown.expect(IllegalArgumentException.class); thrown.expectMessage("Unable to ignore case of java.lang.Integer types, the property 'id' must reference a String"); testIgnoreCase("findByIdIgnoringCase", 3); } @@ -120,7 +120,7 @@ public class PartTreeJpaQueryIntegrationTests { JpaQueryMethod queryMethod = getQueryMethod("existsByFirstname", String.class); PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider); - Query query = jpaQuery.createQuery(new Object[]{"Matthews"}); + Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); assertThat(query.getMaxResults(), is(1)); } @@ -131,11 +131,37 @@ public class PartTreeJpaQueryIntegrationTests { JpaQueryMethod queryMethod = getQueryMethod("existsByFirstname", String.class); PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider); - Query query = jpaQuery.createQuery(new Object[]{"Matthews"}); + Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), containsString(".id from User as")); } + @Test // DATAJPA-863 + public void errorsDueToMismatchOfParametersContainNameOfMethodAndInterface() throws Exception { + + JpaQueryMethod method = getQueryMethod("findByFirstname"); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("UserRepository"); // the repository + thrown.expectMessage("findByFirstname"); // the method being analyzed + thrown.expectMessage(" firstname "); // the property we are looking for + + new PartTreeJpaQuery(method, entityManager, provider); + } + + @Test // DATAJPA-863 + public void errorsDueToMissingPropertyContainNameOfMethodAndInterface() throws Exception { + + JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("UserRepository"); // the repository + thrown.expectMessage("findByNoSuchProperty"); // the method being analyzed + thrown.expectMessage(" noSuchProperty "); // the property we are looking for + + new PartTreeJpaQuery(method, entityManager, provider); + } + private void testIgnoreCase(String methodName, Object... values) throws Exception { Class[] parameterTypes = new Class[values.length]; @@ -192,5 +218,11 @@ public class PartTreeJpaQueryIntegrationTests { boolean existsByFirstname(String firstname); List findByCreatedAtAfter(@Temporal(TemporalType.TIMESTAMP) @Param("refDate") Date refDate); + + // Wrong number of parameters + User findByFirstname(); + + // Wrong property name + User findByNoSuchProperty(String x); } }