From 5c3da551bb8b21dc293f1a3c000a444863706577 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Thu, 5 Jan 2023 12:27:32 -0600 Subject: [PATCH] fix: Detect and warn about conflicting CFs during sync --- CHANGELOG.md | 5 ++ .../JsonTransactionStepTest.cs | 48 +++++++++--- .../CustomFormat/CustomFormatUpdater.cs | 15 +++- .../Models/ConflictingCustomFormat.cs | 6 ++ .../Processors/PersistenceProcessor.cs | 3 +- .../PersistenceSteps/JsonTransactionStep.cs | 73 +++++++++++++------ 6 files changed, 117 insertions(+), 33 deletions(-) create mode 100644 src/Recyclarr.TrashLib/Services/CustomFormat/Models/ConflictingCustomFormat.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index c0db18ed..829ce97a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Custom Formats: Updates that conflict with existing CFs in Sonarr/Radarr are now skipped and a + warning is printed. + ## [4.1.0] - 2022-12-30 ### Added diff --git a/src/Recyclarr.TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs b/src/Recyclarr.TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs index ad4d1a2e..cdfac82f 100644 --- a/src/Recyclarr.TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs +++ b/src/Recyclarr.TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs @@ -2,6 +2,7 @@ using FluentAssertions; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using NUnit.Framework; +using Recyclarr.TestLibrary.AutoFixture; using Recyclarr.TestLibrary.FluentAssertions; using Recyclarr.TrashLib.Services.CustomFormat.Models; using Recyclarr.TrashLib.Services.CustomFormat.Models.Cache; @@ -42,8 +43,9 @@ namespace Recyclarr.TrashLib.Tests.CustomFormat.Processors.PersistenceSteps; [Parallelizable(ParallelScope.All)] public class JsonTransactionStepTest { - [Test] - public void Combination_of_create_update_and_unchanged_and_verify_proper_json_merging() + [Test, AutoMockData] + public void Combination_of_create_update_and_unchanged_and_verify_proper_json_merging( + JsonTransactionStep processor) { var radarrCfs = JsonConvert.DeserializeObject>(@" [{ @@ -124,7 +126,6 @@ public class JsonTransactionStepTest NewCf.Processed("no_change", "id3", guideCfData[2], new TrashIdMapping("id3", "", 3)) }; - var processor = new JsonTransactionStep(); processor.Process(guideCfs, radarrCfs); var expectedJson = new[] @@ -188,10 +189,13 @@ public class JsonTransactionStepTest processor.Transactions.UnchangedCustomFormats.First().Json.Should() .BeEquivalentTo(JObject.Parse(expectedJson[2]), op => op.Using(new JsonEquivalencyStep())); + + processor.Transactions.ConflictingCustomFormats.Should().BeEmpty(); } - [Test] - public void Deletes_happen_before_updates() + [Test, AutoMockData] + public void Deletes_happen_before_updates( + JsonTransactionStep processor) { const string radarrCfData = @"[{ 'id': 1, @@ -237,7 +241,6 @@ public class JsonTransactionStepTest var radarrCfs = JsonConvert.DeserializeObject>(radarrCfData); - var processor = new JsonTransactionStep(); processor.Process(guideCfs, radarrCfs!); processor.RecordDeletions(deletedCfsInCache, radarrCfs!); @@ -261,8 +264,9 @@ public class JsonTransactionStepTest .BeEquivalentTo(JObject.Parse(expectedJson), op => op.Using(new JsonEquivalencyStep())); } - [Test] - public void Only_delete_correct_cfs() + [Test, AutoMockData] + public void Only_delete_correct_cfs( + JsonTransactionStep processor) { const string radarrCfData = @"[{ 'id': 1, @@ -296,11 +300,37 @@ public class JsonTransactionStepTest var radarrCfs = JsonConvert.DeserializeObject>(radarrCfData); - var processor = new JsonTransactionStep(); processor.RecordDeletions(deletedCfsInCache, radarrCfs!); var expectedTransactions = new CustomFormatTransactionData(); expectedTransactions.DeletedCustomFormatIds.Add(new TrashIdMapping("testtrashid", "", 2)); processor.Transactions.Should().BeEquivalentTo(expectedTransactions); } + + [Test, AutoMockData] + public void Conflicting_ids_detected( + JsonTransactionStep processor) + { + const string serviceCfData = @" +[{ + 'id': 1, + 'name': 'first' +}, { + 'id': 2, + 'name': 'second' +}]"; + + var serviceCfs = JsonConvert.DeserializeObject>(serviceCfData)!; + + var guideCfs = new List + { + NewCf.Processed("first", "", new TrashIdMapping("", "first", 2)) + }; + + processor.Process(guideCfs, serviceCfs); + + var expectedTransactions = new CustomFormatTransactionData(); + expectedTransactions.ConflictingCustomFormats.Add(new ConflictingCustomFormat(guideCfs[0], 1)); + processor.Transactions.Should().BeEquivalentTo(expectedTransactions); + } } diff --git a/src/Recyclarr.TrashLib/Services/CustomFormat/CustomFormatUpdater.cs b/src/Recyclarr.TrashLib/Services/CustomFormat/CustomFormatUpdater.cs index a88cb120..72abbe6b 100644 --- a/src/Recyclarr.TrashLib/Services/CustomFormat/CustomFormatUpdater.cs +++ b/src/Recyclarr.TrashLib/Services/CustomFormat/CustomFormatUpdater.cs @@ -50,8 +50,10 @@ internal class CustomFormatUpdater : ICustomFormatUpdater return; } - await _persistenceProcessor.PersistCustomFormats(_guideProcessor.ProcessedCustomFormats, - _guideProcessor.DeletedCustomFormatsInCache, _guideProcessor.ProfileScores); + await _persistenceProcessor.PersistCustomFormats( + _guideProcessor.ProcessedCustomFormats, + _guideProcessor.DeletedCustomFormatsInCache, + _guideProcessor.ProfileScores); PrintApiStatistics(_persistenceProcessor.Transactions); PrintQualityProfileUpdates(); @@ -94,6 +96,15 @@ internal class CustomFormatUpdater : ICustomFormatUpdater private void PrintApiStatistics(CustomFormatTransactionData transactions) { + foreach (var (guideCf, conflictingId) in transactions.ConflictingCustomFormats) + { + _log.Warning( + "Custom Format with name {Name} (Trash ID: {TrashId}) will be skipped because another " + + "CF already exists with that name (ID: {ConflictId}). To fix the conflict, delete or " + + "rename the CF with the mentioned name", + guideCf.Name, guideCf.TrashId, conflictingId); + } + var created = transactions.NewCustomFormats; if (created.Count > 0) { diff --git a/src/Recyclarr.TrashLib/Services/CustomFormat/Models/ConflictingCustomFormat.cs b/src/Recyclarr.TrashLib/Services/CustomFormat/Models/ConflictingCustomFormat.cs new file mode 100644 index 00000000..9a35c771 --- /dev/null +++ b/src/Recyclarr.TrashLib/Services/CustomFormat/Models/ConflictingCustomFormat.cs @@ -0,0 +1,6 @@ +namespace Recyclarr.TrashLib.Services.CustomFormat.Models; + +public record ConflictingCustomFormat( + ProcessedCustomFormatData GuideCf, + int ConflictingId +); diff --git a/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceProcessor.cs b/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceProcessor.cs index f5c7dbe5..4c1f10a9 100644 --- a/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceProcessor.cs +++ b/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceProcessor.cs @@ -41,7 +41,8 @@ internal class PersistenceProcessor : IPersistenceProcessor public IReadOnlyCollection InvalidProfileNames => _steps.ProfileQualityProfileApiPersister.InvalidProfileNames; - public async Task PersistCustomFormats(IReadOnlyCollection guideCfs, + public async Task PersistCustomFormats( + IReadOnlyCollection guideCfs, IEnumerable deletedCfsInCache, IDictionary profileScores) { diff --git a/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs b/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs index f15bd0a6..e7269f23 100644 --- a/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs +++ b/src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs @@ -12,17 +12,18 @@ public class CustomFormatTransactionData public Collection UpdatedCustomFormats { get; } = new(); public Collection DeletedCustomFormatIds { get; } = new(); public Collection UnchangedCustomFormats { get; } = new(); + public Collection ConflictingCustomFormats { get; } = new(); } -internal class JsonTransactionStep : IJsonTransactionStep +public class JsonTransactionStep : IJsonTransactionStep { public CustomFormatTransactionData Transactions { get; } = new(); - public void Process(IEnumerable guideCfs, + public void Process( + IEnumerable guideCfs, IReadOnlyCollection serviceCfs) { - foreach (var (guideCf, serviceCf) in guideCfs - .Select(gcf => (GuideCf: gcf, ServiceCf: FindServiceCf(serviceCfs, gcf)))) + foreach (var (guideCf, serviceCf) in guideCfs.Select(gcf => (gcf, FindServiceCf(serviceCfs, gcf)))) { var guideCfJson = BuildNewServiceCf(guideCf.Json); @@ -31,31 +32,61 @@ internal class JsonTransactionStep : IJsonTransactionStep { guideCf.Json = guideCfJson; Transactions.NewCustomFormats.Add(guideCf); + continue; } - // found match in radarr CFs; update the existing CF + + // If cache entry is NOT null, that means we found the service by its ID + if (guideCf.CacheEntry is not null) + { + // Check for conflicts with upstream CFs with the same name but different ID. + // If found, it is recorded and we skip this CF. + if (DetectConflictingCustomFormats(serviceCfs, guideCf, serviceCf)) + { + continue; + } + } + // Null cache entry use case else { - guideCf.Json = (JObject) serviceCf.DeepClone(); - UpdateServiceCf(guideCf.Json, guideCfJson); - // Set the cache for use later (like updating scores) if it hasn't been updated already. - // This handles CFs that already exist in Radarr but aren't cached (they will be added to cache + // This handles CFs that already exist in the service but aren't cached (they will be added to cache // later). - if (guideCf.CacheEntry == null) - { - guideCf.SetCache(guideCf.Json.Value("id")); - } + guideCf.SetCache(guideCf.Json.Value("id")); + } - if (!JToken.DeepEquals(serviceCf, guideCf.Json)) - { - Transactions.UpdatedCustomFormats.Add(guideCf); - } - else - { - Transactions.UnchangedCustomFormats.Add(guideCf); - } + guideCf.Json = (JObject) serviceCf.DeepClone(); + UpdateServiceCf(guideCf.Json, guideCfJson); + + if (!JToken.DeepEquals(serviceCf, guideCf.Json)) + { + Transactions.UpdatedCustomFormats.Add(guideCf); } + else + { + Transactions.UnchangedCustomFormats.Add(guideCf); + } + } + } + + private bool DetectConflictingCustomFormats( + IReadOnlyCollection serviceCfs, + ProcessedCustomFormatData guideCf, + JObject serviceCf) + { + var conflictingServiceCf = FindServiceCf(serviceCfs, null, guideCf.Name); + if (conflictingServiceCf is null) + { + return false; } + + var conflictingId = conflictingServiceCf.Value("id"); + if (conflictingId == serviceCf.Value("id")) + { + return false; + } + + Transactions.ConflictingCustomFormats.Add(new ConflictingCustomFormat(guideCf, conflictingId)); + return true; } public void RecordDeletions(IEnumerable deletedCfsInCache, IEnumerable serviceCfs)