From c2b66cf52476baea9336b8f023d9c1e446183870 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sun, 26 Mar 2017 12:16:16 -0700 Subject: [PATCH] Fixed: Deleting an episode file from the UI that was already deleted from disk Fixes #1782 --- .../EpisodeFiles/EpisodeFileModule.cs | 35 ++--- .../DeleteEpisodeFileFixture.cs | 140 ++++++++++++++++++ .../NzbDrone.Core.Test.csproj | 1 + .../MediaFiles/MediaFileDeletionService.cs | 84 +++++++++++ .../MediaFiles/RecycleBinProvider.cs | 13 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 1 + src/UI/Cells/DeleteEpisodeFileCell.js | 4 +- .../Editor/EpisodeFileEditorLayout.js | 2 +- 8 files changed, 244 insertions(+), 36 deletions(-) create mode 100644 src/NzbDrone.Core.Test/MediaFiles/MediaFileDeletionService/DeleteEpisodeFileFixture.cs create mode 100644 src/NzbDrone.Core/MediaFiles/MediaFileDeletionService.cs diff --git a/src/NzbDrone.Api/EpisodeFiles/EpisodeFileModule.cs b/src/NzbDrone.Api/EpisodeFiles/EpisodeFileModule.cs index c2044bda3..b061ef343 100644 --- a/src/NzbDrone.Api/EpisodeFiles/EpisodeFileModule.cs +++ b/src/NzbDrone.Api/EpisodeFiles/EpisodeFileModule.cs @@ -1,16 +1,13 @@ using System.Collections.Generic; -using System.IO; -using NLog; -using NzbDrone.Api.REST; -using NzbDrone.Common.Disk; -using NzbDrone.Common.Extensions; using NzbDrone.Core.Datastore.Events; using NzbDrone.Core.MediaFiles; using NzbDrone.Core.MediaFiles.Events; using NzbDrone.Core.Messaging.Events; using NzbDrone.Core.Tv; using NzbDrone.Core.DecisionEngine; +using NzbDrone.Core.Exceptions; using NzbDrone.SignalR; +using HttpStatusCode = System.Net.HttpStatusCode; namespace NzbDrone.Api.EpisodeFiles { @@ -18,27 +15,21 @@ namespace NzbDrone.Api.EpisodeFiles IHandle { private readonly IMediaFileService _mediaFileService; - private readonly IDiskProvider _diskProvider; - private readonly IRecycleBinProvider _recycleBinProvider; + private readonly IDeleteMediaFiles _mediaFileDeletionService; private readonly ISeriesService _seriesService; private readonly IQualityUpgradableSpecification _qualityUpgradableSpecification; - private readonly Logger _logger; public EpisodeFileModule(IBroadcastSignalRMessage signalRBroadcaster, IMediaFileService mediaFileService, - IDiskProvider diskProvider, - IRecycleBinProvider recycleBinProvider, + IDeleteMediaFiles mediaFileDeletionService, ISeriesService seriesService, - IQualityUpgradableSpecification qualityUpgradableSpecification, - Logger logger) + IQualityUpgradableSpecification qualityUpgradableSpecification) : base(signalRBroadcaster) { _mediaFileService = mediaFileService; - _diskProvider = diskProvider; - _recycleBinProvider = recycleBinProvider; + _mediaFileDeletionService = mediaFileDeletionService; _seriesService = seriesService; _qualityUpgradableSpecification = qualityUpgradableSpecification; - _logger = logger; GetResourceById = GetEpisodeFile; GetResourceAll = GetEpisodeFiles; UpdateResource = SetQuality; @@ -77,13 +68,15 @@ namespace NzbDrone.Api.EpisodeFiles private void DeleteEpisodeFile(int id) { var episodeFile = _mediaFileService.Get(id); - var series = _seriesService.GetSeries(episodeFile.SeriesId); - var fullPath = Path.Combine(series.Path, episodeFile.RelativePath); - var subfolder = _diskProvider.GetParentFolder(series.Path).GetRelativePath(_diskProvider.GetParentFolder(fullPath)); - _logger.Info("Deleting episode file: {0}", fullPath); - _recycleBinProvider.DeleteFile(fullPath, subfolder); - _mediaFileService.Delete(episodeFile, DeleteMediaFileReason.Manual); + if (episodeFile == null) + { + throw new NzbDroneClientException(HttpStatusCode.NotFound, "Episode file not found"); + } + + var series = _seriesService.GetSeries(episodeFile.SeriesId); + + _mediaFileDeletionService.DeleteEpisodeFile(series, episodeFile); } public void Handle(EpisodeFileAddedEvent message) diff --git a/src/NzbDrone.Core.Test/MediaFiles/MediaFileDeletionService/DeleteEpisodeFileFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/MediaFileDeletionService/DeleteEpisodeFileFixture.cs new file mode 100644 index 000000000..c2bef40ac --- /dev/null +++ b/src/NzbDrone.Core.Test/MediaFiles/MediaFileDeletionService/DeleteEpisodeFileFixture.cs @@ -0,0 +1,140 @@ +using System.IO; +using FizzWare.NBuilder; +using Moq; +using NUnit.Framework; +using NzbDrone.Common.Disk; +using NzbDrone.Core.Exceptions; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Tv; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.MediaFiles.MediaFileDeletionService +{ + [TestFixture] + public class DeleteEpisodeFileFixture : CoreTest + { + private static readonly string RootFolder = @"C:\Test\TV"; + private Series _series; + private EpisodeFile _episodeFile; + + [SetUp] + public void Setup() + { + _series = Builder.CreateNew() + .With(s => s.Path = Path.Combine(RootFolder, "Series Title")) + .Build(); + + _episodeFile = Builder.CreateNew() + .With(f => f.RelativePath = "Series Title - S01E01") + .With(f => f.Path = Path.Combine(_series.Path, "Series Title - S01E01")) + .Build(); + + Mocker.GetMock() + .Setup(s => s.GetParentFolder(_series.Path)) + .Returns(RootFolder); + + Mocker.GetMock() + .Setup(s => s.GetParentFolder(_episodeFile.Path)) + .Returns(_series.Path); + } + + private void GivenRootFolderExists() + { + Mocker.GetMock() + .Setup(s => s.FolderExists(RootFolder)) + .Returns(true); + } + + private void GivenRootFolderHasFolders() + { + Mocker.GetMock() + .Setup(s => s.GetDirectories(RootFolder)) + .Returns(new[] { _series.Path }); + } + + private void GivenSeriesFolderExists() + { + Mocker.GetMock() + .Setup(s => s.FolderExists(_series.Path)) + .Returns(true); + } + + [Test] + public void should_throw_if_root_folder_does_not_exist() + { + Assert.Throws(() => Subject.DeleteEpisodeFile(_series, _episodeFile)); + } + + [Test] + public void should_should_throw_if_root_folder_is_empty() + { + GivenRootFolderExists(); + Assert.Throws(() => Subject.DeleteEpisodeFile(_series, _episodeFile)); + } + + [Test] + public void should_delete_from_db_if_series_folder_does_not_exist() + { + GivenRootFolderExists(); + GivenRootFolderHasFolders(); + + Subject.DeleteEpisodeFile(_series, _episodeFile); + + Mocker.GetMock().Verify(v => v.Delete(_episodeFile, DeleteMediaFileReason.Manual), Times.Once()); + Mocker.GetMock().Verify(v => v.DeleteFile(_episodeFile.Path, It.IsAny()), Times.Never()); + } + + [Test] + public void should_delete_from_db_if_episode_file_does_not_exist() + { + GivenRootFolderExists(); + GivenRootFolderHasFolders(); + GivenSeriesFolderExists(); + + Subject.DeleteEpisodeFile(_series, _episodeFile); + + Mocker.GetMock().Verify(v => v.Delete(_episodeFile, DeleteMediaFileReason.Manual), Times.Once()); + Mocker.GetMock().Verify(v => v.DeleteFile(_episodeFile.Path, It.IsAny()), Times.Never()); + } + + [Test] + public void should_delete_from_disk_and_db_if_episode_file_exists() + { + GivenRootFolderExists(); + GivenRootFolderHasFolders(); + GivenSeriesFolderExists(); + + Mocker.GetMock() + .Setup(s => s.FileExists(_episodeFile.Path)) + .Returns(true); + + Subject.DeleteEpisodeFile(_series, _episodeFile); + + Mocker.GetMock().Verify(v => v.DeleteFile(_episodeFile.Path, "Series Title"), Times.Once()); + Mocker.GetMock().Verify(v => v.Delete(_episodeFile, DeleteMediaFileReason.Manual), Times.Once()); + } + + [Test] + public void should_handle_error_deleting_episode_file() + { + GivenRootFolderExists(); + GivenRootFolderHasFolders(); + GivenSeriesFolderExists(); + + Mocker.GetMock() + .Setup(s => s.FileExists(_episodeFile.Path)) + .Returns(true); + + Mocker.GetMock() + .Setup(s => s.DeleteFile(_episodeFile.Path, "Series Title")) + .Throws(new IOException()); + + Assert.Throws(() => Subject.DeleteEpisodeFile(_series, _episodeFile)); + + ExceptionVerification.ExpectedErrors(1); + Mocker.GetMock().Verify(v => v.DeleteFile(_episodeFile.Path, "Series Title"), Times.Once()); + Mocker.GetMock().Verify(v => v.Delete(_episodeFile, DeleteMediaFileReason.Manual), Times.Never()); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index c5254bc2f..de84c2cb9 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -290,6 +290,7 @@ + diff --git a/src/NzbDrone.Core/MediaFiles/MediaFileDeletionService.cs b/src/NzbDrone.Core/MediaFiles/MediaFileDeletionService.cs new file mode 100644 index 000000000..65dd2369c --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/MediaFileDeletionService.cs @@ -0,0 +1,84 @@ +using System; +using System.IO; +using System.Net; +using NLog; +using NzbDrone.Common.Disk; +using NzbDrone.Common.Extensions; +using NzbDrone.Core.Exceptions; +using NzbDrone.Core.Messaging.Events; +using NzbDrone.Core.Tv; +using NzbDrone.Core.Tv.Events; + +namespace NzbDrone.Core.MediaFiles +{ + public interface IDeleteMediaFiles + { + void DeleteEpisodeFile(Series series, EpisodeFile episodeFile); + } + + public class MediaFileDeletionService : IDeleteMediaFiles, IHandleAsync + { + private readonly IDiskProvider _diskProvider; + private readonly IRecycleBinProvider _recycleBinProvider; + private readonly IMediaFileService _mediaFileService; + private readonly Logger _logger; + + public MediaFileDeletionService(IDiskProvider diskProvider, + IRecycleBinProvider recycleBinProvider, + IMediaFileService mediaFileService, + Logger logger) + { + _diskProvider = diskProvider; + _recycleBinProvider = recycleBinProvider; + _mediaFileService = mediaFileService; + _logger = logger; + } + + public void DeleteEpisodeFile(Series series, EpisodeFile episodeFile) + { + var fullPath = Path.Combine(series.Path, episodeFile.RelativePath); + var rootFolder = _diskProvider.GetParentFolder(series.Path); + + if (!_diskProvider.FolderExists(rootFolder)) + { + throw new NzbDroneClientException(HttpStatusCode.Conflict, "Series' root folder ({0}) doesn't exist.", rootFolder); + } + + if (_diskProvider.GetDirectories(rootFolder).Empty()) + { + throw new NzbDroneClientException(HttpStatusCode.Conflict, "Series' root folder ({0}) is empty.", rootFolder); + } + + if (_diskProvider.FolderExists(series.Path) && _diskProvider.FileExists(fullPath)) + { + _logger.Info("Deleting episode file: {0}", fullPath); + + var subfolder = _diskProvider.GetParentFolder(series.Path).GetRelativePath(_diskProvider.GetParentFolder(fullPath)); + + try + { + _recycleBinProvider.DeleteFile(fullPath, subfolder); + } + catch (Exception e) + { + _logger.Error(e, "Unable to delete episode file"); + throw new NzbDroneClientException(HttpStatusCode.InternalServerError, "Unable to delete episode file"); + } + } + + // Delete the episode file from the database to clean it up even if the file was already deleted + _mediaFileService.Delete(episodeFile, DeleteMediaFileReason.Manual); + } + + public void HandleAsync(SeriesDeletedEvent message) + { + if (message.DeleteFiles) + { + if (_diskProvider.FolderExists(message.Series.Path)) + { + _recycleBinProvider.DeleteFolder(message.Series.Path); + } + } + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/RecycleBinProvider.cs b/src/NzbDrone.Core/MediaFiles/RecycleBinProvider.cs index 540164a7c..544908cd0 100644 --- a/src/NzbDrone.Core/MediaFiles/RecycleBinProvider.cs +++ b/src/NzbDrone.Core/MediaFiles/RecycleBinProvider.cs @@ -20,7 +20,7 @@ namespace NzbDrone.Core.MediaFiles void Cleanup(); } - public class RecycleBinProvider : IHandleAsync, IExecute, IRecycleBinProvider + public class RecycleBinProvider : IExecute, IRecycleBinProvider { private readonly IDiskTransferService _diskTransferService; private readonly IDiskProvider _diskProvider; @@ -192,17 +192,6 @@ namespace NzbDrone.Core.MediaFiles _logger.Debug("Recycling Bin has been cleaned up."); } - public void HandleAsync(SeriesDeletedEvent message) - { - if (message.DeleteFiles) - { - if (_diskProvider.FolderExists(message.Series.Path)) - { - DeleteFolder(message.Series.Path); - } - } - } - public void Execute(CleanUpRecycleBinCommand message) { Cleanup(); diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 22106bc73..a22cd6751 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -777,6 +777,7 @@ + diff --git a/src/UI/Cells/DeleteEpisodeFileCell.js b/src/UI/Cells/DeleteEpisodeFileCell.js index 88ddf8b82..6266a41bb 100644 --- a/src/UI/Cells/DeleteEpisodeFileCell.js +++ b/src/UI/Cells/DeleteEpisodeFileCell.js @@ -19,9 +19,9 @@ module.exports = Backgrid.Cell.extend({ var self = this; if (window.confirm('Are you sure you want to delete \'{0}\' from disk?'.format(this.model.get('path')))) { - this.model.destroy().done(function() { + this.model.destroy({ wait: true }).done(function() { vent.trigger(vent.Events.EpisodeFileDeleted, { episodeFile : self.model }); }); } } -}); \ No newline at end of file +}); diff --git a/src/UI/EpisodeFile/Editor/EpisodeFileEditorLayout.js b/src/UI/EpisodeFile/Editor/EpisodeFileEditorLayout.js index a974c8f7c..46eb0638d 100644 --- a/src/UI/EpisodeFile/Editor/EpisodeFileEditorLayout.js +++ b/src/UI/EpisodeFile/Editor/EpisodeFileEditorLayout.js @@ -176,7 +176,7 @@ module.exports = Marionette.Layout.extend({ if (reqres.hasHandler(reqres.Requests.GetEpisodeFileById)) { var episodeFile = reqres.request(reqres.Requests.GetEpisodeFileById, episodeFileId); - episodeFile.destroy(); + episodeFile.destroy({ wait: true }); } });