Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down Expand Up @@ -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.

Expand Down
7 changes: 5 additions & 2 deletions app/api/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}/")
Expand Down
7 changes: 5 additions & 2 deletions app/api/decks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}/")
Expand Down
18 changes: 14 additions & 4 deletions app/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
Expand Down Expand Up @@ -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])
Expand Down
30 changes: 27 additions & 3 deletions app/schemas.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Expand All @@ -43,6 +63,10 @@ class Cards(Collection[Card]):
pass


class CardsPage(Page[Card]):
pass


class DeckBase(Base):
name: str
description: str | None = None
Expand All @@ -68,5 +92,5 @@ class DeckWithCards(Deck):
cards: list[Card]


class Decks(Collection[Deck]):
class DecksPage(Page[Deck]):
pass
4 changes: 2 additions & 2 deletions app/use_cases/list_cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions app/use_cases/list_decks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 28 additions & 1 deletion tests/test_cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
46 changes: 46 additions & 0 deletions tests/test_decks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading