From 033ac1f95a3877376b8abc5a077e98c3eeacbfa6 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 12 Sep 2024 14:44:11 +0200 Subject: [PATCH] Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor JdbcLookupStrategy to not generally require BeanFactory. Reintroduce deprecated setBeanFactory(…) method. See #1872 Original pull request: #1874 --- .../repository/query/AbstractJdbcQuery.java | 9 ++- .../query/StringBasedJdbcQuery.java | 7 ++- .../support/JdbcQueryLookupStrategy.java | 62 +++++++++++-------- .../query/StringBasedJdbcQueryUnitTests.java | 8 +-- 4 files changed, 50 insertions(+), 36 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 3884fc2e71..e66aed271a 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,7 +23,6 @@ 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; @@ -171,8 +170,8 @@ public interface RowMapperFactory { * @param reference must not be {@code null}. * @since 3.4 */ - default RowMapper rowMapperByReference(String reference) { - throw new UnsupportedOperationException("rowMapperByReference is not supported"); + default RowMapper getRowMapper(String reference) { + throw new UnsupportedOperationException("getRowMapper is not supported"); } /** @@ -181,8 +180,8 @@ default RowMapper rowMapperByReference(String reference) { * @param reference must not be {@code null}. * @since 3.4 */ - default ResultSetExtractor resultSetExtractorByReference(String reference) { - throw new UnsupportedOperationException("resultSetExtractorByReference is not supported"); + default ResultSetExtractor getResultSetExtractor(String reference) { + throw new UnsupportedOperationException("getResultSetExtractor 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 cf086b2b81..0876d099a3 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 @@ -352,6 +352,9 @@ private static boolean isUnconfigured(@Nullable Class configuredClass, Class< return configuredClass == null || configuredClass == defaultClass; } + @Deprecated(since = "3.4") + public void setBeanFactory(BeanFactory beanFactory) {} + class CachedRowMapperFactory { private final Lazy> cachedRowMapper; @@ -375,7 +378,7 @@ public CachedRowMapperFactory(Supplier> defaultMapper) { this.cachedRowMapper = Lazy.of(() -> { if (!ObjectUtils.isEmpty(rowMapperRef)) { - return rowMapperFactory.rowMapperByReference(rowMapperRef); + return rowMapperFactory.getRowMapper(rowMapperRef); } if (isUnconfigured(rowMapperClass, RowMapper.class)) { @@ -426,7 +429,7 @@ public CachedResultSetExtractorFactory(Supplier> resultSetExtractor this.resultSetExtractorFactory = rowMapper -> { if (!ObjectUtils.isEmpty(resultSetExtractorRef)) { - return rowMapperFactory.resultSetExtractorByReference(resultSetExtractorRef); + return rowMapperFactory.getResultSetExtractor(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 629e777e1f..d265b1becd 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 @@ -73,13 +73,12 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { private final JdbcConverter converter; private final QueryMappingConfiguration queryMappingConfiguration; private final NamedParameterJdbcOperations operations; - @Nullable private final BeanFactory beanfactory; protected final QueryMethodEvaluationContextProvider evaluationContextProvider; JdbcQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - @Nullable BeanFactory beanfactory, QueryMethodEvaluationContextProvider evaluationContextProvider) { + QueryMethodEvaluationContextProvider evaluationContextProvider) { super(context, dialect); @@ -87,7 +86,7 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { Assert.notNull(converter, "JdbcConverter must not be null"); Assert.notNull(queryMappingConfiguration, "QueryMappingConfiguration must not be null"); Assert.notNull(operations, "NamedParameterJdbcOperations must not be null"); - Assert.notNull(evaluationContextProvider, "QueryMethodEvaluationContextProvier must not be null"); + Assert.notNull(evaluationContextProvider, "QueryMethodEvaluationContextProvider must not be null"); this.context = context; this.publisher = publisher; @@ -95,7 +94,6 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { this.converter = converter; this.queryMappingConfiguration = queryMappingConfiguration; this.operations = operations; - this.beanfactory = beanfactory; this.evaluationContextProvider = evaluationContextProvider; } @@ -114,9 +112,9 @@ static class CreateQueryLookupStrategy extends JdbcQueryLookupStrategy { CreateQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - @Nullable BeanFactory beanfactory, QueryMethodEvaluationContextProvider evaluationContextProvider) { + QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, beanfactory, + super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, evaluationContextProvider); } @@ -140,12 +138,16 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository */ static class DeclaredQueryLookupStrategy extends JdbcQueryLookupStrategy { + private final AbstractJdbcQuery.RowMapperFactory rowMapperFactory; + DeclaredQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, @Nullable BeanFactory beanfactory, QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, beanfactory, + super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, evaluationContextProvider); + + this.rowMapperFactory = new BeanFactoryRowMapperFactory(beanfactory); } @Override @@ -163,36 +165,51 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository String queryString = evaluateTableExpressions(repositoryMetadata, queryMethod.getRequiredQuery()); - return new StringBasedJdbcQuery(queryString, queryMethod, getOperations(), - new BeanFactoryRowMapperFactory(getBeanFactory()), getConverter(), evaluationContextProvider); + return new StringBasedJdbcQuery(queryString, queryMethod, getOperations(), rowMapperFactory, getConverter(), + evaluationContextProvider); } throw new IllegalStateException( String.format("Did neither find a NamedQuery nor an annotated query for method %s", method)); } + @SuppressWarnings("unchecked") private class BeanFactoryRowMapperFactory implements AbstractJdbcQuery.RowMapperFactory { - private final BeanFactory beanFactory; + private final @Nullable BeanFactory beanFactory; - BeanFactoryRowMapperFactory(BeanFactory beanFactory) { + BeanFactoryRowMapperFactory(@Nullable BeanFactory beanFactory) { this.beanFactory = beanFactory; } + @Override public RowMapper create(Class result) { return createMapper(result); } @Override - public RowMapper rowMapperByReference(String reference) { + public RowMapper getRowMapper(String reference) { + + if (beanFactory == null) { + throw new IllegalStateException( + "Cannot resolve RowMapper bean reference '" + reference + "'; BeanFactory is not configured."); + } + return beanFactory.getBean(reference, RowMapper.class); } @Override - public ResultSetExtractor resultSetExtractorByReference(String reference) { + public ResultSetExtractor getResultSetExtractor(String reference) { + + if (beanFactory == null) { + throw new IllegalStateException( + "Cannot resolve ResultSetExtractor bean reference '" + reference + "'; BeanFactory is not configured."); + } + return beanFactory.getBean(reference, ResultSetExtractor.class); } } + } /** @@ -217,10 +234,10 @@ static class CreateIfNotFoundQueryLookupStrategy extends JdbcQueryLookupStrategy CreateIfNotFoundQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - @Nullable BeanFactory beanfactory, CreateQueryLookupStrategy createStrategy, + CreateQueryLookupStrategy createStrategy, DeclaredQueryLookupStrategy lookupStrategy, QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, beanfactory, + super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, evaluationContextProvider); Assert.notNull(createStrategy, "CreateQueryLookupStrategy must not be null"); @@ -277,23 +294,23 @@ public static QueryLookupStrategy create(@Nullable Key key, ApplicationEventPubl Assert.notNull(operations, "NamedParameterJdbcOperations must not be null"); CreateQueryLookupStrategy createQueryLookupStrategy = new CreateQueryLookupStrategy(publisher, callbacks, context, - converter, dialect, queryMappingConfiguration, operations, beanFactory, evaluationContextProvider); + converter, dialect, queryMappingConfiguration, operations, evaluationContextProvider); DeclaredQueryLookupStrategy declaredQueryLookupStrategy = new DeclaredQueryLookupStrategy(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, beanFactory, evaluationContextProvider); - Key cleanedKey = key != null ? key : Key.CREATE_IF_NOT_FOUND; + Key keyToUse = key != null ? key : Key.CREATE_IF_NOT_FOUND; - LOG.debug(String.format("Using the queryLookupStrategy %s", cleanedKey)); + LOG.debug(String.format("Using the queryLookupStrategy %s", keyToUse)); - switch (cleanedKey) { + switch (keyToUse) { case CREATE: return createQueryLookupStrategy; case USE_DECLARED_QUERY: return declaredQueryLookupStrategy; case CREATE_IF_NOT_FOUND: return new CreateIfNotFoundQueryLookupStrategy(publisher, callbacks, context, converter, dialect, - queryMappingConfiguration, operations, beanFactory, createQueryLookupStrategy, declaredQueryLookupStrategy, + queryMappingConfiguration, operations, createQueryLookupStrategy, declaredQueryLookupStrategy, evaluationContextProvider); default: throw new IllegalArgumentException(String.format("Unsupported query lookup strategy %s", key)); @@ -308,11 +325,6 @@ NamedParameterJdbcOperations getOperations() { return operations; } - @Nullable - BeanFactory getBeanFactory() { - return beanfactory; - } - @SuppressWarnings("unchecked") RowMapper createMapper(Class returnedObjectType) { 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 2a21215096..d9d0a0cc3e 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 @@ -645,21 +645,21 @@ public RowMapper create(Class result) { } @Override - public RowMapper rowMapperByReference(String reference) { + public RowMapper getRowMapper(String reference) { if (preparedReference.equals(reference)) { return (RowMapper) value; } - return AbstractJdbcQuery.RowMapperFactory.super.rowMapperByReference(reference); + return AbstractJdbcQuery.RowMapperFactory.super.getRowMapper(reference); } @Override - public ResultSetExtractor resultSetExtractorByReference(String reference) { + public ResultSetExtractor getResultSetExtractor(String reference) { if (preparedReference.equals(reference)) { return (ResultSetExtractor) value; } - return AbstractJdbcQuery.RowMapperFactory.super.resultSetExtractorByReference(reference); + return AbstractJdbcQuery.RowMapperFactory.super.getResultSetExtractor(reference); } } }