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 ae6b3e002..30fdcca99 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 @@ -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 javax.persistence.criteria.CriteriaQuery; 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 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { 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. * 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 3a522cd2e..97e03bc49 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 @@ -22,6 +22,7 @@ import static org.springframework.test.util.ReflectionTestUtils.*; 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.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; @@ -169,8 +171,34 @@ public class PartTreeJpaQueryIntegrationTests { 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 @@ public class PartTreeJpaQueryIntegrationTests { } 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 @@ public class PartTreeJpaQueryIntegrationTests { List findByFirstnameIsEmpty(); + // should fail, since we can't compare scalar values to collections + List findById(Collection ids); + + // should fail, since we can't do an IN on a scalar + List findByIdIn(Long id); + // Wrong number of parameters User findByFirstname();