Merge pull request #43 from status-im/OptionsFixes

Options fixes

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) =
@@ -71,7 +44,17 @@ 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
+  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)
@@ -152,7 +135,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:

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,27 @@ 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 = "Empty1"
+  if obj.isSome:
+    let val = obj.get.val
+    result = "Empty2"
+    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":
@@ -107,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.optInObj")
 
   test "Simple paths":
     let r = waitFor rpcSimplePath(%[])
@@ -187,12 +214,46 @@ 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
+      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"
+
+  test "Manually set up JSON for optionals":
+    # Check manual set up json with optionals
+    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()