diff --git a/GerbilManager.Tests/ImportDocxServiceTests.cs b/GerbilManager.Tests/ImportDocxServiceTests.cs index 3fdcc67..fe3f585 100644 --- a/GerbilManager.Tests/ImportDocxServiceTests.cs +++ b/GerbilManager.Tests/ImportDocxServiceTests.cs @@ -2,6 +2,7 @@ using GerbilManagerWebAPI.Import; using GerbilManagerWebAPI.Models; using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Storage; namespace GerbilManager.Tests { @@ -226,6 +227,52 @@ namespace GerbilManager.Tests } } + // ── Test 7: P0 REGRESSION — execute works under a retrying execution strategy ── + + [Fact] + public async Task Execute_works_under_retrying_execution_strategy() + { + // Regression: NpgsqlRetryingExecutionStrategy (MaxRetryCount>0) calls + // OnFirstExecution() at the start of ExecuteAsync, which throws + // InvalidOperationException when it detects a user-initiated transaction + // that was NOT opened through the strategy. This test wires the same check + // (via FakeRetryingStrategy, MaxRetryCount=1) so the bug would surface in CI + // without a live Npgsql instance. + // + // With the BUG (direct BeginTransactionAsync before strategy.ExecuteAsync): + // → OnFirstExecution sees active user tx → InvalidOperationException + // With the FIX (BeginTransactionAsync inside strategy.ExecuteAsync lambda): + // → OnFirstExecution: no tx yet → OK + var conn = new SqliteConnection("DataSource=:memory:"); + conn.Open(); + var opts = new DbContextOptionsBuilder() + .UseSqlite(conn) + .ReplaceService() + .Options; + var db = new ApplicationContext(opts); + db.Database.EnsureCreated(); + + await using (conn) + await using (db) + { + WriteLitters(Array.Empty()); + WriteAnimals(new[] + { + new { wsCode = "3/3", litterDob = "01.01.2023", name = "Pixie", gender = "female", + owner = "Retry Adopter", abgabeDate = "01.03.2023", + deathDate = "", deathCause = "", farbschlag = "" } + }); + + // Must NOT throw InvalidOperationException (user-initiated tx rejected) + var report = await new ImportDocxService(db, _dir).RunAsync(execute: true); + + Assert.True(report.Executed); + Assert.Equal(1, report.Created); + Assert.Equal(1, await db.Gerbils.CountAsync()); + Assert.Equal(1, await db.Contacts.CountAsync()); + } + } + // ── Test 6: P0 REGRESSION — same-name siblings get distinct ExternalRefs ─ [Fact] @@ -273,4 +320,24 @@ namespace GerbilManager.Tests } } } + + // ── Helpers for Test 7 ──────────────────────────────────────────────────────── + + /// + /// Execution strategy with MaxRetryCount=1 so that EF Core's base + /// OnFirstExecution() throws when it detects a user-initiated transaction + /// that was not opened through CreateExecutionStrategy().ExecuteAsync(). + /// ShouldRetryOn=false → no actual retry; the check alone is what we need. + /// + internal sealed class FakeRetryingStrategy(ExecutionStrategyDependencies deps) + : ExecutionStrategy(deps, maxRetryCount: 1, maxRetryDelay: TimeSpan.Zero) + { + protected override bool ShouldRetryOn(Exception exception) => false; + } + + internal sealed class FakeRetryingStrategyFactory(ExecutionStrategyDependencies deps) + : IExecutionStrategyFactory + { + public IExecutionStrategy Create() => new FakeRetryingStrategy(deps); + } } diff --git a/GerbilManagerWebAPI/Import/ImportDocxService.cs b/GerbilManagerWebAPI/Import/ImportDocxService.cs index a2fceb6..9fff02c 100644 --- a/GerbilManagerWebAPI/Import/ImportDocxService.cs +++ b/GerbilManagerWebAPI/Import/ImportDocxService.cs @@ -22,7 +22,10 @@ namespace GerbilManagerWebAPI.Import /// NEVER overwrites a manually-set non-null value (fill-NULL-only for all fields). /// /// Idempotent: running multiple times is safe. Re-run finds existing rows via ExternalRef. - /// Execute wraps all writes in a single transaction (atomic: crash → full rollback). + /// Execute wraps all writes in a single transaction via CreateExecutionStrategy() so that + /// providers using EnableRetryOnFailure (e.g. NpgsqlRetryingExecutionStrategy) are + /// compatible. The strategy lambda resets all mutable state at the top so it is safe + /// to re-run on transient-failure retry. /// Execute is gated by the endpoint; this service only acts when asked. /// public sealed class ImportDocxService @@ -94,6 +97,11 @@ namespace GerbilManagerWebAPI.Import .GroupBy(c => NormalizeName(c.Name)) .ToDictionary(g => g.Key, g => g.First().Id); + // Snapshot of DB contacts before any writes. + // Used to reset contactByNorm on strategy retry (rolled-back contacts vanish from DB + // but would remain in the in-memory dict without this reset). + var contactByNormBase = new Dictionary(contactByNorm); + // ColorVariety lookup: normalized name → Id (for CREATE path Farbschlag matching) var colorVarietyByName = (await _db.ColorVarieties .Select(cv => new { cv.Id, cv.Name }) @@ -105,19 +113,15 @@ namespace GerbilManagerWebAPI.Import int ownerLinked = 0, ownerCreated = 0, skipped = 0; // Ordinal counter for collision-free ExternalRef within this batch. - // Two animals with the same base ref (same ws+name+litterDob) get -2, -3 suffixes. var externalRefOrdinals = new Dictionary(); // Belt-and-suspenders: guard against adding the same ExternalRef twice in one run. var batchRefs = new HashSet(); - // --- Planning pass (dry-run counts + execute writes) --- - // Execute path is wrapped in a single transaction for atomicity. - Microsoft.EntityFrameworkCore.Storage.IDbContextTransaction? tx = null; - if (execute) - tx = await _db.Database.BeginTransactionAsync(); - - try + // Inner loop — shared by dry-run and execute paths. + // All local variables above are captured by reference (C# closure), so the strategy + // lambda can reset them before each retry and RunLoopAsync sees the fresh state. + async Task RunLoopAsync() { foreach (var da in docxAnimals) { @@ -276,21 +280,36 @@ namespace GerbilManagerWebAPI.Import }); } } - - // Flush all gerbil inserts + enrich updates in one shot (within the tx) - if (execute && (animalsCreated + litterLinked + goHomeFilled + deathFilled + ownerCreated) > 0) - await _db.SaveChangesAsync(); - - if (tx is not null) await tx.CommitAsync(); } - catch + + if (!execute) { - // tx.DisposeAsync (in finally) rolls back if not committed - throw; + // Dry-run: just count, no writes, no transaction needed. + await RunLoopAsync(); } - finally + else { - if (tx is not null) await tx.DisposeAsync(); + // Execute: wrap the entire transaction in the execution strategy so that providers + // with EnableRetryOnFailure (NpgsqlRetryingExecutionStrategy) are compatible. + // The lambda resets all mutable state at the top so retries start clean. + var strategy = _db.Database.CreateExecutionStrategy(); + await strategy.ExecuteAsync(async () => + { + // Reset mutable state — idempotent on strategy retry + _db.ChangeTracker.Clear(); + externalRefOrdinals.Clear(); + batchRefs.Clear(); + animalsCreated = 0; litterLinked = 0; goHomeFilled = 0; deathFilled = 0; + ownerLinked = 0; ownerCreated = 0; skipped = 0; + // Rebuild from DB snapshot: contacts added in a failed attempt were rolled back + contactByNorm = new Dictionary(contactByNormBase); + + await using var tx = await _db.Database.BeginTransactionAsync(); + await RunLoopAsync(); + if ((animalsCreated + litterLinked + goHomeFilled + deathFilled + ownerCreated) > 0) + await _db.SaveChangesAsync(); + await tx.CommitAsync(); + }); } notes.Add($"Quelle: {docxLitters.Count} Würfe, {docxAnimals.Count} Tier-Zeilen aus der docx.");