Files
ersatztv/docs/api-conventions.md
T
timothyandClaude Opus 4.8 0009607a09 feat(384): auto-tune DetailPanel content-source member read endpoint
GET /api/v1/channels/auto-tune/members lists the distinct content sources a
proposed auto-tune channel's server-generated SmartCollection resolves to —
parent shows for the TV axes (ItemCount = query-matching episodes), movies for
the movie-genre axis — reusing the existing PagedLibraryBrowseItemsResponseModel
(no new schema). The handler runs the server-owned AutoTuneAxisMap.GenerateQuery
through ISearchIndex (client never sends Lucene, per #69 PR1) and rolls matching
leaf items up to their distinct sources, mirroring GetSmartCollectionItems so the
DetailPanel preview matches what the built playout will contain.

Backend child of #383 (Auto-Tune DetailPanel milestone); read-only, cold review
acceptable. Handler + controller tests (9 new). OpenAPI + endpoint-index
regenerated; d.ts unchanged (reuses existing schema). Docs: api-conventions §5,
decisions.md 2026-07-17, domain-model.

fixes #384

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-17 09:40:44 +02:00

718 lines
58 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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/v1/...")]` on the action, with `Name = "..."` on at least the
primary GET (used by the SPA's OpenAPI-generated client and by route-assertion tests).
**The route is versioned and absolute** (`/api/v1/...`, leading slash, full path on the method attribute —
no class-level `[Route]`). This is enforced: `ApiRouteVersioningTests` (sibling of `ApiControllerSecurityTests`)
reflects over every `[ApiController]` action in `Controllers.Api` and fails if an effective route doesn't match
`^/api/v\d+/`. The **only** controllers with a class-level `[Route]` are the two whose ~all actions share a
parametrized prefix — `ScannerController` (`[Route("/api/v1/scan/{scanId:guid}")]`) and
`ScriptedScheduleController` (`[Route("/api/v1/scripted/playout/build/{buildId:guid}")]`) — and there the
method segments are relative (`[HttpPost("progress")]`). A browser-nav endpoint deliberately outside `/api`
(`AuthController`'s `GET /auth/oidc/login`) is out of scope for the versioning rule. See `docs/decisions.md`
2026-07-13 (#286) for the versioning contract (additive-only after freeze; the legacy `/api/*``/api/v1/*`
in-pipeline rewrite in `ApiVersionRewriteMiddleware`).
- `[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.cs`
`pageNum` 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 `record`s 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: `CollectionController`
`MediaCollectionResponseModel` (`Id, Name, CollectionType, UseCustomPlaybackOrder` — drops the
`MediaCardViewModel` scaffolding and the header-only `Version`), `ScheduleController`
`ProgramScheduleResponseModel` (VM minus `Version`), `SmartCollectionController` → the existing
`SmartCollectionResponseModel`, `ResolutionController.GetResolutionByName``ResolutionResponseModel`,
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/v1/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), **409 if `LockedError`** (a handler's own lock-acquire lost the race, §3a), 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/v1/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/v1/channels/{id:int}/playout/reset`
resolves the channel's playout by the immutable channel **`Id`** (`GetPlayoutIdByChannelId`), guards
on `IsPlayoutLocked` → 409, queues a `BuildPlayout``AcceptedResult`, 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.
- **Handler-side atomic lock loss also maps to 409, via a typed error, not 422** (issue #316 review):
when the *handler itself* is the one that atomically acquires an `IEntityLocker` lock (not just a
controller pre-check) and loses the race, return `new LockedError(...)` (`ErsatzTV.Core/Errors/LockedError.cs`,
sibling of `NotFoundError`/`PreconditionFailedError`) from the handler — `ToErrorResult()` maps it to
409 automatically. Exemplar: `PrepareTroubleshootingPlaybackHandler``TroubleshootController` does a
cheap `IsTroubleshootingPlaybackLocked()` pre-check (advisory, §3a's check-then-act caveat applies),
but the handler's own `LockTroubleshootingPlayback()` is the atomic acquire; if *that* loses the race
it returns `LockedError`, so the 409 survives even when the pre-check passed a moment too early. Don't
let a handler-side lock loss fall through to the generic 422 `BaseError.New(...)`.
### 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/v1/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 `WriteAsync`es `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/v1/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/v1/libraries/scan-status` reads `IScannerProxyService.GetActiveScans()` (per-library, with
percent); `GET /api/v1/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.
- **Dereferencing a request `string` (e.g. `request.Name.Length`) is a latent 500** — request DTOs
carry no `#nullable` context (§2), so a `string Name` binds `null` from `name: null`/an omitted field
and there is no implicit `[Required]`; a raw `.Length`/`.Trim()` throws `NullReferenceException` → an
unhandled **500** (there is no global exception filter). Validate names null-safe: reuse the
`Validators.NotEmpty(x => x.Name).Bind(_ => x.NotLongerThan(50)(x => x.Name))` combinator
(`ErsatzTV.Application/Validators/StringValidation.cs`; both are null-safe via `Optional`), the same
pattern the group-create handlers already use — or at minimum guard `string.IsNullOrWhiteSpace(name)`
before any member access. Fixed across 10 create/replace handlers in issue #172 (was `if
(request.Name.Length > 50)`).
- **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/v1/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/v1/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 three**`ErsatzTV/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.
**Endpoint inventory additions (#69, auto-tuning PR1 backend)**: two `ChannelController` actions,
both requiring the standard credential (session-or-key, per §9 — no `[SkipApiAuthorization]`/
`[RequiresAuthentication]` override):
| Method | Path | Operation | Summary |
|---|---|---|---|
| POST | `/api/v1/channels/auto-tune/preview` | `PreviewAutoTuneChannels` | Preview auto-tuned channels |
| POST | `/api/v1/channels/auto-tune` | `CreateAutoTunedChannels` | Create auto-tuned channels |
**Endpoint inventory addition (#384, Auto-Tune DetailPanel backend)**: one read-only `ChannelController`
GET, standard credential (catalog-read tier — no `[RequiresAuthentication]`):
| Method | Path | Operation | Summary |
|---|---|---|---|
| GET | `/api/v1/channels/auto-tune/members` | `GetAutoTuneChannelMembers` | List a proposed auto-tune channel's distinct content-source members |
It takes `?axis=&value=&pageNum=&pageSize=` and reuses the existing `PagedLibraryBrowseItemsResponseModel`
/ `LibraryBrowseItemResponseModel` DTOs (no new schema). The handler runs the server-owned
`AutoTuneAxisMap.GenerateQuery(axis, value)` through `ISearchIndex.Search` (client never sends Lucene, per
the #69 PR1 decision), then rolls matching leaf items up to their distinct content sources — parent shows for
the TV axes (`ItemCount` = query-matching episodes, **not** the show's total), movies for the movie-genre
axis. Paging is clamped (`pageNum` floored at 0; `pageSize` defaults to 100, clamped 1200) per the §1 Logs
precedent. See `docs/decisions.md` 2026-07-17 (#384) for the search-index-vs-EF-enumeration rationale.
**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.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 (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/v1/templates/{id}`,
`/api/v1/deco-templates/{id}`, `/api/v1/playlists/{id}`, `/api/v1/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/v1/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. Per **RFC 7232 §3.1** (issue #265): a
syntactically-valid entity-tag that does not strong-match → **412**, not 400 — this includes a weak tag
(`W/"3"` never strong-matches), an empty tag (`""`), and a strong tag whose opaque text isn't the exact
canonical decimal we emit (`"03"`, `"3.0"`, an out-of-range value). Only a genuine **grammar** violation
(an unquoted value, SP inside the opaque-tag, an unterminated quote) → **400**. `If-Match` is also a
**list** (`"3", "5"`): any strong member that matches proceeds; weak/non-canonical members drop out. The
parser therefore yields a *set* of candidate versions, and a `Version`-kind with an **empty** set is a
guaranteed 412 (a valid-but-unmatchable tag). Parse/emit with `ErsatzTV.Extensions.ConcurrencyHeaders`
(`ParseIfMatch``IfMatchCondition.ExpectedVersions : Option<Seq<int>>`; `None` = force-write,
`Some(set)` = strong-match against the set, `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`):
```csharp
Either<BaseError, Block> validated = LanguageExtensions.ToEither(validation) // explicit: the native
.Bind(block => block.CheckVersion(request.ExpectedVersions)); // 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
`DbUpdateConcurrencyException`→**500** 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: rebase onto the stored
token — original = stored, current = stored + the pending delta, so a `Version`-bumper's rotation still
advances the ETag *past* the concurrent writer's value instead of silently adopting it — 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 + deletes** — `ErasePlayoutHistoryHandler` 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.
**Cross-editor ETag rotation — completed (#269).** The remaining non-bumping config siblings now rotate the
aggregate ETag so a concurrent editor of the *same root* invalidates on its next save: the Collection
`Add*ToCollection` family (all 11 handlers) and `RemoveItemsFromCollectionHandler` bump `Collection.Version`,
and `UpdateCollectionHandler` (name/flag), `UpdatePlayoutHandler` (`DailyRebuildTime`), and the three
`ScheduleFile` writers — which already force-wrote — now also bump. All rotate via `SaveChangesForcingVersion`
(they take no `If-Match`, so a concurrent bump force-writes, never 412/500). **Correction:** the
`Add*ToCollection` family is *not* repository-mediated — each handler loads the `Collection` into its own
`dbContext` and writes directly (`IMediaCollectionRepository` is read-only), so the rotation bump is purely an
API-layer concern and the scanner's separate membership-write path is unaffected (a background scan does not
rotate the editor ETag). **No-op idempotence (the trap):** these handlers gate their reindex/`BuildPlayout`
fan-out on `SaveChanges() > 0`; an *unconditional* bump makes that gate always-true, so an idempotent re-add /
same-value re-submit would fire spurious rebuilds. Each therefore short-circuits a genuine no-op **before** the
bump — the Add handlers by an explicit membership check (also fixing the latent duplicate-`CollectionItem`
insert on a *sequential* re-add; two *concurrent* adds of the same item can still both pass the check and the
loser 500s on the composite-PK violation — a narrow, pre-existing race, tracked as #308), the scalar
writers by `ChangeTracker.HasChanges()` — so a no-op neither bumps nor rebuilds. This is an
invalidation-completeness refinement; the primary endpoints' own bump+guard already covered the two-tab
lost-update the 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/v1/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/v1/blocks` and `GET /api/v1/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`, plus local-library detail (`GET /api/v1/libraries/local/{id}`), whose response contains
server filesystem paths. The ordinary local-library list remains in the catalog-read opt-out tier.
`ApiControllerSecurityTests` asserts these boundaries reflectively.
- **Internal loopback callbacks** (the scanner's `/api/v1/scan/*`) and the **`/api/v1/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/v1/auth/password`) self-checks the principal. `ApiControllerSecurityTests` asserts these two are the
**only** auth-exempt controllers.
- **The `/api/v1/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`, `GET machine-key`
(returns the server machine key to an authenticated session — the SPA's machine-key-management screen). 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.
- **Never add a side-effecting GET/HEAD under `/api`.** The filter's CSRF check only covers mutating verbs, so
a side-effecting GET is a CSRF vector the moment a session cookie is a normal credential (a `SameSite=Lax`
cookie rides a cross-site top-level GET navigation). The former `GET /api/v1/troubleshoot/playback.m3u8` (started
an FFmpeg workload) and the archive/sample GETs were **POST-ified for #301** (PR2): `POST
/api/v1/troubleshoot/playback/start` (returns `{ url }` pointing at the open `/iptv` manifest), `POST
/api/v1/troubleshoot/playback/archive`, `POST /api/v1/troubleshoot/playback/sample/{mediaItemId}` — so the standard
session-mutation CSRF gate covers them with no new filter machinery (removing HEAD also fixed a latent
`DeleteOnClose`-on-HEAD artifact-destruction bug). Any new endpoint that *does* something must be a mutating
verb; a GET must be a pure read.
- **The SPA cutover shipped in PR2 (#295 + #301).** The browser now authenticates with the session cookie only —
it no longer sends `X-Api-Key`; the machine key is external/MCP-only, surfaced read-only by the machine-key
screen (`GET /api/v1/auth/machine-key`). See `spa-conventions.md §5e` for the SPA seams (boot gate, central
`X-Csrf` on mutations, fetch-blob downloads).
**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`. **Route versioning shipped in #286**: the whole surface is mounted at
`/api/v1` (see §1 and `docs/decisions.md` 2026-07-13) — the OpenAPI document's `paths` are all `/api/v1/…`, and a
legacy unversioned `/api/*` caller is rewritten in-pipeline by `ApiVersionRewriteMiddleware`.