From cda317d3c357e72787139b6c293c137e35e7be37 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sun, 23 Oct 2022 13:27:08 -0500 Subject: [PATCH] fix: Run validation against Custom Formats in Sonarr If custom formats are provided, run validation logic against them. This also was an opportunity to make the ServiceConfiguration validation logic reusable between Sonarr and Radarr. --- CHANGELOG.md | 4 ++ .../Services/ServiceConfigurationTest.cs | 62 +++++++++++++++++++ .../Radarr/RadarrConfigurationTest.cs | 23 ++----- .../Sonarr/SonarrConfigurationTest.cs | 25 ++------ src/TrashLib/Config/ConfigAutofacModule.cs | 2 + .../Services/IServiceValidationMessages.cs | 8 +++ .../Config/Services/ServiceConfiguration.cs | 2 +- .../Services/ServiceConfigurationValidator.cs | 39 ++++++++++++ .../Services/ServiceValidationMessages.cs | 13 ++++ .../Config/IRadarrValidationMessages.cs | 3 - .../Config/RadarrConfigurationValidator.cs | 30 +-------- .../Radarr/Config/RadarrValidationMessages.cs | 9 --- .../Config/ISonarrValidationMessages.cs | 2 - .../Config/SonarrConfigurationValidator.cs | 5 +- .../Sonarr/Config/SonarrValidationMessages.cs | 6 -- 15 files changed, 146 insertions(+), 87 deletions(-) create mode 100644 src/TrashLib.Tests/Config/Services/ServiceConfigurationTest.cs create mode 100644 src/TrashLib/Config/Services/IServiceValidationMessages.cs create mode 100644 src/TrashLib/Config/Services/ServiceConfigurationValidator.cs create mode 100644 src/TrashLib/Config/Services/ServiceValidationMessages.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5f3365..b95cc279 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ you need to make. - **BREAKING**: The deprecated feature that still allowed you to keep your `recyclarr.yml` next to the executable has been removed. +### Fixed + +- Sonarr: Run validation on Custom Formats configuration, if specified, to check for errors. + [breaking3]: https://recyclarr.dev/wiki/upgrade-guide/upgrade-guide-v3.0 ## [2.6.1] - 2022-10-15 diff --git a/src/TrashLib.Tests/Config/Services/ServiceConfigurationTest.cs b/src/TrashLib.Tests/Config/Services/ServiceConfigurationTest.cs new file mode 100644 index 00000000..181b9d43 --- /dev/null +++ b/src/TrashLib.Tests/Config/Services/ServiceConfigurationTest.cs @@ -0,0 +1,62 @@ +using FluentAssertions; +using FluentValidation; +using NUnit.Framework; +using Recyclarr.TestLibrary; +using TrashLib.Config.Services; + +namespace TrashLib.Tests.Config.Services; + +[TestFixture] +[Parallelizable(ParallelScope.All)] +public class ServiceConfigurationTest : IntegrationFixture +{ + [Test] + public void Validation_fails_for_all_missing_required_properties() + { + // default construct which should yield default values (invalid) for all required properties + var config = new ServiceConfiguration(); + + var validator = ServiceLocator.Resolve>(); + + var result = validator.Validate(config); + + var messages = new ServiceValidationMessages(); + var expectedErrorMessageSubstrings = new[] + { + messages.ApiKey, + messages.BaseUrl + }; + + result.IsValid.Should().BeFalse(); + result.Errors.Select(e => e.ErrorMessage) + .Should().BeEquivalentTo(expectedErrorMessageSubstrings); + } + + [Test] + public void Fail_when_trash_ids_missing() + { + var config = new ServiceConfiguration + { + BaseUrl = "valid", + ApiKey = "valid", + CustomFormats = new List + { + new() // Empty to force validation failure + } + }; + + var validator = ServiceLocator.Resolve>(); + + var result = validator.Validate(config); + + var messages = new ServiceValidationMessages(); + var expectedErrorMessageSubstrings = new[] + { + messages.CustomFormatTrashIds + }; + + result.IsValid.Should().BeFalse(); + result.Errors.Select(e => e.ErrorMessage) + .Should().BeEquivalentTo(expectedErrorMessageSubstrings); + } +} diff --git a/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs b/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs index 7a056430..c9d5a02b 100644 --- a/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs +++ b/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs @@ -1,30 +1,17 @@ using System.Collections.ObjectModel; -using Autofac; using FluentAssertions; using FluentValidation; using NUnit.Framework; -using TrashLib.Config; +using Recyclarr.TestLibrary; using TrashLib.Config.Services; -using TrashLib.Services.Radarr; using TrashLib.Services.Radarr.Config; namespace TrashLib.Tests.Radarr; [TestFixture] [Parallelizable(ParallelScope.All)] -public class RadarrConfigurationTest +public class RadarrConfigurationTest : IntegrationFixture { - private IContainer _container = default!; - - [OneTimeSetUp] - public void Setup() - { - var builder = new ContainerBuilder(); - builder.RegisterModule(); - builder.RegisterModule(); - _container = builder.Build(); - } - [Test] public void Custom_format_is_valid_with_trash_id() { @@ -38,7 +25,7 @@ public class RadarrConfigurationTest } }; - var validator = _container.Resolve>(); + var validator = ServiceLocator.Resolve>(); var result = validator.Validate(config); result.IsValid.Should().BeTrue(); @@ -50,7 +37,7 @@ public class RadarrConfigurationTest { // default construct which should yield default values (invalid) for all required properties var config = new RadarrConfiguration(); - var validator = _container.Resolve>(); + var validator = ServiceLocator.Resolve>(); var result = validator.Validate(config); @@ -92,7 +79,7 @@ public class RadarrConfigurationTest } }; - var validator = _container.Resolve>(); + var validator = ServiceLocator.Resolve>(); var result = validator.Validate(config); result.IsValid.Should().BeTrue(); diff --git a/src/TrashLib.Tests/Sonarr/SonarrConfigurationTest.cs b/src/TrashLib.Tests/Sonarr/SonarrConfigurationTest.cs index 2a765a6e..3b99f174 100644 --- a/src/TrashLib.Tests/Sonarr/SonarrConfigurationTest.cs +++ b/src/TrashLib.Tests/Sonarr/SonarrConfigurationTest.cs @@ -1,47 +1,34 @@ -using Autofac; using FluentAssertions; using FluentValidation; using NUnit.Framework; -using TrashLib.Config; -using TrashLib.Services.Sonarr; +using Recyclarr.TestLibrary; using TrashLib.Services.Sonarr.Config; namespace TrashLib.Tests.Sonarr; [TestFixture] [Parallelizable(ParallelScope.All)] -public class SonarrConfigurationTest +public class SonarrConfigurationTest : IntegrationFixture { - private IContainer _container = default!; - - [OneTimeSetUp] - public void Setup() - { - var builder = new ContainerBuilder(); - builder.RegisterModule(); - builder.RegisterModule(); - _container = builder.Build(); - } - [Test] public void Validation_fails_for_all_missing_required_properties() { // default construct which should yield default values (invalid) for all required properties var config = new SonarrConfiguration { + BaseUrl = "valid", + ApiKey = "valid", // validation is only applied to actual release profile elements. Not if it's empty. ReleaseProfiles = new[] {new ReleaseProfileConfig()} }; - var validator = _container.Resolve>(); + var validator = ServiceLocator.Resolve>(); var result = validator.Validate(config); var messages = new SonarrValidationMessages(); var expectedErrorMessageSubstrings = new[] { - messages.ApiKey, - messages.BaseUrl, messages.ReleaseProfileTrashIds }; @@ -63,7 +50,7 @@ public class SonarrConfigurationTest } }; - var validator = _container.Resolve>(); + var validator = ServiceLocator.Resolve>(); var result = validator.Validate(config); result.IsValid.Should().BeTrue(); diff --git a/src/TrashLib/Config/ConfigAutofacModule.cs b/src/TrashLib/Config/ConfigAutofacModule.cs index cd951118..4daddd5f 100644 --- a/src/TrashLib/Config/ConfigAutofacModule.cs +++ b/src/TrashLib/Config/ConfigAutofacModule.cs @@ -1,6 +1,7 @@ using System.Reflection; using Autofac; using FluentValidation; +using TrashLib.Config.Services; using TrashLib.Config.Settings; using Module = Autofac.Module; @@ -16,5 +17,6 @@ public class ConfigAutofacModule : Module builder.RegisterType().As().SingleInstance(); builder.RegisterType().As(); + builder.RegisterType().As(); } } diff --git a/src/TrashLib/Config/Services/IServiceValidationMessages.cs b/src/TrashLib/Config/Services/IServiceValidationMessages.cs new file mode 100644 index 00000000..cdf78338 --- /dev/null +++ b/src/TrashLib/Config/Services/IServiceValidationMessages.cs @@ -0,0 +1,8 @@ +namespace TrashLib.Config.Services; + +public interface IServiceValidationMessages +{ + string BaseUrl { get; } + string ApiKey { get; } + string CustomFormatTrashIds { get; } +} diff --git a/src/TrashLib/Config/Services/ServiceConfiguration.cs b/src/TrashLib/Config/Services/ServiceConfiguration.cs index dfc5b1c0..eae6c1b8 100644 --- a/src/TrashLib/Config/Services/ServiceConfiguration.cs +++ b/src/TrashLib/Config/Services/ServiceConfiguration.cs @@ -2,7 +2,7 @@ using JetBrains.Annotations; namespace TrashLib.Config.Services; -public abstract class ServiceConfiguration : IServiceConfiguration +public class ServiceConfiguration : IServiceConfiguration { public string BaseUrl { get; init; } = ""; public string ApiKey { get; init; } = ""; diff --git a/src/TrashLib/Config/Services/ServiceConfigurationValidator.cs b/src/TrashLib/Config/Services/ServiceConfigurationValidator.cs new file mode 100644 index 00000000..86a8aeaf --- /dev/null +++ b/src/TrashLib/Config/Services/ServiceConfigurationValidator.cs @@ -0,0 +1,39 @@ +using FluentValidation; +using JetBrains.Annotations; +using TrashLib.Services.Radarr.Config; + +namespace TrashLib.Config.Services; + +[UsedImplicitly] +internal class ServiceConfigurationValidator : AbstractValidator +{ + public ServiceConfigurationValidator( + IServiceValidationMessages messages, + IValidator customFormatConfigValidator) + { + RuleFor(x => x.BaseUrl).NotEmpty().WithMessage(messages.BaseUrl); + RuleFor(x => x.ApiKey).NotEmpty().WithMessage(messages.ApiKey); + RuleForEach(x => x.CustomFormats).SetValidator(customFormatConfigValidator); + } +} + +[UsedImplicitly] +internal class CustomFormatConfigValidator : AbstractValidator +{ + public CustomFormatConfigValidator( + IServiceValidationMessages messages, + IValidator qualityProfileScoreConfigValidator) + { + RuleFor(x => x.TrashIds).NotEmpty().WithMessage(messages.CustomFormatTrashIds); + RuleForEach(x => x.QualityProfiles).SetValidator(qualityProfileScoreConfigValidator); + } +} + +[UsedImplicitly] +internal class QualityProfileScoreConfigValidator : AbstractValidator +{ + public QualityProfileScoreConfigValidator(IRadarrValidationMessages messages) + { + RuleFor(x => x.Name).NotEmpty().WithMessage(messages.QualityProfileName); + } +} diff --git a/src/TrashLib/Config/Services/ServiceValidationMessages.cs b/src/TrashLib/Config/Services/ServiceValidationMessages.cs new file mode 100644 index 00000000..7bb8ec45 --- /dev/null +++ b/src/TrashLib/Config/Services/ServiceValidationMessages.cs @@ -0,0 +1,13 @@ +namespace TrashLib.Config.Services; + +internal /*abstract*/ class ServiceValidationMessages : IServiceValidationMessages +{ + public string BaseUrl => + "Property 'base_url' is required"; + + public string ApiKey => + "Property 'api_key' is required"; + + public string CustomFormatTrashIds => + "'custom_formats' elements must contain at least one element under 'trash_ids'"; +} diff --git a/src/TrashLib/Services/Radarr/Config/IRadarrValidationMessages.cs b/src/TrashLib/Services/Radarr/Config/IRadarrValidationMessages.cs index 45d70178..57782ebf 100644 --- a/src/TrashLib/Services/Radarr/Config/IRadarrValidationMessages.cs +++ b/src/TrashLib/Services/Radarr/Config/IRadarrValidationMessages.cs @@ -2,9 +2,6 @@ namespace TrashLib.Services.Radarr.Config; public interface IRadarrValidationMessages { - string BaseUrl { get; } - string ApiKey { get; } - string CustomFormatTrashIds { get; } string QualityProfileName { get; } string QualityDefinitionType { get; } } diff --git a/src/TrashLib/Services/Radarr/Config/RadarrConfigurationValidator.cs b/src/TrashLib/Services/Radarr/Config/RadarrConfigurationValidator.cs index 3f57af86..02650df4 100644 --- a/src/TrashLib/Services/Radarr/Config/RadarrConfigurationValidator.cs +++ b/src/TrashLib/Services/Radarr/Config/RadarrConfigurationValidator.cs @@ -9,35 +9,11 @@ namespace TrashLib.Services.Radarr.Config; internal class RadarrConfigurationValidator : AbstractValidator { public RadarrConfigurationValidator( - IRadarrValidationMessages messages, - IValidator qualityDefinitionConfigValidator, - IValidator customFormatConfigValidator) + IValidator serviceConfigValidator, + IValidator qualityDefinitionConfigValidator) { - RuleFor(x => x.BaseUrl).NotEmpty().WithMessage(messages.BaseUrl); - RuleFor(x => x.ApiKey).NotEmpty().WithMessage(messages.ApiKey); + Include(serviceConfigValidator); RuleFor(x => x.QualityDefinition).SetNonNullableValidator(qualityDefinitionConfigValidator); - RuleForEach(x => x.CustomFormats).SetValidator(customFormatConfigValidator); - } -} - -[UsedImplicitly] -internal class CustomFormatConfigValidator : AbstractValidator -{ - public CustomFormatConfigValidator( - IRadarrValidationMessages messages, - IValidator qualityProfileScoreConfigValidator) - { - RuleFor(x => x.TrashIds).NotEmpty().WithMessage(messages.CustomFormatTrashIds); - RuleForEach(x => x.QualityProfiles).SetValidator(qualityProfileScoreConfigValidator); - } -} - -[UsedImplicitly] -internal class QualityProfileScoreConfigValidator : AbstractValidator -{ - public QualityProfileScoreConfigValidator(IRadarrValidationMessages messages) - { - RuleFor(x => x.Name).NotEmpty().WithMessage(messages.QualityProfileName); } } diff --git a/src/TrashLib/Services/Radarr/Config/RadarrValidationMessages.cs b/src/TrashLib/Services/Radarr/Config/RadarrValidationMessages.cs index 23fd34a9..538e48ed 100644 --- a/src/TrashLib/Services/Radarr/Config/RadarrValidationMessages.cs +++ b/src/TrashLib/Services/Radarr/Config/RadarrValidationMessages.cs @@ -2,15 +2,6 @@ namespace TrashLib.Services.Radarr.Config; internal class RadarrValidationMessages : IRadarrValidationMessages { - public string BaseUrl => - "Property 'base_url' is required"; - - public string ApiKey => - "Property 'api_key' is required"; - - public string CustomFormatTrashIds => - "'custom_formats' elements must contain at least one element under 'trash_ids'"; - public string QualityProfileName => "'name' is required for elements under 'quality_profiles'"; diff --git a/src/TrashLib/Services/Sonarr/Config/ISonarrValidationMessages.cs b/src/TrashLib/Services/Sonarr/Config/ISonarrValidationMessages.cs index fc560bb5..ca123537 100644 --- a/src/TrashLib/Services/Sonarr/Config/ISonarrValidationMessages.cs +++ b/src/TrashLib/Services/Sonarr/Config/ISonarrValidationMessages.cs @@ -2,7 +2,5 @@ namespace TrashLib.Services.Sonarr.Config; public interface ISonarrValidationMessages { - string BaseUrl { get; } - string ApiKey { get; } string ReleaseProfileTrashIds { get; } } diff --git a/src/TrashLib/Services/Sonarr/Config/SonarrConfigurationValidator.cs b/src/TrashLib/Services/Sonarr/Config/SonarrConfigurationValidator.cs index 4ac1012a..bcd678c9 100644 --- a/src/TrashLib/Services/Sonarr/Config/SonarrConfigurationValidator.cs +++ b/src/TrashLib/Services/Sonarr/Config/SonarrConfigurationValidator.cs @@ -1,5 +1,6 @@ using FluentValidation; using JetBrains.Annotations; +using TrashLib.Config.Services; namespace TrashLib.Services.Sonarr.Config; @@ -8,10 +9,10 @@ internal class SonarrConfigurationValidator : AbstractValidator serviceConfigValidator, IValidator releaseProfileConfigValidator) { - RuleFor(x => x.BaseUrl).NotEmpty().WithMessage(messages.BaseUrl); - RuleFor(x => x.ApiKey).NotEmpty().WithMessage(messages.ApiKey); + Include(serviceConfigValidator); RuleForEach(x => x.ReleaseProfiles).SetValidator(releaseProfileConfigValidator); } } diff --git a/src/TrashLib/Services/Sonarr/Config/SonarrValidationMessages.cs b/src/TrashLib/Services/Sonarr/Config/SonarrValidationMessages.cs index 7f54326c..aac37774 100644 --- a/src/TrashLib/Services/Sonarr/Config/SonarrValidationMessages.cs +++ b/src/TrashLib/Services/Sonarr/Config/SonarrValidationMessages.cs @@ -5,12 +5,6 @@ namespace TrashLib.Services.Sonarr.Config; [UsedImplicitly] internal class SonarrValidationMessages : ISonarrValidationMessages { - public string BaseUrl => - "Property 'base_url' is required"; - - public string ApiKey => - "Property 'api_key' is required"; - public string ReleaseProfileTrashIds => "'trash_ids' is required for 'release_profiles' elements"; }