fix(drafts): serialize draft session flushes and register reminders via fds__setReminder
Playwright Tests / test (push) Canceled after 0s

Make invoice/reminder live-draft edits concurrency-safe with an explicit
DraftFlushResult, add the fds__setReminder stored procedure, and harden the
date helper functions. Includes concurrency and SQL contract tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Stefan
2026-10-01 13:57:40 +02:00
co-authored by Claude Fable 5.1
parent d0002ccdad
commit 546648533b
21 changed files with 813 additions and 140 deletions
+388
View File
@@ -0,0 +1,388 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Fuchs.intranet;
using Fuchs.Services;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging.Abstractions;
using MigraDoc.DocumentObjectModel;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using OCORE.security;
using Xunit;
using static OCORE.OCORE_dictionaries;
namespace Fuchs.Tests;
/// <summary>
/// The editor fires patches, state fetches, previews and saves concurrently for one draft token. The
/// session payload (<c>JArray</c>/<c>JObject</c>, history list) is not thread-safe, so the draft
/// services serialise every access on <c>SyncRoot</c> and hand out snapshots. Regression for the
/// production "Collection was modified" crash in <c>InvoiceSetStructure.RecomputeLevels</c>
/// (inv/dpatch, 2026-10-01). Flushes are additionally serialised on <c>FlushGate</c>: the
/// create/update decision in <c>FlushToDbAsync</c> is a read-then-write across an await, so two
/// overlapping saves of a new draft would otherwise both create a DB row.
/// </summary>
public class DraftSessionConcurrencyTests
{
/// <summary>
/// Registration double: records every call, hands out a fresh id per <b>create</b> and echoes the
/// given id for an update — like the DB, so a duplicate create is visible as a second distinct id.
/// <see cref="Block"/>, when set, parks the first registration until released, which lets a test
/// pin down the exact interleaving instead of hoping for one.
/// </summary>
private sealed class FakeInvoiceService : IInvoiceService
{
private int _ids;
public readonly ConcurrentQueue<(bool change, string invId)> Calls = new();
public TaskCompletionSource? Block;
public readonly TaskCompletionSource Entered = new(TaskCreationOptions.RunContinuationsAsynchronously);
public Exception? Throw;
public async Task<FdsInvoiceData> RegisterInvoiceAsync(FdsInvoiceData invoice, bool change, string invId, string userAccountId, DatabaseSecurity dbSec)
{
Calls.Enqueue((change, invId));
Entered.TrySetResult();
if (Block != null) { var b = Block; Block = null; await b.Task; }
if (Throw != null) { var ex = Throw; Throw = null; throw ex; }
string id = change ? invId : "INV" + Interlocked.Increment(ref _ids);
invoice.InvoiceRegistration = new GenericObjectDictionary(new Dictionary<string, object> { ["Id"] = id });
return invoice;
}
public Task<FdsInvoiceData> LoadInvoiceAsync(string id, string u, DatabaseSecurity s) => throw new NotSupportedException();
public Task<Document> GenerateInvoicePdfAsync(FdsInvoiceData i, bool d) => Task.FromResult(new Document());
public Task<byte[]> RenderInvoicePdfBytesAsync(FdsInvoiceData i, bool d) => throw new NotSupportedException();
public Task<byte[]> StoreInvoiceDocumentFileAsync(FdsInvoiceData i, bool d, string u, DatabaseSecurity s) => throw new NotSupportedException();
public Task<byte[]?> GetInvoiceFileAsync(FdsInvoiceData i, bool d, fds.IFdsMfr m) => throw new NotSupportedException();
}
private sealed class FakeReminderService : IReminderService
{
private int _ids;
public readonly ConcurrentQueue<(bool change, string remId)> Calls = new();
public TaskCompletionSource? Block;
public readonly TaskCompletionSource Entered = new(TaskCreationOptions.RunContinuationsAsynchronously);
public async Task<FdsReminderData> RegisterReminderAsync(FdsReminderData reminder, bool change, string remId, string userAccountId, DatabaseSecurity dbSec)
{
Calls.Enqueue((change, remId));
Entered.TrySetResult();
if (Block != null) { var b = Block; Block = null; await b.Task; }
string id = change ? remId : "REM" + Interlocked.Increment(ref _ids);
reminder.ReminderRegistration = new GenericObjectDictionary(new Dictionary<string, object> { ["Id"] = id });
return reminder;
}
public Document GenerateReminderPdf(FdsReminderData reminder, bool draft) => new();
public Task<FdsReminderData> LoadReminderAsync(string id, string u, DatabaseSecurity s) => throw new NotSupportedException();
public Task<byte[]> RenderReminderPdfBytesAsync(FdsReminderData r, bool d) => throw new NotSupportedException();
public Task<byte[]> StoreReminderDocumentFileAsync(FdsReminderData r, bool d, string u, DatabaseSecurity s) => throw new NotSupportedException();
public Task<byte[]> GetReminderFileAsync(FdsReminderData r, bool d, fds.IFdsMfr m, string u, DatabaseSecurity s) => throw new NotSupportedException();
public Task<(System.IO.FileInfo? file, byte[]? content)> GetStoredFileAsync(string id, string u, DatabaseSecurity s) => throw new NotSupportedException();
}
private static (InvoiceDraftEditService svc, FakeInvoiceService inv) NewInvoiceService()
{
var inv = new FakeInvoiceService();
return (new InvoiceDraftEditService(new InvoiceDraftCache(new ConfigurationBuilder().Build()), inv, NullLogger<InvoiceDraftEditService>.Instance), inv);
}
private static (ReminderDraftEditService svc, FakeReminderService rem) NewReminderService()
{
var rem = new FakeReminderService();
return (new ReminderDraftEditService(new ReminderDraftCache(new ConfigurationBuilder().Build()), rem, NullLogger<ReminderDraftEditService>.Instance), rem);
}
private static JObject InvoicePayload() => JObject.Parse(@"{
'admin':{'p13b':false,'type':'r','paymentterms':'10wd'},
'new':{'invoiceemail':'a@b.de','invoiceaddress':'Weg 1','invoicetitle':'Rechnung'},
'req':[{'Id':'1','text':'Auftrag','itm':[{'id':'900','typ':'set','vt':0,'vv':0,'vat':'19%'},
{'id':'901','typ':'material','vt':100,'vv':19,'vat':'19%','SetItmId':'900'}],
'items':[{'id':'900','type':'set','total_net':0,'vat':'19%'},{'id':'901','type':'material','total_net':100,'vat':'19%'}]}]
}");
private static JObject ReminderPayload() => JObject.Parse(@"{
'rem':{'invid':'INV5','type':'R','invoiceid':'R2026-1','invoicedate':'2026-06-01'},
'new':{'invoiceemail':'a@b.de','invoiceaddress':'Weg 1','subject':'Zahlungserinnerung','amount':119,'amount_payed':0}
}");
private static JObject Block(string id, decimal net) => JObject.Parse(
"{'Id':'" + id + "','text':'Auftrag " + id + "','itm':[{'id':'i" + id + "','typ':'material','vt':" + net + ",'vv':0,'vat':'19%'}]," +
"'items':[{'id':'i" + id + "','type':'material','total_net':" + net + ",'vat':'19%'}]}");
/// <summary>Runs <paramref name="writers"/> and <paramref name="readers"/> concurrently from a common start gate.</summary>
private static async Task RunConcurrently(IEnumerable<Action> writers, IEnumerable<Action> readers)
{
using var gate = new ManualResetEventSlim(false);
var tasks = writers.Concat(readers)
.Select(a => Task.Factory.StartNew(() => { gate.Wait(); a(); }, TaskCreationOptions.LongRunning))
.ToList();
gate.Set();
await Task.WhenAll(tasks);
}
private static Task WithTimeout(Task t) => t.WaitAsync(TimeSpan.FromSeconds(10));
private static Task<T> WithTimeout<T>(Task<T> t) => t.WaitAsync(TimeSpan.FromSeconds(10));
// ── payload lock ───────────────────────────────────────────────────────────
[Theory]
[InlineData("block.replace")] // inserts new blocks → s.Req.Add (the crash: RecomputeLevels enumerating Req)
[InlineData("block.order")] // s.Req.Clear() + re-add while others enumerate
public async Task ApplyPatch_ConcurrentPatchesAndStateReads_NeverThrowAndCountEveryPatch(string target)
{
const int patches = 200;
var (svc, _) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
if (target == "block.order")
for (int i = 0; i < 4; i++) svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "block.replace", Ref = "b" + i, Value = Block("b" + i, 10) });
int baseVersion = s.Version;
var writers = Enumerable.Range(0, patches).Select<int, Action>(i => () =>
{
var delta = target == "block.replace"
? new InvoiceDraftDelta { Target = target, Ref = "n" + i, Value = Block("n" + i, 1) }
: new InvoiceDraftDelta { Target = target, Value = new JArray(i % 2 == 0 ? new[] { "b3", "b2", "1" } : new[] { "1", "b0", "b1" }) };
Assert.NotNull(svc.ApplyPatch(s.Token, delta));
});
var readers = Enumerable.Range(0, 50).Select<int, Action>(_ => () =>
{
for (int k = 0; k < 5; k++)
{
JsonConvert.SerializeObject(svc.BuildState(s)); // what JSONAsync does after BuildState returns
_ = svc.GetHistory(s.Token).Count;
}
});
await RunConcurrently(writers, readers);
// Every applied patch bumps the version exactly once and leaves one history entry (no lost
// updates). A reorder to the order that is already current is a no-op by design, so for
// block.order fewer than `patches` may apply.
Assert.Equal(s.Version, svc.GetHistory(s.Token).Count);
if (target == "block.order")
{
Assert.InRange(s.Version, baseVersion + 1, baseVersion + patches);
Assert.Equal(5, s.Req.Count); // reorders never lose or duplicate a block
}
else
{
Assert.Equal(baseVersion + patches, s.Version);
Assert.Equal(1 + patches, s.Req.Count);
Assert.Equal(100m + patches, s.Sums.TotalNet);
}
}
[Fact]
public async Task RenderPreviewAndFlush_ConcurrentWithPatches_NeverThrowAndCreateExactlyOnce()
{
var (svc, inv) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
var results = new ConcurrentBag<DraftFlushResult<FdsInvoiceData>>();
var writers = Enumerable.Range(0, 100).Select<int, Action>(i => () =>
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "block.replace", Ref = "n" + i, Value = Block("n" + i, 1) }));
var readers = Enumerable.Range(0, 20).Select<int, Action>(i => () =>
{
Assert.NotNull(svc.RenderPreviewAsync(s.Token).GetAwaiter().GetResult());
results.Add(svc.FlushToDbAsync(s.Token, "user1", null!).GetAwaiter().GetResult()!);
});
await RunConcurrently(writers, readers);
Assert.Equal(101, s.Req.Count);
// 20 overlapping saves of a brand-new draft: exactly one fds__createInvoice, every other save
// an update of that row — never a second orphaned invoice.
Assert.Equal(20, inv.Calls.Count);
Assert.Single(inv.Calls, c => !c.change);
Assert.Equal("INV1", s.InvId);
Assert.All(inv.Calls.Where(c => c.change), c => Assert.Equal("INV1", c.invId));
Assert.All(results, r => Assert.Equal("INV1", r.Data.Id));
Assert.Single(results, r => !r.Updated);
}
// ── flush gate ─────────────────────────────────────────────────────────────
[Fact]
public async Task FlushToDbAsync_SecondSaveOverlapsFirstRegistration_WaitsThenUpdatesInsteadOfCreating()
{
var (svc, inv) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
var block = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
inv.Block = block; // the fake clears Block once it has parked on it
var first = svc.FlushToDbAsync(s.Token, "user1", null!);
await WithTimeout(inv.Entered.Task); // first save is inside the DB registration, InvId still empty
var second = svc.FlushToDbAsync(s.Token, "user1", null!);
// While the first registration is in flight the second save must not reach the registration at
// all (it would see the empty InvId and create a second row). A false pass here is possible
// only under pathological scheduling; a false failure is not.
await Task.Delay(150);
Assert.False(second.IsCompleted);
Assert.Single(inv.Calls);
block.SetResult();
var r1 = (await WithTimeout(first))!;
var r2 = (await WithTimeout(second))!;
Assert.Equal(new[] { (false, ""), (true, "INV1") }, inv.Calls.ToArray());
Assert.False(r1.Updated); Assert.Equal("INV1", r1.Data.Id);
Assert.True(r2.Updated); Assert.Equal("INV1", r2.Data.Id);
Assert.Equal("INV1", s.InvId);
}
[Fact]
public async Task FlushToDbAsync_SecondSaveCarriesPatchesMadeDuringFirst()
{
var (svc, inv) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
var block = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
inv.Block = block; // the fake clears Block once it has parked on it
var first = svc.FlushToDbAsync(s.Token, "user1", null!);
await WithTimeout(inv.Entered.Task);
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "email", Value = "neu@x.de" }); // edits are not blocked by a running save
var second = svc.FlushToDbAsync(s.Token, "user1", null!);
block.SetResult();
var r1 = (await WithTimeout(first))!;
var r2 = (await WithTimeout(second))!;
// The waiting save snapshots after it acquires the gate, so it persists the newer state —
// serialising never makes a save write stale data.
Assert.Equal("a@b.de", r1.Data.RawInvoiceEmail);
Assert.Equal("neu@x.de", r2.Data.RawInvoiceEmail);
Assert.True(r2.Updated);
}
[Fact]
public async Task FlushToDbAsync_RegistrationThrows_ReleasesGateAndNextSaveCreates()
{
var (svc, inv) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
inv.Throw = new InvalidOperationException("db down");
await Assert.ThrowsAsync<InvalidOperationException>(() => svc.FlushToDbAsync(s.Token, "user1", null!));
// The gate must be released by the failed save; the next one gets through and, since no id
// was committed, still runs as the create.
var r = (await WithTimeout(svc.FlushToDbAsync(s.Token, "user1", null!)))!;
Assert.False(r.Updated);
Assert.Equal("INV1", r.Data.Id);
Assert.Equal("INV1", s.InvId);
Assert.Equal(2, inv.Calls.Count);
}
[Fact]
public async Task FlushToDbAsync_UnknownToken_ReturnsNullWithoutRegistering()
{
var (svc, inv) = NewInvoiceService();
Assert.Null(await svc.FlushToDbAsync("ghost", "user1", null!));
Assert.Empty(inv.Calls);
}
[Fact]
public async Task ReminderFlushToDbAsync_SecondSaveOverlapsFirstRegistration_WaitsThenUpdatesInsteadOfCreating()
{
var (svc, rem) = NewReminderService();
var s = svc.OpenFromPayload(ReminderPayload(), "user1");
var block = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
rem.Block = block; // the fake clears Block once it has parked on it
var first = svc.FlushToDbAsync(s.Token, "user1", null!);
await WithTimeout(rem.Entered.Task);
var second = svc.FlushToDbAsync(s.Token, "user1", null!);
await Task.Delay(150);
Assert.False(second.IsCompleted);
Assert.Single(rem.Calls);
block.SetResult();
var r1 = (await WithTimeout(first))!;
var r2 = (await WithTimeout(second))!;
Assert.Equal(new[] { (false, ""), (true, "REM1") }, rem.Calls.ToArray());
Assert.False(r1.Updated); Assert.Equal("REM1", r1.Data.Id);
Assert.True(r2.Updated); Assert.Equal("REM1", r2.Data.Id);
Assert.Equal("REM1", s.RemId);
}
[Fact]
public async Task ReminderFlushToDbAsync_ManyConcurrentSaves_CreateExactlyOnce()
{
var (svc, rem) = NewReminderService();
var s = svc.OpenFromPayload(ReminderPayload(), "user1");
await RunConcurrently(
Enumerable.Range(0, 20).Select<int, Action>(_ => () => svc.FlushToDbAsync(s.Token, "user1", null!).GetAwaiter().GetResult()),
Enumerable.Empty<Action>());
Assert.Equal(20, rem.Calls.Count);
Assert.Single(rem.Calls, c => !c.change);
Assert.Equal("REM1", s.RemId);
}
// ── snapshots ──────────────────────────────────────────────────────────────
[Fact]
public void BuildState_IsSnapshot_LaterPatchDoesNotChangeReturnedPayload()
{
var (svc, _) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
string before = JsonConvert.SerializeObject(svc.BuildState(s));
var state = svc.BuildState(s);
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "block.replace", Ref = "x", Value = Block("x", 5) });
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "email", Value = "neu@x.de" });
Assert.Equal(before, JsonConvert.SerializeObject(state));
Assert.Contains("neu@x.de", JsonConvert.SerializeObject(svc.BuildState(s)));
}
[Fact]
public void GetHistory_IsSnapshot_LaterPatchDoesNotGrowReturnedList()
{
var (svc, _) = NewInvoiceService();
var s = svc.OpenFromPayload(InvoicePayload(), "user1");
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "email", Value = "x@y.de" });
var history = svc.GetHistory(s.Token);
svc.ApplyPatch(s.Token, new InvoiceDraftDelta { Target = "email", Value = "z@y.de" });
Assert.Single(history);
Assert.Equal(2, svc.GetHistory(s.Token).Count);
}
[Theory]
[InlineData("")]
[InlineData("unknown-token")]
public void GetHistory_UnknownToken_ReturnsEmpty(string token)
{
Assert.Empty(NewInvoiceService().svc.GetHistory(token));
Assert.Empty(NewReminderService().svc.GetHistory(token));
}
[Fact]
public async Task ReminderApplyPatch_ConcurrentPatchesAndStateReads_NeverThrowAndCountEveryPatch()
{
const int patches = 200;
var (svc, _) = NewReminderService();
var s = svc.OpenFromPayload(ReminderPayload(), "user1");
var writers = Enumerable.Range(0, patches).Select<int, Action>(i => () =>
Assert.NotNull(svc.ApplyPatch(s.Token, new ReminderDraftDelta { Target = i % 2 == 0 ? "subject" : "amount_payed", Value = i % 2 == 0 ? "S" + i : "1" })));
var readers = Enumerable.Range(0, 50).Select<int, Action>(_ => () =>
{
for (int k = 0; k < 5; k++)
{
JsonConvert.SerializeObject(svc.BuildState(s));
_ = svc.GetHistory(s.Token).Count;
}
});
await RunConcurrently(writers, readers);
Assert.Equal(patches, s.Version);
Assert.Equal(patches, svc.GetHistory(s.Token).Count);
Assert.Equal(118m, s.Sums.AmountOpen);
}
}
+4 -2
View File
@@ -169,7 +169,8 @@ public class InvoiceDraftServiceTests
var result = await svc.FlushToDbAsync(s.Token, "user1", null!);
Assert.NotNull(result);
Assert.Equal("INV42", result!.Id);
Assert.Equal("INV42", result!.Data.Id);
Assert.False(result.Updated);
Assert.False(inv.LastChange); // new draft (no prior InvId) → create, not update
Assert.Equal("INV42", svc.Get(s.Token)!.InvId);
@@ -680,9 +681,10 @@ public class InvoiceDraftServiceTests
payload["invid"] = "INV7";
var s = svc.OpenFromPayload(payload, "user1");
await svc.FlushToDbAsync(s.Token, "user1", null!);
var result = await svc.FlushToDbAsync(s.Token, "user1", null!);
Assert.True(inv.LastChange); // prior InvId → update path
Assert.True(result!.Updated);
}
[Fact]
+4 -2
View File
@@ -238,7 +238,8 @@ public class ReminderDraftServiceTests
var result = await svc.FlushToDbAsync(s.Token, "user1", null!);
Assert.NotNull(result);
Assert.Equal("REM42", result!.Id);
Assert.Equal("REM42", result!.Data.Id);
Assert.False(result.Updated);
Assert.False(rem.LastChange); // new draft (no prior RemId) → create
Assert.Equal("REM42", svc.Get(s.Token)!.RemId);
// the email/subject the editor set must reach registration
@@ -255,9 +256,10 @@ public class ReminderDraftServiceTests
payload["remid"] = "REM7";
var s = svc.OpenFromPayload(payload, "user1");
await svc.FlushToDbAsync(s.Token, "user1", null!);
var result = await svc.FlushToDbAsync(s.Token, "user1", null!);
Assert.True(rem.LastChange); // prior RemId → update path
Assert.True(result!.Updated);
}
[Fact]
+50
View File
@@ -0,0 +1,50 @@
using System;
using Fuchs.Services;
using Xunit;
namespace Fuchs.Tests;
/// <summary>
/// The reminder save batch (<see cref="ReminderService.BuildRegisterReminderSql"/>): a new draft is
/// created, a re-saved one is updated in place via <c>fds__setReminder</c> — previously the update
/// path ran an empty batch, so "Zwischenspeichern" of an already-registered reminder draft silently
/// saved nothing. Either proc leaving <c>@Id</c> NULL must surface as an error, not as an empty row.
/// </summary>
public class ReminderRegisterSqlTests
{
[Theory]
[InlineData(false, "[dbo].[fds__createReminder] @InvId,", "[dbo].[fds__setReminder]")]
[InlineData(true, "[dbo].[fds__setReminder] @RemId,", "[dbo].[fds__createReminder]")]
public void BuildRegisterReminderSql_ExactlyOneProcPerMode(bool change, string expectedProc, string otherProc)
{
string sql = ReminderService.BuildRegisterReminderSql(change);
Assert.Contains(expectedProc, sql);
Assert.DoesNotContain(otherProc, sql);
Assert.Equal(change, sql.Contains("@RemId"));
Assert.Equal(!change, sql.Contains("@InvId"));
}
[Theory]
[InlineData(false)]
[InlineData(true)]
public void BuildRegisterReminderSql_GuardFollowsTheProc(bool change)
{
string sql = ReminderService.BuildRegisterReminderSql(change);
int proc = sql.IndexOf("EXECUTE [dbo].[fds__", StringComparison.Ordinal);
int guard = sql.IndexOf("IF @Id IS NULL THROW 50001", StringComparison.Ordinal);
Assert.True(proc >= 0 && guard > proc, sql);
Assert.StartsWith("DECLARE @Id varchar(10);", sql);
}
[Theory]
[InlineData(false)]
[InlineData(true)]
public void BuildRegisterReminderSql_EveryStatementIsSemicolonTerminated(bool change)
{
// THROW requires the preceding statement to be terminated with ';'.
foreach (var line in ReminderService.BuildRegisterReminderSql(change).Split('\n'))
Assert.EndsWith(";", line.TrimEnd());
}
}