diff --git a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md index ed44439..302a1bc 100644 --- a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md +++ b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md @@ -124,6 +124,13 @@ public async Task MigrationV029_ShouldCreatePortfolioTablesAndPublicationGuards( Assert.Contains("CREATE INDEX ix_portfolio_games_group ON portfolio_games (group_id, completed_at DESC);", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE INDEX ix_portfolio_game_masters_player ON portfolio_game_masters (player_id, portfolio_game_id);", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE INDEX ix_portfolio_game_reviews_pending ON portfolio_game_reviews (portfolio_game_id, created_at DESC) WHERE moderation_status = 'Pending';", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE FUNCTION lock_portfolio_publication_mutation() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_games_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE OF is_public ON portfolio_games FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_game_sessions_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_sessions FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_game_masters_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_masters FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_sessions_lock_portfolio_publication_mutation BEFORE DELETE OR UPDATE OF scheduled_at ON sessions FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_game_groups_lock_portfolio_publication_mutation_before_delete BEFORE DELETE ON game_groups FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_players_lock_portfolio_publication_mutation_before_delete BEFORE DELETE ON players FOR EACH STATEMENT", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE FUNCTION validate_public_portfolio_game_required_links() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal); Assert.Contains("PERFORM pg_advisory_xact_lock(20260530, 108);", normalizedMigration, StringComparison.Ordinal); Assert.Contains("current_setting('transaction_isolation') <> 'read committed'", normalizedMigration, StringComparison.Ordinal); @@ -155,7 +162,7 @@ public async Task MigrationV029_ShouldStoreProviderNeutralCoverKeys() } ``` -Add `PortfolioSessionDeletionSourceTests.cs`. Normalize whitespace before comparing source text and assert that both session-deletion paths explicitly lock the target session row, unpublish linked cards, and then delete the required session link: +Add `PortfolioSessionDeletionSourceTests.cs`. Normalize whitespace before comparing source text and assert that both session-deletion paths acquire the portfolio mutation lock, explicitly lock the target session row, unpublish linked cards, and then delete the required session link: ```csharp [Fact] @@ -166,11 +173,17 @@ public async Task SharedDeleteSessionHandler_ShouldLockSessionBeforeUnpublishing const string sessionLock = "FROM sessions s WHERE s.id = @SessionId FOR UPDATE OF s"; + const string mutationLock = + "SELECT pg_advisory_xact_lock(20260530, 108)"; const string unpublish = "UPDATE portfolio_games pg SET is_public = false, updated_at = now() FROM portfolio_game_sessions pgs WHERE pgs.portfolio_game_id = pg.id AND pgs.session_id = @SessionId AND pg.is_public = true"; + Assert.Contains(mutationLock, source, StringComparison.Ordinal); Assert.Contains(sessionLock, source, StringComparison.Ordinal); Assert.Contains(unpublish, source, StringComparison.Ordinal); + Assert.True( + source.IndexOf(mutationLock, StringComparison.Ordinal) < + source.IndexOf(sessionLock, StringComparison.Ordinal)); Assert.True( source.IndexOf(sessionLock, StringComparison.Ordinal) < source.IndexOf(unpublish, StringComparison.Ordinal)); @@ -237,7 +250,7 @@ public async Task ExplicitUnpublishThenSessionDelete_ShouldCommitAndPreserveFirs [Theory] [InlineData(true)] [InlineData(false)] -public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard(bool publishCommitsFirst) +public async Task ConcurrentPublishAndLinkDelete_ShouldSerializeBeforeRowsAndRejectInvalidPublicCard(bool publishMutatesFirst) [Theory] [InlineData("portfolio_game_sessions", "session_id")] @@ -261,7 +274,7 @@ public async Task PublishedCardFutureReschedule_ShouldAutomaticallyUnpublishAndP public async Task PublishedCardPastFuturePastReschedule_ShouldRemainPublicAndPreserveFirstPublishedAt() [Fact] -public async Task ConcurrentBatchFutureReschedules_ShouldLockPublicCardsInStableOrderWithoutDeadlock() +public async Task ConcurrentBatchFutureReschedules_ShouldSerializeBeforeSessionRowsWithoutDeadlock() [Fact] public async Task PublishingDraftCardWithAnyFutureLinkedSession_ShouldFailCommit() @@ -272,6 +285,17 @@ public async Task ConcurrentPublishAndFutureReschedule_ShouldNotDeadlockOrCommit [Fact] public async Task ConcurrentNewLinkPublishAndFutureReschedule_ShouldNotCommitInvalidPublicCard() +[Fact] +public async Task PortfolioSessionLinkInsert_ShouldAcquirePublicationLockBeforeRows() + +[Fact] +public async Task FutureReschedule_ShouldAcquirePublicationLockBeforeSessionRows() + +[Theory] +[InlineData(true)] +[InlineData(false)] +public async Task ConcurrentSessionDeleteAndFutureReschedule_ShouldSerializeMutationGateBeforeRowsWithoutDeadlock(bool deleteMutatesFirst) + [Fact] public async Task RepeatableReadStaleSnapshotFutureReschedule_ShouldBeRejectedWithoutInvalidPublicCard() @@ -289,7 +313,7 @@ public async Task RequiredParentCascadeDelete_ShouldFailCommitForPublishedCard(s public async Task ParentCardAndGroupCascadeDeletes_ShouldCommit() ``` -The direct-delete, moved-link, invalid publication, and direct parent-cascade scenarios must expect PostgreSQL `23514` at commit. Every selected linked session must be completed with `scheduled_at < now()`: one future link among multiple selected sessions rejects publication. A final future reschedule must atomically unpublish linked public cards while preserving their first `published_at`; `past -> future -> past` in one transaction must leave the card public. Opposing-order batch reschedules must use an advisory test gate plus `pg_blocking_pids` observation with bounded timeouts, complete without card deadlock, and leave both cards private; do not rely on `pg_sleep` timing. The `READ COMMITTED` concurrency scenarios must launch bounded tasks together, cover both publish/delete lock orders, and prove there is no deadlock, write-skew, or invalid public commit. A session-delete versus future-reschedule race must use the common `sessions` then `portfolio_games` lock order, cover both first-session-lock orders through real blocking transactions, and finish with the card private and session deleted. The publish/reschedule races must finish with the future session committed and the card private, including a new-link draft publication forced behind the post-row-lock advisory gate. The `REPEATABLE READ` scenarios must reject triggered portfolio writes with `0A000`, including both draft-link deletion versus publication commit orders and stale-snapshot final-future reschedules after a newly linked publication, because a stale snapshot after lock acquisition cannot safely validate the invariant. The parent-card and owning-group cascade scenarios must commit successfully. +The direct-delete, moved-link, invalid publication, and direct parent-cascade scenarios must expect PostgreSQL `23514` at commit. Every selected linked session must be completed with `scheduled_at < now()`: one future link among multiple selected sessions rejects publication. A final future reschedule must atomically unpublish linked public cards while preserving their first `published_at`; `past -> future -> past` in one transaction must leave the card public. Opposing-order batch reschedules must prove with `pg_blocking_pids` observation and bounded timeouts that the shared mutation lock serializes statements before session rows, complete without deadlock, and leave both cards private; do not rely on `pg_sleep` timing. The `READ COMMITTED` concurrency scenarios must launch bounded tasks together, cover both publish/delete lock orders, and prove there is no deadlock, write-skew, or invalid public commit. A session-delete versus future-reschedule race must use the common advisory-lock then `sessions` then `portfolio_games` lock order, cover both first-mutation-lock orders through real blocking transactions, and finish with the card private and session deleted. Link insertion and final-future reschedule gate scenarios must prove that invariant-affecting statements acquire the shared mutation lock before rows. The publish/reschedule races must finish with the future session committed and the card private. The `REPEATABLE READ` scenarios must reject triggered portfolio writes with `0A000`, including both draft-link deletion versus publication commit orders and stale-snapshot final-future reschedules after a newly linked publication, because a stale snapshot after lock acquisition cannot safely validate the invariant. The parent-card and owning-group cascade scenarios must commit successfully. - [ ] **Step 3: Run the Task 1 tests to verify RED** @@ -355,6 +379,46 @@ CREATE TABLE portfolio_game_masters ( CREATE INDEX ix_portfolio_game_masters_player ON portfolio_game_masters (player_id, portfolio_game_id); +CREATE FUNCTION lock_portfolio_publication_mutation() +RETURNS TRIGGER +LANGUAGE plpgsql +AS $$ +BEGIN + PERFORM pg_advisory_xact_lock(20260530, 108); + RETURN NULL; +END; +$$; + +CREATE TRIGGER trg_portfolio_games_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE OF is_public ON portfolio_games +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_portfolio_game_sessions_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_sessions +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_portfolio_game_masters_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_masters +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_sessions_lock_portfolio_publication_mutation +BEFORE DELETE OR UPDATE OF scheduled_at ON sessions +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_game_groups_lock_portfolio_publication_mutation_before_delete +BEFORE DELETE ON game_groups +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_players_lock_portfolio_publication_mutation_before_delete +BEFORE DELETE ON players +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + CREATE FUNCTION validate_public_portfolio_game_required_links() RETURNS TRIGGER LANGUAGE plpgsql @@ -524,11 +588,11 @@ CREATE INDEX ix_portfolio_game_reviews_pending WHERE moderation_status = 'Pending'; ``` -The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. At transaction commit validators acquire the same transaction-level advisory lock and reject a surviving published card when either required link set is empty or any linked session has `scheduled_at >= now()`. The intentionally global lock is appropriate for low-volume portfolio publication writes: under the application default `READ COMMITTED` isolation level it serializes validation, prevents write-skew across distinct child links, and gives multi-card transactions one lock order. PostgreSQL retains stale snapshots under `REPEATABLE READ` and `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those isolation levels with `0A000`. The deferred future-reschedule trigger re-reads the final session row, skips intermediate future values that end in the past, and rejects final-future reschedules outside `READ COMMITTED` with `0A000`. Under `READ COMMITTED`, it locks all cards linked to any final-future session in `portfolio_games.id` order. It then acquires the publication advisory lock and runs one guarded public-card unpublish update with a fresh statement snapshot. The row-lock phase deliberately includes committed drafts so a concurrent draft-to-public publication cannot pass validation against the pre-reschedule session snapshot and commit afterward; the post-row-lock advisory phase also serializes a previously invisible concurrent link-add publication without moving the advisory lock above card locks. At `READ COMMITTED`, draft edits, explicit unpublishing, future reschedules, and card or club cascade deletion remain valid. Normal session-deletion handlers use the same `sessions` then `portfolio_games` lock order: explicitly lock the target session row, unpublish linked cards, then delete the session. +The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. Immediate statement triggers acquire one transaction-level advisory lock before any invariant-affecting rows are changed: publication transitions and deletes, required-link edits, session deletes and scheduled-date changes, and parent deletes that can cascade into required links. The intentionally global lock is appropriate for low-volume portfolio and schedule writes: under the application default `READ COMMITTED` isolation level it establishes one advisory-lock then row-lock protocol, prevents write-skew across distinct child links, and removes card/advisory and session/advisory inversions. At transaction commit validators re-acquire the same lock and reject a surviving published card when either required link set is empty or any linked session has `scheduled_at >= now()`. PostgreSQL retains stale snapshots under `REPEATABLE READ` and `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those isolation levels with `0A000`. The deferred future-reschedule trigger re-reads the final session row, skips intermediate future values that end in the past, and rejects final-future reschedules outside `READ COMMITTED` with `0A000`. Under `READ COMMITTED`, it locks all cards linked to any final-future session in `portfolio_games.id` order, re-acquires the shared advisory lock, and runs one guarded public-card unpublish update with a fresh statement snapshot. The row-lock phase deliberately includes committed drafts so a concurrent draft-to-public publication cannot pass validation against the pre-reschedule session snapshot and commit afterward. At `READ COMMITTED`, draft edits, explicit unpublishing, future reschedules, and card or club cascade deletion remain valid. Normal session-deletion handlers use the same advisory-lock then `sessions` then `portfolio_games` order: explicitly acquire the mutation lock, lock the target session row, unpublish linked cards, then delete the session. - [ ] **Step 5: Lock sessions before explicitly unpublishing linked cards in both session-deletion handlers** -In `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`, strengthen the initial session fetch with `FOR UPDATE OF s`. After authorization, run this statement inside the existing transaction before `DELETE FROM sessions`: +In `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`, acquire `pg_advisory_xact_lock(20260530, 108)` immediately after starting the transaction, then strengthen the initial session fetch with `FOR UPDATE OF s`. After authorization, run this statement inside the existing transaction before `DELETE FROM sessions`: ```sql UPDATE portfolio_games pg @@ -540,7 +604,7 @@ WHERE pgs.portfolio_game_id = pg.id AND pg.is_public = true ``` -In `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`, start a transaction before deleting. Lock the guild-scoped target session row with `SELECT s.id ... FOR UPDATE OF s`, preserving the existing not-found result. Run this guild-scoped unpublish statement before the existing guild-scoped `DELETE FROM sessions`, then commit: +In `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`, start a transaction before deleting and acquire `pg_advisory_xact_lock(20260530, 108)`. Lock the guild-scoped target session row with `SELECT s.id ... FOR UPDATE OF s`, preserving the existing not-found result. Run this guild-scoped unpublish statement before the existing guild-scoped `DELETE FROM sessions`, then commit: ```sql UPDATE portfolio_games pg @@ -556,7 +620,7 @@ WHERE pgs.portfolio_game_id = pg.id AND pg.is_public = true ``` -Both handlers deliberately use `sessions` then `portfolio_games` locking before session deletion. This matches future rescheduling, keeps normal deletes successful, preserves the first-publication `published_at`, and leaves the deferred trigger as the direct-SQL and concurrency backstop. +Both handlers deliberately use advisory-lock then `sessions` then `portfolio_games` locking before session deletion. This matches future rescheduling, keeps normal deletes successful, preserves the first-publication `published_at`, and leaves the triggers as the direct-SQL and concurrency backstop. Also add `AND p.platform = 'Discord'` to the Discord manager lookup before casting manager IDs, so cross-platform identities cannot affect authorization. @@ -570,7 +634,7 @@ Run: dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests|FullyQualifiedName~PortfolioSchemaGateSourceTests" ``` -Expected: PASS, including PostgreSQL 17 migration application, rejected direct required-link deletes, rejected moved links and session/player cascades, rejected publication with any future linked session, automatic unpublish with preserved `published_at` after final future reschedule, preserved public state after `past -> future -> past`, opposing-order batch reschedules without card deadlock, bounded `READ COMMITTED` publish/delete in both commit orders, existing-link and new-link publish/reschedule races, session-delete/reschedule serialization in both first-lock orders, and distinct-link deletion without deadlock, write-skew, or invalid public commit, rejected `REPEATABLE READ` triggered writes including both draft-delete versus publish commit orders and stale-snapshot final-future reschedules, successful parent-card and owning-group cascades, Discord identity scoping, and Compose/Aspire HTTP health gating with a non-proxied bot endpoint and matching `gmrelaydb` resource name. +Expected: PASS, including PostgreSQL 17 migration application, rejected direct required-link deletes, rejected moved links and session/player cascades, rejected publication with any future linked session, automatic unpublish with preserved `published_at` after final future reschedule, preserved public state after `past -> future -> past`, statement-level mutation locking before session and required-link rows, opposing-order batch reschedules without card deadlock, bounded `READ COMMITTED` publish/delete in both commit orders, existing-link and new-link publish/reschedule races, session-delete/reschedule serialization in both first-lock orders, and distinct-link deletion without deadlock, write-skew, or invalid public commit, rejected `REPEATABLE READ` triggered writes including both draft-delete versus publish commit orders and stale-snapshot final-future reschedules, successful parent-card and owning-group cascades, Discord identity scoping, and Compose/Aspire HTTP health gating with a non-proxied bot endpoint and matching `gmrelaydb` resource name. - [ ] **Step 7: Commit** diff --git a/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md b/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md index 083627f..e062d9f 100644 --- a/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md +++ b/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md @@ -79,9 +79,9 @@ CHECK (NOT is_public OR ( Application validation additionally requires at least one linked session, every linked session to be completed with `scheduled_at < now()`, and at least one linked GM before publishing because those requirements span child tables. Publishing locks the parent card, validates both required link sets, then sets `is_public = true` and `published_at = COALESCE(published_at, now())` so `published_at` remains the first-publication timestamp. Link replacement locks the parent card and unpublishes it before replacing required links. -Deferred database constraint triggers validate the same invariant at transaction commit after a card transitions to public, a session link is inserted, deleted, moved, or repointed, or a required master link is deleted or moved. They raise a check-violation error if a published card would commit without both required link sets or with any linked session where `scheduled_at >= now()`. Before checking state, each validator acquires the same transaction-level PostgreSQL advisory lock, `pg_advisory_xact_lock(20260530, 108)`. Portfolio publication writes are low volume, so this intentionally global lock serializes invariant validation with one lock order, prevents write-skew under the application default `READ COMMITTED` isolation level, and avoids multi-card deadlocks. PostgreSQL keeps a stale snapshot after waiting under `REPEATABLE READ` or `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those levels; callers must use `READ COMMITTED` for portfolio mutations. +Immediate statement triggers acquire one transaction-level PostgreSQL advisory lock, `pg_advisory_xact_lock(20260530, 108)`, before any invariant-affecting rows are changed: publication transitions and deletes, required-link edits, session deletes and scheduled-date changes, and parent deletes that can cascade into required links. Deferred database constraint triggers validate the same invariant at transaction commit after a card transitions to public, a session link is inserted, deleted, moved, or repointed, or a required master link is deleted or moved. They raise a check-violation error if a published card would commit without both required link sets or with any linked session where `scheduled_at >= now()`. Portfolio and schedule mutations are low volume, so this intentionally global lock establishes one advisory-lock then row-lock protocol, prevents write-skew under the application default `READ COMMITTED` isolation level, and avoids multi-card, card/advisory, and session/advisory deadlocks. PostgreSQL keeps a stale snapshot after waiting under `REPEATABLE READ` or `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those levels; callers must use `READ COMMITTED` for portfolio mutations. -A deferred `sessions.scheduled_at` trigger atomically unpublishes linked public cards when a completed session is finally rescheduled into the future, preserving the first `published_at`. Because deferred row triggers retain their event-time `NEW`, the trigger re-reads the final `sessions.scheduled_at` before acting. It rejects final-future reschedules outside `READ COMMITTED` with `0A000`, because the unpublish pass requires fresh statement snapshots. Under `READ COMMITTED`, it takes row locks for all cards linked to any final-future session in `portfolio_games.id` order, including committed drafts. It then acquires the publication advisory lock and unpublishes matching public cards in a guarded update with a fresh statement snapshot. Including drafts prevents a concurrent draft-to-public publication from validating against the pre-reschedule session snapshot and committing afterward. Taking the shared advisory lock after card rows, but before the guarded update, also serializes a previously invisible concurrent link-add publication without reintroducing the card/advisory lock inversion. Session mutation paths use `sessions` before linked `portfolio_games`; normal session-deletion handlers explicitly lock the target session row, unpublish linked cards in the same transaction, and only then delete the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted at `READ COMMITTED`, deferred validation sees no surviving published card and remains harmless. +A deferred `sessions.scheduled_at` trigger atomically unpublishes linked public cards when a completed session is finally rescheduled into the future, preserving the first `published_at`. Because deferred row triggers retain their event-time `NEW`, the trigger re-reads the final `sessions.scheduled_at` before acting. It rejects final-future reschedules outside `READ COMMITTED` with `0A000`, because the unpublish pass requires fresh statement snapshots. Under `READ COMMITTED`, it takes row locks for all cards linked to any final-future session in `portfolio_games.id` order, including committed drafts. It then re-acquires the publication advisory lock and unpublishes matching public cards in a guarded update with a fresh statement snapshot. Including drafts prevents a concurrent draft-to-public publication from validating against the pre-reschedule session snapshot and committing afterward. Session mutation paths use advisory-lock then `sessions` then linked `portfolio_games`; normal session-deletion handlers explicitly acquire the mutation lock, lock the target session row, unpublish linked cards in the same transaction, and only then delete the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted at `READ COMMITTED`, deferred validation sees no surviving published card and remains harmless. ### `portfolio_game_sessions` @@ -355,8 +355,8 @@ Follow TDD for production changes. ### Schema And Contracts -- Migration source-contract tests assert the four new tables, format constraint, publication guard, case-insensitive slug uniqueness, group and GM-profile indexes, card-oriented pending-review index, completed-session validator, deferred future-reschedule unpublish trigger, session-first deletion locks, and the AppHost HTTP health gate. -- PostgreSQL integration tests apply migrations V001 through V029 to `postgres:17-alpine` and cover direct invalid link removal, moved links, direct session/player cascades, explicit session-lock then unpublish then session deletion, delete/reschedule lock ordering in both first-lock orders, rejection of publication when any linked session is future, automatic unpublish with preserved `published_at` after future reschedule, `past -> future -> past` final-state handling, opposing-order batch future reschedules without card deadlock using an observed advisory test gate instead of timing sleeps, existing-link and new-link draft publication/reschedule races, both bounded publish/delete commit orders, concurrent removal of distinct required links without write-skew or deadlock under `READ COMMITTED`, rejection of equivalent `REPEATABLE READ` writes including both draft-delete versus publish commit orders and stale-snapshot final-future reschedules, and parent/card cascade deletion. +- Migration source-contract tests assert the four new tables, format constraint, publication guard, case-insensitive slug uniqueness, group and GM-profile indexes, card-oriented pending-review index, immediate statement-level mutation locks, completed-session validator, deferred future-reschedule unpublish trigger, advisory-lock then session-row deletion locks, and the AppHost HTTP health gate. +- PostgreSQL integration tests apply migrations V001 through V029 to `postgres:17-alpine` and cover direct invalid link removal, moved links, direct session/player cascades, explicit mutation-lock then session-lock then unpublish then session deletion, delete/reschedule mutation-gate ordering in both first-lock orders, rejection of publication when any linked session is future, automatic unpublish with preserved `published_at` after future reschedule, `past -> future -> past` final-state handling, required-link insertion and final-future reschedule mutation locks before rows, opposing-order batch future reschedules serialized before session rows, existing-link and new-link draft publication/reschedule races, both bounded publish/delete commit orders, concurrent removal of distinct required links without write-skew or deadlock under `READ COMMITTED`, rejection of equivalent `REPEATABLE READ` writes including both draft-delete versus publish commit orders and stale-snapshot final-future reschedules, and parent/card cascade deletion. - Public DTO reflection/source tests assert that private identifiers and physical storage paths are absent. - Existing showcase tests continue to assert the future-session catalog boundary. diff --git a/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql b/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql index 7e6334d..bd34c44 100644 --- a/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql +++ b/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql @@ -52,6 +52,46 @@ CREATE TABLE portfolio_game_masters ( CREATE INDEX ix_portfolio_game_masters_player ON portfolio_game_masters (player_id, portfolio_game_id); +CREATE FUNCTION lock_portfolio_publication_mutation() +RETURNS TRIGGER +LANGUAGE plpgsql +AS $$ +BEGIN + PERFORM pg_advisory_xact_lock(20260530, 108); + RETURN NULL; +END; +$$; + +CREATE TRIGGER trg_portfolio_games_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE OF is_public ON portfolio_games +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_portfolio_game_sessions_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_sessions +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_portfolio_game_masters_lock_publication_mutation +BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_masters +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_sessions_lock_portfolio_publication_mutation +BEFORE DELETE OR UPDATE OF scheduled_at ON sessions +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_game_groups_lock_portfolio_publication_mutation_before_delete +BEFORE DELETE ON game_groups +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + +CREATE TRIGGER trg_players_lock_portfolio_publication_mutation_before_delete +BEFORE DELETE ON players +FOR EACH STATEMENT +EXECUTE FUNCTION lock_portfolio_publication_mutation(); + CREATE FUNCTION validate_public_portfolio_game_required_links() RETURNS TRIGGER LANGUAGE plpgsql diff --git a/src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs b/src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs index 97c5f5c..046a1b7 100644 --- a/src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs +++ b/src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs @@ -45,6 +45,9 @@ public sealed class DiscordDeleteSessionHandler( } await using var transaction = await connection.BeginTransactionAsync(cancellationToken); + await connection.ExecuteAsync( + "SELECT pg_advisory_xact_lock(20260530, 108)", + transaction: transaction); _ = await connection.QuerySingleOrDefaultAsync( """ SELECT s.id diff --git a/src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs b/src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs index 34b9b3f..fd69fc5 100644 --- a/src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs +++ b/src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs @@ -31,7 +31,12 @@ public sealed class DeleteSessionHandler( await using var connection = await dataSource.OpenConnectionAsync(ct); await using var transaction = await connection.BeginTransactionAsync(ct); - // 1. Lock the session before any linked portfolio card and verify group manager. + // 1. Use the database mutation order before locking the session or linked portfolio cards. + await connection.ExecuteAsync( + "SELECT pg_advisory_xact_lock(20260530, 108)", + transaction: transaction); + + // 2. Lock the session before any linked portfolio card and verify group manager. var session = await connection.QuerySingleOrDefaultAsync( """ SELECT s.title AS Title, @@ -63,7 +68,7 @@ public sealed class DeleteSessionHandler( return new DeleteSessionResult(false, "Только owner или co-GM может удалять сессию.", null, null, null, false, 0); } - // 2. Unpublish a linked portfolio card before its required session link cascades away. + // 3. Unpublish a linked portfolio card before its required session link cascades away. await connection.ExecuteAsync( """ UPDATE portfolio_games pg @@ -77,7 +82,7 @@ public sealed class DeleteSessionHandler( new { command.SessionId }, transaction); - // 3. Delete session + // 4. Delete session await connection.ExecuteAsync("DELETE FROM sessions WHERE id = @Id", new { Id = command.SessionId }, transaction); var remainingInTopic = session.ThreadId.HasValue diff --git a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs index 2ae9621..27e5eac 100644 --- a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs +++ b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs @@ -96,51 +96,64 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi [Theory] [InlineData(true)] [InlineData(false)] - public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard(bool publishCommitsFirst) + public async Task ConcurrentPublishAndLinkDelete_ShouldSerializeBeforeRowsAndRejectInvalidPublicCard( + bool publishMutatesFirst) { var database = await fixture.CreateMigratedDatabaseAsync(); await using var publishConnection = await database.OpenConnectionAsync(); await using var deleteConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); var seed = await SeedCardAsync(publishConnection, isPublic: false); await using var publishTransaction = await publishConnection.BeginTransactionAsync(); await using var deleteTransaction = await deleteConnection.BeginTransactionAsync(); + var publishPid = await GetBackendPidAsync(publishConnection, publishTransaction); + var deletePid = await GetBackendPidAsync(deleteConnection, deleteTransaction); - await ExecuteNonQueryAsync( - publishConnection, - """ - UPDATE portfolio_games - SET is_public = true, - published_at = COALESCE(published_at, now()), - updated_at = now() - WHERE id = @portfolioGameId - """, - publishTransaction, - new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId)); - await ExecuteNonQueryAsync(deleteConnection, "SET LOCAL lock_timeout = '2s'", deleteTransaction); + if (publishMutatesFirst) + { + Assert.Equal(1, await PublishPortfolioGameAsync( + publishConnection, + publishTransaction, + seed.PortfolioGameId)); + var deleteTask = DeletePortfolioGameLinksAsync( + deleteConnection, + deleteTransaction, + "portfolio_game_sessions", + seed.PortfolioGameId); - Assert.Equal(1, await ExecuteNonQueryAsync( - deleteConnection, - "DELETE FROM portfolio_game_sessions WHERE portfolio_game_id = @portfolioGameId", - deleteTransaction, - new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId))); + await WaitUntilBlockedByAsync(observerConnection, deletePid, publishPid); + Assert.Null(await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await deleteTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.CheckViolation, + await CommitAndCaptureSqlStateAsync(deleteTransaction).WaitAsync(CommandTimeout)); + } + else + { + Assert.Equal(1, await DeletePortfolioGameLinksAsync( + deleteConnection, + deleteTransaction, + "portfolio_game_sessions", + seed.PortfolioGameId)); + var publishTask = PublishPortfolioGameAsync( + publishConnection, + publishTransaction, + seed.PortfolioGameId); - await AcquirePortfolioValidationLockAsync( - publishCommitsFirst ? publishConnection : deleteConnection, - publishCommitsFirst ? publishTransaction : deleteTransaction); - - var commitStates = await Task.WhenAll( - CommitAndCaptureSqlStateAsync(publishTransaction), - CommitAndCaptureSqlStateAsync(deleteTransaction)).WaitAsync(CommandTimeout); - - Assert.Equal(publishCommitsFirst ? null : PostgresErrorCodes.CheckViolation, commitStates[0]); - Assert.Equal(publishCommitsFirst ? PostgresErrorCodes.CheckViolation : null, commitStates[1]); + await WaitUntilBlockedByAsync(observerConnection, publishPid, deletePid); + Assert.Null(await CommitAndCaptureSqlStateAsync(deleteTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await publishTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.CheckViolation, + await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); + } await using var verificationConnection = await database.OpenConnectionAsync(); - Assert.Equal(publishCommitsFirst, await ExecuteScalarAsync( + Assert.Equal(publishMutatesFirst, await ExecuteScalarAsync( verificationConnection, "SELECT is_public FROM portfolio_games WHERE id = @portfolioGameId", parameters: new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId))); - Assert.Equal(publishCommitsFirst ? 1L : 0L, await ExecuteScalarAsync( + Assert.Equal(publishMutatesFirst ? 1L : 0L, await ExecuteScalarAsync( verificationConnection, "SELECT COUNT(*) FROM portfolio_game_sessions WHERE portfolio_game_id = @portfolioGameId", parameters: new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId))); @@ -162,8 +175,11 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi masterCount: linkTable == "portfolio_game_masters" ? 2 : 1); await using var firstConnection = await database.OpenConnectionAsync(); await using var secondConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); await using var firstTransaction = await firstConnection.BeginTransactionAsync(); await using var secondTransaction = await secondConnection.BeginTransactionAsync(); + var firstPid = await GetBackendPidAsync(firstConnection, firstTransaction); + var secondPid = await GetBackendPidAsync(secondConnection, secondTransaction); var linkIds = linkTable == "portfolio_game_sessions" ? seed.SessionIds : seed.MasterIds; await ExecuteNonQueryAsync( @@ -172,19 +188,19 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi firstTransaction, new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), new NpgsqlParameter("linkId", linkIds[0])); - await ExecuteNonQueryAsync( + var secondDeleteTask = ExecuteNonQueryAsync( secondConnection, $"DELETE FROM {linkTable} WHERE portfolio_game_id = @portfolioGameId AND {linkColumn} = @linkId", secondTransaction, new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), new NpgsqlParameter("linkId", linkIds[1])); - var commitStates = await Task.WhenAll( - CommitAndCaptureSqlStateAsync(firstTransaction), - CommitAndCaptureSqlStateAsync(secondTransaction)); - - Assert.Single(commitStates, state => state is null); - Assert.Single(commitStates, state => state == PostgresErrorCodes.CheckViolation); + await WaitUntilBlockedByAsync(observerConnection, secondPid, firstPid); + Assert.Null(await CommitAndCaptureSqlStateAsync(firstTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await secondDeleteTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.CheckViolation, + await CommitAndCaptureSqlStateAsync(secondTransaction).WaitAsync(CommandTimeout)); await using var verificationConnection = await database.OpenConnectionAsync(); Assert.True(await ExecuteScalarAsync( @@ -213,8 +229,11 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi masterCount: linkTable == "portfolio_game_masters" ? 2 : 1); await using var firstConnection = await database.OpenConnectionAsync(); await using var secondConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); await using var firstTransaction = await firstConnection.BeginTransactionAsync(IsolationLevel.RepeatableRead); await using var secondTransaction = await secondConnection.BeginTransactionAsync(IsolationLevel.RepeatableRead); + var firstPid = await GetBackendPidAsync(firstConnection, firstTransaction); + var secondPid = await GetBackendPidAsync(secondConnection, secondTransaction); var linkIds = linkTable == "portfolio_game_sessions" ? seed.SessionIds : seed.MasterIds; await ExecuteNonQueryAsync( @@ -223,18 +242,21 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi firstTransaction, new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), new NpgsqlParameter("linkId", linkIds[0])); - await ExecuteNonQueryAsync( + var secondDeleteTask = ExecuteNonQueryAsync( secondConnection, $"DELETE FROM {linkTable} WHERE portfolio_game_id = @portfolioGameId AND {linkColumn} = @linkId", secondTransaction, new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), new NpgsqlParameter("linkId", linkIds[1])); - var commitStates = await Task.WhenAll( - CommitAndCaptureSqlStateAsync(firstTransaction), - CommitAndCaptureSqlStateAsync(secondTransaction)); - - Assert.All(commitStates, state => Assert.Equal(PostgresErrorCodes.FeatureNotSupported, state)); + await WaitUntilBlockedByAsync(observerConnection, secondPid, firstPid); + Assert.Equal( + PostgresErrorCodes.FeatureNotSupported, + await CommitAndCaptureSqlStateAsync(firstTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await secondDeleteTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.FeatureNotSupported, + await CommitAndCaptureSqlStateAsync(secondTransaction).WaitAsync(CommandTimeout)); await using var verificationConnection = await database.OpenConnectionAsync(); Assert.True(await ExecuteScalarAsync( @@ -273,43 +295,57 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi [InlineData(true)] [InlineData(false)] public async Task RepeatableReadDraftLinkDeleteRacingPublish_ShouldBeRejectedWithoutInvalidPublicCard( - bool publishCommitsFirst) + bool deleteMutatesFirst) { var database = await fixture.CreateMigratedDatabaseAsync(); await using var seedConnection = await database.OpenConnectionAsync(); var seed = await SeedCardAsync(seedConnection, isPublic: false); await using var deleteConnection = await database.OpenConnectionAsync(); await using var publishConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); await using var deleteTransaction = await deleteConnection.BeginTransactionAsync(IsolationLevel.RepeatableRead); await using var publishTransaction = await publishConnection.BeginTransactionAsync(); + var deletePid = await GetBackendPidAsync(deleteConnection, deleteTransaction); + var publishPid = await GetBackendPidAsync(publishConnection, publishTransaction); - await ExecuteNonQueryAsync( - deleteConnection, - "DELETE FROM portfolio_game_sessions WHERE portfolio_game_id = @portfolioGameId", - deleteTransaction, - new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId)); - await ExecuteNonQueryAsync( - publishConnection, - """ - UPDATE portfolio_games - SET is_public = true, - published_at = COALESCE(published_at, now()), - updated_at = now() - WHERE id = @portfolioGameId - """, - publishTransaction, - new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId)); + if (deleteMutatesFirst) + { + Assert.Equal(1, await DeletePortfolioGameLinksAsync( + deleteConnection, + deleteTransaction, + "portfolio_game_sessions", + seed.PortfolioGameId)); + var publishTask = PublishPortfolioGameAsync( + publishConnection, + publishTransaction, + seed.PortfolioGameId); - await AcquirePortfolioValidationLockAsync( - publishCommitsFirst ? publishConnection : deleteConnection, - publishCommitsFirst ? publishTransaction : deleteTransaction); + await WaitUntilBlockedByAsync(observerConnection, publishPid, deletePid); + Assert.Equal( + PostgresErrorCodes.FeatureNotSupported, + await CommitAndCaptureSqlStateAsync(deleteTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await publishTask.WaitAsync(CommandTimeout)); + Assert.Null(await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); + } + else + { + Assert.Equal(1, await PublishPortfolioGameAsync( + publishConnection, + publishTransaction, + seed.PortfolioGameId)); + var deleteTask = DeletePortfolioGameLinksAsync( + deleteConnection, + deleteTransaction, + "portfolio_game_sessions", + seed.PortfolioGameId); - var commitStates = await Task.WhenAll( - CommitAndCaptureSqlStateAsync(deleteTransaction), - CommitAndCaptureSqlStateAsync(publishTransaction)).WaitAsync(CommandTimeout); - - Assert.Equal(PostgresErrorCodes.FeatureNotSupported, commitStates[0]); - Assert.Null(commitStates[1]); + await WaitUntilBlockedByAsync(observerConnection, deletePid, publishPid); + Assert.Null(await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(1, await deleteTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.FeatureNotSupported, + await CommitAndCaptureSqlStateAsync(deleteTransaction).WaitAsync(CommandTimeout)); + } await using var verificationConnection = await database.OpenConnectionAsync(); Assert.True(await ExecuteScalarAsync( @@ -376,74 +412,36 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi } [Fact] - public async Task ConcurrentBatchFutureReschedules_ShouldLockPublicCardsInStableOrderWithoutDeadlock() + public async Task ConcurrentBatchFutureReschedules_ShouldSerializeBeforeSessionRowsWithoutDeadlock() { var database = await fixture.CreateMigratedDatabaseAsync(); await using var seedConnection = await database.OpenConnectionAsync(); var firstSeed = await SeedCardAsync(seedConnection, isPublic: true, sessionCount: 2); var secondSeed = await SeedCardAsync(seedConnection, isPublic: true, sessionCount: 2); - await ExecuteNonQueryAsync( - seedConnection, - """ - CREATE FUNCTION wait_for_portfolio_card_unpublish_gate() - RETURNS TRIGGER - LANGUAGE plpgsql - AS $$ - BEGIN - PERFORM pg_advisory_xact_lock(20260601, 108); - RETURN NULL; - END; - $$; - - CREATE TRIGGER trg_wait_for_portfolio_card_unpublish_gate - AFTER UPDATE OF is_public ON portfolio_games - FOR EACH ROW - WHEN (OLD.is_public = true AND NEW.is_public = false) - EXECUTE FUNCTION wait_for_portfolio_card_unpublish_gate(); - """); - await using var firstConnection = await database.OpenConnectionAsync(); await using var secondConnection = await database.OpenConnectionAsync(); - await using var gateConnection = await database.OpenConnectionAsync(); await using var observerConnection = await database.OpenConnectionAsync(); await using var firstTransaction = await firstConnection.BeginTransactionAsync(); await using var secondTransaction = await secondConnection.BeginTransactionAsync(); - await using var gateTransaction = await gateConnection.BeginTransactionAsync(); var firstPid = await GetBackendPidAsync(firstConnection, firstTransaction); var secondPid = await GetBackendPidAsync(secondConnection, secondTransaction); - var gatePid = await GetBackendPidAsync(gateConnection, gateTransaction); - await AcquireBatchRescheduleGateAsync(gateConnection, gateTransaction); await RescheduleSessionsAsync( firstConnection, firstTransaction, firstSeed.SessionIds[0], secondSeed.SessionIds[0]); - await RescheduleSessionsAsync( + var secondRescheduleTask = RescheduleSessionsAsync( secondConnection, secondTransaction, secondSeed.SessionIds[1], firstSeed.SessionIds[1]); - var firstCommitTask = CommitAndCaptureSqlStateAsync(firstTransaction); - var secondCommitTask = CommitAndCaptureSqlStateAsync(secondTransaction); - var gateBlockedPid = await WaitUntilEitherBlockedByAsync( - observerConnection, - firstPid, - secondPid, - gatePid); - await WaitUntilBlockedByAnyAsync( - observerConnection, - gateBlockedPid == firstPid ? secondPid : firstPid, - gatePid, - gateBlockedPid); - - await gateTransaction.CommitAsync().WaitAsync(CommandTimeout); - - var commitStates = await Task.WhenAll(firstCommitTask, secondCommitTask).WaitAsync(CommandTimeout); - - Assert.All(commitStates, Assert.Null); + await WaitUntilBlockedByAsync(observerConnection, secondPid, firstPid); + Assert.Null(await CommitAndCaptureSqlStateAsync(firstTransaction).WaitAsync(CommandTimeout)); + Assert.Equal(2, await secondRescheduleTask.WaitAsync(CommandTimeout)); + Assert.Null(await CommitAndCaptureSqlStateAsync(secondTransaction).WaitAsync(CommandTimeout)); await using var verificationConnection = await database.OpenConnectionAsync(); Assert.Equal(0, await ExecuteScalarAsync( @@ -504,31 +502,19 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi var publishPid = await GetBackendPidAsync(publishConnection, publishTransaction); var reschedulePid = await GetBackendPidAsync(rescheduleConnection, rescheduleTransaction); - await ExecuteNonQueryAsync( + Assert.Equal(1, await PublishPortfolioGameAsync( publishConnection, - """ - UPDATE portfolio_games - SET is_public = true, - published_at = COALESCE(published_at, now()), - updated_at = now() - WHERE id = @portfolioGameId - """, publishTransaction, - new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId)); - await ExecuteNonQueryAsync( + seed.PortfolioGameId)); + var rescheduleTask = ExecuteNonQueryAsync( rescheduleConnection, "UPDATE sessions SET scheduled_at = now() + interval '1 day' WHERE id = @sessionId", rescheduleTransaction, new NpgsqlParameter("sessionId", seed.SessionIds[0])); - var forceRescheduleTriggerTask = ExecuteNonQueryAsync( - rescheduleConnection, - "SET CONSTRAINTS trg_sessions_unpublish_public_portfolio_games_for_future_reschedule IMMEDIATE", - rescheduleTransaction); await WaitUntilBlockedByAsync(observerConnection, reschedulePid, publishPid); - Assert.Null(await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); - await forceRescheduleTriggerTask.WaitAsync(CommandTimeout); + Assert.Equal(1, await rescheduleTask.WaitAsync(CommandTimeout)); await rescheduleTransaction.CommitAsync().WaitAsync(CommandTimeout); await using var verificationConnection = await database.OpenConnectionAsync(); @@ -563,15 +549,11 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi await using var rescheduleConnection = await database.OpenConnectionAsync(); await using var publishConnection = await database.OpenConnectionAsync(); - await using var gateConnection = await database.OpenConnectionAsync(); await using var observerConnection = await database.OpenConnectionAsync(); await using var rescheduleTransaction = await rescheduleConnection.BeginTransactionAsync(); await using var publishTransaction = await publishConnection.BeginTransactionAsync(); - await using var gateTransaction = await gateConnection.BeginTransactionAsync(); var reschedulePid = await GetBackendPidAsync(rescheduleConnection, rescheduleTransaction); var publishPid = await GetBackendPidAsync(publishConnection, publishTransaction); - var gatePid = await GetBackendPidAsync(gateConnection, gateTransaction); - await AcquirePortfolioValidationLockAsync(gateConnection, gateTransaction); Assert.Equal(1, await RescheduleSessionAsync( rescheduleConnection, @@ -581,9 +563,9 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi rescheduleConnection, "SET CONSTRAINTS trg_sessions_unpublish_public_portfolio_games_for_future_reschedule IMMEDIATE", rescheduleTransaction); - await WaitUntilBlockedByAsync(observerConnection, reschedulePid, gatePid); + await forceRescheduleTriggerTask.WaitAsync(CommandTimeout); - await ExecuteNonQueryAsync( + var publishMutationTask = ExecuteNonQueryAsync( publishConnection, """ INSERT INTO portfolio_game_sessions (portfolio_game_id, session_id) @@ -598,14 +580,13 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi publishTransaction, new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), new NpgsqlParameter("sessionId", rescheduledSessionId)); - var publishCommitTask = CommitAndCaptureSqlStateAsync(publishTransaction); - await WaitUntilBlockedByAsync(observerConnection, publishPid, gatePid); - - await gateTransaction.CommitAsync().WaitAsync(CommandTimeout); - await forceRescheduleTriggerTask.WaitAsync(CommandTimeout); + await WaitUntilBlockedByAsync(observerConnection, publishPid, reschedulePid); await rescheduleTransaction.CommitAsync().WaitAsync(CommandTimeout); + await publishMutationTask.WaitAsync(CommandTimeout); - Assert.Equal(PostgresErrorCodes.CheckViolation, await publishCommitTask.WaitAsync(CommandTimeout)); + Assert.Equal( + PostgresErrorCodes.CheckViolation, + await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); await using var verificationConnection = await database.OpenConnectionAsync(); Assert.False(await ExecuteScalarAsync( @@ -631,6 +612,78 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi ])); } + [Fact] + public async Task PortfolioSessionLinkInsert_ShouldAcquirePublicationLockBeforeRows() + { + var database = await fixture.CreateMigratedDatabaseAsync(); + await using var seedConnection = await database.OpenConnectionAsync(); + var seed = await SeedCardAsync(seedConnection, isPublic: false); + var sessionId = Guid.NewGuid(); + await ExecuteNonQueryAsync( + seedConnection, + """ + INSERT INTO sessions (id, group_id, title, join_link, scheduled_at) + VALUES (@sessionId, @groupId, 'Completed Session', 'https://example.test/session', now() - interval '1 day'); + """, + parameters: + [ + new NpgsqlParameter("sessionId", sessionId), + new NpgsqlParameter("groupId", seed.GroupId) + ]); + + await using var insertConnection = await database.OpenConnectionAsync(); + await using var gateConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); + await using var insertTransaction = await insertConnection.BeginTransactionAsync(); + await using var gateTransaction = await gateConnection.BeginTransactionAsync(); + var insertPid = await GetBackendPidAsync(insertConnection, insertTransaction); + var gatePid = await GetBackendPidAsync(gateConnection, gateTransaction); + await AcquirePortfolioValidationLockAsync(gateConnection, gateTransaction); + + var insertTask = ExecuteNonQueryAsync( + insertConnection, + """ + INSERT INTO portfolio_game_sessions (portfolio_game_id, session_id) + VALUES (@portfolioGameId, @sessionId); + """, + insertTransaction, + new NpgsqlParameter("portfolioGameId", seed.PortfolioGameId), + new NpgsqlParameter("sessionId", sessionId)); + + await WaitUntilBlockedByAsync(observerConnection, insertPid, gatePid); + await gateTransaction.CommitAsync().WaitAsync(CommandTimeout); + + Assert.Equal(1, await insertTask.WaitAsync(CommandTimeout)); + await insertTransaction.RollbackAsync().WaitAsync(CommandTimeout); + } + + [Fact] + public async Task FutureReschedule_ShouldAcquirePublicationLockBeforeSessionRows() + { + var database = await fixture.CreateMigratedDatabaseAsync(); + await using var seedConnection = await database.OpenConnectionAsync(); + var seed = await SeedCardAsync(seedConnection, isPublic: true); + await using var rescheduleConnection = await database.OpenConnectionAsync(); + await using var gateConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); + await using var rescheduleTransaction = await rescheduleConnection.BeginTransactionAsync(); + await using var gateTransaction = await gateConnection.BeginTransactionAsync(); + var reschedulePid = await GetBackendPidAsync(rescheduleConnection, rescheduleTransaction); + var gatePid = await GetBackendPidAsync(gateConnection, gateTransaction); + await AcquirePortfolioValidationLockAsync(gateConnection, gateTransaction); + + var rescheduleTask = RescheduleSessionAsync( + rescheduleConnection, + rescheduleTransaction, + seed.SessionIds[0]); + + await WaitUntilBlockedByAsync(observerConnection, reschedulePid, gatePid); + await gateTransaction.CommitAsync().WaitAsync(CommandTimeout); + + Assert.Equal(1, await rescheduleTask.WaitAsync(CommandTimeout)); + await rescheduleTransaction.RollbackAsync().WaitAsync(CommandTimeout); + } + [Fact] public async Task RepeatableReadStaleSnapshotFutureReschedule_ShouldBeRejectedWithoutInvalidPublicCard() { @@ -709,8 +762,8 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi [Theory] [InlineData(true)] [InlineData(false)] - public async Task ConcurrentSessionDeleteAndFutureReschedule_ShouldSerializeSessionBeforeCardWithoutDeadlock( - bool deleteLocksSessionFirst) + public async Task ConcurrentSessionDeleteAndFutureReschedule_ShouldSerializeMutationGateBeforeRowsWithoutDeadlock( + bool deleteMutatesFirst) { var database = await fixture.CreateMigratedDatabaseAsync(); await using var seedConnection = await database.OpenConnectionAsync(); @@ -723,8 +776,9 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi var deletePid = await GetBackendPidAsync(deleteConnection, deleteTransaction); var reschedulePid = await GetBackendPidAsync(rescheduleConnection, rescheduleTransaction); - if (deleteLocksSessionFirst) + if (deleteMutatesFirst) { + await AcquirePortfolioValidationLockAsync(deleteConnection, deleteTransaction); await LockSessionAsync(deleteConnection, deleteTransaction, seed.SessionIds[0]); var rescheduleTask = RescheduleSessionAsync( rescheduleConnection, @@ -967,16 +1021,6 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi transaction); } - private static Task AcquireBatchRescheduleGateAsync( - NpgsqlConnection connection, - NpgsqlTransaction transaction) - { - return ExecuteNonQueryAsync( - connection, - "SELECT pg_advisory_xact_lock(20260601, 108)", - transaction); - } - private static Task GetBackendPidAsync( NpgsqlConnection connection, NpgsqlTransaction transaction) @@ -996,6 +1040,37 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi new NpgsqlParameter("sessionId", sessionId)); } + private static Task PublishPortfolioGameAsync( + NpgsqlConnection connection, + NpgsqlTransaction transaction, + Guid portfolioGameId) + { + return ExecuteNonQueryAsync( + connection, + """ + UPDATE portfolio_games + SET is_public = true, + published_at = COALESCE(published_at, now()), + updated_at = now() + WHERE id = @portfolioGameId + """, + transaction, + new NpgsqlParameter("portfolioGameId", portfolioGameId)); + } + + private static Task DeletePortfolioGameLinksAsync( + NpgsqlConnection connection, + NpgsqlTransaction transaction, + string linkTable, + Guid portfolioGameId) + { + return ExecuteNonQueryAsync( + connection, + $"DELETE FROM {linkTable} WHERE portfolio_game_id = @portfolioGameId", + transaction, + new NpgsqlParameter("portfolioGameId", portfolioGameId)); + } + private static Task RescheduleSessionAsync( NpgsqlConnection connection, NpgsqlTransaction transaction, @@ -1036,6 +1111,7 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi Guid portfolioGameId, Guid sessionId) { + await AcquirePortfolioValidationLockAsync(connection, transaction); await LockSessionAsync(connection, transaction, sessionId); await UnpublishAndDeleteSessionAsync(connection, transaction, portfolioGameId, sessionId); await transaction.CommitAsync().WaitAsync(CommandTimeout); @@ -1091,74 +1167,6 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi $"PostgreSQL backend {blockedPid} was not blocked by backend {blockingPid} within {CommandTimeout}."); } - private static async Task WaitUntilEitherBlockedByAsync( - NpgsqlConnection observerConnection, - int firstBlockedPid, - int secondBlockedPid, - int blockingPid) - { - using var timeout = new CancellationTokenSource(CommandTimeout); - while (!timeout.IsCancellationRequested) - { - var blockedPid = await ExecuteScalarAsync( - observerConnection, - """ - SELECT CASE - WHEN @blockingPid = ANY (pg_blocking_pids(@firstBlockedPid)) THEN @firstBlockedPid - WHEN @blockingPid = ANY (pg_blocking_pids(@secondBlockedPid)) THEN @secondBlockedPid - ELSE 0 - END - """, - parameters: - [ - new NpgsqlParameter("firstBlockedPid", firstBlockedPid), - new NpgsqlParameter("secondBlockedPid", secondBlockedPid), - new NpgsqlParameter("blockingPid", blockingPid) - ]); - if (blockedPid != 0) - { - return blockedPid; - } - - await Task.Yield(); - } - - throw new TimeoutException( - $"Neither PostgreSQL backend {firstBlockedPid} nor {secondBlockedPid} was blocked by backend {blockingPid} within {CommandTimeout}."); - } - - private static async Task WaitUntilBlockedByAnyAsync( - NpgsqlConnection observerConnection, - int blockedPid, - int firstBlockingPid, - int secondBlockingPid) - { - using var timeout = new CancellationTokenSource(CommandTimeout); - while (!timeout.IsCancellationRequested) - { - if (await ExecuteScalarAsync( - observerConnection, - """ - SELECT @firstBlockingPid = ANY (pg_blocking_pids(@blockedPid)) - OR @secondBlockingPid = ANY (pg_blocking_pids(@blockedPid)) - """, - parameters: - [ - new NpgsqlParameter("blockedPid", blockedPid), - new NpgsqlParameter("firstBlockingPid", firstBlockingPid), - new NpgsqlParameter("secondBlockingPid", secondBlockingPid) - ])) - { - return; - } - - await Task.Yield(); - } - - throw new TimeoutException( - $"PostgreSQL backend {blockedPid} was not blocked by backend {firstBlockingPid} or {secondBlockingPid} within {CommandTimeout}."); - } - private static async Task ExecuteNonQueryAsync( NpgsqlConnection connection, string sql, diff --git a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs index bd6ad29..28f42fa 100644 --- a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs +++ b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs @@ -36,6 +36,13 @@ public sealed class PortfolioMigrationTests Assert.Contains("CREATE INDEX ix_portfolio_game_reviews_moderator ON portfolio_game_reviews (moderated_by_player_id) WHERE moderated_by_player_id IS NOT NULL;", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE INDEX ix_portfolio_game_reviews_public ON portfolio_game_reviews (portfolio_game_id, created_at DESC) WHERE moderation_status = 'Approved' AND publication_consent_at IS NOT NULL;", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE INDEX ix_portfolio_game_reviews_pending ON portfolio_game_reviews (portfolio_game_id, created_at DESC) WHERE moderation_status = 'Pending';", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE FUNCTION lock_portfolio_publication_mutation() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_games_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE OF is_public ON portfolio_games FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_game_sessions_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_sessions FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_portfolio_game_masters_lock_publication_mutation BEFORE INSERT OR DELETE OR UPDATE ON portfolio_game_masters FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_sessions_lock_portfolio_publication_mutation BEFORE DELETE OR UPDATE OF scheduled_at ON sessions FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_game_groups_lock_portfolio_publication_mutation_before_delete BEFORE DELETE ON game_groups FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("CREATE TRIGGER trg_players_lock_portfolio_publication_mutation_before_delete BEFORE DELETE ON players FOR EACH STATEMENT EXECUTE FUNCTION lock_portfolio_publication_mutation();", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE FUNCTION validate_public_portfolio_game_required_links() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal); Assert.Contains("PERFORM pg_advisory_xact_lock(20260530, 108);", normalizedMigration, StringComparison.Ordinal); Assert.Contains("current_setting('transaction_isolation') <> 'read committed'", normalizedMigration, StringComparison.Ordinal); diff --git a/tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs b/tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs index f621caf..f480e1d 100644 --- a/tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs +++ b/tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs @@ -10,11 +10,18 @@ public sealed class PortfolioSessionDeletionSourceTests const string sessionLock = "FROM sessions s WHERE s.id = @SessionId FOR UPDATE OF s"; + const string advisoryLock = + "SELECT pg_advisory_xact_lock(20260530, 108)"; const string unpublish = "UPDATE portfolio_games pg SET is_public = false, updated_at = now() FROM portfolio_game_sessions pgs WHERE pgs.portfolio_game_id = pg.id AND pgs.session_id = @SessionId AND pg.is_public = true"; + Assert.Contains(advisoryLock, source, StringComparison.Ordinal); Assert.Contains(sessionLock, source, StringComparison.Ordinal); Assert.Contains(unpublish, source, StringComparison.Ordinal); + Assert.True( + source.IndexOf(advisoryLock, StringComparison.Ordinal) < + source.IndexOf(sessionLock, StringComparison.Ordinal), + "The shared delete path must acquire the portfolio advisory lock before locking the session."); Assert.True( source.IndexOf(sessionLock, StringComparison.Ordinal) < source.IndexOf(unpublish, StringComparison.Ordinal), @@ -33,12 +40,19 @@ public sealed class PortfolioSessionDeletionSourceTests const string sessionLock = "SELECT s.id FROM sessions s JOIN game_groups g ON g.id = s.group_id WHERE s.id = @SessionId AND g.platform = 'Discord' AND g.external_group_id = @GuildId FOR UPDATE OF s"; + const string advisoryLock = + "SELECT pg_advisory_xact_lock(20260530, 108)"; const string unpublish = "UPDATE portfolio_games pg SET is_public = false, updated_at = now() FROM portfolio_game_sessions pgs JOIN sessions s ON s.id = pgs.session_id JOIN game_groups g ON g.id = s.group_id WHERE pgs.portfolio_game_id = pg.id AND s.id = @SessionId AND g.platform = 'Discord' AND g.external_group_id = @GuildId AND pg.is_public = true"; + Assert.Contains(advisoryLock, source, StringComparison.Ordinal); Assert.Contains(sessionLock, source, StringComparison.Ordinal); Assert.Contains(unpublish, source, StringComparison.Ordinal); Assert.Contains("AND p.platform = 'Discord'", source, StringComparison.Ordinal); + Assert.True( + source.IndexOf(advisoryLock, StringComparison.Ordinal) < + source.IndexOf(sessionLock, StringComparison.Ordinal), + "The Discord delete path must acquire the portfolio advisory lock before locking the guild-scoped session."); Assert.True( source.IndexOf(sessionLock, StringComparison.Ordinal) < source.IndexOf(unpublish, StringComparison.Ordinal),