Files
ersatztv/docs/api-conventions.md
T
timothy a81f024840
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 6s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m21s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 4m19s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 5m30s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 3m30s
Merge remote-tracking branch 'origin/main' into ci/303-api-docs-blocking
2026-07-12 17:43:29 +02:00

50 KiB
Raw Blame History

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, with Name = "..." 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 in TemplateController.cs and LogsController.cs has 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/ToErrorResult usage.
  • Paged GET with clamped params: ErsatzTV/Controllers/Api/LogsController.cspageNum clamped via Math.Max(0, pageNum), pageSize via Math.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/sortDirection are normalized against a fixed allow-list (AllowedSortFields) rather than trusted or rejected with a 422: an unrecognized sortField silently falls back to the default field, an unrecognized sortDirection falls 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 are records 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).
    • As of #288, all response-model files under ErsatzTV.Core/Api/ carry #nullable enable (including the two enum wire-mirrors Settings/XmltvBlockBehavior.cs / XmltvTimeZone.cs), so the pragma is now universal, not "most". The project sets <Nullable>disable</Nullable> (ErsatzTV.Core/ErsatzTV.Core.csproj), under which a non-null string member emits a spurious nullable: true in the schema — so add #nullable enable at the top of every new response-model file and mark a member ? only when the mapper can actually emit null. FFmpegProfileResponseModel.cs is no longer an exception.
    • Raw-VM wrapping (#288). The last controllers returning Application ViewModels directly were wrapped, so no /api/* action returns a *ViewModel type anymore: CollectionControllerMediaCollectionResponseModel (Id, Name, CollectionType, UseCustomPlaybackOrder — drops the MediaCardViewModel scaffolding and the header-only Version), ScheduleControllerProgramScheduleResponseModel (VM minus Version), SmartCollectionController → the existing SmartCollectionResponseModel, ResolutionController.GetResolutionByNameResolutionResponseModel, and the channel detail GETs/writes → ChannelDetailResponseModel (the full editable field set the SPA channel editor needs — distinct from the lean list ChannelResponseModel; drops only the derived webEncodedName). When wrapping, expose exactly what the client reads: a leaner projection is right for a list, a faithful detail projection for an editor.
  • Request DTOs: ErsatzTV/Controllers/Api/Requests/*Request.cs. No #nullable enable pragma — match the existing files (e.g. CreateFFmpegProfileRequest.cs, ReplaceScheduleItemsRequest.cs). A request record typically carries a ToCommand() (or ToCommand(int parentId), ToReplaceCommand(int index)) method that maps it to the Application-layer command type.
  • ErsatzTV.Application has no nullable context (no <Nullable> = C# default disable for 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 in ErsatzTV.Application but 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: use ErsatzTV.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's watermarks / 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, see docs/decisions.md 2026-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? name on the action; filter server-side only when it has a value. Exemplar: ?fillerKind= on GET /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> LeftToErrorResult(); Right → 201 + Location header
ToUpdatedResult() Either<BaseError, T> LeftToErrorResult(); Right → 200 + body
ToDeletedResult() Either<BaseError, Unit> LeftToErrorResult(); 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 409POST /api/playouts/reset-all stays 202; its handler (ResetAllPlayoutsHandler) skips locked playouts, matching Blazor + the handler semantics. Only per-id mutations 409. As of #235 the handler returns a ResetAllPlayoutsResult (QueuedPlayoutIds / SkippedLocked / SkippedUnsupported) and the controller returns the 202 with a ResetAllPlayoutsResponseModel body reporting what was queued vs. skipped (locked, or an unsupported ExternalJson/None kind) — 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 IsLocked boolean onto the list DTO (PlayoutListItemResponseModel, set from IsPlayoutLocked in 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/{id:int}/playout/reset resolves the channel's playout by the immutable channel Id (GetPlayoutIdByChannelId), guards on IsPlayoutLocked → 409, queues a BuildPlayoutAcceptedResult, and 404s when the channel has no playout). It is keyed on {id:int}, not {channelNumber} — the single-item Channel admin contract keys on Id, never the user-mutable Number (re-keyed in #197 Bundle C; see docs/decisions.md 2026-07-12). The broadcast-side lookup (GetPlayoutIdByChannelNumber, used by HlsSessionWorker) stays number-keyed — a separate contract. 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): QueueLibraryScanByLibraryIdQueueLibraryScanResult { 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).

Status counterpart for a lock-backed async op. A queue-triggering endpoint whose "is it running?" state lives in a lock/registry should expose a GET status surface the SPA can poll to reconcile its optimistic pending flag, rather than relying on a client-side timeout. Two exemplars: GET /api/libraries/scan-status reads IScannerProxyService.GetActiveScans() (per-library, with percent); GET /api/media-sources/collections-scan-status (#271) reads IEntityLocker.Are{X}CollectionsLocked() and returns one {family} entry per family-global collections lock that's held (no id, no percent — the lock granularity dictates the DTO shape). Return only the active entries (empty list = nothing running), mirroring the queue op's own lock.

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 validation BaseError (422).
  • Dictionary-indexer lookups (dict[key]) that can throw KeyNotFoundException → guard or use TryGetValue.
  • 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/TimeSpan inputs 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):

  1. Dispatch the write command; on Left, return the error — nothing is enqueued.
  2. 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).
  3. Iterate the reloaded rows and, per row that needs a sync, LockLibrary(id)skip rows whose lock is already held (mirrors Blazor's if (Locker.LockLibrary(id)) loop; no 409 for the save itself, only a silent skip for that one row's sync).
  4. Enqueue the background message(s) for that row; if a multi-message enqueue can throw partway through, wrap it in try/catch and release the lock in the catch (compensating unlock) before rethrowing — the standard EnqueueWithTraktLock one-lock-⇄-one-release discipline (§3b), just invoked from inside a write endpoint instead of a dedicated "start background job" endpoint.
  5. 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.

4a. Artwork content type is sniffed, never client-supplied (issue #283)

The uploaded-artwork surfaces (channel logo, watermark) must never trust a client-declared content type — doing so was a stored-XSS chain (upload <script> as image/png, serve it back as text/html). The contract:

  • Upload: POST /api/artwork/uploads derives the content type from the actual bytes via ErsatzTV.Core/Images/ImageContentTypes.DetectContentType (SkiaSharp header sniff — no full decode), rejecting non-images 422. That helper (Accepted set + IsAccepted) is the single source of truth for which image types are allowed — reuse it, don't re-list content types.
  • Serve: the image routes derive the served Content-Type from the stored file; there is no ?contentType= query parameter. Never add one back — a client must not be able to choose the Content-Type of an unauthenticated response.
  • Persisted {path, contentType} DTOs (logo/watermark) run their content type through ArtworkContentTypeModel.Sanitized() before storage, blanking anything outside the allow-list. See docs/decisions.md 2026-07-12 (#283) for the fuller rationale.

5. OpenAPI regeneration — commit both generated artifacts

After any controller/DTO change:

  1. dotnet build ErsatzTV.sln (normal build first). Do not skip this. update-openapi.sh runs dotnet-getdocument against the already-built ErsatzTV.dll; an incremental -t:GenerateOpenApiDocuments will not notice a stale assembly and will silently serialize the old spec (e.g. dropping the security/401 blocks the auth transformers add). Build first so the assembly is current. (CI is immune — a fresh checkout has no bin/ and always compiles.)
  2. ./scripts/update-openapi.sh — runs dotnet build -t:GenerateOpenApiDocuments from ErsatzTV/, regenerating ErsatzTV/wwwroot/openapi/v1.json and docs/endpoint-index.md.
  3. cd web && npm run generate:api — runs scripts/generate-openapi-types.mjs, regenerating web/src/api/generated/v1.d.ts.

Commit all threeErsatzTV/wwwroot/openapi/v1.json, web/src/api/generated/v1.d.ts, and docs/endpoint-index.md. The .d.ts file is what the SPA actually imports (web/src/api/*.ts files do import type { components } from './generated/v1'). CI enforces sync in two places:

  • npm run check:api (in the test job, always) = regenerate .d.ts from the committed v1.json + git diff --exit-code.
  • the api-docs job (ersatztv#303 H4/H5) — blocking, fires when a PR diff touches ErsatzTV/Controllers/Api/** or ErsatzTV.Core/Api/**: it rebuilds v1.json / v1.d.ts / endpoint-index.md from source and fails if any of the three is stale in the diff. This is the mechanized half of the "docs-update in the same PR" rule for the API contract.

Resolved wart (#287): DayOfWeek previously serialized as an integer in the OpenAPI schema while the runtime JSON payload is the enum's name string ("Sunday".."Saturday"). It is now added to Startup.UseStringEnumSchemas's hand-list, so the "v1" schema emits it as a string enum matching the wire. The SPA's old manual WithDayNames<T> override in web/src/api/playouts.ts has been removed — the generated daysOfWeek: DayOfWeek[] type is now correct. Do not re-introduce an override for new DayOfWeek DTOs; if you add a new BCL/System enum that serializes as a name string, add its typeof(...) to that same hand-list rather than patching the SPA.

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.csCustomNamingStrategy (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 (mock IMediator). Exemplar: ErsatzTV.Tests/Controllers/TemplateControllerTests.cs — asserts every route via a ShouldHaveActionRoute(actionName, verb, path) helper (route-table regression net), then per-action tests asserting the DTO shape returned and the exact mediator.Received(1).Send(Arg.Is<Command>(...)) call. PlayoutControllerTests.cs is 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 the ErsatzTV.Controllers.Api namespace to find every concrete, [ApiController]-marked controller class — no manual registry to maintain; a new controller is covered automatically. (It intentionally does not filter on ControllerBase: several API controllers — including the lone exempt ScannerController — do not derive from it, and a ControllerBase filter 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 global ApiKeyAuthorizationFilter (or has an explicit [SkipApiKeyAuthorizationAttribute] exemption; only ScannerController is 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. It also asserts the sensitive-read tier (Troubleshoot/Logs/Settings/Maintenance) carries [RequiresApiKey] and that ScannerController carries [LocalhostOnly] — reflectively, so the tier can't silently drop a gate. ApiKeyAuthorizationFilterTests.cs unit-tests the filter itself: writes are always fail-closed, reads are gated when Api:RequireKeyForReads (default true) or [RequiresApiKey], OPTIONS preflight and non-/api paths are exempt.
  • 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), then PRAGMA foreign_keys=OFF so 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.Factory where an IDbContextFactory<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. As of #288 this holds for schedules and collections too: ScheduleController and CollectionController now project through dedicated ProgramScheduleResponseModel / MediaCollectionResponseModel DTOs, so Version is header-only on those bodies — the earlier carve-out ("ProgramScheduleViewModel is returned directly, so version leaks into the GET body") no longer applies. Verified against the SPA: neither editor reads body version; both source If-Match from the ETag response header (client.ts requestWithMeta). 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 (ParseIfMatchIfMatchCondition.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 ApplyLanguageExtensions.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).

Non-If-Match root writers → force-write, no 500 (#269). Activating Version as an IsConcurrencyToken makes EF append WHERE Version=@orig to every UPDATE and DELETE of the root — so a plain SaveChangesAsync writer that is not part of the If-Match contract throws an unhandled DbUpdateConcurrencyException500 when a replace-all editor bumps the row in its load→save window. Every such writer now saves through SaveChangesForcingVersion(ct) (Phase-1 force-write: adopt the stored token and retry; rethrow only if the row was genuinely deleted out from under it). The exposure filter is "any handler that leaves a versioned root Modified or Deleted on plain SaveChangesAsync", NOT just Version-bumpers + deletesErasePlayoutHistoryHandler modifies Playout scalars without bumping and is exposed too. Covered (18 writers): the 9 aggregate delete handlers (a delete has no ETag to rotate, so it needs only the force-write, not a bump); the item add/remove bumpers (AddProgramScheduleItem/DeleteProgramScheduleItem, Add{Items,Movie,Show,Season,Episode}ToPlaylist); UpdateProgramScheduleHandler; ErasePlayoutHistoryHandler (root-scalar erase inside an explicit transaction); and, from PR3, the Playout settings/ScheduleFile/on-demand-checkpoint writers and UpdateCollectionHandler. (The If-Match replace handlers keep the 412 SaveChangesWithConcurrencyGuard; UpdateDefaultDeco's bulk ExecuteUpdate can't throw, so it needs neither.)

Two deliberate boundaries. (1) The background build/time-shift Playout-scalar writers (BuildPlayoutHandler via PlayoutBuilder, PlayoutTimeShifter) are token-guarded too but stay on plain save on purpose: they never surface a request-path 500 (BuildPlayout catches → a build-failure BaseError; PlayoutTimeShifter runs only via the background worker), and force-writing would persist output built from stale config — the concurrent config bump already enqueues a rebuild, so failing and rebuilding with fresh config is correct. (2) Item-add force-write can leave a duplicate/gap Index (accepted Phase-1 effect): the index is computed from the handler's stale child list, so a concurrent replace-all that grew the list makes the add land at a colliding index (no unique constraint). Non-corrupting, self-correcting on next edit, strictly better than the pre-#269 500; a reload-and-recompute-on-conflict refinement is a #197 candidate. Still deferred to #197 (cross-editor ETag rotation only): the non-bumping config siblings (RemoveItemsFromCollectionHandler, UpdatePlayoutHandler's DailyRebuildTime, and the repository-mediated Add*ToCollection family, shared with the scanner hot path) don't bump Version, so editing through them doesn't rotate an open editor's ETag. That is an invalidation-completeness refinement — not a lost-update or a 500; the primary endpoints' own bump+guard fully cover the two-tab lost-update this contract targets.

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 gets None.
  • Background-job handlers keep their token. BuildPlayoutHandler and 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).

7c. Stable child identity in replace lists (schedule items)

§7 indexes replace-list children by array order, which is the reconcile key for most replace PUTs. That is correct only when a child row is pure config: reordering merely re-numbers otherwise-interchangeable rows. Schedule items are the exception (issue #259): a schedule item anchors persisted runtime state — PlayoutScheduleItemFillGroupIndex (fill-group / shuffle enumerator progression) FKs the item row with OnDelete(Cascade). Reconciling those by position makes a moved item inherit the state of whatever item previously occupied its new slot. So PUT /api/schedules/{id}/items carries a stable child identity:

  • ScheduleItemRequest.Id (int?) round-trips each existing item's server id (as returned by the items GET). null / absent / 0 ⇒ a new item (the controller normalizes 0→null so the handler contract is two-state). Never fabricate an id.
  • When any request item carries an id, ReplaceProgramScheduleItemsHandler reconciles by id: matched same-subtype rows are updated in place (keeping the id, so the fill-group index never cascades and follows the logical item across reorders/inserts); a matched row whose TPT subtype changed is delete+insert (state resets, a new id is returned — clients must re-sync from the PUT response); unreferenced existing rows are deleted; id-less request items are inserted. Index is still array-position (ordering is a separate axis from identity).
  • A fully id-less payload falls back to the verbatim positional reconcile (legacy clients). This preserves today's misattribution-on-reorder for such payloads — it is temporary and retires together with the §7a Phase-2 If-Match→428 flip.
  • Guards run inside the handler, after the §7a CheckVersion (so a client that is both version-stale and id-stale gets 412, the reload signal, not 422): a duplicate id in one payload → 422; an id not belonging to this schedule → 422 (under Phase-1 force-write a stale id is a live lost-update signal, not a new item — reject rather than silently duplicate). Both persist nothing.

Deliberate asymmetry: the other positional replace handlers (blocks #2, templates #3, deco-templates #4, playlists #5) do not carry a child id — their children are stateless config rows where positional churn is unobservable (#3/#4 don't even emit a child id on GET). Child ids are added only where a child row anchors server-side state; positional replace stays the default. A per-endpoint child-id contract can be retrofitted later without breaking anything (the field stays optional).

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.

9. Authentication — session-or-key posture (fail-closed)

The whole /api surface is gated by the global ApiAuthorizationFilter (renamed from ApiKeyAuthorizationFilter in #295). A request that requires authentication is accepted on either credential:

  1. a matching machine X-Api-Key header (MCP / external clients — issue #197 Bundle A), or
  2. an authenticated session principal (browser cookie ctv-session, from local login or OIDC — #295).

Which endpoints require authentication is unchanged and still decided by the single shared predicate ApiAuthorizationFilter.EndpointRequiresKey(httpMethod, endpointMetadata, requireKeyForReads) (also used by OpenAPI generation, so the spec can't drift). When you add an endpoint:

  • Do nothing for the common case. Writes (POST/PUT/PATCH/DELETE) always require a credential (fail-closed — there is no "open" mode). Reads (GET/HEAD) require one when Api:RequireKeyForReads is enabled, the default (true). OPTIONS preflight is exempt.
  • The effective machine key comes from IApiKeyProvider (ErsatzTV/Services/ApiKeyProvider.cs): Api:WriteKey if configured, else a key persisted at FileSystemLayout.ApiKeyPath (/config/api.key, 0600), else a freshly generated 256-bit key. It is never empty.
  • CSRF (session credential only). The machine key is CSRF-immune (a browser can't set a custom header cross-origin without a credentialed CORS grant, which is never issued). A cookie session is not: a session-authenticated mutation must additionally carry the X-CSRF header (ApiAuthorizationFilter.CsrfHeaderName) or it is rejected 403 — presence is the whole check (a custom header forces a CORS preflight a cross-site page can't satisfy; reinforced by SameSite=Lax + CORS without AllowCredentials). Key-authed requests are exempt. When you add a SPA mutation, send X-CSRF: 1 (the SPA client does this centrally).
  • Sensitive-read GETs that disclose secrets/paths or trigger work must carry [RequiresAuthentication] (renamed from [RequiresApiKey]) so they stay gated even if an operator sets Api:RequireKeyForReads=false. A valid session satisfies this tier just as the key does. Current tier: Troubleshoot/Logs/Settings/ Maintenance. ApiControllerSecurityTests asserts this reflectively.
  • Internal loopback callbacks (the scanner's /api/scan/*) and the /api/auth/* surface itself use [SkipApiAuthorization] (renamed from [SkipApiKeyAuthorization]). The scanner adds [LocalhostOnly]; the auth surface must be reachable before a caller is authenticated, and its one sensitive action (POST /api/auth/password) self-checks the principal. ApiControllerSecurityTests asserts these two are the only auth-exempt controllers.
  • The /api/auth/* surface (AuthController, [ApiExplorerSettings(IgnoreApi = true)] → excluded from the OpenAPI doc, whose audience is machine clients): GET config (what auth options exist + setupRequired), GET session, POST setup (first-run claim), POST login, POST logout, POST password. The browser-nav OIDC challenge is GET /auth/oidc/login (outside /api). login/setup/password carry a per-IP rate limit ([EnableRateLimiting("auth")], 10 / 5 min). The local admin is a single credential in ConfigElement rows (AuthLocalAdminUsername/AuthLocalAdminPasswordHash (PBKDF2) / AuthSecurityStamp) — no DB migration; a password change rotates the stamp, revoking sessions via the cookie OnValidatePrincipal. Recovery/bootstrap without the browser: set Auth:LocalAdmin:Password (+ optional …:Username, default admin) and restart (LocalAdminSeedService reseeds + rotates the stamp). While that env is set the browser setup-claim is disabled (409) — the env seed owns the credential, which also removes the startup setup-vs-seed race. Logout rotates the security stamp for a local session (ends it server-side, "log out everywhere"), gated on an authenticated session.
  • CORS is opt-in: no cross-origin access by default (the SPA is same-origin from /app); set Api:CorsAllowedOrigins (semicolon-separated exact origins). AllowCredentials is deliberately not set — cross-origin cookie auth is impossible by design (a CSRF defense); cross-origin machine clients use X-Api-Key. The policy permits X-Api-Key/X-CSRF/If-Match and exposes ETag.
  • ForwardedHeaders trust is unchanged from #285 (trust X-Forwarded-* from any peer by default, with a warning; restrict via ForwardedHeaders:KnownProxies/:KnownNetworks). A stricter "ignore unless a proxy is configured" default was considered for #295 but reverted — it would regress /iptv M3U/XMLTV/HLS absolute-URL generation (which reads Request.Scheme/Host) for a proxied deployment that hasn't set KnownProxies. Strongly set KnownProxies/:KnownNetworks when exposing ErsatzTV behind a proxy — it also gives the #295 login rate limiter an unspoofable client IP and lets the session cookie be marked Secure behind TLS.
  • Side-effecting GET endpoints are not CSRF-covered by the filter (it only checks mutating verbs). A few [RequiresAuthentication] GETs have side effects (e.g. GET /api/troubleshoot/playback.m3u8 starts playback). Once a session cookie is a normal SPA credential (PR2), those need POST-ification or an explicit X-CSRF gate — tracked as #301, which gates PR2 (latent in PR1: the SPA still uses the machine key, so no session reaches them in normal use).
  • PR1 scope note (this change is server-only). The SPA still authenticates with the stored key (ctv-api-key, X-Api-Key) until the SPA login flow lands (PR2) — PR1 is backward compatible. The ApiKeyScreen → machine-key-management repurpose, the SPA login/setup screens, and spa-conventions §5e update all land in PR2.

The OpenAPI "v1" document declares the machine posture by construction (#287). An ApiKey security scheme (X-Api-Key, in: header) is declared in components.securitySchemes, and ApiSecurityOperationTransformer injects a per-operation security requirement + a documented 401 for exactly the operations that require a credential — using the same shared predicate ApiAuthorizationFilter.EndpointRequiresKey(...) the runtime filter enforces, so the spec can never drift from enforcement. The document is generated against the effective default (Api:RequireKeyForReads=true), under which every documented operation requires the credential; the browser-session path is an additional accepted credential the spec (machine audience) needn't express. Two companion transformers run on the "v1" document only: OperationIdOpenApiTransformer synthesizes a stable operationId (from controller+action) for the ~90 operations that lacked a Name= — and disambiguates the HEAD/GET pairs that share a controller+action structurally, independent of ApiExplorer visitation order (#197 Bundle C): when 2+ synthesized ops share a base id it suffixes each by its verb (…Get/…Head), so discovery-order churn can't rename a generated client method (a base id unique across the document stays unsuffixed; explicit Name= ids are never touched). ValidationProblemOperationTransformer documents the 400 ValidationProblemDetails a model-binding/FluentValidation failure actually returns for any body/param-binding operation. All four are registered in Startup.cs on "v1" only, mirroring NewtonsoftSchemaNamingTransformer. /api/v1 route versioning remains #286.