From 83291e4c8fd1c2d234ac9f4254af849bed64cabf Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Tue, 7 Mar 2017 16:05:01 -0500 Subject: Merge 3.7 changes into master (#5679) * Save schema version for 3.7 (#5675) * Adding index and cache to reactinos store (#5654) * Fix badge count for push notifications (#5672) --- api/reaction.go | 9 +++++++-- app/web_hub.go | 12 +++++++++++ einterfaces/cluster.go | 1 + store/sql_reaction_store.go | 43 +++++++++++++++++++++++++++++++++++++++- store/sql_reaction_store_test.go | 38 +++++++++++++++++++++++++++++------ store/sql_upgrade.go | 2 ++ store/sql_user_store.go | 8 +++++++- store/store.go | 4 +++- 8 files changed, 106 insertions(+), 11 deletions(-) diff --git a/api/reaction.go b/api/reaction.go index cc1e61efd..85b44b82d 100644 --- a/api/reaction.go +++ b/api/reaction.go @@ -4,12 +4,13 @@ package api import ( + "net/http" + l4g "github.com/alecthomas/log4go" "github.com/gorilla/mux" "github.com/mattermost/platform/app" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" - "net/http" ) func InitReaction() { @@ -72,6 +73,8 @@ func saveReaction(c *Context, w http.ResponseWriter, r *http.Request) { reaction := result.Data.(*model.Reaction) + app.InvalidateCacheForReactions(reaction.PostId) + w.Write([]byte(reaction.ToJson())) } } @@ -126,6 +129,8 @@ func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) { } else { go sendReactionEvent(model.WEBSOCKET_EVENT_REACTION_REMOVED, channelId, reaction, post) + app.InvalidateCacheForReactions(reaction.PostId) + ReturnStatusOK(w) } } @@ -179,7 +184,7 @@ func listReactions(c *Context, w http.ResponseWriter, r *http.Request) { return } - if result := <-app.Srv.Store.Reaction().GetForPost(postId); result.Err != nil { + if result := <-app.Srv.Store.Reaction().GetForPost(postId, true); result.Err != nil { c.Err = result.Err return } else { diff --git a/app/web_hub.go b/app/web_hub.go index 5b7e3623e..65d18481f 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -202,6 +202,18 @@ func InvalidateWebConnSessionCacheForUser(userId string) { } } +func InvalidateCacheForReactions(postId string) { + InvalidateCacheForReactionsSkipClusterSend(postId) + + if cluster := einterfaces.GetClusterInterface(); cluster != nil { + cluster.InvalidateCacheForReactions(postId) + } +} + +func InvalidateCacheForReactionsSkipClusterSend(postId string) { + Srv.Store.Reaction().InvalidateCacheForPost(postId) +} + func (h *Hub) Register(webConn *WebConn) { h.register <- webConn diff --git a/einterfaces/cluster.go b/einterfaces/cluster.go index 389f1f139..b7ba55144 100644 --- a/einterfaces/cluster.go +++ b/einterfaces/cluster.go @@ -20,6 +20,7 @@ type ClusterInterface interface { InvalidateCacheForChannelMembersNotifyProps(channelId string) InvalidateCacheForChannelPosts(channelId string) InvalidateCacheForWebhook(webhookId string) + InvalidateCacheForReactions(postId string) Publish(event *model.WebSocketEvent) UpdateStatus(status *model.Status) GetLogs() ([]string, *model.AppError) diff --git a/store/sql_reaction_store.go b/store/sql_reaction_store.go index 7bd063a15..076710efc 100644 --- a/store/sql_reaction_store.go +++ b/store/sql_reaction_store.go @@ -4,6 +4,7 @@ package store import ( + "github.com/mattermost/platform/einterfaces" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" @@ -11,6 +12,13 @@ import ( "github.com/go-gorp/gorp" ) +const ( + REACTION_CACHE_SIZE = 20000 + REACTION_CACHE_SEC = 1800 // 30 minutes +) + +var reactionCache *utils.Cache = utils.NewLru(REACTION_CACHE_SIZE) + type SqlReactionStore struct { *SqlStore } @@ -30,6 +38,8 @@ func NewSqlReactionStore(sqlStore *SqlStore) ReactionStore { func (s SqlReactionStore) CreateIndexesIfNotExists() { s.CreateIndexIfNotExists("idx_reactions_post_id", "Reactions", "PostId") + s.CreateIndexIfNotExists("idx_reactions_user_id", "Reactions", "UserId") + s.CreateIndexIfNotExists("idx_reactions_emoji_name", "Reactions", "EmojiName") } func (s SqlReactionStore) Save(reaction *model.Reaction) StoreChannel { @@ -149,11 +159,40 @@ func updatePostForReactions(transaction *gorp.Transaction, postId string) error return err } -func (s SqlReactionStore) GetForPost(postId string) StoreChannel { +func (s SqlReactionStore) InvalidateCacheForPost(postId string) { + reactionCache.Remove(postId) +} + +func (s SqlReactionStore) InvalidateCache() { + reactionCache.Purge() +} + +func (s SqlReactionStore) GetForPost(postId string, allowFromCache bool) StoreChannel { storeChannel := make(StoreChannel) go func() { result := StoreResult{} + metrics := einterfaces.GetMetricsInterface() + + if allowFromCache { + if cacheItem, ok := reactionCache.Get(postId); ok { + if metrics != nil { + metrics.IncrementMemCacheHitCounter("Reactions") + } + result.Data = cacheItem.([]*model.Reaction) + storeChannel <- result + close(storeChannel) + return + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("Reactions") + } + } + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("Reactions") + } + } var reactions []*model.Reaction @@ -169,6 +208,8 @@ func (s SqlReactionStore) GetForPost(postId string) StoreChannel { result.Err = model.NewLocAppError("SqlReactionStore.GetForPost", "store.sql_reaction.get_for_post.app_error", nil, "") } else { result.Data = reactions + + reactionCache.AddWithExpiresInSecs(postId, reactions, REACTION_CACHE_SEC) } storeChannel <- result diff --git a/store/sql_reaction_store_test.go b/store/sql_reaction_store_test.go index 5a1cb2d67..01ce14e65 100644 --- a/store/sql_reaction_store_test.go +++ b/store/sql_reaction_store_test.go @@ -4,8 +4,9 @@ package store import ( - "github.com/mattermost/platform/model" "testing" + + "github.com/mattermost/platform/model" ) func TestReactionSave(t *testing.T) { @@ -108,7 +109,7 @@ func TestReactionDelete(t *testing.T) { t.Fatal(result.Err) } - if result := <-store.Reaction().GetForPost(post.Id); result.Err != nil { + if result := <-store.Reaction().GetForPost(post.Id, false); result.Err != nil { t.Fatal(result.Err) } else if len(result.Data.([]*model.Reaction)) != 0 { t.Fatal("should've deleted reaction") @@ -155,7 +156,32 @@ func TestReactionGetForPost(t *testing.T) { Must(store.Reaction().Save(reaction)) } - if result := <-store.Reaction().GetForPost(postId); result.Err != nil { + if result := <-store.Reaction().GetForPost(postId, false); result.Err != nil { + t.Fatal(result.Err) + } else if returned := result.Data.([]*model.Reaction); len(returned) != 3 { + t.Fatal("should've returned 3 reactions") + } else { + for _, reaction := range reactions { + found := false + + for _, returnedReaction := range returned { + if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId && + returnedReaction.EmojiName == reaction.EmojiName { + found = true + break + } + } + + if !found && reaction.PostId == postId { + t.Fatalf("should've returned reaction for post %v", reaction) + } else if found && reaction.PostId != postId { + t.Fatal("shouldn't have returned reaction for another post") + } + } + } + + // Should return cached item + if result := <-store.Reaction().GetForPost(postId, true); result.Err != nil { t.Fatal(result.Err) } else if returned := result.Data.([]*model.Reaction); len(returned) != 3 { t.Fatal("should've returned 3 reactions") @@ -237,7 +263,7 @@ func TestReactionDeleteAllWithEmojiName(t *testing.T) { } // check that the reactions were deleted - if returned := Must(store.Reaction().GetForPost(post.Id)).([]*model.Reaction); len(returned) != 1 { + if returned := Must(store.Reaction().GetForPost(post.Id, false)).([]*model.Reaction); len(returned) != 1 { t.Fatal("should've only removed reactions with emoji name") } else { for _, reaction := range returned { @@ -247,11 +273,11 @@ func TestReactionDeleteAllWithEmojiName(t *testing.T) { } } - if returned := Must(store.Reaction().GetForPost(post2.Id)).([]*model.Reaction); len(returned) != 1 { + if returned := Must(store.Reaction().GetForPost(post2.Id, false)).([]*model.Reaction); len(returned) != 1 { t.Fatal("should've only removed reactions with emoji name") } - if returned := Must(store.Reaction().GetForPost(post3.Id)).([]*model.Reaction); len(returned) != 0 { + if returned := Must(store.Reaction().GetForPost(post3.Id, false)).([]*model.Reaction); len(returned) != 0 { t.Fatal("should've only removed reactions with emoji name") } diff --git a/store/sql_upgrade.go b/store/sql_upgrade.go index d3e8b46c6..1402c0eb1 100644 --- a/store/sql_upgrade.go +++ b/store/sql_upgrade.go @@ -236,5 +236,7 @@ func UpgradeDatabaseToVersion37(sqlStore *SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_6_0, VERSION_3_7_0) { // Add EditAt column to Posts sqlStore.CreateColumnIfNotExists("Posts", "EditAt", " bigint", " bigint", "0") + + saveSchemaVersion(sqlStore, VERSION_3_7_0) } } diff --git a/store/sql_user_store.go b/store/sql_user_store.go index 838c2a80d..23b0c3696 100644 --- a/store/sql_user_store.go +++ b/store/sql_user_store.go @@ -1179,7 +1179,13 @@ func (us SqlUserStore) GetUnreadCount(userId string) StoreChannel { go func() { result := StoreResult{} - if count, err := us.GetReplica().SelectInt("SELECT SUM(CASE WHEN c.Type = 'D' THEN (c.TotalMsgCount - cm.MsgCount) ELSE cm.MentionCount END) FROM Channels c INNER JOIN ChannelMembers cm ON cm.ChannelId = c.Id AND cm.UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil { + if count, err := us.GetReplica().SelectInt(` + SELECT SUM(CASE WHEN c.Type = 'D' THEN (c.TotalMsgCount - cm.MsgCount) ELSE cm.MentionCount END) + FROM Channels c + INNER JOIN ChannelMembers cm + ON cm.ChannelId = c.Id + AND cm.UserId = :UserId + AND c.DeleteAt = 0`, map[string]interface{}{"UserId": userId}); err != nil { result.Err = model.NewLocAppError("SqlUserStore.GetMentionCount", "store.sql_user.get_unread_count.app_error", nil, err.Error()) } else { result.Data = count diff --git a/store/store.go b/store/store.go index a436f9ee7..7aa903f6f 100644 --- a/store/store.go +++ b/store/store.go @@ -348,6 +348,8 @@ type FileInfoStore interface { type ReactionStore interface { Save(reaction *model.Reaction) StoreChannel Delete(reaction *model.Reaction) StoreChannel - GetForPost(postId string) StoreChannel + InvalidateCacheForPost(postId string) + InvalidateCache() + GetForPost(postId string, allowFromCache bool) StoreChannel DeleteAllWithEmojiName(emojiName string) StoreChannel } -- cgit v1.2.3-1-g7c22