From 4dd2b5f039878fafbd069321281988c08496b0dc Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 22 Jun 2022 14:06:00 +0530 Subject: [PATCH] MM-44953: Refactor EnsureBot to be used from plugin.API (#20521) The mattermost-plugin-api code also needs to use this. Hence we make it available from plugin.API. https://mattermost.atlassian.net/browse/MM-44953 ```release-note NONE ``` --- app/app_iface.go | 5 +++++ app/bot.go | 24 ++++++++++++---------- app/opentracing/opentracing_layer.go | 22 ++++++++++++++++++++ app/plugin_api.go | 4 ++++ plugin/api.go | 5 +++++ plugin/api_timer_layer_generated.go | 7 +++++++ plugin/client_rpc_generated.go | 30 ++++++++++++++++++++++++++++ plugin/plugintest/api.go | 21 +++++++++++++++++++ 8 files changed, 108 insertions(+), 10 deletions(-) diff --git a/app/app_iface.go b/app/app_iface.go index bd422c5de9..8d7c1e3b45 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -125,6 +125,11 @@ type AppIface interface { // activation if inactive anywhere in the cluster. // Notifies cluster peers through config change. EnablePlugin(id string) *model.AppError + // EnsureBot provides similar functionality with the plugin-api BotService. It doesn't accept + // any ensureBotOptions hence it is not required for now. + // TODO: Once the focalboard migration completed, we should add this logic to the app and + // let plugin-api use the same code + EnsureBot(c *request.Context, productID string, bot *model.Bot) (string, error) // Expand announcements in incoming webhooks from Slack. Those announcements // can be found in the text attribute, or in the pretext, text, title and value // attributes of the attachment structure. The Slack attachment structure is diff --git a/app/bot.go b/app/bot.go index 0c8cf7136b..803cd83497 100644 --- a/app/bot.go +++ b/app/bot.go @@ -25,11 +25,15 @@ type botServiceWrapper struct { app AppIface } +func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot *model.Bot) (string, error) { + return w.app.EnsureBot(c, productID, bot) +} + // EnsureBot provides similar functionality with the plugin-api BotService. It doesn't accept // any ensureBotOptions hence it is not required for now. // TODO: Once the focalboard migration completed, we should add this logic to the app and // let plugin-api use the same code -func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot *model.Bot) (string, error) { +func (a *App) EnsureBot(c *request.Context, productID string, bot *model.Bot) (string, error) { if bot == nil { return "", errors.New("passed a nil bot") } @@ -38,7 +42,7 @@ func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot return "", errors.New("passed a bot with no username") } - botIDBytes, err := w.app.GetPluginKey(productID, botUserKey) + botIDBytes, err := a.GetPluginKey(productID, botUserKey) if err != nil { return "", err } @@ -54,7 +58,7 @@ func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot Description: &bot.Description, } - if _, err = w.app.PatchBot(botID, botPatch); err != nil { + if _, err = a.PatchBot(botID, botPatch); err != nil { return "", fmt.Errorf("failed to patch bot: %w", err) } @@ -62,13 +66,13 @@ func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot } // Check for an existing bot user with that username. If one exists, then use that. - if user, appErr := w.app.GetUserByUsername(bot.Username); appErr == nil && user != nil { + if user, appErr := a.GetUserByUsername(bot.Username); appErr == nil && user != nil { if user.IsBot { - if appErr := w.app.SetPluginKey(productID, botUserKey, []byte(user.Id)); appErr != nil { - w.app.Srv().Log.Warn("Failed to set claimed bot user id.", mlog.String("userid", user.Id), mlog.Err(appErr)) + if appErr := a.SetPluginKey(productID, botUserKey, []byte(user.Id)); appErr != nil { + return "", fmt.Errorf("failed to set plugin key: %w", err) } } else { - w.app.Srv().Log.Error("Product attempted to use an account that already exists. Convert user to a bot "+ + a.Srv().Log.Error("Product attempted to use an account that already exists. Convert user to a bot "+ "account in the CLI by running 'mattermost user convert --bot'. If the user is an "+ "existing user account you want to preserve, change its username and restart the Mattermost server, "+ "after which the plugin will create a bot account with that name. For more information about bot "+ @@ -81,13 +85,13 @@ func (w *botServiceWrapper) EnsureBot(c *request.Context, productID string, bot return user.Id, nil } - createdBot, err := w.app.CreateBot(c, bot) + createdBot, err := a.CreateBot(c, bot) if err != nil { return "", fmt.Errorf("failed to create bot: %w", err) } - if appErr := w.app.SetPluginKey(productID, botUserKey, []byte(createdBot.UserId)); appErr != nil { - w.app.Srv().Log.Warn("Failed to set created bot user id.", mlog.String("userid", createdBot.UserId), mlog.Err(appErr)) + if appErr := a.SetPluginKey(productID, botUserKey, []byte(createdBot.UserId)); appErr != nil { + return "", fmt.Errorf("failed to set plugin key: %w", err) } return createdBot.UserId, nil diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index c2417e813f..2bcdf12848 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -3911,6 +3911,28 @@ func (a *OpenTracingAppLayer) EnableUserAccessToken(token *model.UserAccessToken return resultVar0 } +func (a *OpenTracingAppLayer) EnsureBot(c *request.Context, productID string, bot *model.Bot) (string, error) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.EnsureBot") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.EnsureBot(c, productID, bot) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) EnvironmentConfig(filter func(reflect.StructField) bool) map[string]interface{} { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.EnvironmentConfig") diff --git a/app/plugin_api.go b/app/plugin_api.go index ba4c5243a0..3c7637a880 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -981,6 +981,10 @@ func (api *PluginAPI) PermanentDeleteBot(userID string) *model.AppError { return api.app.PermanentDeleteBot(userID) } +func (api *PluginAPI) EnsureBotUser(bot *model.Bot) (string, error) { + return api.app.EnsureBot(api.ctx, api.id, bot) +} + func (api *PluginAPI) PublishUserTyping(userID, channelID, parentId string) *model.AppError { return api.app.PublishUserTyping(userID, channelID, parentId) } diff --git a/plugin/api.go b/plugin/api.go index c2e43e4a61..a23ec2331e 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -1152,6 +1152,11 @@ type API interface { // // Minimum server version: 7.0 GetCloudLimits() (*model.ProductLimits, error) + + // EnsureBotUser updates the bot if it exists, otherwise creates it. + // + // Minimum server version: 7.1 + EnsureBotUser(bot *model.Bot) (string, error) } var handshake = plugin.HandshakeConfig{ diff --git a/plugin/api_timer_layer_generated.go b/plugin/api_timer_layer_generated.go index a6d300be20..d0d236802a 100644 --- a/plugin/api_timer_layer_generated.go +++ b/plugin/api_timer_layer_generated.go @@ -1232,3 +1232,10 @@ func (api *apiTimerLayer) GetCloudLimits() (*model.ProductLimits, error) { api.recordTime(startTime, "GetCloudLimits", _returnsB == nil) return _returnsA, _returnsB } + +func (api *apiTimerLayer) EnsureBotUser(bot *model.Bot) (string, error) { + startTime := timePkg.Now() + _returnsA, _returnsB := api.apiImpl.EnsureBotUser(bot) + api.recordTime(startTime, "EnsureBotUser", _returnsB == nil) + return _returnsA, _returnsB +} diff --git a/plugin/client_rpc_generated.go b/plugin/client_rpc_generated.go index 3cef284744..8c9b9162b1 100644 --- a/plugin/client_rpc_generated.go +++ b/plugin/client_rpc_generated.go @@ -5678,3 +5678,33 @@ func (s *apiRPCServer) GetCloudLimits(args *Z_GetCloudLimitsArgs, returns *Z_Get } return nil } + +type Z_EnsureBotUserArgs struct { + A *model.Bot +} + +type Z_EnsureBotUserReturns struct { + A string + B error +} + +func (g *apiRPCClient) EnsureBotUser(bot *model.Bot) (string, error) { + _args := &Z_EnsureBotUserArgs{bot} + _returns := &Z_EnsureBotUserReturns{} + if err := g.client.Call("Plugin.EnsureBotUser", _args, _returns); err != nil { + log.Printf("RPC call to EnsureBotUser API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) EnsureBotUser(args *Z_EnsureBotUserArgs, returns *Z_EnsureBotUserReturns) error { + if hook, ok := s.impl.(interface { + EnsureBotUser(bot *model.Bot) (string, error) + }); ok { + returns.A, returns.B = hook.EnsureBotUser(args.A) + returns.B = encodableError(returns.B) + } else { + return encodableError(fmt.Errorf("API EnsureBotUser called but not implemented.")) + } + return nil +} diff --git a/plugin/plugintest/api.go b/plugin/plugintest/api.go index a19076750e..b42cf52127 100644 --- a/plugin/plugintest/api.go +++ b/plugin/plugintest/api.go @@ -620,6 +620,27 @@ func (_m *API) EnablePlugin(id string) *model.AppError { return r0 } +// EnsureBotUser provides a mock function with given fields: bot +func (_m *API) EnsureBotUser(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 +} + // ExecuteSlashCommand provides a mock function with given fields: commandArgs func (_m *API) ExecuteSlashCommand(commandArgs *model.CommandArgs) (*model.CommandResponse, error) { ret := _m.Called(commandArgs)