Files
Fuchs_Intranet/Fuchs/Docs/Decisions/0025-invoice-document-regenerate-on-miss.md
StefanandClaude Opus 5 4cac109283 fix: detect no-op document stores via OUTPUT rowset; sanitise regenerated filename
Follow-up to 99bf533 from code review:

- The AffectedRows == 0 guard in Store{Invoice,Reminder}DocumentFileAsync
  was dead: fds__setInvoiceFile / fds__setReminderFile run SET NOCOUNT ON,
  so ExecuteNonQuery returns -1 whether or not the UPDATE ... WHERE [file]
  IS NULL matched. Both procs already return the updated Id through their
  OUTPUT clause, so read that result set (getSQLDatatable_async) instead:
  empty rowset = no-op store, logged, and the blob upload / FileCreated
  event are skipped rather than reporting unpersisted bytes as success.
- inv/rdoc's regenerate path built the download name from the raw
  DocumentName; a user-typed title with '/' truncated FileInfo.Name.
  SafeDocumentFileName strips editor markup and every invalid filename
  char, and the path no longer goes through FileInfo at all.
- Reminder counterpart tests for the store-file failure branches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 21:16:49 +02:00

4.0 KiB

status, date, applyTo, supersededBy
status date applyTo supersededBy
Accepted 2026-09-16
Fuchs/Controllers/IntranetController.Invoices*.cs
Fuchs/Controllers/IntranetController.Requests.cs
Fuchs/Services/InvoiceService.cs
Fuchs_Database/dbo/Stored Procedures/fds__getReportDocument.sql
Fuchs_Database/dbo/Stored Procedures/fds__setInvoiceFile.sql

0025 — Invoice document endpoints regenerate a missing final PDF on read

Context

A finalised invoice's PDF is rendered exactly once, at finalisation (req/sconf → StoreInvoiceDocumentFileAsync → fds__setInvoiceFile), and stored in fds__invoices.[file] (primary) plus blob storage (archive). If that single write fails — observed on 2026-09-16 for W7VAL/R2026-0258, where the fds__setInvoiceFile call timed out under DB load — the invoice is left finalised with [file] = NULL and nothing ever retries.

The read endpoints then disagreed: req/idoc regenerated a missing file (a fallback carried over from the legacy VB fuchs_ocms_intranet_requests.vb "idoc" case), while inv/rdoc — the endpoint the invoice list's preview icon actually calls — read fds__getReportDocument and returned a permanent 404.

Decision

Both invoice document read endpoints (req/idoc, inv/rdoc) regenerate and persist the PDF via StoreInvoiceDocumentFileAsync when no stored bytes exist, only when the invoice is

  • finalised (IsFinal, i.e. !FdsInvoiceData.IsDraft), and
  • not external (IsExternal false — an MFR-sourced document has no template to regenerate from; a missing MFR file stays a 404).

A read is therefore allowed to write [file], upload the blob and emit InvoiceFileCreated — the same side effects finalisation would have had.

Every failure inside StoreInvoiceDocumentFileAsync/StoreReminderDocumentFileAsync (render threw, render produced no bytes, DB write threw, DB write matched 0 rows) is logged at Error level individually, because the method signals all four to callers with the same empty array. The 0-rows case is detected via the procs' existing OUTPUT inserted.[Id] result set (read with getSQLDatatable_async; empty ⇒ the UPDATE ... WHERE [file] IS NULL matched nothing). Rows-affected is not usable for this: both procs run SET NOCOUNT ON, so ExecuteNonQuery returns -1 whether or not the UPDATE matched, which is why setSQLValue_async could never distinguish a no-op from a write and the original incident was invisible to every log.

Consequences

  • Drafts are never persisted by a read. fds__setInvoiceFile writes WHERE [file] IS NULL and fds__setInvoiceFinal never clears [file]; a draft render stored early would make the later final write a no-op and the invoice would carry the watermarked draft PDF forever. The !IsDraft gate is load-bearing — do not relax it.
  • The regenerated PDF is rendered from the invoice's current data. That is acceptable because a finalised invoice is immutable in the editor; it is not acceptable for anything mutable.
  • A read on a cold miss now costs a render plus a DB write, so it is exposed to the same fds__setInvoiceFile timeout that caused the miss. The client preview timeout ($inv.pdfPreviewTimeout) must stay generous (120 s).
  • inv/rdoc still serves external (MFR) documents purely from fds__getReportDocument; regeneration never applies to them.

Alternatives considered

  • Retry at finalisation instead of on read. Would cover the store failure but not an already-damaged row; regeneration on read heals both and matches the behaviour req/idoc already had.
  • Regenerate drafts too, to mirror req/idoc's render path. req/idoc renders a draft on the fly without storing it; persisting a draft through fds__setInvoiceFile is what causes the permanent-draft defect above.
  • Return 404 and rely on the _BackupAndClearInvoiceFile recovery links. That proc is a manual-only maintenance tool, and the inv/rdoc?create=1 link it prints was never implemented on that endpoint.