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

@@ -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 {
+		if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil {
-			t.Fatal(result.Err)
+			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 {
+		if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil {
-			t.Fatal(result.Err)
+			t.Fatal(err)
 		} else {
-			reactions = append(reactions, result.Data.(*model.Reaction))
+			reactions = append(reactions, reaction)
 
 		}
 		expectedPostIdsReactionsMap[userReaction.PostId] = reactions

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

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)
+	reactions, err := a.Srv.Store.Reaction().GetForPost(postId, true)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		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
 		}

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 {
+	if _, err := a.Srv.Store.Reaction().Save(reaction); err != nil {
-		return result.Err
+		return err
 	}
 	return nil
 }

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")
 		}
 	}

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)
+	reaction, err = a.Srv.Store.Reaction().Save(reaction)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		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)
+	return a.Srv.Store.Reaction().GetForPost(postId, true)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.Reaction), nil
 }
 
 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)
+	allReactions, err := a.Srv.Store.Reaction().BulkGetForPosts(postIds)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		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 {
+	if _, err := a.Srv.Store.Reaction().Delete(reaction); err != nil {
-		return result.Err
+		return err
 	}
 
 	// The post is always modified since the UpdateAt always changes

store/layered_store.go

@@ -232,40 +232,28 @@ type LayeredReactionStore struct {
 	*LayeredStore
 }
 
-func (s *LayeredReactionStore) Save(reaction *model.Reaction) StoreChannel {
+func (s *LayeredReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionSave(s.TmpContext, reaction)
-		return supplier.ReactionSave(s.TmpContext, reaction)
-	})
 }
 
-func (s *LayeredReactionStore) Delete(reaction *model.Reaction) StoreChannel {
+func (s *LayeredReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionDelete(s.TmpContext, reaction)
-		return supplier.ReactionDelete(s.TmpContext, reaction)
-	})
 }
 
-func (s *LayeredReactionStore) GetForPost(postId string, allowFromCache bool) StoreChannel {
+func (s *LayeredReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionGetForPost(s.TmpContext, postId)
-		return supplier.ReactionGetForPost(s.TmpContext, postId)
-	})
 }
 
-func (s *LayeredReactionStore) BulkGetForPosts(postIds []string) StoreChannel {
+func (s *LayeredReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionsBulkGetForPosts(s.TmpContext, postIds)
-		return supplier.ReactionsBulkGetForPosts(s.TmpContext, postIds)
-	})
 }
 
-func (s *LayeredReactionStore) DeleteAllWithEmojiName(emojiName string) StoreChannel {
+func (s *LayeredReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionDeleteAllWithEmojiName(s.TmpContext, emojiName)
-		return supplier.ReactionDeleteAllWithEmojiName(s.TmpContext, emojiName)
-	})
 }
 
-func (s *LayeredReactionStore) PermanentDeleteBatch(endTime int64, limit int64) StoreChannel {
+func (s *LayeredReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) {
-	return s.RunQuery(func(supplier LayeredStoreSupplier) *LayeredStoreSupplierResult {
+	return s.LayerChainHead.ReactionPermanentDeleteBatch(s.TmpContext, endTime, limit)
-		return supplier.ReactionPermanentDeleteBatch(s.TmpContext, endTime, limit)
-	})
 }
 
 type LayeredRoleStore struct {

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
+	ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError)
-	ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) *LayeredStoreSupplierResult
+	ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError)
-	ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult
+	ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError)
-	ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult
+	ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError
-	ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) *LayeredStoreSupplierResult
+	ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError)
-	ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult
+	ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError)
 
 	// Roles
 	RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) *LayeredStoreSupplierResult

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...)
 }

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 {
+func (s *RedisSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) {
-	var resultdata []*model.Reaction
+	var reaction []*model.Reaction
-	found, err := s.load("reactions:"+postId, &resultdata)
+	found, err := s.load("reactions:"+postId, &reaction)
 	if found {
-		result := NewSupplierResult()
+		return reaction, nil
-		result.Data = resultdata
-		return result
 	}
+
 	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...)
 }

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 {
+func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) {
-	result := store.NewSupplierResult()
-
 	reaction.PreSave()
-	if result.Err = reaction.IsValid(); result.Err != nil {
+	if err := reaction.IsValid(); err != nil {
-		return result
+		return nil, err
 	}
 
-	if transaction, err := s.GetMaster().Begin(); err != nil {
+	transaction, err := s.GetMaster().Begin()
-		result.Err = model.NewAppError("SqlReactionStore.Save", "store.sql_reaction.save.begin.app_error", nil, err.Error(), http.StatusInternalServerError)
-	} else {
-		defer finalizeTransaction(transaction)
-		err := saveReactionAndUpdatePost(transaction, reaction)
-
 	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(err, []string{"reactions_pkey", "PRIMARY"}) {
+		if !IsUniqueConstraintError(appErr, []string{"reactions_pkey", "PRIMARY"}) {
-				result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.save.app_error", nil, err.Error(), http.StatusBadRequest)
+			return nil, model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.save.app_error", nil, appErr.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)
+			return nil, model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.commit.app_error", nil, err.Error(), http.StatusInternalServerError)
 		}
 	}
 
-		if result.Err == nil {
+	return reaction, nil
-			result.Data = reaction
-		}
-	}
-
-	return result
 }
 
-func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult {
+func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) {
-	result := store.NewSupplierResult()
+	transaction, err := s.GetMaster().Begin()
-
-	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)
+		return nil, model.NewAppError("SqlReactionStore.Delete", "store.sql_reaction.delete.begin.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
 	}
+	defer finalizeTransaction(transaction)
+
+	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 {
+func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) {
-	result := store.NewSupplierResult()
-
 	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)
+		return nil, model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.get_for_post.app_error", nil, "", http.StatusInternalServerError)
-	} else {
-		result.Data = reactions
 	}
 
