Commit Graph
3 Commits
Author SHA1 Message Date
timothy 8878bf9e11 docs(review): record deferred If-Match 412-semantics refinement (#265) as an acceptable-defer
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 6s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m27s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m34s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Codex re-review of the fix commit confirmed both prior findings resolved and raised one
new Medium: RFC 7232 would 412 (not 400) a syntactically-valid but non-matching If-Match
(non-canonical "03", weak W/"3", tag lists, empty, overflow). Deferred to #197 (cold
contract pass) as #265 — fail-safe today (the mutation is rejected, never applied) and no
first-party client is affected. Records the deferral where the #253 fan-out will copy the
parser: a code comment in ConcurrencyHeaders + a note in api-conventions §7a.

Refs #253 #265
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 17:18:35 +02:00
timothy ee39effe0b fix(review): close client load-TOCTOU + canonicalize If-Match parse (Codex High/Medium)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m39s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m30s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Codex independent review of #263 surfaced two defects the fork review missed:

- High — client load TOCTOU: BlockEditor read root metadata (getBlock) and items+ETag
  (getBlockItemsWithMeta) concurrently, so a concurrent write landing between them (with
  the items read resolving last) left a stale root paired with a current ETag → the save
  silently overwrote the concurrent change with no 412. Fix: read items+ETag FIRST, then
  the root metadata, so the captured ETag is never newer than the root version and any
  inconsistency fails safe (save 412s → conflict dialog → reload).
- Medium — `ParseIfMatch` accepted non-canonical strong tags ("03", "+3", " 3 ") as
  version 3. An ETag is opaque; only the exact emitted form is valid. Fix: canonical
  decimal only (`NumberStyles.None` + no leading zeros) → else 400.

Tests: new `ConcurrencyHeadersTests` (canonical parse + padded/signed/whitespace/weak/
unquoted/list/overflow/empty → malformed); `ApiResultsTests` gains the 412 mapping case.
Existing BlocksScreen tests still green (load reordering is behavior-preserving for the
non-concurrent path).

Refs #253
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 17:13:26 +02:00
timothyandClaude Opus 4.8 94ebf34ccd feat(api): optimistic-concurrency contract for replace-all PUTs — PR1 infra + Block reference (#253)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m25s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Adds the shared optimistic-concurrency contract so a stale second tab can no longer
silently overwrite a fresher edit. PR1 lands the infra + the Block reference aggregate;
PRs 2–4 fan the same recipe across the other 8 roots (design: #253#issuecomment-8472).

Contract
- `IVersionedAggregate` (`int Version`) on all 9 replace-all roots (ProgramSchedule,
  Block, Template, DecoTemplate, Playlist, Collection, Playout, MultiCollection,
  RerunCollection), EF-mapped `.IsConcurrencyToken()`; one dual-provider migration
  `AddAggregateVersions` (nullable:false, default 0).
- Strong `ETag` of `Version` on the aggregate GET; `If-Match` on the PUT; mismatch →
  412 (distinct from the §3a 409 build-lock guard). Successful PUT returns the new ETag.
- `PreconditionFailedError : BaseError` → 412 in `ApiResults.ToErrorResult`;
  `ConcurrencyHeaders.ParseIfMatch/SetETag`; malformed If-Match → 400; `*`/absent =
  Phase-1 force-write.

Block reference wiring
- Handler: standalone `Either` via `CheckVersion` AFTER validation (never through
  `Apply`, which Join()-flattens the subtype to 422), unconditional `Version++`,
  `SaveChangesWithConcurrencyGuard` backstop (DbUpdateConcurrencyException → 412).
- `BlockViewModel.Version` (header-only, not echoed in the body); controller sets the
  ETag on GET items and on the successful PUT.
- SPA: `client.requestWithMeta` seam; `blocks.getBlockItemsWithMeta` + `replaceBlock`
  If-Match/ETag round-trip; `BlockEditor` holds the ETag, sends If-Match, and on 412
  opens a blocking "changed elsewhere — reload" dialog.

Tests
- Handler contract tests: stale-If-Match → 412 (no mutation), matching/absent → success
  + bump, no-op save still bumps, and a two-context racing save → 412; proven
  non-vacuous (drop `.IsConcurrencyToken()` → the race test fails).
- Controller tests: malformed If-Match → 400, If-Match threaded to the command, ETag on
  GET/PUT, 412 passthrough. SPA: requestWithMeta ETag, replaceBlock If-Match, 412 dialog.

Docs: api-conventions §7a, spa-conventions §4a, domain-model glossary, decisions log.

Refs #253
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 16:51:59 +02:00