From a15ebdbcbafde3f45efdc1e05a583d1876df2bed Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 29 Sep 2025 09:07:54 +0200 Subject: [PATCH] Polishing. Refine error message format. Consistently use QueryCreationException. See #2736 Original pull request: #2738 --- .../query/AbstractStringBasedJpaQuery.java | 6 +++-- .../jpa/repository/query/JpaQueryMethod.java | 7 +++-- .../data/jpa/repository/query/NamedQuery.java | 4 +-- .../repository/query/PartTreeJpaQuery.java | 26 +++++++++---------- .../jpa/repository/query/SimpleJpaQuery.java | 21 +++++++-------- .../JpaQueryLookupStrategyUnitTests.java | 4 ++- .../PartTreeJpaQueryIntegrationTests.java | 23 +++++++--------- .../query/SimpleJpaQueryUnitTests.java | 10 +++---- 8 files changed, 50 insertions(+), 51 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 841eaffff..09f54bbc7 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -29,6 +29,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.expression.ValueEvaluationContextProvider; import org.springframework.data.jpa.repository.QueryRewriter; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.ValueExpressionDelegate; @@ -124,8 +125,9 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { } } - Assert.isTrue(method.isNativeQuery() || !this.query.usesJdbcStyleParameters(), - "JDBC style parameters (?) are not supported for JPA queries"); + if (!method.isNativeQuery() && this.query.usesJdbcStyleParameters()) { + throw QueryCreationException.create(method, "JDBC-style parameters (?) are not supported for JPA queries"); + } } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java index 10b985449..219ce9579 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java @@ -43,6 +43,7 @@ import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersSource; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.util.Lazy; @@ -148,8 +149,10 @@ public class JpaQueryMethod extends QueryMethod { this.metaAnnotation = Lazy .of(() -> Optional.ofNullable(AnnotatedElementUtils.findMergedAnnotation(method, Meta.class))); - Assert.isTrue(!(isModifyingQuery() && getParameters().hasSpecialParameter()), - () -> String.format("Modifying method must not contain %s", Parameters.TYPES)); + if (isModifyingQuery() && getParameters().hasSpecialParameter()) { + throw QueryCreationException.create(this, + String.format("Modifying method must not contain %s", Parameters.TYPES)); + } } private static Class potentiallyUnwrapReturnTypeFor(RepositoryMetadata metadata, Method method) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java index 9bfbb750e..2eaaa0ef8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java @@ -75,9 +75,9 @@ final class NamedQuery extends AbstractJpaQuery { Parameters parameters = method.getParameters(); if (parameters.hasSortParameter()) { - throw new IllegalStateException(String.format("Query method %s is backed by a NamedQuery and must " + throw QueryCreationException.create(method, String.format("Query method is backed by a NamedQuery and must " + "not contain a sort parameter as we cannot modify the query; Use @%s(value=…) instead to apply sorting or remove the 'Sort' parameter.", - method, method.isNativeQuery() ? "NativeQuery" : "Query")); + method.isNativeQuery() ? "NativeQuery" : "Query")); } this.namedCountQueryIsPresent = hasNamedQuery(em, countQueryName); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java index 06ee74cf0..d6d9cb19d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java @@ -38,6 +38,7 @@ import org.springframework.data.jpa.repository.query.JpaQueryExecution.ExistsExe import org.springframework.data.jpa.repository.query.JpaQueryExecution.ScrollExecution; import org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformation; import org.springframework.data.jpa.repository.support.JpqlQueryTemplates; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.parser.Part; @@ -102,13 +103,12 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { try { this.tree = new PartTree(method.getName(), domainClass); - validate(tree, parameters, method.toString()); + validate(tree, parameters); this.countQuery = new CountQueryPreparer(); this.queryPreparer = tree.isCountProjection() ? countQuery : new QueryPreparer(); } catch (Exception o_O) { - throw new IllegalArgumentException( - String.format("Failed to create query for method %s; %s", method, o_O.getMessage()), o_O); + throw QueryCreationException.create(getQueryMethod(), o_O); } } @@ -142,7 +142,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { return super.getExecution(accessor); } - private static void validate(PartTree tree, JpaParameters parameters, String methodName) { + private static void validate(PartTree tree, JpaParameters parameters) { int argCount = 0; @@ -154,14 +154,14 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { for (int i = 0; i < numberOfArguments; i++) { - throwExceptionOnArgumentMismatch(methodName, part, parameters, argCount); + throwExceptionOnArgumentMismatch(part, parameters, argCount); argCount++; } } } - private static void throwExceptionOnArgumentMismatch(String methodName, Part part, JpaParameters parameters, + private static void throwExceptionOnArgumentMismatch(Part part, JpaParameters parameters, int index) { Type type = part.getType(); @@ -169,28 +169,28 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { if (!parameters.getBindableParameters().hasParameterAt(index)) { throw new IllegalStateException(String.format( - "Method %s expects 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)); + "Method expects at least %d arguments but only found %d; This leaves an operator of type '%s' for property '%s' unbound", + index + 1, index, type.name(), property)); } JpaParameter parameter = parameters.getBindableParameter(index); if (expectsCollection(type)) { if (!parameterIsCollectionLike(parameter)) { - throw new IllegalStateException(wrongParameterTypeMessage(methodName, property, type, "Collection", parameter)); + throw new IllegalStateException(wrongParameterTypeMessage(property, type, "Collection", parameter)); } } else { if (!part.getProperty().isCollection() && !parameterIsScalarLike(parameter)) { - throw new IllegalStateException(wrongParameterTypeMessage(methodName, property, type, "scalar", parameter)); + throw new IllegalStateException(wrongParameterTypeMessage(property, type, "scalar", parameter)); } } } - private static String wrongParameterTypeMessage(String methodName, String property, Type operatorType, + private static String wrongParameterTypeMessage(String property, Type operatorType, String expectedArgumentType, JpaParameter parameter) { - return String.format("Operator %s on %s requires a %s argument, found %s in method %s", operatorType.name(), - property, expectedArgumentType, parameter.getType(), methodName); + return String.format("Operator '%s' on '%s' requires a %s argument, found '%s'", operatorType.name(), property, + expectedArgumentType, parameter.getType()); } private static boolean parameterIsCollectionLike(JpaParameter parameter) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java index 3c0410fbf..4d1f5e7f4 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java @@ -20,6 +20,7 @@ import jakarta.persistence.Query; import org.jspecify.annotations.Nullable; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.RepositoryQuery; /** @@ -48,10 +49,10 @@ class SimpleJpaQuery extends AbstractStringBasedJpaQuery { super(method, em, query, countQuery, queryConfiguration); - validateQuery(getQuery(), "Validation failed for query %s for method %s", method); + validateQuery(getQuery(), "Query validation failed for '%s'", method); if (method.isPageQuery()) { - validateQuery(getCountQuery(), "Count query %s validation failed for method %s", method); + validateQuery(getCountQuery(), "Count query validation failed for '%s'", method); } } @@ -67,18 +68,14 @@ class SimpleJpaQuery extends AbstractStringBasedJpaQuery { return; } - EntityManager validatingEm = null; - var queryString = query.getQueryString(); - - try { - validatingEm = getEntityManager().getEntityManagerFactory().createEntityManager(); + String queryString = query.getQueryString(); + try (EntityManager validatingEm = getEntityManager().getEntityManagerFactory().createEntityManager()) { validatingEm.createQuery(queryString); - } catch (RuntimeException e) { - // Needed as there's ambiguities in how an invalid query string shall be expressed by the persistence provider - // https://download.oracle.com/javaee-archive/jpa-spec.java.net/users/2012/07/0404.html - throw new IllegalArgumentException(errorMessage.formatted(query, method), e); - } + // Needed as there's ambiguities in how an invalid query string shall be expressed by the persistence provider + // https://download.oracle.com/javaee-archive/jpa-spec.java.net/users/2012/07/0404.html + throw QueryCreationException.create(method, errorMessage.formatted(queryString), e); + } } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java index e68faf409..0c76c343c 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java @@ -45,6 +45,7 @@ import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.NamedQueries; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.RepositoryQuery; @@ -58,6 +59,7 @@ import org.springframework.data.repository.query.ValueExpressionDelegate; * @author Jens Schauder * @author Réda Housni Alaoui * @author Greg Turnquist + * @author Mark Paluch */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -160,7 +162,7 @@ class JpaQueryLookupStrategyUnitTests { Method method = UserRepository.class.getMethod("customNamedQuery", String.class, Sort.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class); - assertThatIllegalStateException() + assertThatExceptionOfType(QueryCreationException.class) .isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries)) .withMessageContaining( "is backed by a NamedQuery and must not contain a sort parameter as we cannot modify the query; Use @Query(value=…) instead to apply sorting or remove the 'Sort' parameter."); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java index 02d63e677..3b1fe5456 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java @@ -50,6 +50,7 @@ import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; import org.springframework.data.repository.query.Param; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -192,8 +193,7 @@ class PartTreeJpaQueryIntegrationTests { assertThatExceptionOfType(RuntimeException.class) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // - .withMessageContaining("findByIdIn") // - .withMessageContaining(" IN ") // + .withMessageContaining("'IN'") // .withMessageContaining("Collection") // .withMessageContaining("Integer"); } @@ -203,11 +203,10 @@ class PartTreeJpaQueryIntegrationTests { JpaQueryMethod method = getQueryMethod("findById", Collection.class); - assertThatExceptionOfType(RuntimeException.class) // + assertThatExceptionOfType(QueryCreationException.class) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // - .withMessageContaining("findById") // - .withMessageContaining(" SIMPLE_PROPERTY ") // - .withMessageContaining(" scalar ") // + .withMessageContaining("'SIMPLE_PROPERTY'") // + .withMessageContaining("scalar ") // .withMessageContaining("Collection"); } @@ -226,11 +225,9 @@ class PartTreeJpaQueryIntegrationTests { JpaQueryMethod method = getQueryMethod("findByFirstname"); - assertThatExceptionOfType(IllegalArgumentException.class) // + assertThatExceptionOfType(QueryCreationException.class) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // - .withMessageContaining("findByFirstname") // the method being analyzed - .withMessageContaining(" firstname ") // the property we are looking for - .withMessageContaining("UserRepository"); // the repository + .withMessageContaining("'firstname'"); // the property we are looking for } @Test // DATAJPA-863 @@ -238,11 +235,9 @@ class PartTreeJpaQueryIntegrationTests { JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class); - assertThatExceptionOfType(IllegalArgumentException.class) // + assertThatExceptionOfType(QueryCreationException.class) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // - .withMessageContaining("findByNoSuchProperty") // the method being analyzed - .withMessageContaining("'noSuchProperty'") // the property we are looking for - .withMessageContaining("UserRepository"); // the repository + .withMessageContaining("'noSuchProperty'"); // the property we are looking for } @Test // GH-3356 diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java index 064189a55..6866e7c2d 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java @@ -57,6 +57,7 @@ import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.AbstractRepositoryMetadata; import org.springframework.data.repository.query.Param; +import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ValueExpressionDelegate; @@ -192,17 +193,16 @@ class SimpleJpaQueryUnitTests { createJpaQuery(method); } - @Test // DATAJPA-352 + @Test // DATAJPA-352, GH-2736 void validatesAndRejectsCountQueryIfPagingMethod() throws Exception { Method method = SampleRepository.class.getMethod("pageByAnnotatedQuery", Pageable.class); when(em.createQuery(Mockito.contains("count"))).thenThrow(IllegalArgumentException.class); - assertThatIllegalArgumentException() // + assertThatExceptionOfType(QueryCreationException.class) // .isThrownBy(() -> createJpaQuery(method)) // - .withMessageContaining("Count") // - .withMessageContaining(method.getName()); + .withMessageContaining("User u"); } @Test @@ -323,7 +323,7 @@ class SimpleJpaQueryUnitTests { Method illegalMethod = SampleRepository.class.getMethod("illegalUseOfJdbcStyleParameters", String.class); - assertThatIllegalArgumentException().isThrownBy(() -> createJpaQuery(illegalMethod)); + assertThatExceptionOfType(QueryCreationException.class).isThrownBy(() -> createJpaQuery(illegalMethod)); } @Test // GH-3929