#253 PR2 — optimistic-concurrency fan-out (Template, DecoTemplate, Playlist, schedule-items) #268

Merged
timothy merged 11 commits from feat/253-pr2 into main 2026-07-11 19:54:06 +02:00
Owner

Fans the frozen #253 optimistic-concurrency recipe (api-conventions §7a, established by PR1/#263 on Block) across the RR + Reconcile aggregates. Design spec: #253#issuecomment-8472 §6 PR2.

What shipped

Four aggregates wired end-to-end (ETag on the items GET, If-Match on the replace PUT → 412 on mismatch, unconditional Version++, SaveChangesWithConcurrencyGuard backstop, SPA editor ETag round-trip + 412 conflict dialog):

# Aggregate Mechanism Sibling config-writers bumped
3 Template RR delete-and-reinsert none
4 DecoTemplate RR (post-commit playout-Reset enqueue preserved, on CancellationToken.None) none
5 Playlist RR Add{Items,Movie,Episode,Season,Show}ToPlaylist (×5)
1 schedule-items Positional in-place reconcile (PR #258 logic unchanged) Add/DeleteProgramScheduleItem, UpdateProgramSchedule
  • Handler recipe per §7a: concurrency check is a standalone Either after validation (ToEither(validation).Bind(x => x.CheckVersion(request.ExpectedVersion))) so a PreconditionFailedError survives to 412 (never flattened to 422 by Apply/Join()); unconditional root.Version++ (a no-op save still bumps + rotates ETags); save through the guard (racing writer → 412).
  • Controllers: GET-items emits the strong ETag; PUT parses If-Match (malformed/weak/list → 400), threads ExpectedVersion, and returns the fresh post-bump ETag on success. Each PUT keeps its own existing 200 body shape.
  • schedule save-failure propagation: a 412 from the guard returns before the post-save read-side reload + playout-Refresh enqueue — a failed save runs no side effects.
  • SPA (web/src/api/* + four editor screens): getXItemsWithMeta load reads items-with-meta first (ETag ≤ data, fail-safe), save round-trips the ETag, 412 → blocking "changed elsewhere — reload" ConfirmDialog. SchedulesScreen threads the ETag through the existing #242 dirty-guard (itemSeq stale-guard, loadItems reload on conflict), not layered on top.

Notes

  • No migration — PR1's AddAggregateVersions already added all 9 Version columns.
  • Schedule version in body: ScheduleController returns ProgramScheduleViewModel directly (no ProgramScheduleResponseModel exists), so version also surfaces in GET /api/schedules[/{id}] bodies. Intentional/harmless (ETag stays the authority); a ResponseModel purely to hide one field was judged disproportionate. Documented in §7a.
  • OpenAPI + v1.d.ts regenerated once post-merge; drift is exactly 4×(400+412) + If-Match/412 descriptions + the one schedule version field.

Verification

  • dotnet test ErsatzTV.sln — green (ErsatzTV.Tests 1334, Core 493, Scanner 1467, Infrastructure 53, Architecture 5, FFmpeg 20; 0 failed). New non-vacuous concurrency suites per aggregate (stale→412 no-mutation, match/absent→success+bump, no-op still bumps, two-context racing save→412).
  • web: typecheck / build / lint clean; vitest 667/77 (incl. a 412-conflict-dialog test per editor); check:api no drift.
  • Adversarial review + live E2E pending (in progress).

Docs: docs/api-conventions.md §7a updated (fan-out landed + schedule body-version nuance).

Remaining on #253: PR3 (Diff + Scalar: Collection, Playout ×2, MultiCollection, RerunCollection, incl. §9 H1 catch-rethrow) · PR4 (Phase-2 428 flip).

Fans the frozen #253 optimistic-concurrency recipe (api-conventions §7a, established by PR1/#263 on Block) across the RR + Reconcile aggregates. Design spec: [#253#issuecomment-8472](http://192.168.1.95:3000/timothy/ersatztv/issues/253#issuecomment-8472) §6 PR2. ## What shipped Four aggregates wired end-to-end (`ETag` on the items GET, `If-Match` on the replace PUT → **412** on mismatch, unconditional `Version++`, `SaveChangesWithConcurrencyGuard` backstop, SPA editor ETag round-trip + 412 conflict dialog): | # | Aggregate | Mechanism | Sibling config-writers bumped | |---|---|---|---| | 3 | **Template** | RR delete-and-reinsert | none | | 4 | **DecoTemplate** | RR (post-commit playout-Reset enqueue preserved, on `CancellationToken.None`) | none | | 5 | **Playlist** | RR | `Add{Items,Movie,Episode,Season,Show}ToPlaylist` (×5) | | 1 | **schedule-items** | Positional in-place reconcile (PR #258 logic **unchanged**) | `Add`/`Delete`ProgramScheduleItem, `UpdateProgramSchedule` | - Handler recipe per §7a: concurrency check is a **standalone `Either` after validation** (`ToEither(validation).Bind(x => x.CheckVersion(request.ExpectedVersion))`) so a `PreconditionFailedError` survives to 412 (never flattened to 422 by `Apply`/`Join()`); **unconditional** `root.Version++` (a no-op save still bumps + rotates ETags); save through the guard (racing writer → 412). - Controllers: GET-items emits the strong ETag; PUT parses `If-Match` (malformed/weak/list → 400), threads `ExpectedVersion`, and returns the **fresh post-bump ETag** on success. Each PUT keeps its own existing 200 body shape. - **schedule save-failure propagation**: a 412 from the guard returns **before** the post-save read-side reload + playout-Refresh enqueue — a failed save runs no side effects. - SPA (`web/src/api/*` + four editor screens): `getXItemsWithMeta` load reads items-with-meta first (ETag ≤ data, fail-safe), save round-trips the ETag, 412 → blocking "changed elsewhere — reload" `ConfirmDialog`. SchedulesScreen threads the ETag **through** the existing #242 dirty-guard (`itemSeq` stale-guard, `loadItems` reload on conflict), not layered on top. ## Notes - **No migration** — PR1's `AddAggregateVersions` already added all 9 `Version` columns. - **Schedule `version` in body**: `ScheduleController` returns `ProgramScheduleViewModel` directly (no `ProgramScheduleResponseModel` exists), so `version` also surfaces in `GET /api/schedules[/{id}]` bodies. Intentional/harmless (ETag stays the authority); a ResponseModel purely to hide one field was judged disproportionate. Documented in §7a. - OpenAPI + `v1.d.ts` regenerated once post-merge; drift is exactly 4×(400+412) + If-Match/412 descriptions + the one schedule `version` field. ## Verification - `dotnet test ErsatzTV.sln` — green (ErsatzTV.Tests 1334, Core 493, Scanner 1467, Infrastructure 53, Architecture 5, FFmpeg 20; 0 failed). New non-vacuous concurrency suites per aggregate (stale→412 no-mutation, match/absent→success+bump, no-op still bumps, two-context racing save→412). - web: `typecheck` / `build` / `lint` clean; `vitest` 667/77 (incl. a 412-conflict-dialog test per editor); `check:api` no drift. - Adversarial review + live E2E pending (in progress). Docs: `docs/api-conventions.md` §7a updated (fan-out landed + schedule body-version nuance). Remaining on #253: PR3 (Diff + Scalar: Collection, Playout ×2, MultiCollection, RerunCollection, incl. §9 H1 catch-rethrow) · PR4 (Phase-2 428 flip).
timothy added the apireview labels 2026-07-11 18:44:56 +02:00
timothy added 8 commits 2026-07-11 18:44:56 +02:00
Fans the frozen ETag/If-Match/412 recipe (Block reference, #253) onto the
Playlist aggregate:

- ReplacePlaylistItems command carries ExpectedVersion; the handler runs
  CheckVersion as a standalone Either after validation (so a stale write
  survives as 412, not flattened to 422 by Apply/Join), bumps Version
  unconditionally before saving, and persists via
  SaveChangesWithConcurrencyGuard (EF concurrency-token backstop).
- PlaylistViewModel carries Version; the items GET sets a strong ETag and
  the PUT parses If-Match, threads it into the command, and returns the
  refreshed ETag on success (400 on a malformed If-Match).
- Sibling item-adding handlers (AddItemsToPlaylist, AddMovie/Episode/
  Season/ShowToPlaylist) bump Version too, since they mutate the same
  editor-visible item list.
- SPA: playlists.ts exposes getPlaylistItemsWithMeta and an
  If-Match-aware updatePlaylist; PlaylistEditor holds the ETag in a ref,
  round-trips it on save, and opens a "changed elsewhere" ConfirmDialog on
  412 (mirrors BlockEditor).

Tests: new ReplacePlaylistItemsHandlerConcurrencyTests (stale/match/
force-write/no-op-bump/racing-save), new PlaylistController tests
(ETag on GET items, 400/412/thread-version/force-write on PUT), and a
vitest 412-conflict-dialog test for PlaylistsScreen. dotnet test:
1304/1304 green. web: npm run typecheck clean, npm run build clean,
vitest 664/664 green.

Ref #253 PR2.
Wire the frozen #253 ETag/If-Match/412 recipe onto ProgramSchedule /
schedule-items, keeping the PR#258 positional in-place reconcile intact.

Backend:
- ReplaceProgramScheduleItems command gains Option<int> ExpectedVersion;
  ReplaceScheduleItemsRequest.ToCommand threads it.
- Handler: standalone CheckVersion Either AFTER validation (so 412 isn't
  flattened to 422), unconditional Version++ before save, guarded save via
  SaveChangesWithConcurrencyGuard, and 412 propagated without running the
  post-save reload/enqueue.
- ProgramScheduleViewModel + Mapper carry Version.
- ScheduleController: GET /items emits ETag; PUT /items parses If-Match
  (malformed -> 400), threads ExpectedVersion, re-queries for the new ETag,
  and advertises 400/412.
- Sibling config-writers (Add/Delete item, Update schedule) bump Version.

Frontend:
- schedules.ts: getScheduleItemsWithMeta + replaceScheduleItems(ifMatch)
  returning ResponseWithMeta.
- SchedulesScreen: etagRef threaded through the #242 dirty-guard (set from
  load + every successful save); 412 opens a conflict ConfirmDialog whose
  Reload discards the draft and re-runs loadItems.

Tests: handler concurrency suite (stale->412 no mutation + fill-group state
untouched, match/absent success+bump, no-op still bumps, racing save->412);
controller ETag/If-Match/412 cases; SchedulesScreen 412-conflict-dialog test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the frozen ETag/If-Match/412 recipe (Block reference implementation)
onto the Template and DecoTemplate aggregates:

- ReplaceTemplateItems / ReplaceDecoTemplateItems commands gain
  Option<int> ExpectedVersion; ToCommand() on the request DTOs threads it
  through from If-Match.
- Handlers introduce the version check as a standalone Either after
  validation (never via Apply), bump Version unconditionally before
  saving, and persist through SaveChangesWithConcurrencyGuard so a losing
  writer maps to 412 instead of 500. DecoTemplate's post-commit playout
  Reset enqueue now only runs after a successful save.
- TemplateViewModel / DecoTemplateViewModel carry Version (header-only,
  not echoed in the response body), populated in Mapper.
- TemplateController / DecoTemplateController: GET items emits a strong
  ETag of the root's version; PUT parses If-Match (400 on malformed),
  threads the expected version into the command, and returns the new
  ETag from the refreshed root on success. Both PUT actions now use the
  handler's returned item list directly instead of re-querying items.
- SPA: templates.ts / decoTemplates.ts gain getXItemsWithMeta and an
  If-Match-aware replaceX; TemplateEditor / DecoTemplateEditor hold the
  ETag in a ref, read items-with-meta first on load, and open a
  "changed elsewhere" ConfirmDialog on a 412 instead of navigating away.

Tests: new ReplaceTemplateItemsHandlerConcurrencyTests /
ReplaceDecoTemplateItemsHandlerConcurrencyTests mirror the Block
concurrency contract tests (stale/matching/absent If-Match, no-op bump,
racing-save 412, non-vacuous backstop). TemplateControllerTests /
DecoTemplateControllerTests gain ETag/If-Match/412 coverage.
TemplatesScreen.test.tsx / DecoTemplatesScreen.test.tsx gain a 412
conflict-dialog test mirroring BlocksScreen's.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs(#253 PR2): api-conventions §7a — fan-out landed + schedule body-version nuance
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 6s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m29s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
b063bc45c0
timothy added 1 commit 2026-07-11 19:08:27 +02:00
fix(#253 PR2): close review findings (ETag/items consistency + SPA load ordering)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Failing after 40s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m2s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
1a24298105
Independent Codex review of #268 found two Blockers the fork missed + two Mediums:
- Blocker: replace PUTs returned the handler's item snapshot but re-queried the root
  for the ETag separately, so a racing writer could pair stale items with a newer ETag
  (silent overwrite). All four controllers now reload root-then-items (version-first,
  fail-safe) and 404 when the root is gone between commit and reload — matching the
  Block reference. Fixes the Blocker + the Medium '200 without ETag' case together.
- Blocker: PlaylistsScreen loaded items+root via Promise.all (concurrent), pairing a
  stale name with the current ETag; now sequential (items-with-meta first, then root).
- Medium: SchedulesScreen loadItems now marks not-loaded/loading up front so canEdit is
  false through the 412 conflict reload (no stale-draft edits lost).

Controller unit-test mocks updated to stub the new reload query. Full suite green
(ErsatzTV.Tests 1334, web 667, check:api no drift).
timothy added 1 commit 2026-07-11 19:15:48 +02:00
fix(#253 PR2): SPA lint — move conflict-reload gating out of loadItems
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 5s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 5m15s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 6m19s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
7bc84fa369
The prior fix set setItemsLoading/setItemsLoaded at the top of loadItems, but the
activeId effect calls loadItems synchronously → react-hooks/set-state-in-effect lint
error (Main Lint SPA, the real CI failure). Move the not-loaded/loading gating into
reloadAfterConflict (an event handler, lint-clean), which is exactly the 412 conflict-
reload path Codex's Medium-3 targeted; canEdit stays false through that reload window.
Behavior unchanged; the normal switch/initial-load paths (guarded separately by the
#242 dirty-guard) are left untouched. Local: lint clean, vitest 667, check:api no drift,
full dotnet solution test green.
timothy added 1 commit 2026-07-11 19:47:38 +02:00
Merge remote-tracking branch 'origin/main' into feat/253-pr2
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 5s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m34s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m28s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
c93266b6fb
timothy merged commit 550d7ac984 into main 2026-07-11 19:54:06 +02:00
timothy deleted branch feat/253-pr2 2026-07-11 19:54:06 +02:00
Sign in to join this conversation.