Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 102 additions & 4 deletions application/tests/web_main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,62 @@ def test_find_document_by_tag(self) -> None:
response = client.get(f"/rest/v1/tags?tag=CW")
self.assertEqual(404, response.status_code)

expected = {"data": [cres["ca"].todict(), cres["cb"].todict()]}
expected = {
"data": [cres["ca"].todict(), cres["cb"].todict()],
"page": 1,
"total_pages": 1,
}

response = client.get(f"/rest/v1/tags?tag=ta")
self.assertEqual(200, response.status_code)
self.assertCountEqual(json.loads(response.data.decode()), expected)
payload = json.loads(response.data.decode())
self.assertEqual(1, payload["page"])
self.assertEqual(1, payload["total_pages"])
self.assertEqual(
sorted(d["id"] for d in payload["data"]),
sorted(d["id"] for d in expected["data"]),
)

@patch.object(db, "Node_collection")
def test_tags_pagination_and_format_parity(self, db_mock) -> None:
docs = [
defs.CRE(id="111-111", description="A", name="A", tags=["ta"]),
defs.CRE(id="222-222", description="B", name="B", tags=["ta"]),
defs.CRE(id="333-333", description="C", name="C", tags=["ta"]),
]
db_mock.return_value.get_by_tags.return_value = docs

with self.app.test_client() as client:
json_response = client.get("/rest/v1/tags?tag=ta&page=2&items_per_page=1")
payload = json.loads(json_response.data.decode())
self.assertEqual(200, json_response.status_code)
self.assertEqual(2, payload["page"])
self.assertEqual(3, payload["total_pages"])
self.assertEqual("222-222", payload["data"][0]["id"])

md_response = client.get(
"/rest/v1/tags?tag=ta&page=2&items_per_page=1&format=md"
)
self.assertEqual(200, md_response.status_code)
self.assertIn("222-222", md_response.data.decode())

csv_response = client.get(
"/rest/v1/tags?tag=ta&page=2&items_per_page=1&format=csv"
)
self.assertEqual(200, csv_response.status_code)
self.assertGreater(len(csv_response.data), 0)

oscal_response = client.get(
"/rest/v1/tags?tag=ta&page=2&items_per_page=1&format=oscal"
)
oscal_payload = json.loads(oscal_response.data.decode())
self.assertEqual(200, oscal_response.status_code)
self.assertIn("metadata", oscal_payload)

out_of_range_response = client.get(
"/rest/v1/tags?tag=ta&page=99&items_per_page=1"
)
self.assertEqual(404, out_of_range_response.status_code)

def test_test_search(self) -> None:
collection = db.Node_collection()
Expand Down Expand Up @@ -469,7 +520,10 @@ def test_test_search(self) -> None:
for r in requests:
response = client.get(r)
self.assertEqual(200, response.status_code)
self.assertCountEqual(response.json, expected)
payload = response.json
self.assertEqual(1, payload["page"])
self.assertEqual(1, payload["total_pages"])
self.assertCountEqual(payload["data"], expected)

expected = [docs["sa"].todict()]
srequests = [
Expand All @@ -481,7 +535,51 @@ def test_test_search(self) -> None:
for r in srequests:
resp = client.get(r)
self.assertEqual(200, resp.status_code)
self.assertDictEqual(resp.json[0], expected[0])
self.assertDictEqual(resp.json["data"][0], expected[0])

@patch.object(db, "Node_collection")
def test_text_search_pagination_and_format_parity(self, db_mock) -> None:
docs = [
defs.Standard(name="SB", section="s1", subsection="a"),
defs.Standard(name="SB", section="s2", subsection="b"),
defs.Standard(name="SB", section="s3", subsection="c"),
]
db_mock.return_value.text_search.return_value = docs

with self.app.test_client() as client:
json_response = client.get(
"/rest/v1/text_search?text=SB&page=2&items_per_page=1"
)
payload = json.loads(json_response.data.decode())
self.assertEqual(200, json_response.status_code)
self.assertEqual(2, payload["page"])
self.assertEqual(3, payload["total_pages"])
self.assertEqual(1, len(payload["data"]))
self.assertEqual("s2", payload["data"][0]["section"])

md_response = client.get(
"/rest/v1/text_search?text=SB&page=2&items_per_page=1&format=md"
)
self.assertEqual(200, md_response.status_code)
self.assertIn("s2", md_response.data.decode())

csv_response = client.get(
"/rest/v1/text_search?text=SB&page=2&items_per_page=1&format=csv"
)
self.assertEqual(200, csv_response.status_code)
self.assertGreater(len(csv_response.data), 0)

oscal_response = client.get(
"/rest/v1/text_search?text=SB&page=2&items_per_page=1&format=oscal"
)
oscal_payload = json.loads(oscal_response.data.decode())
self.assertEqual(200, oscal_response.status_code)
self.assertEqual(1, len(oscal_payload["controls"]))

out_of_range_response = client.get(
"/rest/v1/text_search?text=SB&page=99&items_per_page=1"
)
self.assertEqual(404, out_of_range_response.status_code)

def test_find_root_cres(self) -> None:
self.maxDiff = None
Expand Down
26 changes: 26 additions & 0 deletions application/web/openapi_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ class NodeListResponseSchema(Schema):

class DataListResponseSchema(Schema):
data = fields.List(fields.Dict(keys=fields.Str(), values=fields.Raw()))
page = fields.Int(
required=False,
metadata={"description": "Current 1-based page (when pagination is applied)"},
)
total_pages = fields.Int(
required=False,
metadata={
"description": "Total pages for the result set (when pagination is applied)"
},
)


class TagQuerySchema(FormatQuerySchema):
Expand All @@ -53,10 +63,26 @@ class TagQuerySchema(FormatQuerySchema):
required=True,
metadata={"description": "Tag name(s)"},
)
page = fields.Int(
required=False,
metadata={"description": "1-based page number for paginated results"},
)
items_per_page = fields.Int(
required=False,
metadata={"description": "Number of items per page (capped server-side)"},
)


