diff options
| -rw-r--r-- | MediaBrowser.Providers/Manager/ProviderManager.cs | 60 | ||||
| -rw-r--r-- | tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs | 47 |
2 files changed, 40 insertions, 67 deletions
diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 1b7357477..855c46720 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -310,31 +310,25 @@ namespace MediaBrowser.Providers.Manager private IEnumerable<IImageProvider> 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; } /// <inheritdoc /> @@ -450,21 +444,6 @@ namespace MediaBrowser.Providers.Manager } } - /// <summary> - /// Gets the order. - /// </summary> - /// <param name="provider">The provider.</param> - /// <returns>System.Int32.</returns> - 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 31b191334..590f50b25 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<int, bool[]?, int[]?, int[]?, int?[]?, int[]> GetImageProvidersOrderData() + private static TheoryData<int, int[]?, int[]?, int?[]?, int[]> 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<int>(), 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<int>(), 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<int>(), 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<int>(), 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<ILocalImageProvider>(nameProvider(i), item, order)); - } - else - { - providerList.Add(MockIImageProvider<IImageProvider>(nameProvider(i), item, order)); - } + providerList.Add(MockIImageProvider<IImageProvider>(nameProvider(i), item, order)); } var libraryOptions = new LibraryOptions(); |