From e942357de6320249083f5a714eff193b5d69bacc Mon Sep 17 00:00:00 2001 From: coffeepots Date: Fri, 23 Nov 2018 16:05:46 +0000 Subject: [PATCH 1/7] Fix iterating through return type --- json_rpc/jsonmarshal.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/json_rpc/jsonmarshal.nim b/json_rpc/jsonmarshal.nim index e9d9907..b048a5b 100644 --- a/json_rpc/jsonmarshal.nim +++ b/json_rpc/jsonmarshal.nim @@ -152,7 +152,7 @@ iterator paramsIter(params: NimNode): tuple[name, ntype: NimNode] = yield (arg[j], argType) iterator paramsRevIter(params: NimNode): tuple[name, ntype: NimNode] = - for i in countDown(params.len-1,0): + for i in countDown(params.len-1,1): let arg = params[i] let argType = arg[^2] for j in 0 ..< arg.len-2: From 51ed6f3000e5ce8624b25dec1361189954e5b3b8 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Fri, 23 Nov 2018 16:10:35 +0000 Subject: [PATCH 2/7] Remove expectType for Option[T], use fromJson on subtype for validation --- json_rpc/jsonmarshal.nim | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/json_rpc/jsonmarshal.nim b/json_rpc/jsonmarshal.nim index b048a5b..e7e35e0 100644 --- a/json_rpc/jsonmarshal.nim +++ b/json_rpc/jsonmarshal.nim @@ -3,27 +3,6 @@ import macros, json, options, typetraits proc expect*(actual, expected: JsonNodeKind, argName: string) = if actual != expected: raise newException(ValueError, "Parameter [" & argName & "] expected " & $expected & " but got " & $actual) -proc expectType*(actual: JsonNodeKind, expected: typedesc, argName: string, allowNull = false) = - var expType: JsonNodeKind - when expected is array: - expType = JArray - elif expected is object: - expType = JObject - elif expected is int: - expType = JInt - elif expected is float: - expType = JFloat - elif expected is bool: - expType = JBool - elif expected is string: - expType = JString - else: - const eStr = "Unable to convert " & expected.name & " to JSON for expectType" - {.fatal: eStr} - if actual != expType: - if allowNull == false or (allowNull and actual != JNull): - raise newException(ValueError, "Parameter [" & argName & "] expected " & expected.name & " but got " & $actual) - proc `%`*(n: byte{not lit}): JsonNode = result = newJInt(int(n)) @@ -54,13 +33,7 @@ proc fromJson(n: JsonNode, argName: string, result: var int64) proc fromJson(n: JsonNode, argName: string, result: var uint64) proc fromJson(n: JsonNode, argName: string, result: var ref int64) proc fromJson(n: JsonNode, argName: string, result: var ref int) - -proc fromJson[T](n: JsonNode, argName: string, result: var Option[T]) = - n.kind.expectType(T, argName, true) # Allow JNull - if n.kind != JNull: - var val: T - fromJson(n, argName, val) - result = some(val) +proc fromJson[T](n: JsonNode, argName: string, result: var Option[T]) # This can't be forward declared: https://github.com/nim-lang/Nim/issues/7868 proc fromJson[T: enum](n: JsonNode, argName: string, result: var T) = @@ -73,6 +46,13 @@ proc fromJson[T: object](n: JsonNode, argName: string, result: var T) = for k, v in fieldPairs(result): fromJson(n[k], k, v) +proc fromJson[T](n: JsonNode, argName: string, result: var Option[T]) = + # Allow JNull for options + if n.kind != JNull: + var val: T + fromJson(n, argName, val) + result = some(val) + proc fromJson(n: JsonNode, argName: string, result: var bool) = n.kind.expect(JBool, argName) result = n.getBool() From 066b06862dacc7adb935fc39e70a35efa2c55694 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Fri, 23 Nov 2018 16:11:12 +0000 Subject: [PATCH 3/7] Add test for Option[T] where T is not a builtin --- tests/testrpcmacro.nim | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index c5f6dbb..2219922 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -19,6 +19,9 @@ type MyOptional = object maybeInt: Option[int] + MyOptionalNotBuiltin = object + val: Option[Test2] + let testObj = %*{ "a": %1, @@ -94,6 +97,9 @@ s.rpc("rpc.mixedOptionalArg") do(a: int, b: Option[int], c: string, result.d = d result.e = e +s.rpc("rpc.optionalArgNotBuiltin") do(obj: Option[MyOptionalNotBuiltin]) -> string: + result = obj.get.val.get.y + # Tests suite "Server types": test "On macro registration": @@ -187,12 +193,23 @@ suite "Server types": check r1 == %int1 check r2 == %int2 - test "mixed optional arg": + test "Mixed optional arg": var ax = waitFor rpcMixedOptionalArg(%[%10, %11, %"hello", %12, %"world"]) check ax == %OptionalFields(a: 10, b: some(11), c: "hello", d: some(12), e: some("world")) var bx = waitFor rpcMixedOptionalArg(%[%10, newJNull(), %"hello"]) check bx == %OptionalFields(a: 10, c: "hello") + test "Non-built-in optional types": + let + testOptionalNonBuiltin = %*{ + "val": %*{ + "x": %[1, 2], + "y": %"Hello" + } + } + var r = waitFor rpcOptionalArgNotBuiltin(%[testOptionalNonBuiltin]) + check r == %"Hello" + s.stop() waitFor s.closeWait() From 0160295743e1b7bc047d1d3589fc1a92c594770e Mon Sep 17 00:00:00 2001 From: coffeepots Date: Fri, 23 Nov 2018 16:56:00 +0000 Subject: [PATCH 4/7] Improve non-builtin optional test --- tests/testrpcmacro.nim | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index 2219922..65b1bce 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -98,7 +98,12 @@ s.rpc("rpc.mixedOptionalArg") do(a: int, b: Option[int], c: string, result.e = e s.rpc("rpc.optionalArgNotBuiltin") do(obj: Option[MyOptionalNotBuiltin]) -> string: - result = obj.get.val.get.y + result = "Empty1" + if obj.isSome: + let val = obj.get.val + result = "Empty2" + if val.isSome: + result = obj.get.val.get.y # Tests suite "Server types": @@ -201,14 +206,15 @@ suite "Server types": test "Non-built-in optional types": let - testOptionalNonBuiltin = %*{ - "val": %*{ - "x": %[1, 2], - "y": %"Hello" - } - } - var r = waitFor rpcOptionalArgNotBuiltin(%[testOptionalNonBuiltin]) - check r == %"Hello" + t2 = Test2(x: [1, 2, 3], y: "Hello") + testOpts1 = MyOptionalNotBuiltin(val: some(t2)) + testOpts2 = MyOptionalNotBuiltin() + var r = waitFor rpcOptionalArgNotBuiltin(%[%testOpts1]) + check r == %t2.y + var r2 = waitFor rpcOptionalArgNotBuiltin(%[]) + check r2 == %"Empty1" + var r3 = waitFor rpcOptionalArgNotBuiltin(%[%testOpts2]) + check r3 == %"Empty2" s.stop() waitFor s.closeWait() From 798e86112c9972fb0d60f929bc17ed55a1281b68 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 26 Nov 2018 18:03:56 +0000 Subject: [PATCH 5/7] Cover optional cases within objects --- json_rpc/jsonmarshal.nim | 5 ++++- tests/testrpcmacro.nim | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/json_rpc/jsonmarshal.nim b/json_rpc/jsonmarshal.nim index e7e35e0..6c4bf03 100644 --- a/json_rpc/jsonmarshal.nim +++ b/json_rpc/jsonmarshal.nim @@ -44,7 +44,10 @@ proc fromJson[T: enum](n: JsonNode, argName: string, result: var T) = proc fromJson[T: object](n: JsonNode, argName: string, result: var T) = n.kind.expect(JObject, argName) for k, v in fieldPairs(result): - fromJson(n[k], k, v) + if v is Option and not n.hasKey(k): + fromJson(newJNull(), k, v) + else: + fromJson(n[k], k, v) proc fromJson[T](n: JsonNode, argName: string, result: var Option[T]) = # Allow JNull for options diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index 65b1bce..f578c10 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -105,6 +105,19 @@ s.rpc("rpc.optionalArgNotBuiltin") do(obj: Option[MyOptionalNotBuiltin]) -> stri if val.isSome: result = obj.get.val.get.y +type + MaybeOptions = object + o1: Option[bool] + o2: Option[bool] + o3: Option[bool] + +s.rpc("rpc.optInObj") do(data: string, options: Option[MaybeOptions]) -> int: + if options.isSome: + let o = options.get + if o.o1.isSome: result += 1 + if o.o2.isSome: result += 2 + if o.o3.isSome: result += 4 + # Tests suite "Server types": test "On macro registration": @@ -118,6 +131,9 @@ suite "Server types": check s.hasMethod("rpc.testreturns") check s.hasMethod("rpc.multivarsofonetype") check s.hasMethod("rpc.optionalArg") + check s.hasMethod("rpc.mixedOptionalArg") + check s.hasMethod("rpc.optionalArgNotBuiltin") + check s.hasMethod("rpc.traceTransaction") test "Simple paths": let r = waitFor rpcSimplePath(%[]) @@ -216,6 +232,12 @@ suite "Server types": var r3 = waitFor rpcOptionalArgNotBuiltin(%[%testOpts2]) check r3 == %"Empty2" + test "Manually set up JSON for optionals": + # Check manual set up json with optionals + let opts = parseJson("""{"o2": true}""") + var r = waitFor rpcOptInObj(%[%"0x31ded", opts]) + check r == %2 + s.stop() waitFor s.closeWait() From 77b485c6ce2e3f7a23752dcc5269b3f4bb968903 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 26 Nov 2018 18:12:26 +0000 Subject: [PATCH 6/7] More tests for optional fields inside objects --- tests/testrpcmacro.nim | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index f578c10..5043085 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -234,9 +234,25 @@ suite "Server types": test "Manually set up JSON for optionals": # Check manual set up json with optionals - let opts = parseJson("""{"o2": true}""") - var r = waitFor rpcOptInObj(%[%"0x31ded", opts]) - check r == %2 + let opts1 = parseJson("""{"o1": true}""") + var r1 = waitFor rpcOptInObj(%[%"0x31ded", opts1]) + check r1 == %1 + let opts2 = parseJson("""{"o2": true}""") + var r2 = waitFor rpcOptInObj(%[%"0x31ded", opts2]) + check r2 == %2 + let opts3 = parseJson("""{"o3": true}""") + var r3 = waitFor rpcOptInObj(%[%"0x31ded", opts3]) + check r3 == %4 + # Combinations + let opts4 = parseJson("""{"o1": true, "o3": true}""") + var r4 = waitFor rpcOptInObj(%[%"0x31ded", opts4]) + check r4 == %5 + let opts5 = parseJson("""{"o2": true, "o3": true}""") + var r5 = waitFor rpcOptInObj(%[%"0x31ded", opts5]) + check r5 == %6 + let opts6 = parseJson("""{"o1": true, "o2": true}""") + var r6 = waitFor rpcOptInObj(%[%"0x31ded", opts6]) + check r6 == %3 s.stop() waitFor s.closeWait() From e1fad2e3ba831fae8fbff72dacf8e2fc40ba8b60 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 26 Nov 2018 18:33:47 +0000 Subject: [PATCH 7/7] Fix test using wrong rpc path --- tests/testrpcmacro.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index 5043085..9d1a560 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -133,7 +133,7 @@ suite "Server types": check s.hasMethod("rpc.optionalArg") check s.hasMethod("rpc.mixedOptionalArg") check s.hasMethod("rpc.optionalArgNotBuiltin") - check s.hasMethod("rpc.traceTransaction") + check s.hasMethod("rpc.optInObj") test "Simple paths": let r = waitFor rpcSimplePath(%[])