From 6726b158b479b668d02b35d6565f0ed8a86ef1d4 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 13:42:24 +0300 Subject: [PATCH] Page deck and card lists by id with limit and after --- AGENTS.md | 7 +++--- app/api/cards.py | 13 ++++++++--- app/api/decks.py | 12 +++++++--- 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, 142 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4a36608..21d9f40 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,9 +44,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 ``` @@ -69,7 +69,8 @@ class ListDecksUseCase: ## Conventions - Routes live in `app/api/`, one module per resource (`decks.py`, `cards.py`), each exposing its own `ROUTER` (`litestar.Router`, prefix `/api`). `application.build_app` registers them all via `route_handlers=[decks.ROUTER, cards.ROUTER]`. Add a new resource by creating `app/api/.py`, defining handlers + a `ROUTER`, and adding it to that list. Its database work goes in `app/use_cases/` with a provider in `ioc.UseCases`. -- Pydantic schemas in `app/schemas.py` use `from_attributes=True` (via `Base`) so they validate directly from ORM instances (`schemas.X.model_validate(orm_instance)`). Collection responses go through `Collection[T].from_models(...)` (e.g. `schemas.Decks`, `schemas.Cards`). +- Pydantic schemas in `app/schemas.py` use `from_attributes=True` (via `Base`) so they validate directly from ORM instances (`schemas.X.model_validate(orm_instance)`). Collection responses go through `Collection[T].from_models(...)` (e.g. `schemas.Cards`). +- 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 use `noload` (no cards query), detail uses `joinedload` via `fetch_with_cards` — so the type states exactly what each endpoint loads. - Domain exceptions: register handlers in `application.build_app`'s `exception_handlers` dict (`DuplicateKeyError` → `exceptions.duplicate_key_error_handler` returns 400, `NotFoundError` → `exceptions.not_found_error_handler` returns 404). Handlers don't raise HTTP errors themselves: a missing row surfaces as advanced-alchemy's `NotFoundError`. - `ruff` is configured with `select = ["ALL"]` and a line length of 120 — expect strict lint. `app/resources/` is exempt from the `TC` rules because modern-di resolves DI creator annotations at runtime. Type-check with `ty`; suppressions already exist for `invalid-argument-type` around `LifespanManager` / `ASGITransport` in `tests/conftest.py`. diff --git a/app/api/cards.py b/app/api/cards.py index 6633190..055dcfa 100644 --- a/app/api/cards.py +++ b/app/api/cards.py @@ -1,8 +1,9 @@ import typing +from typing import Annotated import litestar from litestar.di import NamedDependency -from litestar.params import FromPath +from litestar.params import FromPath, QueryParameter from app import schemas from app.use_cases.create_cards import CreateCardsUseCase @@ -12,8 +13,14 @@ @litestar.get("/decks/{deck_id:int}/cards/") -async def list_cards(deck_id: FromPath[int], list_cards_use_case: NamedDependency[ListCardsUseCase]) -> schemas.Cards: - return schemas.Cards.from_models(await list_cards_use_case(deck_id=deck_id)) +async def list_cards( + deck_id: FromPath[int], + list_cards_use_case: NamedDependency[ListCardsUseCase], + after: Annotated[int | None, QueryParameter(gt=0)] = None, + limit: Annotated[int, QueryParameter(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT, +) -> 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) @litestar.get("/cards/{card_id:int}/") diff --git a/app/api/decks.py b/app/api/decks.py index e57f1cc..9511f80 100644 --- a/app/api/decks.py +++ b/app/api/decks.py @@ -1,8 +1,9 @@ import typing +from typing import Annotated import litestar from litestar.di import NamedDependency -from litestar.params import FromPath +from litestar.params import FromPath, QueryParameter from app import schemas from app.use_cases.create_deck import CreateDeckUseCase @@ -12,8 +13,13 @@ @litestar.get("/decks/") -async def list_decks(list_decks_use_case: NamedDependency[ListDecksUseCase]) -> schemas.Decks: - return schemas.Decks.from_models(await list_decks_use_case()) +async def list_decks( + list_decks_use_case: NamedDependency[ListDecksUseCase], + after: Annotated[int | None, QueryParameter(gt=0)] = None, + limit: Annotated[int, QueryParameter(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT, +) -> schemas.DecksPage: + decks: typing.Final = await list_decks_use_case(after=after, limit=limit) + return schemas.DecksPage.from_models(decks, limit=limit) @litestar.get("/decks/{deck_id:int}/") 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 e162e08..778316c 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_codes.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_update_cards_to_a_duplicate_front(client: AsyncClient) -> None: response = await client.put(f"/api/decks/{deck.id}/cards/", json=[{"id": card2.id, "front": card1.front}]) assert response.status_code == status_codes.HTTP_400_BAD_REQUEST + + +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_codes.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_codes.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_codes.HTTP_400_BAD_REQUEST diff --git a/tests/test_decks.py b/tests/test_decks.py index 02a7a44..a5342c3 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_codes.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_codes.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_codes.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_codes.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_codes.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_codes.HTTP_400_BAD_REQUEST