diff --git a/app/bot.go b/app/bot.go index 484a6b311f..7b212d6f10 100644 --- a/app/bot.go +++ b/app/bot.go @@ -4,6 +4,7 @@ package app import ( + "errors" "fmt" "io" "mime/multipart" @@ -23,10 +24,16 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) { } bot.UserId = user.Id - savedBot, err := a.Srv().Store.Bot().Save(bot) - if err != nil { + savedBot, nErr := a.Srv().Store.Bot().Save(bot) + if nErr != nil { a.Srv().Store.User().PermanentDelete(bot.UserId) - return nil, err + var appErr *model.AppError + switch { + case errors.As(nErr, &appErr): // in case we haven't converted to plain error. + return nil, appErr + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("CreateBot", "app.bot.createbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) + } } // Get the owner of the bot, if one exists. If not, don't send a message @@ -86,17 +93,44 @@ func (a *App) PatchBot(botUserId string, botPatch *model.BotPatch) (*model.Bot, ruser := userUpdate.New a.sendUpdatedUserEvent(*ruser) - return a.Srv().Store.Bot().Update(bot) + bot, nErr := a.Srv().Store.Bot().Update(bot) + if nErr != nil { + var nfErr *store.ErrNotFound + var appErr *model.AppError + switch { + case errors.As(nErr, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + case errors.As(nErr, &appErr): // in case we haven't converted to plain error. + return nil, appErr + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("PatchBot", "app.bot.patchbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) + } + } + return bot, nil } // GetBot returns the given bot. func (a *App) GetBot(botUserId string, includeDeleted bool) (*model.Bot, *model.AppError) { - return a.Srv().Store.Bot().Get(botUserId, includeDeleted) + bot, err := a.Srv().Store.Bot().Get(botUserId, includeDeleted) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("GetBot", "app.bot.getbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } + } + return bot, nil } // GetBots returns the requested page of bots. func (a *App) GetBots(options *model.BotGetOptions) (model.BotList, *model.AppError) { - return a.Srv().Store.Bot().GetAll(options) + bots, err := a.Srv().Store.Bot().GetAll(options) + if err != nil { + return nil, model.NewAppError("GetBots", "app.bot.getbots.internal_error", nil, err.Error(), http.StatusInternalServerError) + } + return bots, nil } // UpdateBotActive marks a bot as active or inactive, along with its corresponding user. @@ -110,9 +144,15 @@ func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model return nil, err } - bot, err := a.Srv().Store.Bot().Get(botUserId, true) - if err != nil { - return nil, err + bot, nErr := a.Srv().Store.Bot().Get(botUserId, true) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("UpdateBotActive", "app.bot.getbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) + } } changed := true @@ -125,9 +165,18 @@ func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model } if changed { - bot, err = a.Srv().Store.Bot().Update(bot) - if err != nil { - return nil, err + bot, nErr = a.Srv().Store.Bot().Update(bot) + if nErr != nil { + var nfErr *store.ErrNotFound + var appErr *model.AppError + switch { + case errors.As(nErr, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + case errors.As(nErr, &appErr): // in case we haven't converted to plain error. + return nil, appErr + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("PatchBot", "app.bot.patchbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError) + } } } @@ -137,7 +186,13 @@ func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model // PermanentDeleteBot permanently deletes a bot and its corresponding user. func (a *App) PermanentDeleteBot(botUserId string) *model.AppError { if err := a.Srv().Store.Bot().PermanentDelete(botUserId); err != nil { - return err + var invErr *store.ErrInvalidInput + switch { + case errors.As(err, &invErr): + return model.NewAppError("PermanentDeleteBot", "app.bot.permenent_delete.bad_id", map[string]interface{}{"user_id": invErr.Value}, invErr.Error(), http.StatusBadRequest) + default: // last fallback in case it doesn't map to an existing app error. + return model.NewAppError("PatchBot", "app.bot.permanent_delete.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } if err := a.Srv().Store.User().PermanentDelete(botUserId); err != nil { @@ -151,14 +206,29 @@ func (a *App) PermanentDeleteBot(botUserId string) *model.AppError { func (a *App) UpdateBotOwner(botUserId, newOwnerId string) (*model.Bot, *model.AppError) { bot, err := a.Srv().Store.Bot().Get(botUserId, true) if err != nil { - return nil, err + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("UpdateBotOwner", "app.bot.getbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } bot.OwnerId = newOwnerId bot, err = a.Srv().Store.Bot().Update(bot) if err != nil { - return nil, err + var nfErr *store.ErrNotFound + var appErr *model.AppError + switch { + case errors.As(err, &nfErr): + return nil, model.MakeBotNotFoundError(nfErr.Id) + case errors.As(err, &appErr): // in case we haven't converted to plain error. + return nil, appErr + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("PatchBot", "app.bot.patchbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } return bot, nil @@ -311,7 +381,17 @@ func (a *App) getDisableBotSysadminMessage(user *model.User, userBots model.BotL // ConvertUserToBot converts a user to bot. func (a *App) ConvertUserToBot(user *model.User) (*model.Bot, *model.AppError) { - return a.Srv().Store.Bot().Save(model.BotFromUser(user)) + bot, err := a.Srv().Store.Bot().Save(model.BotFromUser(user)) + if err != nil { + var appErr *model.AppError + switch { + case errors.As(err, &appErr): // in case we haven't converted to plain error. + return nil, appErr + default: // last fallback in case it doesn't map to an existing app error. + return nil, model.NewAppError("CreateBot", "app.bot.createbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } + } + return bot, nil } // SetBotIconImageFromMultiPartFile sets LHS icon for a bot. @@ -344,8 +424,17 @@ func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppEr } bot.LastIconUpdate = model.GetMillis() - if _, err = a.Srv().Store.Bot().Update(bot); err != nil { - return err + if _, err := a.Srv().Store.Bot().Update(bot); err != nil { + var nfErr *store.ErrNotFound + var appErr *model.AppError + switch { + case errors.As(err, &nfErr): + return model.MakeBotNotFoundError(nfErr.Id) + case errors.As(err, &appErr): // in case we haven't converted to plain error. + return appErr + default: // last fallback in case it doesn't map to an existing app error. + return model.NewAppError("SetBotIconImage", "app.bot.patchbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } a.invalidateUserCacheAndPublish(botUserId) @@ -369,8 +458,17 @@ func (a *App) DeleteBotIconImage(botUserId string) *model.AppError { } bot.LastIconUpdate = int64(0) - if _, err = a.Srv().Store.Bot().Update(bot); err != nil { - return err + if _, err := a.Srv().Store.Bot().Update(bot); err != nil { + var nfErr *store.ErrNotFound + var appErr *model.AppError + switch { + case errors.As(err, &nfErr): + return model.MakeBotNotFoundError(nfErr.Id) + case errors.As(err, &appErr): // in case we haven't converted to plain error. + return appErr + default: // last fallback in case it doesn't map to an existing app error. + return model.NewAppError("DeleteBotIconImage", "app.bot.patchbot.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } a.invalidateUserCacheAndPublish(botUserId) diff --git a/app/user.go b/app/user.go index d94899f0a1..87c77306ea 100644 --- a/app/user.go +++ b/app/user.go @@ -7,6 +7,7 @@ import ( "bytes" b64 "encoding/base64" "encoding/json" + "errors" "fmt" "hash/fnv" "image" @@ -1467,7 +1468,13 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError { } if err := a.Srv().Store.Bot().PermanentDelete(user.Id); err != nil { - return err + var invErr *store.ErrInvalidInput + switch { + case errors.As(err, &invErr): + return model.NewAppError("PermanentDeleteUser", "app.bot.permenent_delete.bad_id", map[string]interface{}{"user_id": invErr.Value}, invErr.Error(), http.StatusBadRequest) + default: // last fallback in case it doesn't map to an existing app error. + return model.NewAppError("PermanentDeleteUser", "app.bot.permanent_delete.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } infos, err := a.Srv().Store.FileInfo().GetForUser(user.Id) diff --git a/cmd/mattermost/commands/user.go b/cmd/mattermost/commands/user.go index 08ed43941d..c3be674aeb 100644 --- a/cmd/mattermost/commands/user.go +++ b/cmd/mattermost/commands/user.go @@ -529,9 +529,9 @@ func botToUser(command *cobra.Command, args []string, a *app.App) error { } } - appErr = a.Srv().Store.Bot().PermanentDelete(user.Id) - if appErr != nil { - return fmt.Errorf("Unable to delete bot. Error: %s", appErr.Error()) + err = a.Srv().Store.Bot().PermanentDelete(user.Id) + if err != nil { + return fmt.Errorf("Unable to delete bot. Error: %v", err) } CommandPrettyPrintln("id: " + user.Id) diff --git a/cmd/mattermost/commands/user_test.go b/cmd/mattermost/commands/user_test.go index 7820e66ebb..82dd2aa7a6 100644 --- a/cmd/mattermost/commands/user_test.go +++ b/cmd/mattermost/commands/user_test.go @@ -126,14 +126,14 @@ func TestDeleteUserBotUser(t *testing.T) { user, err := th.App.Srv().Store.User().Save(model.UserFromBot(bot)) require.Nil(t, err) bot.UserId = user.Id - bot, err = th.App.Srv().Store.Bot().Save(bot) - require.Nil(t, err) + bot, nErr := th.App.Srv().Store.Bot().Save(bot) + require.Nil(t, nErr) th.CheckCommand(t, "user", "delete", bot.Username, "--confirm") _, err = th.App.Srv().Store.User().Get(user.Id) require.Error(t, err) - _, err = th.App.Srv().Store.Bot().Get(user.Id, true) - require.Error(t, err) + _, nErr = th.App.Srv().Store.Bot().Get(user.Id, true) + require.Error(t, nErr) } func TestConvertUser(t *testing.T) { diff --git a/i18n/en.json b/i18n/en.json index 966a3981c6..a6db5105aa 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2926,10 +2926,34 @@ "id": "app.admin.test_site_url.failure", "translation": "This is not a valid live URL" }, + { + "id": "app.bot.createbot.internal_error", + "translation": "Unable to save the bot." + }, { "id": "app.bot.get_disable_bot_sysadmin_message", "translation": "{{if .disableBotsSetting}}{{if .printAllBots}}{{.UserName}} was deactivated. They managed the following bot accounts which have now been disabled.\n\n{{.BotNames}}{{else}}{{.UserName}} was deactivated. They managed {{.NumBots}} bot accounts which have now been disabled, including the following:\n\n{{.BotNames}}{{end}}You can take ownership of each bot by enabling it at **Integrations > Bot Accounts** and creating new tokens for the bot.\n\nFor more information, see our [documentation](https://docs.mattermost.com/developer/bot-accounts.html#what-happens-when-a-user-who-owns-bot-accounts-is-disabled).{{else}}{{if .printAllBots}}{{.UserName}} was deactivated. They managed the following bot accounts which are still enabled.\n\n{{.BotNames}}\n{{else}}{{.UserName}} was deactivated. They managed {{.NumBots}} bot accounts which are still enabled, including the following:\n\n{{.BotNames}}{{end}}We strongly recommend you to take ownership of each bot by re-enabling it at **Integrations > Bot Accounts** and creating new tokens for the bot.\n\nFor more information, see our [documentation](https://docs.mattermost.com/developer/bot-accounts.html#what-happens-when-a-user-who-owns-bot-accounts-is-disabled).\n\nIf you want bot accounts to disable automatically after owner deactivation, set “Disable bot accounts when owner is deactivated” in **System Console > Integrations > Bot Accounts** to true.{{end}}" }, + { + "id": "app.bot.getbot.internal_error", + "translation": "Unable to get the bot." + }, + { + "id": "app.bot.getbots.internal_error", + "translation": "Unable to get the bots." + }, + { + "id": "app.bot.patchbot.internal_error", + "translation": "Unable to update the bot." + }, + { + "id": "app.bot.permanent_delete.internal_error", + "translation": "" + }, + { + "id": "app.bot.permenent_delete.bad_id", + "translation": "Unable to delete the bot." + }, { "id": "app.channel.create_channel.internal_error", "translation": "Unable to save channel." @@ -5790,34 +5814,10 @@ "id": "store.sql_audit.save.saving.app_error", "translation": "We encountered an error saving the audit." }, - { - "id": "store.sql_bot.delete.app_error", - "translation": "Unable to delete the bot." - }, - { - "id": "store.sql_bot.get.app_error", - "translation": "Unable to get the bot." - }, { "id": "store.sql_bot.get.missing.app_error", "translation": "Bot does not exist." }, - { - "id": "store.sql_bot.get_all.app_error", - "translation": "Unable to get the bots." - }, - { - "id": "store.sql_bot.save.app_error", - "translation": "Unable to save the bot." - }, - { - "id": "store.sql_bot.update.app_error", - "translation": "Unable to update the bot." - }, - { - "id": "store.sql_bot.update.updating.app_error", - "translation": "We encountered an error updating the bot." - }, { "id": "store.sql_channel.analytics_deleted_type_count.app_error", "translation": "Unable to get deleted channel type counts." diff --git a/store/errors.go b/store/errors.go index 7fb7a25085..3208e3eef4 100644 --- a/store/errors.go +++ b/store/errors.go @@ -68,6 +68,19 @@ func (e *ErrConflict) Unwrap() error { return e.err } -// TODO: -// type ErrNotFound struct { -// } +// ErrNotFound indicates that a resource was not found +type ErrNotFound struct { + resource string + Id string +} + +func NewErrNotFound(resource, id string) *ErrNotFound { + return &ErrNotFound{ + resource: resource, + Id: id, + } +} + +func (e *ErrNotFound) Error() string { + return "resource: " + e.resource + " id: " + e.Id +} diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index a6de601b57..dc1d6bacee 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -383,7 +383,7 @@ func (s *OpenTracingLayerAuditStore) Save(audit *model.Audit) *model.AppError { return resultVar0 } -func (s *OpenTracingLayerBotStore) Get(userId string, includeDeleted bool) (*model.Bot, *model.AppError) { +func (s *OpenTracingLayerBotStore) Get(userId string, includeDeleted bool) (*model.Bot, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "BotStore.Get") s.Root.Store.SetContext(newCtx) @@ -401,7 +401,7 @@ func (s *OpenTracingLayerBotStore) Get(userId string, includeDeleted bool) (*mod return resultVar0, resultVar1 } -func (s *OpenTracingLayerBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { +func (s *OpenTracingLayerBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "BotStore.GetAll") s.Root.Store.SetContext(newCtx) @@ -419,7 +419,7 @@ func (s *OpenTracingLayerBotStore) GetAll(options *model.BotGetOptions) ([]*mode return resultVar0, resultVar1 } -func (s *OpenTracingLayerBotStore) PermanentDelete(userId string) *model.AppError { +func (s *OpenTracingLayerBotStore) PermanentDelete(userId string) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "BotStore.PermanentDelete") s.Root.Store.SetContext(newCtx) @@ -437,7 +437,7 @@ func (s *OpenTracingLayerBotStore) PermanentDelete(userId string) *model.AppErro return resultVar0 } -func (s *OpenTracingLayerBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { +func (s *OpenTracingLayerBotStore) Save(bot *model.Bot) (*model.Bot, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "BotStore.Save") s.Root.Store.SetContext(newCtx) @@ -455,7 +455,7 @@ func (s *OpenTracingLayerBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppE return resultVar0, resultVar1 } -func (s *OpenTracingLayerBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { +func (s *OpenTracingLayerBotStore) Update(bot *model.Bot) (*model.Bot, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "BotStore.Update") s.Root.Store.SetContext(newCtx) diff --git a/store/searchtest/helper.go b/store/searchtest/helper.go index d97669d717..2e08c810d2 100644 --- a/store/searchtest/helper.go +++ b/store/searchtest/helper.go @@ -218,10 +218,10 @@ func (th *SearchTestHelper) createBot(username, displayName, ownerID string) (*m } botModel.UserId = user.Id - bot, apperr := th.Store.Bot().Save(botModel) - if apperr != nil { + bot, err := th.Store.Bot().Save(botModel) + if err != nil { th.Store.User().PermanentDelete(bot.UserId) - return nil, errors.New(apperr.Error()) + return nil, errors.New(err.Error()) } return bot, nil diff --git a/store/searchtest/user_layer.go b/store/searchtest/user_layer.go index 54bd141a13..7704889006 100644 --- a/store/searchtest/user_layer.go +++ b/store/searchtest/user_layer.go @@ -497,6 +497,7 @@ func testAutocompleteUserByUsernameWithHyphen(t *testing.T, th *SearchTestHelper func testShouldEscapePercentageCharacter(t *testing.T, th *SearchTestHelper) { userAlternate, err := th.createUser("alternateusername", "alternate%nickname", "firstname", "altlastname") require.Nil(t, err) + defer th.deleteUser(userAlternate) err = th.addUserToTeams(userAlternate, []string{th.Team.Id}) require.Nil(t, err) @@ -641,6 +642,7 @@ func testSearchUsersShouldBeCaseInsensitive(t *testing.T, th *SearchTestHelper) func testSearchOneTwoCharUsersnameAndFirstLastNames(t *testing.T, th *SearchTestHelper) { userAlternate, err := th.createUser("ho", "alternatenickname", "zi", "k") require.Nil(t, err) + defer th.deleteUser(userAlternate) err = th.addUserToTeams(userAlternate, []string{th.Team.Id}) require.Nil(t, err) @@ -668,6 +670,7 @@ func testShouldSupportKoreanCharacters(t *testing.T, th *SearchTestHelper) { userAlternate, err := th.createUser("alternate-username", "alternate-nickname", "서강준", "안신원") require.Nil(t, err) defer th.deleteUser(userAlternate) + err = th.addUserToTeams(userAlternate, []string{th.Team.Id}) require.Nil(t, err) _, err = th.addUserToChannels(userAlternate, []string{th.ChannelBasic.Id}) diff --git a/store/sqlstore/bot_store.go b/store/sqlstore/bot_store.go index f37cba48bd..4a8ebf61f2 100644 --- a/store/sqlstore/bot_store.go +++ b/store/sqlstore/bot_store.go @@ -5,12 +5,14 @@ package sqlstore import ( "database/sql" - "net/http" + "fmt" "strings" "github.com/mattermost/mattermost-server/v5/einterfaces" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + + "github.com/pkg/errors" ) // bot is a subset of the model.Bot type, omitting the model.User fields. @@ -64,21 +66,8 @@ func newSqlBotStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) sto func (us SqlBotStore) createIndexesIfNotExists() { } -// traceBot is a helper function for adding to a bot trace when logging. -func traceBot(bot *model.Bot, extra map[string]interface{}) map[string]interface{} { - trace := make(map[string]interface{}) - for key, value := range bot.Trace() { - trace[key] = value - } - for key, value := range extra { - trace[key] = value - } - - return trace -} - // Get fetches the given bot in the database. -func (us SqlBotStore) Get(botUserId string, includeDeleted bool) (*model.Bot, *model.AppError) { +func (us SqlBotStore) Get(botUserId string, includeDeleted bool) (*model.Bot, error) { var excludeDeletedSql = "AND b.DeleteAt = 0" if includeDeleted { excludeDeletedSql = "" @@ -106,16 +95,16 @@ func (us SqlBotStore) Get(botUserId string, includeDeleted bool) (*model.Bot, *m var bot *model.Bot if err := us.GetReplica().SelectOne(&bot, query, map[string]interface{}{"user_id": botUserId}); err == sql.ErrNoRows { - return nil, model.MakeBotNotFoundError(botUserId) + return nil, store.NewErrNotFound("Bot", botUserId) } else if err != nil { - return nil, model.NewAppError("SqlBotStore.Get", "store.sql_bot.get.app_error", map[string]interface{}{"user_id": botUserId}, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "selectone: user_id=%s", botUserId) } return bot, nil } // GetAll fetches from all bots in the database. -func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { +func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, error) { params := map[string]interface{}{ "offset": options.Page * options.PerPage, "limit": options.PerPage, @@ -169,7 +158,7 @@ func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model var bots []*model.Bot if _, err := us.GetReplica().Select(&bots, sql, params); err != nil { - return nil, model.NewAppError("SqlBotStore.GetAll", "store.sql_bot.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "select") } return bots, nil @@ -177,16 +166,16 @@ func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model // Save persists a new bot to the database. // It assumes the corresponding user was saved via the user store. -func (us SqlBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { +func (us SqlBotStore) Save(bot *model.Bot) (*model.Bot, error) { bot = bot.Clone() bot.PreSave() - if err := bot.IsValid(); err != nil { + if err := bot.IsValid(); err != nil { // TODO: change to return error in v6. return nil, err } if err := us.GetMaster().Insert(botFromModel(bot)); err != nil { - return nil, model.NewAppError("SqlBotStore.Save", "store.sql_bot.save.app_error", bot.Trace(), err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "insert: user_id=%s", bot.UserId) } return bot, nil @@ -194,11 +183,11 @@ func (us SqlBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { // Update persists an updated bot to the database. // It assumes the corresponding user was updated via the user store. -func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { +func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, error) { bot = bot.Clone() bot.PreUpdate() - if err := bot.IsValid(); err != nil { + if err := bot.IsValid(); err != nil { // TODO: needs to return error in v6 return nil, err } @@ -215,9 +204,9 @@ func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { bot = oldBot if count, err := us.GetMaster().Update(botFromModel(bot)); err != nil { - return nil, model.NewAppError("SqlBotStore.Update", "store.sql_bot.update.updating.app_error", bot.Trace(), err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "update: user_id=%s", bot.UserId) } else if count != 1 { - return nil, model.NewAppError("SqlBotStore.Update", "store.sql_bot.update.app_error", traceBot(bot, map[string]interface{}{"count": count}), "", http.StatusInternalServerError) + return nil, fmt.Errorf("unexpected count while updating bot: count=%d, userId=%s", count, bot.UserId) } return bot, nil @@ -225,10 +214,10 @@ func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { // PermanentDelete removes the bot from the database altogether. // If the corresponding user is to be deleted, it must be done via the user store. -func (us SqlBotStore) PermanentDelete(botUserId string) *model.AppError { +func (us SqlBotStore) PermanentDelete(botUserId string) error { query := "DELETE FROM Bots WHERE UserId = :user_id" if _, err := us.GetMaster().Exec(query, map[string]interface{}{"user_id": botUserId}); err != nil { - return model.NewAppError("SqlBotStore.Update", "store.sql_bot.delete.app_error", map[string]interface{}{"user_id": botUserId}, err.Error(), http.StatusBadRequest) + return store.NewErrInvalidInput("Bot", "UserId", botUserId) } return nil } diff --git a/store/store.go b/store/store.go index b35f7fa2b3..ef66dc4a4a 100644 --- a/store/store.go +++ b/store/store.go @@ -338,11 +338,11 @@ type UserStore interface { } type BotStore interface { - Get(userId string, includeDeleted bool) (*model.Bot, *model.AppError) - GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) - Save(bot *model.Bot) (*model.Bot, *model.AppError) - Update(bot *model.Bot) (*model.Bot, *model.AppError) - PermanentDelete(userId string) *model.AppError + Get(userId string, includeDeleted bool) (*model.Bot, error) + GetAll(options *model.BotGetOptions) ([]*model.Bot, error) + Save(bot *model.Bot) (*model.Bot, error) + Update(bot *model.Bot) (*model.Bot, error) + PermanentDelete(userId string) error } type SessionStore interface { diff --git a/store/storetest/bot_store.go b/store/storetest/bot_store.go index 5d2b15b782..b92405014f 100644 --- a/store/storetest/bot_store.go +++ b/store/storetest/bot_store.go @@ -4,7 +4,7 @@ package storetest import ( - "net/http" + "errors" "testing" "github.com/stretchr/testify/require" @@ -18,8 +18,8 @@ func makeBotWithUser(t *testing.T, ss store.Store, bot *model.Bot) (*model.Bot, require.Nil(t, err) bot.UserId = user.Id - bot, err = ss.Bot().Save(bot) - require.Nil(t, err) + bot, nErr := ss.Bot().Save(bot) + require.Nil(t, nErr) return bot, user } @@ -80,13 +80,15 @@ func testBotStoreGet(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("get non-existent bot", func(t *testing.T) { _, err := ss.Bot().Get("unknown", false) require.NotNil(t, err) - require.Equal(t, http.StatusNotFound, err.StatusCode) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr)) }) t.Run("get deleted bot", func(t *testing.T) { _, err := ss.Bot().Get(deletedBot.UserId, false) require.NotNil(t, err) - require.Equal(t, http.StatusNotFound, err.StatusCode) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr)) }) t.Run("get deleted bot, include deleted", func(t *testing.T) { @@ -98,7 +100,8 @@ func testBotStoreGet(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("get permanently deleted bot", func(t *testing.T) { _, err := ss.Bot().Get(permanentlyDeletedBot.UserId, false) require.NotNil(t, err) - require.Equal(t, http.StatusNotFound, err.StatusCode) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr)) }) t.Run("get bot 1", func(t *testing.T) { @@ -317,7 +320,9 @@ func testBotStoreSave(t *testing.T, ss store.Store) { _, err := ss.Bot().Save(bot) require.NotNil(t, err) - require.Equal(t, "model.bot.is_valid.username.app_error", err.Id) + var appErr *model.AppError + require.True(t, errors.As(err, &appErr)) + // require.Equal(t, "model.bot.is_valid.username.app_error", err.Id) }) t.Run("normal bot", func(t *testing.T) { @@ -332,8 +337,8 @@ func testBotStoreSave(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() bot.UserId = user.Id - returnedNewBot, err := ss.Bot().Save(bot) - require.Nil(t, err) + returnedNewBot, nErr := ss.Bot().Save(bot) + require.Nil(t, nErr) defer func() { require.Nil(t, ss.Bot().PermanentDelete(bot.UserId)) }() // Verify the returned bot matches the saved bot, modulo expected changes @@ -347,8 +352,8 @@ func testBotStoreSave(t *testing.T, ss store.Store) { require.Equal(t, bot, returnedNewBot) // Verify the actual bot in the database matches the saved bot. - actualNewBot, err := ss.Bot().Get(bot.UserId, false) - require.Nil(t, err) + actualNewBot, nErr := ss.Bot().Get(bot.UserId, false) + require.Nil(t, nErr) require.Equal(t, bot, actualNewBot) }) } @@ -366,7 +371,9 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { bot.Username = "invalid username" _, err := ss.Bot().Update(bot) require.NotNil(t, err) - require.Equal(t, "model.bot.is_valid.username.app_error", err.Id) + var appErr *model.AppError + require.True(t, errors.As(err, &appErr)) + require.Equal(t, "model.bot.is_valid.username.app_error", appErr.Id) }) t.Run("existing bot should update", func(t *testing.T) { @@ -458,7 +465,8 @@ func testBotStorePermanentDelete(t *testing.T, ss store.Store) { require.Nil(t, err) _, err = ss.Bot().Get(b1.UserId, false) - require.NotNil(t, err) - require.Equal(t, http.StatusNotFound, err.StatusCode) + require.Error(t, err) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr)) }) } diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 9213506744..58a3ce0c6e 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -1869,8 +1869,8 @@ func testTeamMembersToRemove(t *testing.T, ss store.Store) { DisplayName: "dn_" + model.NewId(), OwnerId: teamMember.UserId, } - bot, err = ss.Bot().Save(bot) - require.Nil(t, err) + bot, nErr := ss.Bot().Save(bot) + require.Nil(t, nErr) // verify that bot is not returned in results teamMembers, err = ss.Group().TeamMembersToRemove(nil) @@ -1878,8 +1878,8 @@ func testTeamMembersToRemove(t *testing.T, ss store.Store) { require.Len(t, teamMembers, 2) // delete the bot - err = ss.Bot().PermanentDelete(bot.UserId) - require.Nil(t, err) + nErr = ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, nErr) // Should be back to 3 users teamMembers, err = ss.Group().TeamMembersToRemove(nil) @@ -2021,8 +2021,8 @@ func testChannelMembersToRemove(t *testing.T, ss store.Store) { DisplayName: "dn_" + model.NewId(), OwnerId: channelMember.UserId, } - bot, err = ss.Bot().Save(bot) - require.Nil(t, err) + bot, nErr := ss.Bot().Save(bot) + require.Nil(t, nErr) // verify that bot is not returned in results channelMembers, err = ss.Group().ChannelMembersToRemove(nil) @@ -2030,8 +2030,8 @@ func testChannelMembersToRemove(t *testing.T, ss store.Store) { require.Len(t, channelMembers, 2) // delete the bot - err = ss.Bot().PermanentDelete(bot.UserId) - require.Nil(t, err) + nErr = ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, nErr) // Should be back to 3 users channelMembers, err = ss.Group().ChannelMembersToRemove(nil) diff --git a/store/storetest/mocks/BotStore.go b/store/storetest/mocks/BotStore.go index 47e9aad5da..0dd2a00eb4 100644 --- a/store/storetest/mocks/BotStore.go +++ b/store/storetest/mocks/BotStore.go @@ -15,7 +15,7 @@ type BotStore struct { } // Get provides a mock function with given fields: userId, includeDeleted -func (_m *BotStore) Get(userId string, includeDeleted bool) (*model.Bot, *model.AppError) { +func (_m *BotStore) Get(userId string, includeDeleted bool) (*model.Bot, error) { ret := _m.Called(userId, includeDeleted) var r0 *model.Bot @@ -27,20 +27,18 @@ func (_m *BotStore) Get(userId string, includeDeleted bool) (*model.Bot, *model. } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, bool) error); ok { r1 = rf(userId, includeDeleted) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAll provides a mock function with given fields: options -func (_m *BotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { +func (_m *BotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, error) { ret := _m.Called(options) var r0 []*model.Bot @@ -52,36 +50,32 @@ func (_m *BotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.A } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.BotGetOptions) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.BotGetOptions) error); ok { r1 = rf(options) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // PermanentDelete provides a mock function with given fields: userId -func (_m *BotStore) PermanentDelete(userId string) *model.AppError { +func (_m *BotStore) PermanentDelete(userId string) error { ret := _m.Called(userId) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { r0 = rf(userId) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // Save provides a mock function with given fields: bot -func (_m *BotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { +func (_m *BotStore) Save(bot *model.Bot) (*model.Bot, error) { ret := _m.Called(bot) var r0 *model.Bot @@ -93,20 +87,18 @@ func (_m *BotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Bot) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Bot) error); ok { r1 = rf(bot) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Update provides a mock function with given fields: bot -func (_m *BotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { +func (_m *BotStore) Update(bot *model.Bot) (*model.Bot, error) { ret := _m.Called(bot) var r0 *model.Bot @@ -118,13 +110,11 @@ func (_m *BotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Bot) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Bot) error); ok { r1 = rf(bot) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 637f1a80d0..6d1539672a 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -1545,8 +1545,8 @@ func testPostCountsByDay(t *testing.T, ss store.Store) { OwnerId: model.NewId(), UserId: model.NewId(), } - _, err = ss.Bot().Save(bot1) - require.Nil(t, err) + _, nErr = ss.Bot().Save(bot1) + require.Nil(t, nErr) b1 := &model.Post{} b1.Message = "bot message one" diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 1e37a4e0ef..bcebc0df65 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -255,13 +255,13 @@ func testUserStoreGet(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: model.NewId(), }) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u2.Id, Username: u2.Username, Description: "bot description", OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u2.IsBot = true u2.BotDescription = "bot description" defer func() { require.Nil(t, ss.Bot().PermanentDelete(u2.Id)) }() @@ -323,12 +323,12 @@ func testGetAllUsingAuthService(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() @@ -379,12 +379,12 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { Username: "u3" + model.NewId(), }) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() @@ -543,12 +543,12 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { Username: "u3" + model.NewId(), }) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() @@ -685,12 +685,12 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -790,12 +790,12 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -897,12 +897,12 @@ func testUserStoreGetProfilesWithoutTeam(t *testing.T, ss store.Store) { }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -966,12 +966,12 @@ func testUserStoreGetAllProfilesInChannel(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1094,12 +1094,12 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1257,12 +1257,12 @@ func testUserStoreGetProfilesByIds(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1466,12 +1466,12 @@ func testUserStoreGetProfilesByUsernames(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: team2Id, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1537,12 +1537,12 @@ func testUserStoreGetSystemAdminProfiles(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1585,12 +1585,12 @@ func testUserStoreGetByEmail(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1660,12 +1660,12 @@ func testUserStoreGetByAuthData(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1731,12 +1731,12 @@ func testUserStoreGetByUsername(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -1809,12 +1809,12 @@ func testUserStoreGetForLogin(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2086,12 +2086,12 @@ func testUserStoreGetRecentlyActiveUsersForTeam(t *testing.T, ss store.Store, s defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2161,12 +2161,12 @@ func testUserStoreGetNewUsersForTeam(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2258,12 +2258,12 @@ func testUserStoreSearchNotInChannel(t *testing.T, ss store.Store) { _, err = ss.User().Save(u3) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2484,12 +2484,12 @@ func testUserStoreSearchInChannel(t *testing.T, ss store.Store) { _, err = ss.User().Save(u3) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2649,12 +2649,12 @@ func testUserStoreSearchNotInTeam(t *testing.T, ss store.Store) { _, err = ss.User().Save(u3) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2841,12 +2841,12 @@ func testUserStoreSearchWithoutTeam(t *testing.T, ss store.Store) { _, err = ss.User().Save(u3) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -2943,12 +2943,12 @@ func testCount(t *testing.T, ss store.Store) { }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -3067,12 +3067,12 @@ func testUserStoreAnalyticsActiveCount(t *testing.T, ss store.Store, s SqlSuppli require.Nil(t, ss.User().PermanentDelete(u4.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u4.Id, Username: u4.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) millis := model.GetMillis() millisTwoDaysAgo := model.GetMillis() - (2 * DAY_MILLISECONDS) @@ -3241,12 +3241,12 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() @@ -3475,12 +3475,12 @@ func testUserStoreGetAllAfter(t *testing.T, ss store.Store) { }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u2.Id)) }() - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u2.Id, Username: u2.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u2.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u2.Id)) }() @@ -4655,12 +4655,12 @@ func testGetKnownUsers(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1) require.Nil(t, err) - _, err = ss.Bot().Save(&model.Bot{ + _, nErr := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, }) - require.Nil(t, err) + require.Nil(t, nErr) u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() diff --git a/store/timer_layer.go b/store/timer_layer.go index f4f1773170..d900daa599 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -377,7 +377,7 @@ func (s *TimerLayerAuditStore) Save(audit *model.Audit) *model.AppError { return resultVar0 } -func (s *TimerLayerBotStore) Get(userId string, includeDeleted bool) (*model.Bot, *model.AppError) { +func (s *TimerLayerBotStore) Get(userId string, includeDeleted bool) (*model.Bot, error) { start := timemodule.Now() resultVar0, resultVar1 := s.BotStore.Get(userId, includeDeleted) @@ -393,7 +393,7 @@ func (s *TimerLayerBotStore) Get(userId string, includeDeleted bool) (*model.Bot return resultVar0, resultVar1 } -func (s *TimerLayerBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { +func (s *TimerLayerBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, error) { start := timemodule.Now() resultVar0, resultVar1 := s.BotStore.GetAll(options) @@ -409,7 +409,7 @@ func (s *TimerLayerBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, return resultVar0, resultVar1 } -func (s *TimerLayerBotStore) PermanentDelete(userId string) *model.AppError { +func (s *TimerLayerBotStore) PermanentDelete(userId string) error { start := timemodule.Now() resultVar0 := s.BotStore.PermanentDelete(userId) @@ -425,7 +425,7 @@ func (s *TimerLayerBotStore) PermanentDelete(userId string) *model.AppError { return resultVar0 } -func (s *TimerLayerBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { +func (s *TimerLayerBotStore) Save(bot *model.Bot) (*model.Bot, error) { start := timemodule.Now() resultVar0, resultVar1 := s.BotStore.Save(bot) @@ -441,7 +441,7 @@ func (s *TimerLayerBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) return resultVar0, resultVar1 } -func (s *TimerLayerBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { +func (s *TimerLayerBotStore) Update(bot *model.Bot) (*model.Bot, error) { start := timemodule.Now() resultVar0, resultVar1 := s.BotStore.Update(bot)