Fix more issues with migration 23 (#577)

* Tests for migration 23

* Make the migration more robust

For duplicate foreign Ids, create dummy id so correlated subquery
doesn't fail.

If we can't parse the release from the database, put in a dummy release.

* Be more careful with metadata refreshes

Make sure we deal with items that are not linked to the correct parent
This commit is contained in:
ta264 2019-01-12 16:56:13 +00:00 committed by Qstick
parent 1eea545c0a
commit 9185223f53
12 changed files with 356 additions and 32 deletions

View File

@ -0,0 +1,236 @@
using System.Linq;
using FluentAssertions;
using FizzWare.NBuilder;
using NUnit.Framework;
using NzbDrone.Core.Datastore.Migration;
using NzbDrone.Core.Music;
using NzbDrone.Core.Test.Framework;
using NzbDrone.Common.Serializer;
using System.Collections.Generic;
namespace NzbDrone.Core.Test.Datastore.Migration
{
[TestFixture]
public class add_release_groups_etcFixture : MigrationTest<add_release_groups_etc>
{
private void GivenArtist(add_release_groups_etc c, int id, string name)
{
c.Insert.IntoTable("Artists").Row(new
{
Id = id,
ForeignArtistId = id.ToString(),
Name = name,
CleanName = name,
Status = 1,
Images = "",
Path = $"/mnt/data/path/{name}",
Monitored = 1,
AlbumFolder = 1,
LanguageProfileId = 1,
MetadataProfileId = 1
});
}
private void GivenAlbum(add_release_groups_etc c, int id, int artistId, string title, string currentRelease)
{
c.Insert.IntoTable("Albums").Row(new
{
Id = id,
ForeignAlbumId = id.ToString(),
ArtistId = artistId,
Title = title,
CleanTitle = title,
Images = "",
Monitored = 1,
AlbumType = "Studio",
Duration = 100,
Media = "",
Releases = "",
CurrentRelease = currentRelease
});
}
private void GivenTracks(add_release_groups_etc c, int artistid, int albumid, int firstId, int count)
{
for (int i = 0; i < count; i++)
{
var id = firstId + i;
c.Insert.IntoTable("Tracks").Row(new
{
Id = id,
ForeignTrackId = id.ToString(),
ArtistId = artistid,
AlbumId = albumid,
Explicit = 0,
Compilation = 0,
Monitored = 0,
Duration = 100,
MediumNumber = 1,
AbsoluteTrackNumber = i,
TrackNumber = i.ToString()
});
}
}
private IEnumerable<AlbumRelease> VerifyAlbumReleases(IDirectDataMapper db)
{
var releases = db.Query<AlbumRelease>("SELECT * FROM AlbumReleases");
var albums = db.Query<Album>("SELECT * FROM Albums");
// we only put in one release per album
releases.Count().Should().Be(albums.Count());
// each album should be linked to exactly one release
releases.Select(x => x.AlbumId).SequenceEqual(albums.Select(x => x.Id)).Should().Be(true);
// each release should have at least one medium
releases.Select(x => x.Media.Count).Min().Should().BeGreaterOrEqualTo(1);
return releases;
}
private void VerifyTracks(IDirectDataMapper db, int albumId, int albumReleaseId, int expectedCount)
{
var tracks = db.Query<Track>("SELECT Tracks.* FROM Tracks " +
"JOIN AlbumReleases ON Tracks.AlbumReleaseId = AlbumReleases.Id " +
"JOIN Albums ON AlbumReleases.AlbumId = Albums.Id " +
"WHERE Albums.Id = " + albumId).ToList();
tracks.Count.Should().Be(expectedCount);
tracks.First().AlbumReleaseId.Should().Be(albumReleaseId);
}
[Test]
public void migration_023_simple_case()
{
var release = Builder<add_release_groups_etc.LegacyAlbumRelease>
.CreateNew()
.Build();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum", release.ToJson());
GivenTracks(c, 1, 1, 1, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
}
[Test]
public void migration_023_multiple_media()
{
var release = Builder<add_release_groups_etc.LegacyAlbumRelease>
.CreateNew()
.With(e => e.MediaCount = 2)
.Build();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum", release.ToJson());
GivenTracks(c, 1, 1, 1, 10);
});
var migrated = VerifyAlbumReleases(db);
migrated.First().Media.Count.Should().Be(2);
VerifyTracks(db, 1, 1, 10);
}
[Test]
public void migration_023_null_title()
{
var release = Builder<add_release_groups_etc.LegacyAlbumRelease>
.CreateNew()
.With(e => e.Title = null)
.Build();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum", release.ToJson());
GivenTracks(c, 1, 1, 1, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
}
[Test]
public void migration_023_all_default_entries()
{
var release = new add_release_groups_etc.LegacyAlbumRelease();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum", release.ToJson());
GivenTracks(c, 1, 1, 1, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
}
[Test]
public void migration_023_empty_albumrelease()
{
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum", "");
GivenTracks(c, 1, 1, 1, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
}
[Test]
public void migration_023_duplicate_albumrelease()
{
var release = Builder<add_release_groups_etc.LegacyAlbumRelease>
.CreateNew()
.Build();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum1", release.ToJson());
GivenTracks(c, 1, 1, 1, 10);
GivenAlbum(c, 2, 1, "TestAlbum2", release.ToJson());
GivenTracks(c, 1, 2, 100, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
VerifyTracks(db, 2, 2, 10);
}
[Test]
public void migration_023_duplicate_foreignreleaseid()
{
var releases = Builder<add_release_groups_etc.LegacyAlbumRelease>
.CreateListOfSize(2)
.All()
.With(e => e.Id = "TestForeignId")
.Build();
var db = WithMigrationTestDb(c =>
{
GivenArtist(c, 1, "TestArtist");
GivenAlbum(c, 1, 1, "TestAlbum1", releases[0].ToJson());
GivenTracks(c, 1, 1, 1, 10);
GivenAlbum(c, 2, 1, "TestAlbum2", releases[1].ToJson());
GivenTracks(c, 1, 2, 100, 10);
});
VerifyAlbumReleases(db);
VerifyTracks(db, 1, 1, 10);
VerifyTracks(db, 2, 2, 10);
}
}
}

View File

@ -56,6 +56,10 @@ namespace NzbDrone.Core.Test.MusicTests
Mocker.GetMock<IReleaseService>()
.Setup(s => s.GetReleasesByAlbum(album1.Id))
.Returns(new List<AlbumRelease> { release });
Mocker.GetMock<IReleaseService>()
.Setup(s => s.GetReleasesByForeignReleaseId(It.IsAny<List<string>>()))
.Returns(new List<AlbumRelease> { release });
Mocker.GetMock<IProvideAlbumInfo>()
.Setup(s => s.GetAlbumInfo(It.IsAny<string>()))

View File

@ -48,6 +48,10 @@ namespace NzbDrone.Core.Test.MusicTests
Mocker.GetMock<IAlbumService>()
.Setup(s => s.GetAlbumsByArtist(It.IsAny<int>()))
.Returns(new List<Album>());
Mocker.GetMock<IAlbumService>()
.Setup(s => s.FindById(It.IsAny<List<string>>()))
.Returns(new List<Album>());
Mocker.GetMock<IProvideArtistInfo>()
.Setup(s => s.GetArtistInfo(It.IsAny<string>(), It.IsAny<int>()))

View File

@ -122,6 +122,7 @@
<Compile Include="Datastore\MappingExtentionFixture.cs" />
<Compile Include="Datastore\MarrDataLazyLoadingFixture.cs" />
<Compile Include="Datastore\Migration\004_add_various_qualities_in_profileFixture.cs" />
<Compile Include="Datastore\Migration\023_add_release_groups_etcFixture.cs" />
<Compile Include="Datastore\ObjectDatabaseFixture.cs" />
<Compile Include="Datastore\PagingSpecExtensionsTests\PagingOffsetFixture.cs" />
<Compile Include="Datastore\PagingSpecExtensionsTests\ToSortDirectionFixture.cs" />

View File

@ -161,7 +161,12 @@ namespace NzbDrone.Core.Datastore.Migration
{
var releases = ReadReleasesFromAlbums(conn, tran);
var dupeFreeReleases = releases.DistinctBy(x => x.ForeignReleaseId).ToList();
WriteReleasesToReleases(dupeFreeReleases, conn, tran);
var duplicates = releases.Except(dupeFreeReleases);
foreach (var release in duplicates)
{
release.ForeignReleaseId = release.AlbumId.ToString();
}
WriteReleasesToReleases(releases, conn, tran);
}
public class LegacyAlbumRelease : IEmbeddedDocument
@ -192,27 +197,47 @@ namespace NzbDrone.Core.Datastore.Migration
{
while (releaseReader.Read())
{
int rgId = releaseReader.GetInt32(0);
int albumId = releaseReader.GetInt32(0);
var albumRelease = Json.Deserialize<LegacyAlbumRelease>(releaseReader.GetString(1));
var media = new List<Medium>();
for (var i = 1; i <= albumRelease.MediaCount; i++)
AlbumRelease toInsert = null;
if (albumRelease != null)
{
media.Add(new Medium { Number = i, Name = "", Format = albumRelease.Format } );
}
var media = new List<Medium>();
for (var i = 1; i <= Math.Max(albumRelease.MediaCount, 1); i++)
{
media.Add(new Medium { Number = i, Name = "", Format = albumRelease.Format ?? "Unknown" } );
}
releases.Add(new AlbumRelease {
AlbumId = rgId,
ForeignReleaseId = albumRelease.Id,
Title = albumRelease.Title.IsNotNullOrWhiteSpace() ? albumRelease.Title : "",
toInsert = new AlbumRelease {
AlbumId = albumId,
ForeignReleaseId = albumRelease.Id.IsNotNullOrWhiteSpace() ? albumRelease.Id : albumId.ToString(),
Title = albumRelease.Title.IsNotNullOrWhiteSpace() ? albumRelease.Title : "",
Status = "",
Duration = 0,
Label = albumRelease.Label,
Disambiguation = albumRelease.Disambiguation,
Country = albumRelease.Country,
Media = media,
TrackCount = albumRelease.TrackCount,
Monitored = true
};
}
else
{
toInsert = new AlbumRelease {
AlbumId = albumId,
ForeignReleaseId = albumId.ToString(),
Title = "",
Status = "",
Duration = 0,
Label = albumRelease.Label,
Disambiguation = albumRelease.Disambiguation,
Country = albumRelease.Country,
Media=media,
TrackCount = albumRelease.TrackCount,
Label = new List<string>(),
Country = new List<string>(),
Media = new List<Medium> { new Medium { Name = "Unknown", Number = 1, Format = "Unknown" } },
Monitored = true
});
};
}
releases.Add(toInsert);
}
}
}

View File

@ -18,7 +18,8 @@ namespace NzbDrone.Core.Music
Album FindByName(string cleanTitle);
Album FindByTitle(int artistMetadataId, string title);
Album FindByArtistAndName(string artistName, string cleanTitle);
Album FindById(string spotifyId);
Album FindById(string foreignId);
List<Album> FindById(List<string> foreignIds);
PagingSpec<Album> AlbumsWithoutFiles(PagingSpec<Album> pagingSpec);
PagingSpec<Album> AlbumsWhereCutoffUnmet(PagingSpec<Album> pagingSpec, List<QualitiesBelowCutoff> qualitiesBelowCutoff, List<LanguagesBelowCutoff> languagesBelowCutoff);
List<Album> AlbumsBetweenDates(DateTime startDate, DateTime endDate, bool includeUnmonitored);
@ -58,6 +59,16 @@ namespace NzbDrone.Core.Music
return Query.Where(s => s.ForeignAlbumId == foreignAlbumId).SingleOrDefault();
}
public List<Album> FindById(List<string> ids)
{
string query = string.Format("SELECT Albums.* " +
"FROM Albums " +
"WHERE ForeignAlbumId IN ('{0}')",
string.Join("', '", ids));
return Query.QueryText(query).ToList();
}
public PagingSpec<Album> AlbumsWithoutFiles(PagingSpec<Album> pagingSpec)
{
var currentTime = DateTime.UtcNow;

View File

@ -17,7 +17,8 @@ namespace NzbDrone.Core.Music
List<Album> GetAlbumsByArtist(int artistId);
List<Album> GetAlbumsByArtistMetadataId(int artistMetadataId);
Album AddAlbum(Album newAlbum, string albumArtistId);
Album FindById(string spotifyId);
Album FindById(string foreignId);
List<Album> FindById(List<string> foreignIds);
Album FindByTitle(int artistId, string title);
Album FindByTitleInexact(int artistId, string title);
void DeleteAlbum(int albumId, bool deleteFiles);
@ -94,6 +95,11 @@ namespace NzbDrone.Core.Music
return _albumRepository.FindById(lidarrId);
}
public List<Album> FindById(List<string> ids)
{
return _albumRepository.FindById(ids);
}
public Album FindByTitle(int artistId, string title)
{
return _albumRepository.FindByTitle(artistId, title);

View File

@ -112,7 +112,13 @@ namespace NzbDrone.Core.Music
album.AlbumReleases = new List<AlbumRelease>();
var remoteReleases = albumInfo.AlbumReleases.Value.DistinctBy(m => m.ForeignReleaseId).ToList();
var existingReleases = _releaseService.GetReleasesByAlbum(album.Id);
// Search both ways to make sure we properly deal with releases that have been moved from one album to another
// as well as deleting any releases that have been removed from an album.
// note that under normal circumstances, a release would be captured by both queries.
var existingReleasesByAlbum = _releaseService.GetReleasesByAlbum(album.Id);
var existingReleasesById = _releaseService.GetReleasesByForeignReleaseId(remoteReleases.Select(x => x.ForeignReleaseId).ToList());
var existingReleases = existingReleasesByAlbum.Union(existingReleasesById).DistinctBy(x => x.Id).ToList();
var newReleaseList = new List<AlbumRelease>();
var updateReleaseList = new List<AlbumRelease>();
@ -137,9 +143,13 @@ namespace NzbDrone.Core.Music
album.AlbumReleases.Value.Add(release);
}
_releaseService.InsertMany(newReleaseList);
_releaseService.UpdateMany(updateReleaseList);
_logger.Debug("{0} Deleting {1}, Updating {2}, Adding {3} releases",
album, existingReleases.Count, updateReleaseList.Count, newReleaseList.Count);
// Delete first to avoid hitting distinct constraints
_releaseService.DeleteMany(existingReleases);
_releaseService.UpdateMany(updateReleaseList);
_releaseService.InsertMany(newReleaseList);
if (album.AlbumReleases.Value.Count(x => x.Monitored) == 0)
{

View File

@ -96,8 +96,10 @@ namespace NzbDrone.Core.Music
var remoteAlbums = artistInfo.Albums.Value.DistinctBy(m => new { m.ForeignAlbumId, m.ReleaseDate }).ToList();
// Get list of DB current db albums for artist
var existingAlbums = _albumService.GetAlbumsByArtist(artist.Id);
var existingAlbumsByArtist = _albumService.GetAlbumsByArtist(artist.Id);
var existingAlbumsById = _albumService.FindById(remoteAlbums.Select(x => x.ForeignAlbumId).ToList());
var existingAlbums = existingAlbumsByArtist.Union(existingAlbumsById).DistinctBy(x => x.Id).ToList();
var newAlbumsList = new List<Album>();
var updateAlbumsList = new List<Album>();
@ -118,17 +120,17 @@ namespace NzbDrone.Core.Music
}
}
_artistService.UpdateArtist(artist);
_logger.Debug("{0} Deleting {1}, Updating {2}, Adding {3} albums",
artist, existingAlbums.Count, updateAlbumsList.Count, newAlbumsList.Count);
// Delete old albums first - this avoids errors if albums have been merged and we'll
// end up trying to duplicate an existing release under a new album
_albumService.DeleteMany(existingAlbums);
// Update new albums with artist info and correct monitored status
newAlbumsList = UpdateAlbums(artist, newAlbumsList);
_logger.Info("Artist {0}, MetadataId {1}, Metadata.Id {2}", artist, artist.ArtistMetadataId, artist.Metadata.Value.Id);
_artistService.UpdateArtist(artist);
_addAlbumService.AddAlbums(newAlbumsList);
_refreshAlbumService.RefreshAlbumInfo(updateAlbumsList, forceAlbumRefresh);

View File

@ -37,12 +37,17 @@ namespace NzbDrone.Core.Music
foreach (var release in album.AlbumReleases.Value)
{
var existingTracks = _trackService.GetTracksByForeignReleaseId(release.ForeignReleaseId);
var dupeFreeRemoteTracks = release.Tracks.Value.DistinctBy(m => new { m.ForeignTrackId, m.TrackNumber }).ToList();
// Search both ways to make sure we properly deal with tracks that have been moved from one release to another
// as well as deleting any tracks that have been removed from a release.
// note that under normal circumstances, a track would be captured by both queries.
var existingTracksByRelease = _trackService.GetTracksByForeignReleaseId(release.ForeignReleaseId);
var existingTracksById = _trackService.GetTracksByForeignTrackIds(dupeFreeRemoteTracks.Select(x => x.ForeignTrackId).ToList());
var existingTracks = existingTracksByRelease.Union(existingTracksById).DistinctBy(x => x.Id).ToList();
var updateList = new List<Track>();
var newList = new List<Track>();
var dupeFreeRemoteTracks = release.Tracks.Value.DistinctBy(m => new { m.ForeignTrackId, m.TrackNumber }).ToList();
foreach (var track in OrderTracks(dupeFreeRemoteTracks))
{
@ -83,6 +88,9 @@ namespace NzbDrone.Core.Music
}
}
_logger.Debug("{0} Deleting {1}, Updating {2}, Adding {3} tracks",
release, existingTracks.Count, updateList.Count, newList.Count);
_trackService.DeleteMany(existingTracks);
_trackService.UpdateMany(updateList);
_trackService.InsertMany(newList);

View File

@ -18,6 +18,7 @@ namespace NzbDrone.Core.Music
List<Track> GetTracksByAlbum(int albumId);
List<Track> GetTracksByRelease(int albumReleaseId);
List<Track> GetTracksByForeignReleaseId(string foreignReleaseId);
List<Track> GetTracksByForeignTrackIds(List<string> foreignTrackId);
List<Track> GetTracksByMedium(int albumId, int mediumNumber);
List<Track> GetTracksByFileId(int fileId);
List<Track> TracksWithFiles(int artistId);
@ -97,6 +98,16 @@ namespace NzbDrone.Core.Music
return Query.QueryText(query).ToList();
}
public List<Track> GetTracksByForeignTrackIds(List<string> ids)
{
string query = string.Format("SELECT Tracks.* " +
"FROM Tracks " +
"WHERE ForeignTrackId IN ('{0}')",
string.Join("', '", ids));
return Query.QueryText(query).ToList();
}
public List<Track> GetTracksByMedium(int albumId, int mediumNumber)
{
if (mediumNumber < 1)

View File

@ -25,6 +25,7 @@ namespace NzbDrone.Core.Music
List<Track> GetTracksByAlbum(int albumId);
List<Track> GetTracksByRelease(int albumReleaseId);
List<Track> GetTracksByForeignReleaseId(string foreignReleaseId);
List<Track> GetTracksByForeignTrackIds(List<string> ids);
List<Track> TracksWithFiles(int artistId);
List<Track> GetTracksByFileId(int trackFileId);
void UpdateTrack(Track track);
@ -86,6 +87,11 @@ namespace NzbDrone.Core.Music
return _trackRepository.GetTracksByForeignReleaseId(foreignReleaseId);
}
public List<Track> GetTracksByForeignTrackIds(List<string> ids)
{
return _trackRepository.GetTracksByForeignTrackIds(ids);
}
public Track FindTrackByTitle(int artistId, int albumId, int mediumNumber, int trackNumber, string releaseTitle)
{
// TODO: can replace this search mechanism with something smarter/faster/better