From b07bc0e83967e7c5072debeba04fabf36fc0a992 Mon Sep 17 00:00:00 2001 From: Henk Date: Tue, 27 Dec 2022 16:57:01 -0800 Subject: [PATCH 1/8] Issue 400: Add Handler for unknown methods --- server/server.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/server.go b/server/server.go index 3662582d..7374451f 100644 --- a/server/server.go +++ b/server/server.go @@ -100,6 +100,9 @@ type Server struct { // Calls are inserted into this queue, to be handled // by a goroutine running handleCalls() callQueue *mpsc.Queue[*Call] + + // Handler for custom behavior of unknown methods + HandleUnknownMethod func(ctx context.Context, r capnp.Recv) *Method } // New returns a client hook that makes calls to a set of methods. @@ -150,6 +153,9 @@ func (srv *Server) Send(ctx context.Context, s capnp.Send) (*capnp.Answer, capnp // Recv starts a method call. func (srv *Server) Recv(ctx context.Context, r capnp.Recv) capnp.PipelineCaller { mm := srv.methods.find(r.Method) + if mm == nil && srv.HandleUnknownMethod != nil { + mm = srv.HandleUnknownMethod(ctx, r) + } if mm == nil { r.Reject(capnp.Unimplemented("unimplemented")) return nil From 0f0c80d25cc7fb5b25354531337cc8c689e7f3a2 Mon Sep 17 00:00:00 2001 From: Henk Date: Thu, 29 Dec 2022 10:41:49 -0800 Subject: [PATCH 2/8] No need for Recv parameter in HandleUnknownMethod. Simply capnp.Method is sufficient. --- server/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/server.go b/server/server.go index 7374451f..ef26b66e 100644 --- a/server/server.go +++ b/server/server.go @@ -102,7 +102,7 @@ type Server struct { callQueue *mpsc.Queue[*Call] // Handler for custom behavior of unknown methods - HandleUnknownMethod func(ctx context.Context, r capnp.Recv) *Method + HandleUnknownMethod func(ctx context.Context, m capnp.Method) *Method } // New returns a client hook that makes calls to a set of methods. @@ -154,7 +154,7 @@ func (srv *Server) Send(ctx context.Context, s capnp.Send) (*capnp.Answer, capnp func (srv *Server) Recv(ctx context.Context, r capnp.Recv) capnp.PipelineCaller { mm := srv.methods.find(r.Method) if mm == nil && srv.HandleUnknownMethod != nil { - mm = srv.HandleUnknownMethod(ctx, r) + mm = srv.HandleUnknownMethod(ctx, r.Method) } if mm == nil { r.Reject(capnp.Unimplemented("unimplemented")) From bbae535801e08eb5ca11fd78f77156c2f047ab1e Mon Sep 17 00:00:00 2001 From: Henk Date: Thu, 29 Dec 2022 13:30:56 -0800 Subject: [PATCH 3/8] Remove context as it isn't used --- server/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/server.go b/server/server.go index ef26b66e..971394a3 100644 --- a/server/server.go +++ b/server/server.go @@ -102,7 +102,7 @@ type Server struct { callQueue *mpsc.Queue[*Call] // Handler for custom behavior of unknown methods - HandleUnknownMethod func(ctx context.Context, m capnp.Method) *Method + HandleUnknownMethod func(m capnp.Method) *Method } // New returns a client hook that makes calls to a set of methods. @@ -154,7 +154,7 @@ func (srv *Server) Send(ctx context.Context, s capnp.Send) (*capnp.Answer, capnp func (srv *Server) Recv(ctx context.Context, r capnp.Recv) capnp.PipelineCaller { mm := srv.methods.find(r.Method) if mm == nil && srv.HandleUnknownMethod != nil { - mm = srv.HandleUnknownMethod(ctx, r.Method) + mm = srv.HandleUnknownMethod(r.Method) } if mm == nil { r.Reject(capnp.Unimplemented("unimplemented")) From 1603f70b1f3e883aa420f0ee80cc7fde2c9ac454 Mon Sep 17 00:00:00 2001 From: Henk Date: Sat, 31 Dec 2022 13:05:34 -0800 Subject: [PATCH 4/8] Add testcase for unknown method --- server/server_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/server/server_test.go b/server/server_test.go index b9578624..8444fc84 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -3,12 +3,17 @@ package server_test import ( "context" "errors" + "net" "strings" "sync" + "sync/atomic" "testing" + "github.com/stretchr/testify/require" + "capnproto.org/go/capnp/v3" air "capnproto.org/go/capnp/v3/internal/aircraftlib" + "capnproto.org/go/capnp/v3/rpc" "capnproto.org/go/capnp/v3/server" "github.com/stretchr/testify/assert" @@ -91,6 +96,63 @@ func TestServerCall(t *testing.T) { } } }) + t.Run("Unimplemented hook", func(t *testing.T) { + t.Parallel() + var echoText = "are you there?" + var proxyReceived atomic.Value + + // start a proxy server with hook + srv := server.New(nil, nil, nil) + srv.HandleUnknownMethod = func(method capnp.Method) *server.Method { + sm := server.Method{ + Method: method, + Impl: nil, + } + sm.Impl = func(ctx context.Context, call *server.Call) error { + t.Log("welcome to blank") + echoArgs := air.Echo_echo_Params(call.Args()) + inText, err := echoArgs.In() + assert.NoError(t, err) + proxyReceived.Store(inText) + // pretend we received an answer + echo := air.Echo_echo{Call: call} + resp, err := echo.AllocResults() + err = resp.SetOut(inText) + return err + } + return &sm + } + blankBoot := capnp.NewClient(srv) + lis, err := net.Listen("tcp", ":0") + defer lis.Close() + require.NoError(t, err) + go rpc.Serve(lis, blankBoot) + + // invoke the proxy server with the echo client + addr := lis.Addr().String() + conn, err := net.Dial("tcp", addr) + assert.NoError(t, err) + transport := rpc.NewStreamTransport(conn) + rpcConn := rpc.NewConn(transport, nil) + defer rpcConn.Close() + blankClient := rpcConn.Bootstrap(context.Background()) + echoClient := air.Echo(blankClient) + defer echoClient.Release() + + ans, finish := echoClient.Echo(context.Background(), func(p air.Echo_echo_Params) error { + err := p.SetIn(echoText) + return err + }) + defer finish() + resp, err := ans.Struct() + answerOut, _ := resp.Out() + rxValue := proxyReceived.Load() + assert.Equal(t, echoText, rxValue) + assert.Equal(t, echoText, answerOut) + if err != nil { + t.Error("echo.Echo() error != ; want success") + } + }) } type callSeq uint32 From 48794db06cd818b1ba5fe5182ad23c21ac377210 Mon Sep 17 00:00:00 2001 From: Henk Date: Mon, 2 Jan 2023 09:39:34 -0800 Subject: [PATCH 5/8] Fix issues from review. (but left the assert.NoError at #115) --- server/server_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 8444fc84..d9160318 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -109,14 +109,13 @@ func TestServerCall(t *testing.T) { Impl: nil, } sm.Impl = func(ctx context.Context, call *server.Call) error { - t.Log("welcome to blank") echoArgs := air.Echo_echo_Params(call.Args()) inText, err := echoArgs.In() assert.NoError(t, err) proxyReceived.Store(inText) // pretend we received an answer echo := air.Echo_echo{Call: call} - resp, err := echo.AllocResults() + resp, _ := echo.AllocResults() err = resp.SetOut(inText) return err } @@ -147,11 +146,9 @@ func TestServerCall(t *testing.T) { resp, err := ans.Struct() answerOut, _ := resp.Out() rxValue := proxyReceived.Load() - assert.Equal(t, echoText, rxValue) + require.Equal(t, echoText, rxValue) assert.Equal(t, echoText, answerOut) - if err != nil { - t.Error("echo.Echo() error != ; want success") - } + assert.NoError(t, err, "echo.Echo() error != ; want success") }) } From 3284e921c2db2fda5181845b4b1fbc1bd70cb88c Mon Sep 17 00:00:00 2001 From: Henk Date: Mon, 2 Jan 2023 10:17:19 -0800 Subject: [PATCH 6/8] Changed assert into require to avoid confusing test output on failure. --- server/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server_test.go b/server/server_test.go index d9160318..08fec8fd 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -111,7 +111,7 @@ func TestServerCall(t *testing.T) { sm.Impl = func(ctx context.Context, call *server.Call) error { echoArgs := air.Echo_echo_Params(call.Args()) inText, err := echoArgs.In() - assert.NoError(t, err) + require.NoError(t, err) proxyReceived.Store(inText) // pretend we received an answer echo := air.Echo_echo{Call: call} From fb02e1fe19efb9b04cf485ae0f76da17dc2c74ea Mon Sep 17 00:00:00 2001 From: Henk Date: Mon, 2 Jan 2023 21:58:44 -0800 Subject: [PATCH 7/8] Included the method lookup in the Send() function as well, for use by the client, as pointed out by @zenhack. --- server/server.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/server.go b/server/server.go index 971394a3..b3e05af8 100644 --- a/server/server.go +++ b/server/server.go @@ -129,6 +129,9 @@ func New(methods []Method, brand any, shutdown Shutdowner) *Server { // Send starts a method call. func (srv *Server) Send(ctx context.Context, s capnp.Send) (*capnp.Answer, capnp.ReleaseFunc) { mm := srv.methods.find(s.Method) + if mm == nil && srv.HandleUnknownMethod != nil { + mm = srv.HandleUnknownMethod(s.Method) + } if mm == nil { return capnp.ErrorAnswer(s.Method, capnp.Unimplemented("unimplemented")), func() {} } From 96a01942ec9402e8ddc74ab618ddd026eff4f00b Mon Sep 17 00:00:00 2001 From: Henk Date: Mon, 2 Jan 2023 22:25:46 -0800 Subject: [PATCH 8/8] Remove connection from the test as requested. --- server/server_test.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 08fec8fd..d3698aaf 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -3,7 +3,6 @@ package server_test import ( "context" "errors" - "net" "strings" "sync" "sync/atomic" @@ -13,7 +12,6 @@ import ( "capnproto.org/go/capnp/v3" air "capnproto.org/go/capnp/v3/internal/aircraftlib" - "capnproto.org/go/capnp/v3/rpc" "capnproto.org/go/capnp/v3/server" "github.com/stretchr/testify/assert" @@ -122,20 +120,7 @@ func TestServerCall(t *testing.T) { return &sm } blankBoot := capnp.NewClient(srv) - lis, err := net.Listen("tcp", ":0") - defer lis.Close() - require.NoError(t, err) - go rpc.Serve(lis, blankBoot) - - // invoke the proxy server with the echo client - addr := lis.Addr().String() - conn, err := net.Dial("tcp", addr) - assert.NoError(t, err) - transport := rpc.NewStreamTransport(conn) - rpcConn := rpc.NewConn(transport, nil) - defer rpcConn.Close() - blankClient := rpcConn.Bootstrap(context.Background()) - echoClient := air.Echo(blankClient) + echoClient := air.Echo(blankBoot) defer echoClient.Release() ans, finish := echoClient.Echo(context.Background(), func(p air.Echo_echo_Params) error {