add pagination to tags and text_search endpoints#760
add pagination to tags and text_search endpoints#760akshat4703 wants to merge 1 commit intoOWASP:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds pagination support to the /rest/v1/tags and /rest/v1/text_search endpoints to handle large result sets more efficiently. The implementation introduces shared pagination helper functions that work consistently across different response formats (JSON, Markdown, CSV, OSCAL).
Changes:
- Added
_parse_positive_intand_paginate_documentshelper functions for consistent pagination logic - Implemented pagination in
/rest/v1/tagsendpoint with page and total_pages metadata in JSON responses - Implemented pagination in
/rest/v1/text_searchendpoint (though JSON response lacks pagination metadata) - Added comprehensive tests for pagination behavior and format parity
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| application/web/web_main.py | Added pagination helper functions and applied them to tags and text_search endpoints across all output formats |
| application/tests/web_main_test.py | Added comprehensive tests for pagination behavior, format parity, and out-of-range page handling for both endpoints |
| docs/api/openapi.yaml | Updated OpenAPI spec to document pagination parameters (though added to incorrect endpoints) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: page | ||
| in: query | ||
| required: false | ||
| description: 1-based page number for paginated results. | ||
| schema: | ||
| type: integer | ||
| minimum: 1 | ||
| - name: items_per_page | ||
| in: query | ||
| required: false | ||
| description: Number of items per page. | ||
| schema: | ||
| type: integer | ||
| minimum: 1 |
There was a problem hiding this comment.
The pagination parameters were added to the wrong endpoint. This PR adds pagination to /rest/v1/name/{crename}, but this endpoint returns a single CRE (see web_main.py lines 148-152 where it expects at most one result). Pagination doesn't make sense for this endpoint. Instead, these pagination parameters should be added to /rest/v1/tags and /rest/v1/text_search endpoints, which do implement pagination in their code but are missing the OpenAPI documentation for these parameters.
| - name: page | |
| in: query | |
| required: false | |
| description: 1-based page number for paginated results. | |
| schema: | |
| type: integer | |
| minimum: 1 | |
| - name: items_per_page | |
| in: query | |
| required: false | |
| description: Number of items per page. | |
| schema: | |
| type: integer | |
| minimum: 1 |
| _, _, paged_documents = _paginate_documents(documents) | ||
| if opt_format == SupportedFormats.Markdown.value: | ||
| return f"<pre>{mdutils.cre_to_md(documents)}</pre>" | ||
| return f"<pre>{mdutils.cre_to_md(paged_documents)}</pre>" | ||
| elif opt_format == SupportedFormats.CSV.value: | ||
| docs = sheet_utils.ExportSheet().prepare_spreadsheet( | ||
| docs=documents, storage=database | ||
| docs=paged_documents, storage=database | ||
| ) | ||
| return write_csv(docs=docs).getvalue().encode("utf-8") | ||
| elif opt_format == SupportedFormats.OSCAL.value: | ||
| return jsonify(json.loads(oscal_utils.list_to_oscal(documents))) | ||
| return jsonify(json.loads(oscal_utils.list_to_oscal(paged_documents))) | ||
|
|
||
| res = [doc.todict() for doc in documents] | ||
| res = [doc.todict() for doc in paged_documents] | ||
| return jsonify(res) |
There was a problem hiding this comment.
The text_search endpoint is missing pagination metadata (page and total_pages) in its JSON response. The tags endpoint returns {"data": [...], "page": X, "total_pages": Y}, but text_search only returns the raw array. This creates an inconsistency between the two endpoints that were both updated to support pagination in this PR. The JSON response should include page and total_pages fields for consistency.
| - name: page | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| - name: items_per_page | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| - name: format | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: string | ||
| enum: [json, md, csv, oscal] | ||
| - name: page | ||
| in: query | ||
| required: false | ||
| description: 1-based page number for paginated results. | ||
| schema: | ||
| type: integer | ||
| minimum: 1 | ||
| - name: items_per_page | ||
| in: query | ||
| required: false | ||
| description: Number of items per page. | ||
| schema: | ||
| type: integer | ||
| minimum: 1 |
There was a problem hiding this comment.
Duplicate pagination parameters defined for this endpoint. The page and items_per_page parameters are defined twice: first at lines 126-135 (without descriptions) and again at lines 142-155 (with descriptions). Remove the first occurrence (lines 126-135) and keep only the detailed definitions added in this PR.
Summary
Changes
Introduced shared pagination helpers in the API layer.
Applied pagination consistently before rendering all supported formats:
Added out-of-range page handling (404).
Enhanced /rest/v1/tags JSON response with pagination metadata:
Updated OpenAPI docs to include new query parameters for both endpoints.
Added regression tests for:
Impact
Validation