From 327a1c92e80c818d19b513ae768bcac838f20c95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Thu, 20 Jun 2019 03:31:40 -0400 Subject: [PATCH] Migrates Channel.GetChannels to sync by default (#11204) * Migrates Channel.GetChannels to sync by default * Some suggestions * resolved undefined var --- app/channel.go | 6 +---- app/team.go | 8 +++---- manualtesting/manual_testing.go | 12 +++++----- store/sqlstore/channel_store.go | 32 ++++++++++++--------------- store/store.go | 2 +- store/storetest/channel_store.go | 19 ++++++++-------- store/storetest/mocks/ChannelStore.go | 19 +++++++++++----- 7 files changed, 47 insertions(+), 51 deletions(-) diff --git a/app/channel.go b/app/channel.go index a28c397da2..9f0a862b51 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1226,11 +1226,7 @@ func (a *App) GetChannelByNameForTeamName(channelName, teamName string, includeD } func (a *App) GetChannelsForUser(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { - result := <-a.Srv.Store.Channel().GetChannels(teamId, userId, includeDeleted) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.ChannelList), nil + return a.Srv.Store.Channel().GetChannels(teamId, userId, includeDeleted) } func (a *App) GetAllChannels(page, perPage int, opts model.ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) { diff --git a/app/team.go b/app/team.go index 5406823e8c..8e82a02722 100644 --- a/app/team.go +++ b/app/team.go @@ -823,14 +823,12 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string) var channelList *model.ChannelList - if result := <-a.Srv.Store.Channel().GetChannels(team.Id, user.Id, true); result.Err != nil { - if result.Err.Id == "store.sql_channel.get_channels.not_found.app_error" { + if channelList, err = a.Srv.Store.Channel().GetChannels(team.Id, user.Id, true); err != nil { + if err.Id == "store.sql_channel.get_channels.not_found.app_error" { channelList = &model.ChannelList{} } else { - return result.Err + return err } - } else { - channelList = result.Data.(*model.ChannelList) } for _, channel := range *channelList { diff --git a/manualtesting/manual_testing.go b/manualtesting/manual_testing.go index 8c7df063ec..e7d8cc21ef 100644 --- a/manualtesting/manual_testing.go +++ b/manualtesting/manual_testing.go @@ -149,21 +149,19 @@ func manualTest(c *web.Context, w http.ResponseWriter, r *http.Request) { } } -func getChannelID(a *app.App, channelname string, teamid string, userid string) (id string, err bool) { +func getChannelID(a *app.App, channelname string, teamid string, userid string) (string, bool) { // Grab all the channels - result := <-a.Srv.Store.Channel().GetChannels(teamid, userid, false) - if result.Err != nil { + channels, err := a.Srv.Store.Channel().GetChannels(teamid, userid, false) + if err != nil { mlog.Debug("Unable to get channels") return "", false } - data := result.Data.(model.ChannelList) - - for _, channel := range data { + for _, channel := range *channels { if channel.Name == channelname { return channel.Id, true } } - mlog.Debug(fmt.Sprintf("Could not find channel: %v, %v possibilities searched", channelname, strconv.Itoa(len(data)))) + mlog.Debug(fmt.Sprintf("Could not find channel: %v, %v possibilities searched", channelname, strconv.Itoa(len(*channels)))) return "", false } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index aa1f478c51..a2402b2b52 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -929,27 +929,23 @@ func (s SqlChannelStore) PermanentDeleteMembersByChannel(channelId string) *mode return nil } -func (s SqlChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND DeleteAt = 0 AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" - if includeDeleted { - query = "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" - } - data := &model.ChannelList{} - _, err := s.GetReplica().Select(data, query, map[string]interface{}{"TeamId": teamId, "UserId": userId}) +func (s SqlChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { + query := "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND DeleteAt = 0 AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" + if includeDeleted { + query = "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" + } + channels := &model.ChannelList{} + _, err := s.GetReplica().Select(channels, query, map[string]interface{}{"TeamId": teamId, "UserId": userId}) - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.get.app_error", nil, "teamId="+teamId+", userId="+userId+", err="+err.Error(), http.StatusInternalServerError) - return - } + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.get.app_error", nil, "teamId="+teamId+", userId="+userId+", err="+err.Error(), http.StatusInternalServerError) + } - if len(*data) == 0 { - result.Err = model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.not_found.app_error", nil, "teamId="+teamId+", userId="+userId, http.StatusBadRequest) - return - } + if len(*channels) == 0 { + return nil, model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.not_found.app_error", nil, "teamId="+teamId+", userId="+userId, http.StatusBadRequest) + } - result.Data = data - }) + return channels, nil } func (s SqlChannelStore) GetAllChannels(offset int, limit int, opts store.ChannelSearchOpts) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 7eb51bf27d..28af72799c 100644 --- a/store/store.go +++ b/store/store.go @@ -147,7 +147,7 @@ type ChannelStore interface { GetByNameIncludeDeleted(team_id string, name string, allowFromCache bool) StoreChannel GetDeletedByName(team_id string, name string) StoreChannel GetDeleted(team_id string, offset int, limit int) (*model.ChannelList, *model.AppError) - GetChannels(teamId string, userId string, includeDeleted bool) StoreChannel + GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) GetAllChannels(page, perPage int, opts ChannelSearchOpts) StoreChannel GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) GetPublicChannelsForTeam(teamId string, offset int, limit int) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 913a94e0f6..8713bf3552 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -621,9 +621,8 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { t.Fatal(err) } - cresult := <-ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - require.Nil(t, cresult.Err) - list := cresult.Data.(*model.ChannelList) + list, err := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + require.Nil(t, err) if len(*list) != 1 { t.Fatal("invalid number of channels") @@ -636,14 +635,14 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { t.Fatal("invalid number of channels") } - cresult = <-ss.Channel().PermanentDelete(o2.Id) + cresult := <-ss.Channel().PermanentDelete(o2.Id) require.Nil(t, cresult.Err) - cresult = <-ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - if assert.NotNil(t, cresult.Err) { - require.Equal(t, "store.sql_channel.get_channels.not_found.app_error", cresult.Err.Id) + list, err = ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + if assert.NotNil(t, err) { + require.Equal(t, "store.sql_channel.get_channels.not_found.app_error", err.Id) } else { - require.Equal(t, &model.ChannelList{}, cresult.Data.(*model.ChannelList)) + require.Equal(t, &model.ChannelList{}, list) } if r := <-ss.Channel().PermanentDeleteByTeam(o1.TeamId); r.Err != nil { @@ -1032,8 +1031,8 @@ func testChannelStoreGetChannels(t *testing.T, ss store.Store) { m3.NotifyProps = model.GetDefaultChannelNotifyProps() store.Must(ss.Channel().SaveMember(&m3)) - cresult := <-ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - list := cresult.Data.(*model.ChannelList) + list, err := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + require.Nil(t, err) if (*list)[0].Id != o1.Id { t.Fatal("missing channel") diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index c853091c4e..9547d3b26f 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -468,19 +468,28 @@ func (_m *ChannelStore) GetChannelUnread(channelId string, userId string) (*mode } // GetChannels provides a mock function with given fields: teamId, userId, includeDeleted -func (_m *ChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) store.StoreChannel { +func (_m *ChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { ret := _m.Called(teamId, userId, includeDeleted) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string, bool) store.StoreChannel); ok { + var r0 *model.ChannelList + if rf, ok := ret.Get(0).(func(string, string, bool) *model.ChannelList); ok { r0 = rf(teamId, userId, includeDeleted) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.ChannelList) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string, bool) *model.AppError); ok { + r1 = rf(teamId, userId, includeDeleted) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetChannelsBatchForIndexing provides a mock function with given fields: startTime, endTime, limit