diff --git a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs index 9920bfc5c..58a899ff0 100644 --- a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs +++ b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs @@ -89,11 +89,13 @@ public class JellyfinMusicVideoLibraryScanner : protected override IAsyncEnumerable> GetMusicVideoLibraryItems( JellyfinConnectionParameters connectionParameters, - JellyfinLibrary library) => + JellyfinLibrary library, + MediaServerProjectionFailureCounter projectionFailures) => _jellyfinApiClient.GetMusicVideoLibraryItems( connectionParameters.Address, connectionParameters.AuthorizationHeader, - library); + library, + projectionFailures); protected override async Task>> UpdateMetadata( MediaItemScanResult result, diff --git a/ErsatzTV.Scanner/Core/Metadata/MediaServerMusicVideoLibraryScanner.cs b/ErsatzTV.Scanner/Core/Metadata/MediaServerMusicVideoLibraryScanner.cs index d563357d8..8eb9dbfa5 100644 --- a/ErsatzTV.Scanner/Core/Metadata/MediaServerMusicVideoLibraryScanner.cs +++ b/ErsatzTV.Scanner/Core/Metadata/MediaServerMusicVideoLibraryScanner.cs @@ -58,13 +58,19 @@ public abstract class MediaServerMusicVideoLibraryScanner getLocalPath, IAsyncEnumerable> musicVideoEntries, + MediaServerProjectionFailureCounter projectionFailures, bool deepScan, CancellationToken cancellationToken) { @@ -179,6 +186,7 @@ public abstract class MediaServerMusicVideoLibraryScanner incomingItemIds, List incomingLocalPaths, ImmutableDictionary existingMusicVideos, + MediaServerProjectionFailureCounter projectionFailures, CancellationToken cancellationToken) { // Legacy rows (no identity yet) are reconciled by local path. They are counted into the guard's existing @@ -204,11 +213,15 @@ public abstract class MediaServerMusicVideoLibraryScanner> GetMusicVideoLibraryItems( TConnectionParameters connectionParameters, - TLibrary library); + TLibrary library, + MediaServerProjectionFailureCounter projectionFailures); protected abstract Task>> UpdateMetadata( MediaItemScanResult result, diff --git a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs index 2c4757d06..948dbde03 100644 --- a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs +++ b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs @@ -378,6 +378,54 @@ public class JellyfinMusicVideoLibraryScannerTests (await MediaItemStateOf(legacyId)).ShouldBe(MediaItemState.Normal); } + // ersatztv#484 carried into the ersatztv#496 scanner. A swallowed projection exception drops an item the + // server DID return, which is indistinguishable from a deletion at the reconcile step, so one projection + // regression could mass-flag a healthy library. #484's counter is an OPTIONAL parameter defaulting to 0, so a + // scanner that simply never passes it compiles and silently opts out — which is exactly what this scanner did + // until the guard was threaded through. main's own #484 test covered the pre-#496 hard-delete scanner and is + // replaced by these two. + // Non-vacuous: dropping projectionFailures.Count from the ShouldFlagMissing call flags "gone" and fails. + [Test] + public async Task ScanLibrary_Should_Not_Sweep_When_The_Projection_Reported_Failures() + { + int id = await SeedLibraryPath("/data/music"); + JellyfinLibrary library = BuildLibrary(id, "/data/music"); + + (JellyfinMusicVideoLibraryScanner seed, _) = BuildScanner(FakeApi( + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"), + () => BuildIncoming("/data/music/artist2/gone.mkv", "Artist 2", "Gone"))); + (await seed.ScanLibrary(ConnectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + int goneId = await MusicVideoId("/data/music/artist2/gone.mkv"); + + // "gone" is absent from the incoming set ONLY because its projection threw — not because it was deleted + (JellyfinMusicVideoLibraryScanner scanner, _) = BuildScanner(FakeApiWithProjectionFailures( + 1, + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"))); + (await scanner.ScanLibrary(ConnectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + (await MediaItemStateOf(goneId)).ShouldBe(MediaItemState.Normal); + } + + // ersatztv#484 + ersatztv#496: the refusal must also cover the LEGACY (identity-less) path diff, which is + // more exposed — a legacy row has no etag to fall back on. + [Test] + public async Task ScanLibrary_Should_Not_Sweep_Legacy_Rows_When_The_Projection_Reported_Failures() + { + int libraryPathId = await SeedLibraryPath("/data/music"); + int legacyId = await SeedPlainMusicVideo(libraryPathId, "/data/music/artist9/orphan.mkv", "Artist 9"); + + JellyfinLibrary library = BuildLibrary(libraryPathId, "/data/music"); + (JellyfinMusicVideoLibraryScanner scanner, _) = BuildScanner(FakeApiWithProjectionFailures( + 1, + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"))); + (await scanner.ScanLibrary(ConnectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + (await MediaItemStateOf(legacyId)).ShouldBe(MediaItemState.Normal); + } + // ersatztv#494 (Done-when 3): the music-video sweep must never touch a Movie or Show that shares the // same LibraryPath — the cross-flag risk is a LibraryPathId property, not a Mixed-library property. [Test] @@ -562,84 +610,6 @@ public class JellyfinMusicVideoLibraryScannerTests // alone would only ever reach music videos ADDED after it — an EXISTING item whose album/track is set or // corrected in Jellyfin would keep its stale value forever. Same shape as the #497 collection bug, one // layer up. Non-vacuous: reverting the two assignments in UpdateMetadata fails both assertions below. - // #484: music videos are HARD-deleted by this sweep (there is no per-item FileNotFound seam), so a - // silently dropped projection is immediately destructive here. Parameterised with its own positive - // control: identical arrangement, only the failure count differs. - [TestCase(1, false, TestName = "MusicVideo_Sweep_Is_Skipped_When_A_Projection_Failed")] - [TestCase(0, true, TestName = "MusicVideo_Sweep_Still_Runs_With_No_Projection_Failures")] - public async Task MusicVideo_Sweep_Respects_Projection_Failures(int failures, bool expectDelete) - { - JellyfinLibrary library = BuildLibrary(1, "/data/music"); - var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); - const string KeepPath = "/data/music/artist1/song1.mkv"; - const string GonePath = "/data/music/artist1/song2.mkv"; - - var existing = new MusicVideo - { - Id = 7, - ArtistId = 3, - MediaVersions = - [ - new MediaVersion - { - MediaFiles = [new MediaFile { Path = KeepPath }], - Streams = [] - } - ], - MusicVideoMetadata = - [ - new MusicVideoMetadata { Id = 11, Genres = [], Tags = [], Studios = [], Artists = [] } - ] - }; - - var artistRepository = Substitute.For(); - artistRepository.GetArtistByMetadata(Arg.Any(), Arg.Any()) - .Returns(Some(new Artist { Id = 3, ArtistMetadata = new List() })); - artistRepository.DeleteEmptyArtists(Arg.Any()).Returns(new List()); - - var musicVideoRepository = Substitute.For(); - musicVideoRepository - .GetOrAdd(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Right>( - new MediaItemScanResult(existing) { IsAdded = false })); - - // two rows exist locally, only one comes back — a genuine partial deletion, so #477's empty - // branch cannot mask the assertion - musicVideoRepository.FindMusicVideoPaths(Arg.Any()) - .Returns(new List { KeepPath, GonePath }.AsEnumerable()); - musicVideoRepository.DeleteByPath(Arg.Any(), Arg.Any()).Returns(new List { 7 }); - - var metadataRepository = Substitute.For(); - metadataRepository.Update(Arg.Any()).Returns(true); - metadataRepository.UpdateStatistics(Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(false); - - var libraryRepository = Substitute.For(); - libraryRepository.GetParentFolderId(Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Option.None); - libraryRepository.GetOrAddFolder(Arg.Any(), Arg.Any>(), Arg.Any()) - .Returns(new LibraryFolder { Id = 5, Path = "/data/music/artist1" }); - - JellyfinMusicVideoLibraryScanner scanner = BuildScannerWith( - FakeApiWithProjectionFailures(failures, () => BuildIncoming(KeepPath, "Artist 1", "Song 1")), - artistRepository, - musicVideoRepository, - libraryRepository, - metadataRepository); - - (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) - .IsRight.ShouldBeTrue(); - - if (expectDelete) - { - await musicVideoRepository.Received(1).DeleteByPath(Arg.Any(), GonePath); - } - else - { - await musicVideoRepository.DidNotReceive().DeleteByPath(Arg.Any(), Arg.Any()); - } - } - [Test] public async Task ScanLibrary_Should_Update_Album_And_Track_On_Rescan_Of_Existing_Item() { @@ -946,14 +916,39 @@ public class JellyfinMusicVideoLibraryScannerTests { var apiClient = Substitute.For(); apiClient.GetMusicVideoLibraryItems( - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(_ => Items(items)); return apiClient; } + // Same as FakeApi, but the enumeration also reports `failures` swallowed projection exceptions into the + // counter the scanner handed in — what the real JellyfinApiClient does from ProjectToMusicVideo's catch. + private static IJellyfinApiClient FakeApiWithProjectionFailures( + int failures, + params Func[] items) + { + var apiClient = Substitute.For(); + apiClient.GetMusicVideoLibraryItems( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(ci => + { + var counter = ci.ArgAt(3); + for (var i = 0; i < failures; i++) + { + counter.RecordFailure(); + } + + return Items(items); + }); + return apiClient; + } + private static async IAsyncEnumerable> Items(Func[] items) { foreach (Func item in items) diff --git a/docs/decisions.md b/docs/decisions.md index 8c89b262f..57430825c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -3857,7 +3857,7 @@ This is a **second, independent** refusal on the same guard, plus a decision not ## 2026-07-25 — Music videos carry a per-library server identity; reconciliation is an itemId diff + soft trash (#496) `key: scan.musicvideo-server-identity` · `status: active` · `since: 2026-07-25` · `supersedes: scan.musicvideo-reconciliation@2026-07-20` · `superseded-by: none` **Rule:** Jellyfin music videos carry a per-library server identity (`JellyfinMusicVideo : MusicVideo` with `ItemId`/`Etag`, TPT table + ItemId index), so `JellyfinMusicVideoLibraryScanner` folds onto a shared `MediaServerMusicVideoLibraryScanner` base that diffs the **server item id** and soft-trashes (`FlagFileNotFound`) instead of diffing local paths and hard-deleting. Rows predating the identity are **adopted in place** — the identity row is inserted against the same `MediaItem` id, scoped to the scanned library's own `LibraryPath` — never deleted and re-added. -**Signals:** music-video server identity, JellyfinMusicVideo ItemId/Etag, itemId diff, soft FileNotFound trash, adoption of pre-identity rows, cross-library false-trash, path-replaced PathHash · paths: `JellyfinMusicVideo`, `JellyfinMusicVideoRepository`, `IMediaServerMusicVideoRepository`, `MediaServerMusicVideoLibraryScanner`, `JellyfinMusicVideoLibraryScanner` · issues: #496, #494, #477, #488, #497, #500, #606 +**Signals:** music-video server identity, JellyfinMusicVideo ItemId/Etag, itemId diff, soft FileNotFound trash, adoption of pre-identity rows, cross-library false-trash, path-replaced PathHash · paths: `JellyfinMusicVideo`, `JellyfinMusicVideoRepository`, `IMediaServerMusicVideoRepository`, `MediaServerMusicVideoLibraryScanner`, `JellyfinMusicVideoLibraryScanner` · issues: #496, #494, #477, #484, #488, #497, #500, #606 **Mechanics:** dual-provider migration `Add_JellyfinMusicVideo`; `GetOrAdd` takes `localPath` explicitly and never reads the projection's path; adoption probe joins `MediaFile.PathHash` + `NOT EXISTS (JellyfinMusicVideo)` filtered to the library's `LibraryPath`; `GetByItemId` filters `LibraryPath.LibraryId`; legacy (identity-less) rows reconciled by `GetExistingLegacyMusicVideoPaths` + `FlagFileNotFoundByPaths` and counted into the #477 guard; `ScanLibrary_Should_Adopt_Using_The_PathReplaced_Local_Path`, `ScanLibrary_Should_Adopt_PreExisting_MusicVideo_Preserving_Identity_And_Collections`, `ScanLibrary_Should_Not_Adopt_A_MusicVideo_Owned_By_Another_LibraryPath`, `ScanLibrary_Should_Not_Resolve_An_ItemId_Owned_By_Another_Library`, `ScanLibrary_Should_Flag_A_Legacy_Row_The_Server_No_Longer_Reports` #494 gave music videos a trash sweep but had to key it on `(LibraryPathId, path)` and hard-delete, because @@ -3900,6 +3900,12 @@ owner's sweep **destroyed** the row the other library still served. This is the `Normal` and schedulable forever, strictly worse than the hard delete it replaced. Legacy rows are also counted into the #477 empty-fetch guard: on the first scan after this ships they ARE the whole library, so a guard that counted only identity rows would see "0 existing" and sweep all of them on a transient empty fetch. +- **Both sweeps sit behind #484's projection-failure refusal.** `ShouldFlagMissing`'s `projectionFailureCount` + is an OPTIONAL parameter defaulting to 0, so a scanner that never passes it compiles and silently opts out of + the protection — which is what this scanner did until the counter was threaded through + (`MediaServerProjectionFailureCounter`, one per enumeration, never a field on the singleton api client). The + single guard call covers the legacy path diff too, which is if anything more exposed: a legacy row has no etag + to fall back on. - **Scope honesty: this is parity, not a total fix.** One file path is still one `MediaItem` row globally (`MediaFileAlreadyExists` is a global path-hash guard), so a second library serving the same file still gets no row of its own — exactly as for movies/TV. What changes is that the first library's sweep now *flags* diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 103e0d4c8..4237561eb 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -113,9 +113,8 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `scan.collections-scan-status` | `GET /api/v1/media-sources/collections-scan-status` reports a family-global (not per-source), boolean-only active-scan set read from `IEntityLocker`; the SPA reconciles authoritatively against it (with a grace-tick helper) instead of a fixed client-side timeout. | 2026-07-12 | [link](../decisions.md#2026-07-12--external-collections-scans-get-an-authoritative-status-surface-271-the-spa-timeout-is-retired) | | `scan.getoraddfolder-db-lookup` | `ILibraryRepository.GetOrAddFolder` resolves the existing folder via a DB query on `(LibraryPathId, Path)`, not the caller's `LibraryPath.LibraryFolders` in-memory navigation, since that navigation is only eager-loaded on the local scan path and is null on remote (Jellyfin) callers. | 2026-07-20 | [link](../decisions.md#2026-07-20--ilibraryrepositorygetoraddfolder-resolves-the-folder-from-the-db-not-the-callers-librarypathlibraryfolders-navigation-488) | | `scan.jellyfin-mixed-content-library` | A Jellyfin library whose collection type is `mixed` (or absent) maps to one ErsatzTV library of `LibraryMediaKind.Mixed`, scanned by running the movie/television/music-video scanners in sequence against that single library — a library is a place, not a media kind. | 2026-07-20 | [link](../decisions.md#2026-07-20-489--jellyfin-mixed-content-libraries-map-to-one-library-holding-many-kinds) | -| `scan.musicvideo-reconciliation` | `JellyfinMusicVideoLibraryScanner` reconciles removed music videos by a library-scoped local-path diff plus hard delete (`TrashMissingMusicVideos`), not the server-itemId soft-trash pattern the other media-server scanners use, because music videos carry no server identity. | 2026-07-20 | [link](../decisions.md#2026-07-20--jellyfinmusicvideolibraryscanner-reconciles-by-library-scoped-path-diff--hard-delete-not-server-itemid-soft-trash-494) | -| `scan.projection-failure-sweep-guard` | `MediaServerReconciliationGuard` takes a per-enumeration projection-failure count and refuses the file-not-found sweep (logged loudly) whenever it is non-zero against a non-empty existing set — at all six sweeps, including the nested per-show season and per-season episode ones #477 left unguarded (via `ShouldFlagMissingDescendants`, which applies the failure refusal but not #477's empty-fetch branch). Deliberate guard-clause skips (STRM, virtual, unsupported type) are explicitly **not** failures and never suppress a sweep. The missing-fraction / ratio threshold floated by #477 is **rejected**, not deferred. | 2026-07-25 | [link](../decisions.md#2026-07-25--a-media-server-sweep-also-refuses-when-the-api-client-silently-dropped-items-whose-projection-threw-the-ratio-threshold-is-rejected-484) | | `scan.musicvideo-server-identity` | Jellyfin music videos carry a per-library server identity (`JellyfinMusicVideo : MusicVideo` with `ItemId`/`Etag`, TPT table + ItemId index), so `JellyfinMusicVideoLibraryScanner` folds onto a shared `MediaServerMusicVideoLibraryScanner` base that diffs the **server item id** and soft-trashes (`FlagFileNotFound`) instead of diffing local paths and hard-deleting. Rows predating the identity are **adopted in place** — the identity row is inserted against the same `MediaItem` id, scoped to the scanned library's own `LibraryPath` — never deleted and re-added. | 2026-07-25 | [link](../decisions.md#2026-07-25--music-videos-carry-a-per-library-server-identity-reconciliation-is-an-itemid-diff--soft-trash-496) | +| `scan.projection-failure-sweep-guard` | `MediaServerReconciliationGuard` takes a per-enumeration projection-failure count and refuses the file-not-found sweep (logged loudly) whenever it is non-zero against a non-empty existing set — at all six sweeps, including the nested per-show season and per-season episode ones #477 left unguarded (via `ShouldFlagMissingDescendants`, which applies the failure refusal but not #477's empty-fetch branch). Deliberate guard-clause skips (STRM, virtual, unsupported type) are explicitly **not** failures and never suppress a sweep. The missing-fraction / ratio threshold floated by #477 is **rejected**, not deferred. | 2026-07-25 | [link](../decisions.md#2026-07-25--a-media-server-sweep-also-refuses-when-the-api-client-silently-dropped-items-whose-projection-threw-the-ratio-threshold-is-rejected-484) | | `scan.zero-item-fetch-guard` | A media-server library sweep refuses to flag missing items when a successful fetch returns zero incoming items against a non-empty existing set (`MediaServerReconciliationGuard.ShouldFlagMissing`), rather than treating an ambiguous empty result as a full-library deletion. | 2026-07-19 | [link](../decisions.md#2026-07-19--a-media-server-library-sweep-refuses-to-flag-when-a-successful-fetch-returns-zero-items-rather-than-nuking-the-whole-library-477) | | `sched.auto-tune-foundation` | Auto-tune preview enumeration uses EF distinct+count queries for exact counts, while each created channel is persisted as a live SmartCollection; coexistence with existing channels/numbers is additive-only, never mutating. | 2026-07-16 | [link](../decisions.md#2026-07-16--auto-tuning-enumerates-via-ef-persists-via-smartcollection-additive-coexistence-69) | | `sched.autotune-detailpanel-members` | The Auto-Tune DetailPanel's per-channel content-source list is a live `ISearchIndex.Search` roll-up through the server-owned `AutoTuneAxisMap.GenerateQuery`, not an EF distinct+count query, so the preview matches exactly what the built channel's SmartCollection will contain. | 2026-07-17 | [link](../decisions.md#2026-07-17--auto-tune-detailpanel-member-list--live-search-index-roll-up-not-ef-enumeration-384) |