refactoring-backend #3
@@ -1,30 +1,3 @@
|
||||
### Issue #52: MEDIUM - Error Handling Patterns Not Declared on Services
|
||||
|
||||
**Where found**:
|
||||
- `backend/app/services/error_handling.py:1-65` – three patterns (ABORT_ON_ERROR, RETURN_DEFAULT, PARTIAL_RESULT) defined but not annotated on callers
|
||||
|
||||
**Why this is needed**:
|
||||
Callers must read docstrings to know whether a service raises, returns a default, or returns partial results. A service silently changing its pattern is a breaking change that type checking and tests may not catch.
|
||||
|
||||
**Goal**:
|
||||
Make each service's error contract explicit and machine-checkable.
|
||||
|
||||
**What to do**:
|
||||
1. Create typed wrapper classes or protocol types for each error pattern.
|
||||
2. Annotate service return types to reflect the chosen pattern (e.g., `Result[T, E]` or a tagged union).
|
||||
3. Alternatively, use a decorator that records and enforces the declared pattern.
|
||||
|
||||
**Possible traps and issues**:
|
||||
- Adding a full `Result` type may require widespread refactoring; start with documentation-only annotations and migrate incrementally.
|
||||
|
||||
**Docs changes needed**:
|
||||
- `backend/app/services/error_handling.py`: update module docstring with pattern descriptions and usage examples.
|
||||
|
||||
**Doc references**:
|
||||
- `backend/app/services/error_handling.py`
|
||||
|
||||
---
|
||||
|
||||
### Issue #53: MEDIUM - Pagination Contract Inconsistent (Offset vs Cursor)
|
||||
|
||||
**Where found**:
|
||||
|
||||
@@ -140,6 +140,8 @@ class PaginationMetadata(BanGuiBaseModel):
|
||||
Always False for cursor pagination (cannot navigate backward without storing history).
|
||||
cursor: Opaque cursor token for fetching the next page (cursor pagination only).
|
||||
None for offset pagination or when there are no more pages.
|
||||
pagination_mode: Pagination mode used by the endpoint. 'offset' uses page/page_size;
|
||||
'cursor' uses cursor tokens for navigation.
|
||||
|
||||
Example (offset pagination):
|
||||
```python
|
||||
@@ -150,7 +152,8 @@ class PaginationMetadata(BanGuiBaseModel):
|
||||
total_pages=3,
|
||||
has_next_page=True,
|
||||
has_prev_page=True,
|
||||
cursor=None
|
||||
cursor=None,
|
||||
pagination_mode="offset",
|
||||
)
|
||||
```
|
||||
|
||||
@@ -163,7 +166,8 @@ class PaginationMetadata(BanGuiBaseModel):
|
||||
total_pages=-1,
|
||||
has_next_page=True,
|
||||
has_prev_page=False,
|
||||
cursor="eyJpZCI6IDQyN30="
|
||||
cursor="eyJpZCI6IDQyN30=",
|
||||
pagination_mode="cursor",
|
||||
)
|
||||
```
|
||||
"""
|
||||
@@ -178,6 +182,10 @@ class PaginationMetadata(BanGuiBaseModel):
|
||||
default=None,
|
||||
description="Opaque cursor token for fetching the next page (cursor pagination only).",
|
||||
)
|
||||
pagination_mode: Literal["offset", "cursor"] = Field(
|
||||
...,
|
||||
description="Pagination mode used by the endpoint. 'offset' uses page/page_size; 'cursor' uses cursor tokens.",
|
||||
)
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -192,6 +192,7 @@ def create_pagination_metadata(total: int, page: int, page_size: int) -> "Pagina
|
||||
total_pages=total_pages,
|
||||
has_next_page=has_next_page,
|
||||
has_prev_page=has_prev_page,
|
||||
pagination_mode="offset",
|
||||
)
|
||||
|
||||
|
||||
@@ -302,4 +303,5 @@ def create_keyset_pagination_metadata(
|
||||
has_next_page=has_next_page,
|
||||
has_prev_page=False,
|
||||
cursor=next_cursor,
|
||||
pagination_mode="cursor",
|
||||
)
|
||||
|
||||
@@ -102,3 +102,38 @@ class TestComputeTotalPages:
|
||||
|
||||
with pytest.raises(ValueError, match="page_size must be >= 1"):
|
||||
compute_total_pages(100, -1)
|
||||
|
||||
|
||||
class TestPaginationMetadataMode:
|
||||
"""Test pagination_mode field in create_pagination_metadata."""
|
||||
|
||||
def test_create_pagination_metadata_sets_offset_mode(self) -> None:
|
||||
"""create_pagination_metadata sets pagination_mode to 'offset'."""
|
||||
from app.utils.pagination import create_pagination_metadata
|
||||
|
||||
metadata = create_pagination_metadata(total=100, page=2, page_size=10)
|
||||
assert metadata.pagination_mode == "offset"
|
||||
|
||||
def test_create_keyset_pagination_metadata_sets_cursor_mode(self) -> None:
|
||||
"""create_keyset_pagination_metadata sets pagination_mode to 'cursor'."""
|
||||
from app.utils.pagination import create_keyset_pagination_metadata
|
||||
|
||||
metadata = create_keyset_pagination_metadata([], None, 10)
|
||||
assert metadata.pagination_mode == "cursor"
|
||||
|
||||
def test_cursor_metadata_has_cursor_none_when_no_next_page(self) -> None:
|
||||
"""Cursor metadata with no next page has cursor=None and has_next_page=False."""
|
||||
from app.utils.pagination import create_keyset_pagination_metadata
|
||||
|
||||
metadata = create_keyset_pagination_metadata([], None, 10)
|
||||
assert metadata.has_next_page is False
|
||||
assert metadata.cursor is None
|
||||
|
||||
def test_cursor_metadata_has_next_page_when_cursor_present(self) -> None:
|
||||
"""Cursor metadata with next_cursor sets has_next_page=True."""
|
||||
from app.utils.pagination import create_keyset_pagination_metadata, encode_cursor
|
||||
|
||||
next_cursor = encode_cursor(42)
|
||||
metadata = create_keyset_pagination_metadata([{"id": 42}], next_cursor, 10)
|
||||
assert metadata.has_next_page is True
|
||||
assert metadata.cursor == next_cursor
|
||||
|
||||
Reference in New Issue
Block a user