From e87965f39d2ce6dbd0e7883c387956413c663f6a Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Wed, 10 Oct 2018 13:56:54 -0400 Subject: MM-11905: delete plugin commands on removal (#9601) * defer plugin tear down for testing * test expected plugin command unregistration * MM-11905: uninstall plugin commands on remove --- app/plugin_commands_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++ app/plugin_hooks_test.go | 62 +++++++++++++++++--------- app/plugin_install.go | 2 + 3 files changed, 147 insertions(+), 21 deletions(-) create mode 100644 app/plugin_commands_test.go (limited to 'app') diff --git a/app/plugin_commands_test.go b/app/plugin_commands_test.go new file mode 100644 index 000000000..cc9184277 --- /dev/null +++ b/app/plugin_commands_test.go @@ -0,0 +1,104 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/require" +) + +func TestPluginCommand(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + args := &model.CommandArgs{} + args.TeamId = th.BasicTeam.Id + args.ChannelId = th.BasicChannel.Id + args.UserId = th.BasicUser.Id + args.Command = "/plugin" + + t.Run("error before plugin command registered", func(t *testing.T) { + _, err := th.App.ExecuteCommand(args) + require.NotNil(t, err) + }) + + t.Run("command handled by plugin", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PluginSettings.Plugins["testloadpluginconfig"] = map[string]interface{}{ + "TeamId": args.TeamId, + } + }) + + tearDown, pluginIds, activationErrors := SetAppEnvironmentWithPlugins(t, []string{` + package main + + import ( + "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/model" + ) + + type configuration struct { + TeamId string + } + + type MyPlugin struct { + plugin.MattermostPlugin + + configuration configuration + } + + func (p *MyPlugin) OnConfigurationChange() error { + p.API.LogError("hello") + if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil { + return err + } + + return nil + } + + func (p *MyPlugin) OnActivate() error { + p.API.LogError("team", "team", p.configuration.TeamId) + err := p.API.RegisterCommand(&model.Command{ + TeamId: p.configuration.TeamId, + Trigger: "plugin", + DisplayName: "Plugin Command", + }) + if err != nil { + p.API.LogError("error", "err", err) + } + p.API.LogDebug("team", "team", p.configuration.TeamId) + + return err + } + + func (p *MyPlugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*model.CommandResponse, *model.AppError) { + return &model.CommandResponse{ + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + Text: "text", + }, nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `}, th.App, th.App.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 1) + require.Nil(t, nil, activationErrors[0]) + + resp, err := th.App.ExecuteCommand(args) + require.Nil(t, err) + require.Equal(t, model.COMMAND_RESPONSE_TYPE_EPHEMERAL, resp.ResponseType) + require.Equal(t, "text", resp.Text) + + th.App.RemovePlugin(pluginIds[0]) + }) + + t.Run("error after plugin command unregistered", func(t *testing.T) { + _, err := th.App.ExecuteCommand(args) + require.NotNil(t, err) + }) +} diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index f2acd73e4..766385af4 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -35,18 +35,17 @@ func compileGo(t *testing.T, sourceCode, outputPath string) { require.NoError(t, cmd.Run()) } -func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API) []error { +func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API) (func(), []string, []error) { pluginDir, err := ioutil.TempDir("", "") require.NoError(t, err) webappPluginDir, err := ioutil.TempDir("", "") require.NoError(t, err) - defer os.RemoveAll(pluginDir) - defer os.RemoveAll(webappPluginDir) env, err := plugin.NewEnvironment(apiFunc, pluginDir, webappPluginDir, app.Log) require.NoError(t, err) app.Plugins = env + pluginIds := []string{} activationErrors := []error{} for _, code := range pluginCode { pluginId := model.NewId() @@ -55,10 +54,14 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a ioutil.WriteFile(filepath.Join(pluginDir, pluginId, "plugin.json"), []byte(`{"id": "`+pluginId+`", "backend": {"executable": "backend.exe"}}`), 0600) _, _, activationErr := env.Activate(pluginId) + pluginIds = append(pluginIds, pluginId) activationErrors = append(activationErrors, activationErr) } - return activationErrors + return func() { + os.RemoveAll(pluginDir) + os.RemoveAll(webappPluginDir) + }, pluginIds, activationErrors } func TestHookMessageWillBePosted(t *testing.T) { @@ -66,7 +69,7 @@ func TestHookMessageWillBePosted(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -88,6 +91,7 @@ func TestHookMessageWillBePosted(t *testing.T) { } `, }, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -105,7 +109,7 @@ func TestHookMessageWillBePosted(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -128,6 +132,7 @@ func TestHookMessageWillBePosted(t *testing.T) { } `, }, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -145,7 +150,7 @@ func TestHookMessageWillBePosted(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -167,6 +172,7 @@ func TestHookMessageWillBePosted(t *testing.T) { } `, }, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -190,7 +196,7 @@ func TestHookMessageWillBePosted(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -213,6 +219,7 @@ func TestHookMessageWillBePosted(t *testing.T) { } `, }, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -236,7 +243,7 @@ func TestHookMessageWillBePosted(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -281,6 +288,7 @@ func TestHookMessageWillBePosted(t *testing.T) { } `, }, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -304,7 +312,7 @@ func TestHookMessageHasBeenPosted(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "message").Return(nil) - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -326,6 +334,7 @@ func TestHookMessageHasBeenPosted(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -343,7 +352,7 @@ func TestHookMessageWillBeUpdated(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -366,6 +375,7 @@ func TestHookMessageWillBeUpdated(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -394,7 +404,7 @@ func TestHookMessageHasBeenUpdated(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "message_edited").Return(nil) mockAPI.On("LogDebug", "message_").Return(nil) - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -417,6 +427,7 @@ func TestHookMessageHasBeenUpdated(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() post := &model.Post{ UserId: th.BasicUser.Id, @@ -445,7 +456,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "testhook.txt").Return(nil) mockAPI.On("LogDebug", "inputfile").Return(nil) - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -468,6 +479,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { } `, }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() _, err := th.App.UploadFiles( "noteam", @@ -491,7 +503,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "testhook.txt").Return(nil) mockAPI.On("LogDebug", "inputfile").Return(nil) - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -516,6 +528,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { } `, }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() _, err := th.App.UploadFiles( "noteam", @@ -539,7 +552,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "testhook.txt").Return(nil) mockAPI.On("LogDebug", "inputfile").Return(nil) - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -562,6 +575,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { } `, }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() response, err := th.App.UploadFiles( "noteam", @@ -597,7 +611,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) mockAPI.On("LogDebug", "testhook.txt").Return(nil) mockAPI.On("LogDebug", "inputfile").Return(nil) - SetAppEnvironmentWithPlugins(t, []string{ + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -629,6 +643,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { } `, }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() response, err := th.App.UploadFiles( "noteam", @@ -667,7 +682,7 @@ func TestUserWillLogIn_Blocked(t *testing.T) { t.Errorf("Error updating user password: %s", err) } - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -689,6 +704,7 @@ func TestUserWillLogIn_Blocked(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() user, err := th.App.AuthenticateUserForLogin("", th.BasicUser.Email, "hunter2", "", false) @@ -711,7 +727,7 @@ func TestUserWillLogInIn_Passed(t *testing.T) { t.Errorf("Error updating user password: %s", err) } - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -733,6 +749,7 @@ func TestUserWillLogInIn_Passed(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() user, err := th.App.AuthenticateUserForLogin("", th.BasicUser.Email, "hunter2", "", false) @@ -755,7 +772,7 @@ func TestUserHasLoggedIn(t *testing.T) { t.Errorf("Error updating user password: %s", err) } - SetAppEnvironmentWithPlugins(t, + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -778,6 +795,7 @@ func TestUserHasLoggedIn(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() user, err := th.App.AuthenticateUserForLogin("", th.BasicUser.Email, "hunter2", "", false) @@ -803,7 +821,7 @@ func TestErrorString(t *testing.T) { defer th.TearDown() t.Run("errors.New", func(t *testing.T) { - activationErrors := SetAppEnvironmentWithPlugins(t, + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -826,6 +844,7 @@ func TestErrorString(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() require.Len(t, activationErrors, 1) require.NotNil(t, activationErrors[0]) @@ -833,7 +852,7 @@ func TestErrorString(t *testing.T) { }) t.Run("AppError", func(t *testing.T) { - activationErrors := SetAppEnvironmentWithPlugins(t, + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ ` package main @@ -855,6 +874,7 @@ func TestErrorString(t *testing.T) { plugin.ClientMain(&MyPlugin{}) } `}, th.App, th.App.NewPluginAPI) + defer tearDown() require.Len(t, activationErrors, 1) require.NotNil(t, activationErrors[0]) diff --git a/app/plugin_install.go b/app/plugin_install.go index b97fbc5d5..588f86f08 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -120,6 +120,8 @@ func (a *App) removePlugin(id string) *model.AppError { a.Publish(message) } + a.UnregisterPluginCommands(id) + a.Plugins.Deactivate(id) err = os.RemoveAll(pluginPath) -- cgit v1.2.3-1-g7c22