diff --git a/src/NzbDrone.Core.Test/Extras/ExtraServiceFixture.cs b/src/NzbDrone.Core.Test/Extras/ExtraServiceFixture.cs index fadd701ed2..81f394a853 100644 --- a/src/NzbDrone.Core.Test/Extras/ExtraServiceFixture.cs +++ b/src/NzbDrone.Core.Test/Extras/ExtraServiceFixture.cs @@ -27,6 +27,8 @@ public class ExtraServiceFixture : CoreTest private Artist _artist; private Album _album; private List _albumDirExtraFiles; + private string _downloadDir; + private List> _approvedDownloadDecisions; private List _downloadDirExtraFiles; [SetUp] @@ -44,8 +46,6 @@ public void Setup() Artist = _artist, Title = "Twenty Thirties", }; - _albumDir = Path.Join(_artist.Path, $"{_album.Title} (1995) [FLAC]"); - var release = new AlbumRelease() { AlbumId = _album.Id, @@ -53,6 +53,9 @@ public void Setup() }; _album.AlbumReleases = new List { release }; + // manual import files + _albumDir = Path.Join(_artist.Path, $"{_album.Title} (1995) [FLAC]"); + var track = new LocalTrack { Artist = _artist, @@ -65,7 +68,6 @@ public void Setup() { new ImportDecision(track) }; - _albumDirExtraFiles = new List { Path.Join(_albumDir, "album.cue"), @@ -74,18 +76,23 @@ public void Setup() Path.Join(_albumDir, "eac.log"), }; - var downloadDir = @"C:\temp\downloads\TT".AsOsAgnostic(); + // download dir files + _downloadDir = @"C:\temp\downloads\TT".AsOsAgnostic(); + var downloadedTrack = NewTrack(_album, _downloadDir, "01 - First seconds.flac"); + _approvedDownloadDecisions = new List>() + { + new ImportDecision(downloadedTrack), + }; _downloadDirExtraFiles = new List { - Path.Join(downloadDir, "album.cue"), - Path.Join(downloadDir, "cover.nfo"), - Path.Join(downloadDir, "eac.log"), + Path.Join(_downloadDir, "album.cue"), + Path.Join(_downloadDir, "cover.nfo"), + Path.Join(_downloadDir, "eac.log"), }; Mocker.GetMock() - .Setup(x => x.GetParentFolder( - It.Is(arg => arg.AsOsAgnostic() == _approvedDecisions.First().Item.Path.AsOsAgnostic()))) - .Returns(_albumDir); + .Setup(x => x.GetParentFolder(It.IsAny())) + .Returns(arg => Path.GetDirectoryName(arg.AsOsAgnostic())); Mocker.GetMock() .Setup(x => x.ImportExtraFiles).Returns(true); @@ -101,9 +108,7 @@ public void Setup() [Test] public void should_import_extras_during_manual_import_with_naming_config_having_rename_on() { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), false)) - .Returns(_approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); + SetupRecursiveGetFiles(_albumDir, _approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); // act Subject.ImportAlbumExtras(_approvedDecisions, _albumDir); @@ -116,11 +121,10 @@ public void should_import_extras_during_manual_import_with_naming_config_having_ [Test] public void should_import_extras_from_download_location() { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), false)) - .Returns(_approvedDecisions.Select(d => d.Item.Path).Concat(_downloadDirExtraFiles)); + // TODO: continue from here, breaks + SetupRecursiveGetFiles(_downloadDir, _approvedDownloadDecisions.Select(d => d.Item.Path).Concat(_downloadDirExtraFiles)); - Subject.ImportAlbumExtras(_approvedDecisions, _albumDir); + Subject.ImportAlbumExtras(_approvedDownloadDecisions, _albumDir); Mocker.GetMock() .Verify(x => x.Upsert(It.Is>(arg => arg.Count == _downloadDirExtraFiles.Count))); @@ -138,15 +142,13 @@ public void should_import_extras_from_download_location() [Test] public void should_import_with_extensions_from_settings() { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), It.IsAny())) - .Returns(_downloadDirExtraFiles); + SetupRecursiveGetFiles(_downloadDir, _downloadDirExtraFiles); Mocker.GetMock() .Setup(x => x.ExtraFileExtensions) .Returns(".cue, .txt"); - Subject.ImportAlbumExtras(_approvedDecisions, _albumDir); + Subject.ImportAlbumExtras(_approvedDownloadDecisions, _albumDir); Mocker.GetMock() .Verify(x => x.Upsert(It.Is>( @@ -157,9 +159,9 @@ public void should_import_with_extensions_from_settings() [Test] public void should_not_import_extras_with_naming_cfg_having_rename_off() { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), false)) - .Returns(_approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); + SetupRecursiveGetFiles(_downloadDir, + _approvedDownloadDecisions.Select(d => d.Item.Path) + .Concat(_downloadDirExtraFiles)); var cfg = NamingConfig.Default; cfg.RenameTracks = false; // explicitly set for readability @@ -174,9 +176,7 @@ public void should_not_import_extras_with_naming_cfg_having_rename_off() [TestCase(true)] public void should_not_import_extras_when_no_separate_album_dir_set(bool testStandardTrackFormat) { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), false)) - .Returns(_approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); + SetupRecursiveGetFiles(_albumDir, _approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); var cfg = NamingConfig.Default; cfg.RenameTracks = true; @@ -205,9 +205,9 @@ public void should_not_import_extras_when_no_separate_album_dir_set(bool testSta [TestCase("{Album_Title}")] public void should_import_extras_rename_pattern_contains_album_title(string albumDirPattern) { - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == _albumDir), false)) - .Returns(_approvedDecisions.Select(d => d.Item.Path).Concat(_albumDirExtraFiles)); + SetupRecursiveGetFiles(_downloadDir, + _approvedDownloadDecisions.Select(d => d.Item.Path) + .Concat(_downloadDirExtraFiles)); var cfg = NamingConfig.Default; cfg.RenameTracks = true; @@ -220,57 +220,31 @@ public void should_import_extras_rename_pattern_contains_album_title(string albu SetupNamingConfig(cfg); // act - Subject.ImportAlbumExtras(_approvedDecisions, _albumDir); + Subject.ImportAlbumExtras(_approvedDownloadDecisions, _albumDir); // assert Mocker.GetMock() - .Verify(x => x.Upsert(It.Is>(arg => arg.Count == _albumDirExtraFiles.Count))); + .Verify(x => x.Upsert(It.Is>(arg => arg.Count == _downloadDirExtraFiles.Count))); } [Test] public void should_import_extra_from_multi_cd_subdirs() { - var cd1Subdir = Path.Join(_albumDir, "CD1"); - var cd2Subdir = Path.Join(_albumDir, "CD2"); + var cd1Subdir = Path.Join(_downloadDir, "CD1"); + var cd2Subdir = Path.Join(_downloadDir, "CD2"); - var cd1Track = new LocalTrack - { - Artist = _album.Artist, - Album = _album, - Release = _album.AlbumReleases.Value.First(), - Tracks = new List { new Track() { Album = _album, } }, - Path = Path.Join(cd1Subdir, "101 - hurrdurr.flac"), - }; - var cd2Track = new LocalTrack - { - Artist = _album.Artist, - Album = _album, - Release = _album.AlbumReleases.Value.First(), - Tracks = new List { new Track() { Album = _album, } }, - Path = Path.Join(cd2Subdir, "201 - bonustrackbar.flac"), - }; + var cd1Track = NewTrack(_album, cd1Subdir, "101 - Foo Track.flac"); + var cd2Track = NewTrack(_album, cd2Subdir, "201 - bonustrackbar.flac"); var decisions = new List> { new ImportDecision(cd1Track), new ImportDecision(cd2Track), }; var cd1Extra = Path.Join(cd1Subdir, "cd1_foo.cue"); - var cd2Extra = Path.Join(cd1Subdir, "cd2_bar.cue"); + var cd2Extra = Path.Join(cd2Subdir, "cd2_bar.cue"); // TODO: this will break if extras are searched recursively - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == cd1Subdir), false)) - .Returns(new List { cd1Track.Path, cd1Extra }); - Mocker.GetMock() - .Setup(x => x.GetFiles(It.Is(arg => arg == cd2Subdir), false)) - .Returns(new List { cd2Track.Path, cd2Extra }); - - Mocker.GetMock() - .Setup(x => x.GetParentFolder(It.Is(arg => arg.AsOsAgnostic().StartsWith(cd1Subdir.AsOsAgnostic())))) - .Returns(cd1Subdir); - Mocker.GetMock() - .Setup(x => x.GetParentFolder(It.Is(arg => arg.AsOsAgnostic().StartsWith(cd2Subdir.AsOsAgnostic())))) - .Returns(cd2Subdir); + SetupRecursiveGetFiles(_downloadDir, cd1Track.Path, cd1Extra, cd2Track.Path, cd2Extra); Subject.ImportAlbumExtras(decisions, _albumDir); @@ -281,13 +255,54 @@ public void should_import_extra_from_multi_cd_subdirs() [Test] public void should_import_extra_from_multi_cd_root_dir() { - throw new NotImplementedException(); + var cd1Subdir = Path.Join(_albumDir, "CD1"); + var cd2Subdir = Path.Join(_albumDir, "CD2"); + + var cd1Track = NewTrack(_album, cd1Subdir, "101 - Foo Track.flac"); + var cd2Track = NewTrack(_album, cd2Subdir, "201 - bonustrackbar.flac"); + + var extraFileInAlbumRoot = Path.Join(_albumDir, "album.cue"); + + // SetupGetFiles(cd1Track.Path, cd2Track.Path, extraFileInAlbumRoot); + SetupRecursiveGetFiles(_albumDir, cd1Track.Path, cd2Track.Path, extraFileInAlbumRoot); + + // act + var decisions = new List> + { + new ImportDecision(cd1Track), + new ImportDecision(cd2Track), + }; + + Subject.ImportAlbumExtras(decisions, _albumDir); + + // assert + Mocker.GetMock() + .Verify(x => x.Upsert(It.Is>(arg => arg.Count == 1))); + Mocker.GetMock() + .Verify(x => x.Upsert(It.Is>(arg => arg.Single().Extension == ".cue"))); } [Test] public void should_import_from_separate_extras_dir_having_no_tracks() { - throw new NotImplementedException(); + var cd1Track = NewTrack(_album, _albumDir, "101 - Foo Track.flac"); + var cd2Track = NewTrack(_album, _albumDir, "201 - bonustrackbar.flac"); + var extraFileInRoot = Path.Join(_albumDir, "cuesheet.cue"); + var extraFileInSubdir = Path.Join(_albumDir, "artwork", "cover.jpg"); + + // SetupGetFiles(cd1Track.Path, cd2Track.Path, extraFileInRoot, extraFileInSubdir); + + SetupRecursiveGetFiles(_albumDir, cd1Track.Path, cd2Track.Path, extraFileInRoot, extraFileInSubdir); + var decisions = new List> + { + new ImportDecision(cd1Track), + new ImportDecision(cd2Track), + }; + Subject.ImportAlbumExtras(decisions, _albumDir); + + // assert + Mocker.GetMock() + .Verify(x => x.Upsert(It.Is>(arg => arg.Count == 2))); } [Test] @@ -354,5 +369,45 @@ private void SetupNamingConfig(NamingConfig cfg) { Mocker.GetMock().Setup(x => x.GetConfig()).Returns(cfg); } + + private LocalTrack NewTrack(Album album, string trackDir, string trackFileName) + { + return new LocalTrack + { + Artist = album.Artist, + Album = album, + Release = album.AlbumReleases.Value.First(), + Tracks = new List { new Track() { Album = album } }, + Path = Path.Join(trackDir, trackFileName), + }; + } + + private void SetupGetFiles(IEnumerable filePaths) + { + SetupGetFiles(filePaths.ToArray()); + } + + private void SetupGetFiles(params string[] filePaths) + { + var dirGroups = filePaths.GroupBy(x => Path.GetDirectoryName(x.AsOsAgnostic())); + foreach (var directoryFiles in dirGroups) + { + Mocker.GetMock() + .Setup(x => x.GetFiles(It.Is(arg => arg.AsOsAgnostic() == directoryFiles.Key.AsOsAgnostic()), false)) + .Returns(directoryFiles); + } + } + + private void SetupRecursiveGetFiles(string rootDir, IEnumerable filePath) + { + SetupRecursiveGetFiles(rootDir, filePath.ToArray()); + } + + private void SetupRecursiveGetFiles(string rootDir, params string[] filePaths) + { + Mocker.GetMock() + .Setup(x => x.GetFiles(It.Is(arg => arg.AsOsAgnostic() == rootDir.AsOsAgnostic()), true)) + .Returns(filePaths); + } } } diff --git a/src/NzbDrone.Core/Extras/ExtraService.cs b/src/NzbDrone.Core/Extras/ExtraService.cs index 5bd964ab8d..826edfbc44 100644 --- a/src/NzbDrone.Core/Extras/ExtraService.cs +++ b/src/NzbDrone.Core/Extras/ExtraService.cs @@ -65,16 +65,35 @@ public void ImportAlbumExtras(List> importedTracks, s var dirs = importedTracks.GroupBy(x => _diskProvider.GetParentFolder(x.Item.Path)).Select(x => x.Key); - // TODO: if dirs.Count > 1 - // check if dirs share common parent dir - // if yes => check parent as well - var albumFiles = new List(); - foreach (var albumDir in dirs) + if (!dirs.Any()) { - var albumDirFiles = _diskProvider.GetFiles(albumDir, false); + return; + } + + if (dirs.Count() == 1) + { + var albumDirFiles = _diskProvider.GetFiles(dirs.Single(), true); albumFiles.AddRange(albumDirFiles); } + else + { + // look for common parent dir + var parentDirs = dirs.GroupBy(x => _diskProvider.GetParentFolder(x)); + + if (parentDirs.Count() == 1) + { + albumFiles = _diskProvider.GetFiles(parentDirs.Single().Key, true).ToList(); + } + else + { + // otherwise no recursive search, best effort + foreach (var albumSubdir in dirs) + { + albumFiles.AddRange(_diskProvider.GetFiles(albumSubdir, false)); + } + } + } var filtered = albumFiles.Where(x => !importedTracks.Select(t => t.Item.Path).Contains(x));