fix(data): serialize portfolio mutations before rows
This commit is contained in:
@@ -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**
|
||||
|
||||
|
||||
Reference in New Issue
Block a user