From 0b6acaa9ba2d33e7b288edec0891d984cfd0f735 Mon Sep 17 00:00:00 2001 From: PR <45575072+therealpuneeth20@users.noreply.github.com> Date: Tue, 21 May 2019 11:01:30 -0700 Subject: [PATCH] GH-10761: Migrate Post.Get to sync by default (#10831) * GH-10761: Migrate Post.Get to sync by default * instantiate storeChannel with make * fix go vet issue * fix go vet issue * use select for checking channel for value * fix go vet shadow variable issue * addressing review comments * call post.Get() in goroutine * refactor minor values to fix tests * fix: remove err error type, it should be *model.AppErr --- app/file.go | 10 +- app/post.go | 38 ++--- store/sqlstore/post_store.go | 64 ++++---- store/store.go | 2 +- store/storetest/mocks/PostStore.go | 19 ++- store/storetest/post_store.go | 240 +++++++++++++++++++---------- store/storetest/reaction_store.go | 55 +++++-- 7 files changed, 272 insertions(+), 156 deletions(-) diff --git a/app/file.go b/app/file.go index 73759b4012..75f7b30909 100644 --- a/app/file.go +++ b/app/file.go @@ -267,17 +267,17 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo { fileMigrationLock.Lock() defer fileMigrationLock.Unlock() - result := <-a.Srv.Store.Post().Get(post.Id) - if result.Err != nil { - mlog.Error(fmt.Sprintf("Unable to get post when migrating post to use FileInfos, err=%v", result.Err), mlog.String("post_id", post.Id)) + result, err := a.Srv.Store.Post().Get(post.Id) + if err != nil { + mlog.Error(fmt.Sprintf("Unable to get post when migrating post to use FileInfos, err=%v", err), mlog.String("post_id", post.Id)) return []*model.FileInfo{} } - if newPost := result.Data.(*model.PostList).Posts[post.Id]; len(newPost.Filenames) != len(post.Filenames) { + if newPost := result.Posts[post.Id]; len(newPost.Filenames) != len(post.Filenames) { // Another thread has already created FileInfos for this post, so just return those fileInfos, err := a.Srv.Store.FileInfo().GetForPost(post.Id, true, false) if err != nil { - mlog.Error(fmt.Sprintf("Unable to get FileInfos for migrated post, err=%v", result.Err), mlog.String("post_id", post.Id)) + mlog.Error(fmt.Sprintf("Unable to get FileInfos for migrated post, err=%v", err), mlog.String("post_id", post.Id)) return []*model.FileInfo{} } diff --git a/app/post.go b/app/post.go index 9412c021b1..745f68b583 100644 --- a/app/post.go +++ b/app/post.go @@ -158,7 +158,12 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo var pchan store.StoreChannel if len(post.RootId) > 0 { - pchan = a.Srv.Store.Post().Get(post.RootId) + pchan = make(store.StoreChannel, 1) + go func() { + r, pErr := a.Srv.Store.Post().Get(post.RootId) + pchan <- store.StoreResult{Data: r, Err: pErr} + close(pchan) + }() } user, err := a.Srv.Store.User().Get(post.UserId) @@ -453,30 +458,30 @@ func (a *App) DeleteEphemeralPost(userId, postId string) { func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { post.SanitizeProps() - result := <-a.Srv.Store.Post().Get(post.Id) - if result.Err != nil { - return nil, result.Err + postLists, err := a.Srv.Store.Post().Get(post.Id) + if err != nil { + return nil, err } - oldPost := result.Data.(*model.PostList).Posts[post.Id] + oldPost := postLists.Posts[post.Id] if oldPost == nil { - err := model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id, http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id, http.StatusBadRequest) return nil, err } if oldPost.DeleteAt != 0 { - err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]interface{}{"PostId": post.Id}, "", http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]interface{}{"PostId": post.Id}, "", http.StatusBadRequest) return nil, err } if oldPost.IsSystemMessage() { - err := model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+post.Id, http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+post.Id, http.StatusBadRequest) return nil, err } if a.License() != nil { if *a.Config().ServiceSettings.PostEditTimeLimit != -1 && model.GetMillis() > oldPost.CreateAt+int64(*a.Config().ServiceSettings.PostEditTimeLimit*1000) && post.Message != oldPost.Message { - err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_time_limit.app_error", map[string]interface{}{"timeLimit": *a.Config().ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.permissions_time_limit.app_error", map[string]interface{}{"timeLimit": *a.Config().ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest) return nil, err } } @@ -528,7 +533,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model } } - result = <-a.Srv.Store.Post().Update(newPost, oldPost) + result := <-a.Srv.Store.Post().Update(newPost, oldPost) if result.Err != nil { return nil, result.Err } @@ -632,11 +637,7 @@ func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) { } func (a *App) GetPostThread(postId string) (*model.PostList, *model.AppError) { - result := <-a.Srv.Store.Post().Get(postId) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.PostList), nil + return a.Srv.Store.Post().Get(postId) } func (a *App) GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, *model.AppError) { @@ -664,11 +665,10 @@ func (a *App) GetFlaggedPostsForChannel(userId, channelId string, offset int, li } func (a *App) GetPermalinkPost(postId string, userId string) (*model.PostList, *model.AppError) { - result := <-a.Srv.Store.Post().Get(postId) - if result.Err != nil { - return nil, result.Err + list, err := a.Srv.Store.Post().Get(postId) + if err != nil { + return nil, err } - list := result.Data.(*model.PostList) if len(list.Order) != 1 { return nil, model.NewAppError("getPermalinkTmp", "api.post_get_post_by_id.get.app_error", nil, "", http.StatusNotFound) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index e56061ecf5..4664f26d17 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -271,49 +271,43 @@ func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offse }) } -func (s *SqlPostStore) Get(id string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - pl := model.NewPostList() +func (s *SqlPostStore) Get(id string) (*model.PostList, *model.AppError) { + pl := model.NewPostList() - if len(id) == 0 { - result.Err = model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id, http.StatusBadRequest) - return - } + if len(id) == 0 { + return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id, http.StatusBadRequest) + } - var post model.Post - err := s.GetReplica().SelectOne(&post, "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id}) - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound) - return - } + var post model.Post + err := s.GetReplica().SelectOne(&post, "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id}) + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound) + } - pl.AddPost(&post) - pl.AddOrder(id) + pl.AddPost(&post) + pl.AddOrder(id) - rootId := post.RootId + rootId := post.RootId - if rootId == "" { - rootId = post.Id - } + if rootId == "" { + rootId = post.Id + } - if len(rootId) == 0 { - result.Err = model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId, http.StatusInternalServerError) - return - } + if len(rootId) == 0 { + return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId, http.StatusInternalServerError) + } - var posts []*model.Post - _, err = s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId}) - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId+err.Error(), http.StatusInternalServerError) - return - } else { - for _, p := range posts { - pl.AddPost(p) - } - } + var posts []*model.Post + _, err = s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId}) + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId+err.Error(), http.StatusInternalServerError) + } - result.Data = pl - }) + for _, p := range posts { + pl.AddPost(p) + } + + return pl, nil } func (s *SqlPostStore) GetSingle(id string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 3dbcb41ce6..5479ec4875 100644 --- a/store/store.go +++ b/store/store.go @@ -212,7 +212,7 @@ type ChannelMemberHistoryStore interface { type PostStore interface { Save(post *model.Post) StoreChannel Update(newPost *model.Post, oldPost *model.Post) StoreChannel - Get(id string) StoreChannel + Get(id string) (*model.PostList, *model.AppError) GetSingle(id string) StoreChannel Delete(postId string, time int64, deleteByID string) *model.AppError PermanentDeleteByUser(userId string) StoreChannel diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index 8e7a133bfc..a515b3f436 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -83,19 +83,28 @@ func (_m *PostStore) Delete(postId string, time int64, deleteByID string) *model } // Get provides a mock function with given fields: id -func (_m *PostStore) Get(id string) store.StoreChannel { +func (_m *PostStore) Get(id string) (*model.PostList, *model.AppError) { ret := _m.Called(id) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.PostList + if rf, ok := ret.Get(0).(func(string) *model.PostList); ok { r0 = rf(id) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.PostList) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(id) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetDirectPostParentsForExportAfter provides a mock function with given fields: limit, afterId diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 5b970440ce..fb8064d185 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -128,19 +128,19 @@ func testPostStoreGet(t *testing.T, ss store.Store) { t.Fatal("Invalid Etag") } - if r1 := <-ss.Post().Get(o1.Id); r1.Err != nil { - t.Fatal(r1.Err) - } else { - if r1.Data.(*model.PostList).Posts[o1.Id].CreateAt != o1.CreateAt { - t.Fatal("invalid returned post") - } + r1, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + if r1.Posts[o1.Id].CreateAt != o1.CreateAt { + t.Fatal("invalid returned post") } - if err := (<-ss.Post().Get("123")).Err; err == nil { + if _, err = ss.Post().Get("123"); err == nil { t.Fatal("Missing id should have failed") } - if err := (<-ss.Post().Get("")).Err; err == nil { + if _, err = ss.Post().Get(""); err == nil { t.Fatal("should fail for blank post ids") } } @@ -221,9 +221,21 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { o3.Message = "zz" + model.NewId() + "QQQQQQQQQQ" o3 = (<-ss.Post().Save(o3)).Data.(*model.Post) - ro1 := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id] - ro2 := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o2.Id] - ro3 := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id] + r1, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro1 := r1.Posts[o1.Id] + r2, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro2 := r2.Posts[o2.Id] + r3, err := ss.Post().Get(o3.Id) + if err != nil { + t.Fatal(err) + } + ro3 := r3.Posts[o3.Id] if ro1.Message != o1.Message { t.Fatal("Failed to save/get") @@ -236,8 +248,12 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { t.Fatal(result.Err) } - ro1a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id] + r1, err = ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro1a := r1.Posts[o1.Id] if ro1a.Message != o1a.Message { t.Fatal("Failed to update/get") } @@ -249,7 +265,11 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { t.Fatal(result.Err) } - ro2a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o2.Id] + r2, err = ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro2a := r2.Posts[o2.Id] if ro2a.Message != o2a.Message { t.Fatal("Failed to update/get") @@ -262,7 +282,11 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { t.Fatal(result.Err) } - ro3a := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id] + r3, err = ss.Post().Get(o3.Id) + if err != nil { + t.Fatal(err) + } + ro3a := r3.Posts[o3.Id] if ro3a.Message != o3a.Message && ro3a.Hashtags != o3a.Hashtags { t.Fatal("Failed to update/get") @@ -275,7 +299,11 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { Filenames: []string{"test"}, })).(*model.Post) - ro4 := (<-ss.Post().Get(o4.Id)).Data.(*model.PostList).Posts[o4.Id] + r4, err := ss.Post().Get(o4.Id) + if err != nil { + t.Fatal(err) + } + ro4 := r4.Posts[o4.Id] o4a := &model.Post{} *o4a = *ro4 @@ -285,7 +313,12 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { t.Fatal(result.Err) } - if ro4a := store.Must(ss.Post().Get(o4.Id)).(*model.PostList).Posts[o4.Id]; len(ro4a.Filenames) != 0 { + r4, err = ss.Post().Get(o4.Id) + if err != nil { + t.Fatal(err) + } + + if ro4a := r4.Posts[o4.Id]; len(ro4a.Filenames) != 0 { t.Fatal("Failed to clear Filenames") } else if len(ro4a.FileIds) != 1 { t.Fatal("Failed to set FileIds") @@ -306,10 +339,10 @@ func testPostStoreDelete(t *testing.T, ss store.Store) { o1 = (<-ss.Post().Save(o1)).Data.(*model.Post) - if r1 := <-ss.Post().Get(o1.Id); r1.Err != nil { - t.Fatal(r1.Err) + if r1, err := ss.Post().Get(o1.Id); err != nil { + t.Fatal(err) } else { - if r1.Data.(*model.PostList).Posts[o1.Id].CreateAt != o1.CreateAt { + if r1.Posts[o1.Id].CreateAt != o1.CreateAt { t.Fatal("invalid returned post") } } @@ -325,8 +358,8 @@ func testPostStoreDelete(t *testing.T, ss store.Store) { t.Errorf("Expected (*Post).Props[model.POST_PROPS_DELETE_BY] to be %v but got %v.", deleteByID, actual) } - if r3 := (<-ss.Post().Get(o1.Id)); r3.Err == nil { - t.Log(r3.Data) + if r3, err := ss.Post().Get(o1.Id); err == nil { + t.Log(r3) t.Fatal("Missing id should have failed") } @@ -355,11 +388,11 @@ func testPostStoreDelete1Level(t *testing.T, ss store.Store) { t.Fatal(err) } - if r3 := (<-ss.Post().Get(o1.Id)); r3.Err == nil { + if _, err := ss.Post().Get(o1.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r4 := (<-ss.Post().Get(o2.Id)); r4.Err == nil { + if _, err := ss.Post().Get(o2.Id); err == nil { t.Fatal("Deleted id should have failed") } } @@ -397,20 +430,20 @@ func testPostStoreDelete2Level(t *testing.T, ss store.Store) { t.Fatal(err) } - if r3 := (<-ss.Post().Get(o1.Id)); r3.Err == nil { + if _, err := ss.Post().Get(o1.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r4 := (<-ss.Post().Get(o2.Id)); r4.Err == nil { + if _, err := ss.Post().Get(o2.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r5 := (<-ss.Post().Get(o3.Id)); r5.Err == nil { + if _, err := ss.Post().Get(o3.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r6 := <-ss.Post().Get(o4.Id); r6.Err != nil { - t.Fatal(r6.Err) + if _, err := ss.Post().Get(o4.Id); err != nil { + t.Fatal(err) } } @@ -439,11 +472,11 @@ func testPostStorePermDelete1Level(t *testing.T, ss store.Store) { t.Fatal(r2.Err) } - if r3 := (<-ss.Post().Get(o1.Id)); r3.Err != nil { + if _, err := ss.Post().Get(o1.Id); err != nil { t.Fatal("Deleted id shouldn't have failed") } - if r4 := (<-ss.Post().Get(o2.Id)); r4.Err == nil { + if _, err := ss.Post().Get(o2.Id); err == nil { t.Fatal("Deleted id should have failed") } @@ -451,7 +484,7 @@ func testPostStorePermDelete1Level(t *testing.T, ss store.Store) { t.Fatal(r2.Err) } - if r3 := (<-ss.Post().Get(o3.Id)); r3.Err == nil { + if _, err := ss.Post().Get(o3.Id); err == nil { t.Fatal("Deleted id should have failed") } } @@ -481,15 +514,15 @@ func testPostStorePermDelete1Level2(t *testing.T, ss store.Store) { t.Fatal(r2.Err) } - if r3 := (<-ss.Post().Get(o1.Id)); r3.Err == nil { + if _, err := ss.Post().Get(o1.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r4 := (<-ss.Post().Get(o2.Id)); r4.Err == nil { + if _, err := ss.Post().Get(o2.Id); err == nil { t.Fatal("Deleted id should have failed") } - if r5 := (<-ss.Post().Get(o3.Id)); r5.Err != nil { + if _, err := ss.Post().Get(o3.Id); err != nil { t.Fatal("Deleted id shouldn't have failed") } } @@ -517,39 +550,39 @@ func testPostStoreGetWithChildren(t *testing.T, ss store.Store) { o3.RootId = o1.Id o3 = (<-ss.Post().Save(o3)).Data.(*model.Post) - if r1 := <-ss.Post().Get(o1.Id); r1.Err != nil { - t.Fatal(r1.Err) - } else { - pl := r1.Data.(*model.PostList) - if len(pl.Posts) != 3 { - t.Fatal("invalid returned post") - } - } - - if err := ss.Post().Delete(o3.Id, model.GetMillis(), ""); err != nil { + pl, err := ss.Post().Get(o1.Id) + if err != nil { t.Fatal(err) } - if r2 := <-ss.Post().Get(o1.Id); r2.Err != nil { - t.Fatal(r2.Err) - } else { - pl := r2.Data.(*model.PostList) - if len(pl.Posts) != 2 { - t.Fatal("invalid returned post") - } + if len(pl.Posts) != 3 { + t.Fatal("invalid returned post") } - if err := ss.Post().Delete(o2.Id, model.GetMillis(), ""); err != nil { + if dErr := ss.Post().Delete(o3.Id, model.GetMillis(), ""); dErr != nil { + t.Fatal(dErr) + } + + pl, err = ss.Post().Get(o1.Id) + if err != nil { t.Fatal(err) } - if r3 := <-ss.Post().Get(o1.Id); r3.Err != nil { - t.Fatal(r3.Err) - } else { - pl := r3.Data.(*model.PostList) - if len(pl.Posts) != 1 { - t.Fatal("invalid returned post") - } + if len(pl.Posts) != 2 { + t.Fatal("invalid returned post") + } + + if dErr := ss.Post().Delete(o2.Id, model.GetMillis(), ""); dErr != nil { + t.Fatal(dErr) + } + + pl, err = ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + + if len(pl.Posts) != 1 { + t.Fatal("invalid returned post") } } @@ -1627,9 +1660,21 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { o3.Message = "zz" + model.NewId() + "QQQQQQQQQQ" o3 = (<-ss.Post().Save(o3)).Data.(*model.Post) - ro1 := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id] - ro2 := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o2.Id] - ro3 := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id] + r1, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro1 := r1.Posts[o1.Id] + r2, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro2 := r2.Posts[o2.Id] + r3, err := ss.Post().Get(o3.Id) + if err != nil { + t.Fatal(err) + } + ro3 := r3.Posts[o3.Id] if ro1.Message != o1.Message { t.Fatal("Failed to save/get") @@ -1638,11 +1683,16 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { o1a := &model.Post{} *o1a = *ro1 o1a.Message = ro1.Message + "BBBBBBBBBB" - if _, err := ss.Post().Overwrite(o1a); err != nil { + _, err = ss.Post().Overwrite(o1a) + if err != nil { t.Fatal(err) } - ro1a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id] + r1, err = ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro1a := r1.Posts[o1.Id] if ro1a.Message != o1a.Message { t.Fatal("Failed to overwrite/get") @@ -1651,11 +1701,16 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { o2a := &model.Post{} *o2a = *ro2 o2a.Message = ro2.Message + "DDDDDDD" - if _, err := ss.Post().Overwrite(o2a); err != nil { + _, err = ss.Post().Overwrite(o2a) + if err != nil { t.Fatal(err) } - ro2a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o2.Id] + r2, err = ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro2a := r2.Posts[o2.Id] if ro2a.Message != o2a.Message { t.Fatal("Failed to overwrite/get") @@ -1664,11 +1719,16 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { o3a := &model.Post{} *o3a = *ro3 o3a.Message = ro3.Message + "WWWWWWW" - if _, err := ss.Post().Overwrite(o3a); err != nil { + _, err = ss.Post().Overwrite(o3a) + if err != nil { t.Fatal(err) } - ro3a := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id] + r3, err = ss.Post().Get(o3.Id) + if err != nil { + t.Fatal(err) + } + ro3a := r3.Posts[o3.Id] if ro3a.Message != o3a.Message && ro3a.Hashtags != o3a.Hashtags { t.Fatal("Failed to overwrite/get") @@ -1681,17 +1741,29 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { Filenames: []string{"test"}, })).(*model.Post) - ro4 := (<-ss.Post().Get(o4.Id)).Data.(*model.PostList).Posts[o4.Id] + r4, err := ss.Post().Get(o4.Id) + if err != nil { + t.Fatal(err) + } + ro4 := r4.Posts[o4.Id] o4a := &model.Post{} *o4a = *ro4 o4a.Filenames = []string{} o4a.FileIds = []string{model.NewId()} - if _, err := ss.Post().Overwrite(o4a); err != nil { + _, err = ss.Post().Overwrite(o4a) + if err != nil { t.Fatal(err) } - if ro4a := store.Must(ss.Post().Get(o4.Id)).(*model.PostList).Posts[o4.Id]; len(ro4a.Filenames) != 0 { + r4, err = ss.Post().Get(o4.Id) + if err != nil { + t.Fatal(err) + } + + ro4a := r4.Posts[o4.Id] + + if len(ro4a.Filenames) != 0 { t.Fatal("Failed to clear Filenames") } else if len(ro4a.FileIds) != 1 { t.Fatal("Failed to set FileIds") @@ -1717,9 +1789,21 @@ func testPostStoreGetPostsByIds(t *testing.T, ss store.Store) { o3.Message = "zz" + model.NewId() + "QQQQQQQQQQ" o3 = (<-ss.Post().Save(o3)).Data.(*model.Post) - ro1 := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id] - ro2 := (<-ss.Post().Get(o2.Id)).Data.(*model.PostList).Posts[o2.Id] - ro3 := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id] + r1, err := ss.Post().Get(o1.Id) + if err != nil { + t.Fatal(err) + } + ro1 := r1.Posts[o1.Id] + r2, err := ss.Post().Get(o2.Id) + if err != nil { + t.Fatal(err) + } + ro2 := r2.Posts[o2.Id] + r3, err := ss.Post().Get(o3.Id) + if err != nil { + t.Fatal(err) + } + ro3 := r3.Posts[o3.Id] postIds := []string{ ro1.Id, @@ -1831,16 +1915,16 @@ func testPostStorePermanentDeleteBatch(t *testing.T, ss store.Store) { store.Must(ss.Post().PermanentDeleteBatch(2000, 1000)) - if p := <-ss.Post().Get(o1.Id); p.Err == nil { + if _, err := ss.Post().Get(o1.Id); err == nil { t.Fatalf("Should have not found post 1 after purge") } - if p := <-ss.Post().Get(o2.Id); p.Err == nil { + if _, err := ss.Post().Get(o2.Id); err == nil { t.Fatalf("Should have not found post 2 after purge") } - if p := <-ss.Post().Get(o3.Id); p.Err != nil { - t.Fatalf("Should have found post 3 after purge") + if _, err := ss.Post().Get(o3.Id); err != nil { + t.Fatalf("Should have not found post 3 after purge") } } diff --git a/store/storetest/reaction_store.go b/store/storetest/reaction_store.go index e3720ac6bd..16e10ae182 100644 --- a/store/storetest/reaction_store.go +++ b/store/storetest/reaction_store.go @@ -40,7 +40,11 @@ func testReactionSave(t *testing.T, ss store.Store) { } var secondUpdateAt int64 - if postList := store.Must(ss.Post().Get(reaction1.PostId)).(*model.PostList); !postList.Posts[post.Id].HasReactions { + postList, err := ss.Post().Get(reaction1.PostId) + if err != nil { + t.Fatal(err) + } + if !postList.Posts[post.Id].HasReactions { t.Fatal("should've set HasReactions = true on post") } else if postList.Posts[post.Id].UpdateAt == firstUpdateAt { t.Fatal("should've marked post as updated when HasReactions changed") @@ -48,7 +52,7 @@ func testReactionSave(t *testing.T, ss store.Store) { secondUpdateAt = postList.Posts[post.Id].UpdateAt } - if _, err := ss.Reaction().Save(reaction1); err != nil { + if _, err = ss.Reaction().Save(reaction1); err != nil { t.Log(err) t.Fatal("should've allowed saving a duplicate reaction") } @@ -59,11 +63,16 @@ func testReactionSave(t *testing.T, ss store.Store) { PostId: reaction1.PostId, EmojiName: reaction1.EmojiName, } - if _, err := ss.Reaction().Save(reaction2); err != nil { + if _, err = ss.Reaction().Save(reaction2); err != nil { t.Fatal(err) } - if postList := store.Must(ss.Post().Get(reaction2.PostId)).(*model.PostList); postList.Posts[post.Id].UpdateAt == secondUpdateAt { + postList, err = ss.Post().Get(reaction2.PostId) + if err != nil { + t.Fatal(err) + } + + if postList.Posts[post.Id].UpdateAt == secondUpdateAt { t.Fatal("should've marked post as updated even if HasReactions doesn't change") } @@ -111,19 +120,26 @@ func testReactionDelete(t *testing.T, ss store.Store) { _, err := ss.Reaction().Save(reaction) require.Nil(t, err) - firstUpdateAt := store.Must(ss.Post().Get(reaction.PostId)).(*model.PostList).Posts[post.Id].UpdateAt + result, err := ss.Post().Get(reaction.PostId) + if err != nil { + t.Fatal(err) + } + firstUpdateAt := result.Posts[post.Id].UpdateAt - if _, err := ss.Reaction().Delete(reaction); err != nil { + if _, err = ss.Reaction().Delete(reaction); err != nil { t.Fatal(err) } - if reactions, err := ss.Reaction().GetForPost(post.Id, false); err != nil { - t.Fatal(err) + if reactions, rErr := ss.Reaction().GetForPost(post.Id, false); rErr != nil { + t.Fatal(rErr) } else if len(reactions) != 0 { t.Fatal("should've deleted reaction") } - - if postList := store.Must(ss.Post().Get(post.Id)).(*model.PostList); postList.Posts[post.Id].HasReactions { + postList, err := ss.Post().Get(post.Id) + if err != nil { + t.Fatal(err) + } + if postList.Posts[post.Id].HasReactions { t.Fatal("should've set HasReactions = false on post") } else if postList.Posts[post.Id].UpdateAt == firstUpdateAt { t.Fatal("should mark post as updated after deleting reactions") @@ -294,15 +310,28 @@ func testReactionDeleteAllWithEmojiName(t *testing.T, ss store.Store) { } // check that the posts are updated - if postList := store.Must(ss.Post().Get(post.Id)).(*model.PostList); !postList.Posts[post.Id].HasReactions { + postList, err := ss.Post().Get(post.Id) + if err != nil { + t.Fatal(err) + } + if !postList.Posts[post.Id].HasReactions { t.Fatal("post should still have reactions") } - if postList := store.Must(ss.Post().Get(post2.Id)).(*model.PostList); !postList.Posts[post2.Id].HasReactions { + postList, err = ss.Post().Get(post2.Id) + if err != nil { + t.Fatal(err) + } + if !postList.Posts[post2.Id].HasReactions { t.Fatal("post should still have reactions") } - if postList := store.Must(ss.Post().Get(post3.Id)).(*model.PostList); postList.Posts[post3.Id].HasReactions { + postList, err = ss.Post().Get(post3.Id) + if err != nil { + t.Fatal(err) + } + + if postList.Posts[post3.Id].HasReactions { t.Fatal("post shouldn't have reactions any more") } }