From 045a3ce4044734e8d6381fb57ee13d77611643cd Mon Sep 17 00:00:00 2001 From: ta264 Date: Tue, 16 Apr 2019 01:52:43 +0100 Subject: [PATCH] Fixed: Skip albums and releases with no valid tracks (#754) * Fixed: Artist only marked as refreshed if Albums/Tracks refreshed also * Fixed: Skip album releases with no valid tracks * Fixed: Don't add albums with no valid releases * Fixed: Remove Albums with no valid releases --- .../MusicTests/AddAlbumFixture.cs | 24 +++++++++++++++++++ .../MusicTests/RefreshAlbumServiceFixture.cs | 15 ++++++++++++ .../MetadataSource/SkyHook/SkyHookProxy.cs | 2 +- src/NzbDrone.Core/Music/AddAlbumService.cs | 7 ++++++ .../Music/RefreshAlbumService.cs | 7 ++++++ .../Music/RefreshArtistService.cs | 8 +++---- 6 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/NzbDrone.Core.Test/MusicTests/AddAlbumFixture.cs b/src/NzbDrone.Core.Test/MusicTests/AddAlbumFixture.cs index 3b646dbec..16eca2ec2 100644 --- a/src/NzbDrone.Core.Test/MusicTests/AddAlbumFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/AddAlbumFixture.cs @@ -89,6 +89,30 @@ namespace NzbDrone.Core.Test.MusicTests ExceptionVerification.ExpectedErrors(1); } + [Test] + public void should_not_add_if_no_releases() + { + _fakeAlbum.AlbumReleases = new List(); + GivenValidAlbum(_fakeAlbum.ForeignAlbumId); + + Subject.AddAlbum(_fakeAlbum).Should().BeNull(); + + Mocker.GetMock() + .Verify(x => x.AddAlbum(It.IsAny()), Times.Never()); + } + + [Test] + public void should_not_add_item_in_list_if_no_releases() + { + _fakeAlbum.AlbumReleases = new List(); + GivenValidAlbum(_fakeAlbum.ForeignAlbumId); + + Subject.AddAlbums(new List { _fakeAlbum }).Should().BeEquivalentTo(new List { null }); + + Mocker.GetMock() + .Verify(x => x.AddAlbum(It.IsAny()), Times.Never()); + } + [Test] public void should_not_add_duplicate_releases() { diff --git a/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs index 46cbb222e..6161c4d5b 100644 --- a/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs @@ -107,6 +107,21 @@ namespace NzbDrone.Core.Test.MusicTests ExceptionVerification.ExpectedWarns(1); } + [Test] + public void should_remove_album_with_no_valid_releases() + { + var album = _albums.First(); + album.AlbumReleases = new List(); + + GivenNewAlbumInfo(album); + + Subject.RefreshAlbumInfo(album, false); + + Mocker.GetMock() + .Verify(x => x.DeleteMany(It.Is>(y => y.Count == 1 && y.First().ForeignAlbumId == album.ForeignAlbumId)), + Times.Once()); + } + [Test] public void two_equivalent_releases_should_be_equal() { diff --git a/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs b/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs index 4a900fe79..c38c0d7ea 100644 --- a/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs +++ b/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs @@ -319,7 +319,7 @@ namespace NzbDrone.Core.MetadataSource.SkyHook if (resource.Releases != null) { - album.AlbumReleases = resource.Releases.Select(x => MapRelease(x, artistDict)).ToList(); + album.AlbumReleases = resource.Releases.Select(x => MapRelease(x, artistDict)).Where(x => x.TrackCount > 0).ToList(); } album.AnyReleaseOk = true; diff --git a/src/NzbDrone.Core/Music/AddAlbumService.cs b/src/NzbDrone.Core/Music/AddAlbumService.cs index b2afaaa75..c2cf6d417 100644 --- a/src/NzbDrone.Core/Music/AddAlbumService.cs +++ b/src/NzbDrone.Core/Music/AddAlbumService.cs @@ -86,6 +86,13 @@ namespace NzbDrone.Core.Music private Album AddAlbum(Tuple> skyHookData) { var newAlbum = skyHookData.Item2; + + if (newAlbum.AlbumReleases.Value.Count == 0) + { + _logger.Debug($"Skipping album with no valid releases {newAlbum}"); + return null; + } + _logger.ProgressInfo("Adding Album {0}", newAlbum.Title); _artistMetadataRepository.UpsertMany(skyHookData.Item3); diff --git a/src/NzbDrone.Core/Music/RefreshAlbumService.cs b/src/NzbDrone.Core/Music/RefreshAlbumService.cs index b4b1160dc..fa57f80de 100644 --- a/src/NzbDrone.Core/Music/RefreshAlbumService.cs +++ b/src/NzbDrone.Core/Music/RefreshAlbumService.cs @@ -84,6 +84,13 @@ namespace NzbDrone.Core.Music return; } + if (tuple.Item2.AlbumReleases.Value.Count == 0) + { + _logger.Debug($"{album} has no valid releases, removing."); + _albumService.DeleteMany(new List { album }); + return; + } + var remoteMetadata = tuple.Item3.DistinctBy(x => x.ForeignArtistId).ToList(); var existingMetadata = _artistMetadataRepository.FindById(remoteMetadata.Select(x => x.ForeignArtistId).ToList()); var updateMetadataList = new List(); diff --git a/src/NzbDrone.Core/Music/RefreshArtistService.cs b/src/NzbDrone.Core/Music/RefreshArtistService.cs index e1a7f97e4..63360419a 100644 --- a/src/NzbDrone.Core/Music/RefreshArtistService.cs +++ b/src/NzbDrone.Core/Music/RefreshArtistService.cs @@ -139,8 +139,6 @@ namespace NzbDrone.Core.Music } } - _artistService.UpdateArtist(artist); - _logger.Debug("{0} Deleting {1}, Updating {2}, Adding {3} albums", artist, existingAlbums.Count, updateAlbumsList.Count, newAlbumsList.Count); @@ -157,10 +155,12 @@ namespace NzbDrone.Core.Music _refreshAlbumService.RefreshAlbumInfo(updateAlbumsList, forceAlbumRefresh, forceUpdateFileTags); - _eventAggregator.PublishEvent(new AlbumInfoRefreshedEvent(artist, newAlbumsList, updateAlbumsList)); + // Do this last so artist only marked as refreshed if refresh of tracks / albums completed successfully + _artistService.UpdateArtist(artist); - _logger.Debug("Finished artist refresh for {0}", artist.Name); + _eventAggregator.PublishEvent(new AlbumInfoRefreshedEvent(artist, newAlbumsList, updateAlbumsList)); _eventAggregator.PublishEvent(new ArtistUpdatedEvent(artist)); + _logger.Debug("Finished artist refresh for {0}", artist.Name); } private List UpdateAlbums(Artist artist, List albumsToUpdate)