Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1582: rebind flag in apoc.periodic.iterate #3098

Merged
merged 3 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions full/src/main/java/apoc/nodes/NodesExtended.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package apoc.nodes;

import apoc.Extended;
import apoc.util.EntityUtil;
import apoc.util.Util;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.Relationship;
import org.neo4j.graphdb.Transaction;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Name;
import org.neo4j.procedure.UserFunction;

@Extended
public class NodesExtended {

@Context
public Transaction tx;

@UserFunction("apoc.node.rebind")
@Description("apoc.node.rebind(node - to rebind a node (i.e. executing a Transaction.getNodeById(node.getId()) ")
public Node nodeRebind(@Name("node") Node node) {
return Util.rebind(tx, node);
}

@UserFunction("apoc.rel.rebind")
@Description("apoc.rel.rebind(rel) - to rebind a rel (i.e. executing a Transaction.getRelationshipById(rel.getId()) ")
public Relationship relationshipRebind(@Name("rel") Relationship rel) {
return Util.rebind(tx, rel);
}

@UserFunction("apoc.any.rebind")
@Description("apoc.any.rebind(Object) - to rebind any rel, node, path, map, list or combination of them (i.e. executing a Transaction.getNodeById(node.getId()) / Transaction.getRelationshipById(rel.getId()))")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should have an explicit one for path, as otherwise we lose the type information and cypher doesn't recognize it as a path anymore

public Object anyRebind(@Name("any") Object any) {
return EntityUtil.anyRebind(tx, any);
}
}
37 changes: 37 additions & 0 deletions full/src/main/java/apoc/util/EntityUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package apoc.util;

import org.neo4j.graphalgo.impl.util.PathImpl;
import org.neo4j.graphdb.Entity;
import org.neo4j.graphdb.Path;
import org.neo4j.graphdb.Relationship;
import org.neo4j.graphdb.Transaction;
import org.neo4j.internal.helpers.collection.Iterables;

import java.util.Map;
import java.util.stream.Collectors;

public class EntityUtil {

public static <T> T anyRebind(Transaction tx, T any) {
if (any instanceof Map) {
return (T) ((Map<String, Object>) any).entrySet().stream()
.collect(Collectors.toMap(e -> e.getKey(), e -> anyRebind(tx, e.getValue())));
}
if (any instanceof Path) {
final Path path = (Path) any;
PathImpl.Builder builder = new PathImpl.Builder(Util.rebind(tx, path.startNode()));
for (Relationship rel: path.relationships()) {
builder = builder.push(Util.rebind(tx, rel));
}
return (T) builder.build();
}
if (any instanceof Iterable) {
return (T) Iterables.stream((Iterable) any)
.map(i -> anyRebind(tx, i)).collect(Collectors.toList());
}
if (any instanceof Entity) {
return (T) Util.rebind(tx, (Entity) any);
}
return any;
}
}
3 changes: 3 additions & 0 deletions full/src/main/resources/extended.txt
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,11 @@ apoc.uuid.install
apoc.uuid.list
apoc.uuid.remove
apoc.uuid.removeAll
apoc.any.rebind
apoc.coll.avgDuration
apoc.data.email
apoc.node.rebind
apoc.rel.rebind
apoc.static.get
apoc.static.getAll
apoc.trigger.nodesByLabel
Expand Down
69 changes: 69 additions & 0 deletions full/src/test/java/apoc/nodes/NodesExtendedTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package apoc.nodes;

import apoc.create.Create;
import apoc.util.TestUtil;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.neo4j.graphdb.Label;
import org.neo4j.graphdb.Path;
import org.neo4j.graphdb.Relationship;
import org.neo4j.graphdb.RelationshipType;
import org.neo4j.internal.helpers.collection.Iterables;
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;

import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;


public class NodesExtendedTest {

@Rule
public DbmsRule db = new ImpermanentDbmsRule();

@Before
public void setUp() throws Exception {
TestUtil.registerProcedure(db, NodesExtended.class, Create.class);
}

@Test
public void rebind() {
TestUtil.testCall(db, "CREATE (a:Foo)-[r1:MY_REL]->(b:Bar)-[r2:ANOTHER_REL]->(c:Baz) WITH a,b,c,r1,r2 \n" +
"RETURN apoc.any.rebind({first: a, second: b, third: c, rels: [r1, r2]}) as rebind",
(row) -> {
final Map<String, Object> rebind = (Map<String, Object>) row.get("rebind");
final List<Relationship> rels = (List<Relationship>) rebind.get("rels");
final Relationship firstRel = rels.get(0);
final Relationship secondRel = rels.get(1);
assertEquals(firstRel.getStartNode(), rebind.get("first"));
assertEquals(firstRel.getEndNode(), rebind.get("second"));
assertEquals(secondRel.getStartNode(), rebind.get("second"));
assertEquals(secondRel.getEndNode(), rebind.get("third"));
});

TestUtil.testCall(db, "CREATE p1=(a:Foo)-[r1:MY_REL]->(b:Bar), p2=(:Bar)-[r2:ANOTHER_REL]->(c:Baz) \n" +
"RETURN apoc.any.rebind([p1, p2]) as rebind",
(row) -> {
final List<Path> rebindList = (List<Path>) row.get("rebind");
assertEquals(2, rebindList.size());
final Path firstPath = rebindList.get(0);
assertPath(firstPath, List.of("Foo", "Bar"), List.of("MY_REL"));
final Path secondPath = rebindList.get(1);
assertPath(secondPath, List.of("Bar", "Baz"), List.of("ANOTHER_REL"));
});
}

private void assertPath(Path rebind, List<String> labels, List<String> relTypes) {
final List<String> actualLabels = Iterables.stream(rebind.nodes())
.map(i -> i.getLabels().iterator().next())
.map(Label::name).collect(Collectors.toList());
assertEquals(labels, actualLabels);
final List<String> actualRelTypes = Iterables.stream(rebind.relationships()).map(Relationship::getType)
.map(RelationshipType::name).collect(Collectors.toList());
assertEquals(relTypes, actualRelTypes);
}
}
59 changes: 58 additions & 1 deletion full/src/test/java/apoc/periodic/PeriodicExtendedTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package apoc.periodic;

