JpaQueryLookupStrategy shouldn't use exceptions for flow control.

By using exceptions for flow control, other critical exceptions are getting masked. The lack of a resolvable query should instead leverage some sort of null value object.

Closes #2018.

Author: gregturn

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java

@@ -28,6 +28,7 @@
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.query.QueryLookupStrategy;
 import org.springframework.data.repository.query.QueryLookupStrategy.Key;
+import org.springframework.data.repository.query.QueryMethod;
 import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
 import org.springframework.data.repository.query.RepositoryQuery;
 import org.springframework.lang.Nullable;
@@ -47,6 +48,12 @@ public final class JpaQueryLookupStrategy {
 
 	private static final Log LOG = LogFactory.getLog(JpaQueryLookupStrategy.class);
 
+	/**
+	 * A null-value instance used to signal if no declared query could be found. It checks many different formats before
+	 * falling through to this value object.
+	 */
+	private static final RepositoryQuery NO_QUERY = new NoQuery();
+
 	/**
 	 * Private constructor to prevent instantiation.
 	 */
@@ -172,12 +179,9 @@ protected RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewriter quer
 
 			RepositoryQuery query = NamedQuery.lookupFrom(method, em);
 
-			if (null != query) {
-				return query;
-			}
-
-			throw new IllegalStateException(
-					String.format("Did neither find a NamedQuery nor an annotated query for method %s!", method));
+			return query != null //
+					? query //
+					: NO_QUERY;
 		}
 
 		@Nullable
@@ -245,11 +249,13 @@ public CreateIfNotFoundQueryLookupStrategy(EntityManager em, JpaQueryMethodFacto
 		protected RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewriter queryRewriter, EntityManager em,
 				NamedQueries namedQueries) {
 
-			try {
-				return lookupStrategy.resolveQuery(method, queryRewriter, em, namedQueries);
-			} catch (IllegalStateException e) {
-				return createStrategy.resolveQuery(method, queryRewriter, em, namedQueries);
+			RepositoryQuery lookupQuery = lookupStrategy.resolveQuery(method, queryRewriter, em, namedQueries);
+
+			if (lookupQuery != NO_QUERY) {
+				return lookupQuery;
 			}
+
+			return createStrategy.resolveQuery(method, queryRewriter, em, namedQueries);
 		}
 	}
 
@@ -284,4 +290,20 @@ public static QueryLookupStrategy create(EntityManager em, JpaQueryMethodFactory
 				throw new IllegalArgumentException(String.format("Unsupported query lookup strategy %s!", key));
 		}
 	}
+
+	/**
+	 * A null value type that represents the lack of a defined query.
+	 */
+	static class NoQuery implements RepositoryQuery {
+
+		@Override
+		public Object execute(Object[] parameters) {
+			throw new IllegalStateException("NoQuery should not be executed!");
+		}
+
+		@Override
+		public QueryMethod getQueryMethod() {
+			throw new IllegalStateException("NoQuery does not have a QueryMethod!");
+		}
+	}
 }

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/User.java

@@ -15,13 +15,13 @@
  */
 package org.springframework.data.jpa.domain.sample;
 
+import jakarta.persistence.*;
+
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.Set;
 
-import jakarta.persistence.*;
-
 /**
  * Domain class representing a person emphasizing the use of {@code AbstractEntity}. No declaration of an id is
  * required. The id is typed by the parameterizable superclass.
@@ -32,6 +32,7 @@
  * @author Jens Schauder
  * @author Jeff Sheets
  * @author JyotirmoyVS
+ * @author Greg Turnquist
  */
 @Entity
 @NamedEntityGraphs({ @NamedEntityGraph(name = "User.overview", attributeNodes = { @NamedAttributeNode("roles") }),
@@ -91,7 +92,8 @@
 @Table(name = "SD_User")
 public class User {
 
-	@Id @GeneratedValue(strategy = GenerationType.AUTO) private Integer id;
+	@Id
+	@GeneratedValue(strategy = GenerationType.AUTO) private Integer id;
 	private String firstname;
 	private String lastname;
 	private int age;

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java

@@ -169,6 +169,30 @@ void prefersDeclaredQuery() throws Exception {
 		assertThat(repositoryQuery).isInstanceOf(AbstractStringBasedJpaQuery.class);
 	}
 
+	@Test // GH-2018
+	void namedQueryWithSortShouldThrowIllegalStateException() throws NoSuchMethodException {
+
+		QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, queryMethodFactory, Key.CREATE_IF_NOT_FOUND,
+				EVALUATION_CONTEXT_PROVIDER, new BeanFactoryQueryRewriterProvider(beanFactory), EscapeCharacter.DEFAULT);
+
+		Method method = UserRepository.class.getMethod("customNamedQuery", String.class, Sort.class);
+		RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class);
+
+		assertThatIllegalStateException()
+				.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 instead!");
+	}
+
+	@Test // GH-2018
+	void noQueryShouldNotBeInvoked() {
+
+		RepositoryQuery query = new JpaQueryLookupStrategy.NoQuery();
+
+		assertThatIllegalStateException().isThrownBy(() -> query.execute(new Object[] {}));
+		assertThatIllegalStateException().isThrownBy(() -> query.getQueryMethod());
+	}
+
 	interface UserRepository extends Repository<User, Integer> {
 
 		@Query("something absurd")
@@ -185,5 +209,8 @@ interface UserRepository extends Repository<User, Integer> {
 
 		@Query(value = "something absurd", name = "my-query-name")
 		User annotatedQueryWithQueryAndQueryName();
+
+		// This is a named query with Sort parameter, which isn't supported
+		List<User> customNamedQuery(String firstname, Sort sort);
 	}
 }