From 3a50b9fa61b78a1cd37e87b9dbb10af9f0a2eb89 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sun, 27 Aug 2023 20:40:38 -0500 Subject: [PATCH] fix: Add validation for duplicate instances Two separate duplicate checks have been introduced: 1. Within the same YAML file, YamlDotNet has been instructed to error on duplicate keys. 2. Between different YAML files, custom logic enforces that there should be no duplicate instance names. --- CHANGELOG.md | 1 + .../ErrorHandling/ConsoleExceptionHandler.cs | 5 +++ .../Config/ConfigExtensions.cs | 9 +++++ .../Config/ConfigurationRegistry.cs | 6 ++++ .../Config/Yaml/YamlSerializerFactory.cs | 3 +- .../DuplicateInstancesException.cs | 11 ++++++ .../Config/ConfigExtensionsTest.cs | 18 ++++++++++ .../Config/ConfigurationRegistryTest.cs | 36 +++++++++++++++++++ 8 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 src/Recyclarr.TrashLib/ExceptionTypes/DuplicateInstancesException.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5754931c..657bf0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Service failures (e.g. HTTP 500) no longer cause exceptions (#206). +- Error out when duplicate instance names are used. ## [5.3.1] - 2023-08-21 diff --git a/src/Recyclarr.Cli/Processors/ErrorHandling/ConsoleExceptionHandler.cs b/src/Recyclarr.Cli/Processors/ErrorHandling/ConsoleExceptionHandler.cs index 815c6017..a5de6675 100644 --- a/src/Recyclarr.Cli/Processors/ErrorHandling/ConsoleExceptionHandler.cs +++ b/src/Recyclarr.Cli/Processors/ErrorHandling/ConsoleExceptionHandler.cs @@ -39,6 +39,11 @@ public class ConsoleExceptionHandler _log.Error("The following instances do not exist: {Names}", e.InstanceNames); break; + case DuplicateInstancesException e: + _log.Error("The following instance names are duplicated: {Names}", e.InstanceNames); + _log.Error("Instance names are unique and may not be reused"); + break; + case SplitInstancesException e: _log.Error("The following configs share the same `base_url`, which isn't allowed: {Instances}", e.InstanceNames); diff --git a/src/Recyclarr.TrashLib/Config/ConfigExtensions.cs b/src/Recyclarr.TrashLib/Config/ConfigExtensions.cs index 01dbb463..b2e9b1f0 100644 --- a/src/Recyclarr.TrashLib/Config/ConfigExtensions.cs +++ b/src/Recyclarr.TrashLib/Config/ConfigExtensions.cs @@ -52,4 +52,13 @@ public static class ConfigExtensions return criteria.Instances .Where(x => !configInstances.Contains(x, StringComparer.InvariantCultureIgnoreCase)); } + + public static IEnumerable GetDuplicateInstanceNames(this IEnumerable configs) + { + return configs + .GroupBy(x => x.InstanceName, StringComparer.InvariantCultureIgnoreCase) + .Where(x => x.Count() > 1) + .Select(x => x.First().InstanceName) + .ToList(); + } } diff --git a/src/Recyclarr.TrashLib/Config/ConfigurationRegistry.cs b/src/Recyclarr.TrashLib/Config/ConfigurationRegistry.cs index 8a24aac0..666b6efa 100644 --- a/src/Recyclarr.TrashLib/Config/ConfigurationRegistry.cs +++ b/src/Recyclarr.TrashLib/Config/ConfigurationRegistry.cs @@ -51,6 +51,12 @@ public class ConfigurationRegistry : IConfigurationRegistry { var loadedConfigs = configs.SelectMany(x => _loader.Load(x)).ToList(); + var dupeInstances = loadedConfigs.GetDuplicateInstanceNames().ToList(); + if (dupeInstances.Any()) + { + throw new DuplicateInstancesException(dupeInstances); + } + var invalidInstances = loadedConfigs.GetInvalidInstanceNames(filterCriteria).ToList(); if (invalidInstances.Any()) { diff --git a/src/Recyclarr.TrashLib/Config/Yaml/YamlSerializerFactory.cs b/src/Recyclarr.TrashLib/Config/Yaml/YamlSerializerFactory.cs index 3a566235..0f09197f 100644 --- a/src/Recyclarr.TrashLib/Config/Yaml/YamlSerializerFactory.cs +++ b/src/Recyclarr.TrashLib/Config/Yaml/YamlSerializerFactory.cs @@ -30,7 +30,8 @@ public class YamlSerializerFactory : IYamlSerializerFactory builder .WithNodeDeserializer(new ForceEmptySequences(_objectFactory)) .WithNodeTypeResolver(new ReadOnlyCollectionNodeTypeResolver()) - .WithObjectFactory(_objectFactory); + .WithObjectFactory(_objectFactory) + .WithDuplicateKeyChecking(); foreach (var behavior in _behaviors) { diff --git a/src/Recyclarr.TrashLib/ExceptionTypes/DuplicateInstancesException.cs b/src/Recyclarr.TrashLib/ExceptionTypes/DuplicateInstancesException.cs new file mode 100644 index 00000000..390deb5b --- /dev/null +++ b/src/Recyclarr.TrashLib/ExceptionTypes/DuplicateInstancesException.cs @@ -0,0 +1,11 @@ +namespace Recyclarr.TrashLib.ExceptionTypes; + +public class DuplicateInstancesException : Exception +{ + public IReadOnlyCollection InstanceNames { get; } + + public DuplicateInstancesException(IReadOnlyCollection instanceNames) + { + InstanceNames = instanceNames; + } +} diff --git a/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigExtensionsTest.cs b/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigExtensionsTest.cs index d2a0ed4e..778da5c6 100644 --- a/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigExtensionsTest.cs +++ b/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigExtensionsTest.cs @@ -105,4 +105,22 @@ public class ConfigExtensionsTest result.Should().BeEquivalentTo("radarr1", "radarr2", "sonarr2", "sonarr3"); } + + [Test] + public void Get_duplicate_instance_names() + { + var configs = new IServiceConfiguration[] + { + new RadarrConfiguration {InstanceName = "radarr1"}, + new RadarrConfiguration {InstanceName = "radarr2"}, + new RadarrConfiguration {InstanceName = "radarr2"}, + new RadarrConfiguration {InstanceName = "radarr3"}, + new SonarrConfiguration {InstanceName = "sonarr1"}, + new SonarrConfiguration {InstanceName = "sonarr1"} + }; + + var result = configs.GetDuplicateInstanceNames(); + + result.Should().BeEquivalentTo("radarr2", "sonarr1"); + } } diff --git a/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigurationRegistryTest.cs b/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigurationRegistryTest.cs index 1f051100..16bd9469 100644 --- a/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigurationRegistryTest.cs +++ b/src/tests/Recyclarr.TrashLib.Tests/Config/ConfigurationRegistryTest.cs @@ -99,4 +99,40 @@ public class ConfigurationRegistryTest : TrashLibIntegrationFixture act.Should().ThrowExactly() .Which.InstanceNames.Should().BeEquivalentTo("instance1", "instance2"); } + + [Test] + public void Duplicate_instance_names_are_prohibited() + { + var sut = Resolve(); + + Fs.AddFile("config1.yml", new MockFileData( + """ + radarr: + unique_name1: + base_url: http://localhost:7879 + api_key: fdsa + same_instance_name: + base_url: http://localhost:7878 + api_key: asdf + """)); + + Fs.AddFile("config2.yml", new MockFileData( + """ + radarr: + same_instance_name: + base_url: http://localhost:7879 + api_key: fdsa + unique_name2: + base_url: http://localhost:7879 + api_key: fdsa + """)); + + var act = () => sut.FindAndLoadConfigs(new ConfigFilterCriteria + { + ManualConfigFiles = new[] {"config1.yml", "config2.yml"} + }); + + act.Should().ThrowExactly() + .Which.InstanceNames.Should().BeEquivalentTo("same_instance_name"); + } }