import apoc.create.Create;
import apoc.load.Jdbc;
import apoc.nlp.gcp.GCPProcedures;
import apoc.nodes.NodesExtended;
import apoc.util.TestUtil;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -15,12 +18,16 @@
import org.neo4j.test.rule.ImpermanentDbmsRule;

import java.sql.SQLException;
import java.util.Collections;
import java.util.Map;
import java.util.function.Consumer;

import static apoc.util.TestUtil.testCall;
import static apoc.util.TestUtil.testCallCount;
import static apoc.util.TestUtil.testResult;
import static apoc.util.Util.map;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class PeriodicExtendedTest {
Expand All @@ -30,7 +37,57 @@ public class PeriodicExtendedTest {

@Before
public void initDb() {
TestUtil.registerProcedure(db, Periodic.class, PeriodicExtended.class, Jdbc.class);
TestUtil.registerProcedure(db, Periodic.class, NodesExtended.class, GCPProcedures.class, Create.class, PeriodicExtended.class, Jdbc.class);
}

@Test
public void testRebindWithNlpWriteProcedure() {
// use case: https://community.neo4j.com/t5/neo4j-graph-platform/use-of-apoc-periodic-iterate-with-apoc-nlp-gcp-classify-graph/m-p/56846#M33854
final String iterate = "MATCH (node:Article) RETURN node";
final String action = "CALL apoc.nlp.gcp.classify.graph(node, $nlpConf) YIELD graph RETURN null";
testRebindCommon(iterate, action, 0, this::assertNodeDeletedErr);

final String actionRebind = "CALL apoc.nlp.gcp.classify.graph(apoc.node.rebind(node), $nlpConf) YIELD graph RETURN null";
testRebindCommon(iterate, actionRebind, 2, this::assertNoErrors);

// "manual" rebind, i.e. "return id(node) as id" in iterate query, and "match .. where id(n)=id" in action query
final String iterateId = "MATCH (node:Article) RETURN id(node) AS id";
final String actionId = "MATCH (node) WHERE id(node) = id CALL apoc.nlp.gcp.classify.graph(node, $nlpConf) YIELD graph RETURN null";

testRebindCommon(iterateId, actionId, 2, this::assertNoErrors);
}

@Test
public void testRebindWithMapIterationAndCreateRelationshipProcedure() {
final String iterate = "MATCH (art:Article) RETURN {key: art, key2: 'another'} as map";
final String action = "CREATE (node:Category) with map.key as art, node call apoc.create.relationship(art, 'CATEGORY', {b: 1}, node) yield rel return rel";
testRebindCommon(iterate, action, 0, this::assertNodeDeletedErr);

final String actionRebind = "WITH apoc.any.rebind(map) AS map " + action;
testRebindCommon(iterate, actionRebind, 1, this::assertNoErrors);
}

private void assertNoErrors(Map<String, Object> r) {
assertEquals(Collections.emptyMap(), r.get("errorMessages"));
}

private void assertNodeDeletedErr(Map<String, Object> r) {
assertTrue(((Map<String, Object>) r.get("errorMessages"))
.keySet().stream()
.anyMatch(k -> k.matches("Node\\[\\d+] is deleted and cannot be used to create a relationship")));
}

private void testRebindCommon(String iterate, String action, int expected, Consumer<Map<String, Object>> assertions) {
final Map<String, Object> nlpConf = map("key", "myKey", "nodeProperty", "content", "write", true, "unsupportedDummyClient", true);
final Map<String, Object> config = map("params", map("nlpConf", nlpConf));

db.executeTransactionally("CREATE (:Article {content: 'contentBody'})");
testCall(db,"CALL apoc.periodic.iterate($iterate, $action, $config)",
map( "iterate" , iterate, "action", action, "config", config),
assertions);

testCallCount(db, "MATCH p=(:Category)<-[:CATEGORY]-(:Article) RETURN p", expected);
db.executeTransactionally("MATCH (n) DETACH DELETE n");
}

@Test
Expand Down