Skip to content

Commit

Permalink
Remove the client and server "AllowV1" settings. (#61)
Browse files Browse the repository at this point in the history
The intent of these options was to tolerate calls to/from JSON-RPC 1.0
implementations. That alone isn't enough for interoperability, however.

For example:

- Server replies still contain the v2 version marker, which (some) v1 clients
  do not accept.

- The v2 Error object has a stricter structure than v1, and the client only
  accepts v2.

Since this library implements v2 specifically, I do not think it's worthwhile
to add extra plumbing for those cases (e.g., tracking v1 shape through the
handler, tolerating arbitrary Error geometry). On that basis, the tolerance
settings are not carrying their weight.

For context see #44.

Updates #46
  • Loading branch information
creachadair committed Nov 26, 2021
1 parent 40f68b0 commit 63a5fca
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 42 deletions.
22 changes: 7 additions & 15 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ type Client struct {
cbctx context.Context // terminates when the client is closed
cbcancel func() // cancels cbctx

allow1 bool // tolerate v1 replies with no version marker

mu sync.Mutex // protects the fields below
ch channel.Channel // channel to the server
err error // error from a previous operation
Expand All @@ -40,13 +38,12 @@ type Client struct {
func NewClient(ch channel.Channel, opts *ClientOptions) *Client {
cbctx, cbcancel := context.WithCancel(context.Background())
c := &Client{
done: new(sync.WaitGroup),
log: opts.logFunc(),
allow1: opts.allowV1(),
enctx: opts.encodeContext(),
snote: opts.handleNotification(),
scall: opts.handleCallback(),
chook: opts.handleCancel(),
done: new(sync.WaitGroup),
log: opts.logFunc(),
enctx: opts.encodeContext(),
snote: opts.handleNotification(),
scall: opts.handleCallback(),
chook: opts.handleCancel(),

cbctx: cbctx,
cbcancel: cbcancel,
Expand Down Expand Up @@ -423,12 +420,7 @@ func (c *Client) stop(err error) {
c.ch = nil
}

func (c *Client) versionOK(v string) bool {
if v == "" {
return c.allow1
}
return v == Version
}
func (c *Client) versionOK(v string) bool { return v == Version }

// marshalParams validates and marshals params to JSON for a request. The
// value of params must be either nil or encodable as a JSON object or array.
Expand Down
10 changes: 2 additions & 8 deletions jrpc2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,7 @@ func TestClient_Call(t *testing.T) {
loc := server.NewLocal(handler.ServiceMap{
"Test": testService,
}, &server.LocalOptions{
Server: &jrpc2.ServerOptions{
AllowV1: true,
Concurrency: 16,
},
Server: &jrpc2.ServerOptions{Concurrency: 16},
})
defer loc.Close()
c := loc.Client
Expand Down Expand Up @@ -191,10 +188,7 @@ func TestClient_Batch(t *testing.T) {
loc := server.NewLocal(handler.ServiceMap{
"Test": testService,
}, &server.LocalOptions{
Server: &jrpc2.ServerOptions{
AllowV1: true,
Concurrency: 16,
},
Server: &jrpc2.ServerOptions{Concurrency: 16},
})
defer loc.Close()
c := loc.Client
Expand Down
11 changes: 0 additions & 11 deletions opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ type ServerOptions struct {
// received and each response or error returned.
RPCLog RPCLogger

// Instructs the server to tolerate requests that do not include the
// required "jsonrpc" version marker.
AllowV1 bool

// Instructs the server to allow server callbacks and notifications, a
// non-standard extension to the JSON-RPC protocol. If AllowPush is false,
// the Notify and Callback methods of the server report errors if called.
Expand Down Expand Up @@ -79,7 +75,6 @@ func (s *ServerOptions) logFunc() func(string, ...interface{}) {
return s.Logger.Printf
}

func (s *ServerOptions) allowV1() bool { return s != nil && s.AllowV1 }
func (s *ServerOptions) allowPush() bool { return s != nil && s.AllowPush }
func (s *ServerOptions) allowBuiltin() bool { return s == nil || !s.DisableBuiltin }

Expand Down Expand Up @@ -144,10 +139,6 @@ type ClientOptions struct {
// If not nil, send debug text logs here.
Logger Logger

// Instructs the client to tolerate responses that do not include the
// required "jsonrpc" version marker.
AllowV1 bool

// If set, this function is called with the context, method name, and
// encoded request parameters before the request is sent to the server.
// Its return value replaces the request parameters. This allows the client
Expand Down Expand Up @@ -191,8 +182,6 @@ func (c *ClientOptions) logFunc() func(string, ...interface{}) {
return c.Logger.Printf
}

func (c *ClientOptions) allowV1() bool { return c != nil && c.AllowV1 }

type encoder = func(context.Context, string, json.RawMessage) (json.RawMessage, error)

func (c *ClientOptions) encodeContext() encoder {
Expand Down
9 changes: 1 addition & 8 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ type Server struct {
sem *semaphore.Weighted // bounds concurrent execution (default 1)

// Configurable settings
allow1 bool // allow v1 requests with no version marker
allowP bool // allow server notifications to the client
log func(string, ...interface{}) // write debug logs here
rpcLog RPCLogger // log RPC requests and responses here
Expand Down Expand Up @@ -71,7 +70,6 @@ func NewServer(mux Assigner, opts *ServerOptions) *Server {
s := &Server{
mux: mux,
sem: semaphore.NewWeighted(opts.concurrency()),
allow1: opts.allowV1(),
allowP: opts.allowPush(),
log: opts.logFunc(),
rpcLog: opts.rpcLog(),
Expand Down Expand Up @@ -693,12 +691,7 @@ func (s *Server) cancel(id string) bool {
return ok
}

func (s *Server) versionOK(v string) bool {
if v == "" {
return s.allow1 // an empty version is OK if the server allows it
}
return v == Version // ... otherwise it must match the spec
}
func (s *Server) versionOK(v string) bool { return v == Version }

// A task represents a pending method invocation received by the server.
type task struct {
Expand Down

0 comments on commit 63a5fca

Please sign in to comment.