fix: Detect and warn about conflicting CFs during sync

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

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]
+    [Test, AutoMockData]
-    public void Combination_of_create_update_and_unchanged_and_verify_proper_json_merging()
+    public void Combination_of_create_update_and_unchanged_and_verify_proper_json_merging(
+        JsonTransactionStep processor)
     {
         var radarrCfs = JsonConvert.DeserializeObject<List<JObject>>(@"
 [{
@@ -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]
+    [Test, AutoMockData]
-    public void Deletes_happen_before_updates()
+    public void Deletes_happen_before_updates(
+        JsonTransactionStep processor)
     {
         const string radarrCfData = @"[{
   'id': 1,
@@ -237,7 +241,6 @@ public class JsonTransactionStepTest
 
         var radarrCfs = JsonConvert.DeserializeObject<List<JObject>>(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]
+    [Test, AutoMockData]
-    public void Only_delete_correct_cfs()
+    public void Only_delete_correct_cfs(
+        JsonTransactionStep processor)
     {
         const string radarrCfData = @"[{
   'id': 1,
@@ -296,11 +300,37 @@ public class JsonTransactionStepTest
 
         var radarrCfs = JsonConvert.DeserializeObject<List<JObject>>(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<List<JObject>>(serviceCfData)!;
+
+        var guideCfs = new List<ProcessedCustomFormatData>
+        {
+            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);
+    }
 }

src/Recyclarr.TrashLib/Services/CustomFormat/CustomFormatUpdater.cs

@@ -50,8 +50,10 @@ internal class CustomFormatUpdater : ICustomFormatUpdater
             return;
         }
 
-        await _persistenceProcessor.PersistCustomFormats(_guideProcessor.ProcessedCustomFormats,
+        await _persistenceProcessor.PersistCustomFormats(
-            _guideProcessor.DeletedCustomFormatsInCache, _guideProcessor.ProfileScores);
+            _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)
         {

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

src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceProcessor.cs

@@ -41,7 +41,8 @@ internal class PersistenceProcessor : IPersistenceProcessor
     public IReadOnlyCollection<string> InvalidProfileNames
         => _steps.ProfileQualityProfileApiPersister.InvalidProfileNames;
 
-    public async Task PersistCustomFormats(IReadOnlyCollection<ProcessedCustomFormatData> guideCfs,
+    public async Task PersistCustomFormats(
+        IReadOnlyCollection<ProcessedCustomFormatData> guideCfs,
         IEnumerable<TrashIdMapping> deletedCfsInCache,
         IDictionary<string, QualityProfileCustomFormatScoreMapping> profileScores)
     {

src/Recyclarr.TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs

@@ -12,17 +12,18 @@ public class CustomFormatTransactionData
     public Collection<ProcessedCustomFormatData> UpdatedCustomFormats { get; } = new();
     public Collection<TrashIdMapping> DeletedCustomFormatIds { get; } = new();
     public Collection<ProcessedCustomFormatData> UnchangedCustomFormats { get; } = new();
+    public Collection<ConflictingCustomFormat> ConflictingCustomFormats { get; } = new();
 }
 
-internal class JsonTransactionStep : IJsonTransactionStep
+public class JsonTransactionStep : IJsonTransactionStep
 {
     public CustomFormatTransactionData Transactions { get; } = new();
 
-    public void Process(IEnumerable<ProcessedCustomFormatData> guideCfs,
+    public void Process(
+        IEnumerable<ProcessedCustomFormatData> guideCfs,
         IReadOnlyCollection<JObject> serviceCfs)
     {
-        foreach (var (guideCf, serviceCf) in guideCfs
+        foreach (var (guideCf, serviceCf) in guideCfs.Select(gcf => (gcf, FindServiceCf(serviceCfs, gcf))))
-                     .Select(gcf => (GuideCf: gcf, ServiceCf: 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<int>("id"));
-                {
+            }
-                    guideCf.SetCache(guideCf.Json.Value<int>("id"));
-                }
 
-                if (!JToken.DeepEquals(serviceCf, guideCf.Json))
+            guideCf.Json = (JObject) serviceCf.DeepClone();
-                {
+            UpdateServiceCf(guideCf.Json, guideCfJson);
-                    Transactions.UpdatedCustomFormats.Add(guideCf);
+
-                }
+            if (!JToken.DeepEquals(serviceCf, guideCf.Json))
-                else
+            {
-                {
+                Transactions.UpdatedCustomFormats.Add(guideCf);
-                    Transactions.UnchangedCustomFormats.Add(guideCf);
-                }
             }
+            else
+            {
+                Transactions.UnchangedCustomFormats.Add(guideCf);
+            }
+        }
+    }
+
+    private bool DetectConflictingCustomFormats(
+        IReadOnlyCollection<JObject> serviceCfs,
+        ProcessedCustomFormatData guideCf,
+        JObject serviceCf)
+    {
+        var conflictingServiceCf = FindServiceCf(serviceCfs, null, guideCf.Name);
+        if (conflictingServiceCf is null)
+        {
+            return false;
         }
+
+        var conflictingId = conflictingServiceCf.Value<int>("id");
+        if (conflictingId == serviceCf.Value<int>("id"))
+        {
+            return false;
+        }
+
+        Transactions.ConflictingCustomFormats.Add(new ConflictingCustomFormat(guideCf, conflictingId));
+        return true;
     }
 
     public void RecordDeletions(IEnumerable<TrashIdMapping> deletedCfsInCache, IEnumerable<JObject> serviceCfs)