From 51fadf850c162de823cedd1cb71ccef1692e0d8e Mon Sep 17 00:00:00 2001
From: jangko <jangko128@gmail.com>
Date: Sun, 13 Feb 2022 13:44:24 +0700
Subject: [PATCH] Fix #124

---
 json_rpc/jsonmarshal.nim | 21 ++++++++++++++-------
 tests/testrpcmacro.nim   | 30 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/json_rpc/jsonmarshal.nim b/json_rpc/jsonmarshal.nim
index ba509c7..5e602d6 100644
--- a/json_rpc/jsonmarshal.nim
+++ b/json_rpc/jsonmarshal.nim
@@ -243,26 +243,33 @@ proc jsonToNim*(params, jsonIdent: NimNode): NimNode =
         jsonElement = quote do:
           `jsonIdent`.elems[`pos`]
 
-      inc pos
       # declare variable before assignment
       result.add(quote do:
         var `paramIdent`: `paramType`
       )
 
+      # e.g. (A: int, B: Option[int], C: string, D: Option[int], E: Option[string])
       if paramType.isOptionalArg:
         let
-          nullAble  = pos < minLength
           innerType = paramType[1]
           innerNode = jsonToNim(paramIdent, innerType, jsonElement, paramName, true)
 
-        if nullAble:
+        if pos >= minLength:
+          # allow both empty and null after mandatory args
+          # D & E fall into this category
+          result.add(quote do:
+            if `jsonIdent`.len > `pos` and `jsonElement`.kind != JNull: `innerNode`
+          )
+        else:
+          # allow null param for optional args between/before mandatory args
+          # B fall into this category
           result.add(quote do:
             if `jsonElement`.kind != JNull: `innerNode`
           )
-        else:
-          result.add(quote do:
-            if `jsonIdent`.len >= `pos`: `innerNode`
-          )
       else:
+        # mandatory args
+        # A and C fall into this category
         # unpack Nim type and assign from json
         result.add jsonToNim(paramIdent, paramType, jsonElement, paramName)
+
+      inc pos
diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim
index ecc6059..543a906 100644
--- a/tests/testrpcmacro.nim
+++ b/tests/testrpcmacro.nim
@@ -79,6 +79,12 @@ s.rpc("rpc.optionalArg") do(val: int, obj: Option[MyOptional]) -> MyOptional:
   else:
     MyOptional(maybeInt: some(val))
 
+s.rpc("rpc.optionalArg2") do(a, b: string, c, d: Option[string]) -> string:
+  var ret = a & b
+  if c.isSome: ret.add c.get()
+  if d.isSome: ret.add d.get()
+  return ret
+
 type
   OptionalFields = object
     a: int
@@ -211,8 +217,32 @@ suite "Server types":
       int2 = MyOptional(maybeInt: some(117))
       r1 = waitFor s.executeMethod("rpc.optionalArg", %[%117, %int1])
       r2 = waitFor s.executeMethod("rpc.optionalArg", %[%117])
+      r3 = waitFor s.executeMethod("rpc.optionalArg", %[%117, newJNull()])
     check r1 == %int1
     check r2 == %int2
+    check r3 == %int2
+
+  test "Optional arg2":
+    let r1 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B"])
+    check r1 == %"AB"
+
+    let r2 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", newJNull()])
+    check r2 == %"AB"
+
+    let r3 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", newJNull(), newJNull()])
+    check r3 == %"AB"
+
+    let r4 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", newJNull(), %"D"])
+    check r4 == %"ABD"
+
+    let r5 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", %"C", %"D"])
+    check r5 == %"ABCD"
+
+    let r6 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", %"C", newJNull()])
+    check r6 == %"ABC"
+
+    let r7 = waitFor s.executeMethod("rpc.optionalArg2", %[%"A", %"B", %"C"])
+    check r7 == %"ABC"
 
   test "Mixed optional arg":
     var ax = waitFor s.executeMethod("rpc.mixedOptionalArg", %[%10, %11, %"hello", %12, %"world"])