Spring/spring-data-neo4j
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-2340 - Clear exception if mapping recursive immutable objects.

Browse Source 
Also improving the documentation around this topic.

Closes #2340
This commit is contained in:
Gerrit Meier
2021-07-28 17:37:46 +02:00
parent 18bdacfa00
commit 8dee56b242
4 changed files with 102 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/main/asciidoc/object-mapping/sdc-object-mapping.adoc
View File

@@ -222,6 +222,16 @@ It's an established pattern to rather use static factory methods to expose these
* _Use Lombok to avoid boilerplate code_ --
As persistence operations usually require a constructor taking all arguments, their declaration becomes a tedious repetition of boilerplate parameter to field assignments that can best be avoided by using Lombok's `@AllArgsConstructor`.
[[mapping.fundamentals.recommendations.note-immutable]]
=== A note on immutable mapping
Although we recommend to use immutable mapping and constructs wherever possible, there are some limitations when it comes to mapping.
Given a bidirectional relationship where `A` has a constructor reference to `B` and `B` has a reference to `A`, or a more complex scenario.
This hen/egg situation is not solvable for Spring Data Neo4j.
During the instantiation of `A` it eagerly needs to have a fully instantiated `B`, which on the other hand requires an instance (to be precise, the _same_ instance) of `A`.
SDN allows such models in general, but will throw a `MappingException` at runtime if the data that gets returned from the database contains such constellation as described above.
In such cases or scenarios, where you cannot foresee what the data that gets returned looks like, you are better suited with a mutable field for the relationships.
[[mapping.fundamentals.kotlin]]
== Kotlin support

48
src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java
View File

@@ -246,6 +246,15 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter {
Long internalId = getInternalId(queryResult);
Supplier<Object> mappedObjectSupplier = () -> {
if (knownObjects.isInCreation(internalId)) {
throw new MappingException(
String.format(
"The node with id %s has a logical cyclic mapping dependency. " +
"Its creation caused the creation of another node that has a reference to this.",
internalId)
);
}
knownObjects.setInCreation(internalId);
List<String> allLabels = getLabels(queryResult, nodeDescription);
NodeDescriptionAndLabels nodeDescriptionAndLabels = nodeDescriptionStore
@@ -261,6 +270,7 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter {
ET instance = instantiate(concreteNodeDescription, queryResult, allValues, relationships,
nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity);
knownObjects.removeFromInCreation(internalId);
PersistentPropertyAccessor<ET> propertyAccessor = concreteNodeDescription.getPropertyAccessor(instance);
if (concreteNodeDescription.requiresPropertyPopulation()) {
@@ -588,6 +598,7 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter {
private final Lock write = lock.writeLock();
private final Map<Long, Object> internalIdStore = new HashMap<>();
private final Set<Long> idsInCreation = new HashSet<>();
private void storeObject(@Nullable Long internalId, Object object) {
if (internalId == null) {
@@ -595,12 +606,37 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter {
}
try {
write.lock();
idsInCreation.remove(internalId);
internalIdStore.put(internalId, object);
} finally {
write.unlock();
}
}
private void setInCreation(@Nullable Long internalId) {
if (internalId == null) {
return;
}
try {
write.lock();
idsInCreation.add(internalId);
} finally {
write.unlock();
}
}
private boolean isInCreation(@Nullable Long internalId) {
if (internalId == null) {
return false;
}
try {
read.lock();
return idsInCreation.contains(internalId);
} finally {
read.unlock();
}
}
@Nullable
private Object getObject(@Nullable Long internalId) {
if (internalId == null) {
@@ -621,5 +657,17 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter {
}
return null;
}
private void removeFromInCreation(@Nullable Long internalId) {
if (internalId == null) {
return;
}
try {
write.lock();
idsInCreation.remove(internalId);
} finally {
write.unlock();
}
}
}
}

16
src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java
View File

@@ -18,6 +18,7 @@ package org.springframework.data.neo4j.integration.imperative;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.tuple;
import java.time.LocalDate;
@@ -74,6 +75,7 @@ import org.springframework.data.geo.Circle;
import org.springframework.data.geo.Distance;
import org.springframework.data.geo.Metrics;
import org.springframework.data.geo.Polygon;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
import org.springframework.data.neo4j.core.DatabaseSelection;
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
@@ -993,7 +995,7 @@ class RepositoryIT {
}
@Test
void findEntityWithBidirectionalRelationship(@Autowired BidirectionalStartRepository repository) {
void findEntityWithBidirectionalRelationshipInConstructorThrowsException(@Autowired BidirectionalStartRepository repository) {
long startId;
@@ -1007,14 +1009,10 @@ class RepositoryIT {
startId = startNode.id();
}
Optional<BidirectionalStart> entityOptional = repository.findById(startId);
assertThat(entityOptional).isPresent();
BidirectionalStart entity = entityOptional.get();
assertThat(entity.getEnds()).hasSize(1);
BidirectionalEnd end = entity.getEnds().iterator().next();
assertThat(end.getAnotherStart()).isNotNull();
assertThat(end.getAnotherStart().getName()).isEqualTo("Elmo");
assertThatThrownBy(() -> repository.findById(startId))
.hasRootCauseMessage("The node with id " + startId + " has a logical cyclic mapping dependency. " +
"Its creation caused the creation of another node that has a reference to this.")
.hasRootCauseInstanceOf(MappingException.class);
}

69
src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java
View File

@@ -15,35 +15,6 @@
*/
package org.springframework.data.neo4j.integration.reactive;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.tuple;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalAssignedId;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalExternallyGeneratedId;
import org.springframework.data.neo4j.integration.shared.common.DtoPersonProjection;
import org.springframework.data.neo4j.integration.shared.common.EntitiesWithDynamicLabels;
import org.springframework.data.neo4j.integration.shared.common.SimplePerson;
import org.springframework.data.neo4j.integration.shared.common.ThingWithFixedGeneratedId;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Tag;
@@ -69,6 +40,7 @@ import org.springframework.data.domain.Example;
import org.springframework.data.domain.ExampleMatcher;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Sort;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig;
import org.springframework.data.neo4j.core.DatabaseSelection;
import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider;
@@ -79,10 +51,14 @@ import org.springframework.data.neo4j.integration.shared.common.AltHobby;
import org.springframework.data.neo4j.integration.shared.common.AltLikedByPersonRelationship;
import org.springframework.data.neo4j.integration.shared.common.AltPerson;
import org.springframework.data.neo4j.integration.shared.common.AnotherThingWithAssignedId;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalAssignedId;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalEnd;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalExternallyGeneratedId;
import org.springframework.data.neo4j.integration.shared.common.BidirectionalStart;
import org.springframework.data.neo4j.integration.shared.common.Club;
import org.springframework.data.neo4j.integration.shared.common.DeepRelationships;
import org.springframework.data.neo4j.integration.shared.common.DtoPersonProjection;
import org.springframework.data.neo4j.integration.shared.common.EntitiesWithDynamicLabels;
import org.springframework.data.neo4j.integration.shared.common.EntityWithConvertedId;
import org.springframework.data.neo4j.integration.shared.common.Hobby;
import org.springframework.data.neo4j.integration.shared.common.ImmutablePerson;
@@ -94,7 +70,9 @@ import org.springframework.data.neo4j.integration.shared.common.PersonWithRelati
import org.springframework.data.neo4j.integration.shared.common.PersonWithRelationshipWithProperties;
import org.springframework.data.neo4j.integration.shared.common.Pet;
import org.springframework.data.neo4j.integration.shared.common.SimilarThing;
import org.springframework.data.neo4j.integration.shared.common.SimplePerson;
import org.springframework.data.neo4j.integration.shared.common.ThingWithAssignedId;
import org.springframework.data.neo4j.integration.shared.common.ThingWithFixedGeneratedId;
import org.springframework.data.neo4j.integration.shared.common.ThingWithGeneratedId;
import org.springframework.data.neo4j.integration.shared.common.WorksInClubRelationship;
import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository;
@@ -110,6 +88,28 @@ import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
import org.springframework.transaction.ReactiveTransactionManager;
import org.springframework.transaction.annotation.EnableTransactionManagement;
import org.springframework.transaction.reactive.TransactionalOperator;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.tuple;
/**
* @author Gerrit Meier
@@ -869,9 +869,14 @@ class ReactiveRepositoryIT {
startId = startNode.id();
}
StepVerifier.create(repository.findById(startId)).assertNext(entity -> {
assertThat(entity.getEnds()).hasSize(1);
}).verifyComplete();
StepVerifier.create(repository.findById(startId))
.verifyErrorMatches(error -> {
Throwable cause = error.getCause();
return cause instanceof MappingException && cause.getMessage().equals(
"The node with id " + startId + " has a logical cyclic mapping dependency. " +
"Its creation caused the creation of another node that has a reference to this.");
});
}
@Test