From b3f11564a7eff6e3bc6023eb8499c81bd6ff924d Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 3 Feb 2015 20:46:51 +0100 Subject: [PATCH] Fixed: No longer leaves a corrupt file if MediaCover resize failed. --- .../MediaCoverTests/ImageResizerFixture.cs | 21 +++++++++ .../MediaCoverServiceFixture.cs | 46 +++++++++++++++++++ src/NzbDrone.Core/MediaCover/ImageResizer.cs | 23 +++++++--- .../MediaCover/MediaCoverService.cs | 20 +++++--- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/NzbDrone.Core.Test/MediaCoverTests/ImageResizerFixture.cs b/src/NzbDrone.Core.Test/MediaCoverTests/ImageResizerFixture.cs index 72d736a8b..38bc8681a 100644 --- a/src/NzbDrone.Core.Test/MediaCoverTests/ImageResizerFixture.cs +++ b/src/NzbDrone.Core.Test/MediaCoverTests/ImageResizerFixture.cs @@ -26,6 +26,14 @@ public void SetUp() Mocker.GetMock() .Setup(v => v.OpenWriteStream(It.IsAny())) .Returns(s => new FileStream(s, FileMode.Create)); + + Mocker.GetMock() + .Setup(v => v.FileExists(It.IsAny())) + .Returns(s => File.Exists(s)); + + Mocker.GetMock() + .Setup(v => v.DeleteFile(It.IsAny())) + .Callback(s => File.Delete(s)); } [Test] @@ -46,5 +54,18 @@ public void should_resize_image() image.Height.Should().Be(170); image.Width.Should().Be(170); } + + [Test] + public void should_delete_file_if_failed() + { + var mainFile = Path.Combine(TempFolder, "junk.png"); + var resizedFile = Path.Combine(TempFolder, "junk-170.png"); + + File.WriteAllText(mainFile, "Just some junk data that should make it throw an Exception."); + + Assert.Throws(Is.InstanceOf(), () => Subject.Resize(mainFile, resizedFile, 170)); + + File.Exists(resizedFile).Should().BeFalse(); + } } } \ No newline at end of file diff --git a/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs b/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs index 6e4a9c1d4..cd54723af 100644 --- a/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs @@ -11,6 +11,7 @@ using NzbDrone.Core.Test.Framework; using NzbDrone.Core.Tv; using NzbDrone.Core.Tv.Events; +using NzbDrone.Test.Common; namespace NzbDrone.Core.Test.MediaCoverTests { @@ -110,10 +111,55 @@ public void should_not_resize_covers_if_exists() .Setup(v => v.FileExists(It.IsAny())) .Returns(true); + Mocker.GetMock() + .Setup(v => v.GetFileSize(It.IsAny())) + .Returns(1000); + Subject.HandleAsync(new SeriesUpdatedEvent(_series)); Mocker.GetMock() .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); } + + [Test] + public void should_resize_covers_if_existing_is_empty() + { + Mocker.GetMock() + .Setup(v => v.AlreadyExists(It.IsAny(), It.IsAny())) + .Returns(true); + + Mocker.GetMock() + .Setup(v => v.FileExists(It.IsAny())) + .Returns(true); + + Mocker.GetMock() + .Setup(v => v.GetFileSize(It.IsAny())) + .Returns(0); + + Subject.HandleAsync(new SeriesUpdatedEvent(_series)); + + Mocker.GetMock() + .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + } + + [Test] + public void should_log_error_if_resize_failed() + { + Mocker.GetMock() + .Setup(v => v.AlreadyExists(It.IsAny(), It.IsAny())) + .Returns(true); + + Mocker.GetMock() + .Setup(v => v.FileExists(It.IsAny())) + .Returns(false); + + Mocker.GetMock() + .Setup(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny())) + .Throws(); + + Subject.HandleAsync(new SeriesUpdatedEvent(_series)); + + ExceptionVerification.ExpectedErrors(1); + } } } \ No newline at end of file diff --git a/src/NzbDrone.Core/MediaCover/ImageResizer.cs b/src/NzbDrone.Core/MediaCover/ImageResizer.cs index be97a3e2d..0466c0361 100644 --- a/src/NzbDrone.Core/MediaCover/ImageResizer.cs +++ b/src/NzbDrone.Core/MediaCover/ImageResizer.cs @@ -22,18 +22,29 @@ public ImageResizer(IDiskProvider diskProvider) public void Resize(string source, string destination, int height) { - using (var sourceStream = _diskProvider.OpenReadStream(source)) + try { - using (var outputStream = _diskProvider.OpenWriteStream(destination)) + using (var sourceStream = _diskProvider.OpenReadStream(source)) { - var settings = new Instructions(); - settings.Height = height; + using (var outputStream = _diskProvider.OpenWriteStream(destination)) + { + var settings = new Instructions(); + settings.Height = height; - var job = new ImageJob(sourceStream, outputStream, settings); + var job = new ImageJob(sourceStream, outputStream, settings); - ImageBuilder.Current.Build(job); + ImageBuilder.Current.Build(job); + } } } + catch + { + if (_diskProvider.FileExists(destination)) + { + _diskProvider.DeleteFile(destination); + } + throw; + } } } } diff --git a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs index 439494454..d72916f7d 100644 --- a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs +++ b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs @@ -88,16 +88,13 @@ private void EnsureCovers(Series series) foreach (var cover in series.Images) { var fileName = GetCoverPath(series.Id, cover.CoverType); + var alreadyExists = false; try { - if (!_coverExistsSpecification.AlreadyExists(cover.Url, fileName)) + alreadyExists = _coverExistsSpecification.AlreadyExists(cover.Url, fileName); + if (!alreadyExists) { DownloadCover(series, cover); - EnsureResizedCovers(series, cover, true); - } - else - { - EnsureResizedCovers(series, cover, false); } } catch (WebException e) @@ -108,6 +105,15 @@ private void EnsureCovers(Series series) { _logger.ErrorException("Couldn't download media cover for " + series, e); } + + try + { + EnsureResizedCovers(series, cover, !alreadyExists); + } + catch (Exception e) + { + _logger.ErrorException("Couldn't resize media cover for " + series + " using full size image instead.", e); + } } } @@ -148,7 +154,7 @@ private void EnsureResizedCovers(Series series, MediaCover cover, bool forceResi var mainFileName = GetCoverPath(series.Id, cover.CoverType); var resizeFileName = GetCoverPath(series.Id, cover.CoverType, height); - if (forceResize || !_diskProvider.FileExists(resizeFileName)) + if (forceResize || !_diskProvider.FileExists(resizeFileName) || _diskProvider.GetFileSize(resizeFileName) == 0) { _logger.Debug("Resizing {0}-{1} for {2}", cover.CoverType, height, series);