From 0652cfd800c7338f9bb7fcbfff95b9b71644ff89 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Wed, 2 Aug 2023 20:50:54 -0500 Subject: [PATCH] fix: More robust handling of missing qualities list When qualities are not specified by the user, qualities are not modified if the profile exists in the service. If the profile does not exist yet, then an error is shown. Qualities are required when the profile is being created. --- .../QualityProfile/Api/QualityProfileDto.cs | 8 +- .../QualityProfileStatCalculator.cs | 17 ++--- .../QualityProfile/QualityItemOrganizer.cs | 26 ++++--- .../QualityProfileExtensions.cs | 16 ++-- .../QualityProfile/UpdatedQualityProfile.cs | 30 +++++--- .../UpdatedQualityProfileValidator.cs | 11 +++ .../ConfigYamlDataObjectsValidation.cs | 10 ++- .../QualityProfileTransactionPhaseTest.cs | 76 +++++++++++++------ .../QualityItemOrganizerTest.cs | 2 + .../UpdatedQualityProfileTest.cs | 36 ++++++++- .../UpdatedQualityProfileValidatorTest.cs | 75 +++++++++++++++++- .../ConfigYamlDataObjectsValidationTest.cs | 31 ++++++-- 12 files changed, 262 insertions(+), 76 deletions(-) diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs index d5c7f99a..db9f39c8 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs @@ -19,10 +19,10 @@ public record QualityProfileDto { private readonly bool? _upgradeAllowed; private readonly int? _minFormatScore; - private readonly int? _cutoff; + private int? _cutoff; private readonly int? _cutoffFormatScore; private readonly string _name = ""; - private readonly IReadOnlyCollection _items = new List(); + private IReadOnlyCollection _items = new List(); public int? Id { get; set; } @@ -53,7 +53,7 @@ public record QualityProfileDto public int? Cutoff { get => _cutoff; - init => DtoUtil.SetIfNotNull(ref _cutoff, value); + set => DtoUtil.SetIfNotNull(ref _cutoff, value); } public int? CutoffFormatScore @@ -67,7 +67,7 @@ public record QualityProfileDto public IReadOnlyCollection Items { get => _items; - init + set { if (value.Count > 0) { diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileStatCalculator.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileStatCalculator.cs index d5ed9391..eba04148 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileStatCalculator.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileStatCalculator.cs @@ -27,19 +27,18 @@ public class QualityProfileStatCalculator _log.Debug("Updates for profile {ProfileName}", profile.ProfileName); var stats = new ProfileWithStats {Profile = profile}; + var oldDto = profile.ProfileDto; + var newDto = profile.BuildUpdatedDto(); - ProfileUpdates(stats, profile); - QualityUpdates(stats, profile); + ProfileUpdates(stats, oldDto, newDto); + QualityUpdates(stats, oldDto, newDto); ScoreUpdates(stats, profile.ProfileDto, profile.UpdatedScores); return stats; } - private void ProfileUpdates(ProfileWithStats stats, UpdatedQualityProfile profile) + private void ProfileUpdates(ProfileWithStats stats, QualityProfileDto newDto, QualityProfileDto oldDto) { - var oldDto = profile.ProfileDto; - var newDto = profile.BuildUpdatedDto(); - Log("Upgrade Allowed", oldDto.UpgradeAllowed, newDto.UpgradeAllowed); Log("Cutoff", oldDto.Items.FindCutoff(oldDto.Cutoff), newDto.Items.FindCutoff(newDto.Cutoff)); Log("Cutoff Score", oldDto.CutoffFormatScore, newDto.CutoffFormatScore); @@ -53,10 +52,10 @@ public class QualityProfileStatCalculator } } - private static void QualityUpdates(ProfileWithStats stats, UpdatedQualityProfile profile) + private static void QualityUpdates(ProfileWithStats stats, QualityProfileDto newDto, QualityProfileDto oldDto) { - var dtoQualities = JToken.FromObject(profile.ProfileDto.Items); - var updatedQualities = JToken.FromObject(profile.UpdatedQualities.Items); + var dtoQualities = JToken.FromObject(newDto.Items); + var updatedQualities = JToken.FromObject(oldDto.Items); stats.QualitiesChanged = !JToken.DeepEquals(dtoQualities, updatedQualities); } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityItemOrganizer.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityItemOrganizer.cs index 755bd723..64f7c3d8 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityItemOrganizer.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityItemOrganizer.cs @@ -19,6 +19,7 @@ public class QualityItemOrganizer return new UpdatedQualities { InvalidQualityNames = _invalidItemNames, + NumWantedItems = wanted.Count, Items = combined }; } @@ -31,18 +32,6 @@ public class QualityItemOrganizer foreach (var configQuality in configQualities) { - void AddQualityFromDto(ICollection items, string name) - { - var dtoItem = dtoItems.FindQualityByName(name); - if (dtoItem is null) - { - _invalidItemNames.Add(name); - return; - } - - items.Add(dtoItem with {Allowed = configQuality.Enabled}); - } - // If the nested qualities list is NOT empty, then this is considered a quality group. if (configQuality.Qualities.IsNotEmpty()) { @@ -68,6 +57,19 @@ public class QualityItemOrganizer } AddQualityFromDto(updatedItems, configQuality.Name); + continue; + + void AddQualityFromDto(ICollection items, string name) + { + var dtoItem = dtoItems.FindQualityByName(name); + if (dtoItem is null) + { + _invalidItemNames.Add(name); + return; + } + + items.Add(dtoItem with {Allowed = configQuality.Enabled}); + } } return updatedItems; diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityProfileExtensions.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityProfileExtensions.cs index 5cea1d67..0c501200 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityProfileExtensions.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/QualityProfileExtensions.cs @@ -59,6 +59,14 @@ public static class QualityProfileExtensions .FirstOrDefault(x => x.Quality!.Name.EqualsIgnoreCase(name)); } + private static IEnumerable<(string? Name, int? Id)> GetEligibleCutoffs(IEnumerable items) + { + return items + .Where(x => x.Allowed is true) + .Select(x => x.Quality is null ? (x.Name, x.Id) : (x.Quality.Name, x.Quality.Id)) + .Where(x => x.Name is not null); + } + public static int? FindCutoff(this IEnumerable items, string? name) { if (name is null) @@ -66,9 +74,7 @@ public static class QualityProfileExtensions return null; } - var result = items - .Select(x => x.Quality is null ? (x.Name, x.Id) : (x.Quality.Name, x.Quality.Id)) - .Where(x => x.Name is not null) + var result = GetEligibleCutoffs(items) .FirstOrDefault(x => x.Name.EqualsIgnoreCase(name)); return result.Id; @@ -81,9 +87,7 @@ public static class QualityProfileExtensions return null; } - var result = items - .Select(x => x.Quality is null ? (x.Name, x.Id) : (x.Quality.Name, x.Quality.Id)) - .Where(x => x.Name is not null) + var result = GetEligibleCutoffs(items) .FirstOrDefault(x => x.Id == id); return result.Name; diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfile.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfile.cs index 0bc2a24f..03e2145e 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfile.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfile.cs @@ -7,6 +7,7 @@ public record UpdatedQualities { public ICollection InvalidQualityNames { get; init; } = new List(); public IReadOnlyCollection Items { get; init; } = new List(); + public int NumWantedItems { get; init; } } public record UpdatedQualityProfile @@ -34,22 +35,29 @@ public record UpdatedQualityProfile public QualityProfileDto BuildUpdatedDto() { var config = ProfileConfig.Profile; - - // The `qualityprofile` API will still validate `cutoff` even when `upgradeAllowed` is set to `false`. - // Because of this, we cannot set cutoff to null. We pick the first available if the user didn't specify one. - var cutoff = config.UpgradeAllowed - ? UpdatedQualities.Items.FindCutoff(config.UpgradeUntilQuality) - : UpdatedQualities.Items.First().Id; - - return ProfileDto with + var newDto = ProfileDto with { Name = config.Name, // Must keep this for NEW profile syncing. It will only assign if src is not null. UpgradeAllowed = config.UpgradeAllowed, MinFormatScore = config.MinFormatScore, - Cutoff = cutoff, CutoffFormatScore = config.UpgradeUntilScore, - FormatItems = UpdatedScores.Select(x => x.Dto with {Score = x.NewScore}).ToList(), - Items = UpdatedQualities.Items + FormatItems = UpdatedScores.Select(x => x.Dto with {Score = x.NewScore}).ToList() }; + + if (UpdatedQualities.NumWantedItems > 0) + { + newDto.Items = UpdatedQualities.Items; + } + + // The `qualityprofile` API will still validate `cutoff` even when `upgradeAllowed` is set to `false`. + // Because of this, we cannot set cutoff to null. We pick the first available if the user didn't specify one. + // + // Also: It's important that we assign the cutoff *after* we set Items. Because we pull from a different list of + // items depending on if the `qualities` property is set in config. + newDto.Cutoff = config.UpgradeAllowed + ? newDto.Items.FindCutoff(config.UpgradeUntilQuality) + : newDto.Items.FirstOrDefault()?.Id; + + return newDto; } } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfileValidator.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfileValidator.cs index 28f93abd..4e712994 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfileValidator.cs +++ b/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfileValidator.cs @@ -1,4 +1,5 @@ using FluentValidation; +using Recyclarr.Cli.Pipelines.QualityProfile.PipelinePhases; namespace Recyclarr.Cli.Pipelines.QualityProfile; @@ -18,9 +19,19 @@ public class UpdatedQualityProfileValidator : AbstractValidator x.ProfileConfig.Profile.UpgradeUntilQuality) + .Must((o, x) => o.ProfileDto.Items.FindCutoff(x) is not null) + .When(x => x.ProfileConfig.Profile is {UpgradeUntilQuality: not null, Qualities.Count: 0}) + .WithMessage("'until_quality' must refer to an existing and enabled quality or group"); + RuleFor(x => x.ProfileConfig.Profile.UpgradeUntilQuality) .Must((o, x) => !o.UpdatedQualities.InvalidQualityNames.Contains(x, StringComparer.InvariantCultureIgnoreCase)) .WithMessage((_, x) => $"`until_quality` references invalid quality '{x}'"); + + RuleFor(x => x.ProfileConfig.Profile.Qualities) + .NotEmpty() + .When(x => x.UpdateReason == QualityProfileUpdateReason.New) + .WithMessage("`qualities` is required when creating profiles for the first time"); } } diff --git a/src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs b/src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs index f65b69ed..8683a798 100644 --- a/src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs +++ b/src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs @@ -83,6 +83,7 @@ public class QualityProfileFormatUpgradeYamlValidator : AbstractValidator x.UntilQuality) + .Cascade(CascadeMode.Stop) .NotEmpty() .WithMessage("'until_quality' is required when allowing profile upgrades"); } @@ -106,18 +107,18 @@ public class QualityProfileConfigYamlValidator : AbstractValidator !x! .Where(y => y.Qualities is not null) .SelectMany(y => y.Qualities!) - .Contains(o.UpgradesAllowed!.UntilQuality)) + .Contains(o.UpgradesAllowed!.UntilQuality, StringComparer.InvariantCultureIgnoreCase)) .WithMessage(o => $"For profile {o.Name}, 'until_quality' must not refer to qualities contained within groups") .Must((o, x) => !x! .Where(y => y is {Enabled: false, Name: not null}) .Select(y => y.Name!) - .Contains(o.UpgradesAllowed!.UntilQuality)) + .Contains(o.UpgradesAllowed!.UntilQuality, StringComparer.InvariantCultureIgnoreCase)) .WithMessage(o => $"For profile {o.Name}, 'until_quality' must not refer to explicitly disabled qualities") .Must((o, x) => x! .Select(y => y.Name) - .Contains(o.UpgradesAllowed!.UntilQuality)) + .Contains(o.UpgradesAllowed!.UntilQuality, StringComparer.InvariantCultureIgnoreCase)) .WithMessage(o => $"For profile {o.Name}, 'qualities' must contain the quality mentioned in 'until_quality', " + $"which is '{o.UpgradesAllowed!.UntilQuality}'") @@ -125,6 +126,9 @@ public class QualityProfileConfigYamlValidator : AbstractValidator x.Qualities) .Custom(ValidateHaveNoDuplicates!) + .Must(x => x!.Any(y => y.Enabled is true or null)) + .WithMessage(x => + $"For profile {x.Name}, at least one explicitly listed quality under 'qualities' must be enabled.") .When(x => x is {Qualities.Count: > 0}); } diff --git a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs index cac96a38..9b4ca2d7 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs +++ b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs @@ -2,6 +2,7 @@ using Recyclarr.Cli.Pipelines.QualityProfile; using Recyclarr.Cli.Pipelines.QualityProfile.Api; using Recyclarr.Cli.Pipelines.QualityProfile.PipelinePhases; using Recyclarr.Cli.TestLibrary; +using Recyclarr.TrashLib.Config.Services; namespace Recyclarr.Cli.Tests.Pipelines.QualityProfile.PipelinePhases; @@ -15,11 +16,11 @@ public class QualityProfileTransactionPhaseTest { var guideData = new[] { - NewQp.Processed("invalid_profile_name", ("id1", 1, 100)) with + NewQp.Processed("invalid_profile_name") with { ShouldCreate = false }, - NewQp.Processed("profile1", ("id1", 1, 100), ("id2", 2, 500)) + NewQp.Processed("profile1") }; var dtos = new[] @@ -32,28 +33,34 @@ public class QualityProfileTransactionPhaseTest var result = sut.Execute(guideData, serviceData); result.Should().BeEquivalentTo(new QualityProfileTransactionData + { + NonExistentProfiles = new[] {"invalid_profile_name"}, + UpdatedProfiles = { - NonExistentProfiles = new[] {"invalid_profile_name"}, - UpdatedProfiles = + new UpdatedQualityProfile { - new UpdatedQualityProfile - { - ProfileConfig = guideData[1], - ProfileDto = dtos[0], - UpdateReason = QualityProfileUpdateReason.Changed - } + ProfileConfig = guideData[1], + ProfileDto = dtos[0], + UpdateReason = QualityProfileUpdateReason.Changed } - }, - o => o.Excluding(x => x.Name.Contains(nameof(UpdatedQualityProfile.UpdatedScores)))); + } + }); } [Test, AutoMockData] public void New_profiles( QualityProfileTransactionPhase sut) { - var guideData = new[] + var configData = new[] { - NewQp.Processed("profile1", ("id1", 1, 100), ("id2", 2, 500)) + new ProcessedQualityProfileData(new QualityProfileConfig + { + Name = "profile1", + Qualities = new[] + { + new QualityProfileQualityConfig {Name = "quality1", Enabled = true} + } + }) }; var dtos = new[] @@ -61,23 +68,46 @@ public class QualityProfileTransactionPhaseTest new QualityProfileDto {Name = "irrelevant_profile"} }; - var serviceData = new QualityProfileServiceData(dtos, new QualityProfileDto()); + var serviceData = new QualityProfileServiceData(dtos, new QualityProfileDto()) + { + Schema = new QualityProfileDto + { + Items = new[] + { + new ProfileItemDto {Quality = new ProfileItemQualityDto {Name = "quality1"}} + } + } + }; - var result = sut.Execute(guideData, serviceData); + var result = sut.Execute(configData, serviceData); result.Should().BeEquivalentTo(new QualityProfileTransactionData + { + UpdatedProfiles = { - UpdatedProfiles = + new UpdatedQualityProfile { - new UpdatedQualityProfile + ProfileConfig = configData[0], + ProfileDto = serviceData.Schema, + UpdateReason = QualityProfileUpdateReason.New, + UpdatedQualities = new UpdatedQualities { - ProfileConfig = guideData[0], - ProfileDto = serviceData.Schema, - UpdateReason = QualityProfileUpdateReason.New + NumWantedItems = 1, + Items = new[] + { + new ProfileItemDto + { + Allowed = true, + Quality = new ProfileItemQualityDto + { + Name = "quality1" + } + } + } } } - }, - o => o.Excluding(x => x.Name.Contains(nameof(UpdatedQualityProfile.UpdatedScores)))); + } + }); } [Test, AutoMockData] diff --git a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/QualityItemOrganizerTest.cs b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/QualityItemOrganizerTest.cs index 9b4ee13c..97330b68 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/QualityItemOrganizerTest.cs +++ b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/QualityItemOrganizerTest.cs @@ -57,6 +57,7 @@ public class QualityItemOrganizerTest result.Should().BeEquivalentTo(new UpdatedQualities { InvalidQualityNames = new[] {"nonexistent1"}, + NumWantedItems = 7, Items = new[] { // ------ IN CONFIG ------ @@ -92,6 +93,7 @@ public class QualityItemOrganizerTest result.Should().BeEquivalentTo(new UpdatedQualities { InvalidQualityNames = new[] {"nonexistent1"}, + NumWantedItems = 7, Items = new[] { // ------ NOT IN CONFIG ------ diff --git a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileTest.cs b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileTest.cs index 4ca57352..06b6a464 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileTest.cs +++ b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileTest.cs @@ -46,7 +46,7 @@ public class UpdatedQualityProfileTest } [Test] - public void Dto_updated_from_config() + public void Dto_updated_from_config_with_qualities() { var profile = new UpdatedQualityProfile { @@ -68,6 +68,7 @@ public class UpdatedQualityProfileTest }), UpdatedQualities = new UpdatedQualities { + NumWantedItems = 1, Items = new List { NewQp.QualityDto(1, "Quality Item 1", true), @@ -96,6 +97,39 @@ public class UpdatedQualityProfileTest }); } + [Test] + public void Dto_quality_items_updated_from_config_with_no_qualities() + { + var profile = new UpdatedQualityProfile + { + ProfileDto = new QualityProfileDto + { + Items = new List + { + NewQp.QualityDto(8, "Quality Item 8", true), + NewQp.QualityDto(9, "Quality Item 9", true) + } + }, + ProfileConfig = new ProcessedQualityProfileData(new QualityProfileConfig()), + UpdatedQualities = new UpdatedQualities + { + NumWantedItems = 0, + Items = new List + { + NewQp.QualityDto(1, "Quality Item 1", true), + NewQp.QualityDto(2, "Quality Item 2", true), + NewQp.GroupDto(3, "Quality Item 3", true, + NewQp.QualityDto(4, "Quality Item 4", true)) + } + }, + UpdateReason = QualityProfileUpdateReason.New + }; + + var result = profile.BuildUpdatedDto(); + + result.Items.Should().BeEquivalentTo(profile.ProfileDto.Items); + } + [Test] public void Dto_name_is_updated_when_empty() { diff --git a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileValidatorTest.cs b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileValidatorTest.cs index 93068ab1..392c7ac6 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileValidatorTest.cs +++ b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/UpdatedQualityProfileValidatorTest.cs @@ -29,7 +29,7 @@ public class UpdatedQualityProfileValidatorTest }, ProfileDto = new QualityProfileDto {Name = "ProfileName"}, ProfileConfig = new ProcessedQualityProfileData(profileConfig), - UpdateReason = QualityProfileUpdateReason.New + UpdateReason = QualityProfileUpdateReason.Changed }; var validator = new UpdatedQualityProfileValidator(); @@ -49,4 +49,77 @@ public class UpdatedQualityProfileValidatorTest $"positive scores is {expectedTotalScore}"); } } + + [Test] + public void Until_quality_references_invalid_quality() + { + var profileConfig = new QualityProfileConfig + { + UpgradeUntilQuality = "foo1" + }; + + var updatedProfile = new UpdatedQualityProfile + { + UpdatedQualities = new UpdatedQualities + { + InvalidQualityNames = new[] {"foo1"} + }, + ProfileDto = new QualityProfileDto(), + ProfileConfig = new ProcessedQualityProfileData(profileConfig), + UpdateReason = QualityProfileUpdateReason.New + }; + + var validator = new UpdatedQualityProfileValidator(); + var result = validator.TestValidate(updatedProfile); + + result.ShouldHaveValidationErrorFor(x => x.ProfileConfig.Profile.UpgradeUntilQuality) + .WithErrorMessage("`until_quality` references invalid quality 'foo1'"); + } + + [Test] + public void Qualities_required_for_new_profiles() + { + var profileConfig = new QualityProfileConfig(); + + var updatedProfile = new UpdatedQualityProfile + { + ProfileDto = new QualityProfileDto(), + ProfileConfig = new ProcessedQualityProfileData(profileConfig), + UpdateReason = QualityProfileUpdateReason.New + }; + + var validator = new UpdatedQualityProfileValidator(); + var result = validator.TestValidate(updatedProfile); + + result.ShouldHaveValidationErrorFor(x => x.ProfileConfig.Profile.Qualities) + .WithErrorMessage("`qualities` is required when creating profiles for the first time"); + } + + [Test] + public void Cutoff_quality_must_be_enabled_without_qualities_list() + { + var profileConfig = new QualityProfileConfig + { + UpgradeUntilQuality = "disabled_quality" + }; + + var updatedProfile = new UpdatedQualityProfile + { + ProfileDto = new QualityProfileDto + { + Items = new[] + { + NewQp.QualityDto(1, "disabled_quality", false) + } + }, + ProfileConfig = new ProcessedQualityProfileData(profileConfig), + UpdateReason = QualityProfileUpdateReason.New + }; + + var validator = new UpdatedQualityProfileValidator(); + var result = validator.TestValidate(updatedProfile); + + result.ShouldHaveValidationErrorFor(x => x.ProfileConfig.Profile.UpgradeUntilQuality) + .WithErrorMessage("'until_quality' must refer to an existing and enabled quality or group"); + } } diff --git a/src/tests/Recyclarr.TrashLib.Tests/Config/Parsing/ConfigYamlDataObjectsValidationTest.cs b/src/tests/Recyclarr.TrashLib.Tests/Config/Parsing/ConfigYamlDataObjectsValidationTest.cs index 5ec65080..d3523427 100644 --- a/src/tests/Recyclarr.TrashLib.Tests/Config/Parsing/ConfigYamlDataObjectsValidationTest.cs +++ b/src/tests/Recyclarr.TrashLib.Tests/Config/Parsing/ConfigYamlDataObjectsValidationTest.cs @@ -138,6 +138,28 @@ public class ConfigYamlDataObjectsValidationTest $"For profile {data.Name}, 'qualities' contains duplicates for quality 'Dupe Quality 4'"); } + [Test] + public void Quality_profile_qualities_must_have_at_least_one_enabled() + { + var data = new QualityProfileConfigYaml + { + Name = "My QP", + Qualities = new[] + { + new QualityProfileQualityConfigYaml {Name = "Quality 1", Enabled = false}, + new QualityProfileQualityConfigYaml {Name = "Quality 2", Enabled = false} + } + }; + + var validator = new QualityProfileConfigYamlValidator(); + var result = validator.TestValidate(data); + + result.ShouldHaveValidationErrorFor(x => x.Qualities); + + result.Errors.Select(x => x.ErrorMessage).Should().BeEquivalentTo( + $"For profile {data.Name}, at least one explicitly listed quality under 'qualities' must be enabled."); + } + [Test] public void Quality_profile_cutoff_quality_should_not_refer_to_disabled_qualities() { @@ -146,15 +168,12 @@ public class ConfigYamlDataObjectsValidationTest Name = "My QP", UpgradesAllowed = new QualityProfileFormatUpgradeYaml { - UntilQuality = "Test Quality" + UntilQuality = "Disabled Quality" }, Qualities = new[] { - new QualityProfileQualityConfigYaml - { - Name = "Test Quality", - Enabled = false - } + new QualityProfileQualityConfigYaml {Name = "Enabled Quality"}, + new QualityProfileQualityConfigYaml {Name = "Disabled Quality", Enabled = false} } };