fix(data): enforce portfolio validation isolation
This commit is contained in:
@@ -110,6 +110,8 @@ public async Task MigrationV029_ShouldCreatePortfolioTablesAndPublicationGuards(
|
||||
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 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);
|
||||
Assert.Contains("USING ERRCODE = '0A000';", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("USING ERRCODE = '23514';", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("CREATE CONSTRAINT TRIGGER trg_portfolio_games_validate_required_links AFTER INSERT OR UPDATE OF is_public ON portfolio_games DEFERRABLE INITIALLY DEFERRED", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("CREATE CONSTRAINT TRIGGER trg_portfolio_game_sessions_validate_required_links AFTER DELETE OR UPDATE OF portfolio_game_id ON portfolio_game_sessions DEFERRABLE INITIALLY DEFERRED", normalizedMigration, StringComparison.Ordinal);
|
||||
@@ -208,6 +210,11 @@ public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvali
|
||||
[InlineData("portfolio_game_masters", "player_id")]
|
||||
public async Task ConcurrentRequiredLinkDeletes_ShouldSerializeAndRejectInvalidPublicCard(string linkTable, string linkColumn)
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions", "session_id")]
|
||||
[InlineData("portfolio_game_masters", "player_id")]
|
||||
public async Task RepeatableReadConcurrentRequiredLinkDeletes_ShouldBeRejectedWithoutInvalidPublicCard(string linkTable, string linkColumn)
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions")]
|
||||
[InlineData("portfolio_game_masters")]
|
||||
@@ -222,7 +229,7 @@ public async Task RequiredParentCascadeDelete_ShouldFailCommitForPublishedCard(s
|
||||
public async Task ParentCardAndGroupCascadeDeletes_ShouldCommit()
|
||||
```
|
||||
|
||||
The direct-delete, moved-link, and direct parent-cascade theories must expect PostgreSQL `23514` at commit. The explicit-unpublish scenario must delete the session successfully while preserving the first `published_at`. The concurrency scenarios must bound commits with timeouts, prove there is no deadlock or write-skew, and prove that an invalid public card cannot commit. The parent-card and owning-group cascade scenarios must commit successfully.
|
||||
The direct-delete, moved-link, and direct parent-cascade theories must expect PostgreSQL `23514` at commit. The explicit-unpublish scenario must delete the session successfully while preserving the first `published_at`. The `READ COMMITTED` concurrency scenarios must bound commits with timeouts, prove there is no deadlock or write-skew, and prove that an invalid public card cannot commit. The equivalent `REPEATABLE READ` scenario must reject both published-card transactions with `0A000`, 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**
|
||||
|
||||
@@ -303,6 +310,18 @@ BEGIN
|
||||
target_portfolio_game_id := OLD.portfolio_game_id;
|
||||
END IF;
|
||||
|
||||
IF current_setting('transaction_isolation') <> 'read committed'
|
||||
AND EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_games pg
|
||||
WHERE pg.id = target_portfolio_game_id
|
||||
AND pg.is_public = true
|
||||
) THEN
|
||||
RAISE EXCEPTION
|
||||
'portfolio publication validation requires read committed isolation'
|
||||
USING ERRCODE = '0A000';
|
||||
END IF;
|
||||
|
||||
IF EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_games pg
|
||||
@@ -381,7 +400,7 @@ 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 they acquire the same transaction-level advisory lock and reject a surviving published card when either required link set is empty. The intentionally global lock is appropriate for low-volume portfolio publication writes: it serializes validation, prevents write-skew across distinct child links, and gives multi-card transactions one lock order. Child delete triggers do not lock or update the parent card. Normal session-deletion handlers explicitly unpublish linked cards before deleting sessions. Card and club cascade deletion remain harmless because no published parent survives validation.
|
||||
The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. At transaction commit they acquire the same transaction-level advisory lock and reject a surviving published card when either required link set is empty. 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 publication-related writes at those isolation levels with `0A000`. Child delete triggers do not lock or update the parent card. Draft edits, explicit unpublishing, and card or club cascade deletion remain valid. Normal session-deletion handlers explicitly unpublish linked cards before deleting sessions.
|
||||
|
||||
- [ ] **Step 5: Explicitly unpublish linked cards in both session-deletion handlers**
|
||||
|
||||
@@ -427,7 +446,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, successful explicit unpublish plus session delete with preserved `published_at`, bounded concurrent publish/delete and distinct-link deletion without deadlock, write-skew, or invalid public commit, successful parent-card and owning-group cascades, Discord identity scoping, and Compose schema gating.
|
||||
Expected: PASS, including PostgreSQL 17 migration application, rejected direct required-link deletes, rejected moved links and session/player cascades, successful explicit unpublish plus session delete with preserved `published_at`, bounded `READ COMMITTED` concurrent publish/delete and distinct-link deletion without deadlock, write-skew, or invalid public commit, rejected `REPEATABLE READ` published-card writes, successful parent-card and owning-group cascades, Discord identity scoping, and Compose schema gating.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ CHECK (NOT is_public OR (
|
||||
|
||||
Application validation additionally requires at least one linked completed session 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 or a required session/master link is deleted or moved. They raise a check-violation error if a published card would commit without both required link sets. Before checking state, each trigger 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 when concurrent transactions remove different links, and avoids multi-card deadlocks. The deferred guard is a database backstop and deliberately does not lock or update a parent row from a child delete trigger. Normal session-deletion handlers explicitly unpublish linked cards in the same transaction before deleting the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted, deferred validation sees no surviving published card and remains harmless.
|
||||
Deferred database constraint triggers validate the same invariant at transaction commit after a card transitions to public or a required session/master link is deleted or moved. They raise a check-violation error if a published card would commit without both required link sets. Before checking state, each trigger 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 any transaction at those levels that would leave a published card; callers must use `READ COMMITTED` for publication-related writes. Draft changes, explicit unpublishing, and parent-card or club cascade deletion remain allowed. The deferred guard is a database backstop and deliberately does not lock or update a parent row from a child delete trigger. Normal session-deletion handlers explicitly unpublish linked cards in the same transaction before deleting the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted, deferred validation sees no surviving published card and remains harmless.
|
||||
|
||||
### `portfolio_game_sessions`
|
||||
|
||||
@@ -354,7 +354,7 @@ 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, and deferred constraint-trigger backstop.
|
||||
- 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 unpublish before session deletion, concurrent publish/delete ordering, concurrent removal of distinct required links without write-skew or deadlock, and parent/card cascade deletion.
|
||||
- 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 unpublish before session deletion, concurrent publish/delete ordering, concurrent removal of distinct required links without write-skew or deadlock under `READ COMMITTED`, rejection of equivalent `REPEATABLE READ` writes, 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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user