diff --git a/Fuchs.Tests/DraftSessionConcurrencyTests.cs b/Fuchs.Tests/DraftSessionConcurrencyTests.cs new file mode 100644 index 0000000..df32c08 --- /dev/null +++ b/Fuchs.Tests/DraftSessionConcurrencyTests.cs @@ -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; + +/// +/// The editor fires patches, state fetches, previews and saves concurrently for one draft token. The +/// session payload (JArray/JObject, history list) is not thread-safe, so the draft +/// services serialise every access on SyncRoot and hand out snapshots. Regression for the +/// production "Collection was modified" crash in InvoiceSetStructure.RecomputeLevels +/// (inv/dpatch, 2026-10-01). Flushes are additionally serialised on FlushGate: the +/// create/update decision in FlushToDbAsync is a read-then-write across an await, so two +/// overlapping saves of a new draft would otherwise both create a DB row. +/// +public class DraftSessionConcurrencyTests +{ + /// + /// Registration double: records every call, hands out a fresh id per create and echoes the + /// given id for an update — like the DB, so a duplicate create is visible as a second distinct id. + /// , when set, parks the first registration until released, which lets a test + /// pin down the exact interleaving instead of hoping for one. + /// + 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 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 { ["Id"] = id }); + return invoice; + } + public Task LoadInvoiceAsync(string id, string u, DatabaseSecurity s) => throw new NotSupportedException(); + public Task GenerateInvoicePdfAsync(FdsInvoiceData i, bool d) => Task.FromResult(new Document()); + public Task RenderInvoicePdfBytesAsync(FdsInvoiceData i, bool d) => throw new NotSupportedException(); + public Task StoreInvoiceDocumentFileAsync(FdsInvoiceData i, bool d, string u, DatabaseSecurity s) => throw new NotSupportedException(); + public Task 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 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 { ["Id"] = id }); + return reminder; + } + public Document GenerateReminderPdf(FdsReminderData reminder, bool draft) => new(); + public Task LoadReminderAsync(string id, string u, DatabaseSecurity s) => throw new NotSupportedException(); + public Task RenderReminderPdfBytesAsync(FdsReminderData r, bool d) => throw new NotSupportedException(); + public Task StoreReminderDocumentFileAsync(FdsReminderData r, bool d, string u, DatabaseSecurity s) => throw new NotSupportedException(); + public Task 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.Instance), inv); + } + + private static (ReminderDraftEditService svc, FakeReminderService rem) NewReminderService() + { + var rem = new FakeReminderService(); + return (new ReminderDraftEditService(new ReminderDraftCache(new ConfigurationBuilder().Build()), rem, NullLogger.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%'}]}"); + + /// Runs and concurrently from a common start gate. + private static async Task RunConcurrently(IEnumerable writers, IEnumerable 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 WithTimeout(Task 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(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(_ => () => + { + 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>(); + + var writers = Enumerable.Range(0, 100).Select(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(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(() => 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(_ => () => svc.FlushToDbAsync(s.Token, "user1", null!).GetAwaiter().GetResult()), + Enumerable.Empty()); + + 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(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(_ => () => + { + 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); + } +} diff --git a/Fuchs.Tests/InvoiceDraftServiceTests.cs b/Fuchs.Tests/InvoiceDraftServiceTests.cs index c2fa058..ee9aa33 100644 --- a/Fuchs.Tests/InvoiceDraftServiceTests.cs +++ b/Fuchs.Tests/InvoiceDraftServiceTests.cs @@ -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] diff --git a/Fuchs.Tests/ReminderDraftServiceTests.cs b/Fuchs.Tests/ReminderDraftServiceTests.cs index f846db5..ad28c2b 100644 --- a/Fuchs.Tests/ReminderDraftServiceTests.cs +++ b/Fuchs.Tests/ReminderDraftServiceTests.cs @@ -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] diff --git a/Fuchs.Tests/ReminderRegisterSqlTests.cs b/Fuchs.Tests/ReminderRegisterSqlTests.cs new file mode 100644 index 0000000..17017a1 --- /dev/null +++ b/Fuchs.Tests/ReminderRegisterSqlTests.cs @@ -0,0 +1,50 @@ +using System; +using Fuchs.Services; +using Xunit; + +namespace Fuchs.Tests; + +/// +/// The reminder save batch (): a new draft is +/// created, a re-saved one is updated in place via fds__setReminder — previously the update +/// path ran an empty batch, so "Zwischenspeichern" of an already-registered reminder draft silently +/// saved nothing. Either proc leaving @Id NULL must surface as an error, not as an empty row. +/// +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()); + } +} diff --git a/Fuchs/Controllers/IntranetController.InvoiceDraft.cs b/Fuchs/Controllers/IntranetController.InvoiceDraft.cs index ec11835..012bf49 100644 --- a/Fuchs/Controllers/IntranetController.InvoiceDraft.cs +++ b/Fuchs/Controllers/IntranetController.InvoiceDraft.cs @@ -107,16 +107,15 @@ public partial class IntranetController { if (!HasForm("token")) return BadRequest400(); string token = Form("token"); - var before = _invoiceDrafts.Get(token); - if (before == null) return DraftGone(); - bool existed = !string.IsNullOrEmpty(before.InvId); - - var fdInv = await _invoiceDrafts.FlushToDbAsync(token, UserAccountID, DbSec); - if (fdInv == null) return DraftGone(); + // Create vs update is decided inside the flush (flushes of one session are serialised), not + // from the session state seen here: a save overlapping the first registration is an update. + var flush = await _invoiceDrafts.FlushToDbAsync(token, UserAccountID, DbSec); + if (flush == null) return DraftGone(); + var fdInv = flush.Data; if (string.IsNullOrEmpty(fdInv.Id)) return await InvoiceIssueResult("Der Zwischenstand konnte aufgrund eines Fehlers nicht gespeichert werden."); - await _events.InvoiceDraftRegisteredAsync(fdInv, existed, UserAccountID); + await _events.InvoiceDraftRegisteredAsync(fdInv, flush.Updated, UserAccountID); var after = _invoiceDrafts.Get(token); if (after != null) await _draftNotifier.SignalDraftReadyAsync(after.Token, after.Version); return await JSONAsync(new { ok = true, invid = fdInv.Id }); diff --git a/Fuchs/Controllers/IntranetController.ReminderDraft.cs b/Fuchs/Controllers/IntranetController.ReminderDraft.cs index bb6fb9d..bd4885b 100644 --- a/Fuchs/Controllers/IntranetController.ReminderDraft.cs +++ b/Fuchs/Controllers/IntranetController.ReminderDraft.cs @@ -82,16 +82,14 @@ public partial class IntranetController { if (!HasForm("token")) return BadRequest400(); string token = Form("token"); - var before = _reminderDrafts.Get(token); - if (before == null) return DraftGone(); - bool existed = !string.IsNullOrEmpty(before.RemId); - - var fdRem = await _reminderDrafts.FlushToDbAsync(token, UserAccountID, DbSec); - if (fdRem == null) return DraftGone(); + // Create vs update is decided inside the flush (serialised per session) — see HandleDraftSave. + var flush = await _reminderDrafts.FlushToDbAsync(token, UserAccountID, DbSec); + if (flush == null) return DraftGone(); + var fdRem = flush.Data; if (string.IsNullOrEmpty(fdRem.Id)) return await ReminderIssueResult("Der Zwischenstand konnte aufgrund eines Fehlers nicht gespeichert werden."); - await _events.ReminderDraftRegisteredAsync(fdRem, existed, UserAccountID); + await _events.ReminderDraftRegisteredAsync(fdRem, flush.Updated, UserAccountID); var after = _reminderDrafts.Get(token); if (after != null) await _draftNotifier.SignalDraftReadyAsync(after.Token, after.Version); return await JSONAsync(new { ok = true, remid = fdRem.Id }); diff --git a/Fuchs/Docs/Concepts/live-draft-editing.md b/Fuchs/Docs/Concepts/live-draft-editing.md index 1aeaba9..673bd67 100644 --- a/Fuchs/Docs/Concepts/live-draft-editing.md +++ b/Fuchs/Docs/Concepts/live-draft-editing.md @@ -1,6 +1,6 @@ --- status: Active -lastUpdated: 2026-09-09 +lastUpdated: 2026-10-01 applyTo: - "Fuchs/Services/InvoiceDraft*" - "Fuchs/Services/IInvoiceDraft*" @@ -99,6 +99,21 @@ Expiry: Server (timer) --SignalR draftExpiring{token,secondsLeft}--> warn "bit (`RecomputeLevels`), rendered as an indent class in the editor and a per-level indent in the PDF. `item.setprice` sums a header's **transitive** descendants. Structure deltas never change values — only order, membership, levels and therefore position numbers. +- **Concurrency.** The editor fires patches, state fetches, previews and saves for one token + concurrently, and the session payload (`JObject`/`JArray`, the history list) is not thread-safe. + Every read and write of the payload runs under the session's `SyncRoot`, and everything the + controller serialises afterwards (`BuildState`, `GetHistory`, the `FdsInvoiceData` a preview or + flush uses) is a deep-cloned snapshot taken under that lock — never a live reference. Flushes are + additionally serialised per session on `FlushGate` (a `SemaphoreSlim`, awaited — `SyncRoot` + cannot be held across the registration's `await`): the create/update decision in + `FlushToDbAsync` is read-then-write across that await, so two overlapping saves of a + not-yet-registered draft (double-click on *Zwischenspeichern*, or *Zwischenspeichern* followed + by preview → confirm) would otherwise both call `fds__createInvoice` and leave an orphaned + invoice. The waiting save snapshots **after** it acquires the gate, so it persists the newer + state as an update of the row the first save created. `FlushToDbAsync` reports that decision + (`DraftFlushResult.Updated`) and the controller publishes `InvoiceDraftCreated`/`-Updated` from + it rather than from the session state it saw before the flush. Patches are never blocked by a + running save. `DraftSessionConcurrencyTests` pins both the lock and the gate. - **Cache** (`InvoiceDraftCache`, singleton) stores sessions by token with an idle sliding TTL; `InvoiceDraftExpiryService` (a `BackgroundService`) warns before, and evicts after, the TTL. TTL/warn-lead are configurable under `Fuchs:DraftEditing`. @@ -146,9 +161,12 @@ amount pair. - **Orchestration** (`ReminderDraftEditService`, scoped) flushes to the DB by reusing `IReminderService.RegisterReminderAsync`, and renders previews from a synthesised `ReminderRegistration` (including the single-invoice `invoices` row the reminder PDF table - renders) so a preview needs no DB round-trip. **Note:** `RegisterReminderAsync` is create-only - (there is no `fds__setReminder` update proc), so a re-saved reminder draft does not update the - prior DB row — the primary flow (preview → confirm) flushes once immediately before finalising. + renders) so a preview needs no DB round-trip. A first save runs `fds__createReminder`; every + later save of the same session runs `fds__setReminder`, which updates the row in place (version + bump, same result shape) and refuses — `@Id` NULL → the batch throws → "nicht gespeichert" — once + the reminder is finalised or has a stored file. Flushes are serialised on the session's + `FlushGate` exactly like invoice flushes (see *Concurrency* above), so overlapping saves cannot + create two reminder rows either. - **Cache/expiry** (`ReminderDraftCache` singleton + `ReminderDraftExpiryService`) mirror the invoice ones and share the same `Fuchs:DraftEditing` TTL config. - **Signals** reuse the shared `DraftPreviewHub` + `IDraftNotifier` unchanged — the token-keyed diff --git a/Fuchs/Services/DraftFlushResult.cs b/Fuchs/Services/DraftFlushResult.cs new file mode 100644 index 0000000..dd9ed57 --- /dev/null +++ b/Fuchs/Services/DraftFlushResult.cs @@ -0,0 +1,13 @@ +namespace Fuchs.Services; + +/// +/// Outcome of a draft flush ("Zwischenspeichern", / +/// ). The draft service alone decides whether a +/// flush creates a DB row or updates the one an earlier flush created — flushes of one session are +/// serialised, so a save that overlaps the first registration of a new draft becomes an update — and +/// reports that decision here so the caller publishes the matching domain event instead of guessing +/// from the session state it saw before the flush. +/// +/// The registered data; its Id is empty when the registration failed. +/// true when the registration ran as an update of an existing DB draft, false for a create. +public sealed record DraftFlushResult(T Data, bool Updated) where T : class; diff --git a/Fuchs/Services/IInvoiceDraftService.cs b/Fuchs/Services/IInvoiceDraftService.cs index 8339587..fbae1c8 100644 --- a/Fuchs/Services/IInvoiceDraftService.cs +++ b/Fuchs/Services/IInvoiceDraftService.cs @@ -45,10 +45,13 @@ public interface IInvoiceDraftService /// /// Persists the cached session to the DB via the existing invoice registration path - /// ("Zwischenspeichern"). Sets on success. - /// Returns the registered invoice data (for the success event), or null if the token is unknown. + /// ("Zwischenspeichern"). Sets on success. Flushes of one + /// session are serialised (): an overlapping save of a + /// not-yet-registered draft waits for the first registration and then runs as an update, so a + /// double-click never creates two DB rows. Returns the registered data plus whether the flush + /// ran as an update (for the success event), or null if the token is unknown. /// - Task FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec); + Task?> FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec); /// Renders a draft PDF straight from the cached session (no client upload). Null if token unknown. Task RenderPreviewAsync(string token); diff --git a/Fuchs/Services/IReminderDraftService.cs b/Fuchs/Services/IReminderDraftService.cs index 916f079..5ac4682 100644 --- a/Fuchs/Services/IReminderDraftService.cs +++ b/Fuchs/Services/IReminderDraftService.cs @@ -43,10 +43,13 @@ public interface IReminderDraftService /// /// Persists the cached session to the DB via the existing reminder registration path - /// ("Zwischenspeichern"). Sets on success. - /// Returns the registered reminder data (for the success event), or null if the token is unknown. + /// ("Zwischenspeichern"). Sets on success. Flushes of one + /// session are serialised (): an overlapping save of a + /// not-yet-registered draft waits for the first registration and then runs as an update, so a + /// double-click never creates two DB rows. Returns the registered data plus whether the flush + /// ran as an update (for the success event), or null if the token is unknown. /// - Task FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec); + Task?> FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec); /// Renders a draft PDF straight from the cached session (no client upload). Null if token unknown. Document? RenderPreview(string token); diff --git a/Fuchs/Services/InvoiceDraftEditService.cs b/Fuchs/Services/InvoiceDraftEditService.cs index 5d85364..ad68b3e 100644 --- a/Fuchs/Services/InvoiceDraftEditService.cs +++ b/Fuchs/Services/InvoiceDraftEditService.cs @@ -60,27 +60,32 @@ public sealed class InvoiceDraftEditService : IInvoiceDraftService var session = _cache.Get(token); if (session == null) return null; - string oldValue = "", newValue = ""; - bool mutated = ApplyDelta(session, delta, ref oldValue, ref newValue); - if (!mutated) + // The editor fires patches (and state/preview fetches) concurrently for one token; without + // the lock a block.replace on one request mutates Req while another enumerates it. + lock (session.SyncRoot) { - _logger.LogDebug("Draft {Token}: no-op patch target={Target} ref={Ref}", token, delta.Target, delta.Ref); + string oldValue = "", newValue = ""; + bool mutated = ApplyDelta(session, delta, ref oldValue, ref newValue); + if (!mutated) + { + _logger.LogDebug("Draft {Token}: no-op patch target={Target} ref={Ref}", token, delta.Target, delta.Ref); + return session; + } + + Refresh(session); + session.Version++; + session.History.Add(new ChangeHistoryEntry + { + UserAccountId = session.UserAccountId, + Target = delta.Target, + Ref = delta.Ref, + OldValue = oldValue, + NewValue = newValue, + Version = session.Version + }); + _cache.Set(session); return session; } - - Refresh(session); - session.Version++; - session.History.Add(new ChangeHistoryEntry - { - UserAccountId = session.UserAccountId, - Target = delta.Target, - Ref = delta.Ref, - OldValue = oldValue, - NewValue = newValue, - Version = session.Version - }); - _cache.Set(session); - return session; } /// @@ -424,33 +429,40 @@ public sealed class InvoiceDraftEditService : IInvoiceDraftService } // ── View state / history ──────────────────────────────────────────────── + /// + /// The result is serialised by the caller after this returns, so everything mutable is a + /// snapshot taken under the session lock — never a live reference into the session. + /// public object BuildState(InvoiceDraftSession session) { session.Touch(); - return new + lock (session.SyncRoot) { - token = session.Token, - version = session.Version, - invid = session.InvId, - isDraft = session.IsDraft, - admin = session.Admin, - @new = session.New, - req = session.Req, - sums = new + return new { - total_net = session.Sums.TotalNet, - total_gross = session.Sums.TotalGross, - total_vat = session.Sums.TotalVat, - service_net = session.Sums.ServiceNet, - service_vat = session.Sums.ServiceVat, - vat = session.Sums.VatByRate, - block_net = session.Sums.NetByBlock - }, - validation = session.ValidationMessages.Select(v => new { field = v.Field, severity = v.Severity, message = v.Message }), - historyCount = session.History.Count, - notes = BuildNotes(session), - setDisplay = BuildSetDisplay(session) - }; + token = session.Token, + version = session.Version, + invid = session.InvId, + isDraft = session.IsDraft, + admin = session.Admin.DeepClone(), + @new = session.New.DeepClone(), + req = session.Req.DeepClone(), + sums = new + { + total_net = session.Sums.TotalNet, + total_gross = session.Sums.TotalGross, + total_vat = session.Sums.TotalVat, + service_net = session.Sums.ServiceNet, + service_vat = session.Sums.ServiceVat, + vat = new Dictionary(session.Sums.VatByRate), + block_net = new Dictionary(session.Sums.NetByBlock) + }, + validation = session.ValidationMessages.Select(v => new { field = v.Field, severity = v.Severity, message = v.Message }).ToList(), + historyCount = session.History.Count, + notes = BuildNotes(session).ToList(), + setDisplay = BuildSetDisplay(session) + }; + } } /// @@ -497,34 +509,60 @@ public sealed class InvoiceDraftEditService : IInvoiceDraftService .Select(n => new { text = n.Text, style = n.Style }); } - public IReadOnlyList GetHistory(string token) => - _cache.Get(token)?.History ?? (IReadOnlyList)Array.Empty(); + public IReadOnlyList GetHistory(string token) + { + var session = _cache.Get(token); + if (session == null) return Array.Empty(); + lock (session.SyncRoot) return session.History.ToList(); + } // ── Flush / preview ──────────────────────────────────────────────────────── - public async Task FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec) + public async Task?> FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec) { var session = _cache.Get(token); if (session == null) return null; - var fds = BuildFdsData(session); - bool change = !string.IsNullOrEmpty(session.InvId); - var reg = await _invoices.RegisterInvoiceAsync(fds, change, session.InvId, userAccountId, dbSec); - if (!string.IsNullOrEmpty(reg.Id)) + // One flush at a time per session: the create/update decision below is read-then-write across + // an await, so two overlapping saves of a new draft would both see an empty InvId and both + // call fds__createInvoice. The second save waits here, then runs as an update of the row the + // first one created (with its own, newer snapshot). + await session.FlushGate.WaitAsync(); + try { - session.InvId = reg.Id; - _cache.Set(session); - _logger.LogInformation("Draft {Token} flushed to DB invoice {InvId} (change={Change}, user={User})", - token, reg.Id, change, userAccountId); + FdsInvoiceData fds; + string invId; + lock (session.SyncRoot) + { + fds = BuildFdsData(session); // deep-cloned snapshot — the await below runs outside the lock + invId = session.InvId; + } + bool change = !string.IsNullOrEmpty(invId); + var reg = await _invoices.RegisterInvoiceAsync(fds, change, invId, userAccountId, dbSec); + if (!string.IsNullOrEmpty(reg.Id)) + { + lock (session.SyncRoot) session.InvId = reg.Id; + _cache.Set(session); + _logger.LogInformation("Draft {Token} flushed to DB invoice {InvId} (change={Change}, user={User})", + token, reg.Id, change, userAccountId); + } + return new DraftFlushResult(reg, change); + } + finally + { + session.FlushGate.Release(); } - return reg; } public async Task RenderPreviewAsync(string token) { var session = _cache.Get(token); if (session == null) return null; - var fds = BuildFdsData(session); - fds.InvoiceRegistration = SynthesizeRegistration(session); + FdsInvoiceData fds; + lock (session.SyncRoot) + { + fds = BuildFdsData(session); + fds.InvoiceRegistration = SynthesizeRegistration(session); + } fds.IsDraft = true; return await _invoices.GenerateInvoicePdfAsync(fds, draft: true); } diff --git a/Fuchs/Services/ReminderDraftEditService.cs b/Fuchs/Services/ReminderDraftEditService.cs index c608039..99142b8 100644 --- a/Fuchs/Services/ReminderDraftEditService.cs +++ b/Fuchs/Services/ReminderDraftEditService.cs @@ -59,27 +59,30 @@ public sealed class ReminderDraftEditService : IReminderDraftService var session = _cache.Get(token); if (session == null) return null; - string oldValue = "", newValue = ""; - bool mutated = ApplyDelta(session, delta, ref oldValue, ref newValue); - if (!mutated) + lock (session.SyncRoot) // concurrent patch/state/preview requests for one token must not interleave { - _logger.LogDebug("Reminder draft {Token}: no-op patch target={Target} ref={Ref}", token, delta.Target, delta.Ref); + string oldValue = "", newValue = ""; + bool mutated = ApplyDelta(session, delta, ref oldValue, ref newValue); + if (!mutated) + { + _logger.LogDebug("Reminder draft {Token}: no-op patch target={Target} ref={Ref}", token, delta.Target, delta.Ref); + return session; + } + + Refresh(session); + session.Version++; + session.History.Add(new ChangeHistoryEntry + { + UserAccountId = session.UserAccountId, + Target = delta.Target, + Ref = delta.Ref, + OldValue = oldValue, + NewValue = newValue, + Version = session.Version + }); + _cache.Set(session); return session; } - - Refresh(session); - session.Version++; - session.History.Add(new ChangeHistoryEntry - { - UserAccountId = session.UserAccountId, - Target = delta.Target, - Ref = delta.Ref, - OldValue = oldValue, - NewValue = newValue, - Version = session.Version - }); - _cache.Set(session); - return session; } /// @@ -144,24 +147,27 @@ public sealed class ReminderDraftEditService : IReminderDraftService public object BuildState(ReminderDraftSession session) { session.Touch(); - return new + lock (session.SyncRoot) // serialised by the caller later — hand out snapshots, not live references { - token = session.Token, - version = session.Version, - remid = session.RemId, - isDraft = session.IsDraft, - @new = session.New, - rem = session.Rem, - sums = new + return new { - amount_total = session.Sums.AmountTotal, - amount_payed = session.Sums.AmountPayed, - amount_open = session.Sums.AmountOpen - }, - validation = session.ValidationMessages.Select(v => new { field = v.Field, severity = v.Severity, message = v.Message }), - historyCount = session.History.Count, - notes = BuildNotes(session) - }; + token = session.Token, + version = session.Version, + remid = session.RemId, + isDraft = session.IsDraft, + @new = session.New.DeepClone(), + rem = session.Rem.DeepClone(), + sums = new + { + amount_total = session.Sums.AmountTotal, + amount_payed = session.Sums.AmountPayed, + amount_open = session.Sums.AmountOpen + }, + validation = session.ValidationMessages.Select(v => new { field = v.Field, severity = v.Severity, message = v.Message }).ToList(), + historyCount = session.History.Count, + notes = BuildNotes(session) + }; + } } /// @@ -177,34 +183,58 @@ public sealed class ReminderDraftEditService : IReminderDraftService return new { intro, closing }; } - public IReadOnlyList GetHistory(string token) => - _cache.Get(token)?.History ?? (IReadOnlyList)Array.Empty(); + public IReadOnlyList GetHistory(string token) + { + var session = _cache.Get(token); + if (session == null) return Array.Empty(); + lock (session.SyncRoot) return session.History.ToList(); + } // ── Flush / preview ──────────────────────────────────────────────────────── - public async Task FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec) + public async Task?> FlushToDbAsync(string token, string userAccountId, DatabaseSecurity dbSec) { var session = _cache.Get(token); if (session == null) return null; - var fds = BuildReminderData(session); - bool change = !string.IsNullOrEmpty(session.RemId); - var reg = await _reminders.RegisterReminderAsync(fds, change, session.RemId, userAccountId, dbSec); - if (!string.IsNullOrEmpty(reg.Id)) + // One flush at a time per session (see InvoiceDraftEditService.FlushToDbAsync): two overlapping + // saves of a new draft would otherwise both see an empty RemId and both call fds__createReminder. + await session.FlushGate.WaitAsync(); + try { - session.RemId = reg.Id; - _cache.Set(session); - _logger.LogInformation("Reminder draft {Token} flushed to DB reminder {RemId} (change={Change}, user={User})", - token, reg.Id, change, userAccountId); + FdsReminderData fds; + string remId; + lock (session.SyncRoot) + { + fds = BuildReminderData(session); // deep-cloned snapshot — the await below runs outside the lock + remId = session.RemId; + } + bool change = !string.IsNullOrEmpty(remId); + var reg = await _reminders.RegisterReminderAsync(fds, change, remId, userAccountId, dbSec); + if (!string.IsNullOrEmpty(reg.Id)) + { + lock (session.SyncRoot) session.RemId = reg.Id; + _cache.Set(session); + _logger.LogInformation("Reminder draft {Token} flushed to DB reminder {RemId} (change={Change}, user={User})", + token, reg.Id, change, userAccountId); + } + return new DraftFlushResult(reg, change); + } + finally + { + session.FlushGate.Release(); } - return reg; } public Document? RenderPreview(string token) { var session = _cache.Get(token); if (session == null) return null; - var fds = BuildReminderData(session); - fds.ReminderRegistration = SynthesizeRegistration(session); + FdsReminderData fds; + lock (session.SyncRoot) + { + fds = BuildReminderData(session); + fds.ReminderRegistration = SynthesizeRegistration(session); + } fds.IsDraft = true; return _reminders.GenerateReminderPdf(fds, draft: true); } diff --git a/Fuchs/Services/ReminderService.cs b/Fuchs/Services/ReminderService.cs index e61eba2..938b0f4 100644 --- a/Fuchs/Services/ReminderService.cs +++ b/Fuchs/Services/ReminderService.cs @@ -63,6 +63,25 @@ public class ReminderService : IReminderService return rem; } + /// + /// The save batch for : fds__createReminder for a new + /// draft, fds__setReminder (update in place, same result row) for a re-saved one. Either + /// proc leaves @Id NULL when it did not save — the set proc refuses a finalised reminder + /// or one with a stored file — so the guard turns that into an error instead of an empty result + /// row the caller would read as "saved". + /// + internal static string BuildRegisterReminderSql(bool change) + { + var sqlParts = new List { "DECLARE @Id varchar(10);" }; + sqlParts.Add(change + ? "EXECUTE [dbo].[fds__setReminder] @RemId, @type, @amount, @amount_payed, @SendToAddress, @SendToEmail, @subject, @text, @authuser, @Id OUTPUT;" + : "EXECUTE [dbo].[fds__createReminder] @InvId, @type, @amount, @amount_payed, @SendToAddress, @SendToEmail, @subject, @text, @authuser, @Id OUTPUT;"); + sqlParts.Add(change + ? "IF @Id IS NULL THROW 50001, N'fds__setReminder did not return a reminder id - reminder draft not saved (finalised, has a file, or unknown).', 1;" + : "IF @Id IS NULL THROW 50001, N'fds__createReminder did not return a reminder id - reminder not saved.', 1;"); + return string.Join("\n", sqlParts); + } + public async Task RegisterReminderAsync(FdsReminderData reminder, bool change, string remId, string userAccountId, DatabaseSecurity dbSec) { @@ -82,20 +101,17 @@ public class ReminderService : IReminderService SQL_NVarChar("text", reminder.NewValues?.nz("text") ?? "", dbNull_IfEmpty: true) }; - var sqlParts = new List { "DECLARE @Id varchar(10);" }; - if (!change || string.IsNullOrEmpty(remId)) - sqlParts.Add("EXECUTE [dbo].[fds__createReminder] @InvId, @type, @amount, @amount_payed, @SendToAddress, @SendToEmail, @subject, @text, @authuser, @Id OUTPUT;"); - else - pl.Add(SQL_VarChar("RemId", remId)); + change = change && !string.IsNullOrEmpty(remId); + if (change) pl.Add(SQL_VarChar("RemId", remId)); - var remdset = await getSQLDataSet_async(string.Join("\n", sqlParts), + var remdset = await getSQLDataSet_async(BuildRegisterReminderSql(change), Conn, pl, tablenames: new[] { "rem" }, Security: dbSec, options: new FIS_SQLOptions()); if (!string.IsNullOrEmpty(remdset.Exception)) _logger.LogError("RegisterReminderAsync sql exception: {Ex}", remdset.Exception); reminder.ReminderRegistration = new GenericObjectDictionary(remdset.Table("rem").FirstRow.toObjectDictionary()); - _logger.LogInformation("RegisterReminderAsync registered id={Id}", reminder.Id); + _logger.LogInformation("RegisterReminderAsync registered id={Id} (change={Change})", reminder.Id, change); return reminder; } diff --git a/Fuchs/code/InvoiceDraftSession.cs b/Fuchs/code/InvoiceDraftSession.cs index 727da86..32a808a 100644 --- a/Fuchs/code/InvoiceDraftSession.cs +++ b/Fuchs/code/InvoiceDraftSession.cs @@ -60,6 +60,24 @@ public sealed class InvoiceDraftSession /// Automatic change history, appended on every applied patch. Cache-only (never persisted). public List History { get; } = new(); + /// + /// Per-session lock. The editor fires patches, state fetches and previews concurrently for the + /// same token, and the payload (/JObject, the + /// history list) is not thread-safe — the draft service takes this lock around every read and + /// write of the payload, and hands out deep-cloned snapshots for anything serialised later. + /// + public object SyncRoot { get; } = new(); + + /// + /// Serialises flushes ("Zwischenspeichern") of this session. A flush reads , + /// awaits the DB registration and then writes the id back; cannot be held + /// across that await, so without this gate two overlapping saves of a not-yet-registered draft + /// both see an empty id and both create a DB row. Only ever awaited (never held while taking + /// ), so there is no lock-order inversion. Not disposed with the session on + /// purpose: a owns no kernel handle unless its wait handle is touched. + /// + public SemaphoreSlim FlushGate { get; } = new(1, 1); + public void Touch() => LastAccessUtc = DateTime.UtcNow; } diff --git a/Fuchs/code/InvoiceSetStructure.cs b/Fuchs/code/InvoiceSetStructure.cs index ba6473d..d052bab 100644 --- a/Fuchs/code/InvoiceSetStructure.cs +++ b/Fuchs/code/InvoiceSetStructure.cs @@ -1,4 +1,4 @@ -using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Linq; namespace Fuchs.intranet; diff --git a/Fuchs/code/ReminderDraftSession.cs b/Fuchs/code/ReminderDraftSession.cs index 2654042..452eead 100644 --- a/Fuchs/code/ReminderDraftSession.cs +++ b/Fuchs/code/ReminderDraftSession.cs @@ -60,6 +60,23 @@ public sealed class ReminderDraftSession /// Automatic change history, appended on every applied patch. Cache-only (never persisted). public List History { get; } = new(); + /// + /// Per-session lock (see ): the draft service takes it + /// around every read and write of the non-thread-safe payload, so concurrent patch/state/preview + /// requests for the same token cannot interleave. + /// + public object SyncRoot { get; } = new(); + + /// + /// Serialises flushes ("Zwischenspeichern") of this session. A flush reads , + /// awaits the DB registration and then writes the id back; cannot be held + /// across that await, so without this gate two overlapping saves of a not-yet-registered draft + /// both see an empty id and both create a DB row. Only ever awaited (never held while taking + /// ), so there is no lock-order inversion. Not disposed with the session on + /// purpose: a owns no kernel handle unless its wait handle is touched. + /// + public SemaphoreSlim FlushGate { get; } = new(1, 1); + public void Touch() => LastAccessUtc = DateTime.UtcNow; } diff --git a/Fuchs_Database/FuchsDatabase.sqlproj b/Fuchs_Database/FuchsDatabase.sqlproj index d2332d1..0a4c0d3 100644 --- a/Fuchs_Database/FuchsDatabase.sqlproj +++ b/Fuchs_Database/FuchsDatabase.sqlproj @@ -358,6 +358,7 @@ + diff --git a/Fuchs_Database/dbo/Functions/AddBusinessDays.sql b/Fuchs_Database/dbo/Functions/AddBusinessDays.sql index 6250e5a..4c0127e 100644 --- a/Fuchs_Database/dbo/Functions/AddBusinessDays.sql +++ b/Fuchs_Database/dbo/Functions/AddBusinessDays.sql @@ -1,5 +1,9 @@ -CREATE FUNCTION[dbo].[AddBusinessDays](@Date DATE,@n INT) -RETURNS DATE AS +-- INLINE = OFF: scalar UDF inlining of this @@DATEFIRST arithmetic (reached via date_add / +-- date_addcustom) killed dashboard widget queries with error 596 ("session is in the kill state"). +CREATE FUNCTION[dbo].[AddBusinessDays](@Date DATE,@n INT) +RETURNS DATE +WITH INLINE = OFF +AS BEGIN DECLARE @d INT,@f INT,@DW INT; SET @f=CAST(abs(1^SIGN(DATEPART(DW, @Date)-(7-@@DATEFIRST))) AS BIT) diff --git a/Fuchs_Database/dbo/Functions/date_add.sql b/Fuchs_Database/dbo/Functions/date_add.sql index 8faa2b1..ba1e9c1 100644 --- a/Fuchs_Database/dbo/Functions/date_add.sql +++ b/Fuchs_Database/dbo/Functions/date_add.sql @@ -2,6 +2,9 @@ -- Author: -- Create date: -- Description: +-- INLINE = OFF: scalar UDF inlining of this IF/nested-UDF body (-> AddBusinessDays) +-- killed the dashboard widget queries with error 596 ("session is in the kill +-- state", class 21). Called with constant arguments, so per-row cost is negligible. -- ============================================= CREATE FUNCTION [dbo].[date_add] ( @@ -10,6 +13,7 @@ CREATE FUNCTION [dbo].[date_add] ,@shift_to_first bit = 1 ) RETURNS date +WITH INLINE = OFF AS BEGIN DECLARE @Ret date = @date; diff --git a/Fuchs_Database/dbo/Functions/date_addcustom.sql b/Fuchs_Database/dbo/Functions/date_addcustom.sql index c99e92f..22b1f43 100644 --- a/Fuchs_Database/dbo/Functions/date_addcustom.sql +++ b/Fuchs_Database/dbo/Functions/date_addcustom.sql @@ -2,6 +2,8 @@ -- Author: -- Create date: -- Description: +-- INLINE = OFF: scalar UDF inlining (nested -> AddBusinessDays) killed the dashboard +-- widget queries with error 596 ("session is in the kill state", class 21). -- ============================================= CREATE FUNCTION [dbo].[date_addcustom] ( @@ -9,6 +11,7 @@ CREATE FUNCTION [dbo].[date_addcustom] ,@phrase varchar(10) ) RETURNS date +WITH INLINE = OFF AS BEGIN DECLARE @returndate date, @numpart integer = TRY_PARSE(left(@phrase, patindex('%[^0-9]%', @phrase + '.') - 1) as integer), @defpart varchar(10) = LOWER(RIGHT(@phrase, LEN(@phrase) - patindex('%[^0-9]%', @phrase + '.') + 1)); diff --git a/Fuchs_Database/dbo/Stored Procedures/fds__setReminder.sql b/Fuchs_Database/dbo/Stored Procedures/fds__setReminder.sql new file mode 100644 index 0000000..5503b17 --- /dev/null +++ b/Fuchs_Database/dbo/Stored Procedures/fds__setReminder.sql @@ -0,0 +1,66 @@ +-- ============================================= +-- Description: Updates an unfinalised reminder draft in place ("Zwischenspeichern" of a draft +-- that fds__createReminder already registered) — the reminder counterpart of +-- fds__setInvoice. Only a draft that is not finalised and has no stored file is +-- touched; otherwise no row is updated, @Id stays NULL and the caller reports the +-- save as failed instead of silently doing nothing. The reminder stays bound to its +-- invoice: @InvId is not updatable. +-- Returns the same single row shape as fds__createReminder. +-- ============================================= +CREATE PROCEDURE [dbo].[fds__setReminder] + @RemId varchar(10) + , @type varchar(3) + , @amount numeric(10,3) + , @amount_payed numeric(10,3) + , @SendToAddress [nvarchar](1000) + , @SendToEmail [nvarchar](255) + , @subject nvarchar(255) + , @text nvarchar(2000) + , @authuser varchar(25) + , @Id varchar(10) OUT +AS +BEGIN + SET NOCOUNT ON; + + IF [dbo].[fis_getModuleAuth]('fds_reminder', @authuser) < 2 + THROW 60000, N'not authorized', 1; + + INSERT INTO [dbo].[fds__admin_activity] ([activity] ,[authuser] ,[info]) + VALUES ('fds__setReminder' ,@authuser , 'remid: ' + ISNULL(@RemId, '')); + + DECLARE @now datetime = GETUTCDATE(); + DECLARE @OUT [dbo].[fds__tt__reminder_core]; + + UPDATE [dbo].[fds__reminder] + SET [Version] = [Version] + 1 + ,[type] = ISNULL(@type, [type]) + ,[amount] = @amount + ,[amount_payed] = @amount_payed + ,[SendToAddress] = @SendToAddress + ,[SendToEmail] = @SendToEmail + ,[subject] = @subject + ,[text] = @text + ,[DateModified] = @now + ,[UserModified] = @authuser + OUTPUT inserted.* + INTO @OUT + WHERE [Id] = @RemId + AND [DateFinalized] IS NULL -- IsFinal = 0 + AND [file] IS NULL; + + SELECT TOP(1) @Id = [Id] FROM @OUT; -- stays NULL when nothing matched + + With inv as( + select TOP(1) * FROM [dbo].[fds__invoices] WHERE [id] = (SELECT TOP(1) [InvId] FROM @OUT) + ) + SELECT TOP(1) + o.* + , [InvoiceId] = inv.[InvoiceId] + , [InvoiceDate] = [dbo].[fds__fn_invoiceDate](inv.[DateFinalized], inv.[DateCreated]) + , [hasFile] = CAST ( CASE WHEN o.[file] is null THEN 0 ELSE 1 END as bit) + , [UserNameFinalized] = [dbo].[fis_admin_getUserName_byID](o.[UserFinalized]) + , [UserEmailFinalized] = [dbo].[fis_admin_getUserEmail_byID](o.[UserFinalized]) + from @OUT as o + join inv on o.[InvId] = inv.[Id]; + +END