Backport pull request #18139 from jellyfin/release-12.z

Only regenerate extra names that were generated by the scan

Original-merge: 25d97ac5d0c7b71a8d103b6a23fc2b487d4a2a47

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

2 files changed, 110 insertions, 7 deletions

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 4c468569bc..6f3df084c0 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -3466,6 +3466,7 @@ namespace Emby.Server.Implementations.Library
 
             var extras = new List<BaseItem>();
             var typeCounters = new Dictionary<ExtraType, int>();
+            var generatedNames = new Dictionary<ExtraType, HashSet<string>>();
 
             // Order by path so that the numbering handed out below does not depend on the
             // order the file system happened to list the folder in
@@ -3502,10 +3503,12 @@ namespace Emby.Server.Implementations.Library
                     extra = itemById;
                 }
 
-                // An extra is named after its file, so the file is the source of truth. Items created
-                // by older versions, or renamed by a metadata provider, are corrected here;
-                // RefreshExtras persists the change.
-                if (!string.IsNullOrEmpty(name) && extra.LockedFields?.Contains(MetadataField.Name) != true)
+                // The name derived from the file is only a default. A name that came from anywhere else,
+                // such as a local metadata file, is the user's and has to survive the scan, so only a
+                // name this method handed out itself is renewed; RefreshExtras persists the change.
+                if (!string.IsNullOrEmpty(name)
+                    && extra.LockedFields?.Contains(MetadataField.Name) != true
+                    && (itemById is null || IsGeneratedExtraName(extra.Name, candidate)))
                 {
                     extra.Name = name;
                 }
@@ -3527,6 +3530,31 @@ namespace Emby.Server.Implementations.Library
 
                 return null;
             }
+
+            bool IsGeneratedExtraName(string currentName, ExtraCandidate candidate)
+            {
+                // The file name is what an extra was called before it was given a name of its type
+                if (string.Equals(currentName, candidate.Extra.Name, StringComparison.Ordinal))
+                {
+                    return true;
+                }
+
+                if (!generatedNames.TryGetValue(candidate.ExtraType, out var names))
+                {
+                    // Any of the numbers of this type may have been handed out, as the order the extras
+                    // of a type are numbered in shifts as files appear beside them or are taken away
+                    names = new HashSet<string>(StringComparer.Ordinal);
+                    var count = candidates.Count(c => c.ExtraType == candidate.ExtraType);
+                    for (var seen = 0; seen < count; seen++)
+                    {
+                        names.Add(GetNumberedExtraName(candidate.ExtraType, seen));
+                    }
+
+                    generatedNames[candidate.ExtraType] = names;
+                }
+
+                return names.Contains(currentName);
+            }
         }
 
         /// <summary>
@@ -3553,7 +3581,18 @@ namespace Emby.Server.Implementations.Library
             typeCounters.TryGetValue(candidate.ExtraType, out var seen);
             typeCounters[candidate.ExtraType] = seen + 1;
 
-            var typeName = _localization.GetServerLocalizedString(GetExtraTypeNameKey(candidate.ExtraType));
+            return GetNumberedExtraName(candidate.ExtraType, seen);
+        }
+
+        /// <summary>
+        /// Gets the name given to the n-th extra of a type that is named after its type.
+        /// </summary>
+        /// <param name="extraType">The extra type.</param>
+        /// <param name="seen">Number of extras of the type named before this one.</param>
+        /// <returns>The name.</returns>
+        private string GetNumberedExtraName(ExtraType extraType, int seen)
+        {
+            var typeName = _localization.GetServerLocalizedString(GetExtraTypeNameKey(extraType));
 
             return seen == 0
                 ? typeName
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
index a28c1d6dfb..f717d9b665 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
@@ -29,6 +29,7 @@ public class FindExtrasTests
 {
     private readonly Emby.Server.Implementations.Library.LibraryManager _libraryManager;
     private readonly Mock<IFileSystem> _fileSystemMock;
+    private readonly Mock<IItemRepository> _itemRepositoryMock;
 
     public FindExtrasTests()
     {
@@ -36,8 +37,8 @@ public class FindExtrasTests
         fixture.Register(() => new NamingOptions());
         var configMock = fixture.Freeze<Mock<IServerConfigurationManager>>();
         configMock.Setup(c => c.ApplicationPaths.ProgramDataPath).Returns("/data");
-        var itemRepository = fixture.Freeze<Mock<IItemRepository>>();
-        itemRepository.Setup(i => i.RetrieveItem(It.IsAny<Guid>())).Returns<BaseItem>(null);
+        _itemRepositoryMock = fixture.Freeze<Mock<IItemRepository>>();
+        _itemRepositoryMock.Setup(i => i.RetrieveItem(It.IsAny<Guid>())).Returns<BaseItem>(null);
         _fileSystemMock = fixture.Freeze<Mock<IFileSystem>>();
         _fileSystemMock.Setup(f => f.GetFileInfo(It.IsAny<string>())).Returns<string>(path => new FileSystemMetadata { FullName = path });
 
@@ -482,6 +483,69 @@ public class FindExtrasTests
     }
 
     [Fact]
+    public void FindExtras_ExtraNamedByLocalMetadata_KeepsItsNameOnRescan()
+    {
+        var owner = new Movie { Name = "Up", Path = "/movies/Up (2009)/Up (2009).mkv" };
+        var paths = new List<string>
+        {
+            "/movies/Up (2009)/Up (2009).mkv",
+            "/movies/Up (2009)/Up (2009)-trailer.mkv"
+        };
+
+        var files = paths.Select(p => new FileSystemMetadata
+        {
+            FullName = p,
+            IsDirectory = false
+        }).ToList();
+
+        var directoryService = new DirectoryService(_fileSystemMock.Object);
+        var extra = Assert.Single(_libraryManager.FindExtras(owner, files, directoryService));
+        Assert.Equal("Trailer", extra.Name);
+
+        // A local metadata file gives the extra a title of its own, which the refresh persists
+        extra.Name = "Cannes Teaser";
+        _itemRepositoryMock.Setup(i => i.RetrieveItem(extra.Id)).Returns(extra);
+
+        var rescanned = Assert.Single(_libraryManager.FindExtras(owner, files, directoryService));
+        Assert.Equal("Cannes Teaser", rescanned.Name);
+    }
+
+    [Fact]
+    public void FindExtras_ExtraKeptItsGeneratedName_IsRenumberedOnRescan()
+    {
+        var owner = new Movie { Name = "Up", Path = "/movies/Up (2009)/Up (2009).mkv" };
+        var paths = new List<string>
+        {
+            "/movies/Up (2009)/Up (2009).mkv",
+            "/movies/Up (2009)/Up (2009)-trailer2.mkv"
+        };
+
+        var files = paths.Select(p => new FileSystemMetadata
+        {
+            FullName = p,
+            IsDirectory = false
+        }).ToList();
+
+        var directoryService = new DirectoryService(_fileSystemMock.Object);
+        var extra = Assert.Single(_libraryManager.FindExtras(owner, files, directoryService));
+        Assert.Equal("Trailer", extra.Name);
+        _itemRepositoryMock.Setup(i => i.RetrieveItem(extra.Id)).Returns(extra);
+
+        // A trailer sorting before the known one takes the first number, so the known one moves on
+        files.Add(new FileSystemMetadata
+        {
+            FullName = "/movies/Up (2009)/Up (2009)-trailer1.mkv",
+            IsDirectory = false
+        });
+
+        var rescanned = _libraryManager.FindExtras(owner, files, directoryService)
+            .ToDictionary(e => e.Path, e => e.Name, StringComparer.Ordinal);
+
+        Assert.Equal("Trailer", rescanned["/movies/Up (2009)/Up (2009)-trailer1.mkv"]);
+        Assert.Equal("Trailer 2", rescanned["/movies/Up (2009)/Up (2009)-trailer2.mkv"]);
+    }
+
+    [Fact]
     public void FindExtras_ExtraWithOwnTitleBesideOwner_KeepsTitle()
     {
         var owner = new Movie { Name = "Up", Path = "/movies/Up (2009)/Up (2009).mkv" };