From fe5c52602a16bbe84d899d19c2d112885ca3a4de Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Wed, 4 Jan 2023 13:41:12 -0800 Subject: [PATCH] Fixed: Custom Formats upgrading even when profile doesn't allow upgrades Closes #5330 --- .../UpgradeAllowedSpecificationFixture .cs | 214 ++++++++++-------- .../Specifications/QueueSpecification.cs | 7 +- .../Specifications/UpgradableSpecification.cs | 19 +- .../UpgradeAllowedSpecification.cs | 13 +- 4 files changed, 145 insertions(+), 108 deletions(-) diff --git a/src/NzbDrone.Core.Test/DecisionEngineTests/UpgradeAllowedSpecificationFixture .cs b/src/NzbDrone.Core.Test/DecisionEngineTests/UpgradeAllowedSpecificationFixture .cs index 315636e32..7dbcddc91 100644 --- a/src/NzbDrone.Core.Test/DecisionEngineTests/UpgradeAllowedSpecificationFixture .cs +++ b/src/NzbDrone.Core.Test/DecisionEngineTests/UpgradeAllowedSpecificationFixture .cs @@ -1,179 +1,211 @@ +using System.Collections.Generic; using FluentAssertions; using NUnit.Framework; +using NzbDrone.Core.CustomFormats; using NzbDrone.Core.DecisionEngine.Specifications; -using NzbDrone.Core.Languages; +using NzbDrone.Core.Profiles; using NzbDrone.Core.Profiles.Qualities; using NzbDrone.Core.Qualities; using NzbDrone.Core.Test.Framework; -using NzbDrone.Core.Test.Languages; namespace NzbDrone.Core.Test.DecisionEngineTests { [TestFixture] public class UpgradeAllowedSpecificationFixture : CoreTest { - [Test] - public void should_return_false_when_quality_is_better_languages_are_the_same_and_upgrade_allowed_is_false_for_quality_profile() + private CustomFormat _customFormatOne; + private CustomFormat _customFormatTwo; + private QualityProfile _qualityProfile; + + [SetUp] + public void Setup() { - Subject.IsUpgradeAllowed( - new QualityProfile + _customFormatOne = new CustomFormat + { + Id = 1, + Name = "One" + }; + _customFormatTwo = new CustomFormat + { + Id = 2, + Name = "Two" + }; + + _qualityProfile = new QualityProfile + { + Cutoff = Quality.Bluray1080p.Id, + Items = Qualities.QualityFixture.GetDefaultQualities(), + UpgradeAllowed = false, + CutoffFormatScore = 100, + FormatItems = new List { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = false - }, + new ProfileFormatItem + { + Id = 1, + Format = _customFormatOne, + Score = 50 + }, + new ProfileFormatItem + { + Id = 1, + Format = _customFormatTwo, + Score = 100 + } + } + }; + } + + [Test] + public void should_return_false_when_quality_is_better_custom_formats_are_the_same_and_upgrading_is_not_allowed() + { + _qualityProfile.UpgradeAllowed = false; + + Subject.IsUpgradeAllowed( + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.Bluray1080p)) + new List(), + new QualityModel(Quality.Bluray1080p), + new List()) .Should().BeFalse(); } [Test] - public void should_return_true_for_language_upgrade_when_upgrading_is_allowed() + public void should_return_false_when_quality_is_same_and_custom_format_is_upgrade_and_upgrading_is_not_allowed() { + _qualityProfile.UpgradeAllowed = false; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, + new QualityModel(Quality.DVD), + new List { _customFormatOne }, + new QualityModel(Quality.DVD), + new List { _customFormatTwo }) + .Should().BeFalse(); + } + + [Test] + public void should_return_true_for_custom_format_upgrade_when_upgrading_is_allowed() + { + _qualityProfile.UpgradeAllowed = true; + + Subject.IsUpgradeAllowed( + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List { _customFormatOne }, + new QualityModel(Quality.DVD), + new List { _customFormatTwo }) .Should().BeTrue(); } [Test] - public void should_return_true_for_same_language_when_upgrading_is_allowed() + public void should_return_true_for_same_custom_format_score_when_upgrading_is_not_allowed() { + _qualityProfile.UpgradeAllowed = false; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List { _customFormatOne }, + new QualityModel(Quality.DVD), + new List { _customFormatOne }) .Should().BeTrue(); } [Test] - public void should_return_true_for_same_language_when_upgrading_is_not_allowed() + public void should_return_true_for_lower_custom_format_score_when_upgrading_is_allowed() { - Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, - new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) - .Should().BeTrue(); - } + _qualityProfile.UpgradeAllowed = true; - [Test] - public void should_return_true_for_lower_language_when_upgrading_is_allowed() - { Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List { _customFormatTwo }, + new QualityModel(Quality.DVD), + new List { _customFormatOne }) .Should().BeTrue(); } [Test] public void should_return_true_for_lower_language_when_upgrading_is_not_allowed() { + _qualityProfile.UpgradeAllowed = false; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List { _customFormatTwo }, + new QualityModel(Quality.DVD), + new List { _customFormatOne }) .Should().BeTrue(); } [Test] public void should_return_true_for_quality_upgrade_when_upgrading_is_allowed() { + _qualityProfile.UpgradeAllowed = true; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.Bluray1080p)) + new List(), + new QualityModel(Quality.Bluray1080p), + new List()) .Should().BeTrue(); } [Test] public void should_return_true_for_same_quality_when_upgrading_is_allowed() { + _qualityProfile.UpgradeAllowed = true; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List(), + new QualityModel(Quality.DVD), + new List()) .Should().BeTrue(); } [Test] public void should_return_true_for_same_quality_when_upgrading_is_not_allowed() { + _qualityProfile.UpgradeAllowed = false; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = false - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.DVD)) + new List(), + new QualityModel(Quality.DVD), + new List()) .Should().BeTrue(); } [Test] public void should_return_true_for_lower_quality_when_upgrading_is_allowed() { + _qualityProfile.UpgradeAllowed = true; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = true - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.SDTV)) + new List(), + new QualityModel(Quality.SDTV), + new List()) .Should().BeTrue(); } [Test] public void should_return_true_for_lower_quality_when_upgrading_is_not_allowed() { + _qualityProfile.UpgradeAllowed = false; + Subject.IsUpgradeAllowed( - new QualityProfile - { - Cutoff = Quality.Bluray1080p.Id, - Items = Qualities.QualityFixture.GetDefaultQualities(), - UpgradeAllowed = false - }, + _qualityProfile, new QualityModel(Quality.DVD), - new QualityModel(Quality.SDTV)) + new List(), + new QualityModel(Quality.SDTV), + new List()) .Should().BeTrue(); } } diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/QueueSpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/QueueSpecification.cs index a84ec8cb8..d61500d19 100644 --- a/src/NzbDrone.Core/DecisionEngine/Specifications/QueueSpecification.cs +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/QueueSpecification.cs @@ -4,7 +4,6 @@ using NzbDrone.Core.CustomFormats; using NzbDrone.Core.Download.TrackedDownloads; using NzbDrone.Core.IndexerSearch.Definitions; using NzbDrone.Core.Parser.Model; -using NzbDrone.Core.Profiles.Releases; using NzbDrone.Core.Queue; namespace NzbDrone.Core.DecisionEngine.Specifications @@ -68,7 +67,7 @@ namespace NzbDrone.Core.DecisionEngine.Specifications if (!_upgradableSpecification.IsUpgradable(qualityProfile, remoteEpisode.ParsedEpisodeInfo.Quality, - remoteEpisode.CustomFormats, + queuedItemCustomFormats, subject.ParsedEpisodeInfo.Quality, subject.CustomFormats)) { @@ -79,7 +78,9 @@ namespace NzbDrone.Core.DecisionEngine.Specifications if (!_upgradableSpecification.IsUpgradeAllowed(subject.Series.QualityProfile, remoteEpisode.ParsedEpisodeInfo.Quality, - subject.ParsedEpisodeInfo.Quality)) + queuedItemCustomFormats, + subject.ParsedEpisodeInfo.Quality, + subject.CustomFormats)) { return Decision.Reject("Another release is queued and the Quality profile does not allow upgrades"); } diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradableSpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradableSpecification.cs index caf86023a..c49b3a0f7 100644 --- a/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradableSpecification.cs +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradableSpecification.cs @@ -14,7 +14,7 @@ namespace NzbDrone.Core.DecisionEngine.Specifications bool QualityCutoffNotMet(QualityProfile profile, QualityModel currentQuality, QualityModel newQuality = null); bool CutoffNotMet(QualityProfile profile, QualityModel currentQuality, List currentCustomFormats, QualityModel newQuality = null); bool IsRevisionUpgrade(QualityModel currentQuality, QualityModel newQuality); - bool IsUpgradeAllowed(QualityProfile qualityProfile, QualityModel currentQuality, QualityModel newQuality); + bool IsUpgradeAllowed(QualityProfile qualityProfile, QualityModel currentQuality, List currentCustomFormats, QualityModel newQuality, List newCustomFormats); } public class UpgradableSpecification : IUpgradableSpecification @@ -28,13 +28,6 @@ namespace NzbDrone.Core.DecisionEngine.Specifications _logger = logger; } - private bool IsPreferredWordUpgradable(int currentScore, int newScore) - { - _logger.Debug("Comparing preferred word score. Current: {0} New: {1}", currentScore, newScore); - - return newScore > currentScore; - } - public bool IsUpgradable(QualityProfile qualityProfile, QualityModel currentQuality, List currentCustomFormats, QualityModel newQuality, List newCustomFormats) { var qualityComparer = new QualityModelComparer(qualityProfile); @@ -108,6 +101,7 @@ namespace NzbDrone.Core.DecisionEngine.Specifications private bool CustomFormatCutoffNotMet(QualityProfile profile, List currentFormats) { var score = profile.CalculateCustomFormatScore(currentFormats); + return score < profile.CutoffFormatScore; } @@ -142,17 +136,18 @@ namespace NzbDrone.Core.DecisionEngine.Specifications return false; } - public bool IsUpgradeAllowed(QualityProfile qualityProfile, QualityModel currentQuality, QualityModel newQuality) + public bool IsUpgradeAllowed(QualityProfile qualityProfile, QualityModel currentQuality, List currentCustomFormats, QualityModel newQuality, List newCustomFormats) { var isQualityUpgrade = new QualityModelComparer(qualityProfile).Compare(newQuality, currentQuality) > 0; + var isCustomFormatUpgrade = qualityProfile.CalculateCustomFormatScore(newCustomFormats) > qualityProfile.CalculateCustomFormatScore(currentCustomFormats); - if (isQualityUpgrade && qualityProfile.UpgradeAllowed) + if ((isQualityUpgrade || isCustomFormatUpgrade) && qualityProfile.UpgradeAllowed) { - _logger.Debug("At least one profile allows upgrading"); + _logger.Debug("Quality profile allows upgrading"); return true; } - if (isQualityUpgrade && !qualityProfile.UpgradeAllowed) + if ((isQualityUpgrade || isCustomFormatUpgrade) && !qualityProfile.UpgradeAllowed) { _logger.Debug("Quality profile does not allow upgrades, skipping"); return false; diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradeAllowedSpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradeAllowedSpecification.cs index 48e8c36a8..f83e137cc 100644 --- a/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradeAllowedSpecification.cs +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/UpgradeAllowedSpecification.cs @@ -1,5 +1,6 @@ using System.Linq; using NLog; +using NzbDrone.Core.CustomFormats; using NzbDrone.Core.IndexerSearch.Definitions; using NzbDrone.Core.Parser.Model; @@ -8,11 +9,15 @@ namespace NzbDrone.Core.DecisionEngine.Specifications public class UpgradeAllowedSpecification : IDecisionEngineSpecification { private readonly UpgradableSpecification _upgradableSpecification; + private readonly ICustomFormatCalculationService _formatService; private readonly Logger _logger; - public UpgradeAllowedSpecification(UpgradableSpecification upgradableSpecification, Logger logger) + public UpgradeAllowedSpecification(UpgradableSpecification upgradableSpecification, + ICustomFormatCalculationService formatService, + Logger logger) { _upgradableSpecification = upgradableSpecification; + _formatService = formatService; _logger = logger; } @@ -31,11 +36,15 @@ namespace NzbDrone.Core.DecisionEngine.Specifications continue; } + var fileCustomFormats = _formatService.ParseCustomFormat(file, subject.Series); + _logger.Debug("Comparing file quality with report. Existing file is {0}", file.Quality); if (!_upgradableSpecification.IsUpgradeAllowed(qualityProfile, file.Quality, - subject.ParsedEpisodeInfo.Quality)) + fileCustomFormats, + subject.ParsedEpisodeInfo.Quality, + subject.CustomFormats)) { _logger.Debug("Upgrading is not allowed by the quality profile");