From 1e05d37290dc6ace9371ef3cf22c53e3408ceb18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Thu, 20 Jun 2019 11:01:49 -0400 Subject: [PATCH] Migrates Channel.GetAllDirectChannelsForExportAfter to sync by default (#11272) --- app/export.go | 7 +- app/export_test.go | 44 +++++------ store/sqlstore/channel_store.go | 106 +++++++++++++------------- store/store.go | 2 +- store/storetest/channel_store.go | 15 ++-- store/storetest/mocks/ChannelStore.go | 19 +++-- 6 files changed, 97 insertions(+), 96 deletions(-) diff --git a/app/export.go b/app/export.go index 232155160e..a12250a883 100644 --- a/app/export.go +++ b/app/export.go @@ -507,12 +507,11 @@ func (a *App) copyEmojiImages(emojiId string, emojiImagePath string, pathToDir s func (a *App) ExportAllDirectChannels(writer io.Writer) *model.AppError { afterId := strings.Repeat("0", 26) for { - result := <-a.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, afterId) - if result.Err != nil { - return result.Err + channels, err := a.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, afterId) + if err != nil { + return err } - channels := result.Data.([]*model.DirectChannelForExport) if len(channels) == 0 { break } diff --git a/app/export_test.go b/app/export_test.go index 50741609e3..3bbdffd248 100644 --- a/app/export_test.go +++ b/app/export_test.go @@ -227,8 +227,8 @@ func TestExportDMChannel(t *testing.T) { err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) - result := <-th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels := result.Data.([]*model.DirectChannelForExport) + channels, err := th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 1, len(channels)) th1.TearDown() @@ -236,8 +236,8 @@ func TestExportDMChannel(t *testing.T) { th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 0, len(channels)) // import the exported channel @@ -246,8 +246,8 @@ func TestExportDMChannel(t *testing.T) { assert.Equal(t, 0, i) // Ensure the Members of the imported DM channel is the same was from the exported - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 1, len(channels)) assert.ElementsMatch(t, []string{th1.BasicUser.Username, th1.BasicUser2.Username}, *channels[0].Members) } @@ -263,15 +263,15 @@ func TestExportDMChannelToSelf(t *testing.T) { err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) - result := <-th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels := result.Data.([]*model.DirectChannelForExport) + channels, err := th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 1, len(channels)) th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 0, len(channels)) // import the exported channel @@ -280,8 +280,8 @@ func TestExportDMChannelToSelf(t *testing.T) { assert.Equal(t, 0, i) // Ensure no channels were imported - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 0, len(channels)) } @@ -300,8 +300,8 @@ func TestExportGMChannel(t *testing.T) { err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) - result := <-th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels := result.Data.([]*model.DirectChannelForExport) + channels, err := th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 1, len(channels)) th1.TearDown() @@ -309,8 +309,8 @@ func TestExportGMChannel(t *testing.T) { th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 0, len(channels)) } @@ -332,8 +332,8 @@ func TestExportGMandDMChannels(t *testing.T) { err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) - result := <-th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels := result.Data.([]*model.DirectChannelForExport) + channels, err := th1.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 2, len(channels)) th1.TearDown() @@ -341,8 +341,8 @@ func TestExportGMandDMChannels(t *testing.T) { th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) assert.Equal(t, 0, len(channels)) // import the exported channel @@ -351,8 +351,8 @@ func TestExportGMandDMChannels(t *testing.T) { assert.Equal(t, 0, i) // Ensure the Members of the imported GM channel is the same was from the exported - result = <-th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") - channels = result.Data.([]*model.DirectChannelForExport) + channels, err = th2.App.Srv.Store.Channel().GetAllDirectChannelsForExportAfter(1000, "00000000") + require.Nil(t, err) // Adding some deteminism so its possible to assert on slice index sort.Slice(channels, func(i, j int) bool { return channels[i].Type > channels[j].Type }) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index ae22a2efa3..c3406236a4 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -2487,67 +2487,63 @@ func (s SqlChannelStore) GetChannelMembersForExport(userId string, teamId string return members, nil } -func (s SqlChannelStore) GetAllDirectChannelsForExportAfter(limit int, afterId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var directChannelsForExport []*model.DirectChannelForExport - query := s.getQueryBuilder(). - Select("Channels.*"). - From("Channels"). - Where(sq.And{ - sq.Gt{"Channels.Id": afterId}, - sq.Eq{"Channels.DeleteAt": int(0)}, - sq.Eq{"Channels.Type": []string{"D", "G"}}, - }). - OrderBy("Channels.Id"). - Limit(uint64(limit)) +func (s SqlChannelStore) GetAllDirectChannelsForExportAfter(limit int, afterId string) ([]*model.DirectChannelForExport, *model.AppError) { + var directChannelsForExport []*model.DirectChannelForExport + query := s.getQueryBuilder(). + Select("Channels.*"). + From("Channels"). + Where(sq.And{ + sq.Gt{"Channels.Id": afterId}, + sq.Eq{"Channels.DeleteAt": int(0)}, + sq.Eq{"Channels.Type": []string{"D", "G"}}, + }). + OrderBy("Channels.Id"). + Limit(uint64(limit)) - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) + } - if _, err = s.GetReplica().Select(&directChannelsForExport, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if _, err = s.GetReplica().Select(&directChannelsForExport, queryString, args...); err != nil { + return nil, model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var channelIds []string - for _, channel := range directChannelsForExport { - channelIds = append(channelIds, channel.Id) - } - query = s.getQueryBuilder(). - Select("u.Username as Username, ChannelId, UserId, cm.Roles as Roles, LastViewedAt, MsgCount, MentionCount, cm.NotifyProps as NotifyProps, LastUpdateAt, SchemeUser, SchemeAdmin, (SchemeGuest IS NOT NULL AND SchemeGuest) as SchemeGuest"). - From("ChannelMembers cm"). - Join("Users u ON ( u.Id = cm.UserId )"). - Where(sq.And{ - sq.Eq{"cm.ChannelId": channelIds}, - sq.Eq{"u.DeleteAt": int(0)}, - }) + var channelIds []string + for _, channel := range directChannelsForExport { + channelIds = append(channelIds, channel.Id) + } + query = s.getQueryBuilder(). + Select("u.Username as Username, ChannelId, UserId, cm.Roles as Roles, LastViewedAt, MsgCount, MentionCount, cm.NotifyProps as NotifyProps, LastUpdateAt, SchemeUser, SchemeAdmin, (SchemeGuest IS NOT NULL AND SchemeGuest) as SchemeGuest"). + From("ChannelMembers cm"). + Join("Users u ON ( u.Id = cm.UserId )"). + Where(sq.And{ + sq.Eq{"cm.ChannelId": channelIds}, + sq.Eq{"u.DeleteAt": int(0)}, + }) - queryString, args, err = query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err = query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var channelMembers []*model.ChannelMemberForExport - if _, err := s.GetReplica().Select(&channelMembers, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) - } + var channelMembers []*model.ChannelMemberForExport + if _, err := s.GetReplica().Select(&channelMembers, queryString, args...); err != nil { + return nil, model.NewAppError("SqlChannelStore.GetAllDirectChannelsForExportAfter", "store.sql_channel.get_all_direct.app_error", nil, err.Error(), http.StatusInternalServerError) + } - // Populate each channel with its members - dmChannelsMap := make(map[string]*model.DirectChannelForExport) - for _, channel := range directChannelsForExport { - channel.Members = &[]string{} - dmChannelsMap[channel.Id] = channel - } - for _, member := range channelMembers { - members := dmChannelsMap[member.ChannelId].Members - *members = append(*members, member.Username) - } - result.Data = directChannelsForExport - }) + // Populate each channel with its members + dmChannelsMap := make(map[string]*model.DirectChannelForExport) + for _, channel := range directChannelsForExport { + channel.Members = &[]string{} + dmChannelsMap[channel.Id] = channel + } + for _, member := range channelMembers { + members := dmChannelsMap[member.ChannelId].Members + *members = append(*members, member.Username) + } + + return directChannelsForExport, nil } func (s SqlChannelStore) GetChannelsBatchForIndexing(startTime, endTime int64, limit int) ([]*model.Channel, *model.AppError) { diff --git a/store/store.go b/store/store.go index ccf208e44a..d6db68ff01 100644 --- a/store/store.go +++ b/store/store.go @@ -195,7 +195,7 @@ type ChannelStore interface { ClearAllCustomRoleAssignments() *model.AppError MigratePublicChannels() error GetAllChannelsForExportAfter(limit int, afterId string) ([]*model.ChannelForExport, *model.AppError) - GetAllDirectChannelsForExportAfter(limit int, afterId string) StoreChannel + GetAllDirectChannelsForExportAfter(limit int, afterId string) ([]*model.DirectChannelForExport, *model.AppError) GetChannelMembersForExport(userId string, teamId string) ([]*model.ChannelMemberForExport, *model.AppError) RemoveAllDeactivatedMembers(channelId string) *model.AppError GetChannelsBatchForIndexing(startTime, endTime int64, limit int) ([]*model.Channel, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 32c7188c1d..66ebcceef1 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -3467,9 +3467,8 @@ func testChannelStoreExportAllDirectChannels(t *testing.T, ss store.Store, s Sql ss.Channel().SaveDirectChannel(&o1, &m1, &m2) - r1 := <-ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectChannelForExport) + d1, err := ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) assert.Equal(t, 2, len(d1)) assert.ElementsMatch(t, []string{o1.DisplayName, o2.DisplayName}, []string{d1[0].DisplayName, d1[1].DisplayName}) @@ -3527,9 +3526,8 @@ func testChannelStoreExportAllDirectChannelsExcludePrivateAndPublic(t *testing.T ss.Channel().SaveDirectChannel(&o1, &m1, &m2) - r1 := <-ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectChannelForExport) + d1, err := ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) assert.Equal(t, 1, len(d1)) assert.Equal(t, o1.DisplayName, d1[0].DisplayName) @@ -3574,9 +3572,8 @@ func testChannelStoreExportAllDirectChannelsDeletedChannel(t *testing.T, ss stor err := ss.Channel().SetDeleteAt(o1.Id, 1, 1) require.Nil(t, err, "channel should have been deleted") - r1 := <-ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectChannelForExport) + d1, err := ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) assert.Equal(t, 0, len(d1)) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index a7e59d583c..7d2617de6f 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -304,19 +304,28 @@ func (_m *ChannelStore) GetAllChannelsForExportAfter(limit int, afterId string) } // GetAllDirectChannelsForExportAfter provides a mock function with given fields: limit, afterId -func (_m *ChannelStore) GetAllDirectChannelsForExportAfter(limit int, afterId string) store.StoreChannel { +func (_m *ChannelStore) GetAllDirectChannelsForExportAfter(limit int, afterId string) ([]*model.DirectChannelForExport, *model.AppError) { ret := _m.Called(limit, afterId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(int, string) store.StoreChannel); ok { + var r0 []*model.DirectChannelForExport + if rf, ok := ret.Get(0).(func(int, string) []*model.DirectChannelForExport); ok { r0 = rf(limit, afterId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.DirectChannelForExport) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(int, string) *model.AppError); ok { + r1 = rf(limit, afterId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetByName provides a mock function with given fields: team_id, name, allowFromCache