From 5513bbefa830aedc9cf602db4251877b5ff804c3 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 6 Sep 2024 10:35:28 +0200 Subject: [PATCH] StringBasedJdbcQuery no longer requires BeanFactory. The lookup is now performed by the `RowMapperFactory`. Closes #1872 --- .../repository/query/AbstractJdbcQuery.java | 28 +++++++++ .../query/StringBasedJdbcQuery.java | 15 +---- .../support/JdbcQueryLookupStrategy.java | 31 ++++++++-- .../query/StringBasedJdbcQueryUnitTests.java | 62 +++++++++++++------ 4 files changed, 101 insertions(+), 35 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java index a7a0ebb8d6..3884fc2e71 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java @@ -23,6 +23,7 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.dao.EmptyResultDataAccessException; +import org.springframework.data.jdbc.core.convert.JdbcArrayColumns; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; @@ -155,7 +156,34 @@ private JdbcQueryExecution createSingleReadingQueryExecution(ResultSetExt * @since 2.3 */ public interface RowMapperFactory { + + /** + * Create a {@link RowMapper} based on the expected return type passed in as an argument. + * + * @param result must not be {@code null}. + * @return a {@code RowMapper} producing instances of {@code result}. + */ RowMapper create(Class result); + + /** + * Obtain a {@code RowMapper} from some other source, typically a {@link org.springframework.beans.factory.BeanFactory}. + * + * @param reference must not be {@code null}. + * @since 3.4 + */ + default RowMapper rowMapperByReference(String reference) { + throw new UnsupportedOperationException("rowMapperByReference is not supported"); + } + + /** + * Obtain a {@code ResultSetExtractor} from some other source, typically a {@link org.springframework.beans.factory.BeanFactory}. + * + * @param reference must not be {@code null}. + * @since 3.4 + */ + default ResultSetExtractor resultSetExtractorByReference(String reference) { + throw new UnsupportedOperationException("resultSetExtractorByReference is not supported"); + } } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java index 7e8da647ee..cf086b2b81 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java @@ -77,7 +77,6 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery { private final SpelEvaluator spelEvaluator; private final boolean containsSpelExpressions; private final String query; - private BeanFactory beanFactory; private final CachedRowMapperFactory cachedRowMapperFactory; private final CachedResultSetExtractorFactory cachedResultSetExtractorFactory; @@ -353,10 +352,6 @@ private static boolean isUnconfigured(@Nullable Class configuredClass, Class< return configuredClass == null || configuredClass == defaultClass; } - public void setBeanFactory(BeanFactory beanFactory) { - this.beanFactory = beanFactory; - } - class CachedRowMapperFactory { private final Lazy> cachedRowMapper; @@ -380,10 +375,7 @@ public CachedRowMapperFactory(Supplier> defaultMapper) { this.cachedRowMapper = Lazy.of(() -> { if (!ObjectUtils.isEmpty(rowMapperRef)) { - - Assert.notNull(beanFactory, "When a RowMapperRef is specified the BeanFactory must not be null"); - - return (RowMapper) beanFactory.getBean(rowMapperRef); + return rowMapperFactory.rowMapperByReference(rowMapperRef); } if (isUnconfigured(rowMapperClass, RowMapper.class)) { @@ -434,10 +426,7 @@ public CachedResultSetExtractorFactory(Supplier> resultSetExtractor this.resultSetExtractorFactory = rowMapper -> { if (!ObjectUtils.isEmpty(resultSetExtractorRef)) { - - Assert.notNull(beanFactory, "When a ResultSetExtractorRef is specified the BeanFactory must not be null"); - - return (ResultSetExtractor) beanFactory.getBean(resultSetExtractorRef); + return rowMapperFactory.resultSetExtractorByReference(resultSetExtractorRef); } if (isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class)) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java index b1f3f5bd9f..629e777e1f 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java @@ -26,6 +26,7 @@ import org.springframework.data.jdbc.core.convert.EntityRowMapper; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.repository.QueryMappingConfiguration; +import org.springframework.data.jdbc.repository.query.AbstractJdbcQuery; import org.springframework.data.jdbc.repository.query.JdbcQueryMethod; import org.springframework.data.jdbc.repository.query.PartTreeJdbcQuery; import org.springframework.data.jdbc.repository.query.StringBasedJdbcQuery; @@ -42,6 +43,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.SingleColumnRowMapper; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -161,15 +163,36 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository String queryString = evaluateTableExpressions(repositoryMetadata, queryMethod.getRequiredQuery()); - StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryString, queryMethod, getOperations(), - this::createMapper, getConverter(), evaluationContextProvider); - query.setBeanFactory(getBeanFactory()); - return query; + return new StringBasedJdbcQuery(queryString, queryMethod, getOperations(), + new BeanFactoryRowMapperFactory(getBeanFactory()), getConverter(), evaluationContextProvider); } throw new IllegalStateException( String.format("Did neither find a NamedQuery nor an annotated query for method %s", method)); } + + private class BeanFactoryRowMapperFactory implements AbstractJdbcQuery.RowMapperFactory { + + private final BeanFactory beanFactory; + + BeanFactoryRowMapperFactory(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + @Override + public RowMapper create(Class result) { + return createMapper(result); + } + + @Override + public RowMapper rowMapperByReference(String reference) { + return beanFactory.getBean(reference, RowMapper.class); + } + + @Override + public ResultSetExtractor resultSetExtractorByReference(String reference) { + return beanFactory.getBean(reference, ResultSetExtractor.class); + } + } } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java index d5fabc8f7c..2a21215096 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java @@ -33,7 +33,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; -import org.springframework.beans.factory.BeanFactory; import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; import org.springframework.data.convert.ReadingConverter; @@ -163,14 +162,9 @@ void cachesCustomMapperAndExtractorInstances() { @Test // GH-1721 void obtainsCustomRowMapperRef() { - BeanFactory beanFactory = mock(BeanFactory.class); - JdbcQueryMethod queryMethod = createMethod("findAllCustomRowMapperRef"); - StringBasedJdbcQuery query = createQuery(queryMethod); - query.setBeanFactory(beanFactory); - CustomRowMapper customRowMapper = new CustomRowMapper(); - - when(beanFactory.getBean("CustomRowMapper")).thenReturn(customRowMapper); + JdbcQueryMethod queryMethod = createMethod("findAllCustomRowMapperRef"); + StringBasedJdbcQuery query = createQuery(queryMethod, "CustomRowMapper", customRowMapper); RowMapper rowMapper = query.determineRowMapper(queryMethod.getResultProcessor(), false); ResultSetExtractor resultSetExtractor = query.determineResultSetExtractor(() -> { @@ -184,14 +178,9 @@ void obtainsCustomRowMapperRef() { @Test // GH-1721 void obtainsCustomResultSetExtractorRef() { - BeanFactory beanFactory = mock(BeanFactory.class); - JdbcQueryMethod queryMethod = createMethod("findAllCustomResultSetExtractorRef"); - StringBasedJdbcQuery query = createQuery(queryMethod); - query.setBeanFactory(beanFactory); - CustomResultSetExtractor cre = new CustomResultSetExtractor(); - - when(beanFactory.getBean("CustomResultSetExtractor")).thenReturn(cre); + JdbcQueryMethod queryMethod = createMethod("findAllCustomResultSetExtractorRef"); + StringBasedJdbcQuery query = createQuery(queryMethod, "CustomResultSetExtractor", cre); RowMapper rowMapper = query.determineRowMapper(queryMethod.getResultProcessor(), false); ResultSetExtractor resultSetExtractor = query.determineResultSetExtractor(() -> { @@ -332,10 +321,10 @@ void queryByListOfTuples() { String[][] tuples = { new String[] { "Albert", "Einstein" }, new String[] { "Richard", "Feynman" } }; SqlParameterSource parameterSource = forMethod("findByListOfTuples", List.class) // - .withArguments(Arrays.asList(tuples)) // + .withArguments(Arrays.asList(tuples))// .extractParameterSource(); - assertThat(parameterSource.getValue("tuples")).asInstanceOf(LIST) // + assertThat(parameterSource.getValue("tuples")).asInstanceOf(LIST)// .containsExactly(tuples); assertThat(parameterSource.getSqlType("tuples")).isEqualTo(JdbcUtil.TYPE_UNKNOWN.getVendorTypeNumber()); @@ -441,7 +430,11 @@ private JdbcQueryMethod createMethod(String methodName, Class... paramTypes) } private StringBasedJdbcQuery createQuery(JdbcQueryMethod queryMethod) { - return new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, evaluationContextProvider); + return createQuery(queryMethod, null, null); + } + + private StringBasedJdbcQuery createQuery(JdbcQueryMethod queryMethod, String preparedReference, Object value) { + return new StringBasedJdbcQuery(queryMethod, operations, new StubRowMapperFactory(preparedReference, value), converter, evaluationContextProvider); } interface MyRepository extends Repository { @@ -636,4 +629,37 @@ public Object getRootObject() { } } + private class StubRowMapperFactory implements AbstractJdbcQuery.RowMapperFactory { + + private final String preparedReference; + private final Object value; + + StubRowMapperFactory(String preparedReference, Object value) { + this.preparedReference = preparedReference; + this.value = value; + } + + @Override + public RowMapper create(Class result) { + return defaultRowMapper; + } + + @Override + public RowMapper rowMapperByReference(String reference) { + + if (preparedReference.equals(reference)) { + return (RowMapper) value; + } + return AbstractJdbcQuery.RowMapperFactory.super.rowMapperByReference(reference); + } + + @Override + public ResultSetExtractor resultSetExtractorByReference(String reference) { + + if (preparedReference.equals(reference)) { + return (ResultSetExtractor) value; + } + return AbstractJdbcQuery.RowMapperFactory.super.resultSetExtractorByReference(reference); + } + } }