Skip to content

Commit

Permalink
Solve ci 5.13 errors (#3804)
Browse files Browse the repository at this point in the history
* Solve flaky test 5.12 (#3783)

* Autoclose neo4jContainers in CoreExtendedTest

* decrease maxHeapSize consistently to Core

* Cast (InternalTransaction) consistent with core ones

* generalized testWrongType assertion

* Changed apoc.custom procs due to latest 5.13 neo4j changes

* Changes apoc.custom review

* Removed unused import
  • Loading branch information
vga91 authored Oct 25, 2023
1 parent 0dde8d5 commit 4f542a7
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 34 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ subprojects {
'neo4jCommunityDockerImage': System.getProperty("NEO4JVERSION") ? 'neo4j:' + System.getProperty("NEO4JVERSION") + '-debian' : 'neo4j:5.13.0',
'coreDir': 'apoc-core/core'

maxHeapSize = "8G"
maxHeapSize = "5G"
forkEvery = 50
maxParallelForks = 1
minHeapSize = "128m"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ you have to quote the result, e.g `CALL apoc.custom.declareProcedure("procWithNu

.Type Names
The `typeParam` and `typeResult` in the signature parameter can be one of the following values:
* FLOAT, DOUBLE, INT, INTEGER, NUMBER, LONG

* FLOAT, DOUBLE, INT, INTEGER, INTEGER | FLOAT, NUMBER, LONG
* TEXT, STRING
* BOOL, BOOLEAN
* POINT, GEO, GEOMETRY
Expand All @@ -37,6 +38,7 @@ The `typeParam` and `typeResult` in the signature parameter can be one of the fo
* LIST TYPE, LIST OF TYPE (where `TYPE` can be one of the previous values)
* ANY

NOTE: In Neo4j 5.13, the `NUMBER` type was replaced by `INTEGER | FLOAT`, but we can still use it for backwards compatibility.

NOTE: If you override procedures or functions you might need to call `call db.clearQueryCaches()` as lookups to internal id's are kept in compiled query plans.

Expand Down Expand Up @@ -75,8 +77,8 @@ The output will look like the following table:
[%autowidth,opts=header]
|===
| type | name | description | mode | statement | inputs | outputs | forceSingle
| "function" | "answer" | <null> | <null> | "RETURN $input as answer" | [["input","number"]] | "long" | false
| "procedure" | "answer" | "Procedure that answer to the Ultimate Question of Life, the Universe, and Everything" | "read" | "RETURN $input as answer" | [["input","int","42"]] | [["answer","number"]] | <null>
| "function" | "answer" | <null> | <null> | "RETURN $input as answer" | [["input","integer \| float"]] | "long" | false
| "procedure" | "answer" | "Procedure that answer to the Ultimate Question of Life, the Universe, and Everything" | "read" | "RETURN $input as answer" | [["input","int","42"]] | [["answer","integer \| float"]] | <null>
|===


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ CALL apoc.custom.list();
[opts="header"]
|===
| type | name | description | mode | statement | inputs | outputs | forceSingle
| "function" | "double" | "" | NULL | "RETURN $input*2 as answer" | [["input", "number"]] | "integer" | FALSE
| "function" | "double" | "" | NULL | "RETURN $input*2 as answer" | [["input", "integer \| float"]] | "integer" | FALSE
|===
2 changes: 1 addition & 1 deletion extended/src/main/antlr/apoc/custom/Signature.g4
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ defaultValue: value;

list_type: 'LIST''?'?' OF '+opt_type ;
opt_type: base_type'?'? ;
base_type: 'MAP' | 'ANY' | 'NODE' | 'REL' | 'RELATIONSHIP' | 'EDGE' | 'PATH' | 'NUMBER' | 'LONG' | 'INT' | 'INTEGER' | 'FLOAT' | 'DOUBLE' | 'BOOL' | 'BOOLEAN' | 'DATE' | 'TIME' | 'LOCALTIME' | 'DATETIME' | 'LOCALDATETIME' | 'DURATION' | 'POINT' | 'GEO' | 'GEOMETRY' | 'STRING' | 'TEXT' ;
base_type: 'MAP' | 'ANY' | 'NODE' | 'REL' | 'RELATIONSHIP' | 'EDGE' | 'PATH' | 'NUMBER' | 'INTEGER | FLOAT' | 'LONG' | 'INT' | 'INTEGER' | 'FLOAT' | 'DOUBLE' | 'BOOL' | 'BOOLEAN' | 'DATE' | 'TIME' | 'LOCALTIME' | 'DATETIME' | 'LOCALDATETIME' | 'DURATION' | 'POINT' | 'GEO' | 'GEOMETRY' | 'STRING' | 'TEXT' ;
NEWLINE: [\r\n]+ ;
QUOTED_IDENTIFIER: '`' [^`]+? '`' ;
IDENTIFIER: [a-zA-Z_][a-zA-Z0-9_]+ ;
Expand Down
16 changes: 15 additions & 1 deletion extended/src/main/java/apoc/custom/CypherProceduresHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.stream.Stream;

import static apoc.ApocConfig.apocConfig;
import static apoc.custom.Signatures.NUMBER_TYPE;
import static apoc.util.MapUtil.map;
import static java.util.Collections.singletonList;
import static org.neo4j.internal.kernel.api.procs.Neo4jTypes.AnyType;
Expand All @@ -82,7 +83,6 @@
import static org.neo4j.internal.kernel.api.procs.Neo4jTypes.NTTime;

public class CypherProceduresHandler extends LifecycleAdapter implements AvailabilityListener {

public static final String PREFIX = "custom";
public static final String FUNCTION = "function";
public static final String PROCEDURE = "procedure";
Expand Down Expand Up @@ -461,6 +461,10 @@ private static Neo4jTypes.AnyType typeof(String typeName) {
typeName = typeName.toUpperCase();
if (typeName.startsWith("LIST OF ")) return NTList(typeof(typeName.substring(8)));
if (typeName.startsWith("LIST ")) return NTList(typeof(typeName.substring(5)));
if (typeName.startsWith("LIST<") && typeName.endsWith(">")) {
AnyType typeof = typeof(typeName.substring(5, typeName.length() - 1));
return NTList(typeof);
}
switch (typeName) {
case "ANY":
return NTAny;
Expand All @@ -477,6 +481,7 @@ private static Neo4jTypes.AnyType typeof(String typeName) {
case "PATH":
return NTPath;
case "NUMBER":
case NUMBER_TYPE:
return NTNumber;
case "LONG":
return NTInteger;
Expand All @@ -495,12 +500,16 @@ private static Neo4jTypes.AnyType typeof(String typeName) {
case "DATE":
return NTDate;
case "TIME":
case "ZONED TIME":
return NTTime;
case "LOCALTIME":
case "LOCAL TIME":
return NTLocalTime;
case "DATETIME":
case "ZONED DATETIME":
return NTDateTime;
case "LOCALDATETIME":
case "LOCAL DATETIME":
return NTLocalDateTime;
case "DURATION":
return NTDuration;
Expand Down Expand Up @@ -536,6 +545,7 @@ private DefaultParameterValue defaultValue(String typeName, String stringValue)
case "PATH":
return null;
case "NUMBER":
case NUMBER_TYPE:
return value instanceof Float || value instanceof Double ? DefaultParameterValue.ntFloat(((Number) value).doubleValue()) : DefaultParameterValue.ntInteger(((Number) value).longValue());
case "LONG":
case "INT":
Expand All @@ -553,6 +563,10 @@ private DefaultParameterValue defaultValue(String typeName, String stringValue)
case "DATETIME":
case "LOCALDATETIME":
case "DURATION":
case "ZONED TIME":
case "LOCAL TIME":
case "ZONED DATETIME":
case "LOCAL DATETIME":
case "POINT":
case "GEO":
case "GEOMETRY":
Expand Down
7 changes: 6 additions & 1 deletion extended/src/main/java/apoc/custom/Signatures.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import static org.neo4j.internal.kernel.api.procs.Neo4jTypes.*;

public class Signatures {

public static final String NUMBER_TYPE = "INTEGER | FLOAT";
public static final String SIGNATURE_SYNTAX_ERROR = "Syntax error(s) in signature definition %s. " +
"\nNote that procedure/function name, possible map keys, input and output names must have at least 2 character:\n";
private final String prefix;
Expand Down Expand Up @@ -206,6 +206,7 @@ private Neo4jTypes.AnyType type(SignatureParser.Opt_typeContext opt_type) {
case "PATH":
return NTPath;
case "NUMBER":
case NUMBER_TYPE:
return NTNumber;
case "LONG":
return NTInteger;
Expand All @@ -224,12 +225,16 @@ private Neo4jTypes.AnyType type(SignatureParser.Opt_typeContext opt_type) {
case "DATE":
return NTDate;
case "TIME":
case "ZONED TIME":
return NTTime;
case "LOCALTIME":
case "LOCAL TIME":
return NTLocalTime;
case "DATETIME":
case "ZONED DATETIME":
return NTDateTime;
case "LOCALDATETIME":
case "LOCAL DATETIME":
return NTLocalDateTime;
case "DURATION":
return NTDuration;
Expand Down
5 changes: 3 additions & 2 deletions extended/src/main/java/apoc/get/GetProcedures.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import apoc.result.NodeResult;
import apoc.result.RelationshipResult;
import org.neo4j.graphdb.Transaction;
import org.neo4j.kernel.impl.coreapi.InternalTransaction;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Name;
Expand All @@ -20,13 +21,13 @@ public class GetProcedures {
@Procedure
@Description("apoc.get.nodes(node|id|[ids]) - quickly returns all nodes with these id's")
public Stream<NodeResult> nodes(@Name("nodes") Object ids) {
return new Get(tx).nodes(ids);
return new Get((InternalTransaction) tx).nodes(ids);
}

@Procedure
@Description("apoc.get.rels(rel|id|[ids]) - quickly returns all relationships with these id's")
public Stream<RelationshipResult> rels(@Name("relationships") Object ids) {
return new Get(tx).rels(ids);
return new Get((InternalTransaction) tx).rels(ids);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public List<Pair<String, String>> export(Node node, ProgressReporter progressRep
? (String) node.getProperty(outputName)
: getSignature(node, ExtendedSystemPropertyKeys.outputs.name());

String statement = String.format("CALL apoc.custom.declareFunction('%s(%s) :: (%s)', '%s', %s, '%s');",
String statement = String.format("CALL apoc.custom.declareFunction('%s(%s) :: %s', '%s', %s, '%s');",
node.getProperty(SystemPropertyKeys.name.name()), inputs, outputs,
node.getProperty(SystemPropertyKeys.statement.name()),
node.getProperty(ExtendedSystemPropertyKeys.forceSingle.name()),
Expand Down
19 changes: 7 additions & 12 deletions extended/src/test/java/apoc/CoreExtendedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@
public class CoreExtendedTest {
@Test
public void checkForCoreAndExtended() {
try {
Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(ApocPackage.CORE, ApocPackage.EXTENDED), true)
.withNeo4jConfig("dbms.transaction.timeout", "60s")
.withEnv(APOC_IMPORT_FILE_ENABLED, "true");

try(Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(ApocPackage.CORE, ApocPackage.EXTENDED), true)
.withNeo4jConfig("dbms.transaction.timeout", "60s")
.withEnv(APOC_IMPORT_FILE_ENABLED, "true")) {
neo4jContainer.start();

Session session = neo4jContainer.getSession();
Expand All @@ -41,7 +39,6 @@ public void checkForCoreAndExtended() {
assertTrue(coreCount > 0);
assertTrue(extendedCount > 0);

neo4jContainer.close();
} catch (Exception ex) {
if (TestContainerUtil.isDockerImageAvailable(ex)) {
ex.printStackTrace();
Expand All @@ -52,10 +49,9 @@ public void checkForCoreAndExtended() {

@Test
public void matchesSpreadsheet() {
try {
Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.CORE, TestContainerUtil.ApocPackage.EXTENDED), true)
.withNeo4jConfig("dbms.transaction.timeout", "60s")
.withEnv(APOC_IMPORT_FILE_ENABLED, "true");
try(Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.CORE, TestContainerUtil.ApocPackage.EXTENDED), true)
.withNeo4jConfig("dbms.transaction.timeout", "60s")
.withEnv(APOC_IMPORT_FILE_ENABLED, "true")) {

neo4jContainer.start();

Expand All @@ -78,8 +74,7 @@ public void matchesSpreadsheet() {
List<Map.Entry<String, String>> different = spreadsheet.entrySet().stream().filter(entry -> actual.containsKey(entry.getKey()) && !actual.get(entry.getKey()).equals(entry.getValue())).collect(Collectors.toList());

assertEquals(different.toString(), 0, different.size());

neo4jContainer.close();

} catch (Exception ex) {
if (TestContainerUtil.isDockerImageAvailable(ex)) {
ex.printStackTrace();
Expand Down
5 changes: 4 additions & 1 deletion extended/src/test/java/apoc/coll/CollExtendedTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package apoc.coll;

import apoc.util.TestUtil;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
Expand Down Expand Up @@ -68,7 +70,8 @@ private void testWrongType(String query) {
try {
testCall(db, query, row -> fail("should fail due to Wrong argument type"));
} catch (RuntimeException e) {
assertEquals("Wrong argument type: Can't coerce `Long(1)` to Duration", e.getMessage());
String expected = "Can't coerce `Long(1)` to Duration";
MatcherAssert.assertThat(e.getMessage(), Matchers.containsString(expected));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,8 @@ public void registerConcreteParameterAndReturnStatement() {
@Test
public void testAllParameterTypes() {
db.executeTransactionally("CALL apoc.custom.declareProcedure('answer(int::INTEGER, float::FLOAT,string::STRING,map::MAP,listInt::LIST OF INTEGER,bool::BOOLEAN,date::DATE,datetime::DATETIME,point::POINT) :: (data::LIST OF ANY)','RETURN [$int,$float,$string,$map,$listInt,$bool,$date,$datetime,$point] as data')");

TestUtil.testCall(db, "call custom.answer(42,3.14,'foo',{a:1},[1],true,date(),datetime(),point({x:1,y:2}))",
(row) -> assertEquals(9, ((List) row.get("data")).size()));
restartDb();
TestUtil.testCall(db, "call custom.answer(42,3.14,'foo',{a:1},[1],true,date(),datetime(),point({x:1,y:2}))",
(row) -> assertEquals(9, ((List) row.get("data")).size()));
Expand Down Expand Up @@ -370,6 +371,11 @@ public void registerConcreteParameterAndReturnStatementFunction() {
@Test
public void testAllParameterTypesFunction() {
db.executeTransactionally("CALL apoc.custom.declareFunction('answer(int::INTEGER, float::FLOAT,string::STRING,map::MAP,listInt::LIST OF INTEGER,bool::BOOLEAN,date::DATE,datetime::DATETIME,point::POINT) :: LIST OF ANY','RETURN [$int,$float,$string,$map,$listInt,$bool,$date,$datetime,$point] as data')");
TestUtil.testCall(db, "return custom.answer(42,3.14,'foo',{a:1},[1],true,date(),datetime(),point({x:1,y:2})) as data",
(row) -> {
System.out.println(row);
assertEquals(9, ((List<List>) row.get("data")).get(0).size());
});
restartDb();
TestUtil.testCall(db, "return custom.answer(42,3.14,'foo',{a:1},[1],true,date(),datetime(),point({x:1,y:2})) as data",
(row) -> {
Expand Down Expand Up @@ -523,9 +529,9 @@ private void functionAssertions() {
TestUtil.testResult(db, "SHOW FUNCTIONS YIELD signature, name WHERE name STARTS WITH 'custom.sumFun' RETURN DISTINCT name, signature ORDER BY name",
r -> {
Map<String, Object> row = r.next();
assertEquals("custom.sumFun1(input1 = null :: INTEGER?, input2 = null :: INTEGER?) :: (INTEGER?)", row.get("signature"));
assertEquals("custom.sumFun1(input1 = null :: INTEGER, input2 = null :: INTEGER) :: INTEGER", row.get("signature"));
row = r.next();
assertEquals("custom.sumFun2(input1 :: INTEGER?, input2 :: INTEGER?) :: (INTEGER?)", row.get("signature"));
assertEquals("custom.sumFun2(input1 :: INTEGER, input2 :: INTEGER) :: INTEGER", row.get("signature"));
assertFalse(r.hasNext());
});
TestUtil.testResult(db, "call apoc.custom.list() YIELD name RETURN name ORDER BY name",
Expand Down Expand Up @@ -555,9 +561,9 @@ private void procedureAssertions() {
TestUtil.testResult(db, "SHOW PROCEDURES YIELD signature, name WHERE name STARTS WITH 'custom.sum' RETURN DISTINCT name, signature ORDER BY name",
r -> {
Map<String, Object> row = r.next();
assertEquals("custom.sum1(input1 = null :: INTEGER?, input2 = null :: INTEGER?) :: (answer :: INTEGER?)", row.get("signature"));
assertEquals("custom.sum1(input1 = null :: INTEGER, input2 = null :: INTEGER) :: (answer :: INTEGER)", row.get("signature"));
row = r.next();
assertEquals("custom.sum2(input1 :: INTEGER?, input2 :: INTEGER?) :: (answer :: INTEGER?)", row.get("signature"));
assertEquals("custom.sum2(input1 :: INTEGER, input2 :: INTEGER) :: (answer :: INTEGER)", row.get("signature"));
assertFalse(r.hasNext());
});
TestUtil.testResult(db, "call apoc.custom.list() YIELD name RETURN name ORDER BY name",
Expand Down
7 changes: 4 additions & 3 deletions extended/src/test/java/apoc/custom/CypherProceduresTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import static apoc.custom.CypherProceduresHandler.FUNCTION;
import static apoc.custom.CypherProceduresHandler.PROCEDURE;
import static apoc.custom.Signatures.NUMBER_TYPE;
import static apoc.custom.Signatures.SIGNATURE_SYNTAX_ERROR;
import static apoc.util.TestUtil.testCall;
import static java.util.Arrays.asList;
Expand Down Expand Up @@ -180,10 +181,10 @@ public void shouldListAllProceduresAndFunctions() throws Exception {
Map<String, Object> value = row.next();
assertTrue(value.containsKey("type"));
assertTrue(FUNCTION.equals(value.get("type")) || PROCEDURE.equals(value.get("type")));

if(PROCEDURE.equals(value.get("type"))){
assertEquals("answer", value.get("name"));
assertEquals(asList(asList("answer", "number")), value.get("outputs"));
assertEquals(asList(asList("answer", NUMBER_TYPE.toLowerCase())), value.get("outputs"));
assertEquals(asList(asList("input", "integer", "42")), value.get("inputs"));
assertEquals("Procedure that answer to the Ultimate Question of Life, the Universe, and Everything", value.get("description").toString());
assertNull(value.get("forceSingle"));
Expand All @@ -193,7 +194,7 @@ public void shouldListAllProceduresAndFunctions() throws Exception {
if(FUNCTION.equals(value.get("type"))){
assertEquals("answer", value.get("name"));
assertEquals("integer", value.get("outputs"));
assertEquals(asList(asList("input", "number")), value.get("inputs"));
assertEquals(asList(asList("input", NUMBER_TYPE.toLowerCase())), value.get("inputs"));
assertEquals("", value.get("description"));
assertFalse((Boolean) value.get("forceSingle"));
assertNull(value.get("mode"));
Expand Down
Loading

0 comments on commit 4f542a7

Please sign in to comment.