Skip to content

Commit

Permalink
Nullable optional JSON-RPC parameters
Browse files Browse the repository at this point in the history
Fix command marshalling dropping params following params with nil value.

#1591 Allow specifying null parameter value from command line.
  • Loading branch information
lindlof authored and jcvernaleo committed Sep 8, 2020
1 parent bdab8df commit eb05726
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 5 deletions.
16 changes: 11 additions & 5 deletions btcjson/cmdparse.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,20 @@ import (
func makeParams(rt reflect.Type, rv reflect.Value) []interface{} {
numFields := rt.NumField()
params := make([]interface{}, 0, numFields)
lastParam := -1
for i := 0; i < numFields; i++ {
rtf := rt.Field(i)
rvf := rv.Field(i)
params = append(params, rvf.Interface())
if rtf.Type.Kind() == reflect.Ptr {
if rvf.IsNil() {
break
// Omit optional null params unless a non-null param follows
continue
}
rvf.Elem()
}
params = append(params, rvf.Interface())
lastParam = i
}

return params
return params[:lastParam+1]
}

// MarshalCmd marshals the passed command to a JSON-RPC request byte slice that
Expand Down Expand Up @@ -255,6 +256,11 @@ func assignField(paramNum int, fieldName string, dest reflect.Value, src reflect
return nil
}

// Optional variables can be set null using "null" string
if destIndirects > 0 && src.String() == "null" {
return nil
}

// When the destination has more indirects than the source, the extra
// pointers have to be created. Only create enough pointers to reach
// the same level of indirection as the source so the dest can simply be
Expand Down
74 changes: 74 additions & 0 deletions btcjson/cmdparse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,18 @@ func TestAssignField(t *testing.T) {
src: `{"1Address":1.5}`,
expected: map[string]float64{"1Address": 1.5},
},
{
name: `null optional field - "null" -> *int32`,
dest: btcjson.Int32(0),
src: "null",
expected: nil,
},
{
name: `null optional field - "null" -> *string`,
dest: btcjson.String(""),
src: "null",
expected: nil,
},
}

t.Logf("Running %d tests", len(tests))
Expand All @@ -175,6 +187,15 @@ func TestAssignField(t *testing.T) {
continue
}

// Check case where null string is used on optional field
if dst.Kind() == reflect.Ptr && test.src == "null" {
if !dst.IsNil() {
t.Errorf("Test #%d (%s) unexpected value - got %v, "+
"want nil", i, test.name, dst.Interface())
}
continue
}

// Inidirect through to the base types to ensure their values
// are the same.
for dst.Kind() == reflect.Ptr {
Expand Down Expand Up @@ -401,6 +422,59 @@ func TestNewCmdErrors(t *testing.T) {
}
}

// TestMarshalCmd tests the MarshalCmd function.
func TestMarshalCmd(t *testing.T) {
t.Parallel()

tests := []struct {
name string
id interface{}
cmd interface{}
expected string
}{
{
name: "include all parameters",
id: 1,
cmd: btcjson.NewGetNetworkHashPSCmd(btcjson.Int(100), btcjson.Int(2000)),
expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[100,2000],"id":1}`,
},
{
name: "include padding null parameter",
id: 1,
cmd: btcjson.NewGetNetworkHashPSCmd(nil, btcjson.Int(2000)),
expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[null,2000],"id":1}`,
},
{
name: "omit single unnecessary null parameter",
id: 1,
cmd: btcjson.NewGetNetworkHashPSCmd(btcjson.Int(100), nil),
expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[100],"id":1}`,
},
{
name: "omit unnecessary null parameters",
id: 1,
cmd: btcjson.NewGetNetworkHashPSCmd(nil, nil),
expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[],"id":1}`,
},
}

t.Logf("Running %d tests", len(tests))
for i, test := range tests {
bytes, err := btcjson.MarshalCmd(test.id, test.cmd)
if err != nil {
t.Errorf("Test #%d (%s) wrong error - got %T (%v)",
i, test.name, err, err)
continue
}
marshalled := string(bytes)
if marshalled != test.expected {
t.Errorf("Test #%d (%s) mismatched marshall result - got "+
"%v, want %v", i, test.name, marshalled, test.expected)
continue
}
}
}

// TestMarshalCmdErrors tests the error paths of the MarshalCmd function.
func TestMarshalCmdErrors(t *testing.T) {
t.Parallel()
Expand Down

0 comments on commit eb05726

Please sign in to comment.