From 52cfd9f2614a9b626dd755ff2c66209a4747c601 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 19 May 2024 19:59:47 +0200 Subject: [PATCH 01/15] Properly pass replace flag to remote provider logic --- MediaBrowser.Providers/Manager/MetadataService.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index cb8a867f16..e4377e8e6a 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -663,7 +663,7 @@ namespace MediaBrowser.Providers.Manager // If replacing all metadata, run internet providers first if (options.ReplaceAllMetadata) { - var remoteResult = await ExecuteRemoteProviders(temp, logName, id, providers.OfType>(), cancellationToken) + var remoteResult = await ExecuteRemoteProviders(temp, logName, true, id, providers.OfType>(), cancellationToken) .ConfigureAwait(false); refreshResult.UpdateType |= remoteResult.UpdateType; @@ -749,7 +749,7 @@ namespace MediaBrowser.Providers.Manager // Local metadata is king - if any is found don't run remote providers if (!options.ReplaceAllMetadata && (!hasLocalMetadata || options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh || !item.StopRefreshIfLocalMetadataFound)) { - var remoteResult = await ExecuteRemoteProviders(temp, logName, id, providers.OfType>(), cancellationToken) + var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, providers.OfType>(), cancellationToken) .ConfigureAwait(false); refreshResult.UpdateType |= remoteResult.UpdateType; @@ -820,7 +820,7 @@ namespace MediaBrowser.Providers.Manager return new TItemType(); } - private async Task ExecuteRemoteProviders(MetadataResult temp, string logName, TIdType id, IEnumerable> providers, CancellationToken cancellationToken) + private async Task ExecuteRemoteProviders(MetadataResult temp, string logName, bool replaceData, TIdType id, IEnumerable> providers, CancellationToken cancellationToken) { var refreshResult = new RefreshResult(); @@ -845,7 +845,7 @@ namespace MediaBrowser.Providers.Manager { result.Provider = provider.Name; - MergeData(result, temp, Array.Empty(), false, false); + MergeData(result, temp, Array.Empty(), replaceData, false); MergeNewData(temp.Item, id); refreshResult.UpdateType |= ItemUpdateType.MetadataDownload; From 37d7e8f5bf71e40cfa411b82441826317b193744 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 20 May 2024 12:24:57 +0200 Subject: [PATCH 02/15] Fix replacement logic --- .../Manager/MetadataService.cs | 297 +++++++++--------- .../Movies/MovieMetadataService.cs | 4 +- .../Movies/TrailerMetadataService.cs | 9 +- .../Music/AlbumMetadataService.cs | 4 + .../Music/AudioMetadataService.cs | 5 + .../Music/MusicVideoMetadataService.cs | 5 + .../Playlists/PlaylistMetadataService.cs | 21 +- .../TV/SeriesMetadataService.cs | 4 +- .../Manager/MetadataServiceTests.cs | 2 +- 9 files changed, 202 insertions(+), 149 deletions(-) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index e4377e8e6a..ca8242fe3d 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -654,139 +654,123 @@ namespace MediaBrowser.Providers.Manager await RunCustomProvider(provider, item, logName, options, refreshResult, cancellationToken).ConfigureAwait(false); } - var temp = new MetadataResult + if (!item.IsLocked) { - Item = CreateNew() - }; - temp.Item.Path = item.Path; - - // If replacing all metadata, run internet providers first - if (options.ReplaceAllMetadata) - { - var remoteResult = await ExecuteRemoteProviders(temp, logName, true, id, providers.OfType>(), cancellationToken) - .ConfigureAwait(false); - - refreshResult.UpdateType |= remoteResult.UpdateType; - refreshResult.ErrorMessage = remoteResult.ErrorMessage; - refreshResult.Failures += remoteResult.Failures; - } - - var hasLocalMetadata = false; - var foundImageTypes = new List(); - - foreach (var provider in providers.OfType>()) - { - var providerName = provider.GetType().Name; - Logger.LogDebug("Running {Provider} for {Item}", providerName, logName); - - var itemInfo = new ItemInfo(item); - - try + var temp = new MetadataResult { - var localItem = await provider.GetMetadata(itemInfo, options.DirectoryService, cancellationToken).ConfigureAwait(false); + Item = CreateNew() + }; + temp.Item.Path = item.Path; + temp.Item.Id = item.Id; - if (localItem.HasMetadata) + var hasLocalBaseMetadataOrLocked = false; + var foundImageTypes = new List(); + + foreach (var provider in providers.OfType>()) + { + var providerName = provider.GetType().Name; + Logger.LogDebug("Running {Provider} for {Item}", providerName, logName); + + var itemInfo = new ItemInfo(item); + + try { - foreach (var remoteImage in localItem.RemoteImages) + var localItem = await provider.GetMetadata(itemInfo, options.DirectoryService, cancellationToken).ConfigureAwait(false); + + if (localItem.HasMetadata) { - try + foreach (var remoteImage in localItem.RemoteImages) { - if (item.ImageInfos.Any(x => x.Type == remoteImage.Type) - && !options.IsReplacingImage(remoteImage.Type)) + try { - continue; + if (item.ImageInfos.Any(x => x.Type == remoteImage.Type) + && !options.IsReplacingImage(remoteImage.Type)) + { + continue; + } + + await ProviderManager.SaveImage(item, remoteImage.Url, remoteImage.Type, null, cancellationToken).ConfigureAwait(false); + refreshResult.UpdateType |= ItemUpdateType.ImageUpdate; + + // remember imagetype that has just been downloaded + foundImageTypes.Add(remoteImage.Type); + } + catch (HttpRequestException ex) + { + Logger.LogError(ex, "Could not save {ImageType} image: {Url}", Enum.GetName(remoteImage.Type), remoteImage.Url); } - - await ProviderManager.SaveImage(item, remoteImage.Url, remoteImage.Type, null, cancellationToken).ConfigureAwait(false); - refreshResult.UpdateType |= ItemUpdateType.ImageUpdate; - - // remember imagetype that has just been downloaded - foundImageTypes.Add(remoteImage.Type); } - catch (HttpRequestException ex) + + if (foundImageTypes.Count > 0) { - Logger.LogError(ex, "Could not save {ImageType} image: {Url}", Enum.GetName(remoteImage.Type), remoteImage.Url); + imageService.UpdateReplaceImages(options, foundImageTypes); } + + if (imageService.MergeImages(item, localItem.Images, options)) + { + refreshResult.UpdateType |= ItemUpdateType.ImageUpdate; + } + + MergeData(localItem, temp, Array.Empty(), false, true); + refreshResult.UpdateType |= ItemUpdateType.MetadataImport; + + // Only one local provider allowed per item + if (item.IsLocked || localItem.Item.IsLocked || HasBaseMetadata(localItem.Item)) + { + hasLocalBaseMetadataOrLocked = true; + } + + break; } - if (foundImageTypes.Count > 0) - { - imageService.UpdateReplaceImages(options, foundImageTypes); - } - - if (imageService.MergeImages(item, localItem.Images, options)) - { - refreshResult.UpdateType |= ItemUpdateType.ImageUpdate; - } - - MergeData(localItem, temp, Array.Empty(), options.ReplaceAllMetadata, true); - refreshResult.UpdateType |= ItemUpdateType.MetadataImport; - - // Only one local provider allowed per item - if (item.IsLocked || localItem.Item.IsLocked || IsFullLocalMetadata(localItem.Item)) - { - hasLocalMetadata = true; - } - - break; + Logger.LogDebug("{Provider} returned no metadata for {Item}", providerName, logName); } - - Logger.LogDebug("{Provider} returned no metadata for {Item}", providerName, logName); - } - catch (OperationCanceledException) - { - throw; - } - catch (Exception ex) - { - Logger.LogError(ex, "Error in {Provider}", provider.Name); - - // If a local provider fails, consider that a failure - refreshResult.ErrorMessage = ex.Message; - } - } - - // Local metadata is king - if any is found don't run remote providers - if (!options.ReplaceAllMetadata && (!hasLocalMetadata || options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh || !item.StopRefreshIfLocalMetadataFound)) - { - var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, providers.OfType>(), cancellationToken) - .ConfigureAwait(false); - - refreshResult.UpdateType |= remoteResult.UpdateType; - refreshResult.ErrorMessage = remoteResult.ErrorMessage; - refreshResult.Failures += remoteResult.Failures; - } - - if (providers.Any(i => i is not ICustomMetadataProvider)) - { - if (refreshResult.UpdateType > ItemUpdateType.None) - { - if (hasLocalMetadata) + catch (OperationCanceledException) { - MergeData(temp, metadata, item.LockedFields, true, true); + throw; } - else + catch (Exception ex) { - if (!options.RemoveOldMetadata) - { - MergeData(metadata, temp, Array.Empty(), false, false); - } + Logger.LogError(ex, "Error in {Provider}", provider.Name); - // Will always replace all metadata when Scan for new and updated files is used. Else, follow the options. - MergeData(temp, metadata, item.LockedFields, options.MetadataRefreshMode == MetadataRefreshMode.Default || options.ReplaceAllMetadata, false); + // If a local provider fails, consider that a failure + refreshResult.ErrorMessage = ex.Message; } } - } - foreach (var provider in customProviders.Where(i => i is not IPreRefreshProvider)) - { - await RunCustomProvider(provider, item, logName, options, refreshResult, cancellationToken).ConfigureAwait(false); + if (options.ReplaceAllMetadata || !(hasLocalBaseMetadataOrLocked && item.StopRefreshIfLocalMetadataFound) || options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh) + { + var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, providers.OfType>(), cancellationToken) + .ConfigureAwait(false); + + refreshResult.UpdateType |= remoteResult.UpdateType; + refreshResult.ErrorMessage = remoteResult.ErrorMessage; + refreshResult.Failures += remoteResult.Failures; + } + + if (providers.Any(i => i is not ICustomMetadataProvider)) + { + if (refreshResult.UpdateType > ItemUpdateType.None) + { + if (options.RemoveOldMetadata) + { + MergeData(metadata, temp, Array.Empty(), true, true); + } + + MergeData(temp, metadata, item.LockedFields, options.MetadataRefreshMode >= MetadataRefreshMode.Default || options.ReplaceAllMetadata, true); + } + } + + foreach (var provider in customProviders.Where(i => i is not IPreRefreshProvider)) + { + await RunCustomProvider(provider, item, logName, options, refreshResult, cancellationToken).ConfigureAwait(false); + } } return refreshResult; } - protected virtual bool IsFullLocalMetadata(TItemType item) + protected virtual bool HasBaseMetadata(TItemType item) { if (string.IsNullOrWhiteSpace(item.Name)) { @@ -948,11 +932,7 @@ namespace MediaBrowser.Providers.Manager if (replaceData || string.IsNullOrEmpty(target.OriginalTitle)) { - // Safeguard against incoming data having an empty name - if (!string.IsNullOrWhiteSpace(source.OriginalTitle)) - { - target.OriginalTitle = source.OriginalTitle; - } + target.OriginalTitle = source.OriginalTitle; } if (replaceData || !target.CommunityRating.HasValue) @@ -1015,7 +995,7 @@ namespace MediaBrowser.Providers.Manager { targetResult.People = sourceResult.People; } - else if (targetResult.People is not null && sourceResult.People is not null) + else if (sourceResult.People is not null && sourceResult.People.Count >= 0) { MergePeople(sourceResult.People, targetResult.People); } @@ -1048,6 +1028,10 @@ namespace MediaBrowser.Providers.Manager { target.Studios = source.Studios; } + else if (source.Studios.Length >= 0) + { + target.Studios = target.Studios.Concat(source.Studios).Distinct().ToArray(); + } } if (!lockedFields.Contains(MetadataField.Tags)) @@ -1056,6 +1040,10 @@ namespace MediaBrowser.Providers.Manager { target.Tags = source.Tags; } + else if (source.Tags.Length >= 0) + { + target.Tags = target.Tags.Concat(source.Tags).Distinct().ToArray(); + } } if (!lockedFields.Contains(MetadataField.ProductionLocations)) @@ -1064,6 +1052,10 @@ namespace MediaBrowser.Providers.Manager { target.ProductionLocations = source.ProductionLocations; } + else if (source.ProductionLocations.Length >= 0) + { + target.Tags = target.ProductionLocations.Concat(source.ProductionLocations).Distinct().ToArray(); + } } foreach (var id in source.ProviderIds) @@ -1081,17 +1073,28 @@ namespace MediaBrowser.Providers.Manager } } + if (replaceData || !target.CriticRating.HasValue) + { + target.CriticRating = source.CriticRating; + } + + if (replaceData || target.RemoteTrailers.Count == 0) + { + target.RemoteTrailers = source.RemoteTrailers; + } + else if (source.CriticRating.HasValue) + { + target.RemoteTrailers = target.RemoteTrailers.Concat(source.RemoteTrailers).Distinct().ToArray(); + } + MergeAlbumArtist(source, target, replaceData); - MergeCriticRating(source, target, replaceData); - MergeTrailers(source, target, replaceData); MergeVideoInfo(source, target, replaceData); MergeDisplayOrder(source, target, replaceData); if (replaceData || string.IsNullOrEmpty(target.ForcedSortName)) { var forcedSortName = source.ForcedSortName; - - if (!string.IsNullOrWhiteSpace(forcedSortName)) + if (!string.IsNullOrEmpty(forcedSortName)) { target.ForcedSortName = forcedSortName; } @@ -1099,22 +1102,49 @@ namespace MediaBrowser.Providers.Manager if (mergeMetadataSettings) { - target.LockedFields = source.LockedFields; - target.IsLocked = source.IsLocked; + if (replaceData || target.LockedFields.Length == 0) + { + target.LockedFields = source.LockedFields; + } + else + { + target.LockedFields = target.LockedFields.Concat(source.LockedFields).Distinct().ToArray(); + } + + if (replaceData) + { + target.IsLocked = source.IsLocked; + } - // Grab the value if it's there, but if not then don't overwrite with the default if (source.DateCreated != default) { target.DateCreated = source.DateCreated; } - target.PreferredMetadataCountryCode = source.PreferredMetadataCountryCode; - target.PreferredMetadataLanguage = source.PreferredMetadataLanguage; + if (replaceData) + { + target.IsLocked = source.IsLocked; + } + + if (replaceData || string.IsNullOrEmpty(target.PreferredMetadataCountryCode)) + { + target.PreferredMetadataCountryCode = source.PreferredMetadataCountryCode; + } + + if (replaceData || string.IsNullOrEmpty(target.PreferredMetadataLanguage)) + { + target.PreferredMetadataLanguage = source.PreferredMetadataLanguage; + } } } private static void MergePeople(List source, List target) { + if (target is null) + { + target = new List(); + } + foreach (var person in target) { var normalizedName = person.Name.RemoveDiacritics(); @@ -1143,7 +1173,6 @@ namespace MediaBrowser.Providers.Manager if (replaceData || string.IsNullOrEmpty(targetHasDisplayOrder.DisplayOrder)) { var displayOrder = sourceHasDisplayOrder.DisplayOrder; - if (!string.IsNullOrWhiteSpace(displayOrder)) { targetHasDisplayOrder.DisplayOrder = displayOrder; @@ -1161,22 +1190,10 @@ namespace MediaBrowser.Providers.Manager { targetHasAlbumArtist.AlbumArtists = sourceHasAlbumArtist.AlbumArtists; } - } - } - - private static void MergeCriticRating(BaseItem source, BaseItem target, bool replaceData) - { - if (replaceData || !target.CriticRating.HasValue) - { - target.CriticRating = source.CriticRating; - } - } - - private static void MergeTrailers(BaseItem source, BaseItem target, bool replaceData) - { - if (replaceData || target.RemoteTrailers.Count == 0) - { - target.RemoteTrailers = source.RemoteTrailers; + else if (sourceHasAlbumArtist.AlbumArtists.Count >= 0) + { + targetHasAlbumArtist.AlbumArtists = targetHasAlbumArtist.AlbumArtists.Concat(sourceHasAlbumArtist.AlbumArtists).Distinct().ToArray(); + } } } @@ -1184,7 +1201,7 @@ namespace MediaBrowser.Providers.Manager { if (source is Video sourceCast && target is Video targetCast) { - if (replaceData || targetCast.Video3DFormat is null) + if (replaceData || !targetCast.Video3DFormat.HasValue) { targetCast.Video3DFormat = sourceCast.Video3DFormat; } diff --git a/MediaBrowser.Providers/Movies/MovieMetadataService.cs b/MediaBrowser.Providers/Movies/MovieMetadataService.cs index 984a3c122a..a73d98e163 100644 --- a/MediaBrowser.Providers/Movies/MovieMetadataService.cs +++ b/MediaBrowser.Providers/Movies/MovieMetadataService.cs @@ -24,7 +24,7 @@ namespace MediaBrowser.Providers.Movies } /// - protected override bool IsFullLocalMetadata(Movie item) + protected override bool HasBaseMetadata(Movie item) { if (string.IsNullOrWhiteSpace(item.Overview)) { @@ -36,7 +36,7 @@ namespace MediaBrowser.Providers.Movies return false; } - return base.IsFullLocalMetadata(item); + return base.HasBaseMetadata(item); } /// diff --git a/MediaBrowser.Providers/Movies/TrailerMetadataService.cs b/MediaBrowser.Providers/Movies/TrailerMetadataService.cs index ad0c5aaa7b..8d9019e75f 100644 --- a/MediaBrowser.Providers/Movies/TrailerMetadataService.cs +++ b/MediaBrowser.Providers/Movies/TrailerMetadataService.cs @@ -1,5 +1,6 @@ #pragma warning disable CS1591 +using System.Linq; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; @@ -24,7 +25,7 @@ namespace MediaBrowser.Providers.Movies } /// - protected override bool IsFullLocalMetadata(Trailer item) + protected override bool HasBaseMetadata(Trailer item) { if (string.IsNullOrWhiteSpace(item.Overview)) { @@ -36,7 +37,7 @@ namespace MediaBrowser.Providers.Movies return false; } - return base.IsFullLocalMetadata(item); + return base.HasBaseMetadata(item); } /// @@ -48,6 +49,10 @@ namespace MediaBrowser.Providers.Movies { target.Item.TrailerTypes = source.Item.TrailerTypes; } + else + { + target.Item.TrailerTypes = target.Item.TrailerTypes.Concat(source.Item.TrailerTypes).Distinct().ToArray(); + } } } } diff --git a/MediaBrowser.Providers/Music/AlbumMetadataService.cs b/MediaBrowser.Providers/Music/AlbumMetadataService.cs index e4f34776b9..a39bd16cea 100644 --- a/MediaBrowser.Providers/Music/AlbumMetadataService.cs +++ b/MediaBrowser.Providers/Music/AlbumMetadataService.cs @@ -225,6 +225,10 @@ namespace MediaBrowser.Providers.Music { targetItem.Artists = sourceItem.Artists; } + else + { + targetItem.Artists = targetItem.Artists.Concat(sourceItem.Artists).Distinct().ToArray(); + } if (replaceData || string.IsNullOrEmpty(targetItem.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))) { diff --git a/MediaBrowser.Providers/Music/AudioMetadataService.cs b/MediaBrowser.Providers/Music/AudioMetadataService.cs index a5b7cb8959..7b25bc0e49 100644 --- a/MediaBrowser.Providers/Music/AudioMetadataService.cs +++ b/MediaBrowser.Providers/Music/AudioMetadataService.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Library; @@ -60,6 +61,10 @@ namespace MediaBrowser.Providers.Music { targetItem.Artists = sourceItem.Artists; } + else + { + targetItem.Artists = targetItem.Artists.Concat(sourceItem.Artists).Distinct().ToArray(); + } if (replaceData || string.IsNullOrEmpty(targetItem.Album)) { diff --git a/MediaBrowser.Providers/Music/MusicVideoMetadataService.cs b/MediaBrowser.Providers/Music/MusicVideoMetadataService.cs index b97b766309..24c4b55015 100644 --- a/MediaBrowser.Providers/Music/MusicVideoMetadataService.cs +++ b/MediaBrowser.Providers/Music/MusicVideoMetadataService.cs @@ -1,5 +1,6 @@ #pragma warning disable CS1591 +using System.Linq; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; @@ -45,6 +46,10 @@ namespace MediaBrowser.Providers.Music { targetItem.Artists = sourceItem.Artists; } + else + { + targetItem.Artists = targetItem.Artists.Concat(sourceItem.Artists).Distinct().ToArray(); + } } } } diff --git a/MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs b/MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs index 1bd000a48b..43889bfbf5 100644 --- a/MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs +++ b/MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs @@ -1,6 +1,7 @@ #pragma warning disable CS1591 using System.Collections.Generic; +using System.Linq; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; @@ -49,8 +50,24 @@ namespace MediaBrowser.Providers.Playlists if (mergeMetadataSettings) { targetItem.PlaylistMediaType = sourceItem.PlaylistMediaType; - targetItem.LinkedChildren = sourceItem.LinkedChildren; - targetItem.Shares = sourceItem.Shares; + + if (replaceData || targetItem.LinkedChildren.Length == 0) + { + targetItem.LinkedChildren = sourceItem.LinkedChildren; + } + else + { + targetItem.LinkedChildren = sourceItem.LinkedChildren.Concat(targetItem.LinkedChildren).Distinct().ToArray(); + } + + if (replaceData || targetItem.Shares.Count == 0) + { + targetItem.Shares = sourceItem.Shares; + } + else + { + targetItem.Shares = sourceItem.Shares.Concat(targetItem.Shares).DistinctBy(s => s.UserId).ToArray(); + } } } } diff --git a/MediaBrowser.Providers/TV/SeriesMetadataService.cs b/MediaBrowser.Providers/TV/SeriesMetadataService.cs index 2d0bd60b98..c4534329f7 100644 --- a/MediaBrowser.Providers/TV/SeriesMetadataService.cs +++ b/MediaBrowser.Providers/TV/SeriesMetadataService.cs @@ -66,7 +66,7 @@ namespace MediaBrowser.Providers.TV } /// - protected override bool IsFullLocalMetadata(Series item) + protected override bool HasBaseMetadata(Series item) { if (string.IsNullOrWhiteSpace(item.Overview)) { @@ -78,7 +78,7 @@ namespace MediaBrowser.Providers.TV return false; } - return base.IsFullLocalMetadata(item); + return base.HasBaseMetadata(item); } /// diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs index ec4df9981c..227e65989c 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs @@ -77,7 +77,7 @@ namespace Jellyfin.Providers.Tests.Manager [Theory] [InlineData("Name", MetadataField.Name, false)] - [InlineData("OriginalTitle", null, false)] + [InlineData("OriginalTitle", null)] [InlineData("OfficialRating", MetadataField.OfficialRating)] [InlineData("CustomRating")] [InlineData("Tagline")] From e67eb485406c6a5b78b6446be7ae2a146a646125 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 20 May 2024 12:49:26 +0200 Subject: [PATCH 03/15] Never revert locked state --- .../Manager/MetadataService.cs | 17 ++++++----------- .../Manager/MetadataServiceTests.cs | 3 ++- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index ca8242fe3d..ca90536e12 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -1102,7 +1102,12 @@ namespace MediaBrowser.Providers.Manager if (mergeMetadataSettings) { - if (replaceData || target.LockedFields.Length == 0) + if (replaceData || !target.IsLocked) + { + target.IsLocked = target.IsLocked || source.IsLocked; + } + + if (target.LockedFields.Length == 0) { target.LockedFields = source.LockedFields; } @@ -1111,21 +1116,11 @@ namespace MediaBrowser.Providers.Manager target.LockedFields = target.LockedFields.Concat(source.LockedFields).Distinct().ToArray(); } - if (replaceData) - { - target.IsLocked = source.IsLocked; - } - if (source.DateCreated != default) { target.DateCreated = source.DateCreated; } - if (replaceData) - { - target.IsLocked = source.IsLocked; - } - if (replaceData || string.IsNullOrEmpty(target.PreferredMetadataCountryCode)) { target.PreferredMetadataCountryCode = source.PreferredMetadataCountryCode; diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs index 227e65989c..cedcaf9c0f 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Entities.Movies; @@ -19,7 +20,7 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(true, true)] public void MergeBaseItemData_MergeMetadataSettings_MergesWhenSet(bool mergeMetadataSettings, bool defaultDate) { - var newLocked = new[] { MetadataField.Cast }; + var newLocked = new[] { MetadataField.Genres, MetadataField.Cast }; var newString = "new"; var newDate = DateTime.Now; From 8a5a93ee80af501555a4258122c4f2bc188be721 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 21 May 2024 21:14:30 +0200 Subject: [PATCH 04/15] Allow removal of all people from an item --- Emby.Server.Implementations/Data/SqliteItemRepository.cs | 9 +++++---- Emby.Server.Implementations/Library/LibraryManager.cs | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Emby.Server.Implementations/Data/SqliteItemRepository.cs b/Emby.Server.Implementations/Data/SqliteItemRepository.cs index 34d753093d..d27b7185a0 100644 --- a/Emby.Server.Implementations/Data/SqliteItemRepository.cs +++ b/Emby.Server.Implementations/Data/SqliteItemRepository.cs @@ -5222,19 +5222,20 @@ AND Type = @InternalPersonType)"); throw new ArgumentNullException(nameof(itemId)); } - ArgumentNullException.ThrowIfNull(people); - CheckDisposed(); using var connection = GetConnection(); using var transaction = connection.BeginTransaction(); - // First delete chapters + // Delete all existing people first using var command = connection.CreateCommand(); command.CommandText = "delete from People where ItemId=@ItemId"; command.TryBind("@ItemId", itemId); command.ExecuteNonQuery(); - InsertPeople(itemId, people, connection); + if (people is not null) + { + InsertPeople(itemId, people, connection); + } transaction.Commit(); } diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs index cca835e4fa..e66f2496a7 100644 --- a/Emby.Server.Implementations/Library/LibraryManager.cs +++ b/Emby.Server.Implementations/Library/LibraryManager.cs @@ -2812,8 +2812,10 @@ namespace Emby.Server.Implementations.Library } _itemRepository.UpdatePeople(item.Id, people); - - await SavePeopleMetadataAsync(people, cancellationToken).ConfigureAwait(false); + if (people is not null) + { + await SavePeopleMetadataAsync(people, cancellationToken).ConfigureAwait(false); + } } public async Task ConvertImageToLocal(BaseItem item, ItemImageInfo image, int imageIndex, bool removeOnFailure) From f3bf9bcdc8218bd309d58e65bf64ec497a8eccd1 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 21 May 2024 21:47:29 +0200 Subject: [PATCH 05/15] Fix final merge logic --- MediaBrowser.Providers/Manager/MetadataService.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index ca90536e12..1314a6b42f 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -164,7 +164,7 @@ namespace MediaBrowser.Providers.Manager } // Next run remote image providers, but only if local image providers didn't throw an exception - if (!localImagesFailed && refreshOptions.ImageRefreshMode != MetadataRefreshMode.ValidationOnly) + if (!localImagesFailed && refreshOptions.ImageRefreshMode > MetadataRefreshMode.ValidationOnly) { var providers = GetNonLocalImageProviders(item, allImageProviders, refreshOptions).ToList(); @@ -242,7 +242,7 @@ namespace MediaBrowser.Providers.Manager protected async Task SaveItemAsync(MetadataResult result, ItemUpdateType reason, CancellationToken cancellationToken) { - if (result.Item.SupportsPeople && result.People is not null) + if (result.Item.SupportsPeople) { var baseItem = result.Item; @@ -752,9 +752,10 @@ namespace MediaBrowser.Providers.Manager { if (refreshResult.UpdateType > ItemUpdateType.None) { - if (options.RemoveOldMetadata) + if (!options.RemoveOldMetadata) { - MergeData(metadata, temp, Array.Empty(), true, true); + // Add existing metadata to provider result if it does not exist there + MergeData(temp, metadata, Array.Empty(), false, false); } MergeData(temp, metadata, item.LockedFields, options.MetadataRefreshMode >= MetadataRefreshMode.Default || options.ReplaceAllMetadata, true); From 0b64426cf24eadde5c2c56d3e0b0b1ed47f09805 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 21 May 2024 23:08:00 +0200 Subject: [PATCH 06/15] Fix local locked and StopRefreshIfLocalMetadataFound logic --- .../Manager/MetadataService.cs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index 1314a6b42f..21f339b7d8 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -663,9 +663,7 @@ namespace MediaBrowser.Providers.Manager temp.Item.Path = item.Path; temp.Item.Id = item.Id; - var hasLocalBaseMetadataOrLocked = false; var foundImageTypes = new List(); - foreach (var provider in providers.OfType>()) { var providerName = provider.GetType().Name; @@ -714,12 +712,6 @@ namespace MediaBrowser.Providers.Manager MergeData(localItem, temp, Array.Empty(), false, true); refreshResult.UpdateType |= ItemUpdateType.MetadataImport; - // Only one local provider allowed per item - if (item.IsLocked || localItem.Item.IsLocked || HasBaseMetadata(localItem.Item)) - { - hasLocalBaseMetadataOrLocked = true; - } - break; } @@ -738,7 +730,9 @@ namespace MediaBrowser.Providers.Manager } } - if (options.ReplaceAllMetadata || !(hasLocalBaseMetadataOrLocked && item.StopRefreshIfLocalMetadataFound) || options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh) + var hasLocalBaseMetadata = HasBaseMetadata(temp.Item); + var isLocalLocked = temp.Item.IsLocked; + if (!isLocalLocked && !(hasLocalBaseMetadata && item.StopRefreshIfLocalMetadataFound) && (options.ReplaceAllMetadata || options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh)) { var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, providers.OfType>(), cancellationToken) .ConfigureAwait(false); @@ -752,13 +746,21 @@ namespace MediaBrowser.Providers.Manager { if (refreshResult.UpdateType > ItemUpdateType.None) { - if (!options.RemoveOldMetadata) + if (isLocalLocked) { - // Add existing metadata to provider result if it does not exist there - MergeData(temp, metadata, Array.Empty(), false, false); + MergeData(temp, metadata, item.LockedFields, true, true); } + else + { + if (!options.RemoveOldMetadata) + { + // Add existing metadata to provider result if it does not exist there + MergeData(temp, metadata, Array.Empty(), false, false); + } - MergeData(temp, metadata, item.LockedFields, options.MetadataRefreshMode >= MetadataRefreshMode.Default || options.ReplaceAllMetadata, true); + var shouldReplace = options.MetadataRefreshMode >= MetadataRefreshMode.Default || options.ReplaceAllMetadata; + MergeData(temp, metadata, item.LockedFields, shouldReplace, true); + } } } From 2a612611b84a907f21abfa19e2d3d2b9312bc7f5 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 21 May 2024 23:08:30 +0200 Subject: [PATCH 07/15] Extend minimum local metadata requirements --- .../Music/AlbumMetadataService.cs | 21 +++++++++++++++++++ .../Music/AudioMetadataService.cs | 21 +++++++++++++++++++ .../TV/EpisodeMetadataService.cs | 16 ++++++++++++++ .../TV/SeasonMetadataService.cs | 16 ++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/MediaBrowser.Providers/Music/AlbumMetadataService.cs b/MediaBrowser.Providers/Music/AlbumMetadataService.cs index a39bd16cea..07fc2363ba 100644 --- a/MediaBrowser.Providers/Music/AlbumMetadataService.cs +++ b/MediaBrowser.Providers/Music/AlbumMetadataService.cs @@ -208,6 +208,27 @@ namespace MediaBrowser.Providers.Music return updateType; } + /// + protected override bool HasBaseMetadata(MusicAlbum item) + { + if (string.IsNullOrWhiteSpace(item.Album)) + { + return false; + } + + if (item.AlbumArtists.Count == 0 && item.Artists.Count == 0) + { + return false; + } + + if (!item.ProductionYear.HasValue) + { + return false; + } + + return base.HasBaseMetadata(item); + } + /// protected override void MergeData( MetadataResult source, diff --git a/MediaBrowser.Providers/Music/AudioMetadataService.cs b/MediaBrowser.Providers/Music/AudioMetadataService.cs index 7b25bc0e49..f25b848a92 100644 --- a/MediaBrowser.Providers/Music/AudioMetadataService.cs +++ b/MediaBrowser.Providers/Music/AudioMetadataService.cs @@ -49,6 +49,27 @@ namespace MediaBrowser.Providers.Music } } + /// + protected override bool HasBaseMetadata(Audio item) + { + if (item.IndexNumber is null) + { + return false; + } + + if (string.IsNullOrEmpty(item.Album)) + { + return false; + } + + if (item.AlbumArtists.Count == 0 && item.Artists.Count == 0) + { + return false; + } + + return base.HasBaseMetadata(item); + } + /// protected override void MergeData(MetadataResult