From eb4c97c3e8040c9ba4bcc8df8977264480bac952 Mon Sep 17 00:00:00 2001 From: Taufiq Rahman Date: Mon, 8 Jul 2019 22:46:30 +0600 Subject: [PATCH] [MM-16315] Migrate Channel.PermanentDelete to Sync by default (#11485) * [MM-16315] Migrate Channel.PermanentDelete to Sync by default * fix migration at app/channel.go --- app/channel.go | 4 +- store/sqlstore/channel_store.go | 61 ++++++++++++--------------- store/store.go | 2 +- store/storetest/channel_store.go | 12 +++--- store/storetest/mocks/ChannelStore.go | 8 ++-- 5 files changed, 40 insertions(+), 47 deletions(-) diff --git a/app/channel.go b/app/channel.go index 9ea20504f3..afa3ecc35d 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1913,8 +1913,8 @@ func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError { return err } - if result := <-a.Srv.Store.Channel().PermanentDelete(channel.Id); result.Err != nil { - return result.Err + if err := a.Srv.Store.Channel().PermanentDelete(channel.Id); err != nil { + return err } if a.IsESIndexingEnabled() { diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 02fb641053..9d81ba6dcd 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -864,49 +864,42 @@ func (s SqlChannelStore) permanentDeleteByTeamtT(transaction *gorp.Transaction, } // PermanentDelete removes the given channel from the database. -func (s SqlChannelStore) PermanentDelete(channelId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - transaction, err := s.GetMaster().Begin() - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } - defer finalizeTransaction(transaction) +func (s SqlChannelStore) PermanentDelete(channelId string) *model.AppError { + transaction, err := s.GetMaster().Begin() + if err != nil { + return model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer finalizeTransaction(transaction) - *result = s.permanentDeleteT(transaction, channelId) - if result.Err != nil { - return - } + if err := s.permanentDeleteT(transaction, channelId); err != nil { + return err + } - // Additionally propagate the deletion to the PublicChannels table. - if _, err := transaction.Exec(` + // Additionally propagate the deletion to the PublicChannels table. + if _, err := transaction.Exec(` DELETE FROM PublicChannels WHERE Id = :ChannelId `, map[string]interface{}{ - "ChannelId": channelId, - }); err != nil { - result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.delete_public_channel.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) - return - } - - if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } - }) -} - -func (s SqlChannelStore) permanentDeleteT(transaction *gorp.Transaction, channelId string) store.StoreResult { - result := store.StoreResult{} - - if _, err := transaction.Exec("DELETE FROM Channels WHERE Id = :ChannelId", map[string]interface{}{"ChannelId": channelId}); err != nil { - result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) - return result + "ChannelId": channelId, + }); err != nil { + return model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.delete_public_channel.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) } - return result + if err := transaction.Commit(); err != nil { + return model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return nil +} + +func (s SqlChannelStore) permanentDeleteT(transaction *gorp.Transaction, channelId string) *model.AppError { + if _, err := transaction.Exec("DELETE FROM Channels WHERE Id = :ChannelId", map[string]interface{}{"ChannelId": channelId}); err != nil { + return model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) + } + + return nil } func (s SqlChannelStore) PermanentDeleteMembersByChannel(channelId string) *model.AppError { diff --git a/store/store.go b/store/store.go index 412bb67b90..6ddcc8d65e 100644 --- a/store/store.go +++ b/store/store.go @@ -140,8 +140,8 @@ type ChannelStore interface { Delete(channelId string, time int64) *model.AppError Restore(channelId string, time int64) *model.AppError SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError + PermanentDelete(channelId string) *model.AppError PermanentDeleteByTeam(teamId string) *model.AppError - PermanentDelete(channelId string) StoreChannel GetByName(team_id string, name string, allowFromCache bool) (*model.Channel, *model.AppError) GetByNames(team_id string, names []string, allowFromCache bool) ([]*model.Channel, *model.AppError) GetByNameIncludeDeleted(team_id string, name string, allowFromCache bool) (*model.Channel, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index d06b456d0b..75be005e14 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -226,7 +226,7 @@ func testChannelStoreCreateDirectChannel(t *testing.T, ss store.Store) { } defer func() { ss.Channel().PermanentDeleteMembersByChannel(c1.Id) - <-ss.Channel().PermanentDelete(c1.Id) + ss.Channel().PermanentDelete(c1.Id) }() members, err := ss.Channel().GetMembers(c1.Id, 0, 100) @@ -635,8 +635,8 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { t.Fatal("invalid number of channels") } - cresult := <-ss.Channel().PermanentDelete(o2.Id) - require.Nil(t, cresult.Err) + cresult := ss.Channel().PermanentDelete(o2.Id) + require.Nil(t, cresult) list, err = ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) if assert.NotNil(t, err) { @@ -2804,7 +2804,7 @@ func testChannelStoreSearchGroupChannels(t *testing.T, ss store.Store) { defer func() { for _, gc := range []model.Channel{gc1, gc2, gc3} { ss.Channel().PermanentDeleteMembersByChannel(gc3.Id) - <-ss.Channel().PermanentDelete(gc.Id) + ss.Channel().PermanentDelete(gc.Id) } }() @@ -2914,7 +2914,7 @@ func testChannelStoreAnalyticsDeletedTypeCount(t *testing.T, ss store.Store) { } defer func() { ss.Channel().PermanentDeleteMembersByChannel(d4.Id) - <-ss.Channel().PermanentDelete(d4.Id) + ss.Channel().PermanentDelete(d4.Id) }() var openStartCount int64 @@ -3306,7 +3306,7 @@ func testMaterializedPublicChannels(t *testing.T, ss store.Store, s SqlSupplier) require.Equal(t, &model.ChannelList{&o1, &o2}, channels) }) - <-ss.Channel().PermanentDelete(o1.Id) + ss.Channel().PermanentDelete(o1.Id) t.Run("o1 no longer listed in public channels when permanently deleted", func(t *testing.T) { channels, channelErr := ss.Channel().SearchInTeam(teamId, "", true) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index e35132263f..81d1746252 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1151,15 +1151,15 @@ func (_m *ChannelStore) MigratePublicChannels() error { } // PermanentDelete provides a mock function with given fields: channelId -func (_m *ChannelStore) PermanentDelete(channelId string) store.StoreChannel { +func (_m *ChannelStore) PermanentDelete(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) } }