From 0b0fc1e7bce40334bd7cfa1de914eb312acc629a Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Wed, 26 Oct 2022 09:06:51 -0500 Subject: [PATCH] refactor: Delete YamlDotNet ObjectFactory This factory was incorrectly using an Autofac container to resolve types. This led to symptoms like extra elements in arrays being deserialized, due to those types being resolved by the DI container. There's no real reason I can think of to depend on Autofac when deserializing types from YAML. This object has been around since 1.0.0 and I do not remember why it was created. --- .../Services/ConfigurationLoaderTest.cs | 91 +++++++++---------- src/Recyclarr/CompositionRoot.cs | 3 +- src/Recyclarr/Config/ObjectFactory.cs | 21 ----- 3 files changed, 44 insertions(+), 71 deletions(-) delete mode 100644 src/Recyclarr/Config/ObjectFactory.cs diff --git a/src/Recyclarr.Tests/Config/Services/ConfigurationLoaderTest.cs b/src/Recyclarr.Tests/Config/Services/ConfigurationLoaderTest.cs index 1652e2bd..1df88b32 100644 --- a/src/Recyclarr.Tests/Config/Services/ConfigurationLoaderTest.cs +++ b/src/Recyclarr.Tests/Config/Services/ConfigurationLoaderTest.cs @@ -1,7 +1,8 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Abstractions; +using System.IO.Abstractions.Extensions; +using System.IO.Abstractions.TestingHelpers; using System.Text; -using Autofac; using AutoFixture.NUnit3; using Common; using Common.Extensions; @@ -12,19 +13,16 @@ using JetBrains.Annotations; using NSubstitute; using NUnit.Framework; using Recyclarr.Config; -using TestLibrary; +using Recyclarr.TestLibrary; using TestLibrary.AutoFixture; -using TrashLib.Config; using TrashLib.Config.Services; using TrashLib.Services.Sonarr.Config; -using YamlDotNet.Serialization; -using YamlDotNet.Serialization.ObjectFactories; namespace Recyclarr.Tests.Config.Services; [TestFixture] [Parallelizable(ParallelScope.All)] -public class ConfigurationLoaderTest +public class ConfigurationLoaderTest : IntegrationFixture { private static TextReader GetResourceData(string file) { @@ -32,14 +30,6 @@ public class ConfigurationLoaderTest return new StringReader(testData.ReadData(file)); } - private static IContainer BuildContainer() - { - var builder = new ContainerBuilder(); - builder.RegisterType().As(); - builder.RegisterType().As(); - return builder.Build(); - } - [UsedImplicitly] [SuppressMessage("ReSharper", "MemberCanBePrivate.Global")] [SuppressMessage("Microsoft.Design", "CA1034", @@ -53,27 +43,29 @@ public class ConfigurationLoaderTest public bool DeleteOldCustomFormats => false; } - [Test, AutoMockData(typeof(ConfigurationLoaderTest), nameof(BuildContainer))] - public void Load_many_iterations_of_config( - [Frozen] IFileSystem fs, - ConfigurationLoader loader) + [Test] + public void Load_many_iterations_of_config() { - static StreamReader MockYaml(params object[] args) + static string MockYaml(params object[] args) { var str = new StringBuilder("sonarr:"); const string templateYaml = "\n - base_url: {0}\n api_key: abc"; str.Append(args.Aggregate("", (current, p) => current + templateYaml.FormatWith(p))); - return StreamBuilder.FromString(str.ToString()); + return str.ToString(); } - fs.File.OpenText(Arg.Any()).Returns(MockYaml(1, 2), MockYaml(3)); - - var fakeFiles = new List + var baseDir = Fs.CurrentDirectory(); + var fileData = new (string, string)[] { - "config1.yml", - "config2.yml" + (baseDir.File("config1.yml").FullName, MockYaml(1, 2)), + (baseDir.File("config2.yml").FullName, MockYaml(3)) }; + foreach (var (file, data) in fileData) + { + Fs.AddFile(file, new MockFileData(data)); + } + var expected = new List { new() {ApiKey = "abc", BaseUrl = "1"}, @@ -81,44 +73,45 @@ public class ConfigurationLoaderTest new() {ApiKey = "abc", BaseUrl = "3"} }; - var actual = loader.LoadMany(fakeFiles, "sonarr").ToList(); + var loader = Resolve>(); + var actual = loader.LoadMany(fileData.Select(x => x.Item1), "sonarr").ToList(); actual.Should().BeEquivalentTo(expected); } - [Test, AutoMockData(typeof(ConfigurationLoaderTest), nameof(BuildContainer))] - public void Parse_using_stream(ConfigurationLoader configLoader) + [Test] + public void Parse_using_stream() { + var configLoader = Resolve>(); var configs = configLoader.LoadFromStream(GetResourceData("Load_UsingStream_CorrectParsing.yml"), "sonarr"); - configs.Should() - .BeEquivalentTo(new List + configs.Should().BeEquivalentTo(new List + { + new() { - new() + ApiKey = "95283e6b156c42f3af8a9b16173f876b", + BaseUrl = "http://localhost:8989", + ReleaseProfiles = new List { - ApiKey = "95283e6b156c42f3af8a9b16173f876b", - BaseUrl = "http://localhost:8989", - ReleaseProfiles = new List + new() { - new() - { - TrashIds = new[] {"123"}, - StrictNegativeScores = true, - Tags = new List {"anime"} - }, - new() + TrashIds = new[] {"123"}, + StrictNegativeScores = true, + Tags = new List {"anime"} + }, + new() + { + TrashIds = new[] {"456"}, + StrictNegativeScores = false, + Tags = new List { - TrashIds = new[] {"456"}, - StrictNegativeScores = false, - Tags = new List - { - "tv", - "series" - } + "tv", + "series" } } } - }); + } + }); } [Test, AutoMockData] diff --git a/src/Recyclarr/CompositionRoot.cs b/src/Recyclarr/CompositionRoot.cs index aa04b105..6def0834 100644 --- a/src/Recyclarr/CompositionRoot.cs +++ b/src/Recyclarr/CompositionRoot.cs @@ -21,6 +21,7 @@ using TrashLib.Services.Sonarr; using TrashLib.Startup; using VersionControl; using YamlDotNet.Serialization; +using YamlDotNet.Serialization.ObjectFactories; namespace Recyclarr; @@ -79,7 +80,7 @@ public static class CompositionRoot { builder.RegisterModule(); - builder.RegisterType().As(); + builder.RegisterType().As(); builder.RegisterGeneric(typeof(ConfigurationLoader<>)) .WithProperty(new AutowiringParameter()) diff --git a/src/Recyclarr/Config/ObjectFactory.cs b/src/Recyclarr/Config/ObjectFactory.cs deleted file mode 100644 index 01d1fd4c..00000000 --- a/src/Recyclarr/Config/ObjectFactory.cs +++ /dev/null @@ -1,21 +0,0 @@ -using Autofac; -using YamlDotNet.Serialization; -using YamlDotNet.Serialization.ObjectFactories; - -namespace Recyclarr.Config; - -public class ObjectFactory : IObjectFactory -{ - private readonly ILifetimeScope _container; - private readonly DefaultObjectFactory _defaultFactory = new(); - - public ObjectFactory(ILifetimeScope container) - { - _container = container; - } - - public object Create(Type type) - { - return _container.IsRegistered(type) ? _container.Resolve(type) : _defaultFactory.Create(type); - } -}