MM-10833: send down computed channel props (#8953)

* MM-10833: send down computed channel props

This allows channel headers to reference channel mentions for a client
that doesn't already know about the channels in question.

We intentionally don't send down the props for the autocomplete and
search endpoints since they aren't used in that context, and would add
unnecessary overhead.

* update channel props on patch

* revert to treating channel purpose as plaintext

api4/channel.go

@@ -209,13 +209,20 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if rchannel, err := c.App.PatchChannel(oldChannel, patch, c.Session.UserId); err != nil {
+	rchannel, err := c.App.PatchChannel(oldChannel, patch, c.Session.UserId)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		c.LogAudit("")
-		w.Write([]byte(rchannel.ToJson()))
 	}
+
+	err = c.App.FillInChannelProps(rchannel)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	c.LogAudit("")
+	w.Write([]byte(rchannel.ToJson()))
 }
 
 func restoreChannel(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -361,6 +368,12 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	err = c.App.FillInChannelProps(channel)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
 	w.Write([]byte(channel.ToJson()))
 }
 
@@ -444,13 +457,19 @@ func getPublicChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	if channels, err := c.App.GetPublicChannelsForTeam(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage); err != nil {
+	channels, err := c.App.GetPublicChannelsForTeam(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		w.Write([]byte(channels.ToJson()))
+	}
+
+	err = c.App.FillInChannelsProps(channels)
+	if err != nil {
+		c.Err = err
 		return
 	}
+
+	w.Write([]byte(channels.ToJson()))
 }
 
 func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -464,13 +483,19 @@ func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	if channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage); err != nil {
+	channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		w.Write([]byte(channels.ToJson()))
+	}
+
+	err = c.App.FillInChannelsProps(channels)
+	if err != nil {
+		c.Err = err
 		return
 	}
+
+	w.Write([]byte(channels.ToJson()))
 }
 
 func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -497,12 +522,19 @@ func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	if channels, err := c.App.GetPublicChannelsByIdsForTeam(c.Params.TeamId, channelIds); err != nil {
+	channels, err := c.App.GetPublicChannelsByIdsForTeam(c.Params.TeamId, channelIds)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		w.Write([]byte(channels.ToJson()))
 	}
+
+	err = c.App.FillInChannelsProps(channels)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	w.Write([]byte(channels.ToJson()))
 }
 
 func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -521,15 +553,24 @@ func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	if channels, err := c.App.GetChannelsForUser(c.Params.TeamId, c.Params.UserId); err != nil {
+	channels, err := c.App.GetChannelsForUser(c.Params.TeamId, c.Params.UserId)
+	if err != nil {
 		c.Err = err
 		return
-	} else if c.HandleEtag(channels.Etag(), "Get Channels", w, r) {
-		return
-	} else {
-		w.Header().Set(model.HEADER_ETAG_SERVER, channels.Etag())
-		w.Write([]byte(channels.ToJson()))
 	}
+
+	if c.HandleEtag(channels.Etag(), "Get Channels", w, r) {
+		return
+	}
+
+	err = c.App.FillInChannelsProps(channels)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	w.Header().Set(model.HEADER_ETAG_SERVER, channels.Etag())
+	w.Write([]byte(channels.ToJson()))
 }
 
 func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -545,12 +586,15 @@ func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Requ
 
 	name := r.URL.Query().Get("name")
 
-	if channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name); err != nil {
+	channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		w.Write([]byte(channels.ToJson()))
 	}
+
+	// Don't fill in channels props, since unused by client and potentially expensive.
+
+	w.Write([]byte(channels.ToJson()))
 }
 
 func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -570,12 +614,15 @@ func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if channels, err := c.App.SearchChannels(c.Params.TeamId, props.Term); err != nil {
+	channels, err := c.App.SearchChannels(c.Params.TeamId, props.Term)
+	if err != nil {
 		c.Err = err
 		return
-	} else {
-		w.Write([]byte(channels.ToJson()))
 	}
+
+	// Don't fill in channels props, since unused by client and potentially expensive.
+
+	w.Write([]byte(channels.ToJson()))
 }
 
 func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -638,6 +685,12 @@ func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	err = c.App.FillInChannelProps(channel)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
 	w.Write([]byte(channel.ToJson()))
 }
 
@@ -660,6 +713,12 @@ func getChannelByNameForTeamName(c *Context, w http.ResponseWriter, r *http.Requ
 		return
 	}
 
+	err = c.App.FillInChannelProps(channel)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
 	w.Write([]byte(channel.ToJson()))
 }
 

app/channel.go

@@ -1591,3 +1591,67 @@ func (a *App) ToggleMuteChannel(channelId string, userId string) *model.ChannelM
 	a.Srv.Store.Channel().UpdateMember(member)
 	return member
 }
+
+func (a *App) FillInChannelProps(channel *model.Channel) *model.AppError {
+	return a.FillInChannelsProps(&model.ChannelList{channel})
+}
+
+func (a *App) FillInChannelsProps(channelList *model.ChannelList) *model.AppError {
+	// Group the channels by team and call GetChannelsByNames just once per team.
+	channelsByTeam := make(map[string]model.ChannelList)
+	for _, channel := range *channelList {
+		channelsByTeam[channel.TeamId] = append(channelsByTeam[channel.TeamId], channel)
+	}
+
+	for teamId, channelList := range channelsByTeam {
+		allChannelMentions := make(map[string]bool)
+		channelMentions := make(map[*model.Channel][]string, len(channelList))
+
+		// Collect mentions across the channels so as to query just once for this team.
+		for _, channel := range channelList {
+			channelMentions[channel] = model.ChannelMentions(channel.Header)
+
+			for _, channelMention := range channelMentions[channel] {
+				allChannelMentions[channelMention] = true
+			}
+		}
+
+		allChannelMentionNames := make([]string, 0, len(allChannelMentions))
+		for channelName := range allChannelMentions {
+			allChannelMentionNames = append(allChannelMentionNames, channelName)
+		}
+
+		if len(allChannelMentionNames) > 0 {
+			mentionedChannels, err := a.GetChannelsByNames(allChannelMentionNames, teamId)
+			if err != nil {
+				return err
+			}
+
+			mentionedChannelsByName := make(map[string]*model.Channel)
+			for _, channel := range mentionedChannels {
+				mentionedChannelsByName[channel.Name] = channel
+			}
+
+			for _, channel := range channelList {
+				channelMentionsProp := make(map[string]interface{}, len(channelMentions[channel]))
+				for _, channelMention := range channelMentions[channel] {
+					if mentioned, ok := mentionedChannelsByName[channelMention]; ok {
+						if mentioned.Type == model.CHANNEL_OPEN {
+							channelMentionsProp[mentioned.Name] = map[string]interface{}{
+								"display_name": mentioned.DisplayName,
+							}
+						}
+					}
+				}
+
+				if len(channelMentionsProp) > 0 {
+					channel.AddProp("channel_mentions", channelMentionsProp)
+				} else if channel.Props != nil {
+					delete(channel.Props, "channel_mentions")
+				}
+			}
+		}
+	}
+
+	return nil
+}

app/channel_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPermanentDeleteChannel(t *testing.T) {
@@ -399,3 +400,208 @@ func TestAppUpdateChannelScheme(t *testing.T) {
 		t.Fatal("Wrong Channel SchemeId")
 	}
 }
+
+func TestFillInChannelProps(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	channelPublic1, err := th.App.CreateChannel(&model.Channel{DisplayName: "Public 1", Name: "public1", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteChannel(channelPublic1)
+
+	channelPublic2, err := th.App.CreateChannel(&model.Channel{DisplayName: "Public 2", Name: "public2", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteChannel(channelPublic2)
+
+	channelPrivate, err := th.App.CreateChannel(&model.Channel{DisplayName: "Private", Name: "private", Type: model.CHANNEL_PRIVATE, TeamId: th.BasicTeam.Id}, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteChannel(channelPrivate)
+
+	otherTeamId := model.NewId()
+	otherTeam := &model.Team{
+		DisplayName: "dn_" + otherTeamId,
+		Name:        "name" + otherTeamId,
+		Email:       "success+" + otherTeamId + "@simulator.amazonses.com",
+		Type:        model.TEAM_OPEN,
+	}
+	otherTeam, err = th.App.CreateTeam(otherTeam)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteTeam(otherTeam)
+
+	channelOtherTeam, err := th.App.CreateChannel(&model.Channel{DisplayName: "Other Team Channel", Name: "other-team", Type: model.CHANNEL_OPEN, TeamId: otherTeam.Id}, false)
+	require.Nil(t, err)
+	defer th.App.PermanentDeleteChannel(channelOtherTeam)
+
+	// Note that purpose is intentionally plaintext below.
+
+	t.Run("single channels", func(t *testing.T) {
+		testCases := []struct {
+			Description          string
+			Channel              *model.Channel
+			ExpectedChannelProps map[string]interface{}
+		}{
+			{
+				"channel on basic team without references",
+				&model.Channel{
+					TeamId:  th.BasicTeam.Id,
+					Header:  "No references",
+					Purpose: "No references",
+				},
+				nil,
+			},
+			{
+				"channel on basic team",
+				&model.Channel{
+					TeamId:  th.BasicTeam.Id,
+					Header:  "~public1, ~private, ~other-team",
+					Purpose: "~public2, ~private, ~other-team",
+				},
+				map[string]interface{}{
+					"channel_mentions": map[string]interface{}{
+						"public1": map[string]interface{}{
+							"display_name": "Public 1",
+						},
+					},
+				},
+			},
+			{
+				"channel on other team",
+				&model.Channel{
+					TeamId:  otherTeam.Id,
+					Header:  "~public1, ~private, ~other-team",
+					Purpose: "~public2, ~private, ~other-team",
+				},
+				map[string]interface{}{
+					"channel_mentions": map[string]interface{}{
+						"other-team": map[string]interface{}{
+							"display_name": "Other Team Channel",
+						},
+					},
+				},
+			},
+		}
+
+		for _, testCase := range testCases {
+			t.Run(testCase.Description, func(t *testing.T) {
+				err = th.App.FillInChannelProps(testCase.Channel)
+				require.Nil(t, err)
+
+				assert.Equal(t, testCase.ExpectedChannelProps, testCase.Channel.Props)
+			})
+		}
+	})
+
+	t.Run("multiple channels", func(t *testing.T) {
+		testCases := []struct {
+			Description          string
+			Channels             *model.ChannelList
+			ExpectedChannelProps map[string]interface{}
+		}{
+			{
+				"single channel on basic team",
+				&model.ChannelList{
+					{
+						Name:    "test",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "~public1, ~private, ~other-team",
+						Purpose: "~public2, ~private, ~other-team",
+					},
+				},
+				map[string]interface{}{
+					"test": map[string]interface{}{
+						"channel_mentions": map[string]interface{}{
+							"public1": map[string]interface{}{
+								"display_name": "Public 1",
+							},
+						},
+					},
+				},
+			},
+			{
+				"multiple channels on basic team",
+				&model.ChannelList{
+					{
+						Name:    "test",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "~public1, ~private, ~other-team",
+						Purpose: "~public2, ~private, ~other-team",
+					},
+					{
+						Name:    "test2",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "~private, ~other-team",
+						Purpose: "~public2, ~private, ~other-team",
+					},
+					{
+						Name:    "test3",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "No references",
+						Purpose: "No references",
+					},
+				},
+				map[string]interface{}{
+					"test": map[string]interface{}{
+						"channel_mentions": map[string]interface{}{
+							"public1": map[string]interface{}{
+								"display_name": "Public 1",
+							},
+						},
+					},
+					"test2": map[string]interface{}(nil),
+					"test3": map[string]interface{}(nil),
+				},
+			},
+			{
+				"multiple channels across teams",
+				&model.ChannelList{
+					{
+						Name:    "test",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "~public1, ~private, ~other-team",
+						Purpose: "~public2, ~private, ~other-team",
+					},
+					{
+						Name:    "test2",
+						TeamId:  otherTeam.Id,
+						Header:  "~private, ~other-team",
+						Purpose: "~public2, ~private, ~other-team",
+					},
+					{
+						Name:    "test3",
+						TeamId:  th.BasicTeam.Id,
+						Header:  "No references",
+						Purpose: "No references",
+					},
+				},
+				map[string]interface{}{
+					"test": map[string]interface{}{
+						"channel_mentions": map[string]interface{}{
+							"public1": map[string]interface{}{
+								"display_name": "Public 1",
+							},
+						},
+					},
+					"test2": map[string]interface{}{
+						"channel_mentions": map[string]interface{}{
+							"other-team": map[string]interface{}{
+								"display_name": "Other Team Channel",
+							},
+						},
+					},
+					"test3": map[string]interface{}(nil),
+				},
+			},
+		}
+
+		for _, testCase := range testCases {
+			t.Run(testCase.Description, func(t *testing.T) {
+				err = th.App.FillInChannelsProps(testCase.Channels)
+				require.Nil(t, err)
+
+				for _, channel := range *testCase.Channels {
+					assert.Equal(t, testCase.ExpectedChannelProps[channel.Name], channel.Props)
+				}
+			})
+		}
+	})
+}

model/channel.go

@@ -32,21 +32,22 @@ const (
 )
 
 type Channel struct {
-	Id            string  `json:"id"`
-	CreateAt      int64   `json:"create_at"`
-	UpdateAt      int64   `json:"update_at"`
-	DeleteAt      int64   `json:"delete_at"`
-	TeamId        string  `json:"team_id"`
-	Type          string  `json:"type"`
-	DisplayName   string  `json:"display_name"`
-	Name          string  `json:"name"`
-	Header        string  `json:"header"`
-	Purpose       string  `json:"purpose"`
-	LastPostAt    int64   `json:"last_post_at"`
-	TotalMsgCount int64   `json:"total_msg_count"`
-	ExtraUpdateAt int64   `json:"extra_update_at"`
-	CreatorId     string  `json:"creator_id"`
-	SchemeId      *string `json:"scheme_id"`
+	Id            string                 `json:"id"`
+	CreateAt      int64                  `json:"create_at"`
+	UpdateAt      int64                  `json:"update_at"`
+	DeleteAt      int64                  `json:"delete_at"`
+	TeamId        string                 `json:"team_id"`
+	Type          string                 `json:"type"`
+	DisplayName   string                 `json:"display_name"`
+	Name          string                 `json:"name"`
+	Header        string                 `json:"header"`
+	Purpose       string                 `json:"purpose"`
+	LastPostAt    int64                  `json:"last_post_at"`
+	TotalMsgCount int64                  `json:"total_msg_count"`
+	ExtraUpdateAt int64                  `json:"extra_update_at"`
+	CreatorId     string                 `json:"creator_id"`
+	SchemeId      *string                `json:"scheme_id"`
+	Props         map[string]interface{} `json:"props" db:"-"`
 }
 
 type ChannelPatch struct {
@@ -163,6 +164,18 @@ func (o *Channel) Patch(patch *ChannelPatch) {
 	}
 }
 
+func (o *Channel) MakeNonNil() {
+	if o.Props == nil {
+		o.Props = make(map[string]interface{})
+	}
+}
+
+func (o *Channel) AddProp(key string, value interface{}) {
+	o.MakeNonNil()
+
+	o.Props[key] = value
+}
+
 func GetDMNameFromIds(userId1, userId2 string) string {
 	if userId1 > userId2 {
 		return userId2 + "__" + userId1

model/channel_mentions.go

@@ -0,0 +1,28 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package model
+
+import (
+	"regexp"
+	"strings"
+)
+
+var channelMentionRegexp = regexp.MustCompile(`\B~[a-zA-Z0-9\-_]+`)
+
+func ChannelMentions(message string) []string {
+	var names []string
+
+	if strings.Contains(message, "~") {
+		alreadyMentioned := make(map[string]bool)
+		for _, match := range channelMentionRegexp.FindAllString(message, -1) {
+			name := match[1:]
+			if !alreadyMentioned[name] {
+				names = append(names, name)
+				alreadyMentioned[name] = true
+			}
+		}
+	}
+
+	return names
+}

model/post.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"io"
 	"net/http"
-	"regexp"
 	"sort"
 	"strings"
 	"unicode/utf8"
@@ -343,20 +342,8 @@ func PostPatchFromJson(data io.Reader) *PostPatch {
 	return &post
 }
 
-var channelMentionRegexp = regexp.MustCompile(`\B~[a-zA-Z0-9\-_]+`)
-
-func (o *Post) ChannelMentions() (names []string) {
-	if strings.Contains(o.Message, "~") {
-		alreadyMentioned := make(map[string]bool)
-		for _, match := range channelMentionRegexp.FindAllString(o.Message, -1) {
-			name := match[1:]
-			if !alreadyMentioned[name] {
-				names = append(names, name)
-				alreadyMentioned[name] = true
-			}
-		}
-	}
-	return
+func (o *Post) ChannelMentions() []string {
+	return ChannelMentions(o.Message)
 }
 
 func (r *PostActionIntegrationRequest) ToJson() string {