From 33f69071d7cf8424a5ac17db9af6750ece8090b4 Mon Sep 17 00:00:00 2001 From: Love Kristofer Leifland Date: Tue, 29 Oct 2024 11:33:26 +0100 Subject: [PATCH] [bkym7TcZ] Optimise apoc.refactor.cloneSubgraph --- common/build.gradle | 2 +- core/build.gradle | 1 + .../java/apoc/refactor/GraphRefactoring.java | 143 ++++++++-------- .../java/apoc/refactor/CloneSubgraphTest.java | 148 +++++++++++++---- .../apoc/refactor/GraphRefactoringTest.java | 154 ++++++++---------- core/src/test/resources/procedures.json | 2 +- processor/build.gradle | 2 +- 7 files changed, 269 insertions(+), 183 deletions(-) diff --git a/common/build.gradle b/common/build.gradle index 012381a47..50b930913 100644 --- a/common/build.gradle +++ b/common/build.gradle @@ -52,7 +52,7 @@ dependencies { testImplementation group: 'org.neo4j.community', name: 'it-test-support', version: neo4jVersionEffective // , classifier: "tests" testImplementation group: 'org.neo4j', name: 'log-test-utils', version: neo4jVersionEffective // , classifier: "tests" testImplementation group: 'org.neo4j', name: 'neo4j-kernel', version: neo4jVersionEffective, classifier: "tests" - testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.13.2' + testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.26.3' testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.2.0' testImplementation group: 'pl.pragmatists', name: 'JUnitParams', version: '1.1.1' diff --git a/core/build.gradle b/core/build.gradle index b81e73176..e7cade40e 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -68,6 +68,7 @@ dependencies { testImplementation group: 'org.mock-server', name: 'mockserver-netty', version: '5.15.0', { exclude group: 'org.slf4j', module: 'slf4j-api' } + testImplementation 'org.assertj:assertj-core:3.26.3' configurations.all { exclude group: 'org.slf4j', module: 'slf4j-nop' diff --git a/core/src/main/java/apoc/refactor/GraphRefactoring.java b/core/src/main/java/apoc/refactor/GraphRefactoring.java index 10b2a1a15..da962a9c3 100644 --- a/core/src/main/java/apoc/refactor/GraphRefactoring.java +++ b/core/src/main/java/apoc/refactor/GraphRefactoring.java @@ -22,6 +22,7 @@ import static apoc.refactor.util.RefactorConfig.RelationshipSelectionStrategy.MERGE; import static apoc.refactor.util.RefactorUtil.*; import static apoc.util.Util.withTransactionAndRebind; +import static java.util.stream.StreamSupport.stream; import apoc.Pools; import apoc.algo.Cover; @@ -35,7 +36,6 @@ import java.util.function.BiFunction; import java.util.stream.Collectors; import java.util.stream.Stream; -import java.util.stream.StreamSupport; import org.neo4j.graphdb.*; import org.neo4j.graphdb.schema.ConstraintType; import org.neo4j.kernel.impl.coreapi.InternalTransaction; @@ -248,93 +248,110 @@ public Stream cloneSubgraph( """ { standinNodes :: LIST>, - skipProperties :: LIST + skipProperties :: LIST, + createNodesInNewTransactions = false :: BOOLEAN } """) Map config) { if (nodes == null || nodes.isEmpty()) return Stream.empty(); - // empty or missing rels list means get all rels between nodes - if (rels == null || rels.isEmpty()) { - rels = Cover.coverNodes(nodes).collect(Collectors.toList()); - } - - Map copyMap = new HashMap<>(nodes.size()); - List resultStream = new ArrayList<>(); + final var newNodeByOldNode = new HashMap(nodes.size()); + final var resultStream = new ArrayList(); - Map standinMap = - generateStandinMap((List>) config.getOrDefault("standinNodes", Collections.emptyList())); - List skipProperties = (List) config.getOrDefault("skipProperties", Collections.emptyList()); + final var standinMap = asNodePairs(config.get("standinNodes")); + final var skipProps = asStringSet(config.get("skipProperties")); + final var createNodesInInnerTx = + Boolean.TRUE.equals(config.getOrDefault("createNodesInNewTransactions", false)); // clone nodes and populate copy map - for (Node node : nodes) { - if (node == null || standinMap.containsKey(node)) continue; + for (final var oldNode : nodes) { + // standinNodes will NOT be cloned + if (oldNode == null || standinMap.containsKey(oldNode)) continue; - NodeRefactorResult result = new NodeRefactorResult(node.getId()); + final var result = new NodeRefactorResult(oldNode.getId()); try { - Node copy = withTransactionAndRebind(db, tx, transaction -> { - Node copyTemp = transaction.createNode(); - Map properties = node.getAllProperties(); - if (skipProperties != null && !skipProperties.isEmpty()) { - for (String skip : skipProperties) properties.remove(skip); - } - copyProperties(properties, copyTemp); - copyLabels(node, copyTemp); - return copyTemp; - }); - resultStream.add(result.withOther(copy)); - copyMap.put(node, copy); + final Node newNode; + if (!createNodesInInnerTx) newNode = cloneNode(tx, oldNode, skipProps); + else newNode = withTransactionAndRebind(db, tx, innerTx -> cloneNode(innerTx, oldNode, skipProps)); + resultStream.add(result.withOther(newNode)); + newNodeByOldNode.put(oldNode, newNode); } catch (Exception e) { resultStream.add(result.withError(e)); } } + final Iterator relsIterator; + // empty or missing rels list means get all rels between nodes + if (rels == null || rels.isEmpty()) + relsIterator = Cover.coverNodes(nodes).iterator(); + else relsIterator = rels.iterator(); + // clone relationships, will be between cloned nodes and/or standins - for (Relationship rel : rels) { + while (relsIterator.hasNext()) { + final var rel = relsIterator.next(); if (rel == null) continue; Node oldStart = rel.getStartNode(); - Node newStart = standinMap.getOrDefault(oldStart, copyMap.get(oldStart)); + Node newStart = standinMap.getOrDefault(oldStart, newNodeByOldNode.get(oldStart)); Node oldEnd = rel.getEndNode(); - Node newEnd = standinMap.getOrDefault(oldEnd, copyMap.get(oldEnd)); + Node newEnd = standinMap.getOrDefault(oldEnd, newNodeByOldNode.get(oldEnd)); - if (newStart != null && newEnd != null) { - Relationship newrel = newStart.createRelationshipTo(newEnd, rel.getType()); - Map properties = rel.getAllProperties(); - if (skipProperties != null && !skipProperties.isEmpty()) { - for (String skip : skipProperties) properties.remove(skip); - } - copyProperties(properties, newrel); - } + if (newStart != null && newEnd != null) cloneRel(rel, newStart, newEnd, skipProps); } return resultStream.stream(); } - private Map generateStandinMap(List> standins) { - Map standinMap = standins.isEmpty() ? Collections.emptyMap() : new HashMap<>(standins.size()); - - for (List pairing : standins) { - if (pairing == null) continue; + private static Node cloneNode(final Transaction tx, final Node node, final Set skipProps) { + final var newNode = + tx.createNode(stream(node.getLabels().spliterator(), false).toArray(Label[]::new)); + try { + node.getAllProperties().forEach((k, v) -> { + if (skipProps.isEmpty() || !skipProps.contains(k)) newNode.setProperty(k, v); + }); + } catch (Exception e) { + newNode.delete(); + throw e; + } + return newNode; + } - if (pairing.size() != 2) { - throw new IllegalArgumentException("'standinNodes' must be a list of node pairs"); - } + private static void cloneRel(Relationship base, Node from, Node to, final Set skipProps) { + final var rel = from.createRelationshipTo(to, base.getType()); + rel.getAllProperties().forEach((k, v) -> { + if (skipProps.isEmpty() || !skipProps.contains(k)) rel.setProperty(k, v); + }); + } - Node from = pairing.get(0); - Node to = pairing.get(1); + private Map asNodePairs(Object o) { + if (o == null) return Collections.emptyMap(); + else if (o instanceof List list) { + return list.stream() + .filter(Objects::nonNull) + .map(GraphRefactoring::castNodePair) + .collect(Collectors.toUnmodifiableMap(l -> l.get(0), l -> l.get(1))); + } else { + throw new IllegalArgumentException("Expected a list of node pairs but got " + o); + } + } - if (from == null || to == null) { - throw new IllegalArgumentException("'standinNodes' must be a list of node pairs"); - } + private static Set asStringSet(Object o) { + if (o == null) return Collections.emptySet(); + else if (o instanceof Collection c && c.stream().allMatch(i -> i instanceof String)) { + return c.stream().map(Object::toString).collect(Collectors.toSet()); + } else throw new IllegalArgumentException("Expected a list of string parameter keys but got " + o); + } - standinMap.put(from, to); + private static List castNodePair(Object o) { + if (o instanceof List l && l.size() == 2 && l.get(0) instanceof Node && l.get(1) instanceof Node) { + //noinspection unchecked + return (List) l; + } else { + throw new IllegalArgumentException("Expected pair of nodes but got " + o); } - - return standinMap; } public record MergedNodeResult(@Description("The merged node.") Node node) {} @@ -375,7 +392,7 @@ public Stream mergeNodes( final Node first = nodes.get(0); final List existingSelfRelIds = conf.isPreservingExistingSelfRels() - ? StreamSupport.stream(first.getRelationships().spliterator(), false) + ? stream(first.getRelationships().spliterator(), false) .filter(Util::isSelfRel) .map(Entity::getElementId) .collect(Collectors.toList()) @@ -637,11 +654,11 @@ public Stream deleteAndReconnect( return Stream.empty(); } - BiFunction filterRel = (node, direction) -> StreamSupport.stream( - node.getRelationships(direction).spliterator(), false) - .filter(rels::contains) - .findFirst() - .orElse(null); + BiFunction filterRel = + (node, direction) -> stream(node.getRelationships(direction).spliterator(), false) + .filter(rels::contains) + .findFirst() + .orElse(null); nodesToRemove.forEach(node -> { Relationship relationshipIn = filterRel.apply(node, Direction.INCOMING); @@ -695,14 +712,12 @@ public Stream deleteAndReconnect( } private boolean isUniqueConstraintDefinedFor(String label, String key) { - return StreamSupport.stream( - tx.schema().getConstraints(Label.label(label)).spliterator(), false) + return stream(tx.schema().getConstraints(Label.label(label)).spliterator(), false) .anyMatch(c -> { if (!c.isConstraintType(ConstraintType.UNIQUENESS)) { return false; } - return StreamSupport.stream(c.getPropertyKeys().spliterator(), false) - .allMatch(k -> k.equals(key)); + return stream(c.getPropertyKeys().spliterator(), false).allMatch(k -> k.equals(key)); }); } diff --git a/core/src/test/java/apoc/refactor/CloneSubgraphTest.java b/core/src/test/java/apoc/refactor/CloneSubgraphTest.java index 263215624..40a9facde 100644 --- a/core/src/test/java/apoc/refactor/CloneSubgraphTest.java +++ b/core/src/test/java/apoc/refactor/CloneSubgraphTest.java @@ -19,6 +19,8 @@ package apoc.refactor; import static apoc.util.MapUtil.map; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -31,6 +33,8 @@ import apoc.util.TestUtil; import java.util.Collections; import java.util.List; +import java.util.Map; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -77,26 +81,55 @@ public void teardown() { db.shutdown(); } - @Test - public void testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relationships_Between_Nodes() { - TestUtil.testCall( - db, - "MATCH (rootA:Root{name:'A'})" + "CALL apoc.path.subgraphAll(rootA, {}) YIELD nodes, relationships " - + "WITH nodes[1..] as nodes, relationships, [rel in relationships | startNode(rel).name + ' ' + type(rel) + ' ' + endNode(rel).name] as relNames " - + "CALL apoc.refactor.cloneSubgraph(nodes, []) YIELD input, output, error " - + "WITH relNames, collect(output) as clones, collect(output.name) as cloneNames " - + "CALL apoc.algo.cover(clones) YIELD rel " - + "WITH relNames, cloneNames, [rel in collect(rel) | startNode(rel).name + ' ' + type(rel) + ' ' + endNode(rel).name] as cloneRelNames " - + // was seeing odd incorrect behavior with yielded relTypesCount from apoc.meta.stats() - "WITH cloneNames, cloneRelNames, apoc.coll.containsAll(relNames, cloneRelNames) as clonedRelsVerified " - + "RETURN cloneNames, cloneRelNames, clonedRelsVerified", - (row) -> { - assertTrue(((List) row.get("cloneNames")) - .containsAll(List.of( + private static final String cloneWithEmptyRels = + """ + MATCH (rootA:Root{name:'A'}) + CALL apoc.path.subgraphAll(rootA, {}) YIELD nodes, relationships + WITH + nodes[1..] as nodes, + relationships, + [rel in relationships | startNode(rel).name + ' ' + type(rel) + ' ' + endNode(rel).name] as relNames + CALL apoc.refactor.cloneSubgraph(nodes, [], {createNodesInNewTransactions: $newTx}) YIELD input, output, error + WITH + relNames, + collect(output) as clones, + collect(output.name) as cloneNames + CALL apoc.algo.cover(clones) YIELD rel + WITH + relNames, + cloneNames, + [rel in collect(rel) | startNode(rel).name + ' ' + type(rel) + ' ' + endNode(rel).name] as cloneRelNames + WITH + cloneNames, + cloneRelNames, + apoc.coll.containsAll(relNames, cloneRelNames) as clonedRelsVerified + RETURN + cloneNames, + cloneRelNames, + clonedRelsVerified + """; + + private static final String cloneWithEmptyRelsMeta = + """ + CALL apoc.meta.stats() YIELD nodeCount, relCount, labels, relTypes as relTypesMap + CALL db.relationshipTypes() YIELD relationshipType + WITH nodeCount, relCount, labels, collect([relationshipType, relTypesMap['()-[:' + relationshipType + ']->()']]) as relationshipTypesColl + RETURN nodeCount, relCount, labels, apoc.map.fromPairs(relationshipTypesColl) as relTypesCount + """; + + private void cloneWithEmptyRelsTest(boolean newTx, boolean commit) { + try (final var tx = db.beginTx(); + final var res = tx.execute(cloneWithEmptyRels, Map.of("newTx", newTx))) { + assertThat(res.stream()) + .singleElement(InstanceOfAssertFactories.map(String.class, Object.class)) + .hasEntrySatisfying("cloneNames", n -> assertThat(n) + .asInstanceOf(list(String.class)) + .containsExactlyInAnyOrder( "node1", "node2", "node3", "node4", "node5", "node8", "node9", "node10", "node6", - "node7"))); - assertTrue(((List) row.get("cloneRelNames")) - .containsAll(List.of( + "node7")) + .hasEntrySatisfying("cloneRelNames", n -> assertThat(n) + .asInstanceOf(list(String.class)) + .containsExactlyInAnyOrder( "node1 LINK node5", "node1 LINK node2", "node2 LINK node3", @@ -105,22 +138,69 @@ public void testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relati "node7 LINK node6", "node5 LINK node8", "node5 LINK node9", - "node9 DIFFERENT_LINK node10"))); - assertTrue((Boolean) row.get("clonedRelsVerified")); - }); + "node9 DIFFERENT_LINK node10")) + .containsEntry("clonedRelsVerified", true); - TestUtil.testCall( - db, - "CALL apoc.meta.stats() YIELD nodeCount, relCount, labels, relTypes as relTypesMap " - + "CALL db.relationshipTypes() YIELD relationshipType " - + "WITH nodeCount, relCount, labels, collect([relationshipType, relTypesMap['()-[:' + relationshipType + ']->()']]) as relationshipTypesColl " - + "RETURN nodeCount, relCount, labels, apoc.map.fromPairs(relationshipTypesColl) as relTypesCount ", - (row) -> { - assertEquals(row.get("nodeCount"), 24L); // original was 14, 10 nodes cloned - assertEquals(row.get("relCount"), 20L); // original was 11, 9 relationships cloned - assertEquals(row.get("labels"), map("Root", 2L, "Oddball", 2L, "Node", 22L)); - assertEquals(row.get("relTypesCount"), map("LINK", 18L, "DIFFERENT_LINK", 2L)); - }); + if (commit) tx.commit(); + else tx.rollback(); + } + } + + @Test + public void testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relationships_Between_Nodes_NewTx() { + cloneWithEmptyRelsTest(true, true); + try (final var tx = db.beginTx(); + final var res = tx.execute(cloneWithEmptyRelsMeta)) { + assertThat(res.stream()) + .singleElement(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("nodeCount", 24L) + .containsEntry("relCount", 20L) + .containsEntry("labels", map("Root", 2L, "Oddball", 2L, "Node", 22L)) + .containsEntry("relTypesCount", map("LINK", 18L, "DIFFERENT_LINK", 2L)); + } + } + + @Test + public void + testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relationships_Between_Nodes_NewTx_Rollback() { + cloneWithEmptyRelsTest(true, false); + try (final var tx = db.beginTx(); + final var res = tx.execute(cloneWithEmptyRelsMeta)) { + assertThat(res.stream()) + .singleElement(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("nodeCount", 24L) // Committed in inner tx + .containsEntry("relCount", 11L) // Rolled back, not created (weird, but behaves as before) + .containsEntry("labels", map("Root", 2L, "Oddball", 2L, "Node", 22L)) + .containsEntry("relTypesCount", map("LINK", 10L, "DIFFERENT_LINK", 1L)); + } + } + + @Test + public void testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relationships_Between_Nodes() { + cloneWithEmptyRelsTest(false, true); + try (final var tx = db.beginTx(); + final var res = tx.execute(cloneWithEmptyRelsMeta)) { + assertThat(res.stream()) + .singleElement(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("nodeCount", 24L) + .containsEntry("relCount", 20L) + .containsEntry("labels", map("Root", 2L, "Oddball", 2L, "Node", 22L)) + .containsEntry("relTypesCount", map("LINK", 18L, "DIFFERENT_LINK", 2L)); + } + } + + @Test + public void testCloneSubgraph_From_RootA_With_Empty_Rels_Should_Clone_All_Relationships_Between_Nodes_Rollback() { + cloneWithEmptyRelsTest(false, false); + try (final var tx = db.beginTx(); + final var res = tx.execute(cloneWithEmptyRelsMeta)) { + assertThat(res.stream()) + .singleElement(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("nodeCount", 14L) // Rolled back + .containsEntry("relCount", 11L) // Rolled back + .containsEntry("labels", map("Root", 2L, "Oddball", 1L, "Node", 12L)) + .containsEntry("relTypesCount", map("LINK", 10L, "DIFFERENT_LINK", 1L)); + } } @Test diff --git a/core/src/test/java/apoc/refactor/GraphRefactoringTest.java b/core/src/test/java/apoc/refactor/GraphRefactoringTest.java index 8c214d26a..b392fc9f6 100644 --- a/core/src/test/java/apoc/refactor/GraphRefactoringTest.java +++ b/core/src/test/java/apoc/refactor/GraphRefactoringTest.java @@ -24,13 +24,11 @@ import static apoc.util.TestUtil.testCallEmpty; import static apoc.util.TestUtil.testResult; import static apoc.util.Util.isSelfRel; -import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -56,7 +54,6 @@ import java.util.stream.StreamSupport; import junit.framework.TestCase; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -333,27 +330,31 @@ public void deleteAndReconnectWithMergeRelConfigAndPropertiesCombine() { db, "MATCH p=(f:One)-->(b:Two)-->(c:Three)-->(d:Four)-->(e:Five) WITH p, [b,d] as list CALL apoc.refactor.deleteAndReconnect(p, list, {properties: 'combine', relationshipSelectionStrategy: 'merge'}) YIELD nodes, relationships RETURN nodes, relationships", (row) -> { - List nodes = (List) row.get("nodes"); - assertEquals(3, nodes.size()); - Node node1 = nodes.get(0); - assertEquals(singletonList(label("One")), node1.getLabels()); - Node node2 = nodes.get(1); - assertEquals(singletonList(label("Three")), node2.getLabels()); - Node node3 = nodes.get(2); - assertEquals(singletonList(label("Five")), node3.getLabels()); - List rels = (List) row.get("relationships"); - assertEquals(2, rels.size()); - Relationship rel1 = rels.get(0); - assertEquals("ALPHA_BETA", rel1.getType().name()); - assertEquals("f", rel1.getProperty("e")); - assertThat(Arrays.asList((String[]) rel1.getProperty("a")), containsInAnyOrder("b", "d")); - assertEquals("h", rel1.getProperty("g")); - Relationship rel2 = rels.get(1); - assertEquals("GAMMA_DELTA", rel2.getType().name()); - assertThat(Arrays.asList((String[]) rel2.getProperty("aa")), containsInAnyOrder("one", "bb")); - assertEquals("ff", rel2.getProperty("ee")); - assertEquals("dd", rel2.getProperty("cc")); - assertNotNull(row.get("nodes")); + assertThat(row.get("nodes")) + .asInstanceOf(list(Node.class)) + .satisfiesExactly( + n -> assertThat(n.getLabels()).containsExactly(label("One")), + n -> assertThat(n.getLabels()).containsExactly(label("Three")), + n -> assertThat(n.getLabels()).containsExactly(label("Five"))); + assertThat(row.get("relationships")) + .asInstanceOf(list(Relationship.class)) + .satisfiesExactly( + rel -> { + assertThat(rel.getType().name()).isEqualTo("ALPHA_BETA"); + assertThat(rel.getAllProperties()) + .containsEntry("e", "f") + .containsEntry("g", "h") + .hasEntrySatisfying("a", v -> assertThat((String[]) v) + .containsExactlyInAnyOrder("b", "d")); + }, + rel -> { + assertThat(rel.getType().name()).isEqualTo("GAMMA_DELTA"); + assertThat(rel.getAllProperties()) + .containsEntry("ee", "ff") + .containsEntry("cc", "dd") + .hasEntrySatisfying("aa", v -> assertThat((String[]) v) + .containsExactlyInAnyOrder("one", "bb")); + }); }); TestUtil.testCall(db, Util.NODE_COUNT, (row) -> assertEquals(3L, row.get("result"))); @@ -370,27 +371,29 @@ public void deleteAndReconnectWithMergeRelConfigAndPropertiesOverride() { db, "MATCH p=(f:One)-->(b:Two)-->(c:Three)-->(d:Four)-->(e:Five) WITH p, [b,d] as list CALL apoc.refactor.deleteAndReconnect(p, list, {properties: 'override', relationshipSelectionStrategy: 'merge'}) YIELD nodes, relationships RETURN nodes, relationships", (row) -> { - List nodes = (List) row.get("nodes"); - assertEquals(3, nodes.size()); - Node node1 = nodes.get(0); - assertEquals(singletonList(label("One")), node1.getLabels()); - Node node2 = nodes.get(1); - assertEquals(singletonList(label("Three")), node2.getLabels()); - Node node3 = nodes.get(2); - assertEquals(singletonList(label("Five")), node3.getLabels()); - List rels = (List) row.get("relationships"); - assertEquals(2, rels.size()); - Relationship rel1 = rels.get(0); - assertEquals("ALPHA_BETA", rel1.getType().name()); - assertEquals("f", rel1.getProperty("e")); - assertEquals("d", rel1.getProperty("a")); - assertEquals("h", rel1.getProperty("g")); - Relationship rel2 = rels.get(1); - assertEquals("GAMMA_DELTA", rel2.getType().name()); - assertEquals("bb", rel2.getProperty("aa")); - assertEquals("dd", rel2.getProperty("cc")); - assertEquals("ff", rel2.getProperty("ee")); - assertNotNull(row.get("nodes")); + assertThat(row.get("nodes")) + .asInstanceOf(list(Node.class)) + .satisfiesExactly( + n -> assertThat(n.getLabels()).containsExactly(label("One")), + n -> assertThat(n.getLabels()).containsExactly(label("Three")), + n -> assertThat(n.getLabels()).containsExactly(label("Five"))); + assertThat(row.get("relationships")) + .asInstanceOf(list(Relationship.class)) + .satisfiesExactly( + rel -> { + assertThat(rel.getType().name()).isEqualTo("ALPHA_BETA"); + assertThat(rel.getAllProperties()) + .containsEntry("e", "f") + .containsEntry("a", "d") + .containsEntry("g", "h"); + }, + rel -> { + assertThat(rel.getType().name()).isEqualTo("GAMMA_DELTA"); + assertThat(rel.getAllProperties()) + .containsEntry("aa", "bb") + .containsEntry("cc", "dd") + .containsEntry("ee", "ff"); + }); }); TestUtil.testCall(db, Util.NODE_COUNT, (row) -> assertEquals(3L, row.get("result"))); @@ -675,11 +678,7 @@ public void testNormalizeAsBoolean() { testResult( db, "MATCH (n) CALL apoc.refactor.normalizeAsBoolean(n,'prop',['Y','Yes'],['NO']) WITH n ORDER BY n.id RETURN n.prop AS prop", - (r) -> { - List result = new ArrayList<>(); - while (r.hasNext()) result.add((Boolean) r.next().get("prop")); - assertThat(result, equalTo(Arrays.asList(true, true, false, null))); - }); + (rows) -> assertThat(rows.stream()).map(r -> r.get("prop")).containsExactly(true, true, false, null)); } private void categorizeWithDirection(Direction direction) { @@ -701,28 +700,20 @@ private void categorizeWithDirection(Direction direction) { map("direction", outgoing, "label", label, "targetKey", targetKey)); String traversePattern = (outgoing ? "" : "<") + "-[:IS_A]-" + (outgoing ? ">" : ""); - { - List cats = db.executeTransactionally( - "MATCH (n) WITH n ORDER BY n.id MATCH (n)" + traversePattern - + "(cat:Letter) RETURN collect(cat.name) AS cats", - emptyMap(), - innerResult -> Iterators.single(innerResult.columnAs("cats"))); - assertThat(cats, equalTo(asList("A", "A", "C", "B", "C"))); + try (final var tx = db.beginTx(); + final var res = tx.execute("MATCH (n) WITH n ORDER BY n.id MATCH (n)" + traversePattern + + "(cat:Letter) RETURN collect(cat.name) AS cats")) { + assertThat(res.stream()).containsExactly(Map.of("cats", List.of("A", "A", "C", "B", "C"))); } - { - List cats = db.executeTransactionally( - "MATCH (n) WITH n ORDER BY n.id MATCH (n)" + traversePattern - + "(cat:Letter) RETURN collect(cat.k) AS cats", - emptyMap(), - innerResult -> Iterators.single(innerResult.columnAs("cats"))); - assertThat(cats, equalTo(asList("a", "a", "c", "b", "c"))); + try (final var tx = db.beginTx(); + final var res = tx.execute("MATCH (n) WITH n ORDER BY n.id MATCH (n)" + traversePattern + + "(cat:Letter) RETURN collect(cat.k) AS cats")) { + assertThat(res.stream()).containsExactly(Map.of("cats", List.of("a", "a", "c", "b", "c"))); } - testCall( - db, - "MATCH (n) WHERE n.prop IS NOT NULL RETURN count(n) AS count", - (r) -> assertThat(((Number) r.get("count")).longValue(), equalTo(0L))); + testCall(db, "MATCH (n) WHERE n.prop IS NOT NULL RETURN count(n) AS count", (r) -> assertThat(r) + .isEqualTo(Map.of("count", 0L))); db.executeTransactionally("DROP CONSTRAINT constraint"); } @@ -1272,7 +1263,7 @@ public void testRefactorCategorizeNoDups() { final long countries = TestUtil.singleResultFirstColumn(db, "MATCH (c:Country) RETURN count(c) AS countries"); assertEquals(2, countries); final List countryNames = TestUtil.firstColumn(db, "MATCH (c:Country) RETURN c.name"); - assertThat(countryNames, Matchers.containsInAnyOrder("IT", "DE")); + assertThat(countryNames).containsExactlyInAnyOrder("IT", "DE"); final long relsCount = TestUtil.singleResultFirstColumn( db, "MATCH p = (c:Company)-[:OPERATES_IN]->(cc:Country) RETURN count(p) AS relsCount"); @@ -1304,7 +1295,7 @@ public void testRefactorCategoryDoesntAllowCypherInjection() { final long countries = TestUtil.singleResultFirstColumn(db, "MATCH (c:Country) RETURN count(c) AS countries"); assertEquals(2, countries); final List countryNames = TestUtil.firstColumn(db, "MATCH (c:Country) RETURN c.name"); - assertThat(countryNames, Matchers.containsInAnyOrder("IT", "DE")); + assertThat(countryNames).containsExactlyInAnyOrder("IT", "DE"); final long relsCount = TestUtil.singleResultFirstColumn( db, @@ -1621,10 +1612,9 @@ public void testMergeNodeShouldCreateSelfRelationshipsWithPathWithOtherRels() { "match (a:One),(b:Two),(c:Three) with [a,b,c] as nodes CALL apoc.refactor.mergeNodes(nodes, {mergeRels: true}) yield node return node", (r) -> { Node node = (Node) r.get("node"); - List relNodeList = node.getRelationships().stream() + assertThat(node.getRelationships().stream()) .map(i -> i.getType().name()) - .collect(Collectors.toList()); - assertThat(relNodeList, Matchers.containsInAnyOrder("ASD", "QWE", "ZXC", "TEST_REL1", "TEST_REL2")); + .containsExactlyInAnyOrder("ASD", "QWE", "ZXC", "TEST_REL1", "TEST_REL2"); final Relationship relTestRel1 = node.getRelationships(RelationshipType.withName("TEST_REL1")) .iterator() @@ -1737,9 +1727,9 @@ public void issue2797WithCloneSubgraph() { } private void issue2797Common(String extractQuery) { - db.executeTransactionally(("CREATE CONSTRAINT unique_book FOR (book:MyBook) REQUIRE book.name IS UNIQUE")); - - db.executeTransactionally(("CREATE (n:MyBook {name: 1})")); + db.executeTransactionally("CREATE CONSTRAINT unique_book FOR (book:MyBook) REQUIRE book.name IS UNIQUE"); + db.executeTransactionally("CREATE (n:MyBook {name: 1})"); + db.executeTransactionally("CALL db.awaitIndexes()"); testCall(db, extractQuery, r -> { final String actualError = (String) r.get("error"); @@ -1747,10 +1737,10 @@ private void issue2797Common(String extractQuery) { assertNull(r.get("output")); }); - testCall(db, "MATCH (n:MyBook) RETURN properties(n) AS props", r -> { - final Map expected = Map.of("name", 1L); - assertEquals(expected, r.get("props")); - }); + try (final var tx = db.beginTx(); + final var res = tx.execute("MATCH (n:MyBook) RETURN properties(n) AS props")) { + assertThat(res.stream()).containsExactly(Map.of("props", Map.of("name", 1L))); + } testCall(db, "MATCH (n:MyBook) RETURN count(n) as count", r -> { assertEquals(1L, r.get("count")); diff --git a/core/src/test/resources/procedures.json b/core/src/test/resources/procedures.json index bcc913820..8f46da476 100644 --- a/core/src/test/resources/procedures.json +++ b/core/src/test/resources/procedures.json @@ -6510,7 +6510,7 @@ "type" : "LIST" }, { "name" : "config", - "description" : "{\n standinNodes :: LIST>,\n skipProperties :: LIST\n}\n", + "description" : "{\n standinNodes :: LIST>,\n skipProperties :: LIST,\n createNodesInNewTransactions = false :: BOOLEAN\n}\n", "isDeprecated" : false, "default" : "DefaultParameterValue{value={}, type=MAP}", "type" : "MAP" diff --git a/processor/build.gradle b/processor/build.gradle index 45691acae..491be0fa9 100644 --- a/processor/build.gradle +++ b/processor/build.gradle @@ -22,7 +22,7 @@ dependencies { api 'com.squareup:javapoet:1.13.0' testImplementation 'com.google.testing.compile:compile-testing:0.19' testImplementation group: 'junit', name: 'junit', version: '4.13.2' - testImplementation 'org.assertj:assertj-core:3.19.0' + testImplementation 'org.assertj:assertj-core:3.26.3' testImplementation 'org.mockito:mockito-core:4.2.0' }