Polishing.

Extract Single Query Loading branching to SingleQueryFallbackDataAccessStrategy. Inline AggregateReaderFactory into SingleQueryDataAccessStrategy. Move CachingSqlGenerator to AggregateReader as caching root.

Introduce DataAccessStrategyFactory to encapsulate configuration.

Fix Javadoc tag ordering. Remove superfluous MappingContext parameters when Converter is available. Simplify code. Reformat code.

Reorder Functions methods. Tweak Javadoc, move composite function into SingleQuerySqlGenerator.

See #1446
See #1450
See #1445
Original pull request: #1572

Author: mp911de

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java

@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.springframework.data.jdbc.core.convert;
 
 import java.util.ArrayList;
@@ -24,71 +23,59 @@
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.relational.core.dialect.Dialect;
 import org.springframework.data.relational.core.mapping.AggregatePath;
-import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.sqlgeneration.AliasFactory;
-import org.springframework.data.relational.core.sqlgeneration.CachingSqlGenerator;
 import org.springframework.data.relational.core.sqlgeneration.SingleQuerySqlGenerator;
+import org.springframework.data.relational.core.sqlgeneration.SqlGenerator;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
  * Reads complete Aggregates from the database, by generating appropriate SQL using a {@link SingleQuerySqlGenerator}
  * and a matching {@link AggregateResultSetExtractor} and invoking a
  * {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}
- * 
+ *
  * @param <T> the type of aggregate produced by this reader.
- * @since 3.2
  * @author Jens Schauder
+ * @since 3.2
  */
 class AggregateReader<T> {
 
-	private final RelationalMappingContext mappingContext;
 	private final RelationalPersistentEntity<T> aggregate;
-	private final AliasFactory aliasFactory;
 	private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator sqlGenerator;
 	private final JdbcConverter converter;
 	private final NamedParameterJdbcOperations jdbcTemplate;
+	private final AggregateResultSetExtractor<T> extractor;
 
-	AggregateReader(RelationalMappingContext mappingContext, Dialect dialect, JdbcConverter converter,
+	AggregateReader(Dialect dialect, JdbcConverter converter, AliasFactory aliasFactory,
 			NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity<T> aggregate) {
 
-		this.mappingContext = mappingContext;
-
-		this.aggregate = aggregate;
 		this.converter = converter;
+		this.aggregate = aggregate;
 		this.jdbcTemplate = jdbcTemplate;
 
-		this.sqlGenerator = new CachingSqlGenerator(new SingleQuerySqlGenerator(mappingContext, dialect, aggregate));
-		this.aliasFactory = sqlGenerator.getAliasFactory();
+		this.sqlGenerator = new CachingSqlGenerator(
+				new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate));
+
+		this.extractor = new AggregateResultSetExtractor<>(aggregate, converter, createPathToColumnMapping(aliasFactory));
 	}
 
 	public List<T> findAll() {
 
-		String sql = sqlGenerator.findAll();
-
-		PathToColumnMapping pathToColumn = createPathToColumnMapping(aliasFactory);
-		AggregateResultSetExtractor<T> extractor = new AggregateResultSetExtractor<>(mappingContext, aggregate, converter,
-				pathToColumn);
-
-		Iterable<T> result = jdbcTemplate.query(sql, extractor);
+		Iterable<T> result = jdbcTemplate.query(sqlGenerator.findAll(), extractor);
 
 		Assert.state(result != null, "result is null");
 
 		return (List<T>) result;
 	}
 
+	@Nullable
 	public T findById(Object id) {
 
-		PathToColumnMapping pathToColumn = createPathToColumnMapping(aliasFactory);
-		AggregateResultSetExtractor<T> extractor = new AggregateResultSetExtractor<>(mappingContext, aggregate, converter,
-				pathToColumn);
-
-		String sql = sqlGenerator.findById();
-
 		id = converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation());
 
-		Iterator<T> result = jdbcTemplate.query(sql, Map.of("id", id), extractor).iterator();
+		Iterator<T> result = jdbcTemplate.query(sqlGenerator.findById(), Map.of("id", id), extractor).iterator();
 
 		T returnValue = result.hasNext() ? result.next() : null;
 
@@ -101,18 +88,12 @@ public T findById(Object id) {
 
 	public Iterable<T> findAllById(Iterable<?> ids) {
 
-		PathToColumnMapping pathToColumn = createPathToColumnMapping(aliasFactory);
-		AggregateResultSetExtractor<T> extractor = new AggregateResultSetExtractor<>(mappingContext, aggregate, converter,
-				pathToColumn);
-
-		String sql = sqlGenerator.findAllById();
-
 		List<Object> convertedIds = new ArrayList<>();
 		for (Object id : ids) {
 			convertedIds.add(converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation()));
 		}
 
-		return jdbcTemplate.query(sql, Map.of("ids", convertedIds), extractor);
+		return jdbcTemplate.query(sqlGenerator.findAllById(), Map.of("ids", convertedIds), extractor);
 	}
 
 	private PathToColumnMapping createPathToColumnMapping(AliasFactory aliasFactory) {
@@ -121,7 +102,7 @@ private PathToColumnMapping createPathToColumnMapping(AliasFactory aliasFactory)
 			public String column(AggregatePath path) {
 
 				String alias = aliasFactory.getColumnAlias(path);
-				Assert.notNull(alias, () -> "alias for >" + path + "<must not be null");
+				Assert.notNull(alias, () -> "alias for >" + path + "< must not be null");
 				return alias;
 			}
 
@@ -131,4 +112,49 @@ public String keyColumn(AggregatePath path) {
 			}
 		};
 	}
