Add page_size query parameter to query-resources#33
Conversation
Allow API consumers to control result count per page via the page_size query parameter (min: 1, max: 1000, default: 50). The parameter maps directly to OpenSearch's size field and works with all existing query combinations. LFXV2-1135 Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 WalkthroughThe changes implement a Page Size feature across the query service stack. This includes API design enhancements, converter logic updates to propagate page size values, OpenSearch client refactoring to accept page size parameters, test coverage additions, and new constants for page size validation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Layer
participant Converter as Converter Layer
participant OpenSearch as OpenSearch Client
Client->>API: POST /query/resources<br/>(with page_size=20)
API->>Converter: payloadToCriteria(payload)
Note over Converter: Extract PageSize<br/>from payload
Converter->>Converter: Create Criteria<br/>with PageSize=20
API->>OpenSearch: Search(ctx, index, query, pageSize=20)
Note over OpenSearch: Execute search with<br/>page size limit
OpenSearch->>OpenSearch: Check if hasMoreResults<br/>(hits > pageSize)
OpenSearch-->>API: SearchResponse
API-->>Client: Query Results<br/>(with PageToken if needed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a page_size query parameter to the query-resources endpoint, allowing API consumers to control the number of results returned per page (min: 1, max: 1000, default: 50). The implementation properly threads the page size through the entire stack from API definition to OpenSearch client, fixing the pre-existing issue where pagination token generation always used the hardcoded default page size.
Changes:
- Added
page_sizeattribute to theSortableGoa design type with validation constraints - Updated OpenSearch client to use actual page size instead of hardcoded constant for pagination token logic
- Added comprehensive test coverage for page_size handling in converters and searcher
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pkg/constants/query.go |
Added MaxPageSize constant (1000) |
design/types.go |
Added page_size attribute to Sortable type with min/max/default constraints |
design/query-svc.go |
Added page_size parameter to query-resources HTTP endpoint |
gen/* (8 files) |
Goa-generated server/client code for page_size parameter |
cmd/service/converters.go |
Updated to use p.PageSize instead of hardcoded DefaultPageSize |
internal/infrastructure/opensearch/client.go |
Updated Search interface to accept pageSize and use it for pagination token logic |
internal/infrastructure/opensearch/searcher.go |
Updated to pass criteria.PageSize to client |
cmd/service/converters_test.go |
Added test case for explicit page_size value |
internal/infrastructure/opensearch/searcher_test.go |
Added comprehensive tests for page_size forwarding to client |
CLAUDE.md |
Resolved merge conflict and added Page Size documentation section |
charts/lfx-v2-query-service/Chart.yaml |
Bumped version to 0.4.11 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
page_sizequery parameter toquery-resourcesendpoint (min: 1, max: 1000, default: 50)Changes
design/types.go,design/query-svc.gopage_sizetoSortabletype and HTTP paramsgen/(8 files)make apigenregenerationcmd/service/converters.gop.PageSizeinstead of hardcoded constantopensearch/client.go,opensearch/searcher.gopageSizethrough interface and fix pagination token logicpkg/constants/query.goMaxPageSize = 1000searcher_test.go,converters_test.goCLAUDE.mdcharts/.../Chart.yamlVerification
make build— compilesmake test— all tests passmake lint— no new lint issuesLFXV2-1135
🤖 Generated with Claude Code