Browse Source

Polishing.

Refine error message format. Consistently use QueryCreationException.

See #2736
Original pull request: #2738
pull/4030/head
Mark Paluch 4 months ago
parent
commit
a15ebdbcba
No known key found for this signature in database
GPG Key ID: 55BC6374BAA9D973
  1. 6
      spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java
  2. 7
      spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java
  3. 4
      spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java
  4. 26
      spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java
  5. 21
      spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java
  6. 4
      spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java
  7. 23
      spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java
  8. 10
      spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java

6
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.domain.Sort;
import org.springframework.data.expression.ValueEvaluationContextProvider; import org.springframework.data.expression.ValueEvaluationContextProvider;
import org.springframework.data.jpa.repository.QueryRewriter; 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.ResultProcessor;
import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.ReturnedType;
import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.repository.query.ValueExpressionDelegate;
@ -124,8 +125,9 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
} }
} }
Assert.isTrue(method.isNativeQuery() || !this.query.usesJdbcStyleParameters(), if (!method.isNativeQuery() && this.query.usesJdbcStyleParameters()) {
"JDBC style parameters (?) are not supported for JPA queries"); throw QueryCreationException.create(method, "JDBC-style parameters (?) are not supported for JPA queries");
}
} }
@Override @Override

