Repository navigation
feat: make deck creation idempotent with a client-generated key - #90
Merged
Merged
Conversation
Posting twice with one key must return the first deck and leave a single row.
CreateDeckUseCase looks the key up inside its transaction and returns the existing deck, so a postgres_retry rerun after a COMMIT that landed, or a client resend, does not create a second deck. PUT /decks/ takes the new DeckUpdate schema without the key.
A connection holds an uncommitted insert with the key while CreateDeckUseCase runs. The lookup version fails with a duplicate-key error once that insert commits.
The no-op update on idempotency_key returns the existing row on a replay, and a concurrent request with the same key waits on the row lock instead of failing on the unique constraint.
…-key # Conflicts: # app/repositories.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
POST /api/decks/now requires a client-generatedidempotency_key. Sending the same key again returns the deck created the first time instead of creating a second one.This closes the gap the pre-ping PR left open. A
postgres_retryrerun after aCOMMITthat landed but whose connection dropped would create the deck twice. A client resending after a timeout would too. advanced-alchemy has no single-query way to do this. Itsupsertandget_or_upsertboth run a SELECT followed by an INSERT, so the deck is created with one Postgres statement instead.Changes
decks.idempotency_key: a UUID column, not null, with the unique constraintdeck_idempotency_key_uc. The migration backfills existing rows withgen_random_uuid(), then drops the default.schemas.DeckCreategainsidempotency_key: uuid.UUID.PUT /api/decks/{id}/takes a newschemas.DeckUpdatewithout the key, since an update is already safe to repeat.DecksRepository.get_or_createsends oneINSERT ... ON CONFLICT (idempotency_key) DO UPDATE SET idempotency_key = EXCLUDED.idempotency_key RETURNING *. On a conflict, the update leaves the row as it was andRETURNINGhands back the existing deck.DO NOTHINGwould return no row and need a second query.updated_atdoesn't change, because SQLAlchemy only appliesonupdatedefaults to columns listed inset_.CreateDeckUseCasecalls it inside its transaction.pyproject.tomlmarkspydantic.BaseModelandBigIntAuditBaseas runtime-evaluated base classes for ruff'sTCrules. Without that,ruff check --fixmovesimport uuidinto aTYPE_CHECKINGblock inmodels.pyandschemas.py, and SQLAlchemy and pydantic then fail to resolve the annotation.CreateCardsUseCaserelies on the existing(deck_id, front)unique constraint, so a replay is rejected as a duplicate instead of creating the cards twice. The updates write the same values again.Tests
test_post_deckssends a key.GET /api/decks/lists one deck.tests/test_create_deck.py: a separate connection holds an uncommitted insert with the key whileCreateDeckUseCaseruns. The test pollspg_stat_activityuntil the use case is blocked on the lock, commits the insert, and checks that the use case returns that deck. With a lookup-then-create version, this test failed withUniqueViolationError. It deletes its row afterwards.Each new test failed before the change it covers. Ruff and ty pass, and all 40 tests pass with 100% coverage against a local Postgres 14. The migration upgrades and downgrades cleanly.
alembic checkshows no drift on the new column. It does report older drift (idisIntegerin the init migration butBigIntegerin the model, andcards.deck_idis nullable), which this PR leaves alone.Limits
get_or_createis Postgres-specific, and it runs on the session directly, so advanced-alchemy doesn't wrap its errors. The templates only target Postgres.POST /api/decks/now has to send a key, which breaks existing clients.