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

GH-2223 - Consider created objects as processed.

Browse Source 
onSave immutable entities will get re-created if they have generated
ids. In those cases if a node gets referenced more than once (e.g.
A->B->C and A->C) the resulting entities will not be considered as the
same because one has already an identifier set, the other not yet.
In thoses cases the logic either persisted a duplicate or it could not
find the already processed entity because the identifier of the entity
in question is still `null`.

Closes #2223
This commit is contained in:
Gerrit Meier
2021-04-20 13:01:38 +02:00
parent c75df632ca
commit 21f9d3338e
5 changed files with 128 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java
View File

@@ -676,7 +676,8 @@ public final class Neo4jTemplate implements Neo4jOperations, FluentNeo4jOperatio
Long relatedInternalId;
// No need to save values if processed
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
relatedInternalId = queryRelatedNode(newRelatedObject, targetEntity);
Object newRelatedObjectForQuery = stateMachine.getProcessedAs(newRelatedObject);
relatedInternalId = queryRelatedNode(newRelatedObjectForQuery, targetEntity);
} else {
relatedInternalId = saveRelatedNode(newRelatedObject, targetEntity);
}
@@ -712,6 +713,7 @@ public final class Neo4jTemplate implements Neo4jOperations, FluentNeo4jOperatio
// if an internal id is used this must be set to link this entity in the next iteration
if (targetEntity.isUsingInternalIds()) {
targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId);
stateMachine.markValueAsProcessedAs(newRelatedObject, targetPropertyAccessor.getBean());
}
if (processState != ProcessState.PROCESSED_ALL_VALUES) {

4
src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java
View File

@@ -772,7 +772,8 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Rea
Mono<Long> queryOrSave;
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
queryOrSave = queryRelatedNode(newRelatedObject, targetEntity);
Object newRelatedObjectForQuery = stateMachine.getProcessedAs(newRelatedObject);
queryOrSave = queryRelatedNode(newRelatedObjectForQuery, targetEntity);
} else {
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity);
}
@@ -782,6 +783,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Rea
PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);
if (targetEntity.isUsingInternalIds()) {
targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId);
stateMachine.markValueAsProcessedAs(newRelatedObject, targetPropertyAccessor.getBean());
}
Object idValue = idProperty != null

25
src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java
View File

@@ -16,7 +16,9 @@
package org.springframework.data.neo4j.core.mapping;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.locks.Lock;
@@ -56,6 +58,11 @@ public final class NestedRelationshipProcessingStateMachine {
*/
private final Set<Object> processedObjects = new HashSet<>();
/**
* Set of aliases. e.g. One object without id set represents the same as the one with the id set.
*/
private final Map<Object, Object> processedObjectsAlias = new HashMap<>();
public NestedRelationshipProcessingStateMachine(Object initialObject) {
processedObjects.add(initialObject);
}
@@ -179,4 +186,22 @@ public final class NestedRelationshipProcessingStateMachine {
return processedObjects.containsAll(valuesToStore);
}
public void markValueAsProcessedAs(Object relatedValueToStore, Object bean) {
try {
write.lock();
processedObjectsAlias.put(relatedValueToStore, bean);
} finally {
write.unlock();
}
}
public Object getProcessedAs(Object entity) {
try {
read.lock();
return processedObjectsAlias.getOrDefault(entity, entity);
} finally {
read.unlock();
}
}
}

47
src/test/java/org/springframework/data/neo4j/integration/imperative/ImmutableGeneratedIdsIT.java
View File

@@ -15,8 +15,11 @@
*/
package org.springframework.data.neo4j.integration.imperative;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.neo4j.driver.Driver;
import org.neo4j.driver.Record;
import org.neo4j.driver.Session;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -36,6 +39,7 @@ import org.springframework.data.neo4j.test.Neo4jExtension;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.transaction.annotation.EnableTransactionManagement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -54,6 +58,19 @@ public class ImmutableGeneratedIdsIT {
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
private final Driver driver;
public ImmutableGeneratedIdsIT(@Autowired Driver driver) {
this.driver = driver;
}
@BeforeEach
void cleanUp() {
try (Session session = driver.session()) {
session.run("MATCH (n) DETACH DELETE n").consume();
}
}
@Test // GH-2141
void saveWithGeneratedIdsReturnsObjectWithIdSet(
@Autowired ImmutablePersonWithGeneratedIdRepository repository) {
@@ -295,6 +312,36 @@ public class ImmutableGeneratedIdsIT {
assertThat(saved.getChildren()).allMatch(c -> c.getId() != null && children.contains(c));
}
@Test // GH-2223
void saveWithGeneratedIdsWithMultipleRelationshipsToOneNode(
@Autowired ImmutablePersonWithGeneratedIdRepository repository) {
ImmutablePersonWithGeneratedId person1 = new ImmutablePersonWithGeneratedId();
ImmutablePersonWithGeneratedId person2 = ImmutablePersonWithGeneratedId.fallback(person1);
List<ImmutablePersonWithGeneratedId> onboardedBy = new ArrayList<>();
onboardedBy.add(person1);
onboardedBy.add(person2);
ImmutablePersonWithGeneratedId person3 = ImmutablePersonWithGeneratedId.wasOnboardedBy(onboardedBy);
ImmutablePersonWithGeneratedId savedPerson = repository.save(person3);
assertThat(savedPerson.id).isNotNull();
assertThat(savedPerson.wasOnboardedBy).allMatch(ob -> ob.id != null);
ImmutablePersonWithGeneratedId savedPerson2 = savedPerson.wasOnboardedBy.stream().filter(p -> p.fallback != null).findFirst().get();
assertThat(savedPerson2.fallback.id).isNotNull();
try (Session session = driver.session()) {
List<Record> result = session.run(
"MATCH (person3:ImmutablePersonWithGeneratedId) " +
"-[:ONBOARDED_BY]->(person2:ImmutablePersonWithGeneratedId) " +
"-[:FALLBACK]->(person1:ImmutablePersonWithGeneratedId), " +
"(person3)-[:ONBOARDED_BY]->(person1) " +
"return person3")
.list();
assertThat(result).hasSize(1);
}
}
@Configuration
@EnableNeo4jRepositories(considerNestedRepositories = true)
@EnableTransactionManagement

50
src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveImmutableGeneratedIdsIT.java
View File

@@ -17,8 +17,12 @@ package org.springframework.data.neo4j.integration.reactive;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.jupiter.api.BeforeEach;
import org.neo4j.driver.Record;
import org.neo4j.driver.Session;
import reactor.test.StepVerifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -54,6 +58,19 @@ public class ReactiveImmutableGeneratedIdsIT {
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
private final Driver driver;
public ReactiveImmutableGeneratedIdsIT(@Autowired Driver driver) {
this.driver = driver;
}
@BeforeEach
void cleanUp() {
try (Session session = driver.session()) {
session.run("MATCH (n) DETACH DELETE n").consume();
}
}
@Test // GH-2141
void saveWithGeneratedIdsReturnsObjectWithIdSet(
@Autowired ReactiveImmutablePersonWithGeneratedIdRepository repository) {
@@ -302,6 +319,39 @@ public class ReactiveImmutableGeneratedIdsIT {
.verifyComplete();
}
@Test // GH-2223
void saveWithGeneratedIdsWithMultipleRelationshipsToOneNode(
@Autowired ReactiveImmutablePersonWithGeneratedIdRepository repository) {
ImmutablePersonWithGeneratedId person1 = new ImmutablePersonWithGeneratedId();
ImmutablePersonWithGeneratedId person2 = ImmutablePersonWithGeneratedId.fallback(person1);
List<ImmutablePersonWithGeneratedId> onboardedBy = new ArrayList<>();
onboardedBy.add(person1);
onboardedBy.add(person2);
ImmutablePersonWithGeneratedId person3 = ImmutablePersonWithGeneratedId.wasOnboardedBy(onboardedBy);
StepVerifier.create(repository.save(person3))
.assertNext(savedPerson -> {
assertThat(savedPerson.id).isNotNull();
assertThat(savedPerson.wasOnboardedBy).allMatch(ob -> ob.id != null);
ImmutablePersonWithGeneratedId savedPerson2 = savedPerson.wasOnboardedBy.stream().filter(p -> p.fallback != null).findFirst().get();
assertThat(savedPerson2.fallback.id).isNotNull();
})
.verifyComplete();
try (Session session = driver.session()) {
List<Record> result = session.run(
"MATCH (person3:ImmutablePersonWithGeneratedId) " +
"-[:ONBOARDED_BY]->(person2:ImmutablePersonWithGeneratedId) " +
"-[:FALLBACK]->(person1:ImmutablePersonWithGeneratedId), " +
"(person3)-[:ONBOARDED_BY]->(person1) " +
"return person3")
.list();
assertThat(result).hasSize(1);
}
}
@Configuration
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
@EnableTransactionManagement