7
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.core.RepositoryMetadata;
import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.Parameters;
import org.springframework.data.repository.query.ParametersSource; 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.query.QueryMethod;
import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.repository.util.QueryExecutionConverters;
import org.springframework.data.util.Lazy; import org.springframework.data.util.Lazy;
@ -148,8 +149,10 @@ public class JpaQueryMethod extends QueryMethod {
this.metaAnnotation = Lazy this.metaAnnotation = Lazy
.of(() -> Optional.ofNullable(AnnotatedElementUtils.findMergedAnnotation(method, Meta.class))); .of(() -> Optional.ofNullable(AnnotatedElementUtils.findMergedAnnotation(method, Meta.class)));
Assert.isTrue(!(isModifyingQuery() && getParameters().hasSpecialParameter()), if (isModifyingQuery() && getParameters().hasSpecialParameter()) {
() -> String.format("Modifying method must not contain %s", Parameters.TYPES)); throw QueryCreationException.create(this,
String.format("Modifying method must not contain %s", Parameters.TYPES));
}
} }
private static Class<?> potentiallyUnwrapReturnTypeFor(RepositoryMetadata metadata, Method method) { private static Class<?> potentiallyUnwrapReturnTypeFor(RepositoryMetadata metadata, Method method) {

4
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(); Parameters<?, ?> parameters = method.getParameters();
if (parameters.hasSortParameter()) { 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.", + "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); this.namedCountQueryIsPresent = hasNamedQuery(em, countQueryName);

26
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.query.JpaQueryExecution.ScrollExecution;
import org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformation; import org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformation;
import org.springframework.data.jpa.repository.support.JpqlQueryTemplates; 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.ResultProcessor;
import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.ReturnedType;
import org.springframework.data.repository.query.parser.Part; import org.springframework.data.repository.query.parser.Part;
@ -102,13 +103,12 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
try { try {
this.tree = new PartTree(method.getName(), domainClass); this.tree = new PartTree(method.getName(), domainClass);
validate(tree, parameters, method.toString()); validate(tree, parameters);
this.countQuery = new CountQueryPreparer(); this.countQuery = new CountQueryPreparer();
this.queryPreparer = tree.isCountProjection() ? countQuery : new QueryPreparer(); this.queryPreparer = tree.isCountProjection() ? countQuery : new QueryPreparer();
} catch (Exception o_O) { } catch (Exception o_O) {
throw new IllegalArgumentException( throw QueryCreationException.create(getQueryMethod(), o_O);
String.format("Failed to create query for method %s; %s", method, o_O.getMessage()), o_O);
} }
} }
@ -142,7 +142,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
return super.getExecution(accessor); 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; int argCount = 0;
@ -154,14 +154,14 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
for (int i = 0; i < numberOfArguments; i++) { for (int i = 0; i < numberOfArguments; i++) {
throwExceptionOnArgumentMismatch(methodName, part, parameters, argCount); throwExceptionOnArgumentMismatch(part, parameters, argCount);
argCount++; argCount++;
} }
} }
} }
private static void throwExceptionOnArgumentMismatch(String methodName, Part part, JpaParameters parameters, private static void throwExceptionOnArgumentMismatch(Part part, JpaParameters parameters,
int index) { int index) {
Type type = part.getType(); Type type = part.getType();
@ -169,28 +169,28 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
if (!parameters.getBindableParameters().hasParameterAt(index)) { if (!parameters.getBindableParameters().hasParameterAt(index)) {
throw new IllegalStateException(String.format( 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", "Method 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)); index + 1, index, type.name(), property));
} }
JpaParameter parameter = parameters.getBindableParameter(index); JpaParameter parameter = parameters.getBindableParameter(index);
if (expectsCollection(type)) { if (expectsCollection(type)) {
if (!parameterIsCollectionLike(parameter)) { if (!parameterIsCollectionLike(parameter)) {
throw new IllegalStateException(wrongParameterTypeMessage(methodName, property, type, "Collection", parameter)); throw new IllegalStateException(wrongParameterTypeMessage(property, type, "Collection", parameter));
} }
} else { } else {
if (!part.getProperty().isCollection() && !parameterIsScalarLike(parameter)) { 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) { String expectedArgumentType, JpaParameter parameter) {
return String.format("Operator %s on %s requires a %s argument, found %s in method %s", operatorType.name(), return String.format("Operator '%s' on '%s' requires a %s argument, found '%s'", operatorType.name(), property,
property, expectedArgumentType, parameter.getType(), methodName); expectedArgumentType, parameter.getType());
} }
private static boolean parameterIsCollectionLike(JpaParameter parameter) { private static boolean parameterIsCollectionLike(JpaParameter parameter) {

21
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.jspecify.annotations.Nullable;
import org.springframework.data.repository.query.QueryCreationException;
import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.RepositoryQuery;
/** /**
@ -48,10 +49,10 @@ class SimpleJpaQuery extends AbstractStringBasedJpaQuery {
super(method, em, query, countQuery, queryConfiguration); 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()) { 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; return;
} }
EntityManager validatingEm = null; String queryString = query.getQueryString();
var queryString = query.getQueryString(); try (EntityManager validatingEm = getEntityManager().getEntityManagerFactory().createEntityManager()) {
try {
validatingEm = getEntityManager().getEntityManagerFactory().createEntityManager();
validatingEm.createQuery(queryString); validatingEm.createQuery(queryString);
} catch (RuntimeException e) { } catch (RuntimeException e) {
// Needed as there's ambiguities in how an invalid query string shall be expressed by the persistence provider // 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 // https://download.oracle.com/javaee-archive/jpa-spec.java.net/users/2012/07/0404.html
throw new IllegalArgumentException(errorMessage.formatted(query, method), e); throw QueryCreationException.create(method, errorMessage.formatted(queryString), e);
} }
} }
} }

4
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.NamedQueries;
import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; 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;
import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.QueryLookupStrategy.Key;
import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.RepositoryQuery;
@ -58,6 +59,7 @@ import org.springframework.data.repository.query.ValueExpressionDelegate;
* @author Jens Schauder * @author Jens Schauder
* @author Réda Housni Alaoui * @author Réda Housni Alaoui
* @author Greg Turnquist * @author Greg Turnquist
* @author Mark Paluch
*/ */
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT) @MockitoSettings(strictness = Strictness.LENIENT)
@ -160,7 +162,7 @@ class JpaQueryLookupStrategyUnitTests {
Method method = UserRepository.class.getMethod("customNamedQuery", String.class, Sort.class); Method method = UserRepository.class.getMethod("customNamedQuery", String.class, Sort.class);
RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class);
assertThatIllegalStateException() assertThatExceptionOfType(QueryCreationException.class)
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries)) .isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries))
.withMessageContaining( .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."); "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.");

23
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.Repository;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
import org.springframework.data.repository.query.Param; 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.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.context.junit.jupiter.SpringExtension;
@ -192,8 +193,7 @@ class PartTreeJpaQueryIntegrationTests {
assertThatExceptionOfType(RuntimeException.class) // assertThatExceptionOfType(RuntimeException.class) //
.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) //
.withMessageContaining("findByIdIn") // .withMessageContaining("'IN'") //
.withMessageContaining(" IN ") //
.withMessageContaining("Collection") // .withMessageContaining("Collection") //
.withMessageContaining("Integer"); .withMessageContaining("Integer");
} }
@ -203,11 +203,10 @@ class PartTreeJpaQueryIntegrationTests {
JpaQueryMethod method = getQueryMethod("findById", Collection.class); JpaQueryMethod method = getQueryMethod("findById", Collection.class);
assertThatExceptionOfType(RuntimeException.class) // assertThatExceptionOfType(QueryCreationException.class) //
.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) //
.withMessageContaining("findById") // .withMessageContaining("'SIMPLE_PROPERTY'") //
.withMessageContaining(" SIMPLE_PROPERTY ") // .withMessageContaining("scalar ") //
.withMessageContaining(" scalar ") //
.withMessageContaining("Collection"); .withMessageContaining("Collection");
} }
@ -226,11 +225,9 @@ class PartTreeJpaQueryIntegrationTests {
JpaQueryMethod method = getQueryMethod("findByFirstname"); JpaQueryMethod method = getQueryMethod("findByFirstname");
assertThatExceptionOfType(IllegalArgumentException.class) // assertThatExceptionOfType(QueryCreationException.class) //
.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) //
.withMessageContaining("findByFirstname") // the method being analyzed .withMessageContaining("'firstname'"); // the property we are looking for
.withMessageContaining(" firstname ") // the property we are looking for
.withMessageContaining("UserRepository"); // the repository
} }
@Test // DATAJPA-863 @Test // DATAJPA-863
@ -238,11 +235,9 @@ class PartTreeJpaQueryIntegrationTests {
JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class); JpaQueryMethod method = getQueryMethod("findByNoSuchProperty", String.class);
assertThatExceptionOfType(IllegalArgumentException.class) // assertThatExceptionOfType(QueryCreationException.class) //
.isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) // .isThrownBy(() -> new PartTreeJpaQuery(method, entityManager)) //
.withMessageContaining("findByNoSuchProperty") // the method being analyzed .withMessageContaining("'noSuchProperty'"); // the property we are looking for
.withMessageContaining("'noSuchProperty'") // the property we are looking for
.withMessageContaining("UserRepository"); // the repository
} }
@Test // GH-3356 @Test // GH-3356

