StringBasedJdbcQuery no longer requires BeanFactory

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-relational-parent</artifactId>
-	<version>3.4.0-SNAPSHOT</version>
+	<version>3.4.0-1872-refactor-SBJQ-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data Relational Parent</name>

spring-data-jdbc-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>3.4.0-SNAPSHOT</version>
+		<version>3.4.0-1872-refactor-SBJQ-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-jdbc/pom.xml

@@ -6,7 +6,7 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-jdbc</artifactId>
-	<version>3.4.0-SNAPSHOT</version>
+	<version>3.4.0-1872-refactor-SBJQ-SNAPSHOT</version>
 
 	<name>Spring Data JDBC</name>
 	<description>Spring Data module for JDBC repositories.</description>
@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>3.4.0-SNAPSHOT</version>
+		<version>3.4.0-1872-refactor-SBJQ-SNAPSHOT</version>
 	</parent>
 
 	<properties>

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java

@@ -76,7 +76,7 @@ public JdbcQueryMethod getQueryMethod() {
 	 * Creates a {@link JdbcQueryExecution} given a {@link ResultSetExtractor} or a {@link RowMapper}. Prefers the given
 	 * {@link ResultSetExtractor} over {@link RowMapper}.
 	 *
-	 * @param extractor must not be {@literal null}.
+	 * @param extractor may be {@literal null}.
 	 * @param rowMapper must not be {@literal null}.
 	 * @return a JdbcQueryExecution appropriate for {@literal queryMethod}. Guaranteed to be not {@literal null}.
 	 */
@@ -155,7 +155,34 @@ private <T> JdbcQueryExecution<T> 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<Object> 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<Object> getRowMapper(String reference) {
+			throw new UnsupportedOperationException("getRowMapper 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<Object> getResultSetExtractor(String reference) {
+			throw new UnsupportedOperationException("getResultSetExtractor is not supported");
+		}
 	}
 
 	/**

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,9 +352,8 @@ private static boolean isUnconfigured(@Nullable Class<?> configuredClass, Class<
 		return configuredClass == null || configuredClass == defaultClass;
 	}
 
-	public void setBeanFactory(BeanFactory beanFactory) {
-		this.beanFactory = beanFactory;
-	}
+	@Deprecated(since = "3.4")
+	public void setBeanFactory(BeanFactory beanFactory) {}
 
 	class CachedRowMapperFactory {
 
@@ -380,10 +378,7 @@ public CachedRowMapperFactory(Supplier<RowMapper<Object>> 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<Object>) beanFactory.getBean(rowMapperRef);
+					return rowMapperFactory.getRowMapper(rowMapperRef);
 				}
 
 				if (isUnconfigured(rowMapperClass, RowMapper.class)) {
@@ -434,10 +429,7 @@ public CachedResultSetExtractorFactory(Supplier<RowMapper<?>> resultSetExtractor
 			this.resultSetExtractorFactory = rowMapper -> {
 
 				if (!ObjectUtils.isEmpty(resultSetExtractorRef)) {
-
-					Assert.notNull(beanFactory, "When a ResultSetExtractorRef is specified the BeanFactory must not be null");
-
-					return (ResultSetExtractor<Object>) beanFactory.getBean(resultSetExtractorRef);
+					return rowMapperFactory.getResultSetExtractor(resultSetExtractorRef);
 				}
 
 				if (isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class)) {

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;
@@ -71,29 +73,27 @@ 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);
 
 		Assert.notNull(publisher, "ApplicationEventPublisher must not be null");
 		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;
 		this.callbacks = callbacks;
 		this.converter = converter;
 		this.queryMappingConfiguration = queryMappingConfiguration;
 		this.operations = operations;
-		this.beanfactory = beanfactory;
 		this.evaluationContextProvider = evaluationContextProvider;
 	}
 
@@ -112,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);
 		}
 
@@ -138,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
@@ -161,15 +165,51 @@ 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(), 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 @Nullable BeanFactory beanFactory;
+
+			BeanFactoryRowMapperFactory(@Nullable BeanFactory beanFactory) {
+				this.beanFactory = beanFactory;
+			}
+
+			@Override
+			public RowMapper<Object> create(Class<?> result) {
+				return createMapper(result);
+			}
+
+			@Override
+			public RowMapper<Object> 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<Object> 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);
+			}
+		}
+
 	}
 
 	/**
@@ -194,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");
@@ -254,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));
@@ -285,11 +325,6 @@ NamedParameterJdbcOperations getOperations() {
 		return operations;
 	}
 
-	@Nullable
-	BeanFactory getBeanFactory() {
-		return beanfactory;
-	}
-
 	@SuppressWarnings("unchecked")
 	RowMapper<Object> createMapper(Class<?> returnedObjectType) {