From 9ebeee8b4fb04edfb709767b970f5e1d109bc4b9 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sat, 13 Jun 2020 13:11:30 -0700 Subject: [PATCH] Fixed: Exception thrown when marking download as complete Closes #326 (cherry picked from commit 27a9edf33d6ddb5f028f19b308245a067f1d8f2a) --- .../ImportFixture.cs | 5 ++- .../Download/CompletedDownloadService.cs | 32 ++++++++++++------- .../History/DownloadHistoryService.cs | 13 +++++--- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs b/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs index 342cae05d..74e030a8e 100644 --- a/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs +++ b/src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs @@ -61,6 +61,10 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests Mocker.GetMock() .Setup(s => s.GetAuthor("Drone.S01E01.HDTV")) .Returns(remoteBook.Author); + + Mocker.GetMock() + .Setup(s => s.FindByDownloadId(It.IsAny())) + .Returns(new List()); } private Book CreateBook(int id) @@ -82,7 +86,6 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests private void GivenABadlyNamedDownload() { - _trackedDownload.RemoteBook.Author = null; _trackedDownload.DownloadItem.DownloadId = "1234"; _trackedDownload.DownloadItem.Title = "Droned Pilot"; // Set a badly named download Mocker.GetMock() diff --git a/src/NzbDrone.Core/Download/CompletedDownloadService.cs b/src/NzbDrone.Core/Download/CompletedDownloadService.cs index 56b4a5c40..9e1565d58 100644 --- a/src/NzbDrone.Core/Download/CompletedDownloadService.cs +++ b/src/NzbDrone.Core/Download/CompletedDownloadService.cs @@ -3,9 +3,10 @@ using System.Collections.Generic; using System.IO; using System.Linq; using NLog; +using NLog.Fluent; using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Extensions; -using NzbDrone.Core.Books; +using NzbDrone.Common.Instrumentation.Extensions; using NzbDrone.Core.Download.TrackedDownloads; using NzbDrone.Core.History; using NzbDrone.Core.MediaFiles; @@ -27,7 +28,6 @@ namespace NzbDrone.Core.Download private readonly IEventAggregator _eventAggregator; private readonly IHistoryService _historyService; private readonly IDownloadedBooksImportService _downloadedTracksImportService; - private readonly IAuthorService _authorService; private readonly IProvideImportItemService _importItemService; private readonly ITrackedDownloadAlreadyImported _trackedDownloadAlreadyImported; private readonly Logger _logger; @@ -36,7 +36,6 @@ namespace NzbDrone.Core.Download IHistoryService historyService, IProvideImportItemService importItemService, IDownloadedBooksImportService downloadedTracksImportService, - IAuthorService authorService, ITrackedDownloadAlreadyImported trackedDownloadAlreadyImported, Logger logger) { @@ -44,7 +43,6 @@ namespace NzbDrone.Core.Download _historyService = historyService; _importItemService = importItemService; _downloadedTracksImportService = downloadedTracksImportService; - _authorService = authorService; _trackedDownloadAlreadyImported = trackedDownloadAlreadyImported; _logger = logger; } @@ -151,15 +149,17 @@ 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. - if (importResults.Any(c => c.Result == ImportResultType.Imported)) - { - var historyItems = _historyService.FindByDownloadId(trackedDownload.DownloadItem.DownloadId) - .OrderByDescending(h => h.Date) - .ToList(); + var atLeastOneEpisodeImported = importResults.Any(c => c.Result == ImportResultType.Imported); + + var historyItems = _historyService.FindByDownloadId(trackedDownload.DownloadItem.DownloadId) + .OrderByDescending(h => h.Date) + .ToList(); - var allEpisodesImportedInHistory = _trackedDownloadAlreadyImported.IsImported(trackedDownload, historyItems); + var allEpisodesImportedInHistory = _trackedDownloadAlreadyImported.IsImported(trackedDownload, historyItems); - if (allEpisodesImportedInHistory) + if (allEpisodesImportedInHistory) + { + if (atLeastOneEpisodeImported) { _logger.Debug("All books were imported in history for {0}", trackedDownload.DownloadItem.Title); trackedDownload.State = TrackedDownloadState.Imported; @@ -168,8 +168,18 @@ namespace NzbDrone.Core.Download .Select(x => x.AuthorId) .MostCommon(); _eventAggregator.PublishEvent(new DownloadCompletedEvent(trackedDownload, trackedDownload.RemoteBook?.Author.Id ?? importedAuthorId)); + return true; } + + _logger.Debug() + .Message("No Episodes were just imported, but all episodes were previously imported, possible issue with download history.") + .Property("AuthorId", trackedDownload.RemoteBook.Author.Id) + .Property("DownloadId", trackedDownload.DownloadItem.DownloadId) + .Property("Title", trackedDownload.DownloadItem.Title) + .Property("Path", trackedDownload.DownloadItem.OutputPath.ToString()) + .WriteSentryWarn("DownloadHistoryIncomplete") + .Write(); } _logger.Debug("Not all books have been imported for {0}", trackedDownload.DownloadItem.Title); diff --git a/src/NzbDrone.Core/Download/History/DownloadHistoryService.cs b/src/NzbDrone.Core/Download/History/DownloadHistoryService.cs index ec4233a2e..2fd1bb439 100644 --- a/src/NzbDrone.Core/Download/History/DownloadHistoryService.cs +++ b/src/NzbDrone.Core/Download/History/DownloadHistoryService.cs @@ -144,7 +144,8 @@ namespace NzbDrone.Core.Download.History var history = new DownloadHistory { EventType = DownloadHistoryEventType.FileImported, - AuthorId = message.BookInfo.Author.Id, + + AuthorId = message.ImportedBook.Author.Value.Id, DownloadId = downloadId, SourceTitle = message.BookInfo.Path, Date = DateTime.UtcNow, @@ -182,19 +183,21 @@ namespace NzbDrone.Core.Download.History public void Handle(DownloadCompletedEvent message) { + var downloadItem = message.TrackedDownload.DownloadItem; + var history = new DownloadHistory { EventType = DownloadHistoryEventType.DownloadImported, AuthorId = message.AuthorId, - DownloadId = message.TrackedDownload.DownloadItem.DownloadId, - SourceTitle = message.TrackedDownload.DownloadItem.OutputPath.ToString(), + DownloadId = downloadItem.DownloadId, + SourceTitle = downloadItem.Title, Date = DateTime.UtcNow, Protocol = message.TrackedDownload.Protocol, DownloadClientId = message.TrackedDownload.DownloadClient }; - history.Data.Add("DownloadClient", message.TrackedDownload.DownloadItem.DownloadClientInfo.Type); - history.Data.Add("DownloadClientName", message.TrackedDownload.DownloadItem.DownloadClientInfo.Name); + history.Data.Add("DownloadClient", downloadItem.DownloadClientInfo.Type); + history.Data.Add("DownloadClientName", downloadItem.DownloadClientInfo.Name); _repository.Insert(history); }