Skip to content

Commit 91dd085

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 54cd12b commit 91dd085

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
@@ -531,18 +531,27 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Obje
531531
}
532532
stateMachine.markValueAsProcessed(relatedValueToStore);
533533

534+
Object idValue = idProperty != null
535+
? relationshipContext
536+
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty)
537+
: null;
538+
539+
boolean isNewRelationship = idValue == null;
540+
534541
CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatement(
535-
sourceEntity, relationshipContext, relatedValueToStore);
542+
sourceEntity, relationshipContext, relatedValueToStore, isNewRelationship);
536543

537544
Optional<Long> relationshipInternalId = neo4jClient.query(renderer.render(statementHolder.getStatement())).in(inDatabase)
538545
.bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) //
539-
.to(Constants.FROM_ID_PARAMETER_NAME)
546+
.to(Constants.FROM_ID_PARAMETER_NAME) //
540547
.bind(relatedInternalId) //
541548
.to(Constants.TO_ID_PARAMETER_NAME) //
549+
.bind(idValue) //
550+
.to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) //
542551
.bindAll(statementHolder.getProperties())
543552
.fetchAs(Long.class).one();
544553

545-
if (idProperty != null) {
554+
if (idProperty != null && isNewRelationship) {
546555
relationshipContext
547556
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore)
548557
.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
@@ -673,8 +673,14 @@ private Mono<Void> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity,
673673
relatedInternalId);
674674
}
675675

676+
Object idValue = idProperty != null
677+
? relationshipContext
678+
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty)
679+
: null;
680+
681+
boolean isNewRelationship = idValue == null;
676682
CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatement(
677-
sourceEntity, relationshipContext, relatedValueToStore);
683+
sourceEntity, relationshipContext, relatedValueToStore, isNewRelationship);
678684

679685
// in case of no properties the bind will just return an empty map
680686
Mono<Long> relationshipCreationMonoNested = neo4jClient
@@ -683,10 +689,12 @@ private Mono<Void> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity,
683689
.to(Constants.FROM_ID_PARAMETER_NAME) //
684690
.bind(relatedInternalId) //
685691
.to(Constants.TO_ID_PARAMETER_NAME) //
692+
.bind(idValue) //
693+
.to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) //
686694
.bindAll(statementHolder.getProperties())
687695
.fetchAs(Long.class).one()
688696
.doOnNext(relationshipInternalId -> {
689-
if (idProperty != null) {
697+
if (idProperty != null && isNewRelationship) {
690698
relationshipContext
691699
.getRelationshipPropertiesPropertyAccessor(relatedValueToStore)
692700
.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
@@ -41,6 +41,7 @@ public final class Constants {
4141
public static final String NAME_OF_PROPERTIES_PARAM = "__properties__";
4242
public static final String NAME_OF_STATIC_LABELS_PARAM = "__staticLabels__";
4343
public static final String NAME_OF_ENTITY_LIST_PARAM = "__entities__";
44+
public static final String NAME_OF_KNOWN_RELATIONSHIP_PARAM = "__knownRelationShipId__";
4445
public static final String NAME_OF_KNOWN_RELATIONSHIPS_PARAM = "__knownRelationShipIds__";
4546
public static final String NAME_OF_PATHS = "__paths__";
4647
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
@@ -320,7 +320,10 @@ public void setApplicationContext(ApplicationContext applicationContext) throws
320320
this.beanFactory = applicationContext.getAutowireCapableBeanFactory();
321321
}
322322

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

325328
if (relationshipContext.hasRelationshipWithProperties()) {
326329
MappingSupport.RelationshipPropertiesWithEntityHolder relatedValueEntityHolder =
@@ -343,18 +346,20 @@ public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?
343346
}
344347
return createStatementForRelationShipWithProperties(
345348
neo4jPersistentEntity, relationshipContext,
346-
dynamicRelationshipType, relatedValueEntityHolder
349+
dynamicRelationshipType, relatedValueEntityHolder, isNewRelationship
347350
);
348351
} else {
349352
return createStatementForRelationshipWithoutProperties(neo4jPersistentEntity, relationshipContext, relatedValue);
350353
}
351354
}
352355

353356
private CreateRelationshipStatementHolder createStatementForRelationShipWithProperties(Neo4jPersistentEntity<?> neo4jPersistentEntity,
354-
NestedRelationshipContext relationshipContext, @Nullable String dynamicRelationshipType, MappingSupport.RelationshipPropertiesWithEntityHolder relatedValue) {
357+
NestedRelationshipContext relationshipContext, @Nullable String dynamicRelationshipType,
358+
MappingSupport.RelationshipPropertiesWithEntityHolder relatedValue, boolean isNewRelationship) {
355359

356360
Statement relationshipCreationQuery = CypherGenerator.INSTANCE.prepareSaveOfRelationshipWithProperties(
357-
neo4jPersistentEntity, relationshipContext.getRelationship(), dynamicRelationshipType);
361+
neo4jPersistentEntity, relationshipContext.getRelationship(), isNewRelationship, dynamicRelationshipType);
362+
358363
Map<String, Object> propMap = new HashMap<>();
359364
// write relationship properties
360365
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
@@ -2329,6 +2329,33 @@ void saveRelatedEntitesWithSameCustomIdsAndPlainRelationships(
23292329
assertThat(list).hasSize(0);
23302330
}
23312331
}
2332+
2333+
@Test // GH-2196
2334+
void saveSameNodeWithDoubleRelationship(@Autowired HobbyWithRelationshipWithPropertiesRepository repository) {
2335+
AltHobby hobby = new AltHobby();
2336+
hobby.setName("Music");
2337+
2338+
AltPerson altPerson = new AltPerson("Freddie");
2339+
2340+
AltLikedByPersonRelationship rel1 = new AltLikedByPersonRelationship();
2341+
rel1.setRating(5);
2342+
rel1.setAltPerson(altPerson);
2343+
2344+
AltLikedByPersonRelationship rel2 = new AltLikedByPersonRelationship();
2345+
rel2.setRating(1);
2346+
rel2.setAltPerson(altPerson);
2347+
2348+
hobby.getLikedBy().add(rel1);
2349+
hobby.getLikedBy().add(rel2);
2350+
repository.save(hobby);
2351+
2352+
hobby = repository.loadFromCustomQuery(altPerson.getId());
2353+
assertThat(hobby.getName()).isEqualTo("Music");
2354+
List<AltLikedByPersonRelationship> likedBy = hobby.getLikedBy();
2355+
assertThat(likedBy).hasSize(2);
2356+
2357+
assertThat(likedBy).containsExactlyInAnyOrder(rel1, rel2);
2358+
}
23322359
}
23332360

23342361
@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)