Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 10s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m12s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 3m4s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m17s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 10m36s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Version every /api route to /api/v1 (251 controller routes + ~24 Location
headers + the scanner callback URL + the Startup request-log literal),
uniform across the machine API, auth, scanner and scripted-build surfaces.
Add ApiVersionRewriteMiddleware: a legacy unversioned /api/* request is
rewritten (NOT redirected) to /api/v1/* in-pipeline — method, body, auth
headers and query survive — carrying RFC 8594 Deprecation/Sunset headers,
so curl / the future MCP server / bookmarks keep working. An already-
versioned path passes through; a future /api/v2 is never forced to v1.
Standardize the route convention (leading-slash absolute route per method,
no class-[Route] — except the two Scanner/Scripted controllers whose ~all
actions share a parametrized {id} prefix), enforced by ApiRouteVersioningTests
(^/api/v\d+/ over the whole Controllers.Api surface; browser-nav
/auth/oidc/login is out of scope).
Regenerate v1.json (160 paths, all /api/v1)/endpoint-index/v1.d.ts; sweep 945
SPA request literals + the test mocks (regex + positional URL parsers). /api/v1
is additive-only after freeze; the legacy-rewrite shim sunsets in ~2 releases
(owner decision) with removal tracked as a Phase-3 follow-up.
Docs: decisions.md 2026-07-13, api-conventions §1/§9, rest-api/spa-conventions/
blazor-route-parity/e2e-local/domain-model.
fixes #286
refs #197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
160 lines
15 KiB
Markdown
160 lines
15 KiB
Markdown
# REST API Design — ersatztv#2
|
||
|
||
**Status:** Phase 1 signed off → Phase 2 in progress.
|
||
**Tracker:** ersatztv#2. **Sub-issues:** #34 (#2a), #35 (#2b), #36 (#2c), #37 (#2d), #38 (#2e). **Handoff:** `docs/handoffs/rest-api.md`.
|
||
|
||
## 1. Goals
|
||
|
||
1. **Programmatic CRUD** for channels / collections / schedules / playouts via REST — enabling **MCP** and direct API automation.
|
||
2. **Foundation for a new UI** built on top of the API (it replaces read paths too, not just writes).
|
||
3. **Retire fragile direct-SQLite writes.** All mutations route through EF Core + existing domain validation, returning proper HTTP codes — no more TPT/enum/NOT-NULL footguns, no container-stop-for-writes.
|
||
|
||
These goals push three things to the front: **OpenAPI is in-scope** (MCP tool-gen + UI codegen), **stable DTO contracts**, and a **programmatic auth** story.
|
||
|
||
## 2. What the investigation found (code-grounded)
|
||
|
||
### 2.1 No migration needed
|
||
Pure CRUD over existing tables. The full schema — including the four `ProgramScheduleItem` **TPT subtype tables** (`ProgramScheduleOneItem`, `ProgramScheduleMultipleItem`, `ProgramScheduleFloodItem`, `ProgramScheduleDurationItem`, via `.UseTptMappingStrategy()`) — already exists. The `migrations` CI job will stay green; no `scripts/add-migration.sh` run is required for any slice.
|
||
|
||
### 2.2 Every command already exists
|
||
All four resources already have MediatR Create/Update/Delete (+ item ops) handlers, **almost all returning `Either<BaseError, T>`** — which maps cleanly to HTTP. The API is mostly **thin controllers over existing handlers**, not new business logic.
|
||
|
||
| Resource | Create | Update | Delete | Item ops |
|
||
|---|---|---|---|---|
|
||
| **Channels** | `CreateChannel` → `Either<_, CreateChannelResult>` | `UpdateChannel` → `Either<_, ChannelViewModel>` | `DeleteChannel` → `Either<_, Unit>` | — |
|
||
| **Collections** | `CreateCollection` → `Either<_, MediaCollectionViewModel>` | `UpdateCollection` → `Either<_, Unit>` | `DeleteCollection` → `Either<_, Unit>` | `AddItemsToCollection`, `RemoveItemsFromCollection` |
|
||
| **Schedules** | `CreateProgramSchedule` → `Either<_, CreateProgramScheduleResult>` | `UpdateProgramSchedule` → `Either<_, UpdateProgramScheduleResult>` | `DeleteProgramSchedule` → `Either<_, Unit>` | `AddProgramScheduleItem`, `ReplaceProgramScheduleItems` (bulk) |
|
||
| **Playouts** | `CreateClassic/Block/Sequential/Scripted/ExternalJsonPlayout` → `Either<_, CreatePlayoutResponse>` | `UpdatePlayout` → `Either<_, PlayoutNameViewModel>` | `DeletePlayout` → `Either<_, Unit>` | reset (exists, via worker channel) |
|
||
|
||
Net-new command work is essentially **zero** for the happy paths. Effort is in controllers, DTOs, status-code mapping, auth, and tests.
|
||
|
||
### 2.3 Validation already lives in the handlers (mostly)
|
||
The substantive rules are in-handler, so the API inherits them:
|
||
- Channel **number uniqueness** (`CreateChannelHandler.cs:137`, `UpdateChannelHandler.cs:234` excludes self), name/FK-exists, mirror-source validity.
|
||
- Collection / schedule **name uniqueness** + length.
|
||
- Playout business rules: **"Channel already has one playout"** (`CreateClassicPlayoutHandler.cs:77`), **"Program schedule must have items"** (`:93`).
|
||
|
||
A few **format/UX checks are page-only** (FluentValidation in `.razor` / `Validators/`) and would NOT reach the API — port these into the handlers (with test-before/after):
|
||
- Channel: `ShowInEpg` must be false when `!IsEnabled`; external-logo-URL format. (`ChannelEditViewModelValidator.cs`)
|
||
- Playout: required-field shape per kind. (`PlayoutEditViewModelValidator.cs`) — mostly redundant with handler checks; confirm parity.
|
||
|
||
### 2.4 Existing controller conventions (match these)
|
||
- Controllers in `ErsatzTV/Controllers/Api/`, base `ControllerBase`, **routes declared per-action** (full path, no controller `[Route]` prefix).
|
||
- JSON: **Newtonsoft**, **camelCase** (`CustomNamingStrategy`, with `FFmpegProfileId` special-cased), **enums as strings** (`StringEnumConverter`), `NullValueHandling.Ignore` (`Startup.cs:294`).
|
||
- **OpenAPI already wired**: `AddOpenApi("v1")` + **Scalar UI at `/docs`** (`Startup.cs:136`, `:664`); endpoints opt in via `[EndpointGroupName("general")]`.
|
||
- `.ToActionResult()` extensions map: `Either` Left→**400**, Right→**200**; `Option` None→**404**, Some→**200**; `Validation` Failure→**400**. **No 201/422 today.**
|
||
|
||
### 2.5 Existing CRUD controllers are standardized
|
||
`SmartCollectionController` and `FFmpegProfileController` previously used verb-in-path routes (`/new`,
|
||
`/update`, `/delete/{id}`). They have been retrofitted to idiomatic REST routes as part of the REST #2
|
||
slice work, gated by characterization tests and the shared `ProblemDetails` error contract.
|
||
|
||
### 2.6 Auth today
|
||
A **JWT bearer scheme** exists (`JwtHelper`, `JwtOnlyScheme` policy, `access_token` query-param support, 1-day tokens) but is wired **only to `IptvController`** via `ConditionalIptvAuthorizeFilter` (enforced only when `JWT:IssuerSigningKey` is set). `/api/*` currently has **no auth**; CORS is **AllowAll**.
|
||
|
||
## 3. Conventions (decided)
|
||
|
||
### 3.1 Routing — idiomatic REST
|
||
```
|
||
POST /api/v1/channels GET /api/v1/channels GET /api/v1/channels/{id}
|
||
PUT /api/v1/channels/{id} DELETE /api/v1/channels/{id}
|
||
|
||
POST /api/v1/collections GET /api/v1/collections GET /api/v1/collections/{id}
|
||
PUT /api/v1/collections/{id} DELETE /api/v1/collections/{id}
|
||
POST /api/v1/collections/{id}/items
|
||
DELETE /api/v1/collections/{id}/items/{mediaItemId}
|
||
|
||
POST /api/v1/schedules GET /api/v1/schedules GET /api/v1/schedules/{id}
|
||
PUT /api/v1/schedules/{id} DELETE /api/v1/schedules/{id}
|
||
GET /api/v1/schedules/{id}/items
|
||
POST /api/v1/schedules/{id}/items
|
||
PUT /api/v1/schedules/{id}/items # bulk replace (maps to ReplaceProgramScheduleItems)
|
||
DELETE /api/v1/schedules/{id}/items/{itemId}
|
||
|
||
POST /api/v1/playouts GET /api/v1/playouts GET /api/v1/playouts/{id}
|
||
DELETE /api/v1/playouts/{id}
|
||
POST /api/v1/channels/{id}/playout/reset # re-keyed {number} → {id} (#197 Bundle C)
|
||
```
|
||
`{id}` = numeric entity id. Channel reset is keyed on `{id}` like every other single-item admin route
|
||
(re-keyed from `{number}` in #197 Bundle C — `Number` is user-mutable, so it's unusable as a stable
|
||
resource key; broadcast surfaces keep `{number}`. See `docs/decisions.md` 2026-07-12).
|
||
|
||
### 3.2 Status codes
|
||
| Outcome | Code |
|
||
|---|---|
|
||
| Create success | **201 Created** + `Location` header + body |
|
||
| Update success | **200 OK** + body |
|
||
| Delete success | **204 No Content** |
|
||
| Read success | **200 OK** |
|
||
| Resource not found | **404** + `ProblemDetails` body |
|
||
| Validation failure (bad field, uniqueness, business rule) | **422 Unprocessable Entity** + `ProblemDetails` body |
|
||
| Malformed request (unparseable JSON, missing required) | **400** |
|
||
|
||
The current `.ToActionResult()` only yields 200/400/404, so we add **richer mapping helpers** (a deliberate, stated deviation — proper HTTP semantics matter for MCP/UI consumers). **Foundation decision (#2a):** to distinguish 404 from 422, handlers fold "does not exist" into `BaseError` today (flat). Introduce a lightweight typed error (e.g. `NotFoundError : BaseError`) in `ErsatzTV.Core`; mapping helper returns 404 for it, 422 for other `BaseError`s. This is a Core/error-layer change (not an EF model change) — test-before/after applies where it touches existing handlers. If it proves to sprawl, fall back to per-controller existence checks and backlog the typed-error refactor.
|
||
|
||
**Error body contract (#46):** `/docs` and `wwwroot/openapi/v1.json` are generated-client contracts for MCP/UI consumers, so runtime 404/422 bodies MUST match the OpenAPI `ProblemDetails` schema. API helpers return `ProblemDetails` with `status`, `title`, and `detail`; 404 uses title `Resource not found`, and 422 uses title `Validation failed`. Do not return plain string error bodies from REST API #2 endpoints.
|
||
|
||
### 3.3 DTOs
|
||
- **Requests:** dedicated request DTOs (bound from JSON) → mapped to existing Commands. Stable external contract; decouples the wire shape from internal command records.
|
||
- **Responses:** **reuse existing `*ViewModel`s** (already the camelCase read contract used by current GETs). One contract, already proven.
|
||
|
||
### 3.4 Validation
|
||
Reuse handler validation. Port the page-only checks in §2.3 into handlers so the API reaches parity with the Blazor UI. No validation logic in controllers.
|
||
|
||
### 3.5 Auth — dedicated API key for writes (decoupled from IPTV)
|
||
**Mechanism:** a dedicated **API key** for mutations — `Api:WriteKey` / `Api__WriteKey`,
|
||
checked against the `X-Api-Key` request header by a global MVC filter. When the key is configured,
|
||
all mutating `/api/*` requests (`POST`/`PUT`/`PATCH`/`DELETE`) require the header; when the key is
|
||
unset, the LAN-open default is preserved. Reads and all `/iptv/*` routes stay open. The scanner
|
||
callback controller is the designed exemption because scanner child processes call
|
||
`/api/v1/scan/{scanId}/...` without `X-Api-Key`; any future exemption must be explicit via
|
||
`[SkipApiKeyAuthorization]`.
|
||
|
||
**Why not reuse the JWT scheme (important):** JWT is gated by a single global toggle, `JwtHelper.IsEnabled` ← `JWT:IssuerSigningKey` (`Startup.cs:166`). That **same toggle also gates the IPTV endpoints** (`/iptv/channels.m3u`, `/iptv/xmltv.xml`, streams — `ConditionalIptvAuthorizeFilter:18`) which **Jellyfin and Dispatcharr consume**. Enabling JWT to protect writes would force token auth onto those media feeds — and JWT tokens **expire in 1 day** (`JwtHelper.cs:27`), unsuitable for a standing tuner URL. A dedicated API key **decouples write-auth from media-consumer auth**: turning it on changes **nothing** for Jellyfin/Dispatcharr.
|
||
|
||
**Properties:** long-lived credential (no 1-day churn), fit for MCP / new-UI write clients; net-new but small (one global filter + one config key); a standard machine-API pattern.
|
||
|
||
**Backlog:** tighten CORS for mutation routes if writes are exposed beyond LAN.
|
||
|
||
### 3.6 OpenAPI / discoverability
|
||
New endpoints carry `[EndpointGroupName("general")]` → appear in the existing `v1` doc + Scalar `/docs`. This *is* the MCP/UI enabler (tool-gen + client codegen). Keep summaries/`[ProducesResponseType]` accurate per action.
|
||
|
||
### 3.7 Other conventions
|
||
- **Absolute URLs** in responses (logos/streams) use the **request-derived host**, same as M3U/XMLTV (`docs/m3u-xmltv.md`) — never bake a host. (Carries the #1 lesson forward.)
|
||
- **List endpoints:** add simple pagination where the underlying query supports it; defer rich filtering/sorting (backlog — the new UI will want it).
|
||
- **DELETE idempotency:** return 404 if the entity is already absent (clear over silently-204). EF cascade handles dependents (e.g. Channel→Playout cascade, ProgramSchedule→Items cascade) — integration tests prove this per slice.
|
||
|
||
## 4. Standardization scope
|
||
|
||
- Retrofit `FFmpegProfileController` + `SmartCollectionController` to idiomatic REST, each gated by **characterization tests** (capture current behavior → change → prove green). Completed in #35/#38.
|
||
- CORS review in #38: the app still uses the existing global `AllowAll` policy. REST mutations are protected by the optional API-key write filter, and changing CORS defaults would be an operational exposure decision rather than an API-shape cleanup. Keep CORS tightening as backlog if write APIs are exposed beyond the LAN.
|
||
- Sweep result in #38: REST #2 CRUD controllers now use idiomatic routes. Older operational endpoints such as `/api/v1/libraries/{id}/scan`, `/api/v1/maintenance/empty_trash`, and `/api/v1/maintenance/clean_artwork` remain outside the REST #2 CRUD standardization scope.
|
||
- Opportunistic-fix policy: backlog/document unrelated issues found en route; fix-in-place only when limited-scope + useful-now, or when deferring would force rework of the new code.
|
||
|
||
## 5. Increment plan (sub-issues under #2 as tracker)
|
||
|
||
One slice = one branch = one PR. PR runs `test` + `migrations` (both required); merge to `main` adds `build` + smoke/E2E. Tests with **NUnit + Shouldly + NSubstitute**, run locally `TZ=UTC`.
|
||
|
||
- **#2a (#34) — API foundation + Channels (pattern-setter).** Shared conventions: idiomatic routing, status-code mapping helpers (incl. `NotFoundError` typed error + 201/422), request-DTO pattern, response = ViewModels, `[EndpointGroupName("general")]`, API-key mutation auth filter (decoupled from the IPTV JWT toggle). Then Channels CRUD (POST/PUT/DELETE, confirm/standardize GET list + add GET `/{id}`). Port page-only channel validations into handlers. Tests: handler unit (success + 404/422), controller status-mapping, create→read→delete EF integration.
|
||
- **#2b (#35) — Collections** CRUD + add/remove items; **retrofit `SmartCollectionController`** to idiomatic (characterization tests first).
|
||
- **#2c (#36) — Schedules** CRUD + schedule items (**TPT-heavy** — the one to budget for). Design the item DTO around the `PlayoutMode` discriminator (One/Multiple/Flood/Duration); reuse `AddProgramScheduleItem` / `ReplaceProgramScheduleItems`. Integration test proving the correct TPT subtype rows are written.
|
||
- **#2d (#37) — Playouts** create (Classic + 4 kinds via discriminated DTO) / delete; keep existing reset. Validation already strong.
|
||
- **#2e (#38) — Standardization cleanup.** Retrofit `FFmpegProfileController`; OpenAPI/doc polish; CORS review for mutations; sweep for any other non-idiomatic `/api` endpoints; fold in backlog items gathered during #2a–#2d. Completed: FFmpeg profile CRUD now uses `/api/v1/ffmpeg/profiles[/{id}]`, request DTOs, API-key write filtering, `ProblemDetails` 404/422 responses, and generated OpenAPI metadata. Deferred: configurable CORS tightening and legacy operational endpoint reshaping.
|
||
|
||
**Sequencing:** #2a first (sets every convention the others copy), then #2b–#2d in parallel-able order, #2e last. Each slice ships its own read endpoints so the new UI gains coverage incrementally.
|
||
|
||
## 6. Testing strategy (per slice)
|
||
- Handler unit tests: success + each failure (not-found→404, validation→422).
|
||
- Controller tests: status-code/`Location`/DTO mapping.
|
||
- EF integration test (real SQLite): create→read→delete proving TPT subtype creation + cascade correctness.
|
||
- Characterization tests before any existing-controller retrofit.
|
||
- Lean on existing nets: architecture tests (#12 — controllers in `ErsatzTV`, logic in `ErsatzTV.Application`), M3U goldens (#11) if a change touches `ToM3U`.
|
||
|
||
## 7. Open items / backlog seeds
|
||
- CORS tightening for mutation routes (if exposed beyond LAN).
|
||
- Legacy operational `/api` command routes (`libraries/*/scan`, maintenance actions) still use older action-style names. They are outside REST #2 CRUD and should be handled in a separate operational API cleanup if needed.
|
||
- List-endpoint filtering/sorting/pagination depth (new-UI driven).
|
||
- API-key provisioning UX for MCP/UI write clients (how a caller obtains/sets `Api__WriteKey`).
|
||
- Decide whether `NotFoundError` typed-error becomes a repo-wide convention or stays API-local.
|
||
```
|