From f4292be58857727119d7478c2a9c640b9b628738 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sat, 3 Aug 2019 13:20:34 -0700 Subject: [PATCH] New: Limit filenames to a maximum of 255 characters (cherry picked from commit 34d81356a3b3b378ce669ea65c5802b64efaad6e) --- .../TruncatedTrackTitlesFixture.cs | 144 +++++++++++++++++ src/NzbDrone.Core/Fluent.cs | 5 + .../Organizer/FileNameBuilder.cs | 151 +++++++++++++++--- 3 files changed, 274 insertions(+), 26 deletions(-) create mode 100644 src/NzbDrone.Core.Test/OrganizerTests/FileNameBuilderTests/TruncatedTrackTitlesFixture.cs diff --git a/src/NzbDrone.Core.Test/OrganizerTests/FileNameBuilderTests/TruncatedTrackTitlesFixture.cs b/src/NzbDrone.Core.Test/OrganizerTests/FileNameBuilderTests/TruncatedTrackTitlesFixture.cs new file mode 100644 index 000000000..1994a233c --- /dev/null +++ b/src/NzbDrone.Core.Test/OrganizerTests/FileNameBuilderTests/TruncatedTrackTitlesFixture.cs @@ -0,0 +1,144 @@ +using System.Collections.Generic; +using System.Linq; +using FizzWare.NBuilder; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.Music; +using NzbDrone.Core.Organizer; +using NzbDrone.Core.Qualities; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.OrganizerTests.FileNameBuilderTests +{ + [TestFixture] + public class TruncatedTrackTitlesFixture : CoreTest + { + private Artist _artist; + private Album _album; + private AlbumRelease _release; + private List _tracks; + private TrackFile _trackFile; + private NamingConfig _namingConfig; + + [SetUp] + public void Setup() + { + _artist = Builder + .CreateNew() + .With(s => s.Name = "Avenged Sevenfold") + .Build(); + + _album = Builder + .CreateNew() + .With(s => s.Title = "Hail to the King") + .Build(); + + _release = Builder + .CreateNew() + .With(s => s.Media = new List { new () { Number = 14 } }) + .Build(); + + _namingConfig = NamingConfig.Default; + _namingConfig.RenameTracks = true; + + Mocker.GetMock() + .Setup(c => c.GetConfig()).Returns(_namingConfig); + + _tracks = new List + { + Builder.CreateNew() + .With(e => e.Title = "First Track Title 1") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 1) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "Another Track Title") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 2) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "Yet Another Track Title") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 3) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "Yet Another Track Title Take 2") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 4) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "Yet Another Track Title Take 3") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 5) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "Yet Another Track Title Take 4") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 6) + .With(e => e.AlbumRelease = _release) + .Build(), + + Builder.CreateNew() + .With(e => e.Title = "A Really Really Really Really Long Track Title") + .With(e => e.MediumNumber = 1) + .With(e => e.AbsoluteTrackNumber = 7) + .With(e => e.AlbumRelease = _release) + .Build() + }; + + _trackFile = new TrackFile { Quality = new QualityModel(Quality.MP3_320), ReleaseGroup = "LidarrTest" }; + + Mocker.GetMock() + .Setup(v => v.Get(Moq.It.IsAny())) + .Returns(v => Quality.DefaultQualityDefinitions.First(c => c.Quality == v)); + } + + private void GivenProper() + { + _trackFile.Quality.Revision.Version = 2; + } + + [Test] + public void should_truncate_with_ellipsis_between_first_and_last_episode_titles() + { + _namingConfig.StandardTrackFormat = "{Artist Name} - {Album Title} - {track:00} - {Track Title} [{Quality Title}]"; + + var result = Subject.BuildTrackFileName(_tracks, _artist, _album, _trackFile); + result.Length.Should().BeLessOrEqualTo(255); + result.Should().Be("Avenged Sevenfold - Hail to the King - 01 - First Track Title 1...A Really Really Really Really Long Track Title [MP3-320]"); + } + + [Test] + public void should_truncate_with_ellipsis_if_only_first_episode_title_fits() + { + _artist.Name = "Lorem ipsum dolor sit amet, consectetur adipiscing elit Maecenas et magna sem Morbi vitae volutpat quam, id porta arcu Orci varius natoque penatibus et magnis dis parturient montes"; + _namingConfig.StandardTrackFormat = "{Artist Name} - {Album Title} - {track:00} - {Track Title} [{Quality Title}]"; + + var result = Subject.BuildTrackFileName(_tracks, _artist, _album, _trackFile); + result.Should().Be("Lorem ipsum dolor sit amet, consectetur adipiscing elit Maecenas et magna sem Morbi vitae volutpat quam, id porta arcu Orci varius natoque penatibus et magnis dis parturient montes - Hail to the King - 01 - First Track Title 1... [MP3-320]"); + result.Length.Should().BeLessOrEqualTo(255); + } + + [Test] + public void should_truncate_first_episode_title_with_ellipsis_if_only_partially_fits() + { + _artist.Name = "Lorem ipsum dolor sit amet, consectetur adipiscing elit Maecenas et magna sem Morbi vitae volutpat quam, id porta arcu Orci varius natoque penatibus et magnis dis parturient montes nascetur ridiculus musu Cras"; + _namingConfig.StandardTrackFormat = "{Artist Name} - {Album Title} - {track:00} - {Track Title} [{Quality Title}]"; + + var result = Subject.BuildTrackFileName(new List { _tracks.First() }, _artist, _album, _trackFile); + result.Should().Be("Lorem ipsum dolor sit amet, consectetur adipiscing elit Maecenas et magna sem Morbi vitae volutpat quam, id porta arcu Orci varius natoque penatibus et magnis dis parturient montes nascetur ridiculus musu Cras - Hail to the King - 01 - First... [MP3-320]"); + result.Length.Should().BeLessOrEqualTo(255); + } + } +} diff --git a/src/NzbDrone.Core/Fluent.cs b/src/NzbDrone.Core/Fluent.cs index 541d843f7..1e04fc227 100644 --- a/src/NzbDrone.Core/Fluent.cs +++ b/src/NzbDrone.Core/Fluent.cs @@ -97,6 +97,11 @@ public static int MaxOrDefault(this IEnumerable ints) return intList.Max(); } + public static int GetByteCount(this string input) + { + return Encoding.UTF8.GetByteCount(input); + } + public static string Truncate(this string s, int maxLength) { if (Encoding.UTF8.GetByteCount(s) <= maxLength) diff --git a/src/NzbDrone.Core/Organizer/FileNameBuilder.cs b/src/NzbDrone.Core/Organizer/FileNameBuilder.cs index 2535c9296..6ea1a15ef 100644 --- a/src/NzbDrone.Core/Organizer/FileNameBuilder.cs +++ b/src/NzbDrone.Core/Organizer/FileNameBuilder.cs @@ -6,6 +6,7 @@ using System.Text.RegularExpressions; using NLog; using NzbDrone.Common.Cache; +using NzbDrone.Common.Disk; using NzbDrone.Common.EnsureThat; using NzbDrone.Common.Extensions; using NzbDrone.Core.CustomFormats; @@ -82,7 +83,7 @@ public FileNameBuilder(INamingConfigService namingConfigService, _logger = logger; } - public string BuildTrackFileName(List tracks, Artist artist, Album album, TrackFile trackFile, NamingConfig namingConfig = null, List customFormats = null) + private string BuildTrackFileName(List tracks, Artist artist, Album album, TrackFile trackFile, int maxPath, NamingConfig namingConfig = null, List customFormats = null) { if (namingConfig == null) { @@ -106,33 +107,39 @@ public string BuildTrackFileName(List tracks, Artist artist, Album album, pattern = namingConfig.MultiDiscTrackFormat; } - var tokenHandlers = new Dictionary>(FileNameBuilderTokenEqualityComparer.Instance); - tracks = tracks.OrderBy(e => e.AlbumReleaseId).ThenBy(e => e.TrackNumber).ToList(); - AddArtistTokens(tokenHandlers, artist); - AddAlbumTokens(tokenHandlers, album); - AddMediumTokens(tokenHandlers, tracks.First().AlbumRelease.Value.Media.SingleOrDefault(m => m.Number == tracks.First().MediumNumber)); - AddTrackTokens(tokenHandlers, tracks, artist); - AddTrackFileTokens(tokenHandlers, trackFile); - AddQualityTokens(tokenHandlers, artist, trackFile); - AddMediaInfoTokens(tokenHandlers, trackFile); - AddCustomFormats(tokenHandlers, artist, trackFile, customFormats); - var splitPatterns = pattern.Split(new char[] { '\\', '/' }, StringSplitOptions.RemoveEmptyEntries); var components = new List(); foreach (var s in splitPatterns) { var splitPattern = s; - + var tokenHandlers = new Dictionary>(FileNameBuilderTokenEqualityComparer.Instance); splitPattern = FormatTrackNumberTokens(splitPattern, "", tracks); splitPattern = FormatMediumNumberTokens(splitPattern, "", tracks); - var component = ReplaceTokens(splitPattern, tokenHandlers, namingConfig).Trim(); + AddArtistTokens(tokenHandlers, artist); + AddAlbumTokens(tokenHandlers, album); + AddMediumTokens(tokenHandlers, tracks.First().AlbumRelease.Value.Media.SingleOrDefault(m => m.Number == tracks.First().MediumNumber)); + AddTrackTokens(tokenHandlers, tracks, artist); + AddTrackTitlePlaceholderTokens(tokenHandlers); + AddTrackFileTokens(tokenHandlers, trackFile); + AddQualityTokens(tokenHandlers, artist, trackFile); + AddMediaInfoTokens(tokenHandlers, trackFile); + AddCustomFormats(tokenHandlers, artist, trackFile, customFormats); + + var component = ReplaceTokens(splitPattern, tokenHandlers, namingConfig, true).Trim(); + var maxPathSegmentLength = Math.Min(LongPathSupport.MaxFileNameLength, maxPath); + + var maxTrackTitleLength = maxPathSegmentLength - GetLengthWithoutTrackTitle(component, namingConfig); + + AddTrackTitleTokens(tokenHandlers, tracks, maxTrackTitleLength); + component = ReplaceTokens(component, tokenHandlers, namingConfig).Trim(); component = FileNameCleanupRegex.Replace(component, match => match.Captures[0].Value[0].ToString()); component = TrimSeparatorsRegex.Replace(component, string.Empty); + component = component.Replace("{ellipsis}", "..."); if (component.IsNotNullOrWhiteSpace()) { @@ -143,6 +150,11 @@ public string BuildTrackFileName(List tracks, Artist artist, Album album, return Path.Combine(components.ToArray()); } + public string BuildTrackFileName(List tracks, Artist artist, Album album, TrackFile trackFile, NamingConfig namingConfig = null, List customFormats = null) + { + return BuildTrackFileName(tracks, artist, album, trackFile, LongPathSupport.MaxFilePathLength, namingConfig, customFormats); + } + public string BuildTrackFilePath(Artist artist, string fileName, string extension) { Ensure.That(extension, () => extension).IsNotNullOrWhiteSpace(); @@ -300,9 +312,6 @@ private void AddMediumTokens(Dictionary> tokenH private void AddTrackTokens(Dictionary> tokenHandlers, List tracks, Artist artist) { - tokenHandlers["{Track Title}"] = m => GetTrackTitle(tracks, "+"); - tokenHandlers["{Track CleanTitle}"] = m => CleanTitle(GetTrackTitle(tracks, "and")); - // Use the track's ArtistMetadata by default, as it will handle the "Various Artists" case // (where the album artist is "Various Artists" but each track has its own artist). Fall back // to the album artist if we don't have any track ArtistMetadata for whatever reason. @@ -316,6 +325,18 @@ private void AddTrackTokens(Dictionary> tokenHa } } + private void AddTrackTitlePlaceholderTokens(Dictionary> tokenHandlers) + { + tokenHandlers["{Track Title}"] = m => null; + tokenHandlers["{Track CleanTitle}"] = m => null; + } + + private void AddTrackTitleTokens(Dictionary> tokenHandlers, List tracks, int maxLength) + { + tokenHandlers["{Track Title}"] = m => GetTrackTitle(GetTrackTitles(tracks), "+", maxLength); + tokenHandlers["{Track CleanTitle}"] = m => GetTrackTitle(GetTrackTitles(tracks).Select(CleanTitle).ToList(), "and", maxLength); + } + private void AddTrackFileTokens(Dictionary> tokenHandlers, TrackFile trackFile) { tokenHandlers["{Original Title}"] = m => GetOriginalTitle(trackFile); @@ -369,13 +390,29 @@ private void AddCustomFormats(Dictionary> token tokenHandlers["{Custom Formats}"] = m => string.Join(" ", customFormats.Where(x => x.IncludeCustomFormatWhenRenaming)); } - private string ReplaceTokens(string pattern, Dictionary> tokenHandlers, NamingConfig namingConfig) + private string ReplaceTokens(string pattern, Dictionary> tokenHandlers, NamingConfig namingConfig, bool escape = false) { - return TitleRegex.Replace(pattern, match => ReplaceToken(match, tokenHandlers, namingConfig)); + return TitleRegex.Replace(pattern, match => ReplaceToken(match, tokenHandlers, namingConfig, escape)); } - private string ReplaceToken(Match match, Dictionary> tokenHandlers, NamingConfig namingConfig) + private string ReplaceToken(Match match, Dictionary> tokenHandlers, NamingConfig namingConfig, bool escape) { + if (match.Groups["escaped"].Success) + { + if (escape) + { + return match.Value; + } + else if (match.Value == "{{") + { + return "{"; + } + else if (match.Value == "}}") + { + return "}"; + } + } + var tokenMatch = new TokenMatch { RegexMatch = match, @@ -393,7 +430,15 @@ private string ReplaceToken(Match match, Dictionary string.Empty); - var replacementText = tokenHandler(tokenMatch).Trim(); + var replacementText = tokenHandler(tokenMatch); + + if (replacementText == null) + { + // Preserve original token if handler returned null + return match.Value; + } + + replacementText = replacementText.Trim(); if (tokenMatch.Token.All(t => !char.IsLetter(t) || char.IsLower(t))) { @@ -416,6 +461,11 @@ private string ReplaceToken(Match match, Dictionary tracks, string separator) + private List GetTrackTitles(List tracks) { - separator = string.Format(" {0} ", separator.Trim()); - if (tracks.Count == 1) { - return tracks.First().Title.TrimEnd(TrackTitleTrimCharacters); + return new List + { + tracks.First().Title.TrimEnd(TrackTitleTrimCharacters) + }; } var titles = tracks.Select(c => c.Title.TrimEnd(TrackTitleTrimCharacters)) @@ -490,7 +541,44 @@ private string GetTrackTitle(List tracks, string separator) .ToList(); } - return string.Join(separator, titles); + return titles; + } + + private string GetTrackTitle(List titles, string separator, int maxLength) + { + separator = $" {separator.Trim()} "; + + var joined = string.Join(separator, titles); + + if (joined.GetByteCount() <= maxLength) + { + return joined; + } + + var firstTitle = titles.First(); + var firstTitleLength = firstTitle.GetByteCount(); + + if (titles.Count >= 2) + { + var lastTitle = titles.Last(); + var lastTitleLength = lastTitle.GetByteCount(); + if (firstTitleLength + lastTitleLength + 3 <= maxLength) + { + return $"{firstTitle.TrimEnd(' ', '.')}{{ellipsis}}{lastTitle}"; + } + } + + if (titles.Count > 1 && firstTitleLength + 3 <= maxLength) + { + return $"{firstTitle.TrimEnd(' ', '.')}{{ellipsis}}"; + } + + if (titles.Count == 1 && firstTitleLength <= maxLength) + { + return firstTitle; + } + + return $"{firstTitle.Truncate(maxLength - 3).TrimEnd(' ', '.')}{{ellipsis}}"; } private string CleanupTrackTitle(string title) @@ -538,6 +626,17 @@ private string GetOriginalFileName(TrackFile trackFile) return Path.GetFileNameWithoutExtension(trackFile.Path); } + private int GetLengthWithoutTrackTitle(string pattern, NamingConfig namingConfig) + { + var tokenHandlers = new Dictionary>(FileNameBuilderTokenEqualityComparer.Instance); + tokenHandlers["{Track Title}"] = m => string.Empty; + tokenHandlers["{Track CleanTitle}"] = m => string.Empty; + + var result = ReplaceTokens(pattern, tokenHandlers, namingConfig); + + return result.GetByteCount(); + } + private static string CleanFileName(string name, NamingConfig namingConfig) { var result = name;