Fixed: Deleting an trackfile from the UI that was already deleted from disk

src/Lidarr.Api.V3/TrackFiles/TrackFileModule.cs

@@ -3,7 +3,6 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using Nancy;
-using NLog;
 using NzbDrone.Core.Datastore.Events;
 using NzbDrone.Core.DecisionEngine;
 using NzbDrone.Core.MediaFiles;
@@ -13,7 +12,8 @@ using NzbDrone.Core.Music;
 using NzbDrone.SignalR;
 using Lidarr.Http;
 using Lidarr.Http.Extensions;
-using Lidarr.Http.REST;
+using NzbDrone.Core.Exceptions;
+using HttpStatusCode = System.Net.HttpStatusCode;
 
 namespace Lidarr.Api.V3.TrackFiles
 {
@@ -21,27 +21,24 @@ namespace Lidarr.Api.V3.TrackFiles
                                  IHandle<TrackFileAddedEvent>
     {
         private readonly IMediaFileService _mediaFileService;
-        private readonly IRecycleBinProvider _recycleBinProvider;
+        private readonly IDeleteMediaFiles _mediaFileDeletionService;
         private readonly IArtistService _artistService;
         private readonly IAlbumService _albumService;
         private readonly IUpgradableSpecification _upgradableSpecification;
-        private readonly Logger _logger;
 
         public TrackModule(IBroadcastSignalRMessage signalRBroadcaster,
                              IMediaFileService mediaFileService,
-                             IRecycleBinProvider recycleBinProvider,
+                             IDeleteMediaFiles mediaFileDeletionService,
                              IArtistService artistService,
                              IAlbumService albumService,
-                             IUpgradableSpecification upgradableSpecification,
-                             Logger logger)
+                             IUpgradableSpecification upgradableSpecification)
             : base(signalRBroadcaster)
         {
             _mediaFileService = mediaFileService;
-            _recycleBinProvider = recycleBinProvider;
+            _mediaFileDeletionService = mediaFileDeletionService;
             _artistService = artistService;
             _albumService = albumService;
             _upgradableSpecification = upgradableSpecification;
-            _logger = logger;
 
             GetResourceById = GetTrackFile;
             GetResourceAll = GetTrackFiles;
@@ -68,7 +65,7 @@ namespace Lidarr.Api.V3.TrackFiles
 
             if (!artistIdQuery.HasValue && !trackFileIdsQuery.HasValue && !albumIdQuery.HasValue)
             {
-                throw new BadRequestException("artistId, albumId, or trackFileIds must be provided");
+                throw new Lidarr.Http.REST.BadRequestException("artistId, albumId, or trackFileIds must be provided");
             }
 
             if (artistIdQuery.HasValue && !albumIdQuery.HasValue)
@@ -89,9 +86,9 @@ namespace Lidarr.Api.V3.TrackFiles
 
             else
             {
-                string episodeFileIdsValue = trackFileIdsQuery.Value.ToString();
+                string trackFileIdsValue = trackFileIdsQuery.Value.ToString();
 
-                var trackFileIds = episodeFileIdsValue.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
+                var trackFileIds = trackFileIdsValue.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
                                                         .Select(e => Convert.ToInt32(e))
                                                         .ToList();
 
@@ -131,21 +128,25 @@ namespace Lidarr.Api.V3.TrackFiles
 
             _mediaFileService.Update(trackFiles);
 
-            var series = _artistService.GetArtist(trackFiles.First().ArtistId);
+            var artist = _artistService.GetArtist(trackFiles.First().ArtistId);
 
-            return trackFiles.ConvertAll(f => f.ToResource(series, _upgradableSpecification))
-                               .AsResponse(HttpStatusCode.Accepted);
+            return trackFiles.ConvertAll(f => f.ToResource(artist, _upgradableSpecification))
+                               .AsResponse(Nancy.HttpStatusCode.Accepted);
         }
 
         private void DeleteTrackFile(int id)
         {
             var trackFile = _mediaFileService.Get(id);
+
+            if (trackFile == null)
+            {
+                throw new NzbDroneClientException(HttpStatusCode.NotFound, "Track file not found");
+            }
+
             var artist = _artistService.GetArtist(trackFile.ArtistId);
             var fullPath = Path.Combine(artist.Path, trackFile.RelativePath);
 
-            _logger.Info("Deleting track file: {0}", fullPath);
-            _recycleBinProvider.DeleteFile(fullPath);
-            _mediaFileService.Delete(trackFile, DeleteMediaFileReason.Manual);
+            _mediaFileDeletionService.DeleteTrackFile(artist, trackFile);
         }
 
         private Response DeleteTrackFiles()
@@ -158,9 +159,7 @@ namespace Lidarr.Api.V3.TrackFiles
             {
                 var fullPath = Path.Combine(artist.Path, trackFile.RelativePath);
 
-                _logger.Info("Deleting track file: {0}", fullPath);
-                _recycleBinProvider.DeleteFile(fullPath);
-                _mediaFileService.Delete(trackFile, DeleteMediaFileReason.Manual);
+                _mediaFileDeletionService.DeleteTrackFile(artist, trackFile);
             }
 
             return new object().AsResponse();

src/NzbDrone.Core.Test/MediaFiles/MediaFileDeletionService/DeleteTrackFileFixture.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.Music;
+using NzbDrone.Test.Common;
+
+namespace NzbDrone.Core.Test.MediaFiles.MediaFileDeletionService
+{
+    [TestFixture]
+    public class DeleteTrackFileFixture : CoreTest<Core.MediaFiles.MediaFileDeletionService>
+    {
+        private static readonly string RootFolder = @"C:\Test\Music";
+        private Artist _artist;
+        private TrackFile _trackFile;
+
+        [SetUp]
+        public void Setup()
+        {
+            _artist = Builder<Artist>.CreateNew()
+                                     .With(s => s.Path = Path.Combine(RootFolder, "Artist Name"))
+                                     .Build();
+
+            _trackFile = Builder<TrackFile>.CreateNew()
+                                               .With(f => f.RelativePath = "Artist Name - Track01")
+                                               .With(f => f.Path = Path.Combine(_artist.Path, "Artist Name - Track01"))
+                                               .Build();
+
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.GetParentFolder(_artist.Path))
+                  .Returns(RootFolder);
+
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.GetParentFolder(_trackFile.Path))
+                  .Returns(_artist.Path);
+        }
+
+        private void GivenRootFolderExists()
+        {
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.FolderExists(RootFolder))
+                  .Returns(true);
+        }
+
+        private void GivenRootFolderHasFolders()
+        {
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.GetDirectories(RootFolder))
+                  .Returns(new[] { _artist.Path });
+        }
+
+        private void GivenSeriesFolderExists()
+        {
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.FolderExists(_artist.Path))
+                  .Returns(true);
+        }
+
+        [Test]
+        public void should_throw_if_root_folder_does_not_exist()
+        {
+            Assert.Throws<NzbDroneClientException>(() => Subject.DeleteTrackFile(_artist, _trackFile));
+        }
+
+        [Test]
+        public void should_should_throw_if_root_folder_is_empty()
+        {
+            GivenRootFolderExists();
+            Assert.Throws<NzbDroneClientException>(() => Subject.DeleteTrackFile(_artist, _trackFile));
+        }
+
+        [Test]
+        public void should_delete_from_db_if_artist_folder_does_not_exist()
+        {
+            GivenRootFolderExists();
+            GivenRootFolderHasFolders();
+
+            Subject.DeleteTrackFile(_artist, _trackFile);
+
+            Mocker.GetMock<IMediaFileService>().Verify(v => v.Delete(_trackFile, DeleteMediaFileReason.Manual), Times.Once());
+            Mocker.GetMock<IRecycleBinProvider>().Verify(v => v.DeleteFile(_trackFile.Path, It.IsAny<string>()), Times.Never());
+        }
+
+        [Test]
+        public void should_delete_from_db_if_track_file_does_not_exist()
+        {
+            GivenRootFolderExists();
+            GivenRootFolderHasFolders();
+            GivenSeriesFolderExists();
+
+            Subject.DeleteTrackFile(_artist, _trackFile);
+
+            Mocker.GetMock<IMediaFileService>().Verify(v => v.Delete(_trackFile, DeleteMediaFileReason.Manual), Times.Once());
+            Mocker.GetMock<IRecycleBinProvider>().Verify(v => v.DeleteFile(_trackFile.Path, It.IsAny<string>()), Times.Never());
+        }
+
+        [Test]
+        public void should_delete_from_disk_and_db_if_track_file_exists()
+        {
+            GivenRootFolderExists();
+            GivenRootFolderHasFolders();
+            GivenSeriesFolderExists();
+
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.FileExists(_trackFile.Path))
+                  .Returns(true);
+
+            Subject.DeleteTrackFile(_artist, _trackFile);
+
+            Mocker.GetMock<IRecycleBinProvider>().Verify(v => v.DeleteFile(_trackFile.Path, "Series Title"), Times.Once());
+            Mocker.GetMock<IMediaFileService>().Verify(v => v.Delete(_trackFile, DeleteMediaFileReason.Manual), Times.Once());
+        }
+
+        [Test]
+        public void should_handle_error_deleting_track_file()
+        {
+            GivenRootFolderExists();
+            GivenRootFolderHasFolders();
+            GivenSeriesFolderExists();
+
+            Mocker.GetMock<IDiskProvider>()
+                  .Setup(s => s.FileExists(_trackFile.Path))
+                  .Returns(true);
+
+            Mocker.GetMock<IRecycleBinProvider>()
+                  .Setup(s => s.DeleteFile(_trackFile.Path, "Artist Name"))
+                  .Throws(new IOException());
+
+            Assert.Throws<NzbDroneClientException>(() => Subject.DeleteTrackFile(_artist, _trackFile));
+
+            ExceptionVerification.ExpectedErrors(1);
+            Mocker.GetMock<IRecycleBinProvider>().Verify(v => v.DeleteFile(_trackFile.Path, "Artist Name"), Times.Once());
+            Mocker.GetMock<IMediaFileService>().Verify(v => v.Delete(_trackFile, DeleteMediaFileReason.Manual), Times.Never());
+        }
+    }
+}

src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj

@@ -270,6 +270,7 @@
     <Compile Include="MediaFiles\DiskScanServiceTests\ScanFixture.cs" />
     <Compile Include="MediaFiles\DownloadedAlbumsCommandServiceFixture.cs" />
     <Compile Include="MediaFiles\DownloadedTracksImportServiceFixture.cs" />
+    <Compile Include="MediaFiles\MediaFileDeletionService\DeleteTrackFileFixture.cs" />
     <Compile Include="MediaFiles\MediaInfo\MediaInfoFormatterTests\FormatAudioCodecFixture.cs" />
     <Compile Include="MediaFiles\TrackFileMovingServiceTests\MoveTrackFileFixture.cs" />
     <Compile Include="MediaFiles\TrackImport\ImportDecisionMakerFixture.cs" />

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.Music;
+using NzbDrone.Core.Music.Events;
+
+namespace NzbDrone.Core.MediaFiles
+{
+    public interface IDeleteMediaFiles
+    {
+        void DeleteTrackFile(Artist artist, TrackFile trackFile);
+    }
+
+    public class MediaFileDeletionService : IDeleteMediaFiles, IHandleAsync<ArtistDeletedEvent>
+    {
+        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 DeleteTrackFile(Artist artist, TrackFile trackFile)
+        {
+            var fullPath = Path.Combine(artist.Path, trackFile.RelativePath);
+            var rootFolder = _diskProvider.GetParentFolder(artist.Path);
+
+            if (!_diskProvider.FolderExists(rootFolder))
+            {
+                throw new NzbDroneClientException(HttpStatusCode.Conflict, "Artist's root folder ({0}) doesn't exist.", rootFolder);
+            }
+
+            if (_diskProvider.GetDirectories(rootFolder).Empty())
+            {
+                throw new NzbDroneClientException(HttpStatusCode.Conflict, "Artist's root folder ({0}) is empty.", rootFolder);
+            }
+
+            if (_diskProvider.FolderExists(artist.Path) && _diskProvider.FileExists(fullPath))
+            {
+                _logger.Info("Deleting track file: {0}", fullPath);
+
+                var subfolder = _diskProvider.GetParentFolder(artist.Path).GetRelativePath(_diskProvider.GetParentFolder(fullPath));
+
+                try
+                {
+                    _recycleBinProvider.DeleteFile(fullPath, subfolder);
+                }
+                catch (Exception e)
+                {
+                    _logger.Error(e, "Unable to delete track file");
+                    throw new NzbDroneClientException(HttpStatusCode.InternalServerError, "Unable to delete track file");
+                }
+            }
+
+            // Delete the track file from the database to clean it up even if the file was already deleted
+            _mediaFileService.Delete(trackFile, DeleteMediaFileReason.Manual);
+        }
+
+        public void HandleAsync(ArtistDeletedEvent message)
+        {
+            if (message.DeleteFiles)
+            {
+                if (_diskProvider.FolderExists(message.Artist.Path))
+                {
+                    _recycleBinProvider.DeleteFolder(message.Artist.Path);
+                }
+            }
+        }
+    }
+}

src/NzbDrone.Core/MediaFiles/RecycleBinProvider.cs

@@ -20,7 +20,7 @@ namespace NzbDrone.Core.MediaFiles
         void Cleanup();
     }
 
-    public class RecycleBinProvider : IHandleAsync<ArtistDeletedEvent>, IExecute<CleanUpRecycleBinCommand>, IRecycleBinProvider
+    public class RecycleBinProvider : IExecute<CleanUpRecycleBinCommand>, 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(ArtistDeletedEvent message)
-        {
-            if (message.DeleteFiles)
-            {
-                if (_diskProvider.FolderExists(message.Artist.Path))
-                {
-                    DeleteFolder(message.Artist.Path);
-                }
-            }
-        }
-
         public void Execute(CleanUpRecycleBinCommand message)
         {
             Cleanup();

src/NzbDrone.Core/NzbDrone.Core.csproj

@@ -744,6 +744,7 @@
     <Compile Include="MediaFiles\Events\EpisodeFolderCreatedEvent.cs" />
     <Compile Include="MediaFiles\Events\TrackFolderCreatedEvent.cs" />
     <Compile Include="MediaFiles\Events\TrackDownloadedEvent.cs" />
+    <Compile Include="MediaFiles\MediaFileDeletionService.cs" />
     <Compile Include="MediaFiles\MediaInfo\MediaInfoFormatter.cs" />
     <Compile Include="MediaFiles\RenameTrackFilePreview.cs" />
     <Compile Include="MediaFiles\RenameTrackFileService.cs" />