docs: document portfolio concurrency hardening plan
This commit is contained in:
@@ -61,9 +61,16 @@
|
|||||||
|
|
||||||
**Files:**
|
**Files:**
|
||||||
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs`
|
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs`
|
||||||
|
- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.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`
|
- 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: `tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj`
|
||||||
|
- Modify: `tests/GmRelay.Bot.Tests/packages.lock.json`
|
||||||
|
|
||||||
- [ ] **Step 1: Write the failing migration source-contract test**
|
- [ ] **Step 1: Write the failing migration and session-deletion source-contract tests**
|
||||||
|
|
||||||
Add tests that read `V029__add_completed_game_portfolios_and_reviews.sql` and assert:
|
Add tests that read `V029__add_completed_game_portfolios_and_reviews.sql` and assert:
|
||||||
|
|
||||||
@@ -115,17 +122,90 @@ public async Task MigrationV029_ShouldStoreProviderNeutralCoverKeys()
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
- [ ] **Step 2: Run the migration test to verify RED**
|
Add `PortfolioSessionDeletionSourceTests.cs`. Normalize whitespace before comparing source text and assert that both session-deletion paths explicitly unpublish linked cards before deleting the required session link:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[Fact]
|
||||||
|
public async Task SharedDeleteSessionHandler_ShouldUnpublishLinkedPortfolioCardBeforeDeletingSession()
|
||||||
|
{
|
||||||
|
var source = NormalizeSql(await ReadRepositoryFileAsync(
|
||||||
|
"src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs"));
|
||||||
|
|
||||||
|
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(unpublish, source, StringComparison.Ordinal);
|
||||||
|
Assert.True(
|
||||||
|
source.IndexOf(unpublish, StringComparison.Ordinal) <
|
||||||
|
source.IndexOf("DELETE FROM sessions WHERE id = @Id", StringComparison.Ordinal));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task DiscordDeleteSessionHandler_ShouldUnpublishOnlyCardsFromTheInteractionGuildBeforeDeletingSession()
|
||||||
|
{
|
||||||
|
var source = NormalizeSql(await ReadRepositoryFileAsync(
|
||||||
|
"src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs"));
|
||||||
|
|
||||||
|
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(unpublish, source, StringComparison.Ordinal);
|
||||||
|
Assert.True(
|
||||||
|
source.IndexOf(unpublish, StringComparison.Ordinal) <
|
||||||
|
source.IndexOf("DELETE FROM sessions s", StringComparison.Ordinal));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Add the failing PostgreSQL Testcontainers integration fixture and tests**
|
||||||
|
|
||||||
|
Add the package reference:
|
||||||
|
|
||||||
|
```xml
|
||||||
|
<PackageReference Include="Testcontainers.PostgreSql" Version="4.12.0" />
|
||||||
|
```
|
||||||
|
|
||||||
|
Update the locked dependency graph:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
dotnet restore tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --use-lock-file
|
||||||
|
```
|
||||||
|
|
||||||
|
Create `PortfolioMigrationPostgresFixture.cs` with a shared `PostgreSqlContainer` built from `postgres:17-alpine`. For each test, create a fresh database and apply migration files `V001` through `V029` in ordinal filename order.
|
||||||
|
|
||||||
|
Create `PortfolioMigrationPostgresTests.cs` with these executable scenarios:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[Fact]
|
||||||
|
public async Task MigrationsV001ThroughV029_ShouldApplyToPostgres17()
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("portfolio_game_sessions")]
|
||||||
|
[InlineData("portfolio_game_masters")]
|
||||||
|
public async Task DirectRequiredLinkDeletion_ShouldFailCommitForPublishedCard(string linkTable)
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ExplicitUnpublishThenSessionDelete_ShouldCommitAndPreserveFirstPublishedAt()
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard()
|
||||||
|
|
||||||
|
[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.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Run the Task 1 tests to verify RED**
|
||||||
|
|
||||||
Run:
|
Run:
|
||||||
|
|
||||||
```powershell
|
```powershell
|
||||||
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests"
|
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests"
|
||||||
```
|
```
|
||||||
|
|
||||||
Expected: FAIL because `V029__add_completed_game_portfolios_and_reviews.sql` does not exist.
|
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.
|
||||||
|
|
||||||
- [ ] **Step 3: Add migration V029**
|
- [ ] **Step 4: Add migration V029**
|
||||||
|
|
||||||
Create the migration with these exact tables and indexes:
|
Create the migration with these exact tables and indexes:
|
||||||
|
|
||||||
@@ -265,15 +345,53 @@ CREATE INDEX ix_portfolio_game_reviews_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 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.
|
||||||
|
|
||||||
- [ ] **Step 4: Run the migration tests to verify GREEN**
|
- [ ] **Step 5: Explicitly unpublish linked cards in both session-deletion handlers**
|
||||||
|
|
||||||
Run the Task 1 command again. Expected: PASS.
|
In `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`, run this statement inside the existing transaction after authorization and before `DELETE FROM sessions`:
|
||||||
|
|
||||||
- [ ] **Step 5: Commit**
|
```sql
|
||||||
|
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
|
||||||
|
```
|
||||||
|
|
||||||
|
In `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`, start a transaction before deleting. Run this guild-scoped unpublish statement before the existing guild-scoped `DELETE FROM sessions`, then commit:
|
||||||
|
|
||||||
|
```sql
|
||||||
|
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
|
||||||
|
```
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the Task 1 tests to verify GREEN**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
```powershell
|
```powershell
|
||||||
git add src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs
|
dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests"
|
||||||
git commit -m "feat(data): add completed game portfolio schema"
|
```
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
- [ ] **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 commit -m "fix(data): harden portfolio publication concurrency"
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
Reference in New Issue
Block a user