diff --git a/src/NzbDrone.Core.Test/Download/DownloadApprovedReportsTests/DownloadApprovedFixture.cs b/src/NzbDrone.Core.Test/Download/DownloadApprovedReportsTests/DownloadApprovedFixture.cs index 529c1e355..76d22d669 100644 --- a/src/NzbDrone.Core.Test/Download/DownloadApprovedReportsTests/DownloadApprovedFixture.cs +++ b/src/NzbDrone.Core.Test/Download/DownloadApprovedReportsTests/DownloadApprovedFixture.cs @@ -6,6 +6,7 @@ using Moq; using NUnit.Framework; using NzbDrone.Core.DecisionEngine; using NzbDrone.Core.Download; +using NzbDrone.Core.Download.Pending; using NzbDrone.Core.Parser.Model; using NzbDrone.Core.Profiles; using NzbDrone.Core.Qualities; @@ -182,5 +183,47 @@ namespace NzbDrone.Core.Test.Download.DownloadApprovedReportsTests Subject.GetQualifiedReports(decisions).Should().BeEmpty(); } + + [Test] + public void should_not_grab_if_pending() + { + var episodes = new List { GetEpisode(1) }; + var remoteEpisode = GetRemoteEpisode(episodes, new QualityModel(Quality.HDTV720p)); + + var decisions = new List(); + decisions.Add(new DownloadDecision(remoteEpisode, new Rejection("Failure!", RejectionType.Temporary))); + decisions.Add(new DownloadDecision(remoteEpisode)); + + Subject.ProcessDecisions(decisions); + Mocker.GetMock().Verify(v => v.DownloadReport(It.IsAny()), Times.Never()); + } + + [Test] + public void should_not_add_to_pending_if_episode_was_grabbed() + { + var episodes = new List { GetEpisode(1) }; + var remoteEpisode = GetRemoteEpisode(episodes, new QualityModel(Quality.HDTV720p)); + + var decisions = new List(); + decisions.Add(new DownloadDecision(remoteEpisode)); + decisions.Add(new DownloadDecision(remoteEpisode, new Rejection("Failure!", RejectionType.Temporary))); + + Subject.ProcessDecisions(decisions); + Mocker.GetMock().Verify(v => v.Add(It.IsAny()), Times.Never()); + } + + [Test] + public void should_add_to_pending_even_if_already_added_to_pending() + { + var episodes = new List { GetEpisode(1) }; + var remoteEpisode = GetRemoteEpisode(episodes, new QualityModel(Quality.HDTV720p)); + + var decisions = new List(); + decisions.Add(new DownloadDecision(remoteEpisode, new Rejection("Failure!", RejectionType.Temporary))); + decisions.Add(new DownloadDecision(remoteEpisode, new Rejection("Failure!", RejectionType.Temporary))); + + Subject.ProcessDecisions(decisions); + Mocker.GetMock().Verify(v => v.Add(It.IsAny()), Times.Exactly(2)); + } } } diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/ProcessFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs similarity index 97% rename from src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/ProcessFixture.cs rename to src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs index a913b62b1..e7b95035d 100644 --- a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/ProcessFixture.cs +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs @@ -1,9 +1,6 @@ using System; using System.Collections.Generic; -using System.Linq; -using System.Text; using FizzWare.NBuilder; -using FluentAssertions; using Marr.Data; using Moq; using NUnit.Framework; @@ -20,7 +17,7 @@ using NzbDrone.Test.Common; namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests { [TestFixture] - public class ProcessFixture : CoreTest + public class AddFixture : CoreTest { private DownloadDecision _temporarilyRejected; private Series _series; @@ -140,6 +137,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests VerifyInsert(); } + [Test] public void should_add_if_publish_date_is_different() { diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs new file mode 100644 index 000000000..424f61241 --- /dev/null +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs @@ -0,0 +1,143 @@ +using System; +using System.Collections.Generic; +using FizzWare.NBuilder; +using Marr.Data; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.DecisionEngine; +using NzbDrone.Core.Download.Pending; +using NzbDrone.Core.Parser; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Profiles; +using NzbDrone.Core.Qualities; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Tv; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests +{ + [TestFixture] + public class RemoveGrabbedFixture : CoreTest + { + private DownloadDecision _temporarilyRejected; + private Series _series; + private Episode _episode; + private Profile _profile; + private ReleaseInfo _release; + private ParsedEpisodeInfo _parsedEpisodeInfo; + private RemoteEpisode _remoteEpisode; + + [SetUp] + public void Setup() + { + _series = Builder.CreateNew() + .Build(); + + _episode = Builder.CreateNew() + .Build(); + + _profile = new Profile + { + Name = "Test", + Cutoff = Quality.HDTV720p, + GrabDelay = 1, + Items = new List + { + new ProfileQualityItem { Allowed = true, Quality = Quality.HDTV720p }, + new ProfileQualityItem { Allowed = true, Quality = Quality.WEBDL720p }, + new ProfileQualityItem { Allowed = true, Quality = Quality.Bluray720p } + }, + }; + + _series.Profile = new LazyLoaded(_profile); + + _release = Builder.CreateNew().Build(); + + _parsedEpisodeInfo = Builder.CreateNew().Build(); + _parsedEpisodeInfo.Quality = new QualityModel(Quality.HDTV720p); + + _remoteEpisode = new RemoteEpisode(); + _remoteEpisode.Episodes = new List{ _episode }; + _remoteEpisode.Series = _series; + _remoteEpisode.ParsedEpisodeInfo = _parsedEpisodeInfo; + _remoteEpisode.Release = _release; + + _temporarilyRejected = new DownloadDecision(_remoteEpisode, new Rejection("Temp Rejected", RejectionType.Temporary)); + + Mocker.GetMock() + .Setup(s => s.All()) + .Returns(new List()); + + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny())) + .Returns(_series); + + Mocker.GetMock() + .Setup(s => s.GetEpisodes(It.IsAny(), _series, true, null)) + .Returns(new List {_episode}); + + Mocker.GetMock() + .Setup(s => s.PrioritizeDecisions(It.IsAny>())) + .Returns((List d) => d); + } + + private void GivenHeldRelease(QualityModel quality) + { + var parsedEpisodeInfo = _parsedEpisodeInfo.JsonClone(); + parsedEpisodeInfo.Quality = quality; + + var heldReleases = Builder.CreateListOfSize(1) + .All() + .With(h => h.SeriesId = _series.Id) + .With(h => h.Release = _release.JsonClone()) + .With(h => h.ParsedEpisodeInfo = parsedEpisodeInfo) + .Build(); + + Mocker.GetMock() + .Setup(s => s.All()) + .Returns(heldReleases); + } + + [Test] + public void should_delete_if_the_grabbed_quality_is_the_same() + { + GivenHeldRelease(_parsedEpisodeInfo.Quality); + + Subject.RemoveGrabbed(new List { _temporarilyRejected }); + + VerifyDelete(); + } + + [Test] + public void should_delete_if_the_grabbed_quality_is_the_higher() + { + GivenHeldRelease(new QualityModel(Quality.SDTV)); + + Subject.RemoveGrabbed(new List { _temporarilyRejected }); + + VerifyDelete(); + } + + [Test] + public void should_not_delete_if_the_grabbed_quality_is_the_lower() + { + GivenHeldRelease(new QualityModel(Quality.Bluray720p)); + + Subject.RemoveGrabbed(new List { _temporarilyRejected }); + + VerifyNoDelete(); + } + + private void VerifyDelete() + { + Mocker.GetMock() + .Verify(v => v.Delete(It.IsAny()), Times.Once()); + } + + private void VerifyNoDelete() + { + Mocker.GetMock() + .Verify(v => v.Delete(It.IsAny()), Times.Never()); + } + } +} diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs new file mode 100644 index 000000000..fa7560422 --- /dev/null +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs @@ -0,0 +1,155 @@ +using System; +using System.Collections.Generic; +using FizzWare.NBuilder; +using Marr.Data; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.DecisionEngine; +using NzbDrone.Core.Download.Pending; +using NzbDrone.Core.Parser; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Profiles; +using NzbDrone.Core.Qualities; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Tv; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests +{ + [TestFixture] + public class RemoveRejectedFixture : CoreTest + { + private DownloadDecision _temporarilyRejected; + private Series _series; + private Episode _episode; + private Profile _profile; + private ReleaseInfo _release; + private ParsedEpisodeInfo _parsedEpisodeInfo; + private RemoteEpisode _remoteEpisode; + + [SetUp] + public void Setup() + { + _series = Builder.CreateNew() + .Build(); + + _episode = Builder.CreateNew() + .Build(); + + _profile = new Profile + { + Name = "Test", + Cutoff = Quality.HDTV720p, + GrabDelay = 1, + Items = new List + { + new ProfileQualityItem { Allowed = true, Quality = Quality.HDTV720p }, + new ProfileQualityItem { Allowed = true, Quality = Quality.WEBDL720p }, + new ProfileQualityItem { Allowed = true, Quality = Quality.Bluray720p } + }, + }; + + _series.Profile = new LazyLoaded(_profile); + + _release = Builder.CreateNew().Build(); + + _parsedEpisodeInfo = Builder.CreateNew().Build(); + _parsedEpisodeInfo.Quality = new QualityModel(Quality.HDTV720p); + + _remoteEpisode = new RemoteEpisode(); + _remoteEpisode.Episodes = new List{ _episode }; + _remoteEpisode.Series = _series; + _remoteEpisode.ParsedEpisodeInfo = _parsedEpisodeInfo; + _remoteEpisode.Release = _release; + + _temporarilyRejected = new DownloadDecision(_remoteEpisode, new Rejection("Temp Rejected", RejectionType.Temporary)); + + Mocker.GetMock() + .Setup(s => s.All()) + .Returns(new List()); + + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny())) + .Returns(_series); + + Mocker.GetMock() + .Setup(s => s.GetEpisodes(It.IsAny(), _series, true, null)) + .Returns(new List {_episode}); + + Mocker.GetMock() + .Setup(s => s.PrioritizeDecisions(It.IsAny>())) + .Returns((List d) => d); + } + + private void GivenHeldRelease(String title, String indexer, DateTime publishDate) + { + var release = _release.JsonClone(); + release.Indexer = indexer; + release.PublishDate = publishDate; + + + var heldReleases = Builder.CreateListOfSize(1) + .All() + .With(h => h.SeriesId = _series.Id) + .With(h => h.Title = title) + .With(h => h.Release = release) + .Build(); + + Mocker.GetMock() + .Setup(s => s.All()) + .Returns(heldReleases); + } + + [Test] + public void should_remove_if_it_is_the_same_release_from_the_same_indexer() + { + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate); + + Subject.RemoveRejected(new List { _temporarilyRejected }); + + VerifyDelete(); + } + + [Test] + public void should_not_remove_if_title_is_different() + { + GivenHeldRelease(_release.Title + "-RP", _release.Indexer, _release.PublishDate); + + Subject.RemoveRejected(new List { _temporarilyRejected }); + + VerifyNoDelete(); + } + + [Test] + public void should_not_remove_if_indexer_is_different() + { + GivenHeldRelease(_release.Title, "AnotherIndexer", _release.PublishDate); + + Subject.RemoveRejected(new List { _temporarilyRejected }); + + VerifyNoDelete(); + } + + [Test] + public void should_not_remove_if_publish_date_is_different() + { + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate.AddHours(1)); + + Subject.RemoveRejected(new List { _temporarilyRejected }); + + VerifyNoDelete(); + } + + private void VerifyDelete() + { + Mocker.GetMock() + .Verify(v => v.Delete(It.IsAny()), Times.Once()); + } + + private void VerifyNoDelete() + { + Mocker.GetMock() + .Verify(v => v.Delete(It.IsAny()), Times.Never()); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index 0c0179034..a60c289e9 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -135,7 +135,9 @@ - + + + diff --git a/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs b/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs index d60e84e58..9be579624 100644 --- a/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs +++ b/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using NLog; +using NzbDrone.Common; using NzbDrone.Core.DecisionEngine; using NzbDrone.Core.Messaging.Events; using NzbDrone.Core.Parser; @@ -77,15 +78,30 @@ namespace NzbDrone.Core.Download.Pending var decisionLocal = decision; var episodeIds = decisionLocal.RemoteEpisode.Episodes.Select(e => e.Id); - var existingReports = alreadyPending.Where(r => r.RemoteEpisode.Episodes.Select(e => e.Id) .Intersect(episodeIds) - .Any()); + .Any()) + .ToList(); + + if (existingReports.Empty()) + { + continue; + } + var profile = decisionLocal.RemoteEpisode.Series.Profile.Value; + foreach (var existingReport in existingReports) { - _logger.Debug("Removing previously pending release, as it was grabbed."); - Delete(existingReport); + var compare = new QualityModelComparer(profile).Compare(decision.RemoteEpisode.ParsedEpisodeInfo.Quality, + existingReport.RemoteEpisode.ParsedEpisodeInfo.Quality); + + //Only remove lower/equal quality pending releases + //It is safer to retry these releases on the next round than remove it and try to re-add it (if its still in the feed) + if (compare >= 0) + { + _logger.Debug("Removing previously pending release, as it was grabbed."); + Delete(existingReport); + } } } } diff --git a/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs b/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs index 4dab30611..e1250e798 100644 --- a/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs +++ b/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs @@ -35,14 +35,21 @@ namespace NzbDrone.Core.Download { var qualifiedReports = GetQualifiedReports(decisions); var prioritizedDecisions = _prioritizeDownloadDecision.PrioritizeDecisions(qualifiedReports); - var downloadedReports = new List(); - var pendingReports = new List(); + var grabbed = new List(); + var pending = new List(); foreach (var report in prioritizedDecisions) { var remoteEpisode = report.RemoteEpisode; - if (DownloadingOrPending(downloadedReports, pendingReports, remoteEpisode)) + var episodeIds = remoteEpisode.Episodes.Select(e => e.Id).ToList(); + + //Skip if already grabbed + if (grabbed.SelectMany(r => r.RemoteEpisode.Episodes) + .Select(e => e.Id) + .ToList() + .Intersect(episodeIds) + .Any()) { continue; } @@ -50,23 +57,33 @@ namespace NzbDrone.Core.Download if (report.TemporarilyRejected) { _pendingReleaseService.Add(report); - pendingReports.Add(report); + pending.Add(report); + continue; + } + + if (pending.SelectMany(r => r.RemoteEpisode.Episodes) + .Select(e => e.Id) + .ToList() + .Intersect(episodeIds) + .Any()) + { continue; } try { _downloadService.DownloadReport(remoteEpisode); - downloadedReports.Add(report); + grabbed.Add(report); } catch (Exception e) { //TODO: support for store & forward + //We'll need to differentiate between a download client error and an indexer error _logger.WarnException("Couldn't add report to download queue. " + remoteEpisode, e); } } - return new ProcessedDecisions(downloadedReports, pendingReports); + return new ProcessedDecisions(grabbed, pending); } internal List GetQualifiedReports(IEnumerable decisions) @@ -74,30 +91,5 @@ namespace NzbDrone.Core.Download //Process both approved and temporarily rejected return decisions.Where(c => (c.Approved || c.TemporarilyRejected) && c.RemoteEpisode.Episodes.Any()).ToList(); } - - private bool DownloadingOrPending(List downloading, List pending, RemoteEpisode remoteEpisode) - { - var episodeIds = remoteEpisode.Episodes.Select(e => e.Id).ToList(); - - if (downloading.SelectMany(r => r.RemoteEpisode.Episodes) - .Select(e => e.Id) - .ToList() - .Intersect(episodeIds) - .Any()) - { - return true; - } - - if (pending.SelectMany(r => r.RemoteEpisode.Episodes) - .Select(e => e.Id) - .ToList() - .Intersect(episodeIds) - .Any()) - { - return true; - } - - return false; - } } }