Add Batch JSON-RPC support (rpc client & server) #1503
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,15 +73,64 @@ type Request struct { | |
| ID interface{} `json:"id"` | ||
| } | ||
|
|
||
| // UnmarshalJSON is a custom unmarshal func for the Request struct. The param | ||
| // field defaults to an empty json.RawMessage array it is omitted by the request | ||
| // or nil if the supplied value is invalid. | ||
| func (request *Request) UnmarshalJSON(b []byte) error { | ||
| var data map[string]interface{} | ||
| err := json.Unmarshal(b, &data) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| request.ID = data["id"] | ||
| methodValue, hasMethod := data["method"] | ||
| if hasMethod { | ||
| request.Method = methodValue.(string) | ||
|
There was a problem hiding this comment. The cast here and on line 93 seems a bit dangerous to me, would blow up if the fields are accessed later and it turns out we have the wrong type. metod, ok := methodValue.(string)
if ok {
request.Method = method
}There was a problem hiding this comment. Interesting point. I don't see an alternative though There was a problem hiding this comment. @torkelrogstad I've gone ahead and addressed this on #1583
|
||
| } | ||
| jsonrpcValue, hasJsonrpc := data["jsonrpc"] | ||
| if hasJsonrpc { | ||
| request.Jsonrpc = jsonrpcValue.(string) | ||
| } | ||
| paramsValue, hasParams := data["params"] | ||
| if !hasParams { | ||
| // set the request param to an empty array if it is ommited in the request | ||
| request.Params = []json.RawMessage{} | ||
| // assert the request params is an array of data | ||
| } else if params, paramsOk := paramsValue.([]interface{}); paramsOk { | ||
| rawParams := make([]json.RawMessage, 0, len(params)) | ||
| for _, param := range params { | ||
| marshalledParam, err := json.Marshal(param) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| rawMessage := json.RawMessage(marshalledParam) | ||
| rawParams = append(rawParams, rawMessage) | ||
| } | ||
|
|
||
| request.Params = rawParams | ||
| } else { | ||
| return Error{Description: "No response received"} | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // NewRequest returns a new JSON-RPC request object given the provided rpc | ||
| // version, id, method, and parameters. The parameters are marshalled into a | ||
| // json.RawMessage for the Params field of the returned request object. This | ||
| // function is only provided in case the caller wants to construct raw requests | ||
| // for some reason. Typically callers will instead want to create a registered | ||
| // concrete command type with the NewCmd or New<Foo>Cmd functions and call the | ||
| // MarshalCmd function with that command to generate the marshalled JSON-RPC | ||
| // request. | ||
| func NewRequest(rpcVersion string, id interface{}, method string, params []interface{}) (*Request, error) { | ||
| // default to JSON-RPC 1.0 if RPC type is not specified | ||
| if rpcVersion != "2.0" && rpcVersion != "1.0" { | ||
|
There was a problem hiding this comment. This strikes me as a bit dangerous. Two potential solutions, could (should, perhaps) do both IMO:
type rpcVersion string
var (
rpcVersion1 rpcVersion = rpcVersion("1.0")
rpcVersion2 rpcVersion = rpcVersion("2.0")
)
|
||
| str := fmt.Sprintf("rpcversion '%s' is invalid", rpcVersion) | ||
| return nil, makeError(ErrInvalidType, str) | ||
| } | ||
|
|
||
| if !IsValidIDType(id) { | ||
| str := fmt.Sprintf("the id of type '%T' is invalid", id) | ||
| return nil, makeError(ErrInvalidType, str) | ||
|
|
@@ -98,51 +147,63 @@ func NewRequest(id interface{}, method string, params []interface{}) (*Request, | |
| } | ||
|
|
||
| return &Request{ | ||
| Jsonrpc: rpcVersion, | ||
| ID: id, | ||
| Method: method, | ||
| Params: rawParams, | ||
| }, nil | ||
| } | ||
|
|
||
| // Response is the general form of a JSON-RPC response. The type of the | ||
| // Result field varies from one command to the next, so it is implemented as an | ||
| // interface. The ID field has to be a pointer to allow for a nil value when | ||
| // empty. | ||
| type Response struct { | ||
| Jsonrpc string `json:"jsonrpc"` | ||
| Result json.RawMessage `json:"result"` | ||
| Error *RPCError `json:"error"` | ||
| ID *interface{} `json:"id"` | ||
| } | ||
|
|
||
| // NewResponse returns a new JSON-RPC response object given the provided rpc | ||
| // version, id, marshalled result, and RPC error. This function is only | ||
| // provided in case the caller wants to construct raw responses for some reason. | ||
| // Typically callers will instead want to create the fully marshalled JSON-RPC | ||
| // response to send over the wire with the MarshalResponse function. | ||
| func NewResponse(rpcVersion string, id interface{}, marshalledResult []byte, rpcErr *RPCError) (*Response, error) { | ||
| if rpcVersion != "2.0" && rpcVersion != "1.0" { | ||
|
There was a problem hiding this comment. Same comment about RPC versions from above apply here |
||
| str := fmt.Sprintf("rpcversion '%s' is invalid", rpcVersion) | ||
| return nil, makeError(ErrInvalidType, str) | ||
| } | ||
|
|
||
| if !IsValidIDType(id) { | ||
| str := fmt.Sprintf("the id of type '%T' is invalid", id) | ||
| return nil, makeError(ErrInvalidType, str) | ||
| } | ||
|
|
||
| pid := &id | ||
| return &Response{ | ||
| Jsonrpc: rpcVersion, | ||
| Result: marshalledResult, | ||
| Error: rpcErr, | ||
| ID: pid, | ||
| }, nil | ||
| } | ||
|
|
||
| // MarshalResponse marshals the passed rpc version, id, result, and RPCError to | ||
| // a JSON-RPC response byte slice that is suitable for transmission to a | ||
| // JSON-RPC client. | ||
| func MarshalResponse(rpcVersion string, id interface{}, result interface{}, rpcErr *RPCError) ([]byte, error) { | ||
| if rpcVersion != "2.0" && rpcVersion != "1.0" { | ||
|
There was a problem hiding this comment. RPC version comment from above |
||
| str := fmt.Sprintf("rpcversion '%s' is invalid", rpcVersion) | ||
| return nil, makeError(ErrInvalidType, str) | ||
| } | ||
|
|
||
| marshalledResult, err := json.Marshal(result) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| response, err := NewResponse(rpcVersion, id, marshalledResult, rpcErr) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you changing from a variable to a hardcoded number here (and all the places below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point, I'll have to refactor. I've held off on squashing till these revisions are complete, but can do this earlier fi it helps your review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I agree about waiting to squash until revisions are done but this needs the rebase before reviewing it for real so you might as well squash at the same time (I hate making people do extra steps if I don't have to).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem