Skip to content

Commit

Permalink
Fixed: Prevent two Artists pointing to same ArtistMetadata
Browse files Browse the repository at this point in the history
  • Loading branch information
ta264 committed Jul 20, 2019
1 parent 531447a commit 368363d
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 4 deletions.
@@ -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 });
}
}
}
Expand Up @@ -10,6 +10,9 @@
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
{
Expand All @@ -21,6 +24,13 @@ public class ArtistRepositoryFixture : DbTest<ArtistRepository, Artist>
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()
Expand All @@ -42,8 +52,6 @@ private void AddArtist(string name)

private void GivenArtists()
{
_artistRepo = Mocker.Resolve<ArtistRepository>();
_artistMetadataRepo = Mocker.Resolve<ArtistMetadataRepository>();
AddArtist("The Black Eyed Peas");
AddArtist("The Black Keys");
}
Expand Down Expand Up @@ -118,5 +126,30 @@ public void should_not_find_artist_if_multiple_artists_have_same_name()
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>();
}
}
}
6 changes: 5 additions & 1 deletion src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj
Expand Up @@ -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" />
Expand Down Expand Up @@ -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" />
Expand Down Expand Up @@ -640,4 +644,4 @@
</ItemGroup>
<Copy SourceFiles="@(IdentificationTestCases)" DestinationFolder="$(OutputPath)\Files\Identification\" SkipUnchangedFiles="true" />
</Target>
</Project>
</Project>
@@ -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();
}
}
}
3 changes: 2 additions & 1 deletion src/NzbDrone.Core/NzbDrone.Core.csproj
Expand Up @@ -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" />
Expand Down Expand Up @@ -1339,4 +1340,4 @@
<Target Name="AfterBuild">
</Target>
-->
</Project>
</Project>

0 comments on commit 368363d

Please sign in to comment.