MM-21987 Resolve mentions in slash commands (#13762)

* Create infrastructure to manage mentions

Two new files have been added (along with their tests); namely:

- model/at_mentions.go: utilities to parse and manage mentions; for the moment,
it just contains a regex and a couple of functions to parse possible mentions
and to post-process them, but it can be extended in the future.
- model/mention_map.go: it contains two new types (UserMentionMap and
ChannelMentionMap) that both have FromURLValues and ToURLValues. These types
can be used when adding the mentions to the payload of the plugin slash
commands.

* Extend custom commands payload with mentions

Two couples of new fields are added to the payload; namely:

- user_mentions and user_mentions_ids: two aligned arrays of the same length
containing all the different @-mentions found in the command: the i-th element
of user_mentions_ids is the user identifier of the i-th element of
user_mentions.
- channel_mentions and channel_mentions_ids: two aligned arrays of the same
length containing all the different ~-mentions found in the command: the i-th
element of channel_mentions_ids is the channel identifier of the i-th element
of channel_mentions.

* Fix shadowing of variables and redundant return

* Fix shadowing of variable

* Address review comments (HT @lieut-data)

- Improvements in mentionsToTeamMembers and mentionsToPublicChannels:
	- Scope implementation details inside the functions.
	- Improve goroutines synchronization by using a sync.WaitGroup.
	- Retry lookup of username only if the returned error is http.StatusCode,
	  so we can return early if the error is more severe.
- Invert check in PossibleAtMentions to improve readability.
- Make user and channel mention keys private to the module.
- Allow the specification of an empty map of mentions in
(Channel|User)MentionsFromURLValues when both mentions keys are absent.
- Replace custom functions in tests with require.Equal on maps.

* Test functions to parse mentions from messages

* Extend plugin commands payload with mentions

* Add functions to CommandArgs to add mentions

The functions make sure that the maps are initialized before adding any value.

* Address review comments (HT @lieut-data)

- Adds a mlog.Warn to avoid burying the error when the user is not found.
- Improve readability in loop populating the mention map by moving the
initialization of the map closer to the loop and by iterating over the channel
itself, not over its length.

* File was not gofmt-ed with -s

* Close channel when all goroutines are finished

* Again, all code should be checked with gofmt -s

* Refactor code out of a goroutine

This change helps improve the readability of the code and does not affect its
overall performance. Less complexity is always better.

* Close channel and iterate over its range

Adapt mentionsToPublicChannels to have the same structure in the management
of the mentions channel as in mentionsToTeamMembers.

* Adapt mentionsToTeamMembers to new App

Commit 17523fa changed the App structure, making the *Server field
private, which is now accessed through the Srv() function.

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

model/at_mentions.go

@@ -0,0 +1,47 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"regexp"
+	"strings"
+)
+
+var atMentionRegexp = regexp.MustCompile(`\B@[[:alnum:]][[:alnum:]\.\-_]*`)
+
+const usernameSpecialChars = ".-_"
+
+// PossibleAtMentions returns all substrings in message that look like valid @
+// mentions.
+func PossibleAtMentions(message string) []string {
+	var names []string
+
+	if !strings.Contains(message, "@") {
+		return names
+	}
+
+	alreadyMentioned := make(map[string]bool)
+	for _, match := range atMentionRegexp.FindAllString(message, -1) {
+		name := NormalizeUsername(match[1:])
+		if !alreadyMentioned[name] && IsValidUsername(name) {
+			names = append(names, name)
+			alreadyMentioned[name] = true
+		}
+	}
+
+	return names
+}
+
+// TrimUsernameSpecialChar tries to remove the last character from word if it
+// is a special character for usernames (dot, dash or underscore). If not, it
+// returns the same string.
+func TrimUsernameSpecialChar(word string) (string, bool) {
+	len := len(word)
+
+	if len > 0 && strings.LastIndexAny(word, usernameSpecialChars) == (len-1) {
+		return word[:len-1], true
+	}
+
+	return word, false
+}

model/at_mentions_test.go

@@ -0,0 +1,84 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPossibleAtMentions(t *testing.T) {
+	fixture := []struct {
+		message  string
+		expected []string
+	}{
+		{
+			"",
+			[]string{},
+		},
+		{
+			"@user",
+			[]string{"user"},
+		},
+		{
+			"@user-with_special.chars @multiple.-_chars",
+			[]string{"user-with_special.chars", "multiple.-_chars"},
+		},
+		{
+			"@repeated @user @repeated",
+			[]string{"repeated", "user"},
+		},
+		{
+			"@user1 @user2 @user3",
+			[]string{"user1", "user2", "user3"},
+		},
+		{
+			"@李",
+			[]string{},
+		},
+		{
+			"@withfinaldot. @withfinaldash- @withfinalunderscore_",
+			[]string{
+				"withfinaldot.",
+				"withfinaldash-",
+				"withfinalunderscore_",
+			},
+		},
+	}
+
+	for _, data := range fixture {
+		actual := PossibleAtMentions(data.message)
+		require.ElementsMatch(t, actual, data.expected)
+	}
+}
+
+func TestTrimUsernameSpecialChar(t *testing.T) {
+	fixture := []struct {
+		word           string
+		expectedString string
+		expectedBool   bool
+	}{
+		{"user...", "user..", true},
+		{"user..", "user.", true},
+		{"user.", "user", true},
+		{"user--", "user-", true},
+		{"user-", "user", true},
+		{"user_.-", "user_.", true},
+		{"user_.", "user_", true},
+		{"user_", "user", true},
+		{"user", "user", false},
+		{"user.with-inner_chars", "user.with.inner.chars", false},
+	}
+
+	for _, data := range fixture {
+		actualString, actualBool := TrimUsernameSpecialChar(data.word)
+		require.Equal(t, actualBool, data.expectedBool)
+		if actualBool {
+			require.Equal(t, actualString, data.expectedString)
+		} else {
+			require.Equal(t, actualString, data.word)
+		}
+	}
+}

model/channel.go

@@ -243,6 +243,10 @@ func (o *Channel) IsGroupOrDirect() bool {
 	return o.Type == CHANNEL_DIRECT || o.Type == CHANNEL_GROUP
 }
 
+func (o *Channel) IsOpen() bool {
+	return o.Type == CHANNEL_OPEN
+}
+
 func (o *Channel) Patch(patch *ChannelPatch) {
 	if patch.DisplayName != nil {
 		o.DisplayName = *patch.DisplayName

model/command_args.go

@@ -11,16 +11,18 @@ import (
 )
 
 type CommandArgs struct {
-	UserId    string               `json:"user_id"`
-	ChannelId string               `json:"channel_id"`
-	TeamId    string               `json:"team_id"`
-	RootId    string               `json:"root_id"`
-	ParentId  string               `json:"parent_id"`
-	TriggerId string               `json:"trigger_id,omitempty"`
-	Command   string               `json:"command"`
-	SiteURL   string               `json:"-"`
-	T         goi18n.TranslateFunc `json:"-"`
-	Session   Session              `json:"-"`
+	UserId          string               `json:"user_id"`
+	ChannelId       string               `json:"channel_id"`
+	TeamId          string               `json:"team_id"`
+	RootId          string               `json:"root_id"`
+	ParentId        string               `json:"parent_id"`
+	TriggerId       string               `json:"trigger_id,omitempty"`
+	Command         string               `json:"command"`
+	SiteURL         string               `json:"-"`
+	T               goi18n.TranslateFunc `json:"-"`
+	Session         Session              `json:"-"`
+	UserMentions    UserMentionMap       `json:"-"`
+	ChannelMentions ChannelMentionMap    `json:"-"`
 }
 
 func (o *CommandArgs) ToJson() string {
@@ -33,3 +35,23 @@ func CommandArgsFromJson(data io.Reader) *CommandArgs {
 	json.NewDecoder(data).Decode(&o)
 	return o
 }
+
+// AddUserMention adds or overrides an entry in UserMentions with name username
+// and identifier userId
+func (o *CommandArgs) AddUserMention(username, userId string) {
+	if o.UserMentions == nil {
+		o.UserMentions = make(UserMentionMap)
+	}
+
+	o.UserMentions[username] = userId
+}
+
+// AddChannelMention adds or overrides an entry in ChannelMentions with name
+// channelName and identifier channelId
+func (o *CommandArgs) AddChannelMention(channelName, channelId string) {
+	if o.ChannelMentions == nil {
+		o.ChannelMentions = make(ChannelMentionMap)
+	}
+
+	o.ChannelMentions[channelName] = channelId
+}

model/command_args_test.go

@@ -0,0 +1,108 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCommandArgs_AddUserMention(t *testing.T) {
+	fixture := []struct {
+		args     CommandArgs
+		mentions map[string]string
+		expected CommandArgs
+	}{
+		{
+			CommandArgs{},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				UserMentions: map[string]string{"one": "1"},
+			},
+		},
+		{
+			CommandArgs{
+				ChannelMentions: map[string]string{"channel": "1"},
+			},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				UserMentions:    map[string]string{"one": "1"},
+				ChannelMentions: map[string]string{"channel": "1"},
+			},
+		},
+		{
+			CommandArgs{
+				UserMentions: map[string]string{"one": "1"},
+			},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				UserMentions: map[string]string{"one": "1"},
+			},
+		},
+		{
+			CommandArgs{},
+			map[string]string{"one": "1", "two": "2", "three": "3"},
+			CommandArgs{
+				UserMentions: map[string]string{"one": "1", "two": "2", "three": "3"},
+			},
+		},
+	}
+
+	for _, data := range fixture {
+		for name, id := range data.mentions {
+			data.args.AddUserMention(name, id)
+		}
+		require.Equal(t, data.args, data.expected)
+	}
+}
+
+func TestCommandArgs_AddChannelMention(t *testing.T) {
+	fixture := []struct {
+		args     CommandArgs
+		mentions map[string]string
+		expected CommandArgs
+	}{
+		{
+			CommandArgs{},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				ChannelMentions: map[string]string{"one": "1"},
+			},
+		},
+		{
+			CommandArgs{
+				UserMentions: map[string]string{"user": "1"},
+			},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				ChannelMentions: map[string]string{"one": "1"},
+				UserMentions:    map[string]string{"user": "1"},
+			},
+		},
+		{
+			CommandArgs{
+				ChannelMentions: map[string]string{"one": "1"},
+			},
+			map[string]string{"one": "1"},
+			CommandArgs{
+				ChannelMentions: map[string]string{"one": "1"},
+			},
+		},
+		{
+			CommandArgs{},
+			map[string]string{"one": "1", "two": "2", "three": "3"},
+			CommandArgs{
+				ChannelMentions: map[string]string{"one": "1", "two": "2", "three": "3"},
+			},
+		},
+	}
+
+	for _, data := range fixture {
+		for name, id := range data.mentions {
+			data.args.AddChannelMention(name, id)
+		}
+		require.Equal(t, data.args, data.expected)
+	}
+}

model/mention_map.go

@@ -0,0 +1,80 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"fmt"
+	"net/url"
+)
+
+type UserMentionMap map[string]string
+type ChannelMentionMap map[string]string
+
+const (
+	userMentionsKey       = "user_mentions"
+	userMentionsIdsKey    = "user_mentions_ids"
+	channelMentionsKey    = "channel_mentions"
+	channelMentionsIdsKey = "channel_mentions_ids"
+)
+
+func UserMentionMapFromURLValues(values url.Values) (UserMentionMap, error) {
+	return mentionsFromURLValues(values, userMentionsKey, userMentionsIdsKey)
+}
+
+func (m UserMentionMap) ToURLValues() url.Values {
+	return mentionsToURLValues(m, userMentionsKey, userMentionsIdsKey)
+}
+
+func ChannelMentionMapFromURLValues(values url.Values) (ChannelMentionMap, error) {
+	return mentionsFromURLValues(values, channelMentionsKey, channelMentionsIdsKey)
+}
+
+func (m ChannelMentionMap) ToURLValues() url.Values {
+	return mentionsToURLValues(m, channelMentionsKey, channelMentionsIdsKey)
+}
+
+func mentionsFromURLValues(values url.Values, mentionKey, idKey string) (map[string]string, error) {
+	mentions, mentionsOk := values[mentionKey]
+	ids, idsOk := values[idKey]
+
+	if !mentionsOk && !idsOk {
+		return map[string]string{}, nil
+	}
+
+	if !mentionsOk {
+		return nil, fmt.Errorf("%s key not found", mentionKey)
+	}
+
+	if !idsOk {
+		return nil, fmt.Errorf("%s key not found", idKey)
+	}
+
+	if len(mentions) != len(ids) {
+		return nil, fmt.Errorf("keys %s and %s have different length", mentionKey, idKey)
+	}
+
+	mentionsMap := make(map[string]string)
+	for i, mention := range mentions {
+		id := ids[i]
+
+		if oldId, ok := mentionsMap[mention]; ok && oldId != id {
+			return nil, fmt.Errorf("key %s has two different values: %s and %s", mention, oldId, id)
+		}
+
+		mentionsMap[mention] = id
+	}
+
+	return mentionsMap, nil
+}
+
+func mentionsToURLValues(mentions map[string]string, mentionKey, idKey string) url.Values {
+	values := url.Values{}
+
+	for mention, id := range mentions {
+		values.Add(mentionKey, mention)
+		values.Add(idKey, id)
+	}
+
+	return values
+}

model/mention_map_test.go

@@ -0,0 +1,235 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUserMentionMapFromURLValues(t *testing.T) {
+	fixture := []struct {
+		values   url.Values
+		expected UserMentionMap
+		error    bool
+	}{
+		{
+			url.Values{},
+			UserMentionMap{},
+			false,
+		},
+		{
+			url.Values{
+				userMentionsKey:    []string{},
+				userMentionsIdsKey: []string{},
+			},
+			UserMentionMap{},
+			false,
+		},
+		{
+			url.Values{
+				userMentionsKey:    []string{"one", "two", "three"},
+				userMentionsIdsKey: []string{"oneId", "twoId", "threeId"},
+			},
+			UserMentionMap{
+				"one":   "oneId",
+				"two":   "twoId",
+				"three": "threeId",
+			},
+			false,
+		},
+		{
+			url.Values{
+				"wrongKey":         []string{"one", "two", "three"},
+				userMentionsIdsKey: []string{"oneId", "twoId", "threeId"},
+			},
+			nil,
+			true,
+		},
+		{
+			url.Values{
+				userMentionsKey: []string{"one", "two", "three"},
+				"wrongKey":      []string{"oneId", "twoId", "threeId"},
+			},
+			nil,
+			true,
+		},
+		{
+			url.Values{
+				userMentionsKey:    []string{"one", "two"},
+				userMentionsIdsKey: []string{"justone"},
+			},
+			nil,
+			true,
+		},
+	}
+
+	for _, data := range fixture {
+		actualMap, actualError := UserMentionMapFromURLValues(data.values)
+		if data.error {
+			require.Error(t, actualError)
+			require.Nil(t, actualMap)
+		} else {
+			require.NoError(t, actualError)
+			require.Equal(t, actualMap, data.expected)
+		}
+	}
+}
+
+func TestUserMentionMap_ToURLValues(t *testing.T) {
+	fixture := []struct {
+		mentionMap UserMentionMap
+		expected   url.Values
+	}{
+		{
+			UserMentionMap{},
+			url.Values{},
+		},
+		{
+			UserMentionMap{"user": "id"},
+			url.Values{
+				userMentionsKey:    []string{"user"},
+				userMentionsIdsKey: []string{"id"},
+			},
+		},
+		{
+			UserMentionMap{"one": "id1", "two": "id2", "three": "id3"},
+			url.Values{
+				userMentionsKey:    []string{"one", "two", "three"},
+				userMentionsIdsKey: []string{"id1", "id2", "id3"},
+			},
+		},
+	}
+
+	for _, data := range fixture {
+		actualValues := data.mentionMap.ToURLValues()
+
+		// require.EqualValues does not work here directly on the url.Values, as
+		// the slices in the map values may be in different order; what we need to
+		// check is that the pairs are preserved, which can be checked converting
+		// back to a map with FromURLValues. We check that the test is well-formed
+		// by converting back the expected url.Values too.
+		require.Equal(t, len(actualValues), len(data.expected))
+
+		actualMentionMap, actualErr := UserMentionMapFromURLValues(actualValues)
+		expectedMentionMap, expectedErr := UserMentionMapFromURLValues(data.expected)
+
+		require.Equal(t, actualErr, expectedErr)
+		require.Equal(t, actualMentionMap, expectedMentionMap)
+	}
+}
+
+func TestChannelMentionMapFromURLValues(t *testing.T) {
+	fixture := []struct {
+		values   url.Values
+		expected ChannelMentionMap
+		error    bool
+	}{
+		{
+			url.Values{},
+			ChannelMentionMap{},
+			false,
+		},
+		{
+			url.Values{
+				channelMentionsKey:    []string{},
+				channelMentionsIdsKey: []string{},
+			},
+			ChannelMentionMap{},
+			false,
+		},
+		{
+			url.Values{
+				channelMentionsKey:    []string{"one", "two", "three"},
+				channelMentionsIdsKey: []string{"oneId", "twoId", "threeId"},
+			},
+			ChannelMentionMap{
+				"one":   "oneId",
+				"two":   "twoId",
+				"three": "threeId",
+			},
+			false,
+		},
+		{
+			url.Values{
+				"wrongKey":            []string{"one", "two", "three"},
+				channelMentionsIdsKey: []string{"oneId", "twoId", "threeId"},
+			},
+			nil,
+			true,
+		},
+		{
+			url.Values{
+				channelMentionsKey: []string{"one", "two", "three"},
+				"wrongKey":         []string{"oneId", "twoId", "threeId"},
+			},
+			nil,
+			true,
+		},
+		{
+			url.Values{
+				channelMentionsKey:    []string{"one", "two"},
+				channelMentionsIdsKey: []string{"justone"},
+			},
+			nil,
+			true,
+		},
+	}
+
+	for _, data := range fixture {
+		actualMap, actualError := ChannelMentionMapFromURLValues(data.values)
+		if data.error {
+			require.Error(t, actualError)
+			require.Nil(t, actualMap)
+		} else {
+			require.NoError(t, actualError)
+			require.Equal(t, actualMap, data.expected)
+		}
+	}
+}
+
+func TestChannelMentionMap_ToURLValues(t *testing.T) {
+	fixture := []struct {
+		mentionMap ChannelMentionMap
+		expected   url.Values
+	}{
+		{
+			ChannelMentionMap{},
+			url.Values{},
+		},
+		{
+			ChannelMentionMap{"user": "id"},
+			url.Values{
+				channelMentionsKey:    []string{"user"},
+				channelMentionsIdsKey: []string{"id"},
+			},
+		},
+		{
+			ChannelMentionMap{"one": "id1", "two": "id2", "three": "id3"},
+			url.Values{
+				channelMentionsKey:    []string{"one", "two", "three"},
+				channelMentionsIdsKey: []string{"id1", "id2", "id3"},
+			},
+		},
+	}
+
+	for _, data := range fixture {
+		actualValues := data.mentionMap.ToURLValues()
+
+		// require.EqualValues does not work here directly on the url.Values, as
+		// the slices in the map values may be in different order; what we need to
+		// check is that the pairs are preserved, which can be checked converting
+		// back to a map with FromURLValues. We check that the test is well-formed
+		// by converting back the expected url.Values too.
+		require.Equal(t, len(actualValues), len(data.expected))
+
+		actualMentionMap, actualErr := ChannelMentionMapFromURLValues(actualValues)
+		expectedMentionMap, expectedErr := ChannelMentionMapFromURLValues(data.expected)
+
+		require.Equal(t, actualErr, expectedErr)
+		require.Equal(t, actualMentionMap, expectedMentionMap)
+	}
+}