diff --git a/src/NzbDrone.Core.Test/Datastore/Migration/023_add_release_groups_etcFixture.cs b/src/NzbDrone.Core.Test/Datastore/Migration/023_add_release_groups_etcFixture.cs new file mode 100644 index 000000000..7af659710 --- /dev/null +++ b/src/NzbDrone.Core.Test/Datastore/Migration/023_add_release_groups_etcFixture.cs @@ -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 + { + 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 VerifyAlbumReleases(IDirectDataMapper db) + { + var releases = db.Query("SELECT * FROM AlbumReleases"); + var albums = db.Query("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("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 + .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 + .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 + .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 + .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 + .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); + } + } +} diff --git a/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs index 8957256d1..93e8071f8 100644 --- a/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/RefreshAlbumServiceFixture.cs @@ -56,6 +56,10 @@ namespace NzbDrone.Core.Test.MusicTests Mocker.GetMock() .Setup(s => s.GetReleasesByAlbum(album1.Id)) .Returns(new List { release }); + + Mocker.GetMock() + .Setup(s => s.GetReleasesByForeignReleaseId(It.IsAny>())) + .Returns(new List { release }); Mocker.GetMock() .Setup(s => s.GetAlbumInfo(It.IsAny())) diff --git a/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs index cf036d6cb..28b051916 100644 --- a/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs @@ -48,6 +48,10 @@ namespace NzbDrone.Core.Test.MusicTests Mocker.GetMock() .Setup(s => s.GetAlbumsByArtist(It.IsAny())) .Returns(new List()); + + Mocker.GetMock() + .Setup(s => s.FindById(It.IsAny>())) + .Returns(new List()); Mocker.GetMock() .Setup(s => s.GetArtistInfo(It.IsAny(), It.IsAny())) diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index ccd655a00..1abccca9f 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -122,6 +122,7 @@ + diff --git a/src/NzbDrone.Core/Datastore/Migration/023_add_release_groups_etc.cs b/src/NzbDrone.Core/Datastore/Migration/023_add_release_groups_etc.cs index 342697b92..fe244717f 100644 --- a/src/NzbDrone.Core/Datastore/Migration/023_add_release_groups_etc.cs +++ b/src/NzbDrone.Core/Datastore/Migration/023_add_release_groups_etc.cs @@ -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(releaseReader.GetString(1)); - var media = new List(); - 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(); + 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(), + Country = new List(), + Media = new List { new Medium { Name = "Unknown", Number = 1, Format = "Unknown" } }, Monitored = true - }); + }; + } + + releases.Add(toInsert); } } } diff --git a/src/NzbDrone.Core/Music/AlbumRepository.cs b/src/NzbDrone.Core/Music/AlbumRepository.cs index fba92541f..71b8ad140 100644 --- a/src/NzbDrone.Core/Music/AlbumRepository.cs +++ b/src/NzbDrone.Core/Music/AlbumRepository.cs @@ -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 FindById(List foreignIds); PagingSpec AlbumsWithoutFiles(PagingSpec pagingSpec); PagingSpec AlbumsWhereCutoffUnmet(PagingSpec pagingSpec, List qualitiesBelowCutoff, List languagesBelowCutoff); List 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 FindById(List ids) + { + string query = string.Format("SELECT Albums.* " + + "FROM Albums " + + "WHERE ForeignAlbumId IN ('{0}')", + string.Join("', '", ids)); + + return Query.QueryText(query).ToList(); + } + public PagingSpec AlbumsWithoutFiles(PagingSpec pagingSpec) { var currentTime = DateTime.UtcNow; diff --git a/src/NzbDrone.Core/Music/AlbumService.cs b/src/NzbDrone.Core/Music/AlbumService.cs index 12c39cc5c..f6f3876a2 100644 --- a/src/NzbDrone.Core/Music/AlbumService.cs +++ b/src/NzbDrone.Core/Music/AlbumService.cs @@ -17,7 +17,8 @@ namespace NzbDrone.Core.Music List GetAlbumsByArtist(int artistId); List GetAlbumsByArtistMetadataId(int artistMetadataId); Album AddAlbum(Album newAlbum, string albumArtistId); - Album FindById(string spotifyId); + Album FindById(string foreignId); + List FindById(List 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 FindById(List ids) + { + return _albumRepository.FindById(ids); + } + public Album FindByTitle(int artistId, string title) { return _albumRepository.FindByTitle(artistId, title); diff --git a/src/NzbDrone.Core/Music/RefreshAlbumService.cs b/src/NzbDrone.Core/Music/RefreshAlbumService.cs index 13996c496..94e206a72 100644 --- a/src/NzbDrone.Core/Music/RefreshAlbumService.cs +++ b/src/NzbDrone.Core/Music/RefreshAlbumService.cs @@ -112,7 +112,13 @@ namespace NzbDrone.Core.Music album.AlbumReleases = new List(); 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(); var updateReleaseList = new List(); @@ -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) { diff --git a/src/NzbDrone.Core/Music/RefreshArtistService.cs b/src/NzbDrone.Core/Music/RefreshArtistService.cs index 51e7f0c8b..c01d79555 100644 --- a/src/NzbDrone.Core/Music/RefreshArtistService.cs +++ b/src/NzbDrone.Core/Music/RefreshArtistService.cs @@ -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(); var updateAlbumsList = new List(); @@ -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); diff --git a/src/NzbDrone.Core/Music/RefreshTrackService.cs b/src/NzbDrone.Core/Music/RefreshTrackService.cs index 6adbec1ab..4b79ca163 100644 --- a/src/NzbDrone.Core/Music/RefreshTrackService.cs +++ b/src/NzbDrone.Core/Music/RefreshTrackService.cs @@ -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(); var newList = new List(); - 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); diff --git a/src/NzbDrone.Core/Music/TrackRepository.cs b/src/NzbDrone.Core/Music/TrackRepository.cs index f80348b6f..b4df8b820 100644 --- a/src/NzbDrone.Core/Music/TrackRepository.cs +++ b/src/NzbDrone.Core/Music/TrackRepository.cs @@ -18,6 +18,7 @@ namespace NzbDrone.Core.Music List GetTracksByAlbum(int albumId); List GetTracksByRelease(int albumReleaseId); List GetTracksByForeignReleaseId(string foreignReleaseId); + List GetTracksByForeignTrackIds(List foreignTrackId); List GetTracksByMedium(int albumId, int mediumNumber); List GetTracksByFileId(int fileId); List TracksWithFiles(int artistId); @@ -97,6 +98,16 @@ namespace NzbDrone.Core.Music return Query.QueryText(query).ToList(); } + public List GetTracksByForeignTrackIds(List ids) + { + string query = string.Format("SELECT Tracks.* " + + "FROM Tracks " + + "WHERE ForeignTrackId IN ('{0}')", + string.Join("', '", ids)); + + return Query.QueryText(query).ToList(); + } + public List GetTracksByMedium(int albumId, int mediumNumber) { if (mediumNumber < 1) diff --git a/src/NzbDrone.Core/Music/TrackService.cs b/src/NzbDrone.Core/Music/TrackService.cs index 5d1eec687..a6cb4f2c5 100644 --- a/src/NzbDrone.Core/Music/TrackService.cs +++ b/src/NzbDrone.Core/Music/TrackService.cs @@ -25,6 +25,7 @@ namespace NzbDrone.Core.Music List GetTracksByAlbum(int albumId); List GetTracksByRelease(int albumReleaseId); List GetTracksByForeignReleaseId(string foreignReleaseId); + List GetTracksByForeignTrackIds(List ids); List TracksWithFiles(int artistId); List GetTracksByFileId(int trackFileId); void UpdateTrack(Track track); @@ -86,6 +87,11 @@ namespace NzbDrone.Core.Music return _trackRepository.GetTracksByForeignReleaseId(foreignReleaseId); } + public List GetTracksByForeignTrackIds(List 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