[MM-16709] Add helper method for plugins using MessageHasBeenPosted (#12539)

The PR introduces a method in helper interface. The method has common code for filtering to be used across plugins which use MessageHasBeenPosted.

plugin/helpers.go

@@ -57,6 +57,18 @@ type Helpers interface {
 	//
 	// Minimum server version: 5.6
 	KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error
+
+	// ShouldProcessMessage returns if the message should be processed by a message hook.
+	//
+	// Use this method to avoid processing unnecessary messages in a MessageHasBeenPosted
+	// or MessageWillBePosted hook, and indeed in some cases avoid an infinite loop between
+	// two automated bots or plugins.
+	//
+	// The behaviour is customizable using the given options, since plugin needs may vary.
+	// By default, system messages and messages from bots will be skipped.
+	//
+	// Minimum server version: 5.2
+	ShouldProcessMessage(post *model.Post, options ...ShouldProcessMessageOption) (bool, error)
 }
 
 // HelpersImpl implements the helpers interface with an API that retrieves data on behalf of the plugin.

plugin/helpers_bots.go

@@ -32,19 +32,6 @@ func IconImagePath(path string) EnsureBotOption {
 	}
 }
 
-func (p *HelpersImpl) readFile(path string) ([]byte, error) {
-	bundlePath, err := p.API.GetBundlePath()
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to get bundle path")
-	}
-
-	imageBytes, err := ioutil.ReadFile(filepath.Join(bundlePath, path))
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to read image")
-	}
-	return imageBytes, nil
-}
-
 // EnsureBot implements Helpers.EnsureBot
 func (p *HelpersImpl) EnsureBot(bot *model.Bot, options ...EnsureBotOption) (retBotID string, retErr error) {
 	err := p.ensureServerVersion("5.10.0")
@@ -74,6 +61,129 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot, options ...EnsureBotOption) (ret
 	return botID, nil
 }
 
+type ShouldProcessMessageOption func(*shouldProcessMessageOptions)
+
+type shouldProcessMessageOptions struct {
+	AllowSystemMessages bool
+	AllowBots           bool
+	FilterChannelIDs    []string
+	FilterUserIDs       []string
+	OnlyBotDMs          bool
+}
+
+// AllowSystemMessages configures a call to ShouldProcessMessage to return true for system messages.
+//
+// As it is typically desirable only to consume messages from users of the system, ShouldProcessMessage ignores system messages by default.
+func AllowSystemMessages() ShouldProcessMessageOption {
+	return func(options *shouldProcessMessageOptions) {
+		options.AllowSystemMessages = true
+	}
+}
+
+// AllowBots configures a call to ShouldProcessMessage to return true for bot posts.
+//
+// As it is typically desirable only to consume messages from human users of the system, ShouldProcessMessage ignores bot messages by default. When allowing bots, take care to avoid a loop where two plugins respond to each others posts repeatedly.
+func AllowBots() ShouldProcessMessageOption {
+	return func(options *shouldProcessMessageOptions) {
+		options.AllowBots = true
+	}
+}
+
+// FilterChannelIDs configures a call to ShouldProcessMessage to return true only for the given channels.
+//
+// By default, posts from all channels are allowed to be processed.
+func FilterChannelIDs(filterChannelIDs []string) ShouldProcessMessageOption {
+	return func(options *shouldProcessMessageOptions) {
+		options.FilterChannelIDs = filterChannelIDs
+	}
+}
+
+// FilterUserIDs configures a call to ShouldProcessMessage to return true only for the given users.
+//
+// By default, posts from all non-bot users are allowed.
+func FilterUserIDs(filterUserIDs []string) ShouldProcessMessageOption {
+	return func(options *shouldProcessMessageOptions) {
+		options.FilterUserIDs = filterUserIDs
+	}
+}
+
+// OnlyBotDMs configures a call to ShouldProcessMessage to return true only for direct messages sent to the bot created by EnsureBot.
+//
+// By default, posts from all channels are allowed.
+func OnlyBotDMs() ShouldProcessMessageOption {
+	return func(options *shouldProcessMessageOptions) {
+		options.OnlyBotDMs = true
+	}
+}
+
+// ShouldProcessMessage implements Helpers.ShouldProcessMessage
+func (p *HelpersImpl) ShouldProcessMessage(post *model.Post, options ...ShouldProcessMessageOption) (bool, error) {
+	messageProcessOptions := &shouldProcessMessageOptions{}
+	for _, option := range options {
+		option(messageProcessOptions)
+	}
+
+	botIdBytes, kvGetErr := p.API.KVGet(BOT_USER_KEY)
+	if kvGetErr != nil {
+		return false, errors.Wrap(kvGetErr, "failed to get bot")
+	}
+
+	if botIdBytes != nil {
+		if post.UserId == string(botIdBytes) {
+			return false, nil
+		}
+	}
+
+	if post.IsSystemMessage() && !messageProcessOptions.AllowSystemMessages {
+		return false, nil
+	}
+
+	if !messageProcessOptions.AllowBots {
+		user, appErr := p.API.GetUser(post.UserId)
+		if appErr != nil {
+			return false, errors.Wrap(appErr, "unable to get user")
+		}
+
+		if user.IsBot {
+			return false, nil
+		}
+	}
+
+	if len(messageProcessOptions.FilterChannelIDs) != 0 && !utils.StringInSlice(post.ChannelId, messageProcessOptions.FilterChannelIDs) {
+		return false, nil
+	}
+
+	if len(messageProcessOptions.FilterUserIDs) != 0 && !utils.StringInSlice(post.UserId, messageProcessOptions.FilterUserIDs) {
+		return false, nil
+	}
+
+	if botIdBytes != nil && messageProcessOptions.OnlyBotDMs {
+		channel, appErr := p.API.GetChannel(post.ChannelId)
+		if appErr != nil {
+			return false, errors.Wrap(appErr, "unable to get channel")
+		}
+
+		if !model.IsBotDMChannel(channel, string(botIdBytes)) {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
+func (p *HelpersImpl) readFile(path string) ([]byte, error) {
+	bundlePath, err := p.API.GetBundlePath()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get bundle path")
+	}
+
+	imageBytes, err := ioutil.ReadFile(filepath.Join(bundlePath, path))
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to read image")
+	}
+	return imageBytes, nil
+}
+
 func (p *HelpersImpl) ensureBot(bot *model.Bot) (retBotID string, retErr error) {
 	// Must provide a bot with a username
 	if bot == nil || len(bot.Username) < 1 {

plugin/helpers_bots_test.go

@@ -340,3 +340,111 @@ func TestEnsureBot(t *testing.T) {
 		})
 	})
 }
+
+func TestShouldProcessMessage(t *testing.T) {
+	p := &plugin.HelpersImpl{}
+	expectedBotId := model.NewId()
+
+	setupAPI := func() *plugintest.API {
+		return &plugintest.API{}
+	}
+
+	t.Run("should not respond to itself", func(t *testing.T) {
+		api := setupAPI()
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+		p.API = api
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{Type: model.POST_HEADER_CHANGE, UserId: expectedBotId}, plugin.AllowSystemMessages(), plugin.AllowBots())
+
+		assert.False(t, shouldProcessMessage)
+	})
+
+	t.Run("should not process as the post is generated by system", func(t *testing.T) {
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{Type: model.POST_HEADER_CHANGE})
+
+		assert.False(t, shouldProcessMessage)
+	})
+
+	t.Run("should not process as the post is sent to another channel", func(t *testing.T) {
+		channelID := "channel-id"
+		api := setupAPI()
+		api.On("GetChannel", channelID).Return(&model.Channel{Id: channelID, Type: model.CHANNEL_GROUP}, nil)
+		p.API = api
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{ChannelId: channelID}, plugin.AllowSystemMessages(), plugin.AllowBots(), plugin.FilterChannelIDs([]string{"another-channel-id"}))
+
+		assert.False(t, shouldProcessMessage)
+	})
+
+	t.Run("should not process as the post is created by bot", func(t *testing.T) {
+		userID := "user-id"
+		channelID := "1"
+		api := setupAPI()
+		p.API = api
+		api.On("GetUser", userID).Return(&model.User{IsBot: true}, nil)
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: userID, ChannelId: channelID},
+			plugin.AllowSystemMessages(), plugin.FilterUserIDs([]string{"another-user-id"}))
+
+		assert.False(t, shouldProcessMessage)
+	})
+
+	t.Run("should not process the message as the post is not in bot dm channel", func(t *testing.T) {
+		userID := "user-id"
+		channelID := "1"
+		channel := model.Channel{
+			Name: "user1__" + expectedBotId,
+			Type: model.CHANNEL_OPEN,
+		}
+		api := setupAPI()
+		api.On("GetChannel", channelID).Return(&channel, nil)
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+		p.API = api
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: userID, ChannelId: channelID}, plugin.AllowSystemMessages(), plugin.AllowBots(), plugin.OnlyBotDMs())
+
+		assert.False(t, shouldProcessMessage)
+	})
+
+	t.Run("should process the message", func(t *testing.T) {
+		channelID := "1"
+		api := setupAPI()
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+		p.API = api
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID},
+			plugin.AllowSystemMessages(), plugin.FilterChannelIDs([]string{channelID}), plugin.AllowBots(), plugin.FilterUserIDs([]string{"1"}))
+
+		assert.True(t, shouldProcessMessage)
+	})
+
+	t.Run("should process the message for plugin without a bot", func(t *testing.T) {
+		channelID := "1"
+		api := setupAPI()
+		api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil)
+		p.API = api
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID},
+			plugin.AllowSystemMessages(), plugin.FilterChannelIDs([]string{channelID}), plugin.AllowBots(), plugin.FilterUserIDs([]string{"1"}))
+
+		assert.True(t, shouldProcessMessage)
+	})
+
+	t.Run("should process the message when filter channel and filter users list is empty", func(t *testing.T) {
+		channelID := "1"
+		api := setupAPI()
+		channel := model.Channel{
+			Name: "user1__" + expectedBotId,
+			Type: model.CHANNEL_DIRECT,
+		}
+		api.On("GetChannel", channelID).Return(&channel, nil)
+		api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil)
+		p.API = api
+
+		shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID},
+			plugin.AllowSystemMessages(), plugin.AllowBots())
+
+		assert.True(t, shouldProcessMessage)
+	})
+}

plugin/plugintest/helpers.go

@@ -133,3 +133,31 @@ func (_m *Helpers) KVSetWithExpiryJSON(key string, value interface{}, expireInSe
 
 	return r0
 }
+
+// ShouldProcessMessage provides a mock function with given fields: post, options
+func (_m *Helpers) ShouldProcessMessage(post *model.Post, options ...plugin.ShouldProcessMessageOption) (bool, error) {
+	_va := make([]interface{}, len(options))
+	for _i := range options {
+		_va[_i] = options[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, post)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func(*model.Post, ...plugin.ShouldProcessMessageOption) bool); ok {
+		r0 = rf(post, options...)
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.Post, ...plugin.ShouldProcessMessageOption) error); ok {
+		r1 = rf(post, options...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}