diff --git a/Makefile b/Makefile index 1398b66fc9..4ad3846180 100644 --- a/Makefile +++ b/Makefile @@ -338,6 +338,7 @@ plugin-mocks: ## Creates mock files for plugins. go get -u github.com/vektra/mockery/... $(GOPATH)/bin/mockery -dir plugin -name API -output plugin/plugintest -outpkg plugintest -case underscore -note 'Regenerate this file using `make plugin-mocks`.' $(GOPATH)/bin/mockery -dir plugin -name Hooks -output plugin/plugintest -outpkg plugintest -case underscore -note 'Regenerate this file using `make plugin-mocks`.' + $(GOPATH)/bin/mockery -dir plugin -name Helpers -output plugin/plugintest -outpkg plugintest -case underscore -note 'Regenerate this file using `make plugin-mocks`.' pluginapi: ## Generates api and hooks glue code for plugins go generate ./plugin diff --git a/plugin/api.go b/plugin/api.go index 506c898704..766dfd1269 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -457,6 +457,7 @@ type API interface { // KV Store Section // KVSet will store a key-value pair, unique per plugin. + // Provided helper functions and internal plugin code will use the prefix `mmi_` before keys. Do not use this prefix. KVSet(key string, value []byte) *model.AppError // KVCompareAndSet will update a key-value pair, diff --git a/plugin/client.go b/plugin/client.go index 63cedfbcee..bc51540503 100644 --- a/plugin/client.go +++ b/plugin/client.go @@ -7,16 +7,21 @@ import ( "github.com/hashicorp/go-plugin" ) +const ( + INTERNAL_KEY_PREFIX = "mmi_" + BOT_USER_KEY = INTERNAL_KEY_PREFIX + "botid" +) + // Starts the serving of a Mattermost plugin over net/rpc. gRPC is not yet supported. // // Call this when your plugin is ready to start. func ClientMain(pluginImplementation interface{}) { if impl, ok := pluginImplementation.(interface { - SetAPI(api API) + SetAPI(api API, helpers Helpers) }); !ok { panic("Plugin implementation given must embed plugin.MattermostPlugin") } else { - impl.SetAPI(nil) + impl.SetAPI(nil, nil) } pluginMap := map[string]plugin.Plugin{ @@ -31,11 +36,13 @@ func ClientMain(pluginImplementation interface{}) { type MattermostPlugin struct { // API exposes the plugin api, and becomes available just prior to the OnActive hook. - API API + API API + Helpers Helpers } // SetAPI persists the given API interface to the plugin. It is invoked just prior to the // OnActivate hook, exposing the API for use by the plugin. -func (p *MattermostPlugin) SetAPI(api API) { +func (p *MattermostPlugin) SetAPI(api API, helpers Helpers) { p.API = api + p.Helpers = helpers } diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index 51735fc018..228d673ef6 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -196,9 +196,9 @@ func (s *hooksRPCServer) OnActivate(args *Z_OnActivateArgs, returns *Z_OnActivat } if mmplugin, ok := s.impl.(interface { - SetAPI(api API) + SetAPI(api API, helpers Helpers) }); ok { - mmplugin.SetAPI(s.apiRPCClient) + mmplugin.SetAPI(s.apiRPCClient, &HelpersImpl{API: s.apiRPCClient}) } if mmplugin, ok := s.impl.(interface { diff --git a/plugin/helpers.go b/plugin/helpers.go new file mode 100644 index 0000000000..b078844cf3 --- /dev/null +++ b/plugin/helpers.go @@ -0,0 +1,17 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import "github.com/mattermost/mattermost-server/model" + +type Helpers interface { + // EnsureBot ether returns an existing bot user or creates a bot user with + // the specifications of the passed bot. + // Returns the id of the bot created or existing. + EnsureBot(bot *model.Bot) (string, error) +} + +type HelpersImpl struct { + API API +} diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go new file mode 100644 index 0000000000..b83adc7f30 --- /dev/null +++ b/plugin/helpers_bots.go @@ -0,0 +1,66 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "time" + + "github.com/mattermost/mattermost-server/model" + "github.com/pkg/errors" +) + +func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) { + // Must provide a bot with a username + if bot == nil || len(bot.Username) < 1 { + return "", errors.New("passed a bad bot, nil or no username") + } + + // If we fail for any reason, this could be a race between creation of bot and + // retreval from anouther EnsureBot. Just try the basic retrieve existing again. + defer func() { + if retBotId == "" || retErr != nil { + time.Sleep(time.Second) + botIdBytes, err := p.API.KVGet(BOT_USER_KEY) + if err == nil && botIdBytes != nil { + retBotId = string(botIdBytes) + retErr = nil + } + } + }() + + botIdBytes, kvGetErr := p.API.KVGet(BOT_USER_KEY) + if kvGetErr != nil { + return "", errors.Wrap(kvGetErr, "failed to get bot") + } + + // If the bot has already been created, there is nothing to do. + if botIdBytes != nil { + botId := string(botIdBytes) + return botId, nil + } + + // Check for an existing bot user with that username. If one exists, then use that. + if user, userGetErr := p.API.GetUserByUsername(bot.Username); userGetErr == nil && user != nil { + if user.IsBot { + if kvSetErr := p.API.KVSet(BOT_USER_KEY, []byte(user.Id)); kvSetErr != nil { + p.API.LogWarn("Failed to set claimed bot user id.", "userid", user.Id, "err", kvSetErr) + } + return user.Id, nil + } else { + return "", errors.New("unable to create bot because user exists with the same name") + } + } + + // Create a new bot user for the plugin + createdBot, createBotErr := p.API.CreateBot(bot) + if createBotErr != nil { + return "", errors.Wrap(createBotErr, "failed to create bot") + } + + if kvSetErr := p.API.KVSet(BOT_USER_KEY, []byte(createdBot.UserId)); kvSetErr != nil { + p.API.LogWarn("Failed to set created bot user id.", "userid", createdBot.UserId, "err", kvSetErr) + } + + return createdBot.UserId, nil +} diff --git a/plugin/helpers_bots_test.go b/plugin/helpers_bots_test.go new file mode 100644 index 0000000000..30cd05685c --- /dev/null +++ b/plugin/helpers_bots_test.go @@ -0,0 +1,154 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin_test + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/plugin/plugintest" + "github.com/stretchr/testify/assert" +) + +func TestEnsureBot(t *testing.T) { + setupAPI := func() *plugintest.API { + return &plugintest.API{} + } + + testbot := &model.Bot{ + Username: "testbot", + DisplayName: "Test Bot", + Description: "testbotdescription", + } + + t.Run("bad parameters", func(t *testing.T) { + t.Run("no bot", func(t *testing.T) { + p := &plugin.HelpersImpl{} + botId, err := p.EnsureBot(nil) + assert.Equal(t, "", botId) + assert.NotNil(t, err) + }) + t.Run("bad username", func(t *testing.T) { + p := &plugin.HelpersImpl{} + botId, err := p.EnsureBot(&model.Bot{ + Username: "", + }) + assert.Equal(t, "", botId) + assert.NotNil(t, err) + }) + }) + + t.Run("if bot already exists", func(t *testing.T) { + t.Run("should find and return the existing bot ID", func(t *testing.T) { + expectedBotId := model.NewId() + + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should return an error if unable to get bot", func(t *testing.T) { + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, &model.AppError{}) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + assert.Equal(t, "", botId) + assert.NotNil(t, err) + }) + }) + + t.Run("if bot doesn't exist", func(t *testing.T) { + t.Run("should create the bot and return the ID", func(t *testing.T) { + expectedBotId := model.NewId() + + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(nil, nil) + api.On("CreateBot", testbot).Return(&model.Bot{ + UserId: expectedBotId, + }, nil) + api.On("KVSet", plugin.BOT_USER_KEY, []byte(expectedBotId)).Return(nil) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should claim existing bot and return the ID", func(t *testing.T) { + expectedBotId := model.NewId() + + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(&model.User{ + Id: expectedBotId, + IsBot: true, + }, nil) + api.On("KVSet", plugin.BOT_USER_KEY, []byte(expectedBotId)).Return(nil) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should fail if user exists with the same name and is not a bot", func(t *testing.T) { + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(&model.User{ + Id: "conflictingid", + IsBot: false, + }, nil) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + t.Log(botId) + t.Log(err) + assert.Equal(t, "", botId) + assert.NotNil(t, err) + }) + + t.Run("shoudl fail if create bot fails", func(t *testing.T) { + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(nil, nil) + api.On("CreateBot", testbot).Return(nil, &model.AppError{}) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot) + + assert.Equal(t, "", botId) + assert.NotNil(t, err) + }) + }) +} diff --git a/plugin/plugintest/example_hello_user_test.go b/plugin/plugintest/example_hello_user_test.go index 3a12f292c7..b46991d971 100644 --- a/plugin/plugintest/example_hello_user_test.go +++ b/plugin/plugintest/example_hello_user_test.go @@ -42,8 +42,11 @@ func Example() { api.On("GetUser", user.Id).Return(user, nil) defer api.AssertExpectations(t) + helpers := &plugintest.Helpers{} + defer helpers.AssertExpectations(t) + p := &HelloUserPlugin{} - p.SetAPI(api) + p.SetAPI(api, helpers) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/", nil) diff --git a/plugin/plugintest/helpers.go b/plugin/plugintest/helpers.go new file mode 100644 index 0000000000..b77a4cea46 --- /dev/null +++ b/plugin/plugintest/helpers.go @@ -0,0 +1,34 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +// Regenerate this file using `make plugin-mocks`. + +package plugintest + +import mock "github.com/stretchr/testify/mock" +import model "github.com/mattermost/mattermost-server/model" + +// Helpers is an autogenerated mock type for the Helpers type +type Helpers struct { + mock.Mock +} + +// EnsureBot provides a mock function with given fields: bot +func (_m *Helpers) EnsureBot(bot *model.Bot) (string, error) { + ret := _m.Called(bot) + + var r0 string + if rf, ok := ret.Get(0).(func(*model.Bot) string); ok { + r0 = rf(bot) + } else { + r0 = ret.Get(0).(string) + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.Bot) error); ok { + r1 = rf(bot) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +}