Skip to content

Commit a066155

Browse files
authored
[tYbuDF2B] The apoc.export.cypher.* procedures don't create relationships with start/end node without properties (#181)
* Fixes #13: The apoc.export.cypher.* procedures don't create relationships with start/end node without properties * fix tests * small changes review
1 parent 6bb8434 commit a066155

File tree

4 files changed

+191
-17
lines changed

4 files changed

+191
-17
lines changed

common/src/main/java/apoc/export/cypher/formatter/AbstractCypherFormatter.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,13 @@ protected String mergeStatementForNode(CypherFormat cypherFormat, Node node, Map
111111
StringBuilder result = new StringBuilder(1000);
112112
result.append("MERGE ");
113113
result.append(CypherFormatterUtils.formatNodeLookup("n", node, uniqueConstraints, indexNames));
114-
if (node.getPropertyKeys().iterator().hasNext()) {
115-
String notUniqueProperties = CypherFormatterUtils.formatNotUniqueProperties("n", node, uniqueConstraints, indexedProperties, false);
116-
String notUniqueLabels = CypherFormatterUtils.formatNotUniqueLabels("n", node, uniqueConstraints);
117-
if (!"".equals(notUniqueProperties) || !"".equals(notUniqueLabels)) {
118-
result.append(cypherFormat.equals(CypherFormat.ADD_STRUCTURE) ? " ON CREATE SET " : " SET ");
119-
result.append(notUniqueProperties);
120-
result.append(!"".equals(notUniqueProperties) && !"".equals(notUniqueLabels) ? ", " : "");
121-
result.append(notUniqueLabels);
122-
}
114+
String notUniqueProperties = CypherFormatterUtils.formatNotUniqueProperties("n", node, uniqueConstraints, indexedProperties, false);
115+
String notUniqueLabels = CypherFormatterUtils.formatNotUniqueLabels("n", node, uniqueConstraints);
116+
if (!notUniqueProperties.isEmpty() || !notUniqueLabels.isEmpty()) {
117+
result.append(cypherFormat.equals(CypherFormat.ADD_STRUCTURE) ? " ON CREATE SET " : " SET ");
118+
result.append(notUniqueProperties);
119+
result.append(!"".equals(notUniqueProperties) && !"".equals(notUniqueLabels) ? ", " : "");
120+
result.append(notUniqueLabels);
123121
}
124122
result.append(";");
125123
return result.toString();

common/src/main/java/apoc/export/cypher/formatter/CreateCypherFormatter.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,9 @@ public String statementForNode(Node node, Map<String, Set<String>> uniqueConstra
2727
if (!labels.isEmpty()) {
2828
result.append(labels);
2929
}
30-
if (node.getPropertyKeys().iterator().hasNext()) {
31-
result.append(" {");
32-
result.append(CypherFormatterUtils.formatNodeProperties("", node, uniqueConstraints, indexNames, true));
33-
result.append("}");
34-
}
30+
result.append(" {");
31+
result.append(CypherFormatterUtils.formatNodeProperties("", node, uniqueConstraints, indexNames, true));
32+
result.append("}");
3533
result.append(");");
3634
return result.toString();
3735
}

common/src/main/java/apoc/export/cypher/formatter/CypherFormatterUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ public static StringBuilder formatProperties(Map<String, Object> properties) {
186186

187187
public static StringBuilder formatProperties(String id, Map<String, Object> properties, boolean jsonStyle) {
188188
StringBuilder result = new StringBuilder(100);
189-
if (properties != null) {
189+
if (properties != null && !properties.isEmpty()) {
190190
List<String> keys = Iterables.asList(properties.keySet());
191191
Collections.sort(keys);
192192
for (String prop : keys) {

core/src/test/java/apoc/export/cypher/ExportCypherTest.java

Lines changed: 180 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package apoc.export.cypher;
22

3+
import apoc.export.util.ExportConfig;
34
import apoc.graph.Graphs;
5+
import apoc.schema.Schemas;
46
import apoc.util.BinaryTestUtil;
57
import apoc.util.CompressionAlgo;
68
import apoc.util.TestUtil;
@@ -12,6 +14,8 @@
1214
import org.junit.rules.TestName;
1315
import org.neo4j.configuration.GraphDatabaseSettings;
1416
import org.neo4j.graphdb.QueryExecutionException;
17+
import org.neo4j.graphdb.Relationship;
18+
import org.neo4j.graphdb.ResourceIterator;
1519
import org.neo4j.test.rule.DbmsRule;
1620
import org.neo4j.test.rule.ImpermanentDbmsRule;
1721

@@ -28,7 +32,10 @@
2832
import static apoc.util.BinaryTestUtil.getDecompressedData;
2933
import static apoc.util.Util.map;
3034
import static java.nio.charset.StandardCharsets.UTF_8;
31-
import static org.junit.Assert.*;
35+
import static org.junit.Assert.assertEquals;
36+
import static org.junit.Assert.assertFalse;
37+
import static org.junit.Assert.assertNull;
38+
import static org.junit.Assert.assertTrue;
3239
import static org.neo4j.configuration.SettingImpl.newBuilder;
3340
import static org.neo4j.configuration.SettingValueParsers.BOOL;
3441

@@ -61,7 +68,7 @@ public class ExportCypherTest {
6168
@Before
6269
public void setUp() throws Exception {
6370
apocConfig().setProperty(APOC_EXPORT_FILE_ENABLED, true);
64-
TestUtil.registerProcedure(db, ExportCypher.class, Graphs.class);
71+
TestUtil.registerProcedure(db, ExportCypher.class, Graphs.class, Schemas.class);
6572
db.executeTransactionally("CREATE RANGE INDEX barIndex FOR (n:Bar) ON (n.first_name, n.last_name)");
6673
db.executeTransactionally("CREATE RANGE INDEX fooIndex FOR (n:Foo) ON (n.name)");
6774
db.executeTransactionally("CREATE CONSTRAINT uniqueConstraint FOR (b:Bar) REQUIRE b.name IS UNIQUE");
@@ -845,6 +852,133 @@ public void shouldSaveCorrectlyRelIndexesWithNameOptimized() throws FileNotFound
845852
db.executeTransactionally("DROP INDEX rel_index_name");
846853
}
847854

855+
@Test
856+
public void testIssue2886OptimizationsNoneAndCypherFormatCreate() {
857+
String expected = "CREATE (:Person:`UNIQUE IMPORT LABEL` {name:\"First\", `UNIQUE IMPORT ID`:3});\n" +
858+
"CREATE (:Project:`UNIQUE IMPORT LABEL` {`UNIQUE IMPORT ID`:4});\n" +
859+
"CREATE (:Person:`UNIQUE IMPORT LABEL` {name:\"Second\", `UNIQUE IMPORT ID`:5});\n" +
860+
"CREATE (:Project:`UNIQUE IMPORT LABEL` {`UNIQUE IMPORT ID`:6});\n" +
861+
EXPECTED_2886_SCHEMA +
862+
EXPECTED_2886_RELS_WITHOUT_OPTIMIZATION +
863+
EXPECTED_2886_CLEANUP;
864+
865+
final Map<String, Object> config = map("cypherFormat", "create");
866+
issue2886Common(expected, withoutOptimization(config), true);
867+
}
868+
869+
@Test
870+
public void testIssue2886OptimizationsNoneAndCypherFormatAddStructure() {
871+
String expected = "MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:3}) ON CREATE SET n.name=\"First\", n:Person;\n" +
872+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:4}) ON CREATE SET n:Project;\n" +
873+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:5}) ON CREATE SET n.name=\"Second\", n:Person;\n" +
874+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:6}) ON CREATE SET n:Project;\n" +
875+
EXPECTED_2886_RELS_WITHOUT_OPTIMIZATION;
876+
final Map<String, Object> config = map("cypherFormat", "addStructure");
877+
issue2886Common(expected, withoutOptimization(config), true);
878+
}
879+
880+
@Test
881+
public void testIssue2886OptimizationsNoneAndCypherFormatUpdateStructure() {
882+
String expected = "MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:3}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:4}) MERGE (n1)-[r:WORKS_FOR]->(n2) ON CREATE SET r.id=1;\n" +
883+
"MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:5}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:6}) MERGE (n1)-[r:WORKS_FOR]->(n2) ON CREATE SET r.id=2;\n";
884+
final Map<String, Object> config = map("cypherFormat", "updateStructure");
885+
issue2886Common(expected, withoutOptimization(config), false);
886+
}
887+
888+
@Test
889+
public void testIssue2886OptimizationsNoneAndCypherFormatUpdateAll() {
890+
String expected = "MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:3}) SET n.name=\"First\", n:Person;\n" +
891+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:4}) SET n:Project;\n" +
892+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:5}) SET n.name=\"Second\", n:Person;\n" +
893+
"MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:6}) SET n:Project;\n" +
894+
EXPECTED_2886_SCHEMA +
895+
"MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:3}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:4}) MERGE (n1)-[r:WORKS_FOR]->(n2) SET r.id=1;\n" +
896+
"MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:5}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:6}) MERGE (n1)-[r:WORKS_FOR]->(n2) SET r.id=2;\n" +
897+
EXPECTED_2886_CLEANUP;
898+
final Map<String, Object> config = map("cypherFormat", "updateAll");
899+
issue2886Common(expected, withoutOptimization(config), true);
900+
}
901+
902+
@Test
903+
public void testIssue2886CypherFormatCreate() {
904+
final Map<String, Object> config = map("cypherFormat", "create");
905+
String expected = String.format(EXPECTED_2886_UNWIND, "CREATE");
906+
issue2886Common(expected, config, true);
907+
}
908+
909+
@Test
910+
public void testIssue2886CypherFormatAddStructure() {
911+
final Map<String, Object> config = map("cypherFormat", "addStructure");
912+
issue2886Common(EXPECTED_2886_ADD_STRUCTURE, config, true);
913+
}
914+
915+
@Test
916+
public void testIssue2886CypherFormatUpdateStructure() {
917+
final Map<String, Object> config = map("cypherFormat", "updateStructure");
918+
final String expected = String.format(EXPECTED_2886_UPDATE_STRUCTURE, "MERGE");
919+
issue2886Common(expected, config, false);
920+
}
921+
922+
@Test
923+
public void testIssue2886CypherFormatUpdateAll() {
924+
final Map<String, Object> config = map("cypherFormat", "updateAll");
925+
String expected = String.format(EXPECTED_2886_UNWIND, "MERGE");
926+
issue2886Common(expected, config, true);
927+
}
928+
929+
private Map<String, Object> withoutOptimization(Map<String, Object> map) {
930+
map.put("useOptimizations", map("type", ExportConfig.OptimizationType.NONE.name()));
931+
return map;
932+
}
933+
934+
private void issue2886Common(String expected, Map<String, Object> config, boolean isRountrip) {
935+
db.executeTransactionally("match (n) detach delete n");
936+
937+
final String startOne = "First";
938+
final long relOne = 1L;
939+
final String startTwo = "Second";
940+
final long relTwo = 2L;
941+
db.executeTransactionally("create (:Person {name: $name})-[:WORKS_FOR {id: $id}]->(:Project)",
942+
map("name", startOne, "id", relOne));
943+
db.executeTransactionally("create (:Person {name: $name})-[:WORKS_FOR {id: $id}]->(:Project)",
944+
map("name", startTwo, "id", relTwo));
945+
946+
final Map<String, Object> conf = map("format", "plain");
947+
conf.putAll(config);
948+
final String cypherStatements = db.executeTransactionally("CALL apoc.export.cypher.all(null, $config)",
949+
map("config", conf),
950+
r -> (String) r.next().get("cypherStatements"));
951+
952+
beforeAfterIssue2886(startOne, relOne, startTwo, relTwo);
953+
954+
assertEquals(expected, cypherStatements);
955+
956+
if (!isRountrip) {
957+
return;
958+
}
959+
db.executeTransactionally("match (n) detach delete n");
960+
db.executeTransactionally("call apoc.schema.assert({}, {})");
961+
962+
for (String i: cypherStatements.split(";\n")) {
963+
db.executeTransactionally(i);
964+
}
965+
966+
beforeAfterIssue2886(startOne, relOne, startTwo, relTwo);
967+
}
968+
969+
private void beforeAfterIssue2886(String startOne, long relOne, String startTwo, long relTwo) {
970+
TestUtil.testResult(db, "MATCH (start:Person)-[rel:WORKS_FOR]->(end:Project) RETURN rel ORDER BY rel.id", r -> {
971+
final ResourceIterator<Relationship> rels = r.columnAs("rel");
972+
Relationship rel = rels.next();
973+
assertEquals(startOne, rel.getStartNode().getProperty("name"));
974+
assertEquals(relOne, rel.getProperty("id"));
975+
rel = rels.next();
976+
assertEquals(startTwo, rel.getStartNode().getProperty("name"));
977+
assertEquals(relTwo, rel.getProperty("id"));
978+
assertFalse(rels.hasNext());
979+
});
980+
}
981+
848982
private void relIndexTestCommon(String fileName, String expectedSchema, Map<String, Object> config) throws FileNotFoundException {
849983
Map<String, Object> exportConfig = map("separateFiles", true, "format", "neo4j-shell");
850984
exportConfig.putAll(config);
@@ -1410,6 +1544,50 @@ static class ExportCypherResults {
14101544
"MATCH (start:Person{surname: row.start.surname, name: row.start.name})%n" +
14111545
"MATCH (end:Person{surname: row.end.surname, name: row.end.name})%n" +
14121546
"CREATE (start)-[r:KNOWS]->(end) SET r += row.properties;%n");
1547+
1548+
static final String EXPECTED_2886_SCHEMA = """
1549+
CREATE RANGE INDEX FOR (n:Bar) ON (n.first_name, n.last_name);
1550+
CREATE RANGE INDEX FOR (n:Foo) ON (n.name);
1551+
CREATE CONSTRAINT uniqueConstraint FOR (node:Bar) REQUIRE (node.name) IS UNIQUE;
1552+
CREATE CONSTRAINT UNIQUE_IMPORT_NAME FOR (node:`UNIQUE IMPORT LABEL`) REQUIRE (node.`UNIQUE IMPORT ID`) IS UNIQUE;
1553+
""";
1554+
1555+
static final String EXPECTED_2886_CLEANUP = """
1556+
MATCH (n:`UNIQUE IMPORT LABEL`) WITH n LIMIT 20000 REMOVE n:`UNIQUE IMPORT LABEL` REMOVE n.`UNIQUE IMPORT ID`;
1557+
DROP CONSTRAINT UNIQUE_IMPORT_NAME;
1558+
""";
1559+
1560+
static final String EXPECTED_2886_UPDATE_STRUCTURE = """
1561+
UNWIND [{start: {_id:3}, end: {_id:4}, properties:{id:1}}, {start: {_id:5}, end: {_id:6}, properties:{id:2}}] AS row
1562+
MATCH (start:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row.start._id})
1563+
MATCH (end:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row.end._id})
1564+
%1$s (start)-[r:WORKS_FOR]->(end) SET r += row.properties;
1565+
""";
1566+
1567+
static final String EXPECTED_2886_UNWIND = EXPECTED_2886_SCHEMA +
1568+
"UNWIND [{_id:4, properties:{}}, {_id:6, properties:{}}] AS row\n" +
1569+
"%1$s (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row._id}) SET n += row.properties SET n:Project;\n" +
1570+
"UNWIND [{_id:3, properties:{name:\"First\"}}, {_id:5, properties:{name:\"Second\"}}] AS row\n" +
1571+
"%1$s (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row._id}) SET n += row.properties SET n:Person;\n" +
1572+
EXPECTED_2886_UPDATE_STRUCTURE +
1573+
EXPECTED_2886_CLEANUP;
1574+
1575+
static final String EXPECTED_2886_ADD_STRUCTURE = """
1576+
UNWIND [{_id:4, properties:{}}, {_id:6, properties:{}}] AS row
1577+
MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row._id}) ON CREATE SET n += row.properties SET n:Project;
1578+
UNWIND [{_id:3, properties:{name:"First"}}, {_id:5, properties:{name:"Second"}}] AS row
1579+
MERGE (n:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row._id}) ON CREATE SET n += row.properties SET n:Person;
1580+
UNWIND [{start: {_id:3}, end: {_id:4}, properties:{id:1}}, {start: {_id:5}, end: {_id:6}, properties:{id:2}}] AS row
1581+
MATCH (start:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row.start._id})
1582+
MATCH (end:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`: row.end._id})
1583+
CREATE (start)-[r:WORKS_FOR]->(end) SET r += row.properties;
1584+
""";
1585+
1586+
static final String EXPECTED_2886_RELS_WITHOUT_OPTIMIZATION = """
1587+
MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:3}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:4}) CREATE (n1)-[r:WORKS_FOR {id:1}]->(n2);
1588+
MATCH (n1:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:5}), (n2:`UNIQUE IMPORT LABEL`{`UNIQUE IMPORT ID`:6}) CREATE (n1)-[r:WORKS_FOR {id:2}]->(n2);
1589+
""";
1590+
14131591
}
14141592

14151593
}

0 commit comments

Comments
 (0)