New: Refactor metadata update

This commit is contained in:
ta264
2019-07-09 21:14:05 +01:00
parent f5c1858d4c
commit 0b7a42ee3b
19 changed files with 1352 additions and 729 deletions
@@ -11,6 +11,9 @@ using NzbDrone.Core.Test.Framework;
using NzbDrone.Core.Music;
using NzbDrone.Core.Music.Commands;
using NzbDrone.Test.Common;
using NzbDrone.Core.MediaFiles;
using NzbDrone.Core.History;
using NzbDrone.Core.Music.Events;
namespace NzbDrone.Core.Test.MusicTests
{
@@ -41,17 +44,24 @@ namespace NzbDrone.Core.Test.MusicTests
.With(a => a.Metadata = metadata)
.Build();
Mocker.GetMock<IArtistService>()
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.Setup(s => s.GetArtist(_artist.Id))
.Returns(_artist);
Mocker.GetMock<IAlbumService>()
.Setup(s => s.GetAlbumsForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
.Returns(new List<Album>());
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.Setup(s => s.InsertMany(It.IsAny<List<Album>>()));
Mocker.GetMock<IProvideArtistInfo>()
.Setup(s => s.GetArtistInfo(It.IsAny<string>(), It.IsAny<int>()))
.Callback(() => { throw new ArtistNotFoundException(_artist.ForeignArtistId); });
Mocker.GetMock<IMediaFileService>()
.Setup(x => x.GetFilesByArtist(It.IsAny<int>()))
.Returns(new List<TrackFile>());
Mocker.GetMock<IHistoryService>()
.Setup(x => x.GetByArtist(It.IsAny<int>(), It.IsAny<HistoryEventType?>()))
.Returns(new List<History.History>());
}
private void GivenNewArtistInfo(Artist artist)
@@ -60,81 +70,207 @@ namespace NzbDrone.Core.Test.MusicTests
.Setup(s => s.GetArtistInfo(_artist.ForeignArtistId, _artist.MetadataProfileId))
.Returns(artist);
}
private void GivenArtistFiles()
{
Mocker.GetMock<IMediaFileService>()
.Setup(x => x.GetFilesByArtist(It.IsAny<int>()))
.Returns(Builder<TrackFile>.CreateListOfSize(1).BuildList());
}
private void GivenAlbumsForRefresh()
{
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.Setup(s => s.GetAlbumsForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
.Returns(new List<Album>());
}
private void AllowArtistUpdate()
{
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.Setup(x => x.UpdateArtist(It.IsAny<Artist>()))
.Returns((Artist a) => a);
}
[Test]
public void should_log_error_if_musicbrainz_id_not_found()
public void should_not_publish_artist_updated_event_if_metadata_not_updated()
{
var newArtistInfo = _artist.JsonClone();
newArtistInfo.Metadata = _artist.Metadata.Value.JsonClone();
newArtistInfo.Albums = _albums;
GivenNewArtistInfo(newArtistInfo);
GivenAlbumsForRefresh();
AllowArtistUpdate();
Subject.Execute(new RefreshArtistCommand(_artist.Id));
VerifyEventNotPublished<ArtistUpdatedEvent>();
}
[Test]
public void should_publish_artist_updated_event_if_metadata_updated()
{
var newArtistInfo = _artist.JsonClone();
newArtistInfo.Metadata = _artist.Metadata.Value.JsonClone();
newArtistInfo.Metadata.Value.Images = new List<MediaCover.MediaCover> {
new MediaCover.MediaCover(MediaCover.MediaCoverTypes.Logo, "dummy")
};
newArtistInfo.Albums = _albums;
GivenNewArtistInfo(newArtistInfo);
GivenAlbumsForRefresh();
AllowArtistUpdate();
Subject.Execute(new RefreshArtistCommand(_artist.Id));
VerifyEventPublished<ArtistUpdatedEvent>();
}
[Test]
public void should_log_error_and_delete_if_musicbrainz_id_not_found_and_artist_has_no_files()
{
Mocker.GetMock<IArtistService>()
.Setup(x => x.DeleteArtist(It.IsAny<int>(), It.IsAny<bool>(), It.IsAny<bool>()));
Subject.Execute(new RefreshArtistCommand(_artist.Id));
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.IsAny<Artist>()), Times.Never());
Mocker.GetMock<IArtistService>()
.Verify(v => v.DeleteArtist(It.IsAny<int>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Once());
ExceptionVerification.ExpectedErrors(1);
ExceptionVerification.ExpectedWarns(1);
}
[Test]
public void should_log_error_but_not_delete_if_musicbrainz_id_not_found_and_artist_has_files()
{
GivenArtistFiles();
GivenAlbumsForRefresh();
Subject.Execute(new RefreshArtistCommand(_artist.Id));
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.IsAny<Artist>()), Times.Never());
Mocker.GetMock<IArtistService>()
.Verify(v => v.DeleteArtist(It.IsAny<int>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Never());
ExceptionVerification.ExpectedErrors(2);
}
[Test]
public void should_update_if_musicbrainz_id_changed()
public void should_update_if_musicbrainz_id_changed_and_no_clash()
{
var newArtistInfo = _artist.JsonClone();
newArtistInfo.Metadata = _artist.Metadata.Value.JsonClone();
newArtistInfo.Albums = _albums;
newArtistInfo.ForeignArtistId = _artist.ForeignArtistId + 1;
newArtistInfo.Metadata.Value.Id = 100;
GivenNewArtistInfo(newArtistInfo);
var seq = new MockSequence();
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.Setup(x => x.FindById(newArtistInfo.ForeignArtistId))
.Returns(default(Artist));
// Make sure that the artist is updated before we refresh the albums
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.UpdateArtist(It.IsAny<Artist>()))
.Returns((Artist a) => a);
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.GetAlbumsForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
.Returns(new List<Album>());
// Update called twice for a move/merge
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.UpdateArtist(It.IsAny<Artist>()))
.Returns((Artist a) => a);
Subject.Execute(new RefreshArtistCommand(_artist.Id));
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.Is<Artist>(s => s.ForeignArtistId == newArtistInfo.ForeignArtistId)));
.Verify(v => v.UpdateArtist(It.Is<Artist>(s => s.ArtistMetadataId == 100 && s.ForeignArtistId == newArtistInfo.ForeignArtistId)),
Times.Exactly(2));
}
[Test]
public void should_merge_if_musicbrainz_id_changed_and_new_id_already_exists()
{
var existing = _artist;
var clash = _artist.JsonClone();
clash.Id = 100;
clash.Metadata = existing.Metadata.Value.JsonClone();
clash.Metadata.Value.Id = 101;
clash.Metadata.Value.ForeignArtistId = clash.Metadata.Value.ForeignArtistId + 1;
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.Setup(x => x.FindById(clash.Metadata.Value.ForeignArtistId))
.Returns(clash);
var newArtistInfo = clash.JsonClone();
newArtistInfo.Metadata = clash.Metadata.Value.JsonClone();
newArtistInfo.Albums = _albums.JsonClone();
newArtistInfo.Albums.Value.ForEach(x => x.Id = 0);
GivenNewArtistInfo(newArtistInfo);
var seq = new MockSequence();
// Make sure that the artist is updated before we refresh the albums
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.GetAlbumsByArtist(existing.Id))
.Returns(_albums);
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.UpdateMany(It.IsAny<List<Album>>()));
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.DeleteArtist(existing.Id, It.IsAny<bool>(), false));
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.UpdateArtist(It.Is<Artist>(a => a.Id == clash.Id)))
.Returns((Artist a) => a);
Mocker.GetMock<IAlbumService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.GetAlbumsForRefresh(clash.ArtistMetadataId, It.IsAny<IEnumerable<string>>()))
.Returns(_albums);
// Update called twice for a move/merge
Mocker.GetMock<IArtistService>(MockBehavior.Strict)
.InSequence(seq)
.Setup(x => x.UpdateArtist(It.IsAny<Artist>()))
.Returns((Artist a) => a);
Subject.Execute(new RefreshArtistCommand(_artist.Id));
// the retained artist gets updated
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.Is<Artist>(s => s.Id == clash.Id)), Times.Exactly(2));
// the old one gets removed
Mocker.GetMock<IArtistService>()
.Verify(v => v.DeleteArtist(existing.Id, false, false));
Mocker.GetMock<IAlbumService>()
.Verify(v => v.UpdateMany(It.Is<List<Album>>(x => x.Count == _albums.Count)));
ExceptionVerification.ExpectedWarns(1);
}
[Test]
[Ignore("This test needs to be re-written as we no longer store albums in artist table or object")]
public void should_not_throw_if_duplicate_album_is_in_existing_info()
{
var newArtistInfo = _artist.JsonClone();
newArtistInfo.Albums.Value.Add(Builder<Album>.CreateNew()
.With(s => s.ForeignAlbumId = "2")
.Build());
_artist.Albums.Value.Add(Builder<Album>.CreateNew()
.With(s => s.ForeignAlbumId = "2")
.Build());
_artist.Albums.Value.Add(Builder<Album>.CreateNew()
.With(s => s.ForeignAlbumId = "2")
.Build());
GivenNewArtistInfo(newArtistInfo);
Subject.Execute(new RefreshArtistCommand(_artist.Id));
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.Is<Artist>(s => s.Albums.Value.Count == 2)));
}
[Test]
[Ignore("This test needs to be re-written as we no longer store albums in artist table or object")]
public void should_filter_duplicate_albums()
{
var newArtistInfo = _artist.JsonClone();
newArtistInfo.Albums.Value.Add(Builder<Album>.CreateNew()
.With(s => s.ForeignAlbumId = "2")
.Build());
newArtistInfo.Albums.Value.Add(Builder<Album>.CreateNew()
.With(s => s.ForeignAlbumId = "2")
.Build());
GivenNewArtistInfo(newArtistInfo);
Subject.Execute(new RefreshArtistCommand(_artist.Id));
Mocker.GetMock<IArtistService>()
.Verify(v => v.UpdateArtist(It.Is<Artist>(s => s.Albums.Value.Count == 2)));
}
}
}