11460 user.get all profiles in channel (#11515)

app/channel.go

@@ -1893,9 +1893,9 @@ func (a *App) ViewChannel(view *model.ChannelView, userId string, currentSession
 }
 
 func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError {
-	channelUsers := <-a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, false)
+	profiles, err := a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, false)
-	if channelUsers.Err != nil {
+	if err != nil {
-		return channelUsers.Err
+		return err
 	}
 
 	if err := a.Srv.Store.Post().PermanentDeleteByChannel(channel.Id); err != nil {
@@ -1920,7 +1920,7 @@ func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError {
 
 	if a.IsESIndexingEnabled() {
 		a.Srv.Go(func() {
-			for _, user := range channelUsers.Data.(map[string]*model.User) {
+			for _, user := range profiles {
 				if err := a.indexUser(user); err != nil {
 					mlog.Error("Encountered error indexing user", mlog.String("user_id", user.Id), mlog.Err(err))
 				}

app/notification.go

@@ -28,7 +28,12 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 		return []string{}, nil
 	}
 
-	pchan := a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, true)
+	pchan := make(chan store.StoreResult, 1)
+	go func() {
+		props, err := a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, true)
+		pchan <- store.StoreResult{Data: props, Err: err}
+		close(pchan)
+	}()
 
 	cmnchan := make(chan store.StoreResult, 1)
 	go func() {

app/user.go

@@ -550,11 +550,7 @@ func (a *App) GetUsersInChannelPageByStatus(channelId string, page int, perPage
 }
 
 func (a *App) GetUsersNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
-	result := <-a.Srv.Store.User().GetProfilesNotInChannel(teamId, channelId, groupConstrained, offset, limit, viewRestrictions)
+	return a.Srv.Store.User().GetProfilesNotInChannel(teamId, channelId, groupConstrained, offset, limit, viewRestrictions)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.User), nil
 }
 
 func (a *App) GetUsersNotInChannelMap(teamId string, channelId string, groupConstrained bool, offset int, limit int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) (map[string]*model.User, *model.AppError) {

store/sqlstore/user_store.go

@@ -606,92 +606,83 @@ func (us SqlUserStore) GetProfilesInChannelByStatus(channelId string, offset int
 	})
 }
 
-func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) store.StoreChannel {
+func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	if allowFromCache {
-		if allowFromCache {
+		if cacheItem, ok := profilesInChannelCache.Get(channelId); ok {
-			if cacheItem, ok := profilesInChannelCache.Get(channelId); ok {
+			if us.metrics != nil {
-				if us.metrics != nil {
+				us.metrics.IncrementMemCacheHitCounter("Profiles in Channel")
-					us.metrics.IncrementMemCacheHitCounter("Profiles in Channel")
-				}
-				result.Data = cacheItem.(map[string]*model.User)
-				return
-			} else {
-				if us.metrics != nil {
-					us.metrics.IncrementMemCacheMissCounter("Profiles in Channel")
-				}
 			}
+			return cacheItem.(map[string]*model.User), nil
 		} else {
 			if us.metrics != nil {
 				us.metrics.IncrementMemCacheMissCounter("Profiles in Channel")
 			}
 		}
-
+	} else {
-		query := us.usersQuery.
+		if us.metrics != nil {
-			Join("ChannelMembers cm ON ( cm.UserId = u.Id )").
+			us.metrics.IncrementMemCacheMissCounter("Profiles in Channel")
-			Where("cm.ChannelId = ?", channelId).
-			Where("u.DeleteAt = 0").
-			OrderBy("u.Username ASC")
-
-		queryString, args, err := query.ToSql()
-		if err != nil {
-			result.Err = model.NewAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
 		}
+	}
 
-		var users []*model.User
+	query := us.usersQuery.
-		if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
+		Join("ChannelMembers cm ON ( cm.UserId = u.Id )").
-			result.Err = model.NewAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
+		Where("cm.ChannelId = ?", channelId).
-			return
+		Where("u.DeleteAt = 0").
-		}
+		OrderBy("u.Username ASC")
 
-		userMap := make(map[string]*model.User)
+	queryString, args, err := query.ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
 
-		for _, u := range users {
+	var users []*model.User
-			u.Sanitize(map[string]bool{})
+	if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
-			userMap[u.Id] = u
+		return nil, model.NewAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = userMap
+	userMap := make(map[string]*model.User)
 
-		if allowFromCache {
+	for _, u := range users {
-			profilesInChannelCache.AddWithExpiresInSecs(channelId, userMap, PROFILES_IN_CHANNEL_CACHE_SEC)
+		u.Sanitize(map[string]bool{})
-		}
+		userMap[u.Id] = u
-	})
+	}
+
+	if allowFromCache {
+		profilesInChannelCache.AddWithExpiresInSecs(channelId, userMap, PROFILES_IN_CHANNEL_CACHE_SEC)
+	}
+
+	return userMap, nil
 }
 
-func (us SqlUserStore) GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel {
+func (us SqlUserStore) GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	query := us.usersQuery.
-		query := us.usersQuery.
+		Join("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 AND tm.TeamId = ? )", teamId).
-			Join("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 AND tm.TeamId = ? )", teamId).
+		LeftJoin("ChannelMembers cm ON ( cm.UserId = u.Id AND cm.ChannelId = ? )", channelId).
-			LeftJoin("ChannelMembers cm ON ( cm.UserId = u.Id AND cm.ChannelId = ? )", channelId).
+		Where("cm.UserId IS NULL").
-			Where("cm.UserId IS NULL").
+		OrderBy("u.Username ASC").
-			OrderBy("u.Username ASC").
+		Offset(uint64(offset)).Limit(uint64(limit))
-			Offset(uint64(offset)).Limit(uint64(limit))
 
-		query = applyViewRestrictionsFilter(query, viewRestrictions, true)
+	query = applyViewRestrictionsFilter(query, viewRestrictions, true)
 
-		if groupConstrained {
+	if groupConstrained {
-			query = applyChannelGroupConstrainedFilter(query, channelId)
+		query = applyChannelGroupConstrainedFilter(query, channelId)
-		}
+	}
 
-		queryString, args, err := query.ToSql()
+	queryString, args, err := query.ToSql()
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		var users []*model.User
+	var users []*model.User
-		if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
+	if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
-			result.Err = model.NewAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		for _, u := range users {
+	for _, u := range users {
-			u.Sanitize(map[string]bool{})
+		u.Sanitize(map[string]bool{})
-		}
+	}
 
-		result.Data = users
+	return users, nil
-	})
 }
 
 func (us SqlUserStore) GetProfilesWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel {

store/store.go

@@ -265,8 +265,8 @@ type UserStore interface {
 	InvalidateProfilesInChannelCache(channelId string)
 	GetProfilesInChannel(channelId string, offset int, limit int) StoreChannel
 	GetProfilesInChannelByStatus(channelId string, offset int, limit int) StoreChannel
-	GetAllProfilesInChannel(channelId string, allowFromCache bool) StoreChannel
+	GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError)
-	GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel
+	GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)
 	GetProfilesWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel
 	GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) StoreChannel
 	GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError)

store/storetest/mocks/UserStore.go

@@ -227,19 +227,28 @@ func (_m *UserStore) GetAllProfiles(options *model.UserGetOptions) ([]*model.Use
 }
 
 // GetAllProfilesInChannel provides a mock function with given fields: channelId, allowFromCache
-func (_m *UserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) store.StoreChannel {
+func (_m *UserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError) {
 	ret := _m.Called(channelId, allowFromCache)
 
-	var r0 store.StoreChannel
+	var r0 map[string]*model.User
-	if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, bool) map[string]*model.User); ok {
 		r0 = rf(channelId, allowFromCache)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(map[string]*model.User)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok {
+		r1 = rf(channelId, allowFromCache)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetAllUsingAuthService provides a mock function with given fields: authService
@@ -590,19 +599,28 @@ func (_m *UserStore) GetProfilesInChannelByStatus(channelId string, offset int,
 }
 
 // GetProfilesNotInChannel provides a mock function with given fields: teamId, channelId, groupConstrained, offset, limit, viewRestrictions
-func (_m *UserStore) GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel {
+func (_m *UserStore) GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
 	ret := _m.Called(teamId, channelId, groupConstrained, offset, limit, viewRestrictions)
 
-	var r0 store.StoreChannel
+	var r0 []*model.User
-	if rf, ok := ret.Get(0).(func(string, string, bool, int, int, *model.ViewUsersRestrictions) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string, bool, int, int, *model.ViewUsersRestrictions) []*model.User); ok {
 		r0 = rf(teamId, channelId, groupConstrained, offset, limit, viewRestrictions)
 	} 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(string, string, bool, int, int, *model.ViewUsersRestrictions) *model.AppError); ok {
+		r1 = rf(teamId, channelId, groupConstrained, offset, limit, viewRestrictions)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetProfilesNotInTeam provides a mock function with given fields: teamId, groupConstrained, offset, limit, viewRestrictions

store/storetest/user_store.go

@@ -955,37 +955,41 @@ func testUserStoreGetAllProfilesInChannel(t *testing.T, ss store.Store) {
 	}))
 
 	t.Run("all profiles in channel 1, no caching", func(t *testing.T) {
-		result := <-ss.User().GetAllProfilesInChannel(c1.Id, false)
+		var profiles map[string]*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetAllProfilesInChannel(c1.Id, false)
+		require.Nil(t, err)
 		assert.Equal(t, map[string]*model.User{
 			u1.Id: sanitized(u1),
 			u2.Id: sanitized(u2),
 			u3.Id: sanitized(u3),
-		}, result.Data.(map[string]*model.User))
+		}, profiles)
 	})
 
 	t.Run("all profiles in channel 2, no caching", func(t *testing.T) {
-		result := <-ss.User().GetAllProfilesInChannel(c2.Id, false)
+		var profiles map[string]*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetAllProfilesInChannel(c2.Id, false)
+		require.Nil(t, err)
 		assert.Equal(t, map[string]*model.User{
 			u1.Id: sanitized(u1),
-		}, result.Data.(map[string]*model.User))
+		}, profiles)
 	})
 
 	t.Run("all profiles in channel 2, caching", func(t *testing.T) {
-		result := <-ss.User().GetAllProfilesInChannel(c2.Id, true)
+		var profiles map[string]*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetAllProfilesInChannel(c2.Id, true)
+		require.Nil(t, err)
 		assert.Equal(t, map[string]*model.User{
 			u1.Id: sanitized(u1),
-		}, result.Data.(map[string]*model.User))
+		}, profiles)
 	})
 
 	t.Run("all profiles in channel 2, caching [repeated]", func(t *testing.T) {
-		result := <-ss.User().GetAllProfilesInChannel(c2.Id, true)
+		var profiles map[string]*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetAllProfilesInChannel(c2.Id, true)
+		require.Nil(t, err)
 		assert.Equal(t, map[string]*model.User{
 			u1.Id: sanitized(u1),
-		}, result.Data.(map[string]*model.User))
+		}, profiles)
 	})
 
 	ss.User().InvalidateProfilesInChannelCacheByUser(u1.Id)
@@ -1043,23 +1047,25 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) {
 	require.Nil(t, err)
 
 	t.Run("get team 1, channel 1, offset 0, limit 100", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c1.Id, false, 0, 100, nil)
+		var profiles []*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetProfilesNotInChannel(teamId, c1.Id, false, 0, 100, nil)
+		require.Nil(t, err)
 		assert.Equal(t, []*model.User{
 			sanitized(u1),
 			sanitized(u2),
 			sanitized(u3),
-		}, result.Data.([]*model.User))
+		}, profiles)
 	})
 
 	t.Run("get team 1, channel 2, offset 0, limit 100", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c2.Id, false, 0, 100, nil)
+		var profiles []*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetProfilesNotInChannel(teamId, c2.Id, false, 0, 100, nil)
+		require.Nil(t, err)
 		assert.Equal(t, []*model.User{
 			sanitized(u1),
 			sanitized(u2),
 			sanitized(u3),
-		}, result.Data.([]*model.User))
+		}, profiles)
 	})
 
 	store.Must(ss.Channel().SaveMember(&model.ChannelMember{
@@ -1087,24 +1093,27 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) {
 	}))
 
 	t.Run("get team 1, channel 1, offset 0, limit 100, after update", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c1.Id, false, 0, 100, nil)
+		var profiles []*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetProfilesNotInChannel(teamId, c1.Id, false, 0, 100, nil)
-		assert.Equal(t, []*model.User{}, result.Data.([]*model.User))
+		require.Nil(t, err)
+		assert.Equal(t, []*model.User{}, profiles)
 	})
 
 	t.Run("get team 1, channel 2, offset 0, limit 100, after update", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c2.Id, false, 0, 100, nil)
+		var profiles []*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetProfilesNotInChannel(teamId, c2.Id, false, 0, 100, nil)
+		require.Nil(t, err)
 		assert.Equal(t, []*model.User{
 			sanitized(u2),
 			sanitized(u3),
-		}, result.Data.([]*model.User))
+		}, profiles)
 	})
 
 	t.Run("get team 1, channel 2, offset 0, limit 0, setting group constrained when it's not", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c2.Id, true, 0, 100, nil)
+		var profiles []*model.User
-		require.Nil(t, result.Err)
+		profiles, err = ss.User().GetProfilesNotInChannel(teamId, c2.Id, true, 0, 100, nil)
-		assert.Empty(t, result.Data.([]*model.User))
+		require.Nil(t, err)
+		assert.Empty(t, profiles)
 	})
 
 	// create a group
@@ -1131,11 +1140,11 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) {
 	require.Nil(t, err)
 
 	t.Run("get team 1, channel 2, offset 0, limit 0, setting group constrained", func(t *testing.T) {
-		result := <-ss.User().GetProfilesNotInChannel(teamId, c2.Id, true, 0, 100, nil)
+		profiles, err := ss.User().GetProfilesNotInChannel(teamId, c2.Id, true, 0, 100, nil)
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
 		assert.Equal(t, []*model.User{
 			sanitized(u2),
-		}, result.Data.([]*model.User))
+		}, profiles)
 	})
 }