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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class ListDecksUseCase:
### Database layer

- `app/models.py` — `BigIntAuditBase` from `advanced_alchemy` (auto `id`, `created_at`, `updated_at`). The module aliases `orm_registry.metadata` onto `orm.DeclarativeBase.metadata` so Alembic autogenerate sees both. New models go here.
- `app/repositories.py` — Subclass `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` and `upsert_cards` send `INSERT ... ON CONFLICT ... RETURNING`. A statement that can hit a unique constraint runs inside `wrap_sqlalchemy_exception` so that it still raises `DuplicateKeyError`.
- `app/repositories.py` — Subclass `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. `schemas.CardUpdates` rejects duplicate ids with a 422 before any query runs. A statement that can hit a unique constraint runs inside `wrap_sqlalchemy_exception` so that it still raises `DuplicateKeyError`.
- `app/resources/db.py` — engine factories live here and are DI creators: modern-di resolves their parameter annotations at runtime, so `pyproject.toml` exempts `app/resources/` from ruff's `TC` rules. `create_session` passes `join_transaction_mode="create_savepoint"`. This is inert in production (the session binds to an engine) but enables the test rollback pattern below: 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.
- `migrations/env.py` swaps the asyncpg driver for the sync `postgresql` driver and uses `app.models.METADATA` as `target_metadata`.

Expand Down
2 changes: 1 addition & 1 deletion app/api/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async def create_cards(
@ROUTER.put("/decks/{deck_id}/cards/")
async def update_cards(
deck_id: int,
data: list[schemas.CardUpdate],
data: schemas.CardUpdates,
update_cards_use_case: UpdateCardsUseCase = FromDI(UpdateCardsUseCase),
) -> schemas.Cards:
return schemas.Cards.from_models(await update_cards_use_case(deck_id=deck_id, data=data))
30 changes: 17 additions & 13 deletions app/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,24 @@ async def list_for_deck(self, deck_id: int, *, after: int | None, limit: int) ->
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])

async def upsert_cards(self, deck_id: int, cards: list[schemas.CardUpdate]) -> Sequence[models.Card]:
async def update_cards(self, deck_id: int, cards: list[schemas.CardUpdate]) -> Sequence[models.Card]:
if not cards:
return []
statement = postgresql.insert(models.Card).values([{**card.model_dump(), "deck_id": deck_id} for card in cards])
statement = statement.on_conflict_do_update(
index_elements=[models.Card.id],
set_={
"front": statement.excluded.front,
"back": statement.excluded.back,
"hint": statement.excluded.hint,
"deck_id": statement.excluded.deck_id,
"updated_at": sa.func.now(),
},
updates: typing.Final = sa.values(
sa.column("id", sa.BigInteger),
sa.column("front", sa.String),
sa.column("back", sa.String),
sa.column("hint", sa.String),
name="updates",
).data([(card.id, card.front, card.back, card.hint) for card in cards])
statement: typing.Final = (
sa.update(models.Card)
.where(models.Card.id == updates.c.id, models.Card.deck_id == deck_id)
.values(front=updates.c.front, back=updates.c.back, hint=updates.c.hint, updated_at=sa.func.now())
.returning(models.Card)
)
with wrap_sqlalchemy_exception(error_messages=DEFAULT_ERROR_MESSAGE_TEMPLATES, dialect_name="postgresql"):
upserted: typing.Final = (await self.repository.session.scalars(statement.returning(models.Card))).all()
return sorted(upserted, key=operator.attrgetter("id"))
updated: typing.Final = (await self.repository.session.scalars(statement)).all()
if len(updated) != len(cards):
raise NotFoundError
return sorted(updated, key=operator.attrgetter("id"))
12 changes: 11 additions & 1 deletion app/schemas.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import uuid
from typing import TYPE_CHECKING, Final, Protocol, Self
from typing import TYPE_CHECKING, Annotated, Final, Protocol, Self

import pydantic
from pydantic import BaseModel, PositiveInt
Expand Down Expand Up @@ -55,6 +55,16 @@ class CardUpdate(CardBase):
id: PositiveInt


def _ensure_unique_ids(cards: list[CardUpdate]) -> list[CardUpdate]:
if len({card.id for card in cards}) != len(cards):
msg = "card ids must be unique"
raise ValueError(msg)
return cards


CardUpdates = Annotated[list[CardUpdate], pydantic.AfterValidator(_ensure_unique_ids)]


class Card(CardUpdate):
deck_id: PositiveInt

Expand Down
2 changes: 1 addition & 1 deletion app/use_cases/update_cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ class UpdateCardsUseCase:
@postgres_retry
async def __call__(self, *, deck_id: int, data: list[schemas.CardUpdate]) -> Sequence[models.Card]:
async with self.transaction:
cards: typing.Final = await self.cards_repository.upsert_cards(deck_id, data)
cards: typing.Final = await self.cards_repository.update_cards(deck_id, data)
await self.transaction.commit()
return cards
53 changes: 53 additions & 0 deletions tests/test_cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,59 @@ async def test_update_cards_to_a_duplicate_front(client: AsyncClient) -> None:
assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT


async def test_update_cards_does_not_move_a_card_from_another_deck(
client: AsyncClient, db_session: AsyncSession
) -> None:
deck, other_deck = await factories.DeckModelFactory.create_batch_async(size=2)
card = await factories.CardModelFactory.create_async(deck_id=other_deck.id)

response = await client.put(f"/api/decks/{deck.id}/cards/", json=[{"id": card.id, "front": "moved"}])
assert response.status_code == status.HTTP_404_NOT_FOUND
stored = (
await db_session.execute(sa.select(models.Card.deck_id, models.Card.front).where(models.Card.id == card.id))
).one()
assert stored == (other_deck.id, card.front)


async def test_update_cards_with_an_unknown_id_inserts_nothing(client: AsyncClient, db_session: AsyncSession) -> None:
deck = await factories.DeckModelFactory.create_async()
card = await factories.CardModelFactory.create_async(deck_id=deck.id)

response = await client.put(f"/api/decks/{deck.id}/cards/", json=[{"id": card.id + 1, "front": "via put"}])
assert response.status_code == status.HTTP_404_NOT_FOUND
assert (await db_session.scalars(sa.select(models.Card.front).where(models.Card.deck_id == deck.id))).all() == [
card.front
]

response = await client.post(f"/api/decks/{deck.id}/cards/", json=[{"front": "via post"}])
assert response.status_code == status.HTTP_200_OK


async def test_update_cards_with_one_unknown_id_updates_nothing(client: AsyncClient, db_session: AsyncSession) -> None:
deck = await factories.DeckModelFactory.create_async()
card = await factories.CardModelFactory.create_async(deck_id=deck.id)

response = await client.put(
f"/api/decks/{deck.id}/cards/",
json=[{"id": card.id, "front": "updated"}, {"id": card.id + 1, "front": "unknown"}],
)
assert response.status_code == status.HTTP_404_NOT_FOUND
assert (await db_session.scalars(sa.select(models.Card.front).where(models.Card.id == card.id))).one() == card.front


async def test_update_cards_rejects_duplicate_ids(client: AsyncClient, statements: list[str]) -> None:
deck = await factories.DeckModelFactory.create_async()
card = await factories.CardModelFactory.create_async(deck_id=deck.id)
statements.clear()

response = await client.put(
f"/api/decks/{deck.id}/cards/",
json=[{"id": card.id, "front": "first"}, {"id": card.id, "front": "second"}],
)
assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT
assert statements == []


async def test_openapi_card_deck_id_required(client: AsyncClient) -> None:
response = await client.get("/openapi.json")
assert response.status_code == status.HTTP_200_OK
Expand Down
Loading