diff --git a/ErsatzTV.Infrastructure/Data/Repositories/MediaSourceRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/MediaSourceRepository.cs index a66b0d7cf..1282cd1be 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/MediaSourceRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/MediaSourceRepository.cs @@ -1,4 +1,4 @@ -using Dapper; +using Dapper; using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Emby; @@ -477,7 +477,11 @@ public class MediaSourceRepository(IDbContextFactory dbContextFactory { library.Id = 0; library.ShouldSyncItems = false; - library.LastScan = SystemTime.MinValueUtc; + // ersatztv#460: "never scanned" is null, not the 0001-01-01 MinValue sentinel. Every read path + // either coalesces (`LastScan ?? SystemTime.MinValueUtc`) for its own non-nullable + // scan-comparison, or is the API read-boundary coercion (`NormalizeLastScan`, which maps the + // sentinel to null anyway) — so scan-comparison behavior is unchanged. + library.LastScan = null; } await dbContext.PlexLibraries.AddRangeAsync(libraries); @@ -608,7 +612,11 @@ public class MediaSourceRepository(IDbContextFactory dbContextFactory { library.Id = 0; library.ShouldSyncItems = false; - library.LastScan = SystemTime.MinValueUtc; + // ersatztv#460: "never scanned" is null, not the 0001-01-01 MinValue sentinel. Every read path + // either coalesces (`LastScan ?? SystemTime.MinValueUtc`) for its own non-nullable + // scan-comparison, or is the API read-boundary coercion (`NormalizeLastScan`, which maps the + // sentinel to null anyway) — so scan-comparison behavior is unchanged. + library.LastScan = null; } await dbContext.JellyfinLibraries.AddRangeAsync(libraries); @@ -973,7 +981,11 @@ public class MediaSourceRepository(IDbContextFactory dbContextFactory { library.Id = 0; library.ShouldSyncItems = false; - library.LastScan = SystemTime.MinValueUtc; + // ersatztv#460: "never scanned" is null, not the 0001-01-01 MinValue sentinel. Every read path + // either coalesces (`LastScan ?? SystemTime.MinValueUtc`) for its own non-nullable + // scan-comparison, or is the API read-boundary coercion (`NormalizeLastScan`, which maps the + // sentinel to null anyway) — so scan-comparison behavior is unchanged. + library.LastScan = null; } await dbContext.EmbyLibraries.AddRangeAsync(libraries); diff --git a/ErsatzTV.Tests/Integration/MediaSourceRepositoryDisableSyncTests.cs b/ErsatzTV.Tests/Integration/MediaSourceRepositoryDisableSyncTests.cs new file mode 100644 index 000000000..c13aeff52 --- /dev/null +++ b/ErsatzTV.Tests/Integration/MediaSourceRepositoryDisableSyncTests.cs @@ -0,0 +1,109 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Integration; + +// ersatztv#460: the disable-sync (remove-and-recreate) flows used to stamp the 0001-01-01 MinValue +// sentinel into Library.LastScan. #409 fixed the READ side (the API coerces the sentinel to null and a +// migration cleaned historical rows), so the wire contract was already correct — these tests pin the +// WRITE side for every provider, so no NEW sentinel rows are created. They do not (and cannot) speak to +// rows already written before this change: those keep the sentinel at rest and are covered only by the +// read-boundary coercion, since this ships no second cleanup migration. +[TestFixture] +public class MediaSourceRepositoryDisableSyncTests +{ + private InMemoryTvContext _db = null!; + private MediaSourceRepository _repository = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _repository = new MediaSourceRepository(_db.Factory, NullLogger.Instance); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task DisablePlexLibrarySync_Should_Null_LastScan_Not_Write_MinValue() + { + int libraryId = await Seed(new PlexLibrary + { + Name = "Movies", + MediaKind = LibraryMediaKind.Movies, + MediaSourceId = 1, + Key = "key1", + ShouldSyncItems = true, + LastScan = new DateTime(2026, 7, 1, 12, 0, 0, DateTimeKind.Utc), + Paths = [new LibraryPath { Path = "/data/movies" }] + }); + + await _repository.DisablePlexLibrarySync([libraryId]); + + await using TvContext context = _db.CreateContext(); + PlexLibrary library = await context.PlexLibraries.SingleAsync(l => l.Name == "Movies"); + library.LastScan.ShouldBeNull(); + library.ShouldSyncItems.ShouldBeFalse(); + } + + [Test] + public async Task DisableJellyfinLibrarySync_Should_Null_LastScan_Not_Write_MinValue() + { + int libraryId = await Seed(new JellyfinLibrary + { + Name = "Concerts", + MediaKind = LibraryMediaKind.MusicVideos, + MediaSourceId = 1, + ItemId = "item1", + ShouldSyncItems = true, + LastScan = new DateTime(2026, 7, 1, 12, 0, 0, DateTimeKind.Utc), + Paths = [new LibraryPath { Path = "/data/music" }], + PathInfos = [] + }); + + await _repository.DisableJellyfinLibrarySync([libraryId]); + + await using TvContext context = _db.CreateContext(); + JellyfinLibrary library = await context.JellyfinLibraries.SingleAsync(l => l.Name == "Concerts"); + library.LastScan.ShouldBeNull(); + library.ShouldSyncItems.ShouldBeFalse(); + } + + [Test] + public async Task DisableEmbyLibrarySync_Should_Null_LastScan_Not_Write_MinValue() + { + int libraryId = await Seed(new EmbyLibrary + { + Name = "Shows", + MediaKind = LibraryMediaKind.Shows, + MediaSourceId = 1, + ItemId = "item1", + ShouldSyncItems = true, + LastScan = new DateTime(2026, 7, 1, 12, 0, 0, DateTimeKind.Utc), + Paths = [new LibraryPath { Path = "/data/shows" }], + PathInfos = [] + }); + + await _repository.DisableEmbyLibrarySync([libraryId]); + + await using TvContext context = _db.CreateContext(); + EmbyLibrary library = await context.EmbyLibraries.SingleAsync(l => l.Name == "Shows"); + library.LastScan.ShouldBeNull(); + library.ShouldSyncItems.ShouldBeFalse(); + } + + private async Task Seed(Library library) + { + await using TvContext context = _db.CreateContext(); + await context.AddAsync(library); + await context.SaveChangesAsync(); + return library.Id; + } +} diff --git a/docs/decisions.md b/docs/decisions.md index e08c43f2c..ead57c816 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2098,26 +2098,30 @@ Refs: #398 (closed), #420, PR #455. ## 2026-07-18 — Never-scanned `LastScan` surfaces as null at the API boundary, not the 0001-01-01 MinValue sentinel (#409) `key: media.lastscan-null-boundary` · `status: active` · `since: 2026-07-18` · `supersedes: none` · `superseded-by: none` **Rule:** A never-scanned `LastScan` surfaces as `null` at the API/MCP boundary, not the `0001-01-01` MinValue sentinel — enforced by an ongoing read-boundary coercion plus a one-time data migration cleanup. -**Signals:** LastScan, MinValue sentinel, API-boundary honesty · paths: `GetAllMediaSourcesForApiHandler.NormalizeLastScan`, `MediaSourceRepository` · issues: #409 -**Mechanics:** `NullOutNeverScannedLastScan` migration; `LibrariesScreen.tsx` `hasScanned` removal +**Signals:** LastScan, MinValue sentinel, API-boundary honesty · paths: `GetAllMediaSourcesForApiHandler.NormalizeLastScan`, `MediaSourceRepository` · issues: #409, #460 +**Mechanics:** `NullOutNeverScannedLastScan` migration; `LibrariesScreen.tsx` `hasScanned` removal; #460 write-side `null` `Library.LastScan` / `LibraryPath.LastScan` are `DateTime?`; a never-scanned library is `null` at -runtime for a freshly-created row. But the `0001-01-01 00:00:00` MinValue sentinel still appears in the -DB from **two** sources — and the second is ongoing, not historical: +runtime for a freshly-created row. But the `0001-01-01 00:00:00` MinValue sentinel appeared in the +DB from **two** sources — both now closed: 1. Old `Reset_*` migrations wrote it via raw SQL (`UPDATE Library SET LastScan = '0001-01-01 00:00:00'`). -2. **Live code still writes it today**: `MediaSourceRepository` sets `library.LastScan = - SystemTime.MinValueUtc` on the Plex/Jellyfin/Emby remove-and-recreate (disable-sync) flows - (`MediaSourceRepository.cs:480/611/976`). So the sentinel keeps being written during normal use. +2. Live code wrote it on the Plex/Jellyfin/Emby remove-and-recreate (disable-sync) flows in + `MediaSourceRepository`. **Closed by #460**: those three writes now set `null`, so normal use no longer + produces new sentinel rows. Safe because every remaining `LastScan` reader either coalesces + (`LastScan ?? SystemTime.MinValueUtc`) for its own non-nullable scan-comparison, or is the read-boundary + coercion below — scan-comparison behavior is unchanged. `GetAllMediaSourcesForApiHandler` projected `l.LastScan` straight onto its `DateTime?` DTO, so that sentinel leaked to API/MCP clients as a fake midnight timestamp — the SPA papered over it with a client-side year<1900 heuristic (#409 first pass). Decision: the API is the right place to be honest, so **never-scanned reports as null** for API/MCP parity with the UI, via two layers: -- **Read-boundary coercion — the load-bearing, ongoing guard** (provider/history-independent): - `GetAllMediaSourcesForApiHandler.NormalizeLastScan` maps any `< 2000-01-01` value to null. Because - source #2 above keeps writing the sentinel, this coercion is *permanent*, not a stopgap — a - migration-only fix would regress the next time a user toggles a library's sync off. +- **Read-boundary coercion — the load-bearing guard** (provider/history-independent): + `GetAllMediaSourcesForApiHandler.NormalizeLastScan` maps any `< 2000-01-01` value to null. It stays + *permanent* even after #460 closed source #2: it is what makes the contract hold for sentinel rows this + codebase is not going to rewrite — a restored or hand-edited DB, and any row written by source #2 + between #409's cleanup migration and #460 — and a migration-only fix would have regressed the next time + a user toggled a library's sync off. - **Data migration** (`NullOutNeverScannedLastScan`, dual-provider): a one-time cleanup of the historical residue — `UPDATE Library/LibraryPath SET LastScan = NULL WHERE LastScan IS NOT NULL AND LastScan < '2000-01-01'`. Data-only (empty `Up`/`Down` otherwise, both `TvContextModelSnapshot.cs` byte-identical). @@ -2132,8 +2136,17 @@ internal scanner code that coalesces to `SystemTime.MinValueUtc` for its own non `DateTimeOffset` scan-comparison needs and never serializes it to a client. The DTO field was already `DateTime?`, so the OpenAPI schema is unchanged (no regen). The SPA's `hasScanned` heuristic in `LibrariesScreen.tsx` was removed — both call sites revert to a plain null/truthy check now that the API -is honest. (Follow-up option, not done here: have `MediaSourceRepository` write `null` instead of -`MinValue` so the data is clean at rest too; the read coercion makes that non-urgent.) +is honest. + +**#460 (write side, 2026-07-25):** the follow-up this record deferred is done — the three disable-sync +writes set `null` instead of `MinValue`, so **no new** sentinel rows are created. Deliberately ships +**no** second cleanup migration: rows written by source #2 between #409's `NullOutNeverScannedLastScan` +and this change still hold the sentinel at rest, and the read coercion above (not a migration) is what +keeps the contract honest for them. Regression-tested per provider in +`MediaSourceRepositoryDisableSyncTests`. `LibraryPath.LastScan` is deliberately left untouched — the +disable-sync flows re-add `Paths` with their original values, and the only readers of the path-level +column are the local-library scan handlers, so it has no API surface and no remote-scan effect. This +*extends* the rule rather than reversing it: the read-boundary coercion is unchanged and authoritative. ## 2026-07-19 — WeightedShuffle SPA: weights edited on the multi-collection, order offered only on classic MultiCollection schedule items; fair-share is a reset not a mode (#404) `key: sched.weightedshuffle-editor` · `status: active` · `since: 2026-07-19` · `supersedes: none` · `superseded-by: none` diff --git a/docs/testing.md b/docs/testing.md index b224d2335..3b40281b8 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -7,8 +7,8 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points | Project | Covers | Notes | |---|---|---| -| `ErsatzTV.Tests` | API controllers + MediatR handlers | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. 828 tests currently. | -| `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. 543 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). | +| `ErsatzTV.Tests` | API controllers + MediatR handlers | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. ~1,870 tests (approximate on purpose — an exact count goes stale on every PR that adds one; the previous hardcoded 828 was off by over a thousand). | +| `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. ~650 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). | | `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. | | `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. | | `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |