Skip to content

Commit 8f8416e

Browse files
michael-simonsmichael-simons
committed
GH-2347 - Track state of objects with assigned ids correctly.
This change makes the Cypher generator create a statement for persisting multiple instances with assigned ids that not only returns the assigned ID, but also the internally generated id. This id is needed for tracking the root objects state in the `NestedRelationshipProcessingStateMachine`. Those ids must of course be retrieved. By doing so we currently loos the ability to debug log the number of nodes created. While this removal of a log statement can be considered a breaking change, it is necessary. To lighten that change: Storing multiple instances with internally generated did not debug log these informations at all. This fixes #2347.
1 parent 7f8cfd0 commit 8f8416e

File tree

9 files changed

+363
-49
lines changed

9 files changed

+363
-49
lines changed

‎src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.neo4j.cypherdsl.core.Cypher.asterisk;
2020
import static org.neo4j.cypherdsl.core.Cypher.parameter;
2121

22+
import java.util.AbstractMap;
2223
import java.util.ArrayList;
2324
import java.util.Collection;
2425
import java.util.Collections;
@@ -41,9 +42,9 @@
4142
import org.neo4j.cypherdsl.core.Node;
4243
import org.neo4j.cypherdsl.core.Statement;
4344
import org.neo4j.cypherdsl.core.renderer.Renderer;
45+
import org.neo4j.driver.Value;
4446
import org.neo4j.driver.exceptions.NoSuchRecordException;
4547
import org.neo4j.driver.summary.ResultSummary;
46-
import org.neo4j.driver.summary.SummaryCounters;
4748
import org.neo4j.driver.types.Entity;
4849
import org.springframework.beans.BeansException;
4950
import org.springframework.beans.factory.BeanFactory;
@@ -338,21 +339,22 @@ class Tuple3<T> {
338339
Function<T, Map<String, Object>> binderFunction = neo4jMappingContext.getRequiredBinderFunctionFor(domainClass);
339340
List<Map<String, Object>> entityList = entitiesToBeSaved.stream().map(h -> h.t3).map(binderFunction)
340341
.collect(Collectors.toList());
341-
ResultSummary resultSummary = neo4jClient
342+
Map<Value, Long> idToInternalIdMapping = neo4jClient
342343
.query(() -> renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData)))
343-
.in(databaseName)
344-
.bind(entityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM).run();
345-
346-
SummaryCounters counters = resultSummary.counters();
347-
log.debug(() -> String.format(
348-
"Created %d and deleted %d nodes, created %d and deleted %d relationships and set %d properties.",
349-
counters.nodesCreated(), counters.nodesDeleted(), counters.relationshipsCreated(),
350-
counters.relationshipsDeleted(), counters.propertiesSet()));
344+
.bind(entityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM)
345+
.fetchAs(Map.Entry.class)
346+
.mappedBy((t, r) -> new AbstractMap.SimpleEntry<>(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_INTERNAL_ID).asLong()))
347+
.all()
348+
.stream()
349+
.collect(Collectors.toMap(m -> (Value) m.getKey(), m -> (Long) m.getValue()));
351350

352351
// Save related
353352
return entitiesToBeSaved.stream().map(t -> {
354353
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(t.t3);
355-
return processRelations(entityMetaData, t.t1, propertyAccessor, databaseName, t.t2);
354+
Neo4jPersistentProperty idProperty = entityMetaData.getRequiredIdProperty();
355+
Object id = convertIdValues(idProperty, propertyAccessor.getProperty(idProperty));
356+
Long internalId = idToInternalIdMapping.get(id);
357+
return processRelations(entityMetaData, t.t1, internalId, propertyAccessor, databaseName, t.t2);
356358
}).collect(Collectors.toList());
357359
}
358360

@@ -472,14 +474,6 @@ private <T> T processRelations(Neo4jPersistentEntity<?> neo4jPersistentEntity, T
472474
new NestedRelationshipProcessingStateMachine(originalInstance, internalId));
473475
}
474476

