Merge pull request #17456 from Shadowghost/fix-extras

Fix extras naming and version assignment

3 files changed, 119 insertions, 21 deletions

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 983ecced02..de44e2ada5 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -45,6 +45,7 @@ using MediaBrowser.Model.Configuration;
 using MediaBrowser.Model.Drawing;
 using MediaBrowser.Model.Dto;
 using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.Globalization;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Model.Library;
 using MediaBrowser.Model.Querying;
@@ -86,6 +87,7 @@ namespace Emby.Server.Implementations.Library
         private readonly IPeopleRepository _peopleRepository;
         private readonly ExtraResolver _extraResolver;
         private readonly IPathManager _pathManager;
+        private readonly ILocalizationManager _localization;
         private readonly FastConcurrentLru<Guid, BaseItem> _cache;
         private readonly DotIgnoreIgnoreRule _dotIgnoreIgnoreRule;
         private readonly IMediaStreamRepository _mediaStreamRepository;
@@ -132,6 +134,7 @@ namespace Emby.Server.Implementations.Library
         /// <param name="peopleRepository">The people repository.</param>
         /// <param name="pathManager">The path manager.</param>
         /// <param name="dotIgnoreIgnoreRule">The .ignore rule handler.</param>
+        /// <param name="localization">The localization manager.</param>
         /// <param name="mediaStreamRepository">The media stream repository.</param>
         /// <param name="externalDataManagerFactory">The external data manager (lazy, to break the DI cycle through ChapterManager).</param>
         public LibraryManager(
@@ -157,6 +160,7 @@ namespace Emby.Server.Implementations.Library
             IPeopleRepository peopleRepository,
             IPathManager pathManager,
             DotIgnoreIgnoreRule dotIgnoreIgnoreRule,
+            ILocalizationManager localization,
             IMediaStreamRepository mediaStreamRepository,
             Lazy<IExternalDataManager> externalDataManagerFactory)
         {
@@ -184,6 +188,7 @@ namespace Emby.Server.Implementations.Library
             _peopleRepository = peopleRepository;
             _pathManager = pathManager;
             _dotIgnoreIgnoreRule = dotIgnoreIgnoreRule;
+            _localization = localization;
             _extraResolver = new ExtraResolver(loggerFactory.CreateLogger<ExtraResolver>(), namingOptions, directoryService);
 
             _configurationManager.ConfigurationUpdated += ConfigurationUpdated;
@@ -3280,9 +3285,11 @@ namespace Emby.Server.Implementations.Library
             var ownerVideoInfo = VideoResolver.Resolve(owner.Path, isFolder, _namingOptions, libraryRoot: owner.ContainingFolderPath);
             if (ownerVideoInfo is null)
             {
-                yield break;
+                return [];
             }
 
+            var candidates = new List<ExtraCandidate>();
+
             var count = filtered.Count;
             for (var i = 0; i < count; i++)
             {
@@ -3296,35 +3303,50 @@ namespace Emby.Server.Implementations.Library
 
                     foreach (var file in filesInSubFolderList)
                     {
-                        if (!_extraResolver.TryGetExtraTypeForOwner(file.FullName, ownerVideoInfo, out var extraType))
+                        if (!_extraResolver.TryGetExtraTypeForOwner(file.FullName, ownerVideoInfo, out var extraType, out var extraRule))
                         {
                             continue;
                         }
 
-                        var extra = GetExtra(file, extraType.Value, subFolderIsMixedFolder);
-                        if (extra is not null)
-                        {
-                            yield return extra;
-                        }
+                        AddCandidate(file, extraType.Value, extraRule, subFolderIsMixedFolder);
                     }
                 }
-                else if (!current.IsDirectory && _extraResolver.TryGetExtraTypeForOwner(current.FullName, ownerVideoInfo, out var extraType))
+                else if (!current.IsDirectory && _extraResolver.TryGetExtraTypeForOwner(current.FullName, ownerVideoInfo, out var extraType, out var extraRule))
                 {
-                    var extra = GetExtra(current, extraType.Value, false);
-                    if (extra is not null)
-                    {
-                        yield return extra;
-                    }
+                    AddCandidate(current, extraType.Value, extraRule, false);
+                }
+            }
+
+            var extras = new List<BaseItem>();
+            var typeCounters = new Dictionary<ExtraType, int>();
+
+            // 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
+            foreach (var candidate in candidates.OrderBy(c => c.Extra.Path, StringComparer.Ordinal))
+            {
+                var extra = PrepareExtra(candidate);
+                if (extra is not null)
+                {
+                    extras.Add(extra);
                 }
             }
 
-            BaseItem? GetExtra(FileSystemMetadata file, ExtraType extraType, bool isInMixedFolder)
+            return extras;
+
+            void AddCandidate(FileSystemMetadata file, ExtraType extraType, ExtraRule extraRule, bool isInMixedFolder)
             {
                 var extra = ResolvePath(_fileSystem.GetFileInfo(file.FullName), directoryService, _extraResolver.GetResolversForExtraType(extraType));
-                if (extra is not Video && extra is not Audio)
+                if (extra is Video or Audio)
                 {
-                    return null;
+                    candidates.Add(new ExtraCandidate(extra, extraType, extraRule, isInMixedFolder));
                 }
+            }
+
+            BaseItem? PrepareExtra(ExtraCandidate candidate)
+            {
+                var resolved = candidate.Extra;
+                var extra = resolved;
+                var name = GetExtraName(candidate, ownerVideoInfo, typeCounters);
 
                 // Try to retrieve it from the db. If we don't find it, use the resolved version
                 var itemById = GetItemById(extra.Id);
@@ -3333,10 +3355,18 @@ 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)
+                {
+                    extra.Name = name;
+                }
+
                 // Only update extra type if it is more specific then the currently known extra type
-                if (extra.ExtraType is null or ExtraType.Unknown || extraType != ExtraType.Unknown)
+                if (extra.ExtraType is null or ExtraType.Unknown || candidate.ExtraType != ExtraType.Unknown)
                 {
-                    extra.ExtraType = extraType;
+                    extra.ExtraType = candidate.ExtraType;
                 }
 
                 // Only return items that are actual extras (have ExtraType set)
@@ -3344,7 +3374,7 @@ namespace Emby.Server.Implementations.Library
                 // so that RefreshExtras can detect when they need updating and set ForceSave.
                 if (extra.ExtraType is not null)
                 {
-                    extra.IsInMixedFolder = isInMixedFolder;
+                    extra.IsInMixedFolder = candidate.IsInMixedFolder;
                     return extra;
                 }
 
@@ -3352,6 +3382,57 @@ namespace Emby.Server.Implementations.Library
             }
         }
 
+        /// <summary>
+        /// Gets the name to give an extra.
+        /// </summary>
+        /// <param name="candidate">The resolved extra.</param>
+        /// <param name="ownerVideoInfo">The naming info of the owner.</param>
+        /// <param name="typeCounters">Number of extras named after their type so far, per type.</param>
+        /// <returns>The name.</returns>
+        private string GetExtraName(ExtraCandidate candidate, VideoFileInfo ownerVideoInfo, Dictionary<ExtraType, int> typeCounters)
+        {
+            var isNamedAfterOwner = candidate.ExtraRule.RuleType switch
+            {
+                ExtraRuleType.Filename => true,
+                ExtraRuleType.Suffix => string.Equals(candidate.Extra.Name, ownerVideoInfo.Name, StringComparison.OrdinalIgnoreCase),
+                _ => false
+            };
+
+            if (!isNamedAfterOwner)
+            {
+                return candidate.Extra.Name;
+            }
+
+            typeCounters.TryGetValue(candidate.ExtraType, out var seen);
+            typeCounters[candidate.ExtraType] = seen + 1;
+
+            var typeName = _localization.GetServerLocalizedString(GetExtraTypeNameKey(candidate.ExtraType));
+
+            return seen == 0
+                ? typeName
+                : string.Format(
+                    CultureInfo.InvariantCulture,
+                    _localization.GetServerLocalizedString("NameExtraNumbered"),
+                    typeName,
+                    seen + 1);
+        }
+
+        private static string GetExtraTypeNameKey(ExtraType extraType) => extraType switch
+        {
+            ExtraType.Clip => "NameExtraClip",
+            ExtraType.Trailer => "NameExtraTrailer",
+            ExtraType.BehindTheScenes => "NameExtraBehindTheScenes",
+            ExtraType.DeletedScene => "NameExtraDeletedScene",
+            ExtraType.Interview => "NameExtraInterview",
+            ExtraType.Scene => "NameExtraScene",
+            ExtraType.Sample => "NameExtraSample",
+            ExtraType.ThemeSong => "NameExtraThemeSong",
+            ExtraType.ThemeVideo => "NameExtraThemeVideo",
+            ExtraType.Featurette => "NameExtraFeaturette",
+            ExtraType.Short => "NameExtraShort",
+            _ => "NameExtraUnknown"
+        };
+
         public string GetPathAfterNetworkSubstitution(string path, BaseItem? ownerItem)
         {
             foreach (var map in _configurationManager.Configuration.PathSubstitutions)
@@ -3902,5 +3983,7 @@ namespace Emby.Server.Implementations.Library
             SetTopParentOrAncestorIds(query);
             return _itemRepository.GetMediaStreamLanguages(query, mediaStreamType);
         }
+
+        private sealed record ExtraCandidate(BaseItem Extra, ExtraType ExtraType, ExtraRule ExtraRule, bool IsInMixedFolder);
     }
 }
diff --git a/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs b/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
index 6ba4a7bce6..a0f75e4ddb 100644
--- a/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
@@ -54,12 +54,13 @@ namespace Emby.Server.Implementations.Library.Resolvers
             _ => _videoResolvers
         };
 
-        public bool TryGetExtraTypeForOwner(string path, VideoFileInfo ownerVideoFileInfo, [NotNullWhen(true)] out ExtraType? extraType, string? libraryRoot = "")
+        public bool TryGetExtraTypeForOwner(string path, VideoFileInfo ownerVideoFileInfo, [NotNullWhen(true)] out ExtraType? extraType, [NotNullWhen(true)] out ExtraRule? extraRule, string? libraryRoot = "")
         {
             var extraResult = GetExtraInfo(path, _namingOptions, libraryRoot);
-            if (extraResult.ExtraType is null)
+            if (extraResult.ExtraType is null || extraResult.Rule is null)
             {
                 extraType = null;
+                extraRule = null;
                 return false;
             }
 
@@ -88,6 +89,7 @@ namespace Emby.Server.Implementations.Library.Resolvers
             }
 
             extraType = extraResult.ExtraType;
+            extraRule = extraResult.Rule;
             return isValid;
         }
 
diff --git a/Emby.Server.Implementations/Localization/Core/en-US.json b/Emby.Server.Implementations/Localization/Core/en-US.json
index 856941c61a..578c85da9d 100644
--- a/Emby.Server.Implementations/Localization/Core/en-US.json
+++ b/Emby.Server.Implementations/Localization/Core/en-US.json
@@ -28,6 +28,19 @@
     "Movies": "Movies",
     "Music": "Music",
     "MusicVideos": "Music Videos",
+    "NameExtraBehindTheScenes": "Behind The Scenes",
+    "NameExtraClip": "Clip",
+    "NameExtraDeletedScene": "Deleted Scene",
+    "NameExtraFeaturette": "Featurette",
+    "NameExtraInterview": "Interview",
+    "NameExtraNumbered": "{0} {1}",
+    "NameExtraSample": "Sample",
+    "NameExtraScene": "Scene",
+    "NameExtraShort": "Short",
+    "NameExtraThemeSong": "Theme Song",
+    "NameExtraThemeVideo": "Theme Video",
+    "NameExtraTrailer": "Trailer",
+    "NameExtraUnknown": "Extra",
     "NameInstallFailed": "{0} installation failed",
     "NameSeasonNumber": "Season {0}",
     "NameSeasonUnknown": "Season Unknown",