From 2a355fd8adafe5b10d1d99e800e9cc6f0e779a90 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Tue, 4 May 2021 23:27:28 -0400 Subject: [PATCH] handler: Remove the NewService constructor. https://github.com/creachadair/jrpc2/issues/46#issuecomment-831963902 --- cmd/examples/server/server.go | 29 +++++++++++---------- handler/handler.go | 28 -------------------- handler/handler_test.go | 49 ++++++----------------------------- jrpc2_test.go | 17 +++++++++--- 4 files changed, 37 insertions(+), 86 deletions(-) diff --git a/cmd/examples/server/server.go b/cmd/examples/server/server.go index 8640532..5e74f0d 100644 --- a/cmd/examples/server/server.go +++ b/cmd/examples/server/server.go @@ -23,18 +23,13 @@ import ( "github.com/creachadair/jrpc2/server" ) -// The math type defines several arithmetic methods we can expose via the -// service. The exported methods having appropriate types can be automatically -// exposed to the server by jrpc2.NewService. -type math struct{} - // A binop carries a pair of integers for use as parameters. type binop struct { X, Y int } // Add returns the sum of vs, or 0 if len(vs) == 0. -func (math) Add(ctx context.Context, vs []int) int { +func Add(ctx context.Context, vs []int) int { sum := 0 for _, v := range vs { sum += v @@ -43,17 +38,17 @@ func (math) Add(ctx context.Context, vs []int) int { } // Sub returns the difference arg.X - arg.Y. -func (math) Sub(ctx context.Context, arg binop) int { +func Sub(ctx context.Context, arg binop) int { return arg.X - arg.Y } // Mul returns the product arg.X * arg.Y. -func (math) Mul(ctx context.Context, arg binop) int { +func Mul(ctx context.Context, arg binop) int { return arg.X * arg.Y } // Div converts its arguments to floating point and returns their ratio. -func (math) Div(ctx context.Context, arg binop) (float64, error) { +func Div(ctx context.Context, arg binop) (float64, error) { if arg.Y == 0 { return 0, jrpc2.Errorf(code.InvalidParams, "zero divisor") } @@ -62,7 +57,7 @@ func (math) Div(ctx context.Context, arg binop) (float64, error) { // Status simulates a health check, reporting "OK" to all callers. It also // demonstrates the use of server-side push. -func (math) Status(ctx context.Context) (string, error) { +func Status(ctx context.Context) (string, error) { if err := jrpc2.PushNotify(ctx, "pushback", []string{"hello, friend"}); err != nil { return "BAD", err } @@ -86,10 +81,18 @@ func main() { log.Fatal("You must provide a network -address to listen on") } - // Bind the methods of the math type to an assigner. + // Bind the services to their given names. mux := handler.ServiceMap{ - "Math": handler.NewService(math{}), - "Post": handler.Map{"Alert": handler.New(Alert)}, + "Math": handler.Map{ + "Add": handler.New(Add), + "Sub": handler.New(Sub), + "Mul": handler.New(Mul), + "Div": handler.New(Div), + "Status": handler.New(Status), + }, + "Post": handler.Map{ + "Alert": handler.New(Alert), + }, } lst, err := net.Listen(jrpc2.Network(*address), *address) diff --git a/handler/handler.go b/handler/handler.go index 78ab2d6..147185d 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -42,13 +42,6 @@ func (m Map) Names() []string { // A ServiceMap combines multiple assigners into one, permitting a server to // export multiple services under different names. -// -// Example: -// m := handler.ServiceMap{ -// "Foo": handler.NewService(fooService), // methods Foo.A, Foo.B, etc. -// "Bar": handler.NewService(barService), // methods Bar.A, Bar.B, etc. -// } -// type ServiceMap map[string]jrpc2.Assigner // Assign splits the inbound method name as Service.Method, and passes the @@ -105,27 +98,6 @@ func New(fn interface{}) Func { return m } -// NewService adapts the methods of a value to a map from method names to -// Handler implementations as constructed by New. It will panic if obj has no -// exported methods with a suitable signature. -func NewService(obj interface{}) Map { - out := make(Map) - val := reflect.ValueOf(obj) - typ := val.Type() - - // This considers only exported methods, as desired. - for i, n := 0, val.NumMethod(); i < n; i++ { - mi := val.Method(i) - if v, err := newHandler(mi.Interface()); err == nil { - out[typ.Method(i).Name] = v - } - } - if len(out) == 0 { - panic("no matching exported methods") - } - return out -} - var ( ctxType = reflect.TypeOf((*context.Context)(nil)).Elem() // type context.Context errType = reflect.TypeOf((*error)(nil)).Elem() // type error diff --git a/handler/handler_test.go b/handler/handler_test.go index 0b7d43b..8dc7dec 100644 --- a/handler/handler_test.go +++ b/handler/handler_test.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "log" - "strings" "testing" "github.com/creachadair/jrpc2" @@ -56,47 +55,11 @@ func TestNew(t *testing.T) { } } -type dummy struct{} +func y1(context.Context) (int, error) { return 0, nil } -func (dummy) Y1(context.Context) (int, error) { return 0, nil } +func y2(_ context.Context, vs ...int) (int, error) { return len(vs), nil } -func (dummy) N1(string) {} - -func (dummy) Y2(_ context.Context, vs ...int) (int, error) { return len(vs), nil } - -func (dummy) N2() bool { return false } - -func (dummy) Y3(context.Context) error { return errors.New("blah") } - -//lint:ignore U1000 verify unexported methods are not assigned -func (dummy) n3(context.Context, []string) error { return nil } - -// Verify that the NewService function obtains the correct functions. -func TestNewService(t *testing.T) { - var stub dummy - ctx := context.Background() - m := NewService(stub) - for _, test := range []string{"Y1", "Y2", "Y3", "N1", "N2", "n3", "foo"} { - got := m.Assign(ctx, test) != nil - want := strings.HasPrefix(test, "Y") - if got != want { - t.Errorf("Assign %q: got %v, want %v", test, got, want) - } - } -} - -// Verify that a stub with no usable methods panics. -func TestEmptyService(t *testing.T) { - type empty struct{} - - defer func() { - if x := recover(); x != nil { - t.Logf("Received expected panic: %v", x) - } - }() - m := NewService(empty{}) - t.Fatalf("NewService(empty): got %v, want panic", m) -} +func y3(context.Context) error { return errors.New("blah") } // Verify that a ServiceMap assigns names correctly. func TestServiceMap(t *testing.T) { @@ -115,7 +78,11 @@ func TestServiceMap(t *testing.T) { {"Test.N2", false}, } ctx := context.Background() - m := ServiceMap{"Test": NewService(dummy{})} + m := ServiceMap{"Test": Map{ + "Y1": New(y1), + "Y2": New(y2), + "Y3": New(y3), + }} for _, test := range tests { got := m.Assign(ctx, test.name) != nil if got != test.want { diff --git a/jrpc2_test.go b/jrpc2_test.go index 1f2d283..ae57b34 100644 --- a/jrpc2_test.go +++ b/jrpc2_test.go @@ -25,6 +25,15 @@ var testOK = handler.New(func(ctx context.Context) (string, error) { return "OK", nil }) +var testService = handler.Map{ + "Add": handler.New((dummy{}).Add), + "Mul": handler.New((dummy{}).Mul), + "Max": handler.New((dummy{}).Max), + "Nil": handler.New((dummy{}).Nil), + "Ctx": handler.New((dummy{}).Ctx), + "Ping": handler.New((dummy{}).Ping), +} + type dummy struct{} // Add is a request-based method. @@ -101,7 +110,7 @@ var callTests = []struct { func TestMethodNames(t *testing.T) { loc := server.NewLocal(handler.ServiceMap{ - "Test": handler.NewService(dummy{}), + "Test": testService, }, nil) defer loc.Close() s := loc.Server @@ -117,7 +126,7 @@ func TestMethodNames(t *testing.T) { func TestCall(t *testing.T) { loc := server.NewLocal(handler.ServiceMap{ - "Test": handler.NewService(dummy{}), + "Test": testService, }, &server.LocalOptions{ Server: &jrpc2.ServerOptions{ AllowV1: true, @@ -151,7 +160,7 @@ func TestCall(t *testing.T) { func TestCallResult(t *testing.T) { loc := server.NewLocal(handler.ServiceMap{ - "Test": handler.NewService(dummy{}), + "Test": testService, }, &server.LocalOptions{ Server: &jrpc2.ServerOptions{Concurrency: 16}, }) @@ -174,7 +183,7 @@ func TestCallResult(t *testing.T) { func TestBatch(t *testing.T) { loc := server.NewLocal(handler.ServiceMap{ - "Test": handler.NewService(dummy{}), + "Test": testService, }, &server.LocalOptions{ Server: &jrpc2.ServerOptions{ AllowV1: true,