Merge pull request #14540 from TokerX/issue-8641

Improve extra rule resolution and file handling

CONTRIBUTORS.md

@@ -202,6 +202,7 @@
  - [ThunderClapLP](https://github.com/ThunderClapLP)
  - [Shoham Peller](https://github.com/spellr)
  - [theshoeshiner](https://github.com/theshoeshiner)
+ - [TokerX](https://github.com/TokerX)
 
 # Emby Contributors
 

Emby.Naming/Common/NamingOptions.cs

@@ -572,6 +572,18 @@ namespace Emby.Naming.Common
                     "trailer",
                     MediaType.Video),
 
+                new ExtraRule(
+                    ExtraType.Sample,
+                    ExtraRuleType.Filename,
+                    "sample",
+                    MediaType.Video),
+
+                new ExtraRule(
+                    ExtraType.ThemeSong,
+                    ExtraRuleType.Filename,
+                    "theme",
+                    MediaType.Audio),
+
                 new ExtraRule(
                     ExtraType.Trailer,
                     ExtraRuleType.Suffix,
@@ -593,13 +605,7 @@ namespace Emby.Naming.Common
                 new ExtraRule(
                     ExtraType.Trailer,
                     ExtraRuleType.Suffix,
-                    " trailer",
-                    MediaType.Video),
-
-                new ExtraRule(
-                    ExtraType.Sample,
-                    ExtraRuleType.Filename,
-                    "sample",
+                    "- trailer",
                     MediaType.Video),
 
                 new ExtraRule(
@@ -623,15 +629,9 @@ namespace Emby.Naming.Common
                 new ExtraRule(
                     ExtraType.Sample,
                     ExtraRuleType.Suffix,
-                    " sample",
+                    "- sample",
                     MediaType.Video),
 
-                new ExtraRule(
-                    ExtraType.ThemeSong,
-                    ExtraRuleType.Filename,
-                    "theme",
-                    MediaType.Audio),
-
                 new ExtraRule(
                     ExtraType.Scene,
                     ExtraRuleType.Suffix,

Emby.Naming/Video/ExtraRuleResolver.cs

@@ -22,67 +22,45 @@ namespace Emby.Naming.Video
         /// <returns>Returns <see cref="ExtraResult"/> object.</returns>
         public static ExtraResult GetExtraInfo(string path, NamingOptions namingOptions, string? libraryRoot = "")
         {
-            var result = new ExtraResult();
+            ExtraResult result = new ExtraResult();
 
-            for (var i = 0; i < namingOptions.VideoExtraRules.Length; i++)
+            bool isAudioFile = AudioFileParser.IsAudioFile(path, namingOptions);
+            bool isVideoFile = VideoResolver.IsVideoFile(path, namingOptions);
+
+            ReadOnlySpan<char> pathSpan = path.AsSpan();
+            ReadOnlySpan<char> fileName = Path.GetFileName(pathSpan);
+            ReadOnlySpan<char> fileNameWithoutExtension = Path.GetFileNameWithoutExtension(pathSpan);
+            // Trim the digits from the end of the filename so we can recognize things like -trailer2
+            ReadOnlySpan<char> trimmedFileNameWithoutExtension = fileNameWithoutExtension.TrimEnd(_digits);
+            ReadOnlySpan<char> directoryName = Path.GetFileName(Path.GetDirectoryName(pathSpan));
+            string fullDirectory = Path.GetDirectoryName(pathSpan).ToString();
+
+            foreach (ExtraRule rule in namingOptions.VideoExtraRules)
             {
-                var rule = namingOptions.VideoExtraRules[i];
-                if ((rule.MediaType == MediaType.Audio && !AudioFileParser.IsAudioFile(path, namingOptions))
-                    || (rule.MediaType == MediaType.Video && !VideoResolver.IsVideoFile(path, namingOptions)))
+                if ((rule.MediaType == MediaType.Audio && !isAudioFile)
+                    || (rule.MediaType == MediaType.Video && !isVideoFile))
                 {
                     continue;
                 }
 
-                var pathSpan = path.AsSpan();
-                if (rule.RuleType == ExtraRuleType.Filename)
+                bool isMatch = rule.RuleType switch
                 {
-                    var filename = Path.GetFileNameWithoutExtension(pathSpan);
+                    ExtraRuleType.Filename => fileNameWithoutExtension.Equals(rule.Token, StringComparison.OrdinalIgnoreCase),
+                    ExtraRuleType.Suffix => trimmedFileNameWithoutExtension.EndsWith(rule.Token, StringComparison.OrdinalIgnoreCase),
+                    ExtraRuleType.Regex => Regex.IsMatch(fileName, rule.Token, RegexOptions.IgnoreCase | RegexOptions.Compiled),
+                    ExtraRuleType.DirectoryName => directoryName.Equals(rule.Token, StringComparison.OrdinalIgnoreCase)
+                                                 && !string.Equals(fullDirectory, libraryRoot, StringComparison.OrdinalIgnoreCase),
+                    _ => false,
+                };
 
-                    if (filename.Equals(rule.Token, StringComparison.OrdinalIgnoreCase))
-                    {
-                        result.ExtraType = rule.ExtraType;
-                        result.Rule = rule;
-                    }
-                }
-                else if (rule.RuleType == ExtraRuleType.Suffix)
+                if (!isMatch)
                 {
-                    // Trim the digits from the end of the filename so we can recognize things like -trailer2
-                    var filename = Path.GetFileNameWithoutExtension(pathSpan).TrimEnd(_digits);
-
-                    if (filename.EndsWith(rule.Token, StringComparison.OrdinalIgnoreCase))
-                    {
-                        result.ExtraType = rule.ExtraType;
-                        result.Rule = rule;
-                    }
-                }
-                else if (rule.RuleType == ExtraRuleType.Regex)
-                {
-                    var filename = Path.GetFileName(path.AsSpan());
-
-                    var isMatch = Regex.IsMatch(filename, rule.Token, RegexOptions.IgnoreCase | RegexOptions.Compiled);
-
-                    if (isMatch)
-                    {
-                        result.ExtraType = rule.ExtraType;
-                        result.Rule = rule;
-                    }
-                }
-                else if (rule.RuleType == ExtraRuleType.DirectoryName)
-                {
-                    var directoryName = Path.GetFileName(Path.GetDirectoryName(pathSpan));
-                    string fullDirectory = Path.GetDirectoryName(pathSpan).ToString();
-                    if (directoryName.Equals(rule.Token, StringComparison.OrdinalIgnoreCase)
-                        && !string.Equals(fullDirectory, libraryRoot, StringComparison.OrdinalIgnoreCase))
-                    {
-                        result.ExtraType = rule.ExtraType;
-                        result.Rule = rule;
-                    }
+                    continue;
                 }
 
-                if (result.ExtraType is not null)
-                {
-                    return result;
-                }
+                result.ExtraType = rule.ExtraType;
+                result.Rule = rule;
+                return result;
             }
 
             return result;

Emby.Server.Implementations/Library/CoreResolutionIgnoreRule.cs

@@ -38,7 +38,8 @@ namespace Emby.Server.Implementations.Library
             }
 
             // Don't ignore top level folders
-            if (fileInfo.IsDirectory && parent is AggregateFolder)
+            if (fileInfo.IsDirectory
+                && (parent is AggregateFolder || (parent?.IsTopParent ?? false)))
             {
                 return false;
             }
@@ -48,35 +49,21 @@ namespace Emby.Server.Implementations.Library
                 return true;
             }
 
-            var filename = fileInfo.Name;
+            if (parent is null)
+            {
+                return false;
+            }
 
             if (fileInfo.IsDirectory)
             {
-                if (parent is not null)
-                {
-                    // Ignore extras for unsupported types
-                    if (_namingOptions.AllExtrasTypesFolderNames.ContainsKey(filename)
-                        && parent is not AggregateFolder
-                        && parent is not UserRootFolder)
-                    {
-                        return true;
-                    }
-                }
-            }
-            else
-            {
-                if (parent is not null)
-                {
-                    // Don't resolve theme songs
-                    if (Path.GetFileNameWithoutExtension(filename.AsSpan()).Equals(BaseItem.ThemeSongFileName, StringComparison.Ordinal)
-                        && AudioFileParser.IsAudioFile(filename, _namingOptions))
-                    {
-                        return true;
-                    }
-                }
+                // Ignore extras for unsupported types
+                return _namingOptions.AllExtrasTypesFolderNames.ContainsKey(fileInfo.Name)
+                    && parent is not UserRootFolder;
             }
 
-            return false;
+            // Don't resolve theme songs
+            return Path.GetFileNameWithoutExtension(fileInfo.Name.AsSpan()).Equals(BaseItem.ThemeSongFileName, StringComparison.Ordinal)
+                && AudioFileParser.IsAudioFile(fileInfo.Name, _namingOptions);
         }
     }
 }

tests/Jellyfin.Naming.Tests/Video/ExtraTests.cs

@@ -23,7 +23,7 @@ namespace Jellyfin.Naming.Tests.Video
             Test("300-trailer.mp4", ExtraType.Trailer);
             Test("300.trailer.mp4", ExtraType.Trailer);
             Test("300_trailer.mp4", ExtraType.Trailer);
-            Test("300 trailer.mp4", ExtraType.Trailer);
+            Test("300 - trailer.mp4", ExtraType.Trailer);
 
             Test("theme.mp3", ExtraType.ThemeSong);
         }
@@ -132,7 +132,14 @@ namespace Jellyfin.Naming.Tests.Video
             Test("300-sample.mp4", ExtraType.Sample);
             Test("300.sample.mp4", ExtraType.Sample);
             Test("300_sample.mp4", ExtraType.Sample);
-            Test("300 sample.mp4", ExtraType.Sample);
+            Test("300 - sample.mp4", ExtraType.Sample);
+        }
+
+        [Fact]
+        public void TestSuffixPartOfTitle()
+        {
+            Test("I Live In A Trailer.mp4", null);
+            Test("The DNA Sample.mp4", null);
         }
 
         private void Test(string input, ExtraType? expectedType)

tests/Jellyfin.Naming.Tests/Video/VideoListResolverTests.cs

@@ -87,7 +87,7 @@ namespace Jellyfin.Naming.Tests.Video
             var files = new[]
             {
                 "300.mkv",
-                "300 trailer.mkv"
+                "300 - trailer.mkv"
             };
 
             var result = VideoListResolver.Resolve(

tests/Jellyfin.Server.Implementations.Tests/Library/CoreResolutionIgnoreRuleTest.cs

@@ -0,0 +1,129 @@
+using System;
+using System.IO;
+using Emby.Naming.Common;
+using Emby.Naming.Video;
+using Emby.Server.Implementations.Library;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.IO;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Library;
+
+public class CoreResolutionIgnoreRuleTest
+{
+    private readonly CoreResolutionIgnoreRule _rule;
+    private readonly NamingOptions _namingOptions;
+    private readonly Mock<IServerApplicationPaths> _appPathsMock;
+
+    public CoreResolutionIgnoreRuleTest()
+    {
+        _namingOptions = new NamingOptions();
+
+        _namingOptions.AllExtrasTypesFolderNames.TryAdd("extras", ExtraType.Trailer);
+
+        _appPathsMock = new Mock<IServerApplicationPaths>();
+        _appPathsMock.SetupGet(x => x.RootFolderPath).Returns("/server/root");
+
+        _rule = new CoreResolutionIgnoreRule(_namingOptions, _appPathsMock.Object);
+    }
+
+    private FileSystemMetadata MakeFileSystemMetadata(string fullName, bool isDirectory = false)
+        => new FileSystemMetadata { FullName = fullName, Name = Path.GetFileName(fullName), IsDirectory = isDirectory };
+
+    private BaseItem MakeParent(string name = "Parent", bool isTopParent = false, Type? type = null)
+    {
+        return type switch
+        {
+            Type t when t == typeof(Folder) => CreateMock<Folder>(name, isTopParent).Object,
+            Type t when t == typeof(AggregateFolder) => CreateMock<AggregateFolder>(name, isTopParent).Object,
+            Type t when t == typeof(UserRootFolder) => CreateMock<UserRootFolder>(name, isTopParent).Object,
+            _ => CreateMock<BaseItem>(name, isTopParent).Object
+        };
+    }
+
+    private static Mock<T> CreateMock<T>(string name, bool isTopParent)
+    where T : BaseItem
+    {
+        var mock = new Mock<T>();
+        mock.SetupGet(p => p.Name).Returns(name);
+        mock.SetupGet(p => p.IsTopParent).Returns(isTopParent);
+        return mock;
+    }
+
+    [Fact]
+    public void TestApplicationFolder()
+    {
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/server/root/extras", isDirectory: true),
+            null));
+
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/server/root/small.jpg"),
+            null));
+    }
+
+    [Fact]
+    public void TestTopLevelDirectory()
+    {
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("Series/Extras", true),
+            MakeParent(type: typeof(AggregateFolder))));
+
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("Series/Extras/Extras", true),
+            MakeParent(isTopParent: true)));
+    }
+
+    [Fact]
+    public void TestIgnorePatterns()
+    {
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/Media/big.jpg"),
+            MakeParent()));
+
+        Assert.True(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/Media/small.jpg"),
+            MakeParent()));
+    }
+
+    [Fact]
+    public void TestExtrasTypesFolderNames()
+    {
+        FileSystemMetadata fileSystemMetadata = MakeFileSystemMetadata("/Movies/Up/extras", true);
+
+        Assert.False(_rule.ShouldIgnore(
+            fileSystemMetadata,
+            MakeParent(type: typeof(AggregateFolder))));
+
+        Assert.False(_rule.ShouldIgnore(
+            fileSystemMetadata,
+            MakeParent(type: typeof(UserRootFolder))));
+
+        Assert.False(_rule.ShouldIgnore(
+            fileSystemMetadata,
+            null));
+
+        Assert.True(_rule.ShouldIgnore(
+            fileSystemMetadata,
+            MakeParent()));
+
+        Assert.True(_rule.ShouldIgnore(
+            fileSystemMetadata,
+            MakeParent(type: typeof(Folder))));
+    }
+
+    [Fact]
+    public void TestThemeSong()
+    {
+        Assert.False(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/Movies/Up/intro.mp3"),
+            MakeParent()));
+
+        Assert.True(_rule.ShouldIgnore(
+            MakeFileSystemMetadata("/Movies/Up/theme.mp3"),
+            MakeParent()));
+    }
+}

tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs

@@ -301,7 +301,7 @@ namespace Jellyfin.Server.Implementations.Tests.Plugins
 
             var versionInfo = fixture.Create<VersionInfo>();
             versionInfo.Version = new Version(1, 0).ToString();
-            versionInfo.Timestamp = DateTime.UtcNow.ToString(CultureInfo.InvariantCulture);
+            versionInfo.Timestamp = DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture);
 
             var packageInfo = fixture.Create<PackageInfo>();
             packageInfo.Versions = new[] { versionInfo };