Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable optional JSON-RPC parameters #1594

Merged
merged 1 commit into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't see this invocation having any effect so it was removed to clean it up. It doesn't modify rvf in any way and it shouldn't panic here because field type is pointer.

}
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