From 23cbdcdc0f4df861d77b3f057c004c431999de5d Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 13:42:19 +0300 Subject: [PATCH] Page deck and card lists by id with limit and after --- AGENTS.md | 5 ++-- app/api/cards.py | 7 ++++-- app/api/decks.py | 7 ++++-- app/repositories.py | 18 +++++++++++---- app/schemas.py | 30 +++++++++++++++++++++--- app/use_cases/list_cards.py | 4 ++-- app/use_cases/list_decks.py | 4 ++-- tests/test_cards.py | 29 ++++++++++++++++++++++- tests/test_decks.py | 46 +++++++++++++++++++++++++++++++++++++ 9 files changed, 132 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9e5f2be..f84b850 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,9 +65,9 @@ class ListDecksUseCase: decks_repository: DecksRepository @postgres_retry - async def __call__(self) -> Sequence[models.Deck]: + async def __call__(self, *, after: int | None, limit: int) -> Sequence[models.Deck]: async with self.transaction: - decks: typing.Final = await self.decks_repository.get_many() + decks: typing.Final = await self.decks_repository.list_decks(after=after, limit=limit) await self.transaction.commit() return decks ``` @@ -106,6 +106,7 @@ class ListDecksUseCase: - Ruff is configured with `select = ["ALL"]` and a curated ignore list in `pyproject.toml`. Don't sprinkle `# noqa`; prefer fixing or extending the project ignore list if a rule is genuinely wrong for the codebase. - Routes convert ORM objects to schemas explicitly, never via `typing.cast`. Single objects: `schemas.X.model_validate(instance)`. Collections: `schemas.Xs.from_models(objects)` (the `Collection[T]` seam in `app/schemas.py`). Both rely on `from_attributes=True`. +- List endpoints page by id (keyset). They take `limit` (1 to `schemas.MAX_PAGE_LIMIT`, default `schemas.DEFAULT_PAGE_LIMIT`) and `after`, the last id the client has seen, and return a `Page[T]` subclass (`schemas.DecksPage`, `schemas.CardsPage`) with `items` and `next_after`. The repository filters `id > after` and fetches `limit + 1` rows ordered by id. `Page.from_models(rows, limit=limit)` drops the extra row and sets `next_after` to the last returned id when that row existed, or `null` on the last page. Don't page with OFFSET: Postgres reads and discards every skipped row. - Deck responses are deliberately two-shaped: `list_decks`/`create_deck`/`update_deck` return the light `schemas.Deck` (no `cards`), while `get_deck` returns `schemas.DeckWithCards`. The split mirrors loading — lists/writes use `noload` (no cards query), detail uses `joinedload` via `fetch_with_cards` — so the type states exactly what each endpoint loads. - Line length is 120. diff --git a/app/api/cards.py b/app/api/cards.py index 6063c5f..255893b 100644 --- a/app/api/cards.py +++ b/app/api/cards.py @@ -16,9 +16,12 @@ @ROUTER.get("/decks/{deck_id}/cards/") async def list_cards( deck_id: int, + after: typing.Annotated[int | None, fastapi.Query(gt=0)] = None, + limit: typing.Annotated[int, fastapi.Query(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT, list_cards_use_case: ListCardsUseCase = FromDI(ListCardsUseCase), -) -> schemas.Cards: - return schemas.Cards.from_models(await list_cards_use_case(deck_id=deck_id)) +) -> schemas.CardsPage: + cards: typing.Final = await list_cards_use_case(deck_id=deck_id, after=after, limit=limit) + return schemas.CardsPage.from_models(cards, limit=limit) @ROUTER.get("/cards/{card_id}/") diff --git a/app/api/decks.py b/app/api/decks.py index 83d8a4e..3296b1e 100644 --- a/app/api/decks.py +++ b/app/api/decks.py @@ -15,9 +15,12 @@ @ROUTER.get("/decks/") async def list_decks( + after: typing.Annotated[int | None, fastapi.Query(gt=0)] = None, + limit: typing.Annotated[int, fastapi.Query(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT, list_decks_use_case: ListDecksUseCase = FromDI(ListDecksUseCase), -) -> schemas.Decks: - return schemas.Decks.from_models(await list_decks_use_case()) +) -> schemas.DecksPage: + decks: typing.Final = await list_decks_use_case(after=after, limit=limit) + return schemas.DecksPage.from_models(decks, limit=limit) @ROUTER.get("/decks/{deck_id}/") diff --git a/app/repositories.py b/app/repositories.py index 7f0cf21..84e774b 100644 --- a/app/repositories.py +++ b/app/repositories.py @@ -4,6 +4,7 @@ import sqlalchemy as sa from advanced_alchemy.exceptions import NotFoundError, wrap_sqlalchemy_exception +from advanced_alchemy.filters import LimitOffset from advanced_alchemy.repository import DEFAULT_ERROR_MESSAGE_TEMPLATES, SQLAlchemyAsyncRepository from advanced_alchemy.service import SQLAlchemyAsyncRepositoryService from sqlalchemy import orm @@ -22,8 +23,12 @@ class BaseRepository(SQLAlchemyAsyncRepository[models.Deck]): repository_type = BaseRepository - async def list_decks(self) -> Sequence[models.Deck]: - return await self.get_many(order_by=models.Deck.id.asc()) + async def list_decks(self, *, after: int | None, limit: int) -> Sequence[models.Deck]: + return await self.get_many( + *([] if after is None else [models.Deck.id > after]), + LimitOffset(limit=limit + 1, offset=0), + order_by=models.Deck.id.asc(), + ) async def get_or_create(self, data: schemas.DeckCreate) -> models.Deck: statement = postgresql.insert(models.Deck).values(**data.model_dump()) @@ -56,8 +61,13 @@ class BaseRepository(SQLAlchemyAsyncRepository[models.Card]): repository_type = BaseRepository - async def list_for_deck(self, deck_id: int) -> Sequence[models.Card]: - return await self.get_many(models.Card.deck_id == deck_id, order_by=models.Card.id.asc()) + async def list_for_deck(self, deck_id: int, *, after: int | None, limit: int) -> Sequence[models.Card]: + return await self.get_many( + models.Card.deck_id == deck_id, + *([] if after is None else [models.Card.id > after]), + LimitOffset(limit=limit + 1, offset=0), + order_by=models.Card.id.asc(), + ) async def add_cards(self, deck_id: int, cards: list[schemas.CardCreate]) -> Sequence[models.Card]: return await self.create_many([models.Card(**card.model_dump(), deck_id=deck_id) for card in cards]) diff --git a/app/schemas.py b/app/schemas.py index f59a8a4..c15efc3 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -1,12 +1,20 @@ import uuid -from typing import TYPE_CHECKING, Self +from typing import TYPE_CHECKING, Final, Protocol, Self import pydantic from pydantic import BaseModel, PositiveInt if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterable, Sequence + + +DEFAULT_PAGE_LIMIT: Final = 50 +MAX_PAGE_LIMIT: Final = 100 + + +class HasId(Protocol): + id: int class Base(BaseModel): @@ -21,6 +29,18 @@ def from_models(cls, objects: Iterable[object]) -> Self: return cls.model_validate({"items": list(objects)}) +class Page[T: Base](Base): + items: list[T] + next_after: PositiveInt | None + + @classmethod + def from_models(cls, objects: Sequence[HasId], *, limit: int) -> Self: + has_next: Final = len(objects) > limit + return cls.model_validate( + {"items": list(objects[:limit]), "next_after": objects[limit - 1].id if has_next else None} + ) + + class CardBase(Base): front: str back: str | None = None @@ -43,6 +63,10 @@ class Cards(Collection[Card]): pass +class CardsPage(Page[Card]): + pass + + class DeckBase(Base): name: str description: str | None = None @@ -68,5 +92,5 @@ class DeckWithCards(Deck): cards: list[Card] -class Decks(Collection[Deck]): +class DecksPage(Page[Deck]): pass diff --git a/app/use_cases/list_cards.py b/app/use_cases/list_cards.py index 5cda202..70fe4a3 100644 --- a/app/use_cases/list_cards.py +++ b/app/use_cases/list_cards.py @@ -14,8 +14,8 @@ class ListCardsUseCase: cards_repository: CardsRepository @postgres_retry - async def __call__(self, *, deck_id: int) -> Sequence[models.Card]: + async def __call__(self, *, deck_id: int, after: int | None, limit: int) -> Sequence[models.Card]: async with self.transaction: - cards: typing.Final = await self.cards_repository.list_for_deck(deck_id) + cards: typing.Final = await self.cards_repository.list_for_deck(deck_id, after=after, limit=limit) await self.transaction.commit() return cards diff --git a/app/use_cases/list_decks.py b/app/use_cases/list_decks.py index 6628382..05bc3f1 100644 --- a/app/use_cases/list_decks.py +++ b/app/use_cases/list_decks.py @@ -14,8 +14,8 @@ class ListDecksUseCase: decks_repository: DecksRepository @postgres_retry - async def __call__(self) -> Sequence[models.Deck]: + async def __call__(self, *, after: int | None, limit: int) -> Sequence[models.Deck]: async with self.transaction: - decks: typing.Final = await self.decks_repository.list_decks() + decks: typing.Final = await self.decks_repository.list_decks(after=after, limit=limit) await self.transaction.commit() return decks diff --git a/tests/test_cards.py b/tests/test_cards.py index 321ae3e..ef9416d 100644 --- a/tests/test_cards.py +++ b/tests/test_cards.py @@ -70,7 +70,7 @@ async def test_create_cards(client: AsyncClient) -> None: response = await client.get(f"/api/decks/{deck.id}/cards/") assert response.status_code == status.HTTP_200_OK data = response.json() - assert created_data == data + assert created_data["items"] == data["items"] assert len(data["items"]) == len(cards_to_create) for k, v in cards_to_create[0].model_dump().items(): assert data["items"][0][k] == v @@ -188,3 +188,30 @@ async def test_openapi_card_deck_id_required(client: AsyncClient) -> None: card_schema = response.json()["components"]["schemas"]["Card"] assert "deck_id" in card_schema["required"] assert "null" not in {x.get("type") for x in card_schema["properties"]["deck_id"].get("anyOf", [])} + + +async def test_list_cards_pages_by_id_within_deck(client: AsyncClient) -> None: + deck, other_deck = await factories.DeckModelFactory.create_batch_async(size=2) + first, second = await factories.CardModelFactory.create_batch_async(size=2, deck_id=deck.id) + await factories.CardModelFactory.create_async(deck_id=other_deck.id) + third = await factories.CardModelFactory.create_async(deck_id=deck.id) + + response = await client.get(f"/api/decks/{deck.id}/cards/", params={"limit": 2}) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [first.id, second.id] + assert data["next_after"] == second.id + + response = await client.get(f"/api/decks/{deck.id}/cards/", params={"limit": 2, "after": data["next_after"]}) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [third.id] + assert data["next_after"] is None + + +@pytest.mark.parametrize("params", [{"limit": 0}, {"limit": 101}, {"after": 0}]) +async def test_list_cards_rejects_bad_page_params(client: AsyncClient, params: dict[str, int | str]) -> None: + deck = await factories.DeckModelFactory.create_async() + + response = await client.get(f"/api/decks/{deck.id}/cards/", params=params) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT diff --git a/tests/test_decks.py b/tests/test_decks.py index 8e171ce..44da28c 100644 --- a/tests/test_decks.py +++ b/tests/test_decks.py @@ -204,3 +204,49 @@ async def test_put_decks_moves_updated_at(client: AsyncClient, db_session: Async assert response.status_code == status.HTTP_200_OK updated_at = (await db_session.scalars(sa.select(models.Deck.updated_at).where(models.Deck.id == deck.id))).one() assert updated_at > long_ago + + +async def test_list_decks_first_page(client: AsyncClient) -> None: + decks = await factories.DeckModelFactory.create_batch_async(size=3) + + response = await client.get("/api/decks/", params={"limit": 2}) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [decks[0].id, decks[1].id] + assert data["next_after"] == decks[1].id + + +async def test_list_decks_after_cursor(client: AsyncClient) -> None: + decks = await factories.DeckModelFactory.create_batch_async(size=3) + + response = await client.get("/api/decks/", params={"limit": 2, "after": decks[1].id}) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [decks[2].id] + assert data["next_after"] is None + + +async def test_list_decks_exactly_limit_has_no_next_page(client: AsyncClient) -> None: + decks = await factories.DeckModelFactory.create_batch_async(size=2) + + response = await client.get("/api/decks/", params={"limit": 2}) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [x.id for x in decks] + assert data["next_after"] is None + + +async def test_list_decks_default_limit(client: AsyncClient) -> None: + decks = await factories.DeckModelFactory.create_batch_async(size=51) + + response = await client.get("/api/decks/") + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert [x["id"] for x in data["items"]] == [x.id for x in decks[:50]] + assert data["next_after"] == decks[49].id + + +@pytest.mark.parametrize("params", [{"limit": 0}, {"limit": 101}, {"after": 0}, {"after": "x"}]) +async def test_list_decks_rejects_bad_page_params(client: AsyncClient, params: dict[str, int | str]) -> None: + response = await client.get("/api/decks/", params=params) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT