DATAJPA-1182 - Validate method parameters on query construction.

For “In” and “NotIn” queries not providing a collection like argument results in an IllegalStateException at initialisation time.

Conversely providing a collection as argument for a query other than “In” or “NotIn” also throws such an exception.

Original pull request: #228.

Author: schauder

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.jpa.repository.query;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 
@@ -26,13 +27,17 @@
 
 import org.springframework.data.domain.Sort;
 import org.springframework.data.jpa.provider.PersistenceProvider;
+import org.springframework.data.jpa.repository.query.JpaParameters.JpaParameter;
 import org.springframework.data.jpa.repository.query.JpaQueryExecution.DeleteExecution;
 import org.springframework.data.jpa.repository.query.JpaQueryExecution.ExistsExecution;
 import org.springframework.data.jpa.repository.query.ParameterMetadataProvider.ParameterMetadata;
 import org.springframework.data.repository.query.ParametersParameterAccessor;
 import org.springframework.data.repository.query.ResultProcessor;
 import org.springframework.data.repository.query.ReturnedType;
+import org.springframework.data.repository.query.parser.Part;
+import org.springframework.data.repository.query.parser.Part.Type;
 import org.springframework.data.repository.query.parser.PartTree;
+import org.springframework.data.repository.query.parser.PartTree.OrPart;
 import org.springframework.lang.Nullable;
 
 /**
@@ -76,6 +81,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
 		try {
 
 			this.tree = new PartTree(method.getName(), domainClass);
+			validate(tree, parameters, method.toString());
 			this.countQuery = new CountQueryPreparer(persistenceProvider, recreationRequired);
 			this.query = tree.isCountProjection() ? countQuery : new QueryPreparer(persistenceProvider, recreationRequired);
 
@@ -120,6 +126,74 @@ protected JpaQueryExecution getExecution() {
 		return super.getExecution();
 	}
 
+	private static void validate(PartTree tree, JpaParameters parameters, String methodName) {
+
+		int argCount = 0;
+
+		for (OrPart orPart : tree) {
+
+			for (Part part : orPart) {
+
+				int numberOfArguments = part.getNumberOfArguments();
+
+				for (int i = 0; i < numberOfArguments; i++) {
+
+					throwExceptionOnArgumentMismatch(methodName, part, parameters, argCount);
+
+					argCount++;
+				}
+			}
+		}
+	}
+
+	private static void throwExceptionOnArgumentMismatch(String methodName, Part part, JpaParameters parameters,
+			int index) {
+
+		Type type = part.getType();
+		String property = part.getProperty().toDotPath();
+
+		if (!parameters.getBindableParameters().hasParameterAt(index)) {
+			throw new IllegalStateException(String.format(
+					"For the method %s we expect at least %d arguments but only found %d. This leaves an operator of type %s for property %s unbound.",
+					methodName, index + 1, index, type.name(), property));
+		}
+
+		JpaParameter parameter = parameters.getBindableParameter(index);
+
+		if (expectsCollection(type) && !parameterIsCollectionLike(parameter)) {
+			throw new IllegalStateException(wrongParameterTypeMessage(methodName, property, type, "Collection", parameter));
+		} else if (!expectsCollection(type) && !parameterIsScalarLike(parameter)) {
+			throw new IllegalStateException(wrongParameterTypeMessage(methodName, property, type, "scalar", parameter));
+		}
+	}
+
+	private static String wrongParameterTypeMessage(String methodName, String property, Type operatorType,
+			String expectedArgumenType, JpaParameter parameter) {
+
+		return String.format( //
+				"The operator %s on %s requires a %s argument, but we found %s in method %s", //
+				operatorType.name(), //
+				property, expectedArgumenType, //
+				parameter.getType(), //
+				methodName //
+		);
+	}
+
+	private static boolean parameterIsCollectionLike(JpaParameter parameter) {
+		return Collection.class.isAssignableFrom(parameter.getType()) || parameter.getType().isArray();
+	}
+
+	/**
+	 * Arrays are may be treated as collection like or in the case of binary data as scalar
+	 */
+	private static boolean parameterIsScalarLike(JpaParameter parameter) {
+		return !Collection.class.isAssignableFrom(parameter.getType());
+	}
+
+	private static boolean expectsCollection(Type type) {
+		return type == Type.IN || type == Type.NOT_IN;
+	}
+
 	/**
 	 * Query preparer to create {@link CriteriaQuery} instances and potentially cache them.
 	 *

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

@@ -22,6 +22,7 @@
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
@@ -31,6 +32,7 @@
 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;
@@ -169,8 +171,34 @@ public void rejectsIsEmptyOnNonCollectionProperty() throws Exception {
 		jpaQuery.createQuery(new Object[] { "Oliver" });
 	}
 
+	@Test // DATAJPA-1182
+	public void rejectsInPredicateWithNonIterableParameter() throws Exception {
+
+		JpaQueryMethod method = getQueryMethod("findByIdIn", Long.class);
+
+		Assertions.assertThatExceptionOfType(RuntimeException.class) //
+				.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) //
+				.withMessageContaining("findByIdIn") //
+				.withMessageContaining(" IN ") //
+				.withMessageContaining("Collection") //
+				.withMessageContaining("Long");
+	}
+
+	@Test // DATAJPA-1182
+	public void rejectsOtherThanInPredicateWithIterableParameter() throws Exception {
+
+		JpaQueryMethod method = getQueryMethod("findById", Collection.class);
+
+		Assertions.assertThatExceptionOfType(RuntimeException.class) //
+				.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) //
+				.withMessageContaining("findById") //
+				.withMessageContaining(" SIMPLE_PROPERTY ") //
+				.withMessageContaining(" scalar ") //
+				.withMessageContaining("Collection");
+	}
+
 	@Test // DATAJPA-863
-	public void errorsDueToMismatchOfParametersContainNameOfMethodAndInterface() throws Exception {
+	public void errorsDueToMismatchOfParametersContainNameOfMethodInterfaceAndPropertyPath() throws Exception {
 
 		JpaQueryMethod method = getQueryMethod("findByFirstname");
 
@@ -208,6 +236,7 @@ private void testIgnoreCase(String methodName, Object... values) throws Exceptio
 	}
 
 	private JpaQueryMethod getQueryMethod(String methodName, Class<?>... parameterTypes) throws Exception {
+
 		Method method = UserRepository.class.getMethod(methodName, parameterTypes);
 		return new JpaQueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class),
 				new SpelAwareProxyProjectionFactory(), PersistenceProvider.fromEntityManager(entityManager));
@@ -260,6 +289,12 @@ interface UserRepository extends Repository<User, Long> {
 
 		List<User> findByFirstnameIsEmpty();
 
+		// should fail, since we can't compare scalar values to collections
+		List<User> findById(Collection<Long> ids);
+
+		// should fail, since we can't do an IN on a scalar
+		List<User> findByIdIn(Long id);
+
 		// Wrong number of parameters
 		User findByFirstname();