New: Banner will be deleted when series is deleted.

NzbDrone.Common/PathExtentions.cs

@@ -97,7 +97,7 @@ namespace NzbDrone.Common
             return Path.Combine(enviromentProvider.GetMediaCoverPath(), "Banners");
         }
 
-        public static string GetFanArthPath(this EnviromentProvider enviromentProvider)
+        public static string GetFanArtPath(this EnviromentProvider enviromentProvider)
         {
             return Path.Combine(enviromentProvider.GetMediaCoverPath(), "Fanarts");
         }

NzbDrone.Core.Test/JobTests/BannerDownloadJobTest.cs

@@ -18,199 +18,79 @@ namespace NzbDrone.Core.Test.JobTests
     // ReSharper disable InconsistentNaming
     public class BannerDownloadJobTest : CoreTest
     {
+        private ProgressNotification _notification;
 
         [SetUp]
         public void Setup()
         {
-            WithStrictMocker();
+            _notification = new ProgressNotification("Test");
             WithTempAsAppPath();
         }
 
-        [Test]
+        private void WithSuccessfulDownload()
-        public void BannerDownload_all()
         {
-            //Setup
+            Mocker.GetMock<BannerProvider>()
-            var fakeSeries = Builder<Series>.CreateListOfSize(10)
+                .Setup(s => s.Download(It.IsAny<ProgressNotification>(), It.IsAny<Series>()))
-                .Build();
+                    .Returns(true);
-
-            var notification = new ProgressNotification("Banner Download");
-
-            Mocker.GetMock<SeriesProvider>()
-                .Setup(c => c.GetAllSeries())
-                .Returns(fakeSeries);
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()));
-
-            Mocker.GetMock<DiskProvider>()
-                .Setup(S => S.CreateDirectory(It.IsAny<string>()))
-                .Returns("");
-
-            //Act
-            Mocker.Resolve<BannerDownloadJob>().Start(notification, 0, 0);
-
-            //Assert
-            Mocker.VerifyAllMocks();
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Exactly(fakeSeries.Count));
         }
 
-        [Test]
+        private void WithFailedDownload()
-        public void BannerDownload_some_null_BannerUrl()
         {
-            //Setup
+            Mocker.GetMock<BannerProvider>()
-            var fakeSeries = Builder<Series>.CreateListOfSize(10)
+                .Setup(s => s.Download(It.IsAny<ProgressNotification>(), It.IsAny<Series>()))
-                .Random(2)
+                    .Returns(false);
-                .With(s => s.BannerUrl = null)
-                .Build();
-
-          var notification = new ProgressNotification("Banner Download");
-
-            Mocker.GetMock<SeriesProvider>()
-                .Setup(c => c.GetAllSeries())
-                .Returns(fakeSeries);
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()));
-
-            Mocker.GetMock<DiskProvider>()
-                .Setup(S => S.CreateDirectory(It.IsAny<string>()))
-                .Returns("");
-
-            //Act
-            Mocker.Resolve<BannerDownloadJob>().Start(notification, 0, 0);
-
-            //Assert
-            Mocker.VerifyAllMocks();
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Exactly(8));
         }
 
-        [Test]
+        private void VerifyDownloadMock(int times)
-        public void BannerDownload_some_failed_download()
         {
-            //Setup
+            Mocker.GetMock<BannerProvider>().Verify(v => v.Download(_notification, It.IsAny<Series>()), Times.Exactly(times));
-            var fakeSeries = Builder<Series>.CreateListOfSize(4)
-                .Build();
-
-
-            var bannerPath = Mocker.GetMock<EnviromentProvider>().Object.GetBannerPath();
-
-            var notification = new ProgressNotification("Banner Download");
-
-            Mocker.GetMock<SeriesProvider>()
-                .Setup(c => c.GetAllSeries())
-                .Returns(fakeSeries);
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), Path.Combine(bannerPath, "1.jpg")))
-                .Throws(new WebException());
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), Path.Combine(bannerPath, "2.jpg")));
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), Path.Combine(bannerPath, "3.jpg")))
-                .Throws(new WebException());
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), Path.Combine(bannerPath, "4.jpg")));
-
-            Mocker.GetMock<DiskProvider>()
-                .Setup(S => S.CreateDirectory(It.IsAny<string>()))
-                .Returns("");
-
-            //Act
-            Mocker.Resolve<BannerDownloadJob>().Start(notification, 0, 0);
-
-            //Assert
-            Mocker.VerifyAllMocks();
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Exactly(fakeSeries.Count));
         }
 
         [Test]
-        public void BannerDownload_all_failed_download()
+        public void Start_should_download_banners_for_all_series_when_no_targetId_is_passed_in()
         {
-            //Setup
+            WithSuccessfulDownload();
-            var fakeSeries = Builder<Series>.CreateListOfSize(10)
-                .Build();
-
-            var notification = new ProgressNotification("Banner Download");
-
-            Mocker.GetMock<SeriesProvider>()
-                .Setup(c => c.GetAllSeries())
-                .Returns(fakeSeries);
-
-            Mocker.GetMock<HttpProvider>()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()))
-                .Throws(new WebException());
 
-            Mocker.GetMock<DiskProvider>()
+            var series = Builder<Series>.CreateListOfSize(5)
-                .Setup(S => S.CreateDirectory(It.IsAny<string>()))
+                    .Build();
-                .Returns("");
 
-            //Act
+            Mocker.GetMock<SeriesProvider>().Setup(s => s.GetAllSeries())
-            Mocker.Resolve<BannerDownloadJob>().Start(notification, 0, 0);
+                    .Returns(series);
 
-            //Assert
+            Mocker.Resolve<BannerDownloadJob>().Start(_notification, 0, 0);
-            Mocker.VerifyAllMocks();
+            VerifyDownloadMock(series.Count);
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Exactly(fakeSeries.Count));
         }
 
         [Test]
-        public void BannerDownload_single_banner()
+        public void Start_should_only_attempt_to_download_for_series_with_banner_url()
         {
-            //Setup
+            WithSuccessfulDownload();
-            var fakeSeries = Builder<Series>.CreateNew()
-                .With(s => s.SeriesId = 1)
-                .Build();
 
-            var notification = new ProgressNotification("Banner Download");
+            var series = Builder<Series>.CreateListOfSize(5)
+                    .TheFirst(2)
+                    .With(s => s.BannerUrl = null)
+                    .Build();
 
-            Mocker.GetMock<SeriesProvider>()
+            Mocker.GetMock<SeriesProvider>().Setup(s => s.GetAllSeries())
-                .Setup(c => c.GetSeries(1))
+                    .Returns(series);
-                .Returns(fakeSeries);
 
-            Mocker.GetMock<HttpProvider>()
+            Mocker.Resolve<BannerDownloadJob>().Start(_notification, 0, 0);
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()))
+            VerifyDownloadMock(3);
-                .Throws(new WebException());
-
-            Mocker.GetMock<DiskProvider>()
-                .Setup(S => S.CreateDirectory(It.IsAny<string>()))
-                .Returns("");
-
-            //Act
-            Mocker.Resolve<BannerDownloadJob>().Start(notification, 1, 0);
-
-            //Assert
-            Mocker.VerifyAllMocks();
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Once());
         }
 
         [Test]
-        public void Download_Banner()
+        public void Start_should_download_single_banner_when_seriesId_is_passed_in()
         {
-            //Setup
+            WithSuccessfulDownload();
-            var fakeSeries = Builder<Series>.CreateNew()
-                .With(s => s.SeriesId = 1)
-                .Build();
-
-            var notification = new ProgressNotification("Banner Download");
 
-            Mocker.GetMock<HttpProvider>()
+            var series = Builder<Series>.CreateNew()
-                .Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()))
+                    .Build();
-                .Throws(new WebException());
 
-            //Act
+            Mocker.GetMock<SeriesProvider>().Setup(s => s.GetSeries(series.SeriesId))
-            Mocker.Resolve<BannerDownloadJob>().DownloadBanner(notification, fakeSeries);
+                    .Returns(series);
 
-            //Assert
+            Mocker.Resolve<BannerDownloadJob>().Start(_notification, 1, 0);
-            Mocker.VerifyAllMocks();
+            VerifyDownloadMock(1);
-            Mocker.GetMock<HttpProvider>().Verify(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()),
-                                                       Times.Once());
         }
     }
 }

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

@@ -111,6 +111,7 @@
     <Compile Include="JobTests\RecentBacklogSearchJobTest.cs" />
     <Compile Include="ProviderTests\AnalyticsProviderTests\AnalyticsProviderFixture.cs" />
     <Compile Include="ProviderTests\ConfigProviderTests\ConfigCachingFixture.cs" />
+    <Compile Include="ProviderTests\BannerProviderTest.cs" />
     <Compile Include="ProviderTests\PlexProviderTest.cs" />
     <Compile Include="ProviderTests\SeasonProviderTest.cs" />
     <Compile Include="ProviderTests\DecisionEngineTests\RetentionSpecificationFixture.cs" />

NzbDrone.Core.Test/ProviderTests/BannerProviderTest.cs

@@ -0,0 +1,102 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Net;
+using FizzWare.NBuilder;
+using FluentAssertions;
+using Moq;
+using NUnit.Framework;
+using NzbDrone.Common;
+using NzbDrone.Core.Model.Notification;
+using NzbDrone.Core.Providers;
+using NzbDrone.Core.Repository;
+using NzbDrone.Core.Repository.Quality;
+using NzbDrone.Core.Test.Framework;
+using NzbDrone.Test.Common.AutoMoq;
+
+namespace NzbDrone.Core.Test.ProviderTests
+{
+    [TestFixture]
+    // ReSharper disable InconsistentNaming
+    public class BannerProviderTest : CoreTest
+    {
+        private Series _series;
+        private ProgressNotification _notification;
+
+        [SetUp]
+        public void Setup()
+        {
+            _series = Builder<Series>.CreateNew()
+                .With(s => s.SeriesId = 12345)
+                    .Build();
+
+            _notification = new ProgressNotification("Test");
+
+            var path = @"C:\Windows\Temp";
+
+            Mocker.GetMock<DiskProvider>().Setup(s => s.CreateDirectory(path));
+        }
+
+        private void WithSuccessfulDownload()
+        {
+            Mocker.GetMock<HttpProvider>().Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()));
+        }
+
+        private void WithFailedDownload()
+        {
+            Mocker.GetMock<HttpProvider>().Setup(s => s.DownloadFile(It.IsAny<string>(), It.IsAny<string>()))
+                    .Throws(new WebException("Failed to download file (Mocked)"));
+        }
+
+        [Test]
+        public void Download_should_return_true_when_banner_is_downloaded_successfully()
+        {
+            WithSuccessfulDownload();
+            var result = Mocker.Resolve<BannerProvider>().Download(_notification, _series);
+            result.Should().BeTrue();
+        }
+
+        [Test]
+        public void Download_should_return_false_when_banner_download_fails()
+        {
+            WithFailedDownload();
+            var result = Mocker.Resolve<BannerProvider>().Download(_notification, _series);
+            result.Should().BeFalse();
+        }
+
+        [Test]
+        public void Delete_should_delete_banner_file_when_it_exists()
+        {
+            Mocker.GetMock<DiskProvider>().Setup(s => s.FileExists(It.IsAny<string>()))
+                    .Returns(true);
+
+            Mocker.GetMock<DiskProvider>().Setup(s => s.DeleteFile(It.IsAny<string>()));
+
+            var result = Mocker.Resolve<BannerProvider>().Delete(1);
+            result.Should().BeTrue();
+        }
+
+        [Test]
+        public void Delete_should_return_true_even_when_file_sint_deleted()
+        {
+            Mocker.GetMock<DiskProvider>().Setup(s => s.FileExists(It.IsAny<string>()))
+                    .Returns(false);
+
+            var result = Mocker.Resolve<BannerProvider>().Delete(1);
+            result.Should().BeTrue();
+        }
+
+        [Test]
+        public void Delete_should_return_false_when_file_fails_to_delete()
+        {
+            Mocker.GetMock<DiskProvider>().Setup(s => s.FileExists(It.IsAny<string>()))
+                    .Returns(true);
+
+            Mocker.GetMock<DiskProvider>().Setup(s => s.DeleteFile(It.IsAny<string>()))
+                .Throws(new SystemException("File not found."));
+
+            var result = Mocker.Resolve<BannerProvider>().Delete(1);
+            result.Should().BeFalse();
+        }
+    }
+}

NzbDrone.Core/Jobs/BannerDownloadJob.cs

@@ -14,21 +14,16 @@ namespace NzbDrone.Core.Jobs
     public class BannerDownloadJob : IJob
     {
         private readonly SeriesProvider _seriesProvider;
-        private readonly HttpProvider _httpProvider;
+        private readonly BannerProvider _bannerProvider;
-        private readonly DiskProvider _diskProvider;
-        private readonly EnviromentProvider _enviromentProvider;
         private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
 
-        private const string _bannerUrlPrefix = "http://www.thetvdb.com/banners/";
+        private const string BANNER_URL_PREFIX = "http://www.thetvdb.com/banners/";
 
         [Inject]
-        public BannerDownloadJob(SeriesProvider seriesProvider, HttpProvider httpProvider, DiskProvider diskProvider,
+        public BannerDownloadJob(SeriesProvider seriesProvider, BannerProvider bannerProvider)
-            EnviromentProvider enviromentProvider)
         {
             _seriesProvider = seriesProvider;
-            _httpProvider = httpProvider;
+            _bannerProvider = bannerProvider;
-            _diskProvider = diskProvider;
-            _enviromentProvider = enviromentProvider;
         }
 
         public BannerDownloadJob()
@@ -49,15 +44,12 @@ namespace NzbDrone.Core.Jobs
         {
             Logger.Debug("Starting banner download job");
 
-
-            _diskProvider.CreateDirectory(_enviromentProvider.GetBannerPath());
-
             if (targetId > 0)
             {
                 var series = _seriesProvider.GetSeries(targetId);
 
                 if (series != null && !String.IsNullOrEmpty(series.BannerUrl))
-                    DownloadBanner(notification, series);
+                    _bannerProvider.Download(notification, series);
 
                 return;
             }
@@ -66,28 +58,10 @@ namespace NzbDrone.Core.Jobs
 
             foreach (var series in seriesInDb.Where(s => !String.IsNullOrEmpty(s.BannerUrl)))
             {
-                DownloadBanner(notification, series);
+                _bannerProvider.Download(notification, series);
             }
 
             Logger.Debug("Finished banner download job");
         }
-
-        public virtual void DownloadBanner(ProgressNotification notification, Series series)
-        {
-            var bannerFilename = Path.Combine(_enviromentProvider.GetBannerPath(), series.SeriesId.ToString()) + ".jpg";
-
-            notification.CurrentMessage = string.Format("Downloading banner for '{0}'", series.Title);
-
-            try
-            {
-                _httpProvider.DownloadFile(_bannerUrlPrefix + series.BannerUrl, bannerFilename);
-                notification.CurrentMessage = string.Format("Successfully download banner for '{0}'", series.Title);
-            }
-            catch (Exception)
-            {
-                Logger.Debug("Failed to download banner for '{0}'", series.Title);
-                notification.CurrentMessage = string.Format("Failed to download banner for '{0}'", series.Title);
-            }
-        }
     }
 }

NzbDrone.Core/NzbDrone.Core.csproj

@@ -271,6 +271,7 @@
     <Compile Include="Model\Xbmc\TvShowResult.cs" />
     <Compile Include="Model\Xbmc\ErrorResult.cs" />
     <Compile Include="Model\Xbmc\IconType.cs" />
+    <Compile Include="Providers\BannerProvider.cs" />
     <Compile Include="Providers\SeasonProvider.cs" />
     <Compile Include="Jobs\RecentBacklogSearchJob.cs" />
     <Compile Include="Jobs\TrimLogsJob.cs" />

NzbDrone.Core/Providers/BannerProvider.cs

@@ -0,0 +1,86 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using NLog;
+using NzbDrone.Common;
+using NzbDrone.Core.Model.Notification;
+using NzbDrone.Core.Repository;
+
+namespace NzbDrone.Core.Providers
+{
+    public class BannerProvider
+    {
+        private readonly HttpProvider _httpProvider;
+        private readonly EnviromentProvider _enviromentProvider;
+        private readonly DiskProvider _diskProvider;
+        private static readonly Logger logger = LogManager.GetCurrentClassLogger();
+
+        private const string BANNER_URL_PREFIX = "http://www.thetvdb.com/banners/";
+
+        public BannerProvider(HttpProvider httpProvider, EnviromentProvider enviromentProvider,
+                                DiskProvider diskProvider)
+        {
+            _httpProvider = httpProvider;
+            _enviromentProvider = enviromentProvider;
+            _diskProvider = diskProvider;
+        }
+
+        public BannerProvider()
+        {
+            
+        }
+
+        public virtual bool Download(ProgressNotification notification, Series series)
+        {
+            var bannerPath = _enviromentProvider.GetBannerPath();
+            
+            logger.Trace("Ensuring Banner Folder exists: ", bannerPath);
+            _diskProvider.CreateDirectory(bannerPath);
+
+            var bannerFilename = Path.Combine(bannerPath, series.SeriesId.ToString()) + ".jpg";
+
+            notification.CurrentMessage = string.Format("Downloading banner for '{0}'", series.Title);
+            logger.Trace("Downloading banner for '{0}'", series.Title);
+
+            try
+            {
+                _httpProvider.DownloadFile(BANNER_URL_PREFIX + series.BannerUrl, bannerFilename);
+                notification.CurrentMessage = string.Format("Successfully download banner for '{0}'", series.Title);
+                logger.Trace("Successfully download banner for '{0}'", series.Title);
+            }
+            catch (Exception)
+            {
+                logger.Debug("Failed to download banner for '{0}'", series.Title);
+                notification.CurrentMessage = string.Format("Failed to download banner for '{0}'", series.Title);
+                return false;
+            }
+
+            return true;
+        }
+
+        public virtual bool Delete(int seriesId)
+        {
+            var bannerPath = _enviromentProvider.GetBannerPath();
+            var bannerFilename = Path.Combine(bannerPath, seriesId.ToString()) + ".jpg";
+
+            try
+            {
+                logger.Trace("Checking if banner exists: {0}", bannerFilename);
+
+                if (_diskProvider.FileExists(bannerFilename))
+                {
+                    logger.Trace("Deleting existing banner: {0}", bannerFilename);
+                    _diskProvider.DeleteFile(bannerFilename);
+                }
+            }
+            catch(Exception ex)
+            {
+                logger.WarnException("Failed to delete banner: " + bannerFilename, ex);
+                return false;
+            }
+            return true;
+        }
+    }
+}

NzbDrone.Core/Providers/SeriesProvider.cs

@@ -19,15 +19,18 @@ namespace NzbDrone.Core.Providers
         private readonly TvDbProvider _tvDbProvider;
         private readonly IDatabase _database;
         private readonly SceneMappingProvider _sceneNameMappingProvider;
+        private readonly BannerProvider _bannerProvider;
         private static readonly Regex TimeRegex = new Regex(@"^(?<time>\d+:?\d*)\W*(?<meridiem>am|pm)?", RegexOptions.IgnoreCase | RegexOptions.Compiled);
 
         public SeriesProvider(IDatabase database, ConfigProvider configProviderProvider,
-                                TvDbProvider tvDbProviderProvider, SceneMappingProvider sceneNameMappingProvider)
+                                TvDbProvider tvDbProviderProvider, SceneMappingProvider sceneNameMappingProvider,
+                                BannerProvider bannerProvider)
         {
             _database = database;
             _configProvider = configProviderProvider;
             _tvDbProvider = tvDbProviderProvider;
             _sceneNameMappingProvider = sceneNameMappingProvider;
+            _bannerProvider = bannerProvider;
         }
 
         public SeriesProvider()
@@ -188,6 +191,9 @@ namespace NzbDrone.Core.Providers
 
                 tran.Complete();
             }
+
+            Logger.Trace("Beginning deletion of banner for SeriesID: ", seriesId);
+            _bannerProvider.Delete(seriesId);
         }
 
         public virtual bool SeriesPathExists(string path)