Simplify entity equality code and enfore db/metadata split Use a nuget package to remove boilerplate code that needs careful update when adding/removing fields. Add tests to enforce that all fields are allocated to 'UseMetadataFrom' or 'UseDbFieldsFrom' to make metadata refresh more foolproof. Fix NRE when tracks are merged because artist wasn't set. Fix NRE when tracks are merged and the merge target wasn't yet in the database.pull/948/head
ta264
5 years ago
parent
c4578c0b0f
commit
2097bfff94
@ -0,0 +1,298 @@
|
||||
using NUnit.Framework;
|
||||
using NzbDrone.Common.Extensions;
|
||||
using NzbDrone.Core.Music;
|
||||
using NzbDrone.Test.Common;
|
||||
using FluentAssertions;
|
||||
using System.Collections;
|
||||
using System.Reflection;
|
||||
using AutoFixture;
|
||||
using System.Linq;
|
||||
using Equ;
|
||||
using Marr.Data;
|
||||
|
||||
namespace NzbDrone.Core.Test.MusicTests
|
||||
{
|
||||
[TestFixture]
|
||||
public class EntityFixture : LoggingTest
|
||||
{
|
||||
|
||||
Fixture fixture = new Fixture();
|
||||
|
||||
private static bool IsNotMarkedAsIgnore(PropertyInfo propertyInfo)
|
||||
{
|
||||
return !propertyInfo.GetCustomAttributes(typeof(MemberwiseEqualityIgnoreAttribute), true).Any();
|
||||
}
|
||||
|
||||
public class EqualityPropertySource<T>
|
||||
{
|
||||
public static IEnumerable TestCases
|
||||
{
|
||||
get
|
||||
{
|
||||
foreach (var property in typeof(T).GetProperties().Where(x => x.CanRead && x.CanWrite && IsNotMarkedAsIgnore(x)))
|
||||
{
|
||||
yield return new TestCaseData(property).SetName($"{{m}}_{property.Name}");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public class IgnoredPropertySource<T>
|
||||
{
|
||||
public static IEnumerable TestCases
|
||||
{
|
||||
get
|
||||
{
|
||||
foreach (var property in typeof(T).GetProperties().Where(x => x.CanRead && x.CanWrite && !IsNotMarkedAsIgnore(x)))
|
||||
{
|
||||
yield return new TestCaseData(property).SetName($"{{m}}_{property.Name}");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void two_equivalent_artist_metadata_should_be_equal()
|
||||
{
|
||||
var item1 = fixture.Create<ArtistMetadata>();
|
||||
var item2 = item1.JsonClone();
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
[Test, TestCaseSource(typeof(EqualityPropertySource<ArtistMetadata>), "TestCases")]
|
||||
public void two_different_artist_metadata_should_not_be_equal(PropertyInfo prop)
|
||||
{
|
||||
var item1 = fixture.Create<ArtistMetadata>();
|
||||
var item2 = item1.JsonClone();
|
||||
var different = fixture.Create<ArtistMetadata>();
|
||||
|
||||
// make item2 different in the property under consideration
|
||||
var differentEntry = prop.GetValue(different);
|
||||
prop.SetValue(item2, differentEntry);
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().NotBe(item2);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void metadata_and_db_fields_should_replicate_artist_metadata()
|
||||
{
|
||||
var item1 = fixture.Create<ArtistMetadata>();
|
||||
var item2 = fixture.Create<ArtistMetadata>();
|
||||
|
||||
item1.Should().NotBe(item2);
|
||||
|
||||
item1.UseMetadataFrom(item2);
|
||||
item1.UseDbFieldsFrom(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
private Track GivenTrack()
|
||||
{
|
||||
return fixture.Build<Track>()
|
||||
.Without(x => x.AlbumRelease)
|
||||
.Without(x => x.ArtistMetadata)
|
||||
.Without(x => x.TrackFile)
|
||||
.Without(x => x.Artist)
|
||||
.Without(x => x.AlbumId)
|
||||
.Without(x => x.Album)
|
||||
.Create();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void two_equivalent_track_should_be_equal()
|
||||
{
|
||||
var item1 = GivenTrack();
|
||||
var item2 = item1.JsonClone();
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
[Test, TestCaseSource(typeof(EqualityPropertySource<Track>), "TestCases")]
|
||||
public void two_different_tracks_should_not_be_equal(PropertyInfo prop)
|
||||
{
|
||||
var item1 = GivenTrack();
|
||||
var item2 = item1.JsonClone();
|
||||
var different = GivenTrack();
|
||||
|
||||
// make item2 different in the property under consideration
|
||||
var differentEntry = prop.GetValue(different);
|
||||
prop.SetValue(item2, differentEntry);
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().NotBe(item2);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void metadata_and_db_fields_should_replicate_track()
|
||||
{
|
||||
var item1 = GivenTrack();
|
||||
var item2 = GivenTrack();
|
||||
|
||||
item1.Should().NotBe(item2);
|
||||
|
||||
item1.UseMetadataFrom(item2);
|
||||
item1.UseDbFieldsFrom(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
private AlbumRelease GivenAlbumRelease()
|
||||
{
|
||||
return fixture.Build<AlbumRelease>()
|
||||
.Without(x => x.Album)
|
||||
.Without(x => x.Tracks)
|
||||
.Create();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void two_equivalent_album_releases_should_be_equal()
|
||||
{
|
||||
var item1 = GivenAlbumRelease();
|
||||
var item2 = item1.JsonClone();
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
[Test, TestCaseSource(typeof(EqualityPropertySource<AlbumRelease>), "TestCases")]
|
||||
public void two_different_album_releases_should_not_be_equal(PropertyInfo prop)
|
||||
{
|
||||
var item1 = GivenAlbumRelease();
|
||||
var item2 = item1.JsonClone();
|
||||
var different = GivenAlbumRelease();
|
||||
|
||||
// make item2 different in the property under consideration
|
||||
var differentEntry = prop.GetValue(different);
|
||||
prop.SetValue(item2, differentEntry);
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().NotBe(item2);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void metadata_and_db_fields_should_replicate_release()
|
||||
{
|
||||
var item1 = GivenAlbumRelease();
|
||||
var item2 = GivenAlbumRelease();
|
||||
|
||||
item1.Should().NotBe(item2);
|
||||
|
||||
item1.UseMetadataFrom(item2);
|
||||
item1.UseDbFieldsFrom(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
private Album GivenAlbum()
|
||||
{
|
||||
return fixture.Build<Album>()
|
||||
.Without(x => x.ArtistMetadata)
|
||||
.Without(x => x.AlbumReleases)
|
||||
.Without(x => x.Artist)
|
||||
.Without(x => x.ArtistId)
|
||||
.Create();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void two_equivalent_albums_should_be_equal()
|
||||
{
|
||||
var item1 = GivenAlbum();
|
||||
var item2 = item1.JsonClone();
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
[Test, TestCaseSource(typeof(EqualityPropertySource<Album>), "TestCases")]
|
||||
public void two_different_albums_should_not_be_equal(PropertyInfo prop)
|
||||
{
|
||||
var item1 = GivenAlbum();
|
||||
var item2 = item1.JsonClone();
|
||||
var different = GivenAlbum();
|
||||
|
||||
// make item2 different in the property under consideration
|
||||
if (prop.PropertyType == typeof(bool))
|
||||
{
|
||||
prop.SetValue(item2, !(bool)prop.GetValue(item1));
|
||||
}
|
||||
else
|
||||
{
|
||||
prop.SetValue(item2, prop.GetValue(different));
|
||||
}
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().NotBe(item2);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void metadata_and_db_fields_should_replicate_album()
|
||||
{
|
||||
var item1 = GivenAlbum();
|
||||
var item2 = GivenAlbum();
|
||||
|
||||
item1.Should().NotBe(item2);
|
||||
|
||||
item1.UseMetadataFrom(item2);
|
||||
item1.UseDbFieldsFrom(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
private Artist GivenArtist()
|
||||
{
|
||||
return fixture.Build<Artist>()
|
||||
.With(x => x.Metadata, new LazyLoaded<ArtistMetadata>(fixture.Create<ArtistMetadata>()))
|
||||
.Without(x => x.QualityProfile)
|
||||
.Without(x => x.MetadataProfile)
|
||||
.Without(x => x.Albums)
|
||||
.Without(x => x.Name)
|
||||
.Without(x => x.ForeignArtistId)
|
||||
.Create();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void two_equivalent_artists_should_be_equal()
|
||||
{
|
||||
var item1 = GivenArtist();
|
||||
var item2 = item1.JsonClone();
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
|
||||
[Test, TestCaseSource(typeof(EqualityPropertySource<Artist>), "TestCases")]
|
||||
public void two_different_artists_should_not_be_equal(PropertyInfo prop)
|
||||
{
|
||||
var item1 = GivenArtist();
|
||||
var item2 = item1.JsonClone();
|
||||
var different = GivenArtist();
|
||||
|
||||
// make item2 different in the property under consideration
|
||||
if (prop.PropertyType == typeof(bool))
|
||||
{
|
||||
prop.SetValue(item2, !(bool)prop.GetValue(item1));
|
||||
}
|
||||
else
|
||||
{
|
||||
prop.SetValue(item2, prop.GetValue(different));
|
||||
}
|
||||
|
||||
item1.Should().NotBeSameAs(item2);
|
||||
item1.Should().NotBe(item2);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void metadata_and_db_fields_should_replicate_artist()
|
||||
{
|
||||
var item1 = GivenArtist();
|
||||
var item2 = GivenArtist();
|
||||
|
||||
item1.Should().NotBe(item2);
|
||||
|
||||
item1.UseMetadataFrom(item2);
|
||||
item1.UseDbFieldsFrom(item2);
|
||||
item1.Should().Be(item2);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,147 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using FizzWare.NBuilder;
|
||||
using Moq;
|
||||
using NUnit.Framework;
|
||||
using NzbDrone.Common.Extensions;
|
||||
using NzbDrone.Core.Exceptions;
|
||||
using NzbDrone.Core.MetadataSource;
|
||||
using NzbDrone.Core.Test.Framework;
|
||||
using NzbDrone.Core.Music;
|
||||
using NzbDrone.Test.Common;
|
||||
using NzbDrone.Core.MediaFiles;
|
||||
using NzbDrone.Core.History;
|
||||
|
||||
namespace NzbDrone.Core.Test.MusicTests
|
||||
{
|
||||
[TestFixture]
|
||||
public class RefreshAlbumReleaseServiceFixture : CoreTest<RefreshAlbumReleaseService>
|
||||
{
|
||||
private AlbumRelease _release;
|
||||
private List<Track> _tracks;
|
||||
private ArtistMetadata _metadata;
|
||||
|
||||
[SetUp]
|
||||
public void Setup()
|
||||
{
|
||||
|
||||
_release = Builder<AlbumRelease>
|
||||
.CreateNew()
|
||||
.With(s => s.Media = new List<Medium> { new Medium { Number = 1 } })
|
||||
.With(s => s.ForeignReleaseId = "xxx-xxx-xxx-xxx")
|
||||
.With(s => s.Monitored = true)
|
||||
.With(s => s.TrackCount = 10)
|
||||
.Build();
|
||||
|
||||
_metadata = Builder<ArtistMetadata>.CreateNew().Build();
|
||||
|
||||
_tracks = Builder<Track>
|
||||
.CreateListOfSize(10)
|
||||
.All()
|
||||
.With(x => x.AlbumReleaseId = _release.Id)
|
||||
.With(x => x.ArtistMetadata = _metadata)
|
||||
.With(x => x.ArtistMetadataId = _metadata.Id)
|
||||
.BuildList();
|
||||
|
||||
Mocker.GetMock<ITrackService>()
|
||||
.Setup(s => s.GetTracksForRefresh(_release.Id, It.IsAny<IEnumerable<string>>()))
|
||||
.Returns(_tracks);
|
||||
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void should_update_if_musicbrainz_id_changed_and_no_clash()
|
||||
{
|
||||
var newInfo = _release.JsonClone();
|
||||
newInfo.ForeignReleaseId = _release.ForeignReleaseId + 1;
|
||||
newInfo.OldForeignReleaseIds = new List<string> { _release.ForeignReleaseId };
|
||||
newInfo.Tracks = _tracks;
|
||||
|
||||
Subject.RefreshEntityInfo(_release, new List<AlbumRelease> { newInfo }, false, false);
|
||||
|
||||
Mocker.GetMock<IReleaseService>()
|
||||
.Verify(v => v.UpdateMany(It.Is<List<AlbumRelease>>(s => s.First().ForeignReleaseId == newInfo.ForeignReleaseId)));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void should_merge_if_musicbrainz_id_changed_and_new_already_exists()
|
||||
{
|
||||
var existing = _release;
|
||||
|
||||
var clash = existing.JsonClone();
|
||||
clash.Id = 100;
|
||||
clash.ForeignReleaseId = clash.ForeignReleaseId + 1;
|
||||
|
||||
clash.Tracks = Builder<Track>.CreateListOfSize(10)
|
||||
.All()
|
||||
.With(x => x.AlbumReleaseId = clash.Id)
|
||||
.With(x => x.ArtistMetadata = _metadata)
|
||||
.With(x => x.ArtistMetadataId = _metadata.Id)
|
||||
.BuildList();
|
||||
|
||||
Mocker.GetMock<IReleaseService>()
|
||||
.Setup(x => x.GetReleaseByForeignReleaseId(clash.ForeignReleaseId, false))
|
||||
.Returns(clash);
|
||||
|
||||
Mocker.GetMock<ITrackService>()
|
||||
.Setup(x => x.GetTracksForRefresh(It.IsAny<int>(), It.IsAny<IEnumerable<string>>()))
|
||||
.Returns(_tracks);
|
||||
|
||||
var newInfo = existing.JsonClone();
|
||||
newInfo.ForeignReleaseId = _release.ForeignReleaseId + 1;
|
||||
newInfo.OldForeignReleaseIds = new List<string> { _release.ForeignReleaseId };
|
||||
newInfo.Tracks = _tracks;
|
||||
|
||||
Subject.RefreshEntityInfo(new List<AlbumRelease> { clash, existing }, new List<AlbumRelease> { newInfo }, false, false);
|
||||
|
||||
// check old album is deleted
|
||||
Mocker.GetMock<IReleaseService>()
|
||||
.Verify(v => v.DeleteMany(It.Is<List<AlbumRelease>>(x => x.First().ForeignReleaseId == existing.ForeignReleaseId)));
|
||||
|
||||
// check that clash gets updated
|
||||
Mocker.GetMock<IReleaseService>()
|
||||
.Verify(v => v.UpdateMany(It.Is<List<AlbumRelease>>(s => s.First().ForeignReleaseId == newInfo.ForeignReleaseId)));
|
||||
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void child_merge_targets_should_not_be_null_if_target_is_new()
|
||||
{
|
||||
var oldTrack = Builder<Track>
|
||||
.CreateNew()
|
||||
.With(x => x.AlbumReleaseId = _release.Id)
|
||||
.With(x => x.ArtistMetadata = _metadata)
|
||||
.With(x => x.ArtistMetadataId = _metadata.Id)
|
||||
.Build();
|
||||
_release.Tracks = new List<Track> { oldTrack };
|
||||
|
||||
var newInfo = _release.JsonClone();
|
||||
var newTrack = oldTrack.JsonClone();
|
||||
newTrack.ArtistMetadata = _metadata;
|
||||
newTrack.ArtistMetadataId = _metadata.Id;
|
||||
newTrack.ForeignTrackId = "new id";
|
||||
newTrack.OldForeignTrackIds = new List<string> { oldTrack.ForeignTrackId };
|
||||
newInfo.Tracks = new List<Track> { newTrack };
|
||||
|
||||
Mocker.GetMock<ITrackService>()
|
||||
.Setup(s => s.GetTracksForRefresh(_release.Id, It.IsAny<IEnumerable<string>>()))
|
||||
.Returns(new List<Track> { oldTrack });
|
||||
|
||||
Subject.RefreshEntityInfo(_release, new List<AlbumRelease> { newInfo }, false, false);
|
||||
|
||||
Mocker.GetMock<IRefreshTrackService>()
|
||||
.Verify(v => v.RefreshTrackInfo(It.IsAny<List<Track>>(),
|
||||
It.IsAny<List<Track>>(),
|
||||
It.Is<List<Tuple<Track, Track>>>(x => x.All(y => y.Item2 != null)),
|
||||
It.IsAny<List<Track>>(),
|
||||
It.IsAny<List<Track>>(),
|
||||
It.IsAny<List<Track>>(),
|
||||
It.IsAny<bool>()));
|
||||
|
||||
Mocker.GetMock<IReleaseService>()
|
||||
.Verify(v => v.UpdateMany(It.Is<List<AlbumRelease>>(s => s.First().ForeignReleaseId == newInfo.ForeignReleaseId)));
|
||||
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,55 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using FizzWare.NBuilder;
|
||||
using Moq;
|
||||
using NUnit.Framework;
|
||||
using NzbDrone.Common.Extensions;
|
||||
using NzbDrone.Core.Test.Framework;
|
||||
using NzbDrone.Core.Music;
|
||||
using NzbDrone.Core.MediaFiles;
|
||||
|
||||
namespace NzbDrone.Core.Test.MusicTests
|
||||
{
|
||||
[TestFixture]
|
||||
public class RefreshTrackServiceFixture : CoreTest<RefreshTrackService>
|
||||
{
|
||||
private AlbumRelease _release;
|
||||
private List<Track> _allTracks;
|
||||
|
||||
[SetUp]
|
||||
public void Setup()
|
||||
{
|
||||
_release = Builder<AlbumRelease>.CreateNew().Build();
|
||||
_allTracks = Builder<Track>.CreateListOfSize(20)
|
||||
.All()
|
||||
.BuildList();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void updated_track_should_not_have_null_album_release()
|
||||
{
|
||||
var add = new List<Track>();
|
||||
var update = new List<Track>();
|
||||
var merge = new List<Tuple<Track, Track>>();
|
||||
var delete = new List<Track>();
|
||||
var upToDate = new List<Track>();
|
||||
|
||||
upToDate.AddRange(_allTracks.Take(10));
|
||||
|
||||
var toUpdate = _allTracks[10].JsonClone();
|
||||
toUpdate.Title = "title to update";
|
||||
toUpdate.AlbumRelease = _release;
|
||||
|
||||
update.Add(toUpdate);
|
||||
|
||||
Subject.RefreshTrackInfo(add, update, merge, delete, upToDate, _allTracks, false);
|
||||
|
||||
Mocker.GetMock<IAudioTagService>()
|
||||
.Verify(v => v.SyncTags(It.Is<List<Track>>(x => x.Count == 1 &&
|
||||
x[0].AlbumRelease != null &&
|
||||
x[0].AlbumRelease.IsLoaded == true)));
|
||||
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,37 @@
|
||||
using NzbDrone.Core.Datastore;
|
||||
using System;
|
||||
using Equ;
|
||||
|
||||
namespace NzbDrone.Core.Music
|
||||
{
|
||||
public abstract class Entity<T> : ModelBase, IEquatable<T>
|
||||
where T : Entity<T>
|
||||
{
|
||||
private static readonly MemberwiseEqualityComparer<T> _comparer =
|
||||
MemberwiseEqualityComparer<T>.ByProperties;
|
||||
|
||||
public virtual void UseDbFieldsFrom(T other)
|
||||
{
|
||||
Id = other.Id;
|
||||
}
|
||||
|
||||
public virtual void UseMetadataFrom(T other) { }
|
||||
|
||||
public virtual void ApplyChanges(T other) { }
|
||||
|
||||
public bool Equals(T other)
|
||||
{
|
||||
return _comparer.Equals(this as T, other);
|
||||
}
|
||||
|
||||
public override bool Equals(object obj)
|
||||
{
|
||||
return Equals(obj as T);
|
||||
}
|
||||
|
||||
public override int GetHashCode()
|
||||
{
|
||||
return _comparer.GetHashCode(this as T);
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in new issue