MM-15850/MM-15851 GetPosts Before/After sync by default (#11035)

app/post.go

@@ -667,34 +667,18 @@ func (a *App) GetPermalinkPost(postId string, userId string) (*model.PostList, *
 }
 
 func (a *App) GetPostsBeforePost(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
-	result := <-a.Srv.Store.Post().GetPostsBefore(channelId, postId, perPage, page*perPage)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.PostList), nil
+	return a.Srv.Store.Post().GetPostsBefore(channelId, postId, perPage, page*perPage)
 }
 
 func (a *App) GetPostsAfterPost(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
-	result := <-a.Srv.Store.Post().GetPostsAfter(channelId, postId, perPage, page*perPage)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.PostList), nil
+	return a.Srv.Store.Post().GetPostsAfter(channelId, postId, perPage, page*perPage)
 }
 
 func (a *App) GetPostsAroundPost(postId, channelId string, offset, limit int, before bool) (*model.PostList, *model.AppError) {
-	var pchan store.StoreChannel
 	if before {
-		pchan = a.Srv.Store.Post().GetPostsBefore(channelId, postId, limit, offset)
-	} else {
-		pchan = a.Srv.Store.Post().GetPostsAfter(channelId, postId, limit, offset)
+		return a.Srv.Store.Post().GetPostsBefore(channelId, postId, limit, offset)
 	}
-
-	result := <-pchan
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.PostList), nil
+	return a.Srv.Store.Post().GetPostsAfter(channelId, postId, limit, offset)
 }
 
 func (a *App) DeletePost(postId, deleteByID string) (*model.Post, *model.AppError) {

store/sqlstore/post_store.go

@@ -581,98 +581,95 @@ func (s *SqlPostStore) GetPostsSince(channelId string, time int64, allowFromCach
 	})
 }
 
-func (s *SqlPostStore) GetPostsBefore(channelId string, postId string, limit int, offset int) store.StoreChannel {
+func (s *SqlPostStore) GetPostsBefore(channelId string, postId string, limit int, offset int) (*model.PostList, *model.AppError) {
 	return s.getPostsAround(channelId, postId, limit, offset, true)
 }
 
-func (s *SqlPostStore) GetPostsAfter(channelId string, postId string, limit int, offset int) store.StoreChannel {
+func (s *SqlPostStore) GetPostsAfter(channelId string, postId string, limit int, offset int) (*model.PostList, *model.AppError) {
 	return s.getPostsAround(channelId, postId, limit, offset, false)
 }
 
-func (s *SqlPostStore) getPostsAround(channelId string, postId string, limit int, offset int, before bool) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var direction string
-		var sort string
-		if before {
-			direction = "<"
-			sort = "DESC"
-		} else {
-			direction = ">"
-			sort = "ASC"
-		}
+func (s *SqlPostStore) getPostsAround(channelId string, postId string, limit int, offset int, before bool) (*model.PostList, *model.AppError) {
+	var direction string
+	var sort string
+	if before {
+		direction = "<"
+		sort = "DESC"
+	} else {
+		direction = ">"
+		sort = "ASC"
+	}
 
-		var posts []*model.Post
-		var parents []*model.Post
-		_, err1 := s.GetReplica().Select(&posts,
-			`SELECT
-			    *
-			FROM
-			    Posts
-			WHERE
-				CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = :PostId)
-			        AND ChannelId = :ChannelId
-					AND DeleteAt = 0
-			ORDER BY CreateAt `+sort+`
-			LIMIT :Limit
-			OFFSET :Offset`,
-			map[string]interface{}{"ChannelId": channelId, "PostId": postId, "Limit": limit, "Offset": offset})
-		_, err2 := s.GetReplica().Select(&parents,
-			`SELECT
-			    q2.*
-			FROM
-				Posts q2
-					INNER JOIN
-			    (SELECT DISTINCT
-			        q3.Id, q3.RootId
-			    FROM
-					(SELECT
-						Id, RootId
-					FROM
-						Posts
-					WHERE
-						CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = :PostId)
-						AND ChannelId = :ChannelId
-						AND DeleteAt = 0
-					ORDER BY CreateAt `+sort+`
-					LIMIT :Limit OFFSET :Offset) q3 -- q3 contains the Id and RootId of every post in posts
-				) q1 -- q1 is q3 with the duplicates removed
-				ON q1.RootId = q2.Id -- This is the root post of a thread that appears in posts
-					OR q1.Id = q2.RootId -- This is a comment on a post in posts
-					OR (q2.RootId != '' AND q1.RootId = q2.RootId) -- This is a comment on a thread that appears in posts
-			WHERE
-				ChannelId = :ChannelId
+	var posts []*model.Post
+	var parents []*model.Post
+	_, err := s.GetReplica().Select(&posts,
+		`SELECT
+			*
+		FROM
+			Posts
+		WHERE
+			CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = :PostId)
+				AND ChannelId = :ChannelId
 				AND DeleteAt = 0
-			ORDER BY CreateAt DESC`,
-			map[string]interface{}{"ChannelId": channelId, "PostId": postId, "Limit": limit, "Offset": offset})
+		ORDER BY CreateAt `+sort+`
+		LIMIT :Limit
+		OFFSET :Offset`,
+		map[string]interface{}{"ChannelId": channelId, "PostId": postId, "Limit": limit, "Offset": offset})
+	if err != nil {
+		return nil, model.NewAppError("SqlPostStore.GetPostContext", "store.sql_post.get_posts_around.get.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError)
+	}
+	_, err = s.GetReplica().Select(&parents,
+		`SELECT
+			q2.*
+		FROM
+			Posts q2
+				INNER JOIN
+			(SELECT DISTINCT
+				q3.Id, q3.RootId
+			FROM
+				(SELECT
+					Id, RootId
+				FROM
+					Posts
+				WHERE
+					CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = :PostId)
+					AND ChannelId = :ChannelId
+					AND DeleteAt = 0
+				ORDER BY CreateAt `+sort+`
+				LIMIT :Limit OFFSET :Offset) q3 -- q3 contains the Id and RootId of every post in posts
+			) q1 -- q1 is q3 with the duplicates removed
+			ON q1.RootId = q2.Id -- This is the root post of a thread that appears in posts
+				OR q1.Id = q2.RootId -- This is a comment on a post in posts
+				OR (q2.RootId != '' AND q1.RootId = q2.RootId) -- This is a comment on a thread that appears in posts
+		WHERE
+			ChannelId = :ChannelId
+			AND DeleteAt = 0
+		ORDER BY CreateAt DESC`,
+		map[string]interface{}{"ChannelId": channelId, "PostId": postId, "Limit": limit, "Offset": offset})
 
-		if err1 != nil {
-			result.Err = model.NewAppError("SqlPostStore.GetPostContext", "store.sql_post.get_posts_around.get.app_error", nil, "channelId="+channelId+err1.Error(), http.StatusInternalServerError)
-		} else if err2 != nil {
-			result.Err = model.NewAppError("SqlPostStore.GetPostContext", "store.sql_post.get_posts_around.get_parent.app_error", nil, "channelId="+channelId+err2.Error(), http.StatusInternalServerError)
-		} else {
-			list := model.NewPostList()
+	if err != nil {
+		return nil, model.NewAppError("SqlPostStore.GetPostContext", "store.sql_post.get_posts_around.get_parent.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError)
+	}
+	list := model.NewPostList()
 
-			// We need to flip the order if we selected backwards
-			if before {
-				for _, p := range posts {
-					list.AddPost(p)
-					list.AddOrder(p.Id)
-				}
-			} else {
-				l := len(posts)
-				for i := range posts {
-					list.AddPost(posts[l-i-1])
-					list.AddOrder(posts[l-i-1].Id)
-				}
-			}
-
-			for _, p := range parents {
-				list.AddPost(p)
-			}
-
-			result.Data = list
+	// We need to flip the order if we selected backwards
+	if before {
+		for _, p := range posts {
+			list.AddPost(p)
+			list.AddOrder(p.Id)
 		}
-	})
+	} else {
+		l := len(posts)
+		for i := range posts {
+			list.AddPost(posts[l-i-1])
+			list.AddOrder(posts[l-i-1].Id)
+		}
+	}
+
+	for _, p := range parents {
+		list.AddPost(p)
+	}
+	return list, nil
 }
 
 func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int) store.StoreChannel {

store/store.go

@@ -221,8 +221,8 @@ type PostStore interface {
 	GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, *model.AppError)
 	GetFlaggedPostsForTeam(userId, teamId string, offset int, limit int) (*model.PostList, *model.AppError)
 	GetFlaggedPostsForChannel(userId, channelId string, offset int, limit int) (*model.PostList, *model.AppError)
-	GetPostsBefore(channelId string, postId string, numPosts int, offset int) StoreChannel
-	GetPostsAfter(channelId string, postId string, numPosts int, offset int) StoreChannel
+	GetPostsBefore(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError)
+	GetPostsAfter(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError)
 	GetPostsSince(channelId string, time int64, allowFromCache bool) StoreChannel
 	GetEtag(channelId string, allowFromCache bool) string
 	Search(teamId string, userId string, params *model.SearchParams) StoreChannel

store/storetest/mocks/PostStore.go

@@ -302,19 +302,28 @@ func (_m *PostStore) GetPosts(channelId string, offset int, limit int, allowFrom
 }
 
 // GetPostsAfter provides a mock function with given fields: channelId, postId, numPosts, offset
-func (_m *PostStore) GetPostsAfter(channelId string, postId string, numPosts int, offset int) store.StoreChannel {
+func (_m *PostStore) GetPostsAfter(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError) {
 	ret := _m.Called(channelId, postId, numPosts, offset)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, string, int, int) store.StoreChannel); ok {
+	var r0 *model.PostList
+	if rf, ok := ret.Get(0).(func(string, string, int, int) *model.PostList); ok {
 		r0 = rf(channelId, postId, numPosts, offset)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.PostList)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string, int, int) *model.AppError); ok {
+		r1 = rf(channelId, postId, numPosts, offset)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetPostsBatchForIndexing provides a mock function with given fields: startTime, endTime, limit
@@ -334,19 +343,28 @@ func (_m *PostStore) GetPostsBatchForIndexing(startTime int64, endTime int64, li
 }
 
 // GetPostsBefore provides a mock function with given fields: channelId, postId, numPosts, offset
-func (_m *PostStore) GetPostsBefore(channelId string, postId string, numPosts int, offset int) store.StoreChannel {
+func (_m *PostStore) GetPostsBefore(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError) {
 	ret := _m.Called(channelId, postId, numPosts, offset)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, string, int, int) store.StoreChannel); ok {
+	var r0 *model.PostList
+	if rf, ok := ret.Get(0).(func(string, string, int, int) *model.PostList); ok {
 		r0 = rf(channelId, postId, numPosts, offset)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.PostList)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string, int, int) *model.AppError); ok {
+		r1 = rf(channelId, postId, numPosts, offset)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetPostsByIds provides a mock function with given fields: postIds

store/storetest/post_store.go

@@ -731,19 +731,17 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		}
 
 		t.Run("should not return anything before the first post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsBefore(channelId, posts[0].Id, 10, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsBefore(channelId, posts[0].Id, 10, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{}, postList.Posts)
 		})
 
 		t.Run("should return posts before a post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsBefore(channelId, posts[5].Id, 10, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsBefore(channelId, posts[5].Id, 10, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{posts[4].Id, posts[3].Id, posts[2].Id, posts[1].Id, posts[0].Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				posts[0].Id: posts[0],
@@ -755,10 +753,9 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		})
 
 		t.Run("should limit posts before", func(t *testing.T) {
-			res := <-ss.Post().GetPostsBefore(channelId, posts[5].Id, 2, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsBefore(channelId, posts[5].Id, 2, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{posts[4].Id, posts[3].Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				posts[3].Id: posts[3],
@@ -767,19 +764,17 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		})
 
 		t.Run("should not return anything after the last post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsAfter(channelId, posts[len(posts)-1].Id, 10, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsAfter(channelId, posts[len(posts)-1].Id, 10, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{}, postList.Posts)
 		})
 
 		t.Run("should return posts after a post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsAfter(channelId, posts[5].Id, 10, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsAfter(channelId, posts[5].Id, 10, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{posts[9].Id, posts[8].Id, posts[7].Id, posts[6].Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				posts[6].Id: posts[6],
@@ -790,10 +785,9 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		})
 
 		t.Run("should limit posts after", func(t *testing.T) {
-			res := <-ss.Post().GetPostsAfter(channelId, posts[5].Id, 2, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsAfter(channelId, posts[5].Id, 2, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{posts[7].Id, posts[6].Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				posts[6].Id: posts[6],
@@ -866,10 +860,9 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		post2.UpdateAt = post6.UpdateAt
 
 		t.Run("should return each post and thread before a post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsBefore(channelId, post4.Id, 2, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsBefore(channelId, post4.Id, 2, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{post3.Id, post2.Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				post1.Id: post1,
@@ -881,10 +874,9 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 		})
 
 		t.Run("should return each post and the root of each thread after a post", func(t *testing.T) {
-			res := <-ss.Post().GetPostsAfter(channelId, post4.Id, 2, 0)
-			assert.Nil(t, res.Err)
+			postList, err := ss.Post().GetPostsAfter(channelId, post4.Id, 2, 0)
+			assert.Nil(t, err)
 
-			postList := res.Data.(*model.PostList)
 			assert.Equal(t, []string{post6.Id, post5.Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				post2.Id: post2,