Migrates Channel.GetAllDirectChannelsForExportAfter to sync by default (#11272)

2025-02-25 18:55:24 -06:00 · 2019-06-20 11:01:49 -04:00
parent b1631026d8
commit 1e05d37290
6 changed files with 97 additions and 96 deletions
--- 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
 		}
--- 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 })
--- 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) {
--- 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)
--- 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))

--- 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