Skip to content

Commit

Permalink
RPC request can omit params if the handler's only argument is a conte…
Browse files Browse the repository at this point in the history
…xt (#1265)
  • Loading branch information
joshklop authored Sep 25, 2023
1 parent 807ca34 commit 219b89d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 28 deletions.
41 changes: 16 additions & 25 deletions jsonrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,17 @@ func (s *Server) handleRequest(ctx context.Context, req *request) (*response, er
}

func (s *Server) buildArguments(ctx context.Context, params any, method Method) ([]reflect.Value, error) {
handlerType := reflect.TypeOf(method.Handler)

numArgs := handlerType.NumIn()
args := make([]reflect.Value, 0, numArgs)
addContext := 0

if method.needsContext {
args = append(args, reflect.ValueOf(ctx))
addContext = 1
}

if isNil(params) {
allParamsAreOptional := utils.All(method.Params, func(p Parameter) bool {
return p.Optional
Expand All @@ -381,18 +392,12 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
return nil, errors.New("missing non-optional param field")
}

return s.buildDefaultArguments(ctx, method)
}

handlerType := reflect.TypeOf(method.Handler)

numArgs := handlerType.NumIn()
args := make([]reflect.Value, 0, numArgs)
addContext := 0
for i := addContext; i < numArgs; i++ {
arg := reflect.New(handlerType.In(i)).Elem()
args = append(args, arg)
}

if method.needsContext {
args = append(args, reflect.ValueOf(ctx))
addContext = 1
return args, nil
}

switch reflect.TypeOf(params).Kind() {
Expand Down Expand Up @@ -437,20 +442,6 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
return args, nil
}

func (s *Server) buildDefaultArguments(_ context.Context, method Method) ([]reflect.Value, error) {
handlerType := reflect.TypeOf(method.Handler)

numArgs := handlerType.NumIn()
args := make([]reflect.Value, 0, numArgs)

for i := 0; i < numArgs; i++ {
arg := reflect.New(handlerType.In(i)).Elem()
args = append(args, arg)
}

return args, nil
}

func (s *Server) parseParam(param any, t reflect.Type) (reflect.Value, error) {
handlerParam := reflect.New(t)
valueMarshaled, err := json.Marshal(param) // we have to marshal the value into JSON again
Expand Down
14 changes: 11 additions & 3 deletions jsonrpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,17 @@ func TestHandle(t *testing.T) {
},
},
{
Name: "acceptsContext",
Handler: func(_ context.Context) (int, *jsonrpc.Error) { return 0, nil },
Name: "acceptsContext",
Handler: func(ctx context.Context) (int, *jsonrpc.Error) {
require.NotNil(t, ctx)
return 0, nil
},
},
{
Name: "acceptsContextAndTwoParams",
Params: []jsonrpc.Parameter{{Name: "a"}, {Name: "b"}},
Handler: func(_ context.Context, a, b int) (int, *jsonrpc.Error) {
Handler: func(ctx context.Context, a, b int) (int, *jsonrpc.Error) {
require.NotNil(t, ctx)
return b - a, nil
},
},
Expand Down Expand Up @@ -381,6 +385,10 @@ func TestHandle(t *testing.T) {
req: `{"jsonrpc": "2.0", "method": "acceptsContext", "params": [], "id": 1}`,
res: `{"jsonrpc":"2.0","result":0,"id":1}`,
},
"handler accepts context without params": {
req: `{"jsonrpc": "2.0", "method": "acceptsContext","id": 1}`,
res: `{"jsonrpc":"2.0","result":0,"id":1}`,
},
"handler accepts context and two params with array params": {
req: `{"jsonrpc": "2.0", "method": "acceptsContextAndTwoParams", "params": [1, 3], "id": 1}`,
res: `{"jsonrpc":"2.0","result":2,"id":1}`,
Expand Down

0 comments on commit 219b89d

Please sign in to comment.