Fixed: Respect import list search setting when monitoring existing items

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<IArtistService>()
                 .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<Album>.CreateNew()
+                .With(x => x.Id = 1)
+                .With(x => x.ForeignAlbumId = _importListReports.First().AlbumMusicBrainzId)
+                .With(x => x.Monitored = monitored)
+                .Build();
+
+            var artist = Builder<Artist>.CreateNew()
+                .With(x => x.Monitored = monitored)
+                .With(x => x.ForeignArtistId = _importListReports.First().ArtistMusicBrainzId)
+                .With(x => x.Albums = new List<Album> { album })
+                .Build();
+
+            album.Artist = artist;
+
             Mocker.GetMock<IAlbumService>()
                 .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<IImportListFactory>()
                 .Setup(v => v.Get(It.IsAny<int>()))
-                .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<IAlbumService>()
+                .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<IAlbumService>()
+                .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<IAlbumService>()
+                .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<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.Is<AlbumSearchCommand>(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<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.Is<AlbumSearchCommand>(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<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.Is<MissingAlbumSearchCommand>(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<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.Is<MissingAlbumSearchCommand>(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<IArtistService>()
+                .Verify(v => v.UpdateArtist(It.Is<Artist>(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<IArtistService>()
+                .Verify(v => v.UpdateArtist(It.IsAny<Artist>(), It.IsAny<bool>()), 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<IArtistService>()
+                .Verify(v => v.UpdateArtist(It.IsAny<Artist>(), It.IsAny<bool>()), Times.Never);
+        }
+
+        [Test]
+        public void should_search_unmonitored_artist()
+        {
+            WithArtistId();
+            WithExistingArtist(false);
+
+            WithListSettings(ImportListMonitorType.EntireArtist, true, true);
+
+            Subject.Execute(new ImportListSyncCommand());
+
+            Mocker.GetMock<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.Is<MissingAlbumSearchCommand>(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<IManageCommandQueue>()
+                .Verify(v => v.Push<Command>(It.IsAny<MissingAlbumSearchCommand>(), CommandPriority.Normal, CommandTrigger.Unspecified), Times.Never);
+        }
     }
 }

src/NzbDrone.Core/ImportLists/ImportListSyncService.cs

@@ -70,7 +70,7 @@ namespace NzbDrone.Core.ImportLists
 
         private List<Album> 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);
+                _logger.ProgressTrace("Processing list item {0}/{1}", reportNumber++, reports.Count);
-
-                reportNumber++;
 
                 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<ImportListExclusion> listExclusions, List<Album> albumsToAdd, List<Artist> artistsToAdd)
+        private void ProcessAlbumReport(ImportListDefinition importList, ImportListItemInfo report, Dictionary<string, ImportListExclusion> listExclusions, List<Album> albumsToAdd, List<Artist> 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);
+            if (listExclusions.ContainsKey(report.AlbumMusicBrainzId))
-
-            // Check to see if artist excluded
-            var excludedArtist = listExclusions.SingleOrDefault(s => s.ForeignId == report.ArtistMusicBrainzId);
-
-            if (excludedAlbum != null)
             {
                 _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)
+                ProcessAlbumReportForExistingAlbum(importList, existingAlbum);
-                {
-                    if (!existingAlbum.Monitored)
-                    {
-                        _albumService.SetAlbumMonitored(existingAlbum.Id, true);
-
-                        if (importList.ShouldMonitor == ImportListMonitorType.SpecificAlbum)
-                        {
-                            _commandQueueManager.Push(new AlbumSearchCommand(new List<int> { 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));
-                    }
-                }
-
                 return;
             }
 
@@ -230,26 +187,7 @@ namespace NzbDrone.Core.ImportLists
             {
                 var monitored = importList.ShouldMonitor != ImportListMonitorType.None;
 
-                var toAddArtist = new Artist
+                var toAddArtist = ProcessArtistReport(importList, report, listExclusions, artistsToAdd);
-                {
-                    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 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<int> { 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<ImportListExclusion> listExclusions, List<Artist> artistsToAdd)
+        private Artist ProcessArtistReport(ImportListDefinition importList, ImportListItemInfo report, Dictionary<string, ImportListExclusion> listExclusions, List<Artist> artistsToAdd)
         {
             if (report.ArtistMusicBrainzId == null)
             {
                 return null;
             }
 
-            // Check to see if artist in DB
+            if (listExclusions.ContainsKey(report.ArtistMusicBrainzId))
-            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)
             {
                 _logger.Debug("{0} [{1}] Rejected due to list exclusion", report.ArtistMusicBrainzId, report.Artist);
                 return null;
             }
 
-            if (existingArtist != null)
+            // Check to see if artist in import
-            {
+            var existingImportArtist = artistsToAdd.Find(i => i.ForeignArtistId == report.ArtistMusicBrainzId);
-                _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;
-            }
-
             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<Album> processed;
+            var processed = message.DefinitionId.HasValue ? SyncList(_importListFactory.Get(message.DefinitionId.Value)) : SyncAll();
-
-            if (message.DefinitionId.HasValue)
-            {
-                processed = SyncList(_importListFactory.Get(message.DefinitionId.Value));
-            }
-            else
-            {
-                processed = SyncAll();
-            }
 
             _eventAggregator.PublishEvent(new ImportListSyncCompleteEvent(processed));
         }