fix(data): enforce completed portfolio sessions
This commit is contained in:
@@ -45,6 +45,7 @@
|
||||
|
||||
- `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`
|
||||
- `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`
|
||||
- `src/GmRelay.AppHost/Program.cs`
|
||||
- `tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj`
|
||||
- `tests/GmRelay.Bot.Tests/packages.lock.json`
|
||||
- `src/GmRelay.Web/Program.cs`
|
||||
@@ -76,6 +77,7 @@
|
||||
- Create: `src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql`
|
||||
- Modify: `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`
|
||||
- Modify: `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`
|
||||
- Modify: `src/GmRelay.AppHost/Program.cs`
|
||||
- Modify: `tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj`
|
||||
- Modify: `tests/GmRelay.Bot.Tests/packages.lock.json`
|
||||
- Modify: `compose.yaml`
|
||||
@@ -112,10 +114,13 @@ public async Task MigrationV029_ShouldCreatePortfolioTablesAndPublicationGuards(
|
||||
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("s.scheduled_at >= now()", 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);
|
||||
Assert.Contains("CREATE CONSTRAINT TRIGGER trg_portfolio_game_sessions_validate_required_links AFTER INSERT OR DELETE OR UPDATE OF portfolio_game_id, session_id ON portfolio_game_sessions DEFERRABLE INITIALLY DEFERRED", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("CREATE CONSTRAINT TRIGGER trg_portfolio_game_masters_validate_required_links AFTER DELETE OR UPDATE OF portfolio_game_id ON portfolio_game_masters DEFERRABLE INITIALLY DEFERRED", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("CREATE FUNCTION unpublish_public_portfolio_games_for_future_session() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.Contains("CREATE CONSTRAINT TRIGGER trg_sessions_unpublish_public_portfolio_games_for_future_reschedule AFTER UPDATE OF scheduled_at ON sessions DEFERRABLE INITIALLY DEFERRED", normalizedMigration, StringComparison.Ordinal);
|
||||
Assert.DoesNotContain("FOR UPDATE", normalizedMigration, StringComparison.Ordinal);
|
||||
}
|
||||
```
|
||||
@@ -170,7 +175,7 @@ public async Task DiscordDeleteSessionHandler_ShouldUnpublishOnlyCardsFromTheInt
|
||||
}
|
||||
```
|
||||
|
||||
Add `PortfolioSchemaGateSourceTests.cs` and assert that both the `discord` and `web` Compose services depend on a healthy `bot`. The Telegram bot runs `DbMigrator` synchronously before exposing a healthy endpoint, so this dependency is the migration-first schema gate.
|
||||
Add `PortfolioSchemaGateSourceTests.cs` and assert that both the `discord` and `web` Compose services depend on a healthy `bot`. Assert the same schema gate in Aspire: save the `bot` project resource to a variable and make the `discord` and `web` project resources call `.WaitFor(bot)` in addition to `.WaitFor(postgres)`. The Telegram bot runs `DbMigrator` synchronously before exposing a healthy endpoint, so this dependency is the migration-first schema gate.
|
||||
|
||||
- [ ] **Step 2: Add the failing PostgreSQL Testcontainers integration fixture and tests**
|
||||
|
||||
@@ -202,8 +207,10 @@ public async Task DirectRequiredLinkDeletion_ShouldFailCommitForPublishedCard(st
|
||||
[Fact]
|
||||
public async Task ExplicitUnpublishThenSessionDelete_ShouldCommitAndPreserveFirstPublishedAt()
|
||||
|
||||
[Fact]
|
||||
public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard()
|
||||
[Theory]
|
||||
[InlineData(true)]
|
||||
[InlineData(false)]
|
||||
public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard(bool publishCommitsFirst)
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions", "session_id")]
|
||||
@@ -215,8 +222,19 @@ public async Task ConcurrentRequiredLinkDeletes_ShouldSerializeAndRejectInvalidP
|
||||
[InlineData("portfolio_game_masters", "player_id")]
|
||||
public async Task RepeatableReadConcurrentRequiredLinkDeletes_ShouldBeRejectedWithoutInvalidPublicCard(string linkTable, string linkColumn)
|
||||
|
||||
[Theory]
|
||||
[InlineData(true)]
|
||||
[InlineData(false)]
|
||||
public async Task RepeatableReadDraftLinkDeleteRacingPublish_ShouldBeRejectedWithoutInvalidPublicCard(bool publishCommitsFirst)
|
||||
|
||||
[Fact]
|
||||
public async Task RepeatableReadDraftLinkDeleteRacingPublish_ShouldBeRejectedWithoutInvalidPublicCard()
|
||||
public async Task PublishedCardFutureReschedule_ShouldAutomaticallyUnpublishAndPreserveFirstPublishedAt()
|
||||
|
||||
[Fact]
|
||||
public async Task PublishingDraftCardWithAnyFutureLinkedSession_ShouldFailCommit()
|
||||
|
||||
[Fact]
|
||||
public async Task ConcurrentPublishAndFutureReschedule_ShouldNotDeadlockOrCommitInvalidPublicCard()
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions")]
|
||||
@@ -232,7 +250,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 `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 `REPEATABLE READ` scenarios must reject triggered portfolio writes with `0A000`, including draft-link deletion racing with 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 future reschedule must atomically unpublish a linked public card while preserving its first `published_at`. The `READ COMMITTED` concurrency scenarios must launch bounded commit tasks together, cover both publish/delete lock orders, and prove there is no deadlock, write-skew, or invalid public commit. The publish/reschedule race 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, 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**
|
||||
|
||||
@@ -242,7 +260,7 @@ Run:
|
||||
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests|FullyQualifiedName~PortfolioSchemaGateSourceTests"
|
||||
```
|
||||
|
||||
Expected: FAIL because `V029__add_completed_game_portfolios_and_reviews.sql` does not exist and the session-deletion handlers do not explicitly unpublish linked portfolio cards before deleting sessions.
|
||||
Expected during the Task 1 quality-review fix: FAIL because V029 does not yet validate completed linked sessions or automatically unpublish on future reschedule, and the Aspire AppHost does not yet gate `discord` and `web` on `bot`.
|
||||
|
||||
- [ ] **Step 4: Add migration V029**
|
||||
|
||||
@@ -304,13 +322,18 @@ LANGUAGE plpgsql
|
||||
AS $$
|
||||
DECLARE
|
||||
target_portfolio_game_id UUID;
|
||||
target_portfolio_game_ids UUID[];
|
||||
BEGIN
|
||||
PERFORM pg_advisory_xact_lock(20260530, 108);
|
||||
|
||||
IF TG_TABLE_NAME = 'portfolio_games' THEN
|
||||
target_portfolio_game_id := NEW.id;
|
||||
target_portfolio_game_ids := ARRAY[NEW.id];
|
||||
ELSIF TG_OP = 'DELETE' THEN
|
||||
target_portfolio_game_ids := ARRAY[OLD.portfolio_game_id];
|
||||
ELSIF TG_OP = 'INSERT' THEN
|
||||
target_portfolio_game_ids := ARRAY[NEW.portfolio_game_id];
|
||||
ELSE
|
||||
target_portfolio_game_id := OLD.portfolio_game_id;
|
||||
target_portfolio_game_ids := ARRAY[OLD.portfolio_game_id, NEW.portfolio_game_id];
|
||||
END IF;
|
||||
|
||||
IF current_setting('transaction_isolation') <> 'read committed' THEN
|
||||
@@ -319,24 +342,33 @@ BEGIN
|
||||
USING ERRCODE = '0A000';
|
||||
END IF;
|
||||
|
||||
IF EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_games pg
|
||||
WHERE pg.id = target_portfolio_game_id
|
||||
AND pg.is_public = true
|
||||
AND (
|
||||
NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_game_sessions pgs
|
||||
WHERE pgs.portfolio_game_id = target_portfolio_game_id
|
||||
)
|
||||
OR NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_game_masters pgm
|
||||
WHERE pgm.portfolio_game_id = target_portfolio_game_id
|
||||
)
|
||||
SELECT pg.id
|
||||
INTO target_portfolio_game_id
|
||||
FROM portfolio_games pg
|
||||
WHERE pg.id = ANY(target_portfolio_game_ids)
|
||||
AND pg.is_public = true
|
||||
AND (
|
||||
NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_game_sessions pgs
|
||||
WHERE pgs.portfolio_game_id = pg.id
|
||||
)
|
||||
) THEN
|
||||
OR EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_game_sessions pgs
|
||||
JOIN sessions s ON s.id = pgs.session_id
|
||||
WHERE pgs.portfolio_game_id = pg.id
|
||||
AND s.scheduled_at >= now()
|
||||
)
|
||||
OR NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM portfolio_game_masters pgm
|
||||
WHERE pgm.portfolio_game_id = pg.id
|
||||
)
|
||||
)
|
||||
LIMIT 1;
|
||||
|
||||
IF target_portfolio_game_id IS NOT NULL THEN
|
||||
RAISE EXCEPTION
|
||||
'published portfolio game % must have at least one linked session and at least one linked master',
|
||||
target_portfolio_game_id
|
||||
@@ -347,6 +379,32 @@ BEGIN
|
||||
END;
|
||||
$$;
|
||||
|
||||
CREATE FUNCTION unpublish_public_portfolio_games_for_future_session()
|
||||
RETURNS TRIGGER
|
||||
LANGUAGE plpgsql
|
||||
AS $$
|
||||
BEGIN
|
||||
IF OLD.scheduled_at IS DISTINCT FROM NEW.scheduled_at
|
||||
AND NEW.scheduled_at >= now() THEN
|
||||
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 = NEW.id
|
||||
AND pg.is_public = true;
|
||||
END IF;
|
||||
|
||||
RETURN NULL;
|
||||
END;
|
||||
$$;
|
||||
|
||||
CREATE CONSTRAINT TRIGGER trg_sessions_unpublish_public_portfolio_games_for_future_reschedule
|
||||
AFTER UPDATE OF scheduled_at ON sessions
|
||||
DEFERRABLE INITIALLY DEFERRED
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION unpublish_public_portfolio_games_for_future_session();
|
||||
|
||||
CREATE CONSTRAINT TRIGGER trg_portfolio_games_validate_required_links
|
||||
AFTER INSERT OR UPDATE OF is_public ON portfolio_games
|
||||
DEFERRABLE INITIALLY DEFERRED
|
||||
@@ -354,7 +412,7 @@ FOR EACH ROW
|
||||
EXECUTE FUNCTION validate_public_portfolio_game_required_links();
|
||||
|
||||
CREATE CONSTRAINT TRIGGER trg_portfolio_game_sessions_validate_required_links
|
||||
AFTER DELETE OR UPDATE OF portfolio_game_id ON portfolio_game_sessions
|
||||
AFTER INSERT OR DELETE OR UPDATE OF portfolio_game_id, session_id ON portfolio_game_sessions
|
||||
DEFERRABLE INITIALLY DEFERRED
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION validate_public_portfolio_game_required_links();
|
||||
@@ -397,7 +455,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: 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`. Child delete triggers do not lock or update the parent card. At `READ COMMITTED`, draft edits, explicit unpublishing, and card or club cascade deletion remain valid. Normal session-deletion handlers explicitly unpublish linked cards before deleting sessions.
|
||||
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 atomically unpublishes linked public cards while preserving `published_at`; it updates the card before validator lock acquisition so a racing publication cannot create an inverted lock order. At `READ COMMITTED`, draft edits, explicit unpublishing, future reschedules, 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**
|
||||
|
||||
@@ -433,7 +491,7 @@ Both handlers deliberately unpublish before session deletion. This keeps normal
|
||||
|
||||
Also add `AND p.platform = 'Discord'` to the Discord manager lookup before casting manager IDs, so cross-platform identities cannot affect authorization.
|
||||
|
||||
In `compose.yaml`, make both `discord` and `web` depend on a healthy `bot` in addition to the healthy database. `DbMigrator` runs synchronously before the bot health endpoint starts, so this gates consumers on V029 without duplicating the migrator.
|
||||
In `compose.yaml`, make both `discord` and `web` depend on a healthy `bot` in addition to the healthy database. Mirror the same schema gate in `src/GmRelay.AppHost/Program.cs`: save the `bot` project resource and add `.WaitFor(bot)` to both `discord` and `web` after `.WaitFor(postgres)`. `DbMigrator` runs synchronously before the bot health endpoint starts, so this gates consumers on V029 without duplicating the migrator.
|
||||
|
||||
- [ ] **Step 6: Run the Task 1 tests to verify GREEN**
|
||||
|
||||
@@ -443,12 +501,12 @@ 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 `READ COMMITTED` concurrent publish/delete and distinct-link deletion without deadlock, write-skew, or invalid public commit, rejected `REPEATABLE READ` triggered writes including draft-delete versus publish races, 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, rejected publication with any future linked session, automatic unpublish with preserved `published_at` after future reschedule, bounded `READ COMMITTED` publish/delete in both commit orders, publish/reschedule races, 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, successful parent-card and owning-group cascades, Discord identity scoping, and Compose/Aspire schema gating.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```powershell
|
||||
git add src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs compose.yaml tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj tests/GmRelay.Bot.Tests/packages.lock.json tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioSchemaGateSourceTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresFixture.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs
|
||||
git add src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs src/GmRelay.AppHost/Program.cs compose.yaml tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj tests/GmRelay.Bot.Tests/packages.lock.json tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioSchemaGateSourceTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresFixture.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs
|
||||
git commit -m "fix(data): harden portfolio publication concurrency"
|
||||
```
|
||||
|
||||
@@ -938,7 +996,7 @@ Rules:
|
||||
- Update runs in one transaction, locks the portfolio row, updates scalar fields, unpublishes the card before replacing required child links, replaces child links, rejects cross-club or future sessions, and accepts only managers from the same club.
|
||||
- Cover replacement returns the prior storage key after the database update.
|
||||
- Delete returns the cover key after deleting the row.
|
||||
- Publishing locks the row and verifies slug, description, cover key, one or more linked past sessions, and one or more masters before setting `is_public = true` and `published_at = COALESCE(published_at, now())`. The deferred database guard is a backstop for direct SQL and concurrent changes.
|
||||
- Publishing locks the row and verifies slug, description, cover key, one or more linked sessions, every linked session has `scheduled_at < now()`, and one or more masters before setting `is_public = true` and `published_at = COALESCE(published_at, now())`. The deferred database guard is a backstop for direct SQL and concurrent changes.
|
||||
- Unpublishing only sets `is_public = false`.
|
||||
- Moderation accepts `Approved`, `Rejected`, or `Hidden`, stores moderator ID and timestamp, and scopes the review to the managed adventure.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user