From 8717db6618d41cbf9815e290ef2c22fb069176f3 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 | 3 +- 3 files changed, 19 insertions(+), 40 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 428fba44b..43d70ca8a 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 @@ -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 @@ 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); @@ -324,20 +324,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) { @@ -345,20 +350,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) {} } } @@ -375,10 +375,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); @@ -399,24 +399,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 e0f7a2298..586ef0668 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 @@ -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); }