Polishing.

Simplify code flow. Introduce flag to capture whether a stored procedure uses collection return types.

Remove unconditionally the Optional converter as we're already on Java 17 and do not require the Java 8 guard.

See #2915
Original pull request: #2938

Author: mp911de

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

@@ -92,7 +92,7 @@ public AbstractJpaQuery(JpaQueryMethod method, EntityManager em) {
 			if (method.isStreamQuery()) {
 				return new StreamExecution();
 			} else if (method.isProcedureQuery()) {
-				return new ProcedureExecution();
+				return new ProcedureExecution(method.isCollectionQuery());
 			} else if (method.isCollectionQuery()) {
 				return new CollectionExecution();
 			} else if (method.isSliceQuery()) {

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

@@ -68,7 +68,7 @@ public abstract class JpaQueryExecution {
 
 		conversionService.addConverter(JpaResultConverters.BlobToByteArrayConverter.INSTANCE);
 		conversionService.removeConvertible(Collection.class, Object.class);
-		potentiallyRemoveOptionalConverter(conversionService);
+		conversionService.removeConvertible(Object.class, Optional.class);
 
 		CONVERSION_SERVICE = conversionService;
 	}
@@ -197,7 +197,7 @@ static class PagedExecution extends JpaQueryExecution {
 
 		@Override
 		@SuppressWarnings("unchecked")
-		protected Object doExecute(final AbstractJpaQuery repositoryQuery, JpaParametersParameterAccessor accessor) {
+		protected Object doExecute(AbstractJpaQuery repositoryQuery, JpaParametersParameterAccessor accessor) {
 
 			Query query = repositoryQuery.createQuery(accessor);
 
@@ -324,41 +324,41 @@ protected Object doExecute(AbstractJpaQuery query, JpaParametersParameterAccesso
 	 */
 	static class ProcedureExecution extends JpaQueryExecution {
 
+		private final boolean collectionQuery;
+
 		private static final String NO_SURROUNDING_TRANSACTION = "You're trying to execute a @Procedure method without a surrounding transaction that keeps the connection open so that the ResultSet can actually be consumed; Make sure the consumer code uses @Transactional or any other way of declaring a (read-only) transaction";
 
+		ProcedureExecution(boolean collectionQuery) {
+			this.collectionQuery = collectionQuery;
+		}
+
 		@Override
 		protected Object doExecute(AbstractJpaQuery jpaQuery, JpaParametersParameterAccessor accessor) {
 
 			Assert.isInstanceOf(StoredProcedureJpaQuery.class, jpaQuery);
 
-			StoredProcedureJpaQuery storedProcedureJpaQuery = (StoredProcedureJpaQuery) jpaQuery;
-
-			StoredProcedureQuery storedProcedure = storedProcedureJpaQuery.createQuery(accessor);
+			StoredProcedureJpaQuery query = (StoredProcedureJpaQuery) jpaQuery;
+			StoredProcedureQuery procedure = query.createQuery(accessor);
 
 			try {
 
-				boolean returnsResultSet = storedProcedure.execute();
+				boolean returnsResultSet = procedure.execute();
 
 				if (returnsResultSet) {
 
 					if (!SurroundingTransactionDetectorMethodInterceptor.INSTANCE.isSurroundingTransactionActive()) {
 						throw new InvalidDataAccessApiUsageException(NO_SURROUNDING_TRANSACTION);
 					}
 
-					if (storedProcedureJpaQuery.getQueryMethod().isCollectionQuery()) {
-						return storedProcedure.getResultList();
-					} else {
-						return storedProcedure.getSingleResult();
-					}
+					return collectionQuery ? procedure.getResultList() : procedure.getSingleResult();
 				}
 
-				return storedProcedureJpaQuery.extractOutputValue(storedProcedure);
-
+				return query.extractOutputValue(procedure);
 			} finally {
 
-				if (storedProcedure instanceof AutoCloseable autoCloseable) {
+				if (procedure instanceof AutoCloseable ac) {
 					try {
-						autoCloseable.close();
+						ac.close();
 					} catch (Exception ignored) {}
 				}
 			}
@@ -375,10 +375,10 @@ static class StreamExecution extends JpaQueryExecution {
 
 		private static final String NO_SURROUNDING_TRANSACTION = "You're trying to execute a streaming query method without a surrounding transaction that keeps the connection open so that the Stream can actually be consumed; Make sure the code consuming the stream uses @Transactional or any other way of declaring a (read-only) transaction";
 
-		private static Method streamMethod = ReflectionUtils.findMethod(Query.class, "getResultStream");
+		private static final Method streamMethod = ReflectionUtils.findMethod(Query.class, "getResultStream");
 
 		@Override
-		protected Object doExecute(final AbstractJpaQuery query, JpaParametersParameterAccessor accessor) {
+		protected Object doExecute(AbstractJpaQuery query, JpaParametersParameterAccessor accessor) {
 
 			if (!SurroundingTransactionDetectorMethodInterceptor.INSTANCE.isSurroundingTransactionActive()) {
 				throw new InvalidDataAccessApiUsageException(NO_SURROUNDING_TRANSACTION);
@@ -399,24 +399,4 @@ protected Object doExecute(final AbstractJpaQuery query, JpaParametersParameterA
 		}
 	}
 
-	/**
-	 * Removes the converter being able to convert any object into an {@link Optional} from the given
-	 * {@link ConversionService} in case we're running on Java 8.
-	 *
-	 * @param conversionService must not be {@literal null}.
-	 */
-	public static void potentiallyRemoveOptionalConverter(ConfigurableConversionService conversionService) {
-
-		ClassLoader classLoader = JpaQueryExecution.class.getClassLoader();
-
-		if (ClassUtils.isPresent("java.util.Optional", classLoader)) {
-
-			try {
-
-				Class<?> optionalType = ClassUtils.forName("java.util.Optional", classLoader);
-				conversionService.removeConvertible(Object.class, optionalType);
-
-			} catch (ClassNotFoundException | LinkageError o_O) {}
-		}
-	}
 }

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java

@@ -15,7 +15,7 @@
  */
 package org.springframework.data.jpa.repository.support;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.*;
 
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.PersistenceContext;
@@ -59,7 +59,6 @@ class JpaRepositoryTests {
 
 	@BeforeEach
 	void setUp() {
-
 		repository = new JpaRepositoryFactory(em).getRepository(SampleEntityRepository.class);
 		idClassRepository = new JpaRepositoryFactory(em).getRepository(SampleWithIdClassRepository.class);
 	}