diff --git a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs index 94705a0b1..0b2936e22 100644 --- a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs +++ b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core; +using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using ErsatzTV.Core.Extensions; @@ -7,6 +7,7 @@ using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Core.Jellyfin; using ErsatzTV.Core.Metadata; using ErsatzTV.Scanner.Core.Interfaces; +using ErsatzTV.Scanner.Core.Metadata; using Microsoft.Extensions.Logging; namespace ErsatzTV.Scanner.Core.Jellyfin; @@ -77,6 +78,7 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne await _mediaSourceRepository.GetJellyfinPathReplacements(library.MediaSourceId); var processed = 0; + var incomingPaths = new List(); await foreach ((MusicVideo incoming, int totalCount) in _jellyfinApiClient .GetMusicVideoLibraryItems( connectionParameters.Address, @@ -96,6 +98,8 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne return new ScanCanceled(); } + incomingPaths.Add(GetLocalPath(pathReplacements, incoming)); + Either> maybeMusicVideo = await ProcessMusicVideo(library, libraryPath, pathReplacements, incoming, cancellationToken); @@ -114,9 +118,62 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne } } + await TrashMissingMusicVideos(library, libraryPath, incomingPaths, cancellationToken); + return Unit.Default; } + // ersatztv#494: remove music videos (and now-empty artists) that Jellyfin no longer reports. + // + // Identity is LIBRARY-SCOPED by (LibraryPathId, path): FindMusicVideoPaths and DeleteByPath both filter + // LibraryPathId AND join the concrete MusicVideo table, so this can never touch a Movie/Show that shares + // the same LibraryPath (a mixed library) — the cross-delete safety is a property of those queries, not of + // the media kind. Unlike the MediaServer{Movie,Television,OtherVideo} base scanners, music videos carry no + // server ItemId/Etag (there is no JellyfinMusicVideo entity), so we diff on the local path instead of the + // server item id, and hard-delete rather than soft-trash (there is no per-item FileNotFound seam here). + // + // Known limitation: MusicVideoRepository.GetOrAdd dedups a path GLOBALLY (no LibraryPathId predicate), so a + // file served by two libraries with overlapping local paths is a single row owned by whichever library + // scanned it first. If that owning library later stops reporting the file while another library still + // serves it, this sweep removes the shared row. A proper fix needs per-library music-video identity (a + // JellyfinMusicVideo etag entity + migration) — the issue's deferred "option 2"; tracked as a follow-up. + private async Task TrashMissingMusicVideos( + JellyfinLibrary library, + LibraryPath libraryPath, + List incomingPaths, + CancellationToken cancellationToken) + { + var existingPaths = (await _musicVideoRepository.FindMusicVideoPaths(libraryPath)).ToList(); + + // #477: refuse the sweep when a successful fetch returned zero items but rows exist locally — an empty + // incoming set is indistinguishable from a transient error and would otherwise wipe the whole library. + if (!MediaServerReconciliationGuard.ShouldFlagMissing( + _logger, + library.Name, + incomingPaths.Count, + existingPaths.Count)) + { + return; + } + + foreach (string path in existingPaths.Except(incomingPaths)) + { + List musicVideoIds = await _musicVideoRepository.DeleteByPath(libraryPath, path); + if (musicVideoIds.Count > 0 && + !await _scannerProxy.RemoveMediaItems(musicVideoIds.ToArray(), cancellationToken)) + { + _logger.LogWarning("Failed to remove media items from scanner process"); + } + } + + List artistIds = await _artistRepository.DeleteEmptyArtists(libraryPath); + if (artistIds.Count > 0 && + !await _scannerProxy.RemoveMediaItems(artistIds.ToArray(), cancellationToken)) + { + _logger.LogWarning("Failed to remove empty artists from scanner process"); + } + } + private async Task>> ProcessMusicVideo( JellyfinLibrary library, LibraryPath libraryPath, diff --git a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs index cc6da681e..2cd5d22e2 100644 --- a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs +++ b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs @@ -118,6 +118,265 @@ public class JellyfinMusicVideoLibraryScannerTests folder.LibraryPathId.ShouldBe(libraryPathId); } + // ersatztv#494: a music video removed from Jellyfin must be removed from ErsatzTV on the next scan. + [Test] + public async Task ScanLibrary_Should_Remove_MusicVideo_Missing_From_Jellyfin() + { + int id = await SeedLibraryPath("/data/music"); + JellyfinLibrary library = BuildLibrary(id, "/data/music"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + + // first scan seeds two music videos + (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(); + (await MusicVideoPaths(id)).Count.ShouldBe(2); + int goneId = await MusicVideoId("/data/music/artist2/gone.mkv"); + + // second scan: only "keep" is still present upstream + (JellyfinMusicVideoLibraryScanner scanner, IScannerProxy scannerProxy) = BuildScanner(FakeApi( + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"))); + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + List paths = await MusicVideoPaths(id); + paths.ShouldBe(new[] { "/data/music/artist1/keep.mkv" }); + await scannerProxy.Received().RemoveMediaItems( + Arg.Is(ids => ids.Contains(goneId)), + Arg.Any()); + } + + // ersatztv#494: an artist left with zero music videos is cleaned up. + [Test] + public async Task ScanLibrary_Should_Cleanup_Empty_Artist() + { + int id = await SeedLibraryPath("/data/music"); + JellyfinLibrary library = BuildLibrary(id, "/data/music"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + + (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(); + (await ArtistTitles(id)).Count.ShouldBe(2); + + // "Artist 2" loses its only music video + (JellyfinMusicVideoLibraryScanner scanner, _) = BuildScanner(FakeApi( + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"))); + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + (await ArtistTitles(id)).ShouldBe(new[] { "Artist 1" }); + } + + // ersatztv#494 (Done-when 3): the music-video sweep must never delete a Movie or Show that shares the + // same LibraryPath — the cross-delete risk is a LibraryPathId property, not a Mixed-library property. + [Test] + public async Task ScanLibrary_Should_Not_CrossDelete_Movie_Or_Show_Sharing_The_LibraryPath() + { + int id = await SeedLibraryPath("/data/mixed"); + + await using (TvContext seedContext = _db.CreateContext()) + { + seedContext.Add(new Movie { LibraryPathId = id, MovieMetadata = new List() }); + seedContext.Add(new Show { LibraryPathId = id, ShowMetadata = new List() }); + await seedContext.SaveChangesAsync(); + } + + JellyfinLibrary library = BuildLibrary(id, "/data/mixed"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + + // seed one music video under the same LibraryPath + (JellyfinMusicVideoLibraryScanner seed, _) = BuildScanner(FakeApi( + () => BuildIncoming("/data/mixed/song.mkv", "Artist 1", "Song"))); + (await seed.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + // next scan drops "song.mkv" and adds "song2.mkv" — the sweep removes song.mkv + (JellyfinMusicVideoLibraryScanner scanner, _) = BuildScanner(FakeApi( + () => BuildIncoming("/data/mixed/song2.mkv", "Artist 1", "Song 2"))); + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + await using TvContext context = _db.CreateContext(); + (await context.Movies.CountAsync(m => m.LibraryPathId == id)).ShouldBe(1); + (await context.Shows.CountAsync(s => s.LibraryPathId == id)).ShouldBe(1); + (await MusicVideoPaths(id)).ShouldBe(new[] { "/data/mixed/song2.mkv" }); + } + + // ersatztv#477 guard: a successful-but-empty fetch must NOT wipe the library. Negative control — if the + // sweep were ungated, an empty incoming set would remove every existing music video. + [Test] + public async Task ScanLibrary_Should_Not_Sweep_When_Jellyfin_Returns_Zero_Items() + { + int id = await SeedLibraryPath("/data/music"); + JellyfinLibrary library = BuildLibrary(id, "/data/music"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + + (JellyfinMusicVideoLibraryScanner seed, _) = BuildScanner(FakeApi( + () => BuildIncoming("/data/music/artist1/keep.mkv", "Artist 1", "Keep"))); + (await seed.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + // Jellyfin returns zero items (mid-restore / transient) — the sweep must be skipped + (JellyfinMusicVideoLibraryScanner scanner, IScannerProxy scannerProxy) = BuildScanner(FakeApi()); + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + (await MusicVideoPaths(id)).ShouldBe(new[] { "/data/music/artist1/keep.mkv" }); + await scannerProxy.DidNotReceive().RemoveMediaItems(Arg.Any(), Arg.Any()); + } + + // #493 session field data: a MIXED Jellyfin library runs the music-video arm with a legitimately EMPTY + // incoming set on every scan while Movies/Shows exist under the same LibraryPath (the real "Standup" case). + // If existing were computed by LibraryPathId alone, existing.Except([]) would wipe the movies/episodes; the + // MusicVideo-joined FindMusicVideoPaths (not just the empty-fetch guard) is what protects them. Seed a Movie + // that even carries a MediaFile path, to prove the sweep never touches a non-music-video row. + [Test] + public async Task ScanLibrary_Should_Not_Touch_Movies_Or_Shows_When_No_MusicVideos_Present() + { + int id = await SeedLibraryPath("/data/standup"); + + await using (TvContext seedContext = _db.CreateContext()) + { + seedContext.Add(new Movie + { + LibraryPathId = id, + MovieMetadata = new List(), + MediaVersions = new List + { + new() + { + MediaFiles = new List + { + new() { Path = "/data/standup/show.mkv", PathHash = "hash-standup" } + }, + Streams = new List() + } + } + }); + seedContext.Add(new Show { LibraryPathId = id, ShowMetadata = new List() }); + await seedContext.SaveChangesAsync(); + } + + JellyfinLibrary library = BuildLibrary(id, "/data/standup"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + + // the music-video arm returns zero incoming — steady state for a mixed library + (JellyfinMusicVideoLibraryScanner scanner, IScannerProxy scannerProxy) = BuildScanner(FakeApi()); + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + await using TvContext context = _db.CreateContext(); + (await context.Movies.CountAsync(m => m.LibraryPathId == id)).ShouldBe(1); + (await context.Shows.CountAsync(s => s.LibraryPathId == id)).ShouldBe(1); + await scannerProxy.DidNotReceive().RemoveMediaItems(Arg.Any(), Arg.Any()); + } + + private async Task MusicVideoId(string path) + { + await using TvContext context = _db.CreateContext(); + List musicVideos = await context.MusicVideos + .Include(mv => mv.MediaVersions) + .ThenInclude(v => v.MediaFiles) + .ToListAsync(); + return musicVideos + .Single(mv => mv.MediaVersions.Single().MediaFiles.Single().Path == path) + .Id; + } + + private async Task> MusicVideoPaths(int libraryPathId) + { + await using TvContext context = _db.CreateContext(); + List musicVideos = await context.MusicVideos + .Include(mv => mv.MediaVersions) + .ThenInclude(v => v.MediaFiles) + .Where(mv => mv.LibraryPathId == libraryPathId) + .ToListAsync(); + return musicVideos + .Select(mv => mv.MediaVersions.Single().MediaFiles.Single().Path) + .OrderBy(p => p) + .ToList(); + } + + private async Task> ArtistTitles(int libraryPathId) + { + await using TvContext context = _db.CreateContext(); + List artists = await context.Artists + .Include(a => a.ArtistMetadata) + .Where(a => a.LibraryPathId == libraryPathId) + .ToListAsync(); + return artists.Select(a => a.ArtistMetadata.Single().Title).OrderBy(t => t).ToList(); + } + + private (JellyfinMusicVideoLibraryScanner Scanner, IScannerProxy ScannerProxy) BuildScanner( + IJellyfinApiClient apiClient) + { + var pathReplacement = Substitute.For(); + pathReplacement + .GetReplacementJellyfinPath(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(1)); + + var mediaSourceRepository = Substitute.For(); + mediaSourceRepository.GetJellyfinPathReplacements(Arg.Any()) + .Returns(Task.FromResult(new List())); + + var scannerProxy = Substitute.For(); + scannerProxy.UpdateProgress(Arg.Any(), Arg.Any()).Returns(true); + scannerProxy.ReindexMediaItems(Arg.Any(), Arg.Any()).Returns(true); + scannerProxy.RemoveMediaItems(Arg.Any(), Arg.Any()).Returns(true); + + var scanner = new JellyfinMusicVideoLibraryScanner( + scannerProxy, + apiClient, + pathReplacement, + mediaSourceRepository, + new ArtistRepository(_db.Factory), + new MusicVideoRepository(_db.Factory), + new LibraryRepository(Substitute.For(), _db.Factory), + Substitute.For(), + NullLogger.Instance); + + return (scanner, scannerProxy); + } + + // Each Func builds a fresh MusicVideo so the async stream can be re-enumerated across ScanLibrary calls + // (an IAsyncEnumerable iterator is single-use, and the scanner mutates the incoming item). + private static IJellyfinApiClient FakeApi(params Func[] items) + { + var apiClient = Substitute.For(); + apiClient.GetMusicVideoLibraryItems(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(_ => Items(items)); + return apiClient; + } + + private static async IAsyncEnumerable> Items(Func[] items) + { + foreach (Func item in items) + { + yield return new Tuple(item(), items.Length); + } + + await Task.CompletedTask; + } + + private static JellyfinLibrary BuildLibrary(int libraryPathId, string path) + { + var libraryPath = new LibraryPath { Id = libraryPathId, Path = path, LibraryFolders = null }; + return new JellyfinLibrary + { + Id = 42, + MediaSourceId = 1, + ItemId = "lib15", + Name = "Music", + ShouldSyncItems = true, + Paths = new List { libraryPath } + }; + } + private static MusicVideo BuildIncoming(string path, string artistName, string title) => new() { diff --git a/docs/decisions.md b/docs/decisions.md index 9caa80296..e89069f2c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2253,3 +2253,40 @@ remote scanner tripped it, and the feature had never run in prod, CI, or locally watching both tests fail with the issue's `ArgumentNullException('source')`. Note: `CleanEtagsForLibraryPath` still reads `libraryPath.LibraryFolders` directly, but it is only reached on the local (eager-loaded) path, so it is not affected; left as-is (out of #488 scope). + +## 2026-07-20 — `JellyfinMusicVideoLibraryScanner` reconciles by library-scoped path diff + hard delete, not server itemId soft-trash (#494) + +The Jellyfin music-video scanner did add/update only — a music video removed on the Jellyfin side lingered in +ErsatzTV forever and could still be scheduled. It now runs a trash sweep at the end of `ScanLibrary` +(`TrashMissingMusicVideos`), mirroring the `MediaServer{Movie,Television,OtherVideo}LibraryScanner` "gone +upstream ⇒ remove" pattern but with a deliberately different identity function, because music videos lack the +media-server identity those base scanners rely on. + +- **Identity is (LibraryPathId, path), not server itemId.** The base scanners diff `GetExisting*` (keyed by + `MediaServerItemId`) against the incoming server item ids, then soft-trash via `FlagFileNotFound`. Music videos + have **no `JellyfinMusicVideo` entity and no `ItemId`/`Etag`** — the scanner is a standalone + `IJellyfinMusicVideoLibraryScanner` that injects the *local* `IMusicVideoRepository`, which offers no + itemId-keyed existing-set or flag seam. So the sweep diffs the **local path** set instead: existing = + `FindMusicVideoPaths(libraryPath)` `.Except` the incoming items' replaced local paths, then hard-deletes the + remainder with `DeleteByPath` + `IScannerProxy.RemoveMediaItems`, and cleans now-empty artists with + `IArtistRepository.DeleteEmptyArtists`. Hard delete (not soft `FileNotFound` trash) because there is no + per-item FileNotFound seam on this path and the issue's Done-when is "removed". +- **Cross-kind safety is a property of the queries, not the media kind.** `MediaItem` is TPT with `LibraryPathId` + on the abstract base, so a Movie, Show and MusicVideo can share one `LibraryPath` (a mixed Jellyfin library). + Both `FindMusicVideoPaths` and `DeleteByPath` filter `LibraryPathId` **and** join the concrete `MusicVideo` + table, so the sweep can only ever see/delete music videos — a Movie/Show under the same `LibraryPath` is + invisible to it. Pinned by `ScanLibrary_Should_Not_CrossDelete_Movie_Or_Show_Sharing_The_LibraryPath`. +- **Reuses the #477 empty-fetch guard.** The sweep is gated by `MediaServerReconciliationGuard.ShouldFlagMissing` + — a successful fetch that returns zero items (server mid-restore / transient) is indistinguishable from a real + emptying, so the whole-library wipe is refused and logged. Pinned as a negative control by + `ScanLibrary_Should_Not_Sweep_When_Jellyfin_Returns_Zero_Items` (removing the guard flips it red). +- **Known limitation (deferred to per-library identity).** `MusicVideoRepository.GetOrAdd` dedups a path + **globally** (no `LibraryPathId` predicate), so a file served by two libraries with overlapping local paths is + a single row owned by whichever library scanned it first. If that owner later stops reporting the file while + another library still serves it, this sweep removes the shared row. A proper fix needs per-library music-video + identity (a `JellyfinMusicVideo` etag entity + migration) — the issue's "option 2 / fold into the base + scanner" refactor — tracked as #496. +- **Tests.** Integration tests (real `ArtistRepository`/`MusicVideoRepository`/`LibraryRepository` over in-memory + SQLite, extending the #488 harness) pin removal, empty-artist cleanup, cross-kind safety, and the empty-fetch + guard. Proven non-vacuous: all four fail against the pre-fix scanner except the guard control, which only + earns its keep once the sweep exists.