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.

Sign up for GitHub

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

Add ability to test different cypher versions globally #725

Merged
merged 3 commits into from
Feb 3, 2025
Merged

Add ability to test different cypher versions globally #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
188ff80
Add ability to test different cypher versions globally
gem-neo4j Jan 29, 2025
55d4729
Missing procedures fix
gem-neo4j Jan 30, 2025
befb013
PR updates
gem-neo4j Feb 3, 2025
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
12 changes: 12 additions & 0 deletions common/src/main/java/apoc/util/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
import org.neo4j.graphdb.security.URLAccessChecker;
import org.neo4j.graphdb.security.URLAccessValidationError;
import org.neo4j.internal.schema.ConstraintDescriptor;
import org.neo4j.kernel.api.QueryLanguage;
import org.neo4j.kernel.impl.coreapi.InternalTransaction;
import org.neo4j.kernel.impl.util.ValueUtils;
import org.neo4j.kernel.internal.GraphDatabaseAPI;
Expand Down Expand Up @@ -1361,4 +1362,15 @@ public static <T> T withBackOffRetries(Supplier<T> func, long initialTimeout, lo
}
return result;
}

// Get the current supported query language versions, if this list changes
// this function will error, on error please update!
public static List<String> getCypherVersions() {
return QueryLanguage.ALL.stream()
.map(l -> switch (l) {
case QueryLanguage.CYPHER_5 -> "5";
case QueryLanguage.CYPHER_25 -> "25";
})
.toList();
}
}
12 changes: 7 additions & 5 deletions core/src/test/java/apoc/ArgumentDescriptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.neo4j.configuration.GraphDatabaseInternalSettings;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.cypher.internal.CypherVersion;
import org.neo4j.test.rule.DbmsRule;
Expand All @@ -117,8 +116,7 @@ public class ArgumentDescriptionsTest {

@Rule
public DbmsRule db = new ImpermanentDbmsRule()
.withSetting(GraphDatabaseSettings.procedure_unrestricted, singletonList("apoc.*"))
.withSetting(GraphDatabaseInternalSettings.enable_experimental_cypher_versions, true);
.withSetting(GraphDatabaseSettings.procedure_unrestricted, singletonList("apoc.*"));

@Before
public void setUp() {
Expand Down Expand Up @@ -202,9 +200,11 @@ public void teardown() {

@Test
public void functionArgumentDescriptionsDefaultVersion() throws IOException {
// TODO: When Cypher command for getting default is available use that here
// for now we just force this to use the preset default
assertResultAsJsonEquals(
CypherVersion.Default,
showFunctions(null),
showFunctions(CypherVersion.Default),
Copy link
Contributor

@loveleif loveleif Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

CypherVersion.Default is being removed. If you really need a default one you can use:

switch (GraphDatabaseSettings.default_langage.default()) {
  case Cypher5 => CypherVerision.Cypher5;
  case Cypher25 => CypherVerision.Cypher25;
}

"/functions/cypher%s/functions.json".formatted(CypherVersion.Default),
"/functions/common/functions.json");
}
Expand All @@ -223,9 +223,11 @@ public void functionArgumentDescriptions() throws IOException {

@Test
public void procedureArgumentDescriptionsDefaultVersion() throws IOException {
// TODO: When Cypher command for getting default is available use that here
// for now we just force this to use the preset default
assertResultAsJsonEquals(
CypherVersion.Default,
showProcedures(null),
showProcedures(CypherVersion.Default),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

"/procedures/cypher%s/procedures.json".formatted(CypherVersion.Default),
"/procedures/common/procedures.json");
}
Expand Down
29 changes: 22 additions & 7 deletions core/src/test/java/apoc/convert/ConvertJsonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ public void testToTreeIssue1685() {
testCall(
db,
"""
CYPHER 5
MATCH path = (k:Person {name:'Keanu Reeves'})-[*..5]-(x)
WITH collect(path) AS paths
CALL apoc.convert.toTree(paths)
Expand Down Expand Up @@ -486,6 +487,7 @@ public void testToTreeIssue2190() {
testCall(
db,
"""
CYPHER 5
MATCH(root:TreeNode) WHERE root.name = "root"
MATCH path = (root)-[cl:CHILD*]->(c:TreeNode)
WITH path, [r IN relationships(path) | r.order] AS orders
Expand All @@ -507,9 +509,11 @@ WITH COLLECT(path) AS paths
public void testToTree() {
testCall(
db,
"CREATE p1=(m:Movie {title:'M'})<-[:ACTED_IN {role:'R1'}]-(:Actor {name:'A1'}), "
+ " p2 = (m)<-[:ACTED_IN {role:'R2'}]-(:Actor {name:'A2'}) WITH [p1,p2] as paths "
+ " CALL apoc.convert.toTree(paths) YIELD value RETURN value",
"""
CYPHER 5 CREATE p1=(m:Movie {title:'M'})<-[:ACTED_IN {role:'R1'}]-(:Actor {name:'A1'}),
p2 = (m)<-[:ACTED_IN {role:'R2'}]-(:Actor {name:'A2'}) WITH [p1,p2] as paths
CALL apoc.convert.toTree(paths) YIELD value RETURN value
""",
(row) -> {
Map root = (Map) row.get("value");
assertEquals("Movie", root.get("_type"));
Expand All @@ -526,9 +530,11 @@ public void testToTree() {
public void testToTreeUpperCaseRels() {
testCall(
db,
"CREATE p1=(m:Movie {title:'M'})<-[:ACTED_IN {role:'R1'}]-(:Actor {name:'A1'}), "
+ " p2 = (m)<-[:ACTED_IN {role:'R2'}]-(:Actor {name:'A2'}) WITH [p1,p2] as paths "
+ " CALL apoc.convert.toTree(paths,false) YIELD value RETURN value",
"""
CYPHER 5
CREATE p1=(m:Movie {title:'M'})<-[:ACTED_IN {role:'R1'}]-(:Actor {name:'A1'}),
p2 = (m)<-[:ACTED_IN {role:'R2'}]-(:Actor {name:'A2'}) WITH [p1,p2] as paths
CALL apoc.convert.toTree(paths,false) YIELD value RETURN value""",
(row) -> {
Map root = (Map) row.get("value");
assertEquals("Movie", root.get("_type"));
Expand All @@ -543,7 +549,7 @@ public void testToTreeUpperCaseRels() {

@Test
public void testTreeOfEmptyList() {
testCall(db, " CALL apoc.convert.toTree([]) YIELD value RETURN value", (row) -> {
testCall(db, "CYPHER 5 CALL apoc.convert.toTree([]) YIELD value RETURN value", (row) -> {
Map root = (Map) row.get("value");
assertTrue(root.isEmpty());
});
Expand All @@ -561,6 +567,7 @@ public void testToTreeLeafNodes() {

String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -632,6 +639,7 @@ RETURN r.id as givenId, id(r) as id, elementId(r) as elementId

String call =
"""
CYPHER 5
MATCH (parent:Bib {id: '57523a6f-fda9-4a61-c4f6-08d47cdcf0cd'})
WITH parent
OPTIONAL MATCH childFlagPath=(parent)-[:HAS]->(:Comm)<-[:Flag]-(:User)
Expand Down Expand Up @@ -747,6 +755,7 @@ public void testToTreeLeafNodesWithConfigInclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -778,6 +787,7 @@ public void testToTreeLeafNodesWithConfigExclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -809,6 +819,7 @@ public void testToTreeLeafNodesWithConfigExcludeInclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -840,6 +851,7 @@ public void testToTreeLeafNodesWithConfigOnlyInclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -871,6 +883,7 @@ public void testToTreeLeafNodesWithConfigErrorInclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down Expand Up @@ -898,6 +911,7 @@ public void testToTreeDoesNotRemoveNonDuplicateRels() {

String query =
"""
CYPHER 5
MATCH p1 = (n:N {id:'n21'})-[e1]->(m1:N)
WITH COLLECT(p1) as paths
CALL apoc.convert.toTree(paths, false)
Expand Down Expand Up @@ -933,6 +947,7 @@ public void testToTreeLeafNodesWithConfigErrorExclude() {
statementForConfig(db);
String call =
"""
CYPHER 5
MATCH p=(n:Category)-[:subcategory*]->(m)
WHERE NOT (m)-[:subcategory]->() AND NOT ()-[:subcategory]->(n)
WITH COLLECT(p) AS ps
Expand Down
1 change: 1 addition & 0 deletions core/src/test/java/apoc/convert/PathsToJsonTreeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ public void testToTreeMultiLabelFiltersForOldProcedure() {

var query =
"""
CYPHER 5
MATCH path = (n)-[r]->(m)
WITH COLLECT(path) AS paths
CALL apoc.convert.toTree(paths, true, {nodes: { A: ['-nodeName'] } }) YIELD value AS tree
Expand Down
2 changes: 0 additions & 2 deletions core/src/test/java/apoc/cypher/CypherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
import org.neo4j.configuration.GraphDatabaseInternalSettings;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.QueryExecutionException;
Expand All @@ -81,7 +80,6 @@ public class CypherTest {
@ClassRule
public static DbmsRule db = new ImpermanentDbmsRule()
.withSetting(GraphDatabaseSettings.allow_file_urls, true)
.withSetting(GraphDatabaseInternalSettings.enable_experimental_cypher_versions, true)
.withSetting(
GraphDatabaseSettings.load_csv_file_url_root,
new File("src/test/resources").toPath().toAbsolutePath());
Expand Down
4 changes: 3 additions & 1 deletion core/src/test/java/apoc/export/ImportAndLoadCoreSecurityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static apoc.export.SecurityTestUtil.ALLOWED_EXCEPTIONS;
import static apoc.export.SecurityTestUtil.IMPORT_PROCEDURES;
import static apoc.export.SecurityTestUtil.LOAD_PROCEDURES;
import static apoc.export.SecurityTestUtil.cypher5OnlyProcedures;
import static apoc.export.SecurityTestUtil.setImportFileApocConfigs;
import static apoc.util.FileTestUtil.createTempFolder;
import static apoc.util.FileUtils.ACCESS_OUTSIDE_DIR_ERROR;
Expand Down Expand Up @@ -89,7 +90,8 @@ public class ImportAndLoadCoreSecurityTest {
private final String fileName;

public ImportAndLoadCoreSecurityTest(String method, String methodArguments, String fileName) {
this.apocProcedure = "CALL " + method + methodArguments;
var cypherVersion = cypher5OnlyProcedures.contains(method) ? "CYPHER 5 " : "";
this.apocProcedure = cypherVersion + "CALL " + method + methodArguments;
this.importMethod = method;
this.fileName = fileName;
}
Expand Down
2 changes: 2 additions & 0 deletions core/src/test/java/apoc/export/SecurityTestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public class SecurityTestUtil {
Pair.of("xml", "($fileName, '', {}, false)"),
Pair.of("arrow", "($fileName)"));

public static List<String> cypher5OnlyProcedures = List.of("apoc.load.arrow", "apoc.load.jsonParams");

public static void assertPathTraversalError(
GraphDatabaseService db, String query, Map<String, Object> params, Consumer<Map> exceptionConsumer) {

Expand Down
9 changes: 8 additions & 1 deletion core/src/test/java/apoc/export/arrow/ArrowTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,15 @@
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
import org.neo4j.configuration.GraphDatabaseInternalSettings;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.graphdb.Result;
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;

/**
* CYPHER 5 only; moved to extended for Cypher 25
*/
public class ArrowTest {

private static File directory = new File("target/arrow import");
Expand All @@ -60,7 +64,10 @@ public class ArrowTest {
public static DbmsRule db = new ImpermanentDbmsRule()
.withSetting(
GraphDatabaseSettings.load_csv_file_url_root,
directory.toPath().toAbsolutePath());
directory.toPath().toAbsolutePath())
.withSetting(
GraphDatabaseInternalSettings.default_cypher_version,
Copy link
Contributor

Choose a reason for hiding this comment

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

This setting is being replaced by GraphDatabaseSettings.default_language. Unfortunately that is not hooked up yet (PR), so go ahead and merge this and I sort it out later.

GraphDatabaseInternalSettings.CypherVersion.Cypher5);

public static final List<Map<String, Object>> EXPECTED = List.of(
new HashMap<>() {
Expand Down
9 changes: 8 additions & 1 deletion core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,15 @@
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.neo4j.configuration.GraphDatabaseInternalSettings;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.graphdb.QueryExecutionException;
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;

/**
* CYPHER 5 only; moved to extended for Cypher 25
*/
@RunWith(Enclosed.class)
public class ExportArrowSecurityTest {
public static final File directory = new File("target/import");
Expand All @@ -84,7 +88,10 @@ public class ExportArrowSecurityTest {
public static DbmsRule db = new ImpermanentDbmsRule()
.withSetting(
GraphDatabaseSettings.load_csv_file_url_root,
directory.toPath().toAbsolutePath());
directory.toPath().toAbsolutePath())
.withSetting(
GraphDatabaseInternalSettings.default_cypher_version,
GraphDatabaseInternalSettings.CypherVersion.Cypher5);

@BeforeClass
public static void setUp() {
Expand Down
40 changes: 34 additions & 6 deletions core/src/test/java/apoc/help/HelpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,39 +55,67 @@ public void teardown() {
}

@Test
public void info() {
TestUtil.testCall(db, "CALL apoc.help($text)", map("text", "bitwise"), (row) -> {
public void infoCypher5() {
TestUtil.testCall(db, "CYPHER 5 CALL apoc.help($text)", map("text", "bitwise"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.bitwise.op", row.get("name"));
assertTrue(((String) row.get("text")).contains("bitwise operation"));
assertFalse(((Boolean) row.get("isDeprecated")));
});
TestUtil.testCall(
db,
"CALL apoc.help($text)",
"CYPHER 5 CALL apoc.help($text)",
map("text", "operation+"),
(row) -> assertEquals("apoc.bitwise.op", row.get("name")));
TestUtil.testCall(db, "CALL apoc.help($text)", map("text", "toSet"), (row) -> {
TestUtil.testCall(db, "CYPHER 5 CALL apoc.help($text)", map("text", "toSet"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.coll.toSet", row.get("name"));
assertTrue(((String) row.get("text")).contains("unique `LIST<ANY>`"));
assertFalse(((Boolean) row.get("isDeprecated")));
});
TestUtil.testCall(db, "CALL apoc.help($text)", map("text", "diff.nodes"), (row) -> {
TestUtil.testCall(db, "CYPHER 5 CALL apoc.help($text)", map("text", "diff.nodes"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.diff.nodes", row.get("name"));
assertTrue(((String) row.get("text"))
.contains("Returns a `MAP` detailing the differences between the two given `NODE` values."));
assertFalse(((Boolean) row.get("isDeprecated")));
});
TestUtil.testCall(db, "CALL apoc.help($text)", map("text", "apoc.create.uuids"), (row) -> {
TestUtil.testCall(db, "CYPHER 5 CALL apoc.help($text)", map("text", "apoc.create.uuids"), (row) -> {
assertEquals("procedure", row.get("type"));
assertEquals("apoc.create.uuids", row.get("name"));
assertTrue(((String) row.get("text")).contains("Returns a stream of UUIDs."));
assertTrue(((Boolean) row.get("isDeprecated")));
});
}

@Test
public void infoCypher25() {
TestUtil.testCall(db, "CYPHER 25 CALL apoc.help($text)", map("text", "bitwise"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.bitwise.op", row.get("name"));
assertTrue(((String) row.get("text")).contains("bitwise operation"));
assertFalse(((Boolean) row.get("isDeprecated")));
});
TestUtil.testCall(
db,
"CYPHER 25 CALL apoc.help($text)",
map("text", "operation+"),
(row) -> assertEquals("apoc.bitwise.op", row.get("name")));
TestUtil.testCall(db, "CYPHER 25 CALL apoc.help($text)", map("text", "toSet"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.coll.toSet", row.get("name"));
assertTrue(((String) row.get("text")).contains("unique `LIST<ANY>`"));
assertFalse(((Boolean) row.get("isDeprecated")));
});
TestUtil.testCall(db, "CYPHER 25 CALL apoc.help($text)", map("text", "diff.nodes"), (row) -> {
assertEquals("function", row.get("type"));
assertEquals("apoc.diff.nodes", row.get("name"));
assertTrue(((String) row.get("text"))
.contains("Returns a `MAP` detailing the differences between the two given `NODE` values."));
assertFalse(((Boolean) row.get("isDeprecated")));
});
}

@Test
public void indicateCore() {
TestUtil.testCall(
Expand Down
Loading
Loading