From 4ebf325e4ee5fd492f305dc1be8e6907d0ff72e1 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 18 Oct 2024 13:48:37 +0200 Subject: [PATCH 1/2] prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index db989ba7a3..2cb442fe7d 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 84eeb178e0..d57e061227 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.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 266af71b96..db07906735 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index e335b56501..b016a64d37 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 6be1155d38..df4ef94762 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1907-partial-insert-immutables-SNAPSHOT From 0fcf49684de3d94597c35eb072f8467f7b7385fe Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 18 Oct 2024 14:49:53 +0200 Subject: [PATCH 2/2] Fix id setting for partial updates of collections of immutables. We gather immutable entities of which the id has changed, in order to set them as values in the parent entity. We now also gather unchanged entities. So they get set with the changed one in the parent. Closes #1907 --- .../JdbcAggregateChangeExecutionContext.java | 63 ++++++++++------ ...bcRepositoryWithListsIntegrationTests.java | 71 ++++++------------- .../relational/core/conversion/DbAction.java | 13 ++-- 3 files changed, 72 insertions(+), 75 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 23647874f9..df87d3813d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -15,16 +15,7 @@ */ package org.springframework.data.jdbc.core; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.function.BiConsumer; import java.util.stream.Collectors; @@ -241,7 +232,7 @@ private Object getIdFrom(DbAction.WithEntity idOwningAction) { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(idOwningAction.getEntityType()); Object identifier = persistentEntity.getIdentifierAccessor(idOwningAction.getEntity()).getIdentifier(); - Assert.state(identifier != null,() -> "Couldn't obtain a required id value for " + persistentEntity); + Assert.state(identifier != null, () -> "Couldn't obtain a required id value for " + persistentEntity); return identifier; } @@ -268,12 +259,22 @@ List populateIdsIfNecessary() { } // the id property was immutable, so we have to propagate changes up the tree - if (newEntity != action.getEntity() && action instanceof DbAction.Insert insert) { + if (action instanceof DbAction.Insert insert) { Pair qualifier = insert.getQualifier(); + Object qualifierValue = qualifier == null ? null : qualifier.getSecond(); - cascadingValues.stage(insert.getDependingOn(), insert.getPropertyPath(), - qualifier == null ? null : qualifier.getSecond(), newEntity); + if (newEntity != action.getEntity()) { + + cascadingValues.stage(insert.getDependingOn(), insert.getPropertyPath(), + qualifierValue, newEntity); + + } else if (insert.getPropertyPath().getLeafProperty().isCollectionLike()) { + + cascadingValues.gather(insert.getDependingOn(), insert.getPropertyPath(), + qualifierValue, newEntity); + + } } } @@ -360,7 +361,7 @@ private static class StagedValues { static final List aggregators = Arrays.asList(SetAggregator.INSTANCE, MapAggregator.INSTANCE, ListAggregator.INSTANCE, SingleElementAggregator.INSTANCE); - Map> values = new HashMap<>(); + Map> values = new HashMap<>(); /** * Adds a value that needs to be set in an entity higher up in the tree of entities in the aggregate. If the @@ -375,18 +376,26 @@ private static class StagedValues { */ @SuppressWarnings("unchecked") void stage(DbAction action, PersistentPropertyPath path, @Nullable Object qualifier, Object value) { + gather(action, path, qualifier, value); + values.get(action).get(path).isStaged = true; + } + + void gather(DbAction action, PersistentPropertyPath path, @Nullable Object qualifier, Object value) { MultiValueAggregator aggregator = getAggregatorFor(path); - Map valuesForPath = this.values.computeIfAbsent(action, + Map valuesForPath = this.values.computeIfAbsent(action, dbAction -> new HashMap<>()); - T currentValue = (T) valuesForPath.computeIfAbsent(path, - persistentPropertyPath -> aggregator.createEmptyInstance()); + StagedValue stagedValue = valuesForPath.computeIfAbsent(path, + persistentPropertyPath -> new StagedValue(aggregator.createEmptyInstance())); + T currentValue = (T) stagedValue.value; Object newValue = aggregator.add(currentValue, qualifier, value); - valuesForPath.put(path, newValue); + stagedValue.value = newValue; + + valuesForPath.put(path, stagedValue); } private MultiValueAggregator getAggregatorFor(PersistentPropertyPath path) { @@ -408,7 +417,21 @@ private MultiValueAggregator getAggregatorFor(PersistentPropertyPath path) { * property. */ void forEachPath(DbAction dbAction, BiConsumer action) { - values.getOrDefault(dbAction, Collections.emptyMap()).forEach(action); + values.getOrDefault(dbAction, Collections.emptyMap()).forEach((persistentPropertyPath, stagedValue) -> { + if (stagedValue.isStaged) { + action.accept(persistentPropertyPath, stagedValue.value); + } + }); + } + + } + + private static class StagedValue { + Object value; + boolean isStaged; + + public StagedValue(Object value) { + this.value = value; } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithListsIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithListsIntegrationTests.java index 4ab222fdb7..78b33fa65b 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithListsIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithListsIntegrationTests.java @@ -32,6 +32,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.PersistenceCreator; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; import org.springframework.data.jdbc.testing.EnabledOnFeature; import org.springframework.data.jdbc.testing.IntegrationTest; @@ -55,8 +56,7 @@ public class JdbcRepositoryWithListsIntegrationTests { private static DummyEntity createDummyEntity() { - DummyEntity entity = new DummyEntity(); - entity.setName("Entity Name"); + DummyEntity entity = new DummyEntity(null, "Entity Name", new ArrayList<>()); return entity; } @@ -94,7 +94,7 @@ public void saveAndLoadNonEmptyList() { assertThat(reloaded.content) // .isNotNull() // .extracting(e -> e.id) // - .containsExactlyInAnyOrder(element1.id, element2.id); + .containsExactlyInAnyOrder(entity.content.get(0).id, entity.content.get(1).id); } @Test // GH-1159 @@ -147,9 +147,9 @@ public void findAllLoadsList() { @EnabledOnFeature(SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES) public void updateList() { - Element element1 = createElement("one"); - Element element2 = createElement("two"); - Element element3 = createElement("three"); + Element element1 = new Element("one"); + Element element2 = new Element("two"); + Element element3 = new Element("three"); DummyEntity entity = createDummyEntity(); entity.content.add(element1); @@ -157,14 +157,15 @@ public void updateList() { entity = repository.save(entity); - entity.content.remove(element1); - element2.content = "two changed"; + entity.content.remove(0); + entity.content.set(0, new Element(entity.content.get(0).id, "two changed")); entity.content.add(element3); entity = repository.save(entity); assertThat(entity.id).isNotNull(); assertThat(entity.content).allMatch(v -> v.id != null); + assertThat(entity.content).hasSize(2); DummyEntity reloaded = repository.findById(entity.id).orElseThrow(AssertionFailedError::new); @@ -175,8 +176,8 @@ public void updateList() { assertThat(reloaded.content) // .extracting(e -> e.id, e -> e.content) // .containsExactly( // - tuple(element2.id, "two changed"), // - tuple(element3.id, "three") // + tuple(entity.content.get(0).id, "two changed"), // + tuple(entity.content.get(1).id, "three") // ); Long count = template.queryForObject("SELECT count(1) FROM Element", new HashMap<>(), Long.class); @@ -186,8 +187,8 @@ public void updateList() { @Test // DATAJDBC-130 public void deletingWithList() { - Element element1 = createElement("one"); - Element element2 = createElement("two"); + Element element1 = new Element("one"); + Element element2 = new Element("two"); DummyEntity entity = createDummyEntity(); entity.content.add(element1); @@ -203,13 +204,6 @@ public void deletingWithList() { assertThat(count).isEqualTo(0); } - private Element createElement(String content) { - - Element element = new Element(); - element.content = content; - return element; - } - interface DummyEntityRepository extends CrudRepository {} interface RootRepository extends CrudRepository {} @@ -229,43 +223,22 @@ RootRepository rootRepository(JdbcRepositoryFactory factory) { } } - static class DummyEntity { + record DummyEntity(@Id Long id, String name, List content) { + } - String name; - List content = new ArrayList<>(); - @Id private Long id; + record Element(@Id Long id, String content) { - public String getName() { - return this.name; - } + @PersistenceCreator + Element {} - public List getContent() { - return this.content; + Element() { + this(null, null); } - public Long getId() { - return this.id; + Element(String content) { + this(null, content); } - public void setName(String name) { - this.name = name; - } - - public void setContent(List content) { - this.content = content; - } - - public void setId(Long id) { - this.id = id; - } - } - - static class Element { - - String content; - @Id private Long id; - - public Element() {} } static class Root { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index 28699c3bda..1ccca7355e 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -15,9 +15,12 @@ */ package org.springframework.data.relational.core.conversion; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.function.Function; import org.springframework.data.mapping.PersistentPropertyPath; @@ -479,15 +482,13 @@ interface WithDependingOn extends WithPropertyPath, WithEntity { default Pair, Object> getQualifier() { Map, Object> qualifiers = getQualifiers(); - if (qualifiers.isEmpty()) + if (qualifiers.isEmpty()) { return null; - - if (qualifiers.size() > 1) { - throw new IllegalStateException("Can't handle more then one qualifier"); } - Map.Entry, Object> entry = qualifiers.entrySet().iterator() - .next(); + Set, Object>> entries = qualifiers.entrySet(); + Map.Entry, Object> entry = entries.stream().sorted(Comparator.comparing(e -> -e.getKey().getLength())).findFirst().get(); + if (entry.getValue() == null) { return null; }