diff --git a/pom.xml b/pom.xml index bfb0965512..d385f25461 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 271486f02a..54176d5f94 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index c2f44d3f96..86ffae8662 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection 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 4070894735..e1fb0e36cf 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 @@ -48,30 +48,24 @@ * 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. * @author Jens Schauder * @author Mark Paluch * @since 3.2 */ -class AggregateReader implements PathToColumnMapping { +class AggregateReader implements PathToColumnMapping { - private final RelationalPersistentEntity aggregate; - private final Table table; + private final AliasFactory aliasFactory; private final SqlGenerator sqlGenerator; private final JdbcConverter converter; private final NamedParameterJdbcOperations jdbcTemplate; - private final AliasFactory aliasFactory; private final RowDocumentResultSetExtractor extractor; - AggregateReader(Dialect dialect, JdbcConverter converter, AliasFactory aliasFactory, - NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity aggregate) { + AggregateReader(Dialect dialect, JdbcConverter converter, NamedParameterJdbcOperations jdbcTemplate) { + this.aliasFactory = new AliasFactory(); this.converter = converter; - this.aggregate = aggregate; this.jdbcTemplate = jdbcTemplate; - this.table = Table.create(aggregate.getQualifiedTableName()); - this.sqlGenerator = new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate); - this.aliasFactory = aliasFactory; + this.sqlGenerator = new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect); this.extractor = new RowDocumentResultSetExtractor(converter.getMappingContext(), this); } @@ -93,54 +87,55 @@ public String keyColumn(AggregatePath path) { } @Nullable - public T findById(Object id) { + public T findById(Object id, RelationalPersistentEntity entity) { - Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).is(id)).limit(1); + Query query = Query.query(Criteria.where(entity.getRequiredIdProperty().getName()).is(id)).limit(1); - return findOne(query); + return findOne(query, entity); } @Nullable - public T findOne(Query query) { - return doFind(query, this::extractZeroOrOne); + public T findOne(Query query, RelationalPersistentEntity entity) { + return doFind(query, entity, rs -> extractZeroOrOne(rs, entity)); } - public List findAllById(Iterable ids) { + public List findAllById(Iterable ids, RelationalPersistentEntity entity) { Collection identifiers = ids instanceof Collection idl ? idl : Streamable.of(ids).toList(); - Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).in(identifiers)); + Query query = Query.query(Criteria.where(entity.getRequiredIdProperty().getName()).in(identifiers)); - return findAll(query); + return findAll(query, entity); } @SuppressWarnings("ConstantConditions") - public List findAll() { - return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll); + public List findAll(RelationalPersistentEntity entity) { + return jdbcTemplate.query(sqlGenerator.findAll(entity), + (ResultSetExtractor>) rs -> extractAll(rs, entity)); } - public List findAll(Query query) { - return doFind(query, this::extractAll); + public List findAll(Query query, RelationalPersistentEntity entity) { + return doFind(query, entity, rs -> extractAll(rs, entity)); } @SuppressWarnings("ConstantConditions") - private R doFind(Query query, ResultSetExtractor extractor) { + private R doFind(Query query, RelationalPersistentEntity entity, ResultSetExtractor extractor) { MapSqlParameterSource parameterSource = new MapSqlParameterSource(); - Condition condition = createCondition(query, parameterSource); - String sql = sqlGenerator.findAll(condition); + Condition condition = createCondition(query, parameterSource, entity); + String sql = sqlGenerator.findAll(entity, condition); return jdbcTemplate.query(sql, parameterSource, extractor); } @Nullable - private Condition createCondition(Query query, MapSqlParameterSource parameterSource) { + private Condition createCondition(Query query, MapSqlParameterSource parameterSource, + RelationalPersistentEntity entity) { QueryMapper queryMapper = new QueryMapper(converter); Optional criteria = query.getCriteria(); - return criteria - .map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, table, aggregate)) - .orElse(null); + return criteria.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, + Table.create(entity.getQualifiedTableName()), entity)).orElse(null); } /** @@ -152,12 +147,13 @@ private Condition createCondition(Query query, MapSqlParameterSource parameterSo * @return a {@code List} of aggregates, fully converted. * @throws SQLException on underlying JDBC errors. */ - private List extractAll(ResultSet rs) throws SQLException { + private List extractAll(ResultSet rs, RelationalPersistentEntity entity) throws SQLException { - Iterator iterate = extractor.iterate(aggregate, rs); + Iterator iterate = extractor.iterate(entity, rs); List resultList = new ArrayList<>(); + while (iterate.hasNext()) { - resultList.add(converter.read(aggregate.getType(), iterate.next())); + resultList.add(converter.read(entity.getType(), iterate.next())); } return resultList; @@ -175,17 +171,19 @@ private List extractAll(ResultSet rs) throws SQLException { * @throws IncorrectResultSizeDataAccessException when the conversion yields more than one instance. */ @Nullable - private T extractZeroOrOne(ResultSet rs) throws SQLException { + private T extractZeroOrOne(ResultSet rs, RelationalPersistentEntity entity) throws SQLException { + + Iterator iterate = extractor.iterate(entity, rs); - 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 converter.read(entity.getType(), object); } + return null; } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java index d5fc206e0c..6b7c951422 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java @@ -25,9 +25,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.query.Query; -import org.springframework.data.relational.core.sqlgeneration.AliasFactory; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; -import org.springframework.util.ConcurrentLruCache; /** * A {@link ReadingDataAccessStrategy} that uses an {@link AggregateReader} to load entities with a single query. @@ -39,31 +37,28 @@ class SingleQueryDataAccessStrategy implements ReadingDataAccessStrategy { private final RelationalMappingContext mappingContext; - private final AliasFactory aliasFactory; - private final ConcurrentLruCache, AggregateReader> readerCache; + private final AggregateReader aggregateReader; public SingleQueryDataAccessStrategy(Dialect dialect, JdbcConverter converter, NamedParameterJdbcOperations jdbcTemplate) { this.mappingContext = converter.getMappingContext(); - this.aliasFactory = new AliasFactory(); - this.readerCache = new ConcurrentLruCache<>(256, - entity -> new AggregateReader<>(dialect, converter, aliasFactory, jdbcTemplate, entity)); + this.aggregateReader = new AggregateReader(dialect, converter, jdbcTemplate); } @Override public T findById(Object id, Class domainType) { - return getReader(domainType).findById(id); + return aggregateReader.findById(id, getPersistentEntity(domainType)); } @Override public List findAll(Class domainType) { - return getReader(domainType).findAll(); + return aggregateReader.findAll(getPersistentEntity(domainType)); } @Override public List findAllById(Iterable ids, Class domainType) { - return getReader(domainType).findAllById(ids); + return aggregateReader.findAllById(ids, getPersistentEntity(domainType)); } @Override @@ -78,12 +73,12 @@ public List findAll(Class domainType, Pageable pageable) { @Override public Optional findOne(Query query, Class domainType) { - return Optional.ofNullable(getReader(domainType).findOne(query)); + return Optional.ofNullable(aggregateReader.findOne(query, getPersistentEntity(domainType))); } @Override public List findAll(Query query, Class domainType) { - return getReader(domainType).findAll(query); + return aggregateReader.findAll(query, getPersistentEntity(domainType)); } @Override @@ -92,11 +87,7 @@ public List findAll(Query query, Class domainType, Pageable pageable) } @SuppressWarnings("unchecked") - private AggregateReader getReader(Class domainType) { - - RelationalPersistentEntity persistentEntity = (RelationalPersistentEntity) mappingContext - .getRequiredPersistentEntity(domainType); - - return (AggregateReader) readerCache.get(persistentEntity); + private RelationalPersistentEntity getPersistentEntity(Class domainType) { + return (RelationalPersistentEntity) mappingContext.getRequiredPersistentEntity(domainType); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryFallbackDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryFallbackDataAccessStrategy.java index 9628588f7a..2b03127ace 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryFallbackDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryFallbackDataAccessStrategy.java @@ -29,6 +29,7 @@ * Query Loading. * * @author Mark Paluch + * @author Jens Schauder * @since 3.2 */ class SingleQueryFallbackDataAccessStrategy extends DelegatingDataAccessStrategy { @@ -120,23 +121,25 @@ private boolean isSingleSelectQuerySupported(Class entityType) { private boolean entityQualifiesForSingleQueryLoading(Class entityType) { - boolean referenceFound = false; for (PersistentPropertyPath path : converter.getMappingContext() .findPersistentPropertyPaths(entityType, __ -> true)) { RelationalPersistentProperty property = path.getLeafProperty(); if (property.isEntity()) { + // single references are currently not supported + if (!(property.isMap() || property.isCollectionLike())) { + return false; + } + // embedded entities are currently not supported if (property.isEmbedded()) { return false; } - // only a single reference is currently supported - if (referenceFound) { + // nested references are currently not supported + if (path.getLength() > 1) { return false; } - - referenceFound = true; } } return true; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java index 858a69a791..e337383fa4 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java @@ -18,7 +18,6 @@ import static java.util.Arrays.*; import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; -import static org.assertj.core.api.SoftAssertions.*; import static org.springframework.data.jdbc.testing.TestConfiguration.*; import static org.springframework.data.jdbc.testing.TestDatabaseFeatures.Feature.*; @@ -36,7 +35,6 @@ import java.util.function.Function; import java.util.stream.IntStream; -import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; @@ -95,6 +93,7 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { @Autowired JdbcAggregateOperations template; @Autowired NamedParameterJdbcOperations jdbcTemplate; @Autowired RelationalMappingContext mappingContext; + @Autowired NamedParameterJdbcOperations jdbc; LegoSet legoSet = createLegoSet("Star Destroyer"); @@ -185,11 +184,11 @@ private static String asString(int i) { private static LegoSet createLegoSet(String name) { LegoSet entity = new LegoSet(); - entity.name = (name); + entity.name = name; Manual manual = new Manual(); - manual.content = ("Accelerates to 99% of light speed; Destroys almost everything. See https://what-if.xkcd.com/1/"); - entity.manual = (manual); + manual.content = "Accelerates to 99% of light speed; Destroys almost everything. See https://what-if.xkcd.com/1/"; + entity.manual = manual; return entity; } @@ -298,13 +297,10 @@ void saveAndLoadAnEntityWithReferencedEntityById() { assertThat(reloadedLegoSet.manual).isNotNull(); - assertSoftly(softly -> { - softly.assertThat(reloadedLegoSet.manual.id) // - .isEqualTo(legoSet.manual.id) // - .isNotNull(); - softly.assertThat(reloadedLegoSet.manual.content).isEqualTo(legoSet.manual.content); - }); - + assertThat(reloadedLegoSet.manual.id) // + .isEqualTo(legoSet.manual.id) // + .isNotNull(); + assertThat(reloadedLegoSet.manual.content).isEqualTo(legoSet.manual.content); } @Test // DATAJDBC-112 @@ -372,7 +368,6 @@ void findByNonPropertySortFails() { assertThatThrownBy(() -> template.findAll(LegoSet.class, Sort.by("somethingNotExistant"))) .isInstanceOf(InvalidPersistentPropertyPath.class); - } @Test // DATAJDBC-112 @@ -391,7 +386,7 @@ void saveAndLoadManyEntitiesByIdWithReferencedEntity() { @EnabledOnFeature(SUPPORTS_QUOTED_IDS) void saveAndLoadAnEntityWithReferencedNullEntity() { - legoSet.manual = (null); + legoSet.manual = null; template.save(legoSet); @@ -408,11 +403,8 @@ void saveAndDeleteAnEntityWithReferencedEntity() { template.delete(legoSet); - assertSoftly(softly -> { - - softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); - softly.assertThat(template.findAll(Manual.class)).isEmpty(); - }); + assertThat(template.findAll(LegoSet.class)).isEmpty(); + assertThat(template.findAll(Manual.class)).isEmpty(); } @Test // DATAJDBC-112 @@ -423,12 +415,8 @@ void saveAndDeleteAllWithReferencedEntity() { template.deleteAll(LegoSet.class); - assertSoftly(softly -> { - - softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); - softly.assertThat(template.findAll(Manual.class)).isEmpty(); - }); - + assertThat(template.findAll(LegoSet.class)).isEmpty(); + assertThat(template.findAll(Manual.class)).isEmpty(); } @Test // GH-537 @@ -441,11 +429,8 @@ void saveAndDeleteAllByAggregateRootsWithReferencedEntity() { template.deleteAll(List.of(legoSet1, legoSet2)); - assertSoftly(softly -> { - - softly.assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); - softly.assertThat(template.findAll(Manual.class)).hasSize(1); - }); + assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); + assertThat(template.findAll(Manual.class)).hasSize(1); } @Test // GH-537 @@ -458,11 +443,8 @@ void saveAndDeleteAllByIdsWithReferencedEntity() { template.deleteAllById(List.of(legoSet1.id, legoSet2.id), LegoSet.class); - assertSoftly(softly -> { - - softly.assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); - softly.assertThat(template.findAll(Manual.class)).hasSize(1); - }); + assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); + assertThat(template.findAll(Manual.class)).hasSize(1); } @Test @@ -519,9 +501,9 @@ void updateReferencedEntityFromNull() { template.save(legoSet); Manual manual = new Manual(); - manual.id = (23L); - manual.content = ("Some content"); - legoSet.manual = (manual); + manual.id = 23L; + manual.content = "Some content"; + legoSet.manual = manual; template.save(legoSet); @@ -536,18 +518,14 @@ void updateReferencedEntityToNull() { template.save(legoSet); - legoSet.manual = (null); + legoSet.manual = null; template.save(legoSet); LegoSet reloadedLegoSet = template.findById(legoSet.id, LegoSet.class); - SoftAssertions softly = new SoftAssertions(); - - softly.assertThat(reloadedLegoSet.manual).isNull(); - softly.assertThat(template.findAll(Manual.class)).describedAs("Manuals failed to delete").isEmpty(); - - softly.assertAll(); + assertThat(reloadedLegoSet.manual).isNull(); + assertThat(template.findAll(Manual.class)).describedAs("Manuals failed to delete").isEmpty(); } @Test @@ -555,7 +533,7 @@ void updateReferencedEntityToNull() { void updateFailedRootDoesNotExist() { LegoSet entity = new LegoSet(); - entity.id = (100L); // does not exist in the database + entity.id = 100L; // does not exist in the database assertThatExceptionOfType(DbActionExecutionException.class) // .isThrownBy(() -> template.save(entity)) // @@ -569,18 +547,15 @@ void replaceReferencedEntity() { template.save(legoSet); Manual manual = new Manual(); - manual.content = ("other content"); - legoSet.manual = (manual); + manual.content = "other content"; + legoSet.manual = manual; template.save(legoSet); LegoSet reloadedLegoSet = template.findById(legoSet.id, LegoSet.class); - assertSoftly(softly -> { - - softly.assertThat(reloadedLegoSet.manual.content).isEqualTo("other content"); - softly.assertThat(template.findAll(Manual.class)).describedAs("There should be only one manual").hasSize(1); - }); + assertThat(reloadedLegoSet.manual.content).isEqualTo("other content"); + assertThat(template.findAll(Manual.class)).describedAs("There should be only one manual").hasSize(1); } @Test // DATAJDBC-112 @@ -589,7 +564,7 @@ void changeReferencedEntity() { template.save(legoSet); - legoSet.manual.content = ("new content"); + legoSet.manual.content = "new content"; template.save(legoSet); @@ -672,14 +647,11 @@ void saveAndLoadAnEntityWithSecondaryReferenceNotNull() { LegoSet reloadedLegoSet = template.findById(legoSet.id, LegoSet.class); - assertSoftly(softly -> { - - softly.assertThat(reloadedLegoSet.alternativeInstructions).isNotNull(); - softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotNull(); - softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotEqualTo(reloadedLegoSet.manual.id); - softly.assertThat(reloadedLegoSet.alternativeInstructions.content) - .isEqualTo(reloadedLegoSet.alternativeInstructions.content); - }); + assertThat(reloadedLegoSet.alternativeInstructions).isNotNull(); + assertThat(reloadedLegoSet.alternativeInstructions.id).isNotNull(); + assertThat(reloadedLegoSet.alternativeInstructions.id).isNotEqualTo(reloadedLegoSet.manual.id); + assertThat(reloadedLegoSet.alternativeInstructions.content) + .isEqualTo(reloadedLegoSet.alternativeInstructions.content); } @Test // DATAJDBC-276 @@ -921,14 +893,11 @@ void shouldDeleteChainOfListsWithoutIds() { NoIdListChain4 saved = template.save(createNoIdTree()); template.deleteById(saved.four, NoIdListChain4.class); - assertSoftly(softly -> { - - softly.assertThat(count("NO_ID_LIST_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_LIST_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_LIST_CHAIN2")).describedAs("Chain2 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_LIST_CHAIN1")).describedAs("Chain1 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_LIST_CHAIN0")).describedAs("Chain0 elements got deleted").isEqualTo(0); - }); + assertThat(count("NO_ID_LIST_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_LIST_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_LIST_CHAIN2")).describedAs("Chain2 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_LIST_CHAIN1")).describedAs("Chain1 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_LIST_CHAIN0")).describedAs("Chain0 elements got deleted").isEqualTo(0); } @Test @@ -950,14 +919,11 @@ void shouldDeleteChainOfMapsWithoutIds() { NoIdMapChain4 saved = template.save(createNoIdMapTree()); template.deleteById(saved.four, NoIdMapChain4.class); - assertSoftly(softly -> { - - softly.assertThat(count("NO_ID_MAP_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_MAP_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_MAP_CHAIN2")).describedAs("Chain2 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_MAP_CHAIN1")).describedAs("Chain1 elements got deleted").isEqualTo(0); - softly.assertThat(count("NO_ID_MAP_CHAIN0")).describedAs("Chain0 elements got deleted").isEqualTo(0); - }); + assertThat(count("NO_ID_MAP_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_MAP_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_MAP_CHAIN2")).describedAs("Chain2 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_MAP_CHAIN1")).describedAs("Chain1 elements got deleted").isEqualTo(0); + assertThat(count("NO_ID_MAP_CHAIN0")).describedAs("Chain0 elements got deleted").isEqualTo(0); } @Test // DATAJDBC-431 @@ -972,7 +938,7 @@ void readOnlyGetsLoadedButNotWritten() { assertThat( jdbcTemplate.queryForObject("SELECT read_only FROM with_read_only", Collections.emptyMap(), String.class)) - .isEqualTo("from-db"); + .isEqualTo("from-db"); } @Test @@ -1202,6 +1168,103 @@ void readEnumArray() { assertThat(template.findById(entity.id, EnumArrayOwner.class).digits).isEqualTo(new Color[] { Color.BLUE }); } + @Test // GH-1448 + void multipleCollections() { + + MultipleCollections aggregate = new MultipleCollections(); + aggregate.name = "aggregate"; + + aggregate.listElements.add(new ListElement("one")); + aggregate.listElements.add(new ListElement("two")); + aggregate.listElements.add(new ListElement("three")); + + aggregate.setElements.add(new SetElement("one")); + aggregate.setElements.add(new SetElement("two")); + + aggregate.mapElements.put("alpha", new MapElement("one")); + aggregate.mapElements.put("beta", new MapElement("two")); + aggregate.mapElements.put("gamma", new MapElement("three")); + aggregate.mapElements.put("delta", new MapElement("four")); + + template.save(aggregate); + + MultipleCollections reloaded = template.findById(aggregate.id, MultipleCollections.class); + + assertThat(reloaded.name).isEqualTo(aggregate.name); + + assertThat(reloaded.listElements).containsExactly(aggregate.listElements.get(0), aggregate.listElements.get(1), + aggregate.listElements.get(2)); + + assertThat(reloaded.setElements).containsExactlyInAnyOrder(aggregate.setElements.toArray(new SetElement[0])); + + assertThat(reloaded.mapElements.get("alpha")).isEqualTo(new MapElement("one")); + assertThat(reloaded.mapElements.get("beta")).isEqualTo(new MapElement("two")); + assertThat(reloaded.mapElements.get("gamma")).isEqualTo(new MapElement("three")); + assertThat(reloaded.mapElements.get("delta")).isEqualTo(new MapElement("four")); + } + + @Test // GH-1448 + void multipleCollectionsWithEmptySet() { + + MultipleCollections aggregate = new MultipleCollections(); + aggregate.name = "aggregate"; + + aggregate.listElements.add(new ListElement("one")); + aggregate.listElements.add(new ListElement("two")); + aggregate.listElements.add(new ListElement("three")); + + aggregate.mapElements.put("alpha", new MapElement("one")); + aggregate.mapElements.put("beta", new MapElement("two")); + aggregate.mapElements.put("gamma", new MapElement("three")); + aggregate.mapElements.put("delta", new MapElement("four")); + + template.save(aggregate); + + MultipleCollections reloaded = template.findById(aggregate.id, MultipleCollections.class); + + assertThat(reloaded.name).isEqualTo(aggregate.name); + + assertThat(reloaded.listElements).containsExactly(aggregate.listElements.get(0), aggregate.listElements.get(1), + aggregate.listElements.get(2)); + + assertThat(reloaded.setElements).containsExactlyInAnyOrder(aggregate.setElements.toArray(new SetElement[0])); + + assertThat(reloaded.mapElements.get("alpha")).isEqualTo(new MapElement("one")); + assertThat(reloaded.mapElements.get("beta")).isEqualTo(new MapElement("two")); + assertThat(reloaded.mapElements.get("gamma")).isEqualTo(new MapElement("three")); + assertThat(reloaded.mapElements.get("delta")).isEqualTo(new MapElement("four")); + } + + @Test // GH-1448 + void multipleCollectionsWithEmptyList() { + + MultipleCollections aggregate = new MultipleCollections(); + aggregate.name = "aggregate"; + + aggregate.setElements.add(new SetElement("one")); + aggregate.setElements.add(new SetElement("two")); + + aggregate.mapElements.put("alpha", new MapElement("one")); + aggregate.mapElements.put("beta", new MapElement("two")); + aggregate.mapElements.put("gamma", new MapElement("three")); + aggregate.mapElements.put("delta", new MapElement("four")); + + template.save(aggregate); + + MultipleCollections reloaded = template.findById(aggregate.id, MultipleCollections.class); + + assertThat(reloaded.name).isEqualTo(aggregate.name); + + assertThat(reloaded.listElements).containsExactly(); + + assertThat(reloaded.setElements).containsExactlyInAnyOrder(aggregate.setElements.toArray(new SetElement[0])); + + assertThat(reloaded.mapElements.get("alpha")).isEqualTo(new MapElement("one")); + assertThat(reloaded.mapElements.get("beta")).isEqualTo(new MapElement("two")); + assertThat(reloaded.mapElements.get("gamma")).isEqualTo(new MapElement("three")); + assertThat(reloaded.mapElements.get("delta")).isEqualTo(new MapElement("four")); + } + private void saveAndUpdateAggregateWithVersion(VersionedAggregate aggregate, Function toConcreteNumber) { saveAndUpdateAggregateWithVersion(aggregate, toConcreteNumber, 0); @@ -1932,6 +1995,24 @@ static class WithInsertOnly { @InsertOnlyProperty String insertOnly; } + @Table + static class MultipleCollections { + @Id Long id; + String name; + List listElements = new ArrayList<>(); + Set setElements = new HashSet<>(); + Map mapElements = new HashMap<>(); + } + + record ListElement(String name) { + } + + record SetElement(String name) { + } + + record MapElement(String name) { + } + @Configuration @Import(TestConfiguration.class) static class Config { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java index 500c3f1338..30dc8d5860 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java @@ -29,7 +29,6 @@ import java.util.stream.Stream; import org.assertj.core.api.Assertions; -import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -441,7 +440,6 @@ static class ListContainer implements Iterable { this.values = List.of(values); } - @NotNull @Override public Iterator iterator() { return values.iterator(); @@ -460,7 +458,7 @@ public String convert(ListContainer source) { } private static class DummyEntity { - private Long id; + private final Long id; public DummyEntity(Long id) { this.id = id; @@ -488,6 +486,7 @@ public String doSomething() { } } + @Override public Object getRootObject() { return new ExtensionRoot(); } diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql index f086a03b5c..2abc57c279 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql @@ -42,6 +42,11 @@ DROP TABLE WITH_ID_ONLY; DROP TABLE WITH_INSERT_ONLY; +DROP TABLE MULTIPLE_COLLECTIONS; +DROP TABLE MAP_ELEMENT; +DROP TABLE LIST_ELEMENT; +DROP TABLE SET_ELEMENT; + CREATE TABLE LEGO_SET ( "id1" BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, @@ -373,4 +378,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql index a6e5eabad7..1b1da1a1ec 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql @@ -340,4 +340,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID SERIAL PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID SERIAL PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql index dc73899207..b74e716ebd 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql @@ -341,4 +341,30 @@ CREATE TABLE WITH_INSERT_ONLY CREATE TABLE WITH_ID_ONLY ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY -) \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql index 4258e7b438..29f0140d86 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql @@ -314,4 +314,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID BIGINT AUTO_INCREMENT PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql index e9a378f49b..cacd54ed6c 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql @@ -347,4 +347,35 @@ CREATE TABLE WITH_INSERT_ONLY ( ID BIGINT IDENTITY PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +DROP TABLE MULTIPLE_COLLECTIONS; +DROP TABLE MAP_ELEMENT; +DROP TABLE LIST_ELEMENT; +DROP TABLE SET_ELEMENT; + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID BIGINT IDENTITY PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql index 40e32f1692..3efb07a000 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql @@ -319,4 +319,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID BIGINT AUTO_INCREMENT PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql index 5a5c5baf40..c4255de27e 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql @@ -32,6 +32,11 @@ DROP TABLE WITH_LOCAL_DATE_TIME CASCADE CONSTRAINTS PURGE; DROP TABLE WITH_ID_ONLY CASCADE CONSTRAINTS PURGE; DROP TABLE WITH_INSERT_ONLY CASCADE CONSTRAINTS PURGE; +DROP TABLE MULTIPLE_COLLECTIONS CASCADE CONSTRAINTS PURGE; +DROP TABLE MAP_ELEMENT CASCADE CONSTRAINTS PURGE; +DROP TABLE LIST_ELEMENT CASCADE CONSTRAINTS PURGE; +DROP TABLE SET_ELEMENT CASCADE CONSTRAINTS PURGE; + CREATE TABLE LEGO_SET ( "id1" NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY, @@ -354,4 +359,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS NUMBER, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS NUMBER, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS NUMBER, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql index d43b5750b1..e401412e59 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql @@ -35,6 +35,11 @@ DROP TABLE WITH_LOCAL_DATE_TIME; DROP TABLE WITH_ID_ONLY; DROP TABLE WITH_INSERT_ONLY; +DROP TABLE MULTIPLE_COLLECTIONS; +DROP TABLE MAP_ELEMENT; +DROP TABLE LIST_ELEMENT; +DROP TABLE SET_ELEMENT; + CREATE TABLE LEGO_SET ( "id1" SERIAL PRIMARY KEY, @@ -376,4 +381,30 @@ CREATE TABLE WITH_INSERT_ONLY ( ID SERIAL PRIMARY KEY, INSERT_ONLY VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE MULTIPLE_COLLECTIONS +( + ID SERIAL PRIMARY KEY, + NAME VARCHAR(100) +); + +CREATE TABLE SET_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + NAME VARCHAR(100) +); + +CREATE TABLE LIST_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY INT, + NAME VARCHAR(100) +); + +CREATE TABLE MAP_ELEMENT +( + MULTIPLE_COLLECTIONS BIGINT, + MULTIPLE_COLLECTIONS_KEY VARCHAR(10), + NAME VARCHAR(100) +); diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index a60f8e183a..6f1f21e093 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java index aef0f8a3df..e0749185d6 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java @@ -22,8 +22,6 @@ import java.util.List; import java.util.Map; -import org.jetbrains.annotations.NotNull; - import org.springframework.data.r2dbc.convert.R2dbcConverter; import org.springframework.data.r2dbc.core.R2dbcEntityOperations; import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; @@ -157,11 +155,8 @@ private Mono getSpelEvaluator(RelationalParameterA @Override public String toString() { - StringBuffer sb = new StringBuffer(); - sb.append(getClass().getSimpleName()); - sb.append(" [").append(expressionQuery.getQuery()); - sb.append(']'); - return sb.toString(); + String sb = getClass().getSimpleName() + " [" + expressionQuery.getQuery() + ']'; + return sb; } private class ExpandedQuery implements PreparedOperation { @@ -234,7 +229,6 @@ public void bind(String identifier, Object value) { byName.put(identifier, toParameter(value)); } - @NotNull private Parameter toParameter(Object value) { return value instanceof Parameter ? (Parameter) value : Parameter.from(value); } diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 74f350faa8..43c7c0eb38 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-more-than-one-collection diff --git a/spring-data-relational/src/jmh/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorBenchmark.java b/spring-data-relational/src/jmh/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorBenchmark.java index f8a1399b5a..cb49fd72f3 100644 --- a/spring-data-relational/src/jmh/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorBenchmark.java +++ b/spring-data-relational/src/jmh/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorBenchmark.java @@ -38,8 +38,7 @@ public class SingleQuerySqlGeneratorBenchmark extends BenchmarkSettings { @Benchmark public String findAll(StateHolder state) { - return new SingleQuerySqlGenerator(state.context, state.aliasFactory, PostgresDialect.INSTANCE, - state.persistentEntity).findAll(null); + return new SingleQuerySqlGenerator(state.context, state.aliasFactory, PostgresDialect.INSTANCE).findAll(state.persistentEntity, null); } @State(Scope.Benchmark) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java index 3bca90a2e6..3c5b3f2c83 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java @@ -18,7 +18,6 @@ import java.lang.annotation.Annotation; import java.util.Iterator; -import org.jetbrains.annotations.NotNull; import org.springframework.data.mapping.*; import org.springframework.data.mapping.model.PersistentPropertyAccessorFactory; import org.springframework.data.relational.core.sql.SqlIdentifier; @@ -67,7 +66,6 @@ public void addAssociation(Association association @Override public void verify() throws MappingException {} - @NotNull @Override public Iterator iterator() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGenerator.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGenerator.java index 9326a55f1f..5e8c94024c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGenerator.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGenerator.java @@ -20,8 +20,6 @@ import java.util.List; import java.util.Map; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PersistentPropertyPaths; @@ -33,6 +31,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.*; import org.springframework.data.relational.core.sql.render.SqlRenderer; +import org.springframework.lang.Nullable; /** * A {@link SqlGenerator} that creates SQL statements for loading complete aggregates with a single statement. @@ -45,23 +44,20 @@ public class SingleQuerySqlGenerator implements SqlGenerator { private final RelationalMappingContext context; private final Dialect dialect; private final AliasFactory aliases; - private final RelationalPersistentEntity aggregate; - public SingleQuerySqlGenerator(RelationalMappingContext context, AliasFactory aliasFactory, Dialect dialect, - RelationalPersistentEntity aggregate) { + public SingleQuerySqlGenerator(RelationalMappingContext context, AliasFactory aliasFactory, Dialect dialect) { this.context = context; this.aliases = aliasFactory; this.dialect = dialect; - this.aggregate = aggregate; } @Override - public String findAll(@Nullable Condition condition) { - return createSelect(condition); + public String findAll(RelationalPersistentEntity aggregate, @Nullable Condition condition) { + return createSelect(aggregate, condition); } - String createSelect(@Nullable Condition condition) { + String createSelect(RelationalPersistentEntity aggregate, @Nullable Condition condition) { AggregatePath rootPath = context.getAggregatePath(aggregate); QueryMeta queryMeta = createInlineQuery(rootPath, condition); @@ -70,6 +66,7 @@ String createSelect(@Nullable Condition condition) { List rownumbers = new ArrayList<>(); rownumbers.add(queryMeta.rowNumber); + PersistentPropertyPaths entityPaths = context .findPersistentPropertyPaths(aggregate.getType(), PersistentProperty::isEntity); List inlineQueries = createInlineQueries(entityPaths); @@ -83,44 +80,46 @@ String createSelect(@Nullable Condition condition) { columns.add(totalRownumber); InlineQuery inlineQuery = createMainSelect(columns, rootPath, rootQuery, inlineQueries); + Expression rootId = just(aliases.getColumnAlias(rootPath.append(aggregate.getRequiredIdProperty()))); + + List selectList = getSelectList(queryMeta, inlineQueries, rootId); + Select fullQuery = StatementBuilder.select(selectList).from(inlineQuery).orderBy(rootId, just("rn")).build(false); + + return SqlRenderer.create(new RenderContextFactory(dialect).createRenderContext()).render(fullQuery); + } + + private static List getSelectList(QueryMeta queryMeta, List inlineQueries, Expression rootId) { - Expression rootIdExpression = just(aliases.getColumnAlias(rootPath.append(aggregate.getRequiredIdProperty()))); + List expressions = new ArrayList<>(inlineQueries.size() + queryMeta.simpleColumns.size() + 8); - List finalColumns = new ArrayList<>(); queryMeta.simpleColumns - .forEach(e -> finalColumns.add(filteredColumnExpression(queryMeta.rowNumber.toString(), e.toString()))); + .forEach(e -> expressions.add(filteredColumnExpression(queryMeta.rowNumber.toString(), e.toString()))); for (QueryMeta meta : inlineQueries) { + meta.simpleColumns - .forEach(e -> finalColumns.add(filteredColumnExpression(meta.rowNumber.toString(), e.toString()))); + .forEach(e -> expressions.add(filteredColumnExpression(meta.rowNumber.toString(), e.toString()))); + if (meta.id != null) { - finalColumns.add(meta.id); + expressions.add(meta.id); } if (meta.key != null) { - finalColumns.add(meta.key); + expressions.add(meta.key); } } - finalColumns.add(rootIdExpression); - - Select fullQuery = StatementBuilder.select(finalColumns).from(inlineQuery).orderBy(rootIdExpression, just("rn")) - .build(false); - - return SqlRenderer.create(new RenderContextFactory(dialect).createRenderContext()).render(fullQuery); + expressions.add(rootId); + return expressions; } - @NotNull private InlineQuery createMainSelect(List columns, AggregatePath rootPath, InlineQuery rootQuery, List inlineQueries) { SelectBuilder.SelectJoin select = StatementBuilder.select(columns).from(rootQuery); - select = applyJoins(rootPath, inlineQueries, select); - SelectBuilder.BuildSelect buildSelect = applyWhereCondition(rootPath, inlineQueries, select); - Select mainSelect = buildSelect.build(false); - - return InlineQuery.create(mainSelect, "main"); + SelectBuilder.BuildSelect buildSelect = applyWhereCondition(inlineQueries, select); + return InlineQuery.create(buildSelect.build(false), "main"); } /** @@ -159,16 +158,8 @@ private QueryMeta createInlineQuery(AggregatePath basePath, @Nullable Condition RelationalPersistentEntity entity = basePath.getRequiredLeafEntity(); Table table = Table.create(entity.getQualifiedTableName()); - List paths = new ArrayList<>(); - - entity.doWithProperties((RelationalPersistentProperty p) -> { - if (!p.isEntity()) { - paths.add(basePath.append(p)); - } - }); - + List paths = getAggregatePaths(basePath, entity); List columns = new ArrayList<>(); - List columnAliases = new ArrayList<>(); String rowNumberAlias = aliases.getRowNumberAlias(basePath); Expression rownumber = basePath.isRoot() ? new AliasedExpression(SQL.literalOf(1), rowNumberAlias) @@ -180,48 +171,71 @@ private QueryMeta createInlineQuery(AggregatePath basePath, @Nullable Condition : AnalyticFunction.create("count", Expressions.just("*")) .partitionBy(table.column(basePath.getTableInfo().reverseColumnInfo().name())).as(rowCountAlias); columns.add(count); + String backReferenceAlias = null; String keyAlias = null; + if (!basePath.isRoot()) { backReferenceAlias = aliases.getBackReferenceAlias(basePath); columns.add(table.column(basePath.getTableInfo().reverseColumnInfo().name()).as(backReferenceAlias)); - if (basePath.isQualified()) { + keyAlias = aliases.getKeyAlias(basePath); + Expression keyExpression = basePath.isQualified() + ? table.column(basePath.getTableInfo().qualifierColumnInfo().name()).as(keyAlias) + : createRowNumberExpression(basePath, table, keyAlias); + columns.add(keyExpression); + } - keyAlias = aliases.getKeyAlias(basePath); - columns.add(table.column(basePath.getTableInfo().qualifierColumnInfo().name()).as(keyAlias)); - } else { + String id = getIdentifierProperty(paths); + List columnAliases = getColumnAliases(table, paths, columns); + SelectBuilder.SelectWhere select = StatementBuilder.select(columns).from(table); + SelectBuilder.BuildSelect buildSelect = condition != null ? select.where(condition) : select; - String alias = aliases.getColumnAlias(basePath); - columns.add(new AliasedExpression(just("1"), alias)); - columnAliases.add(just(alias)); - } - } - String id = null; + InlineQuery inlineQuery = InlineQuery.create(buildSelect.build(false), aliases.getTableAlias(basePath)); + return QueryMeta.of(basePath, inlineQuery, columnAliases, just(id), just(backReferenceAlias), just(keyAlias), + just(rowNumberAlias), just(rowCountAlias)); + } + + private List getColumnAliases(Table table, List paths, List columns) { + List columnAliases = new ArrayList<>(); for (AggregatePath path : paths) { String alias = aliases.getColumnAlias(path); - if (path.getRequiredLeafProperty().isIdProperty()) { - id = alias; - } else { + if (!path.getRequiredLeafProperty().isIdProperty()) { columnAliases.add(just(alias)); } columns.add(table.column(path.getColumnInfo().name()).as(alias)); } + return columnAliases; + } - SelectBuilder.SelectWhere select = StatementBuilder.select(columns).from(table); + private static List getAggregatePaths(AggregatePath basePath, RelationalPersistentEntity entity) { - SelectBuilder.BuildSelect buildSelect = condition != null ? select.where(condition) : select; + List paths = new ArrayList<>(); - InlineQuery inlineQuery = InlineQuery.create(buildSelect.build(false), - aliases.getTableAlias(context.getAggregatePath(entity))); - return QueryMeta.of(basePath, inlineQuery, columnAliases, just(id), just(backReferenceAlias), just(keyAlias), - just(rowNumberAlias), just(rowCountAlias)); + for (RelationalPersistentProperty property : entity) { + if (!property.isEntity()) { + paths.add(basePath.append(property)); + } + } + + return paths; + } + + @Nullable + private String getIdentifierProperty(List paths) { + + for (AggregatePath path : paths) { + if (path.getRequiredLeafProperty().isIdProperty()) { + return aliases.getColumnAlias(path); + } + } + + return null; } - @NotNull private static AnalyticFunction createRowNumberExpression(AggregatePath basePath, Table table, String rowNumberAlias) { return AnalyticFunction.create("row_number") // @@ -252,6 +266,7 @@ private SelectBuilder.SelectJoin applyJoins(AggregatePath rootPath, List * * - * @param rootPath path to the root entity that gets selected. * @param inlineQueries all in the inline queries for all the children, as returned by * {@link #createInlineQueries(PersistentPropertyPaths)} * @param select the select to which the where clause gets added. * @return the modified select. */ - private SelectBuilder.SelectOrdered applyWhereCondition(AggregatePath rootPath, List inlineQueries, + private SelectBuilder.SelectOrdered applyWhereCondition(List inlineQueries, SelectBuilder.SelectJoin select) { - SelectBuilder.SelectWhereAndOr selectWhere = null; - for (QueryMeta queryMeta : inlineQueries) { + SelectBuilder.SelectWhere selectWhere = (SelectBuilder.SelectWhere) select; + + if (inlineQueries.isEmpty()) { + return selectWhere; + } + + Condition joins = null; - AggregatePath path = queryMeta.basePath; - Expression childRowNumber = just(aliases.getRowNumberAlias(path)); - Condition pseudoJoinCondition = Conditions.isNull(childRowNumber) - .or(Conditions.isEqual(childRowNumber, Expressions.just(aliases.getRowNumberAlias(rootPath)))) - .or(Conditions.isGreater(childRowNumber, Expressions.just(aliases.getRowCountAlias(rootPath)))); + for (int left = 0; left < inlineQueries.size(); left++) { - selectWhere = ((SelectBuilder.SelectWhere) select).where(pseudoJoinCondition); + QueryMeta leftQueryMeta = inlineQueries.get(left); + AggregatePath leftPath = leftQueryMeta.basePath; + Expression leftRowNumber = just(aliases.getRowNumberAlias(leftPath)); + Expression leftRowCount = just(aliases.getRowCountAlias(leftPath)); + + for (int right = left + 1; right < inlineQueries.size(); right++) { + + QueryMeta rightQueryMeta = inlineQueries.get(right); + AggregatePath rightPath = rightQueryMeta.basePath; + Expression rightRowNumber = just(aliases.getRowNumberAlias(rightPath)); + Expression rightRowCount = just(aliases.getRowCountAlias(rightPath)); + + Condition mutualJoin = Conditions.isEqual(leftRowNumber, rightRowNumber).or(Conditions.isNull(leftRowNumber)) + .or(Conditions.isNull(rightRowNumber)) + .or(Conditions.nest(Conditions.isGreater(leftRowNumber, rightRowCount) + .and(Conditions.isEqual(rightRowNumber, SQL.literalOf(1))))) + .or(Conditions.nest(Conditions.isGreater(rightRowNumber, leftRowCount) + .and(Conditions.isEqual(leftRowNumber, SQL.literalOf(1))))); + + mutualJoin = Conditions.nest(mutualJoin); + + if (joins == null) { + joins = mutualJoin; + } else { + joins = joins.and(mutualJoin); + } + } } - return selectWhere == null ? (SelectBuilder.SelectOrdered) select : selectWhere; + return selectWhere.where(joins); } @Override @@ -398,6 +439,5 @@ static QueryMeta of(AggregatePath basePath, InlineQuery inlineQuery, Collection< return new QueryMeta(basePath, inlineQuery, simpleColumns, selectableExpressions, id, backReference, key, rowNumber, rowCount); } - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SqlGenerator.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SqlGenerator.java index fe783882a5..38f2827d77 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SqlGenerator.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sqlgeneration/SqlGenerator.java @@ -15,6 +15,7 @@ */ package org.springframework.data.relational.core.sqlgeneration; +import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.sql.Condition; import org.springframework.lang.Nullable; @@ -26,11 +27,11 @@ */ public interface SqlGenerator { - default String findAll() { - return findAll(null); + default String findAll(RelationalPersistentEntity aggregate) { + return findAll(aggregate, null); } - String findAll(@Nullable Condition condition); + String findAll(RelationalPersistentEntity aggregate, @Nullable Condition condition); AliasFactory getAliasFactory(); } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/AliasFactoryUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/AliasFactoryUnitTests.java index 5f0f988769..54a4b0eeb0 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/AliasFactoryUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/AliasFactoryUnitTests.java @@ -34,7 +34,7 @@ class AliasFactoryUnitTests { @Nested class SimpleAlias { - @Test + @Test // GH-1446 void aliasForRoot() { String alias = aliasFactory @@ -43,7 +43,7 @@ void aliasForRoot() { assertThat(alias).isEqualTo("c_dummy_entity_1"); } - @Test + @Test // GH-1446 void aliasSimpleProperty() { String alias = aliasFactory @@ -52,8 +52,8 @@ void aliasSimpleProperty() { assertThat(alias).isEqualTo("c_name_1"); } - @Test - void nameGetsSanatized() { + @Test // GH-1446 + void nameGetsSanitized() { String alias = aliasFactory.getColumnAlias( context.getAggregatePath( context.getPersistentPropertyPath("evil", DummyEntity.class))); @@ -61,7 +61,7 @@ void nameGetsSanatized() { assertThat(alias).isEqualTo("c_ameannamecontains3illegal_characters_1"); } - @Test + @Test // GH-1446 void aliasIsStable() { String alias1 = aliasFactory.getColumnAlias( @@ -76,7 +76,7 @@ void aliasIsStable() { @Nested class RnAlias { - @Test + @Test // GH-1446 void aliasIsStable() { String alias1 = aliasFactory.getRowNumberAlias( @@ -87,7 +87,7 @@ void aliasIsStable() { assertThat(alias1).isEqualTo(alias2); } - @Test + @Test // GH-1446 void aliasProjectsOnTableReferencingPath() { String alias1 = aliasFactory.getRowNumberAlias( @@ -99,7 +99,7 @@ void aliasProjectsOnTableReferencingPath() { assertThat(alias1).isEqualTo(alias2); } - @Test + @Test // GH-1446 void rnAliasIsIndependentOfTableAlias() { String alias1 = aliasFactory.getRowNumberAlias( @@ -114,7 +114,7 @@ void rnAliasIsIndependentOfTableAlias() { @Nested class BackReferenceAlias { - @Test + @Test // GH-1446 void testBackReferenceAlias() { String alias = aliasFactory.getBackReferenceAlias( @@ -126,7 +126,7 @@ void testBackReferenceAlias() { @Nested class KeyAlias { - @Test + @Test // GH-1446 void testKeyAlias() { String alias = aliasFactory.getKeyAlias( @@ -136,6 +136,21 @@ void testKeyAlias() { } } + @Nested + class TableAlias { + @Test // GH-1448 + void tableAliasIsDifferentForDifferentPathsToSameEntity() { + + String alias = aliasFactory.getTableAlias( + context.getAggregatePath(context.getPersistentPropertyPath("dummy", Reference.class))); + + String alias2 = aliasFactory.getTableAlias( + context.getAggregatePath(context.getPersistentPropertyPath("dummy2", Reference.class))); + + assertThat(alias).isNotEqualTo(alias2); + } + } + static class DummyEntity { String name; @@ -144,5 +159,6 @@ static class DummyEntity { static class Reference { DummyEntity dummy; + DummyEntity dummy2; } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorUnitTests.java index ade6e0dad1..6185d30a6d 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sqlgeneration/SingleQuerySqlGeneratorUnitTests.java @@ -52,7 +52,7 @@ class TrivialAggregateWithoutReferences extends AbstractTestFixture { @Test // GH-1446 void createSelectForFindAll() { - String sql = sqlGenerator.findAll(); + String sql = sqlGenerator.findAll(persistentEntity); SqlAssert fullSelect = assertThatParsed(sql); fullSelect.extractOrderBy().isEqualTo(alias("id") + ", rn"); @@ -79,7 +79,7 @@ void createSelectForFindAll() { void createSelectForFindById() { Table table = Table.create(persistentEntity.getQualifiedTableName()); - String sql = sqlGenerator.findAll(table.column("id").isEqualTo(Conditions.just(":id"))); + String sql = sqlGenerator.findAll(persistentEntity, table.column("id").isEqualTo(Conditions.just(":id"))); SqlAssert baseSelect = assertThatParsed(sql).hasInlineView(); @@ -104,7 +104,7 @@ void createSelectForFindById() { void createSelectForFindAllById() { Table table = Table.create(persistentEntity.getQualifiedTableName()); - String sql = sqlGenerator.findAll(table.column("id").in(Conditions.just(":ids"))); + String sql = sqlGenerator.findAll(persistentEntity, table.column("id").in(Conditions.just(":ids"))); SqlAssert baseSelect = assertThatParsed(sql).hasInlineView(); @@ -138,7 +138,7 @@ private AggregateWithSingleReference() { void createSelectForFindById() { Table table = Table.create(persistentEntity.getQualifiedTableName()); - String sql = sqlGenerator.findAll(table.column("id").isEqualTo(Conditions.just(":id"))); + String sql = sqlGenerator.findAll(persistentEntity, table.column("id").isEqualTo(Conditions.just(":id"))); String rootRowNumber = rnAlias(); String rootCount = rcAlias(); @@ -162,10 +162,7 @@ void createSelectForFindById() { col(backref), // col(keyAlias) // ).extractWhereClause() // - .doesNotContainIgnoringCase("and") // - .containsIgnoringCase(trivialsRowNumber + " is null") // - .containsIgnoringCase(trivialsRowNumber + " = " + rootRowNumber) // - .containsIgnoringCase(trivialsRowNumber + " > " + rootCount); + .isEqualTo(""); baseSelect.hasInlineViewSelectingFrom("\"single_reference_aggregate\"") // .hasExactlyColumns( // lit(1).as(rnAlias()), lit(1).as(rootCount), // @@ -218,7 +215,7 @@ private AbstractTestFixture(Class aggregateRootType) { this.aggregateRootType = aggregateRootType; this.persistentEntity = context.getRequiredPersistentEntity(aggregateRootType); - this.sqlGenerator = new SingleQuerySqlGenerator(context, new AliasFactory(), dialect, persistentEntity); + this.sqlGenerator = new SingleQuerySqlGenerator(context, new AliasFactory(), dialect); this.aliases = sqlGenerator.getAliasFactory(); } diff --git a/src/main/antora/modules/ROOT/pages/jdbc/entity-persistence.adoc b/src/main/antora/modules/ROOT/pages/jdbc/entity-persistence.adoc index 4e3449f8a9..08b0420d3a 100644 --- a/src/main/antora/modules/ROOT/pages/jdbc/entity-persistence.adoc +++ b/src/main/antora/modules/ROOT/pages/jdbc/entity-persistence.adoc @@ -28,11 +28,11 @@ If the aggregate root references other entities those are loaded with separate s With this an arbitrary number of aggregates can be fully loaded with a single SQL query. This should be significant more efficient, especially for complex aggregates, consisting of many entities. + -Currently, Single Query Loading is restricted to: +Currently, Single Query Loading is restricted in different ways: -1. It only works for aggregates that only reference one entity collection.The plan is to remove this constraint in the future. +1. The aggregate must not have nested collections, this includes `Map`.The plan is to remove this constraint in the future. -2. The aggregate must also not use `AggregateReference` or embedded entities.The plan is to remove this constraint in the future. +2. The aggregate must not use `AggregateReference` or embedded entities.The plan is to remove this constraint in the future. 3. The database dialect must support it.Of the dialects provided by Spring Data JDBC all but H2 and HSQL support this.H2 and HSQL don't support analytic functions (aka windowing functions). @@ -40,6 +40,8 @@ Currently, Single Query Loading is restricted to: 5. Single Query Loading needs to be enabled in the `JdbcMappingContext`, by calling `setSingleQueryLoadingEnabled(true)` +If any condition is not fulfilled Spring Data JDBC falls back to the default approach of loading aggregates. + NOTE: Single Query Loading is to be considered experimental. We appreciate feedback on how it works for you.