Coverage for server / utilities / pagination.py: 100%
15 statements
« prev ^ index » next coverage.py v7.13.4, created at 2026-10-04 09:33 +0000
« prev ^ index » next coverage.py v7.13.4, created at 2026-10-04 09:33 +0000
1"""
2One way to handle page/page_size, for every paginated endpoint.
4WHY THIS EXISTS. This service had FOUR behaviours for the same shape of bad
5input, live-verified against QA 0.0.0.336:
7 GET /v1/student/classes/all/fetch page_num=-1 -> 500, leaking
8 "BSON field 'skip' value must be >= 0"
9 GET /v1/student/classes/all/fetch page_size=999999 -> 200, 9,956 rows, 7.8 MB
10 GET /v1/teacher/class/all/fetch page_size=-5 -> 500 "length must be non-negative"
11 GET /v1/teacher/question/all/fetch page_num=abc -> 200, swallowed, data as normal
12 GET /v1/student/account/teacher_find anything bad -> 200, silently clamped
14and a fifth, `validate_params`, which rejects with 400 but is only reachable
15from code no router is wired to.
17THE POLICY, and where it comes from. Google AIP-158 (https://google.aip.dev/158)
18is the only published guidance that resolves reject-vs-clamp rather than picking
19a side, and it splits by case:
21 * a NEGATIVE page size "must" be an error -> 400
22 * page_size 0 or absent: "the API chooses an
23 appropriate default" -> default, not an error
24 * over the maximum: "should coerce down to the
25 maximum permitted page size" -> clamp, not an error
27400 rather than 422 because RFC 9110 15.5.21 scopes 422 to request CONTENT —
28"the syntax of the request content is correct" — and a GET has no content. 422
29for a query string is a FastAPI-ism its own maintainer calls "interpretable and
30subjective"; Azure's REST guidelines and OGC API-Common both specify 400.
32THE CAP IS NOT OPTIONAL. OWASP API4:2023 (Unrestricted Resource Consumption)
33names the missing limit directly: an API is vulnerable if "number of records per
34page to return in a single request-response" is unbounded. Before this, one
35student request returned 7.8 MB.
37WHY 1000 AND NOT 100. eruditiontx-client-mvp sends page_size=1000 for the
38student class list (ClassListPage.jsx ALL_CLASSES_PAGE_SIZE). A cap of 100 would
39silently truncate that list to 100 classes — a quieter bug than the one being
40fixed. 1000 bounds the response without breaking a live caller. That call site
41is "fetch everything" wearing pagination's clothes and should be paginated
42properly; until it is, the cap accommodates it.
44ONE ENDPOINT DELIBERATELY STILL CLAMPS, and it is not an oversight.
45`UserService._clamp_pagination` (server/services/common/users.py) coerces a
46negative page to 1 rather than rejecting it. Zephyr EI-T656 governs that
47endpoint and its objective allows exactly two outcomes — "rejected with 422 or
48clamped to a documented default". This module rejects with 400, which is
49neither, so migrating it would move that endpoint from matching the case to
50matching nothing. Aligning the two needs the case reworded first, then a
51coordinated change across this repo and teacher-student-automation.
53Developer: Allan Ninal
54Date: 2026-09-21
55"""
57from fastapi import HTTPException, status
59# Used when the caller says nothing, or says 0 (AIP-158: 0 means "unspecified").
60DEFAULT_PAGE_SIZE = 100
62# The ceiling. Raising this re-opens the resource-consumption hole; lowering it
63# below 1000 silently truncates the student class list. Change with both in mind.
64MAX_PAGE_SIZE = 1000
67def resolve_pagination(
68 page: int | None,
69 page_size: int | None,
70 *,
71 default_page_size: int = DEFAULT_PAGE_SIZE,
72 max_page_size: int = MAX_PAGE_SIZE,
73) -> tuple[int, int]:
74 """Return a (page, page_size) pair that is always safe to hand to Mongo.
76 Args:
77 page: 1-based page number. None or 0 means "the first one".
78 page_size: rows per page. None or 0 means "use the default".
80 Returns:
81 (page, page_size), both >= 1, with page_size <= max_page_size.
83 Raises:
84 HTTPException: 400, when either value is negative. A negative page is
85 not an under-specified request that we can sensibly guess at — it is
86 a caller bug, and answering it with row one would hide that. It is
87 also the value that reaches MongoDB as a negative `skip` and takes
88 the query down with it.
89 """
90 if page is not None and page < 0:
91 raise HTTPException(
92 status_code=status.HTTP_400_BAD_REQUEST,
93 detail="Page number cannot be negative.",
94 )
95 if page_size is not None and page_size < 0:
96 raise HTTPException(
97 status_code=status.HTTP_400_BAD_REQUEST,
98 detail="Page size cannot be negative.",
99 )
101 resolved_page = page if page else 1
102 resolved_page_size = page_size if page_size else default_page_size
104 return resolved_page, min(resolved_page_size, max_page_size)
107def page_count(total: int, page_size: int) -> int:
108 """Total pages, without dividing by zero.
110 ``page_size`` is already >= 1 coming out of resolve_pagination, but this is
111 the exact expression that produced a 500 ("division by zero") on the live
112 service, so it is not left inline for someone to reintroduce.
113 """
114 if page_size < 1:
115 return 0
116 return -(-total // page_size) # ceil, without importing math