From cbc7c2d461abf0f84c1e7d89c1cc151ccdc7c86b Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Sat, 6 Mar 2021 13:55:19 +0000 Subject: [PATCH 1/8] WIP #552 - Refactor test logic (reusability) This refactor of integration test logic is to boost its reusability. Moving some functionality to attributes & customization classes means I can reuse elsewhere without duplication. --- ...lsaAndCommonTemporalActivitiesAttribute.cs | 21 ++++++++ ...HostBuilderWithElsaAndHangfireAttribute.cs | 21 ++++++++ .../HostBuilderWithElsaAndQuartzAttribute.cs | 20 ++++++++ ...emporalServiceCollectionExtensionsTests.cs | 48 +++++-------------- .../HostBubilderUsingServicesCustomization.cs | 37 ++++++++++++++ ...tofixtureServiceProviderSpecimenBuilder.cs | 4 +- ...ostBubilderUsingServicesSpecimenBuilder.cs | 23 +++++++++ .../SpecimenBuilders/RequestExtensions.cs | 29 +++++++++++ .../Elsa.Testing.Shared.csproj | 1 + 9 files changed, 164 insertions(+), 40 deletions(-) create mode 100644 test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndCommonTemporalActivitiesAttribute.cs create mode 100644 test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndHangfireAttribute.cs create mode 100644 test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndQuartzAttribute.cs create mode 100644 test/shared/Elsa.Testing.Shared/AutoFixture/Customizations/HostBubilderUsingServicesCustomization.cs create mode 100644 test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/HostBubilderUsingServicesSpecimenBuilder.cs create mode 100644 test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/RequestExtensions.cs diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndCommonTemporalActivitiesAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndCommonTemporalActivitiesAttribute.cs new file mode 100644 index 000000000..67a714969 --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndCommonTemporalActivitiesAttribute.cs @@ -0,0 +1,21 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Elsa.Activities.Temporal; +using Elsa.Testing.Shared.AutoFixture.Customizations; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Core.IntegrationTests.Extensions +{ + public class HostBuilderWithElsaAndCommonTemporalActivitiesAttribute : CustomizeAttribute + { + public override ICustomization GetCustomization(ParameterInfo parameter) + { + return new HostBubilderUsingServicesCustomization(services => { + services.AddElsa(elsa => { + CommonTemporalActivityServices.AddCommonTemporalActivities(elsa); + }); + }, parameter); + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndHangfireAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndHangfireAttribute.cs new file mode 100644 index 000000000..5ce7d0a3a --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndHangfireAttribute.cs @@ -0,0 +1,21 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Elsa.Testing.Shared.AutoFixture.Customizations; +using Hangfire; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Core.IntegrationTests.Extensions +{ + public class HostBuilderWithElsaAndHangfireAttribute : CustomizeAttribute + { + public override ICustomization GetCustomization(ParameterInfo parameter) + { + return new HostBubilderUsingServicesCustomization(services => { + services.AddElsa(elsa => { + elsa.AddHangfireTemporalActivities(config => config.UseInMemoryStorage()); + }); + }, parameter); + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndQuartzAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndQuartzAttribute.cs new file mode 100644 index 000000000..81e492d0b --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaAndQuartzAttribute.cs @@ -0,0 +1,20 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Elsa.Testing.Shared.AutoFixture.Customizations; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Core.IntegrationTests.Extensions +{ + public class HostBuilderWithElsaAndQuartzAttribute : CustomizeAttribute + { + public override ICustomization GetCustomization(ParameterInfo parameter) + { + return new HostBubilderUsingServicesCustomization(services => { + services.AddElsa(elsa => { + elsa.AddQuartzTemporalActivities(); + }); + }, parameter); + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Extensions/TemporalServiceCollectionExtensionsTests.cs b/test/integration/Elsa.Core.IntegrationTests/Extensions/TemporalServiceCollectionExtensionsTests.cs index ddf5eabf2..58bb9b76c 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Extensions/TemporalServiceCollectionExtensionsTests.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Extensions/TemporalServiceCollectionExtensionsTests.cs @@ -6,20 +6,15 @@ using Hangfire; using System.Threading.Tasks; using System.Threading; using Elsa.Activities.Temporal; +using Elsa.Core.IntegrationTests.Extensions; namespace Elsa.UnitTests.Extensions { public class TemporalServiceCollectionExtensionsTests { - [Fact(DisplayName = "Starting a hosted app which uses only AddCommonTemporalActivities should throw InvalidOperationException because of the missing impl")] - public void AddCommonTemporalActivitiesThrowsDuringStartupIfNoTemporalImplementationPresent() + [Theory(DisplayName = "Starting a hosted app which uses only AddCommonTemporalActivities should throw InvalidOperationException because of the missing impl"), AutoMoqData] + public void AddCommonTemporalActivitiesThrowsDuringStartupIfNoTemporalImplementationPresent([HostBuilderWithElsaAndCommonTemporalActivities] IHostBuilder hostBuilder) { - var hostBuilder = CreateHostBuilderUsingServices(services => { - services.AddElsa(elsa => { - CommonTemporalActivityServices.AddCommonTemporalActivities(elsa); - }); - }); - var cancellationSource = new CancellationTokenSource(); try { @@ -33,15 +28,9 @@ namespace Elsa.UnitTests.Extensions } } - [Fact(DisplayName = "Starting a hosted app which uses AddHangfireTemporalActivities should not throw")] - public void AddHangfireTemporalActivitiesDoesNotThrowDuringStartup() + [Theory(DisplayName = "Starting a hosted app which uses AddHangfireTemporalActivities should not throw"), AutoMoqData] + public void AddHangfireTemporalActivitiesDoesNotThrowDuringStartup([HostBuilderWithElsaAndHangfire] IHostBuilder hostBuilder) { - var hostBuilder = CreateHostBuilderUsingServices(services => { - services.AddElsa(elsa => { - elsa.AddHangfireTemporalActivities(config => config.UseInMemoryStorage()); - }); - }); - var cancellationSource = new CancellationTokenSource(); try { @@ -57,15 +46,9 @@ namespace Elsa.UnitTests.Extensions } } - [Fact(DisplayName = "Starting a hosted app which uses AddQuartzTemporalActivities should not throw")] - public void AddQuartzTemporalActivitiesDoesNotThrowDuringStartup() + [Theory(DisplayName = "Starting a hosted app which uses AddQuartzTemporalActivities should not throw"), AutoMoqData] + public void AddQuartzTemporalActivitiesDoesNotThrowDuringStartup([HostBuilderWithElsaAndQuartz] IHostBuilder hostBuilder) { - var hostBuilder = CreateHostBuilderUsingServices(services => { - services.AddElsa(elsa => { - elsa.AddQuartzTemporalActivities(); - }); - }); - var cancellationSource = new CancellationTokenSource(); try { @@ -81,15 +64,6 @@ namespace Elsa.UnitTests.Extensions } } - /// - /// Convenience method creates an using the specified services. - /// - /// Services to use in the created host builder - /// A host builder - static IHostBuilder CreateHostBuilderUsingServices(Action serviceConfig) - => Host.CreateDefaultBuilder() - .ConfigureServices((hostBuilder, services) => serviceConfig(services)); - /// /// Starts up the specified instance using console lifetime. /// The current thread is then blocked for a short while, giving the app a chance to start up and run. @@ -111,10 +85,10 @@ namespace Elsa.UnitTests.Extensions /// The cancellation source static void CancelAndSquelchExceptions(CancellationTokenSource cancellationSource) { - try { cancellationSource.Cancel(); } - catch(Exception) { - // This is only ever used as teardown code, which is why we don't care about exceptions. - } + try { cancellationSource.Cancel(); } + catch(Exception) { + // This is only ever used as teardown code, which is why we don't care about exceptions. + } } } } \ No newline at end of file diff --git a/test/shared/Elsa.Testing.Shared/AutoFixture/Customizations/HostBubilderUsingServicesCustomization.cs b/test/shared/Elsa.Testing.Shared/AutoFixture/Customizations/HostBubilderUsingServicesCustomization.cs new file mode 100644 index 000000000..7f3d3fd72 --- /dev/null +++ b/test/shared/Elsa.Testing.Shared/AutoFixture/Customizations/HostBubilderUsingServicesCustomization.cs @@ -0,0 +1,37 @@ +using System; +using System.Reflection; +using AutoFixture; +using AutoFixture.Kernel; +using Elsa.Testing.Shared.AutoFixture.SpecimenBuilders; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Testing.Shared.AutoFixture.Customizations +{ + public class HostBubilderUsingServicesCustomization : ICustomization + { + readonly Action serviceConfig; + readonly ParameterInfo? parameter; + + public void Customize(IFixture fixture) + { + fixture.Customizations.Insert(0, GetSpecimenBuilder()); + } + + ISpecimenBuilder GetSpecimenBuilder() + { + var specimenBuilder = new HostBubilderUsingServicesSpecimenBuilder(serviceConfig); + if(parameter is null) return specimenBuilder; + + var paramSpec = new ParameterSpecification(parameter.ParameterType, parameter.Name); + return new FilteringSpecimenBuilder(specimenBuilder, paramSpec); + } + + public HostBubilderUsingServicesCustomization(Action serviceConfig, + ParameterInfo? parameter = null) + { + this.parameter = parameter ?? throw new ArgumentNullException(nameof(parameter)); + this.serviceConfig = serviceConfig ?? throw new ArgumentNullException(nameof(serviceConfig)); + + } + } +} \ No newline at end of file diff --git a/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/AutofixtureServiceProviderSpecimenBuilder.cs b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/AutofixtureServiceProviderSpecimenBuilder.cs index 4a124efa8..4acc74bff 100644 --- a/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/AutofixtureServiceProviderSpecimenBuilder.cs +++ b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/AutofixtureServiceProviderSpecimenBuilder.cs @@ -13,9 +13,7 @@ namespace Elsa.Testing.Shared.AutoFixture.SpecimenBuilders { public object Create(object request, ISpecimenContext context) { - if(Equals(request, typeof(IServiceProvider))) - return GetServiceProvider(context); - if(request is ParameterInfo paramInfo && paramInfo.ParameterType == typeof(IServiceProvider)) + if(request.IsAnAutofixtureRequestForType()) return GetServiceProvider(context); return new NoSpecimen(); diff --git a/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/HostBubilderUsingServicesSpecimenBuilder.cs b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/HostBubilderUsingServicesSpecimenBuilder.cs new file mode 100644 index 000000000..5e501da39 --- /dev/null +++ b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/HostBubilderUsingServicesSpecimenBuilder.cs @@ -0,0 +1,23 @@ +using System; +using AutoFixture.Kernel; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +namespace Elsa.Testing.Shared.AutoFixture.SpecimenBuilders +{ + public class HostBubilderUsingServicesSpecimenBuilder : ISpecimenBuilder + { + readonly Action serviceConfig; + + public object Create(object request, ISpecimenContext context) + => request.IsAnAutofixtureRequestForType()? GetHostBuilder() : new NoSpecimen(); + + IHostBuilder GetHostBuilder() + => Host.CreateDefaultBuilder().ConfigureServices((hostBuilder, services) => serviceConfig(services)); + + public HostBubilderUsingServicesSpecimenBuilder(Action serviceConfig) + { + this.serviceConfig = serviceConfig; + } + } +} \ No newline at end of file diff --git a/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/RequestExtensions.cs b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/RequestExtensions.cs new file mode 100644 index 000000000..ab8427d75 --- /dev/null +++ b/test/shared/Elsa.Testing.Shared/AutoFixture/SpecimenBuilders/RequestExtensions.cs @@ -0,0 +1,29 @@ +using System.Reflection; + +namespace Elsa.Testing.Shared.AutoFixture.SpecimenBuilders +{ + /// + /// Convenience methods for specimen builders. You're only likey to want to use these extensions + /// within a specimen builder class. + /// + public static class AutofixtureRequestExtensions + { + /// + /// Gets a value which indicates if the specified qualifies as an Autofixture + /// request for an instance of the specified generic type. + /// + /// The request object + /// The desired specimen type + /// true if the is a request for an instance of ; false otherwise. + public static bool IsAnAutofixtureRequestForType(this object request) + { + if(Equals(request, typeof(T))) + return true; + + if(request is ParameterInfo paramInfo && paramInfo.ParameterType == typeof(T)) + return true; + + return false; + } + } +} \ No newline at end of file diff --git a/test/shared/Elsa.Testing.Shared/Elsa.Testing.Shared.csproj b/test/shared/Elsa.Testing.Shared/Elsa.Testing.Shared.csproj index 5df535cd8..18bcc5296 100644 --- a/test/shared/Elsa.Testing.Shared/Elsa.Testing.Shared.csproj +++ b/test/shared/Elsa.Testing.Shared/Elsa.Testing.Shared.csproj @@ -19,6 +19,7 @@ + From 672e32a7caae61355e928bc4c299e3687f468d17 Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Sat, 6 Mar 2021 17:22:37 +0000 Subject: [PATCH 2/8] WIP #552 - Add unit test to repro Actually, this unit test does not repro the issue. Frustratingly I can't get it to save the executed workflow into MongoDB at all. I see workflow execution logs being saved, but the instance doesn't go in. I'm not sure if this is a mistake I'm making or whether I have found a different bug of some sort. --- appveyor.yml | 3 ++ ...thElsaSampleWorkflowAndMongoDbAttribute.cs | 31 +++++++++++++ .../Elsa.Core.IntegrationTests.csproj | 1 + .../MongoDb/MongoDbIntegrationTests.cs | 46 +++++++++++++++++++ .../Workflows/SampleWorkflow.cs | 20 ++++++++ 5 files changed, 101 insertions(+) create mode 100644 test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs create mode 100644 test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs create mode 100644 test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs diff --git a/appveyor.yml b/appveyor.yml index ec959fab0..d6ae6f5af 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -13,6 +13,9 @@ environment: version: 2.0.0-preview7.{build} +services: + - mongodb + init: - cmd: git config --global core.autocrlf true diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs new file mode 100644 index 000000000..f395dfc3e --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs @@ -0,0 +1,31 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Elsa.Activities.UserTask.Activities; +using Elsa.Core.IntegrationTests.Workflows; +using Elsa.Persistence.MongoDb.Extensions; +using Elsa.Testing.Shared.AutoFixture.Customizations; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Elsa.Core.IntegrationTests.Autofixture +{ + public class HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute : CustomizeAttribute + { + public override ICustomization GetCustomization(ParameterInfo parameter) + { + return new HostBubilderUsingServicesCustomization(services => { + services + .AddElsa(elsa => { + elsa.UseMongoDbPersistence(opts => { + opts.ConnectionString = "mongodb://localhost:27017"; + opts.Db = "IntegrationTests"; + }); + elsa.AddActivity(); + elsa.AddWorkflow(); + }) + ; + }, parameter); + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Elsa.Core.IntegrationTests.csproj b/test/integration/Elsa.Core.IntegrationTests/Elsa.Core.IntegrationTests.csproj index 3b1f11ec6..58e23c58f 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Elsa.Core.IntegrationTests.csproj +++ b/test/integration/Elsa.Core.IntegrationTests/Elsa.Core.IntegrationTests.csproj @@ -45,6 +45,7 @@ + diff --git a/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs new file mode 100644 index 000000000..667ed4257 --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs @@ -0,0 +1,46 @@ +using Xunit; +using System.Threading.Tasks; +using Elsa.Core.IntegrationTests.Autofixture; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.DependencyInjection; +using System.Threading; +using Elsa.Services; +using Elsa.Core.IntegrationTests.Workflows; +using Elsa.Persistence; +using System; + +namespace Elsa.Core.IntegrationTests.Persistence.MongoDb +{ + public class MongoDbIntegrationTests + { + [Theory(DisplayName = "A saved workflow instance should be round-trippable without error"), AutoMoqData] + public async Task ASavedWorkflowInstanceShouldBeRoundTrippableWithoutError([HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => services.AddHostedService()); + var host = await hostBuilder.StartAsync(); + } + + class HostedWorkflowRunner : IHostedService + { + readonly IWorkflowRunner workflowRunner; + readonly IWorkflowInstanceStore instanceStore; + + public async Task StartAsync(CancellationToken cancellationToken) + { + var instance = await workflowRunner.RunWorkflowAsync(); + var retrievedInstance = await instanceStore.FindByIdAsync(instance.Id); + + Assert.NotNull(retrievedInstance); + Assert.NotSame(instance, retrievedInstance); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public HostedWorkflowRunner(IWorkflowRunner workflowRunner, IWorkflowInstanceStore instanceStore) + { + this.workflowRunner = workflowRunner ?? throw new System.ArgumentNullException(nameof(workflowRunner)); + this.instanceStore = instanceStore ?? throw new System.ArgumentNullException(nameof(instanceStore)); + } + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs b/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs new file mode 100644 index 000000000..0ee6bd155 --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs @@ -0,0 +1,20 @@ +using Elsa.Activities.UserTask.Activities; +using Elsa.Builders; +using Elsa.Activities.ControlFlow; + +namespace Elsa.Core.IntegrationTests.Workflows +{ + public class SampleWorkflow : IWorkflow + { + public static readonly object Result = new object(); + + public void Build(IWorkflowBuilder builder) + { + builder + .StartWith(setup: s => { + s.Set(x => x.Actions, c => new [] { "Foo", "Bar" }); + }) + .Finish(x => x.WithOutput(Result)); + } + } +} \ No newline at end of file From 2c79a9395c1f748a8b120beab0aea4be3987e94d Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Mon, 8 Mar 2021 19:26:02 +0000 Subject: [PATCH 3/8] WIP #728 - Add test for in-memory persistence This test case shows that in-memory persistence is also affected. It seems that that would mean that this issue is not related to only a single persistence provider. In fact, in retrospect this is not really a persistence problem at all, but a problem with the test case. More information is available in the comments to #728. --- ...tBuilderWithElsaSampleWorkflowAttribute.cs | 25 ++++++++++ .../InMemory/InMemoryStoreIntegrationTests.cs | 47 +++++++++++++++++++ .../MongoDb/MongoDbIntegrationTests.cs | 5 +- 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs create mode 100644 test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs new file mode 100644 index 000000000..1217dbd94 --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs @@ -0,0 +1,25 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Elsa.Activities.UserTask.Activities; +using Elsa.Core.IntegrationTests.Workflows; +using Elsa.Testing.Shared.AutoFixture.Customizations; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Core.IntegrationTests.Autofixture +{ + public class HostBuilderWithElsaSampleWorkflowAttribute : CustomizeAttribute + { + public override ICustomization GetCustomization(ParameterInfo parameter) + { + return new HostBubilderUsingServicesCustomization(services => { + services + .AddElsa(elsa => { + elsa.AddActivity(); + elsa.AddWorkflow(); + }) + ; + }, parameter); + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs b/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs new file mode 100644 index 000000000..ca07003f7 --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs @@ -0,0 +1,47 @@ +using Xunit; +using System.Threading.Tasks; +using Elsa.Core.IntegrationTests.Autofixture; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.DependencyInjection; +using System.Threading; +using Elsa.Services; +using Elsa.Core.IntegrationTests.Workflows; +using Elsa.Persistence; + +namespace Elsa.Core.IntegrationTests.Persistence.InMemory +{ + public class InMemoryStoreIntegrationTests + { + [Theory(DisplayName = "A saved workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task ASavedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => services.AddHostedService()); + var host = await hostBuilder.StartAsync(); + } + + class HostedWorkflowRunner : IHostedService + { + readonly IWorkflowRunner workflowRunner; + readonly IWorkflowInstanceStore instanceStore; + + public async Task StartAsync(CancellationToken cancellationToken) + { + var instance = await workflowRunner.RunWorkflowAsync(); + var retrievedInstance = await instanceStore.FindByIdAsync(instance.Id); + + // An instance should totally be retrieved from the store + Assert.NotNull(retrievedInstance); + // Because we are in-memory, it should be a reference to the same instance + Assert.Same(instance, retrievedInstance); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public HostedWorkflowRunner(IWorkflowRunner workflowRunner, IWorkflowInstanceStore instanceStore) + { + this.workflowRunner = workflowRunner ?? throw new System.ArgumentNullException(nameof(workflowRunner)); + this.instanceStore = instanceStore ?? throw new System.ArgumentNullException(nameof(instanceStore)); + } + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs index 667ed4257..3aa4af2cf 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs @@ -13,8 +13,8 @@ namespace Elsa.Core.IntegrationTests.Persistence.MongoDb { public class MongoDbIntegrationTests { - [Theory(DisplayName = "A saved workflow instance should be round-trippable without error"), AutoMoqData] - public async Task ASavedWorkflowInstanceShouldBeRoundTrippableWithoutError([HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute] IHostBuilder hostBuilder) + [Theory(DisplayName = "A saved workflow instance should be persisted-to and readable-from a MongoDb store after being run"), AutoMoqData] + public async Task ASavedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute] IHostBuilder hostBuilder) { hostBuilder.ConfigureServices((ctx, services) => services.AddHostedService()); var host = await hostBuilder.StartAsync(); @@ -31,7 +31,6 @@ namespace Elsa.Core.IntegrationTests.Persistence.MongoDb var retrievedInstance = await instanceStore.FindByIdAsync(instance.Id); Assert.NotNull(retrievedInstance); - Assert.NotSame(instance, retrievedInstance); } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; From 32bcf9a7511f9fe48c3c475853bf93feb58da0fc Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Tue, 9 Mar 2021 19:18:31 +0000 Subject: [PATCH 4/8] WIP #728 - Add test cases for persistence This also includes a change to the workflow used in the test-case. There are quite complex reasons for this, as explained here: https://github.com/elsa-workflows/elsa-core/issues/728#issuecomment-794319236 The real crux of it is that the workflow must have an activity which suspends it, and that activity that suspends the workflow must not be the starting activity. This is why I added an unused set-variable activity as the starter. This commit shows that the various persistence test cases _mostly_ work OK, with the one exception of ActivityExecuted. --- .../Elsa.Core/Handlers/PersistWorkflow.cs | 15 +++++ src/core/Elsa.Core/Services/WorkflowRunner.cs | 11 ++++ ...thElsaSampleWorkflowAndMongoDbAttribute.cs | 2 +- ...tBuilderWithElsaSampleWorkflowAttribute.cs | 6 +- .../InMemory/InMemoryStoreIntegrationTests.cs | 48 +++++++++++++-- .../MongoDb/MongoDbIntegrationTests.cs | 6 +- .../Workflows/PersistableWorkflow.cs | 59 +++++++++++++++++++ .../Workflows/SampleWorkflow.cs | 20 ------- 8 files changed, 135 insertions(+), 32 deletions(-) create mode 100644 test/integration/Elsa.Core.IntegrationTests/Workflows/PersistableWorkflow.cs delete mode 100644 test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs diff --git a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs index 9cfd3ff2a..b2470959f 100644 --- a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs +++ b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs @@ -28,12 +28,16 @@ namespace Elsa.Handlers public async Task Handle(WorkflowSuspended notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + if (notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior == WorkflowPersistenceBehavior.Suspended) await SaveWorkflowAsync(notification.WorkflowExecutionContext, cancellationToken); } public async Task Handle(WorkflowExecutionBurstStarting notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowBurst || behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted) @@ -42,6 +46,8 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionBurstCompleted notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowBurst) @@ -50,6 +56,8 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionPassCompleted notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted || notification.ActivityExecutionContext.ActivityBlueprint.PersistWorkflow) @@ -58,6 +66,8 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecuted notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted || behavior == WorkflowPersistenceBehavior.WorkflowBurst) @@ -66,6 +76,8 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionFinished notification, CancellationToken cancellationToken) { + _logger.LogTrace("Received notification {notification}", notification); + var workflowExecutionContext = notification.WorkflowExecutionContext; if (workflowExecutionContext.DeleteCompletedInstances) @@ -84,9 +96,12 @@ namespace Elsa.Handlers private async ValueTask SaveWorkflowAsync(WorkflowExecutionContext workflowExecutionContext, CancellationToken cancellationToken) { + _logger.LogTrace("Persisting workflow instance {instanceId}", workflowExecutionContext.WorkflowInstance.Id); + workflowExecutionContext.PruneActivityData(); var workflowInstance = workflowExecutionContext.WorkflowInstance; await _workflowInstanceStore.SaveAsync(workflowInstance, cancellationToken); + _logger.LogDebug("Committed workflow {WorkflowInstanceId} to storage", workflowInstance.Id); } } diff --git a/src/core/Elsa.Core/Services/WorkflowRunner.cs b/src/core/Elsa.Core/Services/WorkflowRunner.cs index f2016859d..2499f3d38 100644 --- a/src/core/Elsa.Core/Services/WorkflowRunner.cs +++ b/src/core/Elsa.Core/Services/WorkflowRunner.cs @@ -171,6 +171,8 @@ namespace Elsa.Services using var workflowExecutionScope = _serviceScopeFactory.CreateScope(); var workflowExecutionContext = new WorkflowExecutionContext(workflowExecutionScope.ServiceProvider, workflowBlueprint, workflowInstance, input); + _logger.LogDebug("{methodName} is running workflow instance Id {instanceId}", nameof(RunWorkflowAsync), workflowInstance.Id); + if (!string.IsNullOrWhiteSpace(workflowInstance.ContextId)) { var loadContext = new LoadWorkflowContext(workflowExecutionContext); @@ -204,7 +206,10 @@ namespace Elsa.Services { case WorkflowStatus.Idle: if (!await BeginWorkflow(workflowExecutionContext, activity, input, cancellationToken)) + { + _logger.LogDebug("Workflow {WorkflowInstanceId} cannot begin from an idle state (perhaps it needs a specific input)", workflowInstance.Id); return workflowInstance; + } break; case WorkflowStatus.Running: @@ -213,7 +218,10 @@ namespace Elsa.Services case WorkflowStatus.Suspended: if (!await ResumeWorkflowAsync(workflowExecutionContext, activity!, input, cancellationToken)) + { + _logger.LogDebug("Workflow {WorkflowInstanceId} cannot be resumed from a suspended state (perhaps it needs a specific input)", workflowInstance.Id); return workflowInstance; + } break; default: throw new ArgumentOutOfRangeException(); @@ -231,7 +239,10 @@ namespace Elsa.Services }; if (statusEvent != null) + { + _logger.LogTrace("Publishing a status event of type {eventType} for workflow {WorkflowInstanceId}", statusEvent.GetType().Name, workflowInstance.Id); await _mediator.Publish(statusEvent, cancellationToken); + } await _mediator.Publish(new WorkflowExecutionFinished(workflowExecutionContext), cancellationToken); return workflowExecutionContext.WorkflowInstance; diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs index f395dfc3e..ca0734145 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute.cs @@ -22,7 +22,7 @@ namespace Elsa.Core.IntegrationTests.Autofixture opts.Db = "IntegrationTests"; }); elsa.AddActivity(); - elsa.AddWorkflow(); + elsa.AddWorkflow(); }) ; }, parameter); diff --git a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs index 1217dbd94..d3048e265 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Autofixture/HostBuilderWithElsaSampleWorkflowAttribute.cs @@ -5,6 +5,7 @@ using Elsa.Activities.UserTask.Activities; using Elsa.Core.IntegrationTests.Workflows; using Elsa.Testing.Shared.AutoFixture.Customizations; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace Elsa.Core.IntegrationTests.Autofixture { @@ -16,9 +17,8 @@ namespace Elsa.Core.IntegrationTests.Autofixture services .AddElsa(elsa => { elsa.AddActivity(); - elsa.AddWorkflow(); - }) - ; + elsa.AddWorkflow(); + }); }, parameter); } } diff --git a/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs b/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs index ca07003f7..20308ff38 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Persistence/InMemory/InMemoryStoreIntegrationTests.cs @@ -12,21 +12,59 @@ namespace Elsa.Core.IntegrationTests.Persistence.InMemory { public class InMemoryStoreIntegrationTests { - [Theory(DisplayName = "A saved workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] - public async Task ASavedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + [Theory(DisplayName = "A persistable workflow instance with default persistence behaviour should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task APersistableWorkflowInstanceWithDefaultPersistanceBehaviourShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) { - hostBuilder.ConfigureServices((ctx, services) => services.AddHostedService()); + hostBuilder.ConfigureServices((ctx, services) => { + services.AddHostedService>(); + }); var host = await hostBuilder.StartAsync(); } - class HostedWorkflowRunner : IHostedService + [Theory(DisplayName = "A persistable-on-suspend workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task APersistableOnSuspendWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => { + services.AddHostedService>(); + }); + var host = await hostBuilder.StartAsync(); + } + + [Theory(DisplayName = "A persistable-on-activity-executed workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task APersistableOnActivityExecutedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => { + services.AddHostedService>(); + }); + var host = await hostBuilder.StartAsync(); + } + + [Theory(DisplayName = "A persistable-on-workflow-burst workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task APersistableOnWorkflowBurstWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => { + services.AddHostedService>(); + }); + var host = await hostBuilder.StartAsync(); + } + + [Theory(DisplayName = "A persistable-on-workflow-pass-completed workflow instance should be persisted-to and readable-from an in-memory store after being run"), AutoMoqData] + public async Task APersistableOnWorkflowPassCompletedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflow] IHostBuilder hostBuilder) + { + hostBuilder.ConfigureServices((ctx, services) => { + services.AddHostedService>(); + }); + var host = await hostBuilder.StartAsync(); + } + + class HostedWorkflowRunner : IHostedService where TWorkflow : PersistableWorkflow { readonly IWorkflowRunner workflowRunner; readonly IWorkflowInstanceStore instanceStore; public async Task StartAsync(CancellationToken cancellationToken) { - var instance = await workflowRunner.RunWorkflowAsync(); + var instance = await workflowRunner.RunWorkflowAsync(); var retrievedInstance = await instanceStore.FindByIdAsync(instance.Id); // An instance should totally be retrieved from the store diff --git a/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs index 3aa4af2cf..27fc34065 100644 --- a/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs +++ b/test/integration/Elsa.Core.IntegrationTests/Persistence/MongoDb/MongoDbIntegrationTests.cs @@ -13,8 +13,8 @@ namespace Elsa.Core.IntegrationTests.Persistence.MongoDb { public class MongoDbIntegrationTests { - [Theory(DisplayName = "A saved workflow instance should be persisted-to and readable-from a MongoDb store after being run"), AutoMoqData] - public async Task ASavedWorkflowInstanceShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute] IHostBuilder hostBuilder) + [Theory(DisplayName = "A persistable workflow instance with default persistence behaviour should be persisted-to and readable-from a MongoDb store after being run"), AutoMoqData] + public async Task APersistableWorkflowInstanceWithDefaultPersistanceBehaviourShouldBeRoundTrippable([HostBuilderWithElsaSampleWorkflowAndMongoDbAttribute] IHostBuilder hostBuilder) { hostBuilder.ConfigureServices((ctx, services) => services.AddHostedService()); var host = await hostBuilder.StartAsync(); @@ -27,7 +27,7 @@ namespace Elsa.Core.IntegrationTests.Persistence.MongoDb public async Task StartAsync(CancellationToken cancellationToken) { - var instance = await workflowRunner.RunWorkflowAsync(); + var instance = await workflowRunner.RunWorkflowAsync(); var retrievedInstance = await instanceStore.FindByIdAsync(instance.Id); Assert.NotNull(retrievedInstance); diff --git a/test/integration/Elsa.Core.IntegrationTests/Workflows/PersistableWorkflow.cs b/test/integration/Elsa.Core.IntegrationTests/Workflows/PersistableWorkflow.cs new file mode 100644 index 000000000..af7a1b36b --- /dev/null +++ b/test/integration/Elsa.Core.IntegrationTests/Workflows/PersistableWorkflow.cs @@ -0,0 +1,59 @@ +using Elsa.Activities.UserTask.Activities; +using Elsa.Builders; +using Elsa.Activities.ControlFlow; +using Elsa.Activities.Primitives; +using Elsa.Models; + +namespace Elsa.Core.IntegrationTests.Workflows +{ + public class PersistableWorkflow : IWorkflow + { + public static readonly object Result = new object(); + + public virtual void Build(IWorkflowBuilder builder) + { + builder + .StartWith(a => a.Set(x => x.VariableName, "Unused").Set(x => x.Value, "Unused")) + .Then(setup: s => { + s.Set(x => x.Actions, c => new [] { "Foo", "Bar" }); + }) + .Finish(x => x.WithOutput(Result)); + } + + public class OnSuspend : PersistableWorkflow + { + public override void Build(IWorkflowBuilder builder) + { + builder.WithPersistenceBehavior(WorkflowPersistenceBehavior.Suspended); + base.Build(builder); + } + } + + public class OnActivityExecuted : PersistableWorkflow + { + public override void Build(IWorkflowBuilder builder) + { + builder.WithPersistenceBehavior(WorkflowPersistenceBehavior.ActivityExecuted); + base.Build(builder); + } + } + + public class OnWorkflowBurst : PersistableWorkflow + { + public override void Build(IWorkflowBuilder builder) + { + builder.WithPersistenceBehavior(WorkflowPersistenceBehavior.WorkflowBurst); + base.Build(builder); + } + } + + public class OnWorkflowPassCompleted : PersistableWorkflow + { + public override void Build(IWorkflowBuilder builder) + { + builder.WithPersistenceBehavior(WorkflowPersistenceBehavior.WorkflowPassCompleted); + base.Build(builder); + } + } + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs b/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs deleted file mode 100644 index 0ee6bd155..000000000 --- a/test/integration/Elsa.Core.IntegrationTests/Workflows/SampleWorkflow.cs +++ /dev/null @@ -1,20 +0,0 @@ -using Elsa.Activities.UserTask.Activities; -using Elsa.Builders; -using Elsa.Activities.ControlFlow; - -namespace Elsa.Core.IntegrationTests.Workflows -{ - public class SampleWorkflow : IWorkflow - { - public static readonly object Result = new object(); - - public void Build(IWorkflowBuilder builder) - { - builder - .StartWith(setup: s => { - s.Set(x => x.Actions, c => new [] { "Foo", "Bar" }); - }) - .Finish(x => x.WithOutput(Result)); - } - } -} \ No newline at end of file From 8751bead5a5a875eb897dd762aeeb8619556fe7e Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Tue, 9 Mar 2021 19:52:36 +0000 Subject: [PATCH 5/8] WIP #728 - Make WorkflowBurst default behaviour In the comments to #728 it was suggested that WorkflowBurst persistence behaviour was a more sane default. --- .../Models/WorkflowPersistenceBehavior.cs | 16 ++++++---- .../Services/WorkflowBuilderTests.cs | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 test/unit/Elsa.UnitTests/Services/WorkflowBuilderTests.cs diff --git a/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs b/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs index e7881dea9..d0b1eea64 100644 --- a/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs +++ b/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs @@ -6,16 +6,22 @@ namespace Elsa.Models [JsonConverter(typeof(StringEnumConverter))] public enum WorkflowPersistenceBehavior { + // TODO: Consider a new service to "get the default persistence behaviour". + // Currently it is set to WorkflowBurst implicitly because it is the default + // for the enum type. This would be more clear if explicit. + // This includes altering all locations which need a default persistence + // behaviour though. + + /// + /// Workflow instances are persisted after the workflow completed a burst of execution. + /// + WorkflowBurst = 0, + /// /// Workflow instances are persisted only when being suspended. /// Suspended, - /// - /// Workflow instances are persisted after the workflow completed a burst of execution. - /// - WorkflowBurst, - /// /// Workflow instances are persisted after the workflow executed scheduled activities. /// diff --git a/test/unit/Elsa.UnitTests/Services/WorkflowBuilderTests.cs b/test/unit/Elsa.UnitTests/Services/WorkflowBuilderTests.cs new file mode 100644 index 000000000..e38ec7990 --- /dev/null +++ b/test/unit/Elsa.UnitTests/Services/WorkflowBuilderTests.cs @@ -0,0 +1,29 @@ +using System; +using AutoFixture.Xunit2; +using Elsa.Builders; +using Elsa.Models; +using Elsa.Testing.Shared.AutoFixture.Attributes; +using Xunit; + +namespace Elsa.Services +{ + public class WorkflowBuilderTests + { + [Theory(DisplayName = "The Build method should return a workflow blueprint with the WorkflowBurst persistence behaviour if no behaviour was specified"), AutoMoqData] + public void BuildShouldReturnWorkflowBlueprintWithWorkflowBurstPersistenceBehaviourIfNoBehaviourSpecified([AutofixtureServiceProvider, Frozen] IServiceProvider serviceProvider, + WorkflowBuilder sut, + string idPrefix) + { + var workflow = new NoOpWorkflow(); + + var blueprint = sut.Build(workflow, idPrefix); + + Assert.Equal(WorkflowPersistenceBehavior.WorkflowBurst, blueprint.PersistenceBehavior); + } + + class NoOpWorkflow : IWorkflow + { + public void Build(IWorkflowBuilder builder) { } + } + } +} \ No newline at end of file From 7f5a6228cfb2d9a51d0597ecd5aa182c8d119e9d Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Tue, 9 Mar 2021 20:09:36 +0000 Subject: [PATCH 6/8] Resolve #728 - Fix ActivityExecuted persistence The PersistWorkflow service will now handle ActivityExecuted persistence behaviour, presumably as-intended. --- src/core/Elsa.Core/Handlers/PersistWorkflow.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs index b2470959f..0e53bff60 100644 --- a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs +++ b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs @@ -15,7 +15,8 @@ namespace Elsa.Handlers INotificationHandler, INotificationHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { private readonly IWorkflowInstanceStore _workflowInstanceStore; private readonly ILogger _logger; @@ -94,6 +95,16 @@ namespace Elsa.Handlers } } + public async Task Handle(ActivityExecuted notification, CancellationToken cancellationToken) + { + _logger.LogTrace("Received notification {notification}", notification); + + var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; + + if (behavior == WorkflowPersistenceBehavior.ActivityExecuted) + await SaveWorkflowAsync(notification.WorkflowExecutionContext, cancellationToken); + } + private async ValueTask SaveWorkflowAsync(WorkflowExecutionContext workflowExecutionContext, CancellationToken cancellationToken) { _logger.LogTrace("Persisting workflow instance {instanceId}", workflowExecutionContext.WorkflowInstance.Id); From 5c927501d0d6156ebc2806d06dc420baa6e5aced Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Wed, 10 Mar 2021 20:08:53 +0000 Subject: [PATCH 7/8] PR rework #728 - Default persistence behavior This reworks the way that the default behavior is selected, in line with comments. The changes to the enum are reverted, and the three places which create a workflow definition are now altered to explicitly choose WorkflowBurst This includes a change to the TS enum definition - update to match its C# counterpart. --- .../Models/WorkflowPersistenceBehavior.cs | 16 +++++----------- src/core/Elsa.Core/Builders/WorkflowBuilder.cs | 1 + .../elsa-designer-tree/elsa-designer-tree.tsx | 4 ++-- .../elsa-workflow-editor.tsx | 14 ++++++++++++-- .../elsa-workflows-studio/src/models/domain.ts | 3 ++- .../elsa-workflows-studio/src/models/view.ts | 3 ++- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs b/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs index d0b1eea64..1bd3cae9d 100644 --- a/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs +++ b/src/core/Elsa.Abstractions/Models/WorkflowPersistenceBehavior.cs @@ -6,22 +6,16 @@ namespace Elsa.Models [JsonConverter(typeof(StringEnumConverter))] public enum WorkflowPersistenceBehavior { - // TODO: Consider a new service to "get the default persistence behaviour". - // Currently it is set to WorkflowBurst implicitly because it is the default - // for the enum type. This would be more clear if explicit. - // This includes altering all locations which need a default persistence - // behaviour though. - - /// - /// Workflow instances are persisted after the workflow completed a burst of execution. - /// - WorkflowBurst = 0, - /// /// Workflow instances are persisted only when being suspended. /// Suspended, + /// + /// Workflow instances are persisted after the workflow completed a burst of execution. + /// + WorkflowBurst, + /// /// Workflow instances are persisted after the workflow executed scheduled activities. /// diff --git a/src/core/Elsa.Core/Builders/WorkflowBuilder.cs b/src/core/Elsa.Core/Builders/WorkflowBuilder.cs index cb2b44b69..1aadcf86d 100644 --- a/src/core/Elsa.Core/Builders/WorkflowBuilder.cs +++ b/src/core/Elsa.Core/Builders/WorkflowBuilder.cs @@ -19,6 +19,7 @@ namespace Elsa.Builders ActivityType = typeof(Workflow); WorkflowBuilder = this; PropertyValueProviders = new Dictionary(); + PersistenceBehavior = WorkflowPersistenceBehavior.WorkflowBurst; } public int Version { get; private set; } diff --git a/src/designer/elsa-workflows-studio/src/components/designers/tree/elsa-designer-tree/elsa-designer-tree.tsx b/src/designer/elsa-workflows-studio/src/components/designers/tree/elsa-designer-tree/elsa-designer-tree.tsx index ec09d8753..ac4f65900 100644 --- a/src/designer/elsa-workflows-studio/src/components/designers/tree/elsa-designer-tree/elsa-designer-tree.tsx +++ b/src/designer/elsa-workflows-studio/src/components/designers/tree/elsa-designer-tree/elsa-designer-tree.tsx @@ -1,7 +1,7 @@ import {Component, Host, h, Prop, State, Event, EventEmitter, Listen, Watch} from '@stencil/core'; import {Map, addConnection, findActivity, getChildActivities, getInboundConnections, getOutboundConnections, removeActivity} from '../../../../utils/utils'; import {cleanup, destroy, updateConnections} from '../../../../utils/jsplumb-helper'; -import {ActivityDescriptor, ActivityDesignDisplayContext, ActivityModel, ConnectionModel, EventTypes, WorkflowModel} from "../../../../models"; +import {ActivityDescriptor, ActivityDesignDisplayContext, ActivityModel, ConnectionModel, EventTypes, WorkflowModel, WorkflowPersistenceBehavior} from "../../../../models"; import {eventBus} from '../../../../services/event-bus'; import jsPlumb from "jsplumb"; import uuid = jsPlumb.jsPlumbUtil.uuid; @@ -15,7 +15,7 @@ import {ActivityIcon} from "../../../icons/activity-icon"; }) export class ElsaWorkflowDesigner { - @Prop() model: WorkflowModel = {activities: [], connections: []}; + @Prop() model: WorkflowModel = { activities: [], connections: [], persistenceBehavior: WorkflowPersistenceBehavior.WorkflowBurst }; @Event({eventName: 'workflow-changed', bubbles: true, composed: true, cancelable: true}) workflowChanged: EventEmitter @State() workflowModel: WorkflowModel el: HTMLElement diff --git a/src/designer/elsa-workflows-studio/src/components/editors/workflow-editor/elsa-workflow-editor/elsa-workflow-editor.tsx b/src/designer/elsa-workflows-studio/src/components/editors/workflow-editor/elsa-workflow-editor/elsa-workflow-editor.tsx index 6ed880cce..03b67d4ce 100644 --- a/src/designer/elsa-workflows-studio/src/components/editors/workflow-editor/elsa-workflow-editor/elsa-workflow-editor.tsx +++ b/src/designer/elsa-workflows-studio/src/components/editors/workflow-editor/elsa-workflow-editor/elsa-workflow-editor.tsx @@ -1,6 +1,6 @@ import {Component, Event, EventEmitter, h, Host, Listen, Method, Prop, State, Watch} from '@stencil/core'; import {eventBus} from '../../../../services/event-bus'; -import {ActivityDefinition, ActivityDescriptor, ActivityModel, ConnectionDefinition, ConnectionModel, EventTypes, WorkflowDefinition, WorkflowModel} from "../../../../models"; +import {ActivityDefinition, ActivityDescriptor, ActivityModel, ConnectionDefinition, ConnectionModel, EventTypes, WorkflowDefinition, WorkflowModel, WorkflowPersistenceBehavior} from "../../../../models"; import {createElsaClient, SaveWorkflowDefinitionRequest} from "../../../../services/elsa-client"; import {pluginManager} from '../../../../services/plugin-manager'; import state from '../../../../utils/store'; @@ -49,7 +49,7 @@ export class ElsaWorkflowDefinitionEditor { @Watch('workflowDefinitionId') async workflowDefinitionIdChangedHandler(newValue: string) { const workflowDefinitionId = newValue; - let workflowDefinition: WorkflowDefinition = {definitionId: this.workflowDefinitionId, version: 1, activities: [], connections: []}; + let workflowDefinition: WorkflowDefinition = this.createWorkflowDefinition(); const client = createElsaClient(this.serverUrl); if (workflowDefinitionId && workflowDefinitionId.length > 0) { @@ -339,4 +339,14 @@ export class ElsaWorkflowDefinitionEditor { onUnPublishClicked={() => this.onUnPublishClicked()} />; } + + private createWorkflowDefinition() : WorkflowDefinition { + return { + definitionId: this.workflowDefinitionId, + version: 1, + activities: [], + connections: [], + persistenceBehavior: WorkflowPersistenceBehavior.WorkflowBurst, + }; + } } diff --git a/src/designer/elsa-workflows-studio/src/models/domain.ts b/src/designer/elsa-workflows-studio/src/models/domain.ts index 3985ad695..0718ca960 100644 --- a/src/designer/elsa-workflows-studio/src/models/domain.ts +++ b/src/designer/elsa-workflows-studio/src/models/domain.ts @@ -61,7 +61,8 @@ export enum WorkflowContextFidelity { export enum WorkflowPersistenceBehavior { Suspended, - WorkflowPAssCompleted, + WorkflowBurst, + WorkflowPassCompleted, ActivityExecuted } diff --git a/src/designer/elsa-workflows-studio/src/models/view.ts b/src/designer/elsa-workflows-studio/src/models/view.ts index ac0110ee7..c8b199164 100644 --- a/src/designer/elsa-workflows-studio/src/models/view.ts +++ b/src/designer/elsa-workflows-studio/src/models/view.ts @@ -1,8 +1,9 @@ -import {ActivityDefinitionProperty} from "./domain"; +import {ActivityDefinitionProperty, WorkflowPersistenceBehavior} from "./domain"; export interface WorkflowModel { activities: Array connections: Array + persistenceBehavior?: WorkflowPersistenceBehavior, } export interface ActivityModel { From 10094942a4788bd3d97cdb012c652aae58dc9ca0 Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Wed, 10 Mar 2021 20:15:52 +0000 Subject: [PATCH 8/8] PR rework #728 - Remove repetitive logging --- src/core/Elsa.Core/Handlers/PersistWorkflow.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs index 0e53bff60..24a0dc7d6 100644 --- a/src/core/Elsa.Core/Handlers/PersistWorkflow.cs +++ b/src/core/Elsa.Core/Handlers/PersistWorkflow.cs @@ -29,16 +29,12 @@ namespace Elsa.Handlers public async Task Handle(WorkflowSuspended notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - if (notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior == WorkflowPersistenceBehavior.Suspended) await SaveWorkflowAsync(notification.WorkflowExecutionContext, cancellationToken); } public async Task Handle(WorkflowExecutionBurstStarting notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowBurst || behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted) @@ -47,8 +43,6 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionBurstCompleted notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowBurst) @@ -57,8 +51,6 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionPassCompleted notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted || notification.ActivityExecutionContext.ActivityBlueprint.PersistWorkflow) @@ -67,8 +59,6 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecuted notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.WorkflowPassCompleted || behavior == WorkflowPersistenceBehavior.WorkflowBurst) @@ -77,8 +67,6 @@ namespace Elsa.Handlers public async Task Handle(WorkflowExecutionFinished notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var workflowExecutionContext = notification.WorkflowExecutionContext; if (workflowExecutionContext.DeleteCompletedInstances) @@ -97,8 +85,6 @@ namespace Elsa.Handlers public async Task Handle(ActivityExecuted notification, CancellationToken cancellationToken) { - _logger.LogTrace("Received notification {notification}", notification); - var behavior = notification.WorkflowExecutionContext.WorkflowBlueprint.PersistenceBehavior; if (behavior == WorkflowPersistenceBehavior.ActivityExecuted)