DATAJPA-863 - Polishing.

The Iterator in ParameterMetadataProvider now exposes which Part we tried to lookup a value for to report it in case no parameter value is available for anymore.

Switched to IllegalArgumentException to consistently produce the same exceptions. Removed wrapping of IllegalArgumentExceptions in CreateQueryLookupStrategy as PartTreeJpaQuery now produces IllegalArgumentExceptions in the first place.

Original pull request: #232.

src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java

@@ -100,15 +100,8 @@ 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);
 		}
-
 	}
 
 	/**

src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java

@@ -21,6 +21,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.function.Supplier;
 
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.ParameterExpression;
@@ -119,7 +120,8 @@ class ParameterMetadataProvider {
 	@SuppressWarnings("unchecked")
 	public <T> ParameterMetadata<T> next(Part part) {
 
-	    Assert.isTrue(parameters.hasNext(), "No more parameters available.");
+		Assert.isTrue(parameters.hasNext(), String.format("No parameter available for part %s.", part));
+
 		Parameter parameter = parameters.next();
 		return (ParameterMetadata<T>) next(part, parameter.getType(), parameter);
 	}
@@ -159,14 +161,19 @@ class ParameterMetadataProvider {
 		@SuppressWarnings("unchecked")
 		Class<T> reifiedType = Expression.class.equals(type) ? (Class<T>) Object.class : type;
 
-		ParameterExpression<T> expression = parameter.isExplicitlyNamed()
-				? builder.parameter(reifiedType, parameter.getName().orElseThrow(() -> new IllegalArgumentException("o_O Parameter needs to be named"))) : builder.parameter(reifiedType);
-		ParameterMetadata<T> value = new ParameterMetadata<>(expression, part.getType(),
-                bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next(),
-                this.persistenceProvider);
-		expressions.add(value);
+		Supplier<String> name = () -> parameter.getName()
+				.orElseThrow(() -> new IllegalArgumentException("o_O Parameter needs to be named"));
 
-		return value;
+		ParameterExpression<T> expression = parameter.isExplicitlyNamed() //
+				? builder.parameter(reifiedType, name.get()) //
+				: builder.parameter(reifiedType);
+
+		Object value = bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next();
+
+		ParameterMetadata<T> metadata = new ParameterMetadata<>(expression, part.getType(), value, persistenceProvider);
+		expressions.add(metadata);
+
+		return metadata;
 	}
 
 	/**
@@ -236,8 +243,9 @@ class ParameterMetadataProvider {
 				}
 			}
 
-			return Collection.class.isAssignableFrom(expressionType)
-					? persistenceProvider.potentiallyConvertEmptyCollection(toCollection(value)) : value;
+			return Collection.class.isAssignableFrom(expressionType) //
+					? persistenceProvider.potentiallyConvertEmptyCollection(toCollection(value)) //
+					: value;
 		}
 
 		/**

src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java

@@ -72,12 +72,14 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
 		boolean recreationRequired = parameters.hasDynamicProjection() || parameters.potentiallySortsDynamically();
 
 		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 e) {
-			throw new IllegalStateException(
-					String.format("Failed to create query for method <%s>: %s", method, e.getMessage()), e);
+
+		} catch (Exception o_O) {
+			throw new IllegalArgumentException(
+					String.format("Failed to create query for method %s! %s", method, o_O.getMessage()), o_O);
 		}
 	}
 

src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java

@@ -17,8 +17,8 @@ import org.springframework.aop.framework.Advised;
  */
 package org.springframework.data.jpa.repository.query;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.springframework.test.util.ReflectionTestUtils.getField;
+import static org.assertj.core.api.Assertions.*;
+import static org.springframework.test.util.ReflectionTestUtils.*;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
@@ -31,7 +31,6 @@ import javax.persistence.PersistenceContext;
 import javax.persistence.Query;
 import javax.persistence.TemporalType;
 
-import org.assertj.core.api.Assertions;
 import org.hibernate.Version;
 import org.junit.Before;
 import org.junit.Rule;
@@ -91,7 +90,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);
 	}
@@ -161,7 +160,7 @@ public class PartTreeJpaQueryIntegrationTests {
 		assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).endsWith("roles is not empty");
 	}
 
-	@Test(expected = IllegalStateException.class) // DATAJPA-1074
+	@Test(expected = IllegalArgumentException.class) // DATAJPA-1074
 	public void rejectsIsEmptyOnNonCollectionProperty() throws Exception {
 
 		JpaQueryMethod method = getQueryMethod("findByFirstnameIsEmpty");
@@ -175,10 +174,11 @@ public class PartTreeJpaQueryIntegrationTests {
 
 		JpaQueryMethod method = getQueryMethod("findByFirstname");
 
-		Assertions.assertThatExceptionOfType(IllegalStateException.class) //
+		assertThatExceptionOfType(IllegalArgumentException.class) //
 				.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) //
-				.withMessageContaining("findByFirstname") //
-				.withMessageContaining("UserRepository");
+				.withMessageContaining("findByFirstname") // the method being analyzed
+				.withMessageContaining(" firstname ") // the property we are looking for
+				.withMessageContaining("UserRepository"); // the repository
 	}
 
 	@Test // DATAJPA-863
@@ -186,7 +186,7 @@ public class PartTreeJpaQueryIntegrationTests {
 
 		JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class);
 
-		Assertions.assertThatExceptionOfType(IllegalStateException.class) //
+		assertThatExceptionOfType(IllegalArgumentException.class) //
 				.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) //
 				.withMessageContaining("findByNoSuchProperty") // the method being analyzed
 				.withMessageContaining(" noSuchProperty ") // the property we are looking for
@@ -214,7 +214,7 @@ public class PartTreeJpaQueryIntegrationTests {
 	}
 
 	@SuppressWarnings("unchecked")
-    private static <T> T getValue(Object source, String path) {
+	private static <T> T getValue(Object source, String path) {
 
 		Iterator<String> split = Arrays.asList(path.split("\\.")).iterator();
 		Object result = source;
@@ -242,7 +242,7 @@ public class PartTreeJpaQueryIntegrationTests {
 	}
 
 	@SuppressWarnings("unused")
-    interface UserRepository extends Repository<User, Long> {
+	interface UserRepository extends Repository<User, Long> {
 
 		Page<User> findByFirstname(String firstname, Pageable pageable);