From 15d5ce6564ef1b28f95db71e0308ff43a6c0cfec Mon Sep 17 00:00:00 2001 From: zhangpp Date: Fri, 8 Jul 2016 16:03:12 +0800 Subject: [PATCH 1/5] Support adding dispatcher func or service dynamically and safely --- dispatcher.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/dispatcher.go b/dispatcher.go index 4eee2fc..5f594c7 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -22,6 +22,7 @@ import ( // // See examples for details. type Dispatcher struct { + mu sync.RWMutex serviceMap map[string]*serviceData } @@ -60,6 +61,8 @@ func NewDispatcher() *Dispatcher { // // See examples for details. func (d *Dispatcher) AddFunc(funcName string, f interface{}) { + d.mu.Lock() + defer d.mu.Unlock() sd, ok := d.serviceMap[""] if !ok { sd = &serviceData{ @@ -90,6 +93,8 @@ func (d *Dispatcher) AddFunc(funcName string, f interface{}) { // // All public methods must conform requirements described in AddFunc(). func (d *Dispatcher) AddService(serviceName string, service interface{}) { + d.mu.Lock() + defer d.mu.Unlock() if serviceName == "" { logPanic("gorpc.Dispatcher: serviceName cannot be empty") } @@ -308,18 +313,14 @@ func init() { // The returned HandlerFunc must be assigned to Server.Handler or // passed to New*Server(). func (d *Dispatcher) NewHandlerFunc() HandlerFunc { - if len(d.serviceMap) == 0 { - logPanic("gorpc.Dispatcher: register at least one service before calling HandlerFunc()") - } - - serviceMap := copyServiceMap(d.serviceMap) - return func(clientAddr string, request interface{}) interface{} { req, ok := request.(*dispatcherRequest) if !ok { logPanic("gorpc.Dispatcher: unsupported request type received from the client: %T", request) } - return dispatchRequest(serviceMap, clientAddr, req) + d.mu.RLock() + defer d.mu.RUnlock() + return dispatchRequest(d.serviceMap, clientAddr, req) } } @@ -463,6 +464,8 @@ func (d *Dispatcher) NewFuncClient(c *Client) *DispatcherClient { // // It is safe creating multiple service clients over a single underlying client. func (d *Dispatcher) NewServiceClient(serviceName string, c *Client) *DispatcherClient { + d.mu.RLock() + defer d.mu.RUnlock() if len(d.serviceMap) == 0 || d.serviceMap[serviceName] == nil { logPanic("gorpc.Dispatcher: service [%s] must be registered with AddService() before calling NewServiceClient()", serviceName) } From 50eebabda29c1d37a2d584bb4b3f7d522859bd42 Mon Sep 17 00:00:00 2001 From: zhangpp Date: Fri, 8 Jul 2016 17:35:18 +0800 Subject: [PATCH 2/5] Add NewDispatcherFuncClient and NewDispatcherServiceClient --- dispatcher.go | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/dispatcher.go b/dispatcher.go index 5f594c7..63fad3a 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -447,13 +447,9 @@ type DispatcherClient struct { serviceName string } -// NewFuncClient returns a client suitable for calling functions registered -// via AddFunc(). -func (d *Dispatcher) NewFuncClient(c *Client) *DispatcherClient { - if len(d.serviceMap) == 0 || d.serviceMap[""] == nil { - logPanic("gorpc.Dispatcher: register at least one function with AddFunc() before calling NewFuncClient()") - } - +// NewDispatcherFuncClient returns a client suitable for calling functions +// registered via AddFunc(). +func NewDispatcherFuncClient(c *Client) *DispatcherClient { return &DispatcherClient{ c: c, } @@ -463,6 +459,27 @@ func (d *Dispatcher) NewFuncClient(c *Client) *DispatcherClient { // of the service with name serviceName registered via AddService(). // // It is safe creating multiple service clients over a single underlying client. +func NewDispatcherServiceClient(serviceName string, c *Client) *DispatcherClient { + return &DispatcherClient{ + c: c, + serviceName: serviceName, + } +} + +// NewFuncClient checks and returns a client suitable for calling functions +// registered via AddFunc(). +func (d *Dispatcher) NewFuncClient(c *Client) *DispatcherClient { + d.mu.RLock() + defer d.mu.RUnlock() + if len(d.serviceMap) == 0 || d.serviceMap[""] == nil { + logPanic("gorpc.Dispatcher: register at least one function with AddFunc() before calling NewFuncClient()") + } + + return NewDispatcherFuncClient(c) +} + +// NewServiceClient checks and returns a client suitable for calling methods +// of the service with name serviceName registered via AddService(). func (d *Dispatcher) NewServiceClient(serviceName string, c *Client) *DispatcherClient { d.mu.RLock() defer d.mu.RUnlock() @@ -470,10 +487,7 @@ func (d *Dispatcher) NewServiceClient(serviceName string, c *Client) *Dispatcher logPanic("gorpc.Dispatcher: service [%s] must be registered with AddService() before calling NewServiceClient()", serviceName) } - return &DispatcherClient{ - c: c, - serviceName: serviceName, - } + return NewDispatcherServiceClient(serviceName, c) } // Call calls the given function with the given request. From 82aab4eabfedcf56653dada3637a87bc8ddea7d7 Mon Sep 17 00:00:00 2001 From: zhangpp Date: Fri, 8 Jul 2016 17:35:50 +0800 Subject: [PATCH 3/5] Add testing for adding func or service later --- dispatcher_test.go | 50 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/dispatcher_test.go b/dispatcher_test.go index 1d59f05..751ad54 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -11,13 +11,6 @@ import ( "unsafe" ) -func TestDispatcherNewHandlerNoFuncs(t *testing.T) { - d := NewDispatcher() - testPanic(t, func() { - d.NewHandlerFunc() - }) -} - func TestDispatcherNewFuncClientNoFuncs(t *testing.T) { c := NewTCPClient(getRandomAddr()) @@ -289,6 +282,26 @@ func TestDispatcherInvalidArgType(t *testing.T) { }) } +func TestDispatcherFuncLater(t *testing.T) { + d := NewDispatcher() + d.AddFunc("foo", func(request string) {}) + testDispatcherFunc(t, d, func(dc *DispatcherClient) { + res, err := dc.Call("foo", nil) + if err == nil { + t.Fatalf("Expected non-nil error") + } + if res != nil { + t.Fatalf("Expected nil response. Got %+v", res) + } + + d.AddFunc("foo0", func() {}) + _, err = dc.Call("foo0", nil) + if err != nil { + t.Fatalf("Expected nil error") + } + }) +} + func TestDispatcherUnknownFuncCall(t *testing.T) { d := NewDispatcher() d.AddFunc("foo", func(request string) {}) @@ -987,6 +1000,29 @@ func TestDispatcherServiceUnknownMethodCall(t *testing.T) { testDispatcherService(t, d, "qwerty", func(dc *DispatcherClient) { testUnknownFuncs(t, dc) }) } +func TestDispatcherServiceLater(t *testing.T) { + d := NewDispatcher() + c, s := getClientServer(t, d) + defer s.Stop() + defer c.Stop() + + dc := NewDispatcherServiceClient("qwerty", c) + + res, err := dc.Call("Get", nil) + if err == nil { + t.Fatalf("Error expected") + } + if res != nil { + t.Fatalf("Expected nil response. Got %+v", res) + } + + d.AddService("qwerty", &testService{}) + _, err = dc.Call("Get", nil) + if err != nil { + t.Fatalf("Expected nil error") + } +} + func TestDispatcherServicePrivateMethodCall(t *testing.T) { service := &testService{} From d825f0a62bd8f1b13920819a9e8d55ba9384bf87 Mon Sep 17 00:00:00 2001 From: zhangpp Date: Mon, 11 Jul 2016 10:58:43 +0800 Subject: [PATCH 4/5] Support interface type --- dispatcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dispatcher.go b/dispatcher.go index 63fad3a..f563f51 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -247,7 +247,7 @@ func validateType(t reflect.Type) (err error) { }) switch t.Kind() { - case reflect.Chan, reflect.Func, reflect.Interface, reflect.UnsafePointer: + case reflect.Chan, reflect.Func, reflect.UnsafePointer: err = fmt.Errorf("%s. Found [%s]", t.Kind(), t) return err case reflect.Array, reflect.Slice: From ffb5af10d927b9abf10ddbf52a9d5d90fdf16a63 Mon Sep 17 00:00:00 2001 From: zhangpp Date: Mon, 11 Jul 2016 11:13:35 +0800 Subject: [PATCH 5/5] Fix testing for supporting interface --- dispatcher_test.go | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/dispatcher_test.go b/dispatcher_test.go index 751ad54..b740246 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -3,7 +3,6 @@ package gorpc import ( "bytes" "fmt" - "io" "reflect" "sync/atomic" "testing" @@ -85,16 +84,6 @@ func TestDispatcherChanArg(t *testing.T) { }) } -func TestDispatcherInterfaceArg(t *testing.T) { - d := NewDispatcher() - testPanic(t, func() { - d.AddFunc("foo", func(req io.Reader) {}) - }) - testPanic(t, func() { - d.AddFunc("foo", func(req interface{}) {}) - }) -} - func TestDispatcherUnsafePointerArg(t *testing.T) { d := NewDispatcher() testPanic(t, func() { @@ -116,16 +105,6 @@ func TestDispatcherChanRes(t *testing.T) { }) } -func TestDispatcherInterfaceRes(t *testing.T) { - d := NewDispatcher() - testPanic(t, func() { - d.AddFunc("foo", func() (res io.Reader) { return }) - }) - testPanic(t, func() { - d.AddFunc("foo", func() (res interface{}) { return }) - }) -} - func TestDispatcherUnsafePointerRes(t *testing.T) { d := NewDispatcher() testPanic(t, func() { @@ -136,7 +115,7 @@ func TestDispatcherUnsafePointerRes(t *testing.T) { func TestDispatcherStructWithInvalidFields(t *testing.T) { type InvalidMsg struct { B int - A io.Reader + A chan bool } d := NewDispatcher() @@ -145,13 +124,6 @@ func TestDispatcherStructWithInvalidFields(t *testing.T) { }) } -func TestDispatcherInvalidMap(t *testing.T) { - d := NewDispatcher() - testPanic(t, func() { - d.AddFunc("foo", func(req map[string]interface{}) {}) - }) -} - func TestDispatcherPassStructArgByValue(t *testing.T) { type RequestType struct { a int