Implement pagination support across various API handlers and repositories#2561
Merged
Merged
Conversation
…ries This commit introduces a new pagination mechanism by adding a `parsePagination` function to standardize the handling of `limit` and `offset` query parameters. The pagination logic has been integrated into multiple API handlers, allowing for more efficient data retrieval and response structuring. Additionally, corresponding changes have been made in the repository layer to support paginated queries for APIs, applications, gateways, and subscription plans, enhancing the overall performance and usability of the API platform.
Thushani-Jayasekera
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub,
tharindu1st and
thivindu
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR replaces the generated error payload with a structured error schema and standardizes list pagination across platform-api and ai-workspace. It adds shared pagination helpers, updates OpenAPI and generated contracts, rewires repositories/services/handlers, and changes client code to consume ChangesStandardized Error Schema
Pagination Standardization Public contracts and generated models
Pagination wiring and consumers
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant Service
participant Repository
Client->>Handler: GET /applications?limit=&offset=&sortBy=&query=
Handler->>Handler: parseListOptions(r)
Handler->>Service: GetApplicationsByOrganization(orgID, opts)
Service->>Repository: CountApplicationsByProjectID(opts.Search)
Repository-->>Service: total
Service->>Repository: GetApplicationsByProjectIDPaginated(opts)
Repository-->>Service: pagedApps
Service-->>Handler: ApplicationListResponse{List, Count, Pagination}
Handler-->>Client: JSON response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/internal/handler/subscription_handler.go (1)
206-213: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the remaining devportal list consumer to
list.
tests/integration-e2e/steps_devportal_lifecycle_test.go:163-166still unmarshals{"subscriptions": ...}fromGET /subscriptions; switch that fixture to the newlistenvelope so it matches the API response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/internal/handler/subscription_handler.go` around lines 206 - 213, The remaining devportal list consumer still expects the old subscriptions envelope, so update the integration fixture to match the new list response shape. In tests/integration-e2e/steps_devportal_lifecycle_test.go, adjust the GET /subscriptions mock/unmarshal payload to use the list field instead of subscriptions so it aligns with SubscriptionHandler’s list response.
🧹 Nitpick comments (5)
platform-api/internal/handler/subscription_plan_handler.go (1)
257-265: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse the generated
api.SubscriptionPlanListResponseinstead of an ad-hoc map.Other rewritten handlers in this cohort (
ListOrganizations,ListAPIs,ListProjects) return the typed generated response struct, but this builds a rawmap[string]any{"list": ..., "count": ..., "pagination": ...}. The generatedSubscriptionPlanListResponse(inplatform-api/api/generated.go, lines 2419-2426) already modelsCount/List/Paginationwith the same field names — using it here keeps the response type-checked against the OpenAPI contract instead of relying on the map keys staying in sync manually.♻️ Suggested refactor
- httputil.WriteJSON(w, http.StatusOK, map[string]any{ - "list": items, - "count": len(items), - "pagination": api.Pagination{ - Total: total, - Offset: offset, - Limit: limit, - }, - }) + httputil.WriteJSON(w, http.StatusOK, api.SubscriptionPlanListResponse{ + List: items, + Count: len(items), + Pagination: api.Pagination{ + Total: total, + Offset: offset, + Limit: limit, + }, + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/internal/handler/subscription_plan_handler.go` around lines 257 - 265, The subscription plan list handler is returning an ad-hoc map instead of the generated typed response, so update the ListSubscriptionPlans flow in subscription_plan_handler to use api.SubscriptionPlanListResponse like ListOrganizations/ListAPIs/ListProjects do. Replace the raw map in the JSON write path with the generated response struct, populating Count, List, and Pagination from the existing items, total, offset, and limit values so the response stays type-checked against the OpenAPI contract.platform-api/internal/service/llm_apikey.go (1)
65-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated in-memory pagination logic vs.
llm_proxy_apikey.go.
ListLLMProviderAPIKeysandListLLMProxyAPIKeys(inplatform-api/internal/service/llm_proxy_apikey.go) are near-identical: fetch keys, filter byCreatedBy == userID, map toapi.APIKeyItem, thentotal := len(items); page := paginateSlice(...). Consider extracting a shared helper (e.g.buildPaginatedAPIKeyItems(keys []*model.APIKey, userID string, identity *IdentityService, limit, offset int) ([]api.APIKeyItem, int)) to avoid maintaining two copies of this logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/internal/service/llm_apikey.go` around lines 65 - 120, `ListLLMProviderAPIKeys` duplicates the same in-memory filtering, identity resolution, item mapping, and pagination flow used by `ListLLMProxyAPIKeys`, so extract that shared logic into a common helper. Move the `CreatedBy == userID` filter, `ResolveIdentityField`, `api.APIKeyItem` construction, and `paginateSlice`/total calculation into a reusable function (for example, a helper that takes the fetched keys, `userID`, and the identity service). Then have both service methods call the helper and keep only the provider/proxy-specific repository fetch and error handling in each method.platform-api/api/generated.go (1)
432-442: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBare enum constant names risk future collisions; inconsistent with sibling enums.
ListRESTAPIsParamsSortBy/ListRESTAPIsParamsSortOrderconstants are generated as unprefixedCreatedAt,Name,Asc,Desc, while the equivalent enums for Applications/Gateways/Projects (lines 359-382, 420-431) are consistently prefixed (ListApplicationsParamsSortByCreatedAt, etc.). oapi-codegen normally prefixes enum constants with their type name to avoid collisions, only emitting bare names when an extension likex-enum-varnames/x-enumNamesexplicitly overrides them. This is likely a spec inconsistency inopenapi.yamlfor theListRESTAPIssortBy/sortOrder parameters. Bare top-level identifiers likeName,Asc,Descin a package this large are a latent naming-collision risk for future additions.🔧 Likely spec fix (in openapi.yaml, not shown in this batch)
- x-enum-varnames: [CreatedAt, Name] + x-enum-varnames: [ListRESTAPIsParamsSortByCreatedAt, ListRESTAPIsParamsSortByName]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/api/generated.go` around lines 432 - 442, The `ListRESTAPIsParamsSortBy` and `ListRESTAPIsParamsSortOrder` enum constants are inconsistently generated as bare names (`CreatedAt`, `Name`, `Asc`, `Desc`) instead of the prefixed style used by sibling enums. Update the OpenAPI spec for the `ListRESTAPIs` sort parameters so oapi-codegen emits prefixed constants matching `ListApplicationsParamsSortBy...` and related enums, avoiding top-level name collisions. Locate the affected generation by `ListRESTAPIsParamsSortBy` and `ListRESTAPIsParamsSortOrder` in the generated API definitions and align their enum var names with the existing prefixed convention.platform-api/internal/handler/pagination.go (1)
93-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate generic pagination-window helper.
pageWindowhere is functionally identical topaginateSliceinplatform-api/internal/service/pagination.go(same clamping logic, same signature shape). Consider exporting one implementation (e.g. from a shared internalpaginationpackage) and having both handler and service import it, to avoid the two copies drifting apart under future edits.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/internal/handler/pagination.go` around lines 93 - 108, The generic pagination helper is duplicated between pageWindow in the handler and paginateSlice in the service, so consolidate the shared clamping/slicing logic into one reusable internal helper and have both call it. Move the implementation to a common internal pagination location (or otherwise reuse one symbol), then update the handler’s pageWindow and the service’s paginateSlice references to delegate to that single function so the behavior cannot drift.platform-api/internal/handler/gateway.go (1)
507-522: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using a typed response struct instead of
map[string]any.Other list handlers in this PR use generated typed structs (
GatewayListResponse,GatewayTokenListResponse, etc.) with proper JSON tags. Themap[string]anyapproach here works but is less type-safe — a key typo would silently break the JSON contract. If no generatedCustomPolicyListResponsetype exists, consider defining a local DTO struct to match the standardized envelope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/internal/handler/gateway.go` around lines 507 - 522, The custom policies list handler currently returns a map, which is less type-safe than the other list handlers in gateway.go. Update the response in the ListCustomPolicies path to use a typed DTO/response struct (prefer a generated CustomPolicyListResponse if available, otherwise define a local struct) and keep the same fields and JSON tags as the existing count/list/pagination envelope. Use the existing httputil.WriteJSON call site and the ListCustomPolicies flow to locate and replace the map-based response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/internal/handler/pagination.go`:
- Around line 43-65: The `parsePagination` limit handling is asymmetric: values
below `minPageLimit` fall back to `defaultPageLimit`, but values above
`maxPageLimit` are clamped instead of using the same fallback behavior. Update
`parsePagination` in the pagination handler so out-of-range `limit` values are
handled consistently on both sides, using one clear rule for invalid limits
rather than mixing defaulting and clamping. Keep the existing `offset` parsing
unchanged.
In `@platform-api/resources/openapi.yaml`:
- Around line 8633-8644: The shared limit-Q query parameter now changes the GET
/secrets default page size from 25 to 20, so verify whether this behavioral
change is intended. Check the GET /secrets operation’s parameter reference and
the limit-Q component in openapi.yaml, then either restore the previous default
to 25 or update the API definition and related consumers/docs if 20 is the new
expected default.
---
Outside diff comments:
In `@platform-api/internal/handler/subscription_handler.go`:
- Around line 206-213: The remaining devportal list consumer still expects the
old subscriptions envelope, so update the integration fixture to match the new
list response shape. In tests/integration-e2e/steps_devportal_lifecycle_test.go,
adjust the GET /subscriptions mock/unmarshal payload to use the list field
instead of subscriptions so it aligns with SubscriptionHandler’s list response.
---
Nitpick comments:
In `@platform-api/api/generated.go`:
- Around line 432-442: The `ListRESTAPIsParamsSortBy` and
`ListRESTAPIsParamsSortOrder` enum constants are inconsistently generated as
bare names (`CreatedAt`, `Name`, `Asc`, `Desc`) instead of the prefixed style
used by sibling enums. Update the OpenAPI spec for the `ListRESTAPIs` sort
parameters so oapi-codegen emits prefixed constants matching
`ListApplicationsParamsSortBy...` and related enums, avoiding top-level name
collisions. Locate the affected generation by `ListRESTAPIsParamsSortBy` and
`ListRESTAPIsParamsSortOrder` in the generated API definitions and align their
enum var names with the existing prefixed convention.
In `@platform-api/internal/handler/gateway.go`:
- Around line 507-522: The custom policies list handler currently returns a map,
which is less type-safe than the other list handlers in gateway.go. Update the
response in the ListCustomPolicies path to use a typed DTO/response struct
(prefer a generated CustomPolicyListResponse if available, otherwise define a
local struct) and keep the same fields and JSON tags as the existing
count/list/pagination envelope. Use the existing httputil.WriteJSON call site
and the ListCustomPolicies flow to locate and replace the map-based response.
In `@platform-api/internal/handler/pagination.go`:
- Around line 93-108: The generic pagination helper is duplicated between
pageWindow in the handler and paginateSlice in the service, so consolidate the
shared clamping/slicing logic into one reusable internal helper and have both
call it. Move the implementation to a common internal pagination location (or
otherwise reuse one symbol), then update the handler’s pageWindow and the
service’s paginateSlice references to delegate to that single function so the
behavior cannot drift.
In `@platform-api/internal/handler/subscription_plan_handler.go`:
- Around line 257-265: The subscription plan list handler is returning an ad-hoc
map instead of the generated typed response, so update the ListSubscriptionPlans
flow in subscription_plan_handler to use api.SubscriptionPlanListResponse like
ListOrganizations/ListAPIs/ListProjects do. Replace the raw map in the JSON
write path with the generated response struct, populating Count, List, and
Pagination from the existing items, total, offset, and limit values so the
response stays type-checked against the OpenAPI contract.
In `@platform-api/internal/service/llm_apikey.go`:
- Around line 65-120: `ListLLMProviderAPIKeys` duplicates the same in-memory
filtering, identity resolution, item mapping, and pagination flow used by
`ListLLMProxyAPIKeys`, so extract that shared logic into a common helper. Move
the `CreatedBy == userID` filter, `ResolveIdentityField`, `api.APIKeyItem`
construction, and `paginateSlice`/total calculation into a reusable function
(for example, a helper that takes the fetched keys, `userID`, and the identity
service). Then have both service methods call the helper and keep only the
provider/proxy-specific repository fetch and error handling in each method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1abee8ef-5333-497f-9384-eb7ba4a5b855
📒 Files selected for processing (41)
platform-api/api/generated.goplatform-api/internal/dto/secret.goplatform-api/internal/handler/api.goplatform-api/internal/handler/api_deployment.goplatform-api/internal/handler/apikey_user.goplatform-api/internal/handler/application.goplatform-api/internal/handler/gateway.goplatform-api/internal/handler/llm.goplatform-api/internal/handler/llm_apikey.goplatform-api/internal/handler/llm_deployment.goplatform-api/internal/handler/llm_proxy_apikey.goplatform-api/internal/handler/mcp.goplatform-api/internal/handler/mcp_deployment.goplatform-api/internal/handler/organization.goplatform-api/internal/handler/pagination.goplatform-api/internal/handler/project.goplatform-api/internal/handler/secret.goplatform-api/internal/handler/secret_integration_test.goplatform-api/internal/handler/subscription_handler.goplatform-api/internal/handler/subscription_plan_handler.goplatform-api/internal/repository/api.goplatform-api/internal/repository/application.goplatform-api/internal/repository/gateway.goplatform-api/internal/repository/interfaces.goplatform-api/internal/repository/pagination.goplatform-api/internal/repository/pagination_test.goplatform-api/internal/repository/project.goplatform-api/internal/repository/subscription_plan_repository.goplatform-api/internal/service/api.goplatform-api/internal/service/apikey_user.goplatform-api/internal/service/application.goplatform-api/internal/service/application_test.goplatform-api/internal/service/custom_policy_test.goplatform-api/internal/service/gateway.goplatform-api/internal/service/llm_apikey.goplatform-api/internal/service/llm_proxy_apikey.goplatform-api/internal/service/pagination.goplatform-api/internal/service/project.goplatform-api/internal/service/secret_service.goplatform-api/internal/service/subscription_plan_service.goplatform-api/resources/openapi.yaml
This commit updates the API key retrieval functions across multiple modules to utilize a new pagination mechanism. The `fetchAllPages` utility is introduced to streamline the fetching of paginated data, ensuring that all pages of API keys are collected efficiently. Additionally, the response structure has been modified to use `list` instead of `items`, and pagination details are now included in the responses, enhancing the overall data handling and user experience.
malinthaprasan
previously approved these changes
Jul 9, 2026
This commit refines the pagination logic by introducing a new `pagination` package that centralizes the windowing functionality. The `parsePagination` function has been updated to clamp out-of-range values, ensuring robust handling of `limit` and `offset` parameters. Additionally, various service methods have been modified to utilize this new pagination mechanism, enhancing code clarity and maintainability. The response structures in API key retrieval functions have also been updated to align with the new pagination approach.
malinthaprasan
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
sortBy/sortOrder/queryto core collection list endpointsSummary
Extends the standard collection
GETcontract (APR-008) with sorting and searchon the four core org-scoped resources. Building on the existing
limit/offsetpagination, list endpoints now accept:
sortByname|createdAt(defaultcreatedAt)sortOrderasc|desc(defaultdesc)queryWired on:
GET /rest-apis,GET /projects,GET /applications,GET /gateways.sortBy/sortOrder/queryare applied at the database (SQLORDER BY+WHERE handle LIKE ?), and the same search filter is mirrored into theCOUNTquery sopagination.totalstays consistent when a search is active.Design notes
sortByis an enum).sortBy/sortOrdercannot be bind parameters — they are interpolated into the
ORDER BYclause. Anew
ListOptions.resolveSortmaps the client'ssortBytoken to a real columnonly through a hardcoded allowlist (
listSortColumns), falling back to thedefault column on anything unrecognized, and constrains the direction to the
ASC/DESCconstants. The raw client string never reaches the SQL.handleSearchClausebuildsAND LOWER(handle) LIKE ? ESCAPE '\'with the term lowercased and its%/_/\escaped, sowildcards match literally. The value is a bind parameter — no injection surface.
sortBy(or garbagesortOrder) silently falls back to the default rather than erroring, matchinghow out-of-range
limit/offsetare already clamped.repository.ListOptionsstruct carries the inputshandler → service → repository, keeping method signatures from ballooning.
Handlers parse via a new
parseListOptionshelper (consistent with theexisting
parsePagination).Changes
Repository
internal/repository/pagination.go(new) —ListOptions,resolveSort(allowlist-based safe
ORDER BY),handleSearchClause(escaped case-insensitiveLIKE).interfaces.go,api.go,project.go,application.go,gateway.go—List*methods takeListOptions;Count*methods take asearcharg; SQLupdated (gateway's inner/outer join order kept in sync).
Service
api.go,project.go,application.go,gateway.go— threadListOptionsthrough; pass
searchto the count call; fixed the extraCountApplicationsByProjectIDcaller inproject.go.Handler
pagination.go— addedparseListOptions(normalizessortOrder, passessortBythrough for downstream allowlist resolution, trimsquery).api.go,project.go,application.go,gateway.go— useparseListOptions.Spec / generated
resources/openapi.yaml— added sharedsortBy-Q/sortOrder-Q/query-Qparameter components and referenced them on the 4 list operations.
api/generated.go— regenerated viamake generate(oapi-codegen v2.5.1).Tests
internal/repository/pagination_test.go(new) — covers the allowlist fallback(including an injection-attempt token) and
LIKEwildcard/backslash escaping.internal/service/application_test.go— updated mock signatures.Verification
go build ./...,go vet ./internal/..., and the service / handler / repositorytest suites all pass.
Live smoke test (running server, file-based auth, 4 seeded projects)
GET /api/v0.9/projects:inventory, payment-gateway, orders-service, payments-api(createdAt desc)?sortBy=name&sortOrder=ascAlpha Orders, Beta Inventory, Mango Payment Gateway, Zeta Payments?sortBy=name&sortOrder=desc?query=paymentpayment-gateway, payments-api—total: 2(matches handle, not display name)?query=PAYMENT?query=payment&sortBy=name&sortOrder=asc?limit=2&offset=0&sortBy=name→offset=2[orders-service, inventory]then[payment-gateway, payments-api],total: 4Safety cases:
?sortBy=created_at;DROP TABLE projects?query=%total: 0—%escaped and matched literally, not as a wildcard?query=zzz-nonexistenttotal: 0Example envelope (
?sortBy=name&sortOrder=asc&limit=1):{ "count": 1, "list": [ { "id": "orders-service", "displayName": "Alpha Orders", "description": "orders", "organizationId": "default", "createdAt": "2026-07-09T10:57:10+05:30", "updatedAt": "2026-07-09T10:57:10+05:30" } ], "pagination": { "limit": 1, "offset": 0, "total": 4 } }Follow-ups (not in this PR)
The remaining SQL-paginated collections (organizations, subscriptions,
subscription plans, secrets, LLM providers / proxies / templates) can adopt the
same pattern — a few lines per repository method plus the service/handler wiring —
when needed.