Minor improvements

* Removed some allocations
* Removed some useless abstractions

MediaBrowser.Controller/Entities/BaseItem.cs

@@ -84,8 +84,6 @@ namespace MediaBrowser.Controller.Entities
             Model.Entities.ExtraType.Scene
         };
 
-        public static readonly char[] SlugReplaceChars = { '?', '/', '&' };
-
         /// <summary>
         /// The supported extra folder names and types. See <see cref="Emby.Naming.Common.NamingOptions" />.
         /// </summary>
@@ -354,11 +352,6 @@ namespace MediaBrowser.Controller.Entities
         {
             get
             {
-                // if (IsOffline)
-                // {
-                //    return LocationType.Offline;
-                // }
-
                 var path = Path;
                 if (string.IsNullOrEmpty(path))
                 {
@@ -391,7 +384,7 @@ namespace MediaBrowser.Controller.Entities
         }
 
         [JsonIgnore]
-        public bool IsFileProtocol => IsPathProtocol(MediaProtocol.File);
+        public bool IsFileProtocol => PathProtocol == MediaProtocol.File;
 
         [JsonIgnore]
         public bool HasPathProtocol => PathProtocol.HasValue;
@@ -583,14 +576,7 @@ namespace MediaBrowser.Controller.Entities
         }
 
         [JsonIgnore]
-        public virtual Guid DisplayParentId
-        {
-            get
-            {
-                var parentId = ParentId;
-                return parentId;
-            }
-        }
+        public virtual Guid DisplayParentId => ParentId;
 
         [JsonIgnore]
         public BaseItem DisplayParent
@@ -853,13 +839,6 @@ namespace MediaBrowser.Controller.Entities
             return Id.ToString("N", CultureInfo.InvariantCulture);
         }
 
-        public bool IsPathProtocol(MediaProtocol protocol)
-        {
-            var current = PathProtocol;
-
-            return current.HasValue && current.Value == protocol;
-        }
-
         private List<Tuple<StringBuilder, bool>> GetSortChunks(string s1)
         {
             var list = new List<Tuple<StringBuilder, bool>>();
@@ -987,7 +966,7 @@ namespace MediaBrowser.Controller.Entities
 
             ReadOnlySpan<char> idString = Id.ToString("N", CultureInfo.InvariantCulture);
 
-            return System.IO.Path.Join(basePath, "library", idString.Slice(0, 2), idString);
+            return System.IO.Path.Join(basePath, "library", idString[..2], idString);
         }
 
         /// <summary>
@@ -1302,8 +1281,7 @@ namespace MediaBrowser.Controller.Entities
                 terms.Add(item.Name);
             }
 
-            var video = item as Video;
-            if (video != null)
+            if (item is Video video)
             {
                 if (video.Video3DFormat.HasValue)
                 {
@@ -1338,7 +1316,7 @@ namespace MediaBrowser.Controller.Entities
                 }
             }
 
-            return string.Join('/', terms.ToArray());
+            return string.Join('/', terms);
         }
 
         /// <summary>
@@ -1361,9 +1339,7 @@ namespace MediaBrowser.Controller.Entities
                 .Select(audio =>
                 {
                     // Try to retrieve it from the db. If we don't find it, use the resolved version
-                    var dbItem = LibraryManager.GetItemById(audio.Id) as Audio.Audio;
-
-                    if (dbItem != null)
+                    if (LibraryManager.GetItemById(audio.Id) is Audio.Audio dbItem)
                     {
                         audio = dbItem;
                     }
@@ -1570,8 +1546,7 @@ namespace MediaBrowser.Controller.Entities
                     }
                 }
 
-                var hasTrailers = this as IHasTrailers;
-                if (hasTrailers != null)
+                if (this is IHasTrailers hasTrailers)
                 {
                     localTrailersChanged = await RefreshLocalTrailers(hasTrailers, options, fileSystemChildren, cancellationToken).ConfigureAwait(false);
                 }
@@ -2268,7 +2243,11 @@ namespace MediaBrowser.Controller.Entities
 
             var existingImage = GetImageInfo(image.Type, index);
 
-            if (existingImage != null)
+            if (existingImage == null)
+            {
+                AddImage(image);
+            }
+            else
             {
                 existingImage.Path = image.Path;
                 existingImage.DateModified = image.DateModified;
@@ -2276,15 +2255,6 @@ namespace MediaBrowser.Controller.Entities
                 existingImage.Height = image.Height;
                 existingImage.BlurHash = image.BlurHash;
             }
-            else
-            {
-                var current = ImageInfos;
-                var currentCount = current.Length;
-                var newArr = new ItemImageInfo[currentCount + 1];
-                current.CopyTo(newArr, 0);
-                newArr[currentCount] = image;
-                ImageInfos = newArr;
-            }
         }
 
         public void SetImagePath(ImageType type, int index, FileSystemMetadata file)
@@ -2298,7 +2268,7 @@ namespace MediaBrowser.Controller.Entities
 
             if (image == null)
             {
-                ImageInfos = ImageInfos.Concat(new[] { GetImageInfo(file, type) }).ToArray();
+                AddImage(GetImageInfo(file, type));
             }
             else
             {
@@ -2342,7 +2312,7 @@ namespace MediaBrowser.Controller.Entities
 
         public void RemoveImage(ItemImageInfo image)
         {
-            RemoveImages(new List<ItemImageInfo> { image });
+            RemoveImages(new[] { image });
         }
 
         public void RemoveImages(IEnumerable<ItemImageInfo> deletedImages)
@@ -2350,6 +2320,16 @@ namespace MediaBrowser.Controller.Entities
             ImageInfos = ImageInfos.Except(deletedImages).ToArray();
         }
 
+        public void AddImage(ItemImageInfo image)
+        {
+            var current = ImageInfos;
+            var currentCount = current.Length;
+            var newArr = new ItemImageInfo[currentCount + 1];
+            current.CopyTo(newArr, 0);
+            newArr[currentCount] = image;
+            ImageInfos = newArr;
+        }
+
         public virtual Task UpdateToRepositoryAsync(ItemUpdateType updateReason, CancellationToken cancellationToken)
          => LibraryManager.UpdateItemAsync(this, GetParent(), updateReason, cancellationToken);
 
@@ -2373,7 +2353,7 @@ namespace MediaBrowser.Controller.Entities
 
             if (deletedImages.Count > 0)
             {
-                ImageInfos = ImageInfos.Except(deletedImages).ToArray();
+                RemoveImages(deletedImages);
             }
 
             return deletedImages.Count > 0;
@@ -2715,7 +2695,7 @@ namespace MediaBrowser.Controller.Entities
 
         protected static string GetMappedPath(BaseItem item, string path, MediaProtocol? protocol)
         {
-            if (protocol.HasValue && protocol.Value == MediaProtocol.File)
+            if (protocol == MediaProtocol.File)
             {
                 return LibraryManager.GetPathAfterNetworkSubstitution(path, item);
             }
@@ -2743,8 +2723,10 @@ namespace MediaBrowser.Controller.Entities
 
         protected Task RefreshMetadataForOwnedItem(BaseItem ownedItem, bool copyTitleMetadata, MetadataRefreshOptions options, CancellationToken cancellationToken)
         {
-            var newOptions = new MetadataRefreshOptions(options);
-            newOptions.SearchResult = null;
+            var newOptions = new MetadataRefreshOptions(options)
+            {
+                SearchResult = null
+            };
 
             var item = this;
 
@@ -2805,8 +2787,10 @@ namespace MediaBrowser.Controller.Entities
 
         protected Task RefreshMetadataForOwnedVideo(MetadataRefreshOptions options, bool copyTitleMetadata, string path, CancellationToken cancellationToken)
         {
-            var newOptions = new MetadataRefreshOptions(options);
-            newOptions.SearchResult = null;
+            var newOptions = new MetadataRefreshOptions(options)
+            {
+                SearchResult = null
+            };
 
             var id = LibraryManager.GetNewItemId(path, typeof(Video));
 
@@ -2820,14 +2804,6 @@ namespace MediaBrowser.Controller.Entities
                 newOptions.ForceSave = true;
             }
 
-            // var parentId = Id;
-            // if (!video.IsOwnedItem || video.ParentId != parentId)
-            // {
-            //    video.IsOwnedItem = true;
-            //    video.ParentId = parentId;
-            //    newOptions.ForceSave = true;
-            // }
-
             if (video == null)
             {
                 return Task.FromResult(true);
@@ -2911,7 +2887,7 @@ namespace MediaBrowser.Controller.Entities
                 .Select(i => i.OfficialRating)
                 .Where(i => !string.IsNullOrEmpty(i))
                 .Distinct(StringComparer.OrdinalIgnoreCase)
-                .Select(i => new Tuple<string, int?>(i, LocalizationManager.GetRatingLevel(i)))
+                .Select(i => (i, LocalizationManager.GetRatingLevel(i)))
                 .OrderBy(i => i.Item2 ?? 1000)
                 .Select(i => i.Item1);
 
@@ -2958,18 +2934,6 @@ namespace MediaBrowser.Controller.Entities
                 .Where(i => i.ExtraType.HasValue && extraTypes.Contains(i.ExtraType.Value));
         }
 
-        public IEnumerable<BaseItem> GetTrailers()
-        {
-            if (this is IHasTrailers)
-            {
-                return ((IHasTrailers)this).LocalTrailerIds.Select(LibraryManager.GetItemById).Where(i => i != null).OrderBy(i => i.SortName);
-            }
-            else
-            {
-                return Array.Empty<BaseItem>();
-            }
-        }
-
         public virtual long GetRunTimeTicksForPlayState()
         {
             return RunTimeTicks ?? 0;

tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs

@@ -28,13 +28,13 @@ namespace Jellyfin.Providers.Tests.Manager
 {
     public class ItemImageProviderTests
     {
-        private static readonly string TestDataImagePath = "Test Data/Images/blank{0}.jpg";
+        private const string TestDataImagePath = "Test Data/Images/blank{0}.jpg";
 
         [Fact]
         public void ValidateImages_PhotoEmptyProviders_NoChange()
         {
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(new Photo(), new List<ILocalImageProvider>(), null);
+            var changed = itemImageProvider.ValidateImages(new Photo(), Enumerable.Empty<ILocalImageProvider>(), null);
 
             Assert.False(changed);
         }
@@ -43,7 +43,7 @@ namespace Jellyfin.Providers.Tests.Manager
         public void ValidateImages_EmptyItemEmptyProviders_NoChange()
         {
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(new MovieWithScreenshots(), new List<ILocalImageProvider>(), null);
+            var changed = itemImageProvider.ValidateImages(new MovieWithScreenshots(), Enumerable.Empty<ILocalImageProvider>(), null);
 
             Assert.False(changed);
         }
@@ -73,7 +73,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var imageProvider = GetImageProvider(imageType, imageCount, true);
 
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, new List<ILocalImageProvider> { imageProvider }, null);
+            var changed = itemImageProvider.ValidateImages(item, new[] { imageProvider }, null);
 
             Assert.True(changed);
             Assert.Equal(imageCount, item.GetImages(imageType).Count());
@@ -86,7 +86,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var item = GetItemWithImages(imageType, imageCount, true);
 
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, new List<ILocalImageProvider>(), null);
+            var changed = itemImageProvider.ValidateImages(item, Enumerable.Empty<ILocalImageProvider>(), null);
 
             Assert.False(changed);
             Assert.Equal(imageCount, item.GetImages(imageType).Count());
@@ -99,7 +99,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var item = GetItemWithImages(imageType, imageCount, false);
 
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.ValidateImages(item, new List<ILocalImageProvider>(), null);
+            var changed = itemImageProvider.ValidateImages(item, Enumerable.Empty<ILocalImageProvider>(), null);
 
             Assert.True(changed);
             Assert.Empty(item.GetImages(imageType));
@@ -109,7 +109,7 @@ namespace Jellyfin.Providers.Tests.Manager
         public void MergeImages_EmptyItemNewImagesEmpty_NoChange()
         {
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.MergeImages(new MovieWithScreenshots(), new List<LocalImageInfo>());
+            var changed = itemImageProvider.MergeImages(new MovieWithScreenshots(), Array.Empty<LocalImageInfo>());
 
             Assert.False(changed);
         }
@@ -237,7 +237,8 @@ namespace Jellyfin.Providers.Tests.Manager
             var refreshOptions = forceRefresh
                 ? new ImageRefreshOptions(null)
                 {
-                    ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true
+                    ImageRefreshMode = MetadataRefreshMode.FullRefresh,
+                    ReplaceAllImages = true
                 }
                 : new ImageRefreshOptions(null);
 
@@ -330,19 +331,20 @@ namespace Jellyfin.Providers.Tests.Manager
             var refreshOptions = forceRefresh
                 ? new ImageRefreshOptions(null)
                 {
-                    ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true
+                    ImageRefreshMode = MetadataRefreshMode.FullRefresh,
+                    ReplaceAllImages = true
                 }
                 : new ImageRefreshOptions(null);
 
-            var remoteInfo = new List<RemoteImageInfo>();
+            var remoteInfo = new RemoteImageInfo[imageCount];
             for (int i = 0; i < imageCount; i++)
             {
-                remoteInfo.Add(new RemoteImageInfo
+                remoteInfo[i] = new RemoteImageInfo
                 {
                     Type = imageType,
                     Url = "image url " + i,
                     Width = 1 // min width is set to 0, this will always pass
-                });
+                };
             }
 
             var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
@@ -383,7 +385,7 @@ namespace Jellyfin.Providers.Tests.Manager
             // seek 2 so it won't short-circuit out of downloading when populated
             var libraryOptions = GetLibraryOptions(item, imageType, 2);
 
-            var content = "Content";
+            const string Content = "Content";
             var remoteProvider = new Mock<IRemoteImageProvider>(MockBehavior.Strict);
             remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider");
             remoteProvider.Setup(rp => rp.GetSupportedImages(item))
@@ -393,7 +395,7 @@ namespace Jellyfin.Providers.Tests.Manager
                 {
                     ReasonPhrase = url,
                     StatusCode = HttpStatusCode.OK,
-                    Content = new StringContent(content, Encoding.UTF8, "image/jpeg")
+                    Content = new StringContent(Content, Encoding.UTF8, "image/jpeg")
                 });
 
             var refreshOptions = fullRefresh
@@ -404,15 +406,15 @@ namespace Jellyfin.Providers.Tests.Manager
                 }
                 : new ImageRefreshOptions(null);
 
-            var remoteInfo = new List<RemoteImageInfo>();
+            var remoteInfo = new RemoteImageInfo[targetImageCount];
             for (int i = 0; i < targetImageCount; i++)
             {
-                remoteInfo.Add(new RemoteImageInfo
+                remoteInfo[i] = new RemoteImageInfo()
                 {
                     Type = imageType,
                     Url = "image url " + i,
                     Width = 1 // min width is set to 0, this will always pass
-                });
+                };
             }
 
             var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
@@ -425,7 +427,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var fileSystem = new Mock<IFileSystem>();
             // match reported file size to image content length - condition for skipping already downloaded multi-images
             fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny<string>()))
-                .Returns(new FileSystemMetadata { Length = content.Length });
+                .Returns(new FileSystemMetadata { Length = Content.Length });
             var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem);
             var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
 
@@ -449,15 +451,16 @@ namespace Jellyfin.Providers.Tests.Manager
             var refreshOptions = new ImageRefreshOptions(null);
 
             // populate remote with double the required images to verify count is trimmed to the library option count
-            var remoteInfo = new List<RemoteImageInfo>();
-            for (int i = 0; i < imageCount * 2; i++)
+            var remoteInfoCount = imageCount * 2;
+            var remoteInfo = new RemoteImageInfo[remoteInfoCount];
+            for (int i = 0; i < remoteInfoCount; i++)
             {
-                remoteInfo.Add(new RemoteImageInfo
+                remoteInfo[i] = new RemoteImageInfo()
                 {
                     Type = imageType,
                     Url = "image url " + i,
                     Width = 1 // min width is set to 0, this will always pass
-                });
+                };
             }
 
             var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
@@ -552,20 +555,20 @@ namespace Jellyfin.Providers.Tests.Manager
         /// <summary>
         /// Creates a list of <see cref="LocalImageInfo"/> references of the specified type and size, optionally pointing to files that exist.
         /// </summary>
-        private static List<LocalImageInfo> GetImages(ImageType type, int count, bool validPaths)
+        private static LocalImageInfo[] GetImages(ImageType type, int count, bool validPaths)
         {
             var path = validPaths ? TestDataImagePath : "invalid path {0}";
-            var images = new List<LocalImageInfo>(count);
+            var images = new LocalImageInfo[count];
             for (int i = 0; i < count; i++)
             {
-                images.Add(new LocalImageInfo
+                images[i] = new LocalImageInfo
                 {
                     Type = type,
                     FileInfo = new FileSystemMetadata
                     {
                         FullName = string.Format(CultureInfo.InvariantCulture, path, i)
                     }
-                });
+                };
             }
 
             return images;