fix: Sync preferred even if it is null

When preferred was null, it was not considered for change detection.
This was a regression likely introduced to support Radarr, which does
not have a preferred setting.

- Fixes #301
- Likely fixes #193 as well, which is a duplicate.

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Incorrect URLs were fixed in the local starter config template.
+- Quality Definition: Preferred quality setting would not sync in certain situations (#301).
 
 ## [7.1.1] - 2024-07-12
 

src/Recyclarr.Cli/Pipelines/QualitySize/PipelinePhases/QualitySizeApiPersistencePhase.cs

@@ -3,11 +3,18 @@ using Recyclarr.ServarrApi.QualityDefinition;
 
 namespace Recyclarr.Cli.Pipelines.QualitySize.PipelinePhases;
 
-public class QualitySizeApiPersistencePhase(IQualityDefinitionApiService api)
+public class QualitySizeApiPersistencePhase(ILogger log, IQualityDefinitionApiService api)
     : IApiPersistencePipelinePhase<QualitySizePipelineContext>
 {
     public async Task Execute(QualitySizePipelineContext context, CancellationToken ct)
     {
-        await api.UpdateQualityDefinition(context.TransactionOutput, ct);
+        var sizeData = context.TransactionOutput;
+        if (sizeData.Count == 0)
+        {
+            log.Debug("No size data available to persist; skipping API call");
+            return;
+        }
+
+        await api.UpdateQualityDefinition(sizeData, ct);
     }
 }

src/Recyclarr.Cli/Pipelines/QualitySize/PipelinePhases/QualitySizeConfigPhase.cs

@@ -10,31 +10,30 @@ public class QualitySizeConfigPhase(ILogger log, IQualitySizeGuideService guide,
 {
     public Task Execute(QualitySizePipelineContext context)
     {
-        var qualityDef = config.QualityDefinition;
+        var configSizeData = config.QualityDefinition;
-        if (qualityDef is null)
+        if (configSizeData is null)
         {
             log.Debug("{Instance} has no quality definition", config.InstanceName);
             return Task.CompletedTask;
         }
 
-        var qualityDefinitions = guide.GetQualitySizeData(config.ServiceType);
+        var guideSizeData = guide.GetQualitySizeData(config.ServiceType)
-        var selectedQuality = qualityDefinitions
+            .FirstOrDefault(x => x.Type.EqualsIgnoreCase(configSizeData.Type));
-            .FirstOrDefault(x => x.Type.EqualsIgnoreCase(qualityDef.Type));
 
-        if (selectedQuality == null)
+        if (guideSizeData == null)
         {
-            context.ConfigError = $"The specified quality definition type does not exist: {qualityDef.Type}";
+            context.ConfigError = $"The specified quality definition type does not exist: {configSizeData.Type}";
             return Task.CompletedTask;
         }
 
-        AdjustPreferredRatio(qualityDef, selectedQuality);
+        AdjustPreferredRatio(configSizeData, guideSizeData);
-        context.ConfigOutput = selectedQuality;
+        context.ConfigOutput = guideSizeData;
         return Task.CompletedTask;
     }
 
-    private void AdjustPreferredRatio(QualityDefinitionConfig qualityDefConfig, QualitySizeData selectedQuality)
+    private void AdjustPreferredRatio(QualityDefinitionConfig configSizeData, QualitySizeData guideSizeData)
     {
-        if (qualityDefConfig.PreferredRatio is null)
+        if (configSizeData.PreferredRatio is null)
         {
             return;
         }
@@ -42,20 +41,20 @@ public class QualitySizeConfigPhase(ILogger log, IQualitySizeGuideService guide,
         log.Information("Using an explicit preferred ratio which will override values from the guide");
 
         // Fix an out of range ratio and warn the user
-        if (qualityDefConfig.PreferredRatio is < 0 or > 1)
+        if (configSizeData.PreferredRatio is < 0 or > 1)
         {
-            var clampedRatio = Math.Clamp(qualityDefConfig.PreferredRatio.Value, 0, 1);
+            var clampedRatio = Math.Clamp(configSizeData.PreferredRatio.Value, 0, 1);
             log.Warning("Your `preferred_ratio` of {CurrentRatio} is out of range. " +
                 "It must be a decimal between 0.0 and 1.0. It has been clamped to {ClampedRatio}",
-                qualityDefConfig.PreferredRatio, clampedRatio);
+                configSizeData.PreferredRatio, clampedRatio);
 
-            qualityDefConfig.PreferredRatio = clampedRatio;
+            configSizeData.PreferredRatio = clampedRatio;
         }
 
         // Apply a calculated preferred size
-        foreach (var quality in selectedQuality.Qualities)
+        foreach (var quality in guideSizeData.Qualities)
         {
-            quality.Preferred = quality.InterpolatedPreferred(qualityDefConfig.PreferredRatio.Value);
+            quality.Preferred = quality.InterpolatedPreferred(configSizeData.PreferredRatio.Value);
         }
     }
 }

src/Recyclarr.Cli/Pipelines/QualitySize/PipelinePhases/QualitySizeLogPhase.cs

@@ -12,7 +12,7 @@ public class QualitySizeLogPhase(ILogger log) : ILogPipelinePhase<QualitySizePip
             return true;
         }
 
-        if (context.ConfigOutput is null)
+        if (context.ConfigOutput is not {Qualities.Count: > 0})
         {
             log.Debug("No Quality Definitions to process");
             return true;

src/Recyclarr.Cli/Pipelines/QualitySize/PipelinePhases/QualitySizeTransactionPhase.cs

@@ -23,7 +23,20 @@ public class QualitySizeTransactionPhase(ILogger log) : ITransactionPipelinePhas
                 continue;
             }
 
-            if (!QualityIsDifferent(serverEntry, qualityData))
+            var isDifferent = QualityIsDifferent(serverEntry, qualityData);
+
+            log.Debug("Processed Quality {Name}: " +
+                "[IsDifferent: {IsDifferent}] " +
+                "[Min: {Min1},{Min2}] " +
+                "[Max: {Max1},{Max2}] " +
+                "[Preferred: {Preferred1},{Preferred2}]",
+                serverEntry.Quality?.Name,
+                isDifferent,
+                serverEntry.MinSize, qualityData.Min,
+                serverEntry.MaxSize, qualityData.Max,
+                serverEntry.PreferredSize, qualityData.Preferred);
+
+            if (!isDifferent)
             {
                 continue;
             }
@@ -33,19 +46,13 @@ public class QualitySizeTransactionPhase(ILogger log) : ITransactionPipelinePhas
             serverEntry.MaxSize = qualityData.MaxForApi;
             serverEntry.PreferredSize = qualityData.PreferredForApi;
             newQuality.Add(serverEntry);
-
-            log.Debug("Setting Quality " +
-                "[Name: {Name}] [Source: {Source}] [Min: {Min}] [Max: {Max}] [Preferred: {Preferred}]",
-                serverEntry.Quality?.Name, serverEntry.Quality?.Source, serverEntry.MinSize, serverEntry.MaxSize,
-                serverEntry.PreferredSize);
         }
 
         context.TransactionOutput = newQuality;
     }
 
-    private static bool QualityIsDifferent(ServiceQualityDefinitionItem a, QualitySizeItem b)
+    private static bool QualityIsDifferent(ServiceQualityDefinitionItem a, QualityItemWithPreferred b)
     {
-        return b.IsMinDifferent(a.MinSize) || b.IsMaxDifferent(a.MaxSize) ||
+        return b.IsMinDifferent(a.MinSize) || b.IsMaxDifferent(a.MaxSize) || b.IsPreferredDifferent(a.PreferredSize);
-            a.PreferredSize is not null && b.IsPreferredDifferent(a.PreferredSize);
     }
 }

src/Recyclarr.TrashGuide/QualitySize/QualityItem.cs

@@ -33,10 +33,18 @@ public class QualityItem(string quality, decimal min, decimal max)
         return serviceValue != Min;
     }
 
-    public bool IsMaxDifferent(decimal? serviceValue)
+    protected static bool ValueWithThresholdIsDifferent(decimal? serviceValue, decimal guideValue, decimal threshold)
     {
         return serviceValue == null
-            ? MaxUnlimitedThreshold != Max
+            // If the service uses null, it's the same if the guide value == the max that null represents
-            : serviceValue != Max || MaxUnlimitedThreshold == Max;
+            ? guideValue != threshold
+            // If the service value is not null, it's the same only if it isn't the max value or the same as the guide
+            // If it's at max, that means we need to switch it to 'null' on the API so it gets treated as unlimited.
+            : guideValue != serviceValue || guideValue == threshold;
+    }
+
+    public bool IsMaxDifferent(decimal? serviceValue)
+    {
+        return ValueWithThresholdIsDifferent(serviceValue, Max, MaxUnlimitedThreshold);
     }
 }

src/Recyclarr.TrashGuide/QualitySize/QualityItemWithPreferred.cs

@@ -1,6 +1,6 @@
 namespace Recyclarr.TrashGuide.QualitySize;
 
-public class QualitySizeItem(string quality, decimal min, decimal max, decimal preferred)
+public class QualityItemWithPreferred(string quality, decimal min, decimal max, decimal preferred)
     : QualityItem(quality, min, max)
 {
     public const decimal PreferredUnlimitedThreshold = 395;
@@ -17,8 +17,6 @@ public class QualitySizeItem(string quality, decimal min, decimal max, decimal p
 
     public bool IsPreferredDifferent(decimal? serviceValue)
     {
-        return serviceValue == null
+        return ValueWithThresholdIsDifferent(serviceValue, Preferred, PreferredUnlimitedThreshold);
-            ? PreferredUnlimitedThreshold != Preferred
-            : serviceValue != Preferred || PreferredUnlimitedThreshold == Preferred;
     }
 }

src/Recyclarr.TrashGuide/QualitySize/QualitySizeData.cs

@@ -3,5 +3,6 @@ namespace Recyclarr.TrashGuide.QualitySize;
 public record QualitySizeData
 {
     public string Type { get; init; } = "";
-    public IReadOnlyCollection<QualitySizeItem> Qualities { get; init; } = Array.Empty<QualitySizeItem>();
+    public IReadOnlyCollection<QualityItemWithPreferred> Qualities { get; init; } =
+        Array.Empty<QualityItemWithPreferred>();
 }

tests/Recyclarr.Cli.Tests/Pipelines/QualitySize/PipelinePhases/QualitySizeConfigPhaseTest.cs

@@ -90,7 +90,7 @@ public class QualitySizeConfigPhaseTest
                 Type = "real",
                 Qualities = new[]
                 {
-                    new QualitySizeItem("quality1", 0, 100, 90)
+                    new QualityItemWithPreferred("quality1", 0, 100, 90)
                 }
             }
         });
@@ -102,7 +102,7 @@ public class QualitySizeConfigPhaseTest
         context.ConfigOutput.Should().NotBeNull();
         context.ConfigOutput!.Qualities.Should().BeEquivalentTo(new[]
             {
-                new QualitySizeItem("quality1", 0, 100, 50)
+                new QualityItemWithPreferred("quality1", 0, 100, 50)
             },
             o => o
                 .Including(x => x.Quality)
@@ -129,7 +129,7 @@ public class QualitySizeConfigPhaseTest
                 Type = "real",
                 Qualities = new[]
                 {
-                    new QualitySizeItem("quality1", 0, 100, 90)
+                    new QualityItemWithPreferred("quality1", 0, 100, 90)
                 }
             }
         });
@@ -141,7 +141,7 @@ public class QualitySizeConfigPhaseTest
         context.ConfigOutput.Should().NotBeNull();
         context.ConfigOutput!.Qualities.Should().BeEquivalentTo(new[]
             {
-                new QualitySizeItem("quality1", 0, 100, 90)
+                new QualityItemWithPreferred("quality1", 0, 100, 90)
             },
             o => o
                 .Including(x => x.Quality)

tests/Recyclarr.Cli.Tests/Pipelines/QualitySize/PipelinePhases/QualitySizeTransactionPhaseTest.cs

@@ -18,8 +18,8 @@ public class QualitySizeTransactionPhaseTest
             {
                 Qualities = new[]
                 {
-                    new QualitySizeItem("non_existent1", 0, 2, 1),
+                    new QualityItemWithPreferred("non_existent1", 0, 2, 1),
-                    new QualitySizeItem("non_existent2", 0, 2, 1)
+                    new QualityItemWithPreferred("non_existent2", 0, 2, 1)
                 }
             },
             ApiFetchOutput = new List<ServiceQualityDefinitionItem>
@@ -46,8 +46,8 @@ public class QualitySizeTransactionPhaseTest
             {
                 Qualities = new[]
                 {
-                    new QualitySizeItem("same1", 0, 2, 1),
+                    new QualityItemWithPreferred("same1", 0, 2, 1),
-                    new QualitySizeItem("same2", 0, 2, 1)
+                    new QualityItemWithPreferred("same2", 0, 2, 1)
                 }
             },
             ApiFetchOutput = new List<ServiceQualityDefinitionItem>
@@ -84,8 +84,8 @@ public class QualitySizeTransactionPhaseTest
             {
                 Qualities = new[]
                 {
-                    new QualitySizeItem("same1", 0, 2, 1),
+                    new QualityItemWithPreferred("same1", 0, 2, 1),
-                    new QualitySizeItem("different1", 0, 3, 1)
+                    new QualityItemWithPreferred("different1", 0, 3, 1)
                 }
             },
             ApiFetchOutput = new List<ServiceQualityDefinitionItem>

tests/Recyclarr.IntegrationTests/QualitySizeGuideParserTest.cs

@@ -25,8 +25,8 @@ public class QualitySizeGuideParserTest : IntegrationTestFixture
                 Type = "series",
                 Qualities = new[]
                 {
-                    new QualitySizeItem("quality1", 1, 2, 3),
+                    new QualityItemWithPreferred("quality1", 1, 2, 3),
-                    new QualitySizeItem("quality2", 4.1m, 5.1m, 6.1m)
+                    new QualityItemWithPreferred("quality2", 4.1m, 5.1m, 6.1m)
                 }
             }
         });

tests/Recyclarr.Tests/TrashGuide/QualitySize/QualityItemWithPreferredTest.cs

@@ -3,7 +3,7 @@ using Recyclarr.TrashGuide.QualitySize;
 namespace Recyclarr.Tests.TrashGuide.QualitySize;
 
 [TestFixture]
-public class QualitySizeItemTest
+public class QualityItemWithPreferredTest
 {
     private static readonly object[] PreferredTestValues =
     [
@@ -11,10 +11,13 @@ public class QualitySizeItemTest
         new object?[] {100m, 101m, true},
         new object?[] {100m, 98m, true},
         new object?[] {100m, null, true},
-        new object?[] {QualitySizeItem.PreferredUnlimitedThreshold, null, false},
+        new object?[] {QualityItemWithPreferred.PreferredUnlimitedThreshold, null, false},
-        new object?[] {QualitySizeItem.PreferredUnlimitedThreshold - 1, null, true},
+        new object?[] {QualityItemWithPreferred.PreferredUnlimitedThreshold - 1, null, true},
         new object?[]
-            {QualitySizeItem.PreferredUnlimitedThreshold, QualitySizeItem.PreferredUnlimitedThreshold, true}
+        {
+            QualityItemWithPreferred.PreferredUnlimitedThreshold, QualityItemWithPreferred.PreferredUnlimitedThreshold,
+            true
+        }
     ];
 
     [TestCaseSource(nameof(PreferredTestValues))]
@@ -23,7 +26,7 @@ public class QualitySizeItemTest
         decimal? radarrValue,
         bool isDifferent)
     {
-        var data = new QualitySizeItem("", 0, 0, guideValue);
+        var data = new QualityItemWithPreferred("", 0, 0, guideValue);
         data.IsPreferredDifferent(radarrValue)
             .Should().Be(isDifferent);
     }
@@ -34,19 +37,19 @@ public class QualitySizeItemTest
         {
             400m,
             1.0m,
-            QualitySizeItem.PreferredUnlimitedThreshold
+            QualityItemWithPreferred.PreferredUnlimitedThreshold
         },
         new[]
         {
-            QualitySizeItem.PreferredUnlimitedThreshold,
+            QualityItemWithPreferred.PreferredUnlimitedThreshold,
             1.0m,
-            QualitySizeItem.PreferredUnlimitedThreshold
+            QualityItemWithPreferred.PreferredUnlimitedThreshold
         },
         new[]
         {
-            QualitySizeItem.PreferredUnlimitedThreshold - 1m,
+            QualityItemWithPreferred.PreferredUnlimitedThreshold - 1m,
             1.0m,
-            QualitySizeItem.PreferredUnlimitedThreshold - 1m
+            QualityItemWithPreferred.PreferredUnlimitedThreshold - 1m
         },
         new[]
         {
@@ -68,54 +71,54 @@ public class QualitySizeItemTest
         decimal ratio,
         decimal expectedResult)
     {
-        var data = new QualitySizeItem("", 0, max, 0);
+        var data = new QualityItemWithPreferred("", 0, max, 0);
         data.InterpolatedPreferred(ratio).Should().Be(expectedResult);
     }
 
     [Test]
     public void AnnotatedPreferred_OutsideThreshold_EqualsSameValueWithUnlimited()
     {
-        const decimal testVal = QualitySizeItem.PreferredUnlimitedThreshold;
+        const decimal testVal = QualityItemWithPreferred.PreferredUnlimitedThreshold;
-        var data = new QualitySizeItem("", 0, 0, testVal);
+        var data = new QualityItemWithPreferred("", 0, 0, testVal);
         data.AnnotatedPreferred.Should().Be($"{testVal} (Unlimited)");
     }
 
     [Test]
     public void AnnotatedPreferred_WithinThreshold_EqualsSameStringValue()
     {
-        const decimal testVal = QualitySizeItem.PreferredUnlimitedThreshold - 1;
+        const decimal testVal = QualityItemWithPreferred.PreferredUnlimitedThreshold - 1;
-        var data = new QualitySizeItem("", 0, 0, testVal);
+        var data = new QualityItemWithPreferred("", 0, 0, testVal);
         data.AnnotatedPreferred.Should().Be($"{testVal}");
     }
 
     [Test]
     public void Preferred_AboveThreshold_EqualsSameValue()
     {
-        const decimal testVal = QualitySizeItem.PreferredUnlimitedThreshold + 1;
+        const decimal testVal = QualityItemWithPreferred.PreferredUnlimitedThreshold + 1;
-        var data = new QualitySizeItem("", 0, 0, testVal);
+        var data = new QualityItemWithPreferred("", 0, 0, testVal);
         data.Preferred.Should().Be(testVal);
     }
 
     [Test]
     public void PreferredForApi_AboveThreshold_EqualsNull()
     {
-        const decimal testVal = QualitySizeItem.PreferredUnlimitedThreshold + 1;
+        const decimal testVal = QualityItemWithPreferred.PreferredUnlimitedThreshold + 1;
-        var data = new QualitySizeItem("", 0, 0, testVal);
+        var data = new QualityItemWithPreferred("", 0, 0, testVal);
         data.PreferredForApi.Should().Be(null);
     }
 
     [Test]
     public void PreferredForApi_HighestWithinThreshold_EqualsSameValue()
     {
-        const decimal testVal = QualitySizeItem.PreferredUnlimitedThreshold - 0.1m;
+        const decimal testVal = QualityItemWithPreferred.PreferredUnlimitedThreshold - 0.1m;
-        var data = new QualitySizeItem("", 0, 0, testVal);
+        var data = new QualityItemWithPreferred("", 0, 0, testVal);
         data.PreferredForApi.Should().Be(testVal).And.Be(data.Preferred);
     }
 
     [Test]
     public void PreferredForApi_LowestWithinThreshold_EqualsSameValue()
     {
-        var data = new QualitySizeItem("", 0, 0, 0);
+        var data = new QualityItemWithPreferred("", 0, 0, 0);
         data.PreferredForApi.Should().Be(0);
     }
 }