Migrating more GroupStore methods to sync by default (#11447)

* Migrating more GroupStore methods to sync by default

* Fixing tests

* Fixing tests

* Fixing compilation

app/group.go

@@ -34,41 +34,28 @@ func (a *App) DeleteGroup(groupID string) (*model.Group, *model.AppError) {
 }
 
 func (a *App) GetGroupMemberUsers(groupID string) ([]*model.User, *model.AppError) {
-	result := <-a.Srv.Store.Group().GetMemberUsers(groupID)
+	return a.Srv.Store.Group().GetMemberUsers(groupID)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.User), nil
 }
 
 func (a *App) GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, int, *model.AppError) {
-	result := <-a.Srv.Store.Group().GetMemberUsersPage(groupID, page, perPage)
+	members, err := a.Srv.Store.Group().GetMemberUsersPage(groupID, page, perPage)
-	if result.Err != nil {
+	if err != nil {
-		return nil, 0, result.Err
+		return nil, 0, err
 	}
-	members := result.Data.([]*model.User)
+
-	result = <-a.Srv.Store.Group().GetMemberCount(groupID)
+	count, err := a.Srv.Store.Group().GetMemberCount(groupID)
-	if result.Err != nil {
+	if err != nil {
-		return nil, 0, result.Err
+		return nil, 0, err
 	}
-	count := int(result.Data.(int64))
+	return members, int(count), nil
-	return members, count, nil
 }
 
 func (a *App) UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
-	result := <-a.Srv.Store.Group().UpsertMember(groupID, userID)
+	return a.Srv.Store.Group().UpsertMember(groupID, userID)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.GroupMember), nil
 }
 
 func (a *App) DeleteGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
-	result := <-a.Srv.Store.Group().DeleteMember(groupID, userID)
+	return a.Srv.Store.Group().DeleteMember(groupID, userID)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.GroupMember), nil
 }
 
 func (a *App) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) {

store/sqlstore/group_store.go

@@ -207,12 +207,10 @@ func (s *SqlGroupStore) Delete(groupID string) (*model.Group, *model.AppError) {
 	return group, nil
 }
 
-func (s *SqlGroupStore) GetMemberUsers(groupID string) store.StoreChannel {
+func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	var groupMembers []*model.User
 
-		var groupMembers []*model.User
+	query := `
-
-		query := `
 		SELECT
 			Users.*
 		FROM
@@ -223,23 +221,17 @@ func (s *SqlGroupStore) GetMemberUsers(groupID string) store.StoreChannel {
 			AND Users.DeleteAt = 0
 			AND GroupId = :GroupId`
 
-		if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID}); err != nil {
+	if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID}); err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.GroupGetAllBySource", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.GroupGetAllBySource", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		result.Data = groupMembers
+	return groupMembers, nil
-
-		return
-	})
 }
 
-func (s *SqlGroupStore) GetMemberUsersPage(groupID string, offset int, limit int) store.StoreChannel {
+func (s *SqlGroupStore) GetMemberUsersPage(groupID string, offset int, limit int) ([]*model.User, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	var groupMembers []*model.User
 
-		var groupMembers []*model.User
+	query := `
-
-		query := `
 		SELECT
 			Users.*
 		FROM
@@ -256,24 +248,15 @@ func (s *SqlGroupStore) GetMemberUsersPage(groupID string, offset int, limit int
 		OFFSET
 			:Offset`
 
-		if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID, "Limit": limit, "Offset": offset}); err != nil {
+	if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID, "Limit": limit, "Offset": offset}); err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		result.Data = groupMembers
+	return groupMembers, nil
-
-		return
-	})
 }
 
-func (s *SqlGroupStore) GetMemberCount(groupID string) store.StoreChannel {
+func (s *SqlGroupStore) GetMemberCount(groupID string) (int64, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	query := `
-
-		var count int64
-		var err error
-
-		query := `
 		SELECT
 			count(*)
 		FROM
@@ -281,95 +264,75 @@ func (s *SqlGroupStore) GetMemberCount(groupID string) store.StoreChannel {
 		WHERE
 			GroupMembers.GroupId = :GroupId`
 
-		if count, err = s.GetReplica().SelectInt(query, map[string]interface{}{"GroupId": groupID}); err != nil {
+	count, err := s.GetReplica().SelectInt(query, map[string]interface{}{"GroupId": groupID})
-			result.Err = model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+	if err != nil {
-			return
+		return int64(0), model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = count
+	return count, nil
-
-		return
-	})
 }
 
-func (s *SqlGroupStore) UpsertMember(groupID string, userID string) store.StoreChannel {
+func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	member := &model.GroupMember{
+		GroupId:  groupID,
+		UserId:   userID,
+		CreateAt: model.GetMillis(),
+	}
 
-		member := &model.GroupMember{
+	if err := member.IsValid(); err != nil {
-			GroupId:  groupID,
+		return nil, err
-			UserId:   userID,
+	}
-			CreateAt: model.GetMillis(),
+
+	var retrievedGroup *model.Group
+	if err := s.GetMaster().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": groupID}); err != nil {
+		return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
+	}
+
+	var retrievedMember *model.GroupMember
+	if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId", map[string]interface{}{"GroupId": member.GroupId, "UserId": member.UserId}); err != nil {
+		if err != sql.ErrNoRows {
+			return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.select_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
 		}
+	}
 
-		if result.Err = member.IsValid(); result.Err != nil {
+	if retrievedMember == nil {
-			return
+		if err := s.GetMaster().Insert(member); err != nil {
-		}
+			if IsUniqueConstraintError(err, []string{"GroupId", "UserId", "groupmembers_pkey", "PRIMARY"}) {
-
+				return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.uniqueness_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusBadRequest)
-		var retrievedGroup *model.Group
-		if err := s.GetMaster().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": groupID}); err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
-			return
-		}
-
-		var retrievedMember *model.GroupMember
-		if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId", map[string]interface{}{"GroupId": member.GroupId, "UserId": member.UserId}); err != nil {
-			if err != sql.ErrNoRows {
-				result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.select_error", nil, "group_id="+member.GroupId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
-				return
 			}
+			return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError)
 		}
-
+	} else {
-		if retrievedMember == nil {
+		member.DeleteAt = 0
-			if err := s.GetMaster().Insert(member); err != nil {
+		var rowsChanged int64
-				if IsUniqueConstraintError(err, []string{"GroupId", "UserId", "groupmembers_pkey", "PRIMARY"}) {
+		var err error
-					result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.uniqueness_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusBadRequest)
+		if rowsChanged, err = s.GetMaster().Update(member); err != nil {
-					return
+			return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.update_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError)
-				}
-				result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError)
-				return
-			}
-		} else {
-			member.DeleteAt = 0
-			var rowsChanged int64
-			var err error
-			if rowsChanged, err = s.GetMaster().Update(member); err != nil {
-				result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.update_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError)
-				return
-			}
-			if rowsChanged != 1 {
-				result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError)
-				return
-			}
 		}
+		if rowsChanged != 1 {
+			return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError)
+		}
+	}
 
-		result.Data = member
+	return member, nil
-		return
-	})
 }
 
-func (s *SqlGroupStore) DeleteMember(groupID string, userID string) store.StoreChannel {
+func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	var retrievedMember *model.GroupMember
-
+	if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId AND DeleteAt = 0", map[string]interface{}{"GroupId": groupID, "UserId": userID}); err != nil {
-		var retrievedMember *model.GroupMember
+		if err == sql.ErrNoRows {
-		if err := s.GetMaster().SelectOne(&retrievedMember, "SELECT * FROM GroupMembers WHERE GroupId = :GroupId AND UserId = :UserId AND DeleteAt = 0", map[string]interface{}{"GroupId": groupID, "UserId": userID}); err != nil {
+			return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.sql_group.no_rows", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusNotFound)
-			if err == sql.ErrNoRows {
-				result.Err = model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.sql_group.no_rows", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusNotFound)
-				return
-			}
-			result.Err = model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.select_error", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusInternalServerError)
-			return
 		}
+		return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.select_error", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusInternalServerError)
+	}
 
-		retrievedMember.DeleteAt = model.GetMillis()
+	retrievedMember.DeleteAt = model.GetMillis()
 
-		if _, err := s.GetMaster().Update(retrievedMember); err != nil {
+	if _, err := s.GetMaster().Update(retrievedMember); err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.update_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.update_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		result.Data = retrievedMember
+	return retrievedMember, nil
-		return
-	})
 }
 
 func (s *SqlGroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) {

store/store.go

@@ -581,11 +581,11 @@ type GroupStore interface {
 	Update(group *model.Group) (*model.Group, *model.AppError)
 	Delete(groupID string) (*model.Group, *model.AppError)
 
-	GetMemberUsers(groupID string) StoreChannel
+	GetMemberUsers(groupID string) ([]*model.User, *model.AppError)
-	GetMemberUsersPage(groupID string, offset int, limit int) StoreChannel
+	GetMemberUsersPage(groupID string, offset int, limit int) ([]*model.User, *model.AppError)
-	GetMemberCount(groupID string) StoreChannel
+	GetMemberCount(groupID string) (int64, *model.AppError)
-	UpsertMember(groupID string, userID string) StoreChannel
+	UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError)
-	DeleteMember(groupID string, userID string) StoreChannel
+	DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError)
 
 	CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError)
 	GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError)

store/storetest/group_store.go

@@ -26,7 +26,7 @@ func TestGroupStore(t *testing.T, ss store.Store) {
 
 	t.Run("GetMemberUsers", func(t *testing.T) { testGroupGetMemberUsers(t, ss) })
 	t.Run("GetMemberUsersPage", func(t *testing.T) { testGroupGetMemberUsersPage(t, ss) })
-	t.Run("UpsertMember", func(t *testing.T) { testGroupCreateOrRestoreMember(t, ss) })
+	t.Run("UpsertMember", func(t *testing.T) { testUpsertMember(t, ss) })
 	t.Run("DeleteMember", func(t *testing.T) { testGroupDeleteMember(t, ss) })
 
 	t.Run("CreateGroupSyncable", func(t *testing.T) { testCreateGroupSyncable(t, ss) })
@@ -445,8 +445,8 @@ func testGroupGetMemberUsers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
 
-	res = <-ss.Group().UpsertMember(group.Id, user1.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user1.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	u2 := &model.User{
 		Email:    MakeEmail(),
@@ -456,25 +456,26 @@ func testGroupGetMemberUsers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user2 := res.Data.(*model.User)
 
-	res = <-ss.Group().UpsertMember(group.Id, user2.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user2.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// Check returns members
-	res = <-ss.Group().GetMemberUsers(group.Id)
+	groupMembers, err := ss.Group().GetMemberUsers(group.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	groupMembers := res.Data.([]*model.User)
 	require.Equal(t, 2, len(groupMembers))
 
 	// Check madeup id
-	res = <-ss.Group().GetMemberUsers(model.NewId())
+	groupMembers, err = ss.Group().GetMemberUsers(model.NewId())
-	require.Equal(t, 0, len(res.Data.([]*model.User)))
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
 
 	// Delete a member
-	<-ss.Group().DeleteMember(group.Id, user1.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user1.Id)
+	require.Nil(t, err)
 
 	// Should not return deleted members
-	res = <-ss.Group().GetMemberUsers(group.Id)
+	groupMembers, err = ss.Group().GetMemberUsers(group.Id)
-	groupMembers = res.Data.([]*model.User)
+	require.Nil(t, err)
 	require.Equal(t, 1, len(groupMembers))
 }
 
@@ -498,8 +499,8 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
 
-	res = <-ss.Group().UpsertMember(group.Id, user1.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user1.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	u2 := &model.User{
 		Email:    MakeEmail(),
@@ -509,43 +510,42 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 	user2 := res.Data.(*model.User)
 
-	res = <-ss.Group().UpsertMember(group.Id, user2.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user2.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// Check returns members
-	res = <-ss.Group().GetMemberUsersPage(group.Id, 0, 100)
+	groupMembers, err := ss.Group().GetMemberUsersPage(group.Id, 0, 100)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	groupMembers := res.Data.([]*model.User)
 	require.Equal(t, 2, len(groupMembers))
 
 	// Check page 1
-	res = <-ss.Group().GetMemberUsersPage(group.Id, 0, 1)
+	groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	groupMembers = res.Data.([]*model.User)
 	require.Equal(t, 1, len(groupMembers))
 	require.Equal(t, user2.Id, groupMembers[0].Id)
 
 	// Check page 2
-	res = <-ss.Group().GetMemberUsersPage(group.Id, 1, 1)
+	groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 1, 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	groupMembers = res.Data.([]*model.User)
 	require.Equal(t, 1, len(groupMembers))
 	require.Equal(t, user1.Id, groupMembers[0].Id)
 
 	// Check madeup id
-	res = <-ss.Group().GetMemberUsersPage(model.NewId(), 0, 100)
+	groupMembers, err = ss.Group().GetMemberUsersPage(model.NewId(), 0, 100)
-	require.Equal(t, 0, len(res.Data.([]*model.User)))
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
 
 	// Delete a member
-	<-ss.Group().DeleteMember(group.Id, user1.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user1.Id)
+	require.Nil(t, err)
 
 	// Should not return deleted members
-	res = <-ss.Group().GetMemberUsersPage(group.Id, 0, 100)
+	groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 100)
-	groupMembers = res.Data.([]*model.User)
+	require.Nil(t, err)
 	require.Equal(t, 1, len(groupMembers))
 }
 
-func testGroupCreateOrRestoreMember(t *testing.T, ss store.Store) {
+func testUpsertMember(t *testing.T, ss store.Store) {
 	// Create group
 	g1 := &model.Group{
 		Name:        model.NewId(),
@@ -566,37 +566,36 @@ func testGroupCreateOrRestoreMember(t *testing.T, ss store.Store) {
 	user := res2.Data.(*model.User)
 
 	// Happy path
-	res3 := <-ss.Group().UpsertMember(group.Id, user.Id)
+	d2, err := ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res3.Err)
+	require.Nil(t, err)
-	d2 := res3.Data.(*model.GroupMember)
 	require.Equal(t, d2.GroupId, group.Id)
 	require.Equal(t, d2.UserId, user.Id)
 	require.NotZero(t, d2.CreateAt)
 	require.Zero(t, d2.DeleteAt)
 
 	// Duplicate composite key (GroupId, UserId)
-	res4 := <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res4.Err)
+	require.Nil(t, err)
 
 	// Invalid GroupId
-	res6 := <-ss.Group().UpsertMember(model.NewId(), user.Id)
+	_, err = ss.Group().UpsertMember(model.NewId(), user.Id)
-	require.Equal(t, res6.Err.Id, "store.insert_error")
+	require.Equal(t, err.Id, "store.insert_error")
 
 	// Restores a deleted member
-	res := <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
-	res = <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
-	res = <-ss.Group().GetMemberUsers(group.Id)
+	groupMembers, err := ss.Group().GetMemberUsers(group.Id)
-	beforeRestoreCount := len(res.Data.([]*model.User))
+	beforeRestoreCount := len(groupMembers)
 
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
-	res = <-ss.Group().GetMemberUsers(group.Id)
+	groupMembers, err = ss.Group().GetMemberUsers(group.Id)
-	afterRestoreCount := len(res.Data.([]*model.User))
+	afterRestoreCount := len(groupMembers)
 
 	require.Equal(t, beforeRestoreCount+1, afterRestoreCount)
 }
@@ -622,30 +621,28 @@ func testGroupDeleteMember(t *testing.T, ss store.Store) {
 	user := res2.Data.(*model.User)
 
 	// Create member
-	res3 := <-ss.Group().UpsertMember(group.Id, user.Id)
+	d1, err := ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res3.Err)
+	require.Nil(t, err)
-	d1 := res3.Data.(*model.GroupMember)
 
 	// Happy path
-	res4 := <-ss.Group().DeleteMember(group.Id, user.Id)
+	d2, err := ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res4.Err)
+	require.Nil(t, err)
-	d2 := res4.Data.(*model.GroupMember)
 	require.Equal(t, d2.GroupId, group.Id)
 	require.Equal(t, d2.UserId, user.Id)
 	require.Equal(t, d2.CreateAt, d1.CreateAt)
 	require.NotZero(t, d2.DeleteAt)
 
 	// Delete an already deleted member
-	res5 := <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Equal(t, res5.Err.Id, "store.sql_group.no_rows")
+	require.Equal(t, err.Id, "store.sql_group.no_rows")
 
 	// Delete with non-existent User
-	res8 := <-ss.Group().DeleteMember(group.Id, model.NewId())
+	_, err = ss.Group().DeleteMember(group.Id, model.NewId())
-	require.Equal(t, res8.Err.Id, "store.sql_group.no_rows")
+	require.Equal(t, err.Id, "store.sql_group.no_rows")
 
 	// Delete non-existent Group
-	res9 := <-ss.Group().DeleteMember(model.NewId(), group.Id)
+	_, err = ss.Group().DeleteMember(model.NewId(), group.Id)
-	require.Equal(t, res9.Err.Id, "store.sql_group.no_rows")
+	require.Equal(t, err.Id, "store.sql_group.no_rows")
 }
 
 func testCreateGroupSyncable(t *testing.T, ss store.Store) {
@@ -926,8 +923,8 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	user = res.Data.(*model.User)
 
 	// Create GroupMember
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// Create Team
 	team := &model.Team{
@@ -960,10 +957,10 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 0)
 
 	// Delete and restore GroupMember should return result
-	res = <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
 	require.Nil(t, err)
 	require.Len(t, teamMembers, 1)
@@ -1046,14 +1043,15 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 1)
 
 	// No result if GroupMember deleted
-	res = <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 	teamMembers, err = ss.Group().TeamMembersToAdd(0)
 	require.Nil(t, err)
 	require.Len(t, teamMembers, 0)
 
 	// restore group member and verify
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
+	require.Nil(t, err)
 	teamMembers, err = ss.Group().TeamMembersToAdd(0)
 	require.Nil(t, err)
 	require.Len(t, teamMembers, 1)
@@ -1089,7 +1087,7 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	user = res.Data.(*model.User)
 
 	// Create GroupMember
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 
 	// Create Channel
@@ -1119,10 +1117,10 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 0)
 
 	// Delete and restore GroupMember should return result
-	res = <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
 	require.Nil(t, err)
 	require.Len(t, channelMembers, 1)
@@ -1204,15 +1202,15 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 1)
 
 	// No result if GroupMember deleted
-	res = <-ss.Group().DeleteMember(group.Id, user.Id)
+	_, err = ss.Group().DeleteMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
 	require.Nil(t, err)
 	require.Len(t, channelMembers, 0)
 
 	// restore group member and verify
-	res = <-ss.Group().UpsertMember(group.Id, user.Id)
+	_, err = ss.Group().UpsertMember(group.Id, user.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
 	require.Nil(t, err)
 	require.Len(t, channelMembers, 1)
@@ -1248,8 +1246,8 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 1)
 	require.Equal(t, data.UserC.Id, teamMembers[0].UserId)
 
-	res := <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
+	_, err = ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// user b and c should now be returned
 	teamMembers, err = ss.Group().TeamMembersToRemove()
@@ -1265,8 +1263,8 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	require.Equal(t, data.ConstrainedTeam.Id, teamMembers[0].TeamId)
 	require.Equal(t, data.ConstrainedTeam.Id, teamMembers[1].TeamId)
 
-	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
+	_, err = ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	teamMembers, err = ss.Group().TeamMembersToRemove()
 	require.Nil(t, err)
@@ -1300,7 +1298,7 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 3)
 
 	// add users back to groups
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
+	res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
 	require.Nil(t, res.Err)
 	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
 	require.Nil(t, res.Err)
@@ -1323,8 +1321,8 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 1)
 	require.Equal(t, data.UserC.Id, channelMembers[0].UserId)
 
-	res := <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
+	_, err = ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// user b and c should now be returned
 	channelMembers, err = ss.Group().ChannelMembersToRemove()
@@ -1340,8 +1338,8 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	require.Equal(t, data.ConstrainedChannel.Id, channelMembers[0].ChannelId)
 	require.Equal(t, data.ConstrainedChannel.Id, channelMembers[1].ChannelId)
 
-	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
+	_, err = ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	channelMembers, err = ss.Group().ChannelMembersToRemove()
 	require.Nil(t, err)
@@ -1375,7 +1373,7 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 3)
 
 	// add users back to groups
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
+	res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
 	require.Nil(t, res.Err)
 	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
 	require.Nil(t, res.Err)
@@ -1439,11 +1437,11 @@ func pendingMemberRemovalsDataSetup(t *testing.T, ss store.Store) *removalsData
 	userC = res.Data.(*model.User)
 
 	// add users to group (but not userC)
-	res = <-ss.Group().UpsertMember(group.Id, userA.Id)
+	_, err = ss.Group().UpsertMember(group.Id, userA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
-	res = <-ss.Group().UpsertMember(group.Id, userB.Id)
+	_, err = ss.Group().UpsertMember(group.Id, userB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// create channels
 	channelConstrained := &model.Channel{
@@ -1631,7 +1629,9 @@ func testGetGroupsByChannel(t *testing.T, ss store.Store) {
 	res := <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().UpsertMember(group1.Id, user1.Id)
+
+	_, err = ss.Group().UpsertMember(group1.Id, user1.Id)
+	require.Nil(t, err)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -1829,7 +1829,9 @@ func testGetGroupsByTeam(t *testing.T, ss store.Store) {
 	res := <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().UpsertMember(group1.Id, user1.Id)
+
+	_, err = ss.Group().UpsertMember(group1.Id, user1.Id)
+	require.Nil(t, err)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -2068,7 +2070,9 @@ func testGetGroups(t *testing.T, ss store.Store) {
 	res := <-ss.User().Save(u1)
 	require.Nil(t, res.Err)
 	user1 := res.Data.(*model.User)
-	<-ss.Group().UpsertMember(group1.Id, user1.Id)
+
+	_, err = ss.Group().UpsertMember(group1.Id, user1.Id)
+	require.Nil(t, err)
 
 	group1WithMemberCount := model.Group(*group1)
 	group1WithMemberCount.MemberCount = model.NewInt(1)
@@ -2270,12 +2274,12 @@ func testTeamMembersMinusGroupMembers(t *testing.T, ss store.Store) {
 	// Add even users to even group, and the inverse
 	for i := 0; i < numberOfUsers; i++ {
 		groupIndex := int(math.Mod(float64(i), 2))
-		res := <-ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
+		_, err := ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 
 		// Add everyone to group 2
-		res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
+		_, err = ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 	}
 
 	testCases := map[string]struct {
@@ -2423,12 +2427,12 @@ func testChannelMembersMinusGroupMembers(t *testing.T, ss store.Store) {
 	// Add even users to even group, and the inverse
 	for i := 0; i < numberOfUsers; i++ {
 		groupIndex := int(math.Mod(float64(i), 2))
-		res := <-ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
+		_, err := ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 
 		// Add everyone to group 2
-		res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
+		_, err = ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 	}
 
 	testCases := map[string]struct {

store/storetest/mocks/GroupStore.go

@@ -6,7 +6,6 @@ package mocks
 
 import mock "github.com/stretchr/testify/mock"
 import model "github.com/mattermost/mattermost-server/model"
-import store "github.com/mattermost/mattermost-server/store"
 
 // GroupStore is an autogenerated mock type for the GroupStore type
 type GroupStore struct {
@@ -281,19 +280,28 @@ func (_m *GroupStore) DeleteGroupSyncable(groupID string, syncableID string, syn
 }
 
 // DeleteMember provides a mock function with given fields: groupID, userID
-func (_m *GroupStore) DeleteMember(groupID string, userID string) store.StoreChannel {
+func (_m *GroupStore) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
 	ret := _m.Called(groupID, userID)
 
-	var r0 store.StoreChannel
+	var r0 *model.GroupMember
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string) *model.GroupMember); ok {
 		r0 = rf(groupID, userID)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.GroupMember)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+		r1 = rf(groupID, userID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // Get provides a mock function with given fields: groupID
@@ -522,51 +530,76 @@ func (_m *GroupStore) GetGroupsByTeam(teamId string, opts model.GroupSearchOpts)
 }
 
 // GetMemberCount provides a mock function with given fields: groupID
-func (_m *GroupStore) GetMemberCount(groupID string) store.StoreChannel {
+func (_m *GroupStore) GetMemberCount(groupID string) (int64, *model.AppError) {
 	ret := _m.Called(groupID)
 
-	var r0 store.StoreChannel
+	var r0 int64
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string) int64); ok {
 		r0 = rf(groupID)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Get(0).(int64)
-			r0 = ret.Get(0).(store.StoreChannel)
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(groupID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
 		}
 	}
 
-	return r0
+	return r0, r1
 }
 
 // GetMemberUsers provides a mock function with given fields: groupID
-func (_m *GroupStore) GetMemberUsers(groupID string) store.StoreChannel {
+func (_m *GroupStore) GetMemberUsers(groupID string) ([]*model.User, *model.AppError) {
 	ret := _m.Called(groupID)
 
-	var r0 store.StoreChannel
+	var r0 []*model.User
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string) []*model.User); ok {
 		r0 = rf(groupID)
 	} 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) *model.AppError); ok {
+		r1 = rf(groupID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetMemberUsersPage provides a mock function with given fields: groupID, offset, limit
-func (_m *GroupStore) GetMemberUsersPage(groupID string, offset int, limit int) store.StoreChannel {
+func (_m *GroupStore) GetMemberUsersPage(groupID string, offset int, limit int) ([]*model.User, *model.AppError) {
 	ret := _m.Called(groupID, offset, limit)
 
-	var r0 store.StoreChannel
+	var r0 []*model.User
-	if rf, ok := ret.Get(0).(func(string, int, int) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok {
 		r0 = rf(groupID, offset, limit)
 	} 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, int, int) *model.AppError); ok {
+		r1 = rf(groupID, offset, limit)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // TeamMembersMinusGroupMembers provides a mock function with given fields: teamID, groupIDs, page, perPage
@@ -695,17 +728,26 @@ func (_m *GroupStore) UpdateGroupSyncable(groupSyncable *model.GroupSyncable) (*
 }
 
 // UpsertMember provides a mock function with given fields: groupID, userID
-func (_m *GroupStore) UpsertMember(groupID string, userID string) store.StoreChannel {
+func (_m *GroupStore) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
 	ret := _m.Called(groupID, userID)
 
-	var r0 store.StoreChannel
+	var r0 *model.GroupMember
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string) *model.GroupMember); ok {
 		r0 = rf(groupID, userID)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.GroupMember)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+		r1 = rf(groupID, userID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }

store/storetest/user_store.go

@@ -1135,8 +1135,8 @@ func testUserStoreGetProfilesNotInChannel(t *testing.T, ss store.Store) {
 
 	// add two members to the group
 	for _, u := range []*model.User{u1, u2} {
-		res := <-ss.Group().UpsertMember(group.Id, u.Id)
+		_, err = ss.Group().UpsertMember(group.Id, u.Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 	}
 
 	// associate the group with the channel
@@ -3466,8 +3466,8 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) {
 
 	// add two members to the group
 	for _, u := range []*model.User{u1, u2} {
-		res := <-ss.Group().UpsertMember(group.Id, u.Id)
+		_, err = ss.Group().UpsertMember(group.Id, u.Id)
-		require.Nil(t, res.Err)
+		require.Nil(t, err)
 	}
 
 	// associate the group with the team
@@ -3772,10 +3772,10 @@ func testUserStoreGetTeamGroupUsers(t *testing.T, ss store.Store) {
 	groupB := testGroups[1]
 
 	// add members to groups
-	res = <-ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
+	_, err = ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
+	_, err = ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// association one group to team
 	_, err = ss.Group().CreateGroupSyncable(&model.GroupSyncable{
@@ -3894,10 +3894,10 @@ func testUserStoreGetChannelGroupUsers(t *testing.T, ss store.Store) {
 	groupB := testGroups[1]
 
 	// add members to groups
-	res = <-ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
+	_, err = ss.Group().UpsertMember(groupA.Id, userGroupA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
+	_, err = ss.Group().UpsertMember(groupB.Id, userGroupB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// association one group to channel
 	_, err = ss.Group().CreateGroupSyncable(&model.GroupSyncable{