PLT-5073 Improve performance of /channels/view endpoint (#4881)

* Improve performance of /channels/view endpoint

* Fix store unit test

api/channel.go

@@ -919,10 +919,6 @@ func SetActiveChannel(userId string, channelId string) *model.AppError {
 
 	AddStatusCache(status)
 
-	if result := <-Srv.Store.Status().SaveOrUpdate(status); result.Err != nil {
-		return result.Err
-	}
-
 	return nil
 }
 
@@ -1067,7 +1063,7 @@ func addMember(c *Context, w http.ResponseWriter, r *http.Request) {
 
 			go PostUserAddRemoveMessage(c, channel.Id, fmt.Sprintf(utils.T("api.channel.add_member.added"), nUser.Username, oUser.Username), model.POST_ADD_REMOVE)
 
-			<-Srv.Store.Channel().UpdateLastViewedAt(id, oUser.Id)
+			<-Srv.Store.Channel().UpdateLastViewedAt([]string{id}, oUser.Id)
 			w.Write([]byte(cm.ToJson()))
 		}
 	}
@@ -1258,47 +1254,48 @@ func autocompleteChannels(c *Context, w http.ResponseWriter, r *http.Request) {
 func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	view := model.ChannelViewFromJson(r.Body)
 
-	go func() {
+	if err := SetActiveChannel(c.Session.UserId, view.ChannelId); err != nil {
-		if err := SetActiveChannel(c.Session.UserId, view.ChannelId); err != nil {
+		c.Err = err
-			l4g.Error(err.Error())
+		return
-		}
-	}()
-
-	if len(view.ChannelId) > 0 {
-
-		if view.Time == 0 {
-			if result := <-Srv.Store.Channel().UpdateLastViewedAt(view.ChannelId, c.Session.UserId); result.Err != nil {
-				c.Err = result.Err
-				return
-			}
-		} else {
-			if result := <-Srv.Store.Channel().SetLastViewedAt(view.ChannelId, c.Session.UserId, view.Time); result.Err != nil {
-				c.Err = result.Err
-				return
-			}
-		}
-
-		if len(view.PrevChannelId) > 0 {
-			Srv.Store.Channel().UpdateLastViewedAt(view.PrevChannelId, c.Session.UserId)
-
-			// Only clear push notifications if a channel switch occured
-			if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() {
-				go func() {
-					if result := <-Srv.Store.User().GetUnreadCountForChannel(c.Session.UserId, view.ChannelId); result.Err != nil {
-						l4g.Error(utils.T("api.channel.update_last_viewed_at.get_unread_count_for_channel.error"), c.Session.UserId, view.ChannelId, result.Err.Error())
-					} else {
-						if result.Data.(int64) > 0 {
-							clearPushNotification(c.Session.UserId, view.ChannelId)
-						}
-					}
-				}()
-			}
-		}
-
-		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
-		message.Add("channel_id", view.ChannelId)
 	}
 
+	if len(view.ChannelId) == 0 {
+		ReturnStatusOK(w)
+		return
+	}
+
+	channelIds := []string{view.ChannelId}
+
+	var pchan store.StoreChannel
+	if len(view.PrevChannelId) > 0 {
+		channelIds = append(channelIds, view.PrevChannelId)
+
+		if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() {
+			pchan = Srv.Store.User().GetUnreadCountForChannel(c.Session.UserId, view.ChannelId)
+		}
+	}
+
+	uchan := Srv.Store.Channel().UpdateLastViewedAt(channelIds, c.Session.UserId)
+
+	if pchan != nil {
+		if result := <-pchan; result.Err != nil {
+			c.Err = result.Err
+			return
+		} else {
+			if result.Data.(int64) > 0 {
+				clearPushNotification(c.Session.UserId, view.ChannelId)
+			}
+		}
+	}
+
+	if result := <-uchan; result.Err != nil {
+		c.Err = result.Err
+		return
+	}
+
+	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
+	message.Add("channel_id", view.ChannelId)
+
 	ReturnStatusOK(w)
 }
 

api/channel_test.go

@@ -1862,14 +1862,12 @@ func TestViewChannel(t *testing.T) {
 	}
 
 	view.PrevChannelId = ""
-	view.Time = 1234567890
 
 	if _, resp := Client.ViewChannel(view); resp.Error != nil {
 		t.Fatal(resp.Error)
 	}
 
 	view.PrevChannelId = "junk"
-	view.Time = 0
 
 	if _, resp := Client.ViewChannel(view); resp.Error != nil {
 		t.Fatal(resp.Error)
@@ -1878,6 +1876,8 @@ func TestViewChannel(t *testing.T) {
 	rdata := Client.Must(Client.GetChannel(th.BasicChannel.Id, "")).Data.(*model.ChannelData)
 
 	if rdata.Channel.TotalMsgCount != rdata.Member.MsgCount {
+		t.Log(rdata.Channel.Id)
+		t.Log(rdata.Member.UserId)
 		t.Log(rdata.Channel.TotalMsgCount)
 		t.Log(rdata.Member.MsgCount)
 		t.Fatal("message counts don't match")

api/deprecated.go

@@ -76,7 +76,7 @@ func updateLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	Srv.Store.Channel().UpdateLastViewedAt(id, c.Session.UserId)
+	Srv.Store.Channel().UpdateLastViewedAt([]string{id}, c.Session.UserId)
 
 	// Must be after update so that unread count is correct
 	if doClearPush {

api/post.go

@@ -95,7 +95,7 @@ func createPost(c *Context, w http.ResponseWriter, r *http.Request) {
 	} else {
 		// Update the LastViewAt only if the post does not have from_webhook prop set (eg. Zapier app)
 		if _, ok := post.Props["from_webhook"]; !ok {
-			if result := <-Srv.Store.Channel().UpdateLastViewedAt(post.ChannelId, c.Session.UserId); result.Err != nil {
+			if result := <-Srv.Store.Channel().UpdateLastViewedAt([]string{post.ChannelId}, c.Session.UserId); result.Err != nil {
 				l4g.Error(utils.T("api.post.create_post.last_viewed.error"), post.ChannelId, c.Session.UserId, result.Err)
 			}
 		}

model/channel_view.go

@@ -11,7 +11,6 @@ import (
 type ChannelView struct {
 	ChannelId     string `json:"channel_id"`
 	PrevChannelId string `json:"prev_channel_id"`
-	Time          int64  `json:"time"`
 }
 
 func (o *ChannelView) ToJson() string {

model/client.go

@@ -1350,10 +1350,9 @@ func (c *Client) UpdateLastViewedAt(channelId string, active bool) (*Result, *Ap
 }
 
 // ViewChannel performs all the actions related to viewing a channel. This includes marking
-// the channel and the previous one as read, marking the channel as being actively viewed.
+// the channel and the previous one as read, and marking the channel as being actively viewed.
 // ChannelId is required but may be blank to indicate no channel is being viewed.
-// PrevChannelId is optional, populate to indicate a channel switch occurred. Optionally
+// PrevChannelId is optional, populate to indicate a channel switch occurred.
-// provide a non-zero Time, in Unix milliseconds, to manually set the viewing time.
 func (c *Client) ViewChannel(params ChannelView) (bool, *ResponseMetadata) {
 	if r, err := c.DoApiPost(c.GetTeamRoute()+"/channels/view", params.ToJson()); err != nil {
 		return false, &ResponseMetadata{StatusCode: r.StatusCode, Error: err}

model/status.go

@@ -22,7 +22,7 @@ type Status struct {
 	Status         string `json:"status"`
 	Manual         bool   `json:"manual"`
 	LastActivityAt int64  `json:"last_activity_at"`
-	ActiveChannel  string `json:"active_channel"`
+	ActiveChannel  string `json:"active_channel" db:"-"`
 }
 
 func (o *Status) ToJson() string {

store/sql_channel_store.go

@@ -6,6 +6,7 @@ package store
 import (
 	"database/sql"
 	"fmt"
+	"strconv"
 	"strings"
 
 	l4g "github.com/alecthomas/log4go"
@@ -13,7 +14,6 @@ import (
 	"github.com/mattermost/platform/einterfaces"
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/utils"
-	"strconv"
 )
 
 const (
@@ -961,13 +961,24 @@ func (s SqlChannelStore) SetLastViewedAt(channelId string, userId string, newLas
 	return storeChannel
 }
 
-func (s SqlChannelStore) UpdateLastViewedAt(channelId string, userId string) StoreChannel {
+func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string) StoreChannel {
 	storeChannel := make(StoreChannel, 1)
 
 	go func() {
 		result := StoreResult{}
 
 		var query string
+		props := make(map[string]interface{})
+
+		idQuery := ""
+		for index, channelId := range channelIds {
+			if len(idQuery) > 0 {
+				idQuery += " OR "
+			}
+
+			props["channelId"+strconv.Itoa(index)] = channelId
+			idQuery += "ChannelId = :channelId" + strconv.Itoa(index)
+		}
 
 		if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 			query = `UPDATE
@@ -982,7 +993,7 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelId string, userId string) Sto
 			WHERE
 			    Channels.Id = ChannelMembers.ChannelId
 			        AND UserId = :UserId
-			        AND ChannelId = :ChannelId`
+			        AND (` + idQuery + `)`
 		} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 			query = `UPDATE
 				ChannelMembers, Channels
@@ -994,12 +1005,14 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelId string, userId string) Sto
 			WHERE
 			    Channels.Id = ChannelMembers.ChannelId
 			        AND UserId = :UserId
-			        AND ChannelId = :ChannelId`
+			        AND (` + idQuery + `)`
 		}
 
-		_, err := s.GetMaster().Exec(query, map[string]interface{}{"ChannelId": channelId, "UserId": userId})
+		props["UserId"] = userId
+
+		_, err := s.GetMaster().Exec(query, props)
 		if err != nil {
-			result.Err = model.NewLocAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error())
+			result.Err = model.NewLocAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_ids="+strings.Join(channelIds, ",")+", user_id="+userId+", "+err.Error())
 		}
 
 		storeChannel <- result

store/sql_channel_store_test.go

@@ -808,12 +808,12 @@ func TestChannelStoreUpdateLastViewedAt(t *testing.T) {
 	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
 	Must(store.Channel().SaveMember(&m1))
 
-	err := (<-store.Channel().UpdateLastViewedAt(m1.ChannelId, m1.UserId)).Err
+	err := (<-store.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, m1.UserId)).Err
 	if err != nil {
 		t.Fatal("failed to update", err)
 	}
 
-	err = (<-store.Channel().UpdateLastViewedAt(m1.ChannelId, "missing id")).Err
+	err = (<-store.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, "missing id")).Err
 	if err != nil {
 		t.Fatal("failed to update")
 	}

store/sql_upgrade.go

@@ -225,6 +225,9 @@ func UpgradeDatabaseToVersion36(sqlStore *SqlStore) {
 	// Add a Position column to users.
 	sqlStore.CreateColumnIfNotExists("Users", "Position", "varchar(64)", "varchar(64)", "")
 
+	// Remove ActiveChannel column from Status
+	sqlStore.RemoveColumnIfExists("Status", "ActiveChannel")
+
 	//saveSchemaVersion(sqlStore, VERSION_3_6_0)
 	//}
 }

store/store.go

@@ -110,7 +110,7 @@ type ChannelStore interface {
 	GetMemberCount(channelId string, allowFromCache bool) StoreChannel
 	RemoveMember(channelId string, userId string) StoreChannel
 	PermanentDeleteMembersByUser(userId string) StoreChannel
-	UpdateLastViewedAt(channelId string, userId string) StoreChannel
+	UpdateLastViewedAt(channelIds []string, userId string) StoreChannel
 	SetLastViewedAt(channelId string, userId string, newLastViewedAt int64) StoreChannel
 	IncrementMentionCount(channelId string, userId string) StoreChannel
 	AnalyticsTypeCount(teamId string, channelType string) StoreChannel

webapp/client/client.jsx

@@ -1397,7 +1397,7 @@ export default class Client {
             end(this.handleResponse.bind(this, 'updateLastViewedAt', success, error));
     }
 
-    // SCHEDULED FOR DEPRECATION IN 3.8 - use viewChannel instead
+    // SCHEDULED FOR DEPRECATION IN 3.8
     setLastViewedAt(channelId, lastViewedAt, success, error) {
         request.
         post(`${this.getChannelNeededRoute(channelId)}/set_last_viewed_at`).