Fix handling of unordered multi-episode NFOs

3 files changed, 137 insertions, 1 deletions

diff --git a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs
index 8f5b1b3c48..24fd6a58e9 100644
--- a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs
+++ b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs
@@ -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);
+        }
     }
 }
diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs
index a04b37f215..3767b5c954 100644
--- a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs
+++ b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs
@@ -124,6 +124,27 @@ namespace Jellyfin.XbmcMetadata.Tests.Parsers
         }
 
         [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()
         {
             var result = new MetadataResult<Episode>()
diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo b/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo
new file mode 100644
index 0000000000..6dbab13566
--- /dev/null
+++ b/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo
@@ -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>