From 3e0c3eff9f2ddec241cdb3f7a91230fd7c51a5f6 Mon Sep 17 00:00:00 2001
From: Joram Wilander <jwawilander@gmail.com>
Date: Fri, 9 Feb 2018 17:47:22 -0500
Subject: ABC-228 Update GetPosts caching to work for non-60 limits (#8233)

* Update GetPosts caching to work for non-60 limits

* Only cache on limits of 30/60 and add test

* Add comments clarifying 30 and 60 limits
---
 store/sqlstore/post_store.go  | 15 ++++++++++-----
 store/storetest/post_store.go | 23 +++++++++++++++++++++--
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go
index bc336e70d..25c3c4913 100644
--- a/store/sqlstore/post_store.go
+++ b/store/sqlstore/post_store.go
@@ -322,7 +322,10 @@ type etagPosts struct {
 
 func (s SqlPostStore) InvalidateLastPostTimeCache(channelId string) {
 	lastPostTimeCache.Remove(channelId)
-	lastPostsCache.Remove(channelId)
+
+	// Keys are "{channelid}{limit}" and caching only occurs on limits of 30 and 60
+	lastPostsCache.Remove(channelId + "30")
+	lastPostsCache.Remove(channelId + "60")
 }
 
 func (s SqlPostStore) GetEtag(channelId string, allowFromCache bool) store.StoreChannel {
@@ -439,8 +442,9 @@ func (s SqlPostStore) GetPosts(channelId string, offset int, limit int, allowFro
 			return
 		}
 
-		if allowFromCache && offset == 0 && limit == 60 {
-			if cacheItem, ok := lastPostsCache.Get(channelId); ok {
+		// Caching only occurs on limits of 30 and 60, the common limits requested by MM clients
+		if allowFromCache && offset == 0 && (limit == 60 || limit == 30) {
+			if cacheItem, ok := lastPostsCache.Get(fmt.Sprintf("%s%v", channelId, limit)); ok {
 				if s.metrics != nil {
 					s.metrics.IncrementMemCacheHitCounter("Last Posts Cache")
 				}
@@ -482,8 +486,9 @@ func (s SqlPostStore) GetPosts(channelId string, offset int, limit int, allowFro
 
 			list.MakeNonNil()
 
-			if offset == 0 && limit == 60 {
-				lastPostsCache.AddWithExpiresInSecs(channelId, list, LAST_POSTS_CACHE_SEC)
+			// Caching only occurs on limits of 30 and 60, the common limits requested by MM clients
+			if offset == 0 && (limit == 60 || limit == 30) {
+				lastPostsCache.AddWithExpiresInSecs(fmt.Sprintf("%s%v", channelId, limit), list, LAST_POSTS_CACHE_SEC)
 			}
 
 			result.Data = list
diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go
index 4deb7f8d4..e663d5a41 100644
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -27,7 +27,7 @@ func TestPostStore(t *testing.T, ss store.Store) {
 	t.Run("PermDelete1Level", func(t *testing.T) { testPostStorePermDelete1Level(t, ss) })
 	t.Run("PermDelete1Level2", func(t *testing.T) { testPostStorePermDelete1Level2(t, ss) })
 	t.Run("GetWithChildren", func(t *testing.T) { testPostStoreGetWithChildren(t, ss) })
-	t.Run("GetPostsWtihDetails", func(t *testing.T) { testPostStoreGetPostsWtihDetails(t, ss) })
+	t.Run("GetPostsWithDetails", func(t *testing.T) { testPostStoreGetPostsWithDetails(t, ss) })
 	t.Run("GetPostsBeforeAfter", func(t *testing.T) { testPostStoreGetPostsBeforeAfter(t, ss) })
 	t.Run("GetPostsSince", func(t *testing.T) { testPostStoreGetPostsSince(t, ss) })
 	t.Run("Search", func(t *testing.T) { testPostStoreSearch(t, ss) })
@@ -490,7 +490,7 @@ func testPostStoreGetWithChildren(t *testing.T, ss store.Store) {
 	}
 }
 
-func testPostStoreGetPostsWtihDetails(t *testing.T, ss store.Store) {
+func testPostStoreGetPostsWithDetails(t *testing.T, ss store.Store) {
 	o1 := &model.Post{}
 	o1.ChannelId = model.NewId()
 	o1.UserId = model.NewId()
@@ -591,6 +591,25 @@ func testPostStoreGetPostsWtihDetails(t *testing.T, ss store.Store) {
 	if r2.Posts[o1.Id].Message != o1.Message {
 		t.Fatal("Missing parent")
 	}
+
+	// Run once to fill cache
+	<-ss.Post().GetPosts(o1.ChannelId, 0, 30, true)
+
+	o6 := &model.Post{}
+	o6.ChannelId = o1.ChannelId
+	o6.UserId = model.NewId()
+	o6.Message = "zz" + model.NewId() + "b"
+	o6 = (<-ss.Post().Save(o6)).Data.(*model.Post)
+
+	// Should only be 6 since we hit the cache
+	r3 := (<-ss.Post().GetPosts(o1.ChannelId, 0, 30, true)).Data.(*model.PostList)
+	assert.Equal(t, 6, len(r3.Order))
+
+	ss.Post().InvalidateLastPostTimeCache(o1.ChannelId)
+
+	// Cache was invalidated, we should get all the posts
+	r4 := (<-ss.Post().GetPosts(o1.ChannelId, 0, 30, true)).Data.(*model.PostList)
+	assert.Equal(t, 7, len(r4.Order))
 }
 
 func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
-- 
cgit v1.2.3-1-g7c22