Fixed: Don't attempt to insert duplicate ids or monitor multiple releases (#684)

src/NzbDrone.Core.Test/MusicTests/AddAlbumFixture.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using FizzWare.NBuilder;
 using FluentAssertions;
 using FluentValidation;
@@ -20,8 +21,8 @@ namespace NzbDrone.Core.Test.MusicTests
     {
         private Album _fakeAlbum;
         private AlbumRelease _fakeRelease;
+        private List<ArtistMetadata> _fakeArtists;
         private readonly string _fakeArtistForeignId = "xxx-xxx-xxx";
-        private readonly List<ArtistMetadata> _fakeArtists = new List<ArtistMetadata> { new ArtistMetadata() };
 
         [SetUp]
         public void Setup()
@@ -34,6 +35,19 @@ namespace NzbDrone.Core.Test.MusicTests
                 .Build();
             _fakeRelease.Tracks = new List<Track>();
             _fakeAlbum.AlbumReleases = new List<AlbumRelease> {_fakeRelease};
+        
+            _fakeArtists = Builder<ArtistMetadata>.CreateListOfSize(1)
+                .TheFirst(1)
+                .With(x => x.ForeignArtistId = _fakeArtistForeignId)
+                .Build() as List<ArtistMetadata>;
+            
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
+                .Returns(new List<AlbumRelease>());
+
+            Mocker.GetMock<IArtistMetadataRepository>()
+                .Setup(x => x.FindById(_fakeArtistForeignId))
+                .Returns(_fakeArtists[0]);
         }
 
         private void GivenValidAlbum(string lidarrId)
@@ -74,5 +88,87 @@ namespace NzbDrone.Core.Test.MusicTests
 
             ExceptionVerification.ExpectedErrors(1);
         }
+
+        [Test]
+        public void should_not_add_duplicate_releases()
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.AlbumId = newAlbum.Id)
+                .TheFirst(4)
+                .With(x => x.ForeignReleaseId = "DuplicateId1")
+                .TheLast(1)
+                .With(x => x.ForeignReleaseId = "DuplicateId2")
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(1)
+                .All()
+                .With(x => x.ForeignReleaseId = "DuplicateId2")
+                .Build() as List<AlbumRelease>;
+            
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(newAlbum.Id, It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+            
+            var updatedReleases = Subject.AddAlbumReleases(newAlbum);
+            
+            updatedReleases.Should().HaveCount(7);
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.UpdateMany(It.Is<List<AlbumRelease>>(l => l.Count == 1 && l.Select(r => r.ForeignReleaseId).Distinct().Count() == 1)), Times.Once());
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.InsertMany(It.Is<List<AlbumRelease>>(l => l.Count == 6 && 
+                                                                    l.Select(r => r.ForeignReleaseId).Distinct().Count() == 6 &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("DuplicateId2"))), 
+                        Times.Once());
+        }
+        
+        [Test]
+        public void should_only_add_one_monitored_release_ignoring_skyhook()
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.Monitored = true)
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            
+            var updatedReleases = Subject.AddAlbumReleases(newAlbum);
+            
+            updatedReleases.Count(x => x.Monitored).Should().Be(1);
+        }
+        
+        [Test]
+        public void should_only_add_one_monitored_release_combining_with_existing()
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.Monitored = false)
+                .Build() as List<AlbumRelease>;
+            releases[1].Monitored = true;
+
+            newAlbum.AlbumReleases = releases;
+            
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(1)
+                .All()
+                .With(x => x.ForeignReleaseId = releases[0].ForeignReleaseId)
+                .With(x => x.Monitored = true)
+                .Build() as List<AlbumRelease>;
+            
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(newAlbum.Id, It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+            
+            var updatedReleases = Subject.AddAlbumReleases(newAlbum);
+            
+            updatedReleases.Count(x => x.Monitored).Should().Be(1);
+        }
     }
 }

src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs

@@ -143,12 +143,235 @@ namespace NzbDrone.Core.Test.MusicTests
         }
 
         [Test]
-        public void should_remove_items_from_list()
+        public void should_not_add_duplicate_releases()
         {
-            var releases = Builder<AlbumRelease>.CreateListOfSize(2).Build();
-            var release = releases[0];
-            releases.Remove(release);
-            releases.Should().HaveCount(1);
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            // this is required because RefreshAlbumInfo will edit the album passed in
+            var albumCopy = Builder<Album>.CreateNew().Build();
+
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.AlbumId = newAlbum.Id)
+                .With(x => x.Monitored = true)
+                .TheFirst(4)
+                .With(x => x.ForeignReleaseId = "DuplicateId1")
+                .TheLast(1)
+                .With(x => x.ForeignReleaseId = "DuplicateId2")
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            albumCopy.AlbumReleases = releases;
+
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(1)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "DuplicateId2")
+                .With(x => x.Monitored = true)
+                .Build() as List<AlbumRelease>;
+
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+
+            Mocker.GetMock<IProvideAlbumInfo>()
+                .Setup(x => x.GetAlbumInfo(It.IsAny<string>()))
+                .Returns(Tuple.Create("dummy string", albumCopy, new List<ArtistMetadata>()));
+
+            Subject.RefreshAlbumInfo(newAlbum, false);
+            
+            newAlbum.AlbumReleases.Value.Should().HaveCount(7);
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.DeleteMany(It.Is<List<AlbumRelease>>(l => l.Count == 0)), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.UpdateMany(It.Is<List<AlbumRelease>>(l => l.Count == 1 && l.Select(r => r.ForeignReleaseId).Distinct().Count() == 1)), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.InsertMany(It.Is<List<AlbumRelease>>(l => l.Count == 6 &&
+                                                                    l.Select(r => r.ForeignReleaseId).Distinct().Count() == l.Count &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("DuplicateId2"))),
+                        Times.Once());
+        }
+        
+        [TestCase(true, true, 1)]
+        [TestCase(true, false, 0)]
+        [TestCase(false, true, 1)]
+        [TestCase(false, false, 0)]
+        public void should_only_leave_one_release_monitored(bool skyhookMonitored, bool existingMonitored, int expectedUpdates)
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            // this is required because RefreshAlbumInfo will edit the album passed in
+            var albumCopy = Builder<Album>.CreateNew().Build();
+
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.AlbumId = newAlbum.Id)
+                .With(x => x.Monitored = skyhookMonitored)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "ExistingId2")
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            albumCopy.AlbumReleases = releases;
+
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(2)
+                .All()
+                .With(x => x.Monitored = existingMonitored)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "ExistingId2")
+                .Build() as List<AlbumRelease>;
+
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+
+            Mocker.GetMock<IProvideAlbumInfo>()
+                .Setup(x => x.GetAlbumInfo(It.IsAny<string>()))
+                .Returns(Tuple.Create("dummy string", albumCopy, new List<ArtistMetadata>()));
+
+            Subject.RefreshAlbumInfo(newAlbum, false);
+            
+            newAlbum.AlbumReleases.Value.Should().HaveCount(10);
+            newAlbum.AlbumReleases.Value.Where(x => x.Monitored).Should().HaveCount(1);
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.DeleteMany(It.Is<List<AlbumRelease>>(l => l.Count == 0)), Times.Once());
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.UpdateMany(It.Is<List<AlbumRelease>>(l => l.Count == expectedUpdates && l.Select(r => r.ForeignReleaseId).Distinct().Count() == expectedUpdates)), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.InsertMany(It.Is<List<AlbumRelease>>(l => l.Count == 8 &&
+                                                                    l.Select(r => r.ForeignReleaseId).Distinct().Count() == l.Count &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId1") &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId2"))),
+                        Times.Once());
+        }
+
+        [Test]
+        public void refreshing_album_should_not_change_monitored_release_if_monitored_release_not_deleted()
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            // this is required because RefreshAlbumInfo will edit the album passed in
+            var albumCopy = Builder<Album>.CreateNew().Build();
+
+            // only ExistingId1 is monitored from dummy skyhook
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.AlbumId = newAlbum.Id)
+                .With(x => x.Monitored = false)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .With(x => x.Monitored = true)
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "ExistingId2")
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            albumCopy.AlbumReleases = releases;
+
+            // ExistingId2 is monitored in DB
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(2)
+                .All()
+                .With(x => x.Monitored = false)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "ExistingId2")
+                .With(x => x.Monitored = true)
+                .Build() as List<AlbumRelease>;
+
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+
+            Mocker.GetMock<IProvideAlbumInfo>()
+                .Setup(x => x.GetAlbumInfo(It.IsAny<string>()))
+                .Returns(Tuple.Create("dummy string", albumCopy, new List<ArtistMetadata>()));
+
+            Subject.RefreshAlbumInfo(newAlbum, false);
+
+            newAlbum.AlbumReleases.Value.Should().HaveCount(10);
+            newAlbum.AlbumReleases.Value.Where(x => x.Monitored).Should().HaveCount(1);
+            newAlbum.AlbumReleases.Value.Single(x => x.Monitored).ForeignReleaseId.Should().Be("ExistingId2");
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.DeleteMany(It.Is<List<AlbumRelease>>(l => l.Count == 0)), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.UpdateMany(It.Is<List<AlbumRelease>>(l => l.Count == 0)), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.InsertMany(It.Is<List<AlbumRelease>>(l => l.Count == 8 &&
+                                                                    l.Select(r => r.ForeignReleaseId).Distinct().Count() == l.Count &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId1") &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId2"))),
+                        Times.Once());
+        }
+        
+        [Test]
+        public void refreshing_album_should_change_monitored_release_if_monitored_release_deleted()
+        {
+            var newAlbum = Builder<Album>.CreateNew().Build();
+            // this is required because RefreshAlbumInfo will edit the album passed in
+            var albumCopy = Builder<Album>.CreateNew().Build();
+            
+            // Only existingId1 monitored in skyhook.  ExistingId2 is missing
+            var releases = Builder<AlbumRelease>.CreateListOfSize(10)
+                .All()
+                .With(x => x.AlbumId = newAlbum.Id)
+                .With(x => x.Monitored = false)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .With(x => x.Monitored = true)
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "NotExistingId2")
+                .Build() as List<AlbumRelease>;
+
+            newAlbum.AlbumReleases = releases;
+            albumCopy.AlbumReleases = releases;
+
+            // ExistingId2 is monitored but will be deleted
+            var existingReleases = Builder<AlbumRelease>.CreateListOfSize(2)
+                .All()
+                .With(x => x.Monitored = false)
+                .TheFirst(1)
+                .With(x => x.ForeignReleaseId = "ExistingId1")
+                .TheNext(1)
+                .With(x => x.ForeignReleaseId = "ExistingId2")
+                .With(x => x.Monitored = true)
+                .Build() as List<AlbumRelease>;
+
+            Mocker.GetMock<IReleaseService>()
+                .Setup(x => x.GetReleasesForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
+                .Returns(existingReleases);
+
+            Mocker.GetMock<IProvideAlbumInfo>()
+                .Setup(x => x.GetAlbumInfo(It.IsAny<string>()))
+                .Returns(Tuple.Create("dummy string", albumCopy, new List<ArtistMetadata>()));
+
+            Subject.RefreshAlbumInfo(newAlbum, false);
+
+            newAlbum.AlbumReleases.Value.Should().HaveCount(10);
+            newAlbum.AlbumReleases.Value.Where(x => x.Monitored).Should().HaveCount(1);
+            newAlbum.AlbumReleases.Value.Single(x => x.Monitored).ForeignReleaseId.Should().NotBe("ExistingId2");
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.DeleteMany(It.Is<List<AlbumRelease>>(l => l.Single().ForeignReleaseId == "ExistingId2")), Times.Once());
+
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.UpdateMany(It.Is<List<AlbumRelease>>(l => l.Count == 0)), Times.Once());
+            
+            Mocker.GetMock<IReleaseService>()
+                .Verify(x => x.InsertMany(It.Is<List<AlbumRelease>>(l => l.Count == 9 &&
+                                                                    l.Select(r => r.ForeignReleaseId).Distinct().Count() == l.Count &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId1") &&
+                                                                    !l.Select(r => r.ForeignReleaseId).Contains("ExistingId2"))),
+                        Times.Once());
         }
     }
 }

src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs

@@ -311,7 +311,6 @@ namespace NzbDrone.Core.MetadataSource.SkyHook
             if (resource.Releases != null)
             {
                 album.AlbumReleases = resource.Releases.Select(x => MapRelease(x, artistDict)).ToList();
-                album.AlbumReleases.Value.OrderByDescending(x => x.TrackCount).First().Monitored = true;
             }
 
             album.AnyReleaseOk = true;

src/NzbDrone.Core/Music/AddAlbumService.cs

@@ -5,6 +5,7 @@ using FluentValidation;
 using FluentValidation.Results;
 using NLog;
 using NzbDrone.Common.EnsureThat;
+using NzbDrone.Common.Extensions;
 using NzbDrone.Common.Instrumentation.Extensions;
 using NzbDrone.Core.Exceptions;
 using NzbDrone.Core.MetadataSource;
@@ -20,41 +21,94 @@ namespace NzbDrone.Core.Music
     public class AddAlbumService : IAddAlbumService
     {
         private readonly IAlbumService _albumService;
+        private readonly IReleaseService _releaseService;
         private readonly IProvideAlbumInfo _albumInfo;
         private readonly IArtistMetadataRepository _artistMetadataRepository;
         private readonly IRefreshTrackService _refreshTrackService;
         private readonly Logger _logger;
 
         public AddAlbumService(IAlbumService albumService,
+                               IReleaseService releaseService,
                                IProvideAlbumInfo albumInfo,
                                IArtistMetadataRepository artistMetadataRepository,
                                IRefreshTrackService refreshTrackService,
                                Logger logger)
         {
             _albumService = albumService;
+            _releaseService = releaseService;
             _albumInfo = albumInfo;
             _artistMetadataRepository = artistMetadataRepository;
             _refreshTrackService = refreshTrackService;
             _logger = logger;
         }
 
-        public Album AddAlbum(Album newAlbum)
+        public List<AlbumRelease> AddAlbumReleases(Album album)
         {
-            Ensure.That(newAlbum, () => newAlbum).IsNotNull();
+            var remoteReleases = album.AlbumReleases.Value.DistinctBy(m => m.ForeignReleaseId).ToList();
+            var existingReleases = _releaseService.GetReleasesForRefresh(album.Id, remoteReleases.Select(x => x.ForeignReleaseId));
+            var newReleaseList = new List<AlbumRelease>();
+            var updateReleaseList = new List<AlbumRelease>();
             
-            var tuple = AddSkyhookData(newAlbum);
-            newAlbum = tuple.Item2;
+            foreach (var release in remoteReleases)
+            {
+                release.AlbumId = album.Id;
+                release.Album = album;
+                var releaseToRefresh = existingReleases.SingleOrDefault(r => r.ForeignReleaseId == release.ForeignReleaseId);
+
+                if (releaseToRefresh != null)
+                {
+                    existingReleases.Remove(releaseToRefresh);
+
+                    // copy across the db keys and check for equality
+                    release.Id = releaseToRefresh.Id;
+                    release.AlbumId = releaseToRefresh.AlbumId;
+
+                    updateReleaseList.Add(release);
+                }
+                else
+                {
+                    newReleaseList.Add(release);
+                }
+            }
+            
+            // Ensure only one release is monitored
+            remoteReleases.ForEach(x => x.Monitored = false);
+            remoteReleases.OrderByDescending(x => x.TrackCount).First().Monitored = true;
+            Ensure.That(remoteReleases.Count(x => x.Monitored) == 1).IsTrue();
+            
+            // Since this is a new album, we can't be deleting any existing releases
+            _releaseService.UpdateMany(updateReleaseList);
+            _releaseService.InsertMany(newReleaseList);
+            
+            return remoteReleases;
+        }
+        
+        private Album AddAlbum(Tuple<string, Album, List<ArtistMetadata>> skyHookData)
+        {
+            var newAlbum = skyHookData.Item2;
             _logger.ProgressInfo("Adding Album {0}", newAlbum.Title);
-            _artistMetadataRepository.UpsertMany(tuple.Item3);
-            _albumService.AddAlbum(newAlbum, tuple.Item1);
             
-            // make sure releases are populated for tag writing in the track refresh
-            newAlbum.AlbumReleases.Value.ForEach(x => x.Album = newAlbum);
+            _artistMetadataRepository.UpsertMany(skyHookData.Item3);
+            newAlbum.ArtistMetadata = _artistMetadataRepository.FindById(skyHookData.Item1);
+            newAlbum.ArtistMetadataId = newAlbum.ArtistMetadata.Value.Id;
+            
+            _albumService.AddAlbum(newAlbum);
+            AddAlbumReleases(newAlbum);
+
             _refreshTrackService.RefreshTrackInfo(newAlbum, false);
 
             return newAlbum;
         }
             
+        public Album AddAlbum(Album newAlbum)
+        {
+            Ensure.That(newAlbum, () => newAlbum).IsNotNull();
+
+            var tuple = AddSkyhookData(newAlbum);
+            
+            return AddAlbum(tuple);
+        }
+
         public List<Album> AddAlbums(List<Album> newAlbums)
         {
             var added = DateTime.UtcNow;
@@ -63,17 +117,10 @@ namespace NzbDrone.Core.Music
             foreach (var newAlbum in newAlbums)
             {
                 var tuple = AddSkyhookData(newAlbum);
-                var album = tuple.Item2;
-                album.Added = added;
-                album.LastInfoSync = added;
-                _logger.ProgressInfo("Adding Album {0}", newAlbum.Title);
-                _artistMetadataRepository.UpsertMany(tuple.Item3);
-                album = _albumService.AddAlbum(album, tuple.Item1);
-
-                // make sure releases are populated for tag writing in the track refresh
-                album.AlbumReleases.Value.ForEach(x => x.Album = album);
-                _refreshTrackService.RefreshTrackInfo(album, false);
-                albumsToAdd.Add(album);
+                tuple.Item2.Added = added;
+                tuple.Item2.LastInfoSync = added;
+                
+                albumsToAdd.Add(AddAlbum(tuple));
             }
 
             return albumsToAdd;

src/NzbDrone.Core/Music/AlbumService.cs

@@ -17,7 +17,7 @@ namespace NzbDrone.Core.Music
         List<Album> GetAlbumsByArtist(int artistId);
         List<Album> GetAlbumsByArtistMetadataId(int artistMetadataId);
         List<Album> GetAlbumsForRefresh(int artistMetadataId, IEnumerable<string> foreignIds);
-        Album AddAlbum(Album newAlbum, string albumArtistId);
+        Album AddAlbum(Album newAlbum);
         Album FindById(string foreignId);
         Album FindByTitle(int artistId, string title);
         Album FindByTitleInexact(int artistId, string title);
@@ -44,41 +44,22 @@ namespace NzbDrone.Core.Music
                                 IHandle<ArtistDeletedEvent>
     {
         private readonly IAlbumRepository _albumRepository;
-        private readonly IReleaseRepository _releaseRepository;
-        private readonly IArtistMetadataRepository _artistMetadataRepository;
         private readonly IEventAggregator _eventAggregator;
-        private readonly ITrackService _trackService;
         private readonly Logger _logger;
 
         public AlbumService(IAlbumRepository albumRepository,
-                            IReleaseRepository releaseRepository,
-                            IArtistMetadataRepository artistMetadataRepository,
                             IEventAggregator eventAggregator,
-                            ITrackService trackService,
                             Logger logger)
         {
             _albumRepository = albumRepository;
-            _releaseRepository = releaseRepository;
-            _artistMetadataRepository = artistMetadataRepository;
             _eventAggregator = eventAggregator;
-            _trackService = trackService;
             _logger = logger;
         }
 
-        public Album AddAlbum(Album newAlbum, string albumArtistId)
+        public Album AddAlbum(Album newAlbum)
         {
             _albumRepository.Insert(newAlbum);
 
-            foreach (var release in newAlbum.AlbumReleases.Value)
-            {
-                release.AlbumId = newAlbum.Id;
-            }
-            _releaseRepository.InsertMany(newAlbum.AlbumReleases.Value);
-            
-            newAlbum.ArtistMetadata = _artistMetadataRepository.FindById(albumArtistId);
-            newAlbum.ArtistMetadataId = newAlbum.ArtistMetadata.Value.Id;
-            
-            _albumRepository.Update(newAlbum);
             //_eventAggregator.PublishEvent(new AlbumAddedEvent(GetAlbum(newAlbum.Id)));
 
             return newAlbum;

src/NzbDrone.Core/Music/RefreshAlbumService.cs

@@ -11,6 +11,7 @@ using NzbDrone.Core.Exceptions;
 using NzbDrone.Core.Messaging.Commands;
 using NzbDrone.Core.Music.Commands;
 using NzbDrone.Core.MediaFiles;
+using NzbDrone.Common.EnsureThat;
 
 namespace NzbDrone.Core.Music
 {
@@ -146,14 +147,23 @@ namespace NzbDrone.Core.Music
 
             var remoteReleases = albumInfo.AlbumReleases.Value.DistinctBy(m => m.ForeignReleaseId).ToList();
             var existingReleases = _releaseService.GetReleasesForRefresh(album.Id, remoteReleases.Select(x => x.ForeignReleaseId));
+            // Keep track of which existing release we want to end up monitored
+            var existingToMonitor = existingReleases.Where(x => x.Monitored).OrderByDescending(x => x.TrackCount).FirstOrDefault();
+
             var newReleaseList = new List<AlbumRelease>();
             var updateReleaseList = new List<AlbumRelease>();
-            var upToDateCount = 0;
+            var upToDateReleaseList = new List<AlbumRelease>();
 
             foreach (var release in remoteReleases)
             {
                 release.AlbumId = album.Id;
                 release.Album = album;
+
+                // force to unmonitored, then fix monitored one later
+                // once we have made sure that it's unique.  This make sure
+                // that we unmonitor anything in database that shouldn't be monitored.
+                release.Monitored = false;
+
                 var releaseToRefresh = existingReleases.SingleOrDefault(r => r.ForeignReleaseId == release.ForeignReleaseId);
 
                 if (releaseToRefresh != null)
@@ -163,7 +173,6 @@ namespace NzbDrone.Core.Music
                     // copy across the db keys and check for equality
                     release.Id = releaseToRefresh.Id;
                     release.AlbumId = releaseToRefresh.AlbumId;
-                    release.Monitored = releaseToRefresh.Monitored;
                     
                     if (!releaseToRefresh.Equals(release))
                     {
@@ -171,18 +180,38 @@ namespace NzbDrone.Core.Music
                     }
                     else
                     {
-                        upToDateCount++;
+                        upToDateReleaseList.Add(release);
                     }
                 }
                 else
                 {
-                    release.Monitored = false;
                     newReleaseList.Add(release);
                 }
+                
                 album.AlbumReleases.Value.Add(release);
             }
             
-            _logger.Debug($"{album} {upToDateCount} releases up to date; Deleting {existingReleases.Count}, Updating {updateReleaseList.Count}, Adding {newReleaseList.Count} releases.");
+            var refreshedToMonitor = remoteReleases.SingleOrDefault(x => x.ForeignReleaseId == existingToMonitor?.ForeignReleaseId) ??
+                remoteReleases.OrderByDescending(x => x.TrackCount).First();
+            refreshedToMonitor.Monitored = true;
+            
+            if (upToDateReleaseList.Contains(refreshedToMonitor))
+            {
+                // we weren't going to update, but have changed monitored so now need to
+                upToDateReleaseList.Remove(refreshedToMonitor);
+                updateReleaseList.Add(refreshedToMonitor);
+            }
+            else if (updateReleaseList.Contains(refreshedToMonitor) && refreshedToMonitor.Equals(existingToMonitor))
+            {
+                // we were going to update because Monitored was incorrect but now it matches
+                // and so no need to update
+                updateReleaseList.Remove(refreshedToMonitor);
+                upToDateReleaseList.Add(refreshedToMonitor);
+            }
+            
+            Ensure.That(album.AlbumReleases.Value.Count(x => x.Monitored) == 1).IsTrue();
+            
+            _logger.Debug($"{album} {upToDateReleaseList.Count} releases up to date; Deleting {existingReleases.Count}, Updating {updateReleaseList.Count}, Adding {newReleaseList.Count} releases.");
 
             // before deleting anything, remove musicbrainz ids for things we are deleting
             _audioTagService.RemoveMusicBrainzTags(existingReleases);
@@ -191,13 +220,6 @@ namespace NzbDrone.Core.Music
             _releaseService.UpdateMany(updateReleaseList);
             _releaseService.InsertMany(newReleaseList);
 
-            if (album.AlbumReleases.Value.Count(x => x.Monitored) == 0)
-            {
-                var toMonitor = album.AlbumReleases.Value.OrderByDescending(x => x.TrackCount).First();
-                toMonitor.Monitored = true;
-                _releaseService.UpdateMany(new List<AlbumRelease> { toMonitor });
-            }
-
             // if we have updated a monitored release, refresh all file tags
             forceUpdateFileTags |= updateReleaseList.Any(x => x.Monitored);
 
@@ -205,7 +227,6 @@ namespace NzbDrone.Core.Music
             _albumService.UpdateMany(new List<Album>{album});
 
             _logger.Debug("Finished album refresh for {0}", album.Title);
-
         }
 
         public void Execute(RefreshAlbumCommand message)