Fixed: Don't fail entire import if Validation error on list item

Fixes #2021
Fixes #2133
pull/2154/head
Qstick 3 years ago
parent 3a7e5c9201
commit d198c9987e

@ -46,7 +46,7 @@ namespace NzbDrone.Common.Test.Http
TestLogger.Info($"{candidates.Length} TestSites available."); TestLogger.Info($"{candidates.Length} TestSites available.");
_httpBinSleep = _httpBinHosts.Count() < 2 ? 100 : 10; _httpBinSleep = _httpBinHosts.Length < 2 ? 100 : 10;
} }
private bool IsTestSiteAvailable(string site) private bool IsTestSiteAvailable(string site)
@ -100,7 +100,7 @@ namespace NzbDrone.Common.Test.Http
Mocker.SetConstant<ICacheManager>(Mocker.Resolve<CacheManager>()); Mocker.SetConstant<ICacheManager>(Mocker.Resolve<CacheManager>());
Mocker.SetConstant<ICreateManagedWebProxy>(Mocker.Resolve<ManagedWebProxyFactory>()); Mocker.SetConstant<ICreateManagedWebProxy>(Mocker.Resolve<ManagedWebProxyFactory>());
Mocker.SetConstant<IRateLimitService>(Mocker.Resolve<RateLimitService>()); Mocker.SetConstant<IRateLimitService>(Mocker.Resolve<RateLimitService>());
Mocker.SetConstant<IEnumerable<IHttpRequestInterceptor>>(new IHttpRequestInterceptor[0]); Mocker.SetConstant<IEnumerable<IHttpRequestInterceptor>>(Array.Empty<IHttpRequestInterceptor>());
Mocker.SetConstant<IHttpDispatcher>(Mocker.Resolve<TDispatcher>()); Mocker.SetConstant<IHttpDispatcher>(Mocker.Resolve<TDispatcher>());
// Used for manual testing of socks proxies. // Used for manual testing of socks proxies.
@ -193,31 +193,21 @@ namespace NzbDrone.Common.Test.Http
[Test] [Test]
public void should_not_throw_on_suppressed_status_codes() 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 }; request.SuppressHttpErrorStatusCodes = new[] { HttpStatusCode.NotFound };
var exception = Assert.Throws<HttpException>(() => Subject.Get<HttpBinResource>(request)); Assert.Throws<HttpException>(() => Subject.Get<HttpBinResource>(request));
ExceptionVerification.IgnoreWarns(); ExceptionVerification.IgnoreWarns();
} }
[Test]
public void should_log_unsuccessful_status_codes()
{
var request = new HttpRequest($"http://{_httpBinHost}/status/{HttpStatusCode.NotFound}");
var exception = Assert.Throws<HttpException>(() => Subject.Get<HttpBinResource>(request));
ExceptionVerification.ExpectedWarns(1);
}
[Test] [Test]
public void should_not_log_unsuccessful_status_codes() 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; request.LogHttpError = false;
var exception = Assert.Throws<HttpException>(() => Subject.Get<HttpBinResource>(request)); Assert.Throws<HttpException>(() => Subject.Get<HttpBinResource>(request));
ExceptionVerification.ExpectedWarns(0); ExceptionVerification.ExpectedWarns(0);
} }

@ -50,12 +50,12 @@ namespace NzbDrone.Core.Test.ImportListTests
.Returns(new List<ImportListExclusion>()); .Returns(new List<ImportListExclusion>());
Mocker.GetMock<IAddArtistService>() Mocker.GetMock<IAddArtistService>()
.Setup(v => v.AddArtists(It.IsAny<List<Artist>>(), false)) .Setup(v => v.AddArtists(It.IsAny<List<Artist>>(), false, true))
.Returns((List<Artist> artists, bool doRefresh) => artists); .Returns((List<Artist> artists, bool doRefresh, bool ignoreErrors) => artists);
Mocker.GetMock<IAddAlbumService>() Mocker.GetMock<IAddAlbumService>()
.Setup(v => v.AddAlbums(It.IsAny<List<Album>>(), false)) .Setup(v => v.AddAlbums(It.IsAny<List<Album>>(), false, true))
.Returns((List<Album> albums, bool doRefresh) => albums); .Returns((List<Album> albums, bool doRefresh, bool ignoreErrors) => albums);
} }
private void WithAlbum() private void WithAlbum()
@ -184,7 +184,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddArtistService>() Mocker.GetMock<IAddArtistService>()
.Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false)); .Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false, It.IsAny<bool>()));
} }
[Test] [Test]
@ -196,7 +196,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddArtistService>() Mocker.GetMock<IAddArtistService>()
.Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false)); .Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false, It.IsAny<bool>()));
} }
[Test] [Test]
@ -208,7 +208,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddAlbumService>() Mocker.GetMock<IAddAlbumService>()
.Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 1), false)); .Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 1), false, It.IsAny<bool>()));
} }
[TestCase(ImportListMonitorType.None, false)] [TestCase(ImportListMonitorType.None, false)]
@ -222,7 +222,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddArtistService>() Mocker.GetMock<IAddArtistService>()
.Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 1 && t.First().Monitored == expectedArtistMonitored), false)); .Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 1 && t.First().Monitored == expectedArtistMonitored), false, It.IsAny<bool>()));
} }
[TestCase(ImportListMonitorType.None, false)] [TestCase(ImportListMonitorType.None, false)]
@ -236,7 +236,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddAlbumService>() Mocker.GetMock<IAddAlbumService>()
.Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 1 && t.First().Monitored == expectedAlbumMonitored), false)); .Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 1 && t.First().Monitored == expectedAlbumMonitored), false, It.IsAny<bool>()));
} }
[Test] [Test]
@ -248,7 +248,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddArtistService>() Mocker.GetMock<IAddArtistService>()
.Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false)); .Verify(v => v.AddArtists(It.Is<List<Artist>>(t => t.Count == 0), false, It.IsAny<bool>()));
} }
[Test] [Test]
@ -260,7 +260,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddAlbumService>() Mocker.GetMock<IAddAlbumService>()
.Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 0), false)); .Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 0), false, It.IsAny<bool>()));
} }
[Test] [Test]
@ -273,7 +273,7 @@ namespace NzbDrone.Core.Test.ImportListTests
Subject.Execute(new ImportListSyncCommand()); Subject.Execute(new ImportListSyncCommand());
Mocker.GetMock<IAddAlbumService>() Mocker.GetMock<IAddAlbumService>()
.Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 0), false)); .Verify(v => v.AddAlbums(It.Is<List<Album>>(t => t.Count == 0), false, It.IsAny<bool>()));
} }
} }
} }

@ -117,8 +117,8 @@ namespace NzbDrone.Core.ImportLists
} }
} }
var addedArtists = _addArtistService.AddArtists(artistsToAdd, false); var addedArtists = _addArtistService.AddArtists(artistsToAdd, false, true);
var addedAlbums = _addAlbumService.AddAlbums(albumsToAdd, false); 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}"); var message = string.Format($"Import List Sync Completed. Items found: {reports.Count}, Artists added: {addedArtists.Count}, Albums added: {addedAlbums.Count}");

@ -13,7 +13,7 @@ namespace NzbDrone.Core.Music
public interface IAddAlbumService public interface IAddAlbumService
{ {
Album AddAlbum(Album album, bool doRefresh = true); Album AddAlbum(Album album, bool doRefresh = true);
List<Album> AddAlbums(List<Album> albums, bool doRefresh = true); List<Album> AddAlbums(List<Album> albums, bool doRefresh = true, bool ignoreErrors = false);
} }
public class AddAlbumService : IAddAlbumService public class AddAlbumService : IAddAlbumService
@ -77,15 +77,33 @@ namespace NzbDrone.Core.Music
return album; return album;
} }
public List<Album> AddAlbums(List<Album> albums, bool doRefresh = true) public List<Album> AddAlbums(List<Album> albums, bool doRefresh = true, bool ignoreErrors = false)
{ {
var added = DateTime.UtcNow; var added = DateTime.UtcNow;
var addedAlbums = new List<Album>(); var addedAlbums = new List<Album>();
foreach (var a in albums) foreach (var a in albums)
{ {
a.Added = added; try
addedAlbums.Add(AddAlbum(a, doRefresh)); {
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; return addedAlbums;

@ -17,7 +17,7 @@ namespace NzbDrone.Core.Music
public interface IAddArtistService public interface IAddArtistService
{ {
Artist AddArtist(Artist newArtist, bool doRefresh = true); Artist AddArtist(Artist newArtist, bool doRefresh = true);
List<Artist> AddArtists(List<Artist> newArtists, bool doRefresh = true); List<Artist> AddArtists(List<Artist> newArtists, bool doRefresh = true, bool ignoreErrors = false);
} }
public class AddArtistService : IAddArtistService public class AddArtistService : IAddArtistService
@ -63,7 +63,7 @@ namespace NzbDrone.Core.Music
return newArtist; return newArtist;
} }
public List<Artist> AddArtists(List<Artist> newArtists, bool doRefresh = true) public List<Artist> AddArtists(List<Artist> newArtists, bool doRefresh = true, bool ignoreErrors = false)
{ {
var added = DateTime.UtcNow; var added = DateTime.UtcNow;
var artistsToAdd = new List<Artist>(); var artistsToAdd = new List<Artist>();
@ -75,12 +75,23 @@ namespace NzbDrone.Core.Music
var artist = AddSkyhookData(s); var artist = AddSkyhookData(s);
artist = SetPropertiesAndValidate(artist); artist = SetPropertiesAndValidate(artist);
artist.Added = added; 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); artistsToAdd.Add(artist);
} }
catch (Exception ex) catch (ValidationException ex)
{ {
if (!ignoreErrors)
{
throw;
}
// Catch Import Errors for now until we get things fixed up // 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);
} }
} }

Loading…
Cancel
Save