diff --git a/app/channel.go b/app/channel.go index fbc72d22fe..9ea20504f3 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1017,11 +1017,10 @@ func (a *App) AddChannelMember(userId string, channel *model.Channel, userReques func (a *App) AddDirectChannels(teamId string, user *model.User) *model.AppError { var profiles []*model.User options := &model.UserGetOptions{InTeamId: teamId, Page: 0, PerPage: 100} - result := <-a.Srv.Store.User().GetProfiles(options) - if result.Err != nil { - return model.NewAppError("AddDirectChannels", "api.user.add_direct_channels_and_forget.failed.error", map[string]interface{}{"UserId": user.Id, "TeamId": teamId, "Error": result.Err.Error()}, "", http.StatusInternalServerError) + profiles, err := a.Srv.Store.User().GetProfiles(options) + if err != nil { + return model.NewAppError("AddDirectChannels", "api.user.add_direct_channels_and_forget.failed.error", map[string]interface{}{"UserId": user.Id, "TeamId": teamId, "Error": err.Error()}, "", http.StatusInternalServerError) } - profiles = result.Data.([]*model.User) var preferences model.Preferences diff --git a/app/command_loadtest.go b/app/command_loadtest.go index 0ce89f1f6d..567b0abefc 100644 --- a/app/command_loadtest.go +++ b/app/command_loadtest.go @@ -301,8 +301,7 @@ func (me *LoadTestProvider) PostsCommand(a *App, args *model.CommandArgs, messag var usernames []string options := &model.UserGetOptions{InTeamId: args.TeamId, Page: 0, PerPage: 1000} - if result := <-a.Srv.Store.User().GetProfiles(options); result.Err == nil { - profileUsers := result.Data.([]*model.User) + if profileUsers, err := a.Srv.Store.User().GetProfiles(options); err == nil { usernames = make([]string, len(profileUsers)) i := 0 for _, userprof := range profileUsers { diff --git a/app/user.go b/app/user.go index c80504e65b..f43d49574f 100644 --- a/app/user.go +++ b/app/user.go @@ -464,11 +464,7 @@ func (a *App) GetUsersEtag(restrictionsHash string) string { } func (a *App) GetUsersInTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError) { - result := <-a.Srv.Store.User().GetProfiles(options) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.User), nil + return a.Srv.Store.User().GetProfiles(options) } func (a *App) GetUsersNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 2b36d133a2..eb71f1b652 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -482,40 +482,37 @@ func (s SqlUserStore) GetEtagForProfiles(teamId string) string { return fmt.Sprintf("%v.%v", model.CurrentVersion, updateAt) } -func (us SqlUserStore) GetProfiles(options *model.UserGetOptions) store.StoreChannel { +func (us SqlUserStore) GetProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) { isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES - return store.Do(func(result *store.StoreResult) { - query := us.usersQuery. - Join("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 )"). - Where("tm.TeamId = ?", options.InTeamId). - OrderBy("u.Username ASC"). - Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) + query := us.usersQuery. + Join("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 )"). + Where("tm.TeamId = ?", options.InTeamId). + OrderBy("u.Username ASC"). + Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) - query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) + query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) - query = applyRoleFilter(query, options.Role, isPostgreSQL) + query = applyRoleFilter(query, options.Role, isPostgreSQL) - if options.Inactive { - query = query.Where("u.DeleteAt != 0") - } + if options.Inactive { + query = query.Where("u.DeleteAt != 0") + } - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlUserStore.GetProfiles", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetProfiles", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var users []*model.User - if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlUserStore.GetProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + var users []*model.User + if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { + return nil, model.NewAppError("SqlUserStore.GetProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) + } - for _, u := range users { - u.Sanitize(map[string]bool{}) - } - result.Data = users - }) + for _, u := range users { + u.Sanitize(map[string]bool{}) + } + + return users, nil } func (us SqlUserStore) InvalidateProfilesInChannelCacheByUser(userId string) { diff --git a/store/store.go b/store/store.go index b6b8c1ac69..df7e018465 100644 --- a/store/store.go +++ b/store/store.go @@ -270,7 +270,7 @@ type UserStore interface { GetProfilesWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) StoreChannel GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) - GetProfiles(options *model.UserGetOptions) StoreChannel + GetProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) GetProfileByIds(userIds []string, options *UserGetByIdsOpts, allowFromCache bool) ([]*model.User, *model.AppError) GetProfileByGroupChannelIdsForUser(userId string, channelIds []string) (map[string][]*model.User, *model.AppError) InvalidatProfileCacheForUser(userId string) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 14e50acd00..051eb25cca 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -535,19 +535,28 @@ func (_m *UserStore) GetProfileByIds(userIds []string, options *store.UserGetByI } // GetProfiles provides a mock function with given fields: options -func (_m *UserStore) GetProfiles(options *model.UserGetOptions) store.StoreChannel { +func (_m *UserStore) GetProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) { ret := _m.Called(options) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.UserGetOptions) store.StoreChannel); ok { + var r0 []*model.User + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok { r0 = rf(options) } 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(*model.UserGetOptions) *model.AppError); ok { + r1 = rf(options) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetProfilesByUsernames provides a mock function with given fields: usernames, viewRestrictions diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 5eebf7b06b..f4e4b7e1dc 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -555,14 +555,13 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u5.Id}, -1)) t.Run("get page 0, perPage 100", func(t *testing.T) { - result := <-ss.User().GetProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetProfiles(&model.UserGetOptions{ InTeamId: teamId, Page: 0, PerPage: 100, }) - require.Nil(t, result.Err) + require.Nil(t, err) - actual := result.Data.([]*model.User) require.Equal(t, []*model.User{ sanitized(u1), sanitized(u2), @@ -573,26 +572,24 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { }) t.Run("get page 0, perPage 1", func(t *testing.T) { - result := <-ss.User().GetProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetProfiles(&model.UserGetOptions{ InTeamId: teamId, Page: 0, PerPage: 1, }) - require.Nil(t, result.Err) + require.Nil(t, err) - actual := result.Data.([]*model.User) require.Equal(t, []*model.User{sanitized(u1)}, actual) }) t.Run("get unknown team id", func(t *testing.T) { - result := <-ss.User().GetProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetProfiles(&model.UserGetOptions{ InTeamId: "123", Page: 0, PerPage: 100, }) - require.Nil(t, result.Err) + require.Nil(t, err) - actual := result.Data.([]*model.User) require.Equal(t, []*model.User{}, actual) }) @@ -610,28 +607,26 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to system_admin role", func(t *testing.T) { - result := <-ss.User().GetProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetProfiles(&model.UserGetOptions{ InTeamId: teamId, Page: 0, PerPage: 10, Role: "system_admin", }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u4), }, actual) }) t.Run("filter to inactive", func(t *testing.T) { - result := <-ss.User().GetProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetProfiles(&model.UserGetOptions{ InTeamId: teamId, Page: 0, PerPage: 10, Inactive: true, }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u5), }, actual)