Skip to content

Commit

Permalink
Updated permanentDelete to receive user context as the first argument (
Browse files Browse the repository at this point in the history
…#26884)

Co-authored-by: Ezekiel <ezekielchow94@gmail.com>
  • Loading branch information
hanzei and ezekielchow committed Apr 29, 2024
1 parent a6aa92d commit 5746bb8
Show file tree
Hide file tree
Showing 21 changed files with 245 additions and 248 deletions.
4 changes: 2 additions & 2 deletions server/channels/api4/user_test.go
Expand Up @@ -2744,15 +2744,15 @@ func TestGetUsersWithoutTeam(t *testing.T) {
})
require.NoError(t, err)
th.LinkUserToTeam(user, th.BasicTeam)
defer th.App.Srv().Store().User().PermanentDelete(user.Id)
defer th.App.Srv().Store().User().PermanentDelete(th.Context, user.Id)

user2, _, err := th.Client.CreateUser(context.Background(), &model.User{
Username: "a000000001" + model.NewId(),
Email: "success+" + model.NewId() + "@simulator.amazonses.com",
Password: "Password1",
})
require.NoError(t, err)
defer th.App.Srv().Store().User().PermanentDelete(user2.Id)
defer th.App.Srv().Store().User().PermanentDelete(th.Context, user2.Id)

rusers, _, err := th.SystemAdminClient.GetUsersWithoutTeam(context.Background(), 0, 100, "")
require.NoError(t, err)
Expand Down
6 changes: 3 additions & 3 deletions server/channels/app/bot.go
Expand Up @@ -124,7 +124,7 @@ func (a *App) CreateBot(rctx request.CTX, bot *model.Bot) (*model.Bot, *model.Ap

savedBot, nErr := a.Srv().Store().Bot().Save(bot)
if nErr != nil {
a.Srv().Store().User().PermanentDelete(bot.UserId)
a.Srv().Store().User().PermanentDelete(rctx, bot.UserId)
var appErr *model.AppError
switch {
case errors.As(nErr, &appErr): // in case we haven't converted to plain error.
Expand Down Expand Up @@ -227,7 +227,7 @@ func (a *App) getOrCreateBot(rctx request.CTX, botDef *model.Bot) (*model.Bot, *
//save the bot
savedBot, nErr := a.Srv().Store().Bot().Save(botDef)
if nErr != nil {
a.Srv().Store().User().PermanentDelete(savedBot.UserId)
a.Srv().Store().User().PermanentDelete(rctx, savedBot.UserId)
var nAppErr *model.AppError
switch {
case errors.As(nErr, &nAppErr): // in case we haven't converted to plain error.
Expand Down Expand Up @@ -414,7 +414,7 @@ func (a *App) PermanentDeleteBot(rctx request.CTX, botUserId string) *model.AppE
}
}

if err := a.Srv().Store().User().PermanentDelete(botUserId); err != nil {
if err := a.Srv().Store().User().PermanentDelete(rctx, botUserId); err != nil {
return model.NewAppError("PermanentDeleteBot", "app.user.permanent_delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}

Expand Down
2 changes: 1 addition & 1 deletion server/channels/app/bot_test.go
Expand Up @@ -169,7 +169,7 @@ func TestEnsureBot(t *testing.T) {
assert.Equal(t, "username", bot.Username)
assert.Equal(t, "a bot", bot.Description)

err = th.App.Srv().Store().User().PermanentDelete(initialBot.UserId)
err = th.App.Srv().Store().User().PermanentDelete(th.Context, initialBot.UserId)
require.NoError(t, err)
botID2, err := th.App.EnsureBot(th.Context, pluginId, &model.Bot{
Username: "another_username",
Expand Down
2 changes: 1 addition & 1 deletion server/channels/app/channel_category_test.go
Expand Up @@ -19,7 +19,7 @@ func TestSidebarCategory(t *testing.T) {
basicChannel2 := th.CreateChannel(th.Context, th.BasicTeam)
defer th.App.PermanentDeleteChannel(th.Context, basicChannel2)
user := th.CreateUser()
defer th.App.Srv().Store().User().PermanentDelete(user.Id)
defer th.App.Srv().Store().User().PermanentDelete(th.Context, user.Id)
th.LinkUserToTeam(user, th.BasicTeam)
th.AddUserToChannel(user, basicChannel2)

Expand Down
2 changes: 1 addition & 1 deletion server/channels/app/teams/helper_test.go
Expand Up @@ -122,7 +122,7 @@ func (th *TestHelper) CreateUser(u *model.User) *model.User {
}

func (th *TestHelper) DeleteUser(u *model.User) {
err := th.dbStore.User().PermanentDelete(u.Id)
err := th.dbStore.User().PermanentDelete(th.Context, u.Id)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion server/channels/app/user.go
Expand Up @@ -1847,7 +1847,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr
return model.NewAppError("PermanentDeleteUser", "app.file_info.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}

if err := a.Srv().Store().User().PermanentDelete(user.Id); err != nil {
if err := a.Srv().Store().User().PermanentDelete(c, user.Id); err != nil {
return model.NewAppError("PermanentDeleteUser", "app.user.permanent_delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}

Expand Down
2 changes: 1 addition & 1 deletion server/channels/app/users/users_test.go
Expand Up @@ -56,7 +56,7 @@ func TestFirstUserPromoted(t *testing.T) {

require.Equal(t, model.SystemUserRoleId, user2.Roles)

th.dbStore.User().PermanentDelete(user.Id)
th.dbStore.User().PermanentDelete(th.Context, user.Id)

b := &model.Bot{
UserId: user2.Id,
Expand Down
4 changes: 2 additions & 2 deletions server/channels/store/opentracinglayer/opentracinglayer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions server/channels/store/retrylayer/retrylayer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions server/channels/store/searchlayer/user_layer.go
Expand Up @@ -90,15 +90,12 @@ func (s *SearchUserStore) Save(rctx request.CTX, user *model.User) (*model.User,
return nuser, err
}

func (s *SearchUserStore) PermanentDelete(userId string) error {
// TODO: Use the actuall request context from the App layer
// https://mattermost.atlassian.net/browse/MM-55738
rctx := request.EmptyContext(s.rootStore.Logger())
func (s *SearchUserStore) PermanentDelete(rctx request.CTX, userId string) error {
user, userErr := s.UserStore.Get(context.Background(), userId)
if userErr != nil {
rctx.Logger().Warn("Encountered error deleting user", mlog.String("user_id", userId), mlog.Err(userErr))
}
err := s.UserStore.PermanentDelete(userId)
err := s.UserStore.PermanentDelete(rctx, userId)
if err == nil && userErr == nil {
s.deleteUserIndex(rctx, user)
}
Expand Down
6 changes: 3 additions & 3 deletions server/channels/store/searchtest/helper.go
Expand Up @@ -192,14 +192,14 @@ func (th *SearchTestHelper) createGuest(username, nickname, firstName, lastName
}

func (th *SearchTestHelper) deleteUser(user *model.User) error {
return th.Store.User().PermanentDelete(user.Id)
return th.Store.User().PermanentDelete(th.Context, user.Id)
}

func (th *SearchTestHelper) deleteBotUser(botID string) error {
if err := th.deleteBot(botID); err != nil {
return err
}
return th.Store.User().PermanentDelete(botID)
return th.Store.User().PermanentDelete(th.Context, botID)
}

func (th *SearchTestHelper) cleanAllUsers() error {
Expand Down Expand Up @@ -233,7 +233,7 @@ func (th *SearchTestHelper) createBot(username, displayName, ownerID string) (*m
botModel.UserId = user.Id
bot, err := th.Store.Bot().Save(botModel)
if err != nil {
th.Store.User().PermanentDelete(bot.UserId)
th.Store.User().PermanentDelete(th.Context, bot.UserId)
return nil, errors.New(err.Error())
}

Expand Down
2 changes: 1 addition & 1 deletion server/channels/store/sqlstore/user_store.go
Expand Up @@ -1337,7 +1337,7 @@ func (us SqlUserStore) VerifyEmail(userId, email string) (string, error) {
return userId, nil
}

func (us SqlUserStore) PermanentDelete(userId string) error {
func (us SqlUserStore) PermanentDelete(rctx request.CTX, userId string) error {
if _, err := us.GetMasterX().Exec("DELETE FROM Users WHERE Id = ?", userId); err != nil {
return errors.Wrapf(err, "failed to delete User with userId=%s", userId)
}
Expand Down
2 changes: 1 addition & 1 deletion server/channels/store/store.go
Expand Up @@ -444,7 +444,7 @@ type UserStore interface {
GetEtagForProfiles(teamID string) string
UpdateFailedPasswordAttempts(userID string, attempts int) error
GetSystemAdminProfiles() (map[string]*model.User, error)
PermanentDelete(userID string) error
PermanentDelete(rctx request.CTX, userID string) error
AnalyticsActiveCount(timestamp int64, options model.UserCountOptions) (int64, error)
AnalyticsActiveCountForPeriod(startTime int64, endTime int64, options model.UserCountOptions) (int64, error)
GetUnreadCount(userID string, isCRTEnabled bool) (int64, error)
Expand Down
36 changes: 18 additions & 18 deletions server/channels/store/storetest/bot_store.go
Expand Up @@ -44,7 +44,7 @@ func testBotStoreGet(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore)
deletedBot, err := ss.Bot().Update(deletedBot)
require.NoError(t, err)
defer func() { require.NoError(t, ss.Bot().PermanentDelete(deletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(deletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, deletedBot.UserId)) }()

permanentlyDeletedBot, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "permanently_deleted_bot",
Expand All @@ -54,7 +54,7 @@ func testBotStoreGet(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore)
DeleteAt: 0,
})
require.NoError(t, ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId))
defer func() { require.NoError(t, ss.User().PermanentDelete(permanentlyDeletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, permanentlyDeletedBot.UserId)) }()

b1, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b1",
Expand All @@ -63,7 +63,7 @@ func testBotStoreGet(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore)
LastIconUpdate: model.GetMillis(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b1.UserId)) }()

b2, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b2",
Expand All @@ -72,7 +72,7 @@ func testBotStoreGet(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore)
LastIconUpdate: 0,
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b2.UserId)) }()

// Artificially set b2.LastIconUpdate to NULL to verify handling of same.
_, sqlErr := s.GetMasterX().Exec("UPDATE Bots SET LastIconUpdate = NULL WHERE UserId = '" + b2.UserId + "'")
Expand Down Expand Up @@ -132,7 +132,7 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
deletedBot, err := ss.Bot().Update(deletedBot)
require.NoError(t, err)
defer func() { require.NoError(t, ss.Bot().PermanentDelete(deletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(deletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, deletedBot.UserId)) }()

permanentlyDeletedBot, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "permanently_deleted_bot",
Expand All @@ -142,7 +142,7 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
DeleteAt: 0,
})
require.NoError(t, ss.Bot().PermanentDelete(permanentlyDeletedBot.UserId))
defer func() { require.NoError(t, ss.User().PermanentDelete(permanentlyDeletedBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, permanentlyDeletedBot.UserId)) }()

b1, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b1",
Expand All @@ -151,7 +151,7 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
LastIconUpdate: model.GetMillis(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b1.UserId)) }()

b2, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b2",
Expand All @@ -160,7 +160,7 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
LastIconUpdate: 0,
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b2.UserId)) }()

// Artificially set b2.LastIconUpdate to NULL to verify handling of same.
_, sqlErr := s.GetMasterX().Exec("UPDATE Bots SET LastIconUpdate = NULL WHERE UserId = '" + b2.UserId + "'")
Expand All @@ -181,15 +181,15 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
OwnerId: OwnerId1,
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b3.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b3.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b3.UserId)) }()

b4, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b4",
Description: "The fourth bot",
OwnerId: OwnerId2,
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b4.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b4.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b4.UserId)) }()

deletedUser := model.User{
Email: MakeEmail(),
Expand All @@ -202,14 +202,14 @@ func testBotStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store, s SqlSto
_, err2 := ss.User().Update(rctx, &deletedUser, true)
require.NoError(t, err2, "couldn't delete user")

defer func() { require.NoError(t, ss.User().PermanentDelete(deletedUser.Id)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, deletedUser.Id)) }()
ob5, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "ob5",
Description: "Orphaned bot 5",
OwnerId: deletedUser.Id,
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b4.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b4.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b4.UserId)) }()

t.Run("get newly created bot stoo", func(t *testing.T) {
bots, err := ss.Bot().GetAll(&model.BotGetOptions{Page: 0, PerPage: 10})
Expand Down Expand Up @@ -335,7 +335,7 @@ func testBotStoreSave(t *testing.T, rctx request.CTX, ss store.Store) {

user, err := ss.User().Save(rctx, model.UserFromBot(bot))
require.NoError(t, err)
defer func() { require.NoError(t, ss.User().PermanentDelete(user.Id)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, user.Id)) }()
bot.UserId = user.Id

returnedNewBot, nErr := ss.Bot().Save(bot)
Expand Down Expand Up @@ -366,7 +366,7 @@ func testBotStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) {
OwnerId: model.NewId(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, existingBot.UserId)) }()

bot := existingBot.Clone()
bot.Username = "invalid username"
Expand All @@ -383,7 +383,7 @@ func testBotStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) {
OwnerId: model.NewId(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, existingBot.UserId)) }()

bot := existingBot.Clone()
bot.OwnerId = model.NewId()
Expand Down Expand Up @@ -418,7 +418,7 @@ func testBotStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) {
OwnerId: model.NewId(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(existingBot.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, existingBot.UserId)) }()

existingBot.DeleteAt = 100000
existingBot, err := ss.Bot().Update(existingBot)
Expand Down Expand Up @@ -447,14 +447,14 @@ func testBotStorePermanentDelete(t *testing.T, rctx request.CTX, ss store.Store)
OwnerId: model.NewId(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b1.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b1.UserId)) }()

b2, _ := makeBotWithUser(t, rctx, ss, &model.Bot{
Username: "b2",
OwnerId: model.NewId(),
})
defer func() { require.NoError(t, ss.Bot().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(b2.UserId)) }()
defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, b2.UserId)) }()

t.Run("permanently delete a non-existent bot", func(t *testing.T) {
err := ss.Bot().PermanentDelete("unknown")
Expand Down
6 changes: 3 additions & 3 deletions server/channels/store/storetest/channel_store_categories.go
Expand Up @@ -2182,7 +2182,7 @@ func testClearSidebarOnTeamLeave(t *testing.T, rctx request.CTX, ss store.Store,
func testDeleteSidebarCategory(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
t.Run("should correctly remove an empty category", func(t *testing.T) {
userId, teamId := setupInitialSidebarCategories(t, rctx, ss)
defer ss.User().PermanentDelete(userId)
defer ss.User().PermanentDelete(rctx, userId)

newCategory, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{})
require.NoError(t, err)
Expand All @@ -2204,7 +2204,7 @@ func testDeleteSidebarCategory(t *testing.T, rctx request.CTX, ss store.Store, s

t.Run("should correctly remove a category and its channels", func(t *testing.T) {
userId, teamId := setupInitialSidebarCategories(t, rctx, ss)
defer ss.User().PermanentDelete(userId)
defer ss.User().PermanentDelete(rctx, userId)

user := &model.User{
Id: userId,
Expand Down Expand Up @@ -2272,7 +2272,7 @@ func testDeleteSidebarCategory(t *testing.T, rctx request.CTX, ss store.Store, s

t.Run("should not allow you to remove non-custom categories", func(t *testing.T) {
userId, teamId := setupInitialSidebarCategories(t, rctx, ss)
defer ss.User().PermanentDelete(userId)
defer ss.User().PermanentDelete(rctx, userId)
res, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId)
require.NoError(t, err)
require.Len(t, res.Categories, 3)
Expand Down
2 changes: 1 addition & 1 deletion server/channels/store/storetest/compliance_store.go
Expand Up @@ -21,7 +21,7 @@ func cleanupStoreState(t *testing.T, rctx request.CTX, ss store.Store) {
allUsers, err := ss.User().GetAll()
require.NoError(t, err, "error cleaning all test users", err)
for _, u := range allUsers {
err = ss.User().PermanentDelete(u.Id)
err = ss.User().PermanentDelete(rctx, u.Id)
require.NoError(t, err, "failed cleaning up test user %s", u.Username)

//remove all posts by this user
Expand Down
4 changes: 2 additions & 2 deletions server/channels/store/storetest/group_store.go
Expand Up @@ -5350,8 +5350,8 @@ func groupTestDistinctGroupMemberCountForSource(t *testing.T, rctx request.CTX,
ss.Group().Delete(customGroup.Id)
ss.Group().Delete(ldapGroup.Id)

ss.User().PermanentDelete(user1.Id)
ss.User().PermanentDelete(user2.Id)
ss.User().PermanentDelete(rctx, user1.Id)
ss.User().PermanentDelete(rctx, user2.Id)
}()

customGroupCount, err := ss.Group().DistinctGroupMemberCountForSource(model.GroupSourceCustom)
Expand Down

0 comments on commit 5746bb8

Please sign in to comment.