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.

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<ProfileItemDto> _items = new List<ProfileItemDto>();
+    private IReadOnlyCollection<ProfileItemDto> _items = new List<ProfileItemDto>();
 
     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<ProfileItemDto> Items
     {
         get => _items;
-        init
+        set
         {
             if (value.Count > 0)
             {

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);
     }
 

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<ProfileItemDto> 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<ProfileItemDto> 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;

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<ProfileItemDto> 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<ProfileItemDto> 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;

src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedQualityProfile.cs

@@ -7,6 +7,7 @@ public record UpdatedQualities
 {
     public ICollection<string> InvalidQualityNames { get; init; } = new List<string>();
     public IReadOnlyCollection<ProfileItemDto> Items { get; init; } = new List<ProfileItemDto>();
+    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;
     }
 }

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<UpdatedQualityPr
             }
         });
 
+        RuleFor(x => 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");
     }
 }

src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs

@@ -83,6 +83,7 @@ public class QualityProfileFormatUpgradeYamlValidator : AbstractValidator<Qualit
     public QualityProfileFormatUpgradeYamlValidator()
     {
         RuleFor(x => x.UntilQuality)
+            .Cascade(CascadeMode.Stop)
             .NotEmpty()
             .WithMessage("'until_quality' is required when allowing profile upgrades");
     }
@@ -106,18 +107,18 @@ public class QualityProfileConfigYamlValidator : AbstractValidator<QualityProfil
             .Must((o, x) => !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<QualityProfil
 
         RuleFor(x => 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});
     }
 

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[]
@@ -43,17 +44,23 @@ public class QualityProfileTransactionPhaseTest
                     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,9 +68,18 @@ 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
         {
@@ -71,13 +87,27 @@ public class QualityProfileTransactionPhaseTest
             {
                 new UpdatedQualityProfile
                 {
-                        ProfileConfig = guideData[0],
+                    ProfileConfig = configData[0],
                     ProfileDto = serviceData.Schema,
-                        UpdateReason = QualityProfileUpdateReason.New
+                    UpdateReason = QualityProfileUpdateReason.New,
+                    UpdatedQualities = new UpdatedQualities
+                    {
+                        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]

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 ------

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<ProfileItemDto>
                 {
                     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<ProfileItemDto>
+                {
+                    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<ProfileItemDto>
+                {
+                    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()
     {

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");
+    }
 }

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}
             }
         };