diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index f45dc4f2..cfd6dd85 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -2562,3 +2562,43 @@ func TestPostgreSQLSessionRevocationIsImmediateOnAnotherReplica(t *testing.T) { t.Fatalf("revoking one session invalidated another: %v", err) } } + +// Reusing an idempotency key with a different payload must be a conflict. +// Both idempotency paths returned a bare error, which writeDomainError maps +// to its 422 default, so the API answered 422 where openapi.json and +// state-transitions.json ("same_key_different_payload": +// "reject_conflict_without_state_change") both require 409. It is the +// difference between "your request was malformed" and "that key is taken", +// and a client acting on the former would rewrite a correct request. +func TestPostgreSQLIdempotencyKeyReuseIsAConflict(t *testing.T) { + db := openIntegrationPostgres(t) + applyIntegrationMigrations(t, db) + + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Microsecond) + if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ('idem-player', 'idem-steam')`); err != nil { + t.Fatal(err) + } + spec := domain.QueueSpec{Playlist: domain.Casual, ClientBuild: "build-1", ProtocolVersion: 1} + if _, err := CreateQueueTicket(ctx, db, "idem-ticket-one", "idem-player", "idem-key-00000001", spec, now); err != nil { + t.Fatalf("first create: %v", err) + } + + // Same key, same payload: replays the original result. + replay, err := CreateQueueTicket(ctx, db, "idem-ticket-one", "idem-player", "idem-key-00000001", spec, now.Add(time.Second)) + if err != nil { + t.Fatalf("identical replay must succeed: %v", err) + } + if replay.TicketID != "idem-ticket-one" { + t.Fatalf("replay returned %q", replay.TicketID) + } + + // Same key, different payload: conflict, not a validation error. + _, err = CreateQueueTicket(ctx, db, "idem-ticket-two", "idem-player", "idem-key-00000001", spec, now.Add(time.Second)) + if err == nil { + t.Fatal("reusing a key with a different ticket was accepted") + } + if !errors.Is(err, domain.ErrConflict) { + t.Fatalf("err = %v; must wrap domain.ErrConflict so the API answers 409, not its 422 default", err) + } +} diff --git a/server/store/queue_sql.go b/server/store/queue_sql.go index 2e97937d..57293e91 100644 --- a/server/store/queue_sql.go +++ b/server/store/queue_sql.go @@ -178,7 +178,12 @@ func CreateQueueTicket(ctx context.Context, db *sql.DB, ticketID, playerID, idem return err } if !bytes.Equal(priorDigest, digest[:]) { - return fmt.Errorf("queue create idempotency conflict") + // Must wrap ErrConflict: writeDomainError maps unrecognised + // errors to 422, but the contract + // (state-transitions.json "same_key_different_payload") and + // openapi.json both require 409 for reusing a key with a + // different payload. + return fmt.Errorf("%w: queue create idempotency conflict", domain.ErrConflict) } var prior queueTicketRecord if err := json.Unmarshal(priorResult, &prior); err != nil { @@ -321,7 +326,7 @@ func mutateQueueTicket(ctx context.Context, db *sql.DB, playerID, ticketID, idem return err } if !bytes.Equal(priorDigest, digest[:]) { - return fmt.Errorf("queue mutation idempotency conflict") + return fmt.Errorf("%w: queue mutation idempotency conflict", domain.ErrConflict) } var prior queueTicketRecord if err := json.Unmarshal(priorResult, &prior); err != nil {