From 4380c0b7a8950f6b0106f56025a89b014a18c6ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Sun, 7 Jul 2019 13:50:18 -0400 Subject: [PATCH] Migrate User.ClearAllCustomRoleAssignments to sync by default (#11506) --- app/permissions.go | 4 +- store/sqlstore/user_store.go | 78 ++++++++++++++---------------- store/store.go | 2 +- store/storetest/mocks/UserStore.go | 8 +-- store/storetest/user_store.go | 2 +- 5 files changed, 45 insertions(+), 49 deletions(-) diff --git a/app/permissions.go b/app/permissions.go index eef7c39a9c..287e863001 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -28,8 +28,8 @@ func (a *App) ResetPermissionsSystem() *model.AppError { } // Reset all Custom Role assignments to Users. - if result := <-a.Srv.Store.User().ClearAllCustomRoleAssignments(); result.Err != nil { - return result.Err + if err := a.Srv.Store.User().ClearAllCustomRoleAssignments(); err != nil { + return err } // Reset all Custom Role assignments to TeamMembers. diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index abd5ec62b0..9d1593bc8b 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1436,60 +1436,56 @@ func (us SqlUserStore) GetEtagForProfilesNotInTeam(teamId string) store.StoreCha }) } -func (us SqlUserStore) ClearAllCustomRoleAssignments() store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - builtInRoles := model.MakeDefaultRoles() - lastUserId := strings.Repeat("0", 26) +func (us SqlUserStore) ClearAllCustomRoleAssignments() *model.AppError { + builtInRoles := model.MakeDefaultRoles() + lastUserId := strings.Repeat("0", 26) - for { - var transaction *gorp.Transaction - var err error + for { + var transaction *gorp.Transaction + var err error - if transaction, err = us.GetMaster().Begin(); err != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } - defer finalizeTransaction(transaction) + if transaction, err = us.GetMaster().Begin(); err != nil { + return model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer finalizeTransaction(transaction) - var users []*model.User - if _, err := transaction.Select(&users, "SELECT * from Users WHERE Id > :Id ORDER BY Id LIMIT 1000", map[string]interface{}{"Id": lastUserId}); err != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + var users []*model.User + if _, err := transaction.Select(&users, "SELECT * from Users WHERE Id > :Id ORDER BY Id LIMIT 1000", map[string]interface{}{"Id": lastUserId}); err != nil { + return model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) + } - if len(users) == 0 { - break - } + if len(users) == 0 { + break + } - for _, user := range users { - lastUserId = user.Id + for _, user := range users { + lastUserId = user.Id - var newRoles []string + var newRoles []string - for _, role := range strings.Fields(user.Roles) { - for name := range builtInRoles { - if name == role { - newRoles = append(newRoles, role) - break - } - } - } - - newRolesString := strings.Join(newRoles, " ") - if newRolesString != user.Roles { - if _, err := transaction.Exec("UPDATE Users SET Roles = :Roles WHERE Id = :Id", map[string]interface{}{"Roles": newRolesString, "Id": user.Id}); err != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) - return + for _, role := range strings.Fields(user.Roles) { + for name := range builtInRoles { + if name == role { + newRoles = append(newRoles, role) + break } } } - if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return + newRolesString := strings.Join(newRoles, " ") + if newRolesString != user.Roles { + if _, err := transaction.Exec("UPDATE Users SET Roles = :Roles WHERE Id = :Id", map[string]interface{}{"Roles": newRolesString, "Id": user.Id}); err != nil { + return model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) + } } } - }) + + if err := transaction.Commit(); err != nil { + return model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return nil } func (us SqlUserStore) InferSystemInstallDate() store.StoreChannel { diff --git a/store/store.go b/store/store.go index ea07a9b33e..1f2f781bf2 100644 --- a/store/store.go +++ b/store/store.go @@ -300,7 +300,7 @@ type UserStore interface { AnalyticsGetSystemAdminCount() (int64, *model.AppError) GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetEtagForProfilesNotInTeam(teamId string) StoreChannel - ClearAllCustomRoleAssignments() StoreChannel + ClearAllCustomRoleAssignments() *model.AppError InferSystemInstallDate() StoreChannel GetAllAfter(limit int, afterId string) ([]*model.User, *model.AppError) GetUsersBatchForIndexing(startTime, endTime int64, limit int) ([]*model.UserForIndexing, *model.AppError) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 10dd6e41a7..4e71b52f68 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -83,15 +83,15 @@ func (_m *UserStore) AnalyticsGetSystemAdminCount() (int64, *model.AppError) { } // ClearAllCustomRoleAssignments provides a mock function with given fields: -func (_m *UserStore) ClearAllCustomRoleAssignments() store.StoreChannel { +func (_m *UserStore) 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) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index cf1a0a6f43..a58b3179b3 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3506,7 +3506,7 @@ func testUserStoreClearAllCustomRoleAssignments(t *testing.T, ss store.Store) { store.Must(ss.User().Save(&u4)) defer func() { require.Nil(t, ss.User().PermanentDelete(u4.Id)) }() - require.Nil(t, (<-ss.User().ClearAllCustomRoleAssignments()).Err) + require.Nil(t, ss.User().ClearAllCustomRoleAssignments()) r1 := <-ss.User().GetByUsername(u1.Username) require.Nil(t, r1.Err)