From 6a0f74e06c2a83f057889fb54c3eafff7e122e3f Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 8 May 2024 09:29:15 +0200 Subject: [PATCH] Fix FlowJoin Activity NRE Bug (#5349) * Update property retrieval in FlowJoin activity This commit modifies the way flowScope is retrieved within the FlowJoin activity in Elsa.Workflows.Core. Instead of directly calling GetProperty, a fallback value is now being provided in case the desired property is not found. This reduces the risk of null reference exceptions. * Add FlowJoins component tests Two new files have been created to facilitate component testing for the FlowJoins scenarios in the Elsa Workflows. The `Tests.cs` file includes a Fact to validate the successful execution of a Flowchart with a single FlowJoin. The `Workflows.cs` file defines a single join workflow for these tests. * Add RabbitMq support to ComponentTests Added RabbitMq to Infrastructure.cs for component testing, allowing both RabbitMqContainer and DbContainer to start and stop asynchronously. Also, adjusted WorkflowServer.cs to configure RabbitMq mass transit alongside existing PostgreSql support, aiming to improve testing robustness and coverage. * Refactor placement of RemoveOrphanedSubscriptions service Move the implementation of AddNotificationHandler() from the notifier block to the singleton section in AzureServiceBusFeature.cs. This will help keep all service registration related to notifications in one place. * Increase prefetch count --- src/bundles/Elsa.Server.Web/Program.cs | 2 +- .../Features/AzureServiceBusFeature.cs | 2 +- .../Flowchart/Activities/FlowJoin.cs | 2 +- .../Helpers/Fixtures/Infrastructure.cs | 13 +++++++++++-- .../Helpers/Fixtures/WorkflowServer.cs | 11 +++++++++++ .../Scenarios/Activities/FlowJoins/Tests.cs | 16 ++++++++++++++++ .../Activities/FlowJoins/Workflows.cs | 18 ++++++++++++++++++ 7 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Tests.cs create mode 100644 test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Workflows.cs diff --git a/src/bundles/Elsa.Server.Web/Program.cs b/src/bundles/Elsa.Server.Web/Program.cs index b5647e504..7efc939b4 100644 --- a/src/bundles/Elsa.Server.Web/Program.cs +++ b/src/bundles/Elsa.Server.Web/Program.cs @@ -323,7 +323,7 @@ services { massTransit.UseAzureServiceBus(azureServiceBusConnectionString, serviceBusFeature => serviceBusFeature.ConfigureServiceBus = bus => { - bus.PrefetchCount = 4; + bus.PrefetchCount = 100; bus.LockDuration = TimeSpan.FromMinutes(5); bus.MaxConcurrentCalls = 32; bus.MaxDeliveryCount = 8; diff --git a/src/modules/Elsa.MassTransit.AzureServiceBus/Features/AzureServiceBusFeature.cs b/src/modules/Elsa.MassTransit.AzureServiceBus/Features/AzureServiceBusFeature.cs index 60192d33f..1c3f4ab3e 100644 --- a/src/modules/Elsa.MassTransit.AzureServiceBus/Features/AzureServiceBusFeature.cs +++ b/src/modules/Elsa.MassTransit.AzureServiceBus/Features/AzureServiceBusFeature.cs @@ -108,7 +108,6 @@ public class AzureServiceBusFeature : FeatureBase { Services.Configure(AzureServiceBusOptions); Services.AddSingleton(ServiceBusAdministrationClientFactory); - Services.AddNotificationHandler(); } private static string GetConnectionString(IServiceProvider serviceProvider) @@ -130,6 +129,7 @@ public class AzureServiceBusFeature : FeatureBase ).ToList(); Services.AddSingleton(new MessageTopologyProvider(subscriptionTopology)); + Services.AddNotificationHandler(); } } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/FlowJoin.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/FlowJoin.cs index 6eca25f4d..28b154495 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/FlowJoin.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/FlowJoin.cs @@ -38,7 +38,7 @@ public class FlowJoin : Activity, IJoinNode var flowchartContext = context.ParentActivityExecutionContext!; var flowchart = (Flowchart)flowchartContext.Activity; var inboundActivities = flowchart.Connections.LeftInboundActivities(this).ToList(); - var flowScope = flowchartContext.GetProperty(Flowchart.ScopeProperty)!; + var flowScope = flowchartContext.GetProperty(Flowchart.ScopeProperty, () => new FlowScope()); var executionCount = flowScope.GetExecutionCount(this); var mode = context.Get(Mode); diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/Infrastructure.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/Infrastructure.cs index f199f9485..e3b3af15b 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/Infrastructure.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/Infrastructure.cs @@ -1,4 +1,5 @@ using Testcontainers.PostgreSql; +using Testcontainers.RabbitMq; namespace Elsa.Workflows.ComponentTests; @@ -11,13 +12,21 @@ public class Infrastructure : IAsyncLifetime .WithPassword("postgres") .Build(); + public readonly RabbitMqContainer RabbitMqContainer = new RabbitMqBuilder() + .WithImage("rabbitmq:3-management") + .Build(); + public Task InitializeAsync() { - return DbContainer.StartAsync(); + return Task.WhenAll( + DbContainer.StartAsync(), + RabbitMqContainer.StartAsync()); } public Task DisposeAsync() { - return DbContainer.StopAsync(); + return Task.WhenAll( + DbContainer.StopAsync(), + RabbitMqContainer.StopAsync()); } } \ No newline at end of file diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs index 39d6b2f4f..4f15b0b30 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs @@ -3,6 +3,7 @@ using Elsa.EntityFrameworkCore.Extensions; using Elsa.EntityFrameworkCore.Modules.Management; using Elsa.Extensions; using Elsa.Identity.Providers; +using Elsa.MassTransit.Extensions; using Elsa.Workflows.ComponentTests.Services; using FluentStorage; using Hangfire.Annotations; @@ -37,6 +38,7 @@ public class WorkflowServer(Infrastructure infrastructure, string url) : WebAppl protected override void ConfigureWebHost(IWebHostBuilder builder) { var dbConnectionString = infrastructure.DbContainer.GetConnectionString(); + var rabbitMqConnectionString = infrastructure.RabbitMqContainer.GetConnectionString(); builder.UseUrls(url); @@ -58,9 +60,18 @@ public class WorkflowServer(Infrastructure infrastructure, string url) : WebAppl var workflowsDirectory = Path.Join(workflowsDirectorySegments); return StorageFactory.Blobs.DirectoryFiles(workflowsDirectory); }); + elsa.UseMassTransit(massTransit => + { + massTransit.UseRabbitMq(rabbitMqConnectionString); + }); elsa.UseWorkflowManagement(management => { management.UseEntityFrameworkCore(ef => ef.UsePostgreSql(dbConnectionString)); + management.UseMassTransitDispatcher(); + }); + elsa.UseWorkflowRuntime(runtime => + { + runtime.UseMassTransitDispatcher(); }); }; } diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Tests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Tests.cs new file mode 100644 index 000000000..b3cb47112 --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Tests.cs @@ -0,0 +1,16 @@ +using Elsa.Workflows.Contracts; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Workflows.ComponentTests.Scenarios.Activities.FlowJoins; + +public class Tests(App app) : AppComponentTest(app) +{ + // https://github.com/elsa-workflows/elsa-core/issues/5348 + [Fact] + public async Task FlowchartWithSingleFlowJoin_ShouldExecuteSuccessfully() + { + var workflowRunner = Scope.ServiceProvider.GetRequiredService(); + var result = await workflowRunner.RunAsync(); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } +} \ No newline at end of file diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Workflows.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Workflows.cs new file mode 100644 index 000000000..bacaa0cc0 --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/FlowJoins/Workflows.cs @@ -0,0 +1,18 @@ +using Elsa.Workflows.Activities.Flowchart.Activities; +using Elsa.Workflows.Contracts; + +namespace Elsa.Workflows.ComponentTests.Scenarios.Activities.FlowJoins; + +public class SingleJoinWorkflow : WorkflowBase +{ + protected override void Build(IWorkflowBuilder builder) + { + builder.Root = new Flowchart + { + Activities = + { + new FlowJoin() + } + }; + } +} \ No newline at end of file