From 8252eab5dafa5516ab2165af38c46d4de219476c Mon Sep 17 00:00:00 2001 From: tengis b Date: Sat, 27 Apr 2019 01:18:07 +0900 Subject: [PATCH] [MM-15274] Migrate "Team.Get" to Sync by default (#10694) * Migrate Team.Get to Sync by default * change parseRange return value to ok * fix formatting * remove err checks * use require.nil * require nil --- app/channel.go | 8 ++--- app/command.go | 9 +++++- app/command_loadtest.go | 31 ++++++++----------- app/post.go | 8 ++--- app/slackimport.go | 10 +++--- app/team.go | 47 +++++++++++++++++++++-------- app/user.go | 7 ++--- cmd/mattermost/commands/teamargs.go | 4 +-- store/sqlstore/group_supplier.go | 5 ++- store/sqlstore/team_store.go | 24 ++++++--------- store/store.go | 2 +- store/storetest/mocks/TeamStore.go | 19 +++++++++--- store/storetest/scheme_store.go | 6 ++-- store/storetest/team_store.go | 33 ++++++++++---------- 14 files changed, 115 insertions(+), 98 deletions(-) diff --git a/app/channel.go b/app/channel.go index 71acbe7c39..363f279295 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1922,12 +1922,10 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model. } // keep instance of the previous team - var previousTeam *model.Team - result := <-a.Srv.Store.Team().Get(channel.TeamId) - if result.Err != nil { - return result.Err + previousTeam, err := a.Srv.Store.Team().Get(channel.TeamId) + if err != nil { + return err } - previousTeam = result.Data.(*model.Team) channel.TeamId = team.Id if result := <-a.Srv.Store.Channel().Update(channel); result.Err != nil { diff --git a/app/command.go b/app/command.go index 66e3a47a42..b68cc9c8f3 100644 --- a/app/command.go +++ b/app/command.go @@ -224,7 +224,14 @@ func (a *App) tryExecuteCustomCommand(args *model.CommandArgs, trigger string, m chanChan <- store.StoreResult{Data: channel, Err: err} close(chanChan) }() - teamChan := a.Srv.Store.Team().Get(args.TeamId) + + teamChan := make(chan store.StoreResult, 1) + go func() { + team, err := a.Srv.Store.Team().Get(args.TeamId) + teamChan <- store.StoreResult{Data: team, Err: err} + close(teamChan) + }() + userChan := make(chan store.StoreResult, 1) go func() { user, err := a.Srv.Store.User().Get(args.UserId) diff --git a/app/command_loadtest.go b/app/command_loadtest.go index fd48f65750..ca461a7968 100644 --- a/app/command_loadtest.go +++ b/app/command_loadtest.go @@ -184,12 +184,9 @@ func (me *LoadTestProvider) SetupCommand(a *App, args *model.CommandArgs, messag } } } else { - - var team *model.Team - if tr := <-a.Srv.Store.Team().Get(args.TeamId); tr.Err != nil { + team, err := a.Srv.Store.Team().Get(args.TeamId) + if err != nil { return &model.CommandResponse{Text: "Failed to create testing environment", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } else { - team = tr.Data.(*model.Team) } client.MockSession(args.Session.Token) @@ -215,16 +212,14 @@ func (me *LoadTestProvider) UsersCommand(a *App, args *model.CommandArgs, messag cmd = strings.TrimSpace(strings.TrimPrefix(cmd, "fuzz")) } - usersr, err := parseRange(cmd, "") - if !err { + usersr, ok := parseRange(cmd, "") + if !ok { usersr = utils.Range{Begin: 2, End: 5} } - var team *model.Team - if tr := <-a.Srv.Store.Team().Get(args.TeamId); tr.Err != nil { + team, err := a.Srv.Store.Team().Get(args.TeamId) + if err != nil { return &model.CommandResponse{Text: "Failed to create testing environment", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } else { - team = tr.Data.(*model.Team) } client := model.NewAPIv4Client(args.SiteURL) @@ -244,16 +239,14 @@ func (me *LoadTestProvider) ChannelsCommand(a *App, args *model.CommandArgs, mes cmd = strings.TrimSpace(strings.TrimPrefix(cmd, "fuzz")) } - channelsr, err := parseRange(cmd, "") - if !err { + channelsr, ok := parseRange(cmd, "") + if !ok { channelsr = utils.Range{Begin: 2, End: 5} } - var team *model.Team - if tr := <-a.Srv.Store.Team().Get(args.TeamId); tr.Err != nil { + team, err := a.Srv.Store.Team().Get(args.TeamId) + if err != nil { return &model.CommandResponse{Text: "Failed to create testing environment", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } else { - team = tr.Data.(*model.Team) } client := model.NewAPIv4Client(args.SiteURL) @@ -274,8 +267,8 @@ func (me *LoadTestProvider) PostsCommand(a *App, args *model.CommandArgs, messag cmd = strings.TrimSpace(strings.TrimPrefix(cmd, "fuzz")) } - postsr, err := parseRange(cmd, "") - if !err { + postsr, ok := parseRange(cmd, "") + if !ok { postsr = utils.Range{Begin: 20, End: 30} } diff --git a/app/post.go b/app/post.go index 06c4bb8ca1..25a686996f 100644 --- a/app/post.go +++ b/app/post.go @@ -363,11 +363,11 @@ func (a *App) FillInPostProps(post *model.Post, channel *model.Channel) *model.A func (a *App) handlePostEvents(post *model.Post, user *model.User, channel *model.Channel, triggerWebhooks bool, parentPostList *model.PostList) error { var team *model.Team if len(channel.TeamId) > 0 { - result := <-a.Srv.Store.Team().Get(channel.TeamId) - if result.Err != nil { - return result.Err + t, err := a.Srv.Store.Team().Get(channel.TeamId) + if err != nil { + return err } - team = result.Data.(*model.Team) + team = t } else { // Blank team for DMs team = &model.Team{} diff --git a/app/slackimport.go b/app/slackimport.go index c810fdebb3..cf1c339039 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -141,12 +141,11 @@ func (a *App) SlackAddUsers(teamId string, slackusers []SlackUser, importerLog * addedUsers := make(map[string]*model.User) // Need the team - result := <-a.Srv.Store.Team().Get(teamId) - if result.Err != nil { + team, err := a.Srv.Store.Team().Get(teamId) + if err != nil { importerLog.WriteString(utils.T("api.slackimport.slack_import.team_fail")) return addedUsers } - team := result.Data.(*model.Team) for _, sUser := range slackusers { firstName := sUser.Profile.FirstName @@ -193,12 +192,11 @@ func (a *App) SlackAddUsers(teamId string, slackusers []SlackUser, importerLog * } func (a *App) SlackAddBotUser(teamId string, log *bytes.Buffer) *model.User { - result := <-a.Srv.Store.Team().Get(teamId) - if result.Err != nil { + team, err := a.Srv.Store.Team().Get(teamId) + if err != nil { log.WriteString(utils.T("api.slackimport.slack_import.team_fail")) return nil } - team := result.Data.(*model.Team) password := model.NewId() username := "slackimportuser_" + model.NewId() diff --git a/app/team.go b/app/team.go index 7cfb6279d0..c2bf112f7d 100644 --- a/app/team.go +++ b/app/team.go @@ -340,7 +340,13 @@ func (a *App) sendUpdatedMemberRoleEvent(userId string, member *model.TeamMember } func (a *App) AddUserToTeam(teamId string, userId string, userRequestorId string) (*model.Team, *model.AppError) { - tchan := a.Srv.Store.Team().Get(teamId) + tchan := make(chan store.StoreResult, 1) + go func() { + team, err := a.Srv.Store.Team().Get(teamId) + tchan <- store.StoreResult{Data: team, Err: err} + close(tchan) + }() + uchan := make(chan store.StoreResult, 1) go func() { user, err := a.Srv.Store.User().Get(userId) @@ -368,11 +374,12 @@ func (a *App) AddUserToTeam(teamId string, userId string, userRequestorId string } func (a *App) AddUserToTeamByTeamId(teamId string, user *model.User) *model.AppError { - result := <-a.Srv.Store.Team().Get(teamId) - if result.Err != nil { - return result.Err + team, err := a.Srv.Store.Team().Get(teamId) + if err != nil { + return err } - return a.JoinUserToTeam(result.Data.(*model.Team), user, "") + + return a.JoinUserToTeam(team, user, "") } func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team, *model.AppError) { @@ -393,7 +400,13 @@ func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team, tokenData := model.MapFromJson(strings.NewReader(token.Extra)) - tchan := a.Srv.Store.Team().Get(tokenData["teamId"]) + tchan := make(chan store.StoreResult, 1) + go func() { + team, err := a.Srv.Store.Team().Get(tokenData["teamId"]) + tchan <- store.StoreResult{Data: team, Err: err} + close(tchan) + }() + uchan := make(chan store.StoreResult, 1) go func() { user, err := a.Srv.Store.User().Get(userId) @@ -556,11 +569,7 @@ func (a *App) JoinUserToTeam(team *model.Team, user *model.User, userRequestorId } func (a *App) GetTeam(teamId string) (*model.Team, *model.AppError) { - result := <-a.Srv.Store.Team().Get(teamId) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.Team), nil + return a.Srv.Store.Team().Get(teamId) } func (a *App) GetTeamByName(name string) (*model.Team, *model.AppError) { @@ -797,7 +806,13 @@ func (a *App) GetTeamUnread(teamId, userId string) (*model.TeamUnread, *model.Ap } func (a *App) RemoveUserFromTeam(teamId string, userId string, requestorId string) *model.AppError { - tchan := a.Srv.Store.Team().Get(teamId) + tchan := make(chan store.StoreResult, 1) + go func() { + team, err := a.Srv.Store.Team().Get(teamId) + tchan <- store.StoreResult{Data: team, Err: err} + close(tchan) + }() + uchan := make(chan store.StoreResult, 1) go func() { user, err := a.Srv.Store.User().Get(userId) @@ -967,7 +982,13 @@ func (a *App) InviteNewUsersToTeam(emailList []string, teamId, senderId string) return err } - tchan := a.Srv.Store.Team().Get(teamId) + tchan := make(chan store.StoreResult, 1) + go func() { + team, err := a.Srv.Store.Team().Get(teamId) + tchan <- store.StoreResult{Data: team, Err: err} + close(tchan) + }() + uchan := make(chan store.StoreResult, 1) go func() { user, err := a.Srv.Store.User().Get(senderId) diff --git a/app/user.go b/app/user.go index 60c59047a4..66398e7fd2 100644 --- a/app/user.go +++ b/app/user.go @@ -67,11 +67,10 @@ func (a *App) CreateUserWithToken(user *model.User, tokenId string) (*model.User tokenData := model.MapFromJson(strings.NewReader(token.Extra)) - result = <-a.Srv.Store.Team().Get(tokenData["teamId"]) - if result.Err != nil { - return nil, result.Err + team, err := a.Srv.Store.Team().Get(tokenData["teamId"]) + if err != nil { + return nil, err } - team := result.Data.(*model.Team) user.Email = tokenData["email"] user.EmailVerified = true diff --git a/cmd/mattermost/commands/teamargs.go b/cmd/mattermost/commands/teamargs.go index aa62d52b8e..da86b16faf 100644 --- a/cmd/mattermost/commands/teamargs.go +++ b/cmd/mattermost/commands/teamargs.go @@ -24,8 +24,8 @@ func getTeamFromTeamArg(a *app.App, teamArg string) *model.Team { } if team == nil { - if result := <-a.Srv.Store.Team().Get(teamArg); result.Err == nil { - team = result.Data.(*model.Team) + if t, err := a.Srv.Store.Team().Get(teamArg); err == nil { + team = t } } diff --git a/store/sqlstore/group_supplier.go b/store/sqlstore/group_supplier.go index b7ab96cd0f..4b8938c725 100644 --- a/store/sqlstore/group_supplier.go +++ b/store/sqlstore/group_supplier.go @@ -398,9 +398,8 @@ func (s *SqlSupplier) GroupCreateGroupSyncable(ctx context.Context, groupSyncabl switch groupSyncable.Type { case model.GroupSyncableTypeTeam: - teamResult := <-s.Team().Get(groupSyncable.SyncableId) - if teamResult.Err != nil { - result.Err = teamResult.Err + if _, err := s.Team().Get(groupSyncable.SyncableId); err != nil { + result.Err = err return result } diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index c5ced3806d..a7a5f96a69 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -238,22 +238,16 @@ func (s SqlTeamStore) UpdateDisplayName(name string, teamId string) store.StoreC }) } -func (s SqlTeamStore) Get(id string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - obj, err := s.GetReplica().Get(model.Team{}, id) - if err != nil { - result.Err = model.NewAppError("SqlTeamStore.Get", "store.sql_team.get.finding.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) - return - } - if obj == nil { - result.Err = model.NewAppError("SqlTeamStore.Get", "store.sql_team.get.find.app_error", nil, "id="+id, http.StatusNotFound) - return - } +func (s SqlTeamStore) Get(id string) (*model.Team, *model.AppError) { + obj, err := s.GetReplica().Get(model.Team{}, id) + if err != nil { + return nil, model.NewAppError("SqlTeamStore.Get", "store.sql_team.get.finding.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + } + if obj == nil { + return nil, model.NewAppError("SqlTeamStore.Get", "store.sql_team.get.find.app_error", nil, "id="+id, http.StatusNotFound) + } - team := obj.(*model.Team) - - result.Data = team - }) + return obj.(*model.Team), nil } func (s SqlTeamStore) GetByInviteId(inviteId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index e82f290d72..989182976c 100644 --- a/store/store.go +++ b/store/store.go @@ -84,7 +84,7 @@ type TeamStore interface { Save(team *model.Team) StoreChannel Update(team *model.Team) (*model.Team, *model.AppError) UpdateDisplayName(name string, teamId string) StoreChannel - Get(id string) StoreChannel + Get(id string) (*model.Team, *model.AppError) GetByName(name string) StoreChannel SearchByName(name string) StoreChannel SearchAll(term string) StoreChannel diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 52aced573c..66f6066e4b 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -62,19 +62,28 @@ func (_m *TeamStore) ClearAllCustomRoleAssignments() store.StoreChannel { } // Get provides a mock function with given fields: id -func (_m *TeamStore) Get(id string) store.StoreChannel { +func (_m *TeamStore) Get(id string) (*model.Team, *model.AppError) { ret := _m.Called(id) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.Team + if rf, ok := ret.Get(0).(func(string) *model.Team); ok { r0 = rf(id) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Team) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(id) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetActiveMemberCount provides a mock function with given fields: teamId diff --git a/store/storetest/scheme_store.go b/store/storetest/scheme_store.go index 7243016da8..da9102637f 100644 --- a/store/storetest/scheme_store.go +++ b/store/storetest/scheme_store.go @@ -4,6 +4,7 @@ package storetest import ( + "github.com/stretchr/testify/require" "testing" "github.com/stretchr/testify/assert" @@ -392,9 +393,8 @@ func testSchemeStoreDelete(t *testing.T, ss store.Store) { sres4 := <-ss.Scheme().Delete(d4.Id) assert.Nil(t, sres4.Err) - tres5 := <-ss.Team().Get(t4.Id) - assert.Nil(t, tres5.Err) - t5 := tres5.Data.(*model.Team) + t5, err := ss.Team().Get(t4.Id) + require.Nil(t, err) assert.Equal(t, "", *t5.SchemeId) // Try deleting a channel scheme that's in use. diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 9aa2d52aa3..2a8914afa2 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -115,10 +115,9 @@ func testTeamStoreUpdateDisplayName(t *testing.T, ss store.Store) { t.Fatal(err) } - ro1 := (<-ss.Team().Get(o1.Id)).Data.(*model.Team) - if ro1.DisplayName != newDisplayName { - t.Fatal("DisplayName not updated") - } + ro1, err := ss.Team().Get(o1.Id) + require.Nil(t, err) + require.Equal(t, newDisplayName, ro1.DisplayName, "DisplayName not updated") } func testTeamStoreGet(t *testing.T, ss store.Store) { @@ -129,17 +128,12 @@ func testTeamStoreGet(t *testing.T, ss store.Store) { o1.Type = model.TEAM_OPEN store.Must(ss.Team().Save(&o1)) - if r1 := <-ss.Team().Get(o1.Id); r1.Err != nil { - t.Fatal(r1.Err) - } else { - if r1.Data.(*model.Team).ToJson() != o1.ToJson() { - t.Fatal("invalid returned team") - } - } + r1, err := ss.Team().Get(o1.Id) + require.Nil(t, err) + require.Equal(t, r1.ToJson(), o1.ToJson()) - if err := (<-ss.Team().Get("")).Err; err == nil { - t.Fatal("Missing id should have failed") - } + _, err = ss.Team().Get("") + require.NotNil(t, err, "Missing id should have failed") } func testTeamStoreGetByName(t *testing.T, ss store.Store) { @@ -1284,7 +1278,9 @@ func testUpdateLastTeamIconUpdate(t *testing.T, ss store.Store) { t.Fatal(err) } - ro1 := (<-ss.Team().Get(o1.Id)).Data.(*model.Team) + ro1, err := ss.Team().Get(o1.Id) + require.Nil(t, err) + if ro1.LastTeamIconUpdate <= lastTeamIconUpdateInitial { t.Fatal("LastTeamIconUpdate not updated") } @@ -1459,8 +1455,11 @@ func testResetAllTeamSchemes(t *testing.T, ss store.Store) { res := <-ss.Team().ResetAllTeamSchemes() assert.Nil(t, res.Err) - t1 = (<-ss.Team().Get(t1.Id)).Data.(*model.Team) - t2 = (<-ss.Team().Get(t2.Id)).Data.(*model.Team) + t1, err := ss.Team().Get(t1.Id) + require.Nil(t, err) + + t2, err = ss.Team().Get(t2.Id) + require.Nil(t, err) assert.Equal(t, "", *t1.SchemeId) assert.Equal(t, "", *t2.SchemeId)