From fad1f9b62ecd3f98f64917df85d82335b87fa1aa Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 9 Dec 2022 20:29:41 -0800 Subject: [PATCH 1/4] Replace the Handler interface with a plain function type. In practice there turns out to be no practical advantage to having the Handler type be an interface. All the existing usage that I can find, including in the handler support package, is based on explicit functions. This change replaces the Handler interface with a type alias to the expected function signature for the interface's Handle method. Any existing use based on the interface can be updated by extracting the method directly. For example, given a type like: type T struct{ ... func (t *T) Handle(ctx context.Context, req *jrpc2.Request) (any, error) { ... } h := &T{ ... } Replace usage like: m := handler.Map{"Method": h} with: m := handler.Map{"Method": h.Handle} This is a breaking change to the package API. --- base.go | 45 +++++++++++++++++++---------------------- handler/handler.go | 19 +++++++---------- handler/handler_test.go | 10 ++++----- internal_test.go | 4 ++-- server.go | 2 +- 5 files changed, 36 insertions(+), 44 deletions(-) diff --git a/base.go b/base.go index 624ac18..da6d45f 100644 --- a/base.go +++ b/base.go @@ -28,30 +28,27 @@ type Namer interface { Names() []string } -// A Handler handles a single request. -type Handler interface { - // Handle invokes the method with the specified request. The response value - // must be JSON-marshalable or nil. In case of error, the handler can - // return a value of type *jrpc2.Error to control the response code sent - // back to the caller; otherwise the server will wrap the resulting value. - // - // The context passed to the handler by a *jrpc2.Server includes two special - // values that the handler may extract. - // - // To obtain the server instance running the handler, write: - // - // srv := jrpc2.ServerFromContext(ctx) - // - // To obtain the inbound request message, write: - // - // req := jrpc2.InboundRequest(ctx) - // - // The latter is primarily useful for handlers generated by handler.New, - // which do not receive the request directly. For a handler that implements - // the Handle method directly, req is the same value passed as a parameter - // to Handle. - Handle(context.Context, *Request) (any, error) -} +// A Handler implements method given a request. The response value must be +// JSON-marshalable or nil. In case of error, the handler can return a value of +// type *jrpc2.Error to control the response code sent back to the caller; +// otherwise the server will wrap the resulting value. +// +// The context passed to the handler by a *jrpc2.Server includes two special +// values that the handler may extract. +// +// To obtain the server instance running the handler, write: +// +// srv := jrpc2.ServerFromContext(ctx) +// +// To obtain the inbound request message, write: +// +// req := jrpc2.InboundRequest(ctx) +// +// The latter is primarily useful for handlers generated by handler.New, +// which do not receive the request directly. For a handler that implements +// the Handle method directly, req is the same value passed as a parameter +// to Handle. +type Handler = func(context.Context, *Request) (any, error) // A Request is a request message from a client to a server. type Request struct { diff --git a/handler/handler.go b/handler/handler.go index ca5602a..59ebb44 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -17,17 +17,12 @@ import ( "github.com/creachadair/jrpc2/code" ) -// A Func adapts a function having the correct signature to a jrpc2.Handler. +// A Func is wrapper for a jrpc2.Handler function. type Func func(context.Context, *jrpc2.Request) (any, error) -// Handle implements the jrpc2.Handler interface by calling m. -func (m Func) Handle(ctx context.Context, req *jrpc2.Request) (any, error) { - return m(ctx, req) -} - // A Map is a trivial implementation of the jrpc2.Assigner interface that looks -// up method names in a map of static jrpc2.Handler values. -type Map map[string]jrpc2.Handler +// up method names in a static map of function values. +type Map map[string]Func // Assign implements part of the jrpc2.Assigner interface. func (m Map) Assign(_ context.Context, method string) jrpc2.Handler { return m[method] } @@ -78,7 +73,7 @@ func (m ServiceMap) Names() []string { return all } -// New adapts a function to a jrpc2.Handler. The concrete value of fn must be +// New adapts a function to a .Handler. The concrete value of fn must be // function accepted by Check. The resulting Func will handle JSON encoding and // decoding, call fn, and report appropriate errors. // @@ -124,9 +119,9 @@ type FuncInfo struct { // for non-struct arguments. func (fi *FuncInfo) SetStrict(strict bool) *FuncInfo { fi.strictFields = strict; return fi } -// Wrap adapts the function represented by fi in a Func that satisfies the -// jrpc2.Handler interface. The wrapped function can obtain the *jrpc2.Request -// value from its context argument using the jrpc2.InboundRequest helper. +// Wrap adapts the function represented by fi in a Func. The wrapped function +// can obtain the *jrpc2.Request value from its context argument using the +// jrpc2.InboundRequest helper. // // This method panics if fi == nil or if it does not represent a valid function // type. A FuncInfo returned by a successful call to Check is always valid. diff --git a/handler/handler_test.go b/handler/handler_test.go index b42172a..2caed4b 100644 --- a/handler/handler_test.go +++ b/handler/handler_test.go @@ -110,9 +110,9 @@ func TestFuncInfo_wrapDecode(t *testing.T) { fmt.Sprintf(`{"jsonrpc":"2.0","id":1,"method":"x","params":%s}`, test.p)) got, err := test.fn(ctx, req) if err != nil { - t.Errorf("Call %+v failed: %v", test.fn, err) + t.Errorf("Call %v failed: %v", test.fn, err) } else if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("Call %+v: wrong result (-want, +got)\n%s", test.fn, diff) + t.Errorf("Call %v: wrong result (-want, +got)\n%s", test.fn, diff) } } } @@ -263,7 +263,7 @@ func TestFuncInfo_SetStrict(t *testing.T) { // introduce another pointer indirection. func TestNew_pointerRegression(t *testing.T) { var got argStruct - call := handler.New(func(_ context.Context, arg *argStruct) error { + method := handler.New(func(_ context.Context, arg *argStruct) error { got = *arg t.Logf("Got argument struct: %+v", got) return nil @@ -276,8 +276,8 @@ func TestNew_pointerRegression(t *testing.T) { "alpha": "xyzzy", "bravo": 23 }}`) - if _, err := call.Handle(context.Background(), req); err != nil { - t.Errorf("Handle failed: %v", err) + if _, err := method(context.Background(), req); err != nil { + t.Errorf("Handler failed: %v", err) } want := argStruct{A: "xyzzy", B: 23} if diff := cmp.Diff(want, got); diff != "" { diff --git a/internal_test.go b/internal_test.go index 08f6bea..1193db3 100644 --- a/internal_test.go +++ b/internal_test.go @@ -233,7 +233,7 @@ func TestServer_specialMethods(t *testing.T) { } } if got := s.assign(ctx, "rpc.nonesuch"); got != nil { - t.Errorf("s.assign(rpc.nonesuch): got %v, want nil", got) + t.Errorf("s.assign(rpc.nonesuch): got %p, want nil", got) } } @@ -252,7 +252,7 @@ func TestServer_disableBuiltinHook(t *testing.T) { // With builtins disabled, the default rpc.* methods should not get assigned. for _, name := range []string{rpcServerInfo} { if got := s.assign(ctx, name); got != nil { - t.Errorf("s.assign(%s): got %+v, wanted nil", name, got) + t.Errorf("s.assign(%s): got %p, wanted nil", name, got) } } diff --git a/server.go b/server.go index c6b17e0..c237e55 100644 --- a/server.go +++ b/server.go @@ -381,7 +381,7 @@ func (s *Server) invoke(base context.Context, h Handler, req *Request) (json.Raw defer s.sem.Release(1) s.rpcLog.LogRequest(ctx, req) - v, err := h.Handle(ctx, req) + v, err := h(ctx, req) if err != nil { if req.IsNotification() { s.log("Discarding error from notification to %q: %v", req.Method(), err) From 8033453d28c6bba1ad56bfcb96753c6d660427ef Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 9 Dec 2022 20:46:06 -0800 Subject: [PATCH 2/4] Fix comment grammar. --- base.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base.go b/base.go index da6d45f..0398c4e 100644 --- a/base.go +++ b/base.go @@ -28,10 +28,10 @@ type Namer interface { Names() []string } -// A Handler implements method given a request. The response value must be -// JSON-marshalable or nil. In case of error, the handler can return a value of -// type *jrpc2.Error to control the response code sent back to the caller; -// otherwise the server will wrap the resulting value. +// A Handler function implements a method given a request. The response value +// must be JSON-marshalable or nil. In case of error, the handler can return a +// value of type *jrpc2.Error to control the response code sent back to the +// caller; otherwise the server will wrap the resulting value. // // The context passed to the handler by a *jrpc2.Server includes two special // values that the handler may extract. From 542dad013b329d44358e631a07a502ca0954dd53 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 9 Dec 2022 20:48:48 -0800 Subject: [PATCH 3/4] Clean up references to the interface. --- base.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/base.go b/base.go index 0398c4e..6cb54f5 100644 --- a/base.go +++ b/base.go @@ -44,10 +44,8 @@ type Namer interface { // // req := jrpc2.InboundRequest(ctx) // -// The latter is primarily useful for handlers generated by handler.New, -// which do not receive the request directly. For a handler that implements -// the Handle method directly, req is the same value passed as a parameter -// to Handle. +// The latter is primarily useful for wrappers generated by handler.New, which +// do not receive the request directly. type Handler = func(context.Context, *Request) (any, error) // A Request is a request message from a client to a server. From d905ac905d9d147ea827b8d170c27dda2f8b8cf8 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 9 Dec 2022 20:51:24 -0800 Subject: [PATCH 4/4] Clean up package comment references. --- doc.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/doc.go b/doc.go index d447831..b6cde3b 100644 --- a/doc.go +++ b/doc.go @@ -8,10 +8,9 @@ defined by http://www.jsonrpc.org/specification. The *Server type implements a JSON-RPC server. A server communicates with a client over a channel.Channel, and dispatches client requests to user-defined -method handlers. Handlers satisfy the jrpc2.Handler interface by exporting a -Handle method with this signature: +method handlers. Method handlers are functions with this signature: - Handle(ctx Context.Context, req *jrpc2.Request) (any, error) + func(ctx Context.Context, req *jrpc2.Request) (any, error) A server finds the handler for a request by looking up its method name in a jrpc2.Assigner provided when the server is set up. A Handler can decode the @@ -26,8 +25,8 @@ request parameters using the UnmarshalParams method on the request: } The handler package makes it easier to use functions that do not have this -exact type signature as handlers, by using reflection to lift functions into -the Handler interface. For example, suppose we want to export this Add function: +exact type signature as handlers, using reflection to wrap them in a Handler +function. For example, suppose we want to export this Add function: // Add returns the sum of a slice of integers. func Add(ctx context.Context, values []int) int { @@ -38,8 +37,7 @@ the Handler interface. For example, suppose we want to export this Add function return sum } -To convert Add to a handler, call handler.New, which wraps its argument in a a -handler.Func, which satisfies the jrpc2.Handler interface: +To convert Add to a handler, call handler.New, which returns a handler.Func: h := handler.New(Add) // h is now a handler.Func that calls Add