From 785cc1bb6ed1cbd3d0c300b9842af6e15167e715 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sun, 5 Dec 2021 17:19:40 +0100 Subject: Implement sort test for ProviderManager.GetImageProviders --- .../Manager/ProviderManagerTests.cs | 177 +++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs new file mode 100644 index 0000000000..31b1913346 --- /dev/null +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -0,0 +1,177 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Entities.Movies; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Providers; +using MediaBrowser.Model.Configuration; +using MediaBrowser.Providers.Manager; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Providers.Tests.Manager +{ + public class ProviderManagerTests + { + private static TheoryData GetImageProvidersOrderData() + => new () + { + { 3, null, null, null, null, new[] { 0, 1, 2 } }, // no order options set + + // library options ordering + { 3, null, Array.Empty(), null, null, new[] { 0, 1, 2 } }, // no order provided + { 3, null, new[] { 1 }, null, null, new[] { 1, 0, 2 } }, // one item in order + { 3, null, new[] { 2, 1, 0 }, null, null, new[] { 2, 1, 0 } }, // full reverse order + + // server options ordering + { 3, null, null, Array.Empty(), null, new[] { 0, 1, 2 } }, // no order provided + { 3, null, null, new[] { 1 }, null, new[] { 1, 0, 2 } }, // one item in order + { 3, null, null, new[] { 2, 1, 0 }, null, new[] { 2, 1, 0 } }, // full reverse order + + // IHasOrder ordering + // TODO unintuitive - default if not IHasOrder is 0, not max + { 3, null, null, null, new int?[] { null, 0, null }, new[] { 0, 1, 2 } }, // one item with order 0, no change because default order value is 0 + { 3, null, null, null, new int?[] { null, 1, null }, new[] { 0, 2, 1 } }, // one item in order (goes to end, not beginning) + { 3, null, null, null, new int?[] { 2, 1, 0 }, new[] { 2, 1, 0 } }, // full reverse order + + // multiple orders set + // TODO should library fall through to server if both are set on different elements? + { 3, null, new[] { 1 }, new[] { 2, 0, 1 }, null, new[] { 1, 0, 2 } }, // library order first, server order ignored + { 3, null, new[] { 1 }, null, new int?[] { 2, 0, 1 }, new[] { 1, 2, 0 } }, // library order first, then orderby + { 3, null, new[] { 2, 1, 0 }, new[] { 1, 2, 0 }, new int?[] { 2, 0, 1 }, new[] { 2, 1, 0 } }, // library order wins + + // ordering with ILocalImageProvider + // TODO what is the value of testing for ILocalImageProvider on the sort, should this be removed? Behavior is unintuitive + { 3, new[] { false, true, false }, new[] { 1, 0, 2 }, null, null, new[] { 0, 2, 1 } }, // ILocalImageProvider - sorts to end even when set first + { 3, new[] { false, true, false }, new[] { 1 }, null, null, new[] { 0, 1, 2 } }, // ILocalImageProvider - set order ignored when only value set + { 2, new[] { true, true }, new[] { 1, 0 }, null, null, new[] { 0, 1 } }, // ILocalImageProvider - set order ignored + { 2, new[] { true, true }, null, null, new int?[] { 1, 0 }, new[] { 1, 0 } }, // ILocalImageProvider - IHasOrder applies + }; + + [Theory] + [MemberData(nameof(GetImageProvidersOrderData))] + public void GetImageProviders_ProviderOrder_MatchesExpected(int providerCount, bool[]? localImageProvider, int[]? libraryOrder, int[]? serverOrder, int?[]? hasOrderOrder, int[] expectedOrder) + { + var item = new Movie(); + + var nameProvider = new Func(i => "Provider" + i); + + var providerList = new List(); + for (var i = 0; i < providerCount; i++) + { + var order = hasOrderOrder?[i]; + if (localImageProvider != null && localImageProvider[i]) + { + providerList.Add(MockIImageProvider(nameProvider(i), item, order)); + } + else + { + providerList.Add(MockIImageProvider(nameProvider(i), item, order)); + } + } + + var libraryOptions = new LibraryOptions(); + if (libraryOrder != null) + { + libraryOptions.TypeOptions = new[] + { + new TypeOptions + { + Type = item.GetType().Name, + ImageFetcherOrder = libraryOrder.Select(nameProvider).ToArray() + } + }; + } + + var serverConfiguration = new ServerConfiguration(); + if (serverOrder != null) + { + serverConfiguration.MetadataOptions = new[] + { + new MetadataOptions + { + ItemType = item.GetType().Name, + ImageFetcherOrder = serverOrder.Select(nameProvider).ToArray() + } + }; + } + + var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, libraryOptions: libraryOptions); + AddParts(providerManager, imageProviders: providerList); + + var refreshOptions = new ImageRefreshOptions(Mock.Of(MockBehavior.Strict)); + var actualProviders = providerManager.GetImageProviders(item, refreshOptions).ToList(); + + Assert.Equal(providerList.Count, actualProviders.Count); + for (var i = 0; i < providerList.Count; i++) + { + Assert.Equal(i, actualProviders.IndexOf(providerList[expectedOrder[i]])); + } + } + + private static IImageProvider MockIImageProvider(string name, BaseItem supportedType, int? order = null) + where T : class, IImageProvider + { + Mock? hasOrder = null; + if (order != null) + { + hasOrder = new Mock(MockBehavior.Strict); + hasOrder.Setup(i => i.Order) + .Returns((int)order); + } + + var provider = hasOrder == null + ? new Mock(MockBehavior.Strict) + : hasOrder.As(); + provider.Setup(p => p.Name) + .Returns(name); + provider.Setup(p => p.Supports(supportedType)) + .Returns(true); + return provider.Object; + } + + private static ProviderManager GetProviderManager(ServerConfiguration? serverConfiguration = null, LibraryOptions? libraryOptions = null) + { + var serverConfigurationManager = new Mock(MockBehavior.Strict); + serverConfigurationManager.Setup(i => i.Configuration) + .Returns(serverConfiguration ?? new ServerConfiguration()); + + var libraryManager = new Mock(MockBehavior.Strict); + libraryManager.Setup(i => i.GetLibraryOptions(It.IsAny())) + .Returns(libraryOptions ?? new LibraryOptions()); + + var providerManager = new ProviderManager( + null, + null, + serverConfigurationManager.Object, + null, + new NullLogger(), + null, + null, + libraryManager.Object, + null); + + return providerManager; + } + + private static void AddParts( + ProviderManager providerManager, + IEnumerable? imageProviders = null, + IEnumerable? metadataServices = null, + IEnumerable? metadataProviders = null, + IEnumerable? metadataSavers = null, + IEnumerable? externalIds = null) + { + imageProviders ??= Array.Empty(); + metadataServices ??= Array.Empty(); + metadataProviders ??= Array.Empty(); + metadataSavers ??= Array.Empty(); + externalIds ??= Array.Empty(); + + providerManager.AddParts(imageProviders, metadataServices, metadataProviders, metadataSavers, externalIds); + } + } +} -- cgit v1.2.3 From 8515e8fbd111278cad95430e50904d6721be3a63 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sun, 5 Dec 2021 21:33:31 +0100 Subject: Improve image provider sorting Remove irrelevant check for ILocalImageProvider Providers that are not IHasOrder default to middle, not beginning --- MediaBrowser.Providers/Manager/ProviderManager.cs | 60 +++++++++------------- .../Manager/ProviderManagerTests.cs | 47 ++++++----------- 2 files changed, 40 insertions(+), 67 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 1b73574774..855c467208 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -310,31 +310,25 @@ namespace MediaBrowser.Providers.Manager private IEnumerable GetImageProviders(BaseItem item, LibraryOptions libraryOptions, MetadataOptions options, ImageRefreshOptions refreshOptions, bool includeDisabled) { - // Avoid implicitly captured closure - var currentOptions = options; - var typeOptions = libraryOptions.GetTypeOptions(item.GetType().Name); - var typeFetcherOrder = typeOptions?.ImageFetcherOrder; + var fetcherOrder = typeOptions?.ImageFetcherOrder ?? options.ImageFetcherOrder; return ImageProviders.Where(i => CanRefresh(i, item, libraryOptions, refreshOptions, includeDisabled)) - .OrderBy(i => - { - // See if there's a user-defined order - if (i is not ILocalImageProvider) - { - var fetcherOrder = typeFetcherOrder ?? currentOptions.ImageFetcherOrder; - var index = Array.IndexOf(fetcherOrder, i.Name); + .OrderBy(i => GetConfiguredOrder(fetcherOrder, i.Name)) + .ThenBy(GetOrder); + } - if (index != -1) - { - return index; - } - } + private static int GetConfiguredOrder(string[] order, string providerName) + { + var index = Array.IndexOf(order, providerName); - // Not configured. Just return some high number to put it at the end. - return 100; - }) - .ThenBy(GetOrder); + if (index != -1) + { + return index; + } + + // default to end + return int.MaxValue; } /// @@ -450,21 +444,6 @@ namespace MediaBrowser.Providers.Manager } } - /// - /// Gets the order. - /// - /// The provider. - /// System.Int32. - private int GetOrder(IImageProvider provider) - { - if (provider is not IHasOrder hasOrder) - { - return 0; - } - - return hasOrder.Order; - } - private int GetConfiguredOrder(BaseItem item, IMetadataProvider provider, LibraryOptions libraryOptions, MetadataOptions globalMetadataOptions) { // See if there's a user-defined order @@ -500,6 +479,17 @@ namespace MediaBrowser.Providers.Manager return 100; } + private static int GetOrder(object provider) + { + if (provider is IHasOrder hasOrder) + { + return hasOrder.Order; + } + + // after items that want to be first (~0) but before items that want to be last (~100) + return 50; + } + private int GetDefaultOrder(IMetadataProvider provider) { if (provider is IHasOrder hasOrder) diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 31b1913346..590f50b256 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -16,44 +16,34 @@ namespace Jellyfin.Providers.Tests.Manager { public class ProviderManagerTests { - private static TheoryData GetImageProvidersOrderData() + private static TheoryData GetImageProvidersOrderData() => new () { - { 3, null, null, null, null, new[] { 0, 1, 2 } }, // no order options set + { 3, null, null, null, new[] { 0, 1, 2 } }, // no order options set // library options ordering - { 3, null, Array.Empty(), null, null, new[] { 0, 1, 2 } }, // no order provided - { 3, null, new[] { 1 }, null, null, new[] { 1, 0, 2 } }, // one item in order - { 3, null, new[] { 2, 1, 0 }, null, null, new[] { 2, 1, 0 } }, // full reverse order + { 3, Array.Empty(), null, null, new[] { 0, 1, 2 } }, // no order provided + { 3, new[] { 1 }, null, null, new[] { 1, 0, 2 } }, // one item in order + { 3, new[] { 2, 1, 0 }, null, null, new[] { 2, 1, 0 } }, // full reverse order // server options ordering - { 3, null, null, Array.Empty(), null, new[] { 0, 1, 2 } }, // no order provided - { 3, null, null, new[] { 1 }, null, new[] { 1, 0, 2 } }, // one item in order - { 3, null, null, new[] { 2, 1, 0 }, null, new[] { 2, 1, 0 } }, // full reverse order + { 3, null, Array.Empty(), null, new[] { 0, 1, 2 } }, // no order provided + { 3, null, new[] { 1 }, null, new[] { 1, 0, 2 } }, // one item in order + { 3, null, new[] { 2, 1, 0 }, null, new[] { 2, 1, 0 } }, // full reverse order // IHasOrder ordering - // TODO unintuitive - default if not IHasOrder is 0, not max - { 3, null, null, null, new int?[] { null, 0, null }, new[] { 0, 1, 2 } }, // one item with order 0, no change because default order value is 0 - { 3, null, null, null, new int?[] { null, 1, null }, new[] { 0, 2, 1 } }, // one item in order (goes to end, not beginning) - { 3, null, null, null, new int?[] { 2, 1, 0 }, new[] { 2, 1, 0 } }, // full reverse order + { 3, null, null, new int?[] { null, 1, null }, new[] { 1, 0, 2 } }, // one item with defined order + { 3, null, null, new int?[] { 2, 1, 0 }, new[] { 2, 1, 0 } }, // full reverse order // multiple orders set - // TODO should library fall through to server if both are set on different elements? - { 3, null, new[] { 1 }, new[] { 2, 0, 1 }, null, new[] { 1, 0, 2 } }, // library order first, server order ignored - { 3, null, new[] { 1 }, null, new int?[] { 2, 0, 1 }, new[] { 1, 2, 0 } }, // library order first, then orderby - { 3, null, new[] { 2, 1, 0 }, new[] { 1, 2, 0 }, new int?[] { 2, 0, 1 }, new[] { 2, 1, 0 } }, // library order wins - - // ordering with ILocalImageProvider - // TODO what is the value of testing for ILocalImageProvider on the sort, should this be removed? Behavior is unintuitive - { 3, new[] { false, true, false }, new[] { 1, 0, 2 }, null, null, new[] { 0, 2, 1 } }, // ILocalImageProvider - sorts to end even when set first - { 3, new[] { false, true, false }, new[] { 1 }, null, null, new[] { 0, 1, 2 } }, // ILocalImageProvider - set order ignored when only value set - { 2, new[] { true, true }, new[] { 1, 0 }, null, null, new[] { 0, 1 } }, // ILocalImageProvider - set order ignored - { 2, new[] { true, true }, null, null, new int?[] { 1, 0 }, new[] { 1, 0 } }, // ILocalImageProvider - IHasOrder applies + { 3, new[] { 1 }, new[] { 2, 0, 1 }, null, new[] { 1, 0, 2 } }, // library order first, server order ignored + { 3, new[] { 1 }, null, new int?[] { 2, 0, 1 }, new[] { 1, 2, 0 } }, // library order first, then orderby + { 3, new[] { 2, 1, 0 }, new[] { 1, 2, 0 }, new int?[] { 2, 0, 1 }, new[] { 2, 1, 0 } }, // library order wins }; [Theory] [MemberData(nameof(GetImageProvidersOrderData))] - public void GetImageProviders_ProviderOrder_MatchesExpected(int providerCount, bool[]? localImageProvider, int[]? libraryOrder, int[]? serverOrder, int?[]? hasOrderOrder, int[] expectedOrder) + public void GetImageProviders_ProviderOrder_MatchesExpected(int providerCount, int[]? libraryOrder, int[]? serverOrder, int?[]? hasOrderOrder, int[] expectedOrder) { var item = new Movie(); @@ -63,14 +53,7 @@ namespace Jellyfin.Providers.Tests.Manager for (var i = 0; i < providerCount; i++) { var order = hasOrderOrder?[i]; - if (localImageProvider != null && localImageProvider[i]) - { - providerList.Add(MockIImageProvider(nameProvider(i), item, order)); - } - else - { - providerList.Add(MockIImageProvider(nameProvider(i), item, order)); - } + providerList.Add(MockIImageProvider(nameProvider(i), item, order)); } var libraryOptions = new LibraryOptions(); -- cgit v1.2.3 From 56900d0fc3bc791fd3c0a92bda22ca2f23f28be1 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Mon, 6 Dec 2021 00:09:46 +0100 Subject: Implement CanRefresh tests for ProviderManager.GetImageProviders --- .../Manager/ProviderManagerTests.cs | 93 ++++++++++++++++++++-- 1 file changed, 87 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 590f50b256..4a1b90895f 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using MediaBrowser.Controller.BaseItemManager; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; @@ -53,7 +54,7 @@ namespace Jellyfin.Providers.Tests.Manager for (var i = 0; i < providerCount; i++) { var order = hasOrderOrder?[i]; - providerList.Add(MockIImageProvider(nameProvider(i), item, order)); + providerList.Add(MockIImageProvider(nameProvider(i), item, order: order)); } var libraryOptions = new LibraryOptions(); @@ -95,7 +96,78 @@ namespace Jellyfin.Providers.Tests.Manager } } - private static IImageProvider MockIImageProvider(string name, BaseItem supportedType, int? order = null) + [Theory] + [InlineData(true, false, true)] + [InlineData(false, false, false)] + [InlineData(true, true, false)] + public void GetImageProviders_CanRefreshBasic_WhenSupportsWithoutError(bool supports, bool errorOnSupported, bool expected) + { + GetImageProviders_CanRefresh_Tester(typeof(IImageProvider), supports, expected, errorOnSupported: errorOnSupported); + } + + [Theory] + [InlineData(typeof(ILocalImageProvider), false, true)] + [InlineData(typeof(ILocalImageProvider), true, true)] + [InlineData(typeof(IImageProvider), false, false)] + [InlineData(typeof(IImageProvider), true, true)] + public void GetImageProviders_CanRefreshLocked_WhenLocalOrFullRefresh(Type providerType, bool fullRefresh, bool expected) + { + GetImageProviders_CanRefresh_Tester(providerType, true, expected, itemLocked: true, fullRefresh: fullRefresh); + } + + [Theory] + [InlineData(typeof(ILocalImageProvider), false, true)] + [InlineData(typeof(IRemoteImageProvider), true, true)] + [InlineData(typeof(IDynamicImageProvider), true, true)] + [InlineData(typeof(IRemoteImageProvider), false, false)] + [InlineData(typeof(IDynamicImageProvider), false, false)] + public void GetImageProviders_CanRefreshEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) + { + GetImageProviders_CanRefresh_Tester(providerType, true, expected, baseItemEnabled: enabled); + } + + private static void GetImageProviders_CanRefresh_Tester(Type providerType, bool supports, bool expected, bool errorOnSupported = false, bool itemLocked = false, bool fullRefresh = false, bool baseItemEnabled = true) + { + var item = new Movie + { + IsLocked = itemLocked + }; + + var providerName = "provider"; + IImageProvider provider = providerType.Name switch + { + "IImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), + "ILocalImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), + "IRemoteImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), + "IDynamicImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), + _ => throw new ArgumentException("Unexpected provider type") + }; + + var refreshOptions = new ImageRefreshOptions(Mock.Of(MockBehavior.Strict)) + { + ImageRefreshMode = fullRefresh ? MetadataRefreshMode.FullRefresh : MetadataRefreshMode.Default + }; + + var baseItemManager = new Mock(MockBehavior.Strict); + baseItemManager.Setup(i => i.IsImageFetcherEnabled(item, It.IsAny(), providerName)) + .Returns(baseItemEnabled); + + var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); + AddParts(providerManager, imageProviders: new[] { provider }); + + var actualProviders = providerManager.GetImageProviders(item, refreshOptions); + + if (expected) + { + Assert.Single(actualProviders); + } + else + { + Assert.Empty(actualProviders); + } + } + + private static IImageProvider MockIImageProvider(string name, BaseItem expectedType, bool supports = true, int? order = null, bool errorOnSupported = false) where T : class, IImageProvider { Mock? hasOrder = null; @@ -111,12 +183,21 @@ namespace Jellyfin.Providers.Tests.Manager : hasOrder.As(); provider.Setup(p => p.Name) .Returns(name); - provider.Setup(p => p.Supports(supportedType)) - .Returns(true); + if (errorOnSupported) + { + provider.Setup(p => p.Supports(It.IsAny())) + .Throws(new ArgumentException()); + } + else + { + provider.Setup(p => p.Supports(expectedType)) + .Returns(supports); + } + return provider.Object; } - private static ProviderManager GetProviderManager(ServerConfiguration? serverConfiguration = null, LibraryOptions? libraryOptions = null) + private static ProviderManager GetProviderManager(ServerConfiguration? serverConfiguration = null, LibraryOptions? libraryOptions = null, IBaseItemManager? baseItemManager = null) { var serverConfigurationManager = new Mock(MockBehavior.Strict); serverConfigurationManager.Setup(i => i.Configuration) @@ -135,7 +216,7 @@ namespace Jellyfin.Providers.Tests.Manager null, null, libraryManager.Object, - null); + baseItemManager); return providerManager; } -- cgit v1.2.3 From 11c7c24f0ef06e6366c075062928976ae0d30600 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Mon, 6 Dec 2021 22:31:16 +0100 Subject: Clarify naming, minor method ordering improvement --- MediaBrowser.Providers/Manager/ProviderManager.cs | 40 +++++++++++----------- .../Manager/ProviderManagerTests.cs | 14 ++++---- 2 files changed, 27 insertions(+), 27 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 9bae738010..633b3b1db7 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -297,18 +297,31 @@ namespace MediaBrowser.Providers.Manager return GetRemoteImageProviders(item, true).Select(i => new ImageProviderInfo(i.Name, i.GetSupportedImages(item).ToArray())); } + private IEnumerable GetRemoteImageProviders(BaseItem item, bool includeDisabled) + { + var options = GetMetadataOptions(item); + var libraryOptions = _libraryManager.GetLibraryOptions(item); + + return GetImageProvidersInternal( + item, + libraryOptions, + options, + new ImageRefreshOptions(new DirectoryService(_fileSystem)), + includeDisabled).OfType(); + } + /// public IEnumerable GetImageProviders(BaseItem item, ImageRefreshOptions refreshOptions) { - return GetImageProviders(item, _libraryManager.GetLibraryOptions(item), GetMetadataOptions(item), refreshOptions, false); + return GetImageProvidersInternal(item, _libraryManager.GetLibraryOptions(item), GetMetadataOptions(item), refreshOptions, false); } - private IEnumerable GetImageProviders(BaseItem item, LibraryOptions libraryOptions, MetadataOptions options, ImageRefreshOptions refreshOptions, bool includeDisabled) + private IEnumerable GetImageProvidersInternal(BaseItem item, LibraryOptions libraryOptions, MetadataOptions options, ImageRefreshOptions refreshOptions, bool includeDisabled) { var typeOptions = libraryOptions.GetTypeOptions(item.GetType().Name); var fetcherOrder = typeOptions?.ImageFetcherOrder ?? options.ImageFetcherOrder; - return _imageProviders.Where(i => CanRefresh(i, item, libraryOptions, refreshOptions, includeDisabled)) + return _imageProviders.Where(i => CanRefreshImages(i, item, libraryOptions, refreshOptions, includeDisabled)) .OrderBy(i => GetConfiguredOrder(fetcherOrder, i.Name)) .ThenBy(GetOrder); } @@ -342,25 +355,12 @@ namespace MediaBrowser.Providers.Manager var currentOptions = globalMetadataOptions; return _metadataProviders.OfType>() - .Where(i => CanRefresh(i, item, libraryOptions, includeDisabled, forceEnableInternetMetadata)) + .Where(i => CanRefreshMetadata(i, item, libraryOptions, includeDisabled, forceEnableInternetMetadata)) .OrderBy(i => GetConfiguredOrder(item, i, libraryOptions, currentOptions)) .ThenBy(GetDefaultOrder); } - private IEnumerable GetRemoteImageProviders(BaseItem item, bool includeDisabled) - { - var options = GetMetadataOptions(item); - var libraryOptions = _libraryManager.GetLibraryOptions(item); - - return GetImageProviders( - item, - libraryOptions, - options, - new ImageRefreshOptions(new DirectoryService(_fileSystem)), - includeDisabled).OfType(); - } - - private bool CanRefresh( + private bool CanRefreshMetadata( IMetadataProvider provider, BaseItem item, LibraryOptions libraryOptions, @@ -401,7 +401,7 @@ namespace MediaBrowser.Providers.Manager return true; } - private bool CanRefresh( + private bool CanRefreshImages( IImageProvider provider, BaseItem item, LibraryOptions libraryOptions, @@ -535,7 +535,7 @@ namespace MediaBrowser.Providers.Manager var libraryOptions = new LibraryOptions(); - var imageProviders = GetImageProviders( + var imageProviders = GetImageProvidersInternal( dummy, libraryOptions, options, diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 4a1b90895f..98c1e19b20 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -100,9 +100,9 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(true, false, true)] [InlineData(false, false, false)] [InlineData(true, true, false)] - public void GetImageProviders_CanRefreshBasic_WhenSupportsWithoutError(bool supports, bool errorOnSupported, bool expected) + public void GetImageProviders_CanRefreshImagesBasic_WhenSupportsWithoutError(bool supports, bool errorOnSupported, bool expected) { - GetImageProviders_CanRefresh_Tester(typeof(IImageProvider), supports, expected, errorOnSupported: errorOnSupported); + GetImageProviders_CanRefreshImages_Tester(typeof(IImageProvider), supports, expected, errorOnSupported: errorOnSupported); } [Theory] @@ -110,9 +110,9 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(typeof(ILocalImageProvider), true, true)] [InlineData(typeof(IImageProvider), false, false)] [InlineData(typeof(IImageProvider), true, true)] - public void GetImageProviders_CanRefreshLocked_WhenLocalOrFullRefresh(Type providerType, bool fullRefresh, bool expected) + public void GetImageProviders_CanRefreshImagesLocked_WhenLocalOrFullRefresh(Type providerType, bool fullRefresh, bool expected) { - GetImageProviders_CanRefresh_Tester(providerType, true, expected, itemLocked: true, fullRefresh: fullRefresh); + GetImageProviders_CanRefreshImages_Tester(providerType, true, expected, itemLocked: true, fullRefresh: fullRefresh); } [Theory] @@ -121,12 +121,12 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(typeof(IDynamicImageProvider), true, true)] [InlineData(typeof(IRemoteImageProvider), false, false)] [InlineData(typeof(IDynamicImageProvider), false, false)] - public void GetImageProviders_CanRefreshEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) + public void GetImageProviders_CanRefreshImagesEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) { - GetImageProviders_CanRefresh_Tester(providerType, true, expected, baseItemEnabled: enabled); + GetImageProviders_CanRefreshImages_Tester(providerType, true, expected, baseItemEnabled: enabled); } - private static void GetImageProviders_CanRefresh_Tester(Type providerType, bool supports, bool expected, bool errorOnSupported = false, bool itemLocked = false, bool fullRefresh = false, bool baseItemEnabled = true) + private static void GetImageProviders_CanRefreshImages_Tester(Type providerType, bool supports, bool expected, bool errorOnSupported = false, bool itemLocked = false, bool fullRefresh = false, bool baseItemEnabled = true) { var item = new Movie { -- cgit v1.2.3 From 91e706d3873440a28f107da04143a374d4277b9a Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Wed, 8 Dec 2021 00:52:57 +0100 Subject: Implement sort test for ProviderManager.GetMetadataProviders --- .../Manager/ProviderManagerTests.cs | 186 ++++++++++++++++++++- 1 file changed, 177 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 98c1e19b20..7a542f0eba 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -37,7 +37,7 @@ namespace Jellyfin.Providers.Tests.Manager { 3, null, null, new int?[] { 2, 1, 0 }, new[] { 2, 1, 0 } }, // full reverse order // multiple orders set - { 3, new[] { 1 }, new[] { 2, 0, 1 }, null, new[] { 1, 0, 2 } }, // library order first, server order ignored + { 3, new[] { 1 }, new[] { 2, 0, 1 }, null, new[] { 1, 0, 2 } }, // partial library order first, server order ignored { 3, new[] { 1 }, null, new int?[] { 2, 0, 1 }, new[] { 1, 2, 0 } }, // library order first, then orderby { 3, new[] { 2, 1, 0 }, new[] { 1, 2, 0 }, new int?[] { 2, 0, 1 }, new[] { 2, 1, 0 } }, // library order wins }; @@ -90,10 +90,8 @@ namespace Jellyfin.Providers.Tests.Manager var actualProviders = providerManager.GetImageProviders(item, refreshOptions).ToList(); Assert.Equal(providerList.Count, actualProviders.Count); - for (var i = 0; i < providerList.Count; i++) - { - Assert.Equal(i, actualProviders.IndexOf(providerList[expectedOrder[i]])); - } + var actualOrder = actualProviders.Select(i => providerList.IndexOf(i)).ToArray(); + Assert.Equal(expectedOrder, actualOrder); } [Theory] @@ -167,8 +165,129 @@ namespace Jellyfin.Providers.Tests.Manager } } - private static IImageProvider MockIImageProvider(string name, BaseItem expectedType, bool supports = true, int? order = null, bool errorOnSupported = false) - where T : class, IImageProvider + private static TheoryData GetMetadataProvidersOrderData() + { + var l = "local"; + var r = "remote"; + return new () + { + { new[] { l, l, r, r }, null, null, null, null, null, new[] { 0, 1, 2, 3 } }, // no order options set + + // library options ordering + { new[] { l, l, r, r }, Array.Empty(), Array.Empty(), null, null, null, new[] { 0, 1, 2, 3 } }, // no order provided + // local only + { new[] { r, l, l, l }, new[] { 2 }, null, null, null, null, new[] { 2, 0, 1, 3 } }, // one item in order + { new[] { r, l, l, l }, new[] { 3, 2, 1 }, null, null, null, null, new[] { 3, 2, 1, 0 } }, // full reverse order + // remote only + { new[] { l, r, r, r }, null, new[] { 2 }, null, null, null, new[] { 2, 0, 1, 3 } }, // one item in order + { new[] { l, r, r, r }, null, new[] { 3, 2, 1 }, null, null, null, new[] { 3, 2, 1, 0 } }, // full reverse order + // local and remote, note that results will be interleaved (odd but expected) + { new[] { l, l, r, r }, new[] { 1 }, new[] { 3 }, null, null, null, new[] { 1, 3, 0, 2 } }, // one item in each order + { new[] { l, l, l, r, r, r }, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, null, null, null, new[] { 2, 5, 1, 4, 0, 3 } }, // full reverse order + + // // server options ordering + { new[] { l, l, r, r }, null, null, Array.Empty(), Array.Empty(), null, new[] { 0, 1, 2, 3 } }, // no order provided + // local only + { new[] { r, l, l, l }, null, null, new[] { 2 }, null, null, new[] { 2, 0, 1, 3 } }, // one item in order + { new[] { r, l, l, l }, null, null, new[] { 3, 2, 1 }, null, null, new[] { 3, 2, 1, 0 } }, // full reverse order + // remote only + { new[] { l, r, r, r }, null, null, null, new[] { 2 }, null, new[] { 2, 0, 1, 3 } }, // one item in order + { new[] { l, r, r, r }, null, null, null, new[] { 3, 2, 1 }, null, new[] { 3, 2, 1, 0 } }, // full reverse order + // local and remote, note that results will be interleaved (odd but expected) + { new[] { l, l, r, r }, null, null, new[] { 1 }, new[] { 3 }, null, new[] { 1, 3, 0, 2 } }, // one item in each order + { new[] { l, l, l, r, r, r }, null, null, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, null, new[] { 2, 5, 1, 4, 0, 3 } }, // full reverse order + + // IHasOrder ordering (not interleaved, doesn't care about types) + // TODO unset goes to beginning, not end + { new[] { l, l, r, r }, null, null, null, null, new int?[] { 2, null, 1, null }, new[] { 1, 3, 2, 0 } }, // partially defined + { new[] { l, l, r, r }, null, null, null, null, new int?[] { 3, 2, 1, 0 }, new[] { 3, 2, 1, 0 } }, // full reverse order + // note odd interaction - orderby determines order of slot when local and remote both have a slot 0 + { new[] { l, l, r, r }, new[] { 1 }, new[] { 3 }, null, null, new int?[] { null, 2, null, 1 }, new[] { 3, 1, 0, 2 } }, // sorts interleaved results + + // multiple orders set + { new[] { l, l, l, r, r, r }, new[] { 1 }, new[] { 4 }, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, null, new[] { 1, 4, 0, 2, 3, 5 } }, // partial library order first, server order ignored + { new[] { l, l, l }, new[] { 1 }, null, null, null, new int?[] { 2, 0, 1 }, new[] { 1, 2, 0 } }, // library order first, then orderby + { new[] { l, l, l, r, r, r }, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, new[] { 1, 2, 0 }, new[] { 4, 5, 3 }, new int?[] { 5, 4, 1, 6, 3, 2 }, new[] { 2, 5, 4, 1, 0, 3 } }, // library order wins (with orderby between local/remote) + }; + } + + [Theory] + [MemberData(nameof(GetMetadataProvidersOrderData))] + public void GetMetadataProviders_ProviderOrder_MatchesExpected(string[] providers, int[]? libraryLocalOrder, int[]? libraryRemoteOrder, int[]? serverLocalOrder, int[]? serverRemoteOrder, int?[]? hasOrderOrder, int[] expectedOrder) + { + var item = new MetadataTestItem(); + var typeNames = new Dictionary + { + { "remote", nameof(IRemoteMetadataProvider) }, + { "local", nameof(ILocalMetadataProvider) }, + { "custom", nameof(ICustomMetadataProvider) } + }; + + var nameProvider = new Func(i => "Provider" + i); + + var providerList = new List>(); + for (var i = 0; i < providers.Length; i++) + { + var order = hasOrderOrder?[i]; + providerList.Add(MockIMetadataProviderMapper(typeNames[providers[i]], nameProvider(i), order: order)); + } + + var libraryOptions = new LibraryOptions(); + if (libraryLocalOrder != null) + { + libraryOptions.LocalMetadataReaderOrder = libraryLocalOrder.Select(nameProvider).ToArray(); + } + + if (libraryRemoteOrder != null) + { + libraryOptions.TypeOptions = new[] + { + new TypeOptions + { + Type = item.GetType().Name, + MetadataFetcherOrder = libraryRemoteOrder.Select(nameProvider).ToArray() + } + }; + } + + var serverConfiguration = new ServerConfiguration(); + if (serverLocalOrder != null || serverRemoteOrder != null) + { + serverConfiguration.MetadataOptions = new[] + { + new MetadataOptions + { + ItemType = item.GetType().Name + } + }; + if (serverLocalOrder != null) + { + serverConfiguration.MetadataOptions[0].LocalMetadataReaderOrder = serverLocalOrder.Select(nameProvider).ToArray(); + } + + if (serverRemoteOrder != null) + { + serverConfiguration.MetadataOptions[0].MetadataFetcherOrder = serverRemoteOrder.Select(nameProvider).ToArray(); + } + } + + var baseItemManager = new Mock(MockBehavior.Strict); + baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), It.IsAny())) + .Returns(true); + + var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, baseItemManager: baseItemManager.Object); + AddParts(providerManager, metadataProviders: providerList); + + // TODO why does this take libraryOptions directly while GetImageProviders did not? + var actualProviders = providerManager.GetMetadataProviders(item, libraryOptions).ToList(); + + Assert.Equal(providerList.Count, actualProviders.Count); + var actualOrder = actualProviders.Select(i => providerList.IndexOf(i)).ToArray(); + Assert.Equal(expectedOrder, actualOrder); + } + + private static IImageProvider MockIImageProvider(string name, BaseItem expectedType, bool supports = true, int? order = null, bool errorOnSupported = false) + where TProviderType : class, IImageProvider { Mock? hasOrder = null; if (order != null) @@ -179,8 +298,8 @@ namespace Jellyfin.Providers.Tests.Manager } var provider = hasOrder == null - ? new Mock(MockBehavior.Strict) - : hasOrder.As(); + ? new Mock(MockBehavior.Strict) + : hasOrder.As(); provider.Setup(p => p.Name) .Returns(name); if (errorOnSupported) @@ -197,6 +316,38 @@ namespace Jellyfin.Providers.Tests.Manager return provider.Object; } + private static IMetadataProvider MockIMetadataProviderMapper(string typeName, string providerName, int? order = null) + where TItemType : BaseItem, IHasLookupInfo + where TLookupInfoType : ItemLookupInfo, new() + => typeName switch + { + "ILocalMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), + "IRemoteMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), + "ICustomMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), + _ => MockIMetadataProvider, TItemType>(providerName, order) + }; + + private static IMetadataProvider MockIMetadataProvider(string name, int? order = null) + where TProviderType : class, IMetadataProvider + where TItemType : BaseItem + { + Mock? hasOrder = null; + if (order != null) + { + hasOrder = new Mock(MockBehavior.Strict); + hasOrder.Setup(i => i.Order) + .Returns((int)order); + } + + var provider = hasOrder == null + ? new Mock(MockBehavior.Strict) + : hasOrder.As(); + provider.Setup(p => p.Name) + .Returns(name); + + return provider.Object; + } + private static ProviderManager GetProviderManager(ServerConfiguration? serverConfiguration = null, LibraryOptions? libraryOptions = null, IBaseItemManager? baseItemManager = null) { var serverConfigurationManager = new Mock(MockBehavior.Strict); @@ -237,5 +388,22 @@ namespace Jellyfin.Providers.Tests.Manager providerManager.AddParts(imageProviders, metadataServices, metadataProviders, metadataSavers, externalIds); } + + /// + /// Simple extension to force SupportsLocalMetadata to true. + /// + public class MetadataTestItem : BaseItem, IHasLookupInfo + { + public override bool SupportsLocalMetadata => true; + + public MetadataTestItemInfo GetLookupInfo() + { + return GetItemLookupInfo(); + } + } + + public class MetadataTestItemInfo : ItemLookupInfo + { + } } } -- cgit v1.2.3 From e7df72de497f25deb7f77bf9de39aeaba1159d11 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Wed, 8 Dec 2021 16:49:09 +0100 Subject: Improve metadata provider sorting Extract configured order up front instead of for each provider Non-IHasOrder providers default to middle, not beginning Merge image and metadata sort helper methods --- MediaBrowser.Providers/Manager/ProviderManager.cs | 78 ++++++---------------- .../Manager/ProviderManagerTests.cs | 16 ++--- 2 files changed, 26 insertions(+), 68 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 633b3b1db7..82d633e234 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -323,20 +323,7 @@ namespace MediaBrowser.Providers.Manager return _imageProviders.Where(i => CanRefreshImages(i, item, libraryOptions, refreshOptions, includeDisabled)) .OrderBy(i => GetConfiguredOrder(fetcherOrder, i.Name)) - .ThenBy(GetOrder); - } - - private static int GetConfiguredOrder(string[] order, string providerName) - { - var index = Array.IndexOf(order, providerName); - - if (index != -1) - { - return index; - } - - // default to end - return int.MaxValue; + .ThenBy(GetDefaultOrder); } /// @@ -351,12 +338,23 @@ namespace MediaBrowser.Providers.Manager private IEnumerable> GetMetadataProvidersInternal(BaseItem item, LibraryOptions libraryOptions, MetadataOptions globalMetadataOptions, bool includeDisabled, bool forceEnableInternetMetadata) where T : BaseItem { - // Avoid implicitly captured closure - var currentOptions = globalMetadataOptions; + var localMetadataReaderOrder = libraryOptions.LocalMetadataReaderOrder ?? globalMetadataOptions.LocalMetadataReaderOrder; + var typeOptions = libraryOptions.GetTypeOptions(item.GetType().Name); + var metadataFetcherOrder = typeOptions?.MetadataFetcherOrder ?? globalMetadataOptions.MetadataFetcherOrder; return _metadataProviders.OfType>() .Where(i => CanRefreshMetadata(i, item, libraryOptions, includeDisabled, forceEnableInternetMetadata)) - .OrderBy(i => GetConfiguredOrder(item, i, libraryOptions, currentOptions)) + .OrderBy(i => + { + // local and remote providers will be interleaved in the final order + // only relative order within a type matters: consumers of the list filter to one or the other + switch (i) + { + case ILocalMetadataProvider: return GetConfiguredOrder(localMetadataReaderOrder, i.Name); + case IRemoteMetadataProvider: return GetConfiguredOrder(metadataFetcherOrder, i.Name); + default: return int.MaxValue; // default to end + } + }) .ThenBy(GetDefaultOrder); } @@ -439,42 +437,20 @@ namespace MediaBrowser.Providers.Manager } } - private int GetConfiguredOrder(BaseItem item, IMetadataProvider provider, LibraryOptions libraryOptions, MetadataOptions globalMetadataOptions) + private static int GetConfiguredOrder(string[] order, string providerName) { - // See if there's a user-defined order - if (provider is ILocalMetadataProvider) - { - var configuredOrder = libraryOptions.LocalMetadataReaderOrder ?? globalMetadataOptions.LocalMetadataReaderOrder; - - var index = Array.IndexOf(configuredOrder, provider.Name); - - if (index != -1) - { - return index; - } - } + var index = Array.IndexOf(order, providerName); - // See if there's a user-defined order - if (provider is IRemoteMetadataProvider) + if (index != -1) { - var typeOptions = libraryOptions.GetTypeOptions(item.GetType().Name); - var typeFetcherOrder = typeOptions?.MetadataFetcherOrder; - - var fetcherOrder = typeFetcherOrder ?? globalMetadataOptions.MetadataFetcherOrder; - - var index = Array.IndexOf(fetcherOrder, provider.Name); - - if (index != -1) - { - return index; - } + return index; } - // Not configured. Just return some high number to put it at the end. - return 100; + // default to end + return int.MaxValue; } - private static int GetOrder(object provider) + private static int GetDefaultOrder(object provider) { if (provider is IHasOrder hasOrder) { @@ -485,16 +461,6 @@ namespace MediaBrowser.Providers.Manager return 50; } - private int GetDefaultOrder(IMetadataProvider provider) - { - if (provider is IHasOrder hasOrder) - { - return hasOrder.Order; - } - - return 0; - } - /// public MetadataPluginSummary[] GetAllMetadataPlugins() { diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 7a542f0eba..d59e4070f9 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -167,8 +167,8 @@ namespace Jellyfin.Providers.Tests.Manager private static TheoryData GetMetadataProvidersOrderData() { - var l = "local"; - var r = "remote"; + var l = nameof(ILocalMetadataProvider); + var r = nameof(IRemoteMetadataProvider); return new () { { new[] { l, l, r, r }, null, null, null, null, null, new[] { 0, 1, 2, 3 } }, // no order options set @@ -198,8 +198,7 @@ namespace Jellyfin.Providers.Tests.Manager { new[] { l, l, l, r, r, r }, null, null, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, null, new[] { 2, 5, 1, 4, 0, 3 } }, // full reverse order // IHasOrder ordering (not interleaved, doesn't care about types) - // TODO unset goes to beginning, not end - { new[] { l, l, r, r }, null, null, null, null, new int?[] { 2, null, 1, null }, new[] { 1, 3, 2, 0 } }, // partially defined + { new[] { l, l, r, r }, null, null, null, null, new int?[] { 2, null, 1, null }, new[] { 2, 0, 1, 3 } }, // partially defined { new[] { l, l, r, r }, null, null, null, null, new int?[] { 3, 2, 1, 0 }, new[] { 3, 2, 1, 0 } }, // full reverse order // note odd interaction - orderby determines order of slot when local and remote both have a slot 0 { new[] { l, l, r, r }, new[] { 1 }, new[] { 3 }, null, null, new int?[] { null, 2, null, 1 }, new[] { 3, 1, 0, 2 } }, // sorts interleaved results @@ -216,12 +215,6 @@ namespace Jellyfin.Providers.Tests.Manager public void GetMetadataProviders_ProviderOrder_MatchesExpected(string[] providers, int[]? libraryLocalOrder, int[]? libraryRemoteOrder, int[]? serverLocalOrder, int[]? serverRemoteOrder, int?[]? hasOrderOrder, int[] expectedOrder) { var item = new MetadataTestItem(); - var typeNames = new Dictionary - { - { "remote", nameof(IRemoteMetadataProvider) }, - { "local", nameof(ILocalMetadataProvider) }, - { "custom", nameof(ICustomMetadataProvider) } - }; var nameProvider = new Func(i => "Provider" + i); @@ -229,7 +222,7 @@ namespace Jellyfin.Providers.Tests.Manager for (var i = 0; i < providers.Length; i++) { var order = hasOrderOrder?[i]; - providerList.Add(MockIMetadataProviderMapper(typeNames[providers[i]], nameProvider(i), order: order)); + providerList.Add(MockIMetadataProviderMapper(providers[i], nameProvider(i), order: order)); } var libraryOptions = new LibraryOptions(); @@ -278,7 +271,6 @@ namespace Jellyfin.Providers.Tests.Manager var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, baseItemManager: baseItemManager.Object); AddParts(providerManager, metadataProviders: providerList); - // TODO why does this take libraryOptions directly while GetImageProviders did not? var actualProviders = providerManager.GetMetadataProviders(item, libraryOptions).ToList(); Assert.Equal(providerList.Count, actualProviders.Count); -- cgit v1.2.3 From d5e2c2fb5e8a1328a2e938a7100a1ceb29b28fa7 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Fri, 10 Dec 2021 00:38:41 +0100 Subject: Implement CanRefreshMetadata tests for GetMetadataProviders Cleanup tests, extract common blocks --- .../Manager/ProviderManagerTests.cs | 285 ++++++++++++++------- 1 file changed, 196 insertions(+), 89 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index d59e4070f9..ba91f5ed2d 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -57,33 +57,10 @@ namespace Jellyfin.Providers.Tests.Manager providerList.Add(MockIImageProvider(nameProvider(i), item, order: order)); } - var libraryOptions = new LibraryOptions(); - if (libraryOrder != null) - { - libraryOptions.TypeOptions = new[] - { - new TypeOptions - { - Type = item.GetType().Name, - ImageFetcherOrder = libraryOrder.Select(nameProvider).ToArray() - } - }; - } + var libraryOptions = CreateLibraryOptions(item.GetType().Name, imageFetcherOrder: libraryOrder?.Select(nameProvider).ToArray()); + var serverConfiguration = CreateServerConfiguration(item.GetType().Name, imageFetcherOrder: serverOrder?.Select(nameProvider).ToArray()); - var serverConfiguration = new ServerConfiguration(); - if (serverOrder != null) - { - serverConfiguration.MetadataOptions = new[] - { - new MetadataOptions - { - ItemType = item.GetType().Name, - ImageFetcherOrder = serverOrder.Select(nameProvider).ToArray() - } - }; - } - - var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, libraryOptions: libraryOptions); + using var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, libraryOptions: libraryOptions); AddParts(providerManager, imageProviders: providerList); var refreshOptions = new ImageRefreshOptions(Mock.Of(MockBehavior.Strict)); @@ -119,12 +96,19 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(typeof(IDynamicImageProvider), true, true)] [InlineData(typeof(IRemoteImageProvider), false, false)] [InlineData(typeof(IDynamicImageProvider), false, false)] - public void GetImageProviders_CanRefreshImagesEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) + public void GetImageProviders_CanRefreshImagesBaseItemEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) { GetImageProviders_CanRefreshImages_Tester(providerType, true, expected, baseItemEnabled: enabled); } - private static void GetImageProviders_CanRefreshImages_Tester(Type providerType, bool supports, bool expected, bool errorOnSupported = false, bool itemLocked = false, bool fullRefresh = false, bool baseItemEnabled = true) + private static void GetImageProviders_CanRefreshImages_Tester( + Type providerType, + bool supports, + bool expected, + bool errorOnSupported = false, + bool itemLocked = false, + bool fullRefresh = false, + bool baseItemEnabled = true) { var item = new Movie { @@ -150,19 +134,12 @@ namespace Jellyfin.Providers.Tests.Manager baseItemManager.Setup(i => i.IsImageFetcherEnabled(item, It.IsAny(), providerName)) .Returns(baseItemEnabled); - var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); + using var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); AddParts(providerManager, imageProviders: new[] { provider }); - var actualProviders = providerManager.GetImageProviders(item, refreshOptions); + var actualProviders = providerManager.GetImageProviders(item, refreshOptions).ToArray(); - if (expected) - { - Assert.Single(actualProviders); - } - else - { - Assert.Empty(actualProviders); - } + Assert.Equal(expected ? 1 : 0, actualProviders.Length); } private static TheoryData GetMetadataProvidersOrderData() @@ -212,7 +189,14 @@ namespace Jellyfin.Providers.Tests.Manager [Theory] [MemberData(nameof(GetMetadataProvidersOrderData))] - public void GetMetadataProviders_ProviderOrder_MatchesExpected(string[] providers, int[]? libraryLocalOrder, int[]? libraryRemoteOrder, int[]? serverLocalOrder, int[]? serverRemoteOrder, int?[]? hasOrderOrder, int[] expectedOrder) + public void GetMetadataProviders_ProviderOrder_MatchesExpected( + string[] providers, + int[]? libraryLocalOrder, + int[]? libraryRemoteOrder, + int[]? serverLocalOrder, + int[]? serverRemoteOrder, + int?[]? hasOrderOrder, + int[] expectedOrder) { var item = new MetadataTestItem(); @@ -225,50 +209,20 @@ namespace Jellyfin.Providers.Tests.Manager providerList.Add(MockIMetadataProviderMapper(providers[i], nameProvider(i), order: order)); } - var libraryOptions = new LibraryOptions(); - if (libraryLocalOrder != null) - { - libraryOptions.LocalMetadataReaderOrder = libraryLocalOrder.Select(nameProvider).ToArray(); - } - - if (libraryRemoteOrder != null) - { - libraryOptions.TypeOptions = new[] - { - new TypeOptions - { - Type = item.GetType().Name, - MetadataFetcherOrder = libraryRemoteOrder.Select(nameProvider).ToArray() - } - }; - } - - var serverConfiguration = new ServerConfiguration(); - if (serverLocalOrder != null || serverRemoteOrder != null) - { - serverConfiguration.MetadataOptions = new[] - { - new MetadataOptions - { - ItemType = item.GetType().Name - } - }; - if (serverLocalOrder != null) - { - serverConfiguration.MetadataOptions[0].LocalMetadataReaderOrder = serverLocalOrder.Select(nameProvider).ToArray(); - } - - if (serverRemoteOrder != null) - { - serverConfiguration.MetadataOptions[0].MetadataFetcherOrder = serverRemoteOrder.Select(nameProvider).ToArray(); - } - } + var libraryOptions = CreateLibraryOptions( + item.GetType().Name, + localMetadataReaderOrder: libraryLocalOrder?.Select(nameProvider).ToArray(), + metadataFetcherOrder: libraryRemoteOrder?.Select(nameProvider).ToArray()); + var serverConfiguration = CreateServerConfiguration( + item.GetType().Name, + localMetadataReaderOrder: serverLocalOrder?.Select(nameProvider).ToArray(), + metadataFetcherOrder: serverRemoteOrder?.Select(nameProvider).ToArray()); var baseItemManager = new Mock(MockBehavior.Strict); baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), It.IsAny())) .Returns(true); - var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, baseItemManager: baseItemManager.Object); + using var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, baseItemManager: baseItemManager.Object); AddParts(providerManager, metadataProviders: providerList); var actualProviders = providerManager.GetMetadataProviders(item, libraryOptions).ToList(); @@ -278,6 +232,87 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal(expectedOrder, actualOrder); } + [Theory] + [InlineData(typeof(IMetadataProvider))] + [InlineData(typeof(ILocalMetadataProvider))] + [InlineData(typeof(IRemoteMetadataProvider))] + [InlineData(typeof(ICustomMetadataProvider))] + public void GetMetadataProviders_CanRefreshMetadataBasic_ReturnsTrue(Type providerType) + { + GetMetadataProviders_CanRefreshMetadata_Tester(providerType, true); + } + + [Theory] + [InlineData(typeof(ILocalMetadataProvider), false, true)] + [InlineData(typeof(IRemoteMetadataProvider), false, false)] + [InlineData(typeof(ICustomMetadataProvider), false, false)] + [InlineData(typeof(ILocalMetadataProvider), true, true)] + [InlineData(typeof(ICustomMetadataProvider), true, false)] + public void GetMetadataProviders_CanRefreshMetadataLocked_WhenLocalOrForced(Type providerType, bool forced, bool expected) + { + GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, itemLocked: true, providerForced: forced); + } + + [Theory] + [InlineData(typeof(ILocalMetadataProvider), false, true)] + [InlineData(typeof(ICustomMetadataProvider), false, true)] + [InlineData(typeof(IRemoteMetadataProvider), false, false)] + [InlineData(typeof(IRemoteMetadataProvider), true, true)] + public void GetMetadataProviders_CanRefreshMetadataBaseItemEnabled_WhenEnabledOrNotRemote(Type providerType, bool baseItemEnabled, bool expected) + { + GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, baseItemEnabled: baseItemEnabled); + } + + [Theory] + [InlineData(typeof(IRemoteMetadataProvider), false, true)] + [InlineData(typeof(ICustomMetadataProvider), false, true)] + [InlineData(typeof(ILocalMetadataProvider), false, false)] + [InlineData(typeof(ILocalMetadataProvider), true, true)] + public void GetMetadataProviders_CanRefreshMetadataSupportsLocal_WhenSupportsOrNotLocal(Type providerType, bool supportsLocalMetadata, bool expected) + { + GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, supportsLocalMetadata: supportsLocalMetadata); + } + + [Theory] + [InlineData(typeof(ICustomMetadataProvider), true)] + [InlineData(typeof(IRemoteMetadataProvider), false)] + [InlineData(typeof(ILocalMetadataProvider), false)] + public void GetMetadataProviders_CanRefreshMetadataOwned_WhenNotLocal(Type providerType, bool expected) + { + GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, ownedItem: true); + } + + private static void GetMetadataProviders_CanRefreshMetadata_Tester( + Type providerType, + bool expected, + bool itemLocked = false, + bool baseItemEnabled = true, + bool providerForced = false, + bool supportsLocalMetadata = true, + bool ownedItem = false) + { + var item = new MetadataTestItem + { + IsLocked = itemLocked, + OwnerId = ownedItem ? Guid.NewGuid() : Guid.Empty, + EnableLocalMetadata = supportsLocalMetadata + }; + + var providerName = "provider"; + var provider = MockIMetadataProviderMapper(providerType.Name, providerName, forced: providerForced); + + var baseItemManager = new Mock(MockBehavior.Strict); + baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), providerName)) + .Returns(baseItemEnabled); + + using var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); + AddParts(providerManager, metadataProviders: new[] { provider }); + + var actualProviders = providerManager.GetMetadataProviders(item, new LibraryOptions()).ToArray(); + + Assert.Equal(expected ? 1 : 0, actualProviders.Length); + } + private static IImageProvider MockIImageProvider(string name, BaseItem expectedType, bool supports = true, int? order = null, bool errorOnSupported = false) where TProviderType : class, IImageProvider { @@ -297,7 +332,7 @@ namespace Jellyfin.Providers.Tests.Manager if (errorOnSupported) { provider.Setup(p => p.Supports(It.IsAny())) - .Throws(new ArgumentException()); + .Throws(new ArgumentException("Provider threw exception on Supports(item)")); } else { @@ -308,25 +343,31 @@ namespace Jellyfin.Providers.Tests.Manager return provider.Object; } - private static IMetadataProvider MockIMetadataProviderMapper(string typeName, string providerName, int? order = null) + private static IMetadataProvider MockIMetadataProviderMapper(string typeName, string providerName, int? order = null, bool forced = false) where TItemType : BaseItem, IHasLookupInfo where TLookupInfoType : ItemLookupInfo, new() => typeName switch { - "ILocalMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), - "IRemoteMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), - "ICustomMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order), - _ => MockIMetadataProvider, TItemType>(providerName, order) + "ILocalMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order, forced), + "IRemoteMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order, forced), + "ICustomMetadataProvider" => MockIMetadataProvider, TItemType>(providerName, order, forced), + _ => MockIMetadataProvider, TItemType>(providerName, order, forced) }; - private static IMetadataProvider MockIMetadataProvider(string name, int? order = null) + private static IMetadataProvider MockIMetadataProvider(string name, int? order = null, bool forced = false) where TProviderType : class, IMetadataProvider where TItemType : BaseItem { + Mock? forcedProvider = null; + if (forced) + { + forcedProvider = new Mock(); + } + Mock? hasOrder = null; if (order != null) { - hasOrder = new Mock(MockBehavior.Strict); + hasOrder = forcedProvider == null ? new Mock() : forcedProvider.As(); hasOrder.Setup(i => i.Order) .Returns((int)order); } @@ -340,7 +381,71 @@ namespace Jellyfin.Providers.Tests.Manager return provider.Object; } - private static ProviderManager GetProviderManager(ServerConfiguration? serverConfiguration = null, LibraryOptions? libraryOptions = null, IBaseItemManager? baseItemManager = null) + private static LibraryOptions CreateLibraryOptions( + string typeName, + string[]? imageFetcherOrder = null, + string[]? localMetadataReaderOrder = null, + string[]? metadataFetcherOrder = null) + { + var libraryOptions = new LibraryOptions + { + LocalMetadataReaderOrder = localMetadataReaderOrder + }; + + // only create type options if populating it with something + if (imageFetcherOrder != null || metadataFetcherOrder != null) + { + imageFetcherOrder ??= Array.Empty(); + metadataFetcherOrder ??= Array.Empty(); + + libraryOptions.TypeOptions = new[] + { + new TypeOptions + { + Type = typeName, + ImageFetcherOrder = imageFetcherOrder, + MetadataFetcherOrder = metadataFetcherOrder + } + }; + } + + return libraryOptions; + } + + private static ServerConfiguration CreateServerConfiguration( + string typeName, + string[]? imageFetcherOrder = null, + string[]? localMetadataReaderOrder = null, + string[]? metadataFetcherOrder = null) + { + var serverConfiguration = new ServerConfiguration(); + + // only create type options if populating it with something + if (imageFetcherOrder != null || localMetadataReaderOrder != null || metadataFetcherOrder != null) + { + imageFetcherOrder ??= Array.Empty(); + localMetadataReaderOrder ??= Array.Empty(); + metadataFetcherOrder ??= Array.Empty(); + + serverConfiguration.MetadataOptions = new[] + { + new MetadataOptions + { + ItemType = typeName, + ImageFetcherOrder = imageFetcherOrder, + LocalMetadataReaderOrder = localMetadataReaderOrder, + MetadataFetcherOrder = metadataFetcherOrder + } + }; + } + + return serverConfiguration; + } + + private static ProviderManager GetProviderManager( + ServerConfiguration? serverConfiguration = null, + LibraryOptions? libraryOptions = null, + IBaseItemManager? baseItemManager = null) { var serverConfigurationManager = new Mock(MockBehavior.Strict); serverConfigurationManager.Setup(i => i.Configuration) @@ -382,11 +487,13 @@ namespace Jellyfin.Providers.Tests.Manager } /// - /// Simple extension to force SupportsLocalMetadata to true. + /// Simple extension to make SupportsLocalMetadata directly settable. /// public class MetadataTestItem : BaseItem, IHasLookupInfo { - public override bool SupportsLocalMetadata => true; + public bool EnableLocalMetadata { get; set; } = true; + + public override bool SupportsLocalMetadata => EnableLocalMetadata; public MetadataTestItemInfo GetLookupInfo() { -- cgit v1.2.3 From a7c009e2eb3e21b7b5c07984866419bb8136423f Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sat, 18 Dec 2021 21:40:27 +0100 Subject: Pass TypeOptions instead of full LibraryOptions --- .../BaseItemManager/BaseItemManager.cs | 14 ++++------ .../BaseItemManager/IBaseItemManager.cs | 8 +++--- MediaBrowser.Providers/Manager/ProviderManager.cs | 12 ++++---- .../BaseItemManagerTests.cs | 32 ++++++++-------------- .../Manager/ProviderManagerTests.cs | 6 ++-- 5 files changed, 31 insertions(+), 41 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Controller/BaseItemManager/BaseItemManager.cs b/MediaBrowser.Controller/BaseItemManager/BaseItemManager.cs index d273b54fc6..61539cae56 100644 --- a/MediaBrowser.Controller/BaseItemManager/BaseItemManager.cs +++ b/MediaBrowser.Controller/BaseItemManager/BaseItemManager.cs @@ -35,7 +35,7 @@ namespace MediaBrowser.Controller.BaseItemManager public SemaphoreSlim MetadataRefreshThrottler { get; private set; } /// - public bool IsMetadataFetcherEnabled(BaseItem baseItem, LibraryOptions libraryOptions, string name) + public bool IsMetadataFetcherEnabled(BaseItem baseItem, TypeOptions? libraryTypeOptions, string name) { if (baseItem is Channel) { @@ -49,10 +49,9 @@ namespace MediaBrowser.Controller.BaseItemManager return !baseItem.EnableMediaSourceDisplay; } - var typeOptions = libraryOptions.GetTypeOptions(baseItem.GetType().Name); - if (typeOptions != null) + if (libraryTypeOptions != null) { - return typeOptions.MetadataFetchers.Contains(name.AsSpan(), StringComparison.OrdinalIgnoreCase); + return libraryTypeOptions.MetadataFetchers.Contains(name.AsSpan(), StringComparison.OrdinalIgnoreCase); } var itemConfig = _serverConfigurationManager.Configuration.MetadataOptions.FirstOrDefault(i => string.Equals(i.ItemType, baseItem.GetType().Name, StringComparison.OrdinalIgnoreCase)); @@ -61,7 +60,7 @@ namespace MediaBrowser.Controller.BaseItemManager } /// - public bool IsImageFetcherEnabled(BaseItem baseItem, LibraryOptions libraryOptions, string name) + public bool IsImageFetcherEnabled(BaseItem baseItem, TypeOptions? libraryTypeOptions, string name) { if (baseItem is Channel) { @@ -75,10 +74,9 @@ namespace MediaBrowser.Controller.BaseItemManager return !baseItem.EnableMediaSourceDisplay; } - var typeOptions = libraryOptions.GetTypeOptions(baseItem.GetType().Name); - if (typeOptions != null) + if (libraryTypeOptions != null) { - return typeOptions.ImageFetchers.Contains(name.AsSpan(), StringComparison.OrdinalIgnoreCase); + return libraryTypeOptions.ImageFetchers.Contains(name.AsSpan(), StringComparison.OrdinalIgnoreCase); } var itemConfig = _serverConfigurationManager.Configuration.MetadataOptions.FirstOrDefault(i => string.Equals(i.ItemType, baseItem.GetType().Name, StringComparison.OrdinalIgnoreCase)); diff --git a/MediaBrowser.Controller/BaseItemManager/IBaseItemManager.cs b/MediaBrowser.Controller/BaseItemManager/IBaseItemManager.cs index e18994214e..b07c80879d 100644 --- a/MediaBrowser.Controller/BaseItemManager/IBaseItemManager.cs +++ b/MediaBrowser.Controller/BaseItemManager/IBaseItemManager.cs @@ -18,18 +18,18 @@ namespace MediaBrowser.Controller.BaseItemManager /// Is metadata fetcher enabled. /// /// The base item. - /// The library options. + /// The type options for baseItem from the library (if defined). /// The metadata fetcher name. /// true if metadata fetcher is enabled, else false. - bool IsMetadataFetcherEnabled(BaseItem baseItem, LibraryOptions libraryOptions, string name); + bool IsMetadataFetcherEnabled(BaseItem baseItem, TypeOptions? libraryTypeOptions, string name); /// /// Is image fetcher enabled. /// /// The base item. - /// The library options. + /// The type options for baseItem from the library (if defined). /// The image fetcher name. /// true if image fetcher is enabled, else false. - bool IsImageFetcherEnabled(BaseItem baseItem, LibraryOptions libraryOptions, string name); + bool IsImageFetcherEnabled(BaseItem baseItem, TypeOptions? libraryTypeOptions, string name); } } diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 82d633e234..d1a5831f98 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -321,7 +321,7 @@ namespace MediaBrowser.Providers.Manager var typeOptions = libraryOptions.GetTypeOptions(item.GetType().Name); var fetcherOrder = typeOptions?.ImageFetcherOrder ?? options.ImageFetcherOrder; - return _imageProviders.Where(i => CanRefreshImages(i, item, libraryOptions, refreshOptions, includeDisabled)) + return _imageProviders.Where(i => CanRefreshImages(i, item, typeOptions, refreshOptions, includeDisabled)) .OrderBy(i => GetConfiguredOrder(fetcherOrder, i.Name)) .ThenBy(GetDefaultOrder); } @@ -343,7 +343,7 @@ namespace MediaBrowser.Providers.Manager var metadataFetcherOrder = typeOptions?.MetadataFetcherOrder ?? globalMetadataOptions.MetadataFetcherOrder; return _metadataProviders.OfType>() - .Where(i => CanRefreshMetadata(i, item, libraryOptions, includeDisabled, forceEnableInternetMetadata)) + .Where(i => CanRefreshMetadata(i, item, typeOptions, includeDisabled, forceEnableInternetMetadata)) .OrderBy(i => { // local and remote providers will be interleaved in the final order @@ -361,7 +361,7 @@ namespace MediaBrowser.Providers.Manager private bool CanRefreshMetadata( IMetadataProvider provider, BaseItem item, - LibraryOptions libraryOptions, + TypeOptions? libraryTypeOptions, bool includeDisabled, bool forceEnableInternetMetadata) { @@ -375,7 +375,7 @@ namespace MediaBrowser.Providers.Manager if (provider is IRemoteMetadataProvider) { - if (!forceEnableInternetMetadata && !_baseItemManager.IsMetadataFetcherEnabled(item, libraryOptions, provider.Name)) + if (!forceEnableInternetMetadata && !_baseItemManager.IsMetadataFetcherEnabled(item, libraryTypeOptions, provider.Name)) { return false; } @@ -402,7 +402,7 @@ namespace MediaBrowser.Providers.Manager private bool CanRefreshImages( IImageProvider provider, BaseItem item, - LibraryOptions libraryOptions, + TypeOptions? libraryTypeOptions, ImageRefreshOptions refreshOptions, bool includeDisabled) { @@ -419,7 +419,7 @@ namespace MediaBrowser.Providers.Manager if (provider is IRemoteImageProvider || provider is IDynamicImageProvider) { - if (!_baseItemManager.IsImageFetcherEnabled(item, libraryOptions, provider.Name)) + if (!_baseItemManager.IsImageFetcherEnabled(item, libraryTypeOptions, provider.Name)) { return false; } diff --git a/tests/Jellyfin.Controller.Tests/BaseItemManagerTests.cs b/tests/Jellyfin.Controller.Tests/BaseItemManagerTests.cs index 463e17ad36..f67e6d1ef0 100644 --- a/tests/Jellyfin.Controller.Tests/BaseItemManagerTests.cs +++ b/tests/Jellyfin.Controller.Tests/BaseItemManagerTests.cs @@ -20,17 +20,13 @@ namespace Jellyfin.Controller.Tests { BaseItem item = (BaseItem)Activator.CreateInstance(itemType)!; - var libraryOptions = new LibraryOptions - { - TypeOptions = new[] + var libraryTypeOptions = itemType == typeof(Book) + ? new TypeOptions { - new TypeOptions - { - Type = "Book", - MetadataFetchers = new[] { "LibraryEnabled" } - } + Type = "Book", + MetadataFetchers = new[] { "LibraryEnabled" } } - }; + : null; var serverConfiguration = new ServerConfiguration(); foreach (var typeConfig in serverConfiguration.MetadataOptions) @@ -43,7 +39,7 @@ namespace Jellyfin.Controller.Tests .Returns(serverConfiguration); var baseItemManager = new BaseItemManager(serverConfigurationManager.Object); - var actual = baseItemManager.IsMetadataFetcherEnabled(item, libraryOptions, fetcherName); + var actual = baseItemManager.IsMetadataFetcherEnabled(item, libraryTypeOptions, fetcherName); Assert.Equal(expected, actual); } @@ -57,17 +53,13 @@ namespace Jellyfin.Controller.Tests { BaseItem item = (BaseItem)Activator.CreateInstance(itemType)!; - var libraryOptions = new LibraryOptions - { - TypeOptions = new[] + var libraryTypeOptions = itemType == typeof(Book) + ? new TypeOptions { - new TypeOptions - { - Type = "Book", - ImageFetchers = new[] { "LibraryEnabled" } - } + Type = "Book", + ImageFetchers = new[] { "LibraryEnabled" } } - }; + : null; var serverConfiguration = new ServerConfiguration(); foreach (var typeConfig in serverConfiguration.MetadataOptions) @@ -80,7 +72,7 @@ namespace Jellyfin.Controller.Tests .Returns(serverConfiguration); var baseItemManager = new BaseItemManager(serverConfigurationManager.Object); - var actual = baseItemManager.IsImageFetcherEnabled(item, libraryOptions, fetcherName); + var actual = baseItemManager.IsImageFetcherEnabled(item, libraryTypeOptions, fetcherName); Assert.Equal(expected, actual); } diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index ba91f5ed2d..560b50f091 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -131,7 +131,7 @@ namespace Jellyfin.Providers.Tests.Manager }; var baseItemManager = new Mock(MockBehavior.Strict); - baseItemManager.Setup(i => i.IsImageFetcherEnabled(item, It.IsAny(), providerName)) + baseItemManager.Setup(i => i.IsImageFetcherEnabled(item, It.IsAny(), providerName)) .Returns(baseItemEnabled); using var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); @@ -219,7 +219,7 @@ namespace Jellyfin.Providers.Tests.Manager metadataFetcherOrder: serverRemoteOrder?.Select(nameProvider).ToArray()); var baseItemManager = new Mock(MockBehavior.Strict); - baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), It.IsAny())) + baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), It.IsAny())) .Returns(true); using var providerManager = GetProviderManager(serverConfiguration: serverConfiguration, baseItemManager: baseItemManager.Object); @@ -302,7 +302,7 @@ namespace Jellyfin.Providers.Tests.Manager var provider = MockIMetadataProviderMapper(providerType.Name, providerName, forced: providerForced); var baseItemManager = new Mock(MockBehavior.Strict); - baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), providerName)) + baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), providerName)) .Returns(baseItemEnabled); using var providerManager = GetProviderManager(baseItemManager: baseItemManager.Object); -- cgit v1.2.3 From 6ab64f4930f61f7f0d0968b88da687a95e0035ad Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sun, 19 Dec 2021 23:33:27 +0100 Subject: Switch to nameof to simplify theory signatures --- .../Manager/ProviderManagerTests.cs | 82 +++++++++++----------- 1 file changed, 41 insertions(+), 41 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 560b50f091..d76d411a78 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -77,32 +77,32 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(true, true, false)] public void GetImageProviders_CanRefreshImagesBasic_WhenSupportsWithoutError(bool supports, bool errorOnSupported, bool expected) { - GetImageProviders_CanRefreshImages_Tester(typeof(IImageProvider), supports, expected, errorOnSupported: errorOnSupported); + GetImageProviders_CanRefreshImages_Tester(nameof(IImageProvider), supports, expected, errorOnSupported: errorOnSupported); } [Theory] - [InlineData(typeof(ILocalImageProvider), false, true)] - [InlineData(typeof(ILocalImageProvider), true, true)] - [InlineData(typeof(IImageProvider), false, false)] - [InlineData(typeof(IImageProvider), true, true)] - public void GetImageProviders_CanRefreshImagesLocked_WhenLocalOrFullRefresh(Type providerType, bool fullRefresh, bool expected) + [InlineData(nameof(ILocalImageProvider), false, true)] + [InlineData(nameof(ILocalImageProvider), true, true)] + [InlineData(nameof(IImageProvider), false, false)] + [InlineData(nameof(IImageProvider), true, true)] + public void GetImageProviders_CanRefreshImagesLocked_WhenLocalOrFullRefresh(string providerType, bool fullRefresh, bool expected) { GetImageProviders_CanRefreshImages_Tester(providerType, true, expected, itemLocked: true, fullRefresh: fullRefresh); } [Theory] - [InlineData(typeof(ILocalImageProvider), false, true)] - [InlineData(typeof(IRemoteImageProvider), true, true)] - [InlineData(typeof(IDynamicImageProvider), true, true)] - [InlineData(typeof(IRemoteImageProvider), false, false)] - [InlineData(typeof(IDynamicImageProvider), false, false)] - public void GetImageProviders_CanRefreshImagesBaseItemEnabled_WhenLocalOrEnabled(Type providerType, bool enabled, bool expected) + [InlineData(nameof(ILocalImageProvider), false, true)] + [InlineData(nameof(IRemoteImageProvider), true, true)] + [InlineData(nameof(IDynamicImageProvider), true, true)] + [InlineData(nameof(IRemoteImageProvider), false, false)] + [InlineData(nameof(IDynamicImageProvider), false, false)] + public void GetImageProviders_CanRefreshImagesBaseItemEnabled_WhenLocalOrEnabled(string providerType, bool enabled, bool expected) { GetImageProviders_CanRefreshImages_Tester(providerType, true, expected, baseItemEnabled: enabled); } private static void GetImageProviders_CanRefreshImages_Tester( - Type providerType, + string providerType, bool supports, bool expected, bool errorOnSupported = false, @@ -116,7 +116,7 @@ namespace Jellyfin.Providers.Tests.Manager }; var providerName = "provider"; - IImageProvider provider = providerType.Name switch + IImageProvider provider = providerType switch { "IImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), "ILocalImageProvider" => MockIImageProvider(providerName, item, supports: supports, errorOnSupported: errorOnSupported), @@ -233,57 +233,57 @@ namespace Jellyfin.Providers.Tests.Manager } [Theory] - [InlineData(typeof(IMetadataProvider))] - [InlineData(typeof(ILocalMetadataProvider))] - [InlineData(typeof(IRemoteMetadataProvider))] - [InlineData(typeof(ICustomMetadataProvider))] - public void GetMetadataProviders_CanRefreshMetadataBasic_ReturnsTrue(Type providerType) + [InlineData(nameof(IMetadataProvider))] + [InlineData(nameof(ILocalMetadataProvider))] + [InlineData(nameof(IRemoteMetadataProvider))] + [InlineData(nameof(ICustomMetadataProvider))] + public void GetMetadataProviders_CanRefreshMetadataBasic_ReturnsTrue(string providerType) { GetMetadataProviders_CanRefreshMetadata_Tester(providerType, true); } [Theory] - [InlineData(typeof(ILocalMetadataProvider), false, true)] - [InlineData(typeof(IRemoteMetadataProvider), false, false)] - [InlineData(typeof(ICustomMetadataProvider), false, false)] - [InlineData(typeof(ILocalMetadataProvider), true, true)] - [InlineData(typeof(ICustomMetadataProvider), true, false)] - public void GetMetadataProviders_CanRefreshMetadataLocked_WhenLocalOrForced(Type providerType, bool forced, bool expected) + [InlineData(nameof(ILocalMetadataProvider), false, true)] + [InlineData(nameof(IRemoteMetadataProvider), false, false)] + [InlineData(nameof(ICustomMetadataProvider), false, false)] + [InlineData(nameof(ILocalMetadataProvider), true, true)] + [InlineData(nameof(ICustomMetadataProvider), true, false)] + public void GetMetadataProviders_CanRefreshMetadataLocked_WhenLocalOrForced(string providerType, bool forced, bool expected) { GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, itemLocked: true, providerForced: forced); } [Theory] - [InlineData(typeof(ILocalMetadataProvider), false, true)] - [InlineData(typeof(ICustomMetadataProvider), false, true)] - [InlineData(typeof(IRemoteMetadataProvider), false, false)] - [InlineData(typeof(IRemoteMetadataProvider), true, true)] - public void GetMetadataProviders_CanRefreshMetadataBaseItemEnabled_WhenEnabledOrNotRemote(Type providerType, bool baseItemEnabled, bool expected) + [InlineData(nameof(ILocalMetadataProvider), false, true)] + [InlineData(nameof(ICustomMetadataProvider), false, true)] + [InlineData(nameof(IRemoteMetadataProvider), false, false)] + [InlineData(nameof(IRemoteMetadataProvider), true, true)] + public void GetMetadataProviders_CanRefreshMetadataBaseItemEnabled_WhenEnabledOrNotRemote(string providerType, bool baseItemEnabled, bool expected) { GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, baseItemEnabled: baseItemEnabled); } [Theory] - [InlineData(typeof(IRemoteMetadataProvider), false, true)] - [InlineData(typeof(ICustomMetadataProvider), false, true)] - [InlineData(typeof(ILocalMetadataProvider), false, false)] - [InlineData(typeof(ILocalMetadataProvider), true, true)] - public void GetMetadataProviders_CanRefreshMetadataSupportsLocal_WhenSupportsOrNotLocal(Type providerType, bool supportsLocalMetadata, bool expected) + [InlineData(nameof(IRemoteMetadataProvider), false, true)] + [InlineData(nameof(ICustomMetadataProvider), false, true)] + [InlineData(nameof(ILocalMetadataProvider), false, false)] + [InlineData(nameof(ILocalMetadataProvider), true, true)] + public void GetMetadataProviders_CanRefreshMetadataSupportsLocal_WhenSupportsOrNotLocal(string providerType, bool supportsLocalMetadata, bool expected) { GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, supportsLocalMetadata: supportsLocalMetadata); } [Theory] - [InlineData(typeof(ICustomMetadataProvider), true)] - [InlineData(typeof(IRemoteMetadataProvider), false)] - [InlineData(typeof(ILocalMetadataProvider), false)] - public void GetMetadataProviders_CanRefreshMetadataOwned_WhenNotLocal(Type providerType, bool expected) + [InlineData(nameof(ICustomMetadataProvider), true)] + [InlineData(nameof(IRemoteMetadataProvider), false)] + [InlineData(nameof(ILocalMetadataProvider), false)] + public void GetMetadataProviders_CanRefreshMetadataOwned_WhenNotLocal(string providerType, bool expected) { GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, ownedItem: true); } private static void GetMetadataProviders_CanRefreshMetadata_Tester( - Type providerType, + string providerType, bool expected, bool itemLocked = false, bool baseItemEnabled = true, @@ -299,7 +299,7 @@ namespace Jellyfin.Providers.Tests.Manager }; var providerName = "provider"; - var provider = MockIMetadataProviderMapper(providerType.Name, providerName, forced: providerForced); + var provider = MockIMetadataProviderMapper(providerType, providerName, forced: providerForced); var baseItemManager = new Mock(MockBehavior.Strict); baseItemManager.Setup(i => i.IsMetadataFetcherEnabled(item, It.IsAny(), providerName)) -- cgit v1.2.3 From bdce435b09b88329dd7f23ff5f4e9bb7998763b5 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sat, 18 Dec 2021 22:30:06 +0100 Subject: Reorder and flatten provider filtering --- MediaBrowser.Providers/Manager/ProviderManager.cs | 101 ++++++++++----------- .../Manager/ProviderManagerTests.cs | 4 +- 2 files changed, 48 insertions(+), 57 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index d1a5831f98..e2882ee06e 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -326,6 +326,39 @@ namespace MediaBrowser.Providers.Manager .ThenBy(GetDefaultOrder); } + private bool CanRefreshImages( + IImageProvider provider, + BaseItem item, + TypeOptions? libraryTypeOptions, + ImageRefreshOptions refreshOptions, + bool includeDisabled) + { + try + { + if (!provider.Supports(item)) + { + return false; + } + } + catch (Exception ex) + { + _logger.LogError(ex, "{ProviderName} failed in Supports for type {ItemType} at {ItemPath}", provider.GetType().Name, item.GetType().Name, item.Path); + return false; + } + + if (includeDisabled || provider is ILocalImageProvider) + { + return true; + } + + if (item.IsLocked && refreshOptions.ImageRefreshMode != MetadataRefreshMode.FullRefresh) + { + return false; + } + + return _baseItemManager.IsImageFetcherEnabled(item, libraryTypeOptions, provider.Name); + } + /// public IEnumerable> GetMetadataProviders(BaseItem item, LibraryOptions libraryOptions) where T : BaseItem @@ -365,76 +398,34 @@ namespace MediaBrowser.Providers.Manager bool includeDisabled, bool forceEnableInternetMetadata) { - if (!includeDisabled) - { - // If locked only allow local providers - if (item.IsLocked && provider is not ILocalMetadataProvider && provider is not IForcedProvider) - { - return false; - } - - if (provider is IRemoteMetadataProvider) - { - if (!forceEnableInternetMetadata && !_baseItemManager.IsMetadataFetcherEnabled(item, libraryTypeOptions, provider.Name)) - { - return false; - } - } - } - if (!item.SupportsLocalMetadata && provider is ILocalMetadataProvider) { return false; } - // If this restriction is ever lifted, movie xml providers will have to be updated to prevent owned items like trailers from reading those files - if (!item.OwnerId.Equals(default)) + // Prevent owned items from reading the same local metadata file as their owner + if (!item.OwnerId.Equals(default) && provider is ILocalMetadataProvider) { - if (provider is ILocalMetadataProvider || provider is IRemoteMetadataProvider) - { - return false; - } + return false; } - return true; - } - - private bool CanRefreshImages( - IImageProvider provider, - BaseItem item, - TypeOptions? libraryTypeOptions, - ImageRefreshOptions refreshOptions, - bool includeDisabled) - { - if (!includeDisabled) + if (includeDisabled) { - // If locked only allow local providers - if (item.IsLocked && provider is not ILocalImageProvider) - { - if (refreshOptions.ImageRefreshMode != MetadataRefreshMode.FullRefresh) - { - return false; - } - } - - if (provider is IRemoteImageProvider || provider is IDynamicImageProvider) - { - if (!_baseItemManager.IsImageFetcherEnabled(item, libraryTypeOptions, provider.Name)) - { - return false; - } - } + return true; } - try + // If locked only allow local providers + if (item.IsLocked && provider is not ILocalMetadataProvider && provider is not IForcedProvider) { - return provider.Supports(item); + return false; } - catch (Exception ex) + + if (forceEnableInternetMetadata || provider is not IRemoteMetadataProvider) { - _logger.LogError(ex, "{ProviderName} failed in Supports for type {ItemType} at {ItemPath}", provider.GetType().Name, item.GetType().Name, item.Path); - return false; + return true; } + + return _baseItemManager.IsMetadataFetcherEnabled(item, libraryTypeOptions, provider.Name); } private static int GetConfiguredOrder(string[] order, string providerName) diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index d76d411a78..8100dcfa6f 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -54,7 +54,7 @@ namespace Jellyfin.Providers.Tests.Manager for (var i = 0; i < providerCount; i++) { var order = hasOrderOrder?[i]; - providerList.Add(MockIImageProvider(nameProvider(i), item, order: order)); + providerList.Add(MockIImageProvider(nameProvider(i), item, order: order)); } var libraryOptions = CreateLibraryOptions(item.GetType().Name, imageFetcherOrder: libraryOrder?.Select(nameProvider).ToArray()); @@ -275,7 +275,7 @@ namespace Jellyfin.Providers.Tests.Manager [Theory] [InlineData(nameof(ICustomMetadataProvider), true)] - [InlineData(nameof(IRemoteMetadataProvider), false)] + [InlineData(nameof(IRemoteMetadataProvider), true)] [InlineData(nameof(ILocalMetadataProvider), false)] public void GetMetadataProviders_CanRefreshMetadataOwned_WhenNotLocal(string providerType, bool expected) { -- cgit v1.2.3 From ee5bd0daa62e68f4f93e7603017c1f028781e387 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Tue, 21 Dec 2021 00:24:07 +0100 Subject: Implement tests on ProviderManager.RefreshSingleItem --- .../Providers/IMetadataService.cs | 5 + .../Manager/ProviderManagerTests.cs | 110 ++++++++++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Controller/Providers/IMetadataService.cs b/MediaBrowser.Controller/Providers/IMetadataService.cs index 05fbb18ee4..f0f1d18624 100644 --- a/MediaBrowser.Controller/Providers/IMetadataService.cs +++ b/MediaBrowser.Controller/Providers/IMetadataService.cs @@ -23,6 +23,11 @@ namespace MediaBrowser.Controller.Providers /// true if this instance can refresh the specified item. bool CanRefresh(BaseItem item); + /// + /// Determines whether this instance primarily targets the specified type. + /// + /// The type. + /// true if this instance primarily targets the specified type. bool CanRefreshPrimary(Type type); /// diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 8100dcfa6f..5845b31be8 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using MediaBrowser.Controller.BaseItemManager; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; @@ -9,6 +11,7 @@ using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Configuration; using MediaBrowser.Providers.Manager; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; using Xunit; @@ -17,6 +20,95 @@ namespace Jellyfin.Providers.Tests.Manager { public class ProviderManagerTests { + private static readonly ILogger _logger = new NullLogger(); + + private static TheoryData[], int> RefreshSingleItemOrderData() + => new () + { + // no order set, uses provided order + { + new[] + { + MockIMetadataService(true, true), + MockIMetadataService(true, true) + }, + 0 + }, + // sort order sets priority when all match + { + new[] + { + MockIMetadataService(true, true, 1), + MockIMetadataService(true, true, 0), + MockIMetadataService(true, true, 2) + }, + 1 + }, + // CanRefreshPrimary prioritized + { + new[] + { + MockIMetadataService(false, true), + MockIMetadataService(true, true), + }, + 1 + }, + // falls back to CanRefresh + { + new[] + { + MockIMetadataService(false, false), + MockIMetadataService(false, true) + }, + 1 + }, + }; + + [Theory] + [MemberData(nameof(RefreshSingleItemOrderData))] + public void RefreshSingleItem_ServiceOrdering_FollowsPriority(Mock[] servicesList, int expectedIndex) + { + var item = new Movie(); + + using var providerManager = GetProviderManager(); + AddParts(providerManager, metadataServices: servicesList.Select(s => s.Object).ToArray()); + + var refreshOptions = new MetadataRefreshOptions(Mock.Of(MockBehavior.Strict)); + var actual = providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None); + + Assert.Equal(ItemUpdateType.MetadataDownload, actual.Result); + for (var i = 0; i < servicesList.Length; i++) + { + if (i == expectedIndex) + { + servicesList[i].Verify(mock => mock.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + } + else + { + servicesList[i].Verify(mock => mock.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + } + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void RefreshSingleItem_RefreshMetadata_WhenServiceFound(bool serviceFound) + { + var item = new Movie(); + + var servicesList = new[] { MockIMetadataService(false, serviceFound) }; + + using var providerManager = GetProviderManager(); + AddParts(providerManager, metadataServices: servicesList.Select(s => s.Object).ToArray()); + + var refreshOptions = new MetadataRefreshOptions(Mock.Of(MockBehavior.Strict)); + var actual = providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None); + + var expectedResult = serviceFound ? ItemUpdateType.MetadataDownload : ItemUpdateType.None; + Assert.Equal(expectedResult, actual.Result); + } + private static TheoryData GetImageProvidersOrderData() => new () { @@ -313,6 +405,20 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal(expected ? 1 : 0, actualProviders.Length); } + private static Mock MockIMetadataService(bool refreshPrimary, bool canRefresh, int order = 0) + { + var service = new Mock(MockBehavior.Strict); + service.Setup(s => s.Order) + .Returns(order); + service.Setup(s => s.CanRefreshPrimary(It.IsAny())) + .Returns(refreshPrimary); + service.Setup(s => s.CanRefresh(It.IsAny())) + .Returns(canRefresh); + service.Setup(s => s.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult(ItemUpdateType.MetadataDownload)); + return service; + } + private static IImageProvider MockIImageProvider(string name, BaseItem expectedType, bool supports = true, int? order = null, bool errorOnSupported = false) where TProviderType : class, IImageProvider { @@ -460,11 +566,11 @@ namespace Jellyfin.Providers.Tests.Manager null, serverConfigurationManager.Object, null, - new NullLogger(), + _logger, null, null, libraryManager.Object, - baseItemManager); + baseItemManager!); return providerManager; } -- cgit v1.2.3 From b03f56c3d6e005b615780bcdc676bcd632e80395 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sat, 1 Jan 2022 00:22:46 +0100 Subject: Remove warnings --- MediaBrowser.Providers/Manager/ProviderManager.cs | 26 +++++++++++-------- .../Manager/ProviderManagerTests.cs | 30 +++++++++++++--------- 2 files changed, 33 insertions(+), 23 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 9be4bc479b..eb4bc3587f 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -45,7 +45,7 @@ namespace MediaBrowser.Providers.Manager /// public class ProviderManager : IProviderManager, IDisposable { - private readonly object _refreshQueueLock = new (); + private readonly object _refreshQueueLock = new(); private readonly ILogger _logger; private readonly IHttpClientFactory _httpClientFactory; private readonly ILibraryMonitor _libraryMonitor; @@ -55,9 +55,9 @@ namespace MediaBrowser.Providers.Manager private readonly ISubtitleManager _subtitleManager; private readonly IServerConfigurationManager _configurationManager; private readonly IBaseItemManager _baseItemManager; - private readonly ConcurrentDictionary _activeRefreshes = new (); - private readonly CancellationTokenSource _disposeCancellationTokenSource = new (); - private readonly SimplePriorityQueue> _refreshQueue = new (); + private readonly ConcurrentDictionary _activeRefreshes = new(); + private readonly CancellationTokenSource _disposeCancellationTokenSource = new(); + private readonly SimplePriorityQueue> _refreshQueue = new(); private IImageProvider[] _imageProviders = Array.Empty(); private IMetadataService[] _metadataServices = Array.Empty(); @@ -164,6 +164,10 @@ namespace MediaBrowser.Providers.Manager { contentType = "image/png"; } + else + { + throw new HttpRequestException("Invalid image received: contentType not set.", null, response.StatusCode); + } } // thetvdb will sometimes serve a rubbish 404 html page with a 200 OK code, because reasons... @@ -589,7 +593,7 @@ namespace MediaBrowser.Providers.Manager foreach (var saver in savers.Where(i => IsSaverEnabledForItem(i, item, libraryOptions, updateType, false))) { - _logger.LogDebug("Saving {0} to {1}.", item.Path ?? item.Name, saver.Name); + _logger.LogDebug("Saving {Item} to {Saver}", item.Path ?? item.Name, saver.Name); if (saver is IMetadataFileSaver fileSaver) { @@ -601,7 +605,7 @@ namespace MediaBrowser.Providers.Manager } catch (Exception ex) { - _logger.LogError(ex, "Error in {0} GetSavePath", saver.Name); + _logger.LogError(ex, "Error in {Saver} GetSavePath", saver.Name); continue; } @@ -688,7 +692,7 @@ namespace MediaBrowser.Providers.Manager } catch (Exception ex) { - _logger.LogError(ex, "Error in {0}.IsEnabledFor", saver.Name); + _logger.LogError(ex, "Error in {Saver}.IsEnabledFor", saver.Name); return false; } } @@ -838,7 +842,7 @@ namespace MediaBrowser.Providers.Manager } catch (Exception ex) { - _logger.LogError(ex, "Error in {0}.Supports", i.GetType().Name); + _logger.LogError(ex, "Error in {Type}.Supports", i.GetType().Name); return false; } }); @@ -904,7 +908,7 @@ namespace MediaBrowser.Providers.Manager /// public void OnRefreshStart(BaseItem item) { - _logger.LogDebug("OnRefreshStart {0}", item.Id.ToString("N", CultureInfo.InvariantCulture)); + _logger.LogDebug("OnRefreshStart {Item}", item.Id.ToString("N", CultureInfo.InvariantCulture)); _activeRefreshes[item.Id] = 0; RefreshStarted?.Invoke(this, new GenericEventArgs(item)); } @@ -912,7 +916,7 @@ namespace MediaBrowser.Providers.Manager /// public void OnRefreshComplete(BaseItem item) { - _logger.LogDebug("OnRefreshComplete {0}", item.Id.ToString("N", CultureInfo.InvariantCulture)); + _logger.LogDebug("OnRefreshComplete {Item}", item.Id.ToString("N", CultureInfo.InvariantCulture)); _activeRefreshes.Remove(item.Id, out _); @@ -934,7 +938,7 @@ namespace MediaBrowser.Providers.Manager public void OnRefreshProgress(BaseItem item, double progress) { var id = item.Id; - _logger.LogDebug("OnRefreshProgress {0} {1}", id.ToString("N", CultureInfo.InvariantCulture), progress); + _logger.LogDebug("OnRefreshProgress {Id} {Progress}", id.ToString("N", CultureInfo.InvariantCulture), progress); // TODO: Need to hunt down the conditions for this happening _activeRefreshes.AddOrUpdate( diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 5845b31be8..6179e31359 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -1,21 +1,29 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net.Http; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +using MediaBrowser.Controller; using MediaBrowser.Controller.BaseItemManager; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Providers; +using MediaBrowser.Controller.Subtitles; using MediaBrowser.Model.Configuration; +using MediaBrowser.Model.IO; using MediaBrowser.Providers.Manager; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; using Xunit; +// Allow Moq to see internal class +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] + namespace Jellyfin.Providers.Tests.Manager { public class ProviderManagerTests @@ -23,7 +31,7 @@ namespace Jellyfin.Providers.Tests.Manager private static readonly ILogger _logger = new NullLogger(); private static TheoryData[], int> RefreshSingleItemOrderData() - => new () + => new() { // no order set, uses provided order { @@ -110,7 +118,7 @@ namespace Jellyfin.Providers.Tests.Manager } private static TheoryData GetImageProvidersOrderData() - => new () + => new() { { 3, null, null, null, new[] { 0, 1, 2 } }, // no order options set @@ -238,7 +246,7 @@ namespace Jellyfin.Providers.Tests.Manager { var l = nameof(ILocalMetadataProvider); var r = nameof(IRemoteMetadataProvider); - return new () + return new() { { new[] { l, l, r, r }, null, null, null, null, null, new[] { 0, 1, 2, 3 } }, // no order options set @@ -269,8 +277,6 @@ namespace Jellyfin.Providers.Tests.Manager // IHasOrder ordering (not interleaved, doesn't care about types) { new[] { l, l, r, r }, null, null, null, null, new int?[] { 2, null, 1, null }, new[] { 2, 0, 1, 3 } }, // partially defined { new[] { l, l, r, r }, null, null, null, null, new int?[] { 3, 2, 1, 0 }, new[] { 3, 2, 1, 0 } }, // full reverse order - // note odd interaction - orderby determines order of slot when local and remote both have a slot 0 - { new[] { l, l, r, r }, new[] { 1 }, new[] { 3 }, null, null, new int?[] { null, 2, null, 1 }, new[] { 3, 1, 0, 2 } }, // sorts interleaved results // multiple orders set { new[] { l, l, l, r, r, r }, new[] { 1 }, new[] { 4 }, new[] { 2, 1, 0 }, new[] { 5, 4, 3 }, null, new[] { 1, 4, 0, 2, 3, 5 } }, // partial library order first, server order ignored @@ -562,13 +568,13 @@ namespace Jellyfin.Providers.Tests.Manager .Returns(libraryOptions ?? new LibraryOptions()); var providerManager = new ProviderManager( - null, - null, + Mock.Of(), + Mock.Of(), serverConfigurationManager.Object, - null, + Mock.Of(), _logger, - null, - null, + Mock.Of(), + Mock.Of(), libraryManager.Object, baseItemManager!); @@ -595,7 +601,7 @@ namespace Jellyfin.Providers.Tests.Manager /// /// Simple extension to make SupportsLocalMetadata directly settable. /// - public class MetadataTestItem : BaseItem, IHasLookupInfo + internal class MetadataTestItem : BaseItem, IHasLookupInfo { public bool EnableLocalMetadata { get; set; } = true; @@ -607,7 +613,7 @@ namespace Jellyfin.Providers.Tests.Manager } } - public class MetadataTestItemInfo : ItemLookupInfo + internal class MetadataTestItemInfo : ItemLookupInfo { } } -- cgit v1.2.3 From 6bf71c0fd355e9c95a1e142019d9bc5cce34200d Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@users.noreply.github.com> Date: Sun, 16 Jan 2022 14:18:44 +0100 Subject: Combine verify calls Co-authored-by: Claus Vium --- tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index 6179e31359..a5673ad838 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -87,14 +87,8 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal(ItemUpdateType.MetadataDownload, actual.Result); for (var i = 0; i < servicesList.Length; i++) { - if (i == expectedIndex) - { - servicesList[i].Verify(mock => mock.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); - } - else - { - servicesList[i].Verify(mock => mock.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); - } + var times = i == expectedIndex ? Times.Once() : Times.Never(); + servicesList[i].Verify(mock => mock.RefreshMetadata(It.IsAny(), It.IsAny(), It.IsAny()), times); } } -- cgit v1.2.3 From 7ad0c9ba24a5f248c2f5b0d89ff096779d22a2b8 Mon Sep 17 00:00:00 2001 From: MrTimscampi Date: Thu, 7 Apr 2022 12:15:25 +0200 Subject: Migrate MusicBrainz plugin to MetaBrainz.MusicBrainz Co-authored-by: crobibero Co-authored-by: Shadowghost --- .../MediaBrowser.Providers.csproj | 1 + .../Configuration/PluginConfiguration.cs | 67 +- .../MusicBrainzAlbumArtistExternalId.cs | 33 +- .../MusicBrainz/MusicBrainzAlbumExternalId.cs | 33 +- .../MusicBrainz/MusicBrainzAlbumProvider.cs | 873 ++++----------------- .../MusicBrainz/MusicBrainzArtistExternalId.cs | 33 +- .../MusicBrainz/MusicBrainzArtistProvider.cs | 332 +++----- .../MusicBrainzOtherArtistExternalId.cs | 33 +- .../MusicBrainzReleaseGroupExternalId.cs | 33 +- .../Plugins/MusicBrainz/MusicBrainzTrackId.cs | 33 +- .../Plugins/MusicBrainz/Plugin.cs | 70 +- src/Jellyfin.Extensions/EnumerableExtensions.cs | 70 +- .../Parsers/MusicAlbumNfoProviderTests.cs | 2 +- .../Parsers/MusicArtistNfoParserTests.cs | 2 +- 14 files changed, 502 insertions(+), 1113 deletions(-) (limited to 'tests') diff --git a/MediaBrowser.Providers/MediaBrowser.Providers.csproj b/MediaBrowser.Providers/MediaBrowser.Providers.csproj index 3a0e9a225b..b00c036e5a 100644 --- a/MediaBrowser.Providers/MediaBrowser.Providers.csproj +++ b/MediaBrowser.Providers/MediaBrowser.Providers.csproj @@ -17,6 +17,7 @@ + diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/Configuration/PluginConfiguration.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/Configuration/PluginConfiguration.cs index 9c27bd7d3f..1d4b88087d 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/Configuration/PluginConfiguration.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/Configuration/PluginConfiguration.cs @@ -1,37 +1,58 @@ -#pragma warning disable CS1591 - using MediaBrowser.Model.Plugins; +using MetaBrainz.MusicBrainz; + +namespace MediaBrowser.Providers.Plugins.MusicBrainz.Configuration; -namespace MediaBrowser.Providers.Plugins.MusicBrainz +/// +/// MusicBrainz plugin configuration. +/// +public class PluginConfiguration : BasePluginConfiguration { - public class PluginConfiguration : BasePluginConfiguration - { - private string _server = Plugin.DefaultServer; + private const string DefaultServer = "musicbrainz.org"; + + private const double DefaultRateLimit = 1.0; - private long _rateLimit = Plugin.DefaultRateLimit; + private string _server = DefaultServer; + + private double _rateLimit = DefaultRateLimit; + + /// + /// Gets or sets the server url. + /// + public string Server + { + get => _server; - public string Server + set { - get => _server; - set => _server = value.TrimEnd('/'); + _server = value.TrimEnd('/'); + Query.DefaultServer = _server; } + } - public long RateLimit + /// + /// Gets or sets the rate limit. + /// + public double RateLimit + { + get => _rateLimit; + set { - get => _rateLimit; - set + if (value < DefaultRateLimit && _server == DefaultServer) { - if (value < Plugin.DefaultRateLimit && _server == Plugin.DefaultServer) - { - _rateLimit = Plugin.DefaultRateLimit; - } - else - { - _rateLimit = value; - } + _rateLimit = DefaultRateLimit; + } + else + { + _rateLimit = value; } - } - public bool ReplaceArtistName { get; set; } + Query.DelayBetweenRequests = _rateLimit; + } } + + /// + /// Gets or sets a value indicating whether to replace the artist name. + /// + public bool ReplaceArtistName { get; set; } } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumArtistExternalId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumArtistExternalId.cs index c54cdda3d3..f7850781e0 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumArtistExternalId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumArtistExternalId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrainz album artist external id. +/// +public class MusicBrainzAlbumArtistExternalId : IExternalId { - public class MusicBrainzAlbumArtistExternalId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzAlbumArtist.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzAlbumArtist.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.AlbumArtist; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.AlbumArtist; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/artist/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/artist/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is Audio; - } + /// + public bool Supports(IHasProviderIds item) => item is Audio; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumExternalId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumExternalId.cs index 8f7fadd060..a9d4472e7d 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumExternalId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumExternalId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrainz album external id. +/// +public class MusicBrainzAlbumExternalId : IExternalId { - public class MusicBrainzAlbumExternalId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzAlbum.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzAlbum.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.Album; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.Album; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/release/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/release/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is Audio || item is MusicAlbum; - } + /// + public bool Supports(IHasProviderIds item) => item is Audio || item is MusicAlbum; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs index 915fb97fd2..e88a51c197 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs @@ -1,805 +1,256 @@ -#nullable disable - -#pragma warning disable CS1591, SA1401 - using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; -using System.IO; using System.Linq; -using System.Net; using System.Net.Http; -using System.Text; using System.Threading; using System.Threading.Tasks; -using System.Xml; -using MediaBrowser.Common.Net; +using Jellyfin.Extensions; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -using Microsoft.Extensions.Logging; - -namespace MediaBrowser.Providers.Music -{ - public class MusicBrainzAlbumProvider : IRemoteMetadataProvider, IHasOrder, IDisposable - { - /// - /// For each single MB lookup/search, this is the maximum number of - /// attempts that shall be made whilst receiving a 503 Server - /// Unavailable (indicating throttled) response. - /// - private const uint MusicBrainzQueryAttempts = 5u; - - /// - /// The Jellyfin user-agent is unrestricted but source IP must not exceed - /// one request per second, therefore we rate limit to avoid throttling. - /// Be prudent, use a value slightly above the minimum required. - /// https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting. - /// - private readonly long _musicBrainzQueryIntervalMs; - - private readonly IHttpClientFactory _httpClientFactory; - private readonly ILogger _logger; - - private readonly string _musicBrainzBaseUrl; - - private SemaphoreSlim _apiRequestLock = new SemaphoreSlim(1, 1); - private Stopwatch _stopWatchMusicBrainz = new Stopwatch(); - - public MusicBrainzAlbumProvider( - IHttpClientFactory httpClientFactory, - ILogger logger) - { - _httpClientFactory = httpClientFactory; - _logger = logger; +using MediaBrowser.Providers.Music; +using MetaBrainz.MusicBrainz; +using MetaBrainz.MusicBrainz.Interfaces.Entities; +using MetaBrainz.MusicBrainz.Interfaces.Searches; - _musicBrainzBaseUrl = Plugin.Instance.Configuration.Server; - _musicBrainzQueryIntervalMs = Plugin.Instance.Configuration.RateLimit; +namespace MediaBrowser.Providers.Plugins.MusicBrainz; - // Use a stopwatch to ensure we don't exceed the MusicBrainz rate limit - _stopWatchMusicBrainz.Start(); +/// +/// Music album metadata provider for MusicBrainz. +/// +public class MusicBrainzAlbumProvider : IRemoteMetadataProvider, IHasOrder, IDisposable +{ + private readonly Query _musicBrainzQuery; - Current = this; - } + /// + /// Initializes a new instance of the class. + /// + public MusicBrainzAlbumProvider() + { + _musicBrainzQuery = new Query(); + } - internal static MusicBrainzAlbumProvider Current { get; private set; } + /// + public string Name => "MusicBrainz"; - /// - public string Name => "MusicBrainz"; + /// + public int Order => 0; - /// - public int Order => 0; + /// + public async Task> GetSearchResults(AlbumInfo searchInfo, CancellationToken cancellationToken) + { + var releaseId = searchInfo.GetReleaseId(); + var releaseGroupId = searchInfo.GetReleaseGroupId(); - /// - public async Task> GetSearchResults(AlbumInfo searchInfo, CancellationToken cancellationToken) + if (!string.IsNullOrEmpty(releaseId)) { - var releaseId = searchInfo.GetReleaseId(); - var releaseGroupId = searchInfo.GetReleaseGroupId(); - - string url; - - if (!string.IsNullOrEmpty(releaseId)) - { - url = "/ws/2/release/?query=reid:" + releaseId.ToString(CultureInfo.InvariantCulture); - } - else if (!string.IsNullOrEmpty(releaseGroupId)) - { - url = "/ws/2/release?release-group=" + releaseGroupId.ToString(CultureInfo.InvariantCulture); - } - else - { - var artistMusicBrainzId = searchInfo.GetMusicBrainzArtistId(); - - if (!string.IsNullOrWhiteSpace(artistMusicBrainzId)) - { - url = string.Format( - CultureInfo.InvariantCulture, - "/ws/2/release/?query=\"{0}\" AND arid:{1}", - WebUtility.UrlEncode(searchInfo.Name), - artistMusicBrainzId); - } - else - { - // I'm sure there is a better way but for now it resolves search for 12" Mixes - var queryName = searchInfo.Name.Replace("\"", string.Empty, StringComparison.Ordinal); - - url = string.Format( - CultureInfo.InvariantCulture, - "/ws/2/release/?query=\"{0}\" AND artist:\"{1}\"", - WebUtility.UrlEncode(queryName), - WebUtility.UrlEncode(searchInfo.GetAlbumArtist())); - } - } - - if (!string.IsNullOrWhiteSpace(url)) - { - using var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - return GetResultsFromResponse(stream); - } - - return Enumerable.Empty(); + var releaseResult = await _musicBrainzQuery.LookupReleaseAsync(new Guid(releaseId), Include.Releases, cancellationToken).ConfigureAwait(false); + return GetResultFromResponse(releaseResult).SingleItemAsEnumerable(); } - private IEnumerable GetResultsFromResponse(Stream stream) + if (!string.IsNullOrEmpty(releaseGroupId)) { - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings() - { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true - }; - - using var reader = XmlReader.Create(oReader, settings); - var results = ReleaseResult.Parse(reader); - - return results.Select(i => - { - var result = new RemoteSearchResult - { - Name = i.Title, - ProductionYear = i.Year - }; - - if (i.Artists.Count > 0) - { - result.AlbumArtist = new RemoteSearchResult - { - SearchProviderName = Name, - Name = i.Artists[0].Item1 - }; - - result.AlbumArtist.SetProviderId(MetadataProvider.MusicBrainzArtist, i.Artists[0].Item2); - } - - if (!string.IsNullOrWhiteSpace(i.ReleaseId)) - { - result.SetProviderId(MetadataProvider.MusicBrainzAlbum, i.ReleaseId); - } - - if (!string.IsNullOrWhiteSpace(i.ReleaseGroupId)) - { - result.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, i.ReleaseGroupId); - } - - return result; - }); + var releaseGroupResult = await _musicBrainzQuery.LookupReleaseGroupAsync(new Guid(releaseGroupId), Include.ReleaseGroups, null, cancellationToken).ConfigureAwait(false); + return GetResultsFromResponse(releaseGroupResult.Releases); } - /// - public async Task> GetMetadata(AlbumInfo info, CancellationToken cancellationToken) - { - var releaseId = info.GetReleaseId(); - var releaseGroupId = info.GetReleaseGroupId(); - - var result = new MetadataResult - { - Item = new MusicAlbum() - }; - - // If we have a release group Id but not a release Id... - if (string.IsNullOrWhiteSpace(releaseId) && !string.IsNullOrWhiteSpace(releaseGroupId)) - { - releaseId = await GetReleaseIdFromReleaseGroupId(releaseGroupId, cancellationToken).ConfigureAwait(false); - result.HasMetadata = true; - } - - if (string.IsNullOrWhiteSpace(releaseId)) - { - var artistMusicBrainzId = info.GetMusicBrainzArtistId(); - - var releaseResult = await GetReleaseResult(artistMusicBrainzId, info.GetAlbumArtist(), info.Name, cancellationToken).ConfigureAwait(false); + var artistMusicBrainzId = searchInfo.GetMusicBrainzArtistId(); - if (releaseResult != null) - { - if (!string.IsNullOrWhiteSpace(releaseResult.ReleaseId)) - { - releaseId = releaseResult.ReleaseId; - result.HasMetadata = true; - } - - if (!string.IsNullOrWhiteSpace(releaseResult.ReleaseGroupId)) - { - releaseGroupId = releaseResult.ReleaseGroupId; - result.HasMetadata = true; - } - - result.Item.ProductionYear = releaseResult.Year; - result.Item.Overview = releaseResult.Overview; - } - } + if (!string.IsNullOrWhiteSpace(artistMusicBrainzId)) + { + var releaseSearchResults = await _musicBrainzQuery.FindReleasesAsync($"\"{searchInfo.Name}\" AND arid:{artistMusicBrainzId}", null, null, false, cancellationToken) + .ConfigureAwait(false); - // If we have a release Id but not a release group Id... - if (!string.IsNullOrWhiteSpace(releaseId) && string.IsNullOrWhiteSpace(releaseGroupId)) + if (releaseSearchResults.Results.Count > 0) { - releaseGroupId = await GetReleaseGroupFromReleaseId(releaseId, cancellationToken).ConfigureAwait(false); - result.HasMetadata = true; + return GetResultsFromResponse(releaseSearchResults.Results); } + } + else + { + // I'm sure there is a better way but for now it resolves search for 12" Mixes + var queryName = searchInfo.Name.Replace("\"", string.Empty, StringComparison.Ordinal); - if (!string.IsNullOrWhiteSpace(releaseId) || !string.IsNullOrWhiteSpace(releaseGroupId)) - { - result.HasMetadata = true; - } + var releaseSearchResults = await _musicBrainzQuery.FindReleasesAsync($"\"{queryName}\" AND artist:\"{searchInfo.GetAlbumArtist()}\"c", null, null, false, cancellationToken) + .ConfigureAwait(false); - if (result.HasMetadata) + if (releaseSearchResults.Results.Count > 0) { - if (!string.IsNullOrEmpty(releaseId)) - { - result.Item.SetProviderId(MetadataProvider.MusicBrainzAlbum, releaseId); - } - - if (!string.IsNullOrEmpty(releaseGroupId)) - { - result.Item.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, releaseGroupId); - } + return GetResultsFromResponse(releaseSearchResults.Results); } - - return result; } - private Task GetReleaseResult(string artistMusicBrainId, string artistName, string albumName, CancellationToken cancellationToken) - { - if (!string.IsNullOrEmpty(artistMusicBrainId)) - { - return GetReleaseResult(albumName, artistMusicBrainId, cancellationToken); - } - - if (string.IsNullOrWhiteSpace(artistName)) - { - return Task.FromResult(new ReleaseResult()); - } + return Enumerable.Empty(); + } - return GetReleaseResultByArtistName(albumName, artistName, cancellationToken); + private IEnumerable GetResultsFromResponse(IEnumerable>? releaseSearchResults) + { + if (releaseSearchResults is null) + { + yield break; } - private async Task GetReleaseResult(string albumName, string artistId, CancellationToken cancellationToken) + foreach (var result in releaseSearchResults) { - var url = string.Format( - CultureInfo.InvariantCulture, - "/ws/2/release/?query=\"{0}\" AND arid:{1}", - WebUtility.UrlEncode(albumName), - artistId); - - using var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings - { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true - }; - - using var reader = XmlReader.Create(oReader, settings); - return ReleaseResult.Parse(reader).FirstOrDefault(); + yield return GetResultFromResponse(result.Item); } + } - private async Task GetReleaseResultByArtistName(string albumName, string artistName, CancellationToken cancellationToken) + private IEnumerable GetResultsFromResponse(IEnumerable? releaseSearchResults) + { + if (releaseSearchResults is null) { - var url = string.Format( - CultureInfo.InvariantCulture, - "/ws/2/release/?query=\"{0}\" AND artist:\"{1}\"", - WebUtility.UrlEncode(albumName), - WebUtility.UrlEncode(artistName)); - - using var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings() - { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true - }; - - using var reader = XmlReader.Create(oReader, settings); - return ReleaseResult.Parse(reader).FirstOrDefault(); + yield break; } - private static (string Name, string ArtistId) ParseArtistCredit(XmlReader reader) + foreach (var result in releaseSearchResults) { - reader.MoveToContent(); - reader.Read(); - - // http://stackoverflow.com/questions/2299632/why-does-xmlreader-skip-every-other-element-if-there-is-no-whitespace-separator - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "name-credit": - { - if (reader.IsEmptyElement) - { - reader.Read(); - break; - } - - using var subReader = reader.ReadSubtree(); - return ParseArtistNameCredit(subReader); - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } - - return default; + yield return GetResultFromResponse(result); } + } - private static (string Name, string ArtistId) ParseArtistNameCredit(XmlReader reader) + private RemoteSearchResult GetResultFromResponse(IRelease releaseSearchResult) + { + var searchResult = new RemoteSearchResult { - reader.MoveToContent(); - reader.Read(); + Name = releaseSearchResult.Title, + ProductionYear = releaseSearchResult.Date?.Year, + PremiereDate = releaseSearchResult.Date?.NearestDate + }; - // http://stackoverflow.com/questions/2299632/why-does-xmlreader-skip-every-other-element-if-there-is-no-whitespace-separator + if (releaseSearchResult.ArtistCredit?.Count > 0) + { + searchResult.AlbumArtist = new RemoteSearchResult + { + SearchProviderName = Name, + Name = releaseSearchResult.ArtistCredit[0].Name + }; - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) + if (releaseSearchResult.ArtistCredit[0].Artist?.Id is not null) { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "artist": - { - if (reader.IsEmptyElement) - { - reader.Read(); - break; - } - - var id = reader.GetAttribute("id"); - using var subReader = reader.ReadSubtree(); - return ParseArtistArtistCredit(subReader, id); - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } + searchResult.AlbumArtist.SetProviderId(MetadataProvider.MusicBrainzArtist, releaseSearchResult.ArtistCredit[0].Artist!.Id.ToString()); } - - return (null, null); } - private static (string Name, string ArtistId) ParseArtistArtistCredit(XmlReader reader, string artistId) - { - reader.MoveToContent(); - reader.Read(); - - string name = null; - - // http://stackoverflow.com/questions/2299632/why-does-xmlreader-skip-every-other-element-if-there-is-no-whitespace-separator - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "name": - { - name = reader.ReadElementContentAsString(); - break; - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } + searchResult.SetProviderId(MetadataProvider.MusicBrainzAlbum, releaseSearchResult.Id.ToString()); - return (name, artistId); + if (releaseSearchResult.ReleaseGroup?.Id is not null) + { + searchResult.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, releaseSearchResult.ReleaseGroup.Id.ToString()); } - private async Task GetReleaseIdFromReleaseGroupId(string releaseGroupId, CancellationToken cancellationToken) - { - var url = "/ws/2/release?release-group=" + releaseGroupId.ToString(CultureInfo.InvariantCulture); + return searchResult; + } - using var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings - { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true - }; + /// + public async Task> GetMetadata(AlbumInfo info, CancellationToken cancellationToken) + { + // TODO: This sets essentially nothing. As-is, it's mostly useless. Make it actually pull metadata and use it. + var releaseId = info.GetReleaseId(); + var releaseGroupId = info.GetReleaseGroupId(); - using var reader = XmlReader.Create(oReader, settings); - var result = ReleaseResult.Parse(reader).FirstOrDefault(); + var result = new MetadataResult + { + Item = new MusicAlbum() + }; - return result?.ReleaseId; + // If there is a release group, but no release ID, try to match the release + if (string.IsNullOrWhiteSpace(releaseId) && !string.IsNullOrWhiteSpace(releaseGroupId)) + { + // TODO: Actually try to match the release. Simply taking the first result is stupid. + var releaseGroup = await _musicBrainzQuery.LookupReleaseGroupAsync(new Guid(releaseGroupId), Include.ReleaseGroups, null, cancellationToken).ConfigureAwait(false); + var release = releaseGroup.Releases?.Count > 0 ? releaseGroup.Releases[0] : null; + releaseId = release?.Id.ToString(); + result.HasMetadata = true; } - /// - /// Gets the release group id internal. - /// - /// The release entry id. - /// The cancellation token. - /// Task{System.String}. - private async Task GetReleaseGroupFromReleaseId(string releaseEntryId, CancellationToken cancellationToken) + // If there is no release ID, lookup a release with the info we have + if (string.IsNullOrWhiteSpace(releaseId)) { - var url = "/ws/2/release-group/?query=reid:" + releaseEntryId.ToString(CultureInfo.InvariantCulture); + var artistMusicBrainzId = info.GetMusicBrainzArtistId(); + IRelease? releaseResult = null; - using var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings + if (!string.IsNullOrEmpty(artistMusicBrainzId)) { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true, - Async = true - }; - - using var reader = XmlReader.Create(oReader, settings); - await reader.MoveToContentAsync().ConfigureAwait(false); - await reader.ReadAsync().ConfigureAwait(false); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "release-group-list": - { - if (reader.IsEmptyElement) - { - await reader.ReadAsync().ConfigureAwait(false); - continue; - } - - using var subReader = reader.ReadSubtree(); - return GetFirstReleaseGroupId(subReader); - } - - default: - { - await reader.SkipAsync().ConfigureAwait(false); - break; - } - } - } - else - { - await reader.ReadAsync().ConfigureAwait(false); - } + var releaseSearchResults = await _musicBrainzQuery.FindReleasesAsync($"\"{info.Name}\" AND arid:{artistMusicBrainzId}", null, null, false, cancellationToken) + .ConfigureAwait(false); + releaseResult = releaseSearchResults.Results.Count > 0 ? releaseSearchResults.Results[0].Item : null; } - - return null; - } - - private string GetFirstReleaseGroupId(XmlReader reader) - { - reader.MoveToContent(); - reader.Read(); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) + else if (!string.IsNullOrEmpty(info.GetAlbumArtist())) { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "release-group": - { - return reader.GetAttribute("id"); - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } + var releaseSearchResults = await _musicBrainzQuery.FindReleasesAsync($"\"{info.Name}\" AND artist:{info.GetAlbumArtist()}", null, null, false, cancellationToken) + .ConfigureAwait(false); + releaseResult = releaseSearchResults.Results.Count > 0 ? releaseSearchResults.Results[0].Item : null; } - return null; - } - - /// - /// Makes request to MusicBrainz server and awaits a response. - /// A 503 Service Unavailable response indicates throttling to maintain a rate limit. - /// A number of retries shall be made in order to try and satisfy the request before - /// giving up and returning null. - /// - /// Address of MusicBrainz server. - /// CancellationToken to use for method. - /// Returns response from MusicBrainz service. - internal async Task GetMusicBrainzResponse(string url, CancellationToken cancellationToken) - { - await _apiRequestLock.WaitAsync(cancellationToken).ConfigureAwait(false); - - try + if (releaseResult != null) { - HttpResponseMessage response; - var attempts = 0u; - var requestUrl = _musicBrainzBaseUrl.TrimEnd('/') + url; + releaseId = releaseResult.Id.ToString(); - do + if (releaseResult.ReleaseGroup?.Id is not null) { - attempts++; - - if (_stopWatchMusicBrainz.ElapsedMilliseconds < _musicBrainzQueryIntervalMs) - { - // MusicBrainz is extremely adamant about limiting to one request per second. - var delayMs = _musicBrainzQueryIntervalMs - _stopWatchMusicBrainz.ElapsedMilliseconds; - await Task.Delay((int)delayMs, cancellationToken).ConfigureAwait(false); - } - - // Write time since last request to debug log as evidence we're meeting rate limit - // requirement, before resetting stopwatch back to zero. - _logger.LogDebug("GetMusicBrainzResponse: Time since previous request: {0} ms", _stopWatchMusicBrainz.ElapsedMilliseconds); - _stopWatchMusicBrainz.Restart(); - - using var request = new HttpRequestMessage(HttpMethod.Get, requestUrl); - response = await _httpClientFactory - .CreateClient(NamedClient.MusicBrainz) - .SendAsync(request, cancellationToken) - .ConfigureAwait(false); - - // We retry a finite number of times, and only whilst MB is indicating 503 (throttling). + releaseGroupId = releaseResult.ReleaseGroup.Id.ToString(); } - while (attempts < MusicBrainzQueryAttempts && response.StatusCode == HttpStatusCode.ServiceUnavailable); - // Log error if unable to query MB database due to throttling. - if (attempts == MusicBrainzQueryAttempts && response.StatusCode == HttpStatusCode.ServiceUnavailable) - { - _logger.LogError("GetMusicBrainzResponse: 503 Service Unavailable (throttled) response received {0} times whilst requesting {1}", attempts, requestUrl); - } - - return response; - } - finally - { - _apiRequestLock.Release(); + result.HasMetadata = true; + result.Item.ProductionYear = releaseResult.Date?.Year; + result.Item.Overview = releaseResult.Annotation; } } - /// - public Task GetImageResponse(string url, CancellationToken cancellationToken) + // If we have a release ID but not a release group ID, lookup the release group + if (!string.IsNullOrWhiteSpace(releaseId) && string.IsNullOrWhiteSpace(releaseGroupId)) { - throw new NotImplementedException(); + var release = await _musicBrainzQuery.LookupReleaseAsync(new Guid(releaseId), Include.Releases, cancellationToken).ConfigureAwait(false); + releaseGroupId = release.ReleaseGroup?.Id.ToString(); + result.HasMetadata = true; } - protected virtual void Dispose(bool disposing) + // If we have a release ID and a release group ID + if (!string.IsNullOrWhiteSpace(releaseId) || !string.IsNullOrWhiteSpace(releaseGroupId)) { - if (disposing) - { - _apiRequestLock?.Dispose(); - } + result.HasMetadata = true; } - /// - public void Dispose() + if (result.HasMetadata) { - Dispose(true); - GC.SuppressFinalize(this); - } - - private class ReleaseResult - { - public string ReleaseId; - public string ReleaseGroupId; - public string Title; - public string Overview; - public int? Year; - - public List<(string, string)> Artists = new(); - - public static IEnumerable Parse(XmlReader reader) + if (!string.IsNullOrEmpty(releaseId)) { - reader.MoveToContent(); - reader.Read(); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "release-list": - { - if (reader.IsEmptyElement) - { - reader.Read(); - continue; - } - - using var subReader = reader.ReadSubtree(); - return ParseReleaseList(subReader).ToList(); - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } - - return Enumerable.Empty(); + result.Item.SetProviderId(MetadataProvider.MusicBrainzAlbum, releaseId); } - private static IEnumerable ParseReleaseList(XmlReader reader) + if (!string.IsNullOrEmpty(releaseGroupId)) { - reader.MoveToContent(); - reader.Read(); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "release": - { - if (reader.IsEmptyElement) - { - reader.Read(); - continue; - } - - var releaseId = reader.GetAttribute("id"); - - using var subReader = reader.ReadSubtree(); - var release = ParseRelease(subReader, releaseId); - if (release != null) - { - yield return release; - } - - break; - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } + result.Item.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, releaseGroupId); } + } - private static ReleaseResult ParseRelease(XmlReader reader, string releaseId) - { - var result = new ReleaseResult - { - ReleaseId = releaseId - }; - - reader.MoveToContent(); - reader.Read(); + return result; + } - // http://stackoverflow.com/questions/2299632/why-does-xmlreader-skip-every-other-element-if-there-is-no-whitespace-separator + /// + public Task GetImageResponse(string url, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "title": - { - result.Title = reader.ReadElementContentAsString(); - break; - } - - case "date": - { - var val = reader.ReadElementContentAsString(); - if (DateTime.TryParse(val, out var date)) - { - result.Year = date.Year; - } - - break; - } - - case "annotation": - { - result.Overview = reader.ReadElementContentAsString(); - break; - } - - case "release-group": - { - result.ReleaseGroupId = reader.GetAttribute("id"); - reader.Skip(); - break; - } - - case "artist-credit": - { - if (reader.IsEmptyElement) - { - reader.Read(); - break; - } - - using var subReader = reader.ReadSubtree(); - var artist = ParseArtistCredit(subReader); - - if (!string.IsNullOrEmpty(artist.Name)) - { - result.Artists.Add(artist); - } - - break; - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } - return result; - } + /// + /// Dispose all resources. + /// + /// Whether to dispose. + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _musicBrainzQuery.Dispose(); } } } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistExternalId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistExternalId.cs index 941ffea721..e2fb5621bc 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistExternalId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistExternalId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrains Artist ExternalId. +/// +public class MusicBrainzArtistExternalId : IExternalId { - public class MusicBrainzArtistExternalId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzArtist.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzArtist.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.Artist; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.Artist; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/artist/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/artist/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is MusicArtist; - } + /// + public bool Supports(IHasProviderIds item) => item is MusicArtist; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs index 906a42f36d..1d2c9c2c81 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs @@ -1,15 +1,7 @@ -#nullable disable - -#pragma warning disable CS1591 - using System; using System.Collections.Generic; -using System.Globalization; -using System.IO; using System.Linq; -using System.Net; using System.Net.Http; -using System.Text; using System.Threading; using System.Threading.Tasks; using System.Xml; @@ -18,257 +10,159 @@ using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; - -namespace MediaBrowser.Providers.Music -{ - public class MusicBrainzArtistProvider : IRemoteMetadataProvider - { - public string Name => "MusicBrainz"; - - /// - public async Task> GetSearchResults(ArtistInfo searchInfo, CancellationToken cancellationToken) - { - var musicBrainzId = searchInfo.GetMusicBrainzArtistId(); - - if (!string.IsNullOrWhiteSpace(musicBrainzId)) - { - var url = "/ws/2/artist/?query=arid:{0}" + musicBrainzId.ToString(CultureInfo.InvariantCulture); - - using var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - return GetResultsFromResponse(stream); - } - else - { - // They seem to throw bad request failures on any term with a slash - var nameToSearch = searchInfo.Name.Replace('/', ' '); +using MediaBrowser.Providers.Music; +using MetaBrainz.MusicBrainz; +using MetaBrainz.MusicBrainz.Interfaces.Entities; +using MetaBrainz.MusicBrainz.Interfaces.Searches; - var url = string.Format(CultureInfo.InvariantCulture, "/ws/2/artist/?query=\"{0}\"&dismax=true", UrlEncode(nameToSearch)); +namespace MediaBrowser.Providers.Plugins.MusicBrainz; - using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) - await using (var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false)) - { - var results = GetResultsFromResponse(stream).ToList(); +/// +/// MusicBrainz artist provider. +/// +public class MusicBrainzArtistProvider : IRemoteMetadataProvider, IDisposable +{ + private readonly Query _musicBrainzQuery; - if (results.Count > 0) - { - return results; - } - } + /// + /// Initializes a new instance of the class. + /// + public MusicBrainzArtistProvider() + { + _musicBrainzQuery = new Query(); + } - if (searchInfo.Name.HasDiacritics()) - { - // Try again using the search with accent characters url - url = string.Format(CultureInfo.InvariantCulture, "/ws/2/artist/?query=artistaccent:\"{0}\"", UrlEncode(nameToSearch)); + /// + public string Name => "MusicBrainz"; - using var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false); - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - return GetResultsFromResponse(stream); - } - } + /// + public async Task> GetSearchResults(ArtistInfo searchInfo, CancellationToken cancellationToken) + { + var artistId = searchInfo.GetMusicBrainzArtistId(); - return Enumerable.Empty(); + if (!string.IsNullOrWhiteSpace(artistId)) + { + var artistResult = await _musicBrainzQuery.LookupArtistAsync(new Guid(artistId), Include.Artists, null, null, cancellationToken).ConfigureAwait(false); + return GetResultFromResponse(artistResult).SingleItemAsEnumerable(); } - private IEnumerable GetResultsFromResponse(Stream stream) + var artistSearchResults = await _musicBrainzQuery.FindArtistsAsync($"\"{searchInfo.Name}\"", null, null, false, cancellationToken) + .ConfigureAwait(false); + if (artistSearchResults.Results.Count > 0) { - using var oReader = new StreamReader(stream, Encoding.UTF8); - var settings = new XmlReaderSettings() - { - ValidationType = ValidationType.None, - CheckCharacters = false, - IgnoreProcessingInstructions = true, - IgnoreComments = true - }; - - using var reader = XmlReader.Create(oReader, settings); - reader.MoveToContent(); - reader.Read(); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "artist-list": - { - if (reader.IsEmptyElement) - { - reader.Read(); - continue; - } - - using var subReader = reader.ReadSubtree(); - return ParseArtistList(subReader).ToList(); - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } - - return Enumerable.Empty(); + return GetResultsFromResponse(artistSearchResults.Results); } - private IEnumerable ParseArtistList(XmlReader reader) + if (searchInfo.Name.HasDiacritics()) { - reader.MoveToContent(); - reader.Read(); - - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) + // Try again using the search with an accented characters query + var artistAccentsSearchResults = await _musicBrainzQuery.FindArtistsAsync($"artistaccent:\"{searchInfo.Name}\"", null, null, false, cancellationToken) + .ConfigureAwait(false); + if (artistAccentsSearchResults.Results.Count > 0) { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "artist": - { - if (reader.IsEmptyElement) - { - reader.Read(); - continue; - } - - var mbzId = reader.GetAttribute("id"); - - using var subReader = reader.ReadSubtree(); - var artist = ParseArtist(subReader, mbzId); - if (artist != null) - { - yield return artist; - } - - break; - } - - default: - { - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } + return GetResultsFromResponse(artistAccentsSearchResults.Results); } } - private RemoteSearchResult ParseArtist(XmlReader reader, string artistId) + return Enumerable.Empty(); + } + + private IEnumerable GetResultsFromResponse(IEnumerable>? releaseSearchResults) + { + if (releaseSearchResults is null) { - var result = new RemoteSearchResult(); + yield break; + } - reader.MoveToContent(); - reader.Read(); + foreach (var result in releaseSearchResults) + { + yield return GetResultFromResponse(result.Item); + } + } - // http://stackoverflow.com/questions/2299632/why-does-xmlreader-skip-every-other-element-if-there-is-no-whitespace-separator + private IEnumerable GetResultsFromResponse(IEnumerable? releaseSearchResults) + { + if (releaseSearchResults is null) + { + yield break; + } - // Loop through each element - while (!reader.EOF && reader.ReadState == ReadState.Interactive) - { - if (reader.NodeType == XmlNodeType.Element) - { - switch (reader.Name) - { - case "name": - { - result.Name = reader.ReadElementContentAsString(); - break; - } + foreach (var result in releaseSearchResults) + { + yield return GetResultFromResponse(result); + } + } - case "annotation": - { - result.Overview = reader.ReadElementContentAsString(); - break; - } + private RemoteSearchResult GetResultFromResponse(IArtist artist) + { + var searchResult = new RemoteSearchResult + { + Name = artist.Name, + ProductionYear = artist.LifeSpan?.Begin?.Year, + PremiereDate = artist.LifeSpan?.Begin?.NearestDate + }; - default: - { - // there is sort-name if ever needed - reader.Skip(); - break; - } - } - } - else - { - reader.Read(); - } - } + searchResult.SetProviderId(MetadataProvider.MusicBrainzArtist, artist.Id.ToString()); - result.SetProviderId(MetadataProvider.MusicBrainzArtist, artistId); + return searchResult; + } - if (string.IsNullOrWhiteSpace(artistId) || string.IsNullOrWhiteSpace(result.Name)) - { - return null; - } + /// + public async Task> GetMetadata(ArtistInfo info, CancellationToken cancellationToken) + { + var result = new MetadataResult { Item = new MusicArtist() }; - return result; - } + var musicBrainzId = info.GetMusicBrainzArtistId(); - /// - public async Task> GetMetadata(ArtistInfo info, CancellationToken cancellationToken) + if (string.IsNullOrWhiteSpace(musicBrainzId)) { - var result = new MetadataResult - { - Item = new MusicArtist() - }; + var searchResults = await GetSearchResults(info, cancellationToken).ConfigureAwait(false); - var musicBrainzId = info.GetMusicBrainzArtistId(); + var singleResult = searchResults.FirstOrDefault(); - if (string.IsNullOrWhiteSpace(musicBrainzId)) + if (singleResult != null) { - var searchResults = await GetSearchResults(info, cancellationToken).ConfigureAwait(false); + musicBrainzId = singleResult.GetProviderId(MetadataProvider.MusicBrainzArtist); + result.Item.Overview = singleResult.Overview; - var singleResult = searchResults.FirstOrDefault(); - - if (singleResult != null) + if (Plugin.Instance!.Configuration.ReplaceArtistName) { - musicBrainzId = singleResult.GetProviderId(MetadataProvider.MusicBrainzArtist); - result.Item.Overview = singleResult.Overview; - - if (Plugin.Instance.Configuration.ReplaceArtistName) - { - result.Item.Name = singleResult.Name; - } + result.Item.Name = singleResult.Name; } } - - if (!string.IsNullOrWhiteSpace(musicBrainzId)) - { - result.HasMetadata = true; - result.Item.SetProviderId(MetadataProvider.MusicBrainzArtist, musicBrainzId); - } - - return result; } - /// - /// Encodes an URL. - /// - /// The name. - /// System.String. - private static string UrlEncode(string name) + if (!string.IsNullOrWhiteSpace(musicBrainzId)) { - return WebUtility.UrlEncode(name); + result.HasMetadata = true; + result.Item.SetProviderId(MetadataProvider.MusicBrainzArtist, musicBrainzId); } - public Task GetImageResponse(string url, CancellationToken cancellationToken) + return result; + } + + /// + public Task GetImageResponse(string url, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Dispose all resources. + /// + /// Whether to dispose. + protected virtual void Dispose(bool disposing) + { + if (disposing) { - throw new NotImplementedException(); + _musicBrainzQuery.Dispose(); } } } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzOtherArtistExternalId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzOtherArtistExternalId.cs index 05db2d98f7..fdaa5574f0 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzOtherArtistExternalId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzOtherArtistExternalId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrainz other artist external id. +/// +public class MusicBrainzOtherArtistExternalId : IExternalId { - public class MusicBrainzOtherArtistExternalId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzArtist.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzArtist.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.OtherArtist; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.OtherArtist; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/artist/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/artist/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is Audio || item is MusicAlbum; - } + /// + public bool Supports(IHasProviderIds item) => item is Audio or MusicAlbum; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzReleaseGroupExternalId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzReleaseGroupExternalId.cs index acb652fe01..0baab9955d 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzReleaseGroupExternalId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzReleaseGroupExternalId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrainz release group external id. +/// +public class MusicBrainzReleaseGroupExternalId : IExternalId { - public class MusicBrainzReleaseGroupExternalId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzReleaseGroup.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzReleaseGroup.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.ReleaseGroup; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.ReleaseGroup; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/release-group/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/release-group/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is Audio || item is MusicAlbum; - } + /// + public bool Supports(IHasProviderIds item) => item is Audio or MusicAlbum; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzTrackId.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzTrackId.cs index 14805b9b79..5c974c4111 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzTrackId.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzTrackId.cs @@ -1,28 +1,27 @@ -#pragma warning disable CS1591 - using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Music +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// MusicBrainz track id. +/// +public class MusicBrainzTrackId : IExternalId { - public class MusicBrainzTrackId : IExternalId - { - /// - public string ProviderName => "MusicBrainz"; + /// + public string ProviderName => "MusicBrainz"; - /// - public string Key => MetadataProvider.MusicBrainzTrack.ToString(); + /// + public string Key => MetadataProvider.MusicBrainzTrack.ToString(); - /// - public ExternalIdMediaType? Type => ExternalIdMediaType.Track; + /// + public ExternalIdMediaType? Type => ExternalIdMediaType.Track; - /// - public string? UrlFormatString => Plugin.Instance.Configuration.Server + "/track/{0}"; + /// + public string? UrlFormatString => Plugin.Instance!.Configuration.Server + "/track/{0}"; - /// - public bool Supports(IHasProviderIds item) => item is Audio; - } + /// + public bool Supports(IHasProviderIds item) => item is Audio; } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs index cfa10dd648..270d76e6db 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs @@ -1,45 +1,63 @@ -#nullable disable -#pragma warning disable CS1591 - using System; using System.Collections.Generic; +using System.Net.Http.Headers; +using System.Reflection; using MediaBrowser.Common.Configuration; using MediaBrowser.Common.Plugins; using MediaBrowser.Model.Plugins; using MediaBrowser.Model.Serialization; +using MediaBrowser.Providers.Plugins.MusicBrainz.Configuration; +using MetaBrainz.MusicBrainz; + +namespace MediaBrowser.Providers.Plugins.MusicBrainz; -namespace MediaBrowser.Providers.Plugins.MusicBrainz +/// +/// Plugin instance. +/// +public class Plugin : BasePlugin, IHasWebPages { - public class Plugin : BasePlugin, IHasWebPages + /// + /// Initializes a new instance of the class. + /// + /// Instance of the interface. + /// Instance of the interface. + public Plugin(IApplicationPaths applicationPaths, IXmlSerializer xmlSerializer) + : base(applicationPaths, xmlSerializer) { - public const string DefaultServer = "https://musicbrainz.org"; - - public const long DefaultRateLimit = 2000u; + Instance = this; - public Plugin(IApplicationPaths applicationPaths, IXmlSerializer xmlSerializer) - : base(applicationPaths, xmlSerializer) - { - Instance = this; - } + // TODO: Change this to "JellyfinMusicBrainzPlugin" once we take it out of the server repo. + Query.DefaultUserAgent.Add(new ProductInfoHeaderValue("Jellyfin", Assembly.GetExecutingAssembly().GetName().Version?.ToString(3))); + Query.DefaultUserAgent.Add(new ProductInfoHeaderValue("(apps@jellyfin.org)")); + Query.DelayBetweenRequests = Instance.Configuration.RateLimit; + Query.DefaultServer = Instance.Configuration.Server; + } - public static Plugin Instance { get; private set; } + /// + /// Gets the current plugin instance. + /// + public static Plugin? Instance { get; private set; } - public override Guid Id => new Guid("8c95c4d2-e50c-4fb0-a4f3-6c06ff0f9a1a"); + /// + public override Guid Id => new Guid("8c95c4d2-e50c-4fb0-a4f3-6c06ff0f9a1a"); - public override string Name => "MusicBrainz"; + /// + public override string Name => "MusicBrainz"; - public override string Description => "Get artist and album metadata from any MusicBrainz server."; + /// + public override string Description => "Get artist and album metadata from any MusicBrainz server."; - // TODO remove when plugin removed from server. - public override string ConfigurationFileName => "Jellyfin.Plugin.MusicBrainz.xml"; + /// + // TODO remove when plugin removed from server. + public override string ConfigurationFileName => "Jellyfin.Plugin.MusicBrainz.xml"; - public IEnumerable GetPages() + /// + public IEnumerable GetPages() + { + yield return new PluginPageInfo { - yield return new PluginPageInfo - { - Name = Name, - EmbeddedResourcePath = GetType().Namespace + ".Configuration.config.html" - }; - } + Name = Name, + EmbeddedResourcePath = GetType().Namespace + ".Configuration.config.html" + }; } } diff --git a/src/Jellyfin.Extensions/EnumerableExtensions.cs b/src/Jellyfin.Extensions/EnumerableExtensions.cs index a31a57dc65..fd46358a4f 100644 --- a/src/Jellyfin.Extensions/EnumerableExtensions.cs +++ b/src/Jellyfin.Extensions/EnumerableExtensions.cs @@ -1,42 +1,31 @@ using System; using System.Collections.Generic; -namespace Jellyfin.Extensions +namespace Jellyfin.Extensions; + +/// +/// Static extensions for the interface. +/// +public static class EnumerableExtensions { /// - /// Static extensions for the interface. + /// Determines whether the value is contained in the source collection. /// - public static class EnumerableExtensions + /// An instance of the interface. + /// The value to look for in the collection. + /// The string comparison. + /// A value indicating whether the value is contained in the collection. + /// The source is null. + public static bool Contains(this IEnumerable source, ReadOnlySpan value, StringComparison stringComparison) { - /// - /// Determines whether the value is contained in the source collection. - /// - /// An instance of the interface. - /// The value to look for in the collection. - /// The string comparison. - /// A value indicating whether the value is contained in the collection. - /// The source is null. - public static bool Contains(this IEnumerable source, ReadOnlySpan value, StringComparison stringComparison) - { - ArgumentNullException.ThrowIfNull(source); - - if (source is IList list) - { - int len = list.Count; - for (int i = 0; i < len; i++) - { - if (value.Equals(list[i], stringComparison)) - { - return true; - } - } - - return false; - } + ArgumentNullException.ThrowIfNull(source); - foreach (string element in source) + if (source is IList list) + { + int len = list.Count; + for (int i = 0; i < len; i++) { - if (value.Equals(element, stringComparison)) + if (value.Equals(list[i], stringComparison)) { return true; } @@ -44,5 +33,26 @@ namespace Jellyfin.Extensions return false; } + + foreach (string element in source) + { + if (value.Equals(element, stringComparison)) + { + return true; + } + } + + return false; + } + + /// + /// Gets an IEnumerable from a single item. + /// + /// The item to return. + /// The type of item. + /// The IEnumerable{T}. + public static IEnumerable SingleItemAsEnumerable(this T item) + { + yield return item; } } diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicAlbumNfoProviderTests.cs b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicAlbumNfoProviderTests.cs index eea8cb50a7..8f276d03fe 100644 --- a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicAlbumNfoProviderTests.cs +++ b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicAlbumNfoProviderTests.cs @@ -7,7 +7,7 @@ using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Music; +using MediaBrowser.Providers.Plugins.MusicBrainz; using MediaBrowser.XbmcMetadata.Parsers; using Microsoft.Extensions.Logging.Abstractions; using Moq; diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicArtistNfoParserTests.cs b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicArtistNfoParserTests.cs index 8ca3dd96e3..78183d9ffd 100644 --- a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicArtistNfoParserTests.cs +++ b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/MusicArtistNfoParserTests.cs @@ -7,7 +7,7 @@ using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Providers; -using MediaBrowser.Providers.Music; +using MediaBrowser.Providers.Plugins.MusicBrainz; using MediaBrowser.XbmcMetadata.Parsers; using Microsoft.Extensions.Logging.Abstractions; using Moq; -- cgit v1.2.3 From 09e8a7e62c9a8ef567f2d336e37a7cc732e3aaab Mon Sep 17 00:00:00 2001 From: photonconvergence <116527579+photonconvergence@users.noreply.github.com> Date: Thu, 27 Oct 2022 18:01:04 -0700 Subject: Fix extra type differentiation Change rules for Featurettes and Shorts so they don't both get classed as ExtraType.Clip. Fix test that these changes break --- Emby.Naming/Common/NamingOptions.cs | 14 ++++++++++---- MediaBrowser.Controller/Entities/BaseItem.cs | 4 +++- MediaBrowser.Model/Entities/ExtraType.cs | 4 +++- tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs | 5 +++-- 4 files changed, 19 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/Emby.Naming/Common/NamingOptions.cs b/Emby.Naming/Common/NamingOptions.cs index 513733ab55..25131e9c53 100644 --- a/Emby.Naming/Common/NamingOptions.cs +++ b/Emby.Naming/Common/NamingOptions.cs @@ -512,13 +512,13 @@ namespace Emby.Naming.Common MediaType.Video), new ExtraRule( - ExtraType.Clip, + ExtraType.Short, ExtraRuleType.DirectoryName, "shorts", MediaType.Video), new ExtraRule( - ExtraType.Clip, + ExtraType.Featurette, ExtraRuleType.DirectoryName, "featurettes", MediaType.Video), @@ -535,6 +535,12 @@ namespace Emby.Naming.Common "other", MediaType.Video), + new ExtraRule( + ExtraType.Clip, + ExtraRuleType.DirectoryName, + "clips", + MediaType.Video), + new ExtraRule( ExtraType.Trailer, ExtraRuleType.Filename, @@ -638,13 +644,13 @@ namespace Emby.Naming.Common MediaType.Video), new ExtraRule( - ExtraType.Clip, + ExtraType.Featurette, ExtraRuleType.Suffix, "-featurette", MediaType.Video), new ExtraRule( - ExtraType.Clip, + ExtraType.Short, ExtraRuleType.Suffix, "-short", MediaType.Video), diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs index 24163f1df9..7f5f9f74bd 100644 --- a/MediaBrowser.Controller/Entities/BaseItem.cs +++ b/MediaBrowser.Controller/Entities/BaseItem.cs @@ -75,7 +75,9 @@ namespace MediaBrowser.Controller.Entities Model.Entities.ExtraType.DeletedScene, Model.Entities.ExtraType.Interview, Model.Entities.ExtraType.Sample, - Model.Entities.ExtraType.Scene + Model.Entities.ExtraType.Scene, + Model.Entities.ExtraType.Featurette, + Model.Entities.ExtraType.Short }; private string _sortName; diff --git a/MediaBrowser.Model/Entities/ExtraType.cs b/MediaBrowser.Model/Entities/ExtraType.cs index aca4bd2829..66da80d96b 100644 --- a/MediaBrowser.Model/Entities/ExtraType.cs +++ b/MediaBrowser.Model/Entities/ExtraType.cs @@ -13,6 +13,8 @@ namespace MediaBrowser.Model.Entities Scene = 6, Sample = 7, ThemeSong = 8, - ThemeVideo = 9 + ThemeVideo = 9, + Featurette = 10, + Short = 11 } } diff --git a/tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs b/tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs index 731580e0c9..2c33ab4929 100644 --- a/tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs +++ b/tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs @@ -51,8 +51,9 @@ namespace Jellyfin.Naming.Tests.Video [InlineData(ExtraType.Interview, "interviews")] [InlineData(ExtraType.Scene, "scenes")] [InlineData(ExtraType.Sample, "samples")] - [InlineData(ExtraType.Clip, "shorts")] - [InlineData(ExtraType.Clip, "featurettes")] + [InlineData(ExtraType.Short, "shorts")] + [InlineData(ExtraType.Featurette, "featurettes")] + [InlineData(ExtraType.Clip, "clips")] [InlineData(ExtraType.ThemeVideo, "backdrops")] [InlineData(ExtraType.Unknown, "extras")] public void TestDirectories(ExtraType type, string dirName) -- cgit v1.2.3 From c7a9759a76995cbb7bd5b0e58d0e1fc9700c51cf Mon Sep 17 00:00:00 2001 From: Dmitry Lyzo Date: Mon, 31 Oct 2022 15:51:06 +0300 Subject: fix tests --- .../Dlna/StreamBuilderTests.cs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs index 9baf6877d9..c279b6b4bb 100644 --- a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs +++ b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs @@ -21,8 +21,8 @@ namespace Jellyfin.Model.Tests [Theory] // Chrome [InlineData("Chrome", "mp4-h264-aac-vtt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Chrome", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Chrome", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectPlay)] // #6450 + [InlineData("Chrome", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 + [InlineData("Chrome", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Chrome", "mp4-h264-ac3-aacExt-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioIsExternal)] // #6450 [InlineData("Chrome", "mp4-h264-ac3-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioCodecNotSupported)] // #6450 [InlineData("Chrome", "mp4-hevc-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.VideoCodecNotSupported, "Transcode")] @@ -32,8 +32,8 @@ namespace Jellyfin.Model.Tests [InlineData("Chrome", "mkv-vp9-vorbis-vtt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 // Firefox [InlineData("Firefox", "mp4-h264-aac-vtt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Firefox", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Firefox", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectPlay)] // #6450 + [InlineData("Firefox", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 + [InlineData("Firefox", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Firefox", "mp4-h264-ac3-aacExt-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioIsExternal)] // #6450 [InlineData("Firefox", "mp4-h264-ac3-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioCodecNotSupported)] // #6450 [InlineData("Firefox", "mp4-hevc-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.VideoCodecNotSupported, "Transcode")] @@ -59,11 +59,11 @@ namespace Jellyfin.Model.Tests [InlineData("AndroidPixel", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.ContainerBitrateExceedsLimit, "Transcode")] // Yatse [InlineData("Yatse", "mp4-h264-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 - [InlineData("Yatse", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 - [InlineData("Yatse", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 + [InlineData("Yatse", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 + [InlineData("Yatse", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Yatse", "mp4-h264-ac3-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioCodecNotSupported)] [InlineData("Yatse", "mp4-hevc-aac-srt-15200k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 - [InlineData("Yatse", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 + [InlineData("Yatse", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 // RokuSSPlus [InlineData("RokuSSPlus", "mp4-h264-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 [InlineData("RokuSSPlus", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 should be DirectPlay @@ -83,8 +83,8 @@ namespace Jellyfin.Model.Tests [InlineData("JellyfinMediaPlayer", "mkv-vp9-vorbis-vtt-2600k", PlayMethod.DirectPlay)] // #6450 // Chrome-NoHLS [InlineData("Chrome-NoHLS", "mp4-h264-aac-vtt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Chrome-NoHLS", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay)] // #6450 - [InlineData("Chrome-NoHLS", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectPlay)] // #6450 + [InlineData("Chrome-NoHLS", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 + [InlineData("Chrome-NoHLS", "mp4-h264-ac3-aacDef-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Chrome-NoHLS", "mp4-h264-ac3-aacExt-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioIsExternal)] // #6450 [InlineData("Chrome-NoHLS", "mp4-h264-ac3-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioCodecNotSupported)] // #6450 [InlineData("Chrome-NoHLS", "mp4-hevc-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.VideoCodecNotSupported, "Transcode", "http")] @@ -273,15 +273,15 @@ namespace Jellyfin.Model.Tests [Theory] // Chrome - [InlineData("Chrome", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay)] // #6450 + [InlineData("Chrome", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Chrome", "mp4-h264-ac3-aacExt-srt-2600k", PlayMethod.DirectStream, TranscodeReason.AudioIsExternal)] // #6450 [InlineData("Chrome", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.VideoCodecNotSupported, "Transcode")] // Firefox - [InlineData("Firefox", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay)] // #6450 + [InlineData("Firefox", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 [InlineData("Firefox", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.Transcode, TranscodeReason.VideoCodecNotSupported, "Transcode")] // Yatse - [InlineData("Yatse", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 - [InlineData("Yatse", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 + [InlineData("Yatse", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 + [InlineData("Yatse", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.DirectStream, TranscodeReason.SecondaryAudioNotSupported, "Remux")] // #6450 // RokuSSPlus [InlineData("RokuSSPlus", "mp4-h264-ac3-aac-srt-2600k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 [InlineData("RokuSSPlus", "mp4-hevc-ac3-aac-srt-15200k", PlayMethod.DirectPlay, (TranscodeReason)0, "Remux")] // #6450 -- cgit v1.2.3 From ba3e7027fee956f695c1b659857aa709eb0b9057 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Sat, 5 Nov 2022 14:11:49 +0100 Subject: Add regression test for #8696 --- .../LiveTv/Listings/XmlTvListingsProvider.cs | 5 +- .../LiveTv/Listings/XmlTvListingsProviderTests.cs | 70 ++++++++++++++++++++++ .../Test Data/LiveTv/Listings/XmlTv/notitle.xml | 10 ++++ 3 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/LiveTv/Listings/XmlTvListingsProviderTests.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Test Data/LiveTv/Listings/XmlTv/notitle.xml (limited to 'tests') diff --git a/Emby.Server.Implementations/LiveTv/Listings/XmlTvListingsProvider.cs b/Emby.Server.Implementations/LiveTv/Listings/XmlTvListingsProvider.cs index e35ba15f61..82f0baf32e 100644 --- a/Emby.Server.Implementations/LiveTv/Listings/XmlTvListingsProvider.cs +++ b/Emby.Server.Implementations/LiveTv/Listings/XmlTvListingsProvider.cs @@ -32,18 +32,15 @@ namespace Emby.Server.Implementations.LiveTv.Listings private readonly IServerConfigurationManager _config; private readonly IHttpClientFactory _httpClientFactory; private readonly ILogger _logger; - private readonly IFileSystem _fileSystem; public XmlTvListingsProvider( IServerConfigurationManager config, IHttpClientFactory httpClientFactory, - ILogger logger, - IFileSystem fileSystem) + ILogger logger) { _config = config; _httpClientFactory = httpClientFactory; _logger = logger; - _fileSystem = fileSystem; } public string Name => "XmlTV"; diff --git a/tests/Jellyfin.Server.Implementations.Tests/LiveTv/Listings/XmlTvListingsProviderTests.cs b/tests/Jellyfin.Server.Implementations.Tests/LiveTv/Listings/XmlTvListingsProviderTests.cs new file mode 100644 index 0000000000..82ce8fc4ec --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/LiveTv/Listings/XmlTvListingsProviderTests.cs @@ -0,0 +1,70 @@ +using System; +using System.IO; +using System.Linq; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using AutoFixture; +using AutoFixture.AutoMoq; +using Emby.Server.Implementations.LiveTv.Listings; +using MediaBrowser.Model.LiveTv; +using Moq; +using Moq.Protected; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.LiveTv.Listings; + +public class XmlTvListingsProviderTests +{ + private readonly Fixture _fixture; + private readonly XmlTvListingsProvider _xmlTvListingsProvider; + + public XmlTvListingsProviderTests() + { + var messageHandler = new Mock(); + messageHandler.Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns( + (m, _) => + { + return Task.FromResult(new HttpResponseMessage() + { + Content = new StreamContent(File.OpenRead(Path.Combine("Test Data/LiveTv/Listings/XmlTv", m.RequestUri!.Segments[^1]))) + }); + }); + + var http = new Mock(); + http.Setup(x => x.CreateClient(It.IsAny())) + .Returns(new HttpClient(messageHandler.Object)); + _fixture = new Fixture(); + _fixture.Customize(new AutoMoqCustomization + { + ConfigureMembers = true + }).Inject(http); + _xmlTvListingsProvider = _fixture.Create(); + } + + [Theory] + [InlineData("Test Data/LiveTv/Listings/XmlTv/notitle.xml")] + [InlineData("https://example.com/notitle.xml")] + public async Task GetProgramsAsync_NoTitle_Success(string path) + { + var info = new ListingsProviderInfo() + { + Path = path + }; + + var startDate = new DateTime(2022, 11, 4); + var programs = await _xmlTvListingsProvider.GetProgramsAsync(info, "3297", startDate, startDate.AddDays(1), CancellationToken.None); + var programsList = programs.ToList(); + Assert.Single(programsList); + var program = programsList[0]; + Assert.Null(program.Name); + Assert.Null(program.SeriesId); + Assert.Null(program.EpisodeTitle); + Assert.True(program.IsSports); + Assert.True(program.HasImage); + Assert.Equal("https://domain.tld/image.png", program.ImageUrl); + Assert.Equal("3297", program.ChannelId); + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Test Data/LiveTv/Listings/XmlTv/notitle.xml b/tests/Jellyfin.Server.Implementations.Tests/Test Data/LiveTv/Listings/XmlTv/notitle.xml new file mode 100644 index 0000000000..5a5be79976 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Test Data/LiveTv/Listings/XmlTv/notitle.xml @@ -0,0 +1,10 @@ + + + sports + 2022-11-04 13:00:00 + + + + -- cgit v1.2.3 From 6252bc399a3a56fc684f11523218093f8ff5f2b0 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:59:50 +0100 Subject: Fix unit tests after merge from master Co-authored-by: Bond-009 --- .../Manager/ProviderManagerTests.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs index a5673ad838..725e295b9a 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs @@ -30,7 +30,7 @@ namespace Jellyfin.Providers.Tests.Manager { private static readonly ILogger _logger = new NullLogger(); - private static TheoryData[], int> RefreshSingleItemOrderData() + public static TheoryData[], int> RefreshSingleItemOrderData() => new() { // no order set, uses provided order @@ -74,7 +74,7 @@ namespace Jellyfin.Providers.Tests.Manager [Theory] [MemberData(nameof(RefreshSingleItemOrderData))] - public void RefreshSingleItem_ServiceOrdering_FollowsPriority(Mock[] servicesList, int expectedIndex) + public async Task RefreshSingleItem_ServiceOrdering_FollowsPriority(Mock[] servicesList, int expectedIndex) { var item = new Movie(); @@ -82,9 +82,9 @@ namespace Jellyfin.Providers.Tests.Manager AddParts(providerManager, metadataServices: servicesList.Select(s => s.Object).ToArray()); var refreshOptions = new MetadataRefreshOptions(Mock.Of(MockBehavior.Strict)); - var actual = providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None); + var actual = await providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None).ConfigureAwait(false); - Assert.Equal(ItemUpdateType.MetadataDownload, actual.Result); + Assert.Equal(ItemUpdateType.MetadataDownload, actual); for (var i = 0; i < servicesList.Length; i++) { var times = i == expectedIndex ? Times.Once() : Times.Never(); @@ -95,7 +95,7 @@ namespace Jellyfin.Providers.Tests.Manager [Theory] [InlineData(true)] [InlineData(false)] - public void RefreshSingleItem_RefreshMetadata_WhenServiceFound(bool serviceFound) + public async Task RefreshSingleItem_RefreshMetadata_WhenServiceFound(bool serviceFound) { var item = new Movie(); @@ -105,13 +105,13 @@ namespace Jellyfin.Providers.Tests.Manager AddParts(providerManager, metadataServices: servicesList.Select(s => s.Object).ToArray()); var refreshOptions = new MetadataRefreshOptions(Mock.Of(MockBehavior.Strict)); - var actual = providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None); + var actual = await providerManager.RefreshSingleItem(item, refreshOptions, CancellationToken.None).ConfigureAwait(false); var expectedResult = serviceFound ? ItemUpdateType.MetadataDownload : ItemUpdateType.None; - Assert.Equal(expectedResult, actual.Result); + Assert.Equal(expectedResult, actual); } - private static TheoryData GetImageProvidersOrderData() + public static TheoryData GetImageProvidersOrderData() => new() { { 3, null, null, null, new[] { 0, 1, 2 } }, // no order options set @@ -236,7 +236,7 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal(expected ? 1 : 0, actualProviders.Length); } - private static TheoryData GetMetadataProvidersOrderData() + public static TheoryData GetMetadataProvidersOrderData() { var l = nameof(ILocalMetadataProvider); var r = nameof(IRemoteMetadataProvider); -- cgit v1.2.3 From 556cc8062debd5370ef907b0c78e8636356a8068 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Wed, 23 Nov 2022 15:58:11 +0100 Subject: Investigate some TODO comments --- .gitignore | 2 -- Emby.Dlna/PlayTo/PlayToController.cs | 1 - .../Library/Resolvers/PlaylistResolver.cs | 20 +++++++++++--------- .../ScheduledTasks/Tasks/ChapterImagesTask.cs | 14 +++++++------- .../MediaEncoding/EncodingHelper.cs | 19 ------------------- .../Net/BasePeriodicWebSocketListener.cs | 3 ++- src/Jellyfin.Extensions/AlphanumericComparator.cs | 2 -- .../Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs | 12 ++++++------ 8 files changed, 26 insertions(+), 47 deletions(-) (limited to 'tests') diff --git a/.gitignore b/.gitignore index c2ae76c1e3..9e9fae7bfc 100644 --- a/.gitignore +++ b/.gitignore @@ -150,8 +150,6 @@ publish/ *.pubxml # NuGet Packages Directory -## TODO: If you have NuGet Package Restore enabled, uncomment the next line -# packages/ dlls/ dllssigned/ diff --git a/Emby.Dlna/PlayTo/PlayToController.cs b/Emby.Dlna/PlayTo/PlayToController.cs index b73ce00b6f..65367e24f2 100644 --- a/Emby.Dlna/PlayTo/PlayToController.cs +++ b/Emby.Dlna/PlayTo/PlayToController.cs @@ -338,7 +338,6 @@ namespace Emby.Dlna.PlayTo SubtitleStreamIndex = info.SubtitleStreamIndex, VolumeLevel = _device.Volume, - // TODO CanSeek = true, PlayMethod = info.IsDirectStream ? PlayMethod.DirectStream : PlayMethod.Transcode diff --git a/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs b/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs index 6b0dfe9864..7a2b3da3a9 100644 --- a/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs +++ b/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs @@ -31,16 +31,18 @@ namespace Emby.Server.Implementations.Library.Resolvers if (args.IsDirectory) { // It's a boxset if the path is a directory with [playlist] in it's the name - // TODO: Should this use Path.GetDirectoryName() instead? - bool isBoxSet = Path.GetFileName(args.Path) - ?.Contains("[playlist]", StringComparison.OrdinalIgnoreCase) - ?? false; - if (isBoxSet) + var filename = Path.GetFileName(Path.TrimEndingDirectorySeparator(args.Path)); + if (string.IsNullOrEmpty(filename)) + { + return null; + } + + if (filename.Contains("[playlist]", StringComparison.OrdinalIgnoreCase)) { return new Playlist { Path = args.Path, - Name = Path.GetFileName(args.Path).Replace("[playlist]", string.Empty, StringComparison.OrdinalIgnoreCase).Trim() + Name = filename.Replace("[playlist]", string.Empty, StringComparison.OrdinalIgnoreCase).Trim() }; } @@ -51,7 +53,7 @@ namespace Emby.Server.Implementations.Library.Resolvers return new Playlist { Path = args.Path, - Name = Path.GetFileName(args.Path) + Name = filename }; } } @@ -60,8 +62,8 @@ namespace Emby.Server.Implementations.Library.Resolvers // It should have the correct collection type and a supported file extension else if (_musicPlaylistCollectionTypes.Contains(args.CollectionType ?? string.Empty, StringComparison.OrdinalIgnoreCase)) { - var extension = Path.GetExtension(args.Path); - if (Playlist.SupportedExtensions.Contains(extension ?? string.Empty, StringComparison.OrdinalIgnoreCase)) + var extension = Path.GetExtension(args.Path.AsSpan()); + if (Playlist.SupportedExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase)) { return new Playlist { diff --git a/Emby.Server.Implementations/ScheduledTasks/Tasks/ChapterImagesTask.cs b/Emby.Server.Implementations/ScheduledTasks/Tasks/ChapterImagesTask.cs index 0bf0838fac..6106ae6c40 100644 --- a/Emby.Server.Implementations/ScheduledTasks/Tasks/ChapterImagesTask.cs +++ b/Emby.Server.Implementations/ScheduledTasks/Tasks/ChapterImagesTask.cs @@ -16,6 +16,7 @@ using MediaBrowser.Model.Entities; using MediaBrowser.Model.Globalization; using MediaBrowser.Model.IO; using MediaBrowser.Model.Tasks; +using Microsoft.Extensions.Logging; namespace Emby.Server.Implementations.ScheduledTasks.Tasks { @@ -24,15 +25,10 @@ namespace Emby.Server.Implementations.ScheduledTasks.Tasks /// public class ChapterImagesTask : IScheduledTask { - /// - /// The _library manager. - /// + private readonly ILogger _logger; private readonly ILibraryManager _libraryManager; - private readonly IItemRepository _itemRepo; - private readonly IApplicationPaths _appPaths; - private readonly IEncodingManager _encodingManager; private readonly IFileSystem _fileSystem; private readonly ILocalizationManager _localization; @@ -40,6 +36,7 @@ namespace Emby.Server.Implementations.ScheduledTasks.Tasks /// /// Initializes a new instance of the class. /// + /// The logger.. /// The library manager.. /// The item repository. /// The application paths. @@ -47,6 +44,7 @@ namespace Emby.Server.Implementations.ScheduledTasks.Tasks /// The filesystem. /// The localization manager. public ChapterImagesTask( + ILogger logger, ILibraryManager libraryManager, IItemRepository itemRepo, IApplicationPaths appPaths, @@ -54,6 +52,7 @@ namespace Emby.Server.Implementations.ScheduledTasks.Tasks IFileSystem fileSystem, ILocalizationManager localization) { + _logger = logger; _libraryManager = libraryManager; _itemRepo = itemRepo; _appPaths = appPaths; @@ -167,9 +166,10 @@ namespace Emby.Server.Implementations.ScheduledTasks.Tasks progress.Report(100 * percent); } - catch (ObjectDisposedException) + catch (ObjectDisposedException ex) { // TODO Investigate and properly fix. + _logger.LogError(ex, "Object Disposed"); break; } } diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index cee08eedac..74abb91b28 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -1176,24 +1176,6 @@ namespace MediaBrowser.Controller.MediaEncoding ":fontsdir='{0}'", _mediaEncoder.EscapeSubtitleFilterPath(fontPath)); - // TODO - // var fallbackFontPath = Path.Combine(_appPaths.ProgramDataPath, "fonts", "DroidSansFallback.ttf"); - // string fallbackFontParam = string.Empty; - - // if (!File.Exists(fallbackFontPath)) - // { - // _fileSystem.CreateDirectory(_fileSystem.GetDirectoryName(fallbackFontPath)); - // using (var stream = _assemblyInfo.GetManifestResourceStream(GetType(), GetType().Namespace + ".DroidSansFallback.ttf")) - // { - // using (var fileStream = new FileStream(fallbackFontPath, FileMode.Create, FileAccess.Write, FileShare.Read)) - // { - // stream.CopyTo(fileStream); - // } - // } - // } - - // fallbackFontParam = string.Format(CultureInfo.InvariantCulture, ":force_style='FontName=Droid Sans Fallback':fontsdir='{0}'", _mediaEncoder.EscapeSubtitleFilterPath(_fileSystem.GetDirectoryName(fallbackFontPath))); - if (state.SubtitleStream.IsExternal) { var charsetParam = string.Empty; @@ -1221,7 +1203,6 @@ namespace MediaBrowser.Controller.MediaEncoding alphaParam, sub2videoParam, fontParam, - // fallbackFontParam, setPtsParam); } diff --git a/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs b/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs index 647de50030..2fe3a54723 100644 --- a/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs +++ b/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs @@ -227,9 +227,10 @@ namespace MediaBrowser.Controller.Net connection.Item2.Cancel(); connection.Item2.Dispose(); } - catch (ObjectDisposedException) + catch (ObjectDisposedException ex) { // TODO Investigate and properly fix. + Logger.LogError(ex, "Object Disposed"); } lock (_activeConnections) diff --git a/src/Jellyfin.Extensions/AlphanumericComparator.cs b/src/Jellyfin.Extensions/AlphanumericComparator.cs index e3c81eba8c..98a32d5b20 100644 --- a/src/Jellyfin.Extensions/AlphanumericComparator.cs +++ b/src/Jellyfin.Extensions/AlphanumericComparator.cs @@ -128,9 +128,7 @@ namespace Jellyfin.Extensions return result; } } -#pragma warning disable SA1500 // TODO remove with StyleCop.Analyzers v1.2.0 https://github.com/DotNetAnalyzers/StyleCopAnalyzers/pull/3196 } while (pos1 < len1 && pos2 < len2); -#pragma warning restore SA1500 return len1 - len2; } diff --git a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs index c279b6b4bb..e1bd2fe0f3 100644 --- a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs +++ b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs @@ -359,7 +359,7 @@ namespace Jellyfin.Model.Tests Assert.Single(val.TargetAudioCodec); // Assert.Single(val.AudioCodecs); - if (transcodeMode == "DirectStream") + if (transcodeMode.Equals("DirectStream", StringComparison.Ordinal)) { Assert.Equal(val.Container, uri.Extension); } @@ -371,14 +371,14 @@ namespace Jellyfin.Model.Tests Assert.NotEmpty(val.AudioCodecs); // Check expected container (todo: this could be a test param) - if (transcodeProtocol == "http") + if (transcodeProtocol.Equals("http", StringComparison.Ordinal)) { // Assert.Equal("webm", val.Container); Assert.Equal(val.Container, uri.Extension); Assert.Equal("stream", uri.Filename); Assert.Equal("http", val.SubProtocol); } - else if (transcodeProtocol == "HLS.mp4") + else if (transcodeProtocol.Equals("HLS.mp4", StringComparison.Ordinal)) { Assert.Equal("mp4", val.Container); Assert.Equal("m3u8", uri.Extension); @@ -394,7 +394,7 @@ namespace Jellyfin.Model.Tests } // Full transcode - if (transcodeMode == "Transcode") + if (transcodeMode.Equals("Transcode", StringComparison.Ordinal)) { if ((val.TranscodeReasons & (StreamBuilder.ContainerReasons | TranscodeReason.DirectPlayError)) == 0) { @@ -413,7 +413,7 @@ namespace Jellyfin.Model.Tests Assert.Contains(targetVideoStream.Codec, val.TargetVideoCodec); Assert.Single(val.TargetVideoCodec); - if (transcodeMode == "DirectStream") + if (transcodeMode.Equals("DirectStream", StringComparison.Ordinal)) { // Check expected audio codecs (1) if (!targetAudioStream.IsExternal) @@ -428,7 +428,7 @@ namespace Jellyfin.Model.Tests } } } - else if (transcodeMode == "Remux") + else if (transcodeMode.Equals("Remux", StringComparison.Ordinal)) { // Check expected audio codecs (1) Assert.Contains(targetAudioStream.Codec, val.AudioCodecs); -- cgit v1.2.3 From 9c1da522c66aa4ec5e46eea943dd1ef60531bfa6 Mon Sep 17 00:00:00 2001 From: Bond-009 Date: Sun, 27 Nov 2022 14:49:21 +0100 Subject: Fix last CA1305 error (#8806) --- .../Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs b/tests/Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs index 48c49bf848..c38faeda17 100644 --- a/tests/Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs +++ b/tests/Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Globalization; using System.IO; using System.Threading; using Emby.Server.Implementations; @@ -28,7 +29,9 @@ namespace Jellyfin.Server.Integration.Tests static JellyfinApplicationFactory() { // Perform static initialization that only needs to happen once per test-run - Log.Logger = new LoggerConfiguration().WriteTo.Console().CreateLogger(); + Log.Logger = new LoggerConfiguration() + .WriteTo.Console(formatProvider: CultureInfo.InvariantCulture) + .CreateLogger(); Program.PerformStaticInitialization(); } -- cgit v1.2.3