From 4cf07be37c0625a581b38de1f2e7a8045e88f9f8 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 8 Sep 2023 16:02:40 +0200 Subject: [PATCH] Polishing. Original pull request #1604 See #1586 --- .../jdbc/core/convert/AggregateReader.java | 70 ++++++++++++------- .../convert/RowDocumentExtractorSupport.java | 13 ++-- ...ava => RowDocumentResultSetExtractor.java} | 10 ++- ...wDocumentResultSetExtractorUnitTests.java} | 26 +++---- .../conversion/DocumentPropertyAccessor.java | 3 +- .../MappingRelationalConverter.java | 4 ++ .../core/conversion/RowDocumentAccessor.java | 1 - .../data/relational/domain/RowDocument.java | 1 + 8 files changed, 81 insertions(+), 47 deletions(-) rename spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/{ResultSetRowDocumentExtractor.java => RowDocumentResultSetExtractor.java} (98%) rename spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/{ResultSetRowDocumentExtractorUnitTests.java => RowDocumentResultSetExtractorUnitTests.java} (97%) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java index 9582edb87..77813ea9b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java @@ -37,7 +37,7 @@ import org.springframework.util.Assert; /** * 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 the type of aggregate produced by this reader. @@ -47,23 +47,23 @@ import org.springframework.util.Assert; */ class AggregateReader { - private final RelationalPersistentEntity entity; + private final RelationalPersistentEntity 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 entity) { + NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity 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 @@ class AggregateReader { @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 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 findAllById(Iterable ids) { List 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 extractAll(ResultSet rs) throws SQLException { - Iterator iterate = extractor.iterate(entity, rs); + Iterator iterate = extractor.iterate(aggregate, rs); List 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 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 diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentExtractorSupport.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentExtractorSupport.java index 217dd9a0f..46e682f9c 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentExtractorSupport.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentExtractorSupport.java @@ -79,6 +79,7 @@ abstract class RowDocumentExtractorSupport { protected AggregateContext(TabularResultAdapter adapter, RelationalMappingContext context, PathToColumnMapping propertyToColumn, Map columnMap) { + this.adapter = adapter; this.context = context; this.propertyToColumn = propertyToColumn; @@ -183,6 +184,7 @@ abstract class RowDocumentExtractorSupport { public RowDocumentSink(AggregateContext aggregateContext, RelationalPersistentEntity entity, AggregatePath basePath) { + this.aggregateContext = aggregateContext; this.entity = entity; this.basePath = basePath; @@ -234,11 +236,13 @@ abstract class RowDocumentExtractorSupport { 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 @@ abstract class RowDocumentExtractorSupport { } } - if (result.isEmpty() && key == null) { - return false; - } - - return true; + return !(result.isEmpty() && key == null); } @Override @@ -308,6 +308,7 @@ abstract class RowDocumentExtractorSupport { @Override void reset() { + result = null; readerState.clear(); } @@ -326,6 +327,7 @@ abstract class RowDocumentExtractorSupport { private @Nullable Object value; public SingleColumnSink(AggregateContext aggregateContext, AggregatePath path) { + this.aggregateContext = aggregateContext; this.columnName = path.getColumnInfo().name().getReference(); } @@ -431,6 +433,7 @@ abstract class RowDocumentExtractorSupport { public Object getResult() { if (componentReader.hasResult()) { + container.add(this.key, componentReader.getResult()); componentReader.reset(); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java similarity index 98% rename from spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractor.java rename to spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java index 02fc1c6d3..2f558c71a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java @@ -40,12 +40,13 @@ import org.springframework.util.LinkedCaseInsensitiveMap; * @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 { + 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 @@ class ResultSetRowDocumentExtractor { public Map getColumnMap(ResultSet result) { try { + ResultSetMetaData metaData = result.getMetaData(); Map columns = new LinkedCaseInsensitiveMap<>(metaData.getColumnCount()); @@ -201,5 +206,4 @@ class ResultSetRowDocumentExtractor { return reader.getResult(); } } - } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractorUnitTests.java similarity index 97% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractorUnitTests.java rename to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractorUnitTests.java index 122dfaadc..e6e180d03 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetRowDocumentExtractorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractorUnitTests.java @@ -41,19 +41,19 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp 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 class ResultSetRowDocumentExtractorUnitTests { } }; - ResultSetRowDocumentExtractor documentExtractor = new ResultSetRowDocumentExtractor(context, column); + RowDocumentResultSetExtractor documentExtractor = new RowDocumentResultSetExtractor(context, column); @Test // GH-1446 void emptyResultSetYieldsEmptyResult() { @@ -272,8 +272,7 @@ public class ResultSetRowDocumentExtractorUnitTests { @Nested class Maps { - @Test - // GH-1446 + @Test // GH-1446 void extractSingleMapReference() { testerFor(WithMaps.class).resultSet(rsc -> { @@ -288,8 +287,7 @@ public class ResultSetRowDocumentExtractorUnitTests { }); } - @Test - // GH-1446 + @Test // GH-1446 void extractMultipleCollectionReference() { testerFor(WithMapsAndList.class).resultSet(rsc -> { @@ -307,8 +305,7 @@ public class ResultSetRowDocumentExtractorUnitTests { }); } - @Test - // GH-1446 + @Test // GH-1446 void extractNestedMapsWithId() { testerFor(WithMaps.class).resultSet(rsc -> { @@ -529,16 +526,19 @@ public class ResultSetRowDocumentExtractorUnitTests { 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 configuration) { + super.resultSet(configuration); return this; } @@ -561,7 +561,9 @@ public class ResultSetRowDocumentExtractorUnitTests { @Override ResultSetTester resultSet(Consumer configuration) { + super.resultSet(configuration); + return this; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DocumentPropertyAccessor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DocumentPropertyAccessor.java index f7d4afd20..3b863f09d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DocumentPropertyAccessor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DocumentPropertyAccessor.java @@ -27,8 +27,7 @@ import org.springframework.lang.Nullable; * {@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 { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java index d7fea6a8f..82185320d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java +++ b/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 class MappingRelationalConverter extends BasicRelationalConverter { * @param conversions must not be {@literal null}. */ public MappingRelationalConverter(RelationalMappingContext context, CustomConversions conversions) { + super(context, conversions); + this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RowDocumentAccessor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RowDocumentAccessor.java index ae6fcb59d..74568aac8 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RowDocumentAccessor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RowDocumentAccessor.java @@ -94,7 +94,6 @@ record RowDocumentAccessor(RowDocument document) { * @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"); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/RowDocument.java b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/RowDocument.java index f2217766d..b2295fbe5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/RowDocument.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/RowDocument.java @@ -42,6 +42,7 @@ public class RowDocument implements Map { } public RowDocument(Map map) { + this.delegate = new LinkedCaseInsensitiveMap<>(); this.delegate.putAll(delegate); }