Browse Source

Merge pull request #7078 from 1337joe/metadata-merge-data

Bond-009 3 years ago
parent
commit
ef0708d876
35 changed files with 906 additions and 561 deletions
  1. 1 1
      Emby.Server.Implementations/ApplicationHost.cs
  2. 2 1
      Jellyfin.Api/Controllers/ItemLookupController.cs
  3. 5 0
      MediaBrowser.Controller/Providers/ImageRefreshOptions.cs
  4. 1 0
      MediaBrowser.Controller/Providers/MetadataRefreshOptions.cs
  5. 1 1
      MediaBrowser.Providers/Books/AudioBookMetadataService.cs
  6. 1 1
      MediaBrowser.Providers/Books/BookMetadataService.cs
  7. 1 1
      MediaBrowser.Providers/BoxSets/BoxSetMetadataService.cs
  8. 0 7
      MediaBrowser.Providers/Channels/ChannelMetadataService.cs
  9. 0 7
      MediaBrowser.Providers/Folders/CollectionFolderMetadataService.cs
  10. 0 7
      MediaBrowser.Providers/Folders/FolderMetadataService.cs
  11. 0 7
      MediaBrowser.Providers/Folders/UserViewMetadataService.cs
  12. 0 7
      MediaBrowser.Providers/Genres/GenreMetadataService.cs
  13. 0 7
      MediaBrowser.Providers/LiveTv/LiveTvMetadataService.cs
  14. 67 54
      MediaBrowser.Providers/Manager/ItemImageProvider.cs
  15. 322 25
      MediaBrowser.Providers/Manager/MetadataService.cs
  16. 0 295
      MediaBrowser.Providers/Manager/ProviderUtils.cs
  17. 1 1
      MediaBrowser.Providers/Movies/MovieMetadataService.cs
  18. 1 1
      MediaBrowser.Providers/Movies/TrailerMetadataService.cs
  19. 1 1
      MediaBrowser.Providers/Music/AlbumMetadataService.cs
  20. 0 7
      MediaBrowser.Providers/Music/ArtistMetadataService.cs
  21. 1 1
      MediaBrowser.Providers/Music/AudioMetadataService.cs
  22. 1 1
      MediaBrowser.Providers/Music/MusicVideoMetadataService.cs
  23. 0 7
      MediaBrowser.Providers/MusicGenres/MusicGenreMetadataService.cs
  24. 0 7
      MediaBrowser.Providers/People/PersonMetadataService.cs
  25. 0 7
      MediaBrowser.Providers/Photos/PhotoAlbumMetadataService.cs
  26. 0 7
      MediaBrowser.Providers/Photos/PhotoMetadataService.cs
  27. 1 1
      MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs
  28. 0 7
      MediaBrowser.Providers/Studios/StudioMetadataService.cs
  29. 1 1
      MediaBrowser.Providers/TV/EpisodeMetadataService.cs
  30. 0 6
      MediaBrowser.Providers/TV/SeasonMetadataService.cs
  31. 1 1
      MediaBrowser.Providers/TV/SeriesMetadataService.cs
  32. 0 7
      MediaBrowser.Providers/Videos/VideoMetadataService.cs
  33. 0 7
      MediaBrowser.Providers/Years/YearMetadataService.cs
  34. 119 70
      tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs
  35. 378 0
      tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs

+ 1 - 1
Emby.Server.Implementations/ApplicationHost.cs

@@ -973,7 +973,7 @@ namespace Emby.Server.Implementations
             yield return typeof(IServerApplicationHost).Assembly;
 
             // Include composable parts in the Providers assembly
-            yield return typeof(ProviderUtils).Assembly;
+            yield return typeof(ProviderManager).Assembly;
 
             // Include composable parts in the Photos assembly
             yield return typeof(PhotoProvider).Assembly;

+ 2 - 1
Jellyfin.Api/Controllers/ItemLookupController.cs

@@ -263,7 +263,8 @@ namespace Jellyfin.Api.Controllers
                     ImageRefreshMode = MetadataRefreshMode.FullRefresh,
                     ReplaceAllMetadata = true,
                     ReplaceAllImages = replaceAllImages,
-                    SearchResult = searchResult
+                    SearchResult = searchResult,
+                    RemoveOldMetadata = true
                 },
                 CancellationToken.None).ConfigureAwait(false);
 

+ 5 - 0
MediaBrowser.Controller/Providers/ImageRefreshOptions.cs

@@ -27,6 +27,11 @@ namespace MediaBrowser.Controller.Providers
 
         public bool IsAutomated { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether old metadata should be removed if it isn't replaced.
+        /// </summary>
+        public bool RemoveOldMetadata { get; set; }
+
         public bool IsReplacingImage(ImageType type)
         {
             return ImageRefreshMode == MetadataRefreshMode.FullRefresh &&

+ 1 - 0
MediaBrowser.Controller/Providers/MetadataRefreshOptions.cs

@@ -30,6 +30,7 @@ namespace MediaBrowser.Controller.Providers
             ReplaceAllImages = copy.ReplaceAllImages;
             ReplaceImages = copy.ReplaceImages;
             SearchResult = copy.SearchResult;
+            RemoveOldMetadata = copy.RemoveOldMetadata;
 
             if (copy.RefreshPaths != null && copy.RefreshPaths.Length > 0)
             {

+ 1 - 1
MediaBrowser.Providers/Books/AudioBookMetadataService.cs

@@ -31,7 +31,7 @@ namespace MediaBrowser.Providers.Books
             bool replaceData,
             bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 1 - 1
MediaBrowser.Providers/Books/BookMetadataService.cs

@@ -26,7 +26,7 @@ namespace MediaBrowser.Providers.Books
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Book> source, MetadataResult<Book> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             if (replaceData || string.IsNullOrEmpty(target.Item.SeriesName))
             {

+ 1 - 1
MediaBrowser.Providers/BoxSets/BoxSetMetadataService.cs

@@ -47,7 +47,7 @@ namespace MediaBrowser.Providers.BoxSets
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<BoxSet> source, MetadataResult<BoxSet> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 7
MediaBrowser.Providers/Channels/ChannelMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Channels;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Channels
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Channel> source, MetadataResult<Channel> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Folders/CollectionFolderMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Folders
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<CollectionFolder> source, MetadataResult<CollectionFolder> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Folders/FolderMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -26,11 +25,5 @@ namespace MediaBrowser.Providers.Folders
         /// <inheritdoc />
         // Make sure the type-specific services get picked first
         public override int Order => 10;
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Folder> source, MetadataResult<Folder> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Folders/UserViewMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Folders
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<UserView> source, MetadataResult<UserView> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Genres/GenreMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Genres
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Genre> source, MetadataResult<Genre> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/LiveTv/LiveTvMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.LiveTv;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.LiveTv
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<LiveTvChannel> source, MetadataResult<LiveTvChannel> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 67 - 54
MediaBrowser.Providers/Manager/ItemImageProvider.cs

@@ -62,6 +62,29 @@ namespace MediaBrowser.Providers.Manager
             _fileSystem = fileSystem;
         }
 
+        /// <summary>
+        /// Removes all existing images from the provided item.
+        /// </summary>
+        /// <param name="item">The <see cref="BaseItem"/> to remove images from.</param>
+        /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
+        public bool RemoveImages(BaseItem item)
+        {
+            var singular = new List<ItemImageInfo>();
+            for (var i = 0; i < _singularImages.Length; i++)
+            {
+                var currentImage = item.GetImageInfo(_singularImages[i], 0);
+                if (currentImage != null)
+                {
+                    singular.Add(currentImage);
+                }
+            }
+
+            singular.AddRange(item.GetImages(ImageType.Backdrop));
+            PruneImages(item, singular);
+
+            return singular.Count > 0;
+        }
+
         /// <summary>
         /// Verifies existing images have valid paths and adds any new local images provided.
         /// </summary>
@@ -100,7 +123,7 @@ namespace MediaBrowser.Providers.Manager
         public async Task<RefreshResult> RefreshImages(
             BaseItem item,
             LibraryOptions libraryOptions,
-            List<IImageProvider> providers,
+            IEnumerable<IImageProvider> providers,
             ImageRefreshOptions refreshOptions,
             CancellationToken cancellationToken)
         {
@@ -160,14 +183,14 @@ namespace MediaBrowser.Providers.Manager
 
                 foreach (var imageType in images)
                 {
-                    if (!IsEnabled(savedOptions, imageType))
+                    if (!savedOptions.IsEnabled(imageType))
                     {
                         continue;
                     }
 
-                    if (!HasImage(item, imageType) || (refreshOptions.IsReplacingImage(imageType) && !downloadedImages.Contains(imageType)))
+                    if (!item.HasImage(imageType) || (refreshOptions.IsReplacingImage(imageType) && !downloadedImages.Contains(imageType)))
                     {
-                        _logger.LogDebug("Running {0} for {1}", provider.GetType().Name, item.Path ?? item.Name);
+                        _logger.LogDebug("Running {Provider} for {Item}", provider.GetType().Name, item.Path ?? item.Name);
 
                         var response = await provider.GetImage(item, imageType, cancellationToken).ConfigureAwait(false);
 
@@ -183,7 +206,7 @@ namespace MediaBrowser.Providers.Manager
                             {
                                 if (response.Protocol == MediaProtocol.Http)
                                 {
-                                    _logger.LogDebug("Setting image url into item {0}", item.Id);
+                                    _logger.LogDebug("Setting image url into item {Item}", item.Id);
                                     var index = item.AllowsMultipleImages(imageType) ? item.GetImages(imageType).Count() : 0;
                                     item.SetImage(
                                         new ItemImageInfo
@@ -220,39 +243,6 @@ namespace MediaBrowser.Providers.Manager
             }
         }
 
-        private bool HasImage(BaseItem item, ImageType type)
-        {
-            return item.HasImage(type);
-        }
-
-        /// <summary>
-        /// Determines if an item already contains the given images.
-        /// </summary>
-        /// <param name="item">The item.</param>
-        /// <param name="images">The images.</param>
-        /// <param name="savedOptions">The saved options.</param>
-        /// <param name="backdropLimit">The backdrop limit.</param>
-        /// <returns><c>true</c> if the specified item contains images; otherwise, <c>false</c>.</returns>
-        private bool ContainsImages(BaseItem item, List<ImageType> images, TypeOptions savedOptions, int backdropLimit)
-        {
-            // Using .Any causes the creation of a DisplayClass aka. variable capture
-            for (var i = 0; i < _singularImages.Length; i++)
-            {
-                var type = _singularImages[i];
-                if (images.Contains(type) && !HasImage(item, type) && savedOptions.GetLimit(type) > 0)
-                {
-                    return false;
-                }
-            }
-
-            if (images.Contains(ImageType.Backdrop) && item.GetImages(ImageType.Backdrop).Count() < backdropLimit)
-            {
-                return false;
-            }
-
-            return true;
-        }
-
         /// <summary>
         /// Refreshes from a remote provider.
         /// </summary>
@@ -289,7 +279,7 @@ namespace MediaBrowser.Providers.Manager
                     return;
                 }
 
-                _logger.LogDebug("Running {0} for {1}", provider.GetType().Name, item.Path ?? item.Name);
+                _logger.LogDebug("Running {Provider} for {Item}", provider.GetType().Name, item.Path ?? item.Name);
 
                 var images = await _providerManager.GetAvailableRemoteImages(
                     item,
@@ -305,12 +295,12 @@ namespace MediaBrowser.Providers.Manager
 
                 foreach (var imageType in _singularImages)
                 {
-                    if (!IsEnabled(savedOptions, imageType))
+                    if (!savedOptions.IsEnabled(imageType))
                     {
                         continue;
                     }
 
-                    if (!HasImage(item, imageType) || (refreshOptions.IsReplacingImage(imageType) && !downloadedImages.Contains(imageType)))
+                    if (!item.HasImage(imageType) || (refreshOptions.IsReplacingImage(imageType) && !downloadedImages.Contains(imageType)))
                     {
                         minWidth = savedOptions.GetMinWidth(imageType);
                         var downloaded = await DownloadImage(item, provider, result, list, minWidth, imageType, cancellationToken).ConfigureAwait(false);
@@ -336,14 +326,37 @@ namespace MediaBrowser.Providers.Manager
             }
         }
 
-        private bool IsEnabled(TypeOptions options, ImageType type)
+        /// <summary>
+        /// Determines if an item already contains the given images.
+        /// </summary>
+        /// <param name="item">The item.</param>
+        /// <param name="images">The images.</param>
+        /// <param name="savedOptions">The saved options.</param>
+        /// <param name="backdropLimit">The backdrop limit.</param>
+        /// <returns><c>true</c> if the specified item contains images; otherwise, <c>false</c>.</returns>
+        private bool ContainsImages(BaseItem item, List<ImageType> images, TypeOptions savedOptions, int backdropLimit)
         {
-            return options.IsEnabled(type);
+            // Using .Any causes the creation of a DisplayClass aka. variable capture
+            for (var i = 0; i < _singularImages.Length; i++)
+            {
+                var type = _singularImages[i];
+                if (images.Contains(type) && !item.HasImage(type) && savedOptions.GetLimit(type) > 0)
+                {
+                    return false;
+                }
+            }
+
+            if (images.Contains(ImageType.Backdrop) && item.GetImages(ImageType.Backdrop).Count() < backdropLimit)
+            {
+                return false;
+            }
+
+            return true;
         }
 
-        private void PruneImages(BaseItem item, ItemImageInfo[] images)
+        private void PruneImages(BaseItem item, IReadOnlyList<ItemImageInfo> images)
         {
-            for (var i = 0; i < images.Length; i++)
+            for (var i = 0; i < images.Count; i++)
             {
                 var image = images[i];
 
@@ -355,6 +368,11 @@ namespace MediaBrowser.Providers.Manager
                     }
                     catch (FileNotFoundException)
                     {
+                        // nothing to do, already gone
+                    }
+                    catch (UnauthorizedAccessException ex)
+                    {
+                        _logger.LogWarning(ex, "Unable to delete {Image}", image.Path);
                     }
                 }
             }
@@ -381,12 +399,7 @@ namespace MediaBrowser.Providers.Manager
                 {
                     var currentImage = item.GetImageInfo(type, 0);
 
-                    if (currentImage == null)
-                    {
-                        item.SetImagePath(type, image.FileInfo);
-                        changed = true;
-                    }
-                    else if (!string.Equals(currentImage.Path, image.FileInfo.FullName, StringComparison.OrdinalIgnoreCase))
+                    if (currentImage == null || !string.Equals(currentImage.Path, image.FileInfo.FullName, StringComparison.OrdinalIgnoreCase))
                     {
                         item.SetImagePath(type, image.FileInfo);
                         changed = true;
@@ -494,7 +507,7 @@ namespace MediaBrowser.Providers.Manager
                     await _providerManager.SaveImage(
                         item,
                         stream,
-                        response.Content.Headers.ContentType.MediaType,
+                        response.Content.Headers.ContentType?.MediaType,
                         type,
                         null,
                         cancellationToken).ConfigureAwait(false);
@@ -617,11 +630,11 @@ namespace MediaBrowser.Providers.Manager
                     await _providerManager.SaveImage(
                         item,
                         stream,
-                        response.Content.Headers.ContentType.MediaType,
+                        response.Content.Headers.ContentType?.MediaType,
                         imageType,
                         null,
                         cancellationToken).ConfigureAwait(false);
-                    result.UpdateType = result.UpdateType | ItemUpdateType.ImageUpdate;
+                    result.UpdateType |= ItemUpdateType.ImageUpdate;
                 }
                 catch (HttpRequestException)
                 {

+ 322 - 25
MediaBrowser.Providers/Manager/MetadataService.cs

@@ -8,8 +8,10 @@ using System.Linq;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
+using Diacritics.Extensions;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Configuration;
@@ -74,14 +76,10 @@ namespace MediaBrowser.Providers.Manager
             var itemOfType = (TItemType)item;
 
             var updateType = ItemUpdateType.None;
-            var requiresRefresh = false;
 
             var libraryOptions = LibraryManager.GetLibraryOptions(item);
 
-            if (!requiresRefresh && libraryOptions.AutomaticRefreshIntervalDays > 0 && (DateTime.UtcNow - item.DateLastRefreshed).TotalDays >= libraryOptions.AutomaticRefreshIntervalDays)
-            {
-                requiresRefresh = true;
-            }
+            var requiresRefresh = libraryOptions.AutomaticRefreshIntervalDays > 0 && (DateTime.UtcNow - item.DateLastRefreshed).TotalDays >= libraryOptions.AutomaticRefreshIntervalDays;
 
             if (!requiresRefresh && refreshOptions.MetadataRefreshMode != MetadataRefreshMode.None)
             {
@@ -90,7 +88,7 @@ namespace MediaBrowser.Providers.Manager
 
                 if (requiresRefresh)
                 {
-                    Logger.LogDebug("Refreshing {0} {1} because item.RequiresRefresh() returned true", typeof(TItemType).Name, item.Path ?? item.Name);
+                    Logger.LogDebug("Refreshing {Type} {Item} because item.RequiresRefresh() returned true", typeof(TItemType).Name, item.Path ?? item.Name);
                 }
             }
 
@@ -98,6 +96,14 @@ namespace MediaBrowser.Providers.Manager
 
             var allImageProviders = ((ProviderManager)ProviderManager).GetImageProviders(item, refreshOptions).ToList();
 
+            if (refreshOptions.RemoveOldMetadata && refreshOptions.ReplaceAllImages)
+            {
+                if (ImageProvider.RemoveImages(item))
+                {
+                    updateType |= ItemUpdateType.ImageUpdate;
+                }
+            }
+
             // Start by validating images
             try
             {
@@ -110,7 +116,7 @@ namespace MediaBrowser.Providers.Manager
             catch (Exception ex)
             {
                 localImagesFailed = true;
-                Logger.LogError(ex, "Error validating images for {0}", item.Path ?? item.Name ?? "Unknown name");
+                Logger.LogError(ex, "Error validating images for {Item}", item.Path ?? item.Name ?? "Unknown name");
             }
 
             var metadataResult = new MetadataResult<TItemType>
@@ -380,8 +386,7 @@ namespace MediaBrowser.Providers.Manager
         {
             var updateType = ItemUpdateType.None;
 
-            var folder = item as Folder;
-            if (folder != null && folder.SupportsDateLastMediaAdded)
+            if (item is Folder folder && folder.SupportsDateLastMediaAdded)
             {
                 var dateLastMediaAdded = DateTime.MinValue;
                 var any = false;
@@ -668,7 +673,7 @@ namespace MediaBrowser.Providers.Manager
             foreach (var provider in providers.OfType<ILocalMetadataProvider<TItemType>>().ToList())
             {
                 var providerName = provider.GetType().Name;
-                Logger.LogDebug("Running {0} for {1}", providerName, logName);
+                Logger.LogDebug("Running {Provider} for {Item}", providerName, logName);
 
                 var itemInfo = new ItemInfo(item);
 
@@ -713,7 +718,7 @@ namespace MediaBrowser.Providers.Manager
                         break;
                     }
 
-                    Logger.LogDebug("{0} returned no metadata for {1}", providerName, logName);
+                    Logger.LogDebug("{Provider} returned no metadata for {Item}", providerName, logName);
                 }
                 catch (OperationCanceledException)
                 {
@@ -749,8 +754,11 @@ namespace MediaBrowser.Providers.Manager
                     }
                     else
                     {
-                        // TODO: If the new metadata from above has some blank data, this can cause old data to get filled into those empty fields
-                        MergeData(metadata, temp, Array.Empty<MetadataField>(), false, false);
+                        if (!options.RemoveOldMetadata)
+                        {
+                            MergeData(metadata, temp, Array.Empty<MetadataField>(), false, false);
+                        }
+
                         MergeData(temp, metadata, item.LockedFields, true, false);
                     }
                 }
@@ -780,7 +788,7 @@ namespace MediaBrowser.Providers.Manager
 
         private async Task RunCustomProvider(ICustomMetadataProvider<TItemType> provider, TItemType item, string logName, MetadataRefreshOptions options, RefreshResult refreshResult, CancellationToken cancellationToken)
         {
-            Logger.LogDebug("Running {0} for {1}", provider.GetType().Name, logName);
+            Logger.LogDebug("Running {Provider} for {Item}", provider.GetType().Name, logName);
 
             try
             {
@@ -811,7 +819,7 @@ namespace MediaBrowser.Providers.Manager
             foreach (var provider in providers)
             {
                 var providerName = provider.GetType().Name;
-                Logger.LogDebug("Running {0} for {1}", providerName, logName);
+                Logger.LogDebug("Running {Provider} for {Item}", providerName, logName);
 
                 if (id != null && !tmpDataMerged)
                 {
@@ -834,7 +842,7 @@ namespace MediaBrowser.Providers.Manager
                     }
                     else
                     {
-                        Logger.LogDebug("{0} returned no metadata for {1}", providerName, logName);
+                        Logger.LogDebug("{Provider} returned no metadata for {Item}", providerName, logName);
                     }
                 }
                 catch (OperationCanceledException)
@@ -867,13 +875,6 @@ namespace MediaBrowser.Providers.Manager
             }
         }
 
-        protected abstract void MergeData(
-            MetadataResult<TItemType> source,
-            MetadataResult<TItemType> target,
-            MetadataField[] lockedFields,
-            bool replaceData,
-            bool mergeMetadataSettings);
-
         private bool HasChanged(BaseItem item, IHasItemChangeMonitor changeMonitor, IDirectoryService directoryService)
         {
             try
@@ -882,16 +883,312 @@ namespace MediaBrowser.Providers.Manager
 
                 if (hasChanged)
                 {
-                    Logger.LogDebug("{0} reports change to {1}", changeMonitor.GetType().Name, item.Path ?? item.Name);
+                    Logger.LogDebug("{Monitor} reports change to {Item}", changeMonitor.GetType().Name, item.Path ?? item.Name);
                 }
 
                 return hasChanged;
             }
             catch (Exception ex)
             {
-                Logger.LogError(ex, "Error in {0}.HasChanged", changeMonitor.GetType().Name);
+                Logger.LogError(ex, "Error in {Monitor}.HasChanged", changeMonitor.GetType().Name);
                 return false;
             }
         }
+
+        /// <summary>
+        /// Merges metadata from source into target.
+        /// </summary>
+        /// <param name="source">The source for new metadata.</param>
+        /// <param name="target">The target to insert new metadata into.</param>
+        /// <param name="lockedFields">The fields that are locked and should not be updated.</param>
+        /// <param name="replaceData"><c>true</c> if existing data should be replaced.</param>
+        /// <param name="mergeMetadataSettings"><c>true</c> if the metadata settings in target should be updated to match source.</param>
+        /// <exception cref="ArgumentException">Thrown if source or target are null.</exception>
+        protected virtual void MergeData(
+            MetadataResult<TItemType> source,
+            MetadataResult<TItemType> target,
+            MetadataField[] lockedFields,
+            bool replaceData,
+            bool mergeMetadataSettings)
+        {
+            MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+        }
+
+        internal static void MergeBaseItemData(
+            MetadataResult<TItemType> sourceResult,
+            MetadataResult<TItemType> targetResult,
+            MetadataField[] lockedFields,
+            bool replaceData,
+            bool mergeMetadataSettings)
+        {
+            var source = sourceResult.Item;
+            var target = targetResult.Item;
+
+            if (source == null)
+            {
+                throw new ArgumentException("Item cannot be null.", nameof(sourceResult));
+            }
+
+            if (target == null)
+            {
+                throw new ArgumentException("Item cannot be null.", nameof(targetResult));
+            }
+
+            if (!lockedFields.Contains(MetadataField.Name))
+            {
+                if (replaceData || string.IsNullOrEmpty(target.Name))
+                {
+                    // Safeguard against incoming data having an empty name
+                    if (!string.IsNullOrWhiteSpace(source.Name))
+                    {
+                        target.Name = source.Name;
+                    }
+                }
+            }
+
+            if (replaceData || string.IsNullOrEmpty(target.OriginalTitle))
+            {
+                // Safeguard against incoming data having an empty name
+                if (!string.IsNullOrWhiteSpace(source.OriginalTitle))
+                {
+                    target.OriginalTitle = source.OriginalTitle;
+                }
+            }
+
+            if (replaceData || !target.CommunityRating.HasValue)
+            {
+                target.CommunityRating = source.CommunityRating;
+            }
+
+            if (replaceData || !target.EndDate.HasValue)
+            {
+                target.EndDate = source.EndDate;
+            }
+
+            if (!lockedFields.Contains(MetadataField.Genres))
+            {
+                if (replaceData || target.Genres.Length == 0)
+                {
+                    target.Genres = source.Genres;
+                }
+            }
+
+            if (replaceData || !target.IndexNumber.HasValue)
+            {
+                target.IndexNumber = source.IndexNumber;
+            }
+
+            if (!lockedFields.Contains(MetadataField.OfficialRating))
+            {
+                if (replaceData || string.IsNullOrEmpty(target.OfficialRating))
+                {
+                    target.OfficialRating = source.OfficialRating;
+                }
+            }
+
+            if (replaceData || string.IsNullOrEmpty(target.CustomRating))
+            {
+                target.CustomRating = source.CustomRating;
+            }
+
+            if (replaceData || string.IsNullOrEmpty(target.Tagline))
+            {
+                target.Tagline = source.Tagline;
+            }
+
+            if (!lockedFields.Contains(MetadataField.Overview))
+            {
+                if (replaceData || string.IsNullOrEmpty(target.Overview))
+                {
+                    target.Overview = source.Overview;
+                }
+            }
+
+            if (replaceData || !target.ParentIndexNumber.HasValue)
+            {
+                target.ParentIndexNumber = source.ParentIndexNumber;
+            }
+
+            if (!lockedFields.Contains(MetadataField.Cast))
+            {
+                if (replaceData || targetResult.People == null || targetResult.People.Count == 0)
+                {
+                    targetResult.People = sourceResult.People;
+                }
+                else if (targetResult.People != null && sourceResult.People != null)
+                {
+                    MergePeople(sourceResult.People, targetResult.People);
+                }
+            }
+
+            if (replaceData || !target.PremiereDate.HasValue)
+            {
+                target.PremiereDate = source.PremiereDate;
+            }
+
+            if (replaceData || !target.ProductionYear.HasValue)
+            {
+                target.ProductionYear = source.ProductionYear;
+            }
+
+            if (!lockedFields.Contains(MetadataField.Runtime))
+            {
+                if (replaceData || !target.RunTimeTicks.HasValue)
+                {
+                    if (target is not Audio && target is not Video)
+                    {
+                        target.RunTimeTicks = source.RunTimeTicks;
+                    }
+                }
+            }
+
+            if (!lockedFields.Contains(MetadataField.Studios))
+            {
+                if (replaceData || target.Studios.Length == 0)
+                {
+                    target.Studios = source.Studios;
+                }
+            }
+
+            if (!lockedFields.Contains(MetadataField.Tags))
+            {
+                if (replaceData || target.Tags.Length == 0)
+                {
+                    target.Tags = source.Tags;
+                }
+            }
+
+            if (!lockedFields.Contains(MetadataField.ProductionLocations))
+            {
+                if (replaceData || target.ProductionLocations.Length == 0)
+                {
+                    target.ProductionLocations = source.ProductionLocations;
+                }
+            }
+
+            foreach (var id in source.ProviderIds)
+            {
+                var key = id.Key;
+
+                // Don't replace existing Id's.
+                if (replaceData)
+                {
+                    target.ProviderIds[key] = id.Value;
+                }
+                else
+                {
+                    target.ProviderIds.TryAdd(key, id.Value);
+                }
+            }
+
+            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))
+                {
+                    target.ForcedSortName = forcedSortName;
+                }
+            }
+
+            if (mergeMetadataSettings)
+            {
+                target.LockedFields = source.LockedFields;
+                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;
+            }
+        }
+
+        private static void MergePeople(List<PersonInfo> source, List<PersonInfo> target)
+        {
+            foreach (var person in target)
+            {
+                var normalizedName = person.Name.RemoveDiacritics();
+                var personInSource = source.FirstOrDefault(i => string.Equals(i.Name.RemoveDiacritics(), normalizedName, StringComparison.OrdinalIgnoreCase));
+
+                if (personInSource != null)
+                {
+                    foreach (var providerId in personInSource.ProviderIds)
+                    {
+                        person.ProviderIds.TryAdd(providerId.Key, providerId.Value);
+                    }
+
+                    if (string.IsNullOrWhiteSpace(person.ImageUrl))
+                    {
+                        person.ImageUrl = personInSource.ImageUrl;
+                    }
+                }
+            }
+        }
+
+        private static void MergeDisplayOrder(BaseItem source, BaseItem target, bool replaceData)
+        {
+            if (source is IHasDisplayOrder sourceHasDisplayOrder
+                && target is IHasDisplayOrder targetHasDisplayOrder)
+            {
+                if (replaceData || string.IsNullOrEmpty(targetHasDisplayOrder.DisplayOrder))
+                {
+                    var displayOrder = sourceHasDisplayOrder.DisplayOrder;
+
+                    if (!string.IsNullOrWhiteSpace(displayOrder))
+                    {
+                        targetHasDisplayOrder.DisplayOrder = displayOrder;
+                    }
+                }
+            }
+        }
+
+        private static void MergeAlbumArtist(BaseItem source, BaseItem target, bool replaceData)
+        {
+            if (source is IHasAlbumArtist sourceHasAlbumArtist
+                && target is IHasAlbumArtist targetHasAlbumArtist)
+            {
+                if (replaceData || targetHasAlbumArtist.AlbumArtists.Count == 0)
+                {
+                    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;
+            }
+        }
+
+        private static void MergeVideoInfo(BaseItem source, BaseItem target, bool replaceData)
+        {
+            if (source is Video sourceCast && target is Video targetCast)
+            {
+                if (replaceData || targetCast.Video3DFormat == null)
+                {
+                    targetCast.Video3DFormat = sourceCast.Video3DFormat;
+                }
+            }
+        }
     }
 }

+ 0 - 295
MediaBrowser.Providers/Manager/ProviderUtils.cs

@@ -1,295 +0,0 @@
-#nullable disable
-
-#pragma warning disable CS1591
-
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using Diacritics.Extensions;
-using MediaBrowser.Controller.Entities;
-using MediaBrowser.Controller.Entities.Audio;
-using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
-
-namespace MediaBrowser.Providers.Manager
-{
-    public static class ProviderUtils
-    {
-        public static void MergeBaseItemData<T>(
-            MetadataResult<T> sourceResult,
-            MetadataResult<T> targetResult,
-            MetadataField[] lockedFields,
-            bool replaceData,
-            bool mergeMetadataSettings)
-            where T : BaseItem
-        {
-            var source = sourceResult.Item;
-            var target = targetResult.Item;
-
-            if (source == null)
-            {
-                throw new ArgumentException("Item cannot be null.", nameof(sourceResult));
-            }
-
-            if (target == null)
-            {
-                throw new ArgumentException("Item cannot be null.", nameof(targetResult));
-            }
-
-            if (!lockedFields.Contains(MetadataField.Name))
-            {
-                if (replaceData || string.IsNullOrEmpty(target.Name))
-                {
-                    // Safeguard against incoming data having an empty name
-                    if (!string.IsNullOrWhiteSpace(source.Name))
-                    {
-                        target.Name = source.Name;
-                    }
-                }
-            }
-
-            if (replaceData || string.IsNullOrEmpty(target.OriginalTitle))
-            {
-                // Safeguard against incoming data having an empty name
-                if (!string.IsNullOrWhiteSpace(source.OriginalTitle))
-                {
-                    target.OriginalTitle = source.OriginalTitle;
-                }
-            }
-
-            if (replaceData || !target.CommunityRating.HasValue)
-            {
-                target.CommunityRating = source.CommunityRating;
-            }
-
-            if (replaceData || !target.EndDate.HasValue)
-            {
-                target.EndDate = source.EndDate;
-            }
-
-            if (!lockedFields.Contains(MetadataField.Genres))
-            {
-                if (replaceData || target.Genres.Length == 0)
-                {
-                    target.Genres = source.Genres;
-                }
-            }
-
-            if (replaceData || !target.IndexNumber.HasValue)
-            {
-                target.IndexNumber = source.IndexNumber;
-            }
-
-            if (!lockedFields.Contains(MetadataField.OfficialRating))
-            {
-                if (replaceData || string.IsNullOrEmpty(target.OfficialRating))
-                {
-                    target.OfficialRating = source.OfficialRating;
-                }
-            }
-
-            if (replaceData || string.IsNullOrEmpty(target.CustomRating))
-            {
-                target.CustomRating = source.CustomRating;
-            }
-
-            if (replaceData || string.IsNullOrEmpty(target.Tagline))
-            {
-                target.Tagline = source.Tagline;
-            }
-
-            if (!lockedFields.Contains(MetadataField.Overview))
-            {
-                if (replaceData || string.IsNullOrEmpty(target.Overview))
-                {
-                    target.Overview = source.Overview;
-                }
-            }
-
-            if (replaceData || !target.ParentIndexNumber.HasValue)
-            {
-                target.ParentIndexNumber = source.ParentIndexNumber;
-            }
-
-            if (!lockedFields.Contains(MetadataField.Cast))
-            {
-                if (replaceData || targetResult.People == null || targetResult.People.Count == 0)
-                {
-                    targetResult.People = sourceResult.People;
-                }
-                else if (targetResult.People != null && sourceResult.People != null)
-                {
-                    MergePeople(sourceResult.People, targetResult.People);
-                }
-            }
-
-            if (replaceData || !target.PremiereDate.HasValue)
-            {
-                target.PremiereDate = source.PremiereDate;
-            }
-
-            if (replaceData || !target.ProductionYear.HasValue)
-            {
-                target.ProductionYear = source.ProductionYear;
-            }
-
-            if (!lockedFields.Contains(MetadataField.Runtime))
-            {
-                if (replaceData || !target.RunTimeTicks.HasValue)
-                {
-                    if (target is not Audio && target is not Video)
-                    {
-                        target.RunTimeTicks = source.RunTimeTicks;
-                    }
-                }
-            }
-
-            if (!lockedFields.Contains(MetadataField.Studios))
-            {
-                if (replaceData || target.Studios.Length == 0)
-                {
-                    target.Studios = source.Studios;
-                }
-            }
-
-            if (!lockedFields.Contains(MetadataField.Tags))
-            {
-                if (replaceData || target.Tags.Length == 0)
-                {
-                    target.Tags = source.Tags;
-                }
-            }
-
-            if (!lockedFields.Contains(MetadataField.ProductionLocations))
-            {
-                if (replaceData || target.ProductionLocations.Length == 0)
-                {
-                    target.ProductionLocations = source.ProductionLocations;
-                }
-            }
-
-            foreach (var id in source.ProviderIds)
-            {
-                var key = id.Key;
-
-                // Don't replace existing Id's.
-                if (replaceData || !target.ProviderIds.ContainsKey(key))
-                {
-                    target.ProviderIds[key] = id.Value;
-                }
-            }
-
-            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))
-                {
-                    target.ForcedSortName = forcedSortName;
-                }
-            }
-
-            if (mergeMetadataSettings)
-            {
-                target.LockedFields = source.LockedFields;
-                target.IsLocked = source.IsLocked;
-
-                // Grab the value if it's there, but if not then don't overwrite the default
-                if (source.DateCreated != default)
-                {
-                    target.DateCreated = source.DateCreated;
-                }
-
-                target.PreferredMetadataCountryCode = source.PreferredMetadataCountryCode;
-                target.PreferredMetadataLanguage = source.PreferredMetadataLanguage;
-            }
-        }
-
-        private static void MergePeople(List<PersonInfo> source, List<PersonInfo> target)
-        {
-            foreach (var person in target)
-            {
-                var normalizedName = person.Name.RemoveDiacritics();
-                var personInSource = source.FirstOrDefault(i => string.Equals(i.Name.RemoveDiacritics(), normalizedName, StringComparison.OrdinalIgnoreCase));
-
-                if (personInSource != null)
-                {
-                    foreach (var providerId in personInSource.ProviderIds)
-                    {
-                        if (!person.ProviderIds.ContainsKey(providerId.Key))
-                        {
-                            person.ProviderIds[providerId.Key] = providerId.Value;
-                        }
-                    }
-
-                    if (string.IsNullOrWhiteSpace(person.ImageUrl))
-                    {
-                        person.ImageUrl = personInSource.ImageUrl;
-                    }
-                }
-            }
-        }
-
-        private static void MergeDisplayOrder(BaseItem source, BaseItem target, bool replaceData)
-        {
-            if (source is IHasDisplayOrder sourceHasDisplayOrder
-                && target is IHasDisplayOrder targetHasDisplayOrder)
-            {
-                if (replaceData || string.IsNullOrEmpty(targetHasDisplayOrder.DisplayOrder))
-                {
-                    var displayOrder = sourceHasDisplayOrder.DisplayOrder;
-
-                    if (!string.IsNullOrWhiteSpace(displayOrder))
-                    {
-                        targetHasDisplayOrder.DisplayOrder = displayOrder;
-                    }
-                }
-            }
-        }
-
-        private static void MergeAlbumArtist(BaseItem source, BaseItem target, bool replaceData)
-        {
-            if (source is IHasAlbumArtist sourceHasAlbumArtist
-                && target is IHasAlbumArtist targetHasAlbumArtist)
-            {
-                if (replaceData || targetHasAlbumArtist.AlbumArtists.Count == 0)
-                {
-                    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;
-            }
-        }
-
-        private static void MergeVideoInfo(BaseItem source, BaseItem target, bool replaceData)
-        {
-            if (source is Video sourceCast && target is Video targetCast)
-            {
-                if (replaceData || targetCast.Video3DFormat == null)
-                {
-                    targetCast.Video3DFormat = sourceCast.Video3DFormat;
-                }
-            }
-        }
-    }
-}

+ 1 - 1
MediaBrowser.Providers/Movies/MovieMetadataService.cs

@@ -42,7 +42,7 @@ namespace MediaBrowser.Providers.Movies
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Movie> source, MetadataResult<Movie> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 1 - 1
MediaBrowser.Providers/Movies/TrailerMetadataService.cs

@@ -42,7 +42,7 @@ namespace MediaBrowser.Providers.Movies
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Trailer> source, MetadataResult<Trailer> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             if (replaceData || target.Item.TrailerTypes.Length == 0)
             {

+ 1 - 1
MediaBrowser.Providers/Music/AlbumMetadataService.cs

@@ -114,7 +114,7 @@ namespace MediaBrowser.Providers.Music
             bool replaceData,
             bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 7
MediaBrowser.Providers/Music/ArtistMetadataService.cs

@@ -6,7 +6,6 @@ using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Entities.Audio;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -39,11 +38,5 @@ namespace MediaBrowser.Providers.Music
                 })
                 : item.GetRecursiveChildren(i => i is IHasArtist && !i.IsFolder);
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<MusicArtist> source, MetadataResult<MusicArtist> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 1 - 1
MediaBrowser.Providers/Music/AudioMetadataService.cs

@@ -26,7 +26,7 @@ namespace MediaBrowser.Providers.Music
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Audio> source, MetadataResult<Audio> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 1 - 1
MediaBrowser.Providers/Music/MusicVideoMetadataService.cs

@@ -31,7 +31,7 @@ namespace MediaBrowser.Providers.Music
             bool replaceData,
             bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 7
MediaBrowser.Providers/MusicGenres/MusicGenreMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities.Audio;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.MusicGenres
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<MusicGenre> source, MetadataResult<MusicGenre> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/People/PersonMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.People
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Person> source, MetadataResult<Person> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Photos/PhotoAlbumMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Photos
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<PhotoAlbum> source, MetadataResult<PhotoAlbum> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Photos/PhotoMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Photos
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Photo> source, MetadataResult<Photo> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 1 - 1
MediaBrowser.Providers/Playlists/PlaylistMetadataService.cs

@@ -41,7 +41,7 @@ namespace MediaBrowser.Providers.Playlists
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Playlist> source, MetadataResult<Playlist> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 7
MediaBrowser.Providers/Studios/StudioMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Studios
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Studio> source, MetadataResult<Studio> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 1 - 1
MediaBrowser.Providers/TV/EpisodeMetadataService.cs

@@ -70,7 +70,7 @@ namespace MediaBrowser.Providers.TV
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Episode> source, MetadataResult<Episode> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 6
MediaBrowser.Providers/TV/SeasonMetadataService.cs

@@ -87,12 +87,6 @@ namespace MediaBrowser.Providers.TV
             return updateType;
         }
 
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Season> source, MetadataResult<Season> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
-
         private ItemUpdateType SaveIsVirtualItem(Season item, IList<BaseItem> episodes)
         {
             var isVirtualItem = item.LocationType == LocationType.Virtual && (episodes.Count == 0 || episodes.All(i => i.LocationType == LocationType.Virtual));

+ 1 - 1
MediaBrowser.Providers/TV/SeriesMetadataService.cs

@@ -63,7 +63,7 @@ namespace MediaBrowser.Providers.TV
         /// <inheritdoc />
         protected override void MergeData(MetadataResult<Series> source, MetadataResult<Series> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
         {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
+            base.MergeData(source, target, lockedFields, replaceData, mergeMetadataSettings);
 
             var sourceItem = source.Item;
             var targetItem = target.Item;

+ 0 - 7
MediaBrowser.Providers/Videos/VideoMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -26,11 +25,5 @@ namespace MediaBrowser.Providers.Videos
         /// <inheritdoc />
         // Make sure the type-specific services get picked first
         public override int Order => 10;
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Video> source, MetadataResult<Video> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 0 - 7
MediaBrowser.Providers/Years/YearMetadataService.cs

@@ -4,7 +4,6 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Providers.Manager;
 using Microsoft.Extensions.Logging;
@@ -22,11 +21,5 @@ namespace MediaBrowser.Providers.Years
             : base(serverConfigurationManager, logger, providerManager, fileSystem, libraryManager)
         {
         }
-
-        /// <inheritdoc />
-        protected override void MergeData(MetadataResult<Year> source, MetadataResult<Year> target, MetadataField[] lockedFields, bool replaceData, bool mergeMetadataSettings)
-        {
-            ProviderUtils.MergeBaseItemData(source, target, lockedFields, replaceData, mergeMetadataSettings);
-        }
     }
 }

+ 119 - 70
tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs

@@ -41,10 +41,7 @@ namespace Jellyfin.Providers.Tests.Manager
         [Fact]
         public void ValidateImages_EmptyItemEmptyProviders_NoChange()
         {
-            var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(new Video(), Enumerable.Empty<ILocalImageProvider>(), null);
-
-            Assert.False(changed);
+            ValidateImages_Test(ImageType.Primary, 0, true, 0, false, 0);
         }
 
         private static TheoryData<ImageType, int> GetImageTypesWithCount()
@@ -53,7 +50,6 @@ namespace Jellyfin.Providers.Tests.Manager
             {
                 // minimal test cases that hit different handling
                 { ImageType.Primary, 1 },
-                { ImageType.Backdrop, 1 },
                 { ImageType.Backdrop, 2 }
             };
 
@@ -64,43 +60,34 @@ namespace Jellyfin.Providers.Tests.Manager
         [MemberData(nameof(GetImageTypesWithCount))]
         public void ValidateImages_EmptyItemAndPopulatedProviders_AddsImages(ImageType imageType, int imageCount)
         {
-            // Has to exist for querying DateModified time on file, results stored but not checked so not populating
-            BaseItem.FileSystem = Mock.Of<IFileSystem>();
-
-            var item = new Video();
-            var imageProvider = GetImageProvider(imageType, imageCount, true);
-
-            var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, new[] { imageProvider }, null);
-
-            Assert.True(changed);
-            Assert.Equal(imageCount, item.GetImages(imageType).Count());
+            ValidateImages_Test(imageType, 0, true, imageCount, true, imageCount);
         }
 
         [Theory]
         [MemberData(nameof(GetImageTypesWithCount))]
         public void ValidateImages_PopulatedItemWithGoodPathsAndEmptyProviders_NoChange(ImageType imageType, int imageCount)
         {
-            var item = GetItemWithImages(imageType, imageCount, true);
-
-            var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, Enumerable.Empty<ILocalImageProvider>(), null);
-
-            Assert.False(changed);
-            Assert.Equal(imageCount, item.GetImages(imageType).Count());
+            ValidateImages_Test(imageType, imageCount, true, 0, false, imageCount);
         }
 
         [Theory]
         [MemberData(nameof(GetImageTypesWithCount))]
         public void ValidateImages_PopulatedItemWithBadPathsAndEmptyProviders_RemovesImage(ImageType imageType, int imageCount)
         {
-            var item = GetItemWithImages(imageType, imageCount, false);
+            ValidateImages_Test(imageType, imageCount, false, 0, true, 0);
+        }
+
+        private void ValidateImages_Test(ImageType imageType, int initialImageCount, bool initialPathsValid, int providerImageCount, bool expectedChange, int expectedImageCount)
+        {
+            var item = GetItemWithImages(imageType, initialImageCount, initialPathsValid);
+
+            var imageProvider = GetImageProvider(imageType, providerImageCount, true);
 
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, Enumerable.Empty<ILocalImageProvider>(), null);
+            var actualChange = itemImageProvider.ValidateImages(item, new[] { imageProvider }, null);
 
-            Assert.True(changed);
-            Assert.Empty(item.GetImages(imageType));
+            Assert.Equal(expectedChange, actualChange);
+            Assert.Equal(expectedImageCount, item.GetImages(imageType).Count());
         }
 
         [Fact]
@@ -137,20 +124,23 @@ namespace Jellyfin.Providers.Tests.Manager
         }
 
         [Theory]
-        [MemberData(nameof(GetImageTypesWithCount))]
-        public void MergeImages_PopulatedItemWithGoodPathsAndSameNewImages_NoChange(ImageType imageType, int imageCount)
+        [InlineData(ImageType.Primary, 1, false)]
+        [InlineData(ImageType.Backdrop, 2, false)]
+        [InlineData(ImageType.Primary, 1, true)]
+        [InlineData(ImageType.Backdrop, 2, true)]
+        public void MergeImages_PopulatedItemWithGoodPathsAndSameNewImages_ResetIfTimeChanges(ImageType imageType, int imageCount, bool updateTime)
         {
             var oldTime = new DateTime(1970, 1, 1);
+            var updatedTime = updateTime ? new DateTime(2021, 1, 1) : oldTime;
 
-            // match update time with time added to item images (unix epoch)
             var fileSystem = new Mock<IFileSystem>();
             fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
-                .Returns(oldTime);
+                .Returns(updatedTime);
             BaseItem.FileSystem = fileSystem.Object;
 
             // all valid paths - matching for strictly updating
             var item = GetItemWithImages(imageType, imageCount, true);
-            // set size to non-zero to allow for updates to occur
+            // set size to non-zero to allow for image size reset to occur
             foreach (var image in item.GetImages(imageType))
             {
                 image.DateModified = oldTime;
@@ -163,45 +153,52 @@ namespace Jellyfin.Providers.Tests.Manager
             var itemImageProvider = GetItemImageProvider(null, fileSystem);
             var changed = itemImageProvider.MergeImages(item, images);
 
-            Assert.False(changed);
+            if (updateTime)
+            {
+                Assert.True(changed);
+                // before and after paths are the same, verify updated by size reset to 0
+                var typedImages = item.GetImages(imageType).ToArray();
+                Assert.Equal(imageCount, typedImages.Length);
+                foreach (var image in typedImages)
+                {
+                    Assert.Equal(updatedTime, image.DateModified);
+                    Assert.Equal(0, image.Height);
+                    Assert.Equal(0, image.Width);
+                }
+            }
+            else
+            {
+                Assert.False(changed);
+            }
         }
 
         [Theory]
-        [MemberData(nameof(GetImageTypesWithCount))]
-        public void MergeImages_PopulatedItemWithGoodPathsAndSameNewImagesWithNewTimestamps_ResetsImageSizes(ImageType imageType, int imageCount)
+        [InlineData(ImageType.Primary, 0)]
+        [InlineData(ImageType.Primary, 1)]
+        [InlineData(ImageType.Backdrop, 2)]
+        public void RemoveImages_DeletesImages_WhenFound(ImageType imageType, int imageCount)
         {
-            var oldTime = new DateTime(1970, 1, 1);
-            var updatedTime = new DateTime(2021, 1, 1);
-
-            var fileSystem = new Mock<IFileSystem>();
-            fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
-                .Returns(updatedTime);
-            BaseItem.FileSystem = fileSystem.Object;
+            var item = GetItemWithImages(imageType, imageCount, false);
 
-            // all valid paths - matching for strictly updating
-            var item = GetItemWithImages(imageType, imageCount, true);
-            // set size to non-zero to allow for image size reset to occur
-            foreach (var image in item.GetImages(imageType))
+            var mockFileSystem = new Mock<IFileSystem>(MockBehavior.Strict);
+            if (imageCount > 0)
             {
-                image.DateModified = oldTime;
-                image.Height = 1;
-                image.Width = 1;
+                mockFileSystem.Setup(fs => fs.DeleteFile("invalid path 0"))
+                    .Verifiable();
             }
 
-            var images = GetImages(imageType, imageCount, true);
-
-            var itemImageProvider = GetItemImageProvider(null, fileSystem);
-            var changed = itemImageProvider.MergeImages(item, images);
-
-            Assert.True(changed);
-            // before and after paths are the same, verify updated by size reset to 0
-            Assert.Equal(imageCount, item.GetImages(imageType).Count());
-            foreach (var image in item.GetImages(imageType))
+            if (imageCount > 1)
             {
-                Assert.Equal(updatedTime, image.DateModified);
-                Assert.Equal(0, image.Height);
-                Assert.Equal(0, image.Width);
+                mockFileSystem.Setup(fs => fs.DeleteFile("invalid path 1"))
+                    .Verifiable();
             }
+
+            var itemImageProvider = GetItemImageProvider(Mock.Of<IProviderManager>(), mockFileSystem);
+            var result = itemImageProvider.RemoveImages(item);
+
+            Assert.Equal(imageCount != 0, result);
+            Assert.Empty(item.GetImages(imageType));
+            mockFileSystem.Verify();
         }
 
         [Theory]
@@ -336,8 +333,7 @@ namespace Jellyfin.Providers.Tests.Manager
                 remoteInfo[i] = new RemoteImageInfo
                 {
                     Type = imageType,
-                    Url = "image url " + i,
-                    Width = 1 // min width is set to 0, this will always pass
+                    Url = "image url " + i
                 };
             }
 
@@ -403,11 +399,10 @@ namespace Jellyfin.Providers.Tests.Manager
             var remoteInfo = new RemoteImageInfo[targetImageCount];
             for (int i = 0; i < targetImageCount; i++)
             {
-                remoteInfo[i] = new RemoteImageInfo()
+                remoteInfo[i] = new RemoteImageInfo
                 {
                     Type = imageType,
-                    Url = "image url " + i,
-                    Width = 1 // min width is set to 0, this will always pass
+                    Url = "image url " + i
                 };
             }
 
@@ -449,11 +444,10 @@ namespace Jellyfin.Providers.Tests.Manager
             var remoteInfo = new RemoteImageInfo[remoteInfoCount];
             for (int i = 0; i < remoteInfoCount; i++)
             {
-                remoteInfo[i] = new RemoteImageInfo()
+                remoteInfo[i] = new RemoteImageInfo
                 {
                     Type = imageType,
-                    Url = "image url " + i,
-                    Width = 1 // min width is set to 0, this will always pass
+                    Url = "image url " + i
                 };
             }
 
@@ -500,6 +494,62 @@ namespace Jellyfin.Providers.Tests.Manager
             Assert.Equal(imageCount, item.GetImages(imageType).Count());
         }
 
+        [Theory]
+        [InlineData(9, false)]
+        [InlineData(10, true)]
+        [InlineData(null, true)]
+        public async void RefreshImages_ProviderRemote_FiltersByWidth(int? remoteImageWidth, bool expectedToUpdate)
+        {
+            var imageType = ImageType.Primary;
+
+            var item = new Video();
+
+            var libraryOptions = new LibraryOptions
+            {
+                TypeOptions = new[]
+                {
+                    new TypeOptions
+                    {
+                        Type = item.GetType().Name,
+                        ImageOptions = new[]
+                        {
+                            new ImageOption
+                            {
+                                Type = imageType,
+                                MinWidth = 10
+                            }
+                        }
+                    }
+                }
+            };
+
+            var remoteProvider = new Mock<IRemoteImageProvider>(MockBehavior.Strict);
+            remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider");
+            remoteProvider.Setup(rp => rp.GetSupportedImages(item))
+                .Returns(new[] { imageType });
+
+            var refreshOptions = new ImageRefreshOptions(Mock.Of<IDirectoryService>());
+
+            // set width on image from remote
+            var remoteInfo = new[]
+            {
+                new RemoteImageInfo()
+                {
+                    Type = imageType,
+                    Url = "image url",
+                    Width = remoteImageWidth
+                }
+            };
+
+            var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
+            providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
+                .ReturnsAsync(remoteInfo);
+            var itemImageProvider = GetItemImageProvider(providerManager.Object, null);
+            var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
+
+            Assert.Equal(expectedToUpdate, result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
+        }
+
         private static ItemImageProvider GetItemImageProvider(IProviderManager? providerManager, Mock<IFileSystem>? mockFileSystem)
         {
             // strict to ensure this isn't accidentally used where a prepared mock is intended
@@ -586,7 +636,6 @@ namespace Jellyfin.Providers.Tests.Manager
                             {
                                 Type = type,
                                 Limit = count,
-                                MinWidth = 0
                             }
                         }
                     }

+ 378 - 0
tests/Jellyfin.Providers.Tests/Manager/MetadataServiceTests.cs

@@ -0,0 +1,378 @@
+using System;
+using System.Collections.Generic;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Entities.TV;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Providers.Manager;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.Manager
+{
+    public class MetadataServiceTests
+    {
+        [Theory]
+        [InlineData(false, false)]
+        [InlineData(true, false)]
+        [InlineData(true, true)]
+        public void MergeBaseItemData_MergeMetadataSettings_MergesWhenSet(bool mergeMetadataSettings, bool defaultDate)
+        {
+            var newLocked = new[] { MetadataField.Cast };
+            var newString = "new";
+            var newDate = DateTime.Now;
+
+            var oldLocked = new[] { MetadataField.Genres };
+            var oldString = "old";
+            var oldDate = DateTime.UnixEpoch;
+
+            var source = new MetadataResult<Movie>
+            {
+                Item = new Movie
+                {
+                    LockedFields = newLocked,
+                    IsLocked = true,
+                    PreferredMetadataCountryCode = newString,
+                    PreferredMetadataLanguage = newString,
+                    DateCreated = newDate
+                }
+            };
+            if (defaultDate)
+            {
+                source.Item.DateCreated = default;
+            }
+
+            var target = new MetadataResult<Movie>
+            {
+                Item = new Movie
+                {
+                    LockedFields = oldLocked,
+                    IsLocked = false,
+                    PreferredMetadataCountryCode = oldString,
+                    PreferredMetadataLanguage = oldString,
+                    DateCreated = oldDate
+                }
+            };
+
+            MetadataService<Movie, MovieInfo>.MergeBaseItemData(source, target, Array.Empty<MetadataField>(), true, mergeMetadataSettings);
+
+            if (mergeMetadataSettings)
+            {
+                Assert.Equal(newLocked, target.Item.LockedFields);
+                Assert.True(target.Item.IsLocked);
+                Assert.Equal(newString, target.Item.PreferredMetadataCountryCode);
+                Assert.Equal(newString, target.Item.PreferredMetadataLanguage);
+                Assert.Equal(defaultDate ? oldDate : newDate, target.Item.DateCreated);
+            }
+            else
+            {
+                Assert.Equal(oldLocked, target.Item.LockedFields);
+                Assert.False(target.Item.IsLocked);
+                Assert.Equal(oldString, target.Item.PreferredMetadataCountryCode);
+                Assert.Equal(oldString, target.Item.PreferredMetadataLanguage);
+                Assert.Equal(oldDate, target.Item.DateCreated);
+            }
+        }
+
+        [Theory]
+        [InlineData("Name", MetadataField.Name, false)]
+        [InlineData("OriginalTitle", null, false)]
+        [InlineData("OfficialRating", MetadataField.OfficialRating)]
+        [InlineData("CustomRating")]
+        [InlineData("Tagline")]
+        [InlineData("Overview", MetadataField.Overview)]
+        [InlineData("DisplayOrder", null, false)]
+        [InlineData("ForcedSortName", null, false)]
+        public void MergeBaseItemData_StringField_ReplacesAppropriately(string propName, MetadataField? lockField = null, bool replacesWithEmpty = true)
+        {
+            var oldValue = "Old";
+            var newValue = "New";
+
+            // Use type Series to hit DisplayOrder
+            Assert.False(TestMergeBaseItemData<Series, SeriesInfo>(propName, oldValue, newValue, null, false, out _));
+            if (lockField != null)
+            {
+                Assert.False(TestMergeBaseItemData<Series, SeriesInfo>(propName, oldValue, newValue, lockField, true, out _));
+                Assert.False(TestMergeBaseItemData<Series, SeriesInfo>(propName, null, newValue, lockField, false, out _));
+                Assert.False(TestMergeBaseItemData<Series, SeriesInfo>(propName, string.Empty, newValue, lockField, false, out _));
+            }
+
+            Assert.True(TestMergeBaseItemData<Series, SeriesInfo>(propName, oldValue, newValue, null, true, out _));
+            Assert.True(TestMergeBaseItemData<Series, SeriesInfo>(propName, null, newValue, null, false, out _));
+            Assert.True(TestMergeBaseItemData<Series, SeriesInfo>(propName, string.Empty, newValue, null, false, out _));
+
+            var replacedWithEmpty = TestMergeBaseItemData<Series, SeriesInfo>(propName, oldValue, string.Empty, null, true, out _);
+            Assert.Equal(replacesWithEmpty, replacedWithEmpty);
+        }
+
+        [Theory]
+        [InlineData("Genres", MetadataField.Genres)]
+        [InlineData("Studios", MetadataField.Studios)]
+        [InlineData("Tags", MetadataField.Tags)]
+        [InlineData("ProductionLocations", MetadataField.ProductionLocations)]
+        [InlineData("AlbumArtists")]
+        public void MergeBaseItemData_StringArrayField_ReplacesAppropriately(string propName, MetadataField? lockField = null)
+        {
+            // Note that arrays are replaced, not merged
+            var oldValue = new[] { "Old" };
+            var newValue = new[] { "New" };
+
+            // Use type Audio to hit AlbumArtists
+            Assert.False(TestMergeBaseItemData<Audio, SongInfo>(propName, oldValue, newValue, null, false, out _));
+            if (lockField != null)
+            {
+                Assert.False(TestMergeBaseItemData<Audio, SongInfo>(propName, oldValue, newValue, lockField, true, out _));
+                Assert.False(TestMergeBaseItemData<Audio, SongInfo>(propName, Array.Empty<string>(), newValue, lockField, false, out _));
+            }
+
+            Assert.True(TestMergeBaseItemData<Audio, SongInfo>(propName, oldValue, newValue, null, true, out _));
+            Assert.True(TestMergeBaseItemData<Audio, SongInfo>(propName, Array.Empty<string>(), newValue, null, false, out _));
+
+            Assert.True(TestMergeBaseItemData<Audio, SongInfo>(propName, oldValue, Array.Empty<string>(), null, true, out _));
+        }
+
+        private static TheoryData<string, object, object> MergeBaseItemData_SimpleField_ReplacesAppropriately_TestData()
+            => new()
+            {
+                { "IndexNumber", 1, 2 },
+                { "ParentIndexNumber", 1, 2 },
+                { "ProductionYear", 1, 2 },
+                { "CommunityRating", 1.0f, 2.0f },
+                { "CriticRating", 1.0f, 2.0f },
+                { "EndDate", DateTime.UnixEpoch, DateTime.Now },
+                { "PremiereDate", DateTime.UnixEpoch, DateTime.Now },
+                { "Video3DFormat", Video3DFormat.HalfSideBySide, Video3DFormat.FullSideBySide }
+            };
+
+        [Theory]
+        [MemberData(nameof(MergeBaseItemData_SimpleField_ReplacesAppropriately_TestData))]
+        public void MergeBaseItemData_SimpleField_ReplacesAppropriately(string propName, object oldValue, object newValue)
+        {
+            // Use type Movie to allow testing of Video3DFormat
+            Assert.False(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, newValue, null, false, out _));
+
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, newValue, null, true, out _));
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, null, newValue, null, false, out _));
+
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, null, null, true, out _));
+        }
+
+        [Fact]
+        public void MergeBaseItemData_MergeTrailers_ReplacesAppropriately()
+        {
+            string propName = "RemoteTrailers";
+            var oldValue = new[]
+            {
+                new MediaUrl
+                {
+                    Name = "Name 1",
+                    Url = "URL 1"
+                }
+            };
+            var newValue = new[]
+            {
+                new MediaUrl
+                {
+                    Name = "Name 2",
+                    Url = "URL 2"
+                }
+            };
+
+            Assert.False(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, newValue, null, false, out _));
+
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, newValue, null, true, out _));
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, Array.Empty<MediaUrl>(), newValue, null, false, out _));
+
+            Assert.True(TestMergeBaseItemData<Movie, MovieInfo>(propName, oldValue, Array.Empty<MediaUrl>(), null, true, out _));
+        }
+
+        [Fact]
+        public void MergeBaseItemData_ProviderIds_MergesAppropriately()
+        {
+            var propName = "ProviderIds";
+            var oldValue = new Dictionary<string, string>
+            {
+                { "provider 1", "id 1" }
+            };
+
+            // overwrite provider id
+            var overwriteNewValue = new Dictionary<string, string>
+            {
+                { "provider 1", "id 2" }
+            };
+            Assert.False(TestMergeBaseItemData<Movie, MovieInfo>(propName, new Dictionary<string, string>(oldValue), overwriteNewValue, null, false, out _));
+            TestMergeBaseItemData<Movie, MovieInfo>(propName, new Dictionary<string, string>(oldValue), overwriteNewValue, null, true, out var overwritten);
+            Assert.Equal(overwriteNewValue, overwritten);
+
+            // merge without overwriting
+            var mergeNewValue = new Dictionary<string, string>
+            {
+                { "provider 1", "id 2" },
+                { "provider 2", "id 3" }
+            };
+            TestMergeBaseItemData<Movie, MovieInfo>(propName, new Dictionary<string, string>(oldValue), mergeNewValue, null, false, out var merged);
+            var actual = (Dictionary<string, string>)merged!;
+            Assert.Equal("id 1", actual["provider 1"]);
+            Assert.Equal("id 3", actual["provider 2"]);
+
+            // empty source results in no change
+            TestMergeBaseItemData<Movie, MovieInfo>(propName, new Dictionary<string, string>(oldValue), new Dictionary<string, string>(), null, true, out var notOverwritten);
+            Assert.Equal(oldValue, notOverwritten);
+        }
+
+        [Fact]
+        public void MergeBaseItemData_MergePeople_MergesAppropriately()
+        {
+            // PersonInfo in list is changed by merge, create new for every call
+            List<PersonInfo> GetOldValue()
+                => new()
+                {
+                    new PersonInfo
+                    {
+                        Name = "Name 1",
+                        ProviderIds = new Dictionary<string, string>
+                        {
+                            { "Provider 1", "1234" }
+                        }
+                    }
+                };
+
+            object? result;
+            List<PersonInfo> actual;
+
+            // overwrite provider id
+            var overwriteNewValue = new List<PersonInfo>
+            {
+                new()
+                {
+                    Name = "Name 2"
+                }
+            };
+            Assert.False(TestMergeBaseItemDataPerson(GetOldValue(), overwriteNewValue, null, false, out result));
+            // People not already in target are not merged into it from source
+            actual = (List<PersonInfo>)result!;
+            Assert.Single(actual);
+            Assert.Equal("Name 1", actual[0].Name);
+
+            Assert.True(TestMergeBaseItemDataPerson(GetOldValue(), overwriteNewValue, null, true, out _));
+            Assert.True(TestMergeBaseItemDataPerson(new List<PersonInfo>(), overwriteNewValue, null, false, out _));
+            Assert.True(TestMergeBaseItemDataPerson(null, overwriteNewValue, null, false, out _));
+
+            Assert.False(TestMergeBaseItemDataPerson(GetOldValue(), overwriteNewValue, MetadataField.Cast, true, out _));
+
+            // providers merge but don't overwrite existing keys
+            var mergeNewValue = new List<PersonInfo>
+            {
+                new()
+                {
+                    Name = "Name 1",
+                    ProviderIds = new Dictionary<string, string>
+                    {
+                        { "Provider 1", "5678" },
+                        { "Provider 2", "5678" }
+                    }
+                }
+            };
+            TestMergeBaseItemDataPerson(GetOldValue(), mergeNewValue, null, false, out result);
+            actual = (List<PersonInfo>)result!;
+            Assert.Single(actual);
+            Assert.Equal("Name 1", actual[0].Name);
+            Assert.Equal(2, actual[0].ProviderIds.Count);
+            Assert.Equal("1234", actual[0].ProviderIds["Provider 1"]);
+            Assert.Equal("5678", actual[0].ProviderIds["Provider 2"]);
+
+            // picture adds if missing but won't overwrite (forcing overwrites entire list, not entries in merged PersonInfo)
+            var mergePicture1 = new List<PersonInfo>
+            {
+                new()
+                {
+                    Name = "Name 1",
+                    ImageUrl = "URL 1"
+                }
+            };
+            TestMergeBaseItemDataPerson(GetOldValue(), mergePicture1, null, false, out result);
+            actual = (List<PersonInfo>)result!;
+            Assert.Single(actual);
+            Assert.Equal("Name 1", actual[0].Name);
+            Assert.Equal("URL 1", actual[0].ImageUrl);
+            var mergePicture2 = new List<PersonInfo>
+            {
+                new()
+                {
+                    Name = "Name 1",
+                    ImageUrl = "URL 2"
+                }
+            };
+            TestMergeBaseItemDataPerson(mergePicture1, mergePicture2, null, false, out result);
+            actual = (List<PersonInfo>)result!;
+            Assert.Single(actual);
+            Assert.Equal("Name 1", actual[0].Name);
+            Assert.Equal("URL 1", actual[0].ImageUrl);
+
+            // empty source can be forced to overwrite a target with data
+            Assert.True(TestMergeBaseItemDataPerson(GetOldValue(), new List<PersonInfo>(), null, true, out _));
+        }
+
+        private static bool TestMergeBaseItemDataPerson(List<PersonInfo>? oldValue, List<PersonInfo>? newValue, MetadataField? lockField, bool replaceData, out object? actualValue)
+        {
+            var source = new MetadataResult<Movie>
+            {
+                Item = new Movie(),
+                People = newValue
+            };
+
+            var target = new MetadataResult<Movie>
+            {
+                Item = new Movie(),
+                People = oldValue
+            };
+
+            var lockedFields = lockField == null ? Array.Empty<MetadataField>() : new[] { (MetadataField)lockField };
+            MetadataService<Movie, MovieInfo>.MergeBaseItemData(source, target, lockedFields, replaceData, false);
+
+            actualValue = target.People;
+            return newValue?.Equals(actualValue) ?? actualValue == null;
+        }
+
+        /// <summary>
+        /// Makes a call to <see cref="MetadataService{TItemType,TIdType}.MergeBaseItemData"/> with the provided parameters and returns whether the target changed or not.
+        ///
+        /// Reflection is used to allow testing of all fields using the same logic, rather than relying on copy/pasting test code for each field.
+        /// </summary>
+        /// <param name="propName">The property to test.</param>
+        /// <param name="oldValue">The initial value in the target object.</param>
+        /// <param name="newValue">The initial value in the source object.</param>
+        /// <param name="lockField">The metadata field that locks this property if the field should be locked, or <c>null</c> to leave unlocked.</param>
+        /// <param name="replaceData">Passed through to <see cref="MetadataService{TItemType,TIdType}.MergeBaseItemData"/>.</param>
+        /// <param name="actualValue">The resulting value set to the target.</param>
+        /// <typeparam name="TItemType">The <see cref="BaseItem"/> type to test on.</typeparam>
+        /// <typeparam name="TIdType">The <see cref="BaseItem"/> info type.</typeparam>
+        /// <returns><c>true</c> if the property on the target updates to match the source value when<see cref="MetadataService{TItemType,TIdType}.MergeBaseItemData"/> is called.</returns>
+        private static bool TestMergeBaseItemData<TItemType, TIdType>(string propName, object? oldValue, object? newValue, MetadataField? lockField, bool replaceData, out object? actualValue)
+            where TItemType : BaseItem, IHasLookupInfo<TIdType>, new()
+            where TIdType : ItemLookupInfo, new()
+        {
+            var property = typeof(TItemType).GetProperty(propName)!;
+
+            var source = new MetadataResult<TItemType>
+            {
+                Item = new TItemType()
+            };
+            property.SetValue(source.Item, newValue);
+
+            var target = new MetadataResult<TItemType>
+            {
+                Item = new TItemType()
+            };
+            property.SetValue(target.Item, oldValue);
+
+            var lockedFields = lockField == null ? Array.Empty<MetadataField>() : new[] { (MetadataField)lockField };
+            // generic type doesn't actually matter to call the static method, just has to be filled in
+            MetadataService<TItemType, TIdType>.MergeBaseItemData(source, target, lockedFields, replaceData, false);
+
+            actualValue = property.GetValue(target.Item);
+            return newValue?.Equals(actualValue) ?? actualValue == null;
+        }
+    }
+}