Fix handling of unordered multi-episode NFOs
This commit is contained in:
@@ -44,6 +44,33 @@ public class EpisodeMetadataService : MetadataService<Episode, EpisodeInfo>
|
||||
{
|
||||
var updatedType = base.BeforeSaveInternal(item, isFullRefresh, updateType);
|
||||
|
||||
// An episode cannot end before it starts. Providers and nfo files occasionally report the range
|
||||
// transposed, which makes clients render the episode number backwards. Both numbers describe the
|
||||
// same set of episodes either way, so restore their order instead of dropping the range.
|
||||
if (item.IndexNumber.HasValue && item.IndexNumberEnd < item.IndexNumber)
|
||||
{
|
||||
Logger.LogWarning(
|
||||
"Correcting reversed episode range {IndexNumber}-{IndexNumberEnd} for {Path}",
|
||||
item.IndexNumber,
|
||||
item.IndexNumberEnd,
|
||||
item.Path);
|
||||
|
||||
(item.IndexNumber, item.IndexNumberEnd) = (item.IndexNumberEnd, item.IndexNumber);
|
||||
updatedType |= ItemUpdateType.MetadataImport;
|
||||
}
|
||||
else if (item.IndexNumberEnd.HasValue && !item.IndexNumber.HasValue)
|
||||
{
|
||||
// Without a first episode the end does not describe a range. Promoting it to the episode number
|
||||
// would invent an identity the metadata never supplied, so drop the orphaned value instead.
|
||||
Logger.LogWarning(
|
||||
"Discarding episode range end {IndexNumberEnd} without an episode number for {Path}",
|
||||
item.IndexNumberEnd,
|
||||
item.Path);
|
||||
|
||||
item.IndexNumberEnd = null;
|
||||
updatedType |= ItemUpdateType.MetadataImport;
|
||||
}
|
||||
|
||||
var seriesName = item.FindSeriesName();
|
||||
if (!string.Equals(item.SeriesName, seriesName, StringComparison.Ordinal))
|
||||
{
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.IO;
|
||||
using System.Linq;
|
||||
using System.Text;
|
||||
using System.Threading;
|
||||
using System.Xml;
|
||||
@@ -44,41 +46,60 @@ namespace MediaBrowser.XbmcMetadata.Parsers
|
||||
|
||||
var xmlFile = File.ReadAllText(metadataFile);
|
||||
|
||||
var srch = "</episodedetails>";
|
||||
var index = xmlFile.IndexOf(srch, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
var xml = xmlFile;
|
||||
|
||||
if (index != -1)
|
||||
// Split the nfo into its episodedetails blocks.
|
||||
// This is needed because XBMC metadata uses multiple episodedetails blocks instead of an episodenumberend tag.
|
||||
const string Srch = "</episodedetails>";
|
||||
var blocks = new List<string>();
|
||||
int index;
|
||||
while ((index = xmlFile.IndexOf(Srch, StringComparison.OrdinalIgnoreCase)) != -1)
|
||||
{
|
||||
xml = xmlFile.Substring(0, index + srch.Length);
|
||||
xmlFile = xmlFile.Substring(index + srch.Length);
|
||||
blocks.Add(xmlFile.Substring(0, index + Srch.Length));
|
||||
xmlFile = xmlFile.Substring(index + Srch.Length);
|
||||
}
|
||||
|
||||
if (blocks.Count == 0)
|
||||
{
|
||||
// No closing tag, let the xml reader deal with whatever is in the file
|
||||
blocks.Add(xmlFile);
|
||||
}
|
||||
|
||||
// These are not going to be valid xml so no sense in causing the provider to fail and spamming the log with exceptions
|
||||
try
|
||||
{
|
||||
// Extract episode details from the first episodedetails block
|
||||
ReadEpisodeDetailsFromXml(item, xml, settings, cancellationToken);
|
||||
if (blocks.Count == 1)
|
||||
{
|
||||
ReadEpisodeDetailsFromXml(item, blocks[0], settings, cancellationToken);
|
||||
return;
|
||||
}
|
||||
|
||||
// Extract the last episode number from nfo
|
||||
// Retrieves all additional episodedetails blocks from the rest of the nfo and concatenates the name, originalTitle and overview tags with the first episode
|
||||
// This is needed because XBMC metadata uses multiple episodedetails blocks instead of episodenumberend tag
|
||||
// The blocks are not guaranteed to be written in ascending episode order, so parse them all
|
||||
// and sort them before merging.
|
||||
var episodes = blocks
|
||||
.Select(block =>
|
||||
{
|
||||
var episode = new MetadataResult<Episode>()
|
||||
{
|
||||
Item = new Episode()
|
||||
};
|
||||
|
||||
ReadEpisodeDetailsFromXml(episode, block, settings, cancellationToken);
|
||||
|
||||
return (Xml: block, Result: episode);
|
||||
})
|
||||
.OrderBy(episode => episode.Result.Item.IndexNumber ?? int.MaxValue)
|
||||
.ToList();
|
||||
|
||||
// Extract the details of the lowest numbered episode into the item that is returned to the caller
|
||||
ReadEpisodeDetailsFromXml(item, episodes[0].Xml, settings, cancellationToken);
|
||||
|
||||
// Concatenate the name, originalTitle and overview tags of the remaining episodes with the first one
|
||||
// and take the highest episode number as the last episode of the file
|
||||
var name = new StringBuilder(item.Item.Name);
|
||||
var originalTitle = new StringBuilder(item.Item.OriginalTitle);
|
||||
var overview = new StringBuilder(item.Item.Overview);
|
||||
while ((index = xmlFile.IndexOf(srch, StringComparison.OrdinalIgnoreCase)) != -1)
|
||||
for (var i = 1; i < episodes.Count; i++)
|
||||
{
|
||||
xml = xmlFile.Substring(0, index + srch.Length);
|
||||
xmlFile = xmlFile.Substring(index + srch.Length);
|
||||
|
||||
var additionalEpisode = new MetadataResult<Episode>()
|
||||
{
|
||||
Item = new Episode()
|
||||
};
|
||||
|
||||
// Extract episode details from additional episodedetails block
|
||||
ReadEpisodeDetailsFromXml(additionalEpisode, xml, settings, cancellationToken);
|
||||
var additionalEpisode = episodes[i].Result;
|
||||
|
||||
if (!string.IsNullOrEmpty(additionalEpisode.Item.Name))
|
||||
{
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using System;
|
||||
using MediaBrowser.Controller.Configuration;
|
||||
using MediaBrowser.Controller.Entities;
|
||||
using MediaBrowser.Controller.Entities.TV;
|
||||
using MediaBrowser.Controller.IO;
|
||||
using MediaBrowser.Controller.Library;
|
||||
@@ -15,9 +16,23 @@ using Xunit;
|
||||
|
||||
namespace Jellyfin.Providers.Tests.TV;
|
||||
|
||||
public class EpisodeMetadataServiceTests
|
||||
// put tests that mock the static LibraryManager in the same collection to avoid test interference
|
||||
[Collection("LibraryManagerTests")]
|
||||
public sealed class EpisodeMetadataServiceTests : IDisposable
|
||||
{
|
||||
private readonly TestEpisodeMetadataService _service = new();
|
||||
private readonly ILibraryManager? _previousLibraryManager;
|
||||
|
||||
public EpisodeMetadataServiceTests()
|
||||
{
|
||||
_previousLibraryManager = BaseItem.LibraryManager;
|
||||
BaseItem.LibraryManager = Mock.Of<ILibraryManager>();
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
BaseItem.LibraryManager = _previousLibraryManager;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MergeData_ProviderSeasonOverridesPathDerivedSeason()
|
||||
@@ -88,6 +103,58 @@ public class EpisodeMetadataServiceTests
|
||||
Assert.Equal(1, target.Item.ParentIndexNumber);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(2, 1)] // e.g. an nfo with its episodedetails blocks in descending order
|
||||
[InlineData(22, 21)]
|
||||
public void BeforeSave_ReversedEpisodeRange_RestoresOrder(int indexNumber, int indexNumberEnd)
|
||||
{
|
||||
var item = new Episode
|
||||
{
|
||||
IndexNumber = indexNumber,
|
||||
IndexNumberEnd = indexNumberEnd
|
||||
};
|
||||
|
||||
var updateType = _service.BeforeSave(item);
|
||||
|
||||
// The range still covers the same episodes, it is just no longer transposed
|
||||
Assert.Equal(indexNumberEnd, item.IndexNumber);
|
||||
Assert.Equal(indexNumber, item.IndexNumberEnd);
|
||||
Assert.True(updateType.HasFlag(ItemUpdateType.MetadataImport));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BeforeSave_EpisodeRangeWithoutStart_ClearsIndexNumberEnd()
|
||||
{
|
||||
var item = new Episode
|
||||
{
|
||||
IndexNumber = null,
|
||||
IndexNumberEnd = 2
|
||||
};
|
||||
|
||||
var updateType = _service.BeforeSave(item);
|
||||
|
||||
Assert.Null(item.IndexNumberEnd);
|
||||
Assert.Null(item.IndexNumber);
|
||||
Assert.True(updateType.HasFlag(ItemUpdateType.MetadataImport));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(1, 2)] // Regular multi episode file
|
||||
[InlineData(1, 1)] // Degenerate but not contradictory
|
||||
public void BeforeSave_ValidEpisodeRange_KeepsIndexNumberEnd(int indexNumber, int indexNumberEnd)
|
||||
{
|
||||
var item = new Episode
|
||||
{
|
||||
IndexNumber = indexNumber,
|
||||
IndexNumberEnd = indexNumberEnd
|
||||
};
|
||||
|
||||
_service.BeforeSave(item);
|
||||
|
||||
Assert.Equal(indexNumber, item.IndexNumber);
|
||||
Assert.Equal(indexNumberEnd, item.IndexNumberEnd);
|
||||
}
|
||||
|
||||
private sealed class TestEpisodeMetadataService : EpisodeMetadataService
|
||||
{
|
||||
public TestEpisodeMetadataService()
|
||||
@@ -106,5 +173,10 @@ public class EpisodeMetadataServiceTests
|
||||
{
|
||||
MergeData(source, target, Array.Empty<MetadataField>(), replaceData, mergeMetadataSettings);
|
||||
}
|
||||
|
||||
public ItemUpdateType BeforeSave(Episode item)
|
||||
{
|
||||
return BeforeSaveInternal(item, false, ItemUpdateType.None);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,6 +123,27 @@ namespace Jellyfin.XbmcMetadata.Tests.Parsers
|
||||
Assert.Equal(2004, item.ProductionYear);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Fetch_Valid_MultiEpisode_Unordered_Success()
|
||||
{
|
||||
var result = new MetadataResult<Episode>()
|
||||
{
|
||||
Item = new Episode()
|
||||
};
|
||||
|
||||
_parser.Fetch(result, "Test Data/Rising-Reversed.nfo", CancellationToken.None);
|
||||
|
||||
var item = result.Item;
|
||||
// The episodedetails blocks are stored in descending order, the merged episode must still be in ascending order
|
||||
Assert.Equal("Rising (1) / Rising (2)", item.Name);
|
||||
Assert.Equal(1, item.IndexNumber);
|
||||
Assert.Equal(2, item.IndexNumberEnd);
|
||||
Assert.Equal(1, item.ParentIndexNumber);
|
||||
Assert.Equal("A new Stargate team embarks on a dangerous mission to a distant galaxy, where they discover a mythical lost city -- and a deadly new enemy. / Sheppard tries to convince Weir to mount a rescue mission to free Colonel Sumner, Teyla, and the others captured by the Wraith.", item.Overview);
|
||||
Assert.Equal(new DateTime(2004, 7, 16), item.PremiereDate);
|
||||
Assert.Equal(2004, item.ProductionYear);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Fetch_Valid_MultiEpisode_With_Missing_Tags_Success()
|
||||
{
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
<episodedetails>
|
||||
<title>Rising (2)</title>
|
||||
<season>1</season>
|
||||
<episode>2</episode>
|
||||
<aired>2004-07-16</aired>
|
||||
<plot>Sheppard tries to convince Weir to mount a rescue mission to free Colonel Sumner, Teyla, and the others captured by the Wraith.</plot>
|
||||
<thumb>https://artworks.thetvdb.com/banners/episodes/70851/25334.jpg</thumb>
|
||||
<watched>false</watched>
|
||||
<rating>7.9</rating>
|
||||
<actor>
|
||||
<name>Joe Flanigan</name>
|
||||
<role>John Sheppard</role>
|
||||
<order>0</order>
|
||||
<thumb>https://image.tmdb.org/t/p/w300_and_h450_bestv2/5AA1ORKIsnMakT6fCVy3JKlzMs6.jpg</thumb>
|
||||
</actor>
|
||||
<actor>
|
||||
<name>David Hewlett</name>
|
||||
<role>Rodney McKay</role>
|
||||
<order>1</order>
|
||||
<thumb>https://image.tmdb.org/t/p/w300_and_h450_bestv2/hUcYyssAPCqnZ4GjolhOWXHTWSa.jpg</thumb>
|
||||
</actor>
|
||||
</episodedetails><episodedetails>
|
||||
<title>Rising (1)</title>
|
||||
<season>1</season>
|
||||
<episode>1</episode>
|
||||
<aired>2004-07-16</aired>
|
||||
<plot>A new Stargate team embarks on a dangerous mission to a distant galaxy, where they discover a mythical lost city -- and a deadly new enemy.</plot>
|
||||
<thumb>https://artworks.thetvdb.com/banners/episodes/70851/25333.jpg</thumb>
|
||||
<watched>false</watched>
|
||||
<rating>8.0</rating>
|
||||
<actor>
|
||||
<name>Joe Flanigan</name>
|
||||
<role>John Sheppard</role>
|
||||
<order>0</order>
|
||||
<thumb>https://image.tmdb.org/t/p/w300_and_h450_bestv2/5AA1ORKIsnMakT6fCVy3JKlzMs6.jpg</thumb>
|
||||
</actor>
|
||||
<actor>
|
||||
<name>David Hewlett</name>
|
||||
<role>Rodney McKay</role>
|
||||
<order>1</order>
|
||||
<thumb>https://image.tmdb.org/t/p/w300_and_h450_bestv2/hUcYyssAPCqnZ4GjolhOWXHTWSa.jpg</thumb>
|
||||
</actor>
|
||||
</episodedetails>
|
||||
Reference in New Issue
Block a user