From a02467698c143f43c27aa32081d8a3153d80440d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Wed, 19 Jun 2019 05:10:58 -0400 Subject: [PATCH] Migrates Channel.ClearAllCustomRoleAssignments to sync by default (#11274) --- app/permissions.go | 4 +- store/sqlstore/channel_store.go | 88 +++++++++++++-------------- store/store.go | 2 +- store/storetest/channel_store.go | 2 +- store/storetest/mocks/ChannelStore.go | 8 +-- 5 files changed, 50 insertions(+), 54 deletions(-) diff --git a/app/permissions.go b/app/permissions.go index 61724d0371..5fdd0a77b3 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -38,8 +38,8 @@ func (a *App) ResetPermissionsSystem() *model.AppError { } // Reset all Custom Role assignments to ChannelMembers. - if result := <-a.Srv.Store.Channel().ClearAllCustomRoleAssignments(); result.Err != nil { - return result.Err + if err := a.Srv.Store.Channel().ClearAllCustomRoleAssignments(); err != nil { + return err } // Purge all schemes from the database. diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 4a29dd945d..cf11e5f604 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -2411,65 +2411,61 @@ func (s SqlChannelStore) resetAllChannelSchemesT(transaction *gorp.Transaction) return result } -func (s SqlChannelStore) ClearAllCustomRoleAssignments() store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - builtInRoles := model.MakeDefaultRoles() - lastUserId := strings.Repeat("0", 26) - lastChannelId := strings.Repeat("0", 26) +func (s SqlChannelStore) ClearAllCustomRoleAssignments() *model.AppError { + builtInRoles := model.MakeDefaultRoles() + lastUserId := strings.Repeat("0", 26) + lastChannelId := strings.Repeat("0", 26) - for { - var transaction *gorp.Transaction - var err error + for { + var transaction *gorp.Transaction + var err error - if transaction, err = s.GetMaster().Begin(); err != nil { - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if transaction, err = s.GetMaster().Begin(); err != nil { + return model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var channelMembers []*channelMember - if _, err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (:ChannelId, :UserId) ORDER BY ChannelId, UserId LIMIT 1000", map[string]interface{}{"ChannelId": lastChannelId, "UserId": lastUserId}); err != nil { - finalizeTransaction(transaction) - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + var channelMembers []*channelMember + if _, err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (:ChannelId, :UserId) ORDER BY ChannelId, UserId LIMIT 1000", map[string]interface{}{"ChannelId": lastChannelId, "UserId": lastUserId}); err != nil { + finalizeTransaction(transaction) + return model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) + } - if len(channelMembers) == 0 { - finalizeTransaction(transaction) - break - } + if len(channelMembers) == 0 { + finalizeTransaction(transaction) + break + } - for _, member := range channelMembers { - lastUserId = member.UserId - lastChannelId = member.ChannelId + for _, member := range channelMembers { + lastUserId = member.UserId + lastChannelId = member.ChannelId - var newRoles []string + var newRoles []string - for _, role := range strings.Fields(member.Roles) { - for name := range builtInRoles { - if name == role { - newRoles = append(newRoles, role) - break - } - } - } - - newRolesString := strings.Join(newRoles, " ") - if newRolesString != member.Roles { - if _, err := transaction.Exec("UPDATE ChannelMembers SET Roles = :Roles WHERE UserId = :UserId AND ChannelId = :ChannelId", map[string]interface{}{"Roles": newRolesString, "ChannelId": member.ChannelId, "UserId": member.UserId}); err != nil { - finalizeTransaction(transaction) - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) - return + for _, role := range strings.Fields(member.Roles) { + for name := range builtInRoles { + if name == role { + newRoles = append(newRoles, role) + break } } } - if err := transaction.Commit(); err != nil { - finalizeTransaction(transaction) - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return + newRolesString := strings.Join(newRoles, " ") + if newRolesString != member.Roles { + if _, err := transaction.Exec("UPDATE ChannelMembers SET Roles = :Roles WHERE UserId = :UserId AND ChannelId = :ChannelId", map[string]interface{}{"Roles": newRolesString, "ChannelId": member.ChannelId, "UserId": member.UserId}); err != nil { + finalizeTransaction(transaction) + return model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) + } } } - }) + + if err := transaction.Commit(); err != nil { + finalizeTransaction(transaction) + return model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return nil } func (s SqlChannelStore) GetAllChannelsForExportAfter(limit int, afterId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index a03bc6128d..551e5184a4 100644 --- a/store/store.go +++ b/store/store.go @@ -192,7 +192,7 @@ type ChannelStore interface { GetChannelsByScheme(schemeId string, offset int, limit int) StoreChannel MigrateChannelMembers(fromChannelId string, fromUserId string) StoreChannel ResetAllChannelSchemes() StoreChannel - ClearAllCustomRoleAssignments() StoreChannel + ClearAllCustomRoleAssignments() *model.AppError MigratePublicChannels() error GetAllChannelsForExportAfter(limit int, afterId string) StoreChannel GetAllDirectChannelsForExportAfter(limit int, afterId string) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index fe8b77b7d3..a924b57396 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -3096,7 +3096,7 @@ func testChannelStoreClearAllCustomRoleAssignments(t *testing.T, ss store.Store) store.Must(ss.Channel().SaveMember(m3)) store.Must(ss.Channel().SaveMember(m4)) - require.Nil(t, (<-ss.Channel().ClearAllCustomRoleAssignments()).Err) + require.Nil(t, ss.Channel().ClearAllCustomRoleAssignments()) member, err := ss.Channel().GetMember(m1.ChannelId, m1.UserId) require.Nil(t, err) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 8fd18117ab..e2e99c1230 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -94,15 +94,15 @@ func (_m *ChannelStore) AutocompleteInTeamForSearch(teamId string, userId string } // ClearAllCustomRoleAssignments provides a mock function with given fields: -func (_m *ChannelStore) ClearAllCustomRoleAssignments() store.StoreChannel { +func (_m *ChannelStore) ClearAllCustomRoleAssignments() *model.AppError { ret := _m.Called() - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func() store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func() *model.AppError); ok { r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } }