diff --git a/app/export.go b/app/export.go index 8e0e686c9d..bd743839c0 100644 --- a/app/export.go +++ b/app/export.go @@ -552,12 +552,11 @@ func (a *App) ExportAllDirectChannels(writer io.Writer) *model.AppError { func (a *App) ExportAllDirectPosts(writer io.Writer) *model.AppError { afterId := strings.Repeat("0", 26) for { - result := <-a.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, afterId) - if result.Err != nil { - return result.Err + posts, err := a.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, afterId) + if err != nil { + return err } - posts := result.Data.([]*model.DirectPostForExport) if len(posts) == 0 { break } diff --git a/app/export_test.go b/app/export_test.go index 46a8e6b21e..50741609e3 100644 --- a/app/export_test.go +++ b/app/export_test.go @@ -407,12 +407,12 @@ func TestExportDMandGMPost(t *testing.T) { } th1.App.CreatePost(p4, gmChannel, false) - result := <-th1.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts := result.Data.([]*model.DirectPostForExport) + posts, err := th1.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) assert.Equal(t, 4, len(posts)) var b bytes.Buffer - err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") + err = th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) th1.TearDown() @@ -420,8 +420,8 @@ func TestExportDMandGMPost(t *testing.T) { th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts = result.Data.([]*model.DirectPostForExport) + posts, err = th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) assert.Equal(t, 0, len(posts)) // import the exported posts @@ -429,8 +429,8 @@ func TestExportDMandGMPost(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 0, i) - result = <-th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts = result.Data.([]*model.DirectPostForExport) + posts, err = th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) // Adding some deteminism so its possible to assert on slice index sort.Slice(posts, func(i, j int) bool { return posts[i].Message > posts[j].Message }) @@ -453,8 +453,8 @@ func TestExportDMPostWithSelf(t *testing.T) { err := th1.App.BulkExport(&b, "somefile", "somePath", "someDir") require.Nil(t, err) - result := <-th1.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts := result.Data.([]*model.DirectPostForExport) + posts, err := th1.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) assert.Equal(t, 1, len(posts)) th1.TearDown() @@ -462,8 +462,8 @@ func TestExportDMPostWithSelf(t *testing.T) { th2 := Setup(t) defer th2.TearDown() - result = <-th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts = result.Data.([]*model.DirectPostForExport) + posts, err = th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) assert.Equal(t, 0, len(posts)) // import the exported posts @@ -471,7 +471,7 @@ func TestExportDMPostWithSelf(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 0, i) - result = <-th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") - posts = result.Data.([]*model.DirectPostForExport) + posts, err = th2.App.Srv.Store.Post().GetDirectPostParentsForExportAfter(1000, "0000000") + require.Nil(t, err) assert.Equal(t, 0, len(posts)) } diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 0f2f8d5a5a..73bd0d36e5 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -1330,76 +1330,72 @@ func (s *SqlPostStore) GetRepliesForExport(parentId string) store.StoreChannel { }) } -func (s *SqlPostStore) GetDirectPostParentsForExportAfter(limit int, afterId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := s.getQueryBuilder(). - Select("p.*", "Users.Username as User"). - From("Posts p"). - Join("Channels ON p.ChannelId = Channels.Id"). - Join("Users ON p.UserId = Users.Id"). - Where(sq.And{ - sq.Gt{"p.Id": afterId}, - sq.Eq{"p.ParentId": string("")}, - sq.Eq{"p.DeleteAt": int(0)}, - sq.Eq{"Channels.DeleteAt": int(0)}, - sq.Eq{"Users.DeleteAt": int(0)}, - sq.Eq{"Channels.Type": []string{"D", "G"}}, - }). - OrderBy("p.Id"). - Limit(uint64(limit)) +func (s *SqlPostStore) GetDirectPostParentsForExportAfter(limit int, afterId string) ([]*model.DirectPostForExport, *model.AppError) { + query := s.getQueryBuilder(). + Select("p.*", "Users.Username as User"). + From("Posts p"). + Join("Channels ON p.ChannelId = Channels.Id"). + Join("Users ON p.UserId = Users.Id"). + Where(sq.And{ + sq.Gt{"p.Id": afterId}, + sq.Eq{"p.ParentId": string("")}, + sq.Eq{"p.DeleteAt": int(0)}, + sq.Eq{"Channels.DeleteAt": int(0)}, + sq.Eq{"Users.DeleteAt": int(0)}, + sq.Eq{"Channels.Type": []string{"D", "G"}}, + }). + OrderBy("p.Id"). + Limit(uint64(limit)) - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var posts []*model.DirectPostForExport - if _, err = s.GetReplica().Select(&posts, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) - } - var channelIds []string - for _, post := range posts { - channelIds = append(channelIds, post.ChannelId) - } - 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.Eq{ - "cm.ChannelId": channelIds, - }) + var posts []*model.DirectPostForExport + if _, err = s.GetReplica().Select(&posts, queryString, args...); err != nil { + return nil, model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) + } + var channelIds []string + for _, post := range posts { + channelIds = append(channelIds, post.ChannelId) + } + 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.Eq{ + "cm.ChannelId": channelIds, + }) - queryString, args, err = query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err = query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.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("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.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("SqlPostStore.GetDirectPostParentsForExportAfter", "store.sql_post.get_direct_posts.app_error", nil, err.Error(), http.StatusInternalServerError) + } - // Build a map of channels and their posts - postsChannelMap := make(map[string][]*model.DirectPostForExport) - for _, post := range posts { - post.ChannelMembers = &[]string{} - postsChannelMap[post.ChannelId] = append(postsChannelMap[post.ChannelId], post) - } + // Build a map of channels and their posts + postsChannelMap := make(map[string][]*model.DirectPostForExport) + for _, post := range posts { + post.ChannelMembers = &[]string{} + postsChannelMap[post.ChannelId] = append(postsChannelMap[post.ChannelId], post) + } - // Build a map of channels and their members - channelMembersMap := make(map[string][]string) - for _, member := range channelMembers { - channelMembersMap[member.ChannelId] = append(channelMembersMap[member.ChannelId], member.Username) - } + // Build a map of channels and their members + channelMembersMap := make(map[string][]string) + for _, member := range channelMembers { + channelMembersMap[member.ChannelId] = append(channelMembersMap[member.ChannelId], member.Username) + } - // Populate each post ChannelMembers extracting it from the channelMembersMap - for channelId := range channelMembersMap { - for _, post := range postsChannelMap[channelId] { - *post.ChannelMembers = channelMembersMap[channelId] - } + // Populate each post ChannelMembers extracting it from the channelMembersMap + for channelId := range channelMembersMap { + for _, post := range postsChannelMap[channelId] { + *post.ChannelMembers = channelMembersMap[channelId] } - result.Data = posts - }) + } + return posts, nil } diff --git a/store/store.go b/store/store.go index d54c679914..df776130c1 100644 --- a/store/store.go +++ b/store/store.go @@ -240,7 +240,7 @@ type PostStore interface { GetMaxPostSize() int GetParentsForExportAfter(limit int, afterId string) StoreChannel GetRepliesForExport(parentId string) StoreChannel - GetDirectPostParentsForExportAfter(limit int, afterId string) StoreChannel + GetDirectPostParentsForExportAfter(limit int, afterId string) ([]*model.DirectPostForExport, *model.AppError) } type UserStore interface { diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index b9921e3158..ad2cda9497 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -108,19 +108,28 @@ func (_m *PostStore) Get(id string) (*model.PostList, *model.AppError) { } // GetDirectPostParentsForExportAfter provides a mock function with given fields: limit, afterId -func (_m *PostStore) GetDirectPostParentsForExportAfter(limit int, afterId string) store.StoreChannel { +func (_m *PostStore) GetDirectPostParentsForExportAfter(limit int, afterId string) ([]*model.DirectPostForExport, *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.DirectPostForExport + if rf, ok := ret.Get(0).(func(int, string) []*model.DirectPostForExport); ok { r0 = rf(limit, afterId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.DirectPostForExport) } } - 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 } // GetEtag provides a mock function with given fields: channelId, allowFromCache diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 4cc2016f8f..338bb128e1 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -2232,11 +2232,10 @@ func testPostStoreGetDirectPostParentsForExportAfter(t *testing.T, ss store.Stor p1.CreateAt = 1000 p1 = (<-ss.Post().Save(p1)).Data.(*model.Post) - r1 := <-ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectPostForExport) + r1, err := ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) - assert.Equal(t, p1.Message, d1[0].Message) + assert.Equal(t, p1.Message, r1[0].Message) // Manually truncate Channels table until testlib can handle cleanups s.GetMaster().Exec("TRUNCATE Channels") @@ -2292,15 +2291,14 @@ func testPostStoreGetDirectPostParentsForExportAfterDeleted(t *testing.T, ss sto *o1a = *p1 o1a.DeleteAt = 1 o1a.Message = p1.Message + "BBBBBBBBBB" - if _, err := ss.Post().Update(o1a, p1); err != nil { + if _, err = ss.Post().Update(o1a, p1); err != nil { t.Fatal(err) } - r1 := <-ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectPostForExport) + r1, err := ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) - assert.Equal(t, 0, len(d1)) + assert.Equal(t, 0, len(r1)) // Manually truncate Channels table until testlib can handle cleanups s.GetMaster().Exec("TRUNCATE Channels") @@ -2352,25 +2350,23 @@ func testPostStoreGetDirectPostParentsForExportAfterBatched(t *testing.T, ss sto sort.Slice(postIds, func(i, j int) bool { return postIds[i] < postIds[j] }) // Get all posts - r1 := <-ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 := r1.Data.([]*model.DirectPostForExport) - assert.Equal(t, len(postIds), len(d1)) + r1, err := ss.Post().GetDirectPostParentsForExportAfter(10000, strings.Repeat("0", 26)) + assert.Nil(t, err) + assert.Equal(t, len(postIds), len(r1)) var exportedPostIds []string - for i := range d1 { - exportedPostIds = append(exportedPostIds, d1[i].Id) + for i := range r1 { + exportedPostIds = append(exportedPostIds, r1[i].Id) } sort.Slice(exportedPostIds, func(i, j int) bool { return exportedPostIds[i] < exportedPostIds[j] }) assert.ElementsMatch(t, postIds, exportedPostIds) // Get 100 - r1 = <-ss.Post().GetDirectPostParentsForExportAfter(100, strings.Repeat("0", 26)) - assert.Nil(t, r1.Err) - d1 = r1.Data.([]*model.DirectPostForExport) - assert.Equal(t, 100, len(d1)) + r1, err = ss.Post().GetDirectPostParentsForExportAfter(100, strings.Repeat("0", 26)) + assert.Nil(t, err) + assert.Equal(t, 100, len(r1)) exportedPostIds = []string{} - for i := range d1 { - exportedPostIds = append(exportedPostIds, d1[i].Id) + for i := range r1 { + exportedPostIds = append(exportedPostIds, r1[i].Id) } sort.Slice(exportedPostIds, func(i, j int) bool { return exportedPostIds[i] < exportedPostIds[j] }) assert.ElementsMatch(t, postIds[:100], exportedPostIds)