From e9097b8dc62e41b6503095dcce08c30b4c01777a Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Fri, 6 Apr 2018 23:52:28 -0500 Subject: [PATCH] Check for MusicBrainz Release Id during Scan (#227) (#277) * Implemented functionality to find album from DB if a track has album in the MusicBrainz Release Id tag. If tag doesn't exist or album is not found, handle via normal routes. * Added a test case * Fixed a bad merge. * Fixed a bug where a track with an empty Album IDv3 tag always was ignored, even if it had MusicBrainz Release Id hardcoded. * fixup: Quick fixes to get this merged tonight due to bug in develop --- .../AlbumRepositoryFixture.cs | 75 +++++++++++ .../NzbDrone.Core.Test.csproj | 2 + .../GetLocalTrackFixture.cs | 126 ++++++++++++++++++ src/NzbDrone.Core/Music/AlbumRepository.cs | 6 + src/NzbDrone.Core/Music/AlbumService.cs | 6 + src/NzbDrone.Core/Parser/Parser.cs | 11 +- src/NzbDrone.Core/Parser/ParsingService.cs | 62 +++++---- 7 files changed, 257 insertions(+), 31 deletions(-) create mode 100644 src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs create mode 100644 src/NzbDrone.Core.Test/ParserTests/ParsingServiceTests/GetLocalTrackFixture.cs diff --git a/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs b/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs new file mode 100644 index 000000000..66cd40904 --- /dev/null +++ b/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs @@ -0,0 +1,75 @@ +using FizzWare.NBuilder; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.Music; +using NzbDrone.Core.Test.Framework; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace NzbDrone.Core.Test.MusicTests.AlbumRepositoryTests +{ + [TestFixture] + public class AlbumRepositoryFixture : DbTest + { + private Artist _artist; + private Album _album; + private AlbumRepository _albumRepo; + + [SetUp] + public void Setup() + { + _artist = new Artist + { + Name = "Alien Ant Farm", + Monitored = true, + MBId = "this is a fake id", + Id = 1 + }; + + _album = new Album + { + Title = "ANThology", + ForeignAlbumId = "1", + CleanTitle = "anthology", + Artist = _artist, + AlbumType = "", + Releases = new List + { + new AlbumRelease + { + Id = "e00e40a3-5ed5-4ed3-9c22-0a8ff4119bdf" + } + } + + }; + + _albumRepo = Mocker.Resolve(); + + _albumRepo.Insert(_album); + } + + + [Test] + public void should_find_album_in_db_by_releaseid() + { + var id = "e00e40a3-5ed5-4ed3-9c22-0a8ff4119bdf"; + + var album = _albumRepo.FindAlbumByRelease(id); + + album.Title.Should().Be(_album.Title); + } + + [Test] + public void should_not_find_album_in_db_by_partial_releaseid() + { + var id = "e00e40a3-5ed5-4ed3-9c22"; + + var album = _albumRepo.FindAlbumByRelease(id); + + album.Should().BeNull(); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index 19145520d..0cfc3eebe 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -299,6 +299,7 @@ + @@ -309,6 +310,7 @@ + diff --git a/src/NzbDrone.Core.Test/ParserTests/ParsingServiceTests/GetLocalTrackFixture.cs b/src/NzbDrone.Core.Test/ParserTests/ParsingServiceTests/GetLocalTrackFixture.cs new file mode 100644 index 000000000..01a52991f --- /dev/null +++ b/src/NzbDrone.Core.Test/ParserTests/ParsingServiceTests/GetLocalTrackFixture.cs @@ -0,0 +1,126 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using FizzWare.NBuilder; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.Music; +using NzbDrone.Core.Parser; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.ParserTests.ParsingServiceTests +{ + [TestFixture] + public class GetLocalTrackFixture : CoreTest + { + private Artist _fakeArtist; + private Album _fakeAlbum; + private Track _fakeTrack; + private ParsedTrackInfo _parsedTrackInfo; + + [SetUp] + public void Setup() + { + _fakeArtist = Builder + .CreateNew() + .Build(); + + _fakeAlbum = Builder + .CreateNew() + .With(e => e.ArtistId = _fakeArtist.Id) + .With(e => e.Releases = new List + { + new AlbumRelease + { + Id = "5ecd552b-e54b-4c37-b62c-9d6234834bad" + } + }) + .Build(); + + _fakeTrack = Builder + .CreateNew() + .With(e => e.ArtistId = _fakeArtist.Id) + .With(e => e.AlbumId = _fakeAlbum.Id) + .With(e => e.Album = null) + .Build(); + + _parsedTrackInfo = Builder + .CreateNew() + .With(e => e.AlbumTitle = _fakeAlbum.Title) + .With(e => e.Title = _fakeTrack.Title) + .With(e => e.ArtistTitle = _fakeArtist.Name) + .Build(); + + Mocker.GetMock() + .Setup(s => s.FindByTitle(_fakeArtist.Id,_fakeAlbum.Title)) + .Returns(_fakeAlbum); + + Mocker.GetMock() + .Setup(s => s.FindAlbumByRelease(_fakeAlbum.Releases.First().Id)) + .Returns(_fakeAlbum); + + Mocker.GetMock() + .Setup(s => s.FindTrackByTitle(_fakeArtist.Id, _fakeAlbum.Id, It.IsAny(), _fakeTrack.Title)) + .Returns(_fakeTrack); + } + + private void HasAlbumTitleNoReleaseId() + { + _parsedTrackInfo.AlbumTitle = _fakeAlbum.Title; + _parsedTrackInfo.ReleaseMBId = ""; + } + + private void HasReleaseMbIdNoTitle() + { + _parsedTrackInfo.AlbumTitle = ""; + _parsedTrackInfo.ReleaseMBId = _fakeAlbum.Releases.First().Id; + } + + private void HasNoReleaseIdOrTitle() + { + _parsedTrackInfo.AlbumTitle = ""; + _parsedTrackInfo.ReleaseMBId = ""; + } + + [Test] + public void should_find_album_with_title_no_MBID() + { + HasAlbumTitleNoReleaseId(); + + var localTrack = Subject.GetLocalTrack("somfile.mp3", _fakeArtist, _parsedTrackInfo); + + localTrack.Artist.Id.Should().Be(_fakeArtist.Id); + localTrack.Album.Id.Should().Be(_fakeAlbum.Id); + localTrack.Tracks.First().Id.Should().Be(_fakeTrack.Id); + } + + [Test] + public void should_find_album_with_release_MBID_no_title() + { + HasReleaseMbIdNoTitle(); + + var localTrack = Subject.GetLocalTrack("somfile.mp3", _fakeArtist, _parsedTrackInfo); + + localTrack.Artist.Id.Should().Be(_fakeArtist.Id); + localTrack.Album.Id.Should().Be(_fakeAlbum.Id); + localTrack.Tracks.First().Id.Should().Be(_fakeTrack.Id); + } + + [Test] + public void should_not_find_album_with_no_release_MBID_no_title() + { + HasNoReleaseIdOrTitle(); + + var localTrack = Subject.GetLocalTrack("somfile.mp3", _fakeArtist, _parsedTrackInfo); + ExceptionVerification.ExpectedWarns(1); + + localTrack.Should().BeNull(); + + } + } +} diff --git a/src/NzbDrone.Core/Music/AlbumRepository.cs b/src/NzbDrone.Core/Music/AlbumRepository.cs index 616e1c2ed..935a74615 100644 --- a/src/NzbDrone.Core/Music/AlbumRepository.cs +++ b/src/NzbDrone.Core/Music/AlbumRepository.cs @@ -24,6 +24,7 @@ namespace NzbDrone.Core.Music List ArtistAlbumsBetweenDates(Artist artist, DateTime startDate, DateTime endDate, bool includeUnmonitored); void SetMonitoredFlat(Album album, bool monitored); void SetMonitored(IEnumerable ids, bool monitored); + Album FindAlbumByRelease(string releaseId); } public class AlbumRepository : BasicRepository, IAlbumRepository @@ -301,5 +302,10 @@ namespace NzbDrone.Core.Music .Where(artist => artist.CleanName == cleanArtistName) .SingleOrDefault(album => album.CleanTitle == cleanTitle); } + + public Album FindAlbumByRelease(string releaseId) + { + return Query.FirstOrDefault(e => e.Releases.Any(r => r.Id == releaseId)); + } } } diff --git a/src/NzbDrone.Core/Music/AlbumService.cs b/src/NzbDrone.Core/Music/AlbumService.cs index 598242e93..bd1d9bfc1 100644 --- a/src/NzbDrone.Core/Music/AlbumService.cs +++ b/src/NzbDrone.Core/Music/AlbumService.cs @@ -31,6 +31,7 @@ namespace NzbDrone.Core.Music void UpdateMany(List albums); void DeleteMany(List albums); void RemoveAddOptions(Album album); + Album FindAlbumByRelease(string releaseId); } public class AlbumService : IAlbumService, @@ -110,6 +111,11 @@ namespace NzbDrone.Core.Music return _albumRepository.GetAlbums(artistId).ToList(); } + public Album FindAlbumByRelease(string releaseId) + { + return _albumRepository.FindAlbumByRelease(releaseId); + } + public void RemoveAddOptions(Album album) { _albumRepository.SetFields(album, s => s.AddOptions); diff --git a/src/NzbDrone.Core/Parser/Parser.cs b/src/NzbDrone.Core/Parser/Parser.cs index 8eba68f55..8a15f8c7c 100644 --- a/src/NzbDrone.Core/Parser/Parser.cs +++ b/src/NzbDrone.Core/Parser/Parser.cs @@ -206,17 +206,11 @@ namespace NzbDrone.Core.Parser { var fileInfo = new FileInfo(path); - var result = new ParsedTrackInfo { }; + ParsedTrackInfo result; if (MediaFiles.MediaFileExtensions.Extensions.Contains(fileInfo.Extension)) { result = ParseAudioTags(path); - - if (CommonTagRegex.IsMatch(result.AlbumTitle)) - { - result.AlbumTitle = CleanAlbumTitle(result.AlbumTitle); - Logger.Debug("Cleaning Album title of common matching issues. Cleaned album title is '{0}'", result.AlbumTitle); - } } else { @@ -226,7 +220,6 @@ namespace NzbDrone.Core.Parser // TODO: Check if it is common that we might need to fallback to parser to gather details //var result = ParseMusicTitle(fileInfo.Name); - if (result == null) { @@ -641,6 +634,8 @@ namespace NzbDrone.Core.Parser ArtistTitleInfo = artistTitleInfo, Title = trackTitle }; + + Logger.Trace("File Tags Parsed: Artist: {0}, Album: {1}, Disc: {2}, Track Numbers(s): {3}, TrackTitle: {4}", result.ArtistTitle, result.AlbumTitle, result.DiscNumber, trackNumber, result.Title); diff --git a/src/NzbDrone.Core/Parser/ParsingService.cs b/src/NzbDrone.Core/Parser/ParsingService.cs index d02126e76..7d1460f0f 100644 --- a/src/NzbDrone.Core/Parser/ParsingService.cs +++ b/src/NzbDrone.Core/Parser/ParsingService.cs @@ -8,6 +8,7 @@ using NzbDrone.Core.MediaFiles; using NzbDrone.Core.Parser.Model; using NzbDrone.Core.Music; using System; +using System.Drawing.Text; namespace NzbDrone.Core.Parser { @@ -227,14 +228,12 @@ namespace NzbDrone.Core.Parser { parsedTrackInfo = folderInfo.JsonClone(); parsedTrackInfo.Quality = QualityParser.ParseQuality(Path.GetFileName(filename), null, 0); - } - - else + } else { parsedTrackInfo = Parser.ParseMusicPath(filename); } - if (parsedTrackInfo == null || parsedTrackInfo.AlbumTitle.IsNullOrWhiteSpace()) + if (parsedTrackInfo == null || (parsedTrackInfo.AlbumTitle.IsNullOrWhiteSpace()) && parsedTrackInfo.ReleaseMBId.IsNullOrWhiteSpace()) { if (MediaFileExtensions.Extensions.Contains(Path.GetExtension(filename))) { @@ -244,10 +243,13 @@ namespace NzbDrone.Core.Parser return null; } - var tracks = GetTracks(artist, parsedTrackInfo); - //var album = _albumService.FindByTitle(artist.Id, parsedTrackInfo.AlbumTitle); - var album = tracks.FirstOrDefault()?.Album; - + var album = GetAlbum(artist, parsedTrackInfo); + var tracks = new List(); + if (album != null) + { + tracks = GetTracks(artist, album, parsedTrackInfo); + } + return new LocalTrack { Artist = artist, @@ -261,37 +263,51 @@ namespace NzbDrone.Core.Parser }; } - private List GetTracks(Artist artist, ParsedTrackInfo parsedTrackInfo) + private Album GetAlbum(Artist artist, ParsedTrackInfo parsedTrackInfo) { - var result = new List(); + Album album = null; - if (parsedTrackInfo.AlbumTitle.IsNullOrWhiteSpace()) + if (parsedTrackInfo != null && parsedTrackInfo.ReleaseMBId.IsNotNullOrWhiteSpace()) { - _logger.Debug("Album title could not be parsed for {0}", parsedTrackInfo); - return new List(); + album = _albumService.FindAlbumByRelease(parsedTrackInfo.ReleaseMBId); } - parsedTrackInfo.AlbumTitle = Parser.CleanAlbumTitle(parsedTrackInfo.AlbumTitle); - _logger.Debug("Cleaning Album title of common matching issues. Cleaned album title is '{0}'", parsedTrackInfo.AlbumTitle); + if (album == null && parsedTrackInfo.AlbumTitle.IsNullOrWhiteSpace()) + { + _logger.Debug("Album title could not be parsed for {0}", parsedTrackInfo); + return null; + } - var album = _albumService.FindByTitle(artist.Id, parsedTrackInfo.AlbumTitle); - _logger.Debug("Album {0} selected for {1}", album, parsedTrackInfo); + var cleanAlbumTitle = Parser.CleanAlbumTitle(parsedTrackInfo.AlbumTitle); + _logger.Debug("Cleaning Album title of common matching issues. Cleaned album title is '{0}'", cleanAlbumTitle); + + if (album == null) + { + album = _albumService.FindByTitle(artist.Id, cleanAlbumTitle); + } if (album == null) { _logger.Debug("Parsed album title not found in Db for {0}", parsedTrackInfo); - return new List(); + return null; } - Track trackInfo = null; + _logger.Debug("Album {0} selected for {1}", album, parsedTrackInfo); + + return album; + } + + private List GetTracks(Artist artist, Album album, ParsedTrackInfo parsedTrackInfo) + { + var result = new List(); if (parsedTrackInfo.Title.IsNotNullOrWhiteSpace()) { - parsedTrackInfo.Title = Parser.CleanTrackTitle(parsedTrackInfo.Title); - _logger.Debug("Cleaning Track title of common matching issues. Cleaned track title is '{0}'", parsedTrackInfo.Title); + var cleanTrackTitle = Parser.CleanTrackTitle(parsedTrackInfo.Title); + _logger.Debug("Cleaning Track title of common matching issues. Cleaned track title is '{0}'", cleanTrackTitle); + + var trackInfo = _trackService.FindTrackByTitle(artist.Id, album.Id, parsedTrackInfo.DiscNumber, cleanTrackTitle); - trackInfo = _trackService.FindTrackByTitle(artist.Id, album.Id, parsedTrackInfo.DiscNumber, parsedTrackInfo.Title); - if (trackInfo != null) { _logger.Debug("Track {0} selected for {1}", trackInfo, parsedTrackInfo);