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 index = Array.IndexOf(order, providerName); + + if (index != -1) { - var configuredOrder = libraryOptions.LocalMetadataReaderOrder ?? globalMetadataOptions.LocalMetadataReaderOrder; - - var index = Array.IndexOf(configuredOrder, provider.Name); - - if (index != -1) - { - return index; - } + return index; } - // See if there's a user-defined order - if (provider is IRemoteMetadataProvider) - { - 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; - } - } - - // 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);