From db28730626aeb947b365d59d2ab517f650d3dc8b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 30 May 2023 14:57:08 +0200 Subject: [PATCH] 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 --- .../repository/query/AbstractJpaQuery.java | 2 +- .../repository/query/JpaQueryExecution.java | 54 ++++++------------- .../support/JpaRepositoryTests.java | 1 - 3 files changed, 18 insertions(+), 39 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 7e70d350f..32c5f438b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -92,7 +92,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { 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()) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java index 4579e67c5..0e96f201a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java @@ -66,7 +66,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; } @@ -167,7 +167,7 @@ public abstract class JpaQueryExecution { @Override @SuppressWarnings("unchecked") - protected Object doExecute(final AbstractJpaQuery repositoryQuery, JpaParametersParameterAccessor accessor) { + protected Object doExecute(AbstractJpaQuery repositoryQuery, JpaParametersParameterAccessor accessor) { Query query = repositoryQuery.createQuery(accessor); @@ -294,20 +294,25 @@ public abstract class JpaQueryExecution { */ 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) { @@ -315,20 +320,15 @@ public abstract class JpaQueryExecution { 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) {} } } @@ -345,10 +345,10 @@ public abstract class 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); @@ -369,24 +369,4 @@ public abstract class JpaQueryExecution { } } - /** - * 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) {} - } - } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java index 6d46876e6..b3e9f7e26 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java @@ -59,7 +59,6 @@ class JpaRepositoryTests { @BeforeEach void setUp() { - repository = new JpaRepositoryFactory(em).getRepository(SampleEntityRepository.class); idClassRepository = new JpaRepositoryFactory(em).getRepository(SampleWithIdClassRepository.class); }