Fixed: Exception thrown when marking download as complete

Closes #326

(cherry picked from commit 27a9edf33d6ddb5f028f19b308245a067f1d8f2a)

src/NzbDrone.Core.Test/Download/CompletedDownloadServiceTests/ImportFixture.cs

@@ -61,6 +61,10 @@ namespace NzbDrone.Core.Test.Download.CompletedDownloadServiceTests
             Mocker.GetMock<IParsingService>()
                   .Setup(s => s.GetAuthor("Drone.S01E01.HDTV"))
                   .Returns(remoteBook.Author);
+
+            Mocker.GetMock<IHistoryService>()
+                .Setup(s => s.FindByDownloadId(It.IsAny<string>()))
+                .Returns(new List<EntityHistory>());
         }
 
         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<IHistoryService>()

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);

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);
         }