# Conflicts: # docs/decisions.md
33 KiB
API conventions — "Add a REST endpoint" checklist
Purpose: a precise, file-path-and-exemplar checklist for adding or changing a /api/* endpoint on
the ErsatzTV fork, so an agent with no prior context in this repo can do it correctly on the first
pass. Update this doc in the same PR that changes any convention below.
This is a companion to docs/contributing.md (general CQRS/LanguageExt/testing conventions) and
docs/ci-cd.md (build/release pipeline) — read those first for anything not API-specific.
1. Controller shape
Controllers live in ErsatzTV/Controllers/Api/*.cs, one per domain (e.g. TemplateController.cs,
BlockController.cs, LogsController.cs). Every action needs this attribute set:
[ApiController]on the class.[HttpGet/Post/Put/Delete("/api/...")]on the action, withName = "..."on at least the primary GET (used by the SPA's OpenAPI-generated client and by route-assertion tests).[Tags("Domain")]— groups the endpoint in Swagger UI / the SPA's generated client namespace.[EndpointSummary("...")]— one-line description; optionally[EndpointDescription("...")]for more detail.[EndpointGroupName("general")]— REQUIRED. Endpoints without it are excluded from the generated OpenAPI document entirely (verified: every action inTemplateController.csandLogsController.cshas it; there is no other endpoint group in use).[ProducesResponseType(typeof(X), StatusCodes.StatusYyy)]for every status code the action can actually return (200/201/204 success, plus 404/422 as applicable) — this drives the generated TypeScript response types on the SPA side.
Exemplars:
- Full CRUD controller:
ErsatzTV/Controllers/Api/TemplateController.cs— group CRUD + item CRUD + copy, all four verbs,ToCreatedResult/ToErrorResultusage. - Paged GET with clamped params:
ErsatzTV/Controllers/Api/LogsController.cs—pageNumclamped viaMath.Max(0, pageNum),pageSizeviaMath.Clamp(pageSize, 1, MaxPageSize)(MaxPageSize = 100). Any new paged endpoint should clamp the same way — don't trust client input for page math. - Sortable GET with allow-listed sort params: same file —
sortField/sortDirectionare normalized against a fixed allow-list (AllowedSortFields) rather than trusted or rejected with a 422: an unrecognizedsortFieldsilently falls back to the default field, an unrecognizedsortDirectionfalls back to the default direction. Copy this pattern (normalize, don't 422) for any new sortable endpoint — it matches the pageNum/pageSize clamp precedent above and keeps a bad query string from ever producing an error response for a read-only listing.
2. DTOs: where they live and their nullable context
- Response DTOs:
ErsatzTV.Core/Api/<Domain>/*ResponseModel.cs. They arerecords that mirror the shape of the Application-layer ViewModel for that domain — never expose a VM type directly from a controller. Convert framework types the SPA can't consume as-is (e.g.CultureInfo→ its name string).- Most response-model files start with
#nullable enable(verified: 60 of 72 files underErsatzTV.Core/Api/, e.g.ErsatzTV.Core/Api/Scheduling/BlockResponseModel.cs,ErsatzTV.Core/Api/Settings/*ResponseModel.cs). A minority of the simplest ones (e.g.FFmpegProfileResponseModel.cs) have no pragma and rely on the project default. CheckErsatzTV.Core/ErsatzTV.Core.csproj— the project sets<Nullable>disable</Nullable>— so add#nullable enableat the top of any new response-model file that has an optional (nullable) member; don't rely on the project default.
- Most response-model files start with
- Request DTOs:
ErsatzTV/Controllers/Api/Requests/*Request.cs. No#nullable enablepragma — match the existing files (e.g.CreateFFmpegProfileRequest.cs,ReplaceScheduleItemsRequest.cs). A request record typically carries aToCommand()(orToCommand(int parentId),ToReplaceCommand(int index)) method that maps it to the Application-layer command type. ErsatzTV.Applicationhas no nullable context (no<Nullable>= C# defaultdisablefor that TFM in this repo — confirms CS8632 would otherwise fire) — do not add?nullable annotations to types living there; that's a Core/Api-layer-only convention. A static mapper that lives inErsatzTV.Applicationbut returns a Core/Api response DTO with nullable members is fine (e.g.ScheduleItemResponseMapper) — the nullability lives on the DTO record, not the mapper.- Shared
{id, name}embeds: useErsatzTV.Core/Api/NamedIdResponseModel.cs(record NamedIdResponseModel(int Id, string Name)) when a response DTO needs to embed a list of named references (e.g. a schedule item'swatermarks/graphicsElements) rather than minting a one-off(int, string)record per domain. - Flatten polymorphic VMs for the wire: when an Application ViewModel is an abstract/polymorphic
record (subtypes carrying extra fields), the OpenAPI schema only captures the base shape — promote
every subtype field to a nullable top-level member on a flat response DTO and pattern-match the
concrete VM in the mapper. Exemplar:
ScheduleItemResponseModel(issue #126, seedocs/decisions.md2026-07-10). Keep the flat DTO's mutation fields named 1:1 with the matching request DTO so GET→PUT is lossless (guard with a round-trip handler test). - Optional enum filter via query param: to filter a list endpoint by an enum, add a nullable
enum parameter to the query record (default
null) and bind it with[FromQuery] TEnum? nameon the action; filter server-side only when it has a value. Exemplar:?fillerKind=onGET /api/filler-presets(GetAllFillerPresetsForApi(FillerKind? FillerKind = null)). An invalid enum value is rejected by model binding (400) — no handler-side guard needed.
3. Error mapping
Central helper: ErsatzTV/Extensions/ApiResults.cs. Use these extension methods instead of
hand-rolling IActionResult status codes:
| Method | Input | Output |
|---|---|---|
ToErrorResult() |
BaseError |
404 if NotFoundError, 412 if PreconditionFailedError (optimistic-concurrency mismatch, §7a), else 422 (ProblemDetails) |
ToCreatedResult(location, body) |
Either<BaseError, T> |
Left → ToErrorResult(); Right → 201 + Location header |
ToUpdatedResult() |
Either<BaseError, T> |
Left → ToErrorResult(); Right → 200 + body |
ToDeletedResult() |
Either<BaseError, Unit> |
Left → ToErrorResult(); Right → 204 |
ToGetResult() |
Option<T> |
Some → 200 + body; None → 404 |
ApiResults.NotFoundProblem(detail?) |
— | 404 ProblemDetails directly (e.g. when a controller has to pre-check existence itself, see TemplateController.DeleteGroup) |
ApiResults.ConflictProblem(title, detail) |
— | 409 ProblemDetails directly — for a mutation that races a background operation holding a lock (see §3a) |
3a. 409 when a mutation races a background lock
When an endpoint mutates an entity that a background operation may be actively rebuilding under an
IEntityLocker lock, guard the mutation and return 409 Conflict (ApiResults.ConflictProblem)
while the lock is held. This mirrors the Blazor UI, which disables the same actions while the lock
event is live.
This guard is advisory check-then-act, not mutual exclusion. It narrows the race but does not eliminate it: a build already queued can acquire the lock a moment after the check passes, and the mutation then interleaves with the build anyway. That residual window is accepted where the consequences are self-healing (a playout half-mutated during a build is corrected by the next rebuild). If an entity's consequences were NOT self-healing, this pattern would be insufficient — the mutation would need to actually acquire the lock for its duration instead.
Established by issue #215 (PlayoutController + ChannelController.ResetPlayout): inject
IEntityLocker, and at the top of every id-keyed mutation (PUT/POST/DELETE) check
IsPlayoutLocked(id) → ConflictProblem("Playout build in progress", ...); add
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] to each guarded
action. Precedent for the 409 shape: TraktController (its private ConflictProblem()). Two nuances:
- Fire-and-forget bulk operations don't 409 —
POST /api/playouts/reset-allstays 202; its handler (ResetAllPlayoutsHandler) skips locked playouts, matching Blazor + the handler semantics. Only per-id mutations 409. As of #235 the handler returns aResetAllPlayoutsResult(QueuedPlayoutIds/SkippedLocked/SkippedUnsupported) and the controller returns the 202 with aResetAllPlayoutsResponseModelbody reporting what was queued vs. skipped (locked, or an unsupportedExternalJson/Nonekind) — a fire-and-forget bulk op still reports its outcome rather than silently swallowing skips. - Surface the lock state to clients so they can pre-disable the buttons: stamp an
IsLockedboolean onto the list DTO (PlayoutListItemResponseModel, set fromIsPlayoutLockedin the controller's list projection) rather than adding a push channel — and (as of #235) onto the single-playout GET DTO (PlayoutResponseModel.IsLocked, set the same way in every action that maps it) so a client polling one playout has the same flag. The SPA reads it and, on a 409, refreshes to pick up the flag. - Async-op success is 202, not 200 — an endpoint whose success path only queues a background
rebuild returns 202 Accepted, not 200 (#235:
POST /api/channels/{channelNumber}/playout/resetqueues aBuildPlayout→AcceptedResult). Reserve 200 for a synchronous durable result.
3b. Map a "queue a background job" outcome to status codes with an enum, not a bool
When an endpoint starts a background operation guarded by an IEntityLocker lock, return an
outcome enum from the handler and map it in the controller — don't collapse distinct outcomes
into a lying bool/200. Exemplar (issue #232): QueueLibraryScanByLibraryId →
QueueLibraryScanResult { Queued | NotFound | SyncDisabled | AlreadyScanning }, mapped by
LibrariesController.ScanLibrary to 202 (AcceptedResult, queued), 404
(ApiResults.NotFoundProblem), 422 (UnprocessableEntityObjectResult + ProblemDetails, a
domain precondition such as sync-disabled), and 409 (ApiResults.ConflictProblem, the lock is
already held = already scanning). Here the acquired lock is the running job, so
LockLibrary(id) == false means "already scanning" → 409 (a variant of §3a where the lock is the
operation itself, not a mutation racing it). Add [ProducesResponseType] for 202/404/409/422 and
typeof(ProblemDetails) on the error ones. Guard the lock→enqueue with the
EnqueueWithTraktLock compensating-unlock pattern (TraktController): if a WriteAsync throws
after a successful Lock*, Unlock* in a catch and rethrow — one lock ⇄ exactly one release.
A second exemplar (issue #235 slice B), where the lock lives on the controller rather than in a
handler: POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep= acquires the
per-source collections lock (entityLocker.LockPlexCollections() etc.) — the lock IS the running
collections scan, so a false = 409 — then WriteAsynces Synchronize{X}Collections(id, ForceScan: true, deep) to the scanner channel and returns 202. ScannerService releases that lock in a
finally when it processes the message; the controller compensating-unlocks in a catch if the
enqueue throws. POST /api/libraries/{id}/scan?deep= similarly threads an optional [FromQuery] bool deep into QueueLibraryScanByLibraryId(id, DeepScan).
NotFoundError : BaseError lives in ErsatzTV.Core/Errors/NotFoundError.cs — return it from a
handler's validation when a lookup fails, so the controller-side mapping falls out for free.
Handler-hardening checklist when you touch (or add) a handler behind a new/changed endpoint:
- Missing FK existence checks that would otherwise 500 → convert to a
NotFoundError(404) or a validationBaseError(422). - Dictionary-indexer lookups (
dict[key]) that can throwKeyNotFoundException→ guard or useTryGetValue. - Silent item filtering (e.g.
.Where(x => x.IsValid)dropping bad rows without telling the caller) → surface as 422 instead of silently returning a shorter list. - Unbounded
int/TimeSpaninputs from the request → clamp or validate, per the Logs pagination pattern above. - Known, deliberate exception: deep FK ids nested inside item-list request bodies (e.g. a
schedule item's
CollectionId) are not existence-checked at that depth — this is established precedent from the schedules endpoints (see issue #172) and intentional to avoid N+1 validation queries; don't "fix" this without discussing it first.
3c. A durable-save PUT that also triggers a background sync as a side effect
Some PUT-replace endpoints (issue #202: PUT /api/media-sources/{plex|jellyfin|emby}/{id}/libraries)
have a synchronous durable write as their primary purpose — the response must reflect that write —
but Blazor's editor also fired off a background sync per newly-enabled library after the save. This
is a different shape from §3b: §3b is for an endpoint whose entire job is starting a
background operation (so the outcome enum drives the status code); here the enqueue is a fire-and-
forget side effect of an otherwise-ordinary write, and the response must still be 200 with the
durably-saved data, not 202.
Pattern (see PlexMediaSourcesController.ReplaceLibraryPreferences / EnqueuePostSaveSync):
- Dispatch the write command; on
Left, return the error — nothing is enqueued. - Reload the saved rows through the same query the GET uses (per §7) — needed both for the response and because ids can change as a result of the save (e.g. a disable-then-re-add).
- Iterate the reloaded rows and, per row that needs a sync,
LockLibrary(id)— skip rows whose lock is already held (mirrors Blazor'sif (Locker.LockLibrary(id))loop; no 409 for the save itself, only a silent skip for that one row's sync). - Enqueue the background message(s) for that row; if a multi-message enqueue can throw partway
through, wrap it in
try/catchand release the lock in thecatch(compensating unlock) before rethrowing — the standardEnqueueWithTraktLockone-lock-⇄-one-release discipline (§3b), just invoked from inside a write endpoint instead of a dedicated "start background job" endpoint. - Return the reloaded data with
200 OK— never 202 — since the durable save already happened; the enqueue is best-effort and its failure (after a caught/compensated exception) surfaces as a 500 on this same request rather than silently dropping the sync.
This is server-side (not SPA-orchestrated) because the SPA has no access to the scanner channel,
and having the SPA fire a second request after the save would open a crash window between "saved"
and "synced". See docs/decisions.md 2026-07-11 (#202) for the fuller rationale and the specific
bug this pattern corrected (Blazor's Plex Unlock ordering released the library lock before a
dependent second message ran).
4. Artwork contract
API response DTOs return rooted, directly-usable artwork URLs — e.g. /artwork/posters/...,
/artwork/thumbnails/..., plus passthrough for http:///https:// absolute URLs and for
Jellyfin/Emby proxy variants. This was established by PR #181 in
ErsatzTV.Application/LibraryBrowse/Queries/GetLibraryBrowseItemsHandler.cs (private Artwork(...)
helper, ~line 1330) — copy that helper's logic (or call a shared version of it) for any new
API surface that returns artwork paths. Comment in that file: "Returns a rooted, directly-usable
artwork URL for the SPA's <img src>. Blazor pages rely on GetPosterUrl to prefix
artwork/posters/... but the SPA [needs it pre-rooted]."
Do not reuse the Application-layer Mappers used by Blazor (e.g. MediaCards/Television
mappers) for new API DTOs — those still return the old Blazor-convention relative paths. Map from
the domain/VM directly and root the path yourself, following the PR #181 pattern.
5. OpenAPI regeneration — commit both generated artifacts
After any controller/DTO change:
dotnet build ErsatzTV.sln(normal build first)../scripts/update-openapi.sh— runsdotnet build -t:GenerateOpenApiDocumentsfromErsatzTV/, regeneratingErsatzTV/wwwroot/openapi/v1.json.cd web && npm run generate:api— runsscripts/generate-openapi-types.mjs, regeneratingweb/src/api/generated/v1.d.ts.
Commit both ErsatzTV/wwwroot/openapi/v1.json and web/src/api/generated/v1.d.ts — the .d.ts
file is what the SPA actually imports (web/src/api/*.ts files do
import type { components } from './generated/v1'), and CI enforces it stays in sync
(npm run check:api = regenerate + git diff --exit-code).
Known type-generation wart: DayOfWeek serializes as an integer in the OpenAPI schema but the
runtime JSON payload is actually the enum's name string ("Sunday".."Saturday") — the generator
gets this wrong. The SPA works around it with a manual override type; see web/src/api/playouts.ts
(type WithDayNames<T> = Omit<T, 'daysOfWeek'> & { daysOfWeek: DayOfWeek[] }, applied to
PlayoutAlternateSchedule, PlayoutTemplate, and their request types). Copy this pattern for any
new DTO with a DayOfWeek (or DayOfWeek[]) member — don't trust the generated numeric type.
5a. Runtime JSON casing vs the generated spec (the ffmpegProfileId wart)
Runtime /api/* JSON is serialized by Newtonsoft (AddNewtonsoftJson in Startup.cs), using
ErsatzTV/Serialization/CustomContractResolver.cs → CustomNamingStrategy (camelCase plus a
special case mapping any FFmpegProfileId member to "ffmpegProfileId", and honoring any
[JsonProperty("...")] attribute, e.g. ChannelResponseModel.FFmpegProfile →
[JsonProperty("ffmpegProfile")]). The OpenAPI document, however, is generated from
System.Text.Json metadata, whose camelCase can differ (it emitted fFmpegProfileId /
fFmpegProfile). That drift silently gave the SPA the wrong key to read (issue #198).
Fix (do not remove): ErsatzTV/Serialization/NewtonsoftSchemaNamingTransformer.cs is an OpenAPI
schema transformer registered on all three documents (options.AddSchemaTransformer(...) in
Startup.cs). For each object schema it resolves the CLR type's Newtonsoft JsonObjectContract
through the same CustomContractResolver the runtime uses and renames schema.Properties (and
schema.Required) keys to the exact names Newtonsoft would emit. This mirrors the wire format by
construction, so future naming-strategy special cases or [JsonProperty] renames can't drift.
Guard: ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs serializes fully-populated
DTOs (ChannelViewModel, FFmpegSettingsResponseModel, WatermarkViewModel,
MediaItemInfoResponseModel) through the runtime Newtonsoft settings and asserts the emitted top-level
keys equal the corresponding v1.json schema's property set. It fails if spec generation ever drifts
from the MVC serializer again. Note: only exact-match FFmpegProfileId gets the special case —
DefaultFFmpegProfileId stays defaultFFmpegProfileId under both serializers, and non-acronym or
single-leading-cap names (fFmpegPath, fFprobePath, zIndex, rFrameRate) already agree.
6. Tests
- Controller tests:
ErsatzTV.Tests/Controllers/<Domain>ControllerTests.cs. NUnit + Shouldly + NSubstitute (mockIMediator). Exemplar:ErsatzTV.Tests/Controllers/TemplateControllerTests.cs— asserts every route via aShouldHaveActionRoute(actionName, verb, path)helper (route-table regression net), then per-action tests asserting the DTO shape returned and the exactmediator.Received(1).Send(Arg.Is<Command>(...))call.PlayoutControllerTests.csis the same pattern for a larger, mixed-verb controller — use it as the template for a new controller with many actions. ApiControllerSecurityTests.cs(ErsatzTV.Tests/Controllers/): reflects over theErsatzTV.Controllers.Apinamespace to find every concrete,[ApiController]-marked controller class — no manual registry to maintain; a new controller is covered automatically. (It intentionally does not filter onControllerBase: several API controllers — including the lone exemptScannerController— do not derive from it, and aControllerBasefilter would silently drop them.) The test walks every mutating (POST/PUT/PATCH/DELETE) action on each scanned controller and asserts it's covered by the globalApiKeyAuthorizationFilter(or has an explicit[SkipApiKeyAuthorizationAttribute]exemption; onlyScannerControlleris exempt today). A minimum-count guard asserts the scan found a sane number of controllers, so a namespace rename can't silently make the scan match nothing and give this test a false pass.- Handler tests (business logic behind the controller) use the shared in-memory SQLite fixture:
ErsatzTV.Tests/Support/InMemoryTvContext.cs. Pattern:SqliteConnection("Data Source=:memory:;Foreign Keys=False")kept open for the fixture's lifetime,EnsureCreatedAsync()(not full migration replay), thenPRAGMA foreign_keys=OFFso partial object graphs can be seeded without satisfying every FK. Exemplar:ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs(_db = await InMemoryTvContext.CreateAsync();in[SetUp],_db.CreateContext()per test body,_db.Factorywhere anIDbContextFactory<TvContext>is needed by the handler under test).
7. PUT-replace list endpoints
For "replace the whole list" endpoints (PUT semantics over a collection, e.g. schedule/template
items), index items from array order in the request body rather than trusting a client-supplied
index/order field. Exemplar: ReplaceScheduleItemsRequest.ToCommand(scheduleId) —
Items.Select((item, index) => item.ToReplaceCommand(index)).
Project the write-path response through the same include chain the GET uses — never off the
freshly-built graph. After SaveChanges, a command's entities carry only the foreign-key ids you set
(e.g. ProgramScheduleItemWatermark.WatermarkId); their reference navs are null, and any mapper that
dereferences one unguarded throws an NRE that surfaces as a 500. Reload with the read-side includes
before mapping. Exemplars: ReplaceProgramScheduleItemsHandler / AddProgramScheduleItemHandler reload
via ProgramScheduleItemQueryExtensions.IncludeScheduleItemDetails() (the one include chain shared with
GetProgramScheduleItemsHandler). Also beware LanguageExt Map is lazy — returning
items.Map(ProjectToViewModel) defers the projection, so a test that only checks .IsRight won't catch
the NRE; the controller's .ToList()/serialization does (regression: ScheduleItemWriteProjectionTests).
GET handlers that feed an ordered list must also .OrderBy(i => i.Index) — id order is not index order.
7a. PUT-replace concurrency (ETag / If-Match / 412)
The replace-all aggregate PUTs (blocks, templates, schedules items, playlists, collections, playouts,
etc.) carry an optimistic-concurrency contract so a stale second tab can't silently overwrite a
fresher edit (issue #253). The Block endpoints are the reference implementation; PR2 fanned the same
recipe onto Template, DecoTemplate, Playlist, and schedule-items (PUT /api/templates/{id},
/api/deco-templates/{id}, /api/playlists/{id}, /api/schedules/{id}/items); PR3 covers the
Diff/Scalar aggregates (Collection, Playout ×2, MultiCollection, RerunCollection) and PR4 is the Phase-2
428 flip.
Each replace PUT keeps its own existing 200 body shape (Template/DecoTemplate return a
…WithItemsResponseModel, Block likewise; Playlist and schedule-items return the item array) and adds the
ETag as a header only — except schedules: ScheduleController returns the Application-layer
ProgramScheduleViewModel directly (no ProgramScheduleResponseModel exists), so the added
int Version also surfaces as a redundant version field in the GET /api/schedules[/{id}] bodies. That
is intentional and harmless (the ETag remains the authority); introducing a ResponseModel purely to hide
one field was judged disproportionate. Every replace PUT's sibling config writers bump Version too
(Playlist: the five Add*ToPlaylist handlers; schedule: AddProgramScheduleItem / DeleteProgramScheduleItem
/ UpdateProgramSchedule; Template/DecoTemplate have none).
Token. Each versioned root implements IVersionedAggregate (int Version, EF-mapped with
.IsConcurrencyToken() in its IEntityTypeConfiguration). A single dual-provider migration
(AddAggregateVersions) adds the column (nullable: false, defaultValue: 0). Do not overload the
existing DateUpdated — a plain int is portable across SQLite/MySQL and decoupled from UI cosmetics.
Transport. The aggregate's GET (the one the editor loads from — e.g. GET /api/blocks/{id}/items)
emits a strong ETag: "3" of Version; the PUT sends it back as If-Match: "3". Mismatch → 412
Precondition Failed (distinct from the §3a 409 "build in progress" lock guard). A successful PUT
returns the new ETag (post-increment) so a same-tab second save doesn't 412 against its own write.
If-Match: * and (Phase 1) a missing header force-write; a non-canonical/weak/list/malformed header
→ 400 (fail-safe; the stricter RFC 7232 "valid-but-non-matching tag → 412" refinement is deferred to
#197 — see #265). Parse/emit with ErsatzTV.Extensions.ConcurrencyHeaders (ParseIfMatch → IfMatchCondition.ExpectedVersion : Option<int>,
SetETag). The items GET returns children, so the controller reads root.Version separately for the
header (here BlockViewModel carries Version, projected but not echoed in the response body —
header-only).
Handler recipe (the error-prone part). Introduce the concurrency check as a standalone Either
AFTER the validation pipeline, never via Apply — LanguageExtensions.Apply/ToEither Join() a
Seq<BaseError> down to a base BaseError, which would flatten PreconditionFailedError to a 422. The
reference shape (ReplaceBlockItemsHandler):
Either<BaseError, Block> validated = LanguageExtensions.ToEither(validation) // explicit: the native
.Bind(block => block.CheckVersion(request.ExpectedVersion)); // Validation.ToEither() shadows ours
return await validated.Match(
Right: block => Persist(dbContext, request, block, cancellationToken),
Left: error => Task.FromResult<Either<BaseError, Unit>>(error));
In Persist, bump unconditionally before saving — root.Version++ — because EF emits the root
UPDATE only when a scalar actually differs, so a same-value/no-op PUT-back would otherwise neither fire
the token nor rotate other clients' ETags. Then save through
dbContext.SaveChangesWithConcurrencyGuard(ct) (maps DbUpdateConcurrencyException → 412), which is the
backstop that closes the load→save TOCTOU the pre-check can't. CheckVersion (pure, on
IVersionedAggregate) and SaveChangesWithConcurrencyGuard live in ErsatzTV.Core /
ErsatzTV.Application respectively.
Add [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] and …412… to the
PUT action. Config-only boundary: every mutating handler of an aggregate's editor-visible config
state bumps Version (incl. bulk ExecuteUpdate/Delete writers, which add
.SetProperty(x => x.Version, x => x.Version + 1)); regenerated build output (playout items/history) is
outside the token — its handlers neither bump nor are guarded. Test the reference with: stale-If-Match →
412 (no mutation), matching/absent → success + bump, no-op save still bumps, and a two-context racing
save → 412 (prove it non-vacuous by dropping .IsConcurrencyToken() and watching the race test fail).
Phase 2 (a later PR) flips a missing If-Match from force-write to 428 Precondition Required once
every editor echoes and one release soaks.
Fan-out status. Block (#2) is the reference. PR2 wired the RR + Reconcile aggregates
(Template #3, DecoTemplate #4, Playlist #5, schedule-items #1). PR3 wired the Diff + Scalar aggregates:
Collection custom-order #6, Playout alternate-schedules #7 and templates #8 (both share Playout.Version;
their catch(Exception)→422 handlers were restructured so the guard's PreconditionFailedError Left
returns before the catch — §9/H1), MultiCollection #9, and RerunCollection #10. Two M2 gate notes for the
SaveChangesAsync() > 0 handlers: the RerunCollection/Collection-custom-order refresh now runs on any
successful save (the bump makes the gate always-true); MultiCollection keeps its "name-only change → no
playout rebuild" optimization by bumping on the first (name) save so the second (items) save's
> 0 still means "items changed". The one bulk writer in scope, UpdateDefaultDecoHandler, bumps via
.SetProperty(x => x.Version, x => x.Version + 1) (bulk ExecuteUpdate can't throw the concurrency
exception, so it needs no guard). Deferred (tracked follow-up): the other same-root config
writers — non-bulk siblings like UpdateCollectionHandler/RemoveItemsFromCollectionHandler,
UpdatePlayoutHandler and the ScheduleFile handlers, and the repository-mediated Add*ToCollection
family — do not yet rotate their aggregate's ETag. The primary endpoints' own bump+guard fully cover
the two-tab lost-update this contract targets; the deferred writers only affect cross-editor ETag rotation,
and adding an unconditional bump to a handler that uses plain SaveChangesAsync (not the guard) would open
a new DbUpdateConcurrencyException→500 path — so they need a uniform guard+bump pass of their own.
7b. Post-commit side effects run on CancellationToken.None
Once a command handler's await dbContext.SaveChangesAsync(cancellationToken) (or repository upsert)
has committed, everything that runs afterwards to complete that mutation's side effect —
channel.WriteAsync(new BuildPlayout(...)) / other worker-channel enqueues, mediator.Publish(...),
ISearchIndex/reindex enqueues, a cache Refresh(...), and any post-commit lookup that gates one
of those enqueues — must be passed CancellationToken.None, not the request cancellationToken.
Rationale (audit #22, issues #251 → #254): the request token is cancelled when the HTTP client
disconnects. If it's threaded into a post-commit enqueue, a late disconnect turns an already-durable
commit into a thrown request and drops the side effect (e.g. the rebuild is never queued → the
persisted change silently never takes visible effect). The commit is the point of no return: past it,
the compensating side effect must not be half-abortable. Exemplar idiom:
ErsatzTV.Application/Scheduling/Commands/UpdateDecoHandler.cs (the affected-playout queries and the
WriteAsync(BuildPlayout..., CancellationToken.None) enqueue all use None).
Two boundaries:
- Response projection is NOT a side effect. The post-commit reload that builds the returned view
model (§7 above) legitimately keeps the request
cancellationToken— if the client disconnected, we don't need to compute a response nobody will read, and the durable work (commit +None-enqueue) has already happened. Only the side effect chain getsNone. - Background-job handlers keep their token.
BuildPlayoutHandlerand other handlers invoked by the worker (not by an HTTP request) receive the worker's shutdown token, not a client-disconnect token — their downstream enqueues correctly stay on that token so a shutdown stops enqueuing more work.
Pre-commit reads/validation and the SaveChangesAsync call itself keep the request token (cancelling
before the commit safely aborts with nothing persisted). A handler that passes no token to a
post-commit WriteAsync() is already behaviorally correct (default == CancellationToken.None);
making it explicit is optional cleanup, not required. Config handlers that commit via several sequential
IConfigElementRepository.Upsert calls are a distinct partial-commit-under-cancellation case not covered
by this rule (tracked separately).
8. Known API warts (don't "fix" without discussion — they're deliberate synthesized rows)
GET /api/blocks and GET /api/templates (via GetAllBlocksHandler / GetAllTemplatesHandler in
ErsatzTV.Application/Scheduling/Queries/) synthesize a fake negative-id "(none)" group row
for items that have no group, so the SPA can render an "ungrouped" bucket
(Id = unusedGroup.Id * -1, Name = "(none)"). See issue #172. If you add a similar "ungrouped"
concept elsewhere, this is the established pattern to follow — but be aware it means Id is not a
reliable real-entity id for those synthetic rows.