From 9a9d5d4081809a8fa2d0dec3b2e571322d981c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 30 Apr 2019 21:34:26 +0200 Subject: [PATCH] Migrate Reactions store to Sync by default (#10737) * Migrate Reactions store to Sync by default * Fixing tests * Fixing tests * Fixing govet * fixing tests * Addressing PR review comments --- api4/reaction_test.go | 12 +- app/emoji.go | 4 +- app/export.go | 11 +- app/import_functions.go | 4 +- app/import_functions_test.go | 4 +- app/reaction.go | 28 ++-- store/layered_store.go | 36 ++--- store/layered_store_supplier.go | 12 +- store/local_cache_supplier_reactions.go | 23 ++-- store/redis_supplier_reactions.go | 30 +++-- store/sqlstore/supplier_reactions.go | 124 +++++++----------- store/store.go | 12 +- .../mocks/LayeredStoreDatabaseLayer.go | 103 ++++++++++----- store/storetest/mocks/LayeredStoreSupplier.go | 103 ++++++++++----- store/storetest/mocks/ReactionStore.go | 104 ++++++++++----- store/storetest/reaction_store.go | 99 ++++++++------ 16 files changed, 408 insertions(+), 301 deletions(-) diff --git a/api4/reaction_test.go b/api4/reaction_test.go index ab3f5a2408..57cb04dc39 100644 --- a/api4/reaction_test.go +++ b/api4/reaction_test.go @@ -263,10 +263,10 @@ func TestGetReactions(t *testing.T) { var reactions []*model.Reaction for _, userReaction := range userReactions { - if result := <-th.App.Srv.Store.Reaction().Save(userReaction); result.Err != nil { - t.Fatal(result.Err) + if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil { + t.Fatal(err) } else { - reactions = append(reactions, result.Data.(*model.Reaction)) + reactions = append(reactions, reaction) } } @@ -618,10 +618,10 @@ func TestGetBulkReactions(t *testing.T) { for _, userReaction := range userReactions { reactions := expectedPostIdsReactionsMap[userReaction.PostId] - if result := <-th.App.Srv.Store.Reaction().Save(userReaction); result.Err != nil { - t.Fatal(result.Err) + if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil { + t.Fatal(err) } else { - reactions = append(reactions, result.Data.(*model.Reaction)) + reactions = append(reactions, reaction) } expectedPostIdsReactionsMap[userReaction.PostId] = reactions diff --git a/app/emoji.go b/app/emoji.go index ac8d5a07a0..e81ad109d9 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -292,8 +292,8 @@ func (a *App) deleteEmojiImage(id string) { } func (a *App) deleteReactionsForEmoji(emojiName string) { - if result := <-a.Srv.Store.Reaction().DeleteAllWithEmojiName(emojiName); result.Err != nil { + if err := a.Srv.Store.Reaction().DeleteAllWithEmojiName(emojiName); err != nil { mlog.Warn(fmt.Sprintf("Unable to delete reactions when deleting emoji with emoji name %v", emojiName)) - mlog.Warn(fmt.Sprint(result.Err)) + mlog.Warn(fmt.Sprint(err)) } } diff --git a/app/export.go b/app/export.go index 2190027c88..8e0e686c9d 100644 --- a/app/export.go +++ b/app/export.go @@ -407,15 +407,14 @@ func (a *App) buildPostReplies(postId string) (*[]ReplyImportData, *model.AppErr func (a *App) BuildPostReactions(postId string) (*[]ReactionImportData, *model.AppError) { var reactionsOfPost []ReactionImportData - result := <-a.Srv.Store.Reaction().GetForPost(postId, true) - if result.Err != nil { - return nil, result.Err + reactions, err := a.Srv.Store.Reaction().GetForPost(postId, true) + if err != nil { + return nil, err } - reactions := result.Data.([]*model.Reaction) - for _, reaction := range reactions { - user, err := a.Srv.Store.User().Get(reaction.UserId) + var user *model.User + user, err = a.Srv.Store.User().Get(reaction.UserId) if err != nil { return nil, err } diff --git a/app/import_functions.go b/app/import_functions.go index 037402b5af..fdbf7c5b82 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -843,8 +843,8 @@ func (a *App) ImportReaction(data *ReactionImportData, post *model.Post, dryRun EmojiName: *data.EmojiName, CreateAt: *data.CreateAt, } - if result := <-a.Srv.Store.Reaction().Save(reaction); result.Err != nil { - return result.Err + if _, err := a.Srv.Store.Reaction().Save(reaction); err != nil { + return err } return nil } diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 43ef3f4d09..dfecf618f4 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -2089,9 +2089,9 @@ func TestImportImportPost(t *testing.T) { t.Fatal("Post properties not as expected") } - if result := <-th.App.Srv.Store.Reaction().GetForPost(post.Id, false); result.Err != nil { + if reactions, err := th.App.Srv.Store.Reaction().GetForPost(post.Id, false); err != nil { t.Fatal("Can't get reaction") - } else if len(result.Data.([]*model.Reaction)) != 1 { + } else if len(reactions) != 1 { t.Fatal("Invalid number of reactions") } } diff --git a/app/reaction.go b/app/reaction.go index c447fa8b5e..fb5f7bf57f 100644 --- a/app/reaction.go +++ b/app/reaction.go @@ -25,7 +25,8 @@ func (a *App) SaveReactionForPost(reaction *model.Reaction) (*model.Reaction, *m } if a.License() != nil && *a.Config().TeamSettings.ExperimentalTownSquareIsReadOnly && channel.Name == model.DEFAULT_CHANNEL { - user, err := a.GetUser(reaction.UserId) + var user *model.User + user, err = a.GetUser(reaction.UserId) if err != nil { return nil, err } @@ -35,13 +36,11 @@ func (a *App) SaveReactionForPost(reaction *model.Reaction) (*model.Reaction, *m } } - result := <-a.Srv.Store.Reaction().Save(reaction) - if result.Err != nil { - return nil, result.Err + reaction, err = a.Srv.Store.Reaction().Save(reaction) + if err != nil { + return nil, err } - reaction = result.Data.(*model.Reaction) - // The post is always modified since the UpdateAt always changes a.InvalidateCacheForChannelPosts(post.ChannelId) @@ -53,22 +52,17 @@ func (a *App) SaveReactionForPost(reaction *model.Reaction) (*model.Reaction, *m } func (a *App) GetReactionsForPost(postId string) ([]*model.Reaction, *model.AppError) { - result := <-a.Srv.Store.Reaction().GetForPost(postId, true) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.Reaction), nil + return a.Srv.Store.Reaction().GetForPost(postId, true) } func (a *App) GetBulkReactionsForPosts(postIds []string) (map[string][]*model.Reaction, *model.AppError) { reactions := make(map[string][]*model.Reaction) - result := <-a.Srv.Store.Reaction().BulkGetForPosts(postIds) - if result.Err != nil { - return nil, result.Err + allReactions, err := a.Srv.Store.Reaction().BulkGetForPosts(postIds) + if err != nil { + return nil, err } - allReactions := result.Data.([]*model.Reaction) for _, reaction := range allReactions { reactionsForPost := reactions[reaction.PostId] reactionsForPost = append(reactionsForPost, reaction) @@ -120,8 +114,8 @@ func (a *App) DeleteReactionForPost(reaction *model.Reaction) *model.AppError { hasReactions = false } - if result := <-a.Srv.Store.Reaction().Delete(reaction); result.Err != nil { - return result.Err + if _, err := a.Srv.Store.Reaction().Delete(reaction); err != nil { + return err } // The post is always modified since the UpdateAt always changes diff --git a/store/layered_store.go b/store/layered_store.go index bf008f20b1..a3d1f23a0b 100644 --- a/store/layered_store.go +++ b/store/layered_store.go @@ -232,40 +232,28 @@ type LayeredReactionStore struct { *LayeredStore } -func (s *LayeredReactionStore) Save(reaction *model.Reaction) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionSave(s.TmpContext, reaction) - }) +func (s *LayeredReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) { + return s.LayerChainHead.ReactionSave(s.TmpContext, reaction) } -func (s *LayeredReactionStore) Delete(reaction *model.Reaction) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionDelete(s.TmpContext, reaction) - }) +func (s *LayeredReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) { + return s.LayerChainHead.ReactionDelete(s.TmpContext, reaction) } -func (s *LayeredReactionStore) GetForPost(postId string, allowFromCache bool) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionGetForPost(s.TmpContext, postId) - }) +func (s *LayeredReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) { + return s.LayerChainHead.ReactionGetForPost(s.TmpContext, postId) } -func (s *LayeredReactionStore) BulkGetForPosts(postIds []string) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionsBulkGetForPosts(s.TmpContext, postIds) - }) +func (s *LayeredReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) { + return s.LayerChainHead.ReactionsBulkGetForPosts(s.TmpContext, postIds) } -func (s *LayeredReactionStore) DeleteAllWithEmojiName(emojiName string) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionDeleteAllWithEmojiName(s.TmpContext, emojiName) - }) +func (s *LayeredReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError { + return s.LayerChainHead.ReactionDeleteAllWithEmojiName(s.TmpContext, emojiName) } -func (s *LayeredReactionStore) PermanentDeleteBatch(endTime int64, limit int64) StoreChannel { - return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult { - return supplier.ReactionPermanentDeleteBatch(s.TmpContext, endTime, limit) - }) +func (s *LayeredReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { + return s.LayerChainHead.ReactionPermanentDeleteBatch(s.TmpContext, endTime, limit) } type LayeredRoleStore struct { diff --git a/store/layered_store_supplier.go b/store/layered_store_supplier.go index f263d2695d..aa33441187 100644 --- a/store/layered_store_supplier.go +++ b/store/layered_store_supplier.go @@ -24,12 +24,12 @@ type LayeredStoreSupplier interface { // // Reactions //), hints ...LayeredStoreHint) - ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult - ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult - ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult - ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult - ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) *LayeredStoreSupplierResult - ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult + ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) + ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) + ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) + ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError + ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) + ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) // Roles RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) *LayeredStoreSupplierResult diff --git a/store/local_cache_supplier_reactions.go b/store/local_cache_supplier_reactions.go index 260d20e9af..d9af607d95 100644 --- a/store/local_cache_supplier_reactions.go +++ b/store/local_cache_supplier_reactions.go @@ -17,42 +17,47 @@ func (s *LocalCacheSupplier) handleClusterInvalidateReaction(msg *model.ClusterM } } -func (s *LocalCacheSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { defer s.doInvalidateCacheCluster(s.reactionCache, reaction.PostId) return s.Next().ReactionSave(ctx, reaction, hints...) } -func (s *LocalCacheSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { defer s.doInvalidateCacheCluster(s.reactionCache, reaction.PostId) return s.Next().ReactionDelete(ctx, reaction, hints...) } -func (s *LocalCacheSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { if result := s.doStandardReadCache(ctx, s.reactionCache, postId, hints...); result != nil { - return result + return result.Data.([]*model.Reaction), nil } - result := s.Next().ReactionGetForPost(ctx, postId, hints...) + reaction, err := s.Next().ReactionGetForPost(ctx, postId, hints...) + if err != nil { + return nil, err + } + result := NewSupplierResult() + result.Data = reaction s.doStandardAddToCache(ctx, s.reactionCache, postId, result, hints...) - return result + return reaction, nil } -func (s *LocalCacheSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError { // This could be improved. Right now we just clear the whole // cache because we don't have a way find what post Ids have this emoji name. defer s.doClearCacheCluster(s.reactionCache) return s.Next().ReactionDeleteAllWithEmojiName(ctx, emojiName, hints...) } -func (s *LocalCacheSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) { // Don't bother to clear the cache as the posts will be gone anyway and the reactions being deleted will // expire from the cache in due course. return s.Next().ReactionPermanentDeleteBatch(ctx, endTime, limit) } -func (s *LocalCacheSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *LocalCacheSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { // Ignoring this. return s.Next().ReactionsBulkGetForPosts(ctx, postIds, hints...) } diff --git a/store/redis_supplier_reactions.go b/store/redis_supplier_reactions.go index eccd87ed8c..b4b4b42645 100644 --- a/store/redis_supplier_reactions.go +++ b/store/redis_supplier_reactions.go @@ -10,52 +10,54 @@ import ( "github.com/mattermost/mattermost-server/model" ) -func (s *RedisSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *RedisSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { if err := s.client.Del("reactions:" + reaction.PostId).Err(); err != nil { mlog.Error("Redis failed to remove key reactions:" + reaction.PostId + " Error: " + err.Error()) } return s.Next().ReactionSave(ctx, reaction, hints...) } -func (s *RedisSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *RedisSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { if err := s.client.Del("reactions:" + reaction.PostId).Err(); err != nil { mlog.Error("Redis failed to remove key reactions:" + reaction.PostId + " Error: " + err.Error()) } return s.Next().ReactionDelete(ctx, reaction, hints...) } -func (s *RedisSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { - var resultdata []*model.Reaction - found, err := s.load("reactions:"+postId, &resultdata) +func (s *RedisSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { + var reaction []*model.Reaction + found, err := s.load("reactions:"+postId, &reaction) if found { - result := NewSupplierResult() - result.Data = resultdata - return result + return reaction, nil } + if err != nil { mlog.Error("Redis encountered an error on read: " + err.Error()) } - result := s.Next().ReactionGetForPost(ctx, postId, hints...) + reaction, appErr := s.Next().ReactionGetForPost(ctx, postId, hints...) + if appErr != nil { + return nil, appErr + } - if err := s.save("reactions:"+postId, result.Data, REDIS_EXPIRY_TIME); err != nil { + if err := s.save("reactions:"+postId, reaction, REDIS_EXPIRY_TIME); err != nil { mlog.Error("Redis encountered and error on write: " + err.Error()) } - return result + return reaction, nil } -func (s *RedisSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *RedisSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError { // Ignoring this. It's probably OK to have the emoji slowly expire from Redis. return s.Next().ReactionDeleteAllWithEmojiName(ctx, emojiName, hints...) } -func (s *RedisSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *RedisSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) { // Ignoring this. It's probably OK to have the emoji slowly expire from Redis. return s.Next().ReactionPermanentDeleteBatch(ctx, endTime, limit, hints...) } -func (s *RedisSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult { +func (s *RedisSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { // Ignoring this. return s.Next().ReactionsBulkGetForPosts(ctx, postIds, hints...) } diff --git a/store/sqlstore/supplier_reactions.go b/store/sqlstore/supplier_reactions.go index 9095c7aa58..77f076feb1 100644 --- a/store/sqlstore/supplier_reactions.go +++ b/store/sqlstore/supplier_reactions.go @@ -23,63 +23,52 @@ func initSqlSupplierReactions(sqlStore SqlStore) { } } -func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() - +func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { reaction.PreSave() - if result.Err = reaction.IsValid(); result.Err != nil { - return result + if err := reaction.IsValid(); err != nil { + return nil, err } - if transaction, err := s.GetMaster().Begin(); err != nil { - result.Err = model.NewAppError("SqlReactionStore.Save", "store.sql_reaction.save.begin.app_error", nil, err.Error(), http.StatusInternalServerError) + transaction, err := s.GetMaster().Begin() + if err != nil { + return nil, model.NewAppError("SqlReactionStore.Save", "store.sql_reaction.save.begin.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer finalizeTransaction(transaction) + appErr := saveReactionAndUpdatePost(transaction, reaction) + if appErr != nil { + // We don't consider duplicated save calls as an error + if !IsUniqueConstraintError(appErr, []string{"reactions_pkey", "PRIMARY"}) { + return nil, model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.save.app_error", nil, appErr.Error(), http.StatusBadRequest) + } } else { - defer finalizeTransaction(transaction) - err := saveReactionAndUpdatePost(transaction, reaction) - - if err != nil { - // We don't consider duplicated save calls as an error - if !IsUniqueConstraintError(err, []string{"reactions_pkey", "PRIMARY"}) { - result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.save.app_error", nil, err.Error(), http.StatusBadRequest) - } - } else { - if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.commit.app_error", nil, err.Error(), http.StatusInternalServerError) - } - } - - if result.Err == nil { - result.Data = reaction + if err := transaction.Commit(); err != nil { + return nil, model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.commit.app_error", nil, err.Error(), http.StatusInternalServerError) } } - return result + return reaction, nil } -func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() +func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { + transaction, err := s.GetMaster().Begin() + if err != nil { + return nil, model.NewAppError("SqlReactionStore.Delete", "store.sql_reaction.delete.begin.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer finalizeTransaction(transaction) - if transaction, err := s.GetMaster().Begin(); err != nil { - result.Err = model.NewAppError("SqlReactionStore.Delete", "store.sql_reaction.delete.begin.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - defer finalizeTransaction(transaction) - err := deleteReactionAndUpdatePost(transaction, reaction) - - if err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.app_error", nil, err.Error(), http.StatusInternalServerError) - } else if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.commit.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - result.Data = reaction - } + appErr := deleteReactionAndUpdatePost(transaction, reaction) + if appErr != nil { + return nil, model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.app_error", nil, appErr.Error(), http.StatusInternalServerError) } - return result + if err := transaction.Commit(); err != nil { + return nil, model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.commit.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return reaction, nil } -func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() - +func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { var reactions []*model.Reaction if _, err := s.GetReplica().Select(&reactions, @@ -91,17 +80,13 @@ func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hin PostId = :PostId ORDER BY CreateAt`, map[string]interface{}{"PostId": postId}); err != nil { - result.Err = model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.get_for_post.app_error", nil, "", http.StatusInternalServerError) - } else { - result.Data = reactions + return nil, model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.get_for_post.app_error", nil, "", http.StatusInternalServerError) } - return result + return reactions, nil } -func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() - +func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { keys, params := MapStringsToQueryParams(postIds, "postId") var reactions []*model.Reaction @@ -113,17 +98,12 @@ func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []st PostId IN `+keys+` ORDER BY CreateAt`, params); err != nil { - result.Err = model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.bulk_get_for_post_ids.app_error", nil, "", http.StatusInternalServerError) - } else { - result.Data = reactions + return nil, model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.bulk_get_for_post_ids.app_error", nil, "", http.StatusInternalServerError) } - - return result + return reactions, nil } -func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() - +func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { var reactions []*model.Reaction if _, err := s.GetReplica().Select(&reactions, @@ -133,10 +113,9 @@ func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiN Reactions WHERE EmojiName = :EmojiName`, map[string]interface{}{"EmojiName": emojiName}); err != nil { - result.Err = model.NewAppError("SqlReactionStore.DeleteAllWithEmojiName", + return model.NewAppError("SqlReactionStore.DeleteAllWithEmojiName", "store.sql_reaction.delete_all_with_emoji_name.get_reactions.app_error", nil, "emoji_name="+emojiName+", error="+err.Error(), http.StatusInternalServerError) - return result } if _, err := s.GetMaster().Exec( @@ -144,10 +123,9 @@ func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiN Reactions WHERE EmojiName = :EmojiName`, map[string]interface{}{"EmojiName": emojiName}); err != nil { - result.Err = model.NewAppError("SqlReactionStore.DeleteAllWithEmojiName", + return model.NewAppError("SqlReactionStore.DeleteAllWithEmojiName", "store.sql_reaction.delete_all_with_emoji_name.delete_reactions.app_error", nil, "emoji_name="+emojiName+", error="+err.Error(), http.StatusInternalServerError) - return result } for _, reaction := range reactions { @@ -157,12 +135,10 @@ func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiN } } - return result + return nil } -func (s *SqlSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { - result := store.NewSupplierResult() - +func (s *SqlSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { var query string if s.DriverName() == "postgres" { query = "DELETE from Reactions WHERE CreateAt = any (array (SELECT CreateAt FROM Reactions WHERE CreateAt < :EndTime LIMIT :Limit))" @@ -172,18 +148,14 @@ func (s *SqlSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"EndTime": endTime, "Limit": limit}) if err != nil { - result.Err = model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - } else { - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { - result.Err = model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - result.Data = int64(0) - } else { - result.Data = rowsAffected - } + return 0, model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } - return result + rowsAffected, err1 := sqlResult.RowsAffected() + if err1 != nil { + return 0, model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) + } + return rowsAffected, nil } func saveReactionAndUpdatePost(transaction *gorp.Transaction, reaction *model.Reaction) error { diff --git a/store/store.go b/store/store.go index 538f1abd77..25274029e1 100644 --- a/store/store.go +++ b/store/store.go @@ -491,12 +491,12 @@ type FileInfoStore interface { } type ReactionStore interface { - Save(reaction *model.Reaction) StoreChannel - Delete(reaction *model.Reaction) StoreChannel - GetForPost(postId string, allowFromCache bool) StoreChannel - DeleteAllWithEmojiName(emojiName string) StoreChannel - PermanentDeleteBatch(endTime int64, limit int64) StoreChannel - BulkGetForPosts(postIds []string) StoreChannel + Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) + Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) + GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) + DeleteAllWithEmojiName(emojiName string) *model.AppError + PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) + BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) } type JobStore interface { diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index 9d700539c9..199352171b 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -815,7 +815,7 @@ func (_m *LayeredStoreDatabaseLayer) Reaction() store.ReactionStore { } // ReactionDelete provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreDatabaseLayer) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -825,20 +825,29 @@ func (_m *LayeredStoreDatabaseLayer) ReactionDelete(ctx context.Context, reactio _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { r0 = rf(ctx, reaction, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, reaction, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionDeleteAllWithEmojiName provides a mock function with given fields: ctx, emojiName, hints -func (_m *LayeredStoreDatabaseLayer) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -848,12 +857,12 @@ func (_m *LayeredStoreDatabaseLayer) ReactionDeleteAllWithEmojiName(ctx context. _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { r0 = rf(ctx, emojiName, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.AppError) } } @@ -861,7 +870,7 @@ func (_m *LayeredStoreDatabaseLayer) ReactionDeleteAllWithEmojiName(ctx context. } // ReactionGetForPost provides a mock function with given fields: ctx, postId, hints -func (_m *LayeredStoreDatabaseLayer) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -871,20 +880,29 @@ func (_m *LayeredStoreDatabaseLayer) ReactionGetForPost(ctx context.Context, pos _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) []*model.Reaction); ok { r0 = rf(ctx, postId, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, postId, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionPermanentDeleteBatch provides a mock function with given fields: ctx, endTime, limit, hints -func (_m *LayeredStoreDatabaseLayer) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -894,20 +912,27 @@ func (_m *LayeredStoreDatabaseLayer) ReactionPermanentDeleteBatch(ctx context.Co _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) int64); ok { r0 = rf(ctx, endTime, limit, hints...) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, endTime, limit, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // ReactionSave provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreDatabaseLayer) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -917,20 +942,29 @@ func (_m *LayeredStoreDatabaseLayer) ReactionSave(ctx context.Context, reaction _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { r0 = rf(ctx, reaction, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, reaction, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionsBulkGetForPosts provides a mock function with given fields: ctx, postIds, hints -func (_m *LayeredStoreDatabaseLayer) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreDatabaseLayer) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -940,16 +974,25 @@ func (_m *LayeredStoreDatabaseLayer) ReactionsBulkGetForPosts(ctx context.Contex _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) []*model.Reaction); ok { r0 = rf(ctx, postIds, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, []string, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, postIds, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // Role provides a mock function with given fields: diff --git a/store/storetest/mocks/LayeredStoreSupplier.go b/store/storetest/mocks/LayeredStoreSupplier.go index 78378f44c4..d4794eb32e 100644 --- a/store/storetest/mocks/LayeredStoreSupplier.go +++ b/store/storetest/mocks/LayeredStoreSupplier.go @@ -491,7 +491,7 @@ func (_m *LayeredStoreSupplier) Next() store.LayeredStoreSupplier { } // ReactionDelete provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -501,20 +501,29 @@ func (_m *LayeredStoreSupplier) ReactionDelete(ctx context.Context, reaction *mo _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { r0 = rf(ctx, reaction, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, reaction, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionDeleteAllWithEmojiName provides a mock function with given fields: ctx, emojiName, hints -func (_m *LayeredStoreSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -524,12 +533,12 @@ func (_m *LayeredStoreSupplier) ReactionDeleteAllWithEmojiName(ctx context.Conte _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { r0 = rf(ctx, emojiName, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.AppError) } } @@ -537,7 +546,7 @@ func (_m *LayeredStoreSupplier) ReactionDeleteAllWithEmojiName(ctx context.Conte } // ReactionGetForPost provides a mock function with given fields: ctx, postId, hints -func (_m *LayeredStoreSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -547,20 +556,29 @@ func (_m *LayeredStoreSupplier) ReactionGetForPost(ctx context.Context, postId s _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) []*model.Reaction); ok { r0 = rf(ctx, postId, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, postId, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionPermanentDeleteBatch provides a mock function with given fields: ctx, endTime, limit, hints -func (_m *LayeredStoreSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -570,20 +588,27 @@ func (_m *LayeredStoreSupplier) ReactionPermanentDeleteBatch(ctx context.Context _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) int64); ok { r0 = rf(ctx, endTime, limit, hints...) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, endTime, limit, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // ReactionSave provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -593,20 +618,29 @@ func (_m *LayeredStoreSupplier) ReactionSave(ctx context.Context, reaction *mode _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { r0 = rf(ctx, reaction, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, reaction, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // ReactionsBulkGetForPosts provides a mock function with given fields: ctx, postIds, hints -func (_m *LayeredStoreSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult { +func (_m *LayeredStoreSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { _va := make([]interface{}, len(hints)) for _i := range hints { _va[_i] = hints[_i] @@ -616,16 +650,25 @@ func (_m *LayeredStoreSupplier) ReactionsBulkGetForPosts(ctx context.Context, po _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 *store.LayeredStoreSupplierResult - if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) []*model.Reaction); ok { r0 = rf(ctx, postIds, hints...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*store.LayeredStoreSupplierResult) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(context.Context, []string, ...store.LayeredStoreHint) *model.AppError); ok { + r1 = rf(ctx, postIds, hints...) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // RoleDelete provides a mock function with given fields: ctx, roldId, hints diff --git a/store/storetest/mocks/ReactionStore.go b/store/storetest/mocks/ReactionStore.go index 0f72a98b37..80cb0486c5 100644 --- a/store/storetest/mocks/ReactionStore.go +++ b/store/storetest/mocks/ReactionStore.go @@ -6,7 +6,6 @@ package mocks import mock "github.com/stretchr/testify/mock" import model "github.com/mattermost/mattermost-server/model" -import store "github.com/mattermost/mattermost-server/store" // ReactionStore is an autogenerated mock type for the ReactionStore type type ReactionStore struct { @@ -14,47 +13,65 @@ type ReactionStore struct { } // BulkGetForPosts provides a mock function with given fields: postIds -func (_m *ReactionStore) BulkGetForPosts(postIds []string) store.StoreChannel { +func (_m *ReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) { ret := _m.Called(postIds) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func([]string) store.StoreChannel); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func([]string) []*model.Reaction); ok { r0 = rf(postIds) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func([]string) *model.AppError); ok { + r1 = rf(postIds) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // Delete provides a mock function with given fields: reaction -func (_m *ReactionStore) Delete(reaction *model.Reaction) store.StoreChannel { +func (_m *ReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) { ret := _m.Called(reaction) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.Reaction) store.StoreChannel); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(*model.Reaction) *model.Reaction); ok { r0 = rf(reaction) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.Reaction) *model.AppError); ok { + r1 = rf(reaction) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // DeleteAllWithEmojiName provides a mock function with given fields: emojiName -func (_m *ReactionStore) DeleteAllWithEmojiName(emojiName string) store.StoreChannel { +func (_m *ReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError { ret := _m.Called(emojiName) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { r0 = rf(emojiName) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -62,49 +79,74 @@ func (_m *ReactionStore) DeleteAllWithEmojiName(emojiName string) store.StoreCha } // GetForPost provides a mock function with given fields: postId, allowFromCache -func (_m *ReactionStore) GetForPost(postId string, allowFromCache bool) store.StoreChannel { +func (_m *ReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) { ret := _m.Called(postId, allowFromCache) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok { + var r0 []*model.Reaction + if rf, ok := ret.Get(0).(func(string, bool) []*model.Reaction); ok { r0 = rf(postId, allowFromCache) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + r1 = rf(postId, allowFromCache) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // PermanentDeleteBatch provides a mock function with given fields: endTime, limit -func (_m *ReactionStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel { +func (_m *ReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { ret := _m.Called(endTime, limit) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(int64, int64) store.StoreChannel); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(int64, int64) int64); ok { r0 = rf(endTime, limit) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(int64, int64) *model.AppError); ok { + r1 = rf(endTime, limit) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // Save provides a mock function with given fields: reaction -func (_m *ReactionStore) Save(reaction *model.Reaction) store.StoreChannel { +func (_m *ReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) { ret := _m.Called(reaction) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.Reaction) store.StoreChannel); ok { + var r0 *model.Reaction + if rf, ok := ret.Get(0).(func(*model.Reaction) *model.Reaction); ok { r0 = rf(reaction) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Reaction) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.Reaction) *model.AppError); ok { + r1 = rf(reaction) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } diff --git a/store/storetest/reaction_store.go b/store/storetest/reaction_store.go index bba82bee63..e3720ac6bd 100644 --- a/store/storetest/reaction_store.go +++ b/store/storetest/reaction_store.go @@ -8,6 +8,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" + "github.com/stretchr/testify/require" ) func TestReactionStore(t *testing.T, ss store.Store) { @@ -31,9 +32,9 @@ func testReactionSave(t *testing.T, ss store.Store) { PostId: post.Id, EmojiName: model.NewId(), } - if result := <-ss.Reaction().Save(reaction1); result.Err != nil { - t.Fatal(result.Err) - } else if saved := result.Data.(*model.Reaction); saved.UserId != reaction1.UserId || + if reaction, err := ss.Reaction().Save(reaction1); err != nil { + t.Fatal(err) + } else if saved := reaction; saved.UserId != reaction1.UserId || saved.PostId != reaction1.PostId || saved.EmojiName != reaction1.EmojiName { t.Fatal("should've saved reaction and returned it") } @@ -47,8 +48,8 @@ func testReactionSave(t *testing.T, ss store.Store) { secondUpdateAt = postList.Posts[post.Id].UpdateAt } - if result := <-ss.Reaction().Save(reaction1); result.Err != nil { - t.Log(result.Err) + if _, err := ss.Reaction().Save(reaction1); err != nil { + t.Log(err) t.Fatal("should've allowed saving a duplicate reaction") } @@ -58,8 +59,8 @@ func testReactionSave(t *testing.T, ss store.Store) { PostId: reaction1.PostId, EmojiName: reaction1.EmojiName, } - if result := <-ss.Reaction().Save(reaction2); result.Err != nil { - t.Fatal(result.Err) + 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 { @@ -72,8 +73,8 @@ func testReactionSave(t *testing.T, ss store.Store) { PostId: model.NewId(), EmojiName: reaction1.EmojiName, } - if result := <-ss.Reaction().Save(reaction3); result.Err != nil { - t.Fatal(result.Err) + if _, err := ss.Reaction().Save(reaction3); err != nil { + t.Fatal(err) } // different emoji @@ -82,8 +83,8 @@ func testReactionSave(t *testing.T, ss store.Store) { PostId: reaction1.PostId, EmojiName: model.NewId(), } - if result := <-ss.Reaction().Save(reaction4); result.Err != nil { - t.Fatal(result.Err) + if _, err := ss.Reaction().Save(reaction4); err != nil { + t.Fatal(err) } // invalid reaction @@ -91,7 +92,7 @@ func testReactionSave(t *testing.T, ss store.Store) { UserId: reaction1.UserId, PostId: reaction1.PostId, } - if result := <-ss.Reaction().Save(reaction5); result.Err == nil { + if _, err := ss.Reaction().Save(reaction5); err == nil { t.Fatal("should've failed for invalid reaction") } } @@ -108,16 +109,17 @@ func testReactionDelete(t *testing.T, ss store.Store) { EmojiName: model.NewId(), } - store.Must(ss.Reaction().Save(reaction)) + _, err := ss.Reaction().Save(reaction) + require.Nil(t, err) firstUpdateAt := store.Must(ss.Post().Get(reaction.PostId)).(*model.PostList).Posts[post.Id].UpdateAt - if result := <-ss.Reaction().Delete(reaction); result.Err != nil { - t.Fatal(result.Err) + if _, err := ss.Reaction().Delete(reaction); err != nil { + t.Fatal(err) } - if result := <-ss.Reaction().GetForPost(post.Id, false); result.Err != nil { - t.Fatal(result.Err) - } else if len(result.Data.([]*model.Reaction)) != 0 { + if reactions, err := ss.Reaction().GetForPost(post.Id, false); err != nil { + t.Fatal(err) + } else if len(reactions) != 0 { t.Fatal("should've deleted reaction") } @@ -157,12 +159,13 @@ func testReactionGetForPost(t *testing.T, ss store.Store) { } for _, reaction := range reactions { - store.Must(ss.Reaction().Save(reaction)) + _, err := ss.Reaction().Save(reaction) + require.Nil(t, err) } - if result := <-ss.Reaction().GetForPost(postId, false); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.Reaction); len(returned) != 3 { + if returned, err := ss.Reaction().GetForPost(postId, false); err != nil { + t.Fatal(err) + } else if len(returned) != 3 { t.Fatal("should've returned 3 reactions") } else { for _, reaction := range reactions { @@ -185,9 +188,9 @@ func testReactionGetForPost(t *testing.T, ss store.Store) { } // Should return cached item - if result := <-ss.Reaction().GetForPost(postId, true); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.Reaction); len(returned) != 3 { + if returned, err := ss.Reaction().GetForPost(postId, true); err != nil { + t.Fatal(err) + } else if len(returned) != 3 { t.Fatal("should've returned 3 reactions") } else { for _, reaction := range reactions { @@ -257,15 +260,18 @@ func testReactionDeleteAllWithEmojiName(t *testing.T, ss store.Store) { } for _, reaction := range reactions { - store.Must(ss.Reaction().Save(reaction)) + _, err := ss.Reaction().Save(reaction) + require.Nil(t, err) } - if result := <-ss.Reaction().DeleteAllWithEmojiName(emojiToDelete); result.Err != nil { - t.Fatal(result.Err) + if err := ss.Reaction().DeleteAllWithEmojiName(emojiToDelete); err != nil { + t.Fatal(err) } // check that the reactions were deleted - if returned := store.Must(ss.Reaction().GetForPost(post.Id, false)).([]*model.Reaction); len(returned) != 1 { + if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil { + t.Fatal(err) + } else if len(returned) != 1 { t.Fatal("should've only removed reactions with emoji name") } else { for _, reaction := range returned { @@ -275,11 +281,15 @@ func testReactionDeleteAllWithEmojiName(t *testing.T, ss store.Store) { } } - if returned := store.Must(ss.Reaction().GetForPost(post2.Id, false)).([]*model.Reaction); len(returned) != 1 { + if returned, err := ss.Reaction().GetForPost(post2.Id, false); err != nil { + t.Fatal(err) + } else if len(returned) != 1 { t.Fatal("should've only removed reactions with emoji name") } - if returned := store.Must(ss.Reaction().GetForPost(post3.Id, false)).([]*model.Reaction); len(returned) != 0 { + if returned, err := ss.Reaction().GetForPost(post3.Id, false); err != nil { + t.Fatal(err) + } else if len(returned) != 0 { t.Fatal("should've only removed reactions with emoji name") } @@ -333,19 +343,27 @@ func testReactionStorePermanentDeleteBatch(t *testing.T, ss store.Store) { // Need to hang on to a reaction to delete later in order to clear the cache, as "allowFromCache" isn't honoured any more. var lastReaction *model.Reaction for _, reaction := range reactions { - lastReaction = store.Must(ss.Reaction().Save(reaction)).(*model.Reaction) + var err *model.AppError + lastReaction, err = ss.Reaction().Save(reaction) + require.Nil(t, err) } - if returned := store.Must(ss.Reaction().GetForPost(post.Id, false)).([]*model.Reaction); len(returned) != 4 { + if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil { + t.Fatal(err) + } else if len(returned) != 4 { t.Fatal("expected 4 reactions") } - store.Must(ss.Reaction().PermanentDeleteBatch(1800, 1000)) + _, err := ss.Reaction().PermanentDeleteBatch(1800, 1000) + require.Nil(t, err) // This is to force a clear of the cache. - store.Must(ss.Reaction().Delete(lastReaction)) + _, err = ss.Reaction().Delete(lastReaction) + require.Nil(t, err) - if returned := store.Must(ss.Reaction().GetForPost(post.Id, false)).([]*model.Reaction); len(returned) != 1 { + if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil { + t.Fatal(err) + } else if len(returned) != 1 { t.Fatalf("expected 1 reaction. Got: %v", len(returned)) } } @@ -392,13 +410,14 @@ func testReactionBulkGetForPosts(t *testing.T, ss store.Store) { } for _, reaction := range reactions { - store.Must(ss.Reaction().Save(reaction)) + _, err := ss.Reaction().Save(reaction) + require.Nil(t, err) } postIds := []string{postId, post2Id, post3Id} - if result := <-ss.Reaction().BulkGetForPosts(postIds); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.Reaction); len(returned) != 5 { + if returned, err := ss.Reaction().BulkGetForPosts(postIds); err != nil { + t.Fatal(err) + } else if len(returned) != 5 { t.Fatal("should've returned 5 reactions") } else { post4IdFound := false