From cbfd68ee1fb38183ad5fa6b60616f4ba800d4c60 Mon Sep 17 00:00:00 2001 From: Kyle Reczek Date: Wed, 12 Jun 2019 23:56:22 -0600 Subject: [PATCH] [MM-16179] Migrate Team.GetTotalMemberCount to Sync by default #11120 (#11149) + GetTotalMemberCount returns (int64, *model.AppError) now instead of StoreChannel. + Updated store mock. + Updated code that referenced GetTotalMemberCount to handle the sync result. --- app/team.go | 7 ++++- store/sqlstore/team_store.go | 16 ++++------ store/store.go | 2 +- store/storetest/mocks/TeamStore.go | 19 ++++++++---- store/storetest/team_store.go | 48 +++++++++++++++--------------- 5 files changed, 50 insertions(+), 42 deletions(-) diff --git a/app/team.go b/app/team.go index 1f93c57ab5..dd0d722c5e 100644 --- a/app/team.go +++ b/app/team.go @@ -1156,7 +1156,12 @@ func (a *App) RestoreTeam(teamId string) *model.AppError { } func (a *App) GetTeamStats(teamId string) (*model.TeamStats, *model.AppError) { - tchan := a.Srv.Store.Team().GetTotalMemberCount(teamId) + tchan := make(chan store.StoreResult, 1) + go func() { + totalMemberCount, err := a.Srv.Store.Team().GetTotalMemberCount(teamId) + tchan <- store.StoreResult{Data: totalMemberCount, Err: err} + close(tchan) + }() achan := a.Srv.Store.Team().GetActiveMemberCount(teamId) stats := &model.TeamStats{} diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 0527c87c98..2be596969c 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -644,9 +644,8 @@ func (s SqlTeamStore) GetMembers(teamId string, offset int, limit int, restricti return dbMembers.ToModel(), nil } -func (s SqlTeamStore) GetTotalMemberCount(teamId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - count, err := s.GetReplica().SelectInt(` +func (s SqlTeamStore) GetTotalMemberCount(teamId string) (int64, *model.AppError) { + count, err := s.GetReplica().SelectInt(` SELECT count(*) FROM @@ -656,13 +655,10 @@ func (s SqlTeamStore) GetTotalMemberCount(teamId string) store.StoreChannel { TeamMembers.UserId = Users.Id AND TeamMembers.TeamId = :TeamId AND TeamMembers.DeleteAt = 0`, map[string]interface{}{"TeamId": teamId}) - if err != nil { - result.Err = model.NewAppError("SqlTeamStore.GetTotalMemberCount", "store.sql_team.get_member_count.app_error", nil, "teamId="+teamId+" "+err.Error(), http.StatusInternalServerError) - return - } - - result.Data = count - }) + if err != nil { + return int64(0), model.NewAppError("SqlTeamStore.GetTotalMemberCount", "store.sql_team.get_member_count.app_error", nil, "teamId="+teamId+" "+err.Error(), http.StatusInternalServerError) + } + return count, nil } func (s SqlTeamStore) GetActiveMemberCount(teamId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 26d3ac8528..f91666ffaf 100644 --- a/store/store.go +++ b/store/store.go @@ -105,7 +105,7 @@ type TeamStore interface { GetMember(teamId string, userId string) StoreChannel GetMembers(teamId string, offset int, limit int, restrictions *model.ViewUsersRestrictions) ([]*model.TeamMember, *model.AppError) GetMembersByIds(teamId string, userIds []string, restrictions *model.ViewUsersRestrictions) ([]*model.TeamMember, *model.AppError) - GetTotalMemberCount(teamId string) StoreChannel + GetTotalMemberCount(teamId string) (int64, *model.AppError) GetActiveMemberCount(teamId string) StoreChannel GetTeamsForUser(userId string) StoreChannel GetTeamsForUserWithPagination(userId string, page, perPage int) StoreChannel diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index d85f4eaa1c..374a3fbeae 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -457,19 +457,26 @@ func (_m *TeamStore) GetTeamsForUserWithPagination(userId string, page int, perP } // GetTotalMemberCount provides a mock function with given fields: teamId -func (_m *TeamStore) GetTotalMemberCount(teamId string) store.StoreChannel { +func (_m *TeamStore) GetTotalMemberCount(teamId string) (int64, *model.AppError) { ret := _m.Called(teamId) - 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(teamId) } 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(teamId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // GetUserTeamIds provides a mock function with given fields: userId, allowFromCache diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 22c237dccc..f84658dcb3 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -931,12 +931,12 @@ func testTeamMembersWithPagination(t *testing.T, ss store.Store) { func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { maxUsersPerTeam := 5 - team, err := ss.Team().Save(&model.Team{ + team, errSave := ss.Team().Save(&model.Team{ DisplayName: "DisplayName", Name: "z-z-z" + model.NewId() + "b", Type: model.TEAM_OPEN, }) - require.Nil(t, err) + require.Nil(t, errSave) defer func() { <-ss.Team().PermanentDelete(team.Id) }() @@ -963,10 +963,10 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { }(userIds[i]) } - if result := <-ss.Team().GetTotalMemberCount(team.Id); result.Err != nil { - t.Fatal(result.Err) - } else if count := result.Data.(int64); int(count) != maxUsersPerTeam { - t.Fatalf("should start with 5 team members, had %v instead", count) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(team.Id); err != nil { + t.Fatal(err) + } else if int(totalMemberCount) != maxUsersPerTeam { + t.Fatalf("should start with 5 team members, had %v instead", totalMemberCount) } newUserId := store.Must(ss.User().Save(&model.User{ @@ -984,10 +984,10 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { t.Fatal("shouldn't be able to save member when at maximum members per team") } - if result := <-ss.Team().GetTotalMemberCount(team.Id); result.Err != nil { - t.Fatal(result.Err) - } else if count := result.Data.(int64); int(count) != maxUsersPerTeam { - t.Fatalf("should still have 5 team members, had %v instead", count) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(team.Id); err != nil { + t.Fatal(err) + } else if int(totalMemberCount) != maxUsersPerTeam { + t.Fatalf("should still have 5 team members, had %v instead", totalMemberCount) } // Leaving the team from the UI sets DeleteAt instead of using TeamStore.RemoveMember @@ -997,10 +997,10 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { DeleteAt: 1234, })) - if result := <-ss.Team().GetTotalMemberCount(team.Id); result.Err != nil { - t.Fatal(result.Err) - } else if count := result.Data.(int64); int(count) != maxUsersPerTeam-1 { - t.Fatalf("should now only have 4 team members, had %v instead", count) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(team.Id); err != nil { + t.Fatal(err) + } else if int(totalMemberCount) != maxUsersPerTeam-1 { + t.Fatalf("should now only have 4 team members, had %v instead", totalMemberCount) } if result := <-ss.Team().SaveMember(&model.TeamMember{TeamId: team.Id, UserId: newUserId}, maxUsersPerTeam); result.Err != nil { @@ -1011,10 +1011,10 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { }(newUserId) } - if result := <-ss.Team().GetTotalMemberCount(team.Id); result.Err != nil { - t.Fatal(result.Err) - } else if count := result.Data.(int64); int(count) != maxUsersPerTeam { - t.Fatalf("should have 5 team members again, had %v instead", count) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(team.Id); err != nil { + t.Fatal(err) + } else if int(totalMemberCount) != maxUsersPerTeam { + t.Fatalf("should have 5 team members again, had %v instead", totalMemberCount) } // Deactivating a user should make them stop counting against max members @@ -1161,10 +1161,10 @@ func testTeamStoreMemberCount(t *testing.T, ss store.Store) { m2 := &model.TeamMember{TeamId: teamId1, UserId: u2.Id} store.Must(ss.Team().SaveMember(m2, -1)) - if result := <-ss.Team().GetTotalMemberCount(teamId1); result.Err != nil { - t.Fatal(result.Err) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(teamId1); err != nil { + t.Fatal(err) } else { - if result.Data.(int64) != 2 { + if totalMemberCount != 2 { t.Fatal("wrong count") } } @@ -1180,10 +1180,10 @@ func testTeamStoreMemberCount(t *testing.T, ss store.Store) { m3 := &model.TeamMember{TeamId: teamId1, UserId: model.NewId()} store.Must(ss.Team().SaveMember(m3, -1)) - if result := <-ss.Team().GetTotalMemberCount(teamId1); result.Err != nil { - t.Fatal(result.Err) + if totalMemberCount, err := ss.Team().GetTotalMemberCount(teamId1); err != nil { + t.Fatal(err) } else { - if result.Data.(int64) != 2 { + if totalMemberCount != 2 { t.Fatal("wrong count") } }