Fixed: Prevent two Artists pointing to same ArtistMetadata

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<add_artistmetadataid_constraint>
+    {
+        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<int> 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<int> { 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<int> { 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<int> { 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<int> { 1, 5 });
+        }
+    }
+}

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<ArtistRepository>();
+            _artistMetadataRepo = Mocker.Resolve<ArtistMetadataRepository>();
+        }
+
         private void AddArtist(string name)
         {
             var metadata = Builder<ArtistMetadata>.CreateNew()
@@ -42,8 +52,6 @@ namespace NzbDrone.Core.Test.MusicTests.ArtistRepositoryTests
 
         private void GivenArtists()
         {
-            _artistRepo = Mocker.Resolve<ArtistRepository>();
-            _artistMetadataRepo = Mocker.Resolve<ArtistMetadataRepository>();
             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<ArtistMetadata>.CreateNew()
+                .With(a => a.Id = 0)
+                .With(a => a.Name = name)
+                .BuildNew();
+            
+            var artist1 = Builder<Artist>.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<SQLiteException>();
+        }
     }
 }

src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj

@@ -52,6 +52,9 @@
     <Reference Include="System.Xml" />
     <Reference Include="System.Xml.Linq" />
     <Reference Include="Microsoft.CSharp" />
+    <Reference Include="System.Data.SQLite">
+      <HintPath>..\Libraries\Sqlite\System.Data.SQLite.dll</HintPath>
+    </Reference>
   </ItemGroup>
   <ItemGroup>
     <Compile Include="ArtistStatsTests\ArtistStatisticsFixture.cs" />
@@ -79,6 +82,7 @@
     <Compile Include="Datastore\Migration\004_add_various_qualities_in_profileFixture.cs" />
     <Compile Include="Datastore\Migration\023_add_release_groups_etcFixture.cs" />
     <Compile Include="Datastore\Migration\030_add_mediafilerepository_mtimeFixture.cs" />
+    <Compile Include="Datastore\Migration\031_add_artistmetadataid_constraintFixture.cs" />
     <Compile Include="Datastore\ObjectDatabaseFixture.cs" />
     <Compile Include="Datastore\PagingSpecExtensionsTests\PagingOffsetFixture.cs" />
     <Compile Include="Datastore\PagingSpecExtensionsTests\ToSortDirectionFixture.cs" />

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();
+        }
+    }
+}

src/NzbDrone.Core/NzbDrone.Core.csproj

@@ -171,6 +171,7 @@
     <Compile Include="Datastore\Migration\026_rename_quality_profiles_add_upgrade_allowed.cs" />
     <Compile Include="Datastore\Migration\028_clean_artistmetadata_table.cs" />
     <Compile Include="Datastore\Migration\030_add_mediafilerepository_mtime.cs" />
+    <Compile Include="Datastore\Migration\031_add_artistmetadataid_constraint.cs" />
     <Compile Include="Datastore\Migration\Framework\MigrationContext.cs" />
     <Compile Include="Datastore\Migration\Framework\MigrationController.cs" />
     <Compile Include="Datastore\Migration\Framework\MigrationDbFactory.cs" />