From 046dd7fa60f2c539e94baf8150a2bdfb67d42e7b Mon Sep 17 00:00:00 2001 From: Claus Vium Date: Wed, 3 Mar 2021 17:24:58 +0100 Subject: [PATCH] Merge pull request #5356 from cvium/fix_provideridextensions return false when providerid is null or empty (cherry picked from commit ddc62a89bab6b5a1ecdee7a9ebc7f9d5887d8be8) Signed-off-by: Joshua M. Boniface --- .../Entities/ProviderIdsExtensions.cs | 12 ++++++-- .../Manager/ProviderManager.cs | 8 ++--- .../Entities/ProviderIdsExtensionsTests.cs | 29 +++++++++++++++---- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs index 16e1d49c76..512a5473f4 100644 --- a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs +++ b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs @@ -22,7 +22,7 @@ namespace MediaBrowser.Model.Entities throw new ArgumentNullException(nameof(instance)); } - return instance.ProviderIds?.ContainsKey(name) ?? false; + return instance.TryGetProviderId(name, out _); } /// @@ -56,7 +56,15 @@ namespace MediaBrowser.Model.Entities return false; } - return instance.ProviderIds.TryGetValue(name, out id); + var foundProviderId = instance.ProviderIds.TryGetValue(name, out id); + // This occurs when searching with Identify (and possibly in other places) + if (string.IsNullOrEmpty(id)) + { + id = null; + foundProviderId = false; + } + + return foundProviderId; } /// diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index a20c47cf2e..b11988f531 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -869,14 +869,14 @@ namespace MediaBrowser.Providers.Manager } } } - catch (Exception) +#pragma warning disable CA1031 // do not catch general exception types + catch (Exception ex) +#pragma warning restore CA1031 // do not catch general exception types { - // Logged at lower levels + _logger.LogError(ex, "Provider {ProviderName} failed to retrieve search results", provider.Name); } } - // _logger.LogDebug("Returning search results {0}", _json.SerializeToString(resultList)); - return resultList; } diff --git a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs index 2b2414ef1b..a1ace84769 100644 --- a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs +++ b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs @@ -18,7 +18,7 @@ namespace Jellyfin.Model.Tests.Entities [Fact] public void HasProviderId_NullProvider_False() { - var nullProvider = new ProviderIdsExtensionsTestsObject() + var nullProvider = new ProviderIdsExtensionsTestsObject { ProviderIds = null! }; @@ -47,6 +47,15 @@ namespace Jellyfin.Model.Tests.Entities Assert.True(provider.HasProviderId(MetadataProvider.Imdb)); } + [Fact] + public void HasProviderId_FoundNameEmptyValue_False() + { + var provider = new ProviderIdsExtensionsTestsObject(); + provider.ProviderIds[MetadataProvider.Imdb.ToString()] = string.Empty; + + Assert.False(provider.HasProviderId(MetadataProvider.Imdb)); + } + [Fact] public void GetProviderId_NullInstance_ThrowsArgumentNullException() { @@ -68,7 +77,7 @@ namespace Jellyfin.Model.Tests.Entities [Fact] public void GetProviderId_NullProvider_Null() { - var nullProvider = new ProviderIdsExtensionsTestsObject() + var nullProvider = new ProviderIdsExtensionsTestsObject { ProviderIds = null! }; @@ -85,7 +94,7 @@ namespace Jellyfin.Model.Tests.Entities [Fact] public void TryGetProviderId_NullProvider_False() { - var nullProvider = new ProviderIdsExtensionsTestsObject() + var nullProvider = new ProviderIdsExtensionsTestsObject { ProviderIds = null! }; @@ -112,6 +121,16 @@ namespace Jellyfin.Model.Tests.Entities Assert.Equal(ExampleImdbId, id); } + [Fact] + public void TryGetProviderId_FoundNameEmptyValue_False() + { + var provider = new ProviderIdsExtensionsTestsObject(); + provider.ProviderIds[MetadataProvider.Imdb.ToString()] = string.Empty; + + Assert.False(provider.TryGetProviderId(MetadataProvider.Imdb, out var id)); + Assert.Null(id); + } + [Fact] public void SetProviderId_NullInstance_ThrowsArgumentNullException() { @@ -146,7 +165,7 @@ namespace Jellyfin.Model.Tests.Entities [Fact] public void SetProviderId_NullProvider_Success() { - var nullProvider = new ProviderIdsExtensionsTestsObject() + var nullProvider = new ProviderIdsExtensionsTestsObject { ProviderIds = null! }; @@ -158,7 +177,7 @@ namespace Jellyfin.Model.Tests.Entities [Fact] public void SetProviderId_NullProviderAndEmptyName_Success() { - var nullProvider = new ProviderIdsExtensionsTestsObject() + var nullProvider = new ProviderIdsExtensionsTestsObject { ProviderIds = null! };