[eBlQUOcs] Bug in apoc.create.cloneToVirtual (neo4j/apoc#375) (#3570)

neo4j-contrib · May 17, 2023 · 77c71cd · 77c71cd
1 parent 581fa59
commit 77c71cd
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 9 deletions.
diff --git a/core/src/main/java/apoc/result/VirtualPath.java b/core/src/main/java/apoc/result/VirtualPath.java
@@ -18,6 +18,7 @@
  */
 package apoc.result;
 
+import org.apache.commons.collections4.CollectionUtils;
 import org.neo4j.graphdb.Entity;
 import org.neo4j.graphdb.Node;
 import org.neo4j.graphdb.Path;
@@ -26,6 +27,7 @@
 
 import javax.annotation.Nonnull;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
@@ -145,19 +147,21 @@ public String toString() {
     }
 
     private void requireConnected(Relationship relationship) {
-        Node previousEndNode;
+        final List<Node> previousNodes = getPreviousNodes();
+        boolean isRelConnectedToPrevious = CollectionUtils.containsAny( previousNodes, relationship.getNodes() );
+        if (!isRelConnectedToPrevious) {
+            throw new IllegalArgumentException("Relationship is not part of current path.");
+        }
+    }
+
+    private List<Node> getPreviousNodes() {
         Relationship previousRelationship = lastRelationship();
         if (previousRelationship != null) {
-            previousEndNode = previousRelationship.getEndNode();
-        } else {
-            previousEndNode = endNode();
-        }
-        if (!relationship.getStartNode().equals(previousEndNode)
-                && !relationship.getEndNode().equals(previousEndNode)) {
-            throw new IllegalArgumentException("Relationship is not part of current path.");
+            return Arrays.asList(previousRelationship.getNodes());
         }
+        return List.of(endNode());
     }
-    
+
     public static final class Builder {
         private final Node start;
         private final List<Relationship> relationships = new ArrayList<>();

diff --git a/core/src/test/java/apoc/create/CreateTest.java b/core/src/test/java/apoc/create/CreateTest.java
@@ -346,6 +346,33 @@ public void testClonePathShouldNotEditOriginalOne() {
                 });
     }
 
+    @Test
+    public void testClonePathWithMixedDirectionRelationships() {
+        // rel `:b` is to the left, rel `d` is to the right
+        db.executeTransactionally("CREATE (:a {id: 1})<-[:b {id: 10}]-(:c {id: 2})-[:d {id: 20}]->(:e {id: 3})");
+
+        testCall(db, "MATCH p = (:a)<-[:b]-(:c)-[:d]->(:e) " +
+                "CALL apoc.create.clonePathToVirtual(p) YIELD path RETURN path", r -> {
+            Path path = (Path) r.get("path");
+            Iterator<Node> nodes = path.nodes().iterator();
+            Node node = nodes.next();
+            assertEquals(Map.of("id", 1L), node.getAllProperties());
+            node = nodes.next();
+            assertEquals(Map.of("id", 2L), node.getAllProperties());
+            node = nodes.next();
+            assertEquals(Map.of("id", 3L), node.getAllProperties());
+            assertFalse(nodes.hasNext());
+
+            Iterator<Relationship> rels = path.relationships().iterator();
+            Relationship rel = rels.next();
+            assertEquals(Map.of("id", 10L), rel.getAllProperties());
+            rel = rels.next();
+            assertEquals(Map.of("id", 20L), rel.getAllProperties());
+            assertFalse(rels.hasNext());
+
+        });
+    }
+
     @Test
     public void testClonePathShouldNotDuplicateRelsWithMultipaths() {
         //create path with single rels