From 65ccd4afb20b6bec40b8561e0eb0865372371b17 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 21 Aug 2017 11:48:07 -0500 Subject: make hooks optional, short circuit if unimplemented (#7263) --- plugin/rpcplugin/hooks.go | 120 +++++++++++++++++++++++++++++++----- plugin/rpcplugin/hooks_test.go | 57 +++++++++++++++-- plugin/rpcplugin/main.go | 6 +- plugin/rpcplugin/main_test.go | 9 --- plugin/rpcplugin/supervisor_test.go | 13 ---- 5 files changed, 156 insertions(+), 49 deletions(-) (limited to 'plugin') diff --git a/plugin/rpcplugin/hooks.go b/plugin/rpcplugin/hooks.go index 008730402..5b97742aa 100644 --- a/plugin/rpcplugin/hooks.go +++ b/plugin/rpcplugin/hooks.go @@ -3,41 +3,87 @@ package rpcplugin import ( "io" "net/rpc" + "reflect" "github.com/mattermost/platform/plugin" ) type LocalHooks struct { - hooks plugin.Hooks + hooks interface{} muxer *Muxer remoteAPI *RemoteAPI } +// Implemented replies with the names of the hooks that are implemented. +func (h *LocalHooks) Implemented(args struct{}, reply *[]string) error { + ifaceType := reflect.TypeOf((*plugin.Hooks)(nil)).Elem() + implType := reflect.TypeOf(h.hooks) + selfType := reflect.TypeOf(h) + var methods []string + for i := 0; i < ifaceType.NumMethod(); i++ { + method := ifaceType.Method(i) + if m, ok := implType.MethodByName(method.Name); !ok { + continue + } else if m.Type.NumIn() != method.Type.NumIn()+1 { + continue + } else if m.Type.NumOut() != method.Type.NumOut() { + continue + } else { + match := true + for j := 0; j < method.Type.NumIn(); j++ { + if m.Type.In(j+1) != method.Type.In(j) { + match = false + break + } + } + for j := 0; j < method.Type.NumOut(); j++ { + if m.Type.Out(j) != method.Type.Out(j) { + match = false + break + } + } + if !match { + continue + } + } + if _, ok := selfType.MethodByName(method.Name); !ok { + continue + } + methods = append(methods, method.Name) + } + *reply = methods + return nil +} + func (h *LocalHooks) OnActivate(args int64, reply *struct{}) error { - stream := h.muxer.Connect(args) if h.remoteAPI != nil { h.remoteAPI.Close() + h.remoteAPI = nil + } + if hook, ok := h.hooks.(interface { + OnActivate(plugin.API) error + }); ok { + stream := h.muxer.Connect(args) + h.remoteAPI = ConnectAPI(stream, h.muxer) + return hook.OnActivate(h.remoteAPI) } - h.remoteAPI = ConnectAPI(stream, h.muxer) - return h.hooks.OnActivate(h.remoteAPI) + return nil } -func (h *LocalHooks) OnDeactivate(args, reply *struct{}) error { - err := h.hooks.OnDeactivate() +func (h *LocalHooks) OnDeactivate(args, reply *struct{}) (err error) { + if hook, ok := h.hooks.(interface { + OnDeactivate() error + }); ok { + err = hook.OnDeactivate() + } if h.remoteAPI != nil { h.remoteAPI.Close() h.remoteAPI = nil } - return err + return } -type RemoteHooks struct { - client *rpc.Client - muxer *Muxer - apiCloser io.Closer -} - -func ServeHooks(hooks plugin.Hooks, conn io.ReadWriteCloser, muxer *Muxer) { +func ServeHooks(hooks interface{}, conn io.ReadWriteCloser, muxer *Muxer) { server := rpc.NewServer() server.Register(&LocalHooks{ hooks: hooks, @@ -46,32 +92,72 @@ func ServeHooks(hooks plugin.Hooks, conn io.ReadWriteCloser, muxer *Muxer) { server.ServeConn(conn) } +const ( + remoteOnActivate = iota + remoteOnDeactivate + maxRemoteHookCount +) + +type RemoteHooks struct { + client *rpc.Client + muxer *Muxer + apiCloser io.Closer + implemented [maxRemoteHookCount]bool +} + var _ plugin.Hooks = (*RemoteHooks)(nil) +func (h *RemoteHooks) Implemented() (impl []string, err error) { + err = h.client.Call("LocalHooks.Implemented", struct{}{}, &impl) + return +} + func (h *RemoteHooks) OnActivate(api plugin.API) error { - id, stream := h.muxer.Serve() if h.apiCloser != nil { h.apiCloser.Close() + h.apiCloser = nil } + if !h.implemented[remoteOnActivate] { + return nil + } + id, stream := h.muxer.Serve() h.apiCloser = stream go ServeAPI(api, stream, h.muxer) return h.client.Call("LocalHooks.OnActivate", id, nil) } func (h *RemoteHooks) OnDeactivate() error { + if !h.implemented[remoteOnDeactivate] { + return nil + } return h.client.Call("LocalHooks.OnDeactivate", struct{}{}, nil) } func (h *RemoteHooks) Close() error { if h.apiCloser != nil { h.apiCloser.Close() + h.apiCloser = nil } return h.client.Close() } -func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer) *RemoteHooks { - return &RemoteHooks{ +func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer) (*RemoteHooks, error) { + remote := &RemoteHooks{ client: rpc.NewClient(conn), muxer: muxer, } + implemented, err := remote.Implemented() + if err != nil { + remote.Close() + return nil, err + } + for _, method := range implemented { + switch method { + case "OnActivate": + remote.implemented[remoteOnActivate] = true + case "OnDeactivate": + remote.implemented[remoteOnDeactivate] = true + } + } + return remote, nil } diff --git a/plugin/rpcplugin/hooks_test.go b/plugin/rpcplugin/hooks_test.go index fbbbbedeb..6cd7ff547 100644 --- a/plugin/rpcplugin/hooks_test.go +++ b/plugin/rpcplugin/hooks_test.go @@ -11,7 +11,7 @@ import ( "github.com/mattermost/platform/plugin/plugintest" ) -func testHooksRPC(hooks plugin.Hooks, f func(plugin.Hooks)) { +func testHooksRPC(hooks interface{}, f func(*RemoteHooks)) error { r1, w1 := io.Pipe() r2, w2 := io.Pipe() @@ -24,10 +24,14 @@ func testHooksRPC(hooks plugin.Hooks, f func(plugin.Hooks)) { id, server := c1.Serve() go ServeHooks(hooks, server, c1) - remote := ConnectHooks(c2.Connect(id), c2) + remote, err := ConnectHooks(c2.Connect(id), c2) + if err != nil { + return err + } defer remote.Close() f(remote) + return nil } func TestHooks(t *testing.T) { @@ -35,24 +39,65 @@ func TestHooks(t *testing.T) { var hooks plugintest.Hooks defer hooks.AssertExpectations(t) - testHooksRPC(&hooks, func(remote plugin.Hooks) { + assert.NoError(t, testHooksRPC(&hooks, func(remote *RemoteHooks) { hooks.On("OnActivate", mock.AnythingOfType("*rpcplugin.RemoteAPI")).Return(nil) assert.NoError(t, remote.OnActivate(&api)) hooks.On("OnDeactivate").Return(nil) assert.NoError(t, remote.OnDeactivate()) - }) + })) +} + +type testHooks struct { + mock.Mock +} + +func (h *testHooks) OnActivate(api plugin.API) error { + return h.Called(api).Error(0) +} + +func TestHooks_PartiallyImplemented(t *testing.T) { + var api plugintest.API + var hooks testHooks + defer hooks.AssertExpectations(t) + + assert.NoError(t, testHooksRPC(&hooks, func(remote *RemoteHooks) { + implemented, err := remote.Implemented() + assert.NoError(t, err) + assert.Equal(t, []string{"OnActivate"}, implemented) + + hooks.On("OnActivate", mock.AnythingOfType("*rpcplugin.RemoteAPI")).Return(nil) + assert.NoError(t, remote.OnActivate(&api)) + + assert.NoError(t, remote.OnDeactivate()) + })) } func BenchmarkOnDeactivate(b *testing.B) { var hooks plugintest.Hooks hooks.On("OnDeactivate").Return(nil) - testHooksRPC(&hooks, func(remote plugin.Hooks) { + if err := testHooksRPC(&hooks, func(remote *RemoteHooks) { + b.ResetTimer() + for n := 0; n < b.N; n++ { + remote.OnDeactivate() + } + b.StopTimer() + }); err != nil { + b.Fatal(err.Error()) + } +} + +func BenchmarkOnDeactivate_Unimplemented(b *testing.B) { + var hooks testHooks + + if err := testHooksRPC(&hooks, func(remote *RemoteHooks) { b.ResetTimer() for n := 0; n < b.N; n++ { remote.OnDeactivate() } b.StopTimer() - }) + }); err != nil { + b.Fatal(err.Error()) + } } diff --git a/plugin/rpcplugin/main.go b/plugin/rpcplugin/main.go index 36177954b..241d70bc9 100644 --- a/plugin/rpcplugin/main.go +++ b/plugin/rpcplugin/main.go @@ -6,12 +6,10 @@ import ( "fmt" "log" "os" - - "github.com/mattermost/platform/plugin" ) // Makes a set of hooks available via RPC. This function never returns. -func Main(hooks plugin.Hooks) { +func Main(hooks interface{}) { ipc, err := InheritedProcessIPC() if err != nil { log.Fatal(err.Error()) @@ -42,5 +40,5 @@ func ConnectMain(muxer *Muxer) (*RemoteHooks, error) { return nil, err } - return ConnectHooks(muxer.Connect(id), muxer), nil + return ConnectHooks(muxer.Connect(id), muxer) } diff --git a/plugin/rpcplugin/main_test.go b/plugin/rpcplugin/main_test.go index a796516e7..cc388787f 100644 --- a/plugin/rpcplugin/main_test.go +++ b/plugin/rpcplugin/main_test.go @@ -23,20 +23,11 @@ func TestMain(t *testing.T) { package main import ( - "github.com/mattermost/platform/plugin" "github.com/mattermost/platform/plugin/rpcplugin" ) type MyPlugin struct {} - func (p *MyPlugin) OnActivate(api plugin.API) error { - return nil - } - - func (p *MyPlugin) OnDeactivate() error { - return nil - } - func main() { rpcplugin.Main(&MyPlugin{}) } diff --git a/plugin/rpcplugin/supervisor_test.go b/plugin/rpcplugin/supervisor_test.go index 438ebe02a..c43fd3dc9 100644 --- a/plugin/rpcplugin/supervisor_test.go +++ b/plugin/rpcplugin/supervisor_test.go @@ -23,20 +23,11 @@ func TestSupervisor(t *testing.T) { package main import ( - "github.com/mattermost/platform/plugin" "github.com/mattermost/platform/plugin/rpcplugin" ) type MyPlugin struct {} - func (p *MyPlugin) OnActivate(api plugin.API) error { - return nil - } - - func (p *MyPlugin) OnDeactivate() error { - return nil - } - func main() { rpcplugin.Main(&MyPlugin{}) } @@ -100,10 +91,6 @@ func TestSupervisor_PluginCrash(t *testing.T) { return nil } - func (p *MyPlugin) OnDeactivate() error { - return nil - } - func main() { rpcplugin.Main(&MyPlugin{}) } -- cgit v1.2.3-1-g7c22