From 16c3e0fc1a6139f8af4570419332416e1e085442 Mon Sep 17 00:00:00 2001 From: Mario Andron Date: Wed, 23 Jan 2019 09:15:09 +0200 Subject: [PATCH 1/4] Enhanced to allow scoped dependecies to be injected into steps --- src/WorkflowCore/Services/WorkflowExecutor.cs | 85 ++++++++++--------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/src/WorkflowCore/Services/WorkflowExecutor.cs b/src/WorkflowCore/Services/WorkflowExecutor.cs index a3e5c0c46..f1aa003a3 100755 --- a/src/WorkflowCore/Services/WorkflowExecutor.cs +++ b/src/WorkflowCore/Services/WorkflowExecutor.cs @@ -87,57 +87,60 @@ public async Task Execute(WorkflowInstance workflow) pointer.StartTime = _datetimeProvider.Now.ToUniversalTime(); } - _logger.LogDebug("Starting step {0} on workflow {1}", step.Name, workflow.Id); + using (var scope = _serviceProvider.CreateScope()) + { + _logger.LogDebug("Starting step {0} on workflow {1}", step.Name, workflow.Id); - IStepBody body = step.ConstructBody(_serviceProvider); + IStepBody body = step.ConstructBody(scope.ServiceProvider); - if (body == null) - { - _logger.LogError("Unable to construct step body {0}", step.BodyType.ToString()); - pointer.SleepUntil = _datetimeProvider.Now.ToUniversalTime().Add(_options.ErrorRetryInterval); - wfResult.Errors.Add(new ExecutionError() + if (body == null) { - WorkflowId = workflow.Id, - ExecutionPointerId = pointer.Id, - ErrorTime = _datetimeProvider.Now.ToUniversalTime(), - Message = String.Format("Unable to construct step body {0}", step.BodyType.ToString()) - }); - continue; - } + _logger.LogError("Unable to construct step body {0}", step.BodyType.ToString()); + pointer.SleepUntil = _datetimeProvider.Now.ToUniversalTime().Add(_options.ErrorRetryInterval); + wfResult.Errors.Add(new ExecutionError() + { + WorkflowId = workflow.Id, + ExecutionPointerId = pointer.Id, + ErrorTime = _datetimeProvider.Now.ToUniversalTime(), + Message = String.Format("Unable to construct step body {0}", step.BodyType.ToString()) + }); + continue; + } - IStepExecutionContext context = new StepExecutionContext() - { - Workflow = workflow, - Step = step, - PersistenceData = pointer.PersistenceData, - ExecutionPointer = pointer, - Item = pointer.ContextItem - }; + IStepExecutionContext context = new StepExecutionContext() + { + Workflow = workflow, + Step = step, + PersistenceData = pointer.PersistenceData, + ExecutionPointer = pointer, + Item = pointer.ContextItem + }; - foreach (var input in step.Inputs) - input.AssignInput(workflow.Data, body, context); + foreach (var input in step.Inputs) + input.AssignInput(workflow.Data, body, context); - switch (step.BeforeExecute(wfResult, context, pointer, body)) - { - case ExecutionPipelineDirective.Defer: - continue; - case ExecutionPipelineDirective.EndWorkflow: - workflow.Status = WorkflowStatus.Complete; - workflow.CompleteTime = _datetimeProvider.Now.ToUniversalTime(); - continue; - } + switch (step.BeforeExecute(wfResult, context, pointer, body)) + { + case ExecutionPipelineDirective.Defer: + continue; + case ExecutionPipelineDirective.EndWorkflow: + workflow.Status = WorkflowStatus.Complete; + workflow.CompleteTime = _datetimeProvider.Now.ToUniversalTime(); + continue; + } - var result = await body.RunAsync(context); + var result = await body.RunAsync(context); - if (result.Proceed) - { - foreach (var output in step.Outputs) - output.AssignOutput(workflow.Data, body, context); - } + if (result.Proceed) + { + foreach (var output in step.Outputs) + output.AssignOutput(workflow.Data, body, context); + } - _executionResultProcessor.ProcessExecutionResult(workflow, def, pointer, step, result, wfResult); - step.AfterExecute(wfResult, context, result, pointer); + _executionResultProcessor.ProcessExecutionResult(workflow, def, pointer, step, result, wfResult); + step.AfterExecute(wfResult, context, result, pointer); + } } catch (Exception ex) { From b25f3f0f16feaff55c65e1938433ece05ca2e9f2 Mon Sep 17 00:00:00 2001 From: Mario Andron Date: Wed, 23 Jan 2019 10:46:12 +0200 Subject: [PATCH 2/4] Fixed unit tests by adding a scope provider --- src/WorkflowCore/Interface/IScopeProvider.cs | 17 +++++++++++++ .../ServiceCollectionExtensions.cs | 1 + src/WorkflowCore/Services/ScopeProvider.cs | 25 +++++++++++++++++++ src/WorkflowCore/Services/WorkflowExecutor.cs | 6 +++-- .../Services/WorkflowExecutorFixture.cs | 8 +++++- 5 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 src/WorkflowCore/Interface/IScopeProvider.cs create mode 100644 src/WorkflowCore/Services/ScopeProvider.cs diff --git a/src/WorkflowCore/Interface/IScopeProvider.cs b/src/WorkflowCore/Interface/IScopeProvider.cs new file mode 100644 index 000000000..c69351731 --- /dev/null +++ b/src/WorkflowCore/Interface/IScopeProvider.cs @@ -0,0 +1,17 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace WorkflowCore.Interface +{ + /// + /// The implemention of this interface will be responsible for + /// providing a new service scope for a DI container + /// + public interface IScopeProvider + { + /// + /// Create a new service scope + /// + /// + IServiceScope CreateScope(); + } +} \ No newline at end of file diff --git a/src/WorkflowCore/ServiceCollectionExtensions.cs b/src/WorkflowCore/ServiceCollectionExtensions.cs index aebae27a1..1d10c5742 100644 --- a/src/WorkflowCore/ServiceCollectionExtensions.cs +++ b/src/WorkflowCore/ServiceCollectionExtensions.cs @@ -46,6 +46,7 @@ public static IServiceCollection AddWorkflow(this IServiceCollection services, A services.AddSingleton(); services.AddSingleton(); + services.AddTransient(); services.AddTransient(); services.AddTransient(); services.AddTransient(); diff --git a/src/WorkflowCore/Services/ScopeProvider.cs b/src/WorkflowCore/Services/ScopeProvider.cs new file mode 100644 index 000000000..ad1ae98d2 --- /dev/null +++ b/src/WorkflowCore/Services/ScopeProvider.cs @@ -0,0 +1,25 @@ +using Microsoft.Extensions.DependencyInjection; +using System; +using WorkflowCore.Interface; + +namespace WorkflowCore.Services +{ + /// + /// A concrete implementation for the IScopeProvider interface + /// Largely to get around the problems of unit testing an extension method (CreateScope()) + /// + public class ScopeProvider : IScopeProvider + { + private readonly IServiceProvider provider; + + public ScopeProvider(IServiceProvider provider) + { + this.provider = provider; + } + + public IServiceScope CreateScope() + { + return provider.CreateScope(); + } + } +} diff --git a/src/WorkflowCore/Services/WorkflowExecutor.cs b/src/WorkflowCore/Services/WorkflowExecutor.cs index f1aa003a3..45716b83d 100755 --- a/src/WorkflowCore/Services/WorkflowExecutor.cs +++ b/src/WorkflowCore/Services/WorkflowExecutor.cs @@ -14,6 +14,7 @@ public class WorkflowExecutor : IWorkflowExecutor { protected readonly IWorkflowRegistry _registry; protected readonly IServiceProvider _serviceProvider; + protected readonly IScopeProvider _scopeProvider; protected readonly IDateTimeProvider _datetimeProvider; protected readonly ILogger _logger; private readonly IExecutionResultProcessor _executionResultProcessor; @@ -23,9 +24,10 @@ public class WorkflowExecutor : IWorkflowExecutor private IWorkflowHost Host => _serviceProvider.GetService(); - public WorkflowExecutor(IWorkflowRegistry registry, IServiceProvider serviceProvider, IDateTimeProvider datetimeProvider, IExecutionResultProcessor executionResultProcessor, ILifeCycleEventPublisher publisher, ICancellationProcessor cancellationProcessor, WorkflowOptions options, ILoggerFactory loggerFactory) + public WorkflowExecutor(IWorkflowRegistry registry, IServiceProvider serviceProvider, IScopeProvider scopeProvider, IDateTimeProvider datetimeProvider, IExecutionResultProcessor executionResultProcessor, ILifeCycleEventPublisher publisher, ICancellationProcessor cancellationProcessor, WorkflowOptions options, ILoggerFactory loggerFactory) { _serviceProvider = serviceProvider; + _scopeProvider = scopeProvider; _registry = registry; _datetimeProvider = datetimeProvider; _publisher = publisher; @@ -87,7 +89,7 @@ public async Task Execute(WorkflowInstance workflow) pointer.StartTime = _datetimeProvider.Now.ToUniversalTime(); } - using (var scope = _serviceProvider.CreateScope()) + using (var scope = _scopeProvider.CreateScope()) { _logger.LogDebug("Starting step {0} on workflow {1}", step.Name, workflow.Id); diff --git a/test/WorkflowCore.UnitTests/Services/WorkflowExecutorFixture.cs b/test/WorkflowCore.UnitTests/Services/WorkflowExecutorFixture.cs index d94ed3416..7597532d5 100644 --- a/test/WorkflowCore.UnitTests/Services/WorkflowExecutorFixture.cs +++ b/test/WorkflowCore.UnitTests/Services/WorkflowExecutorFixture.cs @@ -25,6 +25,7 @@ public class WorkflowExecutorFixture protected ILifeCycleEventPublisher EventHub; protected ICancellationProcessor CancellationProcessor; protected IServiceProvider ServiceProvider; + protected IScopeProvider ScopeProvider; protected IDateTimeProvider DateTimeProvider; protected WorkflowOptions Options; @@ -33,6 +34,7 @@ public WorkflowExecutorFixture() Host = A.Fake(); PersistenceProvider = A.Fake(); ServiceProvider = A.Fake(); + ScopeProvider = A.Fake(); Registry = A.Fake(); ResultProcesser = A.Fake(); EventHub = A.Fake(); @@ -41,13 +43,17 @@ public WorkflowExecutorFixture() Options = new WorkflowOptions(A.Fake()); + var scope = A.Fake(); + A.CallTo(() => ScopeProvider.CreateScope()).Returns(scope); + A.CallTo(() => scope.ServiceProvider).Returns(ServiceProvider); + A.CallTo(() => DateTimeProvider.Now).Returns(DateTime.Now); //config logging var loggerFactory = new LoggerFactory(); loggerFactory.AddConsole(LogLevel.Debug); - Subject = new WorkflowExecutor(Registry, ServiceProvider, DateTimeProvider, ResultProcesser, EventHub, CancellationProcessor, Options, loggerFactory); + Subject = new WorkflowExecutor(Registry, ServiceProvider, ScopeProvider, DateTimeProvider, ResultProcesser, EventHub, CancellationProcessor, Options, loggerFactory); } [Fact(DisplayName = "Should execute active step")] From 8bcd4939275e6512931d9679c810cfae85fb5272 Mon Sep 17 00:00:00 2001 From: Mario Andron Date: Fri, 1 Feb 2019 06:55:57 +0200 Subject: [PATCH 3/4] Added intetgration scenarios to test IoC scope for MS IoC container and the AutoFac IoC container --- .../Scenarios/DiScenario.cs | 255 ++++++++++++++++++ .../WorkflowCore.IntegrationTests.csproj | 1 + test/WorkflowCore.Testing/WorkflowTest.cs | 2 +- 3 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs diff --git a/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs b/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs new file mode 100644 index 000000000..3c8a093c8 --- /dev/null +++ b/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs @@ -0,0 +1,255 @@ +using System; +using WorkflowCore.Interface; +using WorkflowCore.Models; +using Xunit; +using FluentAssertions; +using WorkflowCore.Testing; +using Microsoft.Extensions.DependencyInjection; +using Autofac.Extensions.DependencyInjection; +using Autofac; +using System.Diagnostics; + +namespace WorkflowCore.IntegrationTests.Scenarios +{ + public class DiWorkflow : IWorkflow + { + public string Id => "DiWorkflow"; + public int Version => 1; + + public void Build(IWorkflowBuilder builder) + { + builder + .StartWith() + .Output(_ => _.instance1, _ => _.dependency1.Instance) + .Output(_ => _.instance2, _ => _.dependency2.dependency1.Instance) + .Then(context => + { + return ExecutionResult.Next(); + }); + } + } + + public class DiData + { + public int instance1 { get; set; } = -1; + public int instance2 { get; set; } = -1; + } + + public class Dependency1 + { + public static int InstanceCounter = 0; + + public int Instance { get; set; } = ++InstanceCounter; + } + + public class Dependency2 + { + public readonly Dependency1 dependency1; + + public Dependency2(Dependency1 dependency1) + { + this.dependency1 = dependency1; + } + } + + public class DiStep1 : StepBody + { + public readonly Dependency1 dependency1; + public readonly Dependency2 dependency2; + + public DiStep1(Dependency1 dependency1, Dependency2 dependency2) + { + this.dependency1 = dependency1; + this.dependency2 = dependency2; + } + + public override ExecutionResult Run(IStepExecutionContext context) + { + return ExecutionResult.Next(); + } + } + + /// + /// The DI scenarios are design to test whether the scoped / transient dependecies are honoured with + /// various IoC container implementations. The basic premise is that a step has a dependency on + /// two services, one of which has a dependency on the other. + /// + /// We then use the instance numbers of the services to determine whether the container has created a + /// transient instance or a scoped instance + /// + /// if step.dependency2.dependency1.instance == step.dependency1.instance then + /// we can be assured that dependency1 was created in the same scope as dependency 2 + /// + /// otherwise if the instances are different, they were created as transient + /// + /// + public abstract class DiScenario : WorkflowTest + { + protected void ConfigureHost(IServiceProvider serviceProvider) + { + PersistenceProvider = serviceProvider.GetService(); + Host = serviceProvider.GetService(); + Host.RegisterWorkflow(); + Host.OnStepError += Host_OnStepError; + Host.Start(); + } + } + + /// + /// Because of the static InMemory Persistence provider, this test must run in issolation + /// to prevent other hosts from picking up steps intended for this host and incorrectly + /// cross-referencing the scoped / transient IoC container for step constrcution + /// + [CollectionDefinition("DiMsTransientScenario", DisableParallelization = true)] + [Collection("DiMsTransientScenario")] + public class DiMsTransientScenario : DiScenario + { + public DiMsTransientScenario() + { + //setup dependency injection + IServiceCollection services = new ServiceCollection(); + services.AddTransient(); + services.AddTransient(); + services.AddTransient(); + services.AddLogging(); + ConfigureServices(services); + + var serviceProvider = services.BuildServiceProvider(); + ConfigureHost(serviceProvider); + } + + [Fact] + public void Scenario() + { + var workflowId = StartWorkflow(new DiData()); + WaitForWorkflowToComplete(workflowId, TimeSpan.FromSeconds(5)); + var data = GetData(workflowId); + + // DI provider should have created two transient instances, with different instance ids + data.instance1.Should().NotBe(-1); + data.instance2.Should().NotBe(-1); + data.instance1.Should().NotBe(data.instance2); + } + } + + /// + /// Because of the static InMemory Persistence provider, this test must run in issolation + /// to prevent other hosts from picking up steps intended for this host and incorrectly + /// cross-referencing the scoped / transient IoC container for step constrcution + /// + [CollectionDefinition("DiMsScopedScenario", DisableParallelization = true)] + [Collection("DiMsScopedScenario")] + public class DiMsScopedScenario : DiScenario + { + public DiMsScopedScenario() + { + //setup dependency injection + IServiceCollection services = new ServiceCollection(); + services.AddScoped(); + services.AddScoped(); + services.AddTransient(); + services.AddLogging(); + ConfigureServices(services); + + var serviceProvider = services.BuildServiceProvider(); + ConfigureHost(serviceProvider); + } + + [Fact] + public void Scenario() + { + var workflowId = StartWorkflow(new DiData()); + WaitForWorkflowToComplete(workflowId, TimeSpan.FromSeconds(5)); + var data = GetData(workflowId); + + // scope provider should have created one scoped instance, with the same instance ids + data.instance1.Should().NotBe(-1); + data.instance2.Should().NotBe(-1); + data.instance1.Should().Be(data.instance2); + } + } + + /// + /// Because of the static InMemory Persistence provider, this test must run in issolation + /// to prevent other hosts from picking up steps intended for this host and incorrectly + /// cross-referencing the scoped / transient IoC container for step constrcution + /// + [CollectionDefinition("DiAutoFacTransientScenario", DisableParallelization = true)] + [Collection("DiAutoFacTransientScenario")] + public class DiAutoFacTransientScenario : DiScenario + { + public DiAutoFacTransientScenario() + { + //setup dependency injection + IServiceCollection services = new ServiceCollection(); + services.AddLogging(); + ConfigureServices(services); + + //setup dependency injection + var builder = new ContainerBuilder(); + builder.Populate(services); + builder.RegisterType().InstancePerDependency(); + builder.RegisterType().InstancePerDependency(); + builder.RegisterType().InstancePerDependency(); + var container = builder.Build(); + + var serviceProvider = new AutofacServiceProvider(container); + ConfigureHost(serviceProvider); + } + + [Fact] + public void Scenario() + { + var workflowId = StartWorkflow(new DiData()); + WaitForWorkflowToComplete(workflowId, TimeSpan.FromSeconds(5)); + var data = GetData(workflowId); + + // scope provider should have created one scoped instance, with the same instance ids + data.instance1.Should().NotBe(-1); + data.instance2.Should().NotBe(-1); + data.instance1.Should().NotBe(data.instance2); + } + } + + /// + /// Because of the static InMemory Persistence provider, this test must run in issolation + /// to prevent other hosts from picking up steps intended for this host and incorrectly + /// cross-referencing the scoped / transient IoC container for step constrcution + /// + [CollectionDefinition("DiAutoFacScopedScenario", DisableParallelization = true)] + [Collection("DiAutoFacScopedScenario")] + public class DiAutoFacScopedScenario : DiScenario + { + public DiAutoFacScopedScenario() + { + //setup dependency injection + IServiceCollection services = new ServiceCollection(); + services.AddLogging(); + ConfigureServices(services); + + //setup dependency injection + var builder = new ContainerBuilder(); + builder.Populate(services); + builder.RegisterType().InstancePerLifetimeScope(); + builder.RegisterType().InstancePerLifetimeScope(); + builder.RegisterType().InstancePerLifetimeScope(); + var container = builder.Build(); + + var serviceProvider = new AutofacServiceProvider(container); + ConfigureHost(serviceProvider); + } + + [Fact] + public void Scenario() + { + var workflowId = StartWorkflow(null); + WaitForWorkflowToComplete(workflowId, TimeSpan.FromSeconds(5)); + var data = GetData(workflowId); + + // scope provider should have created one scoped instance, with the same instance ids + data.instance1.Should().NotBe(-1); + data.instance2.Should().NotBe(-1); + data.instance1.Should().Be(data.instance2); + } + } +} diff --git a/test/WorkflowCore.IntegrationTests/WorkflowCore.IntegrationTests.csproj b/test/WorkflowCore.IntegrationTests/WorkflowCore.IntegrationTests.csproj index ddde2f87a..a2a88fb71 100644 --- a/test/WorkflowCore.IntegrationTests/WorkflowCore.IntegrationTests.csproj +++ b/test/WorkflowCore.IntegrationTests/WorkflowCore.IntegrationTests.csproj @@ -18,6 +18,7 @@ + diff --git a/test/WorkflowCore.Testing/WorkflowTest.cs b/test/WorkflowCore.Testing/WorkflowTest.cs index 3ab2ec91b..9c7d0dacc 100644 --- a/test/WorkflowCore.Testing/WorkflowTest.cs +++ b/test/WorkflowCore.Testing/WorkflowTest.cs @@ -39,7 +39,7 @@ protected virtual void Setup() Host.Start(); } - private void Host_OnStepError(WorkflowInstance workflow, WorkflowStep step, Exception exception) + protected void Host_OnStepError(WorkflowInstance workflow, WorkflowStep step, Exception exception) { UnhandledStepErrors.Add(new StepError() { From 8df0ceeee261b7959c08e71dbb9cd90579beaf4b Mon Sep 17 00:00:00 2001 From: Mario Andron Date: Fri, 1 Feb 2019 10:34:41 +0200 Subject: [PATCH 4/4] appease the codacy gods --- .../WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs b/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs index 3c8a093c8..81a782264 100644 --- a/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs +++ b/test/WorkflowCore.IntegrationTests/Scenarios/DiScenario.cs @@ -37,14 +37,14 @@ public class DiData public class Dependency1 { - public static int InstanceCounter = 0; + private static int InstanceCounter = 0; public int Instance { get; set; } = ++InstanceCounter; } public class Dependency2 { - public readonly Dependency1 dependency1; + public Dependency1 dependency1 { get; private set; } public Dependency2(Dependency1 dependency1) { @@ -54,8 +54,8 @@ public Dependency2(Dependency1 dependency1) public class DiStep1 : StepBody { - public readonly Dependency1 dependency1; - public readonly Dependency2 dependency2; + public Dependency1 dependency1 { get; private set; } + public Dependency2 dependency2 { get; private set; } public DiStep1(Dependency1 dependency1, Dependency2 dependency2) {