+
+	/**
+	 * A wrapper for the {@link org.springframework.data.relational.core.sqlgeneration.SqlGenerator} that caches the
+	 * generated statements.
+	 *
+	 * @since 3.2
+	 * @author Jens Schauder
+	 */
+	static class CachingSqlGenerator implements org.springframework.data.relational.core.sqlgeneration.SqlGenerator {
+
+		private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator delegate;
+
+		private final String findAll;
+		private final String findById;
+		private final String findAllById;
+
+		public CachingSqlGenerator(SqlGenerator delegate) {
+
+			this.delegate = delegate;
+
+			findAll = delegate.findAll();
+			findById = delegate.findById();
+			findAllById = delegate.findAllById();
+		}
+
+		@Override
+		public String findAll() {
+			return findAll;
+		}
+
+		@Override
+		public String findById() {
+			return findById;
+		}
+
+		@Override
+		public String findAllById() {
+			return findAllById;
+		}
+
+		@Override
+		public AliasFactory getAliasFactory() {
+			return delegate.getAliasFactory();
+		}
+	}
 }

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReaderFactory.java

@@ -48,10 +48,10 @@
  * which looks somewhat how one would represent an aggregate in a single excel table. The first row contains data of the
  * aggregate root, any single valued reference and the first element of any collection. Following rows do NOT repeat the
  * aggregate root data but contain data of second elements of any collections. For details see accompanying unit tests.
- * 
+ *
  * @param <T> the type of aggregates to extract
- * @since 3.2
  * @author Jens Schauder
+ * @since 3.2
  */
 class AggregateResultSetExtractor<T> implements ResultSetExtractor<Iterable<T>> {
 
@@ -61,26 +61,23 @@ class AggregateResultSetExtractor<T> implements ResultSetExtractor<Iterable<T>>
 	private final PathToColumnMapping propertyToColumn;
 
 	/**
-	 * @param context the {@link org.springframework.data.mapping.context.MappingContext} providing the metadata for the
-	 *          aggregate and its entity. Must not be {@literal null}.
 	 * @param rootEntity the aggregate root. Must not be {@literal null}.
 	 * @param converter Used for converting objects from the database to whatever is required by the aggregate. Must not
 	 *          be {@literal null}.
 	 * @param pathToColumn a mapping from {@link org.springframework.data.relational.core.mapping.AggregatePath} to the
 	 *          column of the {@link ResultSet} that holds the data for that
 	 *          {@link org.springframework.data.relational.core.mapping.AggregatePath}.
 	 */
-	AggregateResultSetExtractor(RelationalMappingContext context, RelationalPersistentEntity<T> rootEntity,
+	AggregateResultSetExtractor(RelationalPersistentEntity<T> rootEntity,
 			JdbcConverter converter, PathToColumnMapping pathToColumn) {
 
-		Assert.notNull(context, "context must not be null");
 		Assert.notNull(rootEntity, "rootEntity must not be null");
 		Assert.notNull(converter, "converter must not be null");
 		Assert.notNull(pathToColumn, "propertyToColumn must not be null");
 
-		this.context = context;
 		this.rootEntity = rootEntity;
 		this.converter = converter;
+		this.context = converter.getMappingContext();
 		this.propertyToColumn = pathToColumn;
 	}
 
@@ -131,7 +128,7 @@ private Object hydrateInstance(EntityInstantiator instantiator, ResultSetParamet
 
 	/**
 	 * A {@link Reader} is responsible for reading a single entity or collection of entities from a set of columns
-	 * 
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */
@@ -145,14 +142,14 @@ private interface Reader {
 		/**
 		 * Checks if this {@literal Reader} has all the data needed for a complete result, or if it needs to read further
 		 * rows.
-		 * 
+		 *
 		 * @return the result of the check.
 		 */
 		boolean hasResult();
 
 		/**
 		 * Constructs the result, returns it and resets the state of the reader to read the next instance.
-		 * 
+		 *
 		 * @return an instance of whatever this {@literal Reader} is supposed to read.
 		 */
 		@Nullable
@@ -161,7 +158,7 @@ private interface Reader {
 
 	/**
 	 * Adapts a {@link Map} to the interface of a {@literal Collection<Map.Entry<Object, Object>>}.
-	 * 
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */
@@ -221,7 +218,7 @@ public boolean add(Map.Entry<Object, Object> entry) {
 
 	/**
 	 * A {@link Reader} for reading entities.
-	 * 
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */
@@ -315,7 +312,7 @@ public String toString() {
 
 	/**
 	 * A {@link Reader} for reading collections of entities.
-	 * 
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */
@@ -413,7 +410,7 @@ public String toString() {
 	/**
 	 * A {@link Reader} for reading collection entries. Most of the work is done by an {@link EntityReader}, but a
 	 * additional key column might get read. The result is
-	 * 
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */
@@ -459,8 +456,9 @@ public Object getResultAndReset() {
 	}
 
 	/**
-	 * A {@link ParameterValueProvider} that provided the values for an entity from a continues set of rows in a {@link ResultSet}. These might be referenced entities or collections of such entities. {@link ResultSet}.
-	 * 
+	 * A {@link ParameterValueProvider} that provided the values for an entity from a continues set of rows in a
+	 * {@link ResultSet}. These might be referenced entities or collections of such entities. {@link ResultSet}.
+	 *
 	 * @since 3.2
 	 * @author Jens Schauder
 	 */

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateResultSetExtractor.java

@@ -26,8 +26,8 @@
  * Despite its name not really a {@link ResultSet}, but it offers the part of the {@literal ResultSet} API that is used
  * by {@link AggregateReader}. It allows peeking in the next row of a ResultSet by caching one row of the ResultSet.
  *
- * @since 3.2
  * @author Jens Schauder
+ * @since 3.2
  */
 class CachingResultSet {