From a57e042dab372219008f5a99711ea80fa66facc8 Mon Sep 17 00:00:00 2001 From: GianOrtiz Date: Tue, 4 Jun 2019 18:24:27 -0300 Subject: [PATCH] [MM-15858] Migrate "Post.GetPostsCreatedAt" to Sync by default (#11021) * Migrate "Post.GetPostsCreatedAt" to Sync by default * Fix variable misuse --- app/import_functions.go | 21 ++++---- app/import_functions_test.go | 86 ++++++++++++------------------ store/sqlstore/post_store.go | 19 +++---- store/store.go | 2 +- store/storetest/mocks/PostStore.go | 19 +++++-- store/storetest/post_store.go | 6 +-- 6 files changed, 68 insertions(+), 85 deletions(-) diff --git a/app/import_functions.go b/app/import_functions.go index 0b59c41441..73503e7200 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -861,11 +861,10 @@ func (a *App) ImportReply(data *ReplyImportData, post *model.Post, teamId string user := result.Data.(*model.User) // Check if this post already exists. - result = <-a.Srv.Store.Post().GetPostsCreatedAt(post.ChannelId, *data.CreateAt) - if result.Err != nil { - return result.Err + replies, err := a.Srv.Store.Post().GetPostsCreatedAt(post.ChannelId, *data.CreateAt) + if err != nil { + return err } - replies := result.Data.([]*model.Post) var reply *model.Post for _, r := range replies { @@ -962,11 +961,10 @@ func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError { user := result.Data.(*model.User) // Check if this post already exists. - result = <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) - if result.Err != nil { - return result.Err + posts, err := a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) + if err != nil { + return err } - posts := result.Data.([]*model.Post) var post *model.Post for _, p := range posts { @@ -1186,11 +1184,10 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A user := result.Data.(*model.User) // Check if this post already exists. - result = <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) - if result.Err != nil { - return result.Err + posts, err := a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) + if err != nil { + return err } - posts := result.Data.([]*model.Post) var post *model.Post for _, p := range posts { diff --git a/app/import_functions_test.go b/app/import_functions_test.go index d91213fd7c..3d8d0b33b9 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -1891,10 +1891,10 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, team.Id) // Check the post values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time); result.Err != nil { - t.Fatal(result.Err.Error()) + posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time) + if err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -1917,10 +1917,10 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, team.Id) // Check the post values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time); result.Err != nil { - t.Fatal(result.Err.Error()) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time) + if err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -1968,10 +1968,10 @@ func TestImportImportPost(t *testing.T) { assert.Nil(t, err) AssertAllPostsCount(t, th.App, initialPostCount, 4, team.Id) - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, hashtagTime); result.Err != nil { - t.Fatal(result.Err.Error()) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, hashtagTime) + if err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -2013,10 +2013,9 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 5, team.Id) // Check the post values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, flagsTime); result.Err != nil { - t.Fatal(result.Err.Error()) + if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, flagsTime); err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -2050,10 +2049,9 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 6, team.Id) // Check the post values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, reactionPostTime); result.Err != nil { - t.Fatal(result.Err.Error()) + if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, reactionPostTime); err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -2090,10 +2088,9 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 8, team.Id) // Check the post values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyPostTime); result.Err != nil { - t.Fatal(result.Err.Error()) + if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyPostTime); err != nil { + t.Fatal(err.Error()) } else { - posts := result.Data.([]*model.Post) if len(posts) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -2103,10 +2100,9 @@ func TestImportImportPost(t *testing.T) { } // Check the reply values. - if result := <-th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyTime); result.Err != nil { - t.Fatal(result.Err.Error()) + if replies, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyTime); err != nil { + t.Fatal(err.Error()) } else { - replies := result.Data.([]*model.Post) if len(replies) != 1 { t.Fatal("Unexpected number of posts found.") } @@ -2434,10 +2430,8 @@ func TestImportImportDirectPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, "") // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts := result.Data.([]*model.Post) + posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post := posts[0] @@ -2451,10 +2445,8 @@ func TestImportImportDirectPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, "") // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2481,10 +2473,8 @@ func TestImportImportDirectPost(t *testing.T) { require.Nil(t, err) AssertAllPostsCount(t, th.App, initialPostCount, 4, "") - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2512,10 +2502,8 @@ func TestImportImportDirectPost(t *testing.T) { require.Nil(t, err) // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(directChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2613,10 +2601,8 @@ func TestImportImportDirectPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, "") // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2630,10 +2616,8 @@ func TestImportImportDirectPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 1, "") // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2660,10 +2644,8 @@ func TestImportImportDirectPost(t *testing.T) { require.Nil(t, err) AssertAllPostsCount(t, th.App, initialPostCount, 4, "") - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] @@ -2692,10 +2674,8 @@ func TestImportImportDirectPost(t *testing.T) { require.Nil(t, err) // Check the post values. - result = <-th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) - require.Nil(t, result.Err) - - posts = result.Data.([]*model.Post) + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(groupChannel.Id, *data.CreateAt) + require.Nil(t, err) require.Equal(t, len(posts), 1) post = posts[0] diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index b50201edef..af7b35c7dc 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -1093,19 +1093,16 @@ func (s *SqlPostStore) AnalyticsPostCount(teamId string, mustHaveFile bool, must }) } -func (s *SqlPostStore) GetPostsCreatedAt(channelId string, time int64) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := `SELECT * FROM Posts WHERE CreateAt = :CreateAt AND ChannelId = :ChannelId` +func (s *SqlPostStore) GetPostsCreatedAt(channelId string, time int64) ([]*model.Post, *model.AppError) { + query := `SELECT * FROM Posts WHERE CreateAt = :CreateAt AND ChannelId = :ChannelId` - var posts []*model.Post - _, err := s.GetReplica().Select(&posts, query, map[string]interface{}{"CreateAt": time, "ChannelId": channelId}) + var posts []*model.Post + _, err := s.GetReplica().Select(&posts, query, map[string]interface{}{"CreateAt": time, "ChannelId": channelId}) - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetPostsCreatedAt", "store.sql_post.get_posts_created_att.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError) - } else { - result.Data = posts - } - }) + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetPostsCreatedAt", "store.sql_post.get_posts_created_att.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError) + } + return posts, nil } func (s *SqlPostStore) GetPostsByIds(postIds []string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 993326fd0d..aac4177619 100644 --- a/store/store.go +++ b/store/store.go @@ -231,7 +231,7 @@ type PostStore interface { AnalyticsPostCount(teamId string, mustHaveFile bool, mustHaveHashtag bool) StoreChannel ClearCaches() InvalidateLastPostTimeCache(channelId string) - GetPostsCreatedAt(channelId string, time int64) StoreChannel + GetPostsCreatedAt(channelId string, time int64) ([]*model.Post, *model.AppError) Overwrite(post *model.Post) (*model.Post, *model.AppError) GetPostsByIds(postIds []string) StoreChannel GetPostsBatchForIndexing(startTime int64, endTime int64, limit int) StoreChannel diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index c1283e2ed0..b5186a1346 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -366,19 +366,28 @@ func (_m *PostStore) GetPostsByIds(postIds []string) store.StoreChannel { } // GetPostsCreatedAt provides a mock function with given fields: channelId, time -func (_m *PostStore) GetPostsCreatedAt(channelId string, time int64) store.StoreChannel { +func (_m *PostStore) GetPostsCreatedAt(channelId string, time int64) ([]*model.Post, *model.AppError) { ret := _m.Called(channelId, time) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, int64) store.StoreChannel); ok { + var r0 []*model.Post + if rf, ok := ret.Get(0).(func(string, int64) []*model.Post); ok { r0 = rf(channelId, time) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.Post) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, int64) *model.AppError); ok { + r1 = rf(channelId, time) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetPostsSince provides a mock function with given fields: channelId, time, allowFromCache diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index e818a33e9b..ccb20fe3fd 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -351,8 +351,8 @@ func testPostStoreDelete(t *testing.T, ss store.Store) { t.Fatal(err) } - r5 := <-ss.Post().GetPostsCreatedAt(o1.ChannelId, o1.CreateAt) - post := r5.Data.([]*model.Post)[0] + posts, _ := ss.Post().GetPostsCreatedAt(o1.ChannelId, o1.CreateAt) + post := posts[0] actual := post.Props[model.POST_PROPS_DELETE_BY] if actual != deleteByID { t.Errorf("Expected (*Post).Props[model.POST_PROPS_DELETE_BY] to be %v but got %v.", deleteByID, actual) @@ -1759,7 +1759,7 @@ func testPostStoreGetPostsCreatedAt(t *testing.T, ss store.Store) { o3.CreateAt = createTime _ = (<-ss.Post().Save(o3)).Data.(*model.Post) - r1 := (<-ss.Post().GetPostsCreatedAt(o1.ChannelId, createTime)).Data.([]*model.Post) + r1, _ := ss.Post().GetPostsCreatedAt(o1.ChannelId, createTime) assert.Equal(t, 2, len(r1)) }