Skip to content

Commit

Permalink
Merge f60e030 into 847c716
Browse files Browse the repository at this point in the history
  • Loading branch information
crspeller committed Mar 6, 2017
2 parents 847c716 + f60e030 commit cd709fe
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 10 deletions.
9 changes: 7 additions & 2 deletions api/reaction.go
Expand Up @@ -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() {
Expand Down Expand Up @@ -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()))
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions app/web_hub.go
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions einterfaces/cluster.go
Expand Up @@ -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)
Expand Down
43 changes: 42 additions & 1 deletion store/sql_reaction_store.go
Expand Up @@ -4,13 +4,21 @@
package store

import (
"github.com/mattermost/platform/einterfaces"
"github.com/mattermost/platform/model"
"github.com/mattermost/platform/utils"

l4g "github.com/alecthomas/log4go"
"github.com/go-gorp/gorp"
)

const (
REACTION_CACHE_SIZE = 5000
REACTION_CACHE_SEC = 1800 // 30 minutes
)

var reactionCache *utils.Cache = utils.NewLru(REACTION_CACHE_SIZE)

type SqlReactionStore struct {
*SqlStore
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
38 changes: 32 additions & 6 deletions store/sql_reaction_store_test.go
Expand Up @@ -4,8 +4,9 @@
package store

import (
"github.com/mattermost/platform/model"
"testing"

"github.com/mattermost/platform/model"
)

func TestReactionSave(t *testing.T) {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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")
}

Expand Down
4 changes: 3 additions & 1 deletion store/store.go
Expand Up @@ -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
}

0 comments on commit cd709fe

Please sign in to comment.