DATAJDBC-290 - Applied review feedback.

If both RowMapper and ResultSetExtractor are configured on a method and the ResultSetExtractor has a public constructor accepting a RowMapper, the two get combined.

Original pull request: #104.

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

@@ -1,7 +1,7 @@
 package org.springframework.data.jdbc.repository;
 
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.jdbc.core.ResultSetExtractor;
+import org.springframework.jdbc.core.RowMapper;
 import org.springframework.lang.Nullable;
 
 /**
@@ -14,7 +14,7 @@ import org.springframework.lang.Nullable;
 public interface QueryMappingConfiguration {
 
 	@Nullable
-	<T> RowMapperOrResultsetExtractor<? extends T> getMapperOrExtractor(Class<T> type);
+	<T> RowMapper<? extends T> getRowMapper(Class<T> type);
 
 	/**
 	 * An immutable empty instance that will return {@literal null} for all arguments.
@@ -22,7 +22,7 @@ public interface QueryMappingConfiguration {
 	QueryMappingConfiguration EMPTY = new QueryMappingConfiguration() {
 
 		@Override
-		public <T> RowMapperOrResultsetExtractor<? extends T> getMapperOrExtractor(Class<T> type) {
+		public <T> RowMapper<? extends T> getRowMapper(Class<T> type) {
 			return null;
 		}
 

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

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.jdbc.repository;
 
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.lang.Nullable;
 
@@ -42,7 +41,7 @@ public interface RowMapperMap extends QueryMappingConfiguration {
 		}
 
 		@Override
-		public <T> RowMapperOrResultsetExtractor<T> getMapperOrExtractor(Class<T> type) {
+		public <T> RowMapper<? extends T> getRowMapper(Class<T> type) {
 			return null;
 		}
 	};
@@ -50,9 +49,8 @@ public interface RowMapperMap extends QueryMappingConfiguration {
 	@Nullable
 	<T> RowMapper<? extends T> rowMapperFor(Class<T> type);
 
-
 	@Override
-	default <T> RowMapperOrResultsetExtractor<? extends T> getMapperOrExtractor(Class<T> type) {
-		return RowMapperOrResultsetExtractor.of(rowMapperFor(type));
+	default <T> RowMapper<? extends T> getRowMapper(Class<T> type) {
+		return rowMapperFor(type);
 	}
 }

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/DefaultQueryMappingConfiguration.java

@@ -4,7 +4,6 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 
 import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.lang.Nullable;
@@ -19,25 +18,25 @@ import org.springframework.util.Assert;
  */
 public class DefaultQueryMappingConfiguration implements QueryMappingConfiguration {
 
-	private Map<Class<?>, RowMapperOrResultsetExtractor<?>> mappers = new LinkedHashMap<>();
+	private Map<Class<?>, RowMapper<?>> mappers = new LinkedHashMap<>();
 
 	@Nullable
-	public <T> RowMapperOrResultsetExtractor<? extends T> getMapperOrExtractor(Class<T> type) {
+	public <T> RowMapper<? extends T> getRowMapper(Class<T> type) {
 
 		Assert.notNull(type, "Type must not be null");
 
-		RowMapperOrResultsetExtractor<?> candidate = mappers.get(type);
+		RowMapper<?> candidate = mappers.get(type);
 
 		if (candidate == null) {
 
-			for (Map.Entry<Class<?>, RowMapperOrResultsetExtractor<?>> entry : mappers.entrySet()) {
+			for (Map.Entry<Class<?>, RowMapper<?>> entry : mappers.entrySet()) {
 
 				if (type.isAssignableFrom(entry.getKey())) {
 					candidate = entry.getValue();
 				}
 			}
 		}
-		return (RowMapperOrResultsetExtractor<? extends T>) candidate;
+		return (RowMapper<? extends T>) candidate;
 	}
 
 	/**
@@ -47,20 +46,7 @@ public class DefaultQueryMappingConfiguration implements QueryMappingConfigurati
 	 */
 	public <T> DefaultQueryMappingConfiguration registerRowMapper(Class<T> type, RowMapper<? extends T> rowMapper) {
 
-		mappers.put(type, RowMapperOrResultsetExtractor.of(rowMapper));
-
-		return this;
-	}
-
-	/**
-	 * Registers a the given {@link ResultSetExtractor} as to be used for the given type.
-	 *
-	 * @return this instance, so this can be used as a fluent interface.
-	 */
-	public <T> DefaultQueryMappingConfiguration registerResultSetExtractor(Class<T> type,
-			ResultSetExtractor resultSetExtractor) {
-
-		mappers.put(type, RowMapperOrResultsetExtractor.of(resultSetExtractor));
+		mappers.put(type, rowMapper);
 
 		return this;
 	}

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java

@@ -21,7 +21,6 @@ import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.data.jdbc.core.DataAccessStrategy;
 import org.springframework.data.jdbc.core.EntityRowMapper;
 import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.relational.core.conversion.RelationalConverter;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
@@ -30,6 +29,7 @@ import org.springframework.data.repository.core.NamedQueries;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.query.QueryLookupStrategy;
 import org.springframework.data.repository.query.RepositoryQuery;
+import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.core.SingleColumnRowMapper;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.util.Assert;
@@ -90,29 +90,28 @@ class JdbcQueryLookupStrategy implements QueryLookupStrategy {
 
 		JdbcQueryMethod queryMethod = new JdbcQueryMethod(method, repositoryMetadata, projectionFactory);
 
-		RowMapperOrResultsetExtractor<?> mapper = queryMethod.isModifyingQuery() ? null : createMapper(queryMethod);
+		RowMapper<?> mapper = queryMethod.isModifyingQuery() ? null : createMapper(queryMethod);
 
 		return new JdbcRepositoryQuery(publisher, context, queryMethod, operations, mapper);
 	}
 
-	private RowMapperOrResultsetExtractor<?> createMapper(JdbcQueryMethod queryMethod) {
+	private RowMapper<?> createMapper(JdbcQueryMethod queryMethod) {
 
 		Class<?> returnedObjectType = queryMethod.getReturnedObjectType();
 
 		RelationalPersistentEntity<?> persistentEntity = context.getPersistentEntity(returnedObjectType);
 
 		if (persistentEntity == null) {
-			return RowMapperOrResultsetExtractor
-					.of(SingleColumnRowMapper.newInstance(returnedObjectType, converter.getConversionService()));
+			return SingleColumnRowMapper.newInstance(returnedObjectType, converter.getConversionService());
 		}
 
 		return determineDefaultMapper(queryMethod);
 	}
 
-	private RowMapperOrResultsetExtractor<?> determineDefaultMapper(JdbcQueryMethod queryMethod) {
+	private RowMapper<?> determineDefaultMapper(JdbcQueryMethod queryMethod) {
 
 		Class<?> domainType = queryMethod.getReturnedObjectType();
-		RowMapperOrResultsetExtractor<?> configuredQueryMapper = queryMappingConfiguration.getMapperOrExtractor(domainType);
+		RowMapper<?> configuredQueryMapper = queryMappingConfiguration.getRowMapper(domainType);
 
 		if (configuredQueryMapper != null)
 			return configuredQueryMapper;
@@ -123,6 +122,6 @@ class JdbcQueryLookupStrategy implements QueryLookupStrategy {
 				converter, //
 				accessStrategy);
 
-		return RowMapperOrResultsetExtractor.of(defaultEntityRowMapper);
+		return defaultEntityRowMapper;
 	}
 }

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryMethod.java

@@ -24,6 +24,8 @@ import org.springframework.data.jdbc.repository.query.Query;
 import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.query.QueryMethod;
+import org.springframework.jdbc.core.ResultSetExtractor;
+import org.springframework.jdbc.core.RowMapper;
 import org.springframework.lang.Nullable;
 
 /**
@@ -32,7 +34,9 @@ import org.springframework.lang.Nullable;
  *
  * @author Jens Schauder
  * @author Kazuki Shimizu
+ * @deprecated Visibility of this class will be reduced to package private.
  */
+@Deprecated
 public class JdbcQueryMethod extends QueryMethod {
 
 	private final Method method;
@@ -50,7 +54,7 @@ public class JdbcQueryMethod extends QueryMethod {
 	 * @return May be {@code null}.
 	 */
 	@Nullable
-	public String getAnnotatedQuery() {
+	String getAnnotatedQuery() {
 		return getMergedAnnotationAttribute("value");
 	}
 
@@ -60,7 +64,7 @@ public class JdbcQueryMethod extends QueryMethod {
 	 * @return May be {@code null}.
 	 */
 	@Nullable
-	public Class<?> getRowMapperClass() {
+	Class<? extends RowMapper> getRowMapperClass() {
 		return getMergedAnnotationAttribute("rowMapperClass");
 	}
 
@@ -70,7 +74,7 @@ public class JdbcQueryMethod extends QueryMethod {
 	 * @return May be {@code null}.
 	 */
 	@Nullable
-	public Class<?> getResultSetExtractorClass() {
+	Class<? extends ResultSetExtractor> getResultSetExtractorClass() {
 		return getMergedAnnotationAttribute("resultSetExtractorClass");
 	}
 

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryQuery.java

@@ -15,12 +15,12 @@
  */
 package org.springframework.data.jdbc.repository.support;
 
+import java.lang.reflect.Constructor;
 import java.util.List;
 
 import org.springframework.beans.BeanUtils;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.dao.EmptyResultDataAccessException;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.mapping.event.AfterLoadEvent;
@@ -32,6 +32,7 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -51,8 +52,7 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 	private final RelationalMappingContext context;
 	private final JdbcQueryMethod queryMethod;
 	private final NamedParameterJdbcOperations operations;
-
-	@Nullable private final RowMapperOrResultsetExtractor<?> mapperOrExtractor;
+	private final QueryExecutor<Object> executor;
 
 	/**
 	 * Creates a new {@link JdbcRepositoryQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
@@ -62,11 +62,10 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 	 * @param context must not be {@literal null}.
 	 * @param queryMethod must not be {@literal null}.
 	 * @param operations must not be {@literal null}.
-	 * @param defaultMapper can be {@literal null} (only in case of a modifying query).
+	 * @param defaultRowMapper can be {@literal null} (only in case of a modifying query).
 	 */
 	JdbcRepositoryQuery(ApplicationEventPublisher publisher, RelationalMappingContext context,
-			JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
-			@Nullable RowMapperOrResultsetExtractor<?> defaultMapper) {
+			JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, RowMapper<?> defaultRowMapper) {
 
 		Assert.notNull(publisher, "Publisher must not be null!");
 		Assert.notNull(context, "Context must not be null!");
@@ -74,14 +73,40 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 		Assert.notNull(operations, "NamedParameterJdbcOperations must not be null!");
 
 		if (!queryMethod.isModifyingQuery()) {
-			Assert.notNull(defaultMapper, "Mapper must not be null!");
+			Assert.notNull(defaultRowMapper, "Mapper must not be null!");
 		}
 
 		this.publisher = publisher;
 		this.context = context;
 		this.queryMethod = queryMethod;
 		this.operations = operations;
-		this.mapperOrExtractor = determineMapper(defaultMapper);
+
+		RowMapper rowMapper = determineRowMapper(defaultRowMapper);
+		executor = createExecutor( //
+				queryMethod, //
+				determineResultSetExtractor(rowMapper != defaultRowMapper ? rowMapper : null), //
+				rowMapper //
+		);
+
+	}
+
+	private QueryExecutor<Object> createExecutor(JdbcQueryMethod queryMethod, @Nullable ResultSetExtractor extractor,
+			RowMapper rowMapper) {
+
+		String query = determineQuery();
+
+		if (queryMethod.isModifyingQuery()) {
+			return createModifyingQueryExecutor(query);
+		}
+		if (queryMethod.isCollectionQuery() || queryMethod.isStreamQuery()) {
+			QueryExecutor<Object> innerExecutor = extractor != null ? createResultSetExtractorQueryExecutor(query, extractor)
+					: createListRowMapperQueryExecutor(query, rowMapper);
+			return createCollectionQueryExecutor(innerExecutor);
+		}
+
+		QueryExecutor<Object> innerExecutor = extractor != null ? createResultSetExtractorQueryExecutor(query, extractor)
+				: createObjectRowMapperQueryExecutor(query, rowMapper);
+		return createObjectQueryExecutor(innerExecutor);
 	}
 
 	/*
@@ -91,45 +116,66 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 	@Override
 	public Object execute(Object[] objects) {
 
-		String query = determineQuery();
-		MapSqlParameterSource parameters = bindParameters(objects);
+		return executor.execute(bindParameters(objects));
+	}
 
-		if (queryMethod.isModifyingQuery()) {
+	private QueryExecutor<Object> createObjectQueryExecutor(QueryExecutor executor) {
 
-			int updatedCount = operations.update(query, parameters);
-			Class<?> returnedObjectType = queryMethod.getReturnedObjectType();
-			return (returnedObjectType == boolean.class || returnedObjectType == Boolean.class) ? updatedCount != 0
-					: updatedCount;
-		}
+		return parameters -> {
 
-		assert this.mapperOrExtractor != null;
+			try {
 
-		if (queryMethod.isCollectionQuery() || queryMethod.isStreamQuery()) {
+				Object result;
+
+				result = executor.execute(parameters);
+
+				publishAfterLoad(result);
+
+				return result;
+
+			} catch (EmptyResultDataAccessException e) {
+				return null;
+			}
+		};
+	}
+
+	private QueryExecutor<Object> createCollectionQueryExecutor(QueryExecutor<Object> executor) {
 
-			List<?> result = this.mapperOrExtractor.isResultSetExtractor()
-					? (List<?>) operations.query(query, parameters, this.mapperOrExtractor.getResultSetExtractor())
-					: operations.query(query, parameters, this.mapperOrExtractor.getRowMapper());
+		return parameters -> {
+
+			List<?> result = (List<?>) executor.execute(parameters);
 
 			Assert.notNull(result, "A collection valued result must never be null.");
 
 			publishAfterLoad(result);
 
 			return result;
-		}
+		};
+	}
 
-		try {
+	private QueryExecutor<Object> createModifyingQueryExecutor(String query) {
 
-			Object result = this.mapperOrExtractor.isResultSetExtractor()
-					? operations.query(query, parameters, this.mapperOrExtractor.getResultSetExtractor())
-					: operations.queryForObject(query, parameters, this.mapperOrExtractor.getRowMapper());
+		return parameters -> {
 
-			publishAfterLoad(result);
+			int updatedCount = operations.update(query, parameters);
+			Class<?> returnedObjectType = queryMethod.getReturnedObjectType();
 
-			return result;
+			return (returnedObjectType == boolean.class || returnedObjectType == Boolean.class) ? updatedCount != 0
+					: updatedCount;
+		};
+	}
 
-		} catch (EmptyResultDataAccessException e) {
-			return null;
-		}
+	private QueryExecutor<Object> createListRowMapperQueryExecutor(String query, RowMapper<?> rowMapper) {
+		return parameters -> operations.query(query, parameters, rowMapper);
+	}
+
+	private QueryExecutor<Object> createObjectRowMapperQueryExecutor(String query, RowMapper<?> rowMapper) {
+		return parameters -> operations.queryForObject(query, parameters, rowMapper);
+	}
+
+	private QueryExecutor<Object> createResultSetExtractorQueryExecutor(String query,
+			ResultSetExtractor<?> resultSetExtractor) {
+		return parameters -> operations.query(query, parameters, resultSetExtractor);
 	}
 
 	/*
@@ -166,50 +212,37 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 	}
 
 	@Nullable
-	private RowMapperOrResultsetExtractor<?> determineMapper(@Nullable RowMapperOrResultsetExtractor<?> defaultMapper) {
-
-		RowMapperOrResultsetExtractor<?> configuredMapper = getConfiguredMapper(queryMethod);
-
-		return (configuredMapper == null) ? defaultMapper : configuredMapper;
-	}
-
-	@Nullable
-	private static RowMapperOrResultsetExtractor<?> getConfiguredMapper(JdbcQueryMethod queryMethod) {
-
-		Class<?> rowMapperClass = queryMethod.getRowMapperClass();
-		Class<?> resultSetExtractorClass = queryMethod.getResultSetExtractorClass();
+	private ResultSetExtractor determineResultSetExtractor(@Nullable RowMapper<?> rowMapper) {
 
-		assertOnlyOneIsConfigured(queryMethod, rowMapperClass, resultSetExtractorClass);
+		Class<? extends ResultSetExtractor> resultSetExtractorClass = (Class<? extends ResultSetExtractor>) queryMethod.getResultSetExtractorClass();
 
-		if (isConfigured(rowMapperClass, RowMapper.class)) {
-			return RowMapperOrResultsetExtractor.of((RowMapper<?>) BeanUtils.instantiateClass(rowMapperClass));
+		if (isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class)) {
+			return null;
 		}
 
-		if (isConfigured(resultSetExtractorClass, ResultSetExtractor.class)) {
-			return RowMapperOrResultsetExtractor
-					.of((ResultSetExtractor<?>) BeanUtils.instantiateClass(resultSetExtractorClass));
+		Constructor<? extends ResultSetExtractor> constructor = ClassUtils
+				.getConstructorIfAvailable(resultSetExtractorClass, RowMapper.class);
+
+		if (constructor != null) {
+			return BeanUtils.instantiateClass(constructor, rowMapper);
 		}
 
-		return null;
+		return BeanUtils.instantiateClass(resultSetExtractorClass);
 	}
 
-	private static void assertOnlyOneIsConfigured(JdbcQueryMethod queryMethod, @Nullable Class<?> rowMapperClass,
-			@Nullable Class<?> resultSetExtractorClass) {
+	private RowMapper determineRowMapper(RowMapper<?> defaultMapper) {
 
-		if (isConfigured(rowMapperClass, RowMapper.class)
-				&& isConfigured(resultSetExtractorClass, ResultSetExtractor.class)) {
+		Class<?> rowMapperClass = queryMethod.getRowMapperClass();
 
-			throw new IllegalStateException( //
-					String.format( //
-							"Cannot use both rowMapperClass and resultSetExtractorClass on @Query annotation. Query // method: [%s] query: [%s]",
-							queryMethod.getName(), //
-							queryMethod.getAnnotatedQuery() //
-					));
+		if (isUnconfigured(rowMapperClass, RowMapper.class)) {
+			return defaultMapper;
 		}
+
+		return (RowMapper) BeanUtils.instantiateClass(rowMapperClass);
 	}
 
-	private static boolean isConfigured(@Nullable Class<?> configuredClass, Class<?> defaultClass) {
-		return configuredClass != null && configuredClass != defaultClass;
+	private static boolean isUnconfigured(@Nullable Class<?> configuredClass, Class<?> defaultClass) {
+		return configuredClass == null || configuredClass == defaultClass;
 	}
 
 	private <T> void publishAfterLoad(Iterable<T> all) {
@@ -232,4 +265,9 @@ class JdbcRepositoryQuery implements RepositoryQuery {
 		}
 
 	}
+
+	private interface QueryExecutor<T> {
+		@Nullable
+		T execute(MapSqlParameterSource parameter);
+	}
 }

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/support/RowMapperOrResultsetExtractor.java

@@ -1,42 +0,0 @@
-package org.springframework.data.jdbc.support;
-
-import lombok.Value;
-
-import org.springframework.jdbc.core.ResultSetExtractor;
-import org.springframework.jdbc.core.RowMapper;
-
-/**
- * Represents either a RowMapper or a ResultSetExtractor
- *
- * @author Evgeni Dimitrov
- * @author Jens Schauder
- */
-@Value
-public class RowMapperOrResultsetExtractor<T> {
-
-	private final RowMapper<T> rowMapper;
-	private final ResultSetExtractor<T> resultSetExtractor;
-
-	private RowMapperOrResultsetExtractor(RowMapper<T> rowMapper, ResultSetExtractor<T> resultSetExtractor) {
-
-		this.rowMapper = rowMapper;
-		this.resultSetExtractor = resultSetExtractor;
-	}
-
-	public static <T> RowMapperOrResultsetExtractor<T> of(RowMapper<T> rowMapper) {
-		return new RowMapperOrResultsetExtractor<>(rowMapper, null);
-	}
-
-	public static <T> RowMapperOrResultsetExtractor<T> of(ResultSetExtractor<T> resultSetExtractor) {
-		return new RowMapperOrResultsetExtractor<>(null, resultSetExtractor);
-	}
-
-	public boolean isRowMapper() {
-		return this.rowMapper != null;
-	}
-
-	public boolean isResultSetExtractor() {
-		return this.resultSetExtractor != null;
-	}
-
-}

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryQueryMappingConfigurationIntegrationTests.java

@@ -53,7 +53,7 @@ import lombok.Data;
  */
 @ContextConfiguration
 @Transactional
-public class JdbcRepositoryMapperMapResultSetExtractorIntegrationTests {
+public class JdbcRepositoryQueryMappingConfigurationIntegrationTests {
 
 	private static String CAR_MODEL = "ResultSetExtractor Car";
 
@@ -64,13 +64,12 @@ public class JdbcRepositoryMapperMapResultSetExtractorIntegrationTests {
 
 		@Bean
 		Class<?> testClass() {
-			return JdbcRepositoryMapperMapResultSetExtractorIntegrationTests.class;
+			return JdbcRepositoryQueryMappingConfigurationIntegrationTests.class;
 		}
 		
 		@Bean
 		QueryMappingConfiguration mappers() {
-			return new DefaultQueryMappingConfiguration()
-					.registerResultSetExtractor(Car.class, new CarResultSetExtractor());
+			return new DefaultQueryMappingConfiguration();
 		}
 	}
 
@@ -81,7 +80,7 @@ public class JdbcRepositoryMapperMapResultSetExtractorIntegrationTests {
 	@Autowired CarRepository carRepository;
 	
 	@Test // DATAJDBC-290
-	public void customFindAllCarsPicksResultSetExtractorFromMapperMap() {
+	public void customFindAllCarsUsesConfiguredResultSetExtractor() {
 
 		carRepository.save(new Car(null, "Some model"));
 		Iterable<Car> cars = carRepository.customFindAll();
@@ -92,7 +91,7 @@ public class JdbcRepositoryMapperMapResultSetExtractorIntegrationTests {
 	
 	interface CarRepository extends CrudRepository<Car, Long> {
 
-		@Query("select * from car")
+		@Query(value = "select * from car", resultSetExtractorClass = CarResultSetExtractor.class)
 		List<Car> customFindAll();
 	}
 	

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/ConfigurableRowMapperMapUnitTests.java

@@ -19,10 +19,7 @@ import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
 import org.junit.Test;
-import org.mockito.Mockito;
 import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
-import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.RowMapper;
 
 /**
@@ -38,7 +35,7 @@ public class ConfigurableRowMapperMapUnitTests {
 
 		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration();
 
-		assertThat(map.getMapperOrExtractor(Object.class)).isNull();
+		assertThat(map.getRowMapper(Object.class)).isNull();
 	}
 
 	@Test // DATAJDBC-166
@@ -48,18 +45,7 @@ public class ConfigurableRowMapperMapUnitTests {
 
 		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerRowMapper(Object.class, rowMapper);
 
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(rowMapper));
-	}
-
-	@Test // DATAJDBC-166
-	public void returnsConfiguredInstanceResultSetExtractorForClass() {
-
-		ResultSetExtractor resultSetExtractor = mock(ResultSetExtractor.class);
-
-		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerResultSetExtractor(Object.class,
-				resultSetExtractor);
-
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(resultSetExtractor));
+		assertThat(map.getRowMapper(Object.class)).isEqualTo(rowMapper);
 	}
 
 	@Test // DATAJDBC-166
@@ -69,20 +55,8 @@ public class ConfigurableRowMapperMapUnitTests {
 
 		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerRowMapper(Number.class, rowMapper);
 
-		assertThat(map.getMapperOrExtractor(Integer.class)).isNull();
-		assertThat(map.getMapperOrExtractor(String.class)).isNull();
-	}
-
-	@Test // DATAJDBC-166
-	public void returnsNullResultSetExtractorForClassNotConfigured() {
-
-		ResultSetExtractor resultSetExtractor = mock(ResultSetExtractor.class);
-
-		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerResultSetExtractor(Number.class,
-				resultSetExtractor);
-
-		assertThat(map.getMapperOrExtractor(Integer.class)).isNull();
-		assertThat(map.getMapperOrExtractor(String.class)).isNull();
+		assertThat(map.getRowMapper(Integer.class)).isNull();
+		assertThat(map.getRowMapper(String.class)).isNull();
 	}
 
 	@Test // DATAJDBC-166
@@ -92,18 +66,7 @@ public class ConfigurableRowMapperMapUnitTests {
 
 		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerRowMapper(String.class, rowMapper);
 
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(rowMapper));
-	}
-
-	@Test // DATAJDBC-290
-	public void returnsInstanceOfResultSetExtractorRegisteredForSubClass() {
-
-		ResultSetExtractor resultSetExtractor = mock(ResultSetExtractor.class);
-
-		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration().registerResultSetExtractor(String.class,
-				resultSetExtractor);
-
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(resultSetExtractor));
+		assertThat(map.getRowMapper(Object.class)).isEqualTo(rowMapper);
 	}
 
 	@Test // DATAJDBC-166
@@ -116,20 +79,7 @@ public class ConfigurableRowMapperMapUnitTests {
 				.registerRowMapper(Integer.class, rowMapper) //
 				.registerRowMapper(Number.class, mock(RowMapper.class));
 
-		assertThat(map.getMapperOrExtractor(Integer.class)).isEqualTo(RowMapperOrResultsetExtractor.of(rowMapper));
-	}
-
-	@Test // DATAJDBC-290
-	public void prefersExactResultSetExtractorTypeMatchClass() {
-
-		ResultSetExtractor resultSetExtractor = mock(ResultSetExtractor.class);
-
-		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration() //
-				.registerResultSetExtractor(Object.class, mock(ResultSetExtractor.class)) //
-				.registerResultSetExtractor(Integer.class, resultSetExtractor) //
-				.registerResultSetExtractor(Number.class, mock(ResultSetExtractor.class));
-
-		assertThat(map.getMapperOrExtractor(Integer.class)).isEqualTo(RowMapperOrResultsetExtractor.of(resultSetExtractor));
+		assertThat(map.getRowMapper(Integer.class)).isEqualTo(rowMapper);
 	}
 
 	@Test // DATAJDBC-166
@@ -141,18 +91,7 @@ public class ConfigurableRowMapperMapUnitTests {
 				.registerRowMapper(Integer.class, mock(RowMapper.class)) //
 				.registerRowMapper(Number.class, rowMapper);
 
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(rowMapper));
+		assertThat(map.getRowMapper(Object.class)).isEqualTo(rowMapper);
 	}
 
-	@Test // DATAJDBC-290
-	public void prefersLatestRegistrationOfResultSetExtractorForSuperTypeMatch() {
-
-		ResultSetExtractor resultSetExtractor = mock(ResultSetExtractor.class);
-
-		QueryMappingConfiguration map = new DefaultQueryMappingConfiguration() //
-				.registerResultSetExtractor(Integer.class, mock(ResultSetExtractor.class)) //
-				.registerResultSetExtractor(Number.class, resultSetExtractor);
-
-		assertThat(map.getMapperOrExtractor(Object.class)).isEqualTo(RowMapperOrResultsetExtractor.of(resultSetExtractor));
-	}
 }

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcRepositoriesIntegrationTests.java

@@ -33,7 +33,6 @@ import org.springframework.data.annotation.Id;
 import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
 import org.springframework.data.jdbc.repository.config.EnableJdbcRepositoriesIntegrationTests.TestConfiguration;
 import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactoryBean;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.RowMapper;
@@ -52,7 +51,8 @@ import org.springframework.util.ReflectionUtils;
 @ContextConfiguration(classes = TestConfiguration.class)
 public class EnableJdbcRepositoriesIntegrationTests {
 
-	static final Field MAPPER_MAP = ReflectionUtils.findField(JdbcRepositoryFactoryBean.class, "queryMappingConfiguration");
+	static final Field MAPPER_MAP = ReflectionUtils.findField(JdbcRepositoryFactoryBean.class,
+			"queryMappingConfiguration");
 	public static final RowMapper DUMMY_ENTITY_ROW_MAPPER = mock(RowMapper.class);
 	public static final RowMapper STRING_ROW_MAPPER = mock(RowMapper.class);
 	public static final ResultSetExtractor<Integer> INTEGER_RESULT_SET_EXTRACTOR = mock(ResultSetExtractor.class);
@@ -75,30 +75,13 @@ public class EnableJdbcRepositoriesIntegrationTests {
 		assertThat(all).isNotNull();
 	}
 
-	@Test // DATAJDBC-290
-	public void customResultSetExtractorConfigurationGetsPickedUp() {
-
-		QueryMappingConfiguration mapping = (QueryMappingConfiguration) ReflectionUtils.getField(MAPPER_MAP, factoryBean);
-		assertThat(mapping.getMapperOrExtractor(Integer.class))
-				.isEqualTo(RowMapperOrResultsetExtractor.of(INTEGER_RESULT_SET_EXTRACTOR));
-	}
-
-	@Test // DATAJDBC-290
-	public void customResultSetExtractorConfigurationIsNotPickedUpIfRowMapperIsRegisteredForTheSameType() {
-
-		QueryMappingConfiguration mapping = (QueryMappingConfiguration) ReflectionUtils.getField(MAPPER_MAP, factoryBean);
-		assertThat(mapping.getMapperOrExtractor(String.class).isResultSetExtractor()).isFalse();
-	}
-
 	@Test // DATAJDBC-166
 	public void customRowMapperConfigurationGetsPickedUp() {
 
 		QueryMappingConfiguration mapping = (QueryMappingConfiguration) ReflectionUtils.getField(MAPPER_MAP, factoryBean);
 
-		assertThat(mapping.getMapperOrExtractor(String.class))
-				.isEqualTo(RowMapperOrResultsetExtractor.of(STRING_ROW_MAPPER));
-		assertThat(mapping.getMapperOrExtractor(DummyEntity.class))
-				.isEqualTo(RowMapperOrResultsetExtractor.of(DUMMY_ENTITY_ROW_MAPPER));
+		assertThat(mapping.getRowMapper(String.class)).isEqualTo(STRING_ROW_MAPPER);
+		assertThat(mapping.getRowMapper(DummyEntity.class)).isEqualTo(DUMMY_ENTITY_ROW_MAPPER);
 	}
 
 	interface DummyRepository extends CrudRepository<DummyEntity, Long> {
@@ -125,8 +108,7 @@ public class EnableJdbcRepositoriesIntegrationTests {
 
 			return new DefaultQueryMappingConfiguration() //
 					.registerRowMapper(DummyEntity.class, DUMMY_ENTITY_ROW_MAPPER) //
-					.registerRowMapper(String.class, STRING_ROW_MAPPER)
-					.registerResultSetExtractor(Integer.class, INTEGER_RESULT_SET_EXTRACTOR);
+					.registerRowMapper(String.class, STRING_ROW_MAPPER);
 		}
 
 	}

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java

@@ -84,20 +84,6 @@ public class JdbcQueryLookupStrategyUnitTests {
 
 		verify(operations).queryForObject(anyString(), any(SqlParameterSource.class), eq(numberFormatMapper));
 	}
-	
-	@Test // DATAJDBC-290
-	@SuppressWarnings("unchecked")
-	public void typeBasedResultSetExtractorGetsUsedForQuery() {
-
-		ResultSetExtractor<? extends NumberFormat> numberFormatMapper = mock(ResultSetExtractor.class);
-		QueryMappingConfiguration mappingConfiguration = new DefaultQueryMappingConfiguration().registerResultSetExtractor(NumberFormat.class, numberFormatMapper);
-
-		RepositoryQuery repositoryQuery = getRepositoryQuery("returningNumberFormat", mappingConfiguration);
-
-		repositoryQuery.execute(new Object[] {});
-
-		verify(operations).query(anyString(), any(SqlParameterSource.class), eq(numberFormatMapper));
-	}
 
 	private RepositoryQuery getRepositoryQuery(String name, QueryMappingConfiguration mappingConfiguration) {
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryQueryUnitTests.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.jdbc.repository.support;
 
+import static org.assertj.core.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
@@ -22,15 +23,14 @@ import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.*;
 
 import java.sql.ResultSet;
-import java.sql.SQLException;
 import java.util.Arrays;
 
 import org.assertj.core.api.Assertions;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.dao.DataAccessException;
-import org.springframework.data.jdbc.support.RowMapperOrResultsetExtractor;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.event.AfterLoadEvent;
 import org.springframework.data.repository.query.DefaultParameters;
@@ -54,7 +54,6 @@ public class JdbcRepositoryQueryUnitTests {
 
 	RowMapper<?> defaultRowMapper;
 	ResultSetExtractor<?> defaultResultSetExtractor;
-	JdbcRepositoryQuery query;
 	NamedParameterJdbcOperations operations;
 	ApplicationEventPublisher publisher;
 	RelationalMappingContext context;
@@ -72,8 +71,6 @@ public class JdbcRepositoryQueryUnitTests {
 		this.operations = mock(NamedParameterJdbcOperations.class);
 		this.publisher = mock(ApplicationEventPublisher.class);
 		this.context = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS);
-
-		this.query = new JdbcRepositoryQuery(publisher, context, queryMethod, operations, RowMapperOrResultsetExtractor.of(defaultRowMapper));
 	}
 
 	@Test // DATAJDBC-165
@@ -82,7 +79,8 @@ public class JdbcRepositoryQueryUnitTests {
 		doReturn(null).when(queryMethod).getAnnotatedQuery();
 
 		Assertions.assertThatExceptionOfType(IllegalStateException.class) //
-				.isThrownBy(() -> query.execute(new Object[] {}));
+				.isThrownBy(() -> new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper)
+						.execute(new Object[] {}));
 	}
 
 	@Test // DATAJDBC-165
@@ -90,6 +88,7 @@ public class JdbcRepositoryQueryUnitTests {
 
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
 		doReturn(RowMapper.class).when(queryMethod).getRowMapperClass();
+		JdbcRepositoryQuery query = new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper);
 
 		query.execute(new Object[] {});
 
@@ -100,6 +99,7 @@ public class JdbcRepositoryQueryUnitTests {
 	public void defaultRowMapperIsUsedForNull() {
 
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
+		JdbcRepositoryQuery query = new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper);
 
 		query.execute(new Object[] {});
 
@@ -112,35 +112,59 @@ public class JdbcRepositoryQueryUnitTests {
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
 		doReturn(CustomRowMapper.class).when(queryMethod).getRowMapperClass();
 
-		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, RowMapperOrResultsetExtractor.of(defaultRowMapper)).execute(new Object[] {});
+		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper).execute(new Object[] {});
 
 		verify(operations) //
 				.queryForObject(anyString(), any(SqlParameterSource.class), isA(CustomRowMapper.class));
 	}
-	
+
 	@Test // DATAJDBC-290
 	public void customResultSetExtractorIsUsedWhenSpecified() {
 
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
 		doReturn(CustomResultSetExtractor.class).when(queryMethod).getResultSetExtractorClass();
 
-		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, RowMapperOrResultsetExtractor.of(defaultRowMapper)).execute(new Object[] {});
+		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper).execute(new Object[] {});
 
-		verify(operations) //
-				.query(anyString(), any(SqlParameterSource.class), isA(CustomResultSetExtractor.class));
+		ArgumentCaptor<CustomResultSetExtractor> captor = ArgumentCaptor.forClass(CustomResultSetExtractor.class);
+
+		verify(operations).query(anyString(), any(SqlParameterSource.class), captor.capture());
+
+		assertThat(captor.getValue()) //
+				.isInstanceOf(CustomResultSetExtractor.class) // not verified by the captor
+				.matches(crse -> crse.rowMapper == null, "RowMapper is expected to be null.");
 	}
 
+	@Test // DATAJDBC-290
+	public void customResultSetExtractorAndRowMapperGetCombined() {
+
+		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
+		doReturn(CustomResultSetExtractor.class).when(queryMethod).getResultSetExtractorClass();
+		doReturn(CustomRowMapper.class).when(queryMethod).getRowMapperClass();
+
+		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper).execute(new Object[] {});
+
+		ArgumentCaptor<CustomResultSetExtractor> captor = ArgumentCaptor.forClass(CustomResultSetExtractor.class);
+
+		verify(operations).query(anyString(), any(SqlParameterSource.class), captor.capture());
+
+		assertThat(captor.getValue()) //
+				.isInstanceOf(CustomResultSetExtractor.class) // not verified by the captor
+				.matches(crse -> crse.rowMapper != null, "RowMapper is not expected to be null");
+	}
 
 	@Test // DATAJDBC-263
 	public void publishesSingleEventWhenQueryReturnsSingleAggregate() {
 
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
 		doReturn(false).when(queryMethod).isCollectionQuery();
-		doReturn(new DummyEntity(1L)).when(operations).queryForObject(anyString(), any(SqlParameterSource.class), any(RowMapper.class));
+		doReturn(new DummyEntity(1L)).when(operations).queryForObject(anyString(), any(SqlParameterSource.class),
+				any(RowMapper.class));
 		doReturn(true).when(context).hasPersistentEntityFor(DummyEntity.class);
-		when(context.getRequiredPersistentEntity(DummyEntity.class).getIdentifierAccessor(any()).getRequiredIdentifier()).thenReturn("some identifier");
+		when(context.getRequiredPersistentEntity(DummyEntity.class).getIdentifierAccessor(any()).getRequiredIdentifier())
+				.thenReturn("some identifier");
 
-		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, RowMapperOrResultsetExtractor.of(defaultRowMapper)).execute(new Object[] {});
+		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper).execute(new Object[] {});
 
 		verify(publisher).publishEvent(any(AfterLoadEvent.class));
 	}
@@ -150,11 +174,13 @@ public class JdbcRepositoryQueryUnitTests {
 
 		doReturn("some sql statement").when(queryMethod).getAnnotatedQuery();
 		doReturn(true).when(queryMethod).isCollectionQuery();
-		doReturn(Arrays.asList(new DummyEntity(1L), new DummyEntity(1L))).when(operations).query(anyString(), any(SqlParameterSource.class), any(RowMapper.class));
+		doReturn(Arrays.asList(new DummyEntity(1L), new DummyEntity(1L))).when(operations).query(anyString(),
+				any(SqlParameterSource.class), any(RowMapper.class));
 		doReturn(true).when(context).hasPersistentEntityFor(DummyEntity.class);
-		when(context.getRequiredPersistentEntity(DummyEntity.class).getIdentifierAccessor(any()).getRequiredIdentifier()).thenReturn("some identifier");
+		when(context.getRequiredPersistentEntity(DummyEntity.class).getIdentifierAccessor(any()).getRequiredIdentifier())
+				.thenReturn("some identifier");
 
-		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, RowMapperOrResultsetExtractor.of(defaultRowMapper)).execute(new Object[] {});
+		new JdbcRepositoryQuery(publisher, context, queryMethod, operations, defaultRowMapper).execute(new Object[] {});
 
 		verify(publisher, times(2)).publishEvent(any(AfterLoadEvent.class));
 	}
@@ -172,11 +198,22 @@ public class JdbcRepositoryQueryUnitTests {
 			return null;
 		}
 	}
-	
+
 	private static class CustomResultSetExtractor implements ResultSetExtractor<Object> {
 
+		private final RowMapper rowMapper;
+
+		CustomResultSetExtractor() {
+			rowMapper = null;
+		}
+
+		public CustomResultSetExtractor(RowMapper rowMapper) {
+
+			this.rowMapper = rowMapper;
+		}
+
 		@Override
-		public Object extractData(ResultSet rs) throws SQLException, DataAccessException {
+		public Object extractData(ResultSet rs) throws DataAccessException {
 			return null;
 		}
 	}