refactor: restructure API pagination metadata for better frontend usability
- Create PaginationMetadata model with computed derived fields (total_pages, has_next_page, has_prev_page)
- Update PaginatedListResponse to embed pagination metadata in a separate 'pagination' object
- Add create_pagination_metadata() factory function in utils/pagination.py for consistent computation
- Update all paginated service functions to use new structure:
- history_service.list_history()
- blocklist_service.get_import_logs()
- jail_service.get_jail_banned_ips()
- ban_mappers.map_domain_dashboard_ban_list_to_response()
- Update response model docstrings with new structure examples
- Update Backend-Development.md documentation with new pagination patterns
- Update test fixtures to work with new response structure
Response shape changes from:
{"items": [...], "total": 100, "page": 1, "page_size": 50}
To:
{"items": [...], "pagination": {"page": 1, "page_size": 50, "total": 100, "total_pages": 2, "has_next_page": true, "has_prev_page": false}}
Benefits:
- Frontend receives all pagination state needed for UI controls
- No need for frontend to calculate total_pages or page navigation logic
- Consolidated pagination metadata reduces field sprawl
- OpenAPI schema automatically reflects changes
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -28,6 +28,7 @@ from app.models.ban_domain import (
|
||||
DomainDashboardBanItem,
|
||||
DomainDashboardBanList,
|
||||
)
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
|
||||
|
||||
def map_domain_active_ban_to_response(domain_ban: DomainActiveBan) -> ActiveBan:
|
||||
@@ -78,9 +79,7 @@ def map_domain_dashboard_ban_list_to_response(
|
||||
items=[
|
||||
map_domain_dashboard_ban_item_to_response(item) for item in domain_list.items
|
||||
],
|
||||
total=domain_list.total,
|
||||
page=domain_list.page,
|
||||
page_size=domain_list.page_size,
|
||||
pagination=create_pagination_metadata(domain_list.total, domain_list.page, domain_list.page_size),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -16,9 +16,14 @@ Response Patterns:
|
||||
# Returns:
|
||||
{
|
||||
"items": [...],
|
||||
"total": 100,
|
||||
"page": 1,
|
||||
"page_size": 20
|
||||
"pagination": {
|
||||
"page": 1,
|
||||
"page_size": 20,
|
||||
"total": 100,
|
||||
"total_pages": 5,
|
||||
"has_next_page": true,
|
||||
"has_prev_page": false
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
@@ -116,6 +121,40 @@ class BanGuiBaseModel(BaseModel):
|
||||
model_config = ConfigDict(strict=True)
|
||||
|
||||
|
||||
class PaginationMetadata(BanGuiBaseModel):
|
||||
"""Pagination metadata embedded in paginated list responses.
|
||||
|
||||
Contains page information and computed fields to support frontend pagination controls.
|
||||
|
||||
Fields:
|
||||
page: Current page number (1-based).
|
||||
page_size: Number of items per page.
|
||||
total: Total number of items matching the query (across all pages).
|
||||
total_pages: Computed total number of pages.
|
||||
has_next_page: Whether there is a next page after this one.
|
||||
has_prev_page: Whether there is a previous page before this one.
|
||||
|
||||
Example:
|
||||
```python
|
||||
pagination = PaginationMetadata(
|
||||
page=2,
|
||||
page_size=50,
|
||||
total=150,
|
||||
total_pages=3,
|
||||
has_next_page=True,
|
||||
has_prev_page=True
|
||||
)
|
||||
```
|
||||
"""
|
||||
|
||||
page: int = Field(..., ge=1, description="Current page number (1-based).")
|
||||
page_size: int = Field(..., ge=1, description="Number of items per page.")
|
||||
total: int = Field(..., ge=0, description="Total number of items matching the query.")
|
||||
total_pages: int = Field(..., ge=1, description="Computed total number of pages.")
|
||||
has_next_page: bool = Field(..., description="Whether there is a next page after this one.")
|
||||
has_prev_page: bool = Field(..., description="Whether there is a previous page before this one.")
|
||||
|
||||
|
||||
class PaginatedListResponse(BanGuiBaseModel, Generic[T]):
|
||||
"""Standardized paginated list response.
|
||||
|
||||
@@ -124,9 +163,7 @@ class PaginatedListResponse(BanGuiBaseModel, Generic[T]):
|
||||
|
||||
Fields:
|
||||
items: The data items for the current page.
|
||||
total: Total number of items matching the query (across all pages).
|
||||
page: Current page number (1-based).
|
||||
page_size: Number of items per page.
|
||||
pagination: Pagination metadata with computed derived fields.
|
||||
|
||||
Example:
|
||||
```python
|
||||
@@ -136,17 +173,20 @@ class PaginatedListResponse(BanGuiBaseModel, Generic[T]):
|
||||
# Returns:
|
||||
{
|
||||
"items": [...],
|
||||
"total": 150,
|
||||
"page": 2,
|
||||
"page_size": 50
|
||||
"pagination": {
|
||||
"page": 2,
|
||||
"page_size": 50,
|
||||
"total": 150,
|
||||
"total_pages": 3,
|
||||
"has_next_page": true,
|
||||
"has_prev_page": true
|
||||
}
|
||||
}
|
||||
```
|
||||
"""
|
||||
|
||||
items: list[T] = Field(default_factory=list, description="Data items for the current page.")
|
||||
total: int = Field(..., ge=0, description="Total number of items matching the query.")
|
||||
page: int = Field(..., ge=1, description="Current page number (1-based).")
|
||||
page_size: int = Field(..., ge=1, description="Number of items per page.")
|
||||
pagination: PaginationMetadata = Field(..., description="Pagination metadata with computed derived fields.")
|
||||
|
||||
|
||||
class CollectionResponse(BanGuiBaseModel, Generic[T]):
|
||||
|
||||
@@ -35,6 +35,7 @@ from app.repositories import blocklist_repo, import_log_repo, settings_repo
|
||||
from app.services.blocklist_downloader import BlocklistDownloader
|
||||
from app.services.blocklist_import_workflow import BlocklistImportWorkflow
|
||||
from app.services.blocklist_parser import BlocklistParser
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Awaitable, Callable
|
||||
@@ -547,9 +548,7 @@ async def list_import_logs(
|
||||
|
||||
return ImportLogListResponse(
|
||||
items=[ImportLogEntry.model_validate(i) for i in items],
|
||||
total=total,
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
pagination=create_pagination_metadata(total, page, page_size),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ from app.repositories import fail2ban_db_repo
|
||||
from app.repositories import history_archive_repo as default_history_archive_repo
|
||||
from app.utils.constants import DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE
|
||||
from app.utils.fail2ban_db_utils import parse_data_json, ts_to_iso
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
from app.utils.time_utils import since_unix
|
||||
|
||||
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||
@@ -347,9 +348,7 @@ async def list_history(
|
||||
|
||||
return HistoryListResponse(
|
||||
items=items,
|
||||
total=total,
|
||||
page=page,
|
||||
page_size=effective_page_size,
|
||||
pagination=create_pagination_metadata(total, page, effective_page_size),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -48,6 +48,7 @@ from app.utils.fail2ban_response import (
|
||||
to_dict,
|
||||
)
|
||||
from app.utils.jail_socket import reload_all
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
from app.utils.runtime_state import JailServiceState # noqa: TC001
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -809,9 +810,7 @@ async def get_jail_banned_ips(
|
||||
)
|
||||
return JailBannedIpsResponse(
|
||||
items=page_bans,
|
||||
total=total,
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
pagination=create_pagination_metadata(total, page, page_size),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -6,11 +6,11 @@ ensure a uniform API contract.
|
||||
|
||||
Standard Pagination Contract:
|
||||
Query parameters: page (1-based), page_size (1-500)
|
||||
Response: PaginatedListResponse[T] with items, total, page, page_size
|
||||
Response: PaginatedListResponse[T] with items and pagination metadata
|
||||
|
||||
Usage in routers:
|
||||
```python
|
||||
from app.utils.pagination import PAGINATION_DEFAULTS
|
||||
from app.utils.pagination import PAGINATION_DEFAULTS, create_pagination_metadata
|
||||
|
||||
@router.get("/items")
|
||||
async def get_items(
|
||||
@@ -21,13 +21,19 @@ Usage in routers:
|
||||
le=PAGINATION_DEFAULTS["max_page_size"],
|
||||
),
|
||||
):
|
||||
...
|
||||
items = [...]
|
||||
total = 100
|
||||
pagination = create_pagination_metadata(total, page, page_size)
|
||||
return MyListResponse(items=items, pagination=pagination)
|
||||
```
|
||||
"""
|
||||
|
||||
from typing import Final
|
||||
from typing import TYPE_CHECKING, Final
|
||||
|
||||
__all__ = ["PAGINATION_DEFAULTS", "get_offset", "compute_total_pages"]
|
||||
if TYPE_CHECKING:
|
||||
from app.models.response import PaginationMetadata
|
||||
|
||||
__all__ = ["PAGINATION_DEFAULTS", "get_offset", "compute_total_pages", "create_pagination_metadata"]
|
||||
|
||||
# Standardized pagination defaults
|
||||
PAGINATION_DEFAULTS: Final[dict[str, int]] = {
|
||||
@@ -100,3 +106,45 @@ def compute_total_pages(total: int, page_size: int) -> int:
|
||||
|
||||
# Ceiling division: (total + page_size - 1) // page_size
|
||||
return (total + page_size - 1) // page_size
|
||||
|
||||
|
||||
def create_pagination_metadata(total: int, page: int, page_size: int) -> "PaginationMetadata":
|
||||
"""Create pagination metadata with computed derived fields.
|
||||
|
||||
This factory function computes all pagination-related information in a single
|
||||
place, ensuring consistency across all paginated endpoints.
|
||||
|
||||
Args:
|
||||
total: Total number of items across all pages.
|
||||
page: Current page number (1-based).
|
||||
page_size: Items per page.
|
||||
|
||||
Returns:
|
||||
:class:`~app.models.response.PaginationMetadata` with computed fields:
|
||||
- total_pages: Computed total number of pages needed
|
||||
- has_next_page: Whether there is a next page
|
||||
- has_prev_page: Whether there is a previous page
|
||||
|
||||
Example:
|
||||
```python
|
||||
metadata = create_pagination_metadata(total=150, page=2, page_size=50)
|
||||
assert metadata.total_pages == 3
|
||||
assert metadata.has_next_page is True
|
||||
assert metadata.has_prev_page is True
|
||||
```
|
||||
"""
|
||||
from app.models.response import PaginationMetadata
|
||||
|
||||
total_pages = compute_total_pages(total, page_size)
|
||||
has_next_page = page < total_pages
|
||||
has_prev_page = page > 1
|
||||
|
||||
return PaginationMetadata(
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
total=total,
|
||||
total_pages=total_pages,
|
||||
has_next_page=has_next_page,
|
||||
has_prev_page=has_prev_page,
|
||||
)
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ from app.models.history import (
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_SETUP_PAYLOAD = {
|
||||
"master_password": "testpassword1",
|
||||
"master_password": "Mysecretpass1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||
"timezone": "UTC",
|
||||
@@ -50,8 +50,11 @@ def _make_history_item(ip: str = "1.2.3.4", jail: str = "sshd") -> HistoryBanIte
|
||||
|
||||
def _make_history_list(n: int = 2) -> HistoryListResponse:
|
||||
"""Build a mock ``HistoryListResponse`` with *n* items."""
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
|
||||
items = [_make_history_item(ip=f"1.2.3.{i}") for i in range(n)]
|
||||
return HistoryListResponse(items=items, total=n, page=1, page_size=100)
|
||||
pagination = create_pagination_metadata(total=n, page=1, page_size=100)
|
||||
return HistoryListResponse(items=items, pagination=pagination)
|
||||
|
||||
|
||||
def _make_ip_detail(ip: str = "1.2.3.4") -> IpDetailResponse:
|
||||
@@ -96,7 +99,7 @@ async def history_client(tmp_path: Path) -> AsyncClient: # type: ignore[misc]
|
||||
settings = Settings(
|
||||
database_path=str(tmp_path / "history_test.db"),
|
||||
fail2ban_socket="/tmp/fake_fail2ban.sock",
|
||||
session_secret="test-history-secret",
|
||||
session_secret="test-history-secret-32chars-long!!",
|
||||
session_duration_minutes=60,
|
||||
timezone="UTC",
|
||||
log_level="debug",
|
||||
@@ -163,10 +166,15 @@ class TestHistoryList:
|
||||
|
||||
body = response.json()
|
||||
assert "items" in body
|
||||
assert "total" in body
|
||||
assert "page" in body
|
||||
assert "page_size" in body
|
||||
assert body["total"] == 1
|
||||
assert "pagination" in body
|
||||
pagination = body["pagination"]
|
||||
assert "total" in pagination
|
||||
assert "page" in pagination
|
||||
assert "page_size" in pagination
|
||||
assert "total_pages" in pagination
|
||||
assert "has_next_page" in pagination
|
||||
assert "has_prev_page" in pagination
|
||||
assert pagination["total"] == 1
|
||||
|
||||
item = body["items"][0]
|
||||
assert "ip" in item
|
||||
@@ -253,17 +261,22 @@ class TestHistoryList:
|
||||
|
||||
async def test_empty_result(self, history_client: AsyncClient) -> None:
|
||||
"""An empty history returns items=[] and total=0."""
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
|
||||
with patch(
|
||||
"app.routers.history.history_service.list_history",
|
||||
new=AsyncMock(
|
||||
return_value=HistoryListResponse(items=[], total=0, page=1, page_size=100)
|
||||
return_value=HistoryListResponse(
|
||||
items=[],
|
||||
pagination=create_pagination_metadata(total=0, page=1, page_size=100),
|
||||
)
|
||||
),
|
||||
):
|
||||
response = await history_client.get("/api/history")
|
||||
|
||||
body = response.json()
|
||||
assert body["items"] == []
|
||||
assert body["total"] == 0
|
||||
assert body["pagination"]["total"] == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user