# 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//*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 `disable` (`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 `` = 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` | `Left` → `ToErrorResult()`; `Right` → 201 + `Location` header | | `ToUpdatedResult()` | `Either` | `Left` → `ToErrorResult()`; `Right` → 200 + body | | `ToDeletedResult()` | `Either` | `Left` → `ToErrorResult()`; `Right` → 204 | | `ToGetResult()` | `Option` | `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 ``. 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 `