diff --git a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs index 0b2936e22..90c5f5528 100644 --- a/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs +++ b/ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs @@ -263,11 +263,112 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne existing.DateUpdated = DateTime.UtcNow; existing.MetadataKind = MetadataKind.External; - return await _metadataRepository.Update(existing); + bool updated = await _metadataRepository.Update(existing); + + // ersatztv#497: the scalar Update above marks only the metadata row Modified; it does NOT touch + // child collections, and MusicVideoRepository.GetOrAdd loads them AsNoTracking — so tag/genre/studio/ + // artist edits made in Jellyfin never reached an EXISTING music video (only the Add path persisted + // them). Reconcile the collections that BOTH the Add path persists AND GetOrAdd eager-loads: + // Genres, Tags, Studios, Artists. (Guids are add-persisted but not eager-loaded here — reconciling + // them would see an empty `existing` and duplicate-insert every scan; Directors are eager-loaded + // but not add-persisted for music videos — both are deliberately out of scope.) Mirrors the + // remove-stale + add-new idiom PlexMovieLibraryScanner.UpdateMetadata uses. + updated = await ReconcileGenres(existing, incoming) || updated; + updated = await ReconcileTags(existing, incoming) || updated; + updated = await ReconcileStudios(existing, incoming) || updated; + updated = await ReconcileArtists(existing, incoming) || updated; + + return updated; } incoming.MusicVideoId = musicVideo.Id; musicVideo.MusicVideoMetadata = [incoming]; return await _metadataRepository.Add(incoming); } + + private async Task ReconcileGenres(MusicVideoMetadata existing, MusicVideoMetadata incoming) + { + existing.Genres ??= []; + List incomingGenres = incoming.Genres ?? []; + var updated = false; + + foreach (Genre genre in existing.Genres.Filter(g => incomingGenres.All(g2 => g2.Name != g.Name)).ToList()) + { + existing.Genres.Remove(genre); + updated = await _metadataRepository.RemoveGenre(genre) || updated; + } + + foreach (Genre genre in incomingGenres.Filter(g => existing.Genres.All(g2 => g2.Name != g.Name)).ToList()) + { + existing.Genres.Add(genre); + updated = await _musicVideoRepository.AddGenre(existing, genre) || updated; + } + + return updated; + } + + private async Task ReconcileTags(MusicVideoMetadata existing, MusicVideoMetadata incoming) + { + existing.Tags ??= []; + List incomingTags = incoming.Tags ?? []; + var updated = false; + + foreach (Tag tag in existing.Tags.Filter(t => incomingTags.All(t2 => t2.Name != t.Name)).ToList()) + { + existing.Tags.Remove(tag); + updated = await _metadataRepository.RemoveTag(tag) || updated; + } + + foreach (Tag tag in incomingTags.Filter(t => existing.Tags.All(t2 => t2.Name != t.Name)).ToList()) + { + existing.Tags.Add(tag); + updated = await _musicVideoRepository.AddTag(existing, tag) || updated; + } + + return updated; + } + + private async Task ReconcileStudios(MusicVideoMetadata existing, MusicVideoMetadata incoming) + { + existing.Studios ??= []; + List incomingStudios = incoming.Studios ?? []; + var updated = false; + + foreach (Studio studio in existing.Studios.Filter(s => incomingStudios.All(s2 => s2.Name != s.Name)).ToList()) + { + existing.Studios.Remove(studio); + updated = await _metadataRepository.RemoveStudio(studio) || updated; + } + + foreach (Studio studio in incomingStudios.Filter(s => existing.Studios.All(s2 => s2.Name != s.Name)).ToList()) + { + existing.Studios.Add(studio); + updated = await _musicVideoRepository.AddStudio(existing, studio) || updated; + } + + return updated; + } + + private async Task ReconcileArtists(MusicVideoMetadata existing, MusicVideoMetadata incoming) + { + existing.Artists ??= []; + List incomingArtists = incoming.Artists ?? []; + var updated = false; + + foreach (MusicVideoArtist artist in existing.Artists + .Filter(a => incomingArtists.All(a2 => a2.Name != a.Name)).ToList()) + { + existing.Artists.Remove(artist); + updated = await _musicVideoRepository.RemoveArtist(artist) || updated; + } + + foreach (MusicVideoArtist artist in incomingArtists + .Filter(a => existing.Artists.All(a2 => a2.Name != a.Name)).ToList()) + { + existing.Artists.Add(artist); + updated = await _musicVideoRepository.AddArtist(existing, artist) || updated; + } + + return updated; + } } diff --git a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs index 2cd5d22e2..d1354f379 100644 --- a/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs +++ b/ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs @@ -3,6 +3,7 @@ using ErsatzTV.Core.Domain; using ErsatzTV.Core.Interfaces.Jellyfin; using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Core.Jellyfin; +using ErsatzTV.Core.Metadata; using ErsatzTV.Infrastructure.Data; using ErsatzTV.Infrastructure.Data.Repositories; using ErsatzTV.Scanner.Core.Interfaces; @@ -16,6 +17,7 @@ using NUnit.Framework; using Shouldly; using IFileSystem = System.IO.Abstractions.IFileSystem; using Unit = LanguageExt.Unit; +using static LanguageExt.Prelude; namespace ErsatzTV.Tests.Integration; @@ -276,6 +278,150 @@ public class JellyfinMusicVideoLibraryScannerTests await scannerProxy.DidNotReceive().RemoveMediaItems(Arg.Any(), Arg.Any()); } + // ersatztv#497: metadata edits (genres/tags/studios/artists) made in Jellyfin to an EXISTING music video + // must reach ErsatzTV on the next scan. Before the fix, UpdateMetadata copied only scalar fields, so the + // update path silently dropped every child collection — add-new AND remove-stale. This is an interaction + // test: the repositories are substituted and GetOrAdd returns a canned existing item so we can verify the + // scanner issues the exact reconcile calls. (The real-DB double-scan approach can't drive this here: the + // in-memory harness shares ONE SQLite connection across contexts, and mid-scan GetOrAdd's + // `MediaVersions.First().MediaFiles.First().Path` predicate mis-resolves once the existing item carries + // metadata children — a harness-only quirk; prod uses per-context pooled connections and looks music videos + // up by that predicate only because they carry no server ItemId. See the #497 close comment.) + // Non-vacuous: reverting the Reconcile* calls in UpdateMetadata drops every Received() below. + [Test] + public async Task ScanLibrary_Should_Reconcile_Metadata_Collections_On_Rescan_Of_Existing_Item() + { + JellyfinLibrary library = BuildLibrary(1, "/data/music"); + var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1); + const string VideoPath = "/data/music/artist1/song1.mkv"; + + // the EXISTING item already in ErsatzTV, with the collections Jellyfin first gave it + var existing = new MusicVideo + { + Id = 7, + ArtistId = 3, + MediaVersions = new List + { + new() { MediaFiles = new List { new() { Path = VideoPath } }, Streams = new List() } + }, + MusicVideoMetadata = new List + { + new() + { + Id = 11, + Genres = new List { new() { Name = "Synthwave" }, new() { Name = "Retro" } }, + Tags = new List { new() { Name = "KeepTag" }, new() { Name = "DropTag" } }, + Studios = new List { new() { Name = "OldStudio" } }, + Artists = new List { new() { Name = "Artist 1" }, new() { Name = "Featured X" } } + } + } + }; + + 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 })); + musicVideoRepository.FindMusicVideoPaths(Arg.Any()) + .Returns(new List { VideoPath }.AsEnumerable()); + musicVideoRepository.AddGenre(Arg.Any(), Arg.Any()).Returns(true); + musicVideoRepository.AddTag(Arg.Any(), Arg.Any()).Returns(true); + musicVideoRepository.AddStudio(Arg.Any(), Arg.Any()).Returns(true); + musicVideoRepository.AddArtist(Arg.Any(), Arg.Any()).Returns(true); + musicVideoRepository.RemoveArtist(Arg.Any()).Returns(true); + + var metadataRepository = Substitute.For(); + metadataRepository.Update(Arg.Any()).Returns(true); + metadataRepository.UpdateStatistics(Arg.Any(), Arg.Any(), Arg.Any()).Returns(false); + metadataRepository.RemoveGenre(Arg.Any()).Returns(true); + metadataRepository.RemoveTag(Arg.Any()).Returns(true); + metadataRepository.RemoveStudio(Arg.Any()).Returns(true); + + 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( + FakeApi(() => BuildIncoming( + VideoPath, "Artist 1", "Song 1", + genres: new[] { "Synthwave", "Vaporwave" }, + tags: new[] { "KeepTag", "NewTag" }, + studios: new[] { "NewStudio" }, + artists: new[] { "Artist 1", "Featured Y" })), + artistRepository, + musicVideoRepository, + libraryRepository, + metadataRepository); + + (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + // remove-stale: Retro / DropTag / OldStudio / "Featured X" gone; kept items are NOT removed + await metadataRepository.Received(1).RemoveGenre(Arg.Is(g => g.Name == "Retro")); + await metadataRepository.DidNotReceive().RemoveGenre(Arg.Is(g => g.Name == "Synthwave")); + await metadataRepository.Received(1).RemoveTag(Arg.Is(t => t.Name == "DropTag")); + await metadataRepository.DidNotReceive().RemoveTag(Arg.Is(t => t.Name == "KeepTag")); + await metadataRepository.Received(1).RemoveStudio(Arg.Is(s => s.Name == "OldStudio")); + await musicVideoRepository.Received(1).RemoveArtist(Arg.Is(a => a.Name == "Featured X")); + await musicVideoRepository.DidNotReceive().RemoveArtist(Arg.Is(a => a.Name == "Artist 1")); + + // add-new: Vaporwave / NewTag / NewStudio / "Featured Y" added; kept items are NOT re-added + await musicVideoRepository.Received(1) + .AddGenre(Arg.Any(), Arg.Is(g => g.Name == "Vaporwave")); + await musicVideoRepository.DidNotReceive() + .AddGenre(Arg.Any(), Arg.Is(g => g.Name == "Synthwave")); + await musicVideoRepository.Received(1) + .AddTag(Arg.Any(), Arg.Is(t => t.Name == "NewTag")); + await musicVideoRepository.DidNotReceive() + .AddTag(Arg.Any(), Arg.Is(t => t.Name == "KeepTag")); + await musicVideoRepository.Received(1) + .AddStudio(Arg.Any(), Arg.Is(s => s.Name == "NewStudio")); + await musicVideoRepository.Received(1) + .AddArtist(Arg.Any(), Arg.Is(a => a.Name == "Featured Y")); + await musicVideoRepository.DidNotReceive() + .AddArtist(Arg.Any(), Arg.Is(a => a.Name == "Artist 1")); + } + + private JellyfinMusicVideoLibraryScanner BuildScannerWith( + IJellyfinApiClient apiClient, + IArtistRepository artistRepository, + IMusicVideoRepository musicVideoRepository, + ILibraryRepository libraryRepository, + IMetadataRepository metadataRepository) + { + 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); + + return new JellyfinMusicVideoLibraryScanner( + scannerProxy, + apiClient, + pathReplacement, + mediaSourceRepository, + artistRepository, + musicVideoRepository, + libraryRepository, + metadataRepository, + NullLogger.Instance); + } + private async Task MusicVideoId(string path) { await using TvContext context = _db.CreateContext(); @@ -377,7 +523,14 @@ public class JellyfinMusicVideoLibraryScannerTests }; } - private static MusicVideo BuildIncoming(string path, string artistName, string title) => + private static MusicVideo BuildIncoming( + string path, + string artistName, + string title, + IEnumerable genres = null, + IEnumerable tags = null, + IEnumerable studios = null, + IEnumerable artists = null) => new() { MediaVersions = new List @@ -385,7 +538,8 @@ public class JellyfinMusicVideoLibraryScannerTests new() { MediaFiles = new List { new() { Path = path } }, - Streams = new List() + Streams = new List(), + Chapters = new List() } }, MusicVideoMetadata = new List @@ -393,7 +547,11 @@ public class JellyfinMusicVideoLibraryScannerTests new() { Title = title, - Artists = new List { new() { Name = artistName } } + Genres = (genres ?? Enumerable.Empty()).Select(g => new Genre { Name = g }).ToList(), + Tags = (tags ?? Enumerable.Empty()).Select(t => new Tag { Name = t }).ToList(), + Studios = (studios ?? Enumerable.Empty()).Select(s => new Studio { Name = s }).ToList(), + Artists = (artists ?? new[] { artistName }) + .Select(a => new MusicVideoArtist { Name = a }).ToList() } } };