From b212acf312ad640455fa715427ac19e6930dc61d Mon Sep 17 00:00:00 2001 From: Corey Hulen Date: Thu, 24 Nov 2016 05:26:45 -0800 Subject: PLT-4429 disabling at_all at_channel metions mentions when channel has more than 1k users (#4627) * PLT-4429 disabling explicit mentions when channel has more than 1k users * Fixing test case * Adding setting to the admin console * Fixing bad translation --- api/channel.go | 15 ++--- api/post.go | 76 ++++++++++++++++++++-- api/post_test.go | 26 ++++---- api/team.go | 2 +- api/web_conn.go | 12 ++++ api/web_hub.go | 5 ++ config/config.json | 3 +- i18n/en.json | 16 +++++ model/config.go | 10 +++ store/sql_channel_store.go | 36 ++++++++-- store/sql_channel_store_test.go | 26 +++++--- store/store.go | 3 +- .../admin_console/users_and_teams_settings.jsx | 22 ++++++- webapp/i18n/en.json | 3 + webapp/utils/constants.jsx | 1 + 15 files changed, 211 insertions(+), 45 deletions(-) diff --git a/api/channel.go b/api/channel.go index ea39ee398..9ec556fe6 100644 --- a/api/channel.go +++ b/api/channel.go @@ -580,7 +580,7 @@ func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM go func() { InvalidateCacheForUser(user.Id) - Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id) + InvalidateCacheForChannel(channel.Id) message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_ADDED, "", channel.Id, "", nil) message.Add("user_id", user.Id) @@ -625,7 +625,7 @@ func JoinDefaultChannels(teamId string, user *model.User, channelRole string) *m l4g.Error(utils.T("api.channel.post_user_add_remove_message_and_forget.error"), err) } - Srv.Store.User().InvalidateProfilesInChannelCache(result.Data.(*model.Channel).Id) + InvalidateCacheForChannel(result.Data.(*model.Channel).Id) } if result := <-Srv.Store.Channel().GetByName(teamId, "off-topic"); result.Err != nil { @@ -649,7 +649,7 @@ func JoinDefaultChannels(teamId string, user *model.User, channelRole string) *m l4g.Error(utils.T("api.channel.post_user_add_remove_message_and_forget.error"), err) } - Srv.Store.User().InvalidateProfilesInChannelCache(result.Data.(*model.Channel).Id) + InvalidateCacheForChannel(result.Data.(*model.Channel).Id) } return err @@ -662,7 +662,7 @@ func leave(c *Context, w http.ResponseWriter, r *http.Request) { sc := Srv.Store.Channel().Get(id) uc := Srv.Store.User().Get(c.Session.UserId) - ccm := Srv.Store.Channel().GetMemberCount(id) + ccm := Srv.Store.Channel().GetMemberCount(id, false) if cresult := <-sc; cresult.Err != nil { c.Err = cresult.Err @@ -718,7 +718,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) { sc := Srv.Store.Channel().Get(id) scm := Srv.Store.Channel().GetMember(id, c.Session.UserId) - cmc := Srv.Store.Channel().GetMemberCount(id) + cmc := Srv.Store.Channel().GetMemberCount(id, false) uc := Srv.Store.User().Get(c.Session.UserId) ihc := Srv.Store.Webhook().GetIncomingByChannel(id) ohc := Srv.Store.Webhook().GetOutgoingByChannel(id) @@ -949,7 +949,7 @@ func getChannelStats(c *Context, w http.ResponseWriter, r *http.Request) { channel = result.Data.(*model.Channel) } - if result := <-Srv.Store.Channel().GetMemberCount(id); result.Err != nil { + if result := <-Srv.Store.Channel().GetMemberCount(id, true); result.Err != nil { c.Err = result.Err return } else { @@ -1107,7 +1107,6 @@ func removeMember(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(model.MapToJson(result))) } } - } func RemoveUserFromChannel(userIdToRemove string, removerUserId string, channel *model.Channel) *model.AppError { @@ -1120,7 +1119,7 @@ func RemoveUserFromChannel(userIdToRemove string, removerUserId string, channel } InvalidateCacheForUser(userIdToRemove) - Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id) + InvalidateCacheForChannel(channel.Id) message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_REMOVED, "", channel.Id, "", nil) message.Add("user_id", userIdToRemove) diff --git a/api/post.go b/api/post.go index ad6c2fdbe..c646b056a 100644 --- a/api/post.go +++ b/api/post.go @@ -475,7 +475,7 @@ func getMentionKeywordsInChannel(profiles map[string]*model.User) map[string][]s } // Add @channel and @all to keywords if user has them turned on - if profile.NotifyProps["channel"] == "true" { + if int64(len(profiles)) < *utils.Cfg.TeamSettings.MaxNotificationsPerChannel && profile.NotifyProps["channel"] == "true" { keywords["@channel"] = append(keywords["@channel"], profile.Id) keywords["@all"] = append(keywords["@all"], profile.Id) } @@ -486,11 +486,13 @@ func getMentionKeywordsInChannel(profiles map[string]*model.User) map[string][]s // Given a message and a map mapping mention keywords to the users who use them, returns a map of mentioned // users and a slice of potencial mention users not in the channel and whether or not @here was mentioned. -func getExplicitMentions(message string, keywords map[string][]string) (map[string]bool, []string, bool) { +func getExplicitMentions(message string, keywords map[string][]string) (map[string]bool, []string, bool, bool, bool) { mentioned := make(map[string]bool) potentialOthersMentioned := make([]string, 0) systemMentions := map[string]bool{"@here": true, "@channel": true, "@all": true} hereMentioned := false + allMentioned := false + channelMentioned := false addMentionedUsers := func(ids []string) { for _, id := range ids { @@ -505,6 +507,14 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri hereMentioned = true } + if word == "@channel" { + channelMentioned = true + } + + if word == "@all" { + allMentioned = true + } + // Non-case-sensitive check for regular keys if ids, match := keywords[strings.ToLower(word)]; match { addMentionedUsers(ids) @@ -529,6 +539,14 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri hereMentioned = true } + if splitWord == "@all" { + allMentioned = true + } + + if splitWord == "@channel" { + channelMentioned = true + } + // Non-case-sensitive check for regular keys if ids, match := keywords[strings.ToLower(splitWord)]; match { addMentionedUsers(ids) @@ -545,7 +563,7 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri } } - return mentioned, potentialOthersMentioned, hereMentioned + return mentioned, potentialOthersMentioned, hereMentioned, channelMentioned, allMentioned } func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel) []string { @@ -569,6 +587,8 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * mentionedUserIds := make(map[string]bool) allActivityPushUserIds := []string{} hereNotification := false + channelNotification := false + allNotification := false updateMentionChans := []store.StoreChannel{} if channel.Type == model.CHANNEL_DIRECT { @@ -584,7 +604,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * keywords := getMentionKeywordsInChannel(profileMap) var potentialOtherMentions []string - mentionedUserIds, potentialOtherMentions, hereNotification = getExplicitMentions(post.Message, keywords) + mentionedUserIds, potentialOtherMentions, hereNotification, channelNotification, allNotification = getExplicitMentions(post.Message, keywords) // get users that have comment thread mentions enabled if len(post.RootId) > 0 { @@ -652,7 +672,13 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * var status *model.Status var err *model.AppError if status, err = GetStatus(id); err != nil { - status = &model.Status{id, model.STATUS_OFFLINE, false, 0, ""} + status = &model.Status{ + UserId: id, + Status: model.STATUS_OFFLINE, + Manual: false, + LastActivityAt: 0, + ActiveChannel: "", + } } if userAllowsEmails && status.Status != model.STATUS_ONLINE { @@ -661,6 +687,46 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * } } + // If the channel has more than 1K users then @here is disabled + if hereNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel { + hereNotification = false + SendEphemeralPost( + c.TeamId, + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: utils.T("api.post.disabled_here", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } + + // If the channel has more than 1K users then @channel is disabled + if channelNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel { + SendEphemeralPost( + c.TeamId, + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: utils.T("api.post.disabled_channel", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } + + // If the channel has more than 1K users then @all is disabled + if allNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel { + SendEphemeralPost( + c.TeamId, + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: utils.T("api.post.disabled_all", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } + if hereNotification { if result := <-Srv.Store.Status().GetOnline(); result.Err != nil { l4g.Warn(utils.T("api.post.notification.here.warn"), result.Err) diff --git a/api/post_test.go b/api/post_test.go index bedb3aa74..0e340561c 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -1062,7 +1062,7 @@ func TestGetExplicitMentionsAtHere(t *testing.T) { } for message, shouldMention := range cases { - if _, _, hereMentioned := getExplicitMentions(message, nil); hereMentioned && !shouldMention { + if _, _, hereMentioned, _, _ := getExplicitMentions(message, nil); hereMentioned && !shouldMention { t.Fatalf("shouldn't have mentioned @here with \"%v\"", message) } else if !hereMentioned && shouldMention { t.Fatalf("should've have mentioned @here with \"%v\"", message) @@ -1071,7 +1071,7 @@ func TestGetExplicitMentionsAtHere(t *testing.T) { // mentioning @here and someone id := model.NewId() - if mentions, potential, hereMentioned := getExplicitMentions("@here @user @potential", map[string][]string{"@user": {id}}); !hereMentioned { + if mentions, potential, hereMentioned, _, _ := getExplicitMentions("@here @user @potential", map[string][]string{"@user": {id}}); !hereMentioned { t.Fatal("should've mentioned @here with \"@here @user\"") } else if len(mentions) != 1 || !mentions[id] { t.Fatal("should've mentioned @user with \"@here @user\"") @@ -1087,74 +1087,74 @@ func TestGetExplicitMentions(t *testing.T) { // not mentioning anybody message := "this is a message" keywords := map[string][]string{} - if mentions, potential, _ := getExplicitMentions(message, keywords); len(mentions) != 0 || len(potential) != 0 { + if mentions, potential, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 || len(potential) != 0 { t.Fatal("shouldn't have mentioned anybody or have any potencial mentions") } // mentioning a user that doesn't exist message = "this is a message for @user" - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 { t.Fatal("shouldn't have mentioned user that doesn't exist") } // mentioning one person keywords = map[string][]string{"@user": {id1}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] { t.Fatal("should've mentioned @user") } // mentioning one person without an @mention message = "this is a message for @user" keywords = map[string][]string{"this": {id1}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] { t.Fatal("should've mentioned this") } // mentioning multiple people with one word message = "this is a message for @user" keywords = map[string][]string{"@user": {id1, id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { t.Fatal("should've mentioned two users with @user") } // mentioning only one of multiple people keywords = map[string][]string{"@user": {id1}, "@mention": {id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] { t.Fatal("should've mentioned @user and not @mention") } // mentioning multiple people with multiple words message = "this is an @mention for @user" keywords = map[string][]string{"@user": {id1}, "@mention": {id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { t.Fatal("should've mentioned two users with @user and @mention") } // mentioning @channel (not a special case, but it's good to double check) message = "this is an message for @channel" keywords = map[string][]string{"@channel": {id1, id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { t.Fatal("should've mentioned two users with @channel") } // mentioning @all (not a special case, but it's good to double check) message = "this is an message for @all" keywords = map[string][]string{"@all": {id1, id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] { t.Fatal("should've mentioned two users with @all") } // mentioning user.period without mentioning user (PLT-3222) message = "user.period doesn't complicate things at all by including periods in their username" keywords = map[string][]string{"user.period": {id1}, "user": {id2}} - if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] { + if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] { t.Fatal("should've mentioned user.period and not user") } // mentioning a potential out of channel user message = "this is an message for @potential and @user" keywords = map[string][]string{"@user": {id1}} - if mentions, potential, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || len(potential) != 1 { + if mentions, potential, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || len(potential) != 1 { t.Fatal("should've mentioned user and have a potential not in channel") } } diff --git a/api/team.go b/api/team.go index e7db79ae2..8cfb4fe77 100644 --- a/api/team.go +++ b/api/team.go @@ -339,7 +339,7 @@ func LeaveTeam(team *model.Team, user *model.User) *model.AppError { for _, channel := range *channelList { if channel.Type != model.CHANNEL_DIRECT { - Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id) + InvalidateCacheForChannel(channel.Id) if result := <-Srv.Store.Channel().RemoveMember(channel.Id, user.Id); result.Err != nil { return result.Err } diff --git a/api/web_conn.go b/api/web_conn.go index f145950b3..a2b801904 100644 --- a/api/web_conn.go +++ b/api/web_conn.go @@ -187,6 +187,18 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool { // Only report events to users who are in the channel for the event if len(msg.Broadcast.ChannelId) > 0 { + // Only broadcast typing messages if less than 1K people in channel + if msg.Event == model.WEBSOCKET_EVENT_TYPING { + if result := <-Srv.Store.Channel().GetMemberCount(msg.Broadcast.ChannelId, true); result.Err != nil { + l4g.Error("webhub.shouldSendEvent: " + result.Err.Error()) + return false + } else { + if result.Data.(int64) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel { + return false + } + } + } + if model.GetMillis()-webCon.LastAllChannelMembersTime > 1000*60*15 { // 15 minutes webCon.AllChannelMembers = nil webCon.LastAllChannelMembersTime = 0 diff --git a/api/web_hub.go b/api/web_hub.go index b607703f2..4136eaf7c 100644 --- a/api/web_hub.go +++ b/api/web_hub.go @@ -102,6 +102,11 @@ func PublishSkipClusterSend(message *model.WebSocketEvent) { } } +func InvalidateCacheForChannel(channelId string) { + Srv.Store.User().InvalidateProfilesInChannelCache(channelId) + Srv.Store.Channel().InvalidateMemberCount(channelId) +} + func InvalidateCacheForUser(userId string) { InvalidateCacheForUserSkipClusterSend(userId) diff --git a/config/config.json b/config/config.json index dbf2eacf7..0815aae30 100644 --- a/config/config.json +++ b/config/config.json @@ -51,7 +51,8 @@ "RestrictPublicChannelManagement": "all", "RestrictPrivateChannelManagement": "all", "UserStatusAwayTimeout": 300, - "MaxChannelsPerTeam": 2000 + "MaxChannelsPerTeam": 2000, + "MaxNotificationsPerChannel": 1000 }, "SqlSettings": { "DriverName": "mysql", diff --git a/i18n/en.json b/i18n/en.json index 673944ade..734145e75 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -675,6 +675,18 @@ "id": "api.context.invalid_param.app_error", "translation": "Invalid {{.Name}} parameter" }, + { + "id": "api.post.disabled_here", + "translation": "@here has been disabled because the channel has more than {{.Users}} users." + }, + { + "id": "api.post.disabled_all", + "translation": "@all has been disabled because the channel has more than {{.Users}} users." + }, + { + "id": "api.post.disabled_channel", + "translation": "@channel has been disabled because the channel has more than {{.Users}} users." + }, { "id": "api.context.invalid_team_url.debug", "translation": "TeamURL accessed when not valid. Team URL should not be used in api functions or those that are team independent" @@ -3331,6 +3343,10 @@ "id": "model.config.is_valid.max_channels.app_error", "translation": "Invalid maximum channels per team for team settings. Must be a positive number." }, + { + "id": "model.config.is_valid.max_notify_per_channel.app_error", + "translation": "Invalid maximum notifications per channel for team settings. Must be a positive number." + }, { "id": "model.config.is_valid.max_file_size.app_error", "translation": "Invalid max file size for file settings. Must be a zero or positive number." diff --git a/model/config.go b/model/config.go index 40c76efe5..f9a1d4861 100644 --- a/model/config.go +++ b/model/config.go @@ -226,6 +226,7 @@ type TeamSettings struct { RestrictPrivateChannelManagement *string UserStatusAwayTimeout *int64 MaxChannelsPerTeam *int64 + MaxNotificationsPerChannel *int64 } type LdapSettings struct { @@ -507,6 +508,11 @@ func (o *Config) SetDefaults() { *o.TeamSettings.MaxChannelsPerTeam = 2000 } + if o.TeamSettings.MaxNotificationsPerChannel == nil { + o.TeamSettings.MaxNotificationsPerChannel = new(int64) + *o.TeamSettings.MaxNotificationsPerChannel = 1000 + } + if o.EmailSettings.EnableSignInWithEmail == nil { o.EmailSettings.EnableSignInWithEmail = new(bool) @@ -1003,6 +1009,10 @@ func (o *Config) IsValid() *AppError { return NewLocAppError("Config.IsValid", "model.config.is_valid.max_channels.app_error", nil, "") } + if *o.TeamSettings.MaxNotificationsPerChannel <= 0 { + return NewLocAppError("Config.IsValid", "model.config.is_valid.max_notify_per_channel.app_error", nil, "") + } + if !(*o.TeamSettings.RestrictDirectMessage == DIRECT_MESSAGE_ANY || *o.TeamSettings.RestrictDirectMessage == DIRECT_MESSAGE_TEAM) { return NewLocAppError("Config.IsValid", "model.config.is_valid.restrict_direct_message.app_error", nil, "") } diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go index aadeed7c3..207484532 100644 --- a/store/sql_channel_store.go +++ b/store/sql_channel_store.go @@ -13,18 +13,23 @@ import ( ) const ( - MISSING_CHANNEL_ERROR = "store.sql_channel.get_by_name.missing.app_error" - MISSING_CHANNEL_MEMBER_ERROR = "store.sql_channel.get_member.missing.app_error" - CHANNEL_EXISTS_ERROR = "store.sql_channel.save_channel.exists.app_error" + MISSING_CHANNEL_ERROR = "store.sql_channel.get_by_name.missing.app_error" + MISSING_CHANNEL_MEMBER_ERROR = "store.sql_channel.get_member.missing.app_error" + CHANNEL_EXISTS_ERROR = "store.sql_channel.save_channel.exists.app_error" + ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE = model.SESSION_CACHE_SIZE ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SEC = 900 // 15 mins + + CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = 20000 + CHANNEL_MEMBERS_COUNTS_CACHE_SEC = 900 // 15 mins ) type SqlChannelStore struct { *SqlStore } -var allChannelMembersForUserCache *utils.Cache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE) +var channelMemberCountsCache = utils.NewLru(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE) +var allChannelMembersForUserCache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE) func NewSqlChannelStore(sqlStore *SqlStore) ChannelStore { s := &SqlChannelStore{sqlStore} @@ -395,7 +400,7 @@ func (s SqlChannelStore) GetMoreChannels(teamId string, userId string) StoreChan data := &model.ChannelList{} _, err := s.GetReplica().Select(data, - `SELECT + `SELECT * FROM Channels @@ -403,7 +408,7 @@ func (s SqlChannelStore) GetMoreChannels(teamId string, userId string) StoreChan TeamId = :TeamId1 AND Type IN ('O') AND DeleteAt = 0 - AND Id NOT IN (SELECT + AND Id NOT IN (SELECT Channels.Id FROM Channels, @@ -751,12 +756,25 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac return storeChannel } -func (s SqlChannelStore) GetMemberCount(channelId string) StoreChannel { +func (us SqlChannelStore) InvalidateMemberCount(channelId string) { + channelMemberCountsCache.Remove(channelId) +} + +func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) StoreChannel { storeChannel := make(StoreChannel, 1) go func() { result := StoreResult{} + if allowFromCache { + if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok { + result.Data = cacheItem.(int64) + storeChannel <- result + close(storeChannel) + return + } + } + count, err := s.GetReplica().SelectInt(` SELECT count(*) @@ -771,6 +789,10 @@ func (s SqlChannelStore) GetMemberCount(channelId string) StoreChannel { result.Err = model.NewLocAppError("SqlChannelStore.GetMemberCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error()) } else { result.Data = count + + if allowFromCache { + channelMemberCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_MEMBERS_COUNTS_CACHE_SEC) + } } storeChannel <- result diff --git a/store/sql_channel_store_test.go b/store/sql_channel_store_test.go index 8a51d2ae3..6776a438b 100644 --- a/store/sql_channel_store_test.go +++ b/store/sql_channel_store_test.go @@ -384,14 +384,24 @@ func TestChannelMemberStore(t *testing.T) { t.Fatal("Member update time incorrect") } - count := (<-store.Channel().GetMemberCount(o1.ChannelId)).Data.(int64) + count := (<-store.Channel().GetMemberCount(o1.ChannelId, true)).Data.(int64) + if count != 2 { + t.Fatal("should have saved 2 members") + } + + count = (<-store.Channel().GetMemberCount(o1.ChannelId, true)).Data.(int64) + if count != 2 { + t.Fatal("should have saved 2 members") + } + + count = (<-store.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 2 { t.Fatal("should have saved 2 members") } Must(store.Channel().RemoveMember(o2.ChannelId, o2.UserId)) - count = (<-store.Channel().GetMemberCount(o1.ChannelId)).Data.(int64) + count = (<-store.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 1 { t.Fatal("should have removed 1 member") } @@ -463,14 +473,14 @@ func TestChannelDeleteMemberStore(t *testing.T) { t.Fatal("Member update time incorrect") } - count := (<-store.Channel().GetMemberCount(o1.ChannelId)).Data.(int64) + count := (<-store.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 2 { t.Fatal("should have saved 2 members") } Must(store.Channel().PermanentDeleteMembersByUser(o2.UserId)) - count = (<-store.Channel().GetMemberCount(o1.ChannelId)).Data.(int64) + count = (<-store.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 1 { t.Fatal("should have removed 1 member") } @@ -927,7 +937,7 @@ func TestGetMemberCount(t *testing.T) { } Must(store.Channel().SaveMember(&m1)) - if result := <-store.Channel().GetMemberCount(c1.Id); result.Err != nil { + if result := <-store.Channel().GetMemberCount(c1.Id, false); result.Err != nil { t.Fatal("failed to get member count: %v", result.Err) } else if result.Data.(int64) != 1 { t.Fatal("got incorrect member count %v", result.Data) @@ -947,7 +957,7 @@ func TestGetMemberCount(t *testing.T) { } Must(store.Channel().SaveMember(&m2)) - if result := <-store.Channel().GetMemberCount(c1.Id); result.Err != nil { + if result := <-store.Channel().GetMemberCount(c1.Id, false); result.Err != nil { t.Fatal("failed to get member count: %v", result.Err) } else if result.Data.(int64) != 2 { t.Fatal("got incorrect member count %v", result.Data) @@ -968,7 +978,7 @@ func TestGetMemberCount(t *testing.T) { } Must(store.Channel().SaveMember(&m3)) - if result := <-store.Channel().GetMemberCount(c1.Id); result.Err != nil { + if result := <-store.Channel().GetMemberCount(c1.Id, false); result.Err != nil { t.Fatal("failed to get member count: %v", result.Err) } else if result.Data.(int64) != 2 { t.Fatal("got incorrect member count %v", result.Data) @@ -989,7 +999,7 @@ func TestGetMemberCount(t *testing.T) { } Must(store.Channel().SaveMember(&m4)) - if result := <-store.Channel().GetMemberCount(c1.Id); result.Err != nil { + if result := <-store.Channel().GetMemberCount(c1.Id, false); result.Err != nil { t.Fatal("failed to get member count: %v", result.Err) } else if result.Data.(int64) != 2 { t.Fatal("got incorrect member count %v", result.Data) diff --git a/store/store.go b/store/store.go index d0790263a..94c8416bd 100644 --- a/store/store.go +++ b/store/store.go @@ -103,7 +103,8 @@ type ChannelStore interface { InvalidateAllChannelMembersForUser(userId string) IsUserInChannelUseCache(userId string, channelId string) bool GetMemberForPost(postId string, userId string) StoreChannel - GetMemberCount(channelId string) StoreChannel + InvalidateMemberCount(channelId string) + GetMemberCount(channelId string, allowFromCache bool) StoreChannel RemoveMember(channelId string, userId string) StoreChannel PermanentDeleteMembersByUser(userId string) StoreChannel UpdateLastViewedAt(channelId string, userId string) StoreChannel diff --git a/webapp/components/admin_console/users_and_teams_settings.jsx b/webapp/components/admin_console/users_and_teams_settings.jsx index dd19005c8..2cb5b4e51 100644 --- a/webapp/components/admin_console/users_and_teams_settings.jsx +++ b/webapp/components/admin_console/users_and_teams_settings.jsx @@ -32,6 +32,7 @@ export default class UsersAndTeamsSettings extends AdminSettings { config.TeamSettings.RestrictCreationToDomains = this.state.restrictCreationToDomains; config.TeamSettings.RestrictDirectMessage = this.state.restrictDirectMessage; config.TeamSettings.MaxChannelsPerTeam = this.parseIntNonZero(this.state.maxChannelsPerTeam, Constants.DEFAULT_MAX_CHANNELS_PER_TEAM); + config.TeamSettings.MaxNotificationsPerChannel = this.parseIntNonZero(this.state.maxNotificationsPerChannel, Constants.DEFAULT_MAX_NOTIFICATIONS_PER_CHANNEL); return config; } @@ -43,7 +44,8 @@ export default class UsersAndTeamsSettings extends AdminSettings { maxUsersPerTeam: config.TeamSettings.MaxUsersPerTeam, restrictCreationToDomains: config.TeamSettings.RestrictCreationToDomains, restrictDirectMessage: config.TeamSettings.RestrictDirectMessage, - maxChannelsPerTeam: config.TeamSettings.MaxChannelsPerTeam + maxChannelsPerTeam: config.TeamSettings.MaxChannelsPerTeam, + maxNotificationsPerChannel: config.TeamSettings.MaxNotificationsPerChannel }; } @@ -131,6 +133,24 @@ export default class UsersAndTeamsSettings extends AdminSettings { value={this.state.maxChannelsPerTeam} onChange={this.handleChange} /> + + } + placeholder={Utils.localizeMessage('admin.team.maxNotificationsPerChannelExample', 'Ex "1000"')} + helpText={ + + } + value={this.state.maxNotificationsPerChannel} + onChange={this.handleChange} + />