handler: Remove the NewService constructor.

#46 (comment)
creachadair · May 5, 2021 · 2a355fd · 2a355fd
1 parent e41d97b
commit 2a355fd
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 86 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,