class TextSearchQuerySchema(FormatQuerySchema):
text = fields.Str(required=True, metadata={"description": "Search query"})
page = fields.Int(
required=False,
metadata={"description": "1-based page number for paginated results"},
)
items_per_page = fields.Int(
required=False,
metadata={"description": "Number of items per page (capped server-side)"},
)


class NodeQuerySchema(FormatQuerySchema):
Expand Down
53 changes: 42 additions & 11 deletions application/web/web_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from functools import wraps
import json
import logging
import math
import os
import io
import pathlib
Expand Down Expand Up @@ -99,6 +100,35 @@ class SupportedFormats(Enum):
OSCAL = "oscal"


def _parse_positive_int(value: str | None, default: int) -> int:
if value is None:
return default
try:
parsed = int(value)
if parsed > 0:
return parsed
except (TypeError, ValueError):
pass
return default


def _paginate_documents(
documents: list[defs.Document],
) -> tuple[int, int, list[defs.Document]]:
"""Slice ``documents`` using ``page`` / ``items_per_page`` query args."""
page = _parse_positive_int(request.args.get("page"), 1)
items_per_page = min(
_parse_positive_int(request.args.get("items_per_page"), ITEMS_PER_PAGE),
MAX_ITEMS_PER_PAGE,
)
Comment on lines +120 to +123

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Maintain API consistency for pagination parameters.

The existing /rest/v1/all_cres endpoint uses per_page for its page-size query parameter (as seen in application/tests/web_main_test.py). Introducing items_per_page here creates an inconsistency in the API's pagination contract.

Consider renaming items_per_page to per_page here, and updating the OpenAPI schemas and tests accordingly, to ensure a uniform API experience across all endpoints.

🤖 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 `@application/web/web_main.py` around lines 120 - 123, Rename the pagination
query parameter handled in the items-per-page calculation from items_per_page to
per_page, and update the corresponding OpenAPI schema definitions and tests to
use per_page consistently with /rest/v1/all_cres.

total_pages = max(1, math.ceil(len(documents) / items_per_page))
if page > total_pages:
abort(404, "Page does not exist")
start = (page - 1) * items_per_page
end = start + items_per_page
return page, total_pages, documents[start:end]


def extend_cre_with_tag_links(
cre: defs.CRE, collection: db.Node_collection
) -> defs.CRE:
Expand Down Expand Up @@ -296,7 +326,6 @@ def find_node_by_name(
abort(404, "Node does not exist")


# TODO: (spyros) paginate
@openapi_documented("find_document_by_tag")
@app.route("/rest/v1/tags", methods=["GET"])
def find_document_by_tag() -> Any:
Expand All @@ -309,15 +338,16 @@ def find_document_by_tag() -> Any:
opt_format = request.args.get("format")
documents = database.get_by_tags(tags)
if documents:
res = [doc.todict() for doc in documents]
result = {"data": res}
page, total_pages, paged_documents = _paginate_documents(documents)
res = [doc.todict() for doc in paged_documents]
result = {"data": res, "page": page, "total_pages": total_pages}
# if opt_osib:
# result["osib"] = odefs.cre2osib(documents).todict()
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:
Expand All @@ -330,7 +360,7 @@ def find_document_by_tag() -> Any:
"(compliance-trestle not installed).",
)

return jsonify(json.loads(oscal_utils.list_to_oscal(documents)))
return jsonify(json.loads(oscal_utils.list_to_oscal(paged_documents)))

return jsonify(result)
abort(404, "Tag does not exist")
Expand Down Expand Up @@ -608,11 +638,12 @@ def text_search() -> Any:
opt_format = request.args.get("format")
documents = database.text_search(text)
if documents:
page, total_pages, 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:
Expand All @@ -625,10 +656,10 @@ def text_search() -> Any:
"(compliance-trestle not installed).",
)

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]
return jsonify(res)
res = [doc.todict() for doc in paged_documents]
return jsonify({"data": res, "page": page, "total_pages": total_pages})
else:
abort(404, "No object matches the given search terms")

Expand Down
30 changes: 30 additions & 0 deletions docs/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,18 @@ paths:
style: form
explode: true
description: Tag name(s)
- name: page
in: query
required: false
schema:
type: integer
description: 1-based page number for paginated results
- name: items_per_page
in: query
required: false
schema:
type: integer
description: Number of items per page (capped server-side)
responses:
'200':
description: Success
Expand Down Expand Up @@ -483,6 +495,18 @@ paths:
schema:
type: string
description: Search query
- name: page
in: query
required: false
schema:
type: integer
description: 1-based page number for paginated results
- name: items_per_page
in: query
required: false
schema:
type: integer
description: Number of items per page (capped server-side)
responses:
'200':
description: Success
Expand Down Expand Up @@ -929,6 +953,12 @@ components:
items:
type: object
additionalProperties: {}
page:
type: integer
description: Current 1-based page (when pagination is applied)
total_pages:
type: integer
description: Total pages for the result set (when pagination is applied)
additionalProperties: false
MapAnalysisResponseSchema:
type: object
Expand Down