From e2288d0b343a67d0aad18ac398cb7e0fbcee8eba Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 5 Apr 2025 10:37:05 +0200 Subject: [PATCH] Fix infinitely waiting Alterations Workflow (#6561) * Refactor TenantId string handling and nullability checks. Moved `StringExtensions` to a common module for reuse. Updated tenant-related logic to utilize null-safe string extensions, enhancing consistency and simplifying nullability handling across the codebase. * Set StrictMode to false by default in ObjectConverter Modified the default value of StrictMode to `false` to enable the original flexible behavior. Developers can opt into strict mode by explicitly setting it to `true`. This change aims to enhance backward compatibility and minimize unexpected strict conversions. * Add tenant ID retrieval to ElsaDbContextBase constructor Retrieve the current tenant ID if available using ITenantAccessor and assign it to the TenantId property. This ensures proper handling of multi-tenancy scenarios in the database context initialization. * Disable DbContext pooling, manual OTEL instrumentation, and strict mode. DbContext pooling is turned off to prevent potential issues with shared context instances. Manual OpenTelemetry instrumentation is disabled to rely on automatic instrumentation instead. Strict mode is also disabled to allow more flexibility in object conversion. * Fix infinitely waiting Alterations Workflow Replaced workflow dispatch logic with BookmarkQueue and StimulusHasher for triggering workflows. This fixes the issue where the Alterations workflow would signal completion while a later step awaits a completion bookmark. The Bookmark Queue now handles this. --- src/apps/Elsa.Server.Web/Program.cs | 6 +++--- .../Handlers/AlterationPlanCompletedHandler.cs | 14 +++++++++----- .../Services/AlterationPlanManager.cs | 2 +- .../Services/DefaultAlterationPlanScheduler.cs | 2 +- .../Extensions/StringExtensions.cs | 0 .../Elsa.Common/Multitenancy/Entities/Tenant.cs | 4 ++-- .../Implementations/DefaultTenantService.cs | 15 ++++++++------- .../Multitenancy/Models/TenantFilter.cs | 4 ++-- .../ElsaDbContextBase.cs | 8 ++++++++ .../EntityHandlers/ApplyTenantId.cs | 3 ++- .../EntityHandlers/SetTenantIdFilter.cs | 4 +++- .../Elsa.Expressions/Helpers/ObjectConverter.cs | 2 +- .../Services/BookmarkQueueSignaler.cs | 2 +- 13 files changed, 41 insertions(+), 25 deletions(-) rename src/modules/{Elsa.Workflows.Core => Elsa.Common}/Extensions/StringExtensions.cs (100%) diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index 3d9f61f27..f88e3edd0 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -86,7 +86,7 @@ using StackExchange.Redis; // ReSharper disable RedundantAssignment const PersistenceProvider persistenceProvider = PersistenceProvider.EntityFrameworkCore; -const bool useDbContextPooling = true; +const bool useDbContextPooling = false; const bool useHangfire = false; const bool useQuartz = true; const bool useMassTransit = true; @@ -105,9 +105,9 @@ const bool useTenantsFromConfiguration = true; const bool useSecrets = false; const bool disableVariableWrappers = false; const bool disableVariableCopying = false; -const bool useManualOtelInstrumentation = true; +const bool useManualOtelInstrumentation = false; -ObjectConverter.StrictMode = true; // Default. +ObjectConverter.StrictMode = false; var builder = WebApplication.CreateBuilder(args); var services = builder.Services; diff --git a/src/modules/Elsa.Alterations/Handlers/AlterationPlanCompletedHandler.cs b/src/modules/Elsa.Alterations/Handlers/AlterationPlanCompletedHandler.cs index 18a210510..f9b605055 100644 --- a/src/modules/Elsa.Alterations/Handlers/AlterationPlanCompletedHandler.cs +++ b/src/modules/Elsa.Alterations/Handlers/AlterationPlanCompletedHandler.cs @@ -1,10 +1,9 @@ using Elsa.Alterations.Bookmarks; using Elsa.Alterations.Core.Notifications; using Elsa.Mediator.Contracts; +using Elsa.Workflows; using Elsa.Workflows.Helpers; using Elsa.Workflows.Runtime; -using Elsa.Workflows.Runtime.Contracts; -using Elsa.Workflows.Runtime.Requests; using JetBrains.Annotations; namespace Elsa.Alterations.Handlers; @@ -13,7 +12,7 @@ namespace Elsa.Alterations.Handlers; /// Handles notifications and triggers any workflows that are waiting for the plan to complete. /// [UsedImplicitly] -public class AlterationPlanCompletedHandler(IWorkflowDispatcher workflowDispatcher) : INotificationHandler +public class AlterationPlanCompletedHandler(IBookmarkQueue bookmarkQueue, IStimulusHasher stimulusHasher) : INotificationHandler { /// public async Task HandleAsync(AlterationPlanCompleted notification, CancellationToken cancellationToken) @@ -21,7 +20,12 @@ public class AlterationPlanCompletedHandler(IWorkflowDispatcher workflowDispatch // Trigger any workflow instances that are waiting for the plan to complete. var planId = notification.Plan.Id; var bookmarkPayload = new AlterationPlanCompletedPayload(planId); - var triggerRequest = new DispatchTriggerWorkflowsRequest(ActivityTypeNameHelper.GenerateTypeName(), bookmarkPayload); - await workflowDispatcher.DispatchAsync(triggerRequest, cancellationToken); + var activityTypeName = ActivityTypeNameHelper.GenerateTypeName(); + var item = new NewBookmarkQueueItem + { + ActivityTypeName = activityTypeName, + StimulusHash = stimulusHasher.Hash(activityTypeName, bookmarkPayload) + }; + await bookmarkQueue.EnqueueAsync(item, cancellationToken); } } \ No newline at end of file diff --git a/src/modules/Elsa.Alterations/Services/AlterationPlanManager.cs b/src/modules/Elsa.Alterations/Services/AlterationPlanManager.cs index eb40d5d7c..23e054216 100644 --- a/src/modules/Elsa.Alterations/Services/AlterationPlanManager.cs +++ b/src/modules/Elsa.Alterations/Services/AlterationPlanManager.cs @@ -25,7 +25,7 @@ public class AlterationPlanManager(IAlterationPlanStore planStore, IAlterationJo var jobFilter = new AlterationJobFilter { PlanId = planId, - Statuses = new[] { AlterationJobStatus.Pending, AlterationJobStatus.Running } + Statuses = [AlterationJobStatus.Pending, AlterationJobStatus.Running] }; return await jobStore.CountAsync(jobFilter, cancellationToken) == 0; diff --git a/src/modules/Elsa.Alterations/Services/DefaultAlterationPlanScheduler.cs b/src/modules/Elsa.Alterations/Services/DefaultAlterationPlanScheduler.cs index fd8275282..4ab30f434 100644 --- a/src/modules/Elsa.Alterations/Services/DefaultAlterationPlanScheduler.cs +++ b/src/modules/Elsa.Alterations/Services/DefaultAlterationPlanScheduler.cs @@ -42,7 +42,7 @@ public class DefaultAlterationPlanScheduler : IAlterationPlanScheduler var workflowGraph = await _workflowDefinitionService.FindWorkflowGraphAsync(definitionId, VersionOptions.Published, cancellationToken); if (workflowGraph == null) - throw new Exception($"Workflow definition with ID '{definitionId}' not found"); + throw new($"Workflow definition with ID '{definitionId}' not found"); var serializedPlan = _jsonSerializer.Serialize(planParams); var request = new DispatchWorkflowDefinitionRequest(workflowGraph.Workflow.Identity.Id) diff --git a/src/modules/Elsa.Workflows.Core/Extensions/StringExtensions.cs b/src/modules/Elsa.Common/Extensions/StringExtensions.cs similarity index 100% rename from src/modules/Elsa.Workflows.Core/Extensions/StringExtensions.cs rename to src/modules/Elsa.Common/Extensions/StringExtensions.cs diff --git a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs index a832baa2c..cba4b371b 100644 --- a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs +++ b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs @@ -13,7 +13,7 @@ public class Tenant : Entity /// /// Gets or sets the name. /// - public string Name { get; set; } = default!; + public string Name { get; set; } = null!; /// /// Gets or sets the configuration. @@ -22,7 +22,7 @@ public class Tenant : Entity public static readonly Tenant Default = new() { - Id = string.Empty, + Id = null!, Name = "Default" }; } diff --git a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs index fca84ebc9..89298e5ac 100644 --- a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs +++ b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs @@ -1,3 +1,4 @@ +using Elsa.Extensions; using Microsoft.Extensions.DependencyInjection; namespace Elsa.Common.Multitenancy; @@ -19,7 +20,7 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop public async Task FindAsync(string id, CancellationToken cancellationToken = default) { var dictionary = await GetTenantsDictionaryAsync(cancellationToken); - return dictionary.TryGetValue(id, out var tenant) ? tenant : null; + return dictionary.TryGetValue(id.EmptyIfNull(), out var tenant) ? tenant : null; } public async Task FindAsync(TenantFilter filter, CancellationToken cancellationToken = default) @@ -31,7 +32,7 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop public async Task GetAsync(string id, CancellationToken cancellationToken = default) { var dictionary = await GetTenantsDictionaryAsync(cancellationToken); - return dictionary[id]; + return dictionary[id.EmptyIfNull()]; } public async Task GetAsync(TenantFilter filter, CancellationToken cancellationToken = default) @@ -76,7 +77,7 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop var tenantsProvider = scope.ServiceProvider.GetRequiredService(); var currentTenants = await GetTenantsDictionaryAsync(cancellationToken); var currentTenantIds = currentTenants.Keys; - var newTenants = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id); + var newTenants = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id.EmptyIfNull()); var newTenantIds = newTenants.Keys; var removedTenantIds = currentTenantIds.Except(newTenantIds).ToArray(); var addedTenantIds = newTenantIds.Except(currentTenantIds).ToArray(); @@ -129,21 +130,21 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop private async Task RegisterTenantAsync(Tenant tenant, CancellationToken cancellationToken = default) { var scope = tenantScopeFactory.CreateScope(tenant); - _tenantsDictionary![tenant.Id] = tenant; + _tenantsDictionary![tenant.Id.EmptyIfNull()] = tenant; _tenantScopesDictionary![tenant] = scope; using (tenantAccessor.PushContext(tenant)) - await tenantEvents.TenantActivatedAsync(new TenantActivatedEventArgs(tenant, scope, cancellationToken)); + await tenantEvents.TenantActivatedAsync(new(tenant, scope, cancellationToken)); } private async Task UnregisterTenantAsync(Tenant tenant, CancellationToken cancellationToken = default) { if (_tenantScopesDictionary!.Remove(tenant, out var scope)) { - _tenantsDictionary!.Remove(tenant.Id, out _); + _tenantsDictionary!.Remove(tenant.Id.EmptyIfNull(), out _); using (tenantAccessor.PushContext(tenant)) - await tenantEvents.TenantDeactivatedAsync(new TenantDeactivatedEventArgs(tenant, scope, cancellationToken)); + await tenantEvents.TenantDeactivatedAsync(new(tenant, scope, cancellationToken)); } } } \ No newline at end of file diff --git a/src/modules/Elsa.Common/Multitenancy/Models/TenantFilter.cs b/src/modules/Elsa.Common/Multitenancy/Models/TenantFilter.cs index 1fc1c1fa1..51a88590a 100644 --- a/src/modules/Elsa.Common/Multitenancy/Models/TenantFilter.cs +++ b/src/modules/Elsa.Common/Multitenancy/Models/TenantFilter.cs @@ -11,7 +11,7 @@ public class TenantFilter /// /// Gets or sets the tenant ID to filter for. /// - public string? Id { get; set; } + public string Id { get; set; } = null!; /// /// Applies the filter to the specified queryable. @@ -20,7 +20,7 @@ public class TenantFilter /// The filtered queryable. public IQueryable Apply(IQueryable queryable) { - if (Id != null) queryable = queryable.Where(x => x.Id == Id); + queryable = queryable.Where(x => x.Id == Id); return queryable; } diff --git a/src/modules/Elsa.EntityFrameworkCore.Common/ElsaDbContextBase.cs b/src/modules/Elsa.EntityFrameworkCore.Common/ElsaDbContextBase.cs index 210f7d5a7..2a22f2a7d 100644 --- a/src/modules/Elsa.EntityFrameworkCore.Common/ElsaDbContextBase.cs +++ b/src/modules/Elsa.EntityFrameworkCore.Common/ElsaDbContextBase.cs @@ -1,4 +1,6 @@ using Elsa.Common.Entities; +using Elsa.Common.Multitenancy; +using Elsa.Extensions; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.Extensions.DependencyInjection; @@ -43,6 +45,12 @@ public abstract class ElsaDbContextBase : DbContext, IElsaDbContextSchema // ReSharper disable once VirtualMemberCallInConstructor Schema = !string.IsNullOrWhiteSpace(_elsaDbContextOptions?.SchemaName) ? _elsaDbContextOptions.SchemaName : ElsaSchema; + + var tenantAccessor = serviceProvider.GetService(); + var tenantId = tenantAccessor?.Tenant?.Id; + + if (!string.IsNullOrWhiteSpace(tenantId)) + TenantId = tenantId.NullIfEmpty(); } /// diff --git a/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/ApplyTenantId.cs b/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/ApplyTenantId.cs index 7167f46d6..b55f9af65 100644 --- a/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/ApplyTenantId.cs +++ b/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/ApplyTenantId.cs @@ -1,4 +1,5 @@ using Elsa.Common.Entities; +using Elsa.Extensions; using Microsoft.EntityFrameworkCore.ChangeTracking; namespace Elsa.EntityFrameworkCore.EntityHandlers; @@ -12,7 +13,7 @@ public class ApplyTenantId : IEntitySavingHandler public ValueTask HandleAsync(ElsaDbContextBase dbContext, EntityEntry entry, CancellationToken cancellationToken = default) { if (entry.Entity is Entity entity) - entity.TenantId = dbContext.TenantId; + entity.TenantId = dbContext.TenantId.NullIfEmpty(); return default; } diff --git a/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/SetTenantIdFilter.cs b/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/SetTenantIdFilter.cs index 0a352aabe..0e5bb9d9d 100644 --- a/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/SetTenantIdFilter.cs +++ b/src/modules/Elsa.EntityFrameworkCore.Common/EntityHandlers/SetTenantIdFilter.cs @@ -1,5 +1,6 @@ using System.Linq.Expressions; using Elsa.Common.Entities; +using Elsa.Extensions; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Query; @@ -17,8 +18,9 @@ public class SetTenantIdFilter : IEntityModelCreatingHandler if (!entityType.ClrType.IsAssignableTo(typeof(Entity))) return; + var tenantId = dbContext.TenantId.NullIfEmpty(); var parameter = Expression.Parameter(entityType.ClrType); - Expression> filterExpr = entity => dbContext.TenantId == entity.TenantId; + Expression> filterExpr = entity => entity.TenantId == tenantId; var body = ReplacingExpressionVisitor.Replace(filterExpr.Parameters[0], parameter, filterExpr.Body); var lambdaExpression = Expression.Lambda(body, parameter); diff --git a/src/modules/Elsa.Expressions/Helpers/ObjectConverter.cs b/src/modules/Elsa.Expressions/Helpers/ObjectConverter.cs index 0714ca238..698e1412c 100644 --- a/src/modules/Elsa.Expressions/Helpers/ObjectConverter.cs +++ b/src/modules/Elsa.Expressions/Helpers/ObjectConverter.cs @@ -27,7 +27,7 @@ public record ObjectConverterOptions(JsonSerializerOptions? SerializerOptions = /// public static class ObjectConverter { - public static bool StrictMode = true; // Set to false to revert to original flexible behavior. + public static bool StrictMode = false; // Set to true to opt into strict mode. /// /// Attempts to convert the source value into the destination type. diff --git a/src/modules/Elsa.Workflows.Runtime/Services/BookmarkQueueSignaler.cs b/src/modules/Elsa.Workflows.Runtime/Services/BookmarkQueueSignaler.cs index d97b76cd1..3ab6af9cc 100644 --- a/src/modules/Elsa.Workflows.Runtime/Services/BookmarkQueueSignaler.cs +++ b/src/modules/Elsa.Workflows.Runtime/Services/BookmarkQueueSignaler.cs @@ -37,7 +37,7 @@ public class BookmarkQueueSignaler : IBookmarkQueueSignaler lock (_lock) { // Reset the TCS for the next wait - _tcs = new TaskCompletionSource(); + _tcs = new(); } } } \ No newline at end of file