# 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.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). - Most response-model files start with `#nullable enable` (verified: 60 of 72 files under `ErsatzTV.Core/Api/`, e.g. `ErsatzTV.Core/Api/Scheduling/BlockResponseModel.cs`, `ErsatzTV.Core/Api/Settings/*ResponseModel.cs`). A minority of the simplest ones (e.g. `FFmpegProfileResponseModel.cs`) have no pragma and rely on the project default. Check `ErsatzTV.Core/ErsatzTV.Core.csproj` — the project sets `disable` — so **add `#nullable enable` at the top of any new response-model file** that has an optional (nullable) member; don't rely on the project default. - **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/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`, 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/playouts/reset-all` stays 202; its handler (`ResetAllPlayoutsHandler`) already *skips* locked playouts, matching Blazor + the handler semantics. Only per-id mutations 409. - **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. The SPA reads it and, on a 409, refreshes the list to pick up the flag. ### 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. `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 ``. Blazor pages rely on GetPosterUrl to prefix `artwork/posters/`... but the SPA [needs it pre-rooted]."* Do **not** reuse the Application-layer Mappers used by Blazor (e.g. `MediaCards`/`Television` mappers) for new API DTOs — those still return the old Blazor-convention relative paths. Map from the domain/VM directly and root the path yourself, following the PR #181 pattern. ## 5. OpenAPI regeneration — commit both generated artifacts After any controller/DTO change: 1. `dotnet build ErsatzTV.sln` (normal build first). 2. `./scripts/update-openapi.sh` — runs `dotnet build -t:GenerateOpenApiDocuments` from `ErsatzTV/`, regenerating `ErsatzTV/wwwroot/openapi/v1.json`. 3. `cd web && npm run generate:api` — runs `scripts/generate-openapi-types.mjs`, regenerating `web/src/api/generated/v1.d.ts`. **Commit both** `ErsatzTV/wwwroot/openapi/v1.json` and `web/src/api/generated/v1.d.ts` — the `.d.ts` file is what the SPA actually imports (`web/src/api/*.ts` files do `import type { components } from './generated/v1'`), and CI enforces it stays in sync (`npm run check:api` = regenerate + `git diff --exit-code`). **Known type-generation wart**: `DayOfWeek` serializes as an integer in the OpenAPI schema but the runtime JSON payload is actually the enum's **name string** ("Sunday".."Saturday") — the generator gets this wrong. The SPA works around it with a manual override type; see `web/src/api/playouts.ts` (`type WithDayNames = Omit & { daysOfWeek: DayOfWeek[] }`, applied to `PlayoutAlternateSchedule`, `PlayoutTemplate`, and their request types). Copy this pattern for any new DTO with a `DayOfWeek` (or `DayOfWeek[]`) member — don't trust the generated numeric type. ### 5a. Runtime JSON casing vs the generated spec (the `ffmpegProfileId` wart) Runtime `/api/*` JSON is serialized by **Newtonsoft** (`AddNewtonsoftJson` in `Startup.cs`), using `ErsatzTV/Serialization/CustomContractResolver.cs` → `CustomNamingStrategy` (camelCase **plus** a special case mapping any `FFmpegProfileId` member to `"ffmpegProfileId"`, and honoring any `[JsonProperty("...")]` attribute, e.g. `ChannelResponseModel.FFmpegProfile` → `[JsonProperty("ffmpegProfile")]`). The OpenAPI document, however, is generated from **System.Text.Json** metadata, whose camelCase can differ (it emitted `fFmpegProfileId` / `fFmpegProfile`). That drift silently gave the SPA the wrong key to read (issue #198). Fix (do not remove): `ErsatzTV/Serialization/NewtonsoftSchemaNamingTransformer.cs` is an OpenAPI **schema transformer** registered on all three documents (`options.AddSchemaTransformer(...)` in `Startup.cs`). For each object schema it resolves the CLR type's Newtonsoft `JsonObjectContract` through the *same* `CustomContractResolver` the runtime uses and renames `schema.Properties` (and `schema.Required`) keys to the exact names Newtonsoft would emit. This mirrors the wire format **by construction**, so future naming-strategy special cases or `[JsonProperty]` renames can't drift. Guard: `ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs` serializes fully-populated DTOs (ChannelViewModel, FFmpegSettingsResponseModel, WatermarkViewModel, MediaItemInfoResponseModel) through the runtime Newtonsoft settings and asserts the emitted top-level keys equal the corresponding `v1.json` schema's property set. It fails if spec generation ever drifts from the MVC serializer again. Note: only exact-match `FFmpegProfileId` gets the special case — `DefaultFFmpegProfileId` stays `defaultFFmpegProfileId` under both serializers, and non-acronym or single-leading-cap names (`fFmpegPath`, `fFprobePath`, `zIndex`, `rFrameRate`) already agree. ## 6. Tests - **Controller tests**: `ErsatzTV.Tests/Controllers/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(...))` 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. - **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` 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. ## 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.