475-
private <T> T processRelations(Neo4jPersistentEntity<?> neo4jPersistentEntity, T originalInstance,
476-
PersistentPropertyAccessor<?> parentPropertyAccessor,
477-
@Nullable String inDatabase, boolean isParentObjectNew) {
478-
479-
return processNestedRelations(neo4jPersistentEntity, parentPropertyAccessor, isParentObjectNew, inDatabase,
480-
new NestedRelationshipProcessingStateMachine(originalInstance));
481-
}
482-
483477
private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, PersistentPropertyAccessor<?> propertyAccessor,
484478
boolean isParentObjectNew, @Nullable String inDatabase, NestedRelationshipProcessingStateMachine stateMachine) {
485479

‎src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@
4545
import org.neo4j.cypherdsl.core.Node;
4646
import org.neo4j.cypherdsl.core.Statement;
4747
import org.neo4j.cypherdsl.core.renderer.Renderer;
48+
import org.neo4j.driver.Value;
4849
import org.neo4j.driver.exceptions.NoSuchRecordException;
49-
import org.neo4j.driver.summary.SummaryCounters;
5050
import org.neo4j.driver.types.Entity;
5151
import org.reactivestreams.Publisher;
5252
import org.springframework.beans.BeansException;
@@ -326,22 +326,27 @@ public <T> Flux<T> saveAll(Iterable<T> instances) {
326326
.map(nested -> Tuples.of(nested.getT1().getT1(), nested.getT1().getT2(), nested.getT2()))
327327
.collectList()
328328
.flatMapMany(entitiesToBeSaved -> Mono.defer(() -> {
329-
// Defer the actual save statement until the previous flux completes
330-
List<Map<String, Object>> boundedEntityList = entitiesToBeSaved.stream()
331-
.map(t -> t.getT3()) // extract PotentiallyModified
332-
.map(binderFunction).collect(Collectors.toList());
333-
return neo4jClient
334-
.query(() -> renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData)))
335-
.in(databaseName.getValue()).bind(boundedEntityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM).run();
336-
}).doOnNext(resultSummary -> {
337-
SummaryCounters counters = resultSummary.counters();
338-
log.debug(() -> String.format(
339-
"Created %d and deleted %d nodes, created %d and deleted %d relationships and set %d properties.",
340-
counters.nodesCreated(), counters.nodesDeleted(), counters.relationshipsCreated(),
341-
counters.relationshipsDeleted(), counters.propertiesSet()));
342-
}).thenMany(Flux.fromIterable(entitiesToBeSaved)
343-
.flatMap(t -> processRelations(entityMetaData, t.getT1(), entityMetaData.getPropertyAccessor(t.getT3()), databaseName.getValue(), t.getT2()))
344-
)));
329+
// Defer the actual save statement until the previous flux completes
330+
List<Map<String, Object>> boundedEntityList = entitiesToBeSaved.stream()
331+
.map(t -> t.getT3()) // extract PotentiallyModified
332+
.map(binderFunction).collect(Collectors.toList());
333+
return neo4jClient
334+
.query(() -> renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData)))
335+
.bind(boundedEntityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM)
336+
.fetchAs(Tuple2.class)
337+
.mappedBy((t, r) -> Tuples.of(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_INTERNAL_ID).asLong()))
338+
.all()
339+
.collectMap(m -> (Value) m.getT1(), m -> (Long) m.getT2());
340+
}).flatMapMany(idToInternalIdMapping -> Flux.fromIterable(entitiesToBeSaved)
341+
.flatMap(t -> {
342+
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(t.getT3());
343+
Neo4jPersistentProperty idProperty = entityMetaData.getRequiredIdProperty();
344+
Object id = convertIdValues(idProperty, propertyAccessor.getProperty(idProperty));
345+
Long internalId = idToInternalIdMapping.get(id);
346+
return processRelations(entityMetaData, t.getT1(), internalId,
347+
propertyAccessor, databaseName.getValue(), t.getT2());
348+
}))
349+
));
345350
}
346351

