From fe7773ea078eebae68d23d37731e02387a073115 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sat, 10 Jun 2023 13:50:14 -0500 Subject: [PATCH] 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 | 5 + .../Models/ProcessedCustomFormatCache.cs | 5 + .../QualityProfile/Api/QualityProfileDto.cs | 5 +- .../QualityProfileApiPersistencePhase.cs | 11 ++- .../QualityProfileConfigPhase.cs | 13 ++- .../QualityProfilePreviewPhase.cs | 6 +- .../QualityProfileTransactionPhase.cs | 58 ++++++------ .../QualityProfile/UpdatedFormatScore.cs | 41 +++++++-- src/tests/Recyclarr.Cli.TestLibrary/NewQp.cs | 33 ++++++- .../QualityProfileConfigPhaseTest.cs | 8 +- .../QualityProfileTransactionPhaseTest.cs | 92 +++++-------------- 11 files changed, 151 insertions(+), 126 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81288072..33e844c5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Recyclarr.Cli/Pipelines/CustomFormat/Models/ProcessedCustomFormatCache.cs b/src/Recyclarr.Cli/Pipelines/CustomFormat/Models/ProcessedCustomFormatCache.cs index e7f49bd0..a115fe72 100644 --- a/src/Recyclarr.Cli/Pipelines/CustomFormat/Models/ProcessedCustomFormatCache.cs +++ b/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); + } } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs index f1552ba9..2cf375ce 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/Api/QualityProfileDto.cs +++ b/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 FormatItems { get; } = new(); + public IReadOnlyCollection FormatItems { get; init; } = Array.Empty(); [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 ExtraJson { get; init; } = new(); diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileApiPersistencePhase.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileApiPersistencePhase.cs index 42af5eee..91f8efe6 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileApiPersistencePhase.cs +++ b/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); } } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhase.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhase.cs index 411047e1..fa69ab3c 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhase.cs +++ b/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 CfScores { get; init; } = new(); + public IList CfScores { get; init; } = new List(); } public class QualityProfileConfigPhase @@ -60,7 +62,7 @@ public class QualityProfileConfigPhase } private void AddCustomFormatScoreData( - IDictionary existingScoreData, + ICollection 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)); } } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfilePreviewPhase.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfilePreviewPhase.cs index 24a7e2a5..aea2dbf7 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfilePreviewPhase.cs +++ b/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()); } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhase.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhase.cs index 8ee1e2a9..d82873ac 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhase.cs +++ b/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 UpdatedScores { get; } = new(); + public required IReadOnlyCollection 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; } } diff --git a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedFormatScore.cs b/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedFormatScore.cs index f8def094..5106a691 100644 --- a/src/Recyclarr.Cli/Pipelines/QualityProfile/UpdatedFormatScore.cs +++ b/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 { + /// + /// A score who's value did not change. + /// + NoChange, + + /// + /// A score that is changed. + /// Updated, - Reset + + /// + /// Scores were reset to a 0 value because `reset_unmatched_scores` was set to `true`. + /// + Reset, + + /// + /// 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. + /// + 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; + } +} diff --git a/src/tests/Recyclarr.Cli.TestLibrary/NewQp.cs b/src/tests/Recyclarr.Cli.TestLibrary/NewQp.cs index 8e52fe5e..f7ffd78b 100644 --- a/src/tests/Recyclarr.Cli.TestLibrary/NewQp.cs +++ b/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 }; } } diff --git a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhaseTest.cs b/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhaseTest.cs index 74899e6b..70e43cf2 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileConfigPhaseTest.cs +++ b/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)) }); } } 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 eed98833..37e2f429 100644 --- a/src/tests/Recyclarr.Cli.Tests/Pipelines/QualityProfile/PipelinePhases/QualityProfileTransactionPhaseTest.cs +++ b/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)); } }