From 9b86e09eba0706d00c13237cb084a424bb30b786 Mon Sep 17 00:00:00 2001 From: Giuseppe Villani Date: Tue, 29 Aug 2023 18:30:27 +0200 Subject: [PATCH] [NOID] Fixes #2978: The apoc.custom.declareProcedure does not accept default string values (#3036) (#3753) * [NOID] Fixes #2978: The apoc.custom.declareProcedure does not accept default string values * [NOID] removed unused imports * [NOID] changed parseInt/Float to correct parseLong/Double --- .../cypher-based-procedures-functions.adoc | 3 + .../src/main/antlr/apoc/custom/Signature.g4 | 7 +- .../src/main/java/apoc/custom/Signatures.java | 45 +++- .../apoc/custom/CypherProceduresTest.java | 213 ++++++++++++++++++ 4 files changed, 257 insertions(+), 11 deletions(-) diff --git a/docs/asciidoc/modules/ROOT/pages/cypher-execution/cypher-based-procedures-functions.adoc b/docs/asciidoc/modules/ROOT/pages/cypher-execution/cypher-based-procedures-functions.adoc index a44b9bf9bc..2026f76a75 100644 --- a/docs/asciidoc/modules/ROOT/pages/cypher-execution/cypher-based-procedures-functions.adoc +++ b/docs/asciidoc/modules/ROOT/pages/cypher-execution/cypher-based-procedures-functions.adoc @@ -22,6 +22,9 @@ That is: Note that, for both procedures and functions, the `= defaultValue` are optionals. The default values are parsed as JSON. +NOTE: If you want to create a procedure/function with a default `String` parameter with whitespaces, quotes (for example: `"my text ' with ' quote"`) or `"null"` (as a string), +you have to quote the result, e.g `CALL apoc.custom.declareProcedure("procWithNullString(param='null'::STRING)::(output::STRING)", 'return $param as output')` + .Type Names The `typeParam` and `typeResult` in the signature parameter can be one of the following values: * FLOAT, DOUBLE, INT, INTEGER, NUMBER, LONG diff --git a/extended/src/main/antlr/apoc/custom/Signature.g4 b/extended/src/main/antlr/apoc/custom/Signature.g4 index 7363f43ac1..f5f7c7bb45 100644 --- a/extended/src/main/antlr/apoc/custom/Signature.g4 +++ b/extended/src/main/antlr/apoc/custom/Signature.g4 @@ -22,9 +22,10 @@ value: nullValue | INT_VALUE | FLOAT_VALUE | boolValue | mapValue | listValue | INT_VALUE: [0-9]+; FLOAT_VALUE: ([0-9]+'.'[0-9]+) | 'NaN'; boolValue: 'true'|'false'; -stringValue: QUOTED_STRING_VALUE | PLAIN_STRING_VALUE; -QUOTED_STRING_VALUE: '"'[^"]+?'"'; -PLAIN_STRING_VALUE: .+?; +stringValue: SINGLE_QUOTED_STRING_VALUE | QUOTED_STRING_VALUE | plainStringValue; +SINGLE_QUOTED_STRING_VALUE: '\'' (~'\'')+ '\''; +QUOTED_STRING_VALUE: '"' (~'"')+ '"'; +plainStringValue: (~'{' | ~'}' | ~'[' | ~']' | ~':')+?; nullValue: 'null'; listValue: '[' ((value',')*value)?']'; mapValue: '{' (((name ':' value)',')*(name ':' value) | ((name '=' value)',')*(name '=' value))? '}'; diff --git a/extended/src/main/java/apoc/custom/Signatures.java b/extended/src/main/java/apoc/custom/Signatures.java index 5ef81dc637..79fa7c657c 100644 --- a/extended/src/main/java/apoc/custom/Signatures.java +++ b/extended/src/main/java/apoc/custom/Signatures.java @@ -9,6 +9,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import java.util.stream.Collectors; import static org.neo4j.internal.kernel.api.procs.Neo4jTypes.*; @@ -16,7 +17,7 @@ public class Signatures { public static final String SIGNATURE_SYNTAX_ERROR = "Syntax error(s) in signature definition %s. " + - "\nNote that procedure/function name, input and output names must have at least 2 character:\n"; + "\nNote that procedure/function name, possible map keys, input and output names must have at least 2 character:\n"; private final String prefix; public Signatures(String prefix) { @@ -126,23 +127,51 @@ private DefaultParameterValue defaultValue(SignatureParser.DefaultValueContext d return DefaultParameterValue.nullValue(type); if (v.boolValue() != null) return DefaultParameterValue.ntBoolean(Boolean.parseBoolean(v.boolValue().getText())); - if (v.stringValue() != null) - return DefaultParameterValue.ntString(v.stringValue().getText()); - if (v.INT_VALUE() != null) - return DefaultParameterValue.ntInteger(Integer.parseInt(v.INT_VALUE().getText())); - if (v.FLOAT_VALUE() != null) - return DefaultParameterValue.ntFloat(Float.parseFloat(v.FLOAT_VALUE().getText())); + final SignatureParser.StringValueContext stringCxt = v.stringValue(); + if (stringCxt != null) { + + String text = stringCxt.getText(); + if (stringCxt.SINGLE_QUOTED_STRING_VALUE() != null || stringCxt.QUOTED_STRING_VALUE() != null) { + text = text.substring(1, text.length() - 1); + } + return DefaultParameterValue.ntString(text); + } + if (v.INT_VALUE() != null) { + final String text = v.INT_VALUE().getText(); + return getDefaultParameterValue(type, text, () -> DefaultParameterValue.ntInteger(Long.parseLong(text))); + } + if (v.FLOAT_VALUE() != null) { + final String text = v.FLOAT_VALUE().getText(); + return getDefaultParameterValue(type, text, () -> DefaultParameterValue.ntFloat(Double.parseDouble(text))); + } if (v.mapValue() != null) { Map map = JsonUtil.parse(v.mapValue().getText(), null, Map.class); return DefaultParameterValue.ntMap(map); } if (v.listValue() != null) { List list = JsonUtil.parse(v.listValue().getText(), null, List.class); - return DefaultParameterValue.ntList(list, ((Neo4jTypes.ListType) type).innerType()); + final AnyType inner = ((ListType) type).innerType(); + if (inner instanceof TextType) { + list = list.stream() + .map(String::valueOf) + .collect(Collectors.toList()); + } + return DefaultParameterValue.ntList(list, inner); + } return DefaultParameterValue.nullValue(type); } + private DefaultParameterValue getDefaultParameterValue(AnyType type, String text, Supplier fun) { + // to differentiate e.g. null (nullValue) from null as a plain string, or 1 (integer) from 1 as a plain text + // we have to obtain the actual data type from type. + // Otherwise we could we can remove the possibility of having plainText string and explicit them via quotes/double-quotes + // or document that null/numbers/boolean as a plain string are not possible. + return type instanceof TextType + ? DefaultParameterValue.ntString(text) + : fun.get(); + } + public String name(SignatureParser.NameContext ns) { if(ns == null) throw new IllegalStateException("Unsupported procedure name, the procedure must have at least two chars"); if (ns.IDENTIFIER() != null) return ns.IDENTIFIER().getText(); diff --git a/extended/src/test/java/apoc/custom/CypherProceduresTest.java b/extended/src/test/java/apoc/custom/CypherProceduresTest.java index 3dc86f0cf3..68c6131ffd 100644 --- a/extended/src/test/java/apoc/custom/CypherProceduresTest.java +++ b/extended/src/test/java/apoc/custom/CypherProceduresTest.java @@ -489,6 +489,219 @@ public void testIssue2605() { }); } + @Test + public void shouldDeclareProcedureWithDefaultListAndMaps() { + db.executeTransactionally("call apoc.custom.declareProcedure('procWithFloatList(minScore = [1.1,2.2,3.3] :: LIST OF FLOAT) :: (res :: BOOLEAN, first :: FLOAT)',\n" + + " 'return size($minScore) < 4 as res, $minScore[0] as first')"); + testCall(db, "call custom.procWithFloatList", (row) -> { + assertEquals(true, row.get("res")); + assertEquals(1.1D, (double) row.get("first"), 0.1D); + }); + testCall(db, "call custom.procWithFloatList([9.1, 2.6, 3.1, 4.3, 5.5])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals(9.1D, (double) row.get("first"), 0.1D); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithIntList(minScore = [1,2,3] :: LIST OF INT) :: (res :: BOOLEAN, first :: FLOAT)',\n" + + " 'return size($minScore) < 4 as res, toInteger($minScore[0]) as first')"); + testCall(db, "call custom.procWithIntList", (row) -> { + assertEquals(true, row.get("res")); + assertEquals(1L, row.get("first")); + }); + testCall(db, "call custom.procWithIntList([9,2,3,4,5])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals(9L, row.get("first")); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithListString(minScore = [\"1\",\"2\",\"3\"] :: LIST OF STRING) :: (res :: BOOLEAN, first :: FLOAT)',\n" + + " 'return size($minScore) < 4 as res, $minScore[0] + \" - suffix\" as first ')"); + testCall(db, "call custom.procWithListString", (row) -> { + assertEquals(true, row.get("res")); + assertEquals("1 - suffix", row.get("first")); + }); + testCall(db, "call custom.procWithListString(['aaa','bbb','ccc','ddd','eee'])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals("aaa - suffix", row.get("first")); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithListPlainString(minScore = [1, 2, 3] :: LIST OF STRING) :: (res :: BOOLEAN, first :: FLOAT)',\n" + + " 'return size($minScore) < 4 as res, $minScore[0] + \" - suffix\" as first ')"); + testCall(db, "call custom.procWithListPlainString", (row) -> { + assertEquals(true, row.get("res")); + assertEquals("1 - suffix", row.get("first")); + }); + testCall(db, "call custom.procWithListPlainString(['aaa','bbb','ccc','ddd','eee'])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals("aaa - suffix", row.get("first")); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure(\"procWithListStringQuoted(minScore = ['1','2','3'] :: LIST OF STRING) :: (res :: BOOLEAN, first :: FLOAT)\",\n" + + " 'return size($minScore) < 4 as res, $minScore[0] + \" - suffix\" as first ')"); + testCall(db, "call custom.procWithListStringQuoted", (row) -> { + assertEquals(true, row.get("res")); + assertEquals("1 - suffix", row.get("first")); + }); + testCall(db, "call custom.procWithListStringQuoted(['aaa','bbb','ccc','ddd','eee'])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals("aaa - suffix", row.get("first")); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithListStringVars(minScore = [true,false,null] :: LIST OF STRING) :: (res :: BOOLEAN, first :: STRING)',\n" + + " 'return size($minScore) < 4 as res, $minScore[0] as first ')"); + testCall(db, "call custom.procWithListStringVars", (row) -> { + assertEquals(true, row.get("res")); + assertEquals("true", row.get("first")); + }); + testCall(db, "call custom.procWithListStringVars(['aaa','bbb','ccc','ddd','eee'])", (row) -> { + assertEquals(false, row.get("res")); + assertEquals("aaa", row.get("first")); + }); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithMapList(minScore = {aa: 1, bb: \"2\"} :: MAP) :: (res :: MAP, first :: ANY)',\n" + + " 'return $minScore as res, $minScore[\"a\"] as first ')"); + testCall(db, "call custom.procWithMapList", (row) -> { + assertEquals(Map.of("aa", 1L, "bb", "2"), row.get("res")); + }); + testCall(db, "call custom.procWithMapList({c: true})", (row) -> { + assertEquals(Map.of("c", true), row.get("res")); + }); + } + + @Test + public void shouldDeclareFunctionWithDefaultListAndMaps() { + db.executeTransactionally("call apoc.custom.declareFunction('funWithFloatList(minScore = [1.1,2.2,3.3] :: LIST OF FLOAT) :: FLOAT',\n" + + " 'return $minScore[0]')"); + testCall(db, "RETURN custom.funWithFloatList() AS res", + (row) -> assertEquals(1.1D, (double) row.get("res"), 0.1D)); + testCall(db, "RETURN custom.funWithFloatList([9.1, 2.6, 3.1, 4.3, 5.5]) AS res", + (row) -> assertEquals(9.1D, (double) row.get("res"), 0.1D)); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithIntList(minScore = [1,2,3] :: LIST OF INT) :: BOOLEAN',\n" + + " 'return size($minScore) < 4')"); + testCall(db, "RETURN custom.funWithIntList() AS res", + (row) -> assertEquals(true, row.get("res"))); + testCall(db, "RETURN custom.funWithIntList([9,2,3,4,5]) AS res", + (row) -> assertEquals(false, row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithListString(minScore = [\"1\",\"2\",\"3\"] :: LIST OF STRING) :: BOOLEAN',\n" + + " 'return size($minScore) < 4')"); + testCall(db, "RETURN custom.funWithListString() AS res", + (row) -> assertEquals(true, row.get("res"))); + testCall(db, "RETURN custom.funWithListString(['aaa','bbb','ccc','ddd','eee']) AS res", + (row) -> assertEquals(false, row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithListStringPlain(minScore = [1, 2, 3] :: LIST OF STRING) :: BOOLEAN',\n" + + " 'return size($minScore) < 4')"); + testCall(db, "RETURN custom.funWithListStringPlain() AS res", + (row) -> assertEquals(true, row.get("res"))); + testCall(db, "RETURN custom.funWithListStringPlain(['aaa','bbb','ccc','ddd','eee']) AS res", + (row) -> assertEquals(false, row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction(\"funWithListStringQuoted(minScore = ['1','2','3'] :: LIST OF STRING) :: BOOLEAN\",\n" + + " 'return size($minScore) < 4')"); + testCall(db, "RETURN custom.funWithListStringQuoted() AS res", + (row) -> assertEquals(true, row.get("res"))); + testCall(db, "RETURN custom.funWithListStringQuoted(['aaa','bbb','ccc','ddd','eee']) AS res", + (row) -> assertEquals(false, row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithListStringVars(minScore = [true,false,null] :: LIST OF STRING) :: BOOLEAN',\n" + + " 'return size($minScore) < 4')"); + testCall(db, "RETURN custom.funWithListStringVars() AS res", + (row) -> assertEquals(true, row.get("res"))); + testCall(db, "RETURN custom.funWithListStringVars(['aaa','bbb','ccc','ddd','eee']) AS res", + (row) -> assertEquals(false, row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithMapList(minScore = {aa: 1, bb: \"2\"} :: MAP) :: MAP',\n" + + " 'return $minScore AS mapRes')"); + testCall(db, "RETURN custom.funWithMapList() AS res", + (row) -> assertEquals(Map.of("mapRes", Map.of("aa", 1L, "bb", "2")), row.get("res"))); + testCall(db, "RETURN custom.funWithMapList({c: true}) AS res", + (row) -> assertEquals(Map.of("mapRes", Map.of("c", true)), row.get("res"))); + } + + @Test + public void shouldDeclareProcedureWithDefaultString() { + String query = "RETURN $minScore + ' - suffix' as res"; + db.executeTransactionally("CALL apoc.custom.declareProcedure(\"procWithSingleQuotedText(minScore=' foo \\\" bar '::STRING)::(res::STRING)\", $query)", + Map.of("query", query)); + testCall(db, "CALL custom.procWithSingleQuotedText", (row) -> { + assertEquals(" foo \" bar - suffix", row.get("res")); + }); + + db.executeTransactionally("CALL apoc.custom.declareProcedure('procWithDoubleQuotedText(minScore=\" foo \\' bar \"::STRING) :: (res::STRING)', $query)", + Map.of("query", query)); + testCall(db, "CALL custom.procWithDoubleQuotedText", (row) -> { + assertEquals(" foo ' bar - suffix", row.get("res")); + }); + testCall(db, "CALL custom.procWithDoubleQuotedText('myText')", (row) -> assertEquals("myText - suffix", row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareProcedure('procWithPlainText(minScore = plainText :: STRING) :: (res::STRING)', $query)", + Map.of("query", query)); + testCall(db, "CALL custom.procWithPlainText", (row) -> assertEquals("plainText - suffix", row.get("res"))); + testCall(db, "CALL custom.procWithPlainText('myText')", (row) -> assertEquals("myText - suffix", row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareProcedure('procWithStringNull(minScore = null :: STRING) :: (res :: STRING)', $query)", + Map.of("query", query)); + testCall(db, "CALL custom.procWithStringNull", (row) -> assertNull(row.get("res"))); + testCall(db, "CALL custom.procWithStringNull('other')", (row) -> assertEquals("other - suffix", row.get("res"))); + } + + @Test + public void shouldDeclareFunctionWithDefaultString() { + String query = "RETURN $minScore + ' - suffix' as res"; + db.executeTransactionally("CALL apoc.custom.declareFunction(\"funWithSingleQuotedText(minScore=' foo \\\" bar '::STRING):: STRING\", $query)", + Map.of("query", query)); + testCall(db, "RETURN custom.funWithSingleQuotedText() AS res", (row) -> { + assertEquals(" foo \" bar - suffix", row.get("res")); + }); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithDoubleQuotedText(minScore=\" foo \\' bar \"::STRING) :: STRING', $query)", + Map.of("query", query)); + testCall(db, "RETURN custom.funWithDoubleQuotedText() AS res", (row) -> { + assertEquals(" foo ' bar - suffix", row.get("res")); + }); + testCall(db, "RETURN custom.funWithDoubleQuotedText('myText') AS res", (row) -> assertEquals("myText - suffix", row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithPlainText(minScore = plainText :: STRING) :: STRING', $query)", + Map.of("query", query)); + testCall(db, "RETURN custom.funWithPlainText() AS res", (row) -> assertEquals("plainText - suffix", row.get("res"))); + testCall(db, "RETURN custom.funWithPlainText('myText') AS res", (row) -> assertEquals("myText - suffix", row.get("res"))); + + db.executeTransactionally("CALL apoc.custom.declareFunction('funWithStringNull(minScore = null :: STRING) :: STRING', $query)", + Map.of("query", query)); + testCall(db, "RETURN custom.funWithStringNull() AS res", (row) -> assertNull(row.get("res"))); + testCall(db, "RETURN custom.funWithStringNull('other') AS res", (row) -> assertEquals("other - suffix", row.get("res"))); + } + + @Test + public void shouldDeclareProcedureWithDefaultBooleanOrNull() { + db.executeTransactionally("call apoc.custom.declareProcedure('procWithBool(minScore = true :: BOOLEAN) :: (res :: INT)',\n" + + " 'RETURN case when $minScore then 1 else 2 end as res')"); + testCall(db, "call custom.procWithBool", (row) -> assertEquals(1L, row.get("res"))); + testCall(db, "call custom.procWithBool(true)", (row) -> assertEquals(1L, row.get("res"))); + testCall(db, "call custom.procWithBool(false)", (row) -> assertEquals(2L, row.get("res"))); + + db.executeTransactionally("call apoc.custom.declareProcedure('procWithNull(minScore = null :: INT) :: (res :: INT)',\n" + + " 'RETURN $minScore as res')"); + testCall(db, "call custom.procWithNull", (row) -> assertNull(row.get("res"))); + testCall(db, "call custom.procWithNull(1)", (row) -> assertEquals(1L, row.get("res"))); + } + + @Test + public void shouldDeclareFunctionWithDefaultBooleanOrNull() { + db.executeTransactionally("call apoc.custom.declareFunction('funWithBool(minScore = true :: BOOLEAN) :: INT',\n" + + " 'RETURN case when $minScore then 1 else 2 end as res')"); + testCall(db, "RETURN custom.funWithBool() AS res", (row) -> assertEquals(1L, row.get("res"))); + testCall(db, "RETURN custom.funWithBool(true) AS res", (row) -> assertEquals(1L, row.get("res"))); + testCall(db, "RETURN custom.funWithBool(false) AS res", (row) -> assertEquals(2L, row.get("res"))); + + db.executeTransactionally("call apoc.custom.declareFunction('funWithNull(minScore = null :: INT) :: INT',\n" + + " 'RETURN $minScore as res')"); + testCall(db, "RETURN custom.funWithNull() AS res", (row) -> assertNull(row.get("res"))); + testCall(db, "RETURN custom.funWithNull(1) AS res", (row) -> assertEquals(1L, row.get("res"))); + + } + @Test public void shouldFailDeclareFunctionWithDefaultNumberParameters() { final String query = "RETURN $base * $exp AS res";