From c339645cec3494cc826d54bfa36f152033e3a563 Mon Sep 17 00:00:00 2001 From: ta264 Date: Mon, 18 Jul 2022 23:56:19 +0100 Subject: [PATCH] Fixed: Respect import list search setting when monitoring existing items --- .../ImportListSyncServiceFixture.cs | 233 +++++++++++++++++- .../ImportLists/ImportListSyncService.cs | 186 ++++++-------- 2 files changed, 299 insertions(+), 120 deletions(-) diff --git a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs index 9718e914d..afb05684c 100644 --- a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs +++ b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs @@ -1,9 +1,12 @@ using System.Collections.Generic; using System.Linq; +using FizzWare.NBuilder; using Moq; using NUnit.Framework; using NzbDrone.Core.ImportLists; using NzbDrone.Core.ImportLists.Exclusions; +using NzbDrone.Core.IndexerSearch; +using NzbDrone.Core.Messaging.Commands; using NzbDrone.Core.MetadataSource; using NzbDrone.Core.Music; using NzbDrone.Core.Parser.Model; @@ -85,18 +88,32 @@ namespace NzbDrone.Core.Test.ImportListTests _importListReports.Add(importListItem2); } - private void WithExistingArtist() + private void WithExistingArtist(bool monitored) { Mocker.GetMock() .Setup(v => v.FindById(_importListReports.First().ArtistMusicBrainzId)) - .Returns(new Artist { Id = 1, ForeignArtistId = _importListReports.First().ArtistMusicBrainzId }); + .Returns(new Artist { Id = 1, ForeignArtistId = _importListReports.First().ArtistMusicBrainzId, Monitored = monitored }); } - private void WithExistingAlbum() + private void WithExistingAlbum(bool monitored) { + var album = Builder.CreateNew() + .With(x => x.Id = 1) + .With(x => x.ForeignAlbumId = _importListReports.First().AlbumMusicBrainzId) + .With(x => x.Monitored = monitored) + .Build(); + + var artist = Builder.CreateNew() + .With(x => x.Monitored = monitored) + .With(x => x.ForeignArtistId = _importListReports.First().ArtistMusicBrainzId) + .With(x => x.Albums = new List { album }) + .Build(); + + album.Artist = artist; + Mocker.GetMock() .Setup(v => v.FindById(_importListReports.First().AlbumMusicBrainzId)) - .Returns(new Album { Id = 1, ForeignAlbumId = _importListReports.First().AlbumMusicBrainzId }); + .Returns(album); } private void WithExcludedArtist() @@ -125,11 +142,11 @@ namespace NzbDrone.Core.Test.ImportListTests }); } - private void WithMonitorType(ImportListMonitorType monitor) + private void WithListSettings(ImportListMonitorType monitor = ImportListMonitorType.EntireArtist, bool shouldMonitorExisting = false, bool shouldSearch = true) { Mocker.GetMock() .Setup(v => v.Get(It.IsAny())) - .Returns(new ImportListDefinition { ShouldMonitor = monitor }); + .Returns(new ImportListDefinition { ShouldMonitor = monitor, ShouldMonitorExisting = shouldMonitorExisting, ShouldSearch = shouldSearch }); } [Test] @@ -191,7 +208,7 @@ namespace NzbDrone.Core.Test.ImportListTests public void should_not_add_if_existing_artist() { WithArtistId(); - WithExistingArtist(); + WithExistingArtist(false); Subject.Execute(new ImportListSyncCommand()); @@ -203,7 +220,7 @@ namespace NzbDrone.Core.Test.ImportListTests public void should_not_add_if_existing_album() { WithAlbumId(); - WithExistingAlbum(); + WithExistingAlbum(false); Subject.Execute(new ImportListSyncCommand()); @@ -216,7 +233,7 @@ namespace NzbDrone.Core.Test.ImportListTests { WithAlbumId(); WithArtistId(); - WithExistingArtist(); + WithExistingArtist(false); Subject.Execute(new ImportListSyncCommand()); @@ -230,7 +247,7 @@ namespace NzbDrone.Core.Test.ImportListTests public void should_add_if_not_existing_artist(ImportListMonitorType monitor, bool expectedArtistMonitored) { WithArtistId(); - WithMonitorType(monitor); + WithListSettings(monitor); Subject.Execute(new ImportListSyncCommand()); @@ -244,7 +261,8 @@ namespace NzbDrone.Core.Test.ImportListTests public void should_add_if_not_existing_album(ImportListMonitorType monitor, bool expectedAlbumMonitored) { WithAlbumId(); - WithMonitorType(monitor); + WithArtistId(); + WithListSettings(monitor); Subject.Execute(new ImportListSyncCommand()); @@ -268,6 +286,7 @@ namespace NzbDrone.Core.Test.ImportListTests public void should_not_add_album_if_excluded_album() { WithAlbumId(); + WithArtistId(); WithExcludedAlbum(); Subject.Execute(new ImportListSyncCommand()); @@ -298,7 +317,7 @@ namespace NzbDrone.Core.Test.ImportListTests WithAlbumId(); WithSecondBook(); WithArtistId(); - WithMonitorType(monitor); + WithListSettings(monitor); Subject.Execute(new ImportListSyncCommand()); @@ -311,5 +330,195 @@ namespace NzbDrone.Core.Test.ImportListTests false, true)); } + + [TestCase(ImportListMonitorType.SpecificAlbum)] + [TestCase(ImportListMonitorType.EntireArtist)] + public void should_monitor_existing_unmonitored_album(ImportListMonitorType monitorType) + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(monitorType, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.SetAlbumMonitored(1, true)); + } + + [TestCase(ImportListMonitorType.SpecificAlbum)] + [TestCase(ImportListMonitorType.EntireArtist)] + public void should_not_monitor_existing_monitored_album(ImportListMonitorType monitorType) + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(true); + + WithListSettings(monitorType, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.SetAlbumMonitored(1, true), Times.Never); + } + + [TestCase(ImportListMonitorType.SpecificAlbum, false)] + [TestCase(ImportListMonitorType.EntireArtist, false)] + [TestCase(ImportListMonitorType.None, false)] + [TestCase(ImportListMonitorType.None, true)] + + public void should_not_monitor_existing_unmonitored_album(ImportListMonitorType monitorType, bool shouldMonitorExisting) + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(monitorType, shouldMonitorExisting); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.SetAlbumMonitored(1, true), Times.Never); + } + + [Test] + public void should_search_specific_existing_unmonitored_album() + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(ImportListMonitorType.SpecificAlbum, true, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.Is(x => x.AlbumIds.Count == 1 && x.AlbumIds.Contains(1)), CommandPriority.Normal, CommandTrigger.Unspecified)); + } + + [Test] + public void should_not_search_specific_existing_unmonitored_album() + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(ImportListMonitorType.SpecificAlbum, true, false); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.Is(x => x.AlbumIds.Count == 1 && x.AlbumIds.Contains(1)), CommandPriority.Normal, CommandTrigger.Unspecified), Times.Never); + } + + [Test] + public void should_search_all_artist_albums() + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(ImportListMonitorType.EntireArtist, true, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.Is(x => x.ArtistId == 1), CommandPriority.Normal, CommandTrigger.Unspecified)); + } + + [Test] + public void should_not_search_all_artist_albums() + { + WithAlbumId(); + WithArtistId(); + WithExistingAlbum(false); + + WithListSettings(ImportListMonitorType.EntireArtist, true, false); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.Is(x => x.ArtistId == 1), CommandPriority.Normal, CommandTrigger.Unspecified), Times.Never); + } + + [TestCase(ImportListMonitorType.SpecificAlbum)] + [TestCase(ImportListMonitorType.EntireArtist)] + [TestCase(ImportListMonitorType.None)] + + public void should_monitor_existing_unmonitored_artist(ImportListMonitorType monitorType) + { + WithArtistId(); + WithExistingArtist(false); + + WithListSettings(monitorType, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.UpdateArtist(It.Is(a => a.Monitored), true)); + } + + [TestCase(ImportListMonitorType.SpecificAlbum)] + [TestCase(ImportListMonitorType.EntireArtist)] + [TestCase(ImportListMonitorType.None)] + + public void should_not_monitor_existing_monitored_artist(ImportListMonitorType monitorType) + { + WithArtistId(); + WithExistingArtist(true); + + WithListSettings(monitorType, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestCase(ImportListMonitorType.SpecificAlbum)] + [TestCase(ImportListMonitorType.EntireArtist)] + [TestCase(ImportListMonitorType.None)] + + public void should_not_monitor_existing_unmonitored_artist(ImportListMonitorType monitorType) + { + WithArtistId(); + WithExistingArtist(false); + + WithListSettings(monitorType, false); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Never); + } + + [Test] + public void should_search_unmonitored_artist() + { + WithArtistId(); + WithExistingArtist(false); + + WithListSettings(ImportListMonitorType.EntireArtist, true, true); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.Is(x => x.ArtistId == 1), CommandPriority.Normal, CommandTrigger.Unspecified)); + } + + [Test] + public void should_not_search_unmonitored_artist() + { + WithArtistId(); + WithExistingArtist(false); + + WithListSettings(ImportListMonitorType.EntireArtist, true, false); + + Subject.Execute(new ImportListSyncCommand()); + + Mocker.GetMock() + .Verify(v => v.Push(It.IsAny(), CommandPriority.Normal, CommandTrigger.Unspecified), Times.Never); + } } } diff --git a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs index 57ae1b530..7a288ab73 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs @@ -70,7 +70,7 @@ namespace NzbDrone.Core.ImportLists private List SyncList(ImportListDefinition definition) { - _logger.ProgressInfo(string.Format("Starting Import List Refresh for List {0}", definition.Name)); + _logger.ProgressInfo("Starting Import List Refresh for List {0}", definition.Name); var rssReleases = _listFetcherAndParser.FetchSingleList(definition); @@ -89,13 +89,11 @@ namespace NzbDrone.Core.ImportLists var reportNumber = 1; - var listExclusions = _importListExclusionService.All(); + var listExclusions = _importListExclusionService.All().ToDictionary(x => x.ForeignId); foreach (var report in reports) { - _logger.ProgressTrace("Processing list item {0}/{1}", reportNumber, reports.Count); - - reportNumber++; + _logger.ProgressTrace("Processing list item {0}/{1}", reportNumber++, reports.Count); var importList = _importListFactory.Get(report.ImportListId); @@ -153,75 +151,34 @@ namespace NzbDrone.Core.ImportLists report.ArtistMusicBrainzId ??= mappedAlbum.ArtistMetadata?.Value?.ForeignArtistId; } - private void ProcessAlbumReport(ImportListDefinition importList, ImportListItemInfo report, List listExclusions, List albumsToAdd, List artistsToAdd) + private void ProcessAlbumReport(ImportListDefinition importList, ImportListItemInfo report, Dictionary listExclusions, List albumsToAdd, List artistsToAdd) { - if (report.AlbumMusicBrainzId == null) + if (report.AlbumMusicBrainzId == null || report.ArtistMusicBrainzId == null) { return; } - // Check to see if album in DB - var existingAlbum = _albumService.FindById(report.AlbumMusicBrainzId); - // Check to see if album excluded - var excludedAlbum = listExclusions.SingleOrDefault(s => s.ForeignId == report.AlbumMusicBrainzId); - - // Check to see if artist excluded - var excludedArtist = listExclusions.SingleOrDefault(s => s.ForeignId == report.ArtistMusicBrainzId); - - if (excludedAlbum != null) + if (listExclusions.ContainsKey(report.AlbumMusicBrainzId)) { _logger.Debug("{0} [{1}] Rejected due to list exclusion", report.AlbumMusicBrainzId, report.Album); return; } - if (excludedArtist != null) + // Check to see if artist excluded + if (listExclusions.ContainsKey(report.ArtistMusicBrainzId)) { _logger.Debug("{0} [{1}] Rejected due to list exclusion for parent artist", report.AlbumMusicBrainzId, report.Album); return; } + // Check to see if album in DB + var existingAlbum = _albumService.FindById(report.AlbumMusicBrainzId); if (existingAlbum != null) { _logger.Debug("{0} [{1}] Rejected, Album Exists in DB. Ensuring Album and Artist monitored.", report.AlbumMusicBrainzId, report.Album); - if (importList.ShouldMonitorExisting && importList.ShouldMonitor != ImportListMonitorType.None) - { - if (!existingAlbum.Monitored) - { - _albumService.SetAlbumMonitored(existingAlbum.Id, true); - - if (importList.ShouldMonitor == ImportListMonitorType.SpecificAlbum) - { - _commandQueueManager.Push(new AlbumSearchCommand(new List { existingAlbum.Id })); - } - } - - var existingArtist = existingAlbum.Artist.Value; - var doSearch = false; - - if (importList.ShouldMonitor == ImportListMonitorType.EntireArtist) - { - if (existingArtist.Albums.Value.Any(x => !x.Monitored)) - { - doSearch = true; - _albumService.SetMonitored(existingArtist.Albums.Value.Select(x => x.Id), true); - } - } - - if (!existingArtist.Monitored) - { - doSearch = true; - existingArtist.Monitored = true; - _artistService.UpdateArtist(existingArtist); - } - - if (doSearch) - { - _commandQueueManager.Push(new MissingAlbumSearchCommand(existingArtist.Id)); - } - } - + ProcessAlbumReportForExistingAlbum(importList, existingAlbum); return; } @@ -230,26 +187,7 @@ namespace NzbDrone.Core.ImportLists { var monitored = importList.ShouldMonitor != ImportListMonitorType.None; - var toAddArtist = new Artist - { - Monitored = monitored, - MonitorNewItems = importList.MonitorNewItems, - RootFolderPath = importList.RootFolderPath, - QualityProfileId = importList.ProfileId, - MetadataProfileId = importList.MetadataProfileId, - Tags = importList.Tags, - AddOptions = new AddArtistOptions - { - SearchForMissingAlbums = importList.ShouldSearch, - Monitored = monitored, - Monitor = monitored ? MonitorTypes.All : MonitorTypes.None - } - }; - - if (report.ArtistMusicBrainzId != null && report.Artist != null) - { - toAddArtist = ProcessArtistReport(importList, report, listExclusions, artistsToAdd); - } + var toAddArtist = ProcessArtistReport(importList, report, listExclusions, artistsToAdd); var toAdd = new Album { @@ -273,6 +211,43 @@ namespace NzbDrone.Core.ImportLists } } + private void ProcessAlbumReportForExistingAlbum(ImportListDefinition importList, Album existingAlbum) + { + if (importList.ShouldMonitorExisting && importList.ShouldMonitor != ImportListMonitorType.None) + { + Command searchCommand = null; + var existingArtist = existingAlbum.Artist.Value; + + if (!existingArtist.Monitored || !existingAlbum.Monitored) + { + searchCommand = importList.ShouldMonitor == ImportListMonitorType.EntireArtist ? new MissingAlbumSearchCommand(existingArtist.Id) : new AlbumSearchCommand(new List { existingAlbum.Id }); + } + + if (!existingAlbum.Monitored) + { + _albumService.SetAlbumMonitored(existingAlbum.Id, true); + } + + if (!existingArtist.Monitored) + { + existingArtist.Monitored = true; + _artistService.UpdateArtist(existingArtist); + } + + // Make sure all artist albums are monitored if required + if (importList.ShouldMonitor == ImportListMonitorType.EntireArtist && existingArtist.Albums.Value.Any(x => !x.Monitored)) + { + _albumService.SetMonitored(existingArtist.Albums.Value.Select(x => x.Id), true); + searchCommand = new MissingAlbumSearchCommand(existingArtist.Id); + } + + if (importList.ShouldSearch && searchCommand != null) + { + _commandQueueManager.Push(searchCommand); + } + } + } + private void MapArtistReport(ImportListItemInfo report) { var mappedArtist = _artistSearchService.SearchForNewArtist(report.Artist) @@ -281,41 +256,21 @@ namespace NzbDrone.Core.ImportLists report.Artist = mappedArtist?.Metadata.Value?.Name; } - private Artist ProcessArtistReport(ImportListDefinition importList, ImportListItemInfo report, List listExclusions, List artistsToAdd) + private Artist ProcessArtistReport(ImportListDefinition importList, ImportListItemInfo report, Dictionary listExclusions, List artistsToAdd) { if (report.ArtistMusicBrainzId == null) { return null; } - // Check to see if artist in DB - var existingArtist = _artistService.FindById(report.ArtistMusicBrainzId); - - // Check to see if artist excluded - var excludedArtist = listExclusions.Where(s => s.ForeignId == report.ArtistMusicBrainzId).SingleOrDefault(); - - // Check to see if artist in import - var existingImportArtist = artistsToAdd.Find(i => i.ForeignArtistId == report.ArtistMusicBrainzId); - - if (excludedArtist != null) + if (listExclusions.ContainsKey(report.ArtistMusicBrainzId)) { _logger.Debug("{0} [{1}] Rejected due to list exclusion", report.ArtistMusicBrainzId, report.Artist); return null; } - if (existingArtist != null) - { - _logger.Debug("{0} [{1}] Rejected, artist exists in DB. Ensuring artist monitored", report.ArtistMusicBrainzId, report.Artist); - - if (importList.ShouldMonitorExisting && !existingArtist.Monitored) - { - existingArtist.Monitored = true; - _artistService.UpdateArtist(existingArtist); - } - - return existingArtist; - } - + // Check to see if artist in import + var existingImportArtist = artistsToAdd.Find(i => i.ForeignArtistId == report.ArtistMusicBrainzId); if (existingImportArtist != null) { _logger.Debug("{0} [{1}] Rejected, artist exists in Import.", report.ArtistMusicBrainzId, report.Artist); @@ -323,6 +278,16 @@ namespace NzbDrone.Core.ImportLists return existingImportArtist; } + // Check to see if artist in DB + var existingArtist = _artistService.FindById(report.ArtistMusicBrainzId); + if (existingArtist != null) + { + _logger.Debug("{0} [{1}] Rejected, artist exists in DB. Ensuring artist monitored", report.ArtistMusicBrainzId, report.Artist); + + ProcessArtistReportForExistingArtist(importList, existingArtist); + return existingArtist; + } + var monitored = importList.ShouldMonitor != ImportListMonitorType.None; var toAdd = new Artist @@ -351,18 +316,23 @@ namespace NzbDrone.Core.ImportLists return toAdd; } + private void ProcessArtistReportForExistingArtist(ImportListDefinition importList, Artist existingArtist) + { + if (importList.ShouldMonitorExisting && !existingArtist.Monitored) + { + existingArtist.Monitored = true; + _artistService.UpdateArtist(existingArtist); + + if (importList.ShouldSearch) + { + _commandQueueManager.Push(new MissingAlbumSearchCommand(existingArtist.Id)); + } + } + } + public void Execute(ImportListSyncCommand message) { - List processed; - - if (message.DefinitionId.HasValue) - { - processed = SyncList(_importListFactory.Get(message.DefinitionId.Value)); - } - else - { - processed = SyncAll(); - } + var processed = message.DefinitionId.HasValue ? SyncList(_importListFactory.Get(message.DefinitionId.Value)) : SyncAll(); _eventAggregator.PublishEvent(new ImportListSyncCompleteEvent(processed)); }