-	return result
+	return reactions, nil
 }
 
-func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult {
+func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) {
-	result := store.NewSupplierResult()
-
 	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)
+		return nil, model.NewAppError("SqlReactionStore.GetForPost", "store.sql_reaction.bulk_get_for_post_ids.app_error", nil, "", http.StatusInternalServerError)
-	} else {
-		result.Data = reactions
 	}
-
+	return reactions, nil
-	return result
 }
 
-func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult {
+func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError {
-	result := store.NewSupplierResult()
-
 	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 {
+func (s *SqlSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) {
-	result := store.NewSupplierResult()
-
 	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)
+		return 0, 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 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 {

store/store.go

@@ -491,12 +491,12 @@ type FileInfoStore interface {
 }
 
 type ReactionStore interface {
-	Save(reaction *model.Reaction) StoreChannel
+	Save(reaction *model.Reaction) (*model.Reaction, *model.AppError)
-	Delete(reaction *model.Reaction) StoreChannel
+	Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError)
-	GetForPost(postId string, allowFromCache bool) StoreChannel
+	GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError)
-	DeleteAllWithEmojiName(emojiName string) StoreChannel
+	DeleteAllWithEmojiName(emojiName string) *model.AppError
-	PermanentDeleteBatch(endTime int64, limit int64) StoreChannel
+	PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError)
-	BulkGetForPosts(postIds []string) StoreChannel
+	BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError)
 }
 
 type JobStore interface {

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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 int64
-	if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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).(int64)
-			r0 = ret.Get(0).(*store.LayeredStoreSupplierResult)
+	}
+
+	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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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:

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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 int64
-	if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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).(int64)
-			r0 = ret.Get(0).(*store.LayeredStoreSupplierResult)
+	}
+
+	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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) *store.LayeredStoreSupplierResult); ok {
+	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

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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func([]string) store.StoreChannel); ok {
+	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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(*model.Reaction) store.StoreChannel); ok {
+	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
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	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
+	var r0 []*model.Reaction
-	if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok {
+	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
+	var r0 int64
-	if rf, ok := ret.Get(0).(func(int64, int64) store.StoreChannel); ok {
+	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).(int64)
-			r0 = ret.Get(0).(store.StoreChannel)
+	}
+
+	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
+	var r0 *model.Reaction
-	if rf, ok := ret.Get(0).(func(*model.Reaction) store.StoreChannel); ok {
+	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
 }

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 {
+	if reaction, err := ss.Reaction().Save(reaction1); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else if saved := result.Data.(*model.Reaction); saved.UserId != reaction1.UserId ||
+	} 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 {
+	if _, err := ss.Reaction().Save(reaction1); err != nil {
-		t.Log(result.Err)
+		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 {
+	if _, err := ss.Reaction().Save(reaction2); err != nil {
-		t.Fatal(result.Err)
+		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 {
+	if _, err := ss.Reaction().Save(reaction3); err != nil {
-		t.Fatal(result.Err)
+		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 {
+	if _, err := ss.Reaction().Save(reaction4); err != nil {
-		t.Fatal(result.Err)
+		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 {
+	if _, err := ss.Reaction().Delete(reaction); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 
-	if result := <-ss.Reaction().GetForPost(post.Id, false); result.Err != nil {
+	if reactions, err := ss.Reaction().GetForPost(post.Id, false); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else if len(result.Data.([]*model.Reaction)) != 0 {
+	} 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 {
+	if returned, err := ss.Reaction().GetForPost(postId, false); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else if returned := result.Data.([]*model.Reaction); len(returned) != 3 {
+	} 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 {
+	if returned, err := ss.Reaction().GetForPost(postId, true); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else if returned := result.Data.([]*model.Reaction); len(returned) != 3 {
+	} 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 {
+	if err := ss.Reaction().DeleteAllWithEmojiName(emojiToDelete); err != nil {
-		t.Fatal(result.Err)
+		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 {
+	if returned, err := ss.Reaction().BulkGetForPosts(postIds); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else if returned := result.Data.([]*model.Reaction); len(returned) != 5 {
+	} else if len(returned) != 5 {
 		t.Fatal("should've returned 5 reactions")
 	} else {
 		post4IdFound := false