Skip to content

Commit 9944cf8

Browse files
meistermeiermeistermeier
committed
GH-2196 - Don't override relationships if they are different.
Relationship properties based relationships were overwritten due to multiple `MERGE` statements if they had the same target node.
1 parent 1fb0945 commit 9944cf8

File tree

7 files changed

+101
-13
lines changed

7 files changed

+101
-13
lines changed

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,18 +543,27 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Obje
543543
}
544544
stateMachine.markValueAsProcessed(relatedValueToStore);
545545

546+
Object idValue = idProperty != null
547+
? relationshipContext
548+
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty)
549+
: null;
550+
551+
boolean isNewRelationship = idValue == null;
552+
546553
CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatement(
547-
sourceEntity, relationshipContext, relatedValueToStore);
554+
sourceEntity, relationshipContext, relatedValueToStore, isNewRelationship);
548555

549556
Optional<Long> relationshipInternalId = neo4jClient.query(renderer.render(statementHolder.getStatement()))
550557
.bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) //
551-
.to(Constants.FROM_ID_PARAMETER_NAME)
558+
.to(Constants.FROM_ID_PARAMETER_NAME) //
552559
.bind(relatedInternalId) //
553560
.to(Constants.TO_ID_PARAMETER_NAME) //
561+
.bind(idValue) //
562+
.to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) //
554563
.bindAll(statementHolder.getProperties())
555564
.fetchAs(Long.class).one();
556565

557-
if (idProperty != null) {
566+
if (idProperty != null && isNewRelationship) {
558567
relationshipContext
559568
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore)
560569
.setProperty(idProperty, relationshipInternalId.get());

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,14 @@ private Mono<Void> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity,
674674
relatedInternalId);
675675
}
676676

677+
Object idValue = idProperty != null
678+
? relationshipContext
679+
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty)
680+
: null;
681+
682+
boolean isNewRelationship = idValue == null;
677683
CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatement(
678-
sourceEntity, relationshipContext, relatedValueToStore);
684+
sourceEntity, relationshipContext, relatedValueToStore, isNewRelationship);
679685

680686
// in case of no properties the bind will just return an empty map
681687
Mono<Long> relationshipCreationMonoNested = neo4jClient
@@ -684,10 +690,12 @@ private Mono<Void> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity,
684690
.to(Constants.FROM_ID_PARAMETER_NAME) //
685691
.bind(relatedInternalId) //
686692
.to(Constants.TO_ID_PARAMETER_NAME) //
693+
.bind(idValue) //
694+
.to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) //
687695
.bindAll(statementHolder.getProperties())
688696
.fetchAs(Long.class).one()
689697
.doOnNext(relationshipInternalId -> {
690-
if (idProperty != null) {
698+
if (idProperty != null && isNewRelationship) {
691699
relationshipContext
692700
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore)
693701
.setProperty(idProperty, relationshipInternalId);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public final class Constants {
5252
*/
5353
public static final String NAME_OF_STATIC_LABELS_PARAM = "__staticLabels__";
5454
public static final String NAME_OF_ENTITY_LIST_PARAM = "__entities__";
55+
public static final String NAME_OF_KNOWN_RELATIONSHIP_PARAM = "__knownRelationShipId__";
5556
public static final String NAME_OF_KNOWN_RELATIONSHIPS_PARAM = "__knownRelationShipIds__";
5657
public static final String NAME_OF_PATHS = "__paths__";
5758
public static final String NAME_OF_ALL_PROPERTIES = "__allProperties__";

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,9 @@ public Statement prepareSaveOfRelationship(Neo4jPersistentEntity<?> neo4jPersist
364364

365365
@NonNull
366366
public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity<?> neo4jPersistentEntity,
367-
RelationshipDescription relationship, @Nullable String dynamicRelationshipType) {
367+
RelationshipDescription relationship,
368+
boolean isNew,
369+
@Nullable String dynamicRelationshipType) {
368370

369371
Assert.isTrue(relationship.hasRelationshipProperties(),
370372
"Properties required to create a relationship with properties");
@@ -383,11 +385,16 @@ public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity<?
383385
startNode.relationshipFrom(endNode, type))
384386
.named(RELATIONSHIP_NAME);
385387

386-
return match(startNode)
388+
StatementBuilder.OngoingReadingWithWhere startAndEndNodeMatch = match(startNode)
387389
.where(neo4jPersistentEntity.isUsingInternalIds() ? startNode.internalId().isEqualTo(idParameter)
388390
: startNode.property(idPropertyName).isEqualTo(idParameter))
389-
.match(endNode).where(endNode.internalId().isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
390-
.merge(relationshipFragment)
391+
.match(endNode).where(endNode.internalId().isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
392+
393+
StatementBuilder.ExposesSet createOrMatch = isNew
394+
? startAndEndNodeMatch.create(relationshipFragment)
395+
: startAndEndNodeMatch.match(relationshipFragment)
396+
.where(Functions.id(relationshipFragment).isEqualTo(Cypher.parameter(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM)));
397+
return createOrMatch
391398
.mutate(RELATIONSHIP_NAME, relationshipProperties)
392399
.returning(Functions.id(relationshipFragment))
393400
.build();

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,10 @@ public void setApplicationContext(ApplicationContext applicationContext) throws
319319
this.beanFactory = applicationContext.getAutowireCapableBeanFactory();
320320
}
321321

322-
public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?> neo4jPersistentEntity, NestedRelationshipContext relationshipContext, Object relatedValue) {
322+
public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?> neo4jPersistentEntity,
323+
NestedRelationshipContext relationshipContext,
324+
Object relatedValue,
325+
boolean isNewRelationship) {
323326

324327
if (relationshipContext.hasRelationshipWithProperties()) {
325328
MappingSupport.RelationshipPropertiesWithEntityHolder relatedValueEntityHolder =
@@ -342,18 +345,20 @@ public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?
342345
}
343346
return createStatementForRelationShipWithProperties(
344347
neo4jPersistentEntity, relationshipContext,
345-
dynamicRelationshipType, relatedValueEntityHolder
348+
dynamicRelationshipType, relatedValueEntityHolder, isNewRelationship
346349
);
347350
} else {
348351
return createStatementForRelationshipWithoutProperties(neo4jPersistentEntity, relationshipContext, relatedValue);
349352
}
350353
}
351354

352355
private CreateRelationshipStatementHolder createStatementForRelationShipWithProperties(Neo4jPersistentEntity<?> neo4jPersistentEntity,
353-
NestedRelationshipContext relationshipContext, @Nullable String dynamicRelationshipType, MappingSupport.RelationshipPropertiesWithEntityHolder relatedValue) {
356+
NestedRelationshipContext relationshipContext, @Nullable String dynamicRelationshipType,
357+
MappingSupport.RelationshipPropertiesWithEntityHolder relatedValue, boolean isNewRelationship) {
354358

355359
Statement relationshipCreationQuery = CypherGenerator.INSTANCE.prepareSaveOfRelationshipWithProperties(
356-
neo4jPersistentEntity, relationshipContext.getRelationship(), dynamicRelationshipType);
360+
neo4jPersistentEntity, relationshipContext.getRelationship(), isNewRelationship, dynamicRelationshipType);
361+
357362
Map<String, Object> propMap = new HashMap<>();
358363
// write relationship properties
359364
getEntityConverter().write(relatedValue.getRelationshipProperties(), propMap);

‎src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2367,6 +2367,33 @@ void saveRelatedEntitesWithSameCustomIdsAndPlainRelationships(
23672367
assertThat(list).hasSize(0);
23682368
}
23692369
}
2370+
2371+
@Test // GH-2196
2372+
void saveSameNodeWithDoubleRelationship(@Autowired HobbyWithRelationshipWithPropertiesRepository repository) {
2373+
AltHobby hobby = new AltHobby();
2374+
hobby.setName("Music");
2375+
2376+
AltPerson altPerson = new AltPerson("Freddie");
2377+
2378+
AltLikedByPersonRelationship rel1 = new AltLikedByPersonRelationship();
2379+
rel1.setRating(5);
2380+
rel1.setAltPerson(altPerson);
2381+
2382+
AltLikedByPersonRelationship rel2 = new AltLikedByPersonRelationship();
2383+
rel2.setRating(1);
2384+
rel2.setAltPerson(altPerson);
2385+
2386+
hobby.getLikedBy().add(rel1);
2387+
hobby.getLikedBy().add(rel2);
2388+
repository.save(hobby);
2389+
2390+
hobby = repository.loadFromCustomQuery(altPerson.getId());
2391+
assertThat(hobby.getName()).isEqualTo("Music");
2392+
List<AltLikedByPersonRelationship> likedBy = hobby.getLikedBy();
2393+
assertThat(likedBy).hasSize(2);
2394+
2395+
assertThat(likedBy).containsExactlyInAnyOrder(rel1, rel2);
2396+
}
23702397
}
23712398

23722399
@Nested

‎src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,6 +2085,37 @@ void saveBidirectionalRelationship(@Autowired BidirectionalStartRepository repos
20852085
assertThat(records).hasSize(1);
20862086
}
20872087
}
2088+
2089+
@Test // GH-2196
2090+
void saveSameNodeWithDoubleRelationship(@Autowired ReactiveHobbyWithRelationshipWithPropertiesRepository repository) {
2091+
AltHobby hobby = new AltHobby();
2092+
hobby.setName("Music");
2093+
2094+
AltPerson altPerson = new AltPerson("Freddie");
2095+
2096+
AltLikedByPersonRelationship rel1 = new AltLikedByPersonRelationship();
2097+
rel1.setRating(5);
2098+
rel1.setAltPerson(altPerson);
2099+
2100+
AltLikedByPersonRelationship rel2 = new AltLikedByPersonRelationship();
2101+
rel2.setRating(1);
2102+
rel2.setAltPerson(altPerson);
2103+
2104+
hobby.getLikedBy().add(rel1);
2105+
hobby.getLikedBy().add(rel2);
2106+
StepVerifier.create(repository.save(hobby))
2107+
.expectNextCount(1)
2108+
.verifyComplete();
2109+
2110+
StepVerifier.create(repository.loadFromCustomQuery(altPerson.getId()))
2111+
.assertNext(loadedHobby -> {
2112+
assertThat(loadedHobby.getName()).isEqualTo("Music");
2113+
List<AltLikedByPersonRelationship> likedBy = loadedHobby.getLikedBy();
2114+
assertThat(likedBy).hasSize(2);
2115+
assertThat(likedBy).containsExactlyInAnyOrder(rel1, rel2);
2116+
})
2117+
.verifyComplete();
2118+
}
20882119
}
20892120

20902121
@Nested

0 commit comments

Comments
 (0)