Skip to content

Commit

Permalink
handler: Remove the NewService constructor.
Browse files Browse the repository at this point in the history 
This constructor is not carrying its weight in API surface.  It cannot handle
unexported methods, and the names it extracts are based on Go-specific lexical
rules rather than whatever a service may require.

Simplify test fixtures and remove unnecessary support code.

See issue #46.
  • Loading branch information
@creachadair
creachadair committed May 9, 2021
1 parent e41d97b commit 7d1330e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 116 deletions.
29 changes: 16 additions & 13 deletions cmd/examples/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
Expand All @@ -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
}
Expand All @@ -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)
Expand Down
34 changes: 13 additions & 21 deletions doc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,37 +155,29 @@ sends an "rpc.cancel" notification to the server for that request's ID. The
Services with Multiple Methods
The example above shows a server with one method using handler.New. To
simplify exporting multiple methods, the handler.NewService function applies
handler.New to all the relevant exported methods of a concrete value, returning
a handler.Map for those methods:
simplify exporting multiple methods, the handler.Map type collects named
methods:
type math struct{}
func (math) Add(ctx context.Context, vals ...int) int { ... }
func (math) Mul(ctx context.Context, vals []int) int { ... }
assigner := handler.NewService(math{})
This assigner maps the name "Add" to the Add method, and the name "Mul" to the
Mul method, of the math value.
mathService := handler.Map{
"Add": handler.New(Add),
"Mul": handler.New(Mul),
}
This may be further combined with the handler.ServiceMap type to allow
Maps may be further combined with the handler.ServiceMap type to allow
different services to work together:
type status struct{}
func (status) Get(context.Context) (string, error) {
func GetStatus(context.Context) (string, error) {
return "all is well", nil
}
assigner := handler.ServiceMap{
"Math": handler.NewService(math{}),
"Status": handler.NewService(status{}),
"Math": mathService,
"Status": handler.Map{"Get": handler.New(Status)},
}
This assigner dispatches "Math.Add" and "Math.Mul" to the math value's methods,
and "Status.Get" to the status value's method. A ServiceMap splits the method
name on the first period ("."), and you may nest ServiceMaps more deeply if you
This assigner dispatches "Math.Add" and "Math.Mul" to the arithmetic functions,
and "Status.Get" to the GetStatus function. A ServiceMap splits the method name
on the first period ("."), and you may nest ServiceMaps more deeply if you
require a more complex hierarchy.
Expand Down
28 changes: 0 additions & 28 deletions handler/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
49 changes: 8 additions & 41 deletions handler/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"errors"
"fmt"
"log"
"strings"
"testing"

"github.com/creachadair/jrpc2"
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down
35 changes: 22 additions & 13 deletions jrpc2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ var testOK = handler.New(func(ctx context.Context) (string, error) {
return "OK", nil
})

var testService = handler.Map{
// Verify that we can bind methods of a value.
"Add": handler.New((dummy{}).Add),
"Mul": handler.New((dummy{}).Mul),
"Max": handler.New((dummy{}).Max),

// Verify that we can bind free functions.
"Nil": handler.New(methodNil),
"Ctx": handler.New(methodCtx),
"Ping": handler.New(methodPing),
}

type dummy struct{}

// Add is a request-based method.
Expand Down Expand Up @@ -62,28 +74,25 @@ func (dummy) Max(_ context.Context, vs ...int) (int, error) {
return max, nil
}

// Nil does not require any parameters.
func (dummy) Nil(_ context.Context) (int, error) { return 42, nil }
// methodNil does not require any parameters.
func methodNil(_ context.Context) (int, error) { return 42, nil }

// Ctx validates that its context includes the request.
func (dummy) Ctx(ctx context.Context, req *jrpc2.Request) (int, error) {
// methodCtx validates that its context includes the request.
func methodCtx(ctx context.Context, req *jrpc2.Request) (int, error) {
if creq := jrpc2.InboundRequest(ctx); creq != req {
return 0, fmt.Errorf("wrong req in context %p ≠ %p", creq, req)
}
return 1, nil
}

// Ping responds only to notifications.
func (dummy) Ping(ctx context.Context, req *jrpc2.Request) error {
// methodPing responds only to notifications.
func methodPing(ctx context.Context, req *jrpc2.Request) error {
if !req.IsNotification() {
return errors.New("called Ping expecting a response")
}
return nil
}

// Unrelated should not be picked up by the server.
func (dummy) Unrelated() string { return "ceci n'est pas une méthode" }

var callTests = []struct {
method string
params interface{}
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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},
})
Expand All @@ -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,
Expand Down

0 comments on commit 7d1330e

Please sign in to comment.