10
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.RepositoryMetadata;
import org.springframework.data.repository.core.support.AbstractRepositoryMetadata; import org.springframework.data.repository.core.support.AbstractRepositoryMetadata;
import org.springframework.data.repository.query.Param; 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.RepositoryQuery;
import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ResultProcessor;
import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.repository.query.ValueExpressionDelegate;
@ -192,17 +193,16 @@ class SimpleJpaQueryUnitTests {
createJpaQuery(method); createJpaQuery(method);
} }
@Test // DATAJPA-352 @Test // DATAJPA-352, GH-2736
void validatesAndRejectsCountQueryIfPagingMethod() throws Exception { void validatesAndRejectsCountQueryIfPagingMethod() throws Exception {
Method method = SampleRepository.class.getMethod("pageByAnnotatedQuery", Pageable.class); Method method = SampleRepository.class.getMethod("pageByAnnotatedQuery", Pageable.class);
when(em.createQuery(Mockito.contains("count"))).thenThrow(IllegalArgumentException.class); when(em.createQuery(Mockito.contains("count"))).thenThrow(IllegalArgumentException.class);
assertThatIllegalArgumentException() // assertThatExceptionOfType(QueryCreationException.class) //
.isThrownBy(() -> createJpaQuery(method)) // .isThrownBy(() -> createJpaQuery(method)) //
.withMessageContaining("Count") // .withMessageContaining("User u");
.withMessageContaining(method.getName());
} }
@Test @Test
@ -323,7 +323,7 @@ class SimpleJpaQueryUnitTests {
Method illegalMethod = SampleRepository.class.getMethod("illegalUseOfJdbcStyleParameters", String.class); Method illegalMethod = SampleRepository.class.getMethod("illegalUseOfJdbcStyleParameters", String.class);
assertThatIllegalArgumentException().isThrownBy(() -> createJpaQuery(illegalMethod)); assertThatExceptionOfType(QueryCreationException.class).isThrownBy(() -> createJpaQuery(illegalMethod));
} }
@Test // GH-3929 @Test // GH-3929

Loading…
Cancel
Save