fix(data): serialize portfolio publication validation
This commit is contained in:
@@ -30,6 +30,7 @@
|
||||
- `src/GmRelay.Web/Components/Pages/PublicPortfolio.razor`
|
||||
- `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`
|
||||
- `tests/GmRelay.Bot.Tests/Web/PortfolioContractsTests.cs`
|
||||
@@ -69,6 +70,7 @@
|
||||
**Files:**
|
||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs`
|
||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs`
|
||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioSchemaGateSourceTests.cs`
|
||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresFixture.cs`
|
||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs`
|
||||
- Create: `src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql`
|
||||
@@ -76,6 +78,7 @@
|
||||
- Modify: `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`
|
||||
- Modify: `tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj`
|
||||
- Modify: `tests/GmRelay.Bot.Tests/packages.lock.json`
|
||||
- Modify: `compose.yaml`
|
||||
|
||||
- [ ] **Step 1: Write the failing migration and session-deletion source-contract tests**
|
||||
|
||||
@@ -106,6 +109,7 @@ public async Task MigrationV029_ShouldCreatePortfolioTablesAndPublicationGuards(
|
||||
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 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("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);
|
||||
@@ -157,12 +161,15 @@ public async Task DiscordDeleteSessionHandler_ShouldUnpublishOnlyCardsFromTheInt
|
||||
"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(unpublish, source, StringComparison.Ordinal);
|
||||
Assert.Contains("AND p.platform = 'Discord'", source, StringComparison.Ordinal);
|
||||
Assert.True(
|
||||
source.IndexOf(unpublish, StringComparison.Ordinal) <
|
||||
source.IndexOf("DELETE FROM sessions s", StringComparison.Ordinal));
|
||||
}
|
||||
```
|
||||
|
||||
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.
|
||||
|
||||
- [ ] **Step 2: Add the failing PostgreSQL Testcontainers integration fixture and tests**
|
||||
|
||||
Add the package reference:
|
||||
@@ -196,18 +203,33 @@ public async Task ExplicitUnpublishThenSessionDelete_ShouldCommitAndPreserveFirs
|
||||
[Fact]
|
||||
public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard()
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions", "session_id")]
|
||||
[InlineData("portfolio_game_masters", "player_id")]
|
||||
public async Task ConcurrentRequiredLinkDeletes_ShouldSerializeAndRejectInvalidPublicCard(string linkTable, string linkColumn)
|
||||
|
||||
[Theory]
|
||||
[InlineData("portfolio_game_sessions")]
|
||||
[InlineData("portfolio_game_masters")]
|
||||
public async Task MovingLastRequiredLinkAway_ShouldFailCommitForPublishedCard(string linkTable)
|
||||
|
||||
[Theory]
|
||||
[InlineData("sessions")]
|
||||
[InlineData("players")]
|
||||
public async Task RequiredParentCascadeDelete_ShouldFailCommitForPublishedCard(string parentTable)
|
||||
|
||||
[Fact]
|
||||
public async Task ParentCardAndGroupCascadeDeletes_ShouldCommit()
|
||||
```
|
||||
|
||||
The direct-delete theory must expect PostgreSQL `23514` at commit for each required-link table. The explicit-unpublish scenario must delete the session successfully while preserving the first `published_at`. The concurrency scenario must bound both commits with timeouts, prove there is no deadlock, 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 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.
|
||||
|
||||
- [ ] **Step 3: Run the Task 1 tests to verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```powershell
|
||||
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests"
|
||||
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.
|
||||
@@ -273,6 +295,8 @@ AS $$
|
||||
DECLARE
|
||||
target_portfolio_game_id UUID;
|
||||
BEGIN
|
||||
PERFORM pg_advisory_xact_lock(20260530, 108);
|
||||
|
||||
IF TG_TABLE_NAME = 'portfolio_games' THEN
|
||||
target_portfolio_game_id := NEW.id;
|
||||
ELSE
|
||||
@@ -357,7 +381,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 reject a surviving published card when either required link set is empty. Child delete triggers do not lock or update the parent card, avoiding reverse lock order. 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: 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.
|
||||
|
||||
- [ ] **Step 5: Explicitly unpublish linked cards in both session-deletion handlers**
|
||||
|
||||
@@ -391,20 +415,24 @@ WHERE pgs.portfolio_game_id = pg.id
|
||||
|
||||
Both handlers deliberately unpublish before session deletion. This keeps normal deletes successful, preserves the first-publication `published_at`, and leaves the deferred trigger 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.
|
||||
|
||||
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.
|
||||
|
||||
- [ ] **Step 6: Run the Task 1 tests to verify GREEN**
|
||||
|
||||
Run:
|
||||
|
||||
```powershell
|
||||
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests"
|
||||
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, successful explicit unpublish plus session delete with preserved `published_at`, bounded concurrent publish/delete without deadlock or invalid public commit, and successful parent-card and owning-group cascades.
|
||||
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.
|
||||
|
||||
- [ ] **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 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/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 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"
|
||||
```
|
||||
|
||||
|
||||
Reference in New Issue
Block a user