From 444da8204fe7c8d34383ca776d754688a6b38ab0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 19 Nov 2019 19:03:14 +0100 Subject: [PATCH] rpc: remove 'exported or builtin' restriction for parameters There is no technial reason for this restriction because package reflect can create values of any type. Requiring parameters and return values to be exported causes a lot of noise in package exports. --- rpc/client_test.go | 32 ++++++++++++++++---------------- rpc/doc.go | 10 +--------- rpc/service.go | 28 ++-------------------------- rpc/testservice_test.go | 14 +++++++------- rpc/websocket_test.go | 2 +- 5 files changed, 27 insertions(+), 59 deletions(-) diff --git a/rpc/client_test.go b/rpc/client_test.go index 315bd6d7d1..0d55140222 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -40,11 +40,11 @@ func TestClientRequest(t *testing.T) { client := DialInProc(server) defer client.Close() - var resp Result - if err := client.Call(&resp, "test_echo", "hello", 10, &Args{"world"}); err != nil { + var resp echoResult + if err := client.Call(&resp, "test_echo", "hello", 10, &echoArgs{"world"}); err != nil { t.Fatal(err) } - if !reflect.DeepEqual(resp, Result{"hello", 10, &Args{"world"}}) { + if !reflect.DeepEqual(resp, echoResult{"hello", 10, &echoArgs{"world"}}) { t.Errorf("incorrect result %#v", resp) } } @@ -58,13 +58,13 @@ func TestClientBatchRequest(t *testing.T) { batch := []BatchElem{ { Method: "test_echo", - Args: []interface{}{"hello", 10, &Args{"world"}}, - Result: new(Result), + Args: []interface{}{"hello", 10, &echoArgs{"world"}}, + Result: new(echoResult), }, { Method: "test_echo", - Args: []interface{}{"hello2", 11, &Args{"world"}}, - Result: new(Result), + Args: []interface{}{"hello2", 11, &echoArgs{"world"}}, + Result: new(echoResult), }, { Method: "no_such_method", @@ -78,13 +78,13 @@ func TestClientBatchRequest(t *testing.T) { wantResult := []BatchElem{ { Method: "test_echo", - Args: []interface{}{"hello", 10, &Args{"world"}}, - Result: &Result{"hello", 10, &Args{"world"}}, + Args: []interface{}{"hello", 10, &echoArgs{"world"}}, + Result: &echoResult{"hello", 10, &echoArgs{"world"}}, }, { Method: "test_echo", - Args: []interface{}{"hello2", 11, &Args{"world"}}, - Result: &Result{"hello2", 11, &Args{"world"}}, + Args: []interface{}{"hello2", 11, &echoArgs{"world"}}, + Result: &echoResult{"hello2", 11, &echoArgs{"world"}}, }, { Method: "no_such_method", @@ -104,7 +104,7 @@ func TestClientNotify(t *testing.T) { client := DialInProc(server) defer client.Close() - if err := client.Notify(context.Background(), "test_echo", "hello", 10, &Args{"world"}); err != nil { + if err := client.Notify(context.Background(), "test_echo", "hello", 10, &echoArgs{"world"}); err != nil { t.Fatal(err) } } @@ -393,9 +393,9 @@ func TestClientHTTP(t *testing.T) { // Launch concurrent requests. var ( - results = make([]Result, 100) + results = make([]echoResult, 100) errc = make(chan error) - wantResult = Result{"a", 1, new(Args)} + wantResult = echoResult{"a", 1, new(echoArgs)} ) defer client.Close() for i := range results { @@ -450,7 +450,7 @@ func TestClientReconnect(t *testing.T) { } // Perform a call. This should work because the server is up. - var resp Result + var resp echoResult if err := client.CallContext(ctx, &resp, "test_echo", "", 1, nil); err != nil { t.Fatal(err) } @@ -478,7 +478,7 @@ func TestClientReconnect(t *testing.T) { for i := 0; i < cap(errors); i++ { go func() { <-start - var resp Result + var resp echoResult errors <- client.CallContext(ctx, &resp, "test_echo", "", 3, nil) }() } diff --git a/rpc/doc.go b/rpc/doc.go index c9eaef54f0..e0a6324675 100644 --- a/rpc/doc.go +++ b/rpc/doc.go @@ -29,8 +29,6 @@ Methods that satisfy the following criteria are made available for remote access - method must be exported - method returns 0, 1 (response or error) or 2 (response and error) values - - method argument(s) must be exported or builtin types - - method returned value(s) must be exported or builtin types An example method: @@ -74,13 +72,8 @@ An example server which uses the JSON codec: calculator := new(CalculatorService) server := NewServer() server.RegisterName("calculator", calculator) - l, _ := net.ListenUnix("unix", &net.UnixAddr{Net: "unix", Name: "/tmp/calculator.sock"}) - for { - c, _ := l.AcceptUnix() - codec := v2.NewJSONCodec(c) - go server.ServeCodec(codec, 0) - } + server.ServeListener(l) Subscriptions @@ -90,7 +83,6 @@ criteria: - method must be exported - first method argument type must be context.Context - - method argument(s) must be exported or builtin types - method must have return types (rpc.Subscription, error) An example method: diff --git a/rpc/service.go b/rpc/service.go index 81e65f810b..bef891ea11 100644 --- a/rpc/service.go +++ b/rpc/service.go @@ -25,7 +25,6 @@ import ( "strings" "sync" "unicode" - "unicode/utf8" "github.com/ethereum/go-ethereum/log" ) @@ -139,16 +138,14 @@ func newCallback(receiver, fn reflect.Value) *callback { c := &callback{fn: fn, rcvr: receiver, errPos: -1, isSubscribe: isPubSub(fntype)} // Determine parameter types. They must all be exported or builtin types. c.makeArgTypes() - if !allExportedOrBuiltin(c.argTypes) { - return nil - } + // Verify return types. The function must return at most one error // and/or one other non-error value. outs := make([]reflect.Type, fntype.NumOut()) for i := 0; i < fntype.NumOut(); i++ { outs[i] = fntype.Out(i) } - if len(outs) > 2 || !allExportedOrBuiltin(outs) { + if len(outs) > 2 { return nil } // If an error is returned, it must be the last returned value. @@ -218,27 +215,6 @@ func (c *callback) call(ctx context.Context, method string, args []reflect.Value return results[0].Interface(), nil } -// Is this an exported - upper case - name? -func isExported(name string) bool { - rune, _ := utf8.DecodeRuneInString(name) - return unicode.IsUpper(rune) -} - -// Are all those types exported or built-in? -func allExportedOrBuiltin(types []reflect.Type) bool { - for _, typ := range types { - for typ.Kind() == reflect.Ptr { - typ = typ.Elem() - } - // PkgPath will be non-empty even for an exported type, - // so we need to check the type name as well. - if !isExported(typ.Name()) && typ.PkgPath() != "" { - return false - } - } - return true -} - // Is t context.Context or *context.Context? func isContextType(t reflect.Type) bool { for t.Kind() == reflect.Ptr { diff --git a/rpc/testservice_test.go b/rpc/testservice_test.go index 98871b5d6c..bfcdbc5649 100644 --- a/rpc/testservice_test.go +++ b/rpc/testservice_test.go @@ -53,24 +53,24 @@ func sequentialIDGenerator() func() ID { type testService struct{} -type Args struct { +type echoArgs struct { S string } -type Result struct { +type echoResult struct { String string Int int - Args *Args + Args *echoArgs } func (s *testService) NoArgsRets() {} -func (s *testService) Echo(str string, i int, args *Args) Result { - return Result{str, i, args} +func (s *testService) Echo(str string, i int, args *echoArgs) echoResult { + return echoResult{str, i, args} } -func (s *testService) EchoWithCtx(ctx context.Context, str string, i int, args *Args) Result { - return Result{str, i, args} +func (s *testService) EchoWithCtx(ctx context.Context, str string, i int, args *echoArgs) echoResult { + return echoResult{str, i, args} } func (s *testService) Sleep(ctx context.Context, duration time.Duration) { diff --git a/rpc/websocket_test.go b/rpc/websocket_test.go index 9dc1084797..f2a8438d7c 100644 --- a/rpc/websocket_test.go +++ b/rpc/websocket_test.go @@ -96,7 +96,7 @@ func TestWebsocketLargeCall(t *testing.T) { defer client.Close() // This call sends slightly less than the limit and should work. - var result Result + var result echoResult arg := strings.Repeat("x", maxRequestContentLength-200) if err := client.Call(&result, "test_echo", arg, 1); err != nil { t.Fatalf("valid call didn't work: %v", err)