Skip to content

Commit

Permalink
Handle common multicd dir structure variations
Browse files Browse the repository at this point in the history 
- Improve test setup and readability
- Separate download dir file mocks
- Check for extras recursively
  • Loading branch information
@tty418
tty418 committed Feb 23, 2024
1 parent 0dc5e04 commit 4a607c4
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 72 deletions.
187 changes: 121 additions & 66 deletions src/NzbDrone.Core.Test/Extras/ExtraServiceFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public class ExtraServiceFixture : CoreTest<ExtraService>
private Artist _artist;
private Album _album;
private List<string> _albumDirExtraFiles;
private string _downloadDir;
private List<ImportDecision<LocalTrack>> _approvedDownloadDecisions;
private List<string> _downloadDirExtraFiles;

[SetUp]
Expand All @@ -44,15 +46,16 @@ public void Setup()
Artist = _artist,
Title = "Twenty Thirties",
};
_albumDir = Path.Join(_artist.Path, $"{_album.Title} (1995) [FLAC]");

var release = new AlbumRelease()
{
AlbumId = _album.Id,
Monitored = true,
};
_album.AlbumReleases = new List<AlbumRelease> { release };

// manual import files
_albumDir = Path.Join(_artist.Path, $"{_album.Title} (1995) [FLAC]");

var track = new LocalTrack
{
Artist = _artist,
Expand All @@ -65,7 +68,6 @@ public void Setup()
{
new ImportDecision<LocalTrack>(track)
};

_albumDirExtraFiles = new List<string>
{
Path.Join(_albumDir, "album.cue"),
Expand All @@ -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<ImportDecision<LocalTrack>>()
{
new ImportDecision<LocalTrack>(downloadedTrack),
};
_downloadDirExtraFiles = new List<string>
{
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<IDiskProvider>()
.Setup(x => x.GetParentFolder(
It.Is<string>(arg => arg.AsOsAgnostic() == _approvedDecisions.First().Item.Path.AsOsAgnostic())))
.Returns(_albumDir);
.Setup(x => x.GetParentFolder(It.IsAny<string>()))
.Returns<string>(arg => Path.GetDirectoryName(arg.AsOsAgnostic()));

Mocker.GetMock<IConfigService>()
.Setup(x => x.ImportExtraFiles).Returns(true);
Expand All @@ -101,9 +108,7 @@ public void Setup()
[Test]
public void should_import_extras_during_manual_import_with_naming_config_having_rename_on()
{
Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(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);
Expand All @@ -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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(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<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(arg => arg.Count == _downloadDirExtraFiles.Count)));
Expand All @@ -138,15 +142,13 @@ public void should_import_extras_from_download_location()
[Test]
public void should_import_with_extensions_from_settings()
{
Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(arg => arg == _albumDir), It.IsAny<bool>()))
.Returns(_downloadDirExtraFiles);
SetupRecursiveGetFiles(_downloadDir, _downloadDirExtraFiles);

Mocker.GetMock<IConfigService>()
.Setup(x => x.ExtraFileExtensions)
.Returns(".cue, .txt");

Subject.ImportAlbumExtras(_approvedDecisions, _albumDir);
Subject.ImportAlbumExtras(_approvedDownloadDecisions, _albumDir);

Mocker.GetMock<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(
Expand All @@ -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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(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
Expand All @@ -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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(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;
Expand Down Expand Up @@ -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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(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;
Expand All @@ -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<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(arg => arg.Count == _albumDirExtraFiles.Count)));
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(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<Track> { 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<Track> { 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<ImportDecision<LocalTrack>>
{
new ImportDecision<LocalTrack>(cd1Track),
new ImportDecision<LocalTrack>(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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(arg => arg == cd1Subdir), false))
.Returns(new List<string> { cd1Track.Path, cd1Extra });
Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(arg => arg == cd2Subdir), false))
.Returns(new List<string> { cd2Track.Path, cd2Extra });

Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetParentFolder(It.Is<string>(arg => arg.AsOsAgnostic().StartsWith(cd1Subdir.AsOsAgnostic()))))
.Returns(cd1Subdir);
Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetParentFolder(It.Is<string>(arg => arg.AsOsAgnostic().StartsWith(cd2Subdir.AsOsAgnostic()))))
.Returns(cd2Subdir);
SetupRecursiveGetFiles(_downloadDir, cd1Track.Path, cd1Extra, cd2Track.Path, cd2Extra);

Subject.ImportAlbumExtras(decisions, _albumDir);

Expand All @@ -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<ImportDecision<LocalTrack>>
{
new ImportDecision<LocalTrack>(cd1Track),
new ImportDecision<LocalTrack>(cd2Track),
};

Subject.ImportAlbumExtras(decisions, _albumDir);

// assert
Mocker.GetMock<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(arg => arg.Count == 1)));
Mocker.GetMock<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(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<ImportDecision<LocalTrack>>
{
new ImportDecision<LocalTrack>(cd1Track),
new ImportDecision<LocalTrack>(cd2Track),
};
Subject.ImportAlbumExtras(decisions, _albumDir);

// assert
Mocker.GetMock<IOtherExtraFileService>()
.Verify(x => x.Upsert(It.Is<List<OtherExtraFile>>(arg => arg.Count == 2)));
}

[Test]
Expand Down Expand Up @@ -354,5 +369,45 @@ private void SetupNamingConfig(NamingConfig cfg)
{
Mocker.GetMock<INamingConfigService>().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<Track> { new Track() { Album = album } },
Path = Path.Join(trackDir, trackFileName),
};
}

private void SetupGetFiles(IEnumerable<string> 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<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(arg => arg.AsOsAgnostic() == directoryFiles.Key.AsOsAgnostic()), false))
.Returns(directoryFiles);
}
}

private void SetupRecursiveGetFiles(string rootDir, IEnumerable<string> filePath)
{
SetupRecursiveGetFiles(rootDir, filePath.ToArray());
}

private void SetupRecursiveGetFiles(string rootDir, params string[] filePaths)
{
Mocker.GetMock<IDiskProvider>()
.Setup(x => x.GetFiles(It.Is<string>(arg => arg.AsOsAgnostic() == rootDir.AsOsAgnostic()), true))
.Returns(filePaths);
}
}
}
31 changes: 25 additions & 6 deletions src/NzbDrone.Core/Extras/ExtraService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,35 @@ public void ImportAlbumExtras(List<ImportDecision<LocalTrack>> 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<string>();
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));

Expand Down

0 comments on commit 4a607c4

Please sign in to comment.