MM-16921 Fix getPostsSince caching invalid data (#11618)

* MM-16921 Fix getPostsSince caching invalid data

* Remove workaround for invalid caching

store/sqlstore/post_store.go

@@ -335,7 +335,7 @@ func (s *SqlPostStore) InvalidateLastPostTimeCache(channelId string) {
 
 func (s *SqlPostStore) GetEtag(channelId string, allowFromCache bool) string {
 	if allowFromCache {
-		if cacheItem, ok := s.lastPostTimeCache.Get(channelId); ok && cacheItem.(int64) > 0 {
+		if cacheItem, ok := s.lastPostTimeCache.Get(channelId); ok {
 			if s.metrics != nil {
 				s.metrics.IncrementMemCacheHitCounter("Last Post Time")
 			}
@@ -554,7 +554,7 @@ func (s *SqlPostStore) GetPostsSince(channelId string, time int64, allowFromCach
 
 	list := model.NewPostList()
 
-	var latestUpdate int64 = 0
+	latestUpdate := time
 
 	for _, p := range posts {
 		list.AddPost(p)

store/storetest/post_store.go

@@ -932,100 +932,126 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 }
 
 func testPostStoreGetPostsSince(t *testing.T, ss store.Store) {
-	o0 := &model.Post{}
+	t.Run("should return posts created after the given time", func(t *testing.T) {
-	o0.ChannelId = model.NewId()
+		channelId := model.NewId()
-	o0.UserId = model.NewId()
+		userId := model.NewId()
-	o0.Message = "zz" + model.NewId() + "b"
-	_, err := ss.Post().Save(o0)
-	require.Nil(t, err)
-	time.Sleep(2 * time.Millisecond)
 
-	o1 := &model.Post{}
+		post1, err := ss.Post().Save(&model.Post{
-	o1.ChannelId = model.NewId()
+			ChannelId: channelId,
-	o1.UserId = model.NewId()
+			UserId:    userId,
-	o1.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o1, err = ss.Post().Save(o1)
+		})
-	require.Nil(t, err)
+		require.Nil(t, err)
-	time.Sleep(2 * time.Millisecond)
+		time.Sleep(time.Millisecond)
 
-	o2 := &model.Post{}
+		_, err = ss.Post().Save(&model.Post{
-	o2.ChannelId = o1.ChannelId
+			ChannelId: channelId,
-	o2.UserId = model.NewId()
+			UserId:    userId,
-	o2.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o2.ParentId = o1.Id
+		})
-	o2.RootId = o1.Id
+		require.Nil(t, err)
-	_, err = ss.Post().Save(o2)
+		time.Sleep(time.Millisecond)
-	require.Nil(t, err)
-	time.Sleep(2 * time.Millisecond)
 
-	o2a := &model.Post{}
+		post3, err := ss.Post().Save(&model.Post{
-	o2a.ChannelId = o1.ChannelId
+			ChannelId: channelId,
-	o2a.UserId = model.NewId()
+			UserId:    userId,
-	o2a.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o2a.ParentId = o1.Id
+		})
-	o2a.RootId = o1.Id
+		require.Nil(t, err)
-	o2a, err = ss.Post().Save(o2a)
+		time.Sleep(time.Millisecond)
-	require.Nil(t, err)
-	time.Sleep(2 * time.Millisecond)
 
-	o3 := &model.Post{}
+		post4, err := ss.Post().Save(&model.Post{
-	o3.ChannelId = o1.ChannelId
+			ChannelId: channelId,
-	o3.UserId = model.NewId()
+			UserId:    userId,
-	o3.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o3.ParentId = o1.Id
+		})
-	o3.RootId = o1.Id
+		require.Nil(t, err)
-	o3, err = ss.Post().Save(o3)
+		time.Sleep(time.Millisecond)
-	require.Nil(t, err)
-	time.Sleep(2 * time.Millisecond)
 
-	o4 := &model.Post{}
+		post5, err := ss.Post().Save(&model.Post{
-	o4.ChannelId = o1.ChannelId
+			ChannelId: channelId,
-	o4.UserId = model.NewId()
+			UserId:    userId,
-	o4.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o4, err = ss.Post().Save(o4)
+			RootId:    post3.Id,
-	require.Nil(t, err)
+		})
-	time.Sleep(2 * time.Millisecond)
+		require.Nil(t, err)
+		time.Sleep(time.Millisecond)
 
-	o5 := &model.Post{}
+		post6, err := ss.Post().Save(&model.Post{
-	o5.ChannelId = o1.ChannelId
+			ChannelId: channelId,
-	o5.UserId = model.NewId()
+			UserId:    userId,
-	o5.Message = "zz" + model.NewId() + "b"
+			Message:   "message",
-	o5.ParentId = o4.Id
+			RootId:    post1.Id,
-	o5.RootId = o4.Id
+		})
-	o5, err = ss.Post().Save(o5)
+		require.Nil(t, err)
-	require.Nil(t, err)
+		time.Sleep(time.Millisecond)
 
-	r1, _ := ss.Post().GetPostsSince(o1.ChannelId, o1.CreateAt, false)
+		postList, err := ss.Post().GetPostsSince(channelId, post3.CreateAt, false)
+		assert.Nil(t, err)
 
-	if r1.Order[0] != o5.Id {
+		assert.Equal(t, []string{
-		t.Fatal("invalid order")
+			post6.Id,
-	}
+			post5.Id,
+			post4.Id,
+			post3.Id,
+			post1.Id,
+		}, postList.Order)
 
-	if r1.Order[1] != o4.Id {
+		assert.Len(t, postList.Posts, 5)
-		t.Fatal("invalid order")
+		assert.NotNil(t, postList.Posts[post1.Id], "should return the parent post")
-	}
+		assert.NotNil(t, postList.Posts[post3.Id])
+		assert.NotNil(t, postList.Posts[post4.Id])
+		assert.NotNil(t, postList.Posts[post5.Id])
+		assert.NotNil(t, postList.Posts[post6.Id])
+	})
 
-	if r1.Order[2] != o3.Id {
+	t.Run("should return empty list when nothing has changed", func(t *testing.T) {
-		t.Fatal("invalid order")
+		channelId := model.NewId()
-	}
+		userId := model.NewId()
 
-	if r1.Order[3] != o2a.Id {
+		post1, err := ss.Post().Save(&model.Post{
-		t.Fatal("invalid order")
+			ChannelId: channelId,
-	}
+			UserId:    userId,
+			Message:   "message",
+		})
+		require.Nil(t, err)
+		time.Sleep(time.Millisecond)
 
-	if len(r1.Posts) != 6 {
+		postList, err := ss.Post().GetPostsSince(channelId, post1.CreateAt, false)
-		t.Fatal("wrong size")
+		assert.Nil(t, err)
-	}
 
-	if r1.Posts[o1.Id].Message != o1.Message {
+		assert.Equal(t, []string{}, postList.Order)
-		t.Fatal("Missing parent")
+		assert.Len(t, postList.Posts, 0)
-	}
+	})
 
-	r2, _ := ss.Post().GetPostsSince(o1.ChannelId, o5.UpdateAt, true)
+	t.Run("should not cache a timestamp of 0 when nothing has changed", func(t *testing.T) {
+		ss.Post().ClearCaches()
 
-	if len(r2.Order) != 0 {
+		channelId := model.NewId()
-		t.Fatal("wrong size ", len(r2.Posts))
+		userId := model.NewId()
-	}
+
+		post1, err := ss.Post().Save(&model.Post{
+			ChannelId: channelId,
+			UserId:    userId,
+			Message:   "message",
+		})
+		require.Nil(t, err)
+		time.Sleep(time.Millisecond)
+
+		// Make a request that returns no results
+		postList, err := ss.Post().GetPostsSince(channelId, post1.CreateAt, true)
+		require.Nil(t, err)
+		require.Equal(t, model.NewPostList(), postList)
+
+		// And then ensure that it doesn't cause future requests to also return no results
+		postList, err = ss.Post().GetPostsSince(channelId, post1.CreateAt-1, true)
+		assert.Nil(t, err)
+
+		assert.Equal(t, []string{post1.Id}, postList.Order)
+
+		assert.Len(t, postList.Posts, 1)
+		assert.NotNil(t, postList.Posts[post1.Id])
+	})
 }
 
 func testPostStoreGetPostBeforeAfter(t *testing.T, ss store.Store) {