From 6dac9e432dffe39c2a3bd0eb246c7a920d34daed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Gr=C3=BCnwald?= Date: Mon, 6 Jun 2022 21:47:41 +0200 Subject: [PATCH] Remove ChangeLogPipelineBuilder Remove ChangeLogPipelineBuilder and directory create ChangeLogPipeline through the DI container (injecting all regsitered implementations of IChangeLogTask) --- .../IntegrationsExtensionsTest.cs | 35 ++------ .../Pipeline/ChangeLogPipelineBuilderTest.cs | 89 ------------------- .../Pipeline/ChangeLogPipelineTest.cs | 26 ++++-- src/ChangeLog/Commands/GenerateCommand.cs | 43 +++------ .../Integrations/IntegrationsExtensions.cs | 10 +-- src/ChangeLog/Pipeline/ChangeLogPipeline.cs | 10 ++- .../Pipeline/ChangeLogPipelineBuilder.cs | 40 --------- 7 files changed, 52 insertions(+), 201 deletions(-) delete mode 100644 src/ChangeLog.Test/Pipeline/ChangeLogPipelineBuilderTest.cs delete mode 100644 src/ChangeLog/Pipeline/ChangeLogPipelineBuilder.cs diff --git a/src/ChangeLog.Test/Integrations/IntegrationsExtensionsTest.cs b/src/ChangeLog.Test/Integrations/IntegrationsExtensionsTest.cs index d81a47ac..25168216 100644 --- a/src/ChangeLog.Test/Integrations/IntegrationsExtensionsTest.cs +++ b/src/ChangeLog.Test/Integrations/IntegrationsExtensionsTest.cs @@ -1,4 +1,5 @@ -using System.Linq; +using System.Collections.Generic; +using System.Linq; using Autofac; using Grynwald.ChangeLog.Configuration; using Grynwald.ChangeLog.Git; @@ -37,32 +38,12 @@ public void RegisterIntegrations_registers_expected_types() }); // ASSERT - AutofacAssert.CanResolveType(container); - AutofacAssert.CanResolveType(container); - - AutofacAssert.CanResolveType(container); - AutofacAssert.CanResolveType(container); - } - - [Fact] - public void AddIntegrationTasks_adds_expected_tasks() - { - // ARRANGE - var configuration = new ChangeLogConfiguration(); - - using var container = BuildContainer(b => b.RegisterInstance(configuration)); - - var pipelineBuilderMock = new Mock(MockBehavior.Strict); - pipelineBuilderMock.Setup(x => x.Container).Returns(container); - pipelineBuilderMock.Setup(x => x.AddTask()).Returns(pipelineBuilderMock.Object); - pipelineBuilderMock.Setup(x => x.AddTask()).Returns(pipelineBuilderMock.Object); - - // ACT - pipelineBuilderMock.Object.AddIntegrationTasks(); - - // ASSERT - pipelineBuilderMock.Verify(x => x.AddTask(), Times.Once); - pipelineBuilderMock.Verify(x => x.AddTask(), Times.Once); + var tasks = container.Resolve>(); + Assert.Collection( + tasks.OrderBy(x => x.GetType().Name), + x => Assert.IsType(x), + x => Assert.IsType(x) + ); } } } diff --git a/src/ChangeLog.Test/Pipeline/ChangeLogPipelineBuilderTest.cs b/src/ChangeLog.Test/Pipeline/ChangeLogPipelineBuilderTest.cs deleted file mode 100644 index 7df04279..00000000 --- a/src/ChangeLog.Test/Pipeline/ChangeLogPipelineBuilderTest.cs +++ /dev/null @@ -1,89 +0,0 @@ -using System; -using System.Threading.Tasks; -using Autofac; -using Grynwald.ChangeLog.Model; -using Grynwald.ChangeLog.Pipeline; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Moq; -using Xunit; - -namespace Grynwald.ChangeLog.Test.Pipeline -{ - /// - /// Tests for - /// - public class ChangeLogPipelineBuilderTest : ContainerTestBase - { - [Fact] - public void Container_must_not_be_null() - { - Assert.Throws(() => new ChangeLogPipelineBuilder(null!)); - } - - [Fact] - public void Constructor_initializes_Container_property() - { - // ARRANGE - var container = Mock.Of(MockBehavior.Strict); - - // ACT - var pipelineBuilder = new ChangeLogPipelineBuilder(container); - - // ASSERT - Assert.NotNull(pipelineBuilder.Container); - Assert.Same(container, pipelineBuilder.Container); - } - - private class TestTask : IChangeLogTask - { - public Task RunAsync(ApplicationChangeLog changeLog) => throw new NotImplementedException(); - } - - [Fact] - public void AddTask_resolves_task_from_the_container() - { - // ARRANGE - var task = new TestTask(); - - using var container = BuildContainer(b => - { - b.RegisterInstance(task); - }); - - var pipelineBuilder = new ChangeLogPipelineBuilder(container); - - // ACT - _ = pipelineBuilder.AddTask(); - - // ASSERT - var registeredTask = Assert.Single(pipelineBuilder.Tasks); - Assert.Same(task, registeredTask); - } - - [Fact] - public void Build_creates_a_ChangeLogPipeline_using_the_container() - { - // ARRANGE - var task = new TestTask(); - - using var container = BuildContainer(b => - { - b.RegisterType(); - b.RegisterInstance(NullLogger.Instance).As>(); - b.RegisterInstance(task); - }); - - var pipelineBuilder = new ChangeLogPipelineBuilder(container); - _ = pipelineBuilder.AddTask(); - - // ACT - var pipeline = pipelineBuilder.Build(); - - // ASSERT - Assert.NotNull(pipeline); - var registeredTask = Assert.Single(pipeline.Tasks); - Assert.Same(task, registeredTask); - } - } -} diff --git a/src/ChangeLog.Test/Pipeline/ChangeLogPipelineTest.cs b/src/ChangeLog.Test/Pipeline/ChangeLogPipelineTest.cs index fc14bf36..741eb709 100644 --- a/src/ChangeLog.Test/Pipeline/ChangeLogPipelineTest.cs +++ b/src/ChangeLog.Test/Pipeline/ChangeLogPipelineTest.cs @@ -30,7 +30,7 @@ public void Logger_must_not_be_null() // ARRANGE // ACT - var ex = Record.Exception(() => new ChangeLogPipeline(null!, Array.Empty())); + var ex = Record.Exception(() => new ChangeLogPipeline(logger: null!, tasks: new[] { Mock.Of() })); // ASSERT var argumentNullException = Assert.IsType(ex); @@ -43,15 +43,29 @@ public void Tasks_must_not_be_null() // ARRANGE // ACT - var ex = Record.Exception(() => new ChangeLogPipeline(m_Logger, null!)); + var ex = Record.Exception(() => new ChangeLogPipeline(logger: m_Logger, tasks: null!)); // ASSERT var argumentNullException = Assert.IsType(ex); Assert.Equal("tasks", argumentNullException.ParamName); } + [Fact] + public void Tasks_must_not_be_empty() + { + // ARRANGE + + // ACT + var ex = Record.Exception(() => new ChangeLogPipeline(logger: m_Logger, tasks: Array.Empty())); + + // ASSERT + var argumentException = Assert.IsType(ex); + Assert.Equal("tasks", argumentException.ParamName); + Assert.Contains("Task list must not be empty", ex.Message); + } + + [Theory] - [InlineData(0)] [InlineData(1)] [InlineData(10)] public async Task Run_executes_all_tasks_in_the_insertion_order(int numberOfTasks) @@ -66,7 +80,7 @@ public async Task Run_executes_all_tasks_in_the_insertion_order(int numberOfTask }) .ToArray(); - var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object)); + var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object).ToArray()); // ACT var result = await sut.RunAsync(); @@ -102,7 +116,7 @@ public async Task Run_aborts_execution_if_a_task_fails() tasks[1].Setup(x => x.RunAsync(It.IsAny())).Returns(Task.FromResult(ChangeLogTaskResult.Error)); - var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object)); + var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object).ToArray()); // ACT var result = await sut.RunAsync(); @@ -149,7 +163,7 @@ public async Task Run_continues_execution_if_a_task_is_skipped() tasks[1].Setup(x => x.RunAsync(It.IsAny())).Returns(Task.FromResult(ChangeLogTaskResult.Skipped)); - var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object)); + var sut = new ChangeLogPipeline(m_Logger, tasks.Select(x => x.Object).ToArray()); // ACT var result = await sut.RunAsync(); diff --git a/src/ChangeLog/Commands/GenerateCommand.cs b/src/ChangeLog/Commands/GenerateCommand.cs index 74ac69d7..5977249f 100644 --- a/src/ChangeLog/Commands/GenerateCommand.cs +++ b/src/ChangeLog/Commands/GenerateCommand.cs @@ -95,19 +95,19 @@ public async Task RunAsync() containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); - containerBuilder.RegisterType(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); + containerBuilder.RegisterType().As(); containerBuilder.RegisterIntegrations(); @@ -129,22 +129,7 @@ public async Task RunAsync() return 1; } - var pipeline = new ChangeLogPipelineBuilder(container) - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddTask() - .AddIntegrationTasks() - .AddTask() - .Build(); + var pipeline = container.Resolve(); var result = await pipeline.RunAsync(); return result.Success ? 0 : 1; diff --git a/src/ChangeLog/Integrations/IntegrationsExtensions.cs b/src/ChangeLog/Integrations/IntegrationsExtensions.cs index dce0a641..7b68568b 100644 --- a/src/ChangeLog/Integrations/IntegrationsExtensions.cs +++ b/src/ChangeLog/Integrations/IntegrationsExtensions.cs @@ -10,16 +10,10 @@ internal static class IntegrationsExtensions public static void RegisterIntegrations(this ContainerBuilder containerBuilder) { containerBuilder.RegisterType().As(); - containerBuilder.RegisterType(); + containerBuilder.RegisterType().As(); containerBuilder.RegisterType().As(); - containerBuilder.RegisterType(); + containerBuilder.RegisterType().As(); } - - - public static IChangeLogPipelineBuilder AddIntegrationTasks(this IChangeLogPipelineBuilder pipelineBuilder) => - pipelineBuilder - .AddTask() - .AddTask(); } } diff --git a/src/ChangeLog/Pipeline/ChangeLogPipeline.cs b/src/ChangeLog/Pipeline/ChangeLogPipeline.cs index a62aa2c5..aa4e7cc9 100644 --- a/src/ChangeLog/Pipeline/ChangeLogPipeline.cs +++ b/src/ChangeLog/Pipeline/ChangeLogPipeline.cs @@ -17,9 +17,15 @@ public sealed class ChangeLogPipeline public IEnumerable Tasks => m_Tasks; - public ChangeLogPipeline(ILogger logger, IEnumerable tasks) + public ChangeLogPipeline(ILogger logger, IReadOnlyList tasks) { - m_Tasks = (tasks ?? throw new ArgumentNullException(nameof(tasks))).ToList(); + if (tasks is null) + throw new ArgumentNullException(nameof(tasks)); + + if (tasks.Count == 0) + throw new ArgumentException("Task list must not be empty", nameof(tasks)); + + m_Tasks = tasks; m_Logger = logger ?? throw new ArgumentNullException(nameof(logger)); } diff --git a/src/ChangeLog/Pipeline/ChangeLogPipelineBuilder.cs b/src/ChangeLog/Pipeline/ChangeLogPipelineBuilder.cs deleted file mode 100644 index b6183b9c..00000000 --- a/src/ChangeLog/Pipeline/ChangeLogPipelineBuilder.cs +++ /dev/null @@ -1,40 +0,0 @@ -using System; -using System.Collections.Generic; -using Autofac; -using Grynwald.ChangeLog.Tasks; - -namespace Grynwald.ChangeLog.Pipeline -{ - internal class ChangeLogPipelineBuilder : IChangeLogPipelineBuilder - { - private readonly List m_Tasks; - - - public IContainer Container { get; } - - public IEnumerable Tasks => m_Tasks; - - - public ChangeLogPipelineBuilder(IContainer container) - { - Container = container ?? throw new ArgumentNullException(nameof(container)); - m_Tasks = new List(); - } - - - public IChangeLogPipelineBuilder AddTask() where T : IChangeLogTask - { - var task = Container.Resolve(); - m_Tasks.Add(task); - return this; - } - - - public ChangeLogPipeline Build() - { - return Container.Resolve( - TypedParameter.From>(m_Tasks) - ); - } - } -}