From 5001594a8fb683a38b96050620003e04a739ad54 Mon Sep 17 00:00:00 2001 From: Jannis Metrikat <120120832+jmetrikat@users.noreply.github.com> Date: Sun, 29 Dec 2024 17:52:17 +0100 Subject: [PATCH] GO-4459 Refactor pagination and 'total' calculation --- cmd/api/docs/docs.go | 4 +- cmd/api/docs/swagger.json | 4 +- cmd/api/docs/swagger.yaml | 4 +- cmd/api/handlers.go | 102 ++++++++++++-------------------------- cmd/api/handlers_test.go | 4 +- cmd/api/helper.go | 23 ++++++++- cmd/api/schemas.go | 2 +- 7 files changed, 62 insertions(+), 81 deletions(-) diff --git a/cmd/api/docs/docs.go b/cmd/api/docs/docs.go index 426156ff3..fa14eb412 100644 --- a/cmd/api/docs/docs.go +++ b/cmd/api/docs/docs.go @@ -1364,9 +1364,9 @@ const docTemplate = `{ "example": 0 }, "total": { - "description": "the total number of items returned", + "description": "the total number of items available on that endpoint", "type": "integer", - "example": 100 + "example": 1024 } } }, diff --git a/cmd/api/docs/swagger.json b/cmd/api/docs/swagger.json index bb3358676..1cddd64a3 100644 --- a/cmd/api/docs/swagger.json +++ b/cmd/api/docs/swagger.json @@ -1358,9 +1358,9 @@ "example": 0 }, "total": { - "description": "the total number of items returned", + "description": "the total number of items available on that endpoint", "type": "integer", - "example": 100 + "example": 1024 } } }, diff --git a/cmd/api/docs/swagger.yaml b/cmd/api/docs/swagger.yaml index 5d8c8a223..08959f27e 100644 --- a/cmd/api/docs/swagger.yaml +++ b/cmd/api/docs/swagger.yaml @@ -266,8 +266,8 @@ definitions: example: 0 type: integer total: - description: the total number of items returned - example: 100 + description: the total number of items available on that endpoint + example: 1024 type: integer type: object api.Reactions: diff --git a/cmd/api/handlers.go b/cmd/api/handlers.go index 0bb3f2401..2728a5f48 100644 --- a/cmd/api/handlers.go +++ b/cmd/api/handlers.go @@ -129,9 +129,7 @@ func (a *ApiServer) getSpacesHandler(c *gin.Context) { Type: model.BlockContentDataviewSort_Asc, }, }, - Keys: []string{"targetSpaceId", "name", "iconEmoji", "iconImage"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"targetSpaceId", "name", "iconEmoji", "iconImage"}, }) if resp.Error.Code != pb.RpcObjectSearchResponseError_NULL { @@ -144,8 +142,10 @@ func (a *ApiServer) getSpacesHandler(c *gin.Context) { return } - spaces := make([]Space, 0, len(resp.Records)) - for _, record := range resp.Records { + paginatedSpaces, hasMore := paginate(resp.Records, offset, limit) + spaces := make([]Space, 0, len(paginatedSpaces)) + + for _, record := range paginatedSpaces { workspace, statusCode, errorMessage := a.getWorkspaceInfo(record.Fields["targetSpaceId"].GetStringValue()) if statusCode != http.StatusOK { c.JSON(statusCode, gin.H{"message": errorMessage}) @@ -158,13 +158,7 @@ func (a *ApiServer) getSpacesHandler(c *gin.Context) { spaces = append(spaces, workspace) } - hasNext := false - if len(spaces) > limit { - hasNext = true - spaces = spaces[:limit] - } - - respondWithPagination(c, http.StatusOK, spaces, len(spaces), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, spaces, len(resp.Records), offset, limit, hasMore) } // createSpaceHandler creates a new space @@ -254,9 +248,7 @@ func (a *ApiServer) getMembersHandler(c *gin.Context) { Type: model.BlockContentDataviewSort_Asc, }, }, - Keys: []string{"id", "name", "iconEmoji", "iconImage", "identity", "globalName", "participantPermissions"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"id", "name", "iconEmoji", "iconImage", "identity", "globalName", "participantPermissions"}, }) if resp.Error.Code != pb.RpcObjectSearchResponseError_NULL { @@ -269,8 +261,10 @@ func (a *ApiServer) getMembersHandler(c *gin.Context) { return } - members := make([]Member, 0, len(resp.Records)) - for _, record := range resp.Records { + paginatedMembers, hasMore := paginate(resp.Records, offset, limit) + members := make([]Member, 0, len(paginatedMembers)) + + for _, record := range paginatedMembers { icon := a.getIconFromEmojiOrImage(record.Fields["iconEmoji"].GetStringValue(), record.Fields["iconImage"].GetStringValue()) member := Member{ @@ -286,13 +280,7 @@ func (a *ApiServer) getMembersHandler(c *gin.Context) { members = append(members, member) } - hasNext := false - if len(members) > limit { - hasNext = true - members = members[:limit] - } - - respondWithPagination(c, http.StatusOK, members, len(members), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, members, len(resp.Records), offset, limit, hasMore) } // getObjectsHandler retrieves objects in a specific space @@ -339,9 +327,7 @@ func (a *ApiServer) getObjectsHandler(c *gin.Context) { IncludeTime: true, EmptyPlacement: model.BlockContentDataviewSort_NotSpecified, }}, - Keys: []string{"id", "name", "type", "layout", "iconEmoji", "iconImage"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"id", "name", "type", "layout", "iconEmoji", "iconImage"}, }) if resp.Error.Code != pb.RpcObjectSearchResponseError_NULL { @@ -354,8 +340,10 @@ func (a *ApiServer) getObjectsHandler(c *gin.Context) { return } - objects := make([]Object, 0, len(resp.Records)) - for _, record := range resp.Records { + paginatedObjects, hasMore := paginate(resp.Records, offset, limit) + objects := make([]Object, 0, len(paginatedObjects)) + + for _, record := range paginatedObjects { icon := a.getIconFromEmojiOrImage(record.Fields["iconEmoji"].GetStringValue(), record.Fields["iconImage"].GetStringValue()) objectTypeName, statusCode, errorMessage := a.resolveTypeToName(spaceId, record.Fields["type"].GetStringValue()) if statusCode != http.StatusOK { @@ -384,13 +372,7 @@ func (a *ApiServer) getObjectsHandler(c *gin.Context) { objects = append(objects, object) } - hasNext := false - if len(objects) > limit { - hasNext = true - objects = objects[:limit] - } - - respondWithPagination(c, http.StatusOK, objects, len(objects), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, objects, len(resp.Records), offset, limit, hasMore) } // getObjectHandler retrieves a specific object in a space @@ -559,9 +541,7 @@ func (a *ApiServer) getObjectTypesHandler(c *gin.Context) { Type: model.BlockContentDataviewSort_Asc, }, }, - Keys: []string{"id", "uniqueKey", "name", "iconEmoji"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"id", "uniqueKey", "name", "iconEmoji"}, }) if resp.Error.Code != pb.RpcObjectSearchResponseError_NULL { @@ -574,8 +554,10 @@ func (a *ApiServer) getObjectTypesHandler(c *gin.Context) { return } - objectTypes := make([]ObjectType, 0, len(resp.Records)) - for _, record := range resp.Records { + paginatedTypes, hasMore := paginate(resp.Records, offset, limit) + objectTypes := make([]ObjectType, 0, len(paginatedTypes)) + + for _, record := range paginatedTypes { objectTypes = append(objectTypes, ObjectType{ Type: "object_type", Id: record.Fields["id"].GetStringValue(), @@ -585,13 +567,7 @@ func (a *ApiServer) getObjectTypesHandler(c *gin.Context) { }) } - hasNext := false - if len(objectTypes) > limit { - hasNext = true - objectTypes = objectTypes[:limit] - } - - respondWithPagination(c, http.StatusOK, objectTypes, len(objectTypes), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, objectTypes, len(resp.Records), offset, limit, hasMore) } // getObjectTypeTemplatesHandler retrieves a list of templates for a specific object type in a space @@ -650,9 +626,7 @@ func (a *ApiServer) getObjectTypeTemplatesHandler(c *gin.Context) { Value: pbtypes.String(templateTypeId), }, }, - Keys: []string{"id", "targetObjectType", "name", "iconEmoji"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"id", "targetObjectType", "name", "iconEmoji"}, }) if templateObjectsResp.Error.Code != pb.RpcObjectSearchResponseError_NULL { @@ -673,8 +647,10 @@ func (a *ApiServer) getObjectTypeTemplatesHandler(c *gin.Context) { } // Finally, open each template and populate the response - templates := make([]ObjectTemplate, 0, len(templateIds)) - for _, templateId := range templateIds { + paginatedTemplates, hasMore := paginate(templateIds, offset, limit) + templates := make([]ObjectTemplate, 0, len(paginatedTemplates)) + + for _, templateId := range paginatedTemplates { templateResp := a.mw.ObjectShow(c.Request.Context(), &pb.RpcObjectShowRequest{ SpaceId: spaceId, ObjectId: templateId, @@ -693,13 +669,7 @@ func (a *ApiServer) getObjectTypeTemplatesHandler(c *gin.Context) { }) } - hasNext := false - if len(templates) > limit { - hasNext = true - templates = templates[:limit] - } - - respondWithPagination(c, http.StatusOK, templates, len(templates), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, templates, len(templateIds), offset, limit, hasMore) } // searchHandler searches and retrieves objects across all the spaces @@ -808,9 +778,7 @@ func (a *ApiServer) searchHandler(c *gin.Context) { IncludeTime: true, EmptyPlacement: model.BlockContentDataviewSort_NotSpecified, }}, - Keys: []string{"id", "name", "type", "layout", "iconEmoji", "iconImage"}, - Offset: int32(offset), - Limit: int32(limit + 1), + Keys: []string{"id", "name", "type", "layout", "iconEmoji", "iconImage"}, // TODO split limit between spaces // Limit: paginationLimitPerSpace, // FullText: searchTerm, @@ -858,13 +826,9 @@ func (a *ApiServer) searchHandler(c *gin.Context) { }) // TODO: solve global pagination vs per space pagination - hasNext := false - if len(searchResults) > limit { - hasNext = true - searchResults = searchResults[:limit] - } + paginatedResults, hasMore := paginate(searchResults, offset, limit) - respondWithPagination(c, http.StatusOK, searchResults, len(searchResults), offset, limit, hasNext) + respondWithPagination(c, http.StatusOK, paginatedResults, len(searchResults), offset, limit, hasMore) } // getChatMessagesHandler retrieves last chat messages diff --git a/cmd/api/handlers_test.go b/cmd/api/handlers_test.go index 22cf41b90..19d32f2b4 100644 --- a/cmd/api/handlers_test.go +++ b/cmd/api/handlers_test.go @@ -232,9 +232,7 @@ func TestApiServer_GetSpacesHandler(t *testing.T) { Type: model.BlockContentDataviewSort_Asc, }, }, - Keys: []string{"targetSpaceId", "name", "iconEmoji", "iconImage"}, - Offset: offset, - Limit: limit + 1, + Keys: []string{"targetSpaceId", "name", "iconEmoji", "iconImage"}, }).Return(&pb.RpcObjectSearchResponse{ Records: []*types.Struct{ { diff --git a/cmd/api/helper.go b/cmd/api/helper.go index 34f532496..8bd9839ca 100644 --- a/cmd/api/helper.go +++ b/cmd/api/helper.go @@ -242,14 +242,33 @@ func (a *ApiServer) getTags(resp *pb.RpcObjectShowResponse) []Tag { return tags } -func respondWithPagination[T any](c *gin.Context, statusCode int, data []T, total, offset, limit int, hasNext bool) { +// respondWithPagination returns a json response with the paginated data and corresponding metadata +func respondWithPagination[T any](c *gin.Context, statusCode int, data []T, total, offset, limit int, hasMore bool) { c.JSON(statusCode, PaginatedResponse[T]{ Data: data, Pagination: PaginationMeta{ Total: total, Offset: offset, Limit: limit, - HasMore: hasNext, + HasMore: hasMore, }, }) } + +// paginate paginates the given records based on the offset and limit +func paginate[T any](records []T, offset, limit int) ([]T, bool) { + total := len(records) + start := offset + end := offset + limit + + if start > total { + start = total + } + if end > total { + end = total + } + + paginated := records[start:end] + hasMore := end < total + return paginated, hasMore +} diff --git a/cmd/api/schemas.go b/cmd/api/schemas.go index 3e9578697..33461598c 100644 --- a/cmd/api/schemas.go +++ b/cmd/api/schemas.go @@ -1,7 +1,7 @@ package api type PaginationMeta struct { - Total int `json:"total" example:"100"` // the total number of items returned + Total int `json:"total" example:"1024"` // the total number of items available on that endpoint Offset int `json:"offset" example:"0"` // the current offset Limit int `json:"limit" example:"100"` // the current limit HasMore bool `json:"has_more" example:"true"` // whether there are more items available