[MM-55143] Disallow reacting with an emoji that does not exist, limit the total number of unique reactions per post (#25331)

* [MM-55143] Disallow reacting with an emoji that does not exist * WIP for server limit on emoji reactions * WIP * Implement default limit of 25 unique emoji reactions * Add modal for reaction limit * Fix test * PR feedback * Fix i18n * Update admin string * Merge'd * Fixing some issues, check limits correctly based on other users reactions * Fix typos * Fix lint/test * Add tests, fix other tests --------- Co-authored-by: Mattermost Build <build@mattermost.com>
2023-11-27 09:11:04 -05:00
--- a/server/channels/api4/reaction_test.go
+++ b/server/channels/api4/reaction_test.go
@@ -55,7 +55,7 @@ func TestSaveReaction(t *testing.T) {
 	})

 	t.Run("save-second-reaction", func(t *testing.T) {
-		reaction.EmojiName = "sad"
+		reaction.EmojiName = "cry"

 		rr, _, err := client.SaveReaction(context.Background(), reaction)
 		require.NoError(t, err)
@@ -290,7 +290,7 @@ func TestDeleteReaction(t *testing.T) {
 	r2 := &model.Reaction{
 		UserId:    userId,
 		PostId:    postId,
-		EmojiName: "smile-",
+		EmojiName: "cry",
 	}

 	r3 := &model.Reaction{
@@ -302,7 +302,7 @@ func TestDeleteReaction(t *testing.T) {
 	r4 := &model.Reaction{
 		UserId:    user2Id,
 		PostId:    postId,
-		EmojiName: "smile_",
+		EmojiName: "grin",
 	}

 	// Check the appropriate permissions are enforced.
--- a/server/channels/app/export_test.go
+++ b/server/channels/app/export_test.go
@@ -29,18 +29,20 @@ func TestReactionsOfPost(t *testing.T) {
 	reactionObject := model.Reaction{
 		UserId:    th.BasicUser.Id,
 		PostId:    post.Id,
-		EmojiName: "emoji",
+		EmojiName: "smile",
 		CreateAt:  model.GetMillis(),
 	}
 	reactionObjectDeleted := model.Reaction{
 		UserId:    th.BasicUser2.Id,
 		PostId:    post.Id,
-		EmojiName: "emoji",
+		EmojiName: "smile",
 		CreateAt:  model.GetMillis(),
 	}

-	th.App.SaveReactionForPost(th.Context, &reactionObject)
-	th.App.SaveReactionForPost(th.Context, &reactionObjectDeleted)
+	_, err := th.App.SaveReactionForPost(th.Context, &reactionObject)
+	require.Nil(t, err)
+	_, err = th.App.SaveReactionForPost(th.Context, &reactionObjectDeleted)
+	require.Nil(t, err)
 	reactionsOfPost, err := th.App.BuildPostReactions(th.Context, post.Id)
 	require.Nil(t, err)

--- a/server/channels/app/reaction.go
+++ b/server/channels/app/reaction.go
@@ -20,6 +20,30 @@ func (a *App) SaveReactionForPost(c request.CTX, reaction *model.Reaction) (*mod
 		return nil, err
 	}

+	// Check whether this is a valid emoji
+	if _, ok := model.GetSystemEmojiId(reaction.EmojiName); !ok {
+		if _, emojiErr := a.GetEmojiByName(c, reaction.EmojiName); emojiErr != nil {
+			return nil, emojiErr
+		}
+	}
+
+	existing, dErr := a.Srv().Store().Reaction().ExistsOnPost(reaction.PostId, reaction.EmojiName)
+	if dErr != nil {
+		return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.app_error", nil, "", http.StatusInternalServerError).Wrap(dErr)
+	}
+
+	// If it exists already, we don't need to check for the limit
+	if !existing {
+		count, dErr := a.Srv().Store().Reaction().GetUniqueCountForPost(reaction.PostId)
+		if dErr != nil {
+			return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.app_error", nil, "", http.StatusInternalServerError).Wrap(dErr)
+		}
+
+		if count >= *a.Config().ServiceSettings.UniqueEmojiReactionLimitPerPost {
+			return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.too_many_reactions", nil, "", http.StatusBadRequest)
+		}
+	}
+
 	channel, err := a.GetChannel(c, post.ChannelId)
 	if err != nil {
 		return nil, err
--- a/server/channels/app/reaction_test.go
+++ b/server/channels/app/reaction_test.go
@@ -13,6 +13,89 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/testlib"
 )

+func TestSaveReactionForPost(t *testing.T) {
+	th := Setup(t).InitBasic()
+
+	post := th.CreatePost(th.BasicChannel)
+	reaction1, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{
+		UserId:    th.BasicUser.Id,
+		PostId:    post.Id,
+		EmojiName: "cry",
+	})
+	require.NotNil(t, reaction1)
+	require.Nil(t, err)
+	reaction2, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{
+		UserId:    th.BasicUser.Id,
+		PostId:    post.Id,
+		EmojiName: "smile",
+	})
+	require.NotNil(t, reaction2)
+	require.Nil(t, err)
+	reaction3, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{
+		UserId:    th.BasicUser.Id,
+		PostId:    post.Id,
+		EmojiName: "rofl",
+	})
+	require.NotNil(t, reaction3)
+	require.Nil(t, err)
+
+	t.Run("should not add reaction if it does not exist on the system", func(t *testing.T) {
+		reaction := &model.Reaction{
+			UserId:    th.BasicUser.Id,
+			PostId:    th.BasicPost.Id,
+			EmojiName: "definitely-not-a-real-emoji",
+		}
+
+		result, err := th.App.SaveReactionForPost(th.Context, reaction)
+		require.NotNil(t, err)
+		require.Nil(t, result)
+	})
+
+	t.Run("should not add reaction if we are over the limit", func(t *testing.T) {
+		var originalLimit *int
+		th.UpdateConfig(func(cfg *model.Config) {
+			originalLimit = cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost
+			*cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = 3
+		})
+		defer th.UpdateConfig(func(cfg *model.Config) {
+			cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = originalLimit
+		})
+
+		reaction := &model.Reaction{
+			UserId:    th.BasicUser.Id,
+			PostId:    post.Id,
+			EmojiName: "joy",
+		}
+
+		result, err := th.App.SaveReactionForPost(th.Context, reaction)
+		require.NotNil(t, err)
+		require.Nil(t, result)
+	})
+
+	t.Run("should always add reaction if we are over the limit but the reaction is not unique", func(t *testing.T) {
+		user := th.CreateUser()
+
+		var originalLimit *int
+		th.UpdateConfig(func(cfg *model.Config) {
+			originalLimit = cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost
+			*cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = 3
+		})
+		defer th.UpdateConfig(func(cfg *model.Config) {
+			cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = originalLimit
+		})
+
+		reaction := &model.Reaction{
+			UserId:    user.Id,
+			PostId:    post.Id,
+			EmojiName: "cry",
+		}
+
+		result, err := th.App.SaveReactionForPost(th.Context, reaction)
+		require.Nil(t, err)
+		require.NotNil(t, result)
+	})
+}
+
 func TestSharedChannelSyncForReactionActions(t *testing.T) {
 	t.Run("adding a reaction in a shared channel performs a content sync when sync service is running on that node", func(t *testing.T) {
 		th := Setup(t).InitBasic()
@@ -84,3 +167,15 @@ func TestSharedChannelSyncForReactionActions(t *testing.T) {
 		assert.Equal(t, channel.Id, sharedChannelService.channelNotifications[1])
 	})
 }
+
+func (th *TestHelper) UpdateConfig(f func(*model.Config)) {
+	if th.ConfigStore.IsReadOnly() {
+		return
+	}
+	old := th.ConfigStore.Get()
+	updated := old.Clone()
+	f(updated)
+	if _, _, err := th.ConfigStore.Set(updated); err != nil {
+		panic(err)
+	}
+}
--- a/server/channels/store/opentracinglayer/opentracinglayer.go
+++ b/server/channels/store/opentracinglayer/opentracinglayer.go
@@ -7463,6 +7463,24 @@ func (s *OpenTracingLayerReactionStore) DeleteOrphanedRowsByIds(r *model.Retenti
 	return err
 }

+func (s *OpenTracingLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.ExistsOnPost")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.ReactionStore.ExistsOnPost(postId, emojiName)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.GetForPost")
@@ -7499,6 +7517,24 @@ func (s *OpenTracingLayerReactionStore) GetForPostSince(postId string, since int
 	return result, err
 }

+func (s *OpenTracingLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.GetUniqueCountForPost")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.ReactionStore.GetUniqueCountForPost(postId)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.PermanentDeleteBatch")
--- a/server/channels/store/retrylayer/retrylayer.go
+++ b/server/channels/store/retrylayer/retrylayer.go
@@ -8474,6 +8474,27 @@ func (s *RetryLayerReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsF

 }

+func (s *RetryLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) {
+
+	tries := 0
+	for {
+		result, err := s.ReactionStore.ExistsOnPost(postId, emojiName)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) {

 	tries := 0
@@ -8516,6 +8537,27 @@ func (s *RetryLayerReactionStore) GetForPostSince(postId string, since int64, ex

 }

+func (s *RetryLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) {
+
+	tries := 0
+	for {
+		result, err := s.ReactionStore.GetUniqueCountForPost(postId)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) {

 	tries := 0
--- a/server/channels/store/sqlstore/reaction_store.go
+++ b/server/channels/store/sqlstore/reaction_store.go
@@ -4,6 +4,7 @@
 package sqlstore

 import (
+	"database/sql"
 	"time"

 	sq "github.com/mattermost/squirrel"
@@ -107,6 +108,25 @@ func (s *SqlReactionStore) GetForPost(postId string, allowFromCache bool) ([]*mo
 	return reactions, nil
 }

+func (s *SqlReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) {
+	query := s.getQueryBuilder().
+		Select("1").
+		From("Reactions").
+		Where(sq.Eq{"PostId": postId}).
+		Where(sq.Eq{"EmojiName": emojiName}).
+		Where(sq.Eq{"COALESCE(DeleteAt, 0)": 0})
+
+	var hasRows bool
+	if err := s.GetReplicaX().GetBuilder(&hasRows, query); err != nil {
+		if err == sql.ErrNoRows {
+			return false, nil
+		}
+		return false, errors.Wrap(err, "failed to check for existing reaction")
+	}
+
+	return hasRows, nil
+}
+
 // GetForPostSince returns all reactions associated with `postId` updated after `since`.
 func (s *SqlReactionStore) GetForPostSince(postId string, since int64, excludeRemoteId string, inclDeleted bool) ([]*model.Reaction, error) {
 	query := s.getQueryBuilder().
@@ -138,6 +158,21 @@ func (s *SqlReactionStore) GetForPostSince(postId string, since int64, excludeRe
 	return reactions, nil
 }

+func (s *SqlReactionStore) GetUniqueCountForPost(postId string) (int, error) {
+	query := s.getQueryBuilder().
+		Select("COUNT(DISTINCT EmojiName)").
+		From("Reactions").
+		Where(sq.Eq{"PostId": postId}).
+		Where(sq.Eq{"DeleteAt": 0})
+
+	var count int64
+	err := s.GetReplicaX().GetBuilder(&count, query)
+	if err != nil {
+		return 0, errors.Wrap(err, "failed to count Reactions")
+	}
+	return int(count), nil
+}
+
 func (s *SqlReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, error) {
 	placeholder, values := constructArrayArgs(postIds)
 	var reactions []*model.Reaction
--- a/server/channels/store/store.go
+++ b/server/channels/store/store.go
@@ -744,6 +744,8 @@ type ReactionStore interface {
 	Delete(reaction *model.Reaction) (*model.Reaction, error)
 	GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error)
 	GetForPostSince(postId string, since int64, excludeRemoteId string, inclDeleted bool) ([]*model.Reaction, error)
+	GetUniqueCountForPost(postId string) (int, error)
+	ExistsOnPost(postId string, emojiName string) (bool, error)
 	DeleteAllWithEmojiName(emojiName string) error
 	BulkGetForPosts(postIds []string) ([]*model.Reaction, error)
 	DeleteOrphanedRowsByIds(r *model.RetentionIdsForDeletion) error
--- a/server/channels/store/storetest/mocks/ReactionStore.go
+++ b/server/channels/store/storetest/mocks/ReactionStore.go
@@ -94,6 +94,30 @@ func (_m *ReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsForDeletio
 	return r0
 }

+// ExistsOnPost provides a mock function with given fields: postId, emojiName
+func (_m *ReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) {
+	ret := _m.Called(postId, emojiName)
+
+	var r0 bool
+	var r1 error
+	if rf, ok := ret.Get(0).(func(string, string) (bool, error)); ok {
+		return rf(postId, emojiName)
+	}
+	if rf, ok := ret.Get(0).(func(string, string) bool); ok {
+		r0 = rf(postId, emojiName)
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
+		r1 = rf(postId, emojiName)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // GetForPost provides a mock function with given fields: postID, allowFromCache
 func (_m *ReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) {
 	ret := _m.Called(postID, allowFromCache)
@@ -146,6 +170,30 @@ func (_m *ReactionStore) GetForPostSince(postId string, since int64, excludeRemo
 	return r0, r1
 }

+// GetUniqueCountForPost provides a mock function with given fields: postId
+func (_m *ReactionStore) GetUniqueCountForPost(postId string) (int, error) {
+	ret := _m.Called(postId)
+
+	var r0 int
+	var r1 error
+	if rf, ok := ret.Get(0).(func(string) (int, error)); ok {
+		return rf(postId)
+	}
+	if rf, ok := ret.Get(0).(func(string) int); ok {
+		r0 = rf(postId)
+	} else {
+		r0 = ret.Get(0).(int)
+	}
+
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(postId)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // PermanentDeleteBatch provides a mock function with given fields: endTime, limit
 func (_m *ReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) {
 	ret := _m.Called(endTime, limit)
--- a/server/channels/store/storetest/reaction_store.go
+++ b/server/channels/store/storetest/reaction_store.go
@@ -29,6 +29,8 @@ func TestReactionStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStor
 	t.Run("PermanentDeleteBatch", func(t *testing.T) { testReactionStorePermanentDeleteBatch(t, rctx, ss) })
 	t.Run("ReactionBulkGetForPosts", func(t *testing.T) { testReactionBulkGetForPosts(t, rctx, ss) })
 	t.Run("ReactionDeadlock", func(t *testing.T) { testReactionDeadlock(t, rctx, ss) })
+	t.Run("ExistsOnPost", func(t *testing.T) { testExistsOnPost(t, rctx, ss) })
+	t.Run("GetUniqueCountForPost", func(t *testing.T) { testGetUniqueCountForPost(t, rctx, ss) })
 }

 func testReactionSave(t *testing.T, rctx request.CTX, ss store.Store) {
@@ -873,3 +875,66 @@ func testReactionDeadlock(t *testing.T, rctx request.CTX, ss store.Store) {
 	}()
 	wg.Wait()
 }
+
+func testExistsOnPost(t *testing.T, rctx request.CTX, ss store.Store) {
+	post, _ := ss.Post().Save(&model.Post{
+		ChannelId: model.NewId(),
+		UserId:    model.NewId(),
+	})
+	emojiName := model.NewId()
+	reaction := &model.Reaction{
+		UserId:    model.NewId(),
+		PostId:    post.Id,
+		EmojiName: emojiName,
+	}
+	_, nErr := ss.Reaction().Save(reaction)
+	require.NoError(t, nErr)
+	exists, err := ss.Reaction().ExistsOnPost(post.Id, emojiName)
+	require.NoError(t, err)
+	require.True(t, exists)
+	exists, err = ss.Reaction().ExistsOnPost(post.Id, model.NewId())
+	require.NoError(t, err)
+	require.False(t, exists)
+}
+
+func testGetUniqueCountForPost(t *testing.T, rctx request.CTX, ss store.Store) {
+	post, _ := ss.Post().Save(&model.Post{
+		ChannelId: model.NewId(),
+		UserId:    model.NewId(),
+	})
+
+	userId := model.NewId()
+	emojiName := model.NewId()
+
+	reaction := &model.Reaction{
+		UserId:    userId,
+		PostId:    post.Id,
+		EmojiName: emojiName,
+	}
+	_, nErr := ss.Reaction().Save(reaction)
+	require.NoError(t, nErr)
+
+	sameReaction := &model.Reaction{
+		UserId:    model.NewId(),
+		PostId:    post.Id,
+		EmojiName: emojiName,
+	}
+	_, nErr = ss.Reaction().Save(sameReaction)
+	require.NoError(t, nErr)
+
+	newReaction := &model.Reaction{
+		UserId:    userId,
+		PostId:    post.Id,
+		EmojiName: model.NewId(),
+	}
+	_, nErr = ss.Reaction().Save(newReaction)
+	require.NoError(t, nErr)
+
+	totalReactions, err := ss.Reaction().GetForPost(post.Id, false)
+	require.NoError(t, err)
+	require.Equal(t, 3, len(totalReactions))
+
+	count, err := ss.Reaction().GetUniqueCountForPost(post.Id)
+	require.NoError(t, err)
+	require.Equal(t, 2, count)
+}
--- a/server/channels/store/timerlayer/timerlayer.go
+++ b/server/channels/store/timerlayer/timerlayer.go
@@ -6745,6 +6745,22 @@ func (s *TimerLayerReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsF
 	return err
 }

+func (s *TimerLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) {
+	start := time.Now()
+
+	result, err := s.ReactionStore.ExistsOnPost(postId, emojiName)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("ReactionStore.ExistsOnPost", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) {
 	start := time.Now()

@@ -6777,6 +6793,22 @@ func (s *TimerLayerReactionStore) GetForPostSince(postId string, since int64, ex
 	return result, err
 }

+func (s *TimerLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) {
+	start := time.Now()
+
+	result, err := s.ReactionStore.GetUniqueCountForPost(postId)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("ReactionStore.GetUniqueCountForPost", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) {
 	start := time.Now()