347352
@Override
@@ -585,14 +590,6 @@ private <T> Mono<T> processRelations(Neo4jPersistentEntity<?> neo4jPersistentEnt
585590
new NestedRelationshipProcessingStateMachine(originalInstance, internalId));
586591
}
587592

588-
private <T> Mono<T> processRelations(Neo4jPersistentEntity<?> neo4jPersistentEntity, T originalInstance,
589-
PersistentPropertyAccessor<?> parentPropertyAccessor,
590-
@Nullable String inDatabase, boolean isParentObjectNew) {
591-
592-
return processNestedRelations(neo4jPersistentEntity, parentPropertyAccessor, isParentObjectNew, inDatabase,
593-
new NestedRelationshipProcessingStateMachine(originalInstance));
594-
}
595-
596593
private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, PersistentPropertyAccessor<?> parentPropertyAccessor,
597594
boolean isParentObjectNew, @Nullable String inDatabase, NestedRelationshipProcessingStateMachine stateMachine) {
598595

‎src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,8 @@ public Statement prepareSaveOfMultipleInstancesOf(NodeDescription<?> nodeDescrip
364364
return Cypher.unwind(parameter(Constants.NAME_OF_ENTITY_LIST_PARAM)).as(row)
365365
.merge(rootNode.withProperties(nameOfIdProperty, Cypher.property(row, Constants.NAME_OF_ID)))
366366
.mutate(rootNode, Cypher.property(row, Constants.NAME_OF_PROPERTIES_PARAM))
367-
.returning(Functions.collect(rootNode.property(nameOfIdProperty)).as(Constants.NAME_OF_IDS)).build();
367+
.returning(rootNode.internalId().as(Constants.NAME_OF_INTERNAL_ID), rootNode.property(nameOfIdProperty).as(Constants.NAME_OF_ID))
368+
.build();
368369
}
369370

370371
@NonNull

‎src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.apiguardian.api.API;
2828
import org.springframework.lang.Nullable;
29+
import org.springframework.util.Assert;
2930

3031
/**
3132
* This stores all processed nested relations and objects during save of objects so that the recursive descent can be
@@ -71,12 +72,12 @@ public enum ProcessState {
7172
private final Map<Object, Long> processedObjectsIds = new HashMap<>();
7273

7374
public NestedRelationshipProcessingStateMachine(Object initialObject, Long internalId) {
74-
this(initialObject);
75-
processedObjectsIds.put(initialObject, internalId);
76-
}
7775

78-
public NestedRelationshipProcessingStateMachine(Object initialObject) {
76+
Assert.notNull(initialObject, "Initial object must not be null.");
77+
Assert.notNull(internalId, "The initial objects internal ID must not be null.");
78+
7979
processedObjects.add(initialObject);
80+
processedObjectsIds.put(initialObject, internalId);
8081
}
8182

8283
/**

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2347/Application.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2347;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
21+
import org.springframework.data.neo4j.core.schema.Id;
22+
import org.springframework.data.neo4j.core.schema.Node;
23+
import org.springframework.data.neo4j.core.schema.Relationship;
24+
25+
/**
26+
* @author Michael J. Simons
27+
*/
28+
@Node
29+
public class Application {
30+
31+
@Id
32+
private String id;
33+
34+
@Relationship(direction = Relationship.Direction.OUTGOING, type = "CONTAINS_WORKFLOW")
35+
private List<Workflow> workflows = new ArrayList<>();
36+
37+
public Application(String id) {
38+
this.id = id;
39+
}
40+
41+
public String getId() {
42+
return id;
43+
}
44+
45+
public List<Workflow> getWorkflows() {
46+
return workflows;
47+
}
48+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2347/GH2347IT.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2347;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import java.util.Collections;
21+
import java.util.List;
22+
23+
import org.junit.jupiter.api.Test;
24+
import org.neo4j.driver.Driver;
25+
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.context.annotation.Bean;
27+
import org.springframework.context.annotation.Configuration;
28+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
29+
import org.springframework.data.neo4j.repository.Neo4jRepository;
30+
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
31+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
32+
import org.springframework.transaction.annotation.EnableTransactionManagement;
33+
34+
/**
35+
* @author Michael J. Simons
36+
*/
37+
@Neo4jIntegrationTest
38+
class GH2347IT extends TestBase {
39+
40+
@Test
41+
void entitiesWithAssignedIdsSavedInBatchMustBeIdentifiableWithTheirInternalIds(
42+
@Autowired ApplicationRepository applicationRepository,
43+
@Autowired Driver driver
44+
) {
45+
List<Application> savedApplications = applicationRepository.saveAll(Collections.singletonList(createData()));
46+
47+
assertThat(savedApplications).hasSize(1);
48+
assertDatabase(driver);
49+
}
50+
51+
@Test
52+
void entitiesWithAssignedIdsMustBeIdentifiableWithTheirInternalIds(
53+
@Autowired ApplicationRepository applicationRepository,
54+
@Autowired Driver driver
55+
) {
56+
applicationRepository.save(createData());
57+
assertDatabase(driver);
58+
}
59+
60+
interface ApplicationRepository extends Neo4jRepository<Application, String> {
61+
}
62+
63+
@Configuration
64+
@EnableTransactionManagement
65+
@EnableNeo4jRepositories(considerNestedRepositories = true)
66+
static class Config extends AbstractNeo4jConfig {
67+
68+
@Bean
69+
public Driver driver() {
70+
71+
return neo4jConnectionSupport.getDriver();
72+
}
73+
}
74+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2347/ReactiveGH2347IT.java

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2347;
17+
18+
import reactor.test.StepVerifier;
19+
20+
import java.util.Collections;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.driver.Driver;
24+
import org.springframework.beans.factory.annotation.Autowired;
25+
import org.springframework.context.annotation.Bean;
26+
import org.springframework.context.annotation.Configuration;
27+
import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig;
28+
import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository;
29+
import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories;
30+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
31+
import org.springframework.transaction.annotation.EnableTransactionManagement;
32+
33+
/**
34+
* @author Michael J. Simons
35+
*/
36+
@Neo4jIntegrationTest
37+
class ReactiveGH2347IT extends TestBase {
38+
39+
@Test
40+
void entitiesWithAssignedIdsSavedInBatchMustBeIdentifiableWithTheirInternalIds(
41+
@Autowired ApplicationRepository applicationRepository,
42+
@Autowired Driver driver
43+
) {
44+
applicationRepository
45+
.saveAll(Collections.singletonList(createData()))
46+
.as(StepVerifier::create)
47+
.expectNextCount(1L)
48+
.verifyComplete();
49+
50+
assertDatabase(driver);
51+
}
52+
53+
@Test
54+
void entitiesWithAssignedIdsMustBeIdentifiableWithTheirInternalIds(
55+
@Autowired ApplicationRepository applicationRepository,
56+
@Autowired Driver driver
57+
) {
58+
applicationRepository
59+
.save(createData())
60+
.as(StepVerifier::create)
61+
.expectNextCount(1L)
62+
.verifyComplete();
63+
assertDatabase(driver);
64+
}
65+
66+
interface ApplicationRepository extends ReactiveNeo4jRepository<Application, String> {
67+
}
68+
69+
@Configuration
70+
@EnableTransactionManagement
71+
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
72+
static class Config extends AbstractReactiveNeo4jConfig {
73+
74+
@Bean
75+
public Driver driver() {
76+
77+
return neo4jConnectionSupport.getDriver();
78+
}
79+
}
80+
}

0 commit comments

Comments
 (0)