diff --git a/app/bot.go b/app/bot.go index 2f57a10ce6..a2d0c63d5b 100644 --- a/app/bot.go +++ b/app/bot.go @@ -18,10 +18,10 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) { } bot.UserId = result.Data.(*model.User).Id - result = <-a.Srv.Store.Bot().Save(bot) - if result.Err != nil { + savedBot, err := a.Srv.Store.Bot().Save(bot) + if err != nil { a.Srv.Store.User().PermanentDelete(bot.UserId) - return nil, result.Err + return nil, err } // Get the owner of the bot, if one exists. If not, don't send a message @@ -32,7 +32,7 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) { } else if ownerUser != nil { // Send a message to the bot's creator to inform them that the bot needs to be added // to a team and channel after it's created - channel, err := a.GetOrCreateDirectChannel(result.Data.(*model.Bot).UserId, bot.OwnerId) + channel, err := a.GetOrCreateDirectChannel(savedBot.UserId, bot.OwnerId) if err != nil { return nil, err } @@ -40,7 +40,7 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) { T := utils.GetUserTranslations(ownerUser.Locale) botAddPost := &model.Post{ Type: model.POST_ADD_BOT_TEAMS_CHANNELS, - UserId: result.Data.(*model.Bot).UserId, + UserId: savedBot.UserId, ChannelId: channel.Id, Message: T("api.bot.teams_channels.add_message_mobile"), } @@ -50,7 +50,7 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) { } } - return result.Data.(*model.Bot), nil + return savedBot, nil } // PatchBot applies the given patch to the bot and corresponding user. @@ -76,32 +76,17 @@ func (a *App) PatchBot(botUserId string, botPatch *model.BotPatch) (*model.Bot, return nil, err } - result := <-a.Srv.Store.Bot().Update(bot) - if result.Err != nil { - return nil, result.Err - } - - return result.Data.(*model.Bot), nil + return a.Srv.Store.Bot().Update(bot) } // GetBot returns the given bot. func (a *App) GetBot(botUserId string, includeDeleted bool) (*model.Bot, *model.AppError) { - result := <-a.Srv.Store.Bot().Get(botUserId, includeDeleted) - if result.Err != nil { - return nil, result.Err - } - - return result.Data.(*model.Bot), nil + return a.Srv.Store.Bot().Get(botUserId, includeDeleted) } // GetBots returns the requested page of bots. func (a *App) GetBots(options *model.BotGetOptions) (model.BotList, *model.AppError) { - result := <-a.Srv.Store.Bot().GetAll(options) - if result.Err != nil { - return nil, result.Err - } - - return result.Data.([]*model.Bot), nil + return a.Srv.Store.Bot().GetAll(options) } // UpdateBotActive marks a bot as active or inactive, along with its corresponding user. @@ -111,15 +96,14 @@ func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model return nil, err } - if _, err := a.UpdateActive(user, active); err != nil { + if _, err = a.UpdateActive(user, active); err != nil { return nil, err } - result := <-a.Srv.Store.Bot().Get(botUserId, true) - if result.Err != nil { - return nil, result.Err + bot, err := a.Srv.Store.Bot().Get(botUserId, true) + if err != nil { + return nil, err } - bot := result.Data.(*model.Bot) changed := true if active && bot.DeleteAt != 0 { @@ -131,11 +115,10 @@ func (a *App) UpdateBotActive(botUserId string, active bool) (*model.Bot, *model } if changed { - result := <-a.Srv.Store.Bot().Update(bot) - if result.Err != nil { - return nil, result.Err + bot, err = a.Srv.Store.Bot().Update(bot) + if err != nil { + return nil, err } - bot = result.Data.(*model.Bot) } return bot, nil @@ -143,8 +126,8 @@ 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 result := <-a.Srv.Store.Bot().PermanentDelete(botUserId); result.Err != nil { - return result.Err + if err := a.Srv.Store.Bot().PermanentDelete(botUserId); err != nil { + return err } if err := a.Srv.Store.User().PermanentDelete(botUserId); err != nil { @@ -156,19 +139,19 @@ func (a *App) PermanentDeleteBot(botUserId string) *model.AppError { // UpdateBotOwner changes a bot's owner to the given value func (a *App) UpdateBotOwner(botUserId, newOwnerId string) (*model.Bot, *model.AppError) { - result := <-a.Srv.Store.Bot().Get(botUserId, true) - if result.Err != nil { - return nil, result.Err + bot, err := a.Srv.Store.Bot().Get(botUserId, true) + if err != nil { + return nil, err } - bot := result.Data.(*model.Bot) bot.OwnerId = newOwnerId - if result = <-a.Srv.Store.Bot().Update(bot); result.Err != nil { - return nil, result.Err + bot, err = a.Srv.Store.Bot().Update(bot) + if err != nil { + return nil, err } - return result.Data.(*model.Bot), nil + return bot, nil } // disableUserBots disables all bots owned by the given user @@ -207,9 +190,5 @@ func (a *App) disableUserBots(userId string) *model.AppError { // ConvertUserToBot converts a user to bot func (a *App) ConvertUserToBot(user *model.User) (*model.Bot, *model.AppError) { - result := <-a.Srv.Store.Bot().Save(model.BotFromUser(user)) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.Bot), nil + return a.Srv.Store.Bot().Save(model.BotFromUser(user)) } diff --git a/cmd/mattermost/commands/user.go b/cmd/mattermost/commands/user.go index b2cfc0f464..5f838269d7 100644 --- a/cmd/mattermost/commands/user.go +++ b/cmd/mattermost/commands/user.go @@ -512,9 +512,9 @@ func botToUser(command *cobra.Command, args []string, a *app.App) error { } } - result := <-a.Srv.Store.Bot().PermanentDelete(user.Id) - if result.Err != nil { - return fmt.Errorf("Unable to delete bot. Error: %s", result.Err.Error()) + appErr = a.Srv.Store.Bot().PermanentDelete(user.Id) + if appErr != nil { + return fmt.Errorf("Unable to delete bot. Error: %s", appErr.Error()) } CommandPrettyPrintln("id: " + user.Id) diff --git a/cmd/mattermost/commands/user_test.go b/cmd/mattermost/commands/user_test.go index 6b88ac1f8b..5ec2a6af38 100644 --- a/cmd/mattermost/commands/user_test.go +++ b/cmd/mattermost/commands/user_test.go @@ -132,8 +132,8 @@ func TestConvertUser(t *testing.T) { t.Run("Convert to bot from username", func(t *testing.T) { th.CheckCommand(t, "user", "convert", th.BasicUser.Username, "anotherinvaliduser", "--bot") - result := <-th.App.Srv.Store.Bot().Get(th.BasicUser.Id, false) - require.Nil(t, result.Err) + _, err := th.App.Srv.Store.Bot().Get(th.BasicUser.Id, false) + require.Nil(t, err) }) t.Run("Unable to convert to user with missing password", func(t *testing.T) { @@ -152,14 +152,14 @@ func TestConvertUser(t *testing.T) { err := th.RunCommand(t, "user", "convert", th.BasicUser.Username, "--user", "--password", "password") require.Nil(t, err) - result := <-th.App.Srv.Store.Bot().Get(th.BasicUser.Id, false) - require.NotNil(t, result.Err) + _, err = th.App.Srv.Store.Bot().Get(th.BasicUser.Id, false) + require.NotNil(t, err) }) t.Run("Convert to bot from email", func(t *testing.T) { th.CheckCommand(t, "user", "convert", th.BasicUser2.Email, "--bot") - result := <-th.App.Srv.Store.Bot().Get(th.BasicUser2.Id, false) - require.Nil(t, result.Err) + _, err := th.App.Srv.Store.Bot().Get(th.BasicUser2.Id, false) + require.Nil(t, err) }) t.Run("Convert to user with all flags", func(t *testing.T) { @@ -174,8 +174,8 @@ func TestConvertUser(t *testing.T) { "--system_admin") require.Nil(t, err) - result := <-th.App.Srv.Store.Bot().Get(th.BasicUser2.Id, false) - require.NotNil(t, result.Err) + _, err = th.App.Srv.Store.Bot().Get(th.BasicUser2.Id, false) + require.NotNil(t, err) user, appErr := th.App.Srv.Store.User().Get(th.BasicUser2.Id) require.Nil(t, appErr) diff --git a/store/sqlstore/bot_store.go b/store/sqlstore/bot_store.go index 7be1a8fe68..d2a257a487 100644 --- a/store/sqlstore/bot_store.go +++ b/store/sqlstore/bot_store.go @@ -76,72 +76,69 @@ func traceBot(bot *model.Bot, extra map[string]interface{}) map[string]interface } // Get fetches the given bot in the database. -func (us SqlBotStore) Get(botUserId string, includeDeleted bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var excludeDeletedSql = "AND b.DeleteAt = 0" - if includeDeleted { - excludeDeletedSql = "" - } +func (us SqlBotStore) Get(botUserId string, includeDeleted bool) (*model.Bot, *model.AppError) { + var excludeDeletedSql = "AND b.DeleteAt = 0" + if includeDeleted { + excludeDeletedSql = "" + } - var bot *model.Bot - if err := us.GetReplica().SelectOne(&bot, ` - SELECT - b.UserId, - u.Username, - u.FirstName AS DisplayName, - b.Description, - b.OwnerId, - b.CreateAt, - b.UpdateAt, - b.DeleteAt - FROM - Bots b - JOIN - Users u ON (u.Id = b.UserId) - WHERE - b.UserId = :user_id - `+excludeDeletedSql+` - `, map[string]interface{}{ - "user_id": botUserId, - }); err == sql.ErrNoRows { - result.Err = model.MakeBotNotFoundError(botUserId) - } else if err != nil { - result.Err = model.NewAppError("SqlBotStore.Get", "store.sql_bot.get.app_error", map[string]interface{}{"user_id": botUserId}, err.Error(), http.StatusInternalServerError) - } else { - result.Data = bot - } - }) + query := ` + SELECT + b.UserId, + u.Username, + u.FirstName AS DisplayName, + b.Description, + b.OwnerId, + b.CreateAt, + b.UpdateAt, + b.DeleteAt + FROM + Bots b + JOIN + Users u ON (u.Id = b.UserId) + WHERE + b.UserId = :user_id + ` + excludeDeletedSql + ` + ` + + 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) + } 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 bot, nil } // GetAll fetches from all bots in the database. -func (us SqlBotStore) GetAll(options *model.BotGetOptions) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - params := map[string]interface{}{ - "offset": options.Page * options.PerPage, - "limit": options.PerPage, - } +func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { + params := map[string]interface{}{ + "offset": options.Page * options.PerPage, + "limit": options.PerPage, + } - var conditions []string - var conditionsSql string - var additionalJoin string + var conditions []string + var conditionsSql string + var additionalJoin string - if !options.IncludeDeleted { - conditions = append(conditions, "b.DeleteAt = 0") - } - if options.OwnerId != "" { - conditions = append(conditions, "b.OwnerId = :creator_id") - params["creator_id"] = options.OwnerId - } - if options.OnlyOrphaned { - additionalJoin = "JOIN Users o ON (o.Id = b.OwnerId)" - conditions = append(conditions, "o.DeleteAt != 0") - } + if !options.IncludeDeleted { + conditions = append(conditions, "b.DeleteAt = 0") + } + if options.OwnerId != "" { + conditions = append(conditions, "b.OwnerId = :creator_id") + params["creator_id"] = options.OwnerId + } + if options.OnlyOrphaned { + additionalJoin = "JOIN Users o ON (o.Id = b.OwnerId)" + conditions = append(conditions, "o.DeleteAt != 0") + } - if len(conditions) > 0 { - conditionsSql = "WHERE " + strings.Join(conditions, " AND ") - } + if len(conditions) > 0 { + conditionsSql = "WHERE " + strings.Join(conditions, " AND ") + } - sql := ` + sql := ` SELECT b.UserId, u.Username, @@ -166,82 +163,67 @@ func (us SqlBotStore) GetAll(options *model.BotGetOptions) store.StoreChannel { :offset ` - var data []*model.Bot - if _, err := us.GetReplica().Select(&data, sql, params); err != nil { - result.Err = model.NewAppError("SqlBotStore.GetAll", "store.sql_bot.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) - } + 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) + } - result.Data = data - }) + return bots, nil } // 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) store.StoreChannel { +func (us SqlBotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { bot = bot.Clone() + bot.PreSave() - return store.Do(func(result *store.StoreResult) { - bot.PreSave() - if result.Err = bot.IsValid(); result.Err != nil { - return - } + if err := bot.IsValid(); err != nil { + return nil, err + } - if err := us.GetMaster().Insert(botFromModel(bot)); err != nil { - result.Err = model.NewAppError("SqlBotStore.Save", "store.sql_bot.save.app_error", bot.Trace(), err.Error(), http.StatusInternalServerError) - return - } + 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) + } - result.Data = bot - }) + return bot, nil } // 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) store.StoreChannel { +func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { bot = bot.Clone() - return store.Do(func(result *store.StoreResult) { - bot.PreUpdate() - if result.Err = bot.IsValid(); result.Err != nil { - return - } + bot.PreUpdate() + if err := bot.IsValid(); err != nil { + return nil, err + } - oldBotResult := <-us.Get(bot.UserId, true) - if oldBotResult.Err != nil { - result.Err = oldBotResult.Err - return - } - oldBot := oldBotResult.Data.(*model.Bot) + oldBot, err := us.Get(bot.UserId, true) + if err != nil { + return nil, err + } - oldBot.Description = bot.Description - oldBot.OwnerId = bot.OwnerId - oldBot.UpdateAt = bot.UpdateAt - oldBot.DeleteAt = bot.DeleteAt - bot = oldBot + oldBot.Description = bot.Description + oldBot.OwnerId = bot.OwnerId + oldBot.UpdateAt = bot.UpdateAt + oldBot.DeleteAt = bot.DeleteAt + bot = oldBot - if count, err := us.GetMaster().Update(botFromModel(bot)); err != nil { - result.Err = model.NewAppError("SqlBotStore.Update", "store.sql_bot.update.updating.app_error", bot.Trace(), err.Error(), http.StatusInternalServerError) - } else if count != 1 { - result.Err = model.NewAppError("SqlBotStore.Update", "store.sql_bot.update.app_error", traceBot(bot, map[string]interface{}{"count": count}), "", http.StatusInternalServerError) - } + 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) + } 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) + } - result.Data = bot - }) + return bot, nil } // 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) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if _, err := us.GetMaster().Exec(` - DELETE FROM - Bots - WHERE - UserId = :user_id - `, map[string]interface{}{ - "user_id": botUserId, - }); err != nil { - result.Err = model.NewAppError("SqlBotStore.Update", "store.sql_bot.delete.app_error", map[string]interface{}{"user_id": botUserId}, err.Error(), http.StatusBadRequest) - } - }) +func (us SqlBotStore) PermanentDelete(botUserId string) *model.AppError { + 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 nil } diff --git a/store/store.go b/store/store.go index 8572134f04..ee19a81b12 100644 --- a/store/store.go +++ b/store/store.go @@ -304,11 +304,11 @@ type UserStore interface { } type BotStore interface { - Get(userId string, includeDeleted bool) StoreChannel - GetAll(options *model.BotGetOptions) StoreChannel - Save(bot *model.Bot) StoreChannel - Update(bot *model.Bot) StoreChannel - PermanentDelete(userId string) StoreChannel + 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 } type SessionStore interface { diff --git a/store/storetest/bot_store.go b/store/storetest/bot_store.go index c599602f11..28ffd894b8 100644 --- a/store/storetest/bot_store.go +++ b/store/storetest/bot_store.go @@ -6,6 +6,7 @@ package storetest import ( "net/http" "testing" + "time" "github.com/stretchr/testify/require" @@ -17,7 +18,11 @@ func makeBotWithUser(ss store.Store, bot *model.Bot) (*model.Bot, *model.User) { user := store.Must(ss.User().Save(model.UserFromBot(bot))).(*model.User) bot.UserId = user.Id - bot = store.Must(ss.Bot().Save(bot)).(*model.Bot) + bot, err := ss.Bot().Save(bot) + if err != nil { + time.Sleep(time.Second) + panic(err) + } return bot, user } @@ -37,8 +42,9 @@ func testBotStoreGet(t *testing.T, ss store.Store) { OwnerId: model.NewId(), }) deletedBot.DeleteAt = 1 - deletedBot = store.Must(ss.Bot().Update(deletedBot)).(*model.Bot) - defer func() { store.Must(ss.Bot().PermanentDelete(deletedBot.UserId)) }() + deletedBot, err := ss.Bot().Update(deletedBot) + require.Nil(t, err) + defer func() { require.Nil(t, ss.Bot().PermanentDelete(deletedBot.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(deletedBot.UserId)) }() permanentlyDeletedBot, _ := makeBotWithUser(ss, &model.Bot{ @@ -47,7 +53,7 @@ func testBotStoreGet(t *testing.T, ss store.Store) { OwnerId: model.NewId(), DeleteAt: 0, }) - store.Must(ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId)) + require.Nil(t, ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId)) defer func() { require.Nil(t, ss.User().PermanentDelete(permanentlyDeletedBot.UserId)) }() b1, _ := makeBotWithUser(ss, &model.Bot{ @@ -55,7 +61,7 @@ func testBotStoreGet(t *testing.T, ss store.Store) { Description: "The first bot", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(b1.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b1.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b1.UserId)) }() b2, _ := makeBotWithUser(ss, &model.Bot{ @@ -63,43 +69,43 @@ func testBotStoreGet(t *testing.T, ss store.Store) { Description: "The second bot", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(b2.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b2.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b2.UserId)) }() t.Run("get non-existent bot", func(t *testing.T) { - result := <-ss.Bot().Get("unknown", false) - require.NotNil(t, result.Err) - require.Equal(t, http.StatusNotFound, result.Err.StatusCode) + _, err := ss.Bot().Get("unknown", false) + require.NotNil(t, err) + require.Equal(t, http.StatusNotFound, err.StatusCode) }) t.Run("get deleted bot", func(t *testing.T) { - result := <-ss.Bot().Get(deletedBot.UserId, false) - require.NotNil(t, result.Err) - require.Equal(t, http.StatusNotFound, result.Err.StatusCode) + _, err := ss.Bot().Get(deletedBot.UserId, false) + require.NotNil(t, err) + require.Equal(t, http.StatusNotFound, err.StatusCode) }) t.Run("get deleted bot, include deleted", func(t *testing.T) { - result := <-ss.Bot().Get(deletedBot.UserId, true) - require.Nil(t, result.Err) - require.Equal(t, deletedBot, result.Data.(*model.Bot)) + bot, err := ss.Bot().Get(deletedBot.UserId, true) + require.Nil(t, err) + require.Equal(t, deletedBot, bot) }) t.Run("get permanently deleted bot", func(t *testing.T) { - result := <-ss.Bot().Get(permanentlyDeletedBot.UserId, false) - require.NotNil(t, result.Err) - require.Equal(t, http.StatusNotFound, result.Err.StatusCode) + _, err := ss.Bot().Get(permanentlyDeletedBot.UserId, false) + require.NotNil(t, err) + require.Equal(t, http.StatusNotFound, err.StatusCode) }) t.Run("get bot 1", func(t *testing.T) { - result := <-ss.Bot().Get(b1.UserId, false) - require.Nil(t, result.Err) - require.Equal(t, b1, result.Data.(*model.Bot)) + bot, err := ss.Bot().Get(b1.UserId, false) + require.Nil(t, err) + require.Equal(t, b1, bot) }) t.Run("get bot 2", func(t *testing.T) { - result := <-ss.Bot().Get(b2.UserId, false) - require.Nil(t, result.Err) - require.Equal(t, b2, result.Data.(*model.Bot)) + bot, err := ss.Bot().Get(b2.UserId, false) + require.Nil(t, err) + require.Equal(t, b2, bot) }) } @@ -113,8 +119,9 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { OwnerId: OwnerId1, }) deletedBot.DeleteAt = 1 - deletedBot = store.Must(ss.Bot().Update(deletedBot)).(*model.Bot) - defer func() { store.Must(ss.Bot().PermanentDelete(deletedBot.UserId)) }() + deletedBot, err := ss.Bot().Update(deletedBot) + require.Nil(t, err) + defer func() { require.Nil(t, ss.Bot().PermanentDelete(deletedBot.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(deletedBot.UserId)) }() permanentlyDeletedBot, _ := makeBotWithUser(ss, &model.Bot{ @@ -123,7 +130,7 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { OwnerId: OwnerId1, DeleteAt: 0, }) - store.Must(ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId)) + require.Nil(t, ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId)) defer func() { require.Nil(t, ss.User().PermanentDelete(permanentlyDeletedBot.UserId)) }() b1, _ := makeBotWithUser(ss, &model.Bot{ @@ -131,7 +138,7 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Description: "The first bot", OwnerId: OwnerId1, }) - defer func() { store.Must(ss.Bot().PermanentDelete(b1.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b1.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b1.UserId)) }() b2, _ := makeBotWithUser(ss, &model.Bot{ @@ -139,16 +146,16 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Description: "The second bot", OwnerId: OwnerId1, }) - defer func() { store.Must(ss.Bot().PermanentDelete(b2.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b2.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b2.UserId)) }() t.Run("get original bots", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10}) - require.Nil(t, result.Err) + bot, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b1, b2, - }, result.Data.([]*model.Bot)) + }, bot) }) b3, _ := makeBotWithUser(ss, &model.Bot{ @@ -156,7 +163,7 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Description: "The third bot", OwnerId: OwnerId1, }) - defer func() { store.Must(ss.Bot().PermanentDelete(b3.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b3.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b3.UserId)) }() b4, _ := makeBotWithUser(ss, &model.Bot{ @@ -164,7 +171,7 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Description: "The fourth bot", OwnerId: OwnerId2, }) - defer func() { store.Must(ss.Bot().PermanentDelete(b4.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b4.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b4.UserId)) }() deletedUser := model.User{ @@ -184,106 +191,106 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Description: "Orphaned bot 5", OwnerId: deletedUser.Id, }) - defer func() { store.Must(ss.Bot().PermanentDelete(b4.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b4.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b4.UserId)) }() t.Run("get newly created bot stoo", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b1, b2, b3, b4, ob5, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get orphaned", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OnlyOrphaned: true}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OnlyOrphaned: true}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ ob5, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get page=0, per_page=2", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 2}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 2}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b1, b2, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get page=1, limit=2", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 1, PerPage: 2}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 1, PerPage: 2}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b3, b4, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get page=5, perpage=1000", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 5, PerPage: 1000}) - require.Nil(t, result.Err) - require.Equal(t, []*model.Bot{}, result.Data.([]*model.Bot)) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 5, PerPage: 1000}) + require.Nil(t, err) + require.Equal(t, []*model.Bot{}, bots) }) t.Run("get offset=0, limit=2, include deleted", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 2, IncludeDeleted: true}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 2, IncludeDeleted: true}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ deletedBot, b1, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get offset=2, limit=2, include deleted", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 1, PerPage: 2, IncludeDeleted: true}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 1, PerPage: 2, IncludeDeleted: true}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b2, b3, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get offset=0, limit=10, creator id 1", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OwnerId: OwnerId1}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OwnerId: OwnerId1}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b1, b2, b3, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get offset=0, limit=10, creator id 2", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OwnerId: OwnerId2}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, OwnerId: OwnerId2}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b4, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get offset=0, limit=10, include deleted, creator id 1", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, IncludeDeleted: true, OwnerId: OwnerId1}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, IncludeDeleted: true, OwnerId: OwnerId1}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ deletedBot, b1, b2, b3, - }, result.Data.([]*model.Bot)) + }, bots) }) t.Run("get offset=0, limit=10, include deleted, creator id 2", func(t *testing.T) { - result := <-ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, IncludeDeleted: true, OwnerId: OwnerId2}) - require.Nil(t, result.Err) + bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10, IncludeDeleted: true, OwnerId: OwnerId2}) + require.Nil(t, err) require.Equal(t, []*model.Bot{ b4, - }, result.Data.([]*model.Bot)) + }, bots) }) } @@ -295,9 +302,9 @@ func testBotStoreSave(t *testing.T, ss store.Store) { Description: "description", } - result := <-ss.Bot().Save(bot) - require.NotNil(t, result.Err) - require.Equal(t, "model.bot.is_valid.username.app_error", result.Err.Id) + _, err := ss.Bot().Save(bot) + require.NotNil(t, err) + require.Equal(t, "model.bot.is_valid.username.app_error", err.Id) }) t.Run("normal bot", func(t *testing.T) { @@ -311,12 +318,11 @@ func testBotStoreSave(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() bot.UserId = user.Id - result := <-ss.Bot().Save(bot) - require.Nil(t, result.Err) - defer func() { store.Must(ss.Bot().PermanentDelete(bot.UserId)) }() + returnedNewBot, err := ss.Bot().Save(bot) + require.Nil(t, err) + defer func() { require.Nil(t, ss.Bot().PermanentDelete(bot.UserId)) }() // Verify the returned bot matches the saved bot, modulo expected changes - returnedNewBot := result.Data.(*model.Bot) require.NotEqual(t, 0, returnedNewBot.CreateAt) require.NotEqual(t, 0, returnedNewBot.UpdateAt) require.Equal(t, returnedNewBot.CreateAt, returnedNewBot.UpdateAt) @@ -327,9 +333,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. - result = <-ss.Bot().Get(bot.UserId, false) - require.Nil(t, result.Err) - actualNewBot := result.Data.(*model.Bot) + actualNewBot, err := ss.Bot().Get(bot.UserId, false) + require.Nil(t, err) require.Equal(t, bot, actualNewBot) }) } @@ -340,14 +345,14 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { Username: "existing_bot", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(existingBot.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(existingBot.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(existingBot.UserId)) }() bot := existingBot.Clone() bot.Username = "invalid username" - result := <-ss.Bot().Update(bot) - require.NotNil(t, result.Err) - require.Equal(t, "model.bot.is_valid.username.app_error", result.Err.Id) + _, err := ss.Bot().Update(bot) + require.NotNil(t, err) + require.Equal(t, "model.bot.is_valid.username.app_error", err.Id) }) t.Run("existing bot should update", func(t *testing.T) { @@ -355,7 +360,7 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { Username: "existing_bot", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(existingBot.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(existingBot.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(existingBot.UserId)) }() bot := existingBot.Clone() @@ -365,11 +370,10 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { bot.UpdateAt = 999999 // Ignored bot.DeleteAt = 100000 // Allowed - result := <-ss.Bot().Update(bot) - require.Nil(t, result.Err) + returnedBot, err := ss.Bot().Update(bot) + require.Nil(t, err) // Verify the returned bot matches the updated bot, modulo expected timestamp changes - returnedBot := result.Data.(*model.Bot) require.Equal(t, existingBot.CreateAt, returnedBot.CreateAt) require.NotEqual(t, bot.UpdateAt, returnedBot.UpdateAt, "update should have advanced UpdateAt") require.True(t, returnedBot.UpdateAt > bot.UpdateAt, "update should have advanced UpdateAt") @@ -378,9 +382,9 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { bot.UpdateAt = returnedBot.UpdateAt // Verify the actual (now deleted) bot in the database - result = <-ss.Bot().Get(bot.UserId, true) - require.Nil(t, result.Err) - require.Equal(t, bot, result.Data.(*model.Bot)) + actualBot, err := ss.Bot().Get(bot.UserId, true) + require.Nil(t, err) + require.Equal(t, bot, actualBot) }) t.Run("deleted bot should update, restoring", func(t *testing.T) { @@ -388,27 +392,27 @@ func testBotStoreUpdate(t *testing.T, ss store.Store) { Username: "existing_bot", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(existingBot.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(existingBot.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(existingBot.UserId)) }() existingBot.DeleteAt = 100000 - existingBot = store.Must(ss.Bot().Update(existingBot)).(*model.Bot) + existingBot, err := ss.Bot().Update(existingBot) + require.Nil(t, err) bot := existingBot.Clone() bot.DeleteAt = 0 - result := <-ss.Bot().Update(bot) - require.Nil(t, result.Err) + returnedBot, err := ss.Bot().Update(bot) + require.Nil(t, err) // Verify the returned bot matches the updated bot, modulo expected timestamp changes - returnedBot := result.Data.(*model.Bot) require.EqualValues(t, 0, returnedBot.DeleteAt) bot.UpdateAt = returnedBot.UpdateAt // Verify the actual bot in the database - result = <-ss.Bot().Get(bot.UserId, false) - require.Nil(t, result.Err) - require.Equal(t, bot, result.Data.(*model.Bot)) + actualBot, err := ss.Bot().Get(bot.UserId, false) + require.Nil(t, err) + require.Equal(t, bot, actualBot) }) } @@ -417,27 +421,27 @@ func testBotStorePermanentDelete(t *testing.T, ss store.Store) { Username: "b1", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(b1.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b1.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b1.UserId)) }() b2, _ := makeBotWithUser(ss, &model.Bot{ Username: "b2", OwnerId: model.NewId(), }) - defer func() { store.Must(ss.Bot().PermanentDelete(b2.UserId)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(b2.UserId)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(b2.UserId)) }() t.Run("permanently delete a non-existent bot", func(t *testing.T) { - result := <-ss.Bot().PermanentDelete("unknown") - require.Nil(t, result.Err) + err := ss.Bot().PermanentDelete("unknown") + require.Nil(t, err) }) t.Run("permanently delete bot", func(t *testing.T) { - result := <-ss.Bot().PermanentDelete(b1.UserId) - require.Nil(t, result.Err) + err := ss.Bot().PermanentDelete(b1.UserId) + require.Nil(t, err) - result = <-ss.Bot().Get(b1.UserId, false) - require.NotNil(t, result.Err) - require.Equal(t, http.StatusNotFound, result.Err.StatusCode) + _, err = ss.Bot().Get(b1.UserId, false) + require.NotNil(t, err) + require.Equal(t, http.StatusNotFound, err.StatusCode) }) } diff --git a/store/storetest/group_supplier.go b/store/storetest/group_supplier.go index baf1b33dfb..a5a5c3272f 100644 --- a/store/storetest/group_supplier.go +++ b/store/storetest/group_supplier.go @@ -1259,9 +1259,8 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) { DisplayName: "dn_" + model.NewId(), OwnerId: teamMember.UserId, } - res = <-ss.Bot().Save(bot) - require.Nil(t, res.Err) - bot = res.Data.(*model.Bot) + bot, err = ss.Bot().Save(bot) + require.Nil(t, err) // verify that bot is not returned in results teamMembers, err = ss.Group().TeamMembersToRemove() @@ -1269,8 +1268,8 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) { require.Len(t, teamMembers, 2) // delete the bot - res = <-ss.Bot().PermanentDelete(bot.UserId) - require.Nil(t, res.Err) + err = ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, err) // Should be back to 3 users teamMembers, err = ss.Group().TeamMembersToRemove() @@ -1335,9 +1334,8 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) { DisplayName: "dn_" + model.NewId(), OwnerId: channelMember.UserId, } - res = <-ss.Bot().Save(bot) - require.Nil(t, res.Err) - bot = res.Data.(*model.Bot) + bot, err = ss.Bot().Save(bot) + require.Nil(t, err) // verify that bot is not returned in results channelMembers, err = ss.Group().ChannelMembersToRemove() @@ -1345,8 +1343,8 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) { require.Len(t, channelMembers, 2) // delete the bot - res = <-ss.Bot().PermanentDelete(bot.UserId) - require.Nil(t, res.Err) + err = ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, err) // Should be back to 3 users channelMembers, err = ss.Group().ChannelMembersToRemove() diff --git a/store/storetest/mocks/BotStore.go b/store/storetest/mocks/BotStore.go index 65c2b92497..d4514ddbaf 100644 --- a/store/storetest/mocks/BotStore.go +++ b/store/storetest/mocks/BotStore.go @@ -6,7 +6,6 @@ package mocks import mock "github.com/stretchr/testify/mock" import model "github.com/mattermost/mattermost-server/model" -import store "github.com/mattermost/mattermost-server/store" // BotStore is an autogenerated mock type for the BotStore type type BotStore struct { @@ -14,47 +13,65 @@ type BotStore struct { } // Get provides a mock function with given fields: userId, includeDeleted -func (_m *BotStore) Get(userId string, includeDeleted bool) store.StoreChannel { +func (_m *BotStore) Get(userId string, includeDeleted bool) (*model.Bot, *model.AppError) { ret := _m.Called(userId, includeDeleted) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok { + var r0 *model.Bot + if rf, ok := ret.Get(0).(func(string, bool) *model.Bot); ok { r0 = rf(userId, includeDeleted) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Bot) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + r1 = rf(userId, includeDeleted) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetAll provides a mock function with given fields: options -func (_m *BotStore) GetAll(options *model.BotGetOptions) store.StoreChannel { +func (_m *BotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) { ret := _m.Called(options) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.BotGetOptions) store.StoreChannel); ok { + var r0 []*model.Bot + if rf, ok := ret.Get(0).(func(*model.BotGetOptions) []*model.Bot); ok { r0 = rf(options) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.Bot) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.BotGetOptions) *model.AppError); ok { + r1 = rf(options) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // PermanentDelete provides a mock function with given fields: userId -func (_m *BotStore) PermanentDelete(userId string) store.StoreChannel { +func (_m *BotStore) PermanentDelete(userId string) *model.AppError { ret := _m.Called(userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { r0 = rf(userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -62,33 +79,51 @@ func (_m *BotStore) PermanentDelete(userId string) store.StoreChannel { } // Save provides a mock function with given fields: bot -func (_m *BotStore) Save(bot *model.Bot) store.StoreChannel { +func (_m *BotStore) Save(bot *model.Bot) (*model.Bot, *model.AppError) { ret := _m.Called(bot) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.Bot) store.StoreChannel); ok { + var r0 *model.Bot + if rf, ok := ret.Get(0).(func(*model.Bot) *model.Bot); ok { r0 = rf(bot) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Bot) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.Bot) *model.AppError); ok { + r1 = rf(bot) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // Update provides a mock function with given fields: bot -func (_m *BotStore) Update(bot *model.Bot) store.StoreChannel { +func (_m *BotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) { ret := _m.Called(bot) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.Bot) store.StoreChannel); ok { + var r0 *model.Bot + if rf, ok := ret.Get(0).(func(*model.Bot) *model.Bot); ok { r0 = rf(bot) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Bot) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.Bot) *model.AppError); ok { + r1 = rf(bot) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 6a068844d8..6bedc86f92 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -257,15 +257,16 @@ func testUserStoreGet(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: model.NewId(), })).(*model.User) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u2.Id, Username: u2.Username, Description: "bot description", OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u2.IsBot = true u2.BotDescription = "bot description" - defer func() { store.Must(ss.Bot().PermanentDelete(u2.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u2.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u2.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: model.NewId(), UserId: u1.Id}, -1)) @@ -317,13 +318,14 @@ func testGetAllUsingAuthService(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() t.Run("get by unknown auth service", func(t *testing.T) { @@ -371,13 +373,14 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: "u3" + model.NewId(), })).(*model.User) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() u4 := store.Must(ss.User().Save(&model.User{ @@ -535,13 +538,14 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: "u3" + model.NewId(), })).(*model.User) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) @@ -673,13 +677,14 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() ch1 := &model.Channel{ TeamId: teamId, @@ -765,13 +770,14 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() ch1 := &model.Channel{ TeamId: teamId, @@ -862,13 +868,14 @@ func testUserStoreGetProfilesWithoutTeam(t *testing.T, ss store.Store) { Username: "u3" + model.NewId(), })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get, offset 0, limit 100", func(t *testing.T) { result := <-ss.User().GetProfilesWithoutTeam(0, 100, nil) @@ -912,13 +919,14 @@ func testUserStoreGetAllProfilesInChannel(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() ch1 := &model.Channel{ TeamId: teamId, @@ -1023,13 +1031,14 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() ch1 := &model.Channel{ TeamId: teamId, @@ -1168,13 +1177,14 @@ func testUserStoreGetProfilesByIds(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get u1 by id, no caching", func(t *testing.T) { result := <-ss.User().GetProfileByIds([]string{u1.Id}, false, nil) @@ -1231,13 +1241,14 @@ func testUserStoreGetProfilesByUsernames(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: team2Id, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get by u1 and u2 usernames, team id 1", func(t *testing.T) { result := <-ss.User().GetProfilesByUsernames([]string{u1.Username, u2.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}}) @@ -1295,13 +1306,14 @@ func testUserStoreGetSystemAdminProfiles(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("all system admin profiles", func(t *testing.T) { result := <-ss.User().GetSystemAdminProfiles() @@ -1336,13 +1348,14 @@ func testUserStoreGetByEmail(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get u1 by email", func(t *testing.T) { u, err := ss.User().GetByEmail(u1.Email) @@ -1404,13 +1417,14 @@ func testUserStoreGetByAuthData(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get by u1 auth", func(t *testing.T) { u, err := ss.User().GetByAuth(u1.AuthData, u1.AuthService) @@ -1468,13 +1482,14 @@ func testUserStoreGetByUsername(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get u1 by username", func(t *testing.T) { result := <-ss.User().GetByUsername(u1.Username) @@ -1539,13 +1554,14 @@ func testUserStoreGetForLogin(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get u1 by username, allow both", func(t *testing.T) { result := <-ss.User().GetForLogin(u1.Username, true, true) @@ -1818,13 +1834,14 @@ func testUserStoreGetRecentlyActiveUsersForTeam(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() millis := model.GetMillis() u3.LastActivityAt = millis @@ -1886,13 +1903,14 @@ func testUserStoreGetNewUsersForTeam(t *testing.T, ss store.Store) { })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u3.Id}, -1)) - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() u4 := store.Must(ss.User().Save(&model.User{ Email: MakeEmail(), @@ -1996,13 +2014,14 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u3)) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() u5 := &model.User{ Username: "yu" + model.NewId(), @@ -2333,13 +2352,14 @@ func testUserStoreSearchNotInChannel(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u3)) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() tid := model.NewId() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}, -1)) @@ -2549,13 +2569,14 @@ func testUserStoreSearchInChannel(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u3)) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() tid := model.NewId() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}, -1)) @@ -2704,13 +2725,14 @@ func testUserStoreSearchNotInTeam(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u3)) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() u4 := &model.User{ Username: "simon" + model.NewId(), @@ -2883,13 +2905,14 @@ func testUserStoreSearchWithoutTeam(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u3)) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() tid := model.NewId() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u3.Id}, -1)) @@ -2979,13 +3002,14 @@ func testCount(t *testing.T, ss store.Store) { Email: MakeEmail(), })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() result := <-ss.User().Count(model.UserCountOptions{ IncludeBotAccounts: false, @@ -3166,13 +3190,14 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { Username: "u3" + model.NewId(), })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u3.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err = ss.Bot().Save(&model.Bot{ UserId: u3.Id, Username: u3.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u3.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u3.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() var etag1, etag2, etag3 string @@ -3391,13 +3416,14 @@ func testUserStoreGetAllAfter(t *testing.T, ss store.Store) { Username: "u2" + model.NewId(), })).(*model.User) defer func() { require.Nil(t, ss.User().PermanentDelete(u2.Id)) }() - store.Must(ss.Bot().Save(&model.Bot{ + _, err := ss.Bot().Save(&model.Bot{ UserId: u2.Id, Username: u2.Username, OwnerId: u1.Id, - })) + }) + require.Nil(t, err) u2.IsBot = true - defer func() { store.Must(ss.Bot().PermanentDelete(u2.Id)) }() + defer func() { require.Nil(t, ss.Bot().PermanentDelete(u2.Id)) }() expected := []*model.User{u1, u2} if strings.Compare(u2.Id, u1.Id) < 0 {