Fixed: Errors loading queue after albums in artist are removed

Fixes #1989

Co-Authored-By: Mark McDowall <markus101@users.noreply.github.com>

src/NzbDrone.Core.Test/Download/TrackedDownloads/TrackedDownloadServiceFixture.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Linq;
 using FluentAssertions;
 using Moq;
@@ -136,5 +136,164 @@ namespace NzbDrone.Core.Test.Download.TrackedDownloads
             trackedDownloads.Should().HaveCount(1);
             trackedDownloads.First().RemoteAlbum.Should().BeNull();
         }
+
+        [Test]
+        public void should_unmap_tracked_download_if_album_removed()
+        {
+            GivenDownloadHistory();
+
+            var remoteAlbum = new RemoteAlbum
+            {
+                Artist = new Artist() { Id = 5 },
+                Albums = new List<Album> { new Album { Id = 4 } },
+                ParsedAlbumInfo = new ParsedAlbumInfo()
+                {
+                    AlbumTitle = "Audio Album",
+                    ArtistName = "Audio Artist"
+                }
+            };
+
+            Mocker.GetMock<IParsingService>()
+                  .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                  .Returns(remoteAlbum);
+
+            var client = new DownloadClientDefinition()
+            {
+                Id = 1,
+                Protocol = DownloadProtocol.Torrent
+            };
+
+            var item = new DownloadClientItem()
+            {
+                Title = "Audio Artist - Audio Album [2018 - FLAC]",
+                DownloadId = "35238",
+                DownloadClientInfo = new DownloadClientItemClientInfo
+                {
+                    Protocol = client.Protocol,
+                    Id = client.Id,
+                    Name = client.Name
+                }
+            };
+
+            Subject.TrackDownload(client, item);
+            Subject.GetTrackedDownloads().Should().HaveCount(1);
+
+            // simulate deletion - album no longer maps
+            Mocker.GetMock<IParsingService>()
+                .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                .Returns(default(RemoteAlbum));
+
+            Subject.Handle(new AlbumInfoRefreshedEvent(remoteAlbum.Artist, new List<Album>(), new List<Album>(), remoteAlbum.Albums));
+
+            var trackedDownloads = Subject.GetTrackedDownloads();
+            trackedDownloads.Should().HaveCount(1);
+            trackedDownloads.First().RemoteAlbum.Should().BeNull();
+        }
+
+        [Test]
+        public void should_not_throw_when_processing_deleted_albums()
+        {
+            GivenDownloadHistory();
+
+            var remoteAlbum = new RemoteAlbum
+            {
+                Artist = new Artist() { Id = 5 },
+                Albums = new List<Album> { new Album { Id = 4 } },
+                ParsedAlbumInfo = new ParsedAlbumInfo()
+                {
+                    AlbumTitle = "Audio Album",
+                    ArtistName = "Audio Artist"
+                }
+            };
+
+            Mocker.GetMock<IParsingService>()
+                  .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                  .Returns(remoteAlbum);
+
+            var client = new DownloadClientDefinition()
+            {
+                Id = 1,
+                Protocol = DownloadProtocol.Torrent
+            };
+
+            var item = new DownloadClientItem()
+            {
+                Title = "Audio Artist - Audio Album [2018 - FLAC]",
+                DownloadId = "35238",
+                DownloadClientInfo = new DownloadClientItemClientInfo
+                {
+                    Protocol = client.Protocol,
+                    Id = client.Id,
+                    Name = client.Name
+                }
+            };
+
+            Subject.TrackDownload(client, item);
+            Subject.GetTrackedDownloads().Should().HaveCount(1);
+
+            // simulate deletion - album no longer maps
+            Mocker.GetMock<IParsingService>()
+                .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                .Returns(default(RemoteAlbum));
+
+            Subject.Handle(new AlbumInfoRefreshedEvent(remoteAlbum.Artist, new List<Album>(), new List<Album>(), remoteAlbum.Albums));
+
+            var trackedDownloads = Subject.GetTrackedDownloads();
+            trackedDownloads.Should().HaveCount(1);
+            trackedDownloads.First().RemoteAlbum.Should().BeNull();
+        }
+
+        [Test]
+        public void should_not_throw_when_processing_deleted_artist()
+        {
+            GivenDownloadHistory();
+
+            var remoteAlbum = new RemoteAlbum
+            {
+                Artist = new Artist() { Id = 5 },
+                Albums = new List<Album> { new Album { Id = 4 } },
+                ParsedAlbumInfo = new ParsedAlbumInfo()
+                {
+                    AlbumTitle = "Audio Album",
+                    ArtistName = "Audio Artist"
+                }
+            };
+
+            Mocker.GetMock<IParsingService>()
+                  .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                  .Returns(remoteAlbum);
+
+            var client = new DownloadClientDefinition()
+            {
+                Id = 1,
+                Protocol = DownloadProtocol.Torrent
+            };
+
+            var item = new DownloadClientItem()
+            {
+                Title = "Audio Artist - Audio Album [2018 - FLAC]",
+                DownloadId = "35238",
+                DownloadClientInfo = new DownloadClientItemClientInfo
+                {
+                    Protocol = client.Protocol,
+                    Id = client.Id,
+                    Name = client.Name
+                }
+            };
+
+            Subject.TrackDownload(client, item);
+            Subject.GetTrackedDownloads().Should().HaveCount(1);
+
+            // simulate deletion - album no longer maps
+            Mocker.GetMock<IParsingService>()
+                .Setup(s => s.Map(It.Is<ParsedAlbumInfo>(i => i.AlbumTitle == "Audio Album" && i.ArtistName == "Audio Artist"), It.IsAny<int>(), It.IsAny<IEnumerable<int>>()))
+                .Returns(default(RemoteAlbum));
+
+            Subject.Handle(new ArtistsDeletedEvent(new List<Artist> { remoteAlbum.Artist }, true, true));
+
+            var trackedDownloads = Subject.GetTrackedDownloads();
+            trackedDownloads.Should().HaveCount(1);
+            trackedDownloads.First().RemoteAlbum.Should().BeNull();
+        }
     }
 }

src/NzbDrone.Core/Download/TrackedDownloads/TrackedDownloadService.cs

@@ -56,22 +56,11 @@ namespace NzbDrone.Core.Download.TrackedDownloads
             return _cache.Find(downloadId);
         }
 
-        public void UpdateAlbumCache(int albumId)
+        private void UpdateCachedItem(TrackedDownload trackedDownload)
         {
-            var updateCacheItems = _cache.Values.Where(x => x.RemoteAlbum != null && x.RemoteAlbum.Albums.Any(a => a.Id == albumId)).ToList();
+            var parsedAlbumInfo = Parser.Parser.ParseAlbumTitle(trackedDownload.DownloadItem.Title);
 
-            foreach (var item in updateCacheItems)
+            trackedDownload.RemoteAlbum = parsedAlbumInfo == null ? null : _parsingService.Map(parsedAlbumInfo, 0, new List<int> { });
-            {
-                var parsedAlbumInfo = Parser.Parser.ParseAlbumTitle(item.DownloadItem.Title);
-                item.RemoteAlbum = null;
-
-                if (parsedAlbumInfo != null)
-                {
-                    item.RemoteAlbum = _parsingService.Map(parsedAlbumInfo);
-                }
-            }
-
-            _eventAggregator.PublishEvent(new TrackedDownloadRefreshedEvent(GetTrackedDownloads()));
         }
 
         public void StopTracking(string downloadId)
@@ -259,7 +248,54 @@ namespace NzbDrone.Core.Download.TrackedDownloads
 
         public void Handle(AlbumDeletedEvent message)
         {
-            UpdateAlbumCache(message.Album.Id);
+            var cachedItems = _cache.Values.Where(x => x.RemoteAlbum != null && x.RemoteAlbum.Albums.Any(a => a.Id == message.Album.Id)).ToList();
+
+            if (cachedItems.Any())
+            {
+                cachedItems.ForEach(UpdateCachedItem);
+
+                _eventAggregator.PublishEvent(new TrackedDownloadRefreshedEvent(GetTrackedDownloads()));
+            }
+        }
+
+        public void Handle(AlbumInfoRefreshedEvent message)
+        {
+            var needsToUpdate = false;
+
+            foreach (var album in message.Removed)
+            {
+                var cachedItems = _cache.Values.Where(t =>
+                                            t.RemoteAlbum?.Albums != null &&
+                                            t.RemoteAlbum.Albums.Any(e => e.Id == album.Id))
+                                        .ToList();
+
+                if (cachedItems.Any())
+                {
+                    needsToUpdate = true;
+                }
+
+                cachedItems.ForEach(UpdateCachedItem);
+            }
+
+            if (needsToUpdate)
+            {
+                _eventAggregator.PublishEvent(new TrackedDownloadRefreshedEvent(GetTrackedDownloads()));
+            }
+        }
+
+        public void Handle(ArtistsDeletedEvent message)
+        {
+            var cachedItems = _cache.Values.Where(t =>
+                                        t.RemoteAlbum?.Artist != null &&
+                                        message.Artists.Select(a => a.Id).Contains(t.RemoteAlbum.Artist.Id))
+                                    .ToList();
+
+            if (cachedItems.Any())
+            {
+                cachedItems.ForEach(UpdateCachedItem);
+
+                _eventAggregator.PublishEvent(new TrackedDownloadRefreshedEvent(GetTrackedDownloads()));
+            }
         }
     }
 }

src/NzbDrone.Core/Music/Events/AlbumInfoRefreshedEvent.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using NzbDrone.Common.Messaging;
 
@@ -9,12 +9,13 @@ namespace NzbDrone.Core.Music.Events
         public Artist Artist { get; set; }
         public ReadOnlyCollection<Album> Added { get; private set; }
         public ReadOnlyCollection<Album> Updated { get; private set; }
+        public ReadOnlyCollection<Album> Removed { get; private set; }
 
-        public AlbumInfoRefreshedEvent(Artist artist, IList<Album> added, IList<Album> updated)
+        public AlbumInfoRefreshedEvent(Artist artist, IList<Album> added, IList<Album> updated, IList<Album> removed)
         {
             Artist = artist;
             Added = new ReadOnlyCollection<Album>(added);
-            Updated = new ReadOnlyCollection<Album>(updated);
+            Removed = new ReadOnlyCollection<Album>(removed);
         }
     }
 }

src/NzbDrone.Core/Music/Services/RefreshArtistService.cs

@@ -272,9 +272,9 @@ namespace NzbDrone.Core.Music
             _eventAggregator.PublishEvent(new ArtistRefreshCompleteEvent(entity));
         }
 
-        protected override void PublishChildrenUpdatedEvent(Artist entity, List<Album> newChildren, List<Album> updateChildren)
+        protected override void PublishChildrenUpdatedEvent(Artist entity, List<Album> newChildren, List<Album> updateChildren, List<Album> removedChildren)
         {
-            _eventAggregator.PublishEvent(new AlbumInfoRefreshedEvent(entity, newChildren, updateChildren));
+            _eventAggregator.PublishEvent(new AlbumInfoRefreshedEvent(entity, newChildren, updateChildren, removedChildren));
         }
 
         private void Rescan(List<Artist> artists, bool isNew, CommandTrigger trigger, bool infoUpdated)

src/NzbDrone.Core/Music/Services/RefreshEntityServiceBase.cs

@@ -109,7 +109,7 @@ namespace NzbDrone.Core.Music
         {
         }
 
-        protected virtual void PublishChildrenUpdatedEvent(TEntity entity, List<TChild> newChildren, List<TChild> updateChildren)
+        protected virtual void PublishChildrenUpdatedEvent(TEntity entity, List<TChild> newChildren, List<TChild> updateChildren, List<TChild> removedChildren)
         {
         }
 
@@ -287,7 +287,7 @@ namespace NzbDrone.Core.Music
             // now trigger updates
             var updated = RefreshChildren(sortedChildren, remoteChildren, forceChildRefresh, forceUpdateFileTags, lastUpdate);
 
-            PublishChildrenUpdatedEvent(entity, sortedChildren.Added, sortedChildren.Updated);
+            PublishChildrenUpdatedEvent(entity, sortedChildren.Added, sortedChildren.Updated, sortedChildren.Deleted);
             return updated;
         }
     }