diff --git a/migrations/advanced_permissions_phase_2.go b/migrations/advanced_permissions_phase_2.go index e4a72b0bc0..6cd32015b2 100644 --- a/migrations/advanced_permissions_phase_2.go +++ b/migrations/advanced_permissions_phase_2.go @@ -71,19 +71,18 @@ func (worker *Worker) runAdvancedPermissionsPhase2Migration(lastDone string) (bo if progress.CurrentTable == "TeamMembers" { // Run a TeamMembers migration batch. - if result := <-worker.app.Srv.Store.Team().MigrateTeamMembers(progress.LastTeamId, progress.LastUserId); result.Err != nil { - return false, progress.ToJson(), result.Err + if result, err := worker.app.Srv.Store.Team().MigrateTeamMembers(progress.LastTeamId, progress.LastUserId); err != nil { + return false, progress.ToJson(), err } else { - if result.Data == nil { + if result == nil { // We haven't progressed. That means that we've reached the end of this stage of the migration, and should now advance to the next stage. progress.LastUserId = strings.Repeat("0", 26) progress.CurrentTable = "ChannelMembers" return false, progress.ToJson(), nil } - data := result.Data.(map[string]string) - progress.LastTeamId = data["TeamId"] - progress.LastUserId = data["UserId"] + progress.LastTeamId = result["TeamId"] + progress.LastUserId = result["UserId"] } } else if progress.CurrentTable == "ChannelMembers" { // Run a ChannelMembers migration batch. diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index f569c6ea17..9dee1b5ed9 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -802,70 +802,65 @@ func (s SqlTeamStore) GetTeamsByScheme(schemeId string, offset int, limit int) ( // in batches as a single transaction per batch to ensure consistency but to also minimise execution time to avoid // causing unnecessary table locks. **THIS FUNCTION SHOULD NOT BE USED FOR ANY OTHER PURPOSE.** Executing this function // *after* the new Schemes functionality has been used on an installation will have unintended consequences. -func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var transaction *gorp.Transaction - var err error +func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) (map[string]string, *model.AppError) { + var transaction *gorp.Transaction + var err error - if transaction, err = s.GetMaster().Begin(); err != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return + if transaction, err = s.GetMaster().Begin(); err != nil { + return nil, model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer finalizeTransaction(transaction) + + var teamMembers []teamMember + if _, err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (:FromTeamId, :FromUserId) ORDER BY TeamId, UserId LIMIT 100", map[string]interface{}{"FromTeamId": fromTeamId, "FromUserId": fromUserId}); err != nil { + return nil, model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.select.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + if len(teamMembers) == 0 { + // No more team members in query result means that the migration has finished. + return nil, nil + } + + for _, member := range teamMembers { + roles := strings.Fields(member.Roles) + var newRoles []string + if !member.SchemeAdmin.Valid { + member.SchemeAdmin = sql.NullBool{Bool: false, Valid: true} } - defer finalizeTransaction(transaction) + if !member.SchemeUser.Valid { + member.SchemeUser = sql.NullBool{Bool: false, Valid: true} + } + if !member.SchemeGuest.Valid { + member.SchemeGuest = sql.NullBool{Bool: false, Valid: true} + } + for _, role := range roles { + if role == model.TEAM_ADMIN_ROLE_ID { + member.SchemeAdmin = sql.NullBool{Bool: true, Valid: true} + } else if role == model.TEAM_USER_ROLE_ID { + member.SchemeUser = sql.NullBool{Bool: true, Valid: true} + } else if role == model.TEAM_GUEST_ROLE_ID { + member.SchemeGuest = sql.NullBool{Bool: true, Valid: true} + } else { + newRoles = append(newRoles, role) + } + } + member.Roles = strings.Join(newRoles, " ") - var teamMembers []teamMember - if _, err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (:FromTeamId, :FromUserId) ORDER BY TeamId, UserId LIMIT 100", map[string]interface{}{"FromTeamId": fromTeamId, "FromUserId": fromUserId}); err != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.select.app_error", nil, err.Error(), http.StatusInternalServerError) - return + if _, err := transaction.Update(&member); err != nil { + return nil, model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.update.app_error", nil, err.Error(), http.StatusInternalServerError) } - if len(teamMembers) == 0 { - // No more team members in query result means that the migration has finished. - return - } + } - for _, member := range teamMembers { - roles := strings.Fields(member.Roles) - var newRoles []string - if !member.SchemeAdmin.Valid { - member.SchemeAdmin = sql.NullBool{Bool: false, Valid: true} - } - if !member.SchemeUser.Valid { - member.SchemeUser = sql.NullBool{Bool: false, Valid: true} - } - if !member.SchemeGuest.Valid { - member.SchemeGuest = sql.NullBool{Bool: false, Valid: true} - } - for _, role := range roles { - if role == model.TEAM_ADMIN_ROLE_ID { - member.SchemeAdmin = sql.NullBool{Bool: true, Valid: true} - } else if role == model.TEAM_USER_ROLE_ID { - member.SchemeUser = sql.NullBool{Bool: true, Valid: true} - } else if role == model.TEAM_GUEST_ROLE_ID { - member.SchemeGuest = sql.NullBool{Bool: true, Valid: true} - } else { - newRoles = append(newRoles, role) - } - } - member.Roles = strings.Join(newRoles, " ") + if err := transaction.Commit(); err != nil { + return nil, model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } - if _, err := transaction.Update(&member); err != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.update.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + data := make(map[string]string) + data["TeamId"] = teamMembers[len(teamMembers)-1].TeamId + data["UserId"] = teamMembers[len(teamMembers)-1].UserId - } - - if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } - - data := make(map[string]string) - data["TeamId"] = teamMembers[len(teamMembers)-1].TeamId - data["UserId"] = teamMembers[len(teamMembers)-1].UserId - result.Data = data - }) + return data, nil } func (s SqlTeamStore) ResetAllTeamSchemes() *model.AppError { diff --git a/store/store.go b/store/store.go index b3bf46f291..deb7394a5b 100644 --- a/store/store.go +++ b/store/store.go @@ -116,7 +116,7 @@ type TeamStore interface { RemoveAllMembersByUser(userId string) *model.AppError UpdateLastTeamIconUpdate(teamId string, curTime int64) *model.AppError GetTeamsByScheme(schemeId string, offset int, limit int) ([]*model.Team, *model.AppError) - MigrateTeamMembers(fromTeamId string, fromUserId string) StoreChannel + MigrateTeamMembers(fromTeamId string, fromUserId string) (map[string]string, *model.AppError) ResetAllTeamSchemes() *model.AppError ClearAllCustomRoleAssignments() StoreChannel AnalyticsGetTeamCountForScheme(schemeId string) StoreChannel diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index e957eb473d..e8002bb7f0 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -650,19 +650,28 @@ func (_m *TeamStore) InvalidateAllTeamIdsForUser(userId string) { } // MigrateTeamMembers provides a mock function with given fields: fromTeamId, fromUserId -func (_m *TeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) store.StoreChannel { +func (_m *TeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) (map[string]string, *model.AppError) { ret := _m.Called(fromTeamId, fromUserId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 map[string]string + if rf, ok := ret.Get(0).(func(string, string) map[string]string); ok { r0 = rf(fromTeamId, fromUserId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(map[string]string) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + r1 = rf(fromTeamId, fromUserId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // PermanentDelete provides a mock function with given fields: teamId diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 113be6cae8..e5b66c8bb8 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -1413,14 +1413,13 @@ func testTeamStoreMigrateTeamMembers(t *testing.T, ss store.Store) { lastDoneUserId := strings.Repeat("0", 26) for { - res := <-ss.Team().MigrateTeamMembers(lastDoneTeamId, lastDoneUserId) - if assert.Nil(t, res.Err) { - if res.Data == nil { + res, e := ss.Team().MigrateTeamMembers(lastDoneTeamId, lastDoneUserId) + if assert.Nil(t, e) { + if res == nil { break } - data := res.Data.(map[string]string) - lastDoneTeamId = data["TeamId"] - lastDoneUserId = data["UserId"] + lastDoneTeamId = res["TeamId"] + lastDoneUserId = res["UserId"] } }