aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCody Robibero <cody@robibe.ro>2026-08-22 08:49:07 -0400
committerGitHub <noreply@github.com>2026-08-22 08:49:07 -0400
commit49112163ebb5cb6fbec37c3b7b3e9a4875f47940 (patch)
tree1fc16ae1bd610095efc019e546ddd7faee3c694a
parent0d4cbb999bbac9920caeeb5cd7174b1dc9758bff (diff)
parent42c70fba63d72a4113a7b8f2e06293e512cc6914 (diff)
Merge pull request #17584 from Shadowghost/safeguard-invalid-provider-ids
Safeguard against invalid provider ids
-rw-r--r--Jellyfin.Api/Controllers/ItemLookupController.cs3
-rw-r--r--Jellyfin.Api/Controllers/ItemUpdateController.cs10
-rw-r--r--MediaBrowser.Model/Entities/ProviderIdsExtensions.cs89
-rw-r--r--MediaBrowser.Providers/Manager/MetadataService.cs157
-rw-r--r--MediaBrowser.Providers/Music/AlbumInfoExtensions.cs21
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetImageProvider.cs4
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetProvider.cs5
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieImageProvider.cs4
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieProvider.cs24
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonImageProvider.cs5
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonProvider.cs9
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeImageProvider.cs6
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeProvider.cs3
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonImageProvider.cs6
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonProvider.cs5
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesImageProvider.cs8
-rwxr-xr-xMediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesProvider.cs4
-rw-r--r--MediaBrowser.Providers/Plugins/Tmdb/TmdbUtils.cs28
-rw-r--r--tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs103
-rw-r--r--tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs253
-rw-r--r--tests/Jellyfin.Providers.Tests/Music/AlbumInfoExtensionsTests.cs59
-rw-r--r--tests/Jellyfin.Providers.Tests/Tmdb/TmdbUtilsTests.cs37
22 files changed, 759 insertions, 84 deletions
diff --git a/Jellyfin.Api/Controllers/ItemLookupController.cs b/Jellyfin.Api/Controllers/ItemLookupController.cs
index d009f80a96..39ba5ab186 100644
--- a/Jellyfin.Api/Controllers/ItemLookupController.cs
+++ b/Jellyfin.Api/Controllers/ItemLookupController.cs
@@ -13,6 +13,7 @@ using MediaBrowser.Controller.Entities.Movies;
using MediaBrowser.Controller.Entities.TV;
using MediaBrowser.Controller.Library;
using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Entities;
using MediaBrowser.Model.IO;
using MediaBrowser.Model.Providers;
using Microsoft.AspNetCore.Authorization;
@@ -263,7 +264,7 @@ public class ItemLookupController : BaseJellyfinApiController
searchResult.ProviderIds);
// Since the refresh process won't erase provider Ids, we need to set this explicitly now.
- item.ProviderIds = searchResult.ProviderIds;
+ item.SetProviderIds(searchResult.ProviderIds);
await _providerManager.RefreshFullItem(
item,
new MetadataRefreshOptions(new DirectoryService(_fileSystem))
diff --git a/Jellyfin.Api/Controllers/ItemUpdateController.cs b/Jellyfin.Api/Controllers/ItemUpdateController.cs
index 36c82cf461..65fffc4181 100644
--- a/Jellyfin.Api/Controllers/ItemUpdateController.cs
+++ b/Jellyfin.Api/Controllers/ItemUpdateController.cs
@@ -428,15 +428,7 @@ public class ItemUpdateController : BaseJellyfinApiController
if (request.ProviderIds is not null)
{
- foreach (var pair in request.ProviderIds.ToList())
- {
- if (string.IsNullOrEmpty(pair.Value))
- {
- request.ProviderIds.Remove(pair.Key);
- }
- }
-
- item.ProviderIds = request.ProviderIds;
+ item.SetProviderIds(request.ProviderIds);
}
if (item is Video video)
diff --git a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs
index 385a86d31c..09eba92d9e 100644
--- a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs
+++ b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs
@@ -1,14 +1,16 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
using System.Linq;
+using System.Text.RegularExpressions;
namespace MediaBrowser.Model.Entities;
/// <summary>
/// Class ProviderIdsExtensions.
/// </summary>
-public static class ProviderIdsExtensions
+public static partial class ProviderIdsExtensions
{
/// <summary>
/// Case-insensitive dictionary of <see cref="MetadataProvider"/> string representation.
@@ -21,6 +23,27 @@ public static class ProviderIdsExtensions
StringComparer.OrdinalIgnoreCase);
/// <summary>
+ /// The known id formats, keyed by provider name.
+ /// </summary>
+ private static readonly Dictionary<string, Func<string, bool>> _providerIdValidators =
+ new(StringComparer.OrdinalIgnoreCase)
+ {
+ [MetadataProvider.Imdb.ToString()] = value => ImdbIdRegex().IsMatch(value),
+ [MetadataProvider.Tmdb.ToString()] = IsPositiveNumber,
+ [MetadataProvider.TmdbCollection.ToString()] = IsPositiveNumber,
+ [MetadataProvider.AudioDbArtist.ToString()] = IsPositiveNumber,
+ [MetadataProvider.AudioDbAlbum.ToString()] = IsPositiveNumber,
+
+ // Every MusicBrainz id is an MBID.
+ [MetadataProvider.MusicBrainzAlbum.ToString()] = IsGuid,
+ [MetadataProvider.MusicBrainzAlbumArtist.ToString()] = IsGuid,
+ [MetadataProvider.MusicBrainzArtist.ToString()] = IsGuid,
+ [MetadataProvider.MusicBrainzReleaseGroup.ToString()] = IsGuid,
+ [MetadataProvider.MusicBrainzRecording.ToString()] = IsGuid,
+ [MetadataProvider.MusicBrainzTrack.ToString()] = IsGuid
+ };
+
+ /// <summary>
/// Checks if this instance has an id for the given provider.
/// </summary>
/// <param name="instance">The instance.</param>
@@ -102,6 +125,26 @@ public static class ProviderIdsExtensions
}
/// <summary>
+ /// Checks whether a value can be an id of the given provider.
+ /// </summary>
+ /// <param name="name">The provider name.</param>
+ /// <param name="value">The provider id.</param>
+ /// <returns><c>true</c> if the value has a plausible format for the provider; otherwise, <c>false</c>.</returns>
+ /// <remarks>
+ /// Providers regularly hand out an id belonging to a different service, e.g. an IMDb person id in the
+ /// TMDb field. Such an id is not just useless, it also makes the owning provider fail for the item.
+ /// </remarks>
+ public static bool IsValidProviderId(string? name, string? value)
+ {
+ if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(value))
+ {
+ return false;
+ }
+
+ return !_providerIdValidators.TryGetValue(name, out var isValid) || isValid(value);
+ }
+
+ /// <summary>
/// Sets a provider id.
/// </summary>
/// <param name="instance">The instance.</param>
@@ -121,6 +164,14 @@ public static class ProviderIdsExtensions
return false;
}
+ name = name.Trim();
+ value = value.Trim();
+
+ if (!IsValidProviderId(name, value))
+ {
+ return false;
+ }
+
// Ensure it exists
instance.ProviderIds ??= new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
@@ -153,7 +204,6 @@ public static class ProviderIdsExtensions
/// <param name="instance">The instance.</param>
/// <param name="name">The name, this should not contain a '=' character.</param>
/// <param name="value">The value.</param>
- /// <remarks>Due to how deserialization from the database works the name cannot contain '='.</remarks>
public static void SetProviderId(this IHasProviderIds instance, string name, string value)
{
ArgumentNullException.ThrowIfNull(instance);
@@ -166,17 +216,27 @@ public static class ProviderIdsExtensions
throw new ArgumentException("Provider id name cannot contain '='", nameof(name));
}
- // Ensure it exists
- instance.ProviderIds ??= new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+ instance.TrySetProviderId(name, value);
+ }
- // Match on internal MetadataProvider enum string values before adding arbitrary providers
- if (_metadataProviderEnumDictionary.TryGetValue(name, out var enumValue))
+ /// <summary>
+ /// Replaces all provider ids, dropping the ones that cannot belong to the provider they are filed under.
+ /// </summary>
+ /// <param name="instance">The instance.</param>
+ /// <param name="providerIds">The provider ids to set.</param>
+ public static void SetProviderIds(this IHasProviderIds instance, IReadOnlyDictionary<string, string>? providerIds)
+ {
+ ArgumentNullException.ThrowIfNull(instance);
+
+ instance.ProviderIds = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+ if (providerIds is null)
{
- instance.ProviderIds[enumValue] = value;
+ return;
}
- else
+
+ foreach (var (name, value) in providerIds)
{
- instance.ProviderIds[name] = value;
+ instance.TrySetProviderId(name, value);
}
}
@@ -213,4 +273,15 @@ public static class ProviderIdsExtensions
instance.ProviderIds?.Remove(provider.ToString());
}
+
+ private static bool IsPositiveNumber(string value)
+ => int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out var id) && id > 0;
+
+ private static bool IsGuid(string value)
+ => Guid.TryParse(value, CultureInfo.InvariantCulture, out _);
+
+ // An IMDb id is a type prefix (tt for titles, nm for people, co for companies, ...) followed by
+ // digits. The prefix is optional because a bare number has always been accepted for a title.
+ [GeneratedRegex(@"^(tt|nm|co|ev|ch|ni)?[0-9]+$", RegexOptions.IgnoreCase)]
+ private static partial Regex ImdbIdRegex();
}
diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs
index 40f2775bd3..d11db8f531 100644
--- a/MediaBrowser.Providers/Manager/MetadataService.cs
+++ b/MediaBrowser.Providers/Manager/MetadataService.cs
@@ -260,21 +260,40 @@ namespace MediaBrowser.Providers.Manager
switch (lookupInfo)
{
case EpisodeInfo episodeInfo:
- episodeInfo.SeriesProviderIds = result.ProviderIds;
+ episodeInfo.SeriesProviderIds = GetValidProviderIds(result.ProviderIds);
episodeInfo.ProviderIds.Clear();
break;
case SeasonInfo seasonInfo:
- seasonInfo.SeriesProviderIds = result.ProviderIds;
+ seasonInfo.SeriesProviderIds = GetValidProviderIds(result.ProviderIds);
seasonInfo.ProviderIds.Clear();
break;
default:
- lookupInfo.ProviderIds = result.ProviderIds;
+ lookupInfo.SetProviderIds(result.ProviderIds);
lookupInfo.Name = result.Name;
lookupInfo.Year = result.ProductionYear;
break;
}
}
+ private static Dictionary<string, string> GetValidProviderIds(IReadOnlyDictionary<string, string> providerIds)
+ {
+ var validProviderIds = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+ if (providerIds is null)
+ {
+ return validProviderIds;
+ }
+
+ foreach (var (name, value) in providerIds)
+ {
+ if (ProviderIdsExtensions.IsValidProviderId(name, value))
+ {
+ validProviderIds[name] = value;
+ }
+ }
+
+ return validProviderIds;
+ }
+
protected async Task SaveItemAsync(MetadataResult<TItemType> result, ItemUpdateType reason, bool reattachUserData, CancellationToken cancellationToken)
{
await result.Item.UpdateToRepositoryAsync(reason, cancellationToken).ConfigureAwait(false);
@@ -835,6 +854,7 @@ namespace MediaBrowser.Providers.Manager
}
}
+ var hasRemoteMetadata = false;
var isLocalLocked = temp.Item.IsLocked;
if (!isLocalLocked && (options.ReplaceAllMetadata || options.MetadataRefreshMode > MetadataRefreshMode.ValidationOnly))
{
@@ -849,6 +869,7 @@ namespace MediaBrowser.Providers.Manager
var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, remoteProviders, cancellationToken).ConfigureAwait(false);
+ hasRemoteMetadata = remoteResult.UpdateType.HasFlag(ItemUpdateType.MetadataDownload);
refreshResult.UpdateType |= remoteResult.UpdateType;
refreshResult.ErrorMessage = remoteResult.ErrorMessage;
refreshResult.Failures += remoteResult.Failures;
@@ -858,7 +879,12 @@ namespace MediaBrowser.Providers.Manager
{
if (refreshResult.UpdateType > ItemUpdateType.None)
{
- if (!options.RemoveOldMetadata)
+ // Erasing the old values is only safe when a remote provider returned something to
+ // replace them with. If every one of them failed there is no replacement, and wiping the
+ // item would turn a provider being temporarily unreachable into permanent data loss.
+ // A single failure is not enough: Identify asks for the erasure precisely because the
+ // previous match was wrong, and an unrelated provider throwing must not undo that.
+ if (!options.RemoveOldMetadata || (refreshResult.Failures > 0 && !hasRemoteMetadata))
{
// Add existing metadata to provider result if it does not exist there
MergeData(metadata, temp, [], false, false);
@@ -932,6 +958,8 @@ namespace MediaBrowser.Providers.Manager
{
result.Provider = provider.Name;
+ LogInvalidProviderIds(result, providerName, logName);
+
MergeData(result, temp, [], replaceData, false);
MergeNewData(temp.Item, id);
@@ -957,6 +985,58 @@ namespace MediaBrowser.Providers.Manager
return refreshResult;
}
+ /// <summary>
+ /// Reports the ids a provider returned that cannot belong to the provider they are filed under.
+ /// </summary>
+ /// <remarks>
+ /// The ids are dropped when merging, this names the provider that produced them so the source of a
+ /// recurring bad id can be found.
+ /// </remarks>
+ private void LogInvalidProviderIds(MetadataResult<TItemType> result, string providerName, string logName)
+ {
+ if (!Logger.IsEnabled(LogLevel.Debug))
+ {
+ return;
+ }
+
+ LogInvalidProviderIds(result.Item?.ProviderIds, providerName, logName, null);
+
+ if (result.People is null)
+ {
+ return;
+ }
+
+ foreach (var person in result.People)
+ {
+ LogInvalidProviderIds(person.ProviderIds, providerName, logName, person.Name);
+ }
+ }
+
+ private void LogInvalidProviderIds(IReadOnlyDictionary<string, string> providerIds, string providerName, string logName, string personName)
+ {
+ if (providerIds is null)
+ {
+ return;
+ }
+
+ foreach (var (key, value) in providerIds)
+ {
+ if (ProviderIdsExtensions.IsValidProviderId(key, value))
+ {
+ continue;
+ }
+
+ if (personName is null)
+ {
+ Logger.LogDebug("Discarding {Key} id '{Value}' returned by {Provider} for {Item}", key, value, providerName, logName);
+ }
+ else
+ {
+ Logger.LogDebug("Discarding {Key} id '{Value}' returned by {Provider} for {Person} of {Item}", key, value, providerName, personName, logName);
+ }
+ }
+ }
+
private void MergeNewData(TItemType source, TIdType lookupInfo)
{
// Copy new provider id's that may have been obtained
@@ -964,8 +1044,18 @@ namespace MediaBrowser.Providers.Manager
{
var key = providerId.Key;
- // Don't replace existing Id's.
- lookupInfo.ProviderIds.TryAdd(key, providerId.Value);
+ if (!ProviderIdsExtensions.IsValidProviderId(key, providerId.Value))
+ {
+ continue;
+ }
+
+ // Don't replace existing Id's, unless the one already there is unusable - handing that
+ // one to the providers that have yet to run is what makes them fail.
+ if (!lookupInfo.ProviderIds.TryGetValue(key, out var existingId)
+ || !ProviderIdsExtensions.IsValidProviderId(key, existingId))
+ {
+ lookupInfo.ProviderIds[key] = providerId.Value;
+ }
}
}
@@ -1104,6 +1194,9 @@ namespace MediaBrowser.Providers.Manager
if (!lockedFields.Contains(MetadataField.Cast))
{
+ RemoveInvalidProviderIds(sourceResult.People);
+ RemoveInvalidProviderIds(targetResult.People);
+
if (replaceData || targetResult.People is null || targetResult.People.Count == 0)
{
targetResult.People = sourceResult.People;
@@ -1175,17 +1268,33 @@ namespace MediaBrowser.Providers.Manager
{
var key = id.Key;
- // Don't replace existing Id's.
- if (replaceData)
+ // An id that cannot belong to the provider it is filed under only breaks that provider on
+ // the next refresh, so never let one in - not even when replacing all metadata.
+ if (!ProviderIdsExtensions.IsValidProviderId(key, id.Value))
{
- target.ProviderIds[key] = id.Value;
+ continue;
}
- else
+
+ // Don't replace existing Id's, unless the stored one is unusable - that one is the bad
+ // match the refresh is meant to repair.
+ if (replaceData
+ || !target.ProviderIds.TryGetValue(key, out var existingId)
+ || !ProviderIdsExtensions.IsValidProviderId(key, existingId))
{
- target.ProviderIds.TryAdd(key, id.Value);
+ target.ProviderIds[key] = id.Value;
}
}
+ // A bad id no provider offered a replacement for still has to go, otherwise the item keeps
+ // failing the same way on every refresh.
+ foreach (var key in target.ProviderIds
+ .Where(id => !ProviderIdsExtensions.IsValidProviderId(id.Key, id.Value))
+ .Select(id => id.Key)
+ .ToArray())
+ {
+ target.ProviderIds.Remove(key);
+ }
+
if (replaceData || !target.CriticRating.HasValue)
{
target.CriticRating = source.CriticRating;
@@ -1251,6 +1360,32 @@ namespace MediaBrowser.Providers.Manager
}
}
+ private static void RemoveInvalidProviderIds(IReadOnlyList<PersonInfo> people)
+ {
+ if (people is null)
+ {
+ return;
+ }
+
+ foreach (var person in people)
+ {
+ if (person.ProviderIds is null || person.ProviderIds.Count == 0)
+ {
+ continue;
+ }
+
+ var invalidKeys = person.ProviderIds
+ .Where(id => !ProviderIdsExtensions.IsValidProviderId(id.Key, id.Value))
+ .Select(id => id.Key)
+ .ToArray();
+
+ foreach (var key in invalidKeys)
+ {
+ person.ProviderIds.Remove(key);
+ }
+ }
+ }
+
private static void MergePeople(IReadOnlyList<PersonInfo> source, IReadOnlyList<PersonInfo> target)
{
var sourceByName = source.ToLookup(p => p.Name.RemoveDiacritics(), StringComparer.OrdinalIgnoreCase);
diff --git a/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs b/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs
index d3fce37c71..2923dd3290 100644
--- a/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs
+++ b/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs
@@ -23,11 +23,11 @@ namespace MediaBrowser.Providers.Music
public static string? GetReleaseGroupId(this AlbumInfo info)
{
- var id = info.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup);
+ var id = MusicBrainzId(MetadataProvider.MusicBrainzReleaseGroup, info.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup));
if (string.IsNullOrEmpty(id))
{
- return info.SongInfos.Select(i => i.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup))
+ return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzReleaseGroup, i.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup)))
.FirstOrDefault(i => !string.IsNullOrEmpty(i));
}
@@ -36,11 +36,11 @@ namespace MediaBrowser.Providers.Music
public static string? GetReleaseId(this AlbumInfo info)
{
- var id = info.GetProviderId(MetadataProvider.MusicBrainzAlbum);
+ var id = MusicBrainzId(MetadataProvider.MusicBrainzAlbum, info.GetProviderId(MetadataProvider.MusicBrainzAlbum));
if (string.IsNullOrEmpty(id))
{
- return info.SongInfos.Select(i => i.GetProviderId(MetadataProvider.MusicBrainzAlbum))
+ return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbum, i.GetProviderId(MetadataProvider.MusicBrainzAlbum)))
.FirstOrDefault(i => !string.IsNullOrEmpty(i));
}
@@ -50,15 +50,17 @@ namespace MediaBrowser.Providers.Music
public static string? GetMusicBrainzArtistId(this AlbumInfo info)
{
info.ProviderIds.TryGetValue(MetadataProvider.MusicBrainzAlbumArtist.ToString(), out string? id);
+ id = MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, id);
if (string.IsNullOrEmpty(id))
{
info.ArtistProviderIds.TryGetValue(MetadataProvider.MusicBrainzArtist.ToString(), out id);
+ id = MusicBrainzId(MetadataProvider.MusicBrainzArtist, id);
}
if (string.IsNullOrEmpty(id))
{
- return info.SongInfos.Select(i => i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))
+ return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist)))
.FirstOrDefault(i => !string.IsNullOrEmpty(i));
}
@@ -68,14 +70,21 @@ namespace MediaBrowser.Providers.Music
public static string? GetMusicBrainzArtistId(this ArtistInfo info)
{
info.ProviderIds.TryGetValue(MetadataProvider.MusicBrainzArtist.ToString(), out var id);
+ id = MusicBrainzId(MetadataProvider.MusicBrainzArtist, id);
if (string.IsNullOrEmpty(id))
{
- return info.SongInfos.Select(i => i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))
+ return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist)))
.FirstOrDefault(i => !string.IsNullOrEmpty(i));
}
return id;
}
+
+ /// <summary>
+ /// Returns the id if it can be an id of the given provider, otherwise <c>null</c>.
+ /// </summary>
+ private static string? MusicBrainzId(MetadataProvider provider, string? id)
+ => ProviderIdsExtensions.IsValidProviderId(provider.ToString(), id) ? id : null;
}
}
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetImageProvider.cs
index 78be5804e3..23f8d89c67 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetImageProvider.cs
@@ -1,6 +1,4 @@
-using System;
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -56,7 +54,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.BoxSets
/// <inheritdoc />
public async Task<IEnumerable<RemoteImageInfo>> GetImages(BaseItem item, CancellationToken cancellationToken)
{
- var tmdbId = Convert.ToInt32(item.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ item.TryGetTmdbId(out var tmdbId);
if (tmdbId <= 0)
{
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetProvider.cs
index a7bba2d539..11ac477378 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/BoxSets/TmdbBoxSetProvider.cs
@@ -1,4 +1,3 @@
-using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
@@ -42,7 +41,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.BoxSets
/// <inheritdoc />
public async Task<IEnumerable<RemoteSearchResult>> GetSearchResults(BoxSetInfo searchInfo, CancellationToken cancellationToken)
{
- var tmdbId = Convert.ToInt32(searchInfo.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ searchInfo.TryGetTmdbId(out var tmdbId);
var language = searchInfo.MetadataLanguage;
if (tmdbId > 0)
@@ -97,7 +96,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.BoxSets
/// <inheritdoc />
public async Task<MetadataResult<BoxSet>> GetMetadata(BoxSetInfo info, CancellationToken cancellationToken)
{
- var tmdbId = Convert.ToInt32(info.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ info.TryGetTmdbId(out var tmdbId);
var language = info.MetadataLanguage;
// We don't already have an Id, need to fetch it
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieImageProvider.cs
index b188f5deb4..e686577311 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieImageProvider.cs
@@ -1,6 +1,4 @@
-using System;
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -61,7 +59,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
var language = item.GetPreferredMetadataLanguage();
var countryCode = item.GetPreferredMetadataCountryCode();
- var movieTmdbId = Convert.ToInt32(item.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ item.TryGetTmdbId(out var movieTmdbId);
if (movieTmdbId <= 0)
{
var movieImdbId = item.GetProviderId(MetadataProvider.Imdb);
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieProvider.cs
index 8811a1787a..ef952082da 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/Movies/TmdbMovieProvider.cs
@@ -54,11 +54,11 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
/// <inheritdoc />
public async Task<IEnumerable<RemoteSearchResult>> GetSearchResults(MovieInfo searchInfo, CancellationToken cancellationToken)
{
- if (searchInfo.TryGetProviderId(MetadataProvider.Tmdb, out var id))
+ if (searchInfo.TryGetTmdbId(out var tmdbId))
{
var movie = await _tmdbClientManager
.GetMovieAsync(
- int.Parse(id, CultureInfo.InvariantCulture),
+ tmdbId,
searchInfo.MetadataLanguage,
TmdbUtils.GetImageLanguagesParam(searchInfo.MetadataLanguage, searchInfo.MetadataCountryCode),
searchInfo.MetadataCountryCode,
@@ -90,7 +90,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
}
IReadOnlyList<SearchMovie>? movieResults = null;
- if (searchInfo.TryGetProviderId(MetadataProvider.Imdb, out id))
+ if (searchInfo.TryGetProviderId(MetadataProvider.Imdb, out var id))
{
var result = await _tmdbClientManager.FindByExternalIdAsync(
id,
@@ -151,11 +151,13 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
/// <inheritdoc />
public async Task<MetadataResult<Movie>> GetMetadata(MovieInfo info, CancellationToken cancellationToken)
{
- var tmdbId = info.GetProviderId(MetadataProvider.Tmdb);
+ // A stored id that is not a TMDb id is treated as no id, so the search below can repair it
+ // rather than the lookup failing for as long as the bad id stays on the item.
+ info.TryGetTmdbId(out var tmdbId);
var imdbId = info.GetProviderId(MetadataProvider.Imdb);
var config = Plugin.Instance.Configuration;
- if (string.IsNullOrEmpty(tmdbId) && string.IsNullOrEmpty(imdbId))
+ if (tmdbId <= 0 && string.IsNullOrEmpty(imdbId))
{
// ParseName is required here.
// Caller provides the filename with extension stripped and NOT the parsed filename
@@ -166,26 +168,26 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
if (searchResults?.Count > 0)
{
- tmdbId = searchResults[0].Id.ToString(CultureInfo.InvariantCulture);
+ tmdbId = searchResults[0].Id;
}
}
- if (string.IsNullOrEmpty(tmdbId) && !string.IsNullOrEmpty(imdbId))
+ if (tmdbId <= 0 && !string.IsNullOrEmpty(imdbId))
{
var movieResultFromImdbId = await _tmdbClientManager.FindByExternalIdAsync(imdbId, FindExternalSource.Imdb, info.MetadataLanguage, info.MetadataCountryCode, cancellationToken).ConfigureAwait(false);
if (movieResultFromImdbId?.MovieResults?.Count > 0)
{
- tmdbId = movieResultFromImdbId.MovieResults[0].Id.ToString(CultureInfo.InvariantCulture);
+ tmdbId = movieResultFromImdbId.MovieResults[0].Id;
}
}
- if (string.IsNullOrEmpty(tmdbId))
+ if (tmdbId <= 0)
{
return new MetadataResult<Movie>();
}
var movieResult = await _tmdbClientManager
- .GetMovieAsync(Convert.ToInt32(tmdbId, CultureInfo.InvariantCulture), info.MetadataLanguage, TmdbUtils.GetImageLanguagesParam(info.MetadataLanguage, info.MetadataCountryCode), info.MetadataCountryCode, cancellationToken)
+ .GetMovieAsync(tmdbId, info.MetadataLanguage, TmdbUtils.GetImageLanguagesParam(info.MetadataLanguage, info.MetadataCountryCode), info.MetadataCountryCode, cancellationToken)
.ConfigureAwait(false);
if (movieResult is null)
@@ -208,7 +210,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.Movies
Item = movie
};
- movie.SetProviderId(MetadataProvider.Tmdb, tmdbId);
+ movie.SetProviderId(MetadataProvider.Tmdb, tmdbId.ToString(CultureInfo.InvariantCulture));
movie.TrySetProviderId(MetadataProvider.Imdb, movieResult.ImdbId);
if (movieResult.BelongsToCollection is not null)
{
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonImageProvider.cs
index 33888ddf4f..d38614811c 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonImageProvider.cs
@@ -1,5 +1,4 @@
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -54,14 +53,14 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.People
{
var person = (Person)item;
- if (!person.TryGetProviderId(MetadataProvider.Tmdb, out var personTmdbId))
+ if (!person.TryGetTmdbId(out var personTmdbId))
{
return Enumerable.Empty<RemoteImageInfo>();
}
var language = item.GetPreferredMetadataLanguage();
var countryCode = item.GetPreferredMetadataCountryCode();
- var personResult = await _tmdbClientManager.GetPersonAsync(int.Parse(personTmdbId, CultureInfo.InvariantCulture), language, countryCode, cancellationToken).ConfigureAwait(false);
+ var personResult = await _tmdbClientManager.GetPersonAsync(personTmdbId, language, countryCode, cancellationToken).ConfigureAwait(false);
if (personResult?.Images?.Profiles is null)
{
return Enumerable.Empty<RemoteImageInfo>();
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonProvider.cs
index 64ab98b262..61294676f7 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/People/TmdbPersonProvider.cs
@@ -1,4 +1,3 @@
-using System;
using System.Collections.Generic;
using System.Globalization;
using System.Net.Http;
@@ -37,9 +36,9 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.People
/// <inheritdoc />
public async Task<IEnumerable<RemoteSearchResult>> GetSearchResults(PersonLookupInfo searchInfo, CancellationToken cancellationToken)
{
- if (searchInfo.TryGetProviderId(MetadataProvider.Tmdb, out var personTmdbId))
+ if (searchInfo.TryGetTmdbId(out var personTmdbId))
{
- var personResult = await _tmdbClientManager.GetPersonAsync(int.Parse(personTmdbId, CultureInfo.InvariantCulture), searchInfo.MetadataLanguage, searchInfo.MetadataCountryCode, cancellationToken).ConfigureAwait(false);
+ var personResult = await _tmdbClientManager.GetPersonAsync(personTmdbId, searchInfo.MetadataLanguage, searchInfo.MetadataCountryCode, cancellationToken).ConfigureAwait(false);
if (personResult is not null)
{
@@ -89,7 +88,9 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.People
/// <inheritdoc />
public async Task<MetadataResult<Person>> GetMetadata(PersonLookupInfo info, CancellationToken cancellationToken)
{
- var personTmdbId = Convert.ToInt32(info.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ // A person can carry another provider's id under the TMDb key, which is no more usable here
+ // than no id at all, so both take the search path and get the stored id repaired.
+ info.TryGetTmdbId(out var personTmdbId);
// We don't already have an Id, need to fetch it
if (personTmdbId <= 0)
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeImageProvider.cs
index 7ae54cdcd3..1f8c87397d 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeImageProvider.cs
@@ -1,6 +1,4 @@
-using System;
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -56,9 +54,9 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
var episode = (Controller.Entities.TV.Episode)item;
var series = episode.Series;
- var seriesTmdbId = Convert.ToInt32(series?.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ var seriesTmdbId = 0;
- if (series is null || seriesTmdbId <= 0)
+ if (series?.TryGetTmdbId(out seriesTmdbId) != true)
{
return Enumerable.Empty<RemoteImageInfo>();
}
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeProvider.cs
index 21b822c97c..8172ab14df 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbEpisodeProvider.cs
@@ -91,8 +91,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
info.SeriesProviderIds.TryGetValue(MetadataProvider.Tmdb.ToString(), out string? tmdbId);
- var seriesTmdbId = Convert.ToInt32(tmdbId, CultureInfo.InvariantCulture);
- if (seriesTmdbId <= 0)
+ if (!TmdbUtils.TryParseTmdbId(tmdbId, out var seriesTmdbId))
{
return metadataResult;
}
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonImageProvider.cs
index 5b2f0d26e4..bc44d0266d 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonImageProvider.cs
@@ -1,6 +1,4 @@
-using System;
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -57,9 +55,9 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
var season = (Season)item;
var series = season?.Series;
- var seriesTmdbId = Convert.ToInt32(series?.GetProviderId(MetadataProvider.Tmdb), CultureInfo.InvariantCulture);
+ var seriesTmdbId = 0;
- if (seriesTmdbId <= 0 || season?.IndexNumber is null)
+ if (season?.IndexNumber is null || series?.TryGetTmdbId(out seriesTmdbId) != true)
{
return Enumerable.Empty<RemoteImageInfo>();
}
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonProvider.cs
index 9c41d64253..06313810a1 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeasonProvider.cs
@@ -1,4 +1,3 @@
-using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
@@ -48,13 +47,13 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
var seasonNumber = info.IndexNumber;
- if (string.IsNullOrWhiteSpace(seriesTmdbId) || !seasonNumber.HasValue)
+ if (!seasonNumber.HasValue || !TmdbUtils.TryParseTmdbId(seriesTmdbId, out var seriesId))
{
return result;
}
var seasonResult = await _tmdbClientManager
- .GetSeasonAsync(Convert.ToInt32(seriesTmdbId, CultureInfo.InvariantCulture), seasonNumber.Value, info.MetadataLanguage, TmdbUtils.GetImageLanguagesParam(info.MetadataLanguage, info.MetadataCountryCode), info.MetadataCountryCode, cancellationToken)
+ .GetSeasonAsync(seriesId, seasonNumber.Value, info.MetadataLanguage, TmdbUtils.GetImageLanguagesParam(info.MetadataLanguage, info.MetadataCountryCode), info.MetadataCountryCode, cancellationToken)
.ConfigureAwait(false);
if (seasonResult is null)
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesImageProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesImageProvider.cs
index f2e7d0c6e4..dc4f860604 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesImageProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesImageProvider.cs
@@ -1,6 +1,4 @@
-using System;
using System.Collections.Generic;
-using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
@@ -57,9 +55,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
/// <inheritdoc />
public async Task<IEnumerable<RemoteImageInfo>> GetImages(BaseItem item, CancellationToken cancellationToken)
{
- var tmdbId = item.GetProviderId(MetadataProvider.Tmdb);
-
- if (string.IsNullOrEmpty(tmdbId))
+ if (!item.TryGetTmdbId(out var tmdbId))
{
return Enumerable.Empty<RemoteImageInfo>();
}
@@ -68,7 +64,7 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
// TODO use image languages if All Languages isn't toggled, but there's currently no way to get that value in here
var series = await _tmdbClientManager
- .GetSeriesAsync(Convert.ToInt32(tmdbId, CultureInfo.InvariantCulture), null, null, null, cancellationToken)
+ .GetSeriesAsync(tmdbId, null, null, null, cancellationToken)
.ConfigureAwait(false);
if (series?.Images is null)
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesProvider.cs b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesProvider.cs
index 9bb15ca479..9e201f2d7c 100755
--- a/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesProvider.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TV/TmdbSeriesProvider.cs
@@ -54,10 +54,10 @@ namespace MediaBrowser.Providers.Plugins.Tmdb.TV
/// <inheritdoc />
public async Task<IEnumerable<RemoteSearchResult>> GetSearchResults(SeriesInfo searchInfo, CancellationToken cancellationToken)
{
- if (searchInfo.TryGetProviderId(MetadataProvider.Tmdb, out var tmdbId))
+ if (searchInfo.TryGetTmdbId(out var tmdbId))
{
var series = await _tmdbClientManager
- .GetSeriesAsync(Convert.ToInt32(tmdbId, CultureInfo.InvariantCulture), searchInfo.MetadataLanguage, searchInfo.MetadataLanguage, searchInfo.MetadataCountryCode, cancellationToken)
+ .GetSeriesAsync(tmdbId, searchInfo.MetadataLanguage, searchInfo.MetadataLanguage, searchInfo.MetadataCountryCode, cancellationToken)
.ConfigureAwait(false);
if (series is not null)
diff --git a/MediaBrowser.Providers/Plugins/Tmdb/TmdbUtils.cs b/MediaBrowser.Providers/Plugins/Tmdb/TmdbUtils.cs
index 7e6b9beee9..c83174f97f 100644
--- a/MediaBrowser.Providers/Plugins/Tmdb/TmdbUtils.cs
+++ b/MediaBrowser.Providers/Plugins/Tmdb/TmdbUtils.cs
@@ -2,6 +2,7 @@ using System;
using System.Collections.Frozen;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
using System.Text.RegularExpressions;
using Jellyfin.Data.Enums;
using MediaBrowser.Model.Entities;
@@ -63,6 +64,33 @@ namespace MediaBrowser.Providers.Plugins.Tmdb
private static partial Regex NonWordRegex();
/// <summary>
+ /// Gets the TMDb id of an item, if it has one TMDb can be queried with.
+ /// </summary>
+ /// <param name="instance">The item.</param>
+ /// <param name="tmdbId">The TMDb id.</param>
+ /// <returns><c>true</c> if the item has a usable TMDb id; otherwise, <c>false</c>.</returns>
+ public static bool TryGetTmdbId(this IHasProviderIds instance, out int tmdbId)
+ {
+ instance.TryGetProviderId(MetadataProvider.Tmdb, out var value);
+
+ return TryParseTmdbId(value, out tmdbId);
+ }
+
+ /// <summary>
+ /// Parses a TMDb id.
+ /// </summary>
+ /// <param name="value">The stored id.</param>
+ /// <param name="tmdbId">The TMDb id.</param>
+ /// <returns><c>true</c> if the value is a usable TMDb id; otherwise, <c>false</c>.</returns>
+ public static bool TryParseTmdbId(string? value, out int tmdbId)
+ {
+ // Another provider can have filed one of its own ids under the TMDb key, e.g. an IMDb person
+ // id. Reporting that as "no id" lets the caller fall back to a search and repair the id,
+ // instead of throwing on every refresh of the item.
+ return int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out tmdbId) && tmdbId > 0;
+ }
+
+ /// <summary>
/// Cleans the name according to TMDb requirements.
/// </summary>
/// <param name="name">The name of the entity.</param>
diff --git a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs
index a6f4164144..2347c08961 100644
--- a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs
+++ b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs
@@ -186,6 +186,109 @@ namespace Jellyfin.Model.Tests.Entities
Assert.Null(nullProvider.ProviderIds);
}
+ [Theory]
+ [InlineData(nameof(MetadataProvider.Imdb), "tt0113375", true)]
+ [InlineData(nameof(MetadataProvider.Imdb), "nm0000123", true)]
+ [InlineData(nameof(MetadataProvider.Imdb), "0113375", true)]
+ [InlineData(nameof(MetadataProvider.Imdb), "https://www.imdb.com/title/tt0113375", false)]
+ [InlineData(nameof(MetadataProvider.Tmdb), "11", true)]
+ [InlineData(nameof(MetadataProvider.Tmdb), "nm0000123", false)]
+ [InlineData(nameof(MetadataProvider.Tmdb), "0", false)]
+ [InlineData(nameof(MetadataProvider.Tmdb), "-11", false)]
+ [InlineData(nameof(MetadataProvider.TmdbCollection), "nm0000123", false)]
+ [InlineData(nameof(MetadataProvider.AudioDbArtist), "111239", true)]
+ [InlineData(nameof(MetadataProvider.AudioDbArtist), "a3cb23fc-acd3-4ce0-8f36-1e5aa6a18432", false)]
+ [InlineData(nameof(MetadataProvider.MusicBrainzArtist), "a3cb23fc-acd3-4ce0-8f36-1e5aa6a18432", true)]
+ [InlineData(nameof(MetadataProvider.MusicBrainzArtist), "111239", false)]
+ [InlineData(nameof(MetadataProvider.MusicBrainzAlbum), "not-an-mbid", false)]
+ [InlineData(nameof(MetadataProvider.Tvdb), "anything-goes", true)]
+ [InlineData("SomePlugin", "anything-goes", true)]
+ [InlineData(nameof(MetadataProvider.Tmdb), null, false)]
+ [InlineData(null, "11", false)]
+ public void IsValidProviderId_ChecksKnownFormats(string? name, string? value, bool expected)
+ {
+ Assert.Equal(expected, ProviderIdsExtensions.IsValidProviderId(name, value));
+ }
+
+ [Fact]
+ public void TrySetProviderId_ForeignId_False()
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+
+ Assert.False(provider.TrySetProviderId(MetadataProvider.Tmdb, "nm0000123"));
+ Assert.Empty(provider.ProviderIds);
+ }
+
+ [Fact]
+ public void TrySetProviderId_ForeignId_KeepsExisting()
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+ provider.ProviderIds[MetadataProvider.Tmdb.ToString()] = "11";
+
+ Assert.False(provider.TrySetProviderId(MetadataProvider.Tmdb, "nm0000123"));
+ Assert.Equal("11", provider.GetProviderId(MetadataProvider.Tmdb));
+ }
+
+ [Theory]
+ [InlineData(nameof(MetadataProvider.Imdb), " tt0113375 ")]
+ [InlineData(" Imdb", ExampleImdbId)]
+ public void TrySetProviderId_SurroundingWhitespace_Trimmed(string name, string value)
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+
+ Assert.True(provider.TrySetProviderId(name, value));
+ Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb));
+ }
+
+ [Fact]
+ public void SetProviderIds_ReplacesAll()
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+ provider.ProviderIds[MetadataProvider.Tvdb.ToString()] = "12345";
+
+ provider.SetProviderIds(new Dictionary<string, string>
+ {
+ [MetadataProvider.Imdb.ToString()] = ExampleImdbId
+ });
+
+ Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb));
+ Assert.False(provider.HasProviderId(MetadataProvider.Tvdb));
+ }
+
+ [Fact]
+ public void SetProviderIds_ForeignId_Dropped()
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+
+ provider.SetProviderIds(new Dictionary<string, string>
+ {
+ [MetadataProvider.Tmdb.ToString()] = "nm0000123",
+ [MetadataProvider.Imdb.ToString()] = ExampleImdbId,
+ [MetadataProvider.Tvdb.ToString()] = string.Empty
+ });
+
+ Assert.False(provider.HasProviderId(MetadataProvider.Tmdb));
+ Assert.False(provider.HasProviderId(MetadataProvider.Tvdb));
+ Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb));
+ }
+
+ [Fact]
+ public void SetProviderIds_Null_Clears()
+ {
+ var provider = new ProviderIdsExtensionsTestsObject();
+ provider.ProviderIds[MetadataProvider.Imdb.ToString()] = ExampleImdbId;
+
+ provider.SetProviderIds(null);
+
+ Assert.Empty(provider.ProviderIds);
+ }
+
+ [Fact]
+ public void SetProviderIds_NullInstance_ThrowsArgumentNullException()
+ {
+ Assert.Throws<ArgumentNullException>(() => ProviderIdsExtensions.SetProviderIds(null!, new Dictionary<string, string>()));
+ }
+
[Fact]
public void RemoveProviderId_Null_Remove()
{
diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs
new file mode 100644
index 0000000000..1d2fb2e760
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs
@@ -0,0 +1,253 @@
+using System;
+using System.Collections.Generic;
+using System.Net.Http;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Data.Enums;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.IO;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Persistence;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.IO;
+using MediaBrowser.Providers.Manager;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.Manager
+{
+ public class MetadataServiceRefreshTests
+ {
+ [Theory]
+ // RemoveOldMetadata is only ever set by an explicit user action - a refresh with "replace all
+ // metadata", or Identify. A provider failing must not silently downgrade that to a merge: the
+ // providers that did answer supplied the replacement, and the old values are the wrong match
+ // the user asked to get rid of.
+ [InlineData(false)]
+ [InlineData(true)]
+ public async Task RefreshWithProviders_ReplaceAllMetadata_ErasesOldDataWhenAProviderAnswers(bool allProvidersSucceed)
+ {
+ var item = new Movie
+ {
+ Name = "Test Movie",
+ Overview = "existing overview"
+ };
+
+ // The provider owning the overview fails, so it contributes nothing to the replacement.
+ var failing = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ failing.Setup(p => p.Name).Returns("Failing");
+ failing.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .Returns(allProvidersSucceed
+ ? Task.FromResult(new MetadataResult<Movie> { HasMetadata = true, Item = new Movie() })
+ : Task.FromException<MetadataResult<Movie>>(new FormatException("bad id")));
+
+ var succeeding = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ succeeding.Setup(p => p.Name).Returns("Succeeding");
+ succeeding.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(new MetadataResult<Movie>
+ {
+ HasMetadata = true,
+ Item = new Movie { Name = "Test Movie", Tagline = "new tagline" }
+ });
+
+ var service = new TestMetadataService();
+ var result = await service.RefreshWithProvidersInternal(
+ new MetadataResult<Movie> { Item = item },
+ new MovieInfo { Name = item.Name },
+ new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+ {
+ MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllMetadata = true,
+ RemoveOldMetadata = true
+ },
+ [failing.Object, succeeding.Object]).ConfigureAwait(true);
+
+ Assert.Equal(allProvidersSucceed ? 0 : 1, result.Failures);
+ Assert.Equal("new tagline", item.Tagline);
+ Assert.Null(item.Overview);
+ }
+
+ [Fact]
+ public async Task RefreshWithProviders_ReplaceAllMetadata_KeepsExistingDataWhenEveryRemoteProviderFails()
+ {
+ var item = new Movie
+ {
+ Name = "Test Movie",
+ Overview = "existing overview"
+ };
+
+ // Something has to contribute for the merge to run at all, otherwise the item is never touched
+ // and the case is moot. The local provider is the replacement the remote ones did not deliver.
+ var local = new Mock<ILocalMetadataProvider<Movie>>(MockBehavior.Loose);
+ local.Setup(p => p.Name).Returns("Local");
+ local.Setup(p => p.GetMetadata(It.IsAny<ItemInfo>(), It.IsAny<IDirectoryService>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(new MetadataResult<Movie>
+ {
+ HasMetadata = true,
+ Item = new Movie { Name = "Test Movie", Tagline = "new tagline" }
+ });
+
+ var remote = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ remote.Setup(p => p.Name).Returns("Failing");
+ remote.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .Returns(Task.FromException<MetadataResult<Movie>>(new HttpRequestException("unreachable")));
+
+ var service = new TestMetadataService();
+ var result = await service.RefreshWithProvidersInternal(
+ new MetadataResult<Movie> { Item = item },
+ new MovieInfo { Name = item.Name },
+ new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+ {
+ MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllMetadata = true,
+ RemoveOldMetadata = true
+ },
+ [local.Object, remote.Object]).ConfigureAwait(true);
+
+ Assert.Equal(1, result.Failures);
+ Assert.Equal("new tagline", item.Tagline);
+
+ // No remote provider answered, so erasing the overview would lose it for good.
+ Assert.Equal("existing overview", item.Overview);
+ }
+
+ [Fact]
+ public async Task RefreshWithProviders_ForeignProviderId_NotStored()
+ {
+ var item = new Movie { Name = "Test Movie" };
+
+ var provider = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ provider.Setup(p => p.Name).Returns("Provider");
+ provider.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(() =>
+ {
+ var found = new Movie { Name = "Test Movie" };
+ found.ProviderIds[MetadataProvider.Tmdb.ToString()] = "nm0000123";
+ found.ProviderIds[MetadataProvider.Imdb.ToString()] = "tt0113375";
+ return new MetadataResult<Movie> { HasMetadata = true, Item = found };
+ });
+
+ var service = new TestMetadataService();
+ await service.RefreshWithProvidersInternal(
+ new MetadataResult<Movie> { Item = item },
+ new MovieInfo { Name = item.Name },
+ new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+ {
+ MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllMetadata = true
+ },
+ [provider.Object]).ConfigureAwait(true);
+
+ Assert.False(item.HasProviderId(MetadataProvider.Tmdb));
+ Assert.Equal("tt0113375", item.GetProviderId(MetadataProvider.Imdb));
+ }
+
+ [Fact]
+ public async Task RefreshWithProviders_ForeignProviderId_ReplacedInLookupInfo()
+ {
+ var item = new Movie { Name = "Test Movie" };
+ var lookupInfo = new MovieInfo { Name = item.Name };
+ lookupInfo.ProviderIds[MetadataProvider.Tmdb.ToString()] = "nm0000123";
+
+ var answering = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ answering.Setup(p => p.Name).Returns("Answering");
+ answering.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(() =>
+ {
+ var found = new Movie { Name = "Test Movie" };
+ found.ProviderIds[MetadataProvider.Tmdb.ToString()] = "12345";
+ return new MetadataResult<Movie> { HasMetadata = true, Item = found };
+ });
+
+ string? tmdbIdSeenBySecondProvider = null;
+ var following = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ following.Setup(p => p.Name).Returns("Following");
+ following.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync((MovieInfo info, CancellationToken _) =>
+ {
+ tmdbIdSeenBySecondProvider = info.GetProviderId(MetadataProvider.Tmdb);
+ return new MetadataResult<Movie> { HasMetadata = false };
+ });
+
+ var service = new TestMetadataService();
+ await service.RefreshWithProvidersInternal(
+ new MetadataResult<Movie> { Item = item },
+ lookupInfo,
+ new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+ {
+ MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllMetadata = true
+ },
+ [answering.Object, following.Object]).ConfigureAwait(true);
+
+ // The stored id cannot be a TMDb one, so the provider that still has to run must get the id
+ // that was just found instead of failing on the same bad one.
+ Assert.Equal("12345", tmdbIdSeenBySecondProvider);
+ }
+
+ [Theory]
+ [InlineData(true)]
+ [InlineData(false)]
+ public async Task RefreshWithProviders_ForeignPersonProviderId_NotStored(bool replaceAllMetadata)
+ {
+ var item = new Movie { Name = "Test Movie" };
+ var existing = new MetadataResult<Movie> { Item = item };
+ existing.AddPerson(new PersonInfo { Name = "Some Actor", Type = PersonKind.Actor });
+
+ var provider = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+ provider.Setup(p => p.Name).Returns("Provider");
+ provider.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(() =>
+ {
+ var person = new PersonInfo { Name = "Some Actor", Type = PersonKind.Actor };
+ person.ProviderIds[MetadataProvider.Tmdb.ToString()] = "nm0000123";
+ person.ProviderIds[MetadataProvider.Imdb.ToString()] = "nm0000123";
+
+ var found = new MetadataResult<Movie> { HasMetadata = true, Item = new Movie { Name = "Test Movie" } };
+ found.AddPerson(person);
+ return found;
+ });
+
+ var service = new TestMetadataService();
+ await service.RefreshWithProvidersInternal(
+ existing,
+ new MovieInfo { Name = item.Name },
+ new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+ {
+ MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllMetadata = replaceAllMetadata
+ },
+ [provider.Object]).ConfigureAwait(true);
+
+ var mergedPerson = Assert.Single(existing.People);
+ Assert.False(mergedPerson.HasProviderId(MetadataProvider.Tmdb));
+ Assert.Equal("nm0000123", mergedPerson.GetProviderId(MetadataProvider.Imdb));
+ }
+
+ private sealed class TestMetadataService : MetadataService<Movie, MovieInfo>
+ {
+ public TestMetadataService()
+ : base(
+ Mock.Of<IServerConfigurationManager>(),
+ NullLogger<MetadataService<Movie, MovieInfo>>.Instance,
+ Mock.Of<IProviderManager>(),
+ Mock.Of<IFileSystem>(),
+ Mock.Of<ILibraryManager>(),
+ Mock.Of<IExternalDataManager>(),
+ Mock.Of<IItemRepository>())
+ {
+ }
+
+ public Task<RefreshResult> RefreshWithProvidersInternal(
+ MetadataResult<Movie> metadata,
+ MovieInfo id,
+ MetadataRefreshOptions options,
+ ICollection<IMetadataProvider> providers)
+ => RefreshWithProviders(metadata, id, options, providers, ImageProvider, false, CancellationToken.None);
+ }
+ }
+}
diff --git a/tests/Jellyfin.Providers.Tests/Music/AlbumInfoExtensionsTests.cs b/tests/Jellyfin.Providers.Tests/Music/AlbumInfoExtensionsTests.cs
new file mode 100644
index 0000000000..c5ec0de02c
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Music/AlbumInfoExtensionsTests.cs
@@ -0,0 +1,59 @@
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Providers.Music;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.Music;
+
+public static class AlbumInfoExtensionsTests
+{
+ private const string ExampleMbid = "59b5a40b-e2fd-3f18-a218-e8c9aae12ab5";
+ private const string SongMbid = "6c301dbd-6ccb-3403-a6c4-6a22240a0297";
+
+ [Theory]
+ [InlineData(ExampleMbid, ExampleMbid)]
+ // Another provider's id under a MusicBrainz key reads as no id, so the caller searches instead of
+ // handing a value the MusicBrainz client throws on.
+ [InlineData("111239", null)]
+ [InlineData("", null)]
+ public static void GetReleaseId_OnlyReturnsMbids(string id, string? expected)
+ {
+ var info = new AlbumInfo();
+ info.ProviderIds[MetadataProvider.MusicBrainzAlbum.ToString()] = id;
+
+ Assert.Equal(expected, info.GetReleaseId());
+ }
+
+ [Fact]
+ public static void GetReleaseId_ForeignId_FallsBackToSongs()
+ {
+ var song = new SongInfo();
+ song.ProviderIds[MetadataProvider.MusicBrainzAlbum.ToString()] = SongMbid;
+
+ var info = new AlbumInfo { SongInfos = [song] };
+ info.ProviderIds[MetadataProvider.MusicBrainzAlbum.ToString()] = "111239";
+
+ Assert.Equal(SongMbid, info.GetReleaseId());
+ }
+
+ [Fact]
+ public static void GetMusicBrainzArtistId_ForeignId_FallsBackToArtistIds()
+ {
+ var info = new AlbumInfo();
+ info.ProviderIds[MetadataProvider.MusicBrainzAlbumArtist.ToString()] = "111239";
+ info.ArtistProviderIds[MetadataProvider.MusicBrainzArtist.ToString()] = ExampleMbid;
+
+ Assert.Equal(ExampleMbid, info.GetMusicBrainzArtistId());
+ }
+
+ [Theory]
+ [InlineData(ExampleMbid, ExampleMbid)]
+ [InlineData("111239", null)]
+ public static void GetMusicBrainzArtistId_ArtistInfo_OnlyReturnsMbids(string id, string? expected)
+ {
+ var info = new ArtistInfo();
+ info.ProviderIds[MetadataProvider.MusicBrainzArtist.ToString()] = id;
+
+ Assert.Equal(expected, info.GetMusicBrainzArtistId());
+ }
+}
diff --git a/tests/Jellyfin.Providers.Tests/Tmdb/TmdbUtilsTests.cs b/tests/Jellyfin.Providers.Tests/Tmdb/TmdbUtilsTests.cs
index fb0a08c29c..4c4dd5e92f 100644
--- a/tests/Jellyfin.Providers.Tests/Tmdb/TmdbUtilsTests.cs
+++ b/tests/Jellyfin.Providers.Tests/Tmdb/TmdbUtilsTests.cs
@@ -1,3 +1,5 @@
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Model.Entities;
using MediaBrowser.Providers.Plugins.Tmdb;
using Xunit;
@@ -34,5 +36,40 @@ namespace Jellyfin.Providers.Tests.Tmdb
{
Assert.Equal(expected, TmdbUtils.AdjustImageLanguage(imageLanguage, requestLanguage));
}
+
+ [Theory]
+ [InlineData("11", true, 11)]
+ // An id another provider filed under the TMDb key must not throw, it is simply not a TMDb id.
+ [InlineData("nm0000123", false, 0)]
+ [InlineData("tt0113375", false, 0)]
+ [InlineData("11.0", false, 0)]
+ [InlineData("-11", false, 0)]
+ [InlineData("0", false, 0)]
+ [InlineData("", false, 0)]
+ [InlineData(null, false, 0)]
+ public static void TryParseTmdbId_OnlyAcceptsTmdbIds(string? value, bool expected, int expectedId)
+ {
+ Assert.Equal(expected, TmdbUtils.TryParseTmdbId(value, out var tmdbId));
+ Assert.Equal(expectedId, tmdbId);
+ }
+
+ [Theory]
+ [InlineData("11", true, 11)]
+ [InlineData("nm0000123", false, 0)]
+ public static void TryGetTmdbId_OnlyAcceptsTmdbIds(string value, bool expected, int expectedId)
+ {
+ var item = new Movie();
+ item.ProviderIds[MetadataProvider.Tmdb.ToString()] = value;
+
+ Assert.Equal(expected, item.TryGetTmdbId(out var tmdbId));
+ Assert.Equal(expectedId, tmdbId);
+ }
+
+ [Fact]
+ public static void TryGetTmdbId_NoId_False()
+ {
+ Assert.False(new Movie().TryGetTmdbId(out var tmdbId));
+ Assert.Equal(0, tmdbId);
+ }
}
}