diff --git a/backend/src/BigRegister.Api/Data/ApplicationStore.cs b/backend/src/BigRegister.Api/Data/ApplicationStore.cs index 3eb3829..b8e291e 100644 --- a/backend/src/BigRegister.Api/Data/ApplicationStore.cs +++ b/backend/src/BigRegister.Api/Data/ApplicationStore.cs @@ -33,6 +33,13 @@ public sealed class Aanvraag /// endpoint does, via ) to keep the seam's write /// surface at "return data", not "reach into another store". public string? ZaakUrl { get; set; } + + /// WP-60: non-null means the ZGW side of this submit (or its document link) did not + /// complete — the local aanvraag is authoritative and is NOT rolled back (that risks an + /// orphan zaak if the failure landed after the zaak POST succeeded). The zaak, if it exists, + /// is re-findable by identificatie == Referentie. Cleared by a future repair path; + /// none exists yet (see openzaak-integration.md's "Write resilience" section). + public string? ZgwError { get; set; } } /// @@ -193,4 +200,18 @@ public static class ApplicationStore db.SaveChanges(); } } + + /// Flag (or clear, once a repair path exists) that this aanvraag's ZGW write did + /// not complete — see . No-op if the aanvraag is gone. + public static void SetZgwError(string id, string? error) + { + lock (_gate) + { + using var db = Db.Create(); + var a = db.Applications.Find(id); + if (a is null) return; + a.ZgwError = error; + db.SaveChanges(); + } + } } diff --git a/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.Designer.cs b/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.Designer.cs new file mode 100644 index 0000000..749c0c0 --- /dev/null +++ b/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.Designer.cs @@ -0,0 +1,282 @@ +// +using System; +using BigRegister.Api.Data; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace BigRegister.Api.Data.Migrations +{ + [DbContext(typeof(AppDbContext))] + [Migration("20260730155659_ZgwSyncError")] + partial class ZgwSyncError + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder.HasAnnotation("ProductVersion", "10.0.9"); + + modelBuilder.Entity("BigRegister.Api.Data.Aanvraag", b => + { + b.Property("Id") + .HasColumnType("TEXT"); + + b.Property("AutoApprovable") + .HasColumnType("INTEGER"); + + b.Property("CreatedAt") + .HasColumnType("TEXT"); + + b.Property("DocumentIds") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Draft") + .HasColumnType("TEXT"); + + b.Property("Owner") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Reden") + .HasColumnType("TEXT"); + + b.Property("Referentie") + .HasColumnType("TEXT"); + + b.Property("StepCount") + .HasColumnType("INTEGER"); + + b.Property("StepIndex") + .HasColumnType("INTEGER"); + + b.Property("Submitted") + .HasColumnType("INTEGER"); + + b.Property("SubmittedAt") + .HasColumnType("TEXT"); + + b.Property("Type") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("UpdatedAt") + .HasColumnType("TEXT"); + + b.Property("ZaakUrl") + .HasColumnType("TEXT"); + + b.Property("ZgwError") + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.ToTable("Applications"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.AuditEntry", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("Action") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Actor") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("At") + .HasColumnType("TEXT"); + + b.Property("CategoryId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("DocumentId") + .IsRequired() + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.ToTable("AuditEntries"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.AuthzAuditEntry", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("Action") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("At") + .HasColumnType("TEXT"); + + b.Property("CorrelationId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Decision") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Resource") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Role") + .IsRequired() + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.ToTable("AuthzAudit"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.BriefEntity", b => + { + b.Property("BriefId") + .HasColumnType("TEXT"); + + b.Property("ArchivedHtml") + .HasColumnType("TEXT"); + + b.Property("Beroep") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("DrafterId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Owner") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Placeholders") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Sections") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("SentOrgTemplateVersion") + .HasColumnType("INTEGER"); + + b.Property("Status") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("SubOrgId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("TemplateId") + .IsRequired() + .HasColumnType("TEXT"); + + b.HasKey("BriefId"); + + b.HasIndex("Owner") + .IsUnique(); + + b.ToTable("Briefs"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.FeatureFlagEntity", b => + { + b.Property("Key") + .HasColumnType("TEXT"); + + b.Property("Enabled") + .HasColumnType("INTEGER"); + + b.HasKey("Key"); + + b.ToTable("FeatureFlags"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.OrgTemplateEntity", b => + { + b.Property("SubOrgId") + .HasColumnType("TEXT"); + + b.Property("Draft") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("History") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("PublishedVersion") + .HasColumnType("INTEGER"); + + b.HasKey("SubOrgId"); + + b.ToTable("OrgTemplates"); + }); + + modelBuilder.Entity("BigRegister.Api.Data.StoredDocument", b => + { + b.Property("DocumentId") + .HasColumnType("TEXT"); + + b.Property("CategoryId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Content") + .IsRequired() + .HasColumnType("BLOB"); + + b.Property("ContentType") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("DrcUrl") + .HasColumnType("TEXT"); + + b.Property("FileName") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Linked") + .HasColumnType("INTEGER"); + + b.Property("LocalId") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Owner") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("SizeBytes") + .HasColumnType("INTEGER"); + + b.Property("UploadedAt") + .HasColumnType("TEXT"); + + b.Property("WizardId") + .IsRequired() + .HasColumnType("TEXT"); + + b.HasKey("DocumentId"); + + b.ToTable("Documents"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.cs b/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.cs new file mode 100644 index 0000000..4238f22 --- /dev/null +++ b/backend/src/BigRegister.Api/Data/Migrations/20260730155659_ZgwSyncError.cs @@ -0,0 +1,28 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace BigRegister.Api.Data.Migrations +{ + /// + public partial class ZgwSyncError : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "ZgwError", + table: "Applications", + type: "TEXT", + nullable: true); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropColumn( + name: "ZgwError", + table: "Applications"); + } + } +} diff --git a/backend/src/BigRegister.Api/Data/Migrations/AppDbContextModelSnapshot.cs b/backend/src/BigRegister.Api/Data/Migrations/AppDbContextModelSnapshot.cs index 1ff72c5..909dace 100644 --- a/backend/src/BigRegister.Api/Data/Migrations/AppDbContextModelSnapshot.cs +++ b/backend/src/BigRegister.Api/Data/Migrations/AppDbContextModelSnapshot.cs @@ -67,6 +67,9 @@ namespace BigRegister.Api.Data.Migrations b.Property("ZaakUrl") .HasColumnType("TEXT"); + b.Property("ZgwError") + .HasColumnType("TEXT"); + b.HasKey("Id"); b.ToTable("Applications"); diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index fbda8bd..584f609 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -58,9 +58,12 @@ if (zgw.Enabled) { builder.Services.AddSingleton(zgw); builder.Services.AddSingleton(); - builder.Services.AddHttpClient(); + // WP-60: a bounded client timeout matters once ZgwHttpClient retries — without one, the + // sources' sync-over-async call (no CancellationToken threaded through) could block a + // thread-pool thread for HttpClient's 100s default times 3 attempts. + builder.Services.AddHttpClient(c => c.Timeout = TimeSpan.FromSeconds(15)); // WP-51: the documents (Documenten API / DRC) seam — same pattern as IZaakSource above. - builder.Services.AddHttpClient(); + builder.Services.AddHttpClient(c => c.Timeout = TimeSpan.FromSeconds(15)); } else { @@ -351,13 +354,38 @@ api.MapPost("/applications/{id}/submit", (string id, SubmitApplicationRequest re // in OpenZaak and maps its result back into this same response shape (ADR-0001/ADR-0005: // zero FE contract change either way). WP-53: the caller is threaded through so the minted // ZGW JWT's user_id/user_representation reflect the acting citizen, not a static config value. - var (referentie, status, zaakUrl) = zaken.CreateZaak(submitted, DateTimeOffset.UtcNow, ctx.Caller()); - if (zaakUrl is not null) ApplicationStore.SetZaakUrl(id, zaakUrl); + // + // WP-60: the local submit above already committed — it is never rolled back on a ZGW + // failure (an orphan zaak from a rolled-back-then-retried submit is worse than a flagged + // one, see openzaak-integration.md's "Write resilience" section). Each ZGW half is caught + // separately so a create-zaak failure doesn't also skip the (still-local) document link. + var referentie = submitted.Referentie!; + var status = submitted.ToStatusDto(DateTimeOffset.UtcNow); + string? zaakUrl = null; + try + { + (referentie, status, zaakUrl) = zaken.CreateZaak(submitted, DateTimeOffset.UtcNow, ctx.Caller()); + if (zaakUrl is not null) ApplicationStore.SetZaakUrl(id, zaakUrl); + } + catch (Exception ex) + { + RecordZgwDivergence(ctx, id, referentie, ex); + } // WP-51: link the submitted documents to the zaak — LocalDocumentSource is exactly the // DocumentStore.Link call this used to make inline; OpenZaakDocumentSource additionally // POSTs a zaakinformatieobject per document, now that the zaak (zaakUrl) exists. - if (documentIds is not null) documents.LinkToZaak(documentIds, zaakUrl, ctx.Caller()); + if (documentIds is not null) + { + try + { + documents.LinkToZaak(documentIds, zaakUrl, ctx.Caller()); + } + catch (Exception ex) + { + RecordZgwDivergence(ctx, id, referentie, ex); + } + } return Results.Ok(new SubmitApplicationResponse(referentie, status)); }) @@ -676,6 +704,18 @@ void AuditAuthz(HttpContext ctx, string action, string resource, bool allowed, P AuthzAuditStore.Record(action, resource, allowed, principal.Role.ToString(), cid); } +// WP-60: the local write already committed — this records that its ZGW counterpart didn't, +// rather than letting the two sides diverge silently (openzaak-integration.md's "Write +// resilience" section). Same audit trail AuditAuthz writes to (/beheer/audit), so a +// divergence is visible next to every other decision, not a separate mechanism. +void RecordZgwDivergence(HttpContext ctx, string id, string referentie, Exception ex) +{ + app.Logger.LogError(ex, "zgw divergence aanvraag={Id} reference={Reference}", id, referentie); + ApplicationStore.SetZgwError(id, ex.Message); + var cid = ctx.Items.TryGetValue("CorrelationId", out var v) ? (string)v! : "none"; + AuthzAuditStore.Record("zgw:divergence", referentie, allowed: false, Authz.ResolvePrincipal(ctx).Role.ToString(), cid); +} + // Keep the last `keep` characters, mask the rest — mirrors the FE maskTail // (src/app/shared/ui/debug-state/mask.ts) so wire redaction and the dev panel agree. static string MaskTail(string value, int keep) => diff --git a/backend/src/BigRegister.Api/Zgw/OpenZaakDocumentSource.cs b/backend/src/BigRegister.Api/Zgw/OpenZaakDocumentSource.cs index 6a8f63d..47a22a4 100644 --- a/backend/src/BigRegister.Api/Zgw/OpenZaakDocumentSource.cs +++ b/backend/src/BigRegister.Api/Zgw/OpenZaakDocumentSource.cs @@ -4,6 +4,7 @@ using BigRegister.Api.Contracts; using BigRegister.Api.Data; using BigRegister.Domain.Authorization; using BigRegister.Stamdata; +using Microsoft.Extensions.Logging; namespace BigRegister.Api.Zgw; @@ -20,7 +21,9 @@ namespace BigRegister.Api.Zgw; /// — creating a document needs write scope on Documenten; /// linking one to a zaak needs write scope on Zaken (the zaakinformatieobject resource). /// -public sealed class OpenZaakDocumentSource(HttpClient http, ZgwTokenProvider tokens, ZgwOptions options) : IDocumentSource +public sealed class OpenZaakDocumentSource( + HttpClient http, ZgwTokenProvider tokens, ZgwOptions options, ILogger? log = null) + : IDocumentSource { private readonly ZgwHttpClient zgw = new(http, tokens); @@ -41,37 +44,54 @@ public sealed class OpenZaakDocumentSource(HttpClient http, ZgwTokenProvider tok UploadAsync(localId, categoryId, wizardId, fileName, contentType, content, caller) .GetAwaiter().GetResult(); + // WP-60: once DocumentStore.Add (below) has committed, the local document is the record of + // truth (per the class doc above) — a ZGW failure past that point is caught, logged, and + // leaves DrcUrl null rather than throwing. DrcUrl == null is already the meaningful "not + // registered in ZGW yet" detector LinkToZaak skips on, so no separate flag column is needed + // here the way ApplicationStore.ZgwError is for the zaak side (see openzaak-integration.md's + // "Write resilience" section for why the two write paths differ). private async Task UploadAsync( string localId, string categoryId, string wizardId, string fileName, string contentType, byte[] content, CallerIdentity caller) { var doc = DocumentStore.Add(localId, categoryId, wizardId, fileName, contentType, content, caller.Bsn); - if (!options.InformatieobjecttypeUrls.TryGetValue(categoryId, out var informatieobjecttypeUrl)) - throw new InvalidOperationException( - $"Zgw:InformatieobjecttypeUrls has no entry for category '{categoryId}'."); + try + { + if (!options.InformatieobjecttypeUrls.TryGetValue(categoryId, out var informatieobjecttypeUrl)) + throw new InvalidOperationException( + $"Zgw:InformatieobjecttypeUrls has no entry for category '{categoryId}'."); - var eio = await zgw.PostAsync($"{options.DrcBaseUrl}/enkelvoudiginformatieobjecten", new CreateEioRequest( - Bronorganisatie: options.Bronorganisatie, - Creatiedatum: DateOnly.FromDateTime(doc.UploadedAt.UtcDateTime), - Titel: fileName, - Auteur: options.UserRepresentation, - Taal: "nld", - Formaat: contentType, - Bestandsnaam: fileName, - Inhoud: Convert.ToBase64String(content), - Informatieobjecttype: informatieobjecttypeUrl, - Identificatie: doc.DocumentId, - Vertrouwelijkheidaanduiding: ConfidentialiteitFor(categoryId)), caller); + var eio = await zgw.PostAsync($"{options.DrcBaseUrl}/enkelvoudiginformatieobjecten", new CreateEioRequest( + Bronorganisatie: options.Bronorganisatie, + Creatiedatum: DateOnly.FromDateTime(doc.UploadedAt.UtcDateTime), + Titel: fileName, + Auteur: options.UserRepresentation, + Taal: "nld", + Formaat: contentType, + Bestandsnaam: fileName, + Inhoud: Convert.ToBase64String(content), + Informatieobjecttype: informatieobjecttypeUrl, + Identificatie: doc.DocumentId, + Vertrouwelijkheidaanduiding: ConfidentialiteitFor(categoryId)), caller); + + DocumentStore.SetDrcUrl(doc.DocumentId, eio.Url); + } + catch (Exception ex) + { + log?.LogError(ex, "zgw divergence document={DocumentId} category={CategoryId}", doc.DocumentId, categoryId); + } - DocumentStore.SetDrcUrl(doc.DocumentId, eio.Url); return new UploadResponse(doc.DocumentId, doc.LocalId); } /// Local link always happens (dual-write, same reasoning as upload); additionally, /// once a zaak exists, POST a zaakinformatieobject for every document that has a DRC url — /// documents uploaded before Zgw:Enabled was ever true (or under a config gap) simply have - /// no DrcUrl yet and are skipped, matching "nothing extra to link" for the local case. + /// no DrcUrl yet and are skipped, matching "nothing extra to link" for the local case. + /// WP-60: unlike Upload, a ZGW failure here still throws — DocumentStore.Link (the local + /// half) already ran above, so the caller (Program.cs's submit endpoint) catching this and + /// recording it as a flagged divergence is what closes the gap, not a try/catch in here. public void LinkToZaak(IReadOnlyList documentIds, string? zaakUrl, CallerIdentity caller) { DocumentStore.Link(documentIds); diff --git a/backend/src/BigRegister.Api/Zgw/OpenZaakZaakSource.cs b/backend/src/BigRegister.Api/Zgw/OpenZaakZaakSource.cs index 6b017b6..5903a4b 100644 --- a/backend/src/BigRegister.Api/Zgw/OpenZaakZaakSource.cs +++ b/backend/src/BigRegister.Api/Zgw/OpenZaakZaakSource.cs @@ -86,10 +86,12 @@ public sealed class OpenZaakZaakSource(HttpClient http, ZgwTokenProvider tokens, /// reason as (see the ponytail note there) — a submit is already a /// single request/response round trip, so no extra concurrency concern. /// - /// ponytail: no compensating transaction — if any ZGW call here throws, the aanvraag is - /// already marked Submitted locally (ApplicationStore.Submit already ran) but has no zaak. - /// Acceptable for a first write slice against a demo backend; a production arc would need a - /// retry/reconciliation story (or an outbox) before this dual-write can be trusted. + /// WP-60: still no compensating transaction — if any call here throws (after + /// 's retry gives up), the aanvraag stays Submitted locally with + /// no zaak; rolling it back risks an orphan zaak if the failure landed after the zaak POST + /// succeeded. The caller (Program.cs's submit endpoint) catches this and records it as a + /// flagged divergence (Aanvraag.ZgwError) instead of letting it fail (or diverge) silently — + /// see openzaak-integration.md's "Write resilience" section. public (string Referentie, AanvraagStatusDto Status, string? ZaakUrl) CreateZaak(Aanvraag aanvraag, DateTimeOffset now, CallerIdentity caller) => CreateZaakAsync(aanvraag, now, caller).GetAwaiter().GetResult(); diff --git a/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs b/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs index 76720a8..6890fa6 100644 --- a/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs +++ b/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs @@ -1,3 +1,4 @@ +using System.Net; using System.Net.Http.Headers; using System.Net.Http.Json; using BigRegister.Domain.Authorization; @@ -14,26 +15,77 @@ namespace BigRegister.Api.Zgw; /// internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens) { + // WP-60: bounded retry for transport-shaped failures only (gateway restarts, timeouts) — + // never a substitute for reconciliation. 3 attempts, doubling from 200ms. + private const int MaxAttempts = 3; + private static readonly TimeSpan BaseDelay = TimeSpan.FromMilliseconds(200); + public async Task GetAsync(string url, CallerIdentity? caller = null) { - using var req = new HttpRequestMessage(HttpMethod.Get, url); - Authorize(req, caller); - using var res = await http.SendAsync(req); - res.EnsureSuccessStatusCode(); + using var res = await SendWithRetryAsync(() => new HttpRequestMessage(HttpMethod.Get, url), caller); return (await res.Content.ReadFromJsonAsync()) ?? throw new InvalidOperationException($"ZGW GET {url} returned null body."); } public async Task PostAsync(string url, object body, CallerIdentity? caller = null) { - using var req = new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) }; - Authorize(req, caller); - using var res = await http.SendAsync(req); - res.EnsureSuccessStatusCode(); + using var res = await SendWithRetryAsync( + () => new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) }, caller); return (await res.Content.ReadFromJsonAsync()) ?? throw new InvalidOperationException($"ZGW POST {url} returned null body."); } + /// + /// A fresh (and JWT) per attempt — a sent request/content + /// cannot be resent. Only transport-shaped failures are retried (429/502/503/504/408, plus + /// connection errors and timeouts); 500 is deliberately excluded because it can follow a + /// partial commit on the two non-idempotent ZGW POSTs (/statussen, /rollen) and + /// retrying risks a duplicate write — the create-zaak/document POSTs are additionally + /// protected by OpenZaak's own uniqueness constraint on (bronorganisatie, identificatie). + /// A non-transient (or exhausted) failure throws with the status + a body snippet, which + /// Program.cs's submit endpoint catches and records as a flagged divergence rather + /// than letting it diverge silently (see openzaak-integration.md's "Write resilience" section). + /// + private async Task SendWithRetryAsync(Func newRequest, CallerIdentity? caller) + { + for (var attempt = 1; ; attempt++) + { + using var req = newRequest(); + Authorize(req, caller); + + HttpResponseMessage res; + try + { + res = await http.SendAsync(req); + } + catch (Exception ex) when (attempt < MaxAttempts && ex is HttpRequestException or TaskCanceledException) + { + await Task.Delay(BaseDelay * (1 << (attempt - 1))); + continue; + } + + if (res.IsSuccessStatusCode) return res; + + if (attempt < MaxAttempts && IsTransient(res.StatusCode)) + { + res.Dispose(); + await Task.Delay(BaseDelay * (1 << (attempt - 1))); + continue; + } + + var body = await res.Content.ReadAsStringAsync(); + var snippet = body.Length > 500 ? body[..500] : body; + var message = $"ZGW {req.Method} {req.RequestUri} failed: {(int)res.StatusCode} {snippet}"; + var status = res.StatusCode; + res.Dispose(); + throw new HttpRequestException(message, null, status); + } + } + + private static bool IsTransient(HttpStatusCode status) => status is + HttpStatusCode.RequestTimeout or HttpStatusCode.TooManyRequests or + HttpStatusCode.BadGateway or HttpStatusCode.ServiceUnavailable or HttpStatusCode.GatewayTimeout; + private void Authorize(HttpRequestMessage req, CallerIdentity? caller) { req.Headers.Authorization = new AuthenticationHeaderValue("Bearer", caller is null ? tokens.Mint() : tokens.Mint(caller)); diff --git a/backend/tests/BigRegister.Tests/OpenZaakDocumentSourceTests.cs b/backend/tests/BigRegister.Tests/OpenZaakDocumentSourceTests.cs index 9960ce4..0f24b91 100644 --- a/backend/tests/BigRegister.Tests/OpenZaakDocumentSourceTests.cs +++ b/backend/tests/BigRegister.Tests/OpenZaakDocumentSourceTests.cs @@ -1,3 +1,4 @@ +using System.Net; using BigRegister.Api.Data; using BigRegister.Api.Zgw; using BigRegister.Domain.Authorization; @@ -77,15 +78,38 @@ public class OpenZaakDocumentSourceTests Assert.Contains("\"vertrouwelijkheidaanduiding\":\"openbaar\"", body); } + // WP-60: once DocumentStore.Add has committed, a ZGW-side failure (config gap or transport) + // no longer throws — the local document is authoritative and DrcUrl stays null (the same + // detector LinkToZaak already skips on for pre-Zgw documents). + [Fact] - public void Upload_throws_when_the_category_has_no_configured_informatieobjecttype() + public void Upload_keeps_the_local_document_when_the_category_has_no_configured_informatieobjecttype() { var options = Options(); var handler = new ZgwStubHandler(url => throw new InvalidOperationException($"no HTTP call expected, got {url}")); var source = new OpenZaakDocumentSource(new HttpClient(handler), new ZgwTokenProvider(options), options); - Assert.Throws(() => - source.Upload("local-1", "unknown-category", "registratie", "f.pdf", "application/pdf", [1, 2, 3], Caller)); + var response = source.Upload("local-1", "unknown-category", "registratie", "f.pdf", "application/pdf", [1, 2, 3], Caller); + + Assert.Equal("local-1", response.LocalId); + Assert.Empty(handler.Requests); + Assert.Null(DocumentStore.Get(response.DocumentId)!.DrcUrl); + } + + [Fact] + public void Upload_keeps_the_local_document_and_does_not_throw_when_drc_rejects_it() + { + var options = Options(); + var handler = new ZgwStubHandler( + url => throw new InvalidOperationException($"unexpected success body requested for {url}"), + (_, _) => HttpStatusCode.BadRequest); + var source = new OpenZaakDocumentSource(new HttpClient(handler), new ZgwTokenProvider(options), options); + + var response = source.Upload("local-1", "identiteit", "registratie", "paspoort.pdf", "application/pdf", + "%PDF-1.4 fake"u8.ToArray(), Caller); + + Assert.Equal("local-1", response.LocalId); + Assert.Null(DocumentStore.Get(response.DocumentId)!.DrcUrl); } [Fact] diff --git a/backend/tests/BigRegister.Tests/OpenZaakZaakSourceTests.cs b/backend/tests/BigRegister.Tests/OpenZaakZaakSourceTests.cs index 864ed70..de997f4 100644 --- a/backend/tests/BigRegister.Tests/OpenZaakZaakSourceTests.cs +++ b/backend/tests/BigRegister.Tests/OpenZaakZaakSourceTests.cs @@ -1,3 +1,4 @@ +using System.Net; using BigRegister.Api.Data; using BigRegister.Api.Zgw; using BigRegister.Domain.Authorization; @@ -159,4 +160,95 @@ public class OpenZaakZaakSourceTests Assert.Throws(() => source.CreateZaak(aanvraag, DateTimeOffset.UtcNow, caller)); } + + // --- WP-60: bounded retry in ZgwHttpClient, exercised through the create-zaak write path --- + + private static (ZgwOptions options, Aanvraag aanvraag, CallerIdentity caller) CreateZaakFixture() + { + const string zaaktypeUrl = $"{ZtBase}/zaaktypen/zt-registratie"; + var options = new ZgwOptions + { + ZrcBaseUrl = ZrcBase, + ZtcBaseUrl = ZtBase, + ClientId = "c", + Secret = "s", + Bronorganisatie = "123443210", + VerantwoordelijkeOrganisatie = "123443210", + ZaaktypeUrls = new() { ["registratie"] = zaaktypeUrl }, + }; + var aanvraag = new Aanvraag { Id = "a1", Type = "registratie", Owner = "111222333", Referentie = "BIG-2026-000123" }; + var caller = new CallerIdentity(aanvraag.Owner, "Dr. Test", PrincipalRole.Drafter); + return (options, aanvraag, caller); + } + + private static string RespondFor(string zaaktypeUrl, string url) => url switch + { + _ when url == $"{ZrcBase}/zaken" => $$""" + { "url": "{{ZrcBase}}/zaken/uuid-new", "identificatie": "BIG-2026-000123", + "zaaktype": "{{zaaktypeUrl}}", "startdatum": "2026-07-28", + "einddatum": null, "registratiedatum": "2026-07-28" } + """, + _ when url.StartsWith($"{ZtBase}/statustypen") => """ + { "count": 1, "next": null, + "results": [ { "url": "https://oz.example/catalogi/api/v1/statustypen/st-1", "volgnummer": 1 } ] } + """, + _ when url.StartsWith($"{ZtBase}/roltypen") => """ + { "count": 1, "next": null, + "results": [ { "url": "https://oz.example/catalogi/api/v1/roltypen/rt-initiator" } ] } + """, + _ when url == $"{ZrcBase}/statussen" => "{}", + _ when url == $"{ZrcBase}/rollen" => "{}", + _ => throw new InvalidOperationException($"unexpected ZGW call {url}"), + }; + + [Fact] + public void CreateZaak_retries_a_transient_failure_and_then_succeeds() + { + var (options, aanvraag, caller) = CreateZaakFixture(); + var zaaktypeUrl = options.ZaaktypeUrls["registratie"]; + var handler = new ZgwStubHandler( + url => RespondFor(zaaktypeUrl, url), + (url, attempt) => url == $"{ZrcBase}/zaken" && attempt == 0 ? HttpStatusCode.ServiceUnavailable : HttpStatusCode.OK); + var source = new OpenZaakZaakSource(new HttpClient(handler), new ZgwTokenProvider(options), options); + + var (referentie, _, zaakUrl) = source.CreateZaak(aanvraag, new DateTimeOffset(2026, 7, 28, 12, 0, 0, TimeSpan.Zero), caller); + + Assert.Equal("BIG-2026-000123", referentie); + Assert.Equal($"{ZrcBase}/zaken/uuid-new", zaakUrl); + Assert.Equal(2, handler.Requests.Count(r => r == $"{ZrcBase}/zaken")); + } + + [Fact] + public void CreateZaak_gives_up_after_three_attempts_on_a_persistent_transient_failure() + { + var (options, aanvraag, caller) = CreateZaakFixture(); + var zaaktypeUrl = options.ZaaktypeUrls["registratie"]; + var handler = new ZgwStubHandler( + url => RespondFor(zaaktypeUrl, url), + (url, _) => url == $"{ZrcBase}/zaken" ? HttpStatusCode.ServiceUnavailable : HttpStatusCode.OK); + var source = new OpenZaakZaakSource(new HttpClient(handler), new ZgwTokenProvider(options), options); + + var ex = Assert.Throws(() => source.CreateZaak(aanvraag, DateTimeOffset.UtcNow, caller)); + + Assert.Contains("503", ex.Message); + Assert.Equal(3, handler.Requests.Count(r => r == $"{ZrcBase}/zaken")); + } + + [Fact] + public void CreateZaak_does_not_retry_a_permanent_rejection() + { + var (options, aanvraag, caller) = CreateZaakFixture(); + var zaaktypeUrl = options.ZaaktypeUrls["registratie"]; + var handler = new ZgwStubHandler( + url => RespondFor(zaaktypeUrl, url), + (url, _) => url == $"{ZrcBase}/statussen" ? HttpStatusCode.BadRequest : HttpStatusCode.OK); + var source = new OpenZaakZaakSource(new HttpClient(handler), new ZgwTokenProvider(options), options); + + Assert.Throws(() => source.CreateZaak(aanvraag, DateTimeOffset.UtcNow, caller)); + + // No retry on 400, and — the property that makes the whole design safe — no duplicate + // zaak was created by a retry that never should have happened. + Assert.Equal(1, handler.Requests.Count(r => r == $"{ZrcBase}/statussen")); + Assert.Equal(1, handler.Requests.Count(r => r == $"{ZrcBase}/zaken")); + } } diff --git a/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs b/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs new file mode 100644 index 0000000..881e922 --- /dev/null +++ b/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs @@ -0,0 +1,120 @@ +using System.Net; +using System.Net.Http.Json; +using BigRegister.Api.Contracts; +using BigRegister.Api.Data; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; + +namespace BigRegister.Tests; + +/// +/// WP-60's required verification: a ZGW failure mid-submit must not leave the two write sides +/// silently diverged — it's flagged (, an audit row) instead. +/// Not an off : that +/// fixture hardcodes Zgw:Enabled=false, so this builds its own factory the same way +/// does, but with a stub primary handler +/// () instead of a live OpenZaak. +/// +public class ZgwDivergenceTests +{ + private const string ZrcBase = "https://oz.example/zaken/api/v1"; + private const string ZtBase = "https://oz.example/catalogi/api/v1"; + private const string ZaaktypeUrl = $"{ZtBase}/zaaktypen/zt-1"; + + private static WebApplicationFactory Factory(ZgwStubHandler stub) + { + var dbPath = Path.Combine(Path.GetTempPath(), $"bigregister-zgw-divergence-{Guid.NewGuid():N}.db"); + return new WebApplicationFactory().WithWebHostBuilder(builder => builder + .UseSetting("ConnectionStrings:AppDb", $"Data Source={dbPath}") + .UseSetting("Zgw:Enabled", "true") + .UseSetting("Zgw:ZrcBaseUrl", ZrcBase) + .UseSetting("Zgw:ZtcBaseUrl", ZtBase) + .UseSetting("Zgw:ClientId", "c") + .UseSetting("Zgw:Secret", "s") + .UseSetting("Zgw:Bronorganisatie", "123443210") + .UseSetting("Zgw:VerantwoordelijkeOrganisatie", "123443210") + .UseSetting("Zgw:ZaaktypeUrls:registratie", ZaaktypeUrl) + .ConfigureServices(services => services.ConfigureHttpClientDefaults(b => + b.ConfigurePrimaryHttpMessageHandler(() => stub)))); + } + + /// Doesn't call GET /applications first (unlike ApplicationTests.Create) — under + /// Zgw:Enabled=true that route goes through IZaakSource too, which this test's stub doesn't + /// need to answer since every test here uses a fresh db and creates exactly one aanvraag. + private static async Task CreateConcept(HttpClient client, string type = "registratie") + { + var res = await client.PostAsJsonAsync("/api/v1/applications", new { type }); + res.EnsureSuccessStatusCode(); + var body = (await res.Content.ReadFromJsonAsync())!; + return body.Id; + } + + private static string SuccessBody(string url) => url switch + { + _ when url == $"{ZrcBase}/zaken" => $$""" + { "url": "{{ZrcBase}}/zaken/uuid-new", "identificatie": "BIG-2026-000123", + "zaaktype": "{{ZaaktypeUrl}}", "startdatum": "2026-07-30", + "einddatum": null, "registratiedatum": "2026-07-30" } + """, + _ when url.StartsWith($"{ZtBase}/statustypen") => """ + { "count": 1, "next": null, + "results": [ { "url": "https://oz.example/catalogi/api/v1/statustypen/st-1", "volgnummer": 1 } ] } + """, + _ when url.StartsWith($"{ZtBase}/roltypen") => """ + { "count": 1, "next": null, + "results": [ { "url": "https://oz.example/catalogi/api/v1/roltypen/rt-initiator" } ] } + """, + _ when url == $"{ZrcBase}/statussen" => "{}", + _ when url == $"{ZrcBase}/rollen" => "{}", + _ => throw new InvalidOperationException($"unexpected ZGW call {url}"), + }; + + [Fact] + public async Task Submit_with_a_failing_zgw_flags_the_divergence_instead_of_diverging_silently() + { + var stub = new ZgwStubHandler(SuccessBody, (url, _) => url == $"{ZrcBase}/zaken" ? HttpStatusCode.ServiceUnavailable : HttpStatusCode.OK); + using var factory = Factory(stub); + using var client = factory.CreateClient(); + + var id = await CreateConcept(client); + var res = await client.PostAsJsonAsync($"/api/v1/applications/{id}/submit", new { diplomaHerkomst = "duo" }); + + // The local write is still authoritative: 200 with a real reference, not a 500. + res.EnsureSuccessStatusCode(); + var body = (await res.Content.ReadFromJsonAsync())!; + Assert.NotEmpty(body.Referentie); + + var stored = ApplicationStore.ListAll().Single(a => a.Id == id); + Assert.Null(stored.ZaakUrl); + Assert.NotNull(stored.ZgwError); + + var audit = await client.SendAsync(AdminRequest(HttpMethod.Get, "/api/v1/admin/audit")); + audit.EnsureSuccessStatusCode(); + var entries = (await audit.Content.ReadFromJsonAsync>())!; + Assert.Contains(entries, e => e.Action == "zgw:divergence" && e.Decision == "deny" && e.Resource == body.Referentie); + } + + [Fact] + public async Task Submit_with_a_healthy_zgw_leaves_no_divergence_flag() + { + var stub = new ZgwStubHandler(SuccessBody); + using var factory = Factory(stub); + using var client = factory.CreateClient(); + + var id = await CreateConcept(client); + var res = await client.PostAsJsonAsync($"/api/v1/applications/{id}/submit", new { diplomaHerkomst = "duo" }); + res.EnsureSuccessStatusCode(); + + var stored = ApplicationStore.ListAll().Single(a => a.Id == id); + Assert.Equal($"{ZrcBase}/zaken/uuid-new", stored.ZaakUrl); + Assert.Null(stored.ZgwError); + } + + private static HttpRequestMessage AdminRequest(HttpMethod method, string path) + { + var req = new HttpRequestMessage(method, path); + req.Headers.Add("X-Role", "admin"); + return req; + } +} diff --git a/backend/tests/BigRegister.Tests/ZgwStubHandler.cs b/backend/tests/BigRegister.Tests/ZgwStubHandler.cs index dfe67d3..d5747f5 100644 --- a/backend/tests/BigRegister.Tests/ZgwStubHandler.cs +++ b/backend/tests/BigRegister.Tests/ZgwStubHandler.cs @@ -9,8 +9,15 @@ namespace BigRegister.Tests; /// URL across GET/POST). Records every request's url/body/auth-scheme for assertion. /// Factored out of OpenZaakZaakSourceTests once OpenZaakDocumentSourceTests needed the /// identical stub. +/// +/// WP-60: an optional callback lets a test inject a failing status +/// for a given url on a given (0-based) attempt — e.g. "503 on the first call to /zaken, then +/// let it through" — to exercise ZgwHttpClient's retry without a live server. When it returns +/// a non-2xx code, is not called for that attempt (so a test that +/// models an "always fails" url never has to also teach `respond` a success body it never +/// reaches). /// -internal sealed class ZgwStubHandler(Func respond) : HttpMessageHandler +internal sealed class ZgwStubHandler(Func respond, Func? status = null) : HttpMessageHandler { public List Requests { get; } = new(); public List AuthSchemes { get; } = new(); @@ -21,9 +28,18 @@ internal sealed class ZgwStubHandler(Func respond) : HttpMessage protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { var url = request.RequestUri!.ToString(); + var attempt = Requests.Count(r => r == url); Requests.Add(url); AuthSchemes.Add(request.Headers.Authorization?.Scheme); Bodies.Add(request.Content?.ReadAsStringAsync(cancellationToken).GetAwaiter().GetResult() ?? ""); + + var code = status?.Invoke(url, attempt) ?? HttpStatusCode.OK; + if (!((int)code >= 200 && (int)code < 300)) + return Task.FromResult(new HttpResponseMessage(code) + { + Content = new StringContent("{\"detail\":\"stub failure\"}", Encoding.UTF8, "application/json"), + }); + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(respond(url), Encoding.UTF8, "application/json"), diff --git a/docs/project/backlog/README.md b/docs/project/backlog/README.md index f469c69..0df2f7a 100644 --- a/docs/project/backlog/README.md +++ b/docs/project/backlog/README.md @@ -110,7 +110,7 @@ for its existing violations, so every WP ends green. | [WP-57](WP-57-openzaak-least-privilege-scopes.md) | Least-privilege client scopes | 10 · OpenZaak hardening | done | | [WP-58](WP-58-openzaak-notifications.md) | Real notifications (celery + scripted abonnement) | 10 · OpenZaak hardening | done | | [WP-59](WP-59-document-confidentialiteit-config.md) | Per-document-type confidentialiteit config | 10 · OpenZaak hardening | done | -| [WP-60](WP-60-write-divergence-resilience.md) | Write-divergence resilience (local + ZGW writes) | 10 · OpenZaak hardening | todo | +| [WP-60](WP-60-write-divergence-resilience.md) | Write-divergence resilience (local + ZGW writes) | 10 · OpenZaak hardening | done | | [WP-61](WP-61-behandelportal-bootstrap.md) | Bootstrap the behandelportal app | 11 · Behandelportal | todo | | [WP-62](WP-62-medewerker-identity-authz.md) | Backend: medewerker caller identity + authz seam | 11 · Behandelportal | todo | | [WP-63](WP-63-aanvraag-status-lifecycle.md) | Backend: aanvraag status lifecycle as a published DTO | 11 · Behandelportal | todo | @@ -149,17 +149,16 @@ deployment of 49–52) and **54** (a docker OpenZaak harness + opt-in integratio CRUD arc and can land any time; 54 depends on 49 (something to read) and unlocks realistic testing for the rest. Both are self-contained (each WP file carries its own current-state handoff) and sized for a fresh Sonnet session. -Phase 10 (OpenZaak production hardening, WP-55..60) and Phase 11 (Behandelportal, -WP-61..66) are two independent tracks that can be worked concurrently — neither blocks -the other. Within phase 10: 55/59/60 are fully independent; 57 and 58 both build on 56's -provisioning mechanism, otherwise independent of each other. Within phase 11: 61 +Phase 10 (OpenZaak production hardening, WP-55..60 — now **done**) and Phase 11 +(Behandelportal, WP-61..66) are two independent tracks that can be worked concurrently — +neither blocks the other. Within phase 10: 55/59/60 were fully independent; 57 and 58 both +built on 56's provisioning mechanism, otherwise independent of each other. Within phase 11: 61 (bootstrap), 62 (backend medewerker identity), and 63 (backend status DTO) are independent of each other and can land in any order; 64 needs all three (61 for the app -to exist, 62 for identity, 63 for the status it reads); 65 needs 64; 66 needs 65 and -benefits from — but doesn't strictly require — phase 10's WP-60 landing first (WP-66 is -a second, currently-unprotected write pair otherwise). WP-60 is the one slice in phase 10 -sized for a `planner`-agent kickoff rather than direct implementation — its Decisions -block is deliberately left open (outbox vs. retry+reconcile). +to exist, 62 for identity, 63 for the status it reads); 65 needs 64; 66 needs 65 and — now +that WP-60 has landed (bounded retry + flagged divergence in `ZgwHttpClient`/`Program.cs`) — +inherits that retry for free, but must call `RecordZgwDivergence` on its own besluit write path +to get the flagging half too. ## WP template diff --git a/docs/project/backlog/WP-59-document-confidentialiteit-config.md b/docs/project/backlog/WP-59-document-confidentialiteit-config.md index 6988048..e57ccf0 100644 --- a/docs/project/backlog/WP-59-document-confidentialiteit-config.md +++ b/docs/project/backlog/WP-59-document-confidentialiteit-config.md @@ -68,10 +68,10 @@ not "must every category be configured." ## Verification -`cd backend && dotnet test` (161/161 green, incl. the 2 new `OpenZaakDocumentSourceTests` -+ the new `StamdataValidationTests` reference entry); `dotnet format --verify-no-changes` -clean. Manual: `/beheer/stamdata` shows and edits the new table; an upload for a mapped -document type carries the mapped confidentiality level (test asserted). +`cd backend && dotnet test` (161/161 green, incl. the 2 new `OpenZaakDocumentSourceTests` plus +the new `StamdataValidationTests` reference entry); `dotnet format --verify-no-changes` clean. +Manual: `/beheer/stamdata` shows and edits the new table; an upload for a mapped document type +carries the mapped confidentiality level (test asserted). ## Out of scope diff --git a/docs/project/backlog/WP-60-write-divergence-resilience.md b/docs/project/backlog/WP-60-write-divergence-resilience.md index 60d5745..215b5e7 100644 --- a/docs/project/backlog/WP-60-write-divergence-resilience.md +++ b/docs/project/backlog/WP-60-write-divergence-resilience.md @@ -1,6 +1,6 @@ # WP-60 — Write-divergence resilience (local + ZGW writes) -Status: todo +Status: done Phase: 10 — OpenZaak production hardening ## Why @@ -23,37 +23,57 @@ production. ## Decisions -Intentionally left open for kickoff — this is exactly the kind of ambiguous-root-cause, -multi-file design call the `planner` agent should make, not something pre-decided here. -Options to weigh at kickoff: +Picked **(b), narrowed further: bounded synchronous retry + flag, no reconcile job.** The +`planner` agent's kickoff review found the write side smaller than either option assumed: -- (a) an outbox table — write local + an outbox row in one local transaction, a background - worker drains the outbox to ZGW with retry. -- (b) a simpler synchronous retry-with-backoff at the call site, plus a reconciliation job - that periodically diffs local vs. ZGW state and flags/repairs divergence. +- The only ZGW writes are `OpenZaakZaakSource.CreateZaak` (zaak/status/rol, one POST sequence + per submit) and `OpenZaakDocumentSource.Upload`/`LinkToZaak` (DRC + zaakinformatieobject). + There is no standalone status-transition write path yet (that's WP-66) — Step 2 below is + corrected accordingly. +- Every path already does the local write first and never rolls it back on a ZGW failure — "the + ZGW half fails, local succeeded" is the only real scenario; the reverse can't happen. +- An outbox was rejected: three request-triggered write paths don't justify a persisted queue, + and a ZGW call's `CallerIdentity` (needed for the JWT's audit claims, WP-53) would mean PII + sitting in a new table — the "generic outbox framework" this WP's own Risks section warns + against. +- A reconcile job was judged unnecessary for the acceptance criteria: flagging (not silent + divergence) is sufficient, and repair is always possible on demand because a zaak's + `identificatie` equals the aanvraag's `Referentie` — no reconcile job ships in this WP. -Pick the smaller one that closes the gap — don't build a generic outbox framework if a -bounded retry+reconcile suffices for this POC's actual write volume. +Shipped: bounded retry (3 attempts, doubling backoff from 200ms) in `ZgwHttpClient` for +transport-shaped failures only (429/502/503/504/408 + connection errors/timeouts — deliberately +**not** 500, which can follow a partial commit on the non-idempotent `/statussen`/`/rollen` +POSTs); `Aanvraag.ZgwError` + a `zgw:divergence` audit row when a ZGW write still fails after +retry (`Program.cs`'s submit endpoint, two separate try/catches so a create-zaak failure doesn't +also skip the still-local document link); `OpenZaakDocumentSource.Upload` catches and logs +without a separate flag column (`DrcUrl == null` already means "not registered in ZGW yet"). +Full reasoning + rejected sub-options: [openzaak-integration.md](../reference/openzaak-integration.md)'s +"Write resilience" section. ## Files -Likely `Data/ApplicationStore.cs`, a new reconciliation/outbox mechanism, -`Zgw/OpenZaakZaakSource.cs`, `Program.cs` (background job registration if needed). +`Zgw/ZgwHttpClient.cs` (retry), `Data/ApplicationStore.cs` (`ZgwError` column + migration), +`Program.cs` (submit endpoint rewire + `RecordZgwDivergence` + HttpClient timeouts), +`Zgw/OpenZaakDocumentSource.cs` (non-throwing upload). No new file for a mechanism — no +outbox/background worker shipped (see Decisions). ## Steps -1. Design review with the `planner` agent — pick outbox vs. retry+reconcile. -2. Implement the chosen mechanism for the create-zaak and status-transition write paths. +1. Design review with the `planner` agent — pick outbox vs. retry+reconcile. Done: retry+flag + (see Decisions). +2. Implement the chosen mechanism for the create-zaak and document (upload + link) write + paths — not "status-transition" as originally scoped here; that path doesn't exist yet + (arrives with WP-66). 3. Add a test that simulates a ZGW failure mid-write and asserts the system recovers (retries successfully, or is left in a detectably-inconsistent-but-flagged state) rather than silently diverging. ## Acceptance criteria -- [ ] A simulated ZGW failure after a successful local write no longer leaves permanent +- [x] A simulated ZGW failure after a successful local write no longer leaves permanent silent divergence — either it retries to consistency or the divergence is detectable/flagged. -- [ ] No new synchronous latency added to the happy path beyond what the chosen mechanism +- [x] No new synchronous latency added to the happy path beyond what the chosen mechanism requires. ## Verification diff --git a/docs/project/backlog/WP-66-behandelportal-openzaak-write.md b/docs/project/backlog/WP-66-behandelportal-openzaak-write.md index 26f6ee7..7061bdf 100644 --- a/docs/project/backlog/WP-66-behandelportal-openzaak-write.md +++ b/docs/project/backlog/WP-66-behandelportal-openzaak-write.md @@ -62,8 +62,11 @@ Any further behandelportal screens beyond beoordeling. ## Risks -If Phase 10's WP-60 (write-divergence resilience) hasn't landed yet, this introduces a -second unprotected write pair — call this out explicitly if the two phases aren't -sequenced together in practice. +WP-60 (write-divergence resilience) has landed: bounded retry lives in `ZgwHttpClient`, so +this write pair inherits it automatically. It does **not** get the flagging half for free — +call `RecordZgwDivergence` (or the equivalent for whichever endpoint hosts the besluit write) on +this path's catch too, the same way `Program.cs`'s submit endpoint does for create-zaak/document +writes, or this becomes the "second, currently-unprotected write pair" WP-60's own scope note +anticipated. -Depends on: WP-65. Benefits from (but doesn't strictly require) WP-60. +Depends on: WP-65. diff --git a/docs/reference/architecture/0005-openzaak-behind-bff.md b/docs/reference/architecture/0005-openzaak-behind-bff.md index 6f2b8c0..cc75a84 100644 --- a/docs/reference/architecture/0005-openzaak-behind-bff.md +++ b/docs/reference/architecture/0005-openzaak-behind-bff.md @@ -67,5 +67,13 @@ up front — the migration stance ADR-0001 already prescribes. - **Also shipped (WP-50):** `IZaakSource.CreateZaak` — the first write. Submitting an aanvraag now also creates a Zaak + Status + Rol in OpenZaak when `Zgw:Enabled=true`, routed through the existing submit endpoint with zero DTO change (same seam, same anti-corruption boundary). -- **Deferred:** real inbound OIDC/JWT auth (still header-stubbed), Documenten/DRC upload + link - (WP-51), Notificaties/NRC webhooks (WP-52), adding OpenZaak to docker-compose. +- **Also shipped (WP-51):** `IDocumentSource` (`LocalDocumentSource`/`OpenZaakDocumentSource`, + same config-gated seam shape) — an upload registers a Documenten/DRC enkelvoudiginformatieobject + and, once a zaak exists, a submit links it in with a zaakinformatieobject. +- **Also shipped (WP-60):** bounded retry in `ZgwHttpClient` for transport-shaped ZGW failures, + plus a flagged (not silent) divergence — `Aanvraag.ZgwError` + a `zgw:divergence` audit row — + when a ZGW write still fails after retry. No outbox/background worker (see WP-60 for the + ladder check that ruled it out for this POC's write volume). +- **Deferred:** real inbound OIDC/JWT auth (still header-stubbed), Notificaties/NRC webhooks + (WP-52, shipped instead as a direct-to-BFF delivery in WP-58), adding OpenZaak to + docker-compose, an automated reconciliation/repair job for a flagged divergence (WP-60). diff --git a/docs/reference/openzaak-integration.md b/docs/reference/openzaak-integration.md index f31715c..b8acea9 100644 --- a/docs/reference/openzaak-integration.md +++ b/docs/reference/openzaak-integration.md @@ -61,11 +61,51 @@ The created zaak's `identificatie` becomes the returned `Referentie`; its status same coarse `InBehandeling` shape `ZgwZaakMapper` already uses for a freshly-opened zaak (`ZgwZaakMapper.ToCreatedStatusDto`). -ponytail shortcuts, marked at the call sites: (a) "first statustype/roltype Catalogi returns" -rather than a fully-configured per-type map — fine while a zaaktype has exactly one initial -status and initiator role; (b) no compensating transaction — if any ZGW call throws, the -aanvraag is already `Submitted` locally with no matching zaak (acceptable for a demo backend; -a production arc needs retry/reconciliation or an outbox before trusting this dual-write). +ponytail shortcut still standing: "first statustype/roltype Catalogi returns" rather than a +fully-configured per-type map — fine while a zaaktype has exactly one initial status and +initiator role. The "no compensating transaction" gap this section used to flag here is closed +by WP-60 — see "Write resilience" below. + +## Write resilience (WP-60) + +The local write (`ApplicationStore.Submit`, `DocumentStore.Add`/`Link`) and its paired ZGW +write aren't transactional — this section covers what happens when the ZGW half fails after the +local half already committed, closing the one gap the sections above used to flag as needing +"retry/reconciliation or an outbox" before this integration could be called production-ready. +Deliberately **not** an outbox: three write paths, each triggered by exactly one interactive +request, don't justify a persisted queue (which would also need to carry the acting citizen's +BSN for the JWT's audit claims — PII in a new table) — see WP-60 for the full reasoning. + +- **Bounded retry, in `ZgwHttpClient`.** Every ZGW call gets up to 3 attempts (200ms, doubling) + on transport-shaped failures — 429/502/503/504/408, connection errors, timeouts — with a + fresh request and JWT per attempt (a sent request/content can't be resent). **500 is + deliberately not retried**: it can follow a partial commit on the two non-idempotent POSTs + (`/statussen`, `/rollen`), so retrying risks a duplicate write. The create-zaak/document POSTs + are additionally safe to retry because OpenZaak enforces uniqueness on + (`bronorganisatie`, `identificatie`) — and WP-50/51 already set `identificatie` to the + locally-generated reference/document id, so a retry after a lost response 400s instead of + duplicating. +- **The local write is never rolled back.** Un-submitting a local aanvraag after a partial ZGW + failure (e.g. the zaak POST succeeded but `/statussen` didn't) would let the citizen resubmit + under a _new_ reference, orphaning the first zaak — worse than leaving it flagged. +- **A caught ZGW failure is flagged, not silent.** `Program.cs`'s submit endpoint wraps + `CreateZaak` and `LinkToZaak` in separate try/catches (separate so a create-zaak failure + doesn't also skip the still-local document link) and, on catch, logs the error, sets + `Aanvraag.ZgwError` (non-null = "the ZGW side of this submit didn't complete"), and records a + `zgw:divergence` audit row (same `AuthzAuditStore` trail every other decision uses, visible at + `/beheer/audit`) — see `RecordZgwDivergence`. The endpoint still returns 200 with the local + reference/status: that's truthful (the reference _is_ what would become the zaak's + `identificatie`) and never branches on `Zgw:Enabled` (an offline `LocalZaakSource` never + throws, so the catch is dead code there). +- **The document upload path flags differently.** `OpenZaakDocumentSource.Upload` catches its + own ZGW failure (config gap or transport) and logs it, but doesn't set a separate flag column + — `DocumentStore.Get(id).DrcUrl == null` is already the meaningful "not registered in ZGW yet" + detector `LinkToZaak` skips on, so no second mechanism is needed for that half. +- **Repair.** No automated reconcile job exists yet — a flagged zaak is repairable on demand + because its (would-be) `identificatie` always equals the aanvraag's `Referentie`, so a future + admin action can `GET /zaken?identificatie=...` and either adopt the existing zaak or retry + `CreateZaak`. Deferred until a second write pair (WP-66) or a real deployment makes it worth + building — at which point the outbox question above is also worth re-asking. ## Documenten / DRC upload + zaak link (WP-51) @@ -88,8 +128,11 @@ happens first — it stays the record of truth for preview/download/audit regard `ZgwHttpClient` (shared GET/POST-with-bearer-JWT plumbing) was factored out of `OpenZaakZaakSource` once `OpenZaakDocumentSource` needed the identical boilerplate. -ponytail shortcut: `vertrouwelijkheidaanduiding` is hardcoded to `"openbaar"` — a per-category -confidentiality level would matter for production but isn't needed to prove the seam. +`vertrouwelijkheidaanduiding` is driven by a per-document-type stamdata table (WP-59, +`Stamdata/documentconfidentialiteit.json`, ADR-0004), falling back to `"openbaar"` for any +category absent from it. Unlike the zaak side, an upload's ZGW failure (past +`DocumentStore.Add`) is caught and logged rather than persisted as a separate flag column — +see "Write resilience" below for why the two write paths differ. ## The ZGW client (`backend/src/BigRegister.Api/Zgw/`)