diff --git a/CLAUDE.md b/CLAUDE.md index 50f23fd4e..5867c5ccf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,6 +53,7 @@ docker build -f docker/Dockerfile -t ersatztv:dev . ## Conventions +- **Read [`docs/contributing.md`](docs/contributing.md)** before non-trivial changes — it documents the established patterns (layering, CQRS handlers, LanguageExt, Blazor/MudBlazor, EF Core + dual-provider migrations, the FFmpeg pipeline, analyzers, testing) and the **deviation policy**: match the established style; diverge only with a concrete, stated reason. - Follow existing MediatR CQRS pattern for new features - Domain logic in `ErsatzTV.Core`, infrastructure in `ErsatzTV.Infrastructure` - Keep Blazor pages thin — delegate to MediatR handlers diff --git a/docs/contributing.md b/docs/contributing.md new file mode 100644 index 000000000..33977d08f --- /dev/null +++ b/docs/contributing.md @@ -0,0 +1,140 @@ +# Contributing to the ErsatzTV fork + +A **descriptive** guide to the conventions already in this codebase — written so future changes +(human or AI) extend the established architecture instead of reinventing it. It's a living +document: update it when a pattern legitimately evolves. + +## The guiding principle + +**Match the established style and patterns. Diverge only when there's a genuinely strong reason — +when it's the real best or only option — even if that means rearchitecting. Never deviate +gratuitously or just because a different approach is locally convenient. When a deviation is +justified, state why** (in the PR/commit, and in code comments where it'll surprise a reader). + +Several of the rules below are now **enforced in CI** (layering tests, analyzers, formatting, +migration checks) — see the pointers per section. + +## 1. Architecture & layering + +CQRS via MediatR. Dependency direction is enforced by `ErsatzTV.Architecture.Tests` (NetArchTest): + +| Project | Responsibility | May depend on | +|---|---|---| +| `ErsatzTV.FFmpeg` | ffmpeg process/pipeline wrapper | (nothing — lowest layer) | +| `ErsatzTV.Core` | domain entities, interfaces, pure logic | `FFmpeg` only — **no** EF Core, **no** Infrastructure/Application | +| `ErsatzTV.Infrastructure` | EF Core data access, external clients | `Core` | +| `ErsatzTV.Infrastructure.Sqlite` / `.MySql` | provider-specific EF (migrations) | `Core`, `Infrastructure` | +| `ErsatzTV.Application` | MediatR handlers (business logic) | `Core`, `Infrastructure` (the **abstraction**, not the concrete providers) | +| `ErsatzTV.Scanner` | library scanning host | `Core`, `Infrastructure*` | +| `ErsatzTV` | ASP.NET Core host, Blazor, controllers, DI composition root | everything | + +If a test fails here, the fix is almost always to move the type, not to relax the rule. (ersatztv#12) + +## 2. CQRS handler conventions + +- Requests are **`record` types** named for the action — `GetX` / `CreateX` / `UpdateX` / `DeleteX` — + implementing `IRequest`, living under `/Queries/` or `/Commands/` + **next to** their handler. +- **Result types** (LanguageExt, see §3): queries return `Option` or `List`; commands return + `Option` (void success) or `Either` (value success). +- Handlers implement `IRequestHandler` with **primary-constructor DI** and an + `async Task Handle(TRequest request, CancellationToken cancellationToken)`. +- **Validation lives in the handler** as private static methods returning `Validation`, + composed with tuple `.Apply(...)` — **not** FluentValidation or MediatR pipeline behaviors. +- Examples: `ErsatzTV.Application/Channels/Queries/GetChannelById.cs` (+ `…/GetChannelByIdHandler.cs`, + `Option`), `ErsatzTV.Application/FFmpegProfiles/Commands/DeleteFFmpegProfileHandler.cs` + (tuple-`Apply` validation → `Either`). + +## 3. Functional style (LanguageExt) + +Every project globally imports `using static LanguageExt.Prelude;`. Use the monads instead of +exceptions for control flow. + +- **`Option`** — `Optional(x)`, `Some`/`None`, `HeadOrNone()` to pluck from a collection; + branch with `Match`/`MatchAsync(onSome, onNone)`; default with `IfNone(...)`; transform with + `Map`/`Bind`; `foreach (var x in option)` to unpack conditionally. +- **`Either` / `Validation`** — the error backbone. `BaseError.New("…")` + for failures, `Unit.Default` for void success. Accumulate multiple checks with `Validation` (tuple + `.Apply((a, b, c) => …)`) then `ToEither()`. `MapLeft` to transform errors. +- Examples: `ErsatzTV.Core/FFmpeg/…`, `ErsatzTV.Application/FFmpegProfiles/Commands/DeleteFFmpegProfileHandler.cs`. + +## 4. Blazor / MudBlazor UI + +- **Keep pages thin** — inject `IMediator` and `await Mediator.Send(new SomeQuery(...), token)`; + no business logic or data access in the component. +- Pages `@implements IDisposable` and own a `CancellationTokenSource` per lifecycle phase, cancelled + on parameter change / disposal. +- Forms: `MudForm` + **Blazored FluentValidation** (`Validation="@(validator.ValidateValue)"`, + `For="@(() => _model.Prop)"`); surface errors via `Snackbar.Add(msg, Severity.Error)`. +- Dialogs: pass `DialogParameters`, accept `[CascadingParameter] IMudDialogInstance`, close with + `MudDialog.Close(DialogResult.Ok(...))`. Global providers + event subscriptions live in + `Shared/MainLayout.razor` (unsubscribed in `Dispose()`). +- Examples: `ErsatzTV/Pages/Channels.razor`, `ErsatzTV/Pages/ChannelEditor.razor`, + `ErsatzTV/Shared/ChannelPreviewDialog.razor`. + +## 5. Data access & EF Core + +- `TvContext` (`ErsatzTV.Infrastructure/Data/TvContext.cs`) is the single DbContext; resolve it via + `IDbContextFactory` in handlers. +- **Two providers**: `ErsatzTV.Infrastructure.Sqlite` (the prod default, `/config/ersatztv.sqlite3`) + and `…MySql`, each with its own `Migrations/` set + `TvContextModelSnapshot`. The provider is + chosen by the `provider` config key in `Startup`. +- **A model change needs a migration in BOTH providers** — run `scripts/add-migration.sh `. + CI's `migrations` job enforces model-drift + apply-to-fresh-DB per provider (see `docs/ci-cd.md` + → Migration integrity). Review non-transactional ops (e.g. SQLite `PRAGMA foreign_keys`) carefully. + (ersatztv#13) + +## 6. FFmpeg pipeline + +- **Step-based composition**: each task is an `IPipelineStep` (`ErsatzTV.FFmpeg/IPipelineStep.cs`) that + contributes `GlobalOptions`/`InputOptions`/`FilterOptions`/`OutputOptions` and a `NextState()` that + threads frame metadata (pixel format, data location, dimensions). `CommandGenerator.GenerateArguments` + assembles the steps into the final argument list. +- **Detect capabilities at runtime; don't hardcode** — `IHardwareCapabilities` implementations + (`Vaapi`/`Nvidia`/… `HardwareCapabilities`) report available codecs/profiles, and builders pick the + encoder/decoder accordingly. +- **Version-gate features explicitly** — e.g. `Capabilities/NvidiaHardwareCapabilities.cs` parses + `FFmpegCapabilities.Version` and disables 10-bit H.264 decode hwaccel below ffmpeg 8. Follow this + pattern for the ffmpeg 8 upgrade (ersatztv#9). +- Examples: `ErsatzTV.FFmpeg/Encoder/EncoderBase.cs`, `ErsatzTV.FFmpeg/Filter/ScaleFilter.cs`. + +## 7. Naming, formatting, analyzers + +- **`.editorconfig` is the source of truth** for formatting + rule severities (plus + `ErsatzTV.sln.DotSettings` for ReSharper). Run `dotnet format` before committing. +- **`TreatWarningsAsErrors=true`** in the app projects — a warning fails the build. `NoWarn` carries a + small, documented exemption list (e.g. `VSTHRD200`, `CA1873`); NuGet-audit `NU1901-1903` are demoted + to warnings in `Directory.Build.props` while `NU1904` (critical) blocks. +- **Static-analysis packs** (Roslynator, SonarAnalyzer, Meziantou, AsyncFixer) run at `suggestion` and + are promoted to `warning`/`error` rule-by-rule; promotion is the enforcement (the TWAE build). New + rules start at suggestion — never flip a wall of rules to error at once. (ersatztv#15) + +## 8. Testing + +- **NUnit** (`[TestFixture]`/`[Test]`/`[TestCase]`) + **Shouldly** (`.ShouldBe(...)`) + **NSubstitute** + + Testably.Abstractions for a fake filesystem. **xUnit is not used.** Tests live in `*.Tests` + projects mirroring the source. +- Established kinds: **FFmpeg command-string assertions** (build a pipeline → assert the exact arg + string — `ErsatzTV.FFmpeg.Tests/PipelineBuilderBaseTests.cs`); **golden-file tests** for + Jellyfin-facing output (`ErsatzTV.Core.Tests/Iptv/ChannelPlaylistGoldenTests.cs`, regen via + `ETV_UPDATE_GOLDENS=1`, ersatztv#11); **architecture tests** (§1, ersatztv#12). +- Run local tests with **`TZ=UTC`** to match CI (a couple of `DateTimeOffset` scheduling tests are + timezone-sensitive — ersatztv#24). CI uses `dotnet test … --blame-hang-timeout 2m`. + +## 9. Build / CI + +- **Central Package Management**: versions live in the repo-root `Directory.Packages.props`; csproj + reference packages by name only — never put `Version=` back on a `` (NU1008). + Repo-wide MSBuild config is in `Directory.Build.props` / `Directory.Build.targets`. (ersatztv#14) +- The Gitea Actions pipeline (`.gitea/workflows/docker-build.yml`): `test` → `migrations` → + `build` (image + smoke/IPTV-E2E). **Renovate** opens dependency PRs; a weekly `dependency-scan` + surfaces advisories. Full details: **`docs/ci-cd.md`**. +- Versioning is CalVer `vYY..` (not year.month); never `[skip ci]` a commit you'll + tag. See `docs/ci-cd.md` → Versioning. + +## 10. Deviation policy + +Re-stating §0 because it's the whole point: **follow the established pattern; diverge only with a +concrete, stated reason.** Record an intentional departure in the PR description and a code comment at +the point of surprise (e.g. "using X instead of the usual Y because …"). If a pattern itself should +change, change it deliberately and update this guide in the same PR.