diff --git a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs index de37f5ec1..e41cbb2c7 100644 --- a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs +++ b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs @@ -46,7 +46,7 @@ namespace NzbDrone.Common.Test.Http TestLogger.Info($"{candidates.Length} TestSites available."); - _httpBinSleep = _httpBinHosts.Count() < 2 ? 100 : 10; + _httpBinSleep = _httpBinHosts.Length < 2 ? 100 : 10; } private bool IsTestSiteAvailable(string site) @@ -100,7 +100,7 @@ namespace NzbDrone.Common.Test.Http Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant(Mocker.Resolve()); - Mocker.SetConstant>(new IHttpRequestInterceptor[0]); + Mocker.SetConstant>(Array.Empty()); Mocker.SetConstant(Mocker.Resolve()); // Used for manual testing of socks proxies. @@ -193,31 +193,21 @@ namespace NzbDrone.Common.Test.Http [Test] public void should_not_throw_on_suppressed_status_codes() { - var request = new HttpRequest($"http://{_httpBinHost}/status/{HttpStatusCode.NotFound}"); + var request = new HttpRequest($"https://{_httpBinHost}/status/{HttpStatusCode.NotFound}"); request.SuppressHttpErrorStatusCodes = new[] { HttpStatusCode.NotFound }; - var exception = Assert.Throws(() => Subject.Get(request)); + Assert.Throws(() => Subject.Get(request)); ExceptionVerification.IgnoreWarns(); } - [Test] - public void should_log_unsuccessful_status_codes() - { - var request = new HttpRequest($"http://{_httpBinHost}/status/{HttpStatusCode.NotFound}"); - - var exception = Assert.Throws(() => Subject.Get(request)); - - ExceptionVerification.ExpectedWarns(1); - } - [Test] public void should_not_log_unsuccessful_status_codes() { - var request = new HttpRequest($"http://{_httpBinHost}/status/{HttpStatusCode.NotFound}"); + var request = new HttpRequest($"https://{_httpBinHost}/status/{HttpStatusCode.NotFound}"); request.LogHttpError = false; - var exception = Assert.Throws(() => Subject.Get(request)); + Assert.Throws(() => Subject.Get(request)); ExceptionVerification.ExpectedWarns(0); } diff --git a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs index 2f4030901..285623b34 100644 --- a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs +++ b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs @@ -50,12 +50,12 @@ namespace NzbDrone.Core.Test.ImportListTests .Returns(new List()); Mocker.GetMock() - .Setup(v => v.AddArtists(It.IsAny>(), false)) - .Returns((List artists, bool doRefresh) => artists); + .Setup(v => v.AddArtists(It.IsAny>(), false, true)) + .Returns((List artists, bool doRefresh, bool ignoreErrors) => artists); Mocker.GetMock() - .Setup(v => v.AddAlbums(It.IsAny>(), false)) - .Returns((List albums, bool doRefresh) => albums); + .Setup(v => v.AddAlbums(It.IsAny>(), false, true)) + .Returns((List albums, bool doRefresh, bool ignoreErrors) => albums); } private void WithAlbum() @@ -184,7 +184,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false)); + .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false, It.IsAny())); } [Test] @@ -196,7 +196,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false)); + .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false, It.IsAny())); } [Test] @@ -208,7 +208,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 1), false)); + .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 1), false, It.IsAny())); } [TestCase(ImportListMonitorType.None, false)] @@ -222,7 +222,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddArtists(It.Is>(t => t.Count == 1 && t.First().Monitored == expectedArtistMonitored), false)); + .Verify(v => v.AddArtists(It.Is>(t => t.Count == 1 && t.First().Monitored == expectedArtistMonitored), false, It.IsAny())); } [TestCase(ImportListMonitorType.None, false)] @@ -236,7 +236,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 1 && t.First().Monitored == expectedAlbumMonitored), false)); + .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 1 && t.First().Monitored == expectedAlbumMonitored), false, It.IsAny())); } [Test] @@ -248,7 +248,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false)); + .Verify(v => v.AddArtists(It.Is>(t => t.Count == 0), false, It.IsAny())); } [Test] @@ -260,7 +260,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 0), false)); + .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 0), false, It.IsAny())); } [Test] @@ -273,7 +273,7 @@ namespace NzbDrone.Core.Test.ImportListTests Subject.Execute(new ImportListSyncCommand()); Mocker.GetMock() - .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 0), false)); + .Verify(v => v.AddAlbums(It.Is>(t => t.Count == 0), false, It.IsAny())); } } } diff --git a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs index f115e74af..0a99a2195 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs @@ -117,8 +117,8 @@ namespace NzbDrone.Core.ImportLists } } - var addedArtists = _addArtistService.AddArtists(artistsToAdd, false); - var addedAlbums = _addAlbumService.AddAlbums(albumsToAdd, false); + var addedArtists = _addArtistService.AddArtists(artistsToAdd, false, true); + var addedAlbums = _addAlbumService.AddAlbums(albumsToAdd, false, true); var message = string.Format($"Import List Sync Completed. Items found: {reports.Count}, Artists added: {addedArtists.Count}, Albums added: {addedAlbums.Count}"); diff --git a/src/NzbDrone.Core/Music/Services/AddAlbumService.cs b/src/NzbDrone.Core/Music/Services/AddAlbumService.cs index 08a673bd7..1f1bb1071 100644 --- a/src/NzbDrone.Core/Music/Services/AddAlbumService.cs +++ b/src/NzbDrone.Core/Music/Services/AddAlbumService.cs @@ -13,7 +13,7 @@ namespace NzbDrone.Core.Music public interface IAddAlbumService { Album AddAlbum(Album album, bool doRefresh = true); - List AddAlbums(List albums, bool doRefresh = true); + List AddAlbums(List albums, bool doRefresh = true, bool ignoreErrors = false); } public class AddAlbumService : IAddAlbumService @@ -77,15 +77,33 @@ namespace NzbDrone.Core.Music return album; } - public List AddAlbums(List albums, bool doRefresh = true) + public List AddAlbums(List albums, bool doRefresh = true, bool ignoreErrors = false) { var added = DateTime.UtcNow; var addedAlbums = new List(); foreach (var a in albums) { - a.Added = added; - addedAlbums.Add(AddAlbum(a, doRefresh)); + try + { + a.Added = added; + if (addedAlbums.Any(f => f.ForeignAlbumId == a.ForeignAlbumId)) + { + _logger.Debug("Musicbrainz ID {0} was not added due to validation failure: Album already exists on list", a.ForeignAlbumId); + continue; + } + + addedAlbums.Add(AddAlbum(a, doRefresh)); + } + catch (ValidationException ex) + { + if (!ignoreErrors) + { + throw; + } + + _logger.Debug("Musicbrainz ID {0} was not added due to validation failures. {1}", a.ForeignAlbumId, ex.Message); + } } return addedAlbums; diff --git a/src/NzbDrone.Core/Music/Services/AddArtistService.cs b/src/NzbDrone.Core/Music/Services/AddArtistService.cs index 2abf43199..ef135d68b 100644 --- a/src/NzbDrone.Core/Music/Services/AddArtistService.cs +++ b/src/NzbDrone.Core/Music/Services/AddArtistService.cs @@ -17,7 +17,7 @@ namespace NzbDrone.Core.Music public interface IAddArtistService { Artist AddArtist(Artist newArtist, bool doRefresh = true); - List AddArtists(List newArtists, bool doRefresh = true); + List AddArtists(List newArtists, bool doRefresh = true, bool ignoreErrors = false); } public class AddArtistService : IAddArtistService @@ -63,7 +63,7 @@ namespace NzbDrone.Core.Music return newArtist; } - public List AddArtists(List newArtists, bool doRefresh = true) + public List AddArtists(List newArtists, bool doRefresh = true, bool ignoreErrors = false) { var added = DateTime.UtcNow; var artistsToAdd = new List(); @@ -75,12 +75,23 @@ namespace NzbDrone.Core.Music var artist = AddSkyhookData(s); artist = SetPropertiesAndValidate(artist); artist.Added = added; + if (artistsToAdd.Any(f => f.ForeignArtistId == artist.ForeignArtistId)) + { + _logger.Debug("Musicbrainz ID {0} was not added due to validation failure: Artist already exists on list", s.ForeignArtistId); + continue; + } + artistsToAdd.Add(artist); } - catch (Exception ex) + catch (ValidationException ex) { + if (!ignoreErrors) + { + throw; + } + // Catch Import Errors for now until we get things fixed up - _logger.Error(ex, "Failed to import id: {0} - {1}", s.Metadata.Value.ForeignArtistId, s.Metadata.Value.Name); + _logger.Debug(ex, "Failed to import id: {0} - {1}", s.Metadata.Value.ForeignArtistId, s.Metadata.Value.Name); } }