From 5f917a945265274aafcda22136c35fca6d81fc12 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 10 Nov 2017 10:03:20 +0100 Subject: [PATCH] DATAJPA-863 - Improved error message for failed method construction. The error message now mentions the method including interface name. Original pull request: #232. --- .../query/ParameterMetadataProvider.java | 2 + .../repository/query/PartTreeJpaQuery.java | 11 ++-- .../ParameterMetadataProviderUnitTests.java | 54 +++++++++++++++++++ .../PartTreeJpaQueryIntegrationTests.java | 37 +++++++++++-- 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java 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 d1149839f..e8718fc91 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 @@ -44,6 +44,7 @@ import org.springframework.util.ObjectUtils; * @author Thomas Darimont * @author Mark Paluch * @author Christoph Strobl + * @author Jens Schauder */ class ParameterMetadataProvider { @@ -121,6 +122,7 @@ class ParameterMetadataProvider { @SuppressWarnings("unchecked") public ParameterMetadata next(Part part) { + Assert.isTrue(parameters.hasNext(), "No more parameters available."); Parameter parameter = parameters.next(); return (ParameterMetadata) next(part, parameter.getType(), parameter); } 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 c0ecfd493..47c11b2a2 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 @@ -67,13 +67,18 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { this.em = em; Class 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 e) { + throw new IllegalStateException( + String.format("Failed to create query for method <%s>: %s", method, e.getMessage()), e); + } } /* 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..f13f5626a --- /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.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.*; + +import java.util.Collections; + +import javax.persistence.criteria.CriteriaBuilder; + +import org.junit.Test; +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 + */ +public class ParameterMetadataProviderUnitTests { + + @Test // DATAJPA-863 + public void errorMessageMentionesParametersWhenParametersAreExhausted() { + + PersistenceProvider persistenceProvider = mock(PersistenceProvider.class); + CriteriaBuilder builder = mock(CriteriaBuilder.class); + + Parameters parameters = mock(Parameters.class, RETURNS_DEEP_STUBS); + when(parameters.getBindableParameters().iterator()).thenReturn(Collections.emptyListIterator()); + + ParameterMetadataProvider metadataProvider = new ParameterMetadataProvider(builder, parameters, + persistenceProvider); + + assertThatExceptionOfType(RuntimeException.class) // + .isThrownBy(() -> metadataProvider.next(mock(Part.class))) // + .withMessageContaining("parameter"); + } + +} 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 30a7bb251..71ff0486c 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 @@ -18,8 +18,8 @@ import org.springframework.aop.framework.Advised; package org.springframework.data.jpa.repository.query; import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; -import static org.springframework.test.util.ReflectionTestUtils.*; +import static org.junit.Assert.assertThat; +import static org.springframework.test.util.ReflectionTestUtils.getField; import java.lang.reflect.Method; import java.util.Arrays; @@ -32,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; @@ -159,7 +160,7 @@ public class PartTreeJpaQueryIntegrationTests { assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), endsWith("roles is not empty")); } - @Test(expected = IllegalArgumentException.class) // DATAJPA-1074 + @Test(expected = IllegalStateException.class) // DATAJPA-1074 public void rejectsIsEmptyOnNonCollectionProperty() throws Exception { JpaQueryMethod method = getQueryMethod("findByFirstnameIsEmpty"); @@ -168,6 +169,29 @@ public class PartTreeJpaQueryIntegrationTests { jpaQuery.createQuery(new Object[] { "Oliver" }); } + @Test // DATAJPA-863 + public void errorsDueToMismatchOfParametersContainNameOfMethodAndInterface() throws Exception { + + JpaQueryMethod method = getQueryMethod("findByFirstname"); + + Assertions.assertThatExceptionOfType(IllegalStateException.class) // + .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) // + .withMessageContaining("findByFirstname") // + .withMessageContaining("UserRepository"); + } + + @Test // DATAJPA-863 + public void errorsDueToMissingPropertyContainNameOfMethodAndInterface() throws Exception { + + JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class); + + Assertions.assertThatExceptionOfType(IllegalStateException.class) // + .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager, provider)) // + .withMessageContaining("findByNoSuchProperty") // the method being analyzed + .withMessageContaining(" noSuchProperty ") // the property we are looking for + .withMessageContaining("UserRepository"); // the repository + } + private void testIgnoreCase(String methodName, Object... values) throws Exception { Class[] parameterTypes = new Class[values.length]; @@ -230,5 +254,12 @@ public class PartTreeJpaQueryIntegrationTests { List findByRolesIsNotEmpty(); List findByFirstnameIsEmpty(); + + // Wrong number of parameters + User findByFirstname(); + + // Wrong property name + User findByNoSuchProperty(String x); } + }