From 368363de964f3b02ac4535fb6e730290aed275cb Mon Sep 17 00:00:00 2001 From: ta264 Date: Sun, 14 Jul 2019 21:05:55 +0100 Subject: [PATCH] Fixed: Prevent two Artists pointing to same ArtistMetadata --- ..._add_artistmetadataid_constraintFixture.cs | 113 ++++++++++++++++++ .../ArtistRepositoryFixture.cs | 37 +++++- .../NzbDrone.Core.Test.csproj | 6 +- .../031_add_artistmetadataid_constraint.cs | 25 ++++ src/NzbDrone.Core/NzbDrone.Core.csproj | 3 +- 5 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 src/NzbDrone.Core.Test/Datastore/Migration/031_add_artistmetadataid_constraintFixture.cs create mode 100644 src/NzbDrone.Core/Datastore/Migration/031_add_artistmetadataid_constraint.cs diff --git a/src/NzbDrone.Core.Test/Datastore/Migration/031_add_artistmetadataid_constraintFixture.cs b/src/NzbDrone.Core.Test/Datastore/Migration/031_add_artistmetadataid_constraintFixture.cs new file mode 100644 index 000000000..cb4943710 --- /dev/null +++ b/src/NzbDrone.Core.Test/Datastore/Migration/031_add_artistmetadataid_constraintFixture.cs @@ -0,0 +1,113 @@ +using NUnit.Framework; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Datastore.Migration; +using NzbDrone.Test.Common; +using System.Linq; +using FluentAssertions; +using System.Collections.Generic; + +namespace NzbDrone.Core.Test.Datastore.Migration +{ + [TestFixture] + public class add_artistmetadataid_constraintFixture : MigrationTest + { + private string _artistPath = null; + + private void GivenArtistMetadata(add_artistmetadataid_constraint c, int id, string name) + { + c.Insert.IntoTable("ArtistMetadata").Row(new + { + Id = id, + ForeignArtistId = id, + Name = name, + Status = 1, + Images = "images" + }); + } + + private void GivenArtist(add_artistmetadataid_constraint c, int id, int artistMetadataId, string name) + { + _artistPath = $"/mnt/data/path/{name}".AsOsAgnostic(); + c.Insert.IntoTable("Artists").Row(new + { + Id = id, + ArtistMetadataId = artistMetadataId, + CleanName = name, + Path = _artistPath, + Monitored = 1, + AlbumFolder = 1, + LanguageProfileId = 1, + MetadataProfileId = 1, + }); + } + + private void VerifyArtists(IDirectDataMapper db, List ids) + { + var artists = db.Query("SELECT Artists.* from Artists"); + + artists.Select(x => x["Id"]).ShouldBeEquivalentTo(ids); + + var duplicates = artists.GroupBy(x => x["ArtistMetadataId"]) + .Where(x => x.Count() > 1); + + duplicates.Should().BeEmpty(); + } + + [Test] + public void migration_031_should_not_remove_unique_artist() + { + var db = WithMigrationTestDb(c => { + GivenArtistMetadata(c, 1, "test"); + GivenArtist(c, 1, 1, "test"); + }); + + VerifyArtists(db, new List { 1 }); + } + + [Test] + public void migration_031_should_not_remove_either_unique_artist() + { + var db = WithMigrationTestDb(c => { + GivenArtistMetadata(c, 1, "test"); + GivenArtist(c, 1, 1, "test"); + + GivenArtistMetadata(c, 2, "test2"); + GivenArtist(c, 2, 2, "test2"); + }); + + VerifyArtists(db, new List { 1, 2 }); + } + + [Test] + public void migration_031_should_remove_duplicate_artist() + { + var db = WithMigrationTestDb(c => { + GivenArtistMetadata(c, 1, "test"); + GivenArtist(c, 1, 1, "test"); + + GivenArtist(c, 2, 1, "test2"); + }); + + VerifyArtists(db, new List { 1 }); + } + + [Test] + public void migration_031_should_remove_all_duplicate_artists() + { + var db = WithMigrationTestDb(c => { + GivenArtistMetadata(c, 1, "test"); + GivenArtist(c, 1, 1, "test"); + GivenArtist(c, 2, 1, "test"); + GivenArtist(c, 3, 1, "test"); + GivenArtist(c, 4, 1, "test"); + + GivenArtistMetadata(c, 2, "test2"); + GivenArtist(c, 5, 2, "test2"); + GivenArtist(c, 6, 2, "test2"); + + }); + + VerifyArtists(db, new List { 1, 5 }); + } + } +} diff --git a/src/NzbDrone.Core.Test/MusicTests/ArtistRepositoryTests/ArtistRepositoryFixture.cs b/src/NzbDrone.Core.Test/MusicTests/ArtistRepositoryTests/ArtistRepositoryFixture.cs index 0de888b97..b3af8b7a8 100644 --- a/src/NzbDrone.Core.Test/MusicTests/ArtistRepositoryTests/ArtistRepositoryFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/ArtistRepositoryTests/ArtistRepositoryFixture.cs @@ -10,6 +10,9 @@ using NzbDrone.Core.Test.Framework; using NzbDrone.Core.Music; using NzbDrone.Core.Profiles.Languages; using NzbDrone.Core.Profiles.Metadata; +using NzbDrone.Common.Extensions; +using System; +using System.Data.SQLite; namespace NzbDrone.Core.Test.MusicTests.ArtistRepositoryTests { @@ -21,6 +24,13 @@ namespace NzbDrone.Core.Test.MusicTests.ArtistRepositoryTests private ArtistMetadataRepository _artistMetadataRepo; private int _id = 1; + [SetUp] + public void Setup() + { + _artistRepo = Mocker.Resolve(); + _artistMetadataRepo = Mocker.Resolve(); + } + private void AddArtist(string name) { var metadata = Builder.CreateNew() @@ -42,8 +52,6 @@ namespace NzbDrone.Core.Test.MusicTests.ArtistRepositoryTests private void GivenArtists() { - _artistRepo = Mocker.Resolve(); - _artistMetadataRepo = Mocker.Resolve(); AddArtist("The Black Eyed Peas"); AddArtist("The Black Keys"); } @@ -118,5 +126,30 @@ namespace NzbDrone.Core.Test.MusicTests.ArtistRepositoryTests var artist = _artistRepo.FindByName(Parser.Parser.CleanArtistName(name)); artist.Should().BeNull(); } + + [Test] + public void should_throw_sql_exception_adding_duplicate_artist() + { + var name = "test"; + var metadata = Builder.CreateNew() + .With(a => a.Id = 0) + .With(a => a.Name = name) + .BuildNew(); + + var artist1 = Builder.CreateNew() + .With(a => a.Id = 0) + .With(a => a.Metadata = metadata) + .With(a => a.CleanName = Parser.Parser.CleanArtistName(name)) + .BuildNew(); + + var artist2 = artist1.JsonClone(); + artist2.Metadata = metadata; + + _artistMetadataRepo.Insert(metadata); + _artistRepo.Insert(artist1); + + Action insertDupe = () => _artistRepo.Insert(artist2); + insertDupe.ShouldThrow(); + } } } diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index 1fb26f1f3..183c46f2f 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -52,6 +52,9 @@ + + ..\Libraries\Sqlite\System.Data.SQLite.dll + @@ -79,6 +82,7 @@ + @@ -640,4 +644,4 @@ - \ No newline at end of file + diff --git a/src/NzbDrone.Core/Datastore/Migration/031_add_artistmetadataid_constraint.cs b/src/NzbDrone.Core/Datastore/Migration/031_add_artistmetadataid_constraint.cs new file mode 100644 index 000000000..8dfefa4b8 --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/031_add_artistmetadataid_constraint.cs @@ -0,0 +1,25 @@ +using FluentMigrator; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(31)] + public class add_artistmetadataid_constraint : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + // Remove any duplicate artists + Execute.Sql(@"DELETE FROM Artists + WHERE Id NOT IN ( + SELECT MIN(Artists.id) from Artists + JOIN ArtistMetadata ON Artists.ArtistMetadataId = ArtistMetadata.Id + GROUP BY ArtistMetadata.Id)"); + + // The index exists but will be recreated as part of unique constraint + Delete.Index().OnTable("Artists").OnColumn("ArtistMetadataId"); + + // Add a constraint to prevent any more duplicates + Alter.Column("ArtistMetadataId").OnTable("Artists").AsInt32().Unique(); + } + } +} diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 2bce75734..4913d1eb5 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -171,6 +171,7 @@ + @@ -1339,4 +1340,4 @@ --> - \ No newline at end of file +