Skip to content

Commit aef9562

Browse files
authored
fix: bound card writes and ids, map missing decks to 404, cache the card update (#75)
Cap card lists at MAX_CARDS_PER_REQUEST and ids at bigint so oversized input is a 400 instead of a 500. Map ForeignKeyError to 404. Make LOG_LEVEL case-insensitive and pass it to app logging. Order deck detail cards by id. Send PUT cards through unnest so the statement is cached and prepared once. Add a (deck_id, id) index for card pagination.
1 parent 93129d1 commit aef9562

15 files changed

Lines changed: 187 additions & 36 deletions

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class ListDecksUseCase:
5858
- `CreateCardsUseCase` relies on the `(deck_id, front)` unique constraint. A replay is rejected as a duplicate-key error rather than creating the cards twice.
5959
- Updates (`UpdateDeckUseCase`, `UpdateCardsUseCase`) write the same values again, so a replay is harmless.
6060

61-
**Persistence**: Models inherit `advanced_alchemy.base.BigIntAuditBase` (gives `id: BigInt`, `created_at`, `updated_at`). Metadata is shared with `orm.DeclarativeBase.metadata` in `app/models.py` so Alembic autogen sees everything. Repositories are `SQLAlchemyAsyncRepositoryService[Model]` with a nested `BaseRepository(SQLAlchemyAsyncRepository[Model])`. Use cases call the service methods (`get_many`, `get_one`, `create_many`) for reads and plain inserts. Writes that advanced-alchemy would spread over several round trips are one statement on the session instead: its `update` reads the row twice and refreshes it after the flush, and `upsert_many` selects the rows before and after writing them. `update_deck` sends `UPDATE ... RETURNING` and `get_or_create` sends `INSERT ... ON CONFLICT ... RETURNING`. `update_cards` sends one `UPDATE cards ... FROM (VALUES ...) WHERE cards.id = updates.id AND cards.deck_id = :deck_id RETURNING *` and raises `NotFoundError` (404) if fewer rows come back than ids were sent, which rolls the whole request back. PUT never inserts. An `INSERT ... ON CONFLICT (id)` upsert would move a card in from another deck and would insert client-chosen ids without advancing `cards_id_seq`, so a later POST would collide on the primary key. The `update_cards` handler rejects duplicate ids with a 400 before any query runs. A statement that can hit a unique constraint runs inside `wrap_sqlalchemy_exception` so that it still raises `DuplicateKeyError`. `create_session` in `app/resources/db.py` builds a plain `AsyncSession` with `join_transaction_mode="create_savepoint"`. This is inert in production (the session binds to an engine) but enables the per-test rollback fixture: when a test binds the session to a connection already in a transaction, the session owns its own savepoint so the outer transaction survives commits. Do not "fix" it.
61+
**Persistence**: Models inherit `advanced_alchemy.base.BigIntAuditBase` (gives `id: BigInt`, `created_at`, `updated_at`). Metadata is shared with `orm.DeclarativeBase.metadata` in `app/models.py` so Alembic autogen sees everything. Repositories are `SQLAlchemyAsyncRepositoryService[Model]` with a nested `BaseRepository(SQLAlchemyAsyncRepository[Model])`. Use cases call the service methods (`get_many`, `get_one`, `create_many`) for reads and plain inserts. Writes that advanced-alchemy would spread over several round trips are one statement on the session instead: its `update` reads the row twice and refreshes it after the flush, and `upsert_many` selects the rows before and after writing them. `update_deck` sends `UPDATE ... RETURNING` and `get_or_create` sends `INSERT ... ON CONFLICT ... RETURNING`. `update_cards` sends one `UPDATE cards ... FROM unnest(:ids, :fronts, :backs, :hints) AS updates(...) WHERE cards.id = updates.id AND cards.deck_id = :deck_id RETURNING *`. It binds one array per column, so the SQL text stays the same for any number of cards: SQLAlchemy caches it and asyncpg prepares it once per connection, which a multi-row `VALUES` would not allow. It raises `NotFoundError` (404) if fewer rows come back than ids were sent, which rolls the whole request back. PUT never inserts. An `INSERT ... ON CONFLICT (id)` upsert would move a card in from another deck and would insert client-chosen ids without advancing `cards_id_seq`, so a later POST would collide on the primary key. The card handlers cap a request at `schemas.MAX_CARDS_PER_REQUEST` cards, and `update_cards` rejects duplicate ids, both with a 400 before any query runs. Ids in paths, `after` and card bodies are bounded to Postgres `bigint` (`schemas.MAX_ID`), so an oversized id is a 400 and never reaches asyncpg. A POST to a deck that does not exist fails on the foreign key, and `ForeignKeyError` maps to 404. A statement that can hit a unique constraint runs inside `wrap_sqlalchemy_exception` so that it still raises `DuplicateKeyError`. `create_session` in `app/resources/db.py` builds a plain `AsyncSession` with `join_transaction_mode="create_savepoint"`. This is inert in production (the session binds to an engine) but enables the per-test rollback fixture: when a test binds the session to a connection already in a transaction, the session owns its own savepoint so the outer transaction survives commits. Do not "fix" it.
6262

6363
**Test isolation** (`tests/conftest.py`): `db_session` fixture opens a connection, starts a transaction, then **overrides** `Database.dynamic_engine` in the DI container to return that connection. All requests in the test reuse this connection. Each session built against it uses `join_transaction_mode="create_savepoint"`, so `transaction.commit()` releases the session's own savepoint while the outer transaction is rolled back in teardown. DB state is clean between tests with no truncation needed. `app` and `client` fixtures build the real app and run it through `httpx.ASGITransport` + `asgi_lifespan.LifespanManager`. Polyfactory `SQLAlchemyFactory` is wired up via `set_async_session_in_base_sqlalchemy_factory`.
6464

‎app/__main__.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import granian
22
from granian.constants import Interfaces, Loops
3-
from granian.log import LogLevels
43

54
from app.settings import settings
65

@@ -12,6 +11,6 @@
1211
address=settings.app_host,
1312
port=settings.app_port,
1413
interface=Interfaces.ASGI,
15-
log_level=LogLevels(settings.log_level),
14+
log_level=settings.log_level,
1615
loop=Loops.uvloop,
1716
).serve()

‎app/api/__init__.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
from typing import Annotated
2+
3+
from litestar.params import PathParameter
4+
5+
from app import schemas
6+
7+
8+
PathId = Annotated[int, PathParameter(gt=0, le=schemas.MAX_ID)]

‎app/api/cards.py‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,54 @@
44
import litestar
55
from litestar.di import NamedDependency
66
from litestar.exceptions import ValidationException
7-
from litestar.params import FromPath, QueryParameter
7+
from litestar.params import QueryParameter
88

99
from app import schemas
10+
from app.api import PathId
1011
from app.use_cases.create_cards import CreateCardsUseCase
1112
from app.use_cases.get_card import GetCardUseCase
1213
from app.use_cases.list_cards import ListCardsUseCase
1314
from app.use_cases.update_cards import UpdateCardsUseCase
1415

1516

17+
TOO_MANY_CARDS: typing.Final = f"at most {schemas.MAX_CARDS_PER_REQUEST} cards per request"
18+
19+
1620
@litestar.get("/decks/{deck_id:int}/cards/")
1721
async def list_cards(
18-
deck_id: FromPath[int],
22+
deck_id: PathId,
1923
list_cards_use_case: NamedDependency[ListCardsUseCase],
20-
after: Annotated[int | None, QueryParameter(gt=0)] = None,
24+
after: Annotated[int | None, QueryParameter(gt=0, le=schemas.MAX_ID)] = None,
2125
limit: Annotated[int, QueryParameter(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT,
2226
) -> schemas.CardsPage:
2327
cards: typing.Final = await list_cards_use_case(deck_id=deck_id, after=after, limit=limit)
2428
return schemas.CardsPage.from_models(cards, limit=limit)
2529

2630

2731
@litestar.get("/cards/{card_id:int}/")
28-
async def get_card(card_id: FromPath[int], get_card_use_case: NamedDependency[GetCardUseCase]) -> schemas.Card:
32+
async def get_card(card_id: PathId, get_card_use_case: NamedDependency[GetCardUseCase]) -> schemas.Card:
2933
return schemas.Card.model_validate(await get_card_use_case(card_id=card_id))
3034

3135

3236
@litestar.post("/decks/{deck_id:int}/cards/")
3337
async def create_cards(
34-
deck_id: FromPath[int],
38+
deck_id: PathId,
3539
data: list[schemas.CardCreate],
3640
create_cards_use_case: NamedDependency[CreateCardsUseCase],
3741
) -> schemas.Cards:
42+
if len(data) > schemas.MAX_CARDS_PER_REQUEST:
43+
raise ValidationException(detail=TOO_MANY_CARDS)
3844
return schemas.Cards.from_models(await create_cards_use_case(deck_id=deck_id, data=data))
3945

4046

4147
@litestar.put("/decks/{deck_id:int}/cards/")
4248
async def update_cards(
43-
deck_id: FromPath[int],
49+
deck_id: PathId,
4450
data: list[schemas.CardUpdate],
4551
update_cards_use_case: NamedDependency[UpdateCardsUseCase],
4652
) -> schemas.Cards:
53+
if len(data) > schemas.MAX_CARDS_PER_REQUEST:
54+
raise ValidationException(detail=TOO_MANY_CARDS)
4755
if len({card.id for card in data}) != len(data):
4856
raise ValidationException(detail="card ids must be unique")
4957
return schemas.Cards.from_models(await update_cards_use_case(deck_id=deck_id, data=data))

‎app/api/decks.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33

44
import litestar
55
from litestar.di import NamedDependency
6-
from litestar.params import FromPath, QueryParameter
6+
from litestar.params import QueryParameter
77

88
from app import schemas
9+
from app.api import PathId
910
from app.use_cases.create_deck import CreateDeckUseCase
1011
from app.use_cases.get_deck import GetDeckUseCase
1112
from app.use_cases.list_decks import ListDecksUseCase
@@ -15,21 +16,21 @@
1516
@litestar.get("/decks/")
1617
async def list_decks(
1718
list_decks_use_case: NamedDependency[ListDecksUseCase],
18-
after: Annotated[int | None, QueryParameter(gt=0)] = None,
19+
after: Annotated[int | None, QueryParameter(gt=0, le=schemas.MAX_ID)] = None,
1920
limit: Annotated[int, QueryParameter(ge=1, le=schemas.MAX_PAGE_LIMIT)] = schemas.DEFAULT_PAGE_LIMIT,
2021
) -> schemas.DecksPage:
2122
decks: typing.Final = await list_decks_use_case(after=after, limit=limit)
2223
return schemas.DecksPage.from_models(decks, limit=limit)
2324

2425

2526
@litestar.get("/decks/{deck_id:int}/")
26-
async def get_deck(deck_id: FromPath[int], get_deck_use_case: NamedDependency[GetDeckUseCase]) -> schemas.DeckWithCards:
27+
async def get_deck(deck_id: PathId, get_deck_use_case: NamedDependency[GetDeckUseCase]) -> schemas.DeckWithCards:
2728
return schemas.DeckWithCards.model_validate(await get_deck_use_case(deck_id=deck_id))
2829

2930

3031
@litestar.put("/decks/{deck_id:int}/")
3132
async def update_deck(
32-
deck_id: FromPath[int],
33+
deck_id: PathId,
3334
data: schemas.DeckUpdate,
3435
update_deck_use_case: NamedDependency[UpdateDeckUseCase],
3536
) -> schemas.Deck:

‎app/application.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import modern_di
55
import modern_di_litestar
6-
from advanced_alchemy.exceptions import DuplicateKeyError, NotFoundError
6+
from advanced_alchemy.exceptions import DuplicateKeyError, ForeignKeyError, NotFoundError
77
from lite_bootstrap import LitestarBootstrapper
88
from litestar.config.app import AppConfig
99
from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor
@@ -25,6 +25,7 @@ def build_app() -> litestar.Litestar:
2525
exception_handlers={
2626
DuplicateKeyError: exceptions.duplicate_key_error_handler,
2727
NotFoundError: exceptions.not_found_error_handler,
28+
ForeignKeyError: exceptions.not_found_error_handler,
2829
},
2930
route_handlers=[decks.ROUTER, cards.ROUTER],
3031
plugins=[modern_di_litestar.ModernDIPlugin(di_container, autowired_groups=[ioc.UseCases])],

‎app/exceptions.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66

77
if typing.TYPE_CHECKING:
8-
from advanced_alchemy.exceptions import DuplicateKeyError, NotFoundError
8+
from advanced_alchemy.exceptions import DuplicateKeyError, ForeignKeyError, NotFoundError
99

1010

1111
def duplicate_key_error_handler(_: object, exc: DuplicateKeyError) -> litestar.Response[dict[str, typing.Any]]:
@@ -16,7 +16,7 @@ def duplicate_key_error_handler(_: object, exc: DuplicateKeyError) -> litestar.R
1616
)
1717

1818

19-
def not_found_error_handler(_: object, __: NotFoundError) -> litestar.Response[dict[str, typing.Any]]:
19+
def not_found_error_handler(_: object, __: NotFoundError | ForeignKeyError) -> litestar.Response[dict[str, typing.Any]]:
2020
return litestar.Response(
2121
media_type=litestar.MediaType.JSON,
2222
content={"detail": "Not found"},

‎app/models.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@ class Deck(BigIntAuditBase):
1717
idempotency_key: orm.Mapped[uuid.UUID] = orm.mapped_column(sa.Uuid, nullable=False)
1818
name: orm.Mapped[str] = orm.mapped_column(sa.String, nullable=False)
1919
description: orm.Mapped[str | None] = orm.mapped_column(sa.String, nullable=True)
20-
cards: orm.Mapped[list[Card]] = orm.relationship("Card", lazy="noload", uselist=True)
20+
cards: orm.Mapped[list[Card]] = orm.relationship("Card", lazy="noload", uselist=True, order_by="Card.id")
2121

2222

2323
class Card(BigIntAuditBase):
2424
__tablename__ = "cards"
25-
__table_args__ = (sa.UniqueConstraint("deck_id", "front", name="card_deck_id_front_uc"),)
25+
__table_args__ = (
26+
sa.UniqueConstraint("deck_id", "front", name="card_deck_id_front_uc"),
27+
sa.Index("ix_cards_deck_id_id", "deck_id", "id"),
28+
)
2629

2730
front: orm.Mapped[str] = orm.mapped_column(sa.String, nullable=False)
2831
back: orm.Mapped[str | None] = orm.mapped_column(sa.String, nullable=True)

‎app/repositories.py‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,25 @@ async def add_cards(self, deck_id: int, cards: list[schemas.CardCreate]) -> Sequ
7575
async def update_cards(self, deck_id: int, cards: list[schemas.CardUpdate]) -> Sequence[models.Card]:
7676
if not cards:
7777
return []
78-
updates: typing.Final = sa.values(
79-
sa.column("id", sa.BigInteger),
80-
sa.column("front", sa.String),
81-
sa.column("back", sa.String),
82-
sa.column("hint", sa.String),
83-
name="updates",
84-
).data([(card.id, card.front, card.back, card.hint) for card in cards])
78+
updates: typing.Final = (
79+
sa.func.unnest(
80+
sa.bindparam("ids", [card.id for card in cards], type_=postgresql.ARRAY(sa.BigInteger)),
81+
sa.bindparam("fronts", [card.front for card in cards], type_=postgresql.ARRAY(sa.String)),
82+
sa.bindparam("backs", [card.back for card in cards], type_=postgresql.ARRAY(sa.String)),
83+
sa.bindparam("hints", [card.hint for card in cards], type_=postgresql.ARRAY(sa.String)),
84+
)
85+
.table_valued(
86+
sa.column("id", sa.BigInteger),
87+
sa.column("front", sa.String),
88+
sa.column("back", sa.String),
89+
sa.column("hint", sa.String),
90+
)
91+
.render_derived(name="updates")
92+
)
8593
statement: typing.Final = (
8694
sa.update(models.Card)
8795
.where(models.Card.id == updates.c.id, models.Card.deck_id == deck_id)
88-
.values(front=updates.c.front, back=updates.c.back, hint=updates.c.hint, updated_at=sa.func.now())
96+
.values(front=updates.c.front, back=updates.c.back, hint=updates.c.hint)
8997
.returning(models.Card)
9098
)
9199
with wrap_sqlalchemy_exception(error_messages=DEFAULT_ERROR_MESSAGE_TEMPLATES, dialect_name="postgresql"):

‎app/schemas.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import uuid
2-
from typing import TYPE_CHECKING, Final, Protocol, Self
2+
from typing import TYPE_CHECKING, Annotated, Final, Protocol, Self
33

44
import pydantic
5-
from pydantic import BaseModel, PositiveInt
5+
from pydantic import BaseModel, Field, PositiveInt
66

77

88
if TYPE_CHECKING:
@@ -11,6 +11,10 @@
1111

1212
DEFAULT_PAGE_LIMIT: Final = 50
1313
MAX_PAGE_LIMIT: Final = 100
14+
MAX_CARDS_PER_REQUEST: Final = 1000
15+
MAX_ID: Final = 2**63 - 1
16+
17+
Id = Annotated[int, Field(gt=0, le=MAX_ID)]
1418

1519

1620
class HasId(Protocol):
@@ -52,7 +56,7 @@ class CardCreate(CardBase):
5256

5357

5458
class CardUpdate(CardBase):
55-
id: PositiveInt
59+
id: Id
5660

5761

5862
class Card(CardUpdate):
@@ -87,7 +91,7 @@ class Deck(DeckBase):
8791

8892

8993
class DeckWithCards(Deck):
90-
"""Deck detail view; cards are eager-loaded (selectinload) and always present."""
94+
"""Deck detail view; cards are eager-loaded (joinedload) and always present."""
9195

9296
cards: list[Card]
9397

0 commit comments

Comments
 (0)