diff options
| -rw-r--r-- | Emby.Server.Implementations/Library/LibraryManager.cs | 49 | ||||
| -rw-r--r-- | tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs | 68 |
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" }; |