From e92ecf6696958fa1908d6d916404ced5d7fbc842 Mon Sep 17 00:00:00 2001 From: Shobhit Gupta Date: Tue, 18 Jun 2019 14:09:15 -0700 Subject: [PATCH] Migrate Channel.RemoveAllDeactivatedMembers to Sync by default (#11270) * Migrate Channel.RemoveAllDeactivatedMembers to Sync by default * Indent query string --- app/channel.go | 4 +-- store/sqlstore/channel_store.go | 45 +++++++++++++-------------- store/store.go | 2 +- store/storetest/channel_store.go | 2 +- store/storetest/mocks/ChannelStore.go | 8 ++--- 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/app/channel.go b/app/channel.go index eea511302f..3c6527c745 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1956,8 +1956,8 @@ func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError { // is in progress, and therefore should not be used from the API without first fixing this potential race condition. func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError { if removeDeactivatedMembers { - if result := <-a.Srv.Store.Channel().RemoveAllDeactivatedMembers(channel.Id); result.Err != nil { - return result.Err + if err := a.Srv.Store.Channel().RemoveAllDeactivatedMembers(channel.Id); err != nil { + return err } } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 330a477a06..4b470863c0 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1693,30 +1693,29 @@ func (s SqlChannelStore) RemoveMember(channelId string, userId string) *model.Ap return nil } -func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := ` - DELETE - FROM - ChannelMembers - WHERE - UserId IN ( - SELECT - Id - FROM - Users - WHERE - Users.DeleteAt != 0 - ) - AND - ChannelMembers.ChannelId = :ChannelId - ` +func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) *model.AppError { + query := ` + DELETE + FROM + ChannelMembers + WHERE + UserId IN ( + SELECT + Id + FROM + Users + WHERE + Users.DeleteAt != 0 + ) + AND + ChannelMembers.ChannelId = :ChannelId + ` - _, err := s.GetMaster().Exec(query, map[string]interface{}{"ChannelId": channelId}) - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.RemoveAllDeactivatedMembers", "store.sql_channel.remove_all_deactivated_members.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) - } - }) + _, err := s.GetMaster().Exec(query, map[string]interface{}{"ChannelId": channelId}) + if err != nil { + return model.NewAppError("SqlChannelStore.RemoveAllDeactivatedMembers", "store.sql_channel.remove_all_deactivated_members.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) + } + return nil } func (s SqlChannelStore) PermanentDeleteMembersByUser(userId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 1934e9ce73..bae858d58d 100644 --- a/store/store.go +++ b/store/store.go @@ -197,7 +197,7 @@ type ChannelStore interface { GetAllChannelsForExportAfter(limit int, afterId string) StoreChannel GetAllDirectChannelsForExportAfter(limit int, afterId string) StoreChannel GetChannelMembersForExport(userId string, teamId string) ([]*model.ChannelMemberForExport, *model.AppError) - RemoveAllDeactivatedMembers(channelId string) StoreChannel + RemoveAllDeactivatedMembers(channelId string) *model.AppError GetChannelsBatchForIndexing(startTime, endTime int64, limit int) ([]*model.Channel, *model.AppError) UserBelongsToChannels(userId string, channelIds []string) (bool, *model.AppError) } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 166d5114d4..d45c15dc9e 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -3426,7 +3426,7 @@ func testChannelStoreRemoveAllDeactivatedMembers(t *testing.T, ss store.Store) { require.Nil(t, err) // Remove all deactivated users from the channel. - assert.Nil(t, (<-ss.Channel().RemoveAllDeactivatedMembers(c1.Id)).Err) + assert.Nil(t, ss.Channel().RemoveAllDeactivatedMembers(c1.Id)) // Get all the channel members. Check there is now only 1: m3. r2 := <-ss.Channel().GetMembers(c1.Id, 0, 1000) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index edb0874c88..578826cd37 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -980,15 +980,15 @@ func (_m *ChannelStore) PermanentDeleteMembersByUser(userId string) store.StoreC } // RemoveAllDeactivatedMembers provides a mock function with given fields: channelId -func (_m *ChannelStore) RemoveAllDeactivatedMembers(channelId string) store.StoreChannel { +func (_m *ChannelStore) RemoveAllDeactivatedMembers(channelId string) *model.AppError { ret := _m.Called(channelId) - 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(channelId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } }