From 88202a76d9612bf49f7df57731d945b4fc07d4c3 Mon Sep 17 00:00:00 2001 From: Bolarinwa Balogun Date: Tue, 11 Jun 2019 09:35:17 -0400 Subject: [PATCH] [MM-16159] Migrate "Team.GetByName" to Sync by default (#11107) * [MM-16159] Migrate "Team.GetByName" to Sync by default * Refactor to correct mistakes and remove irrelevant code --- app/channel.go | 11 ++++++----- app/email_batching.go | 10 +++++----- app/import_functions.go | 22 ++++++++++------------ app/team.go | 12 ++++++------ cmd/mattermost/commands/teamargs.go | 8 +++----- output | 0 store/sqlstore/team_store.go | 16 +++++++--------- store/store.go | 2 +- store/storetest/mocks/TeamStore.go | 19 ++++++++++++++----- store/storetest/team_store.go | 8 ++++---- 10 files changed, 56 insertions(+), 52 deletions(-) create mode 100644 output diff --git a/app/channel.go b/app/channel.go index bb101e0cb3..3845f8e0f8 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1208,12 +1208,13 @@ func (a *App) GetChannelsByNames(channelNames []string, teamId string) ([]*model func (a *App) GetChannelByNameForTeamName(channelName, teamName string, includeDeleted bool) (*model.Channel, *model.AppError) { var team *model.Team - result := <-a.Srv.Store.Team().GetByName(teamName) - if result.Err != nil { - result.Err.StatusCode = http.StatusNotFound - return nil, result.Err + team, err := a.Srv.Store.Team().GetByName(teamName) + if err != nil { + err.StatusCode = http.StatusNotFound + return nil, err } - team = result.Data.(*model.Team) + + var result store.StoreResult if includeDeleted { result = <-a.Srv.Store.Channel().GetByNameIncludeDeleted(team.Id, channelName, false) diff --git a/app/email_batching.go b/app/email_batching.go index b5ac5a7228..d75ec43995 100644 --- a/app/email_batching.go +++ b/app/email_batching.go @@ -140,19 +140,19 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu continue } - result := <-job.server.Store.Team().GetByName(notifications[0].teamName) - if result.Err != nil { - mlog.Error(fmt.Sprint("Unable to find Team id for notification", result.Err)) + team, err := job.server.Store.Team().GetByName(notifications[0].teamName) + if err != nil { + mlog.Error(fmt.Sprint("Unable to find Team id for notification", err)) continue } - if team, ok := result.Data.(*model.Team); ok { + if team != nil { inspectedTeamNames[notification.teamName] = team.Id } // if the user has viewed any channels in this team since the notification was queued, delete // all queued notifications - result = <-job.server.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId) + result := <-job.server.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId) if result.Err != nil { mlog.Error(fmt.Sprint("Unable to find ChannelMembers for user", result.Err)) continue diff --git a/app/import_functions.go b/app/import_functions.go index 73503e7200..467bbaf429 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -162,9 +162,9 @@ func (a *App) ImportTeam(data *TeamImportData, dryRun bool) *model.AppError { } var team *model.Team - if result := <-a.Srv.Store.Team().GetByName(*data.Name); result.Err == nil { - team = result.Data.(*model.Team) - } else { + team, err := a.Srv.Store.Team().GetByName(*data.Name) + + if err != nil { team = &model.Team{} } @@ -220,11 +220,10 @@ func (a *App) ImportChannel(data *ChannelImportData, dryRun bool) *model.AppErro return nil } - result := <-a.Srv.Store.Team().GetByName(*data.Team) - if result.Err != nil { - return model.NewAppError("BulkImport", "app.import.import_channel.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, result.Err.Error(), http.StatusBadRequest) + team, err := a.Srv.Store.Team().GetByName(*data.Team) + if err != nil { + return model.NewAppError("BulkImport", "app.import.import_channel.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, err.Error(), http.StatusBadRequest) } - team := result.Data.(*model.Team) var channel *model.Channel if result := <-a.Srv.Store.Channel().GetByNameIncludeDeleted(team.Id, *data.Name, true); result.Err == nil { @@ -942,13 +941,12 @@ func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError { return nil } - result := <-a.Srv.Store.Team().GetByName(*data.Team) - if result.Err != nil { - return model.NewAppError("BulkImport", "app.import.import_post.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, result.Err.Error(), http.StatusBadRequest) + team, err := a.Srv.Store.Team().GetByName(*data.Team) + if err != nil { + return model.NewAppError("BulkImport", "app.import.import_post.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, err.Error(), http.StatusBadRequest) } - team := result.Data.(*model.Team) - result = <-a.Srv.Store.Channel().GetByName(team.Id, *data.Channel, false) + result := <-a.Srv.Store.Channel().GetByName(team.Id, *data.Channel, false) if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.channel_not_found.error", map[string]interface{}{"ChannelName": *data.Channel}, result.Err.Error(), http.StatusBadRequest) } diff --git a/app/team.go b/app/team.go index a114d8584c..140fa13e5e 100644 --- a/app/team.go +++ b/app/team.go @@ -592,12 +592,12 @@ func (a *App) GetTeam(teamId string) (*model.Team, *model.AppError) { } func (a *App) GetTeamByName(name string) (*model.Team, *model.AppError) { - result := <-a.Srv.Store.Team().GetByName(name) - if result.Err != nil { - result.Err.StatusCode = http.StatusNotFound - return nil, result.Err + team, err := a.Srv.Store.Team().GetByName(name) + if err != nil { + err.StatusCode = http.StatusNotFound + return nil, err } - return result.Data.(*model.Team), nil + return team, nil } func (a *App) GetTeamByInviteId(inviteId string) (*model.Team, *model.AppError) { @@ -1045,7 +1045,7 @@ func (a *App) InviteNewUsersToTeam(emailList []string, teamId, senderId string) } func (a *App) FindTeamByName(name string) bool { - if result := <-a.Srv.Store.Team().GetByName(name); result.Err != nil { + if _, err := a.Srv.Store.Team().GetByName(name); err != nil { return false } return true diff --git a/cmd/mattermost/commands/teamargs.go b/cmd/mattermost/commands/teamargs.go index da86b16faf..fe69e4576f 100644 --- a/cmd/mattermost/commands/teamargs.go +++ b/cmd/mattermost/commands/teamargs.go @@ -19,15 +19,13 @@ func getTeamsFromTeamArgs(a *app.App, teamArgs []string) []*model.Team { func getTeamFromTeamArg(a *app.App, teamArg string) *model.Team { var team *model.Team - if result := <-a.Srv.Store.Team().GetByName(teamArg); result.Err == nil { - team = result.Data.(*model.Team) - } + team, err := a.Srv.Store.Team().GetByName(teamArg) if team == nil { - if t, err := a.Srv.Store.Team().Get(teamArg); err == nil { + var t *model.Team + if t, err = a.Srv.Store.Team().Get(teamArg); err == nil { team = t } } - return team } diff --git a/output b/output new file mode 100644 index 0000000000..e69de29bb2 diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 00b4efc46b..6adc06fee5 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -295,17 +295,15 @@ func (s SqlTeamStore) GetByInviteId(inviteId string) store.StoreChannel { }) } -func (s SqlTeamStore) GetByName(name string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - team := model.Team{} +func (s SqlTeamStore) GetByName(name string) (*model.Team, *model.AppError) { - if err := s.GetReplica().SelectOne(&team, "SELECT * FROM Teams WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - result.Err = model.NewAppError("SqlTeamStore.GetByName", "store.sql_team.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError) - return - } + team := model.Team{} - result.Data = &team - }) + err := s.GetReplica().SelectOne(&team, "SELECT * FROM Teams WHERE Name = :Name", map[string]interface{}{"Name": name}) + if err != nil { + return nil, model.NewAppError("SqlTeamStore.GetByName", "store.sql_team.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError) + } + return &team, nil } func (s SqlTeamStore) SearchByName(name string) ([]*model.Team, *model.AppError) { diff --git a/store/store.go b/store/store.go index 074edba494..f709e82c3c 100644 --- a/store/store.go +++ b/store/store.go @@ -85,7 +85,7 @@ type TeamStore interface { Update(team *model.Team) (*model.Team, *model.AppError) UpdateDisplayName(name string, teamId string) StoreChannel Get(id string) (*model.Team, *model.AppError) - GetByName(name string) StoreChannel + GetByName(name string) (*model.Team, *model.AppError) SearchByName(name string) ([]*model.Team, *model.AppError) SearchAll(term string) StoreChannel SearchOpen(term string) StoreChannel diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 9ab52076f0..2a1a5bb982 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -245,19 +245,28 @@ func (_m *TeamStore) GetByInviteId(inviteId string) store.StoreChannel { } // GetByName provides a mock function with given fields: name -func (_m *TeamStore) GetByName(name string) store.StoreChannel { +func (_m *TeamStore) GetByName(name string) (*model.Team, *model.AppError) { ret := _m.Called(name) - 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(name) } 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(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetChannelUnreadsForAllTeams provides a mock function with given fields: excludeTeamId, userId diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index a4936619cb..f913d18213 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -149,15 +149,15 @@ func testTeamStoreGetByName(t *testing.T, ss store.Store) { t.Fatal(err) } - if r1 := <-ss.Team().GetByName(o1.Name); r1.Err != nil { - t.Fatal(r1.Err) + if team, err := ss.Team().GetByName(o1.Name); err != nil { + t.Fatal(err) } else { - if r1.Data.(*model.Team).ToJson() != o1.ToJson() { + if team.ToJson() != o1.ToJson() { t.Fatal("invalid returned team") } } - if err := (<-ss.Team().GetByName("")).Err; err == nil { + if _, err := ss.Team().GetByName(""); err == nil { t.Fatal("Missing id should have failed") } }