Fixes #303 - constraint issue (create before delete) with mergeNodes #389
Conversation
| copyRelationships(source, copyProperties(source, copyLabels(source, target)), delete); | ||
| if (delete) source.delete(); | ||
| Map<String, Object> propTemp = new HashMap<>(); | ||
| List<String> keys = new ArrayList<>(); |
| @@ -299,16 +299,30 @@ public void categorize( | |||
| } | |||
|
|
|||
| private Node mergeNodes(Node source, Node target, boolean delete) { | |||
| copyRelationships(source, copyProperties(source, copyLabels(source, target)), delete); | |||
| if (delete) source.delete(); | |||
| Map<String, Object> propTemp = new HashMap<>(); | |||
There was a problem hiding this comment.
can we find a better name? why not just Map<String, Object> properties = source.getAllProperties()
Didn't we say that we just store/keep the existing properties and then set the new ones after deletion of the source node? Not sure why we have to remove the properties individually?
| return target; | ||
| } | ||
|
|
||
| private void setProperty(Map<String, Object> propTemp, List<String> keys, Node target) { |
There was a problem hiding this comment.
isn't that the same as copy properties? only from a map source? Best name it the same and have it return the target node too.
There was a problem hiding this comment.
@jexp so we remane it copyProperties with this signature private Node copyProperties(Map<String, Object> properties, Node target) right?
| private Node copyRelationships(Node source, Node target, boolean delete) { | ||
| for (Relationship rel : source.getRelationships()) { | ||
| copyRelationship(rel, source, target); | ||
| if (delete) rel.delete(); | ||
| } | ||
| if (delete) source.delete(); |
There was a problem hiding this comment.
will this break any other code that uses this? I think the original contract for this method was that rels are deleted but the nodes are left alone.
so I'd rather move this out of the method back into mergeNodes where it originally was.
There was a problem hiding this comment.
@jexp ok so we move back the delete into mergeNodes before the call of copyProperties method
|
@jexp change requests fixed |
|
Looks great, thanks a lot. |
No description provided.