From 22fb7d9116b097a14347cc3453e0b263e2b1db48 Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Fri, 28 Jun 2019 12:40:09 +0200 Subject: [PATCH] Migrate User.GetAllAfter to Sync by default (#11446) --- app/export.go | 8 +++----- store/sqlstore/user_store.go | 31 ++++++++++++++---------------- store/store.go | 2 +- store/storetest/mocks/UserStore.go | 19 +++++++++++++----- store/storetest/user_store.go | 15 ++++++--------- 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/app/export.go b/app/export.go index a12250a883..f432842466 100644 --- a/app/export.go +++ b/app/export.go @@ -177,14 +177,12 @@ func (a *App) ExportAllChannels(writer io.Writer) *model.AppError { func (a *App) ExportAllUsers(writer io.Writer) *model.AppError { afterId := strings.Repeat("0", 26) for { - result := <-a.Srv.Store.User().GetAllAfter(1000, afterId) + users, err := a.Srv.Store.User().GetAllAfter(1000, afterId) - if result.Err != nil { - return result.Err + if err != nil { + return err } - users := result.Data.([]*model.User) - if len(users) == 0 { break } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 1c2701d34a..c8c9159f21 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -366,26 +366,23 @@ func (us SqlUserStore) GetAll() store.StoreChannel { }) } -func (us SqlUserStore) GetAllAfter(limit int, afterId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := us.usersQuery. - Where("Id > ?", afterId). - OrderBy("Id ASC"). - Limit(uint64(limit)) +func (us SqlUserStore) GetAllAfter(limit int, afterId string) ([]*model.User, *model.AppError) { + query := us.usersQuery. + Where("Id > ?", afterId). + OrderBy("Id ASC"). + Limit(uint64(limit)) - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlUserStore.GetAllAfter", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetAllAfter", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var data []*model.User - if _, err := us.GetReplica().Select(&data, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlUserStore.GetAllAfter", "store.sql_user.get.app_error", nil, err.Error(), http.StatusInternalServerError) - } + var users []*model.User + if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { + return nil, model.NewAppError("SqlUserStore.GetAllAfter", "store.sql_user.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = data - }) + return users, nil } func (s SqlUserStore) GetEtagForAllProfiles() store.StoreChannel { diff --git a/store/store.go b/store/store.go index c2229712f8..ea2ab4f969 100644 --- a/store/store.go +++ b/store/store.go @@ -299,7 +299,7 @@ type UserStore interface { GetEtagForProfilesNotInTeam(teamId string) StoreChannel ClearAllCustomRoleAssignments() StoreChannel InferSystemInstallDate() StoreChannel - GetAllAfter(limit int, afterId string) StoreChannel + GetAllAfter(limit int, afterId string) ([]*model.User, *model.AppError) GetUsersBatchForIndexing(startTime, endTime int64, limit int) ([]*model.UserForIndexing, *model.AppError) Count(options model.UserCountOptions) (int64, *model.AppError) GetTeamGroupUsers(teamID string) ([]*model.User, *model.AppError) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index cb9320d06b..8bac6ed63c 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -147,19 +147,28 @@ func (_m *UserStore) GetAll() store.StoreChannel { } // GetAllAfter provides a mock function with given fields: limit, afterId -func (_m *UserStore) GetAllAfter(limit int, afterId string) store.StoreChannel { +func (_m *UserStore) GetAllAfter(limit int, afterId string) ([]*model.User, *model.AppError) { ret := _m.Called(limit, afterId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(int, string) store.StoreChannel); ok { + var r0 []*model.User + if rf, ok := ret.Get(0).(func(int, string) []*model.User); ok { r0 = rf(limit, afterId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.User) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(int, string) *model.AppError); ok { + r1 = rf(limit, afterId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetAllProfiles provides a mock function with given fields: options diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index d494c1e8f5..21c5d8a4ab 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3567,26 +3567,23 @@ func testUserStoreGetAllAfter(t *testing.T, ss store.Store) { } t.Run("get after lowest possible id", func(t *testing.T) { - result := <-ss.User().GetAllAfter(10000, strings.Repeat("0", 26)) - require.Nil(t, result.Err) + actual, err := ss.User().GetAllAfter(10000, strings.Repeat("0", 26)) + require.Nil(t, err) - actual := result.Data.([]*model.User) assert.Equal(t, expected, actual) }) t.Run("get after first user", func(t *testing.T) { - result := <-ss.User().GetAllAfter(10000, expected[0].Id) - require.Nil(t, result.Err) + actual, err := ss.User().GetAllAfter(10000, expected[0].Id) + require.Nil(t, err) - actual := result.Data.([]*model.User) assert.Equal(t, []*model.User{expected[1]}, actual) }) t.Run("get after second user", func(t *testing.T) { - result := <-ss.User().GetAllAfter(10000, expected[1].Id) - require.Nil(t, result.Err) + actual, err := ss.User().GetAllAfter(10000, expected[1].Id) + require.Nil(t, err) - actual := result.Data.([]*model.User) assert.Equal(t, []*model.User{}, actual) }) }