Polishing.

Original pull request #1604
See #1586

Author: schauder

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

@@ -37,7 +37,7 @@
 /**
  * Reads complete Aggregates from the database, by generating appropriate SQL using a {@link SingleQuerySqlGenerator}
  * through {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}. Results are converterd into an
- * intermediate {@link ResultSetRowDocumentExtractor RowDocument} and mapped via
+ * intermediate {@link RowDocumentResultSetExtractor RowDocument} and mapped via
  * {@link org.springframework.data.relational.core.conversion.RelationalConverter#read(Class, RowDocument)}.
  *
  * @param <T> the type of aggregate produced by this reader.
@@ -47,23 +47,23 @@
  */
 class AggregateReader<T> {
 
-	private final RelationalPersistentEntity<T> entity;
+	private final RelationalPersistentEntity<T> aggregate;
 	private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator sqlGenerator;
 	private final JdbcConverter converter;
 	private final NamedParameterJdbcOperations jdbcTemplate;
-	private final ResultSetRowDocumentExtractor extractor;
+	private final RowDocumentResultSetExtractor extractor;
 
 	AggregateReader(Dialect dialect, JdbcConverter converter, AliasFactory aliasFactory,
-			NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity<T> entity) {
+			NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity<T> aggregate) {
 
 		this.converter = converter;
-		this.entity = entity;
+		this.aggregate = aggregate;
 		this.jdbcTemplate = jdbcTemplate;
 
 		this.sqlGenerator = new CachingSqlGenerator(
-				new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, entity));
+				new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate));
 
-		this.extractor = new ResultSetRowDocumentExtractor(converter.getMappingContext(),
+		this.extractor = new RowDocumentResultSetExtractor(converter.getMappingContext(),
 				createPathToColumnMapping(aliasFactory));
 	}
 
@@ -74,44 +74,66 @@ public List<T> findAll() {
 	@Nullable
 	public T findById(Object id) {
 
-		id = converter.writeValue(id, entity.getRequiredIdProperty().getTypeInformation());
+		id = converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation());
 
-		return jdbcTemplate.query(sqlGenerator.findById(), Map.of("id", id), rs -> {
-
-			Iterator<RowDocument> iterate = extractor.iterate(entity, rs);
-			if (iterate.hasNext()) {
-
-				RowDocument object = iterate.next();
-				if (iterate.hasNext()) {
-					throw new IncorrectResultSizeDataAccessException(1);
-				}
-				return converter.read(entity.getType(), object);
-			}
-			return null;
-		});
+		return jdbcTemplate.query(sqlGenerator.findById(), Map.of("id", id), this::extractZeroOrOne);
 	}
 
 	public Iterable<T> findAllById(Iterable<?> ids) {
 
 		List<Object> convertedIds = new ArrayList<>();
 		for (Object id : ids) {
-			convertedIds.add(converter.writeValue(id, entity.getRequiredIdProperty().getTypeInformation()));
+			convertedIds.add(converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation()));
 		}
 
 		return jdbcTemplate.query(sqlGenerator.findAllById(), Map.of("ids", convertedIds), this::extractAll);
 	}
 
+	/**
+	 * Extracts a list of aggregates from the given {@link ResultSet} by utilizing the
+	 * {@link RowDocumentResultSetExtractor} and the {@link JdbcConverter}. When used as a method reference this conforms
+	 * to the {@link org.springframework.jdbc.core.ResultSetExtractor} contract.
+	 * 
+	 * @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
+	 * @return a {@code List} of aggregates, fully converted.
+	 * @throws SQLException
+	 */
 	private List<T> extractAll(ResultSet rs) throws SQLException {
 
-		Iterator<RowDocument> iterate = extractor.iterate(entity, rs);
+		Iterator<RowDocument> iterate = extractor.iterate(aggregate, rs);
 		List<T> resultList = new ArrayList<>();
 		while (iterate.hasNext()) {
-			resultList.add(converter.read(entity.getType(), iterate.next()));
+			resultList.add(converter.read(aggregate.getType(), iterate.next()));
 		}
 
 		return resultList;
 	}
 
+	/**
+	 * Extracts a single aggregate or {@literal null} from the given {@link ResultSet} by utilizing the
+	 * {@link RowDocumentResultSetExtractor} and the {@link JdbcConverter}. When used as a method reference this conforms
+	 * to the {@link org.springframework.jdbc.core.ResultSetExtractor} contract.
+	 *
+	 * @param @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
+	 * @return The single instance when the conversion results in exactly one instance. If the {@literal ResultSet} is empty, null is returned.
+	 * @throws SQLException
+	 * @throws IncorrectResultSizeDataAccessException when the conversion yields more than one instance.
+	 */
+	@Nullable
+	private T extractZeroOrOne(ResultSet rs) throws SQLException {
+
+		Iterator<RowDocument> iterate = extractor.iterate(aggregate, rs);
+		if (iterate.hasNext()) {
+
+			RowDocument object = iterate.next();
+			if (iterate.hasNext()) {
+				throw new IncorrectResultSizeDataAccessException(1);
+			}
+			return converter.read(aggregate.getType(), object);
+		}
+		return null;
+	}
+
 	private PathToColumnMapping createPathToColumnMapping(AliasFactory aliasFactory) {
 		return new PathToColumnMapping() {
 			@Override

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentExtractorSupport.java

@@ -79,6 +79,7 @@ protected static class AggregateContext<RS> {
 
 		protected AggregateContext(TabularResultAdapter<RS> adapter, RelationalMappingContext context,
 				PathToColumnMapping propertyToColumn, Map<String, Integer> columnMap) {
+
 			this.adapter = adapter;
 			this.context = context;
 			this.propertyToColumn = propertyToColumn;
@@ -183,6 +184,7 @@ protected static class RowDocumentSink<RS> extends TabularSink<RS> {
 
 		public RowDocumentSink(AggregateContext<RS> aggregateContext, RelationalPersistentEntity<?> entity,
 				AggregatePath basePath) {
+
 			this.aggregateContext = aggregateContext;
 			this.entity = entity;
 			this.basePath = basePath;
@@ -234,11 +236,13 @@ private void readEntity(RS row, RowDocument document, AggregatePath basePath,
 				AggregatePath path = basePath.append(property);
 
 				if (property.isEntity() && !property.isEmbedded() && (property.isCollectionLike() || property.isQualified())) {
+
 					readerState.put(property, new ContainerSink<>(aggregateContext, property, path));
 					continue;
 				}
 
 				if (property.isEmbedded()) {
+
 					RelationalPersistentEntity<?> embeddedEntity = aggregateContext.getRequiredPersistentEntity(property);
 					readEntity(row, document, path, embeddedEntity);
 					continue;
@@ -286,11 +290,7 @@ boolean hasResult() {
 				}
 			}
 
-			if (result.isEmpty() && key == null) {
-				return false;
-			}
-
-			return true;
+			return !(result.isEmpty() && key == null);
 		}
 
 		@Override
@@ -308,6 +308,7 @@ RowDocument getResult() {
 
 		@Override
 		void reset() {
+
 			result = null;
 			readerState.clear();
 		}
@@ -326,6 +327,7 @@ private static class SingleColumnSink<RS> extends TabularSink<RS> {
 		private @Nullable Object value;
 
 		public SingleColumnSink(AggregateContext<RS> aggregateContext, AggregatePath path) {
+
 			this.aggregateContext = aggregateContext;
 			this.columnName = path.getColumnInfo().name().getReference();
 		}
@@ -431,6 +433,7 @@ public boolean hasResult() {
 		public Object getResult() {
 
 			if (componentReader.hasResult()) {
+
 				container.add(this.key, componentReader.getResult());
 				componentReader.reset();
 			}

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractor.java renamed to spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java

@@ -40,12 +40,13 @@
  * @author Mark Paluch
  * @since 3.2
  */
-class ResultSetRowDocumentExtractor {
+class RowDocumentResultSetExtractor {
 
 	private final RelationalMappingContext context;
 	private final PathToColumnMapping propertyToColumn;
 
-	ResultSetRowDocumentExtractor(RelationalMappingContext context, PathToColumnMapping propertyToColumn) {
+	RowDocumentResultSetExtractor(RelationalMappingContext context, PathToColumnMapping propertyToColumn) {
+
 		this.context = context;
 		this.propertyToColumn = propertyToColumn;
 	}
@@ -54,11 +55,14 @@ class ResultSetRowDocumentExtractor {
 	 * Adapter to extract values and column metadata from a {@link ResultSet}.
 	 */
 	enum ResultSetAdapter implements TabularResultAdapter<ResultSet> {
+
 		INSTANCE;
 
 		@Override
 		public Object getObject(ResultSet row, int index) {
+
 			try {
+
 				Object resultSetValue = JdbcUtils.getResultSetValue(row, index);
 
 				if (resultSetValue instanceof Array a) {
@@ -75,6 +79,7 @@ public Object getObject(ResultSet row, int index) {
 		public Map<String, Integer> getColumnMap(ResultSet result) {
 
 			try {
+
 				ResultSetMetaData metaData = result.getMetaData();
 				Map<String, Integer> columns = new LinkedCaseInsensitiveMap<>(metaData.getColumnCount());
 
@@ -201,5 +206,4 @@ public RowDocument next() {
 			return reader.getResult();
 		}
 	}
-
 }

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractorUnitTests.java renamed to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractorUnitTests.java

@@ -41,19 +41,19 @@
 import org.springframework.data.relational.domain.RowDocument;
 
 /**
- * Unit tests for the {@link ResultSetRowDocumentExtractor}.
+ * Unit tests for the {@link RowDocumentResultSetExtractor}.
  *
  * @author Jens Schauder
  * @author Mark Paluch
  */
-public class ResultSetRowDocumentExtractorUnitTests {
+public class RowDocumentResultSetExtractorUnitTests {
 
 	RelationalMappingContext context = new JdbcMappingContext(new DefaultNamingStrategy());
 
 	private final PathToColumnMapping column = new PathToColumnMapping() {
 		@Override
 		public String column(AggregatePath path) {
-			return ResultSetRowDocumentExtractorUnitTests.this.column(path);
+			return RowDocumentResultSetExtractorUnitTests.this.column(path);
 		}
 
 		@Override
@@ -62,7 +62,7 @@ public String keyColumn(AggregatePath path) {
 		}
 	};
 
-	ResultSetRowDocumentExtractor documentExtractor = new ResultSetRowDocumentExtractor(context, column);
+	RowDocumentResultSetExtractor documentExtractor = new RowDocumentResultSetExtractor(context, column);
 
 	@Test // GH-1446
 	void emptyResultSetYieldsEmptyResult() {
@@ -272,8 +272,7 @@ void extractSingleUnorderedListReference() {
 	@Nested
 	class Maps {
 
-		@Test
-		// GH-1446
+		@Test // GH-1446
 		void extractSingleMapReference() {
 
 			testerFor(WithMaps.class).resultSet(rsc -> {
@@ -288,8 +287,7 @@ void extractSingleMapReference() {
 			});
 		}
 
-		@Test
-		// GH-1446
+		@Test // GH-1446
 		void extractMultipleCollectionReference() {
 
 			testerFor(WithMapsAndList.class).resultSet(rsc -> {
@@ -307,8 +305,7 @@ void extractMultipleCollectionReference() {
 			});
 		}
 
-		@Test
-		// GH-1446
+		@Test // GH-1446
 		void extractNestedMapsWithId() {
 
 			testerFor(WithMaps.class).resultSet(rsc -> {
@@ -529,16 +526,19 @@ public ResultSetConfigurer withRow(Object... rowValues) {
 	private static class DocumentTester extends AbstractTester {
 
 		private final Class<?> entityType;
-		private final ResultSetRowDocumentExtractor extractor;
+		private final RowDocumentResultSetExtractor extractor;
+
+		DocumentTester(Class<?> entityType, RelationalMappingContext context, RowDocumentResultSetExtractor extractor) {
 
-		DocumentTester(Class<?> entityType, RelationalMappingContext context, ResultSetRowDocumentExtractor extractor) {
 			super(entityType, context);
+
 			this.entityType = entityType;
 			this.extractor = extractor;
 		}
 
 		@Override
 		DocumentTester resultSet(Consumer<ResultSetConfigurer> configuration) {
+
 			super.resultSet(configuration);
 			return this;
 		}
@@ -561,7 +561,9 @@ private static class ResultSetTester extends AbstractTester {
 
 		@Override
 		ResultSetTester resultSet(Consumer<ResultSetConfigurer> configuration) {
+
 			super.resultSet(configuration);
+
 			return this;
 		}
 

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DocumentPropertyAccessor.java

@@ -27,8 +27,7 @@
  * {@link org.springframework.expression.PropertyAccessor} to allow entity based field access to
  * {@link org.springframework.data.relational.domain.RowDocument}s.
  *
- * @author Oliver Gierke
- * @author Christoph Strobl
+ * @author Mark Paluch
  */
 class DocumentPropertyAccessor extends MapAccessor {
 

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java

@@ -67,7 +67,9 @@ public class MappingRelationalConverter extends BasicRelationalConverter {
 	 * @param context must not be {@literal null}.
 	 */
 	public MappingRelationalConverter(RelationalMappingContext context) {
+
 		super(context);
+
 		this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE);
 	}
 
@@ -79,7 +81,9 @@ public MappingRelationalConverter(RelationalMappingContext context) {
 	 * @param conversions must not be {@literal null}.
 	 */
 	public MappingRelationalConverter(RelationalMappingContext context, CustomConversions conversions) {
+
 		super(context, conversions);
+
 		this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE);
 	}
 

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RowDocumentAccessor.java

@@ -94,7 +94,6 @@ public Object get(RelationalPersistentProperty property) {
 	 * @param property must not be {@literal null}.
 	 * @return {@literal true} if no non {@literal null} value present.
 	 */
-	@SuppressWarnings("unchecked")
 	public boolean hasValue(RelationalPersistentProperty property) {
 
 		Assert.notNull(property, "Property must not be null");

spring-data-relational/src/main/java/org/springframework/data/relational/domain/RowDocument.java

@@ -42,6 +42,7 @@ public RowDocument() {
 	}
 
 	public RowDocument(Map<String, ? extends Object> map) {
+
 		this.delegate = new LinkedCaseInsensitiveMap<>();
 		this.delegate.putAll(delegate);
 	}