From cf9b480ea28b3149029e8fe2dda7dfab2abb19b4 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 18 Sep 2019 14:34:48 +0200 Subject: [PATCH] DATAJDBC-417 - Fixed saving an entity containing a null embeddable with a reference to a further entity. Constructing the DbActions assumed that the parent of a path exists (i.e. is not null) and created parent nodes. Original pull request: #169. --- .../core/conversion/WritingContext.java | 15 ++++- .../RelationalEntityWriterUnitTests.java | 65 ++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java index faca7bd19..ece16ad04 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java @@ -202,7 +202,8 @@ class WritingContext { } else { - List pathNodes = nodesCache.get(path.getParentPath()); + List pathNodes = nodesCache.getOrDefault(path.getParentPath(), Collections.emptyList()); + pathNodes.forEach(parentNode -> { // todo: this should go into pathnode @@ -238,7 +239,17 @@ class WritingContext { @Nullable private Object getFromRootValue(PersistentPropertyPath path) { - return path.getBaseProperty().getOwner().getPropertyAccessor(entity).getProperty(path); + + if (path.getLength() == 0) + return entity; + + Object parent = getFromRootValue(path.getParentPath()); + if (parent == null) { + return null; + } + + return context.getRequiredPersistentEntity(parent.getClass()).getPropertyAccessor(parent) + .getProperty(path.getRequiredLeafProperty()); } private List createNodes(PersistentPropertyPath path, diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java index ff381b630..d66044d7d 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java @@ -29,7 +29,6 @@ import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; - import org.springframework.data.annotation.Id; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PersistentPropertyPaths; @@ -118,6 +117,7 @@ public class RelationalEntityWriterUnitTests { ); } + @Test // DATAJDBC-112 public void newEntityWithReferenceGetsConvertedToTwoInserts() { @@ -530,6 +530,51 @@ public class RelationalEntityWriterUnitTests { ); } + @Test // DATAJDBC-417 + public void savingANullEmbeddedWithEntity() { + + EmbeddedReferenceChainEntity entity = new EmbeddedReferenceChainEntity(null); + // the embedded is null !!! + + AggregateChange aggregateChange = // + new AggregateChange<>(Kind.SAVE, EmbeddedReferenceChainEntity.class, entity); + + converter.write(entity, aggregateChange); + + assertThat(aggregateChange.getActions()) // + .extracting(DbAction::getClass, // + DbAction::getEntityType, // + DbActionTestSupport::extractPath, // + DbActionTestSupport::actualEntityType, // + DbActionTestSupport::isWithDependsOn) // + .containsExactly( // + tuple(InsertRoot.class, EmbeddedReferenceChainEntity.class, "", EmbeddedReferenceChainEntity.class, false) // + ); + } + @Test // DATAJDBC-417 + public void savingInnerNullEmbeddedWithEntity() { + + RootWithEmbeddedReferenceChainEntity root = new RootWithEmbeddedReferenceChainEntity(null); + root.other = new EmbeddedReferenceChainEntity(null); + // the embedded is null !!! + + AggregateChange aggregateChange = // + new AggregateChange<>(Kind.SAVE, RootWithEmbeddedReferenceChainEntity.class, root); + + converter.write(root, aggregateChange); + + assertThat(aggregateChange.getActions()) // + .extracting(DbAction::getClass, // + DbAction::getEntityType, // + DbActionTestSupport::extractPath, // + DbActionTestSupport::actualEntityType, // + DbActionTestSupport::isWithDependsOn) // + .containsExactly( // + tuple(InsertRoot.class, RootWithEmbeddedReferenceChainEntity.class, "", RootWithEmbeddedReferenceChainEntity.class, false), // + tuple(Insert.class, EmbeddedReferenceChainEntity.class, "other", EmbeddedReferenceChainEntity.class, true) // + ); + } + private CascadingReferenceMiddleElement createMiddleElement(Element first, Element second) { CascadingReferenceMiddleElement middleElement1 = new CascadingReferenceMiddleElement(null); @@ -585,6 +630,19 @@ public class RelationalEntityWriterUnitTests { @Embedded(onEmpty = OnEmpty.USE_NULL, prefix = "prefix_") Element other; } + @RequiredArgsConstructor + static class EmbeddedReferenceChainEntity { + + @Id final Long id; + @Embedded(onEmpty = OnEmpty.USE_NULL, prefix = "prefix_") ElementReference other; + } + @RequiredArgsConstructor + static class RootWithEmbeddedReferenceChainEntity { + + @Id final Long id; + EmbeddedReferenceChainEntity other; + } + @RequiredArgsConstructor static class ReferenceWoIdEntity { @@ -641,6 +699,11 @@ public class RelationalEntityWriterUnitTests { @Id final Long id; } + @RequiredArgsConstructor + private static class ElementReference { + final Element element; + } + @RequiredArgsConstructor private static class NoIdListMapContainer {