fix: Fix false-positive duplicate score warnings

When doing a `sync --preview`, new custom formats are not created and
thus they never get an ID greater than `0`. Because of this, a
dictionary that tracks duplicates based on ID would result in warnings
about duplicate scores that made no sense.

We now index by Trash ID instead of Format ID, which is more accurate.

CHANGELOG.md

@@ -34,6 +34,11 @@ changes you may need to make.
 - **BREAKING**: Removed `reset_unmatched_scores` support under quality profile score section.
 - **BREAKING**: Migration steps that dealt with the old `trash.yml` have been removed.
 
+### Fixed
+
+- False-positive duplicate score warnings no longer occur when doing `sync --preview` for the first
+  time.
+
 ## [4.4.1] - 2023-04-08
 
 ### Fixed

src/Recyclarr.Cli/Pipelines/CustomFormat/Models/ProcessedCustomFormatCache.cs

@@ -21,4 +21,9 @@ public class ProcessedCustomFormatCache : IPipelineCache
     {
         return _customFormats.FirstOrDefault(x => x.TrashId.EqualsIgnoreCase(trashId));
     }
+
+    public CustomFormatData? LookupByServiceId(int id)
+    {
+        return _customFormats.FirstOrDefault(x => x.Id == id);
+    }
 }

src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs

@@ -1,4 +1,3 @@
-using System.Collections.ObjectModel;
 using JetBrains.Annotations;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -14,7 +13,7 @@ public record QualityProfileDto
     public int MinFormatScore { get; init; }
     public int Cutoff { get; init; }
     public int CutoffFormatScore { get; init; }
-    public Collection<ProfileFormatItemDto> FormatItems { get; } = new();
+    public IReadOnlyCollection<ProfileFormatItemDto> FormatItems { get; init; } = Array.Empty<ProfileFormatItemDto>();
 
     [JsonExtensionData]
     public JObject? ExtraJson { get; init; }
@@ -25,7 +24,7 @@ public record ProfileFormatItemDto
 {
     public int Format { get; init; }
     public string Name { get; init; } = "";
-    public int Score { get; set; }
+    public int Score { get; init; }
 
     [JsonExtensionData]
     public Dictionary<string, object> ExtraJson { get; init; } = new();

src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileApiPersistencePhase.cs

@@ -16,7 +16,12 @@ public class QualityProfileApiPersistencePhase
 
     public async Task Execute(IServiceConfiguration config, QualityProfileTransactionData transactions)
     {
-        foreach (var profile in transactions.UpdatedProfiles.Select(x => x.UpdatedProfile))
+        var profilesToUpdate = transactions.UpdatedProfiles.Select(x => x.UpdatedProfile with
+        {
+            FormatItems = x.UpdatedScores.Select(y => y.Dto with {Score = y.NewScore}).ToList()
+        });
+
+        foreach (var profile in profilesToUpdate)
         {
             await _api.UpdateQualityProfile(config, profile);
         }
@@ -36,10 +41,10 @@ public class QualityProfileApiPersistencePhase
             {
                 _log.Debug("> Scores updated for quality profile: {ProfileName}", profileName);
 
-                foreach (var (customFormatName, oldScore, newScore, reason) in scores)
+                foreach (var (dto, newScore, reason) in scores)
                 {
                     _log.Debug("  - {Format}: {OldScore} -> {NewScore} ({Reason})",
-                        customFormatName, oldScore, newScore, reason);
+                        dto.Name, dto.Score, newScore, reason);
                 }
             }
 

src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhase.cs

@@ -5,9 +5,11 @@ using Recyclarr.TrashLib.Models;
 
 namespace Recyclarr.Cli.Pipelines.QualityProfile.PipelinePhases;
 
+public record ProcessedQualityProfileScore(string TrashId, string CfName, int FormatId, int Score);
+
 public record ProcessedQualityProfileData(QualityProfileConfig Profile)
 {
-    public Dictionary<int, int> CfScores { get; init; } = new();
+    public IList<ProcessedQualityProfileScore> CfScores { get; init; } = new List<ProcessedQualityProfileScore>();
 }
 
 public class QualityProfileConfigPhase
@@ -60,7 +62,7 @@ public class QualityProfileConfigPhase
     }
 
     private void AddCustomFormatScoreData(
-        IDictionary<int, int> existingScoreData,
+        ICollection<ProcessedQualityProfileScore> existingScoreData,
         QualityProfileScoreConfig profile,
         CustomFormatData cf)
     {
@@ -71,9 +73,10 @@ public class QualityProfileConfigPhase
             return;
         }
 
-        if (existingScoreData.TryGetValue(cf.Id, out var existingScore))
+        var existingScore = existingScoreData.FirstOrDefault(x => x.TrashId.EqualsIgnoreCase(cf.TrashId));
+        if (existingScore is not null)
         {
-            if (existingScore != scoreToUse)
+            if (existingScore.Score != scoreToUse)
             {
                 _log.Warning(
                     "Custom format {Name} ({TrashId}) is duplicated in quality profile {ProfileName} with a score " +
@@ -88,6 +91,6 @@ public class QualityProfileConfigPhase
             return;
         }
 
-        existingScoreData.Add(cf.Id, scoreToUse.Value);
+        existingScoreData.Add(new ProcessedQualityProfileScore(cf.TrashId, cf.Name, cf.Id, scoreToUse.Value));
     }
 }

src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfilePreviewPhase.cs

@@ -26,11 +26,11 @@ public class QualityProfilePreviewPhase
                 .AddColumn("[bold]New[/]")
                 .AddColumn("[bold]Reason[/]");
 
-            foreach (var updatedScore in updatedScores)
+            foreach (var updatedScore in updatedScores.Where(x => x.Reason != FormatScoreUpdateReason.NoChange))
             {
                 table.AddRow(
-                    updatedScore.CustomFormatName,
-                    updatedScore.OldScore.ToString(),
+                    updatedScore.Dto.Name,
+                    updatedScore.Dto.Score.ToString(),
                     updatedScore.NewScore.ToString(),
                     updatedScore.Reason.ToString());
             }

src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhase.cs

@@ -6,7 +6,7 @@ namespace Recyclarr.Cli.Pipelines.QualityProfile.PipelinePhases;
 
 public record UpdatedQualityProfile(QualityProfileDto UpdatedProfile)
 {
-    public Collection<UpdatedFormatScore> UpdatedScores { get; } = new();
+    public required IReadOnlyCollection<UpdatedFormatScore> UpdatedScores { get; init; }
 }
 
 public record QualityProfileTransactionData
@@ -67,33 +67,33 @@ public class QualityProfileTransactionPhase
         ProcessedQualityProfileData profileData,
         QualityProfileDto profileDto)
     {
-        var updatedProfile = new UpdatedQualityProfile(profileDto);
-
-        void UpdateScore(ProfileFormatItemDto item, int newScore, FormatScoreUpdateReason reason)
-        {
-            if (item.Score == newScore)
-            {
-                return;
-            }
-
-            updatedProfile.UpdatedScores.Add(new UpdatedFormatScore(item.Name, item.Score, newScore, reason));
-            item.Score = newScore;
-        }
-
-        var scoreMap = profileData.CfScores;
-
-        foreach (var formatItem in profileDto.FormatItems)
-        {
-            if (scoreMap.TryGetValue(formatItem.Format, out var existingScore))
-            {
-                UpdateScore(formatItem, existingScore, FormatScoreUpdateReason.Updated);
-            }
-            else if (profileData.Profile is {ResetUnmatchedScores: true})
-            {
-                UpdateScore(formatItem, 0, FormatScoreUpdateReason.Reset);
-            }
-        }
-
-        return updatedProfile.UpdatedScores.Any() ? updatedProfile : null;
+        var scoreMap = profileData.CfScores
+            .FullJoin(profileDto.FormatItems,
+                x => x.FormatId,
+                x => x.Format,
+                l => new UpdatedFormatScore
+                {
+                    Dto = new ProfileFormatItemDto {Format = l.FormatId, Name = l.CfName},
+                    NewScore = l.Score,
+                    Reason = FormatScoreUpdateReason.New
+                },
+                r => new UpdatedFormatScore
+                {
+                    Dto = r,
+                    NewScore = 0,
+                    Reason = FormatScoreUpdateReason.Reset
+                },
+                (l, r) => new UpdatedFormatScore
+                {
+                    Dto = r,
+                    NewScore = l.Score,
+                    Reason = FormatScoreUpdateReason.Updated
+                })
+            .Select(x => x.Dto.Score == x.NewScore ? x with {Reason = FormatScoreUpdateReason.NoChange} : x)
+            .ToList();
+
+        return scoreMap.Any(x => x.Reason != FormatScoreUpdateReason.NoChange)
+            ? new UpdatedQualityProfile(profileDto) {UpdatedScores = scoreMap}
+            : null;
     }
 }

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

@@ -1,13 +1,42 @@
+using Recyclarr.Cli.Pipelines.QualityProfile.Api;
+
 namespace Recyclarr.Cli.Pipelines.QualityProfile;
 
 public enum FormatScoreUpdateReason
 {
+    /// <summary>
+    ///     A score who's value did not change.
+    /// </summary>
+    NoChange,
+
+    /// <summary>
+    ///     A score that is changed.
+    /// </summary>
     Updated,
-    Reset
+
+    /// <summary>
+    ///     Scores were reset to a 0 value because `reset_unmatched_scores` was set to `true`.
+    /// </summary>
+    Reset,
+
+    /// <summary>
+    ///     New custom format scores (format items) shouldn't exist normally. They do exist during
+    ///     `--preview` runs since new custom formats that aren't synced yet won't be available when
+    ///     processing quality profiles.
+    /// </summary>
+    New
 }
 
-public record UpdatedFormatScore(
-    string CustomFormatName,
-    int OldScore,
-    int NewScore,
-    FormatScoreUpdateReason Reason);
+public record UpdatedFormatScore
+{
+    public required ProfileFormatItemDto Dto { get; init; }
+    public required int NewScore { get; init; }
+    public required FormatScoreUpdateReason Reason { get; init; }
+
+    public void Deconstruct(out ProfileFormatItemDto dto, out int newScore, out FormatScoreUpdateReason reason)
+    {
+        dto = Dto;
+        newScore = NewScore;
+        reason = Reason;
+    }
+}

src/tests/Recyclarr.Cli.TestLibrary/NewQp.cs

@@ -1,3 +1,5 @@
+using Recyclarr.Cli.Pipelines.QualityProfile;
+using Recyclarr.Cli.Pipelines.QualityProfile.Api;
 using Recyclarr.Cli.Pipelines.QualityProfile.PipelinePhases;
 using Recyclarr.TrashLib.Config.Services;
 
@@ -7,7 +9,7 @@ public static class NewQp
 {
     public static ProcessedQualityProfileData Processed(
         string profileName,
-        params (int FormatId, int Score)[] scores)
+        params (string TrashId, int FormatId, int Score)[] scores)
     {
         return Processed(profileName, null, scores);
     }
@@ -15,14 +17,39 @@ public static class NewQp
     public static ProcessedQualityProfileData Processed(
         string profileName,
         bool? resetUnmatchedScores,
-        params (int FormatId, int Score)[] scores)
+        params (string TrashId, int FormatId, int Score)[] scores)
+    {
+        return Processed(profileName, resetUnmatchedScores,
+            scores.Select(x => ("", x.TrashId, x.FormatId, x.Score)).ToArray());
+    }
+
+    public static ProcessedQualityProfileData Processed(
+        string profileName,
+        bool? resetUnmatchedScores,
+        params (string CfName, string TrashId, int FormatId, int Score)[] scores)
     {
         return new ProcessedQualityProfileData(new QualityProfileConfig
         {
             Name = profileName, ResetUnmatchedScores = resetUnmatchedScores
         })
         {
-            CfScores = scores.ToDictionary(x => x.FormatId, x => x.Score)
+            CfScores = scores
+                .Select(x => new ProcessedQualityProfileScore(x.TrashId, x.CfName, x.FormatId, x.Score))
+                .ToList()
+        };
+    }
+
+    public static UpdatedFormatScore UpdatedScore(
+        string name,
+        int oldScore,
+        int newScore,
+        FormatScoreUpdateReason reason)
+    {
+        return new UpdatedFormatScore
+        {
+            Dto = new ProfileFormatItemDto {Name = name, Score = oldScore},
+            NewScore = newScore,
+            Reason = reason
         };
     }
 }

src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhaseTest.cs

@@ -47,7 +47,7 @@ public class QualityProfileConfigPhaseTest
 
         result.Should().BeEquivalentTo(new[]
         {
-            NewQp.Processed("test_profile", (1, 100), (2, 100))
+            NewQp.Processed("test_profile", ("id1", 1, 100), ("id2", 2, 100))
         });
     }
 
@@ -78,7 +78,7 @@ public class QualityProfileConfigPhaseTest
 
         result.Should().BeEquivalentTo(new[]
         {
-            NewQp.Processed("test_profile", (1, 100), (2, 200))
+            NewQp.Processed("test_profile", ("id1", 1, 100), ("id2", 2, 200))
         });
     }
 
@@ -163,8 +163,8 @@ public class QualityProfileConfigPhaseTest
 
         result.Should().BeEquivalentTo(new[]
         {
-            NewQp.Processed("test_profile1", (1, 100)),
-            NewQp.Processed("test_profile2", (1, 200))
+            NewQp.Processed("test_profile1", ("id1", 1, 100)),
+            NewQp.Processed("test_profile2", ("id1", 1, 200))
         });
     }
 }

src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs

@@ -16,7 +16,7 @@ public class QualityProfileTransactionPhaseTest
     {
         var guideData = new[]
         {
-            NewQp.Processed("invalid_profile_name", (1, 100))
+            NewQp.Processed("invalid_profile_name", ("id1", 1, 100))
         };
 
         var serviceData = new[]
@@ -41,7 +41,7 @@ public class QualityProfileTransactionPhaseTest
     {
         var guideData = new[]
         {
-            NewQp.Processed("profile1", (1, 100), (2, 500))
+            NewQp.Processed("profile1", ("id1", 1, 100), ("id2", 2, 500))
         };
 
         var serviceData = new[]
@@ -49,7 +49,7 @@ public class QualityProfileTransactionPhaseTest
             new QualityProfileDto
             {
                 Name = "profile1",
-                FormatItems =
+                FormatItems = new[]
                 {
                     new ProfileFormatItemDto
                     {
@@ -69,38 +69,13 @@ public class QualityProfileTransactionPhaseTest
 
         var result = sut.Execute(guideData, serviceData);
 
-        result.Should().BeEquivalentTo(new QualityProfileTransactionData
-        {
-            UpdatedProfiles =
+        result.UpdatedProfiles.Should()
+            .ContainSingle().Which.UpdatedScores.Should()
+            .BeEquivalentTo(new[]
             {
-                new UpdatedQualityProfile(new QualityProfileDto
-                {
-                    Name = "profile1",
-                    FormatItems =
-                    {
-                        new ProfileFormatItemDto
-                        {
-                            Name = "quality1",
-                            Format = 1,
-                            Score = 100
-                        },
-                        new ProfileFormatItemDto
-                        {
-                            Name = "quality2",
-                            Format = 2,
-                            Score = 500
-                        }
-                    }
-                })
-                {
-                    UpdatedScores =
-                    {
-                        new UpdatedFormatScore("quality1", 200, 100, FormatScoreUpdateReason.Updated),
-                        new UpdatedFormatScore("quality2", 300, 500, FormatScoreUpdateReason.Updated)
-                    }
-                }
-            }
-        });
+                NewQp.UpdatedScore("quality1", 200, 100, FormatScoreUpdateReason.Updated),
+                NewQp.UpdatedScore("quality2", 300, 500, FormatScoreUpdateReason.Updated)
+            }, o => o.Excluding(x => x.Dto.Format));
     }
 
     [Test, AutoMockData]
@@ -114,7 +89,7 @@ public class QualityProfileTransactionPhaseTest
             new QualityProfileDto
             {
                 Name = "profile1",
-                FormatItems =
+                FormatItems = new[]
                 {
                     new ProfileFormatItemDto
                     {
@@ -145,7 +120,7 @@ public class QualityProfileTransactionPhaseTest
         // Profile name must match but the format IDs for each quality should not
         var guideData = new[]
         {
-            NewQp.Processed("profile1", (1, 200), (2, 300))
+            NewQp.Processed("profile1", ("id1", 1, 200), ("id2", 2, 300))
         };
 
         var serviceData = new[]
@@ -153,7 +128,7 @@ public class QualityProfileTransactionPhaseTest
             new QualityProfileDto
             {
                 Name = "profile1",
-                FormatItems =
+                FormatItems = new[]
                 {
                     new ProfileFormatItemDto
                     {
@@ -182,7 +157,7 @@ public class QualityProfileTransactionPhaseTest
     {
         var guideData = new[]
         {
-            NewQp.Processed("profile1", true, (3, 100), (4, 500))
+            NewQp.Processed("profile1", true, ("quality3", "id3", 3, 100), ("quality4", "id4", 4, 500))
         };
 
         var serviceData = new[]
@@ -190,7 +165,7 @@ public class QualityProfileTransactionPhaseTest
             new QualityProfileDto
             {
                 Name = "profile1",
-                FormatItems =
+                FormatItems = new[]
                 {
                     new ProfileFormatItemDto
                     {
@@ -210,37 +185,14 @@ public class QualityProfileTransactionPhaseTest
 
         var result = sut.Execute(guideData, serviceData);
 
-        result.Should().BeEquivalentTo(new QualityProfileTransactionData
-        {
-            UpdatedProfiles =
+        result.UpdatedProfiles.Should()
+            .ContainSingle().Which.UpdatedScores.Should()
+            .BeEquivalentTo(new[]
             {
-                new UpdatedQualityProfile(new QualityProfileDto
-                {
-                    Name = "profile1",
-                    FormatItems =
-                    {
-                        new ProfileFormatItemDto
-                        {
-                            Name = "quality1",
-                            Format = 1,
-                            Score = 0
-                        },
-                        new ProfileFormatItemDto
-                        {
-                            Name = "quality2",
-                            Format = 2,
-                            Score = 0
-                        }
-                    }
-                })
-                {
-                    UpdatedScores =
-                    {
-                        new UpdatedFormatScore("quality1", 200, 0, FormatScoreUpdateReason.Reset),
-                        new UpdatedFormatScore("quality2", 300, 0, FormatScoreUpdateReason.Reset)
-                    }
-                }
-            }
-        });
+                NewQp.UpdatedScore("quality1", 200, 0, FormatScoreUpdateReason.Reset),
+                NewQp.UpdatedScore("quality2", 300, 0, FormatScoreUpdateReason.Reset),
+                NewQp.UpdatedScore("quality3", 0, 100, FormatScoreUpdateReason.New),
+                NewQp.UpdatedScore("quality4", 0, 500, FormatScoreUpdateReason.New)
+            }, o => o.Excluding(x => x.Dto.Format));
     }
 }