Mm 43609 (#20657)

* return first inaccessible post time instead of has inaccessible posts

api4/post.go

@@ -401,7 +401,7 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) {
 
 		// Post is inaccessible due to cloud plan's limit.
 		if err.Id == "app.post.cloud.get.app_error" {
-			w.Header().Set(model.HeaderHasInaccessiblePosts, "true")
+			w.Header().Set(model.HeaderFirstInaccessiblePostTime, "1")
 		}
 
 		return
@@ -438,7 +438,7 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	postsList, hasInaccessiblePosts, err := c.App.GetPostsByIds(postIDs)
+	postsList, firstInaccessiblePostTime, err := c.App.GetPostsByIds(postIDs)
 	if err != nil {
 		c.Err = err
 		return
@@ -471,7 +471,7 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) {
 		posts = append(posts, post)
 	}
 
-	w.Header().Set(model.HeaderHasInaccessiblePosts, strconv.FormatBool(hasInaccessiblePosts))
+	w.Header().Set(model.HeaderFirstInaccessiblePostTime, strconv.FormatInt(firstInaccessiblePostTime, 10))
 
 	if err := json.NewEncoder(w).Encode(posts); err != nil {
 		mlog.Warn("Error while writing response", mlog.Err(err))

app/app_iface.go

@@ -202,7 +202,7 @@ type AppIface interface {
 	// lock instead.
 	GetPluginsEnvironment() *plugin.Environment
 	// GetPostsByIds response bool value indicates, if the post is inaccessible due to cloud plan's limit.
-	GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError)
+	GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError)
 	// GetPostsUsage returns the total posts count rounded down to the most
 	// significant digit
 	GetPostsUsage() (int64, *model.AppError)

app/opentracing/opentracing_layer.go

@@ -7881,7 +7881,7 @@ func (a *OpenTracingAppLayer) GetPostsBeforePost(options model.GetPostsOptions)
 	return resultVar0, resultVar1
 }
 
-func (a *OpenTracingAppLayer) GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError) {
+func (a *OpenTracingAppLayer) GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetPostsByIds")
 

app/post.go

@@ -870,11 +870,11 @@ func (a *App) GetSinglePost(postID string, includeDeleted bool) (*model.Post, *m
 		}
 	}
 
-	isInaccessible, appErr := a.isInaccessiblePost(post)
+	firstInaccessiblePostTime, appErr := a.isInaccessiblePost(post)
 	if appErr != nil {
 		return nil, appErr
 	}
-	if isInaccessible {
+	if firstInaccessiblePostTime != 0 {
 		return nil, model.NewAppError("GetSinglePost", "app.post.cloud.get.app_error", nil, "", http.StatusForbidden)
 	}
 
@@ -1881,24 +1881,24 @@ func (a *App) GetPostIfAuthorized(c request.CTX, postID string, session *model.S
 }
 
 // GetPostsByIds response bool value indicates, if the post is inaccessible due to cloud plan's limit.
-func (a *App) GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError) {
+func (a *App) GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError) {
 	posts, err := a.Srv().Store.Post().GetPostsByIds(postIDs)
 	if err != nil {
 		var nfErr *store.ErrNotFound
 		switch {
 		case errors.As(err, &nfErr):
-			return nil, false, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, nfErr.Error(), http.StatusNotFound)
+			return nil, 0, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, nfErr.Error(), http.StatusNotFound)
 		default:
-			return nil, false, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, 0, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, err.Error(), http.StatusInternalServerError)
 		}
 	}
 
-	posts, hasInaccessiblePosts, appErr := a.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
+	posts, firstInaccessiblePostTime, appErr := a.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
 	if appErr != nil {
-		return nil, false, appErr
+		return nil, 0, appErr
 	}
 
-	return posts, hasInaccessiblePosts, nil
+	return posts, firstInaccessiblePostTime, nil
 }
 
 func (a *App) GetTopThreadsForTeamSince(c request.CTX, teamID, userID string, opts *model.InsightsOpts) (*model.TopThreadList, *model.AppError) {

app/post_helpers.go

@@ -27,6 +27,19 @@ func (b accessibleBounds) noAccessible() bool {
 	return b.start == noAccessibleBounds.start && b.end == noAccessibleBounds.end
 }
 
+// assumes checking was already performed that at least one post is inaccessible
+func (b accessibleBounds) getInaccessibleRange(listLength int) (int, int) {
+	var start, end int
+	if b.start == 0 {
+		start = b.end + 1
+		end = listLength - 1
+	} else {
+		start = 0
+		end = b.start - 1
+	}
+	return start, end
+}
+
 var noAccessibleBounds = accessibleBounds{start: -1, end: -1}
 var allAccessibleBounds = func(lenPosts int) accessibleBounds { return accessibleBounds{start: 0, end: lenPosts - 1} }
 
@@ -82,11 +95,13 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64
 
 	n := 0
 	for i, postId := range order {
-		if posts[postId].CreateAt >= earliestAccessibleTime {
+		if createAt := posts[postId].CreateAt; createAt >= earliestAccessibleTime {
 			order[n] = order[i]
 			n++
 		} else {
-			postList.HasInaccessiblePosts = true
+			if createAt > postList.FirstInaccessiblePostTime {
+				postList.FirstInaccessiblePostTime = createAt
+			}
 			delete(posts, postId)
 		}
 	}
@@ -96,8 +111,10 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64
 	// for example GetPosts in the CollapsedThreads = false path, parents are not added
 	// to Order
 	for postId := range posts {
-		if posts[postId].CreateAt < earliestAccessibleTime {
-			postList.HasInaccessiblePosts = true
+		if createAt := posts[postId].CreateAt; createAt < earliestAccessibleTime {
+			if createAt > postList.FirstInaccessiblePostTime {
+				postList.FirstInaccessiblePostTime = createAt
+			}
 			delete(posts, postId)
 		}
 	}
@@ -106,18 +123,20 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64
 // linearFilterPostsSlice make no assumptions about ordering, go through posts one by one
 // this is the slower fallback that is still safe if we can not
 // assume posts are ordered by CreatedAt
-func linearFilterPostsSlice(posts []*model.Post, earliestAccessibleTime int64) ([]*model.Post, bool) {
-	hasInaccessiblePosts := false
+func linearFilterPostsSlice(posts []*model.Post, earliestAccessibleTime int64) ([]*model.Post, int64) {
+	var firstInaccessiblePostTime int64 = 0
 	n := 0
 	for i := range posts {
-		if posts[i].CreateAt >= earliestAccessibleTime {
+		if createAt := posts[i].CreateAt; createAt >= earliestAccessibleTime {
 			posts[n] = posts[i]
 			n++
 		} else {
-			hasInaccessiblePosts = true
+			if createAt > firstInaccessiblePostTime {
+				firstInaccessiblePostTime = createAt
+			}
 		}
 	}
-	return posts[:n], hasInaccessiblePosts
+	return posts[:n], firstInaccessiblePostTime
 }
 
 // filterInaccessiblePosts filters out the posts, past the cloud limit
@@ -146,13 +165,18 @@ func (a *App) filterInaccessiblePosts(postList *model.PostList, options filterPo
 		}
 		if bounds.noAccessible() {
 			if lenPosts > 0 {
-				postList.HasInaccessiblePosts = true
+				firstPostCreatedAt := postList.Posts[postList.Order[0]].CreateAt
+				lastPostCreatedAt := postList.Posts[postList.Order[len(postList.Order)-1]].CreateAt
+				postList.FirstInaccessiblePostTime = max(firstPostCreatedAt, lastPostCreatedAt)
 			}
 			postList.Posts = map[string]*model.Post{}
 			postList.Order = []string{}
 			return nil
 		}
-		postList.HasInaccessiblePosts = true
+		startInaccessibleIndex, endInaccessibleIndex := bounds.getInaccessibleRange(len(postList.Order))
+		startInaccessibleCreatedAt := postList.Posts[postList.Order[startInaccessibleIndex]].CreateAt
+		endInaccessibleCreatedAt := postList.Posts[postList.Order[endInaccessibleIndex]].CreateAt
+		postList.FirstInaccessiblePostTime = max(startInaccessibleCreatedAt, endInaccessibleCreatedAt)
 
 		posts := postList.Posts
 		order := postList.Order
@@ -183,9 +207,9 @@ func (a *App) filterInaccessiblePosts(postList *model.PostList, options filterPo
 }
 
 // isInaccessiblePost indicates if the post is past the cloud plan's limit.
-func (a *App) isInaccessiblePost(post *model.Post) (bool, *model.AppError) {
+func (a *App) isInaccessiblePost(post *model.Post) (int64, *model.AppError) {
 	if post == nil {
-		return false, nil
+		return 0, nil
 	}
 
 	pl := &model.PostList{
@@ -193,22 +217,22 @@ func (a *App) isInaccessiblePost(post *model.Post) (bool, *model.AppError) {
 		Posts: map[string]*model.Post{post.Id: post},
 	}
 
-	return pl.HasInaccessiblePosts, a.filterInaccessiblePosts(pl, filterPostOptions{assumeSortedCreatedAt: true})
+	return pl.FirstInaccessiblePostTime, a.filterInaccessiblePosts(pl, filterPostOptions{assumeSortedCreatedAt: true})
 }
 
 // getFilteredAccessiblePosts returns accessible posts filtered as per the cloud plan's limit and also indicates if there were any inaccessible posts
-func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPostOptions) ([]*model.Post, bool, *model.AppError) {
+func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPostOptions) ([]*model.Post, int64, *model.AppError) {
 	if len(posts) == 0 {
-		return posts, false, nil
+		return posts, 0, nil
 	}
 
 	filteredPosts := []*model.Post{}
 	lastAccessiblePostTime, appErr := a.GetLastAccessiblePostTime()
 	if appErr != nil {
-		return filteredPosts, false, model.NewAppError("getFilteredAccessiblePosts", "app.last_accessible_post.app_error", nil, appErr.Error(), http.StatusInternalServerError)
+		return filteredPosts, 0, model.NewAppError("getFilteredAccessiblePosts", "app.last_accessible_post.app_error", nil, appErr.Error(), http.StatusInternalServerError)
 	} else if lastAccessiblePostTime == 0 {
 		// No need to filter, all posts are accessible
-		return posts, false, nil
+		return posts, 0, nil
 	}
 
 	if options.assumeSortedCreatedAt {
@@ -216,16 +240,26 @@ func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPost
 		getCreateAt := func(i int) int64 { return posts[i].CreateAt }
 		bounds := getTimeSortedPostAccessibleBounds(lastAccessiblePostTime, lenPosts, getCreateAt)
 		if bounds.allAccessible(lenPosts) {
-			return posts, false, nil
+			return posts, 0, nil
 		}
 		if bounds.noAccessible() {
-			return filteredPosts, lenPosts > 0, nil
+			var firstInaccessiblePostTime int64 = 0
+			if lenPosts > 0 {
+				firstPostCreatedAt := posts[0].CreateAt
+				lastPostCreatedAt := posts[len(posts)-1].CreateAt
+				firstInaccessiblePostTime = max(firstPostCreatedAt, lastPostCreatedAt)
+			}
+			return filteredPosts, firstInaccessiblePostTime, nil
 		}
 
+		startInaccessibleIndex, endInaccessibleIndex := bounds.getInaccessibleRange(len(posts))
+		firstPostCreatedAt := posts[startInaccessibleIndex].CreateAt
+		lastPostCreatedAt := posts[endInaccessibleIndex].CreateAt
+		firstInaccessiblePostTime := max(firstPostCreatedAt, lastPostCreatedAt)
 		filteredPosts = posts[bounds.start : bounds.end+1]
-		return filteredPosts, true, nil
+		return filteredPosts, firstInaccessiblePostTime, nil
 	}
 
-	filteredPosts, hasInaccessiblePosts := linearFilterPostsSlice(posts, lastAccessiblePostTime)
-	return filteredPosts, hasInaccessiblePosts, nil
+	filteredPosts, firstInaccessiblePostTime := linearFilterPostsSlice(posts, lastAccessiblePostTime)
+	return filteredPosts, firstInaccessiblePostTime, nil
 }

app/post_helpers_test.go

@@ -231,6 +231,7 @@ func TestFilterInaccessiblePosts(t *testing.T) {
 			"post_d",
 			"post_e",
 		}, postList.Order)
+		assert.Equal(t, int64(1), postList.FirstInaccessiblePostTime)
 	})
 
 	t.Run("descending order returns correct posts", func(t *testing.T) {
@@ -259,6 +260,8 @@ func TestFilterInaccessiblePosts(t *testing.T) {
 			"post_d",
 			"post_c",
 		}, postList.Order)
+
+		assert.Equal(t, int64(1), postList.FirstInaccessiblePostTime)
 	})
 
 	t.Run("handles mixed create at ordering correctly if correct options given", func(t *testing.T) {
@@ -332,18 +335,20 @@ func TestGetFilteredAccessiblePosts(t *testing.T) {
 
 	t.Run("ascending order returns correct posts", func(t *testing.T) {
 		posts := []*model.Post{postFromCreateAt(0), postFromCreateAt(1), postFromCreateAt(2), postFromCreateAt(3), postFromCreateAt(4)}
-		filteredPosts, _, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
+		filteredPosts, firstInaccessiblePostTime, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
 
 		assert.Nil(t, appErr)
 		assert.Equal(t, []*model.Post{postFromCreateAt(2), postFromCreateAt(3), postFromCreateAt(4)}, filteredPosts)
+		assert.Equal(t, int64(1), firstInaccessiblePostTime)
 	})
 
 	t.Run("descending order returns correct posts", func(t *testing.T) {
 		posts := []*model.Post{postFromCreateAt(4), postFromCreateAt(3), postFromCreateAt(2), postFromCreateAt(1), postFromCreateAt(0)}
-		filteredPosts, _, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
+		filteredPosts, firstInaccessiblePostTime, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true})
 
 		assert.Nil(t, appErr)
 		assert.Equal(t, []*model.Post{postFromCreateAt(4), postFromCreateAt(3), postFromCreateAt(2)}, filteredPosts)
+		assert.Equal(t, int64(1), firstInaccessiblePostTime)
 	})
 
 	t.Run("handles mixed create at ordering correctly if correct options given", func(t *testing.T) {
@@ -366,12 +371,40 @@ func TestIsInaccessiblePost(t *testing.T) {
 	defer th.TearDown()
 
 	post := &model.Post{CreateAt: 3}
-	r, appErr := th.App.isInaccessiblePost(post)
+	firstInaccessiblePostTime, appErr := th.App.isInaccessiblePost(post)
 	assert.Nil(t, appErr)
-	assert.Equal(t, false, r)
+	assert.Equal(t, int64(0), firstInaccessiblePostTime)
 
 	post = &model.Post{CreateAt: 1}
-	r, appErr = th.App.isInaccessiblePost(post)
+	firstInaccessiblePostTime, appErr = th.App.isInaccessiblePost(post)
 	assert.Nil(t, appErr)
-	assert.Equal(t, true, r)
+	assert.Equal(t, int64(1), firstInaccessiblePostTime)
+}
+
+func Test_getInaccessibleRange(t *testing.T) {
+	type test struct {
+		label         string
+		bounds        accessibleBounds
+		listLength    int
+		expectedStart int
+		expectedEnd   int
+	}
+	tests := []test{
+		{
+			label:         "inaccessible at end",
+			bounds:        accessibleBounds{start: 0, end: 3},
+			listLength:    6,
+			expectedStart: 4,
+			expectedEnd:   5,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.label, func(t *testing.T) {
+			start, end := test.bounds.getInaccessibleRange(test.listLength)
+
+			assert.Equal(t, test.expectedStart, start)
+			assert.Equal(t, test.expectedEnd, end)
+		})
+	}
 }

app/post_metadata.go

@@ -54,12 +54,12 @@ func (s *Server) initPostMetadata() {
 
 func (a *App) PreparePostListForClient(c request.CTX, originalList *model.PostList) *model.PostList {
 	list := &model.PostList{
-		Posts:                make(map[string]*model.Post, len(originalList.Posts)),
-		Order:                originalList.Order,
-		NextPostId:           originalList.NextPostId,
-		PrevPostId:           originalList.PrevPostId,
-		HasNext:              originalList.HasNext,
-		HasInaccessiblePosts: originalList.HasInaccessiblePosts,
+		Posts:                     make(map[string]*model.Post, len(originalList.Posts)),
+		Order:                     originalList.Order,
+		NextPostId:                originalList.NextPostId,
+		PrevPostId:                originalList.PrevPostId,
+		HasNext:                   originalList.HasNext,
+		FirstInaccessiblePostTime: originalList.FirstInaccessiblePostTime,
 	}
 
 	for id, originalPost := range originalList.Posts {

model/client4.go

@@ -18,30 +18,30 @@ import (
 )
 
 const (
-	HeaderRequestId            = "X-Request-ID"
-	HeaderVersionId            = "X-Version-ID"
-	HeaderClusterId            = "X-Cluster-ID"
-	HeaderEtagServer           = "ETag"
-	HeaderEtagClient           = "If-None-Match"
-	HeaderForwarded            = "X-Forwarded-For"
-	HeaderRealIP               = "X-Real-IP"
-	HeaderForwardedProto       = "X-Forwarded-Proto"
-	HeaderToken                = "token"
-	HeaderCsrfToken            = "X-CSRF-Token"
-	HeaderBearer               = "BEARER"
-	HeaderAuth                 = "Authorization"
-	HeaderCloudToken           = "X-Cloud-Token"
-	HeaderRemoteclusterToken   = "X-RemoteCluster-Token"
-	HeaderRemoteclusterId      = "X-RemoteCluster-Id"
-	HeaderRequestedWith        = "X-Requested-With"
-	HeaderRequestedWithXML     = "XMLHttpRequest"
-	HeaderHasInaccessiblePosts = "Has-Inaccessible-Posts"
-	HeaderRange                = "Range"
-	STATUS                     = "status"
-	StatusOk                   = "OK"
-	StatusFail                 = "FAIL"
-	StatusUnhealthy            = "UNHEALTHY"
-	StatusRemove               = "REMOVE"
+	HeaderRequestId                 = "X-Request-ID"
+	HeaderVersionId                 = "X-Version-ID"
+	HeaderClusterId                 = "X-Cluster-ID"
+	HeaderEtagServer                = "ETag"
+	HeaderEtagClient                = "If-None-Match"
+	HeaderForwarded                 = "X-Forwarded-For"
+	HeaderRealIP                    = "X-Real-IP"
+	HeaderForwardedProto            = "X-Forwarded-Proto"
+	HeaderToken                     = "token"
+	HeaderCsrfToken                 = "X-CSRF-Token"
+	HeaderBearer                    = "BEARER"
+	HeaderAuth                      = "Authorization"
+	HeaderCloudToken                = "X-Cloud-Token"
+	HeaderRemoteclusterToken        = "X-RemoteCluster-Token"
+	HeaderRemoteclusterId           = "X-RemoteCluster-Id"
+	HeaderRequestedWith             = "X-Requested-With"
+	HeaderRequestedWithXML          = "XMLHttpRequest"
+	HeaderFirstInaccessiblePostTime = "First-Inaccessible-Post-Time"
+	HeaderRange                     = "Range"
+	STATUS                          = "status"
+	StatusOk                        = "OK"
+	StatusFail                      = "FAIL"
+	StatusUnhealthy                 = "UNHEALTHY"
+	StatusRemove                    = "REMOVE"
 
 	ClientDir = "client"
 

model/post_list.go

@@ -16,8 +16,8 @@ type PostList struct {
 	PrevPostId string           `json:"prev_post_id"`
 	// HasNext indicates whether there are more items to be fetched or not.
 	HasNext bool `json:"has_next"`
-	// HasInaccessiblePosts tells if there are inaccessible posts, past the cloud limit.
-	HasInaccessiblePosts bool `json:"has_inaccessible_posts"`
+	// If there are inaccessible posts, FirstInaccessiblePostTime is the time of the latest inaccessible post
+	FirstInaccessiblePostTime int64 `json:"first_inaccessible_post_time"`
 }
 
 func NewPostList() *PostList {
@@ -37,12 +37,12 @@ func (o *PostList) Clone() *PostList {
 		postsCopy[k] = v.Clone()
 	}
 	return &PostList{
-		Order:                orderCopy,
-		Posts:                postsCopy,
-		NextPostId:           o.NextPostId,
-		PrevPostId:           o.PrevPostId,
-		HasNext:              o.HasNext,
-		HasInaccessiblePosts: o.HasInaccessiblePosts,
+		Order:                     orderCopy,
+		Posts:                     postsCopy,
+		NextPostId:                o.NextPostId,
+		PrevPostId:                o.PrevPostId,
+		HasNext:                   o.HasNext,
+		FirstInaccessiblePostTime: o.FirstInaccessiblePostTime,
 	}
 }