From 6292f223aca1ff01fdb46270ea3fe918bb966bad Mon Sep 17 00:00:00 2001 From: Bogdan Date: Tue, 17 Sep 2024 23:54:40 +0300 Subject: [PATCH] Fixed: Attempt to ensure all import results are imported Fixes #2746 Closes #4815 --- .../ImportFixture.cs | 19 ++++++++++----- .../Download/CompletedDownloadService.cs | 23 ++++++++++--------- .../TrackImport/Manual/ManualImportService.cs | 14 +++++------ 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs b/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs index 16f6cfd1a..9719b7f1f 100644 --- a/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs +++ b/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs @@ -183,6 +183,8 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests { GivenArtistMatch(); + var tracks = Builder.CreateListOfSize(3).BuildList(); + _trackedDownload.RemoteAlbum.Albums = new List { CreateAlbum(1, 3) @@ -192,9 +194,9 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests .Setup(v => v.ProcessPath(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new List { - new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() })), - new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() })), - new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() })), + new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic(), Tracks = new List { tracks[0] } })), + new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic(), Tracks = new List { tracks[1] } })), + new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic(), Tracks = new List { tracks[2] } })), new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() }), "Test Failure") }); @@ -290,6 +292,9 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests [Test] public void should_mark_as_imported_if_all_tracks_were_imported() { + var track1 = new Track { Id = 1 }; + var track2 = new Track { Id = 2 }; + _trackedDownload.RemoteAlbum.Albums = new List { CreateAlbum(1, 2) @@ -301,11 +306,11 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests { new ImportResult( new ImportDecision( - new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() })), + new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic(), Tracks = new List { track1 } })), new ImportResult( new ImportDecision( - new LocalTrack { Path = @"C:\TestPath\Droned.S01E02.mkv".AsOsAgnostic() })) + new LocalTrack { Path = @"C:\TestPath\Droned.S01E02.mkv".AsOsAgnostic(), Tracks = new List { track2 } })) }); Subject.Import(_trackedDownload); @@ -367,11 +372,13 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests { GivenABadlyNamedDownload(); + var track1 = new Track { Id = 1 }; + Mocker.GetMock() .Setup(v => v.ProcessPath(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new List { - new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic() })) + new ImportResult(new ImportDecision(new LocalTrack { Path = @"C:\TestPath\Droned.S01E01.mkv".AsOsAgnostic(), Tracks = new List { track1 } })) }); Mocker.GetMock() diff --git a/src/NzbDrone.Core/Download/CompletedDownloadService.cs b/src/NzbDrone.Core/Download/CompletedDownloadService.cs index eca048e78..7fb40d994 100644 --- a/src/NzbDrone.Core/Download/CompletedDownloadService.cs +++ b/src/NzbDrone.Core/Download/CompletedDownloadService.cs @@ -178,9 +178,11 @@ namespace NzbDrone.Core.Download public bool VerifyImport(TrackedDownload trackedDownload, List importResults) { - var allTracksImported = importResults.All(c => c.Result == ImportResultType.Imported) || - importResults.Count(c => c.Result == ImportResultType.Imported) >= - Math.Max(1, trackedDownload.RemoteAlbum.Albums.Sum(x => x.AlbumReleases.Value.Where(y => y.Monitored).Sum(z => z.TrackCount))); + var allTracksImported = + (importResults.Any() && importResults.All(c => c.Result == ImportResultType.Imported)) || + importResults.Where(c => c.Result == ImportResultType.Imported) + .SelectMany(c => c.ImportDecision.Item.Tracks) + .Count() >= Math.Max(1, trackedDownload.RemoteAlbum.Albums.Sum(x => x.AlbumReleases.Value.Where(y => y.Monitored).Sum(z => z.TrackCount))); if (allTracksImported) { @@ -191,6 +193,10 @@ namespace NzbDrone.Core.Download return true; } + var historyItems = _historyService.FindByDownloadId(trackedDownload.DownloadItem.DownloadId) + .OrderByDescending(h => h.Date) + .ToList(); + // Double check if all episodes were imported by checking the history if at least one // file was imported. This will allow the decision engine to reject already imported // episode files and still mark the download complete when all files are imported. @@ -199,19 +205,14 @@ namespace NzbDrone.Core.Download // and an episode is removed, but later comes back with a different ID then Sonarr will treat it as incomplete. // Since imports should be relatively fast and these types of data changes are infrequent this should be quite // safe, but commenting for future benefit. - var atLeastOneEpisodeImported = importResults.Any(c => c.Result == ImportResultType.Imported); - - var historyItems = _historyService.FindByDownloadId(trackedDownload.DownloadItem.DownloadId) - .OrderByDescending(h => h.Date) - .ToList(); - + var atLeastOneTrackImported = importResults.Any(c => c.Result == ImportResultType.Imported); var allTracksImportedInHistory = _trackedDownloadAlreadyImported.IsImported(trackedDownload, historyItems); if (allTracksImportedInHistory) { // Log different error messages depending on the circumstances, but treat both as fully imported, because that's the reality. // The second message shouldn't be logged in most cases, but continued reporting would indicate an ongoing issue. - if (atLeastOneEpisodeImported) + if (atLeastOneTrackImported) { _logger.Debug("All albums were imported in history for {0}", trackedDownload.DownloadItem.Title); } @@ -233,7 +234,7 @@ namespace NzbDrone.Core.Download return true; } - _logger.Debug("Not all albums have been imported for {0}", trackedDownload.DownloadItem.Title); + _logger.Debug("Not all albums have been imported for the release '{0}'", trackedDownload.DownloadItem.Title); return false; } diff --git a/src/NzbDrone.Core/MediaFiles/TrackImport/Manual/ManualImportService.cs b/src/NzbDrone.Core/MediaFiles/TrackImport/Manual/ManualImportService.cs index 7cf6b7c85..d7d0d2d4e 100644 --- a/src/NzbDrone.Core/MediaFiles/TrackImport/Manual/ManualImportService.cs +++ b/src/NzbDrone.Core/MediaFiles/TrackImport/Manual/ManualImportService.cs @@ -391,28 +391,26 @@ namespace NzbDrone.Core.MediaFiles.TrackImport.Manual { var trackedDownload = groupedTrackedDownload.First().TrackedDownload; var importArtist = groupedTrackedDownload.First().ImportResult.ImportDecision.Item.Artist; - var outputPath = trackedDownload.ImportItem.OutputPath.FullPath; if (_diskProvider.FolderExists(outputPath)) { if (_downloadedTracksImportService.ShouldDeleteFolder( - _diskProvider.GetDirectoryInfo(outputPath), - importArtist) && trackedDownload.DownloadItem.CanMoveFiles) + _diskProvider.GetDirectoryInfo(outputPath), importArtist) && + trackedDownload.DownloadItem.CanMoveFiles) { _diskProvider.DeleteFolder(outputPath, true); } } - var remoteTrackCount = Math.Max(1, - trackedDownload.RemoteAlbum?.Albums.Sum(x => - x.AlbumReleases.Value.Where(y => y.Monitored).Sum(z => z.TrackCount)) ?? 1); + var remoteTrackCount = Math.Max(1, trackedDownload.RemoteAlbum?.Albums.Sum(x => x.AlbumReleases.Value.Where(y => y.Monitored).Sum(z => z.TrackCount)) ?? 1); - var importResults = groupedTrackedDownload.Select(x => x.ImportResult).ToList(); + var importResults = groupedTrackedDownload.Select(c => c.ImportResult).ToList(); var importedTrackCount = importResults.Where(c => c.Result == ImportResultType.Imported) .SelectMany(c => c.ImportDecision.Item.Tracks) .Count(); - var allTracksImported = importResults.All(c => c.Result == ImportResultType.Imported) || importedTrackCount >= remoteTrackCount; + + var allTracksImported = (importResults.Any() && importResults.All(c => c.Result == ImportResultType.Imported)) || importedTrackCount >= remoteTrackCount; if (allTracksImported) {