From bcaab359a4f5001f37d2020db9f8d161a71d7998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 2 Jul 2019 17:18:04 +0200 Subject: [PATCH] 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 | 35 ++--- store/sqlstore/group_store.go | 175 +++++++++-------------- store/store.go | 10 +- store/storetest/group_store.go | 212 ++++++++++++++-------------- store/storetest/mocks/GroupStore.go | 96 +++++++++---- store/storetest/user_store.go | 24 ++-- 6 files changed, 274 insertions(+), 278 deletions(-) diff --git a/app/group.go b/app/group.go index d87b861b40..b1b841dcb6 100644 --- a/app/group.go +++ b/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) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.User), nil + return a.Srv.Store.Group().GetMemberUsers(groupID) } func (a *App) GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, int, *model.AppError) { - result := <-a.Srv.Store.Group().GetMemberUsersPage(groupID, page, perPage) - if result.Err != nil { - return nil, 0, result.Err + members, err := a.Srv.Store.Group().GetMemberUsersPage(groupID, page, perPage) + if err != nil { + return nil, 0, err } - members := result.Data.([]*model.User) - result = <-a.Srv.Store.Group().GetMemberCount(groupID) - if result.Err != nil { - return nil, 0, result.Err + + count, err := a.Srv.Store.Group().GetMemberCount(groupID) + if err != nil { + return nil, 0, err } - count := int(result.Data.(int64)) - return members, count, nil + return members, int(count), nil } func (a *App) UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) { - result := <-a.Srv.Store.Group().UpsertMember(groupID, userID) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.GroupMember), nil + return a.Srv.Store.Group().UpsertMember(groupID, userID) } func (a *App) DeleteGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) { - result := <-a.Srv.Store.Group().DeleteMember(groupID, userID) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.GroupMember), nil + return a.Srv.Store.Group().DeleteMember(groupID, userID) } func (a *App) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index d4c3606960..dffa34d2a4 100644 --- a/store/sqlstore/group_store.go +++ b/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 { - return store.Do(func(result *store.StoreResult) { +func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, *model.AppError) { + 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 { - result.Err = model.NewAppError("SqlGroupStore.GroupGetAllBySource", "store.select_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID}); err != nil { + return nil, model.NewAppError("SqlGroupStore.GroupGetAllBySource", "store.select_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = groupMembers - - return - }) + return groupMembers, nil } -func (s *SqlGroupStore) GetMemberUsersPage(groupID string, offset int, limit int) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { +func (s *SqlGroupStore) GetMemberUsersPage(groupID string, offset int, limit int) ([]*model.User, *model.AppError) { + 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 { - result.Err = model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID, "Limit": limit, "Offset": offset}); err != nil { + return nil, model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = groupMembers - - return - }) + return groupMembers, nil } -func (s *SqlGroupStore) GetMemberCount(groupID string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - - var count int64 - var err error - - query := ` +func (s *SqlGroupStore) GetMemberCount(groupID string) (int64, *model.AppError) { + 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 { - result.Err = model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError) - return - } + count, err := s.GetReplica().SelectInt(query, map[string]interface{}{"GroupId": groupID}) + if err != nil { + return int64(0), model.NewAppError("SqlGroupStore.GroupGetMemberUsersPage", "store.select_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = count - - return - }) + return count, nil } -func (s *SqlGroupStore) UpsertMember(groupID string, userID string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { +func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) { + member := &model.GroupMember{ + GroupId: groupID, + UserId: userID, + CreateAt: model.GetMillis(), + } - member := &model.GroupMember{ - GroupId: groupID, - UserId: userID, - CreateAt: model.GetMillis(), + if err := member.IsValid(); err != nil { + return nil, err + } + + 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 { - return - } - - 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 + if retrievedMember == nil { + 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) } + return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.insert_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError) } - - if retrievedMember == nil { - if err := s.GetMaster().Insert(member); err != nil { - if IsUniqueConstraintError(err, []string{"GroupId", "UserId", "groupmembers_pkey", "PRIMARY"}) { - result.Err = model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.uniqueness_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusBadRequest) - return - } - 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 - } + } else { + member.DeleteAt = 0 + var rowsChanged int64 + var err error + if rowsChanged, err = s.GetMaster().Update(member); err != nil { + return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.update_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError) } + if rowsChanged != 1 { + return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError) + } + } - result.Data = member - return - }) + return member, nil } -func (s *SqlGroupStore) DeleteMember(groupID string, userID string) store.StoreChannel { - 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 { - 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 +func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) { + 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 { + if err == sql.ErrNoRows { + return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.sql_group.no_rows", nil, "group_id="+groupID+"user_id="+userID+","+err.Error(), http.StatusNotFound) } + 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 { - result.Err = model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.update_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if _, err := s.GetMaster().Update(retrievedMember); err != nil { + return nil, model.NewAppError("SqlGroupStore.GroupDeleteMember", "store.update_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = retrievedMember - return - }) + return retrievedMember, nil } func (s *SqlGroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { diff --git a/store/store.go b/store/store.go index c77521c78d..94afe17ab3 100644 --- a/store/store.go +++ b/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 - GetMemberUsersPage(groupID string, offset int, limit int) StoreChannel - GetMemberCount(groupID string) StoreChannel - UpsertMember(groupID string, userID string) StoreChannel - DeleteMember(groupID string, userID string) StoreChannel + GetMemberUsers(groupID string) ([]*model.User, *model.AppError) + GetMemberUsersPage(groupID string, offset int, limit int) ([]*model.User, *model.AppError) + GetMemberCount(groupID string) (int64, *model.AppError) + UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) + 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) diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index cea3910aea..54247fd7c1 100644 --- a/store/storetest/group_store.go +++ b/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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user1.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user2.Id) + require.Nil(t, err) // Check returns members - res = <-ss.Group().GetMemberUsers(group.Id) - require.Nil(t, res.Err) - groupMembers := res.Data.([]*model.User) + groupMembers, err := ss.Group().GetMemberUsers(group.Id) + require.Nil(t, err) require.Equal(t, 2, len(groupMembers)) // Check madeup id - res = <-ss.Group().GetMemberUsers(model.NewId()) - require.Equal(t, 0, len(res.Data.([]*model.User))) + groupMembers, err = ss.Group().GetMemberUsers(model.NewId()) + 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 = res.Data.([]*model.User) + groupMembers, err = ss.Group().GetMemberUsers(group.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user1.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user2.Id) + require.Nil(t, err) // Check returns members - res = <-ss.Group().GetMemberUsersPage(group.Id, 0, 100) - require.Nil(t, res.Err) - groupMembers := res.Data.([]*model.User) + groupMembers, err := ss.Group().GetMemberUsersPage(group.Id, 0, 100) + require.Nil(t, err) require.Equal(t, 2, len(groupMembers)) // Check page 1 - res = <-ss.Group().GetMemberUsersPage(group.Id, 0, 1) - require.Nil(t, res.Err) - groupMembers = res.Data.([]*model.User) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 1) + require.Nil(t, err) 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) - require.Nil(t, res.Err) - groupMembers = res.Data.([]*model.User) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 1, 1) + require.Nil(t, err) 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) - require.Equal(t, 0, len(res.Data.([]*model.User))) + groupMembers, err = ss.Group().GetMemberUsersPage(model.NewId(), 0, 100) + 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 = res.Data.([]*model.User) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 100) + 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) - require.Nil(t, res3.Err) - d2 := res3.Data.(*model.GroupMember) + d2, err := ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) 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) - require.Nil(t, res4.Err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) // Invalid GroupId - res6 := <-ss.Group().UpsertMember(model.NewId(), user.Id) - require.Equal(t, res6.Err.Id, "store.insert_error") + _, err = ss.Group().UpsertMember(model.NewId(), user.Id) + require.Equal(t, err.Id, "store.insert_error") // Restores a deleted member - res := <-ss.Group().UpsertMember(group.Id, user.Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) - res = <-ss.Group().DeleteMember(group.Id, user.Id) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(group.Id, user.Id) + require.Nil(t, err) - res = <-ss.Group().GetMemberUsers(group.Id) - beforeRestoreCount := len(res.Data.([]*model.User)) + groupMembers, err := ss.Group().GetMemberUsers(group.Id) + beforeRestoreCount := len(groupMembers) - res = <-ss.Group().UpsertMember(group.Id, user.Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) - res = <-ss.Group().GetMemberUsers(group.Id) - afterRestoreCount := len(res.Data.([]*model.User)) + groupMembers, err = ss.Group().GetMemberUsers(group.Id) + 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) - require.Nil(t, res3.Err) - d1 := res3.Data.(*model.GroupMember) + d1, err := ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) // Happy path - res4 := <-ss.Group().DeleteMember(group.Id, user.Id) - require.Nil(t, res4.Err) - d2 := res4.Data.(*model.GroupMember) + d2, err := ss.Group().DeleteMember(group.Id, user.Id) + require.Nil(t, err) 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) - require.Equal(t, res5.Err.Id, "store.sql_group.no_rows") + _, err = ss.Group().DeleteMember(group.Id, user.Id) + require.Equal(t, err.Id, "store.sql_group.no_rows") // Delete with non-existent User - res8 := <-ss.Group().DeleteMember(group.Id, model.NewId()) - require.Equal(t, res8.Err.Id, "store.sql_group.no_rows") + _, err = ss.Group().DeleteMember(group.Id, model.NewId()) + require.Equal(t, err.Id, "store.sql_group.no_rows") // Delete non-existent Group - res9 := <-ss.Group().DeleteMember(model.NewId(), group.Id) - require.Equal(t, res9.Err.Id, "store.sql_group.no_rows") + _, err = ss.Group().DeleteMember(model.NewId(), group.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) - res = <-ss.Group().UpsertMember(group.Id, user.Id) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(group.Id, user.Id) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) - res = <-ss.Group().UpsertMember(group.Id, user.Id) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(group.Id, user.Id) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, user.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(data.Group.Id, data.UserB.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(data.Group.Id, data.UserA.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(data.Group.Id, data.UserB.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().DeleteMember(data.Group.Id, data.UserA.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, userA.Id) + require.Nil(t, err) - res = <-ss.Group().UpsertMember(group.Id, userB.Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, userB.Id) + 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) - require.Nil(t, res.Err) + _, err := ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id) + require.Nil(t, err) // Add everyone to group 2 - res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id) + 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) - require.Nil(t, res.Err) + _, err := ss.Group().UpsertMember(groups[groupIndex].Id, users[i].Id) + require.Nil(t, err) // Add everyone to group 2 - res = <-ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(groups[numberOfGroups-1].Id, users[i].Id) + require.Nil(t, err) } testCases := map[string]struct { diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index 7589a18c11..3a370fd158 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/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 - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 *model.GroupMember + 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 - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(string) int64); ok { r0 = rf(groupID) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(int64) + } + + 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 - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 []*model.User + 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 - if rf, ok := ret.Get(0).(func(string, int, int) store.StoreChannel); ok { + var r0 []*model.User + 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 - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 *model.GroupMember + 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 } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 8468ea8686..449be169ad 100644 --- a/store/storetest/user_store.go +++ b/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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, u.Id) + 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) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(group.Id, u.Id) + 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) - require.Nil(t, res.Err) - res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(groupA.Id, userGroupA.Id) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(groupB.Id, userGroupB.Id) + 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) - require.Nil(t, res.Err) - res = <-ss.Group().UpsertMember(groupB.Id, userGroupB.Id) - require.Nil(t, res.Err) + _, err = ss.Group().UpsertMember(groupA.Id, userGroupA.Id) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(groupB.Id, userGroupB.Id) + require.Nil(t, err) // association one group to channel _, err = ss.Group().CreateGroupSyncable(&model.GroupSyncable{