From 15fd780bb6aa0e84aef1f89a5ea24ceea7359894 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 27 Jan 2026 14:30:09 +0100 Subject: [PATCH 01/21] Enable multitenancy support and normalize tenant ID handling. - Activate multitenancy in `Program.cs`. - Introduce `NormalizeTenantId` method for consistent tenant ID usage. - Update tenant-related classes and features to support normalization logic. --- src/apps/Elsa.Server.Web/Program.cs | 6 ++- .../Contexts/TenantResolverContext.cs | 5 ++- .../Multitenancy/Entities/Tenant.cs | 9 ++++- .../Extensions/TenantsProviderExtensions.cs | 5 +++ .../Extensions/ModuleExtensions.cs | 38 +++++++++++-------- .../DefaultTenantResolverPipelineInvoker.cs | 5 ++- 6 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index 45c7a17931..835124979f 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -29,7 +29,7 @@ // ReSharper disable RedundantAssignment const bool useReadOnlyMode = false; const bool useSignalR = false; // Disabled until Elsa Studio sends authenticated requests. -const bool useMultitenancy = false; +const bool useMultitenancy = true; const bool disableVariableWrappers = false; ObjectConverter.StrictMode = true; @@ -118,6 +118,10 @@ http.ConfigureHttpOptions = options => configuration.GetSection("Http").Bind(options); http.UseCache(); }); + + if(useMultitenancy) + elsa.UseTenants(); + ConfigureForTest?.Invoke(elsa); }); diff --git a/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs b/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs index 72b4619fbf..eea5ab3d51 100644 --- a/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs +++ b/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs @@ -30,9 +30,10 @@ public TenantResolverContext(IDictionary tenants, CancellationTo /// /// The tenant ID. /// The found tenant or null if no tenant with the provided ID exists. - public Tenant? FindTenant(string tenantId) + public Tenant? FindTenant(string? tenantId) { - return _tenantsDictionary.TryGetValue(tenantId, out var tenant) ? tenant : null; + var normalizedId = tenantId.NormalizeTenantId(); + return _tenantsDictionary.TryGetValue(normalizedId, out var tenant) ? tenant : null; } /// diff --git a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs index cba4b371bc..c9884b4845 100644 --- a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs +++ b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs @@ -10,6 +10,11 @@ namespace Elsa.Common.Multitenancy; [UsedImplicitly] public class Tenant : Entity { + /// + /// The ID used for the default tenant. An empty string is used as dictionaries cannot have null keys. + /// + public const string DefaultTenantId = ""; + /// /// Gets or sets the name. /// @@ -19,10 +24,10 @@ public class Tenant : Entity /// Gets or sets the configuration. /// public IConfiguration Configuration { get; set; } = new ConfigurationBuilder().Build(); - + public static readonly Tenant Default = new() { - Id = null!, + Id = DefaultTenantId, Name = "Default" }; } diff --git a/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs b/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs index a60e1f9c35..0e5a66b217 100644 --- a/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs +++ b/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs @@ -5,6 +5,11 @@ namespace Elsa.Common.Multitenancy; [UsedImplicitly] public static class TenantsProviderExtensions { + /// + /// Normalizes a tenant ID by converting null to empty string, ensuring consistency with the default tenant convention. + /// + public static string NormalizeTenantId(this string? tenantId) => tenantId ?? Tenant.DefaultTenantId; + public static async Task FindByIdAsync(this ITenantsProvider tenantsProvider, string id, CancellationToken cancellationToken = default) { var filter = new TenantFilter diff --git a/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs b/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs index b8c03eaa0e..2c453baf7e 100644 --- a/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs +++ b/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs @@ -1,5 +1,6 @@ using Elsa.Features.Services; using Elsa.Tenants.Features; +using JetBrains.Annotations; // ReSharper disable once CheckNamespace namespace Elsa.Tenants.Extensions; @@ -7,32 +8,39 @@ namespace Elsa.Tenants.Extensions; /// /// Extensions for that installs the feature. /// +[UsedImplicitly] public static class ModuleExtensions { /// /// Installs and configures the feature. /// - public static IModule UseTenants(this IModule module, Action? configure = default) + [UsedImplicitly] + public static IModule UseTenants(this IModule module, Action? configure = null) { module.Configure(configure); return module; } - /// - /// Installs and configures the feature. - /// - public static TenantsFeature UseTenantManagementEndpoints(this TenantsFeature feature, Action? configure = default) + extension(TenantsFeature feature) { - feature.Module.Configure(configure); - return feature; - } + /// + /// Installs and configures the feature. + /// + [UsedImplicitly] + public TenantsFeature UseTenantManagementEndpoints(Action? configure = null) + { + feature.Module.Configure(configure); + return feature; + } - /// - /// Installs and configures the feature. - /// - public static TenantsFeature UseTenantManagement(this TenantsFeature feature, Action? configure = default) - { - feature.Module.Configure(configure); - return feature; + /// + /// Installs and configures the feature. + /// + [UsedImplicitly] + public TenantsFeature UseTenantManagement(Action? configure = null) + { + feature.Module.Configure(configure); + return feature; + } } } \ No newline at end of file diff --git a/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs b/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs index c0b832f329..14c7cf3f62 100644 --- a/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs +++ b/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs @@ -17,7 +17,8 @@ public class DefaultTenantResolverPipelineInvoker( public async Task InvokePipelineAsync(CancellationToken cancellationToken = default) { var resolutionPipeline = options.Value.TenantResolverPipelineBuilder.Build(serviceProvider); - var tenantsDictionary = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id); + var tenants = await tenantsProvider.ListAsync(cancellationToken); + var tenantsDictionary = tenants.ToDictionary(x => x.Id.NormalizeTenantId()); var context = new TenantResolverContext(tenantsDictionary, cancellationToken); foreach (var resolver in resolutionPipeline) @@ -26,7 +27,7 @@ public class DefaultTenantResolverPipelineInvoker( if (result.IsResolved) { - var resolvedTenantId = result.ResolveTenantId(); + var resolvedTenantId = result.ResolveTenantId().NormalizeTenantId(); if (tenantsDictionary.TryGetValue(resolvedTenantId, out var tenant)) return tenant; From 847a42725761dc383d7d0a0f88c93fa16fefb1c8 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 27 Jan 2026 14:32:44 +0100 Subject: [PATCH 02/21] Add ADR for adopting empty string as the default tenant ID - Standardized the tenant ID for the default tenant to use an empty string (`""`) instead of `null`. - Documented the rationale and migration considerations in ADR 0007. - Updated ADR table of contents and graph for new entry. --- .../0007-empty-string-as-default-tenant-id.md | 48 +++++++++++++++++++ doc/adr/graph.dot | 30 +++++++----- doc/adr/toc.md | 4 +- 3 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 doc/adr/0007-empty-string-as-default-tenant-id.md diff --git a/doc/adr/0007-empty-string-as-default-tenant-id.md b/doc/adr/0007-empty-string-as-default-tenant-id.md new file mode 100644 index 0000000000..e8b1ba0739 --- /dev/null +++ b/doc/adr/0007-empty-string-as-default-tenant-id.md @@ -0,0 +1,48 @@ +# 7. Empty String as Default Tenant ID + +Date: 2026-01-27 + +## Status + +Accepted + +## Context + +The multitenancy system in Elsa supports an optional mode where, when multitenancy is disabled, the system assumes a single tenant. When enabled, there's still a default tenant involved. The convention has been to use `null` as the tenant ID for the default tenant. + +However, this convention created several issues: + +1. **Dictionary compatibility**: The `DefaultTenantResolverPipelineInvoker` attempts to build a dictionary of tenants by their ID using `ToDictionary(x => x.Id)`, which throws an exception because dictionaries do not support null keys. +2. **Inconsistency**: The codebase used `null`, empty string (`""`), and string literal `"default"` interchangeably to refer to the default tenant across different parts of the system (e.g., in configuration files and database records). +3. **Code clarity**: Using `null` as a sentinel value for "default" is implicit and can be unclear to developers reading the code. + +## Decision + +We will standardize on using an **empty string** (`""`) as the tenant ID for the default tenant instead of `null`. This decision includes: + +1. **Define a constant**: Add `Tenant.DefaultTenantId = ""` to explicitly document the convention. +2. **Update Tenant.Default**: Change `Tenant.Default.Id` from `null!` to use the `DefaultTenantId` constant. +3. **Add normalization helper**: Create a `NormalizeTenantId()` extension method that converts `null` to empty string, ensuring backwards compatibility with code that still uses null. +4. **Apply normalization consistently**: Use the normalization method in: + - Dictionary creation in `DefaultTenantResolverPipelineInvoker` + - Tenant lookups in `TenantResolverContext` + - Any other places where tenant IDs are compared or used as dictionary keys + +## Consequences + +### Positive + +- **No more exceptions**: Empty string is a valid dictionary key, eliminating the runtime exception in `DefaultTenantResolverPipelineInvoker`. +- **Backwards compatible**: The `NormalizeTenantId()` extension method ensures that existing code using `null` or empty string will work correctly. +- **Explicit convention**: The `DefaultTenantId` constant makes the convention clear and self-documenting. +- **Simplified logic**: Reduces the need for null-checking throughout the multitenancy code. +- **Consistency**: Aligns with parts of the codebase that were already using empty string (e.g., in configuration files). + +### Negative + +- **Migration consideration**: Existing data stores that have `null` tenant IDs will need to be normalized to empty strings, though the normalization helper provides a runtime solution. +- **String vs null semantics**: Some developers may find using empty string less intuitive than null for representing "no tenant", though this is mitigated by the explicit constant. + +### Neutral + +- The empty string convention is common in multitenancy systems and aligns with string-based identifier patterns used elsewhere in the codebase. diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index 4818df2a23..3719a0849f 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -1,14 +1,20 @@ digraph { -node [shape=plaintext]; -subgraph { -_1 [label="1. Record architecture decisions"; URL="0001-record-architecture-decisions.html"]; -_2 [label="2. Fault Propagation from Child to Parent Activities"; URL="0002-fault-propagation-from-child-to-parent-activities.html"]; -_1 -> _2 [style="dotted", weight=1]; -_3 [label="3. Direct Bookmark Management in WorkflowExecutionContext"; URL="0003-direct-bookmark-management-in-workflowexecutioncontext.html"]; -_2 -> _3 [style="dotted", weight=1]; -_4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; -_3 -> _4 [style="dotted", weight=1]; -_5 [label="5. Tenant Deleted Event"; URL="0005-tenant-deleted-event.html"]; -_4 -> _5 [style="dotted", weight=1]; -} + node [shape=plaintext]; + subgraph { + _1 [label="1. Record architecture decisions"; URL="0001-record-architecture-decisions.html"]; + _2 [label="2. Fault Propagation from Child to Parent Activities"; URL="0002-fault-propagation-from-child-to-parent-activities.html"]; + _1 -> _2 [style="dotted", weight=1]; + _3 [label="3. Direct Bookmark Management in WorkflowExecutionContext"; URL="0003-direct-bookmark-management-in-workflowexecutioncontext.html"]; + _2 -> _3 [style="dotted", weight=1]; + _4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; + _3 -> _4 [style="dotted", weight=1]; + _4 [label="4. Token-Centric Flowchart Execution Model"; URL="0004-token-centric-flowchart-execution-model.html"]; + _3 -> _4 [style="dotted", weight=1]; + _5 [label="5. Tenant Deleted Event"; URL="0005-tenant-deleted-event.html"]; + _4 -> _5 [style="dotted", weight=1]; + _6 [label="6. Adoption of Explicit Merge Modes for Flowchart Joins"; URL="0006-adoption-of-explicit-merge-modes-for-flowchart-joins.html"]; + _5 -> _6 [style="dotted", weight=1]; + _7 [label="7. Empty String as Default Tenant ID"; URL="0007-empty-string-as-default-tenant-id.html"]; + _6 -> _7 [style="dotted", weight=1]; + } } \ No newline at end of file diff --git a/doc/adr/toc.md b/doc/adr/toc.md index 26b15683d9..8ca7b2f27e 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -4,4 +4,6 @@ * [2. Fault Propagation from Child to Parent Activities](0002-fault-propagation-from-child-to-parent-activities.md) * [3. Direct Bookmark Management in WorkflowExecutionContext](0003-direct-bookmark-management-in-workflowexecutioncontext.md) * [4. Activity Execution Snapshots](0004-activity-execution-snapshots.md) -* [5. Tenant Deleted Event](0005-tenant-deleted-event.md) \ No newline at end of file +* [5. Tenant Deleted Event](0005-tenant-deleted-event.md) +* [6. Adoption of Explicit Merge Modes for Flowchart Joins](0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md) +* [7. Empty String as Default Tenant ID](0007-empty-string-as-default-tenant-id.md) \ No newline at end of file From 07fe0e77426cdd29ed2aa82ee7acc63cdf436389 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:27:51 +0100 Subject: [PATCH 03/21] Apply suggestion from @sfmskywalker --- src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs index c9884b4845..6d3632d515 100644 --- a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs +++ b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs @@ -11,7 +11,7 @@ namespace Elsa.Common.Multitenancy; public class Tenant : Entity { /// - /// The ID used for the default tenant. An empty string is used as dictionaries cannot have null keys. + /// The ID used for the default tenant. /// public const string DefaultTenantId = ""; From e385234481e3bed05337b2ba71e8e9062c0e37c8 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:42:22 +0100 Subject: [PATCH 04/21] Update doc/adr/graph.dot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- doc/adr/graph.dot | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index 3719a0849f..11dbf0c37a 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -8,8 +8,8 @@ digraph { _2 -> _3 [style="dotted", weight=1]; _4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; _3 -> _4 [style="dotted", weight=1]; - _4 [label="4. Token-Centric Flowchart Execution Model"; URL="0004-token-centric-flowchart-execution-model.html"]; - _3 -> _4 [style="dotted", weight=1]; + _8 [label="4. Token-Centric Flowchart Execution Model"; URL="0004-token-centric-flowchart-execution-model.html"]; + _3 -> _8 [style="dotted", weight=1]; _5 [label="5. Tenant Deleted Event"; URL="0005-tenant-deleted-event.html"]; _4 -> _5 [style="dotted", weight=1]; _6 [label="6. Adoption of Explicit Merge Modes for Flowchart Joins"; URL="0006-adoption-of-explicit-merge-modes-for-flowchart-joins.html"]; From 62c9989cb3d9ccfeba82ffff4dfc355a3cefe251 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:43:40 +0100 Subject: [PATCH 05/21] Normalize spacing and improve readability in `Program.cs`. Fix multitenancy condition formatting. --- src/apps/Elsa.Server.Web/Program.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index 835124979f..d16503af39 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -118,10 +118,10 @@ http.ConfigureHttpOptions = options => configuration.GetSection("Http").Bind(options); http.UseCache(); }); - - if(useMultitenancy) + + if (useMultitenancy) elsa.UseTenants(); - + ConfigureForTest?.Invoke(elsa); }); From 0fe09192f02c383f3228372328a09d525a79d783 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:47:21 +0100 Subject: [PATCH 06/21] Fix ADR numbering and update TOC --- Elsa.sln | 6 +- ...token-centric-flowchart-execution-model.md | 83 ------- doc/adr/0005-tenant-deleted-event.md | 23 -- ...xplicit-merge-modes-for-flowchart-joins.md | 203 ------------------ .../0007-empty-string-as-default-tenant-id.md | 48 ----- doc/adr/graph.dot | 36 ++-- doc/adr/toc.md | 7 +- 7 files changed, 26 insertions(+), 380 deletions(-) delete mode 100644 doc/adr/0004-token-centric-flowchart-execution-model.md delete mode 100644 doc/adr/0005-tenant-deleted-event.md delete mode 100644 doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md delete mode 100644 doc/adr/0007-empty-string-as-default-tenant-id.md diff --git a/Elsa.sln b/Elsa.sln index 4827949841..549f42e5d9 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -196,12 +196,14 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{0A04B1FD-06C doc\adr\0001-record-architecture-decisions.md = doc\adr\0001-record-architecture-decisions.md doc\adr\0002-fault-propagation-from-child-to-parent-activities.md = doc\adr\0002-fault-propagation-from-child-to-parent-activities.md doc\adr\0003-direct-bookmark-management-in-workflowexecutioncontext.md = doc\adr\0003-direct-bookmark-management-in-workflowexecutioncontext.md - doc\adr\0004-token-centric-flowchart-execution-model.md = doc\adr\0004-token-centric-flowchart-execution-model.md doc\adr\graph.dot = doc\adr\graph.dot doc\adr\toc.md = doc\adr\toc.md doc\adr\0005-activity-execution-snapshots.md = doc\adr\0004-activity-execution-snapshots.md doc\adr\0006-tenant-deleted-event.md = doc\adr\0005-tenant-deleted-event.md - doc\adr\0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md = doc\adr\0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md + doc\adr\0005-token-centric-flowchart-execution-model.md = doc\adr\0005-token-centric-flowchart-execution-model.md + doc\adr\0006-tenant-deleted-event.md = doc\adr\0006-tenant-deleted-event.md + doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md = doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md + doc\adr\0008-empty-string-as-default-tenant-id.md = doc\adr\0008-empty-string-as-default-tenant-id.md EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bounty", "bounty", "{9B80A705-2E31-4012-964A-83963DCDB384}" diff --git a/doc/adr/0004-token-centric-flowchart-execution-model.md b/doc/adr/0004-token-centric-flowchart-execution-model.md deleted file mode 100644 index 4c01e65366..0000000000 --- a/doc/adr/0004-token-centric-flowchart-execution-model.md +++ /dev/null @@ -1,83 +0,0 @@ -# 4. Token-Centric Flowchart Execution Model - -Date: 2025-05-06 - -## Status - -Accepted - -## Context - -Elsa Workflows’ original flowchart used execution-count heuristics to drive joins, which fails in loops, XOR splits and resumable activities: - -- Loop-back edges never emit a “forward” token, stalling AND-joins. -- Counting executions across iterations causes premature or missed firings. -- Resumable activities (e.g. `Delay`) clear join state on resume. -- Users cannot declaratively control join semantics without deep framework hacks. - -We need a model that: - -1. Handles loops, forks, XORs and resumable activities reliably. -2. Lets designers choose per-activity join behavior. -3. Cleans up state to avoid memory leaks. -4. Supports cancellation of in-flight branches. - -## Decision - -Adopt a **token-centric** execution model with explicit **MergeMode** and **blocking**: - -1. **Tokens** - - On each activity completion, for each active outbound connection, emit a `Token` with: - - `FromActivityId`, `Outcome`, `ToActivityId`, - - Flags: `Consumed = false`, `Blocked = false`. - - Persist the list in `ActivityExecutionContext.Properties["Flowchart.Tokens"]`. - -2. **MergeMode** - - Query each target activity’s `MergeMode` via `GetMergeModeAsync(...)`. Supported values: - - **Race**: “first wins” - - **Stream**: “first wins, but don’t cancel ancestors” - - **Converge** (default): “wait for all” - - **Race** - 1. Cancel inbound ancestors (`CancelInboundAncestorsAsync`). - 2. If no existing blocked token for this inbound connection, schedule the target and then block all other inbound branches by emitting `Token.Block()` for each. - 3. Subsequent branches see their blocked token and simply consume it. - - **Stream** - - Same as Race except you do _not_ cancel inbound ancestors. - - **Converge** - - Wait until _every_ inbound connection for the target has at least one unblocked, unconsumed token. Then schedule once. - -3. **Scheduling Loop** - On each child completion: - - _Emit_ tokens for its outbound edges. - - _Consume_ any tokens whose `ToActivityId` matches the completed activity. - - _For each_ active outbound connection, inspect its target’s `MergeMode` and apply the rules above to decide whether to schedule it. - -4. **State Cleanup** - - After scheduling (or skipping) a target, remove any _consumed_ tokens whose `ToActivityId` equals the completed activity. - - When the flow has no pending work (`HasPendingWork()` is false), clear the entire token list and complete the flowchart. - - On activity cancellation (`OnTokenFlowActivityCanceledAsync`), remove _all_ tokens from or to that activity, then re-check for completion. - -## Sequence Diagram - -```mermaid -sequenceDiagram - participant A as Activity A - participant F as Flowchart - participant B as Activity B - - A-->>F: Completed(outcome="Done") - F->>F: Emit Token(A→B,Block=false,Consumed=false) - F->>F: Consume any inbound tokens for A - F->>F: Get B.MergeMode() - alt Race & first branch - F->>F: CancelInboundAncestors(B) - F-->>B: Schedule B - F->>F: Emit blocked Tokens for other inbound edges into B - else Race & later branch - F->>F: Consume blocked Token - else Converge until all arrived - Note over F: wait - end - F->>F: Purge consumed tokens for A - F-->>F: Complete if no pending work -``` \ No newline at end of file diff --git a/doc/adr/0005-tenant-deleted-event.md b/doc/adr/0005-tenant-deleted-event.md deleted file mode 100644 index d0d2fa9f13..0000000000 --- a/doc/adr/0005-tenant-deleted-event.md +++ /dev/null @@ -1,23 +0,0 @@ -# 5. Tenant Deleted Event - -Date: 2025-08-05 - -## Status - -Accepted - -## Context - -As outlined in issue [#6661](https://github.com/elsa-workflows/elsa-core/issues/6661), there is a need to differentiate between **Tenant Deactivating** and **Tenant Deleting** events. - -Currently, the `TenantDeactivated` event is used to unregister timer-based triggers. However, this causes the triggers to be unregistered when the application host shuts down, which is not the desired behavior. Instead, we want the triggers to remain registered until the tenant is explicitly deleted. - -## Decision - -To address this, we will introduce a new event called `TenantDeleted`. This event will be raised when a tenant is deleted and will be responsible for unregistering timer-based triggers. This ensures that the triggers remain active until the tenant is explicitly deleted. - -## Consequences - -- Timer-based triggers will no longer be unregistered during tenant deactivation. Instead, they will remain active until the tenant is deleted. -- The `TenantDeactivated` event will continue to be used for deactivating tenants without affecting the registration of timer-based triggers. -- The new `TenantDeleted` event will specifically handle the cleanup of resources associated with a tenant when it is deleted, ensuring a clear separation of responsibilities. diff --git a/doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md b/doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md deleted file mode 100644 index f6f279a50c..0000000000 --- a/doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md +++ /dev/null @@ -1,203 +0,0 @@ -# 6. Adoption of Explicit Merge Modes for Flowchart Joins - -Date: 2025-09-30 - -## Status - -Accepted - -## Context - -The Flowchart activity serves as a container for orchestrating workflows through activities connected via directed edges, using a token-based model for control flow. Initially, the execution logic relied on a combination of counter-based and token-based approaches, with implicit handling for merging paths (joins). However, this led to inconsistencies: - -- **Premature Scheduling in Forks**: In conditional forks with converges, untaken branches (e.g., false decision outcomes) allowed downstream activities to execute unexpectedly, violating expected blocking behavior. -- **Stalling in Loops**: Strict token checks for all inbounds broke loops by consuming entry tokens and failing to reschedule on backward connections. -- **Inconsistent Merges in Complex Flows**: In workflows with switches and multiple branches (e.g., MatchAny modes), dead paths (untaken defaults) caused hangs under strict rules but proceeded under approximations, leading to conflicting expectations. - -The root issue was the lack of explicit, configurable semantics for joins, relying instead on heuristics (e.g., inbound connection count >1). This made behavior opaque and error-prone, especially in unstructured flowcharts. Inspired by BPMN gateway semantics (e.g., AND-join for strict sync, OR-join for partial), we needed a clearer model to balance safety (blocking on required paths) and flexibility (proceeding on dead paths). - -## Decision - -We refine the Flowchart's token-based execution logic (`OnChildCompletedTokenBasedLogicAsync`) to use an explicit `MergeMode` enum on activities. This eliminates null/default fallbacks, making behaviors self-documenting and configurable via activity properties. - -- **MergeMode Enum Definition** (in `MergeMode.cs`): - ```csharp - namespace Elsa.Workflows.Activities.Flowchart.Models; - - /// - /// Specifies the strategy for handling multiple inbound execution paths in a workflow. - /// Uses flow-based terminology to describe merge behavior. - /// - public enum MergeMode - { - /// - /// Flows freely when possible, ignoring dead/untaken paths. - /// Opportunistic execution based on upstream completion. - /// - Stream, - - /// - /// Merges only the activated/flowing inbound branches. - /// Waits for all branches that received tokens, ignoring unactivated ones. - /// - Merge, - - /// - /// Converges all inbound paths, requiring every connection to execute. - /// Strictest mode - will block on dead/untaken paths. - /// - Converge, - - /// - /// Cascades execution for each arriving token independently. - /// Allows multiple concurrent executions (one per arriving token). - /// - Cascade, - - /// - /// Races inbound branches, executing on first arrival and blocking others. - /// - Race - } - ``` - -- **Key Changes in Flowchart Execution**: - - **Token Emission and Consumption**: On activity completion, emit tokens only for active outcomes (matching connections). Consume inbound tokens post-execution. - - **Scheduling Logic**: For each outbound connection, evaluate the target's `MergeMode` (via `GetMergeModeAsync`). Handle each mode explicitly in a switch statement. - - **Graph Reliance**: Use `FlowGraph` for forward inbound connections (acyclic); backward connections (e.g., loops) are handled naturally without inflating counts. - - **Dead Path Handling**: Varies by mode (strict blocking in Converge; approximation in None). - - **Loop Support**: Converge mode checks inbound count >1 to schedule immediately for sequentials/loops (<=1 forwards). - - **Cancellation and Purging**: Retained for races and overall cleanup. - -- **Implementation Snippet** (from `Flowchart` partial class; full code in PR): - ```csharp - switch (mergeMode) - { - case MergeMode.Cascade: - case MergeMode.Race: - // Schedule on arrival; for Race, block/cancel others. - // ... - break; - - case MergeMode.Merge: - // Wait for tokens from all forward inbound connections (activated branches only). - var inboundConnections = flowGraph.GetForwardInboundConnections(targetActivity); - if (inboundConnections.Count > 1) - { - var hasAllTokens = inboundConnections.All(inbound => /* token check */); - if (hasAllTokens) await flowContext.ScheduleActivityAsync(...); - } - else - { - await flowContext.ScheduleActivityAsync(...); - } - break; - - case MergeMode.Converge: - // Strictest mode: Wait for tokens from ALL inbound connections (forward + backward). - var allInboundConnections = flowGraph.GetInboundConnections(targetActivity); - if (allInboundConnections.Count > 1) - { - var hasAllTokens = allInboundConnections.All(inbound => /* token check */); - if (hasAllTokens) await flowContext.ScheduleActivityAsync(...); - } - else - { - await flowContext.ScheduleActivityAsync(...); - } - break; - - case MergeMode.Stream: - default: - // Flows freely - approximation that proceeds when upstream completes. - var inboundConnections = flowGraph.GetForwardInboundConnections(targetActivity); - var hasUnconsumed = inboundConnections.Any(inbound => /* source token check */); - if (!hasUnconsumed) await flowContext.ScheduleActivityAsync(...); - break; - } - ``` - -### Functional Overview -Flowchart execution starts with scheduling the root/start activity. As activities complete: -1. Emit tokens for matching outbound connections. -2. Consume the activity's inbound tokens. -3. For each emitted token's target: - - Fetch its `MergeMode`. - - Apply mode-specific logic to decide scheduling. -4. Purge consumed tokens and complete the flowchart if no pending work. - -This ensures acyclic forward flow with support for backward loops, using tokens to track control without global state beyond the list. - -### Merge Modes Explained -Each mode defines how tokens from multiple inbounds are synchronized using flow-based terminology: - -- **Stream (Flexible/Opportunistic Flow)**: - - **Behavior**: Flows freely when possible, ignoring dead/untaken paths. Schedules if there are no unconsumed tokens *to the sources* of inbounds (i.e., all upstream activities have completed, treating dead paths as "done"). - - **When to Use**: Flexible merges in unstructured flows; optional/exclusive branches (e.g., switch defaults) shouldn't block. - - **Scenarios**: - - **Forks with Untaken Paths**: Proceeds after active branches (e.g., in complex switch with dangling default). - - **Loops**: Schedules on loop-back tokens (backward ignored in forward inbounds). - - **Dead Paths**: Ignores untaken outcomes; no blocking. - - **Example**: In a switch with MatchAny, untaken default doesn't hang the merge. - -- **Merge (Activated Branches Synchronization)**: - - **Behavior**: Merges only activated/flowing inbound branches. Requires unconsumed, non-blocked tokens from *all* forward inbounds that received tokens. For <=1 forward, schedules immediately (loop/sequential friendly). - - **When to Use**: Synchronization points where only activated paths matter; block if any activated branch hasn't completed. - - **Scenarios**: - - **Conditional Forks**: Blocks downstream if decision returns false and subsequent activities are connected to the true branch, but only waits for activated branches. - - **Loops**: Works if forward inbounds <=1; reschedules on backward tokens. - - **Dead Paths**: Ignores untaken branches; waits only for activated ones. - - **Example**: Merge after parallel approvals—only proceed if all activated approval branches complete. - -- **Converge (Strictest - All Paths Required)**: - - **Behavior**: Converges ALL inbound paths, requiring every connection to execute (forward AND backward). Most strict mode. - - **When to Use**: When every single inbound path must execute before proceeding, regardless of activation status. - - **Scenarios**: - - **Strict Barriers**: Forces all possible paths to complete before proceeding. - - **Dead Paths**: Blocks on dead/untaken paths (desired for maximum safety). - - **Example**: Critical synchronization point requiring absolute completion of all defined paths. - -- **Cascade (Per-Token Execution)**: - - **Behavior**: Cascades execution for each arriving token independently; may allow multiple concurrent executions of the target. - - **When to Use**: Streaming scenarios where each branch should trigger separate processing (e.g., event streams). - - **Scenarios**: - - **Forks**: Executes target per branch. - - **Loops**: Executes per iteration. - - **Dead Paths**: Ignores; only active tokens trigger. - - **Example**: Logging each branch outcome separately. - -- **Race (First-Wins)**: - - **Behavior**: Races inbound branches; schedules on first token, blocks/cancels others (e.g., via blocked tokens and ancestor cancellation). - - **When to Use**: Racing conditions where first result wins (e.g., first response). - - **Scenarios**: - - **Forks**: Only first branch proceeds. - - **Loops**: May race iterations if concurrent. - - **Dead Paths**: First active wins; others blocked. - - **Example**: Waiting for fastest API response; cancel slower ones. - -### Handling Common Scenarios - -- **Simple Sequential**: Any mode schedules on token arrival (single inbound). -- **Fork-Join with Condition**: Merge waits for activated branches; Converge blocks on all paths; Stream proceeds opportunistically. -- **Looping Construct**: All modes work; Merge and Converge use count check to avoid strictness on single inbound. -- **Switch with Dangling Branches**: Stream ignores untaken; Merge waits for activated; Converge blocks on all. -- **BPMN Alignment**: Stream ≈ XOR/OR-join (flexible); Merge ≈ AND-join for active paths; Converge ≈ strict AND-join; Race ≈ Event-based; Cascade ≈ parallel multi-instance. - -## Consequences - -- **Positive**: - - Clearer semantics: Explicit modes reduce bugs and improve workflow design. - - Flexibility: Users choose behavior per activity. - - Reliability: Fixes identified flaws across forks, loops, and complexes. - - Extensibility: Enum can grow (e.g., for BPMN Complex). - -- **Negative**: - - Complexity: More modes mean more testing; document well. - - Performance: Token checks add minor overhead (optimize with caching). - -## Alternatives Considered - -- **Heuristics-Only**: Relied on inbound count/graph structure—too brittle, led to conflicts. -- **Full BPMN Gateways**: Dedicated activities per type (e.g., ParallelGateway)—overkill for Elsa's simplicity; would require major refactor. -- **Dead Path Propagation**: Emit blocked tokens on untaken paths—adds complexity; deferred for future if needed (e.g., for OR-join). -- **Counter-Based Fallback**: Retained old logic—deprecated for token purity. \ No newline at end of file diff --git a/doc/adr/0007-empty-string-as-default-tenant-id.md b/doc/adr/0007-empty-string-as-default-tenant-id.md deleted file mode 100644 index e8b1ba0739..0000000000 --- a/doc/adr/0007-empty-string-as-default-tenant-id.md +++ /dev/null @@ -1,48 +0,0 @@ -# 7. Empty String as Default Tenant ID - -Date: 2026-01-27 - -## Status - -Accepted - -## Context - -The multitenancy system in Elsa supports an optional mode where, when multitenancy is disabled, the system assumes a single tenant. When enabled, there's still a default tenant involved. The convention has been to use `null` as the tenant ID for the default tenant. - -However, this convention created several issues: - -1. **Dictionary compatibility**: The `DefaultTenantResolverPipelineInvoker` attempts to build a dictionary of tenants by their ID using `ToDictionary(x => x.Id)`, which throws an exception because dictionaries do not support null keys. -2. **Inconsistency**: The codebase used `null`, empty string (`""`), and string literal `"default"` interchangeably to refer to the default tenant across different parts of the system (e.g., in configuration files and database records). -3. **Code clarity**: Using `null` as a sentinel value for "default" is implicit and can be unclear to developers reading the code. - -## Decision - -We will standardize on using an **empty string** (`""`) as the tenant ID for the default tenant instead of `null`. This decision includes: - -1. **Define a constant**: Add `Tenant.DefaultTenantId = ""` to explicitly document the convention. -2. **Update Tenant.Default**: Change `Tenant.Default.Id` from `null!` to use the `DefaultTenantId` constant. -3. **Add normalization helper**: Create a `NormalizeTenantId()` extension method that converts `null` to empty string, ensuring backwards compatibility with code that still uses null. -4. **Apply normalization consistently**: Use the normalization method in: - - Dictionary creation in `DefaultTenantResolverPipelineInvoker` - - Tenant lookups in `TenantResolverContext` - - Any other places where tenant IDs are compared or used as dictionary keys - -## Consequences - -### Positive - -- **No more exceptions**: Empty string is a valid dictionary key, eliminating the runtime exception in `DefaultTenantResolverPipelineInvoker`. -- **Backwards compatible**: The `NormalizeTenantId()` extension method ensures that existing code using `null` or empty string will work correctly. -- **Explicit convention**: The `DefaultTenantId` constant makes the convention clear and self-documenting. -- **Simplified logic**: Reduces the need for null-checking throughout the multitenancy code. -- **Consistency**: Aligns with parts of the codebase that were already using empty string (e.g., in configuration files). - -### Negative - -- **Migration consideration**: Existing data stores that have `null` tenant IDs will need to be normalized to empty strings, though the normalization helper provides a runtime solution. -- **String vs null semantics**: Some developers may find using empty string less intuitive than null for representing "no tenant", though this is mitigated by the explicit constant. - -### Neutral - -- The empty string convention is common in multitenancy systems and aligns with string-based identifier patterns used elsewhere in the codebase. diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index 11dbf0c37a..b659a835a3 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -1,20 +1,20 @@ digraph { - node [shape=plaintext]; - subgraph { - _1 [label="1. Record architecture decisions"; URL="0001-record-architecture-decisions.html"]; - _2 [label="2. Fault Propagation from Child to Parent Activities"; URL="0002-fault-propagation-from-child-to-parent-activities.html"]; - _1 -> _2 [style="dotted", weight=1]; - _3 [label="3. Direct Bookmark Management in WorkflowExecutionContext"; URL="0003-direct-bookmark-management-in-workflowexecutioncontext.html"]; - _2 -> _3 [style="dotted", weight=1]; - _4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; - _3 -> _4 [style="dotted", weight=1]; - _8 [label="4. Token-Centric Flowchart Execution Model"; URL="0004-token-centric-flowchart-execution-model.html"]; - _3 -> _8 [style="dotted", weight=1]; - _5 [label="5. Tenant Deleted Event"; URL="0005-tenant-deleted-event.html"]; - _4 -> _5 [style="dotted", weight=1]; - _6 [label="6. Adoption of Explicit Merge Modes for Flowchart Joins"; URL="0006-adoption-of-explicit-merge-modes-for-flowchart-joins.html"]; - _5 -> _6 [style="dotted", weight=1]; - _7 [label="7. Empty String as Default Tenant ID"; URL="0007-empty-string-as-default-tenant-id.html"]; - _6 -> _7 [style="dotted", weight=1]; - } +node [shape=plaintext]; +subgraph { +_1 [label="1. Record architecture decisions"; URL="0001-record-architecture-decisions.html"]; +_2 [label="2. Fault Propagation from Child to Parent Activities"; URL="0002-fault-propagation-from-child-to-parent-activities.html"]; +_1 -> _2 [style="dotted", weight=1]; +_3 [label="3. Direct Bookmark Management in WorkflowExecutionContext"; URL="0003-direct-bookmark-management-in-workflowexecutioncontext.html"]; +_2 -> _3 [style="dotted", weight=1]; +_4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; +_3 -> _4 [style="dotted", weight=1]; +_5 [label="5. Token-Centric Flowchart Execution Model"; URL="0005-token-centric-flowchart-execution-model.html"]; +_4 -> _5 [style="dotted", weight=1]; +_6 [label="6. Tenant Deleted Event"; URL="0006-tenant-deleted-event.html"]; +_5 -> _6 [style="dotted", weight=1]; +_7 [label="7. Adoption of Explicit Merge Modes for Flowchart Joins"; URL="0007-adoption-of-explicit-merge-modes-for-flowchart-joins.html"]; +_6 -> _7 [style="dotted", weight=1]; +_8 [label="8. Empty String as Default Tenant ID"; URL="0008-empty-string-as-default-tenant-id.html"]; +_7 -> _8 [style="dotted", weight=1]; +} } \ No newline at end of file diff --git a/doc/adr/toc.md b/doc/adr/toc.md index 8ca7b2f27e..cb3c2a4364 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -4,6 +4,7 @@ * [2. Fault Propagation from Child to Parent Activities](0002-fault-propagation-from-child-to-parent-activities.md) * [3. Direct Bookmark Management in WorkflowExecutionContext](0003-direct-bookmark-management-in-workflowexecutioncontext.md) * [4. Activity Execution Snapshots](0004-activity-execution-snapshots.md) -* [5. Tenant Deleted Event](0005-tenant-deleted-event.md) -* [6. Adoption of Explicit Merge Modes for Flowchart Joins](0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md) -* [7. Empty String as Default Tenant ID](0007-empty-string-as-default-tenant-id.md) \ No newline at end of file +* [5. Token-Centric Flowchart Execution Model](0005-token-centric-flowchart-execution-model.md) +* [6. Tenant Deleted Event](0006-tenant-deleted-event.md) +* [7. Adoption of Explicit Merge Modes for Flowchart Joins](0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md) +* [8. Empty String as Default Tenant ID](0008-empty-string-as-default-tenant-id.md)string-as-default-tenant-id.md) \ No newline at end of file From f027e055be7f86c9f61f6e857c8de7695acb6007 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:47:35 +0100 Subject: [PATCH 07/21] Add ADRs for flowchart execution model, tenant deletion event, merge modes, and default tenant ID - Introduced ADR 0005: Token-centric flowchart execution model for improved loop and join handling. - Added ADR 0006: Tenant Deleted event for distinct handling of tenant removal. - Documented ADR 0007: Explicit merge modes for flowchart joins, improving reliability and configurability. - Included ADR 0008: Standardization of empty string as the default tenant ID for consistency and clarity. --- ...token-centric-flowchart-execution-model.md | 83 +++++++ doc/adr/0006-tenant-deleted-event.md | 23 ++ ...xplicit-merge-modes-for-flowchart-joins.md | 203 ++++++++++++++++++ .../0008-empty-string-as-default-tenant-id.md | 48 +++++ 4 files changed, 357 insertions(+) create mode 100644 doc/adr/0005-token-centric-flowchart-execution-model.md create mode 100644 doc/adr/0006-tenant-deleted-event.md create mode 100644 doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md create mode 100644 doc/adr/0008-empty-string-as-default-tenant-id.md diff --git a/doc/adr/0005-token-centric-flowchart-execution-model.md b/doc/adr/0005-token-centric-flowchart-execution-model.md new file mode 100644 index 0000000000..c633c4bb6f --- /dev/null +++ b/doc/adr/0005-token-centric-flowchart-execution-model.md @@ -0,0 +1,83 @@ +# 5. Token-Centric Flowchart Execution Model + +Date: 2025-05-06 + +## Status + +Accepted + +## Context + +Elsa Workflows’ original flowchart used execution-count heuristics to drive joins, which fails in loops, XOR splits and resumable activities: + +- Loop-back edges never emit a “forward” token, stalling AND-joins. +- Counting executions across iterations causes premature or missed firings. +- Resumable activities (e.g. `Delay`) clear join state on resume. +- Users cannot declaratively control join semantics without deep framework hacks. + +We need a model that: + +1. Handles loops, forks, XORs and resumable activities reliably. +2. Lets designers choose per-activity join behavior. +3. Cleans up state to avoid memory leaks. +4. Supports cancellation of in-flight branches. + +## Decision + +Adopt a **token-centric** execution model with explicit **MergeMode** and **blocking**: + +1. **Tokens** + - On each activity completion, for each active outbound connection, emit a `Token` with: + - `FromActivityId`, `Outcome`, `ToActivityId`, + - Flags: `Consumed = false`, `Blocked = false`. + - Persist the list in `ActivityExecutionContext.Properties["Flowchart.Tokens"]`. + +2. **MergeMode** + - Query each target activity’s `MergeMode` via `GetMergeModeAsync(...)`. Supported values: + - **Race**: “first wins” + - **Stream**: “first wins, but don’t cancel ancestors” + - **Converge** (default): “wait for all” + - **Race** + 1. Cancel inbound ancestors (`CancelInboundAncestorsAsync`). + 2. If no existing blocked token for this inbound connection, schedule the target and then block all other inbound branches by emitting `Token.Block()` for each. + 3. Subsequent branches see their blocked token and simply consume it. + - **Stream** + - Same as Race except you do _not_ cancel inbound ancestors. + - **Converge** + - Wait until _every_ inbound connection for the target has at least one unblocked, unconsumed token. Then schedule once. + +3. **Scheduling Loop** + On each child completion: + - _Emit_ tokens for its outbound edges. + - _Consume_ any tokens whose `ToActivityId` matches the completed activity. + - _For each_ active outbound connection, inspect its target’s `MergeMode` and apply the rules above to decide whether to schedule it. + +4. **State Cleanup** + - After scheduling (or skipping) a target, remove any _consumed_ tokens whose `ToActivityId` equals the completed activity. + - When the flow has no pending work (`HasPendingWork()` is false), clear the entire token list and complete the flowchart. + - On activity cancellation (`OnTokenFlowActivityCanceledAsync`), remove _all_ tokens from or to that activity, then re-check for completion. + +## Sequence Diagram + +```mermaid +sequenceDiagram + participant A as Activity A + participant F as Flowchart + participant B as Activity B + + A-->>F: Completed(outcome="Done") + F->>F: Emit Token(A→B,Block=false,Consumed=false) + F->>F: Consume any inbound tokens for A + F->>F: Get B.MergeMode() + alt Race & first branch + F->>F: CancelInboundAncestors(B) + F-->>B: Schedule B + F->>F: Emit blocked Tokens for other inbound edges into B + else Race & later branch + F->>F: Consume blocked Token + else Converge until all arrived + Note over F: wait + end + F->>F: Purge consumed tokens for A + F-->>F: Complete if no pending work +``` \ No newline at end of file diff --git a/doc/adr/0006-tenant-deleted-event.md b/doc/adr/0006-tenant-deleted-event.md new file mode 100644 index 0000000000..0fd7a8d244 --- /dev/null +++ b/doc/adr/0006-tenant-deleted-event.md @@ -0,0 +1,23 @@ +# 6. Tenant Deleted Event + +Date: 2025-08-05 + +## Status + +Accepted + +## Context + +As outlined in issue [#6661](https://github.com/elsa-workflows/elsa-core/issues/6661), there is a need to differentiate between **Tenant Deactivating** and **Tenant Deleting** events. + +Currently, the `TenantDeactivated` event is used to unregister timer-based triggers. However, this causes the triggers to be unregistered when the application host shuts down, which is not the desired behavior. Instead, we want the triggers to remain registered until the tenant is explicitly deleted. + +## Decision + +To address this, we will introduce a new event called `TenantDeleted`. This event will be raised when a tenant is deleted and will be responsible for unregistering timer-based triggers. This ensures that the triggers remain active until the tenant is explicitly deleted. + +## Consequences + +- Timer-based triggers will no longer be unregistered during tenant deactivation. Instead, they will remain active until the tenant is deleted. +- The `TenantDeactivated` event will continue to be used for deactivating tenants without affecting the registration of timer-based triggers. +- The new `TenantDeleted` event will specifically handle the cleanup of resources associated with a tenant when it is deleted, ensuring a clear separation of responsibilities. diff --git a/doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md b/doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md new file mode 100644 index 0000000000..66ece0e875 --- /dev/null +++ b/doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md @@ -0,0 +1,203 @@ +# 7. Adoption of Explicit Merge Modes for Flowchart Joins + +Date: 2025-09-30 + +## Status + +Accepted + +## Context + +The Flowchart activity serves as a container for orchestrating workflows through activities connected via directed edges, using a token-based model for control flow. Initially, the execution logic relied on a combination of counter-based and token-based approaches, with implicit handling for merging paths (joins). However, this led to inconsistencies: + +- **Premature Scheduling in Forks**: In conditional forks with converges, untaken branches (e.g., false decision outcomes) allowed downstream activities to execute unexpectedly, violating expected blocking behavior. +- **Stalling in Loops**: Strict token checks for all inbounds broke loops by consuming entry tokens and failing to reschedule on backward connections. +- **Inconsistent Merges in Complex Flows**: In workflows with switches and multiple branches (e.g., MatchAny modes), dead paths (untaken defaults) caused hangs under strict rules but proceeded under approximations, leading to conflicting expectations. + +The root issue was the lack of explicit, configurable semantics for joins, relying instead on heuristics (e.g., inbound connection count >1). This made behavior opaque and error-prone, especially in unstructured flowcharts. Inspired by BPMN gateway semantics (e.g., AND-join for strict sync, OR-join for partial), we needed a clearer model to balance safety (blocking on required paths) and flexibility (proceeding on dead paths). + +## Decision + +We refine the Flowchart's token-based execution logic (`OnChildCompletedTokenBasedLogicAsync`) to use an explicit `MergeMode` enum on activities. This eliminates null/default fallbacks, making behaviors self-documenting and configurable via activity properties. + +- **MergeMode Enum Definition** (in `MergeMode.cs`): + ```csharp + namespace Elsa.Workflows.Activities.Flowchart.Models; + + /// + /// Specifies the strategy for handling multiple inbound execution paths in a workflow. + /// Uses flow-based terminology to describe merge behavior. + /// + public enum MergeMode + { + /// + /// Flows freely when possible, ignoring dead/untaken paths. + /// Opportunistic execution based on upstream completion. + /// + Stream, + + /// + /// Merges only the activated/flowing inbound branches. + /// Waits for all branches that received tokens, ignoring unactivated ones. + /// + Merge, + + /// + /// Converges all inbound paths, requiring every connection to execute. + /// Strictest mode - will block on dead/untaken paths. + /// + Converge, + + /// + /// Cascades execution for each arriving token independently. + /// Allows multiple concurrent executions (one per arriving token). + /// + Cascade, + + /// + /// Races inbound branches, executing on first arrival and blocking others. + /// + Race + } + ``` + +- **Key Changes in Flowchart Execution**: + - **Token Emission and Consumption**: On activity completion, emit tokens only for active outcomes (matching connections). Consume inbound tokens post-execution. + - **Scheduling Logic**: For each outbound connection, evaluate the target's `MergeMode` (via `GetMergeModeAsync`). Handle each mode explicitly in a switch statement. + - **Graph Reliance**: Use `FlowGraph` for forward inbound connections (acyclic); backward connections (e.g., loops) are handled naturally without inflating counts. + - **Dead Path Handling**: Varies by mode (strict blocking in Converge; approximation in None). + - **Loop Support**: Converge mode checks inbound count >1 to schedule immediately for sequentials/loops (<=1 forwards). + - **Cancellation and Purging**: Retained for races and overall cleanup. + +- **Implementation Snippet** (from `Flowchart` partial class; full code in PR): + ```csharp + switch (mergeMode) + { + case MergeMode.Cascade: + case MergeMode.Race: + // Schedule on arrival; for Race, block/cancel others. + // ... + break; + + case MergeMode.Merge: + // Wait for tokens from all forward inbound connections (activated branches only). + var inboundConnections = flowGraph.GetForwardInboundConnections(targetActivity); + if (inboundConnections.Count > 1) + { + var hasAllTokens = inboundConnections.All(inbound => /* token check */); + if (hasAllTokens) await flowContext.ScheduleActivityAsync(...); + } + else + { + await flowContext.ScheduleActivityAsync(...); + } + break; + + case MergeMode.Converge: + // Strictest mode: Wait for tokens from ALL inbound connections (forward + backward). + var allInboundConnections = flowGraph.GetInboundConnections(targetActivity); + if (allInboundConnections.Count > 1) + { + var hasAllTokens = allInboundConnections.All(inbound => /* token check */); + if (hasAllTokens) await flowContext.ScheduleActivityAsync(...); + } + else + { + await flowContext.ScheduleActivityAsync(...); + } + break; + + case MergeMode.Stream: + default: + // Flows freely - approximation that proceeds when upstream completes. + var inboundConnections = flowGraph.GetForwardInboundConnections(targetActivity); + var hasUnconsumed = inboundConnections.Any(inbound => /* source token check */); + if (!hasUnconsumed) await flowContext.ScheduleActivityAsync(...); + break; + } + ``` + +### Functional Overview +Flowchart execution starts with scheduling the root/start activity. As activities complete: +1. Emit tokens for matching outbound connections. +2. Consume the activity's inbound tokens. +3. For each emitted token's target: + - Fetch its `MergeMode`. + - Apply mode-specific logic to decide scheduling. +4. Purge consumed tokens and complete the flowchart if no pending work. + +This ensures acyclic forward flow with support for backward loops, using tokens to track control without global state beyond the list. + +### Merge Modes Explained +Each mode defines how tokens from multiple inbounds are synchronized using flow-based terminology: + +- **Stream (Flexible/Opportunistic Flow)**: + - **Behavior**: Flows freely when possible, ignoring dead/untaken paths. Schedules if there are no unconsumed tokens *to the sources* of inbounds (i.e., all upstream activities have completed, treating dead paths as "done"). + - **When to Use**: Flexible merges in unstructured flows; optional/exclusive branches (e.g., switch defaults) shouldn't block. + - **Scenarios**: + - **Forks with Untaken Paths**: Proceeds after active branches (e.g., in complex switch with dangling default). + - **Loops**: Schedules on loop-back tokens (backward ignored in forward inbounds). + - **Dead Paths**: Ignores untaken outcomes; no blocking. + - **Example**: In a switch with MatchAny, untaken default doesn't hang the merge. + +- **Merge (Activated Branches Synchronization)**: + - **Behavior**: Merges only activated/flowing inbound branches. Requires unconsumed, non-blocked tokens from *all* forward inbounds that received tokens. For <=1 forward, schedules immediately (loop/sequential friendly). + - **When to Use**: Synchronization points where only activated paths matter; block if any activated branch hasn't completed. + - **Scenarios**: + - **Conditional Forks**: Blocks downstream if decision returns false and subsequent activities are connected to the true branch, but only waits for activated branches. + - **Loops**: Works if forward inbounds <=1; reschedules on backward tokens. + - **Dead Paths**: Ignores untaken branches; waits only for activated ones. + - **Example**: Merge after parallel approvals—only proceed if all activated approval branches complete. + +- **Converge (Strictest - All Paths Required)**: + - **Behavior**: Converges ALL inbound paths, requiring every connection to execute (forward AND backward). Most strict mode. + - **When to Use**: When every single inbound path must execute before proceeding, regardless of activation status. + - **Scenarios**: + - **Strict Barriers**: Forces all possible paths to complete before proceeding. + - **Dead Paths**: Blocks on dead/untaken paths (desired for maximum safety). + - **Example**: Critical synchronization point requiring absolute completion of all defined paths. + +- **Cascade (Per-Token Execution)**: + - **Behavior**: Cascades execution for each arriving token independently; may allow multiple concurrent executions of the target. + - **When to Use**: Streaming scenarios where each branch should trigger separate processing (e.g., event streams). + - **Scenarios**: + - **Forks**: Executes target per branch. + - **Loops**: Executes per iteration. + - **Dead Paths**: Ignores; only active tokens trigger. + - **Example**: Logging each branch outcome separately. + +- **Race (First-Wins)**: + - **Behavior**: Races inbound branches; schedules on first token, blocks/cancels others (e.g., via blocked tokens and ancestor cancellation). + - **When to Use**: Racing conditions where first result wins (e.g., first response). + - **Scenarios**: + - **Forks**: Only first branch proceeds. + - **Loops**: May race iterations if concurrent. + - **Dead Paths**: First active wins; others blocked. + - **Example**: Waiting for fastest API response; cancel slower ones. + +### Handling Common Scenarios + +- **Simple Sequential**: Any mode schedules on token arrival (single inbound). +- **Fork-Join with Condition**: Merge waits for activated branches; Converge blocks on all paths; Stream proceeds opportunistically. +- **Looping Construct**: All modes work; Merge and Converge use count check to avoid strictness on single inbound. +- **Switch with Dangling Branches**: Stream ignores untaken; Merge waits for activated; Converge blocks on all. +- **BPMN Alignment**: Stream ≈ XOR/OR-join (flexible); Merge ≈ AND-join for active paths; Converge ≈ strict AND-join; Race ≈ Event-based; Cascade ≈ parallel multi-instance. + +## Consequences + +- **Positive**: + - Clearer semantics: Explicit modes reduce bugs and improve workflow design. + - Flexibility: Users choose behavior per activity. + - Reliability: Fixes identified flaws across forks, loops, and complexes. + - Extensibility: Enum can grow (e.g., for BPMN Complex). + +- **Negative**: + - Complexity: More modes mean more testing; document well. + - Performance: Token checks add minor overhead (optimize with caching). + +## Alternatives Considered + +- **Heuristics-Only**: Relied on inbound count/graph structure—too brittle, led to conflicts. +- **Full BPMN Gateways**: Dedicated activities per type (e.g., ParallelGateway)—overkill for Elsa's simplicity; would require major refactor. +- **Dead Path Propagation**: Emit blocked tokens on untaken paths—adds complexity; deferred for future if needed (e.g., for OR-join). +- **Counter-Based Fallback**: Retained old logic—deprecated for token purity. \ No newline at end of file diff --git a/doc/adr/0008-empty-string-as-default-tenant-id.md b/doc/adr/0008-empty-string-as-default-tenant-id.md new file mode 100644 index 0000000000..fedbedd36d --- /dev/null +++ b/doc/adr/0008-empty-string-as-default-tenant-id.md @@ -0,0 +1,48 @@ +# 8. Empty String as Default Tenant ID + +Date: 2026-01-27 + +## Status + +Accepted + +## Context + +The multitenancy system in Elsa supports an optional mode where, when multitenancy is disabled, the system assumes a single tenant. When enabled, there's still a default tenant involved. The convention has been to use `null` as the tenant ID for the default tenant. + +However, this convention created several issues: + +1. **Dictionary compatibility**: The `DefaultTenantResolverPipelineInvoker` attempts to build a dictionary of tenants by their ID using `ToDictionary(x => x.Id)`, which throws an exception because dictionaries do not support null keys. +2. **Inconsistency**: The codebase used `null`, empty string (`""`), and string literal `"default"` interchangeably to refer to the default tenant across different parts of the system (e.g., in configuration files and database records). +3. **Code clarity**: Using `null` as a sentinel value for "default" is implicit and can be unclear to developers reading the code. + +## Decision + +We will standardize on using an **empty string** (`""`) as the tenant ID for the default tenant instead of `null`. This decision includes: + +1. **Define a constant**: Add `Tenant.DefaultTenantId = ""` to explicitly document the convention. +2. **Update Tenant.Default**: Change `Tenant.Default.Id` from `null!` to use the `DefaultTenantId` constant. +3. **Add normalization helper**: Create a `NormalizeTenantId()` extension method that converts `null` to empty string, ensuring backwards compatibility with code that still uses null. +4. **Apply normalization consistently**: Use the normalization method in: + - Dictionary creation in `DefaultTenantResolverPipelineInvoker` + - Tenant lookups in `TenantResolverContext` + - Any other places where tenant IDs are compared or used as dictionary keys + +## Consequences + +### Positive + +- **No more exceptions**: Empty string is a valid dictionary key, eliminating the runtime exception in `DefaultTenantResolverPipelineInvoker`. +- **Backwards compatible**: The `NormalizeTenantId()` extension method ensures that existing code using `null` or empty string will work correctly. +- **Explicit convention**: The `DefaultTenantId` constant makes the convention clear and self-documenting. +- **Simplified logic**: Reduces the need for null-checking throughout the multitenancy code. +- **Consistency**: Aligns with parts of the codebase that were already using empty string (e.g., in configuration files). + +### Negative + +- **Migration consideration**: Existing data stores that have `null` tenant IDs will need to be normalized to empty strings, though the normalization helper provides a runtime solution. +- **String vs null semantics**: Some developers may find using empty string less intuitive than null for representing "no tenant", though this is mitigated by the explicit constant. + +### Neutral + +- The empty string convention is common in multitenancy systems and aligns with string-based identifier patterns used elsewhere in the codebase. From 1c92763c9b093ca751f5524ccc9b0cbb63ff5140 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 18:51:46 +0100 Subject: [PATCH 08/21] Add unit tests for tenant ID normalization and multitenancy pipeline invoker - Added comprehensive unit tests for tenant ID normalization to ensure consistent handling of null, empty, and valid IDs. - Introduced tests for the multitenancy pipeline invoker covering various tenant resolution scenarios. - Updated solution to include new unit testing projects for `Elsa.Tenants` and `Elsa.Common`. --- Elsa.sln | 7 + .../Multitenancy/MultitenancyTests.cs | 105 +++++++++- .../TenantIdNormalizationTests.cs | 49 +++++ .../TenantResolverContextTests.cs | 155 +++++++++++++++ .../Elsa.Tenants.UnitTests.csproj | 14 ++ ...faultTenantResolverPipelineInvokerTests.cs | 181 ++++++++++++++++++ .../Models/ActivityConstructionResultTests.cs | 116 +++++++++++ 7 files changed, 623 insertions(+), 4 deletions(-) create mode 100644 test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs create mode 100644 test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs create mode 100644 test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj create mode 100644 test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs diff --git a/Elsa.sln b/Elsa.sln index 549f42e5d9..b68e81eecd 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -329,6 +329,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Resilience.Core.UnitTe EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Common.UnitTests", "test\unit\Elsa.Common.UnitTests\Elsa.Common.UnitTests.csproj", "{A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Tenants.UnitTests", "test\unit\Elsa.Tenants.UnitTests\Elsa.Tenants.UnitTests.csproj", "{DC476900-D836-4920-A696-CF8796668723}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -593,6 +595,10 @@ Global {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Debug|Any CPU.Build.0 = Debug|Any CPU {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Release|Any CPU.ActiveCfg = Release|Any CPU {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Release|Any CPU.Build.0 = Release|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Debug|Any CPU.Build.0 = Debug|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Release|Any CPU.ActiveCfg = Release|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -696,6 +702,7 @@ Global {874F5A44-DB06-47AB-A18C-2D13942E0147} = {477C2416-312D-46AE-BCD6-8FA1FAB43624} {B8006D70-1630-43DB-A043-FA89FAC70F37} = {18453B51-25EB-4317-A4B3-B10518252E92} {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC} = {18453B51-25EB-4317-A4B3-B10518252E92} + {DC476900-D836-4920-A696-CF8796668723} = {18453B51-25EB-4317-A4B3-B10518252E92} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {D4B5CEAA-7D70-4FCB-A68E-B03FBE5E0E5E} diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs index 06b5d600c7..838ed8bbad 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs @@ -1,4 +1,5 @@ using Elsa.Common.Models; +using Elsa.Common.Multitenancy; using Elsa.Workflows.ComponentTests.Abstractions; using Elsa.Workflows.ComponentTests.Fixtures; using Elsa.Workflows.Management; @@ -7,18 +8,114 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.Multitenancy; +/// +/// Tests for multitenancy tenant ID normalization. +/// public class MultitenancyTests(App app) : AppComponentTest(app) { - [Fact(Skip = "Multitenancy disabled. This test doesn't work because not all workflows are assigned the Tenant1 tenant.")] - public async Task LoadingWorkflows_ShouldReturnWorkflows_FromCurrentTenant() + [Fact] + public void DefaultTenant_ShouldUseEmptyStringAsId() { + // Assert + Assert.Equal(string.Empty, Tenant.DefaultTenantId); + Assert.Equal(Tenant.DefaultTenantId, Tenant.Default.Id); + } + + [Fact] + public void NormalizeTenantId_WithNull_ShouldReturnEmptyString() + { + // Arrange + string? tenantId = null; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, normalizedId); + Assert.Equal(string.Empty, normalizedId); + } + + [Fact] + public void NormalizeTenantId_WithEmptyString_ShouldReturnEmptyString() + { + // Arrange + var tenantId = string.Empty; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, normalizedId); + } + + [Fact] + public void NormalizeTenantId_WithValidTenantId_ShouldReturnSameValue() + { + // Arrange + var tenantId = "tenant-123"; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal("tenant-123", normalizedId); + } + + [Fact] + public async Task WorkflowDefinitionStore_ShouldWorkWithTenantNormalization() + { + // Arrange var store = Scope.ServiceProvider.GetRequiredService(); var filter = new WorkflowDefinitionFilter { IsSystem = false, VersionOptions = VersionOptions.Latest }; + + // Act & Assert - Should not throw exceptions related to tenant ID handling var workflows = await store.FindManyAsync(filter); - Assert.All(workflows, workflow => Assert.Equal("Tenant1", workflow.TenantId)); + Assert.NotNull(workflows); + } + + [Fact] + public void TenantResolverContext_FindTenant_WithNull_ShouldNormalize() + { + // Arrange + var defaultTenant = new Tenant { Id = Tenant.DefaultTenantId, Name = "Default" }; + var tenant1 = new Tenant { Id = "tenant1", Name = "Tenant 1" }; + var tenantsDictionary = new Dictionary + { + { defaultTenant.Id, defaultTenant }, + { tenant1.Id, tenant1 } + }; + var context = new TenantResolverContext(tenantsDictionary, CancellationToken.None); + + // Act + string? nullTenantId = null; + var result = context.FindTenant(nullTenantId); + + // Assert + Assert.NotNull(result); + Assert.Equal(Tenant.DefaultTenantId, result.Id); + } + + [Fact] + public void TenantResolverContext_FindTenant_WithEmptyString_ShouldFindDefaultTenant() + { + // Arrange + var defaultTenant = new Tenant { Id = Tenant.DefaultTenantId, Name = "Default" }; + var tenantsDictionary = new Dictionary + { + { defaultTenant.Id, defaultTenant } + }; + var context = new TenantResolverContext(tenantsDictionary, CancellationToken.None); + + // Act + var result = context.FindTenant(string.Empty); + + // Assert + Assert.NotNull(result); + Assert.Equal(Tenant.DefaultTenantId, result.Id); + Assert.Equal("Default", result.Name); } -} \ No newline at end of file +} diff --git a/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs new file mode 100644 index 0000000000..8d02b77de7 --- /dev/null +++ b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs @@ -0,0 +1,49 @@ +using Elsa.Common.Multitenancy; + +namespace Elsa.Common.UnitTests.Multitenancy; + +public class TenantIdNormalizationTests +{ + [Theory] + [InlineData(null)] + [InlineData("")] + public void NormalizeTenantId_WithNullOrEmpty_ReturnsDefaultTenantId(string? tenantId) + { + // Act + var result = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, result); + Assert.Equal(string.Empty, result); + } + + [Theory] + [InlineData("tenant1")] + [InlineData("tenant-abc-123")] + [InlineData("DEFAULT")] + [InlineData("my-custom-tenant")] + [InlineData(" ")] // Whitespace is not normalized + public void NormalizeTenantId_WithNonNullString_ReturnsOriginalValue(string tenantId) + { + // Act + var result = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(tenantId, result); + } + + [Fact] + public void DefaultTenantId_IsEmptyString() + { + // Assert + Assert.Equal(Tenant.DefaultTenantId, string.Empty); + } + + [Fact] + public void DefaultTenant_UsesDefaultTenantId() + { + // Assert + Assert.Equal(Tenant.DefaultTenantId, Tenant.Default.Id); + Assert.Equal(string.Empty, Tenant.Default.Id); + } +} diff --git a/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs new file mode 100644 index 0000000000..94cb6d86f0 --- /dev/null +++ b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs @@ -0,0 +1,155 @@ +using Elsa.Common.Multitenancy; + +namespace Elsa.Common.UnitTests.Multitenancy; + +public class TenantResolverContextTests +{ + [Theory] + [InlineData(null, "Default")] + [InlineData("", "Default")] + [InlineData("tenant1", "Tenant 1")] + [InlineData("tenant2", "Tenant 2")] + public void FindTenant_ById_FindsCorrectTenant(string? tenantId, string expectedName) + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant(tenantId!); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedName, result.Name); + } + + [Fact] + public void FindTenant_WithNonExistentId_ReturnsNull() + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant("non-existent"); + + // Assert + Assert.Null(result); + } + + [Theory] + [InlineData("Alpha", "tenant1", "Tenant Alpha")] + [InlineData("Beta", "tenant2", "Tenant Beta")] + public void FindTenant_WithPredicate_FindsMatchingTenant(string searchTerm, string expectedId, string expectedName) + { + // Arrange + var context = CreateContextWithNamedTenants(); + + // Act + var result = context.FindTenant(t => t.Name.Contains(searchTerm)); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedId, result.Id); + Assert.Equal(expectedName, result.Name); + } + + [Fact] + public void FindTenant_WithPredicate_NoMatch_ReturnsNull() + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant(t => t.Name == "NonExistent"); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Constructor_StoresCancellationToken() + { + // Arrange + using var cts = new CancellationTokenSource(); + + // Act + var context = new TenantResolverContext(new Dictionary(), cts.Token); + + // Assert + Assert.Equal(cts.Token, context.CancellationToken); + } + + [Fact] + public void FindTenant_NormalizesNullAndEmptyStringToSameValue() + { + // Arrange + var context = CreateContext(); + + // Act + var resultFromNull = context.FindTenant((string?)null); + var resultFromEmptyString = context.FindTenant(string.Empty); + + // Assert + Assert.NotNull(resultFromNull); + Assert.NotNull(resultFromEmptyString); + Assert.Same(resultFromNull, resultFromEmptyString); + } + + // Helper methods + private static TenantResolverContext CreateContext() + { + var tenants = new Dictionary + { + { + Tenant.DefaultTenantId, new() + { + Id = Tenant.DefaultTenantId, + Name = "Default" + } + }, + { + "tenant1", new() + { + Id = "tenant1", + Name = "Tenant 1" + } + }, + { + "tenant2", new() + { + Id = "tenant2", + Name = "Tenant 2" + } + } + }; + return new(tenants, CancellationToken.None); + } + + private static TenantResolverContext CreateContextWithNamedTenants() + { + var tenants = new Dictionary + { + { + Tenant.DefaultTenantId, new() + { + Id = Tenant.DefaultTenantId, + Name = "Default" + } + }, + { + "tenant1", new() + { + Id = "tenant1", + Name = "Tenant Alpha" + } + }, + { + "tenant2", new() + { + Id = "tenant2", + Name = "Tenant Beta" + } + } + }; + return new(tenants, CancellationToken.None); + } +} \ No newline at end of file diff --git a/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj b/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj new file mode 100644 index 0000000000..057369bbd1 --- /dev/null +++ b/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj @@ -0,0 +1,14 @@ + + + + [Elsa.Tenants]* + 0 + + + + + + + + + diff --git a/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs b/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs new file mode 100644 index 0000000000..2e886f9c9a --- /dev/null +++ b/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs @@ -0,0 +1,181 @@ +using Elsa.Common.Multitenancy; +using Elsa.Tenants.Options; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; + +namespace Elsa.Tenants.UnitTests.Services; + +public class DefaultTenantResolverPipelineInvokerTests +{ + [Fact] + public async Task InvokePipelineAsync_WithEmptyStringTenantId_FindsDefaultTenant() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("Default", result.Name); + } + + [Fact] + public async Task InvokePipelineAsync_WithValidTenantId_FindsCorrectTenant() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("tenant1")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("Tenant 1", result.Name); + } + + [Fact] + public async Task InvokePipelineAsync_WithNonExistentTenantId_ReturnsNull() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("non-existent")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithNullTenantIdsInList_DoesNotThrowDictionaryException() + { + // Arrange - Simulates legacy data with null IDs + var tenants = new List + { + new() { Id = null!, Name = "Legacy Null Tenant" }, + new() { Id = "tenant1", Name = "Tenant 1" } + }; + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Unresolved()); + + // Act & Assert - Should not throw + var result = await invoker.InvokePipelineAsync(); + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithUnresolvedResult_ReturnsNull() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Unresolved()); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithMultipleResolvers_UsesFirstResolvedResult() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var mockResolver1 = CreateMockResolver(TenantResolverResult.Resolved("tenant1")); + var mockResolver2 = CreateMockResolver(TenantResolverResult.Resolved("tenant2")); + var invoker = CreateInvokerWithMultipleResolvers(tenants, mockResolver1, mockResolver2); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("tenant1", result.Id); + await mockResolver2.DidNotReceive().ResolveAsync(Arg.Any()); + } + + [Fact] + public async Task InvokePipelineAsync_WithLegacyNullTenantIds_NormalizesAndFindsDefaultTenant() + { + // Arrange - Simulates legacy data with null ID that gets normalized + var tenants = new List + { + new() { Id = null!, Name = "Legacy" }, // Will be normalized to "" + new() { Id = "tenant1", Name = "Tenant 1" } + }; + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert - The null tenant ID gets normalized to "" in dictionary, so it should be found + Assert.NotNull(result); + Assert.Equal("Legacy", result.Name); // Should find the Legacy tenant (normalized from null) + } + + // Helper methods + private static List CreateDefaultTenantList() => new() + { + new() { Id = Tenant.DefaultTenantId, Name = "Default" }, + new() { Id = "tenant1", Name = "Tenant 1" }, + new() { Id = "tenant2", Name = "Tenant 2" } + }; + + private static ITenantResolver CreateMockResolver(TenantResolverResult result) + { + var mockResolver = Substitute.For(); + mockResolver.ResolveAsync(Arg.Any()).Returns(result); + return mockResolver; + } + + private static (DefaultTenantResolverPipelineInvoker Invoker, ITenantResolver Resolver) CreateInvoker( + List tenants, + TenantResolverResult resolverResult) + { + var tenantsProvider = Substitute.For(); + tenantsProvider.ListAsync(Arg.Any()).Returns(tenants); + + var mockResolver = CreateMockResolver(resolverResult); + + // Use a mock pipeline builder that directly returns our mock resolver + var pipelineBuilder = Substitute.For(); + pipelineBuilder.Build(Arg.Any()).Returns(new[] { mockResolver }); + + var options = Microsoft.Extensions.Options.Options.Create(new MultitenancyOptions + { + TenantResolverPipelineBuilder = pipelineBuilder + }); + + var serviceProvider = Substitute.For(); + var logger = NullLogger.Instance; + var invoker = new DefaultTenantResolverPipelineInvoker(options, tenantsProvider, serviceProvider, logger); + + return (invoker, mockResolver); + } + + private static DefaultTenantResolverPipelineInvoker CreateInvokerWithMultipleResolvers( + List tenants, + params ITenantResolver[] resolvers) + { + var tenantsProvider = Substitute.For(); + tenantsProvider.ListAsync(Arg.Any()).Returns(tenants); + + // Use a mock pipeline builder that directly returns our mock resolvers + var pipelineBuilder = Substitute.For(); + pipelineBuilder.Build(Arg.Any()).Returns(resolvers); + + var options = Microsoft.Extensions.Options.Options.Create(new MultitenancyOptions + { + TenantResolverPipelineBuilder = pipelineBuilder + }); + + var serviceProvider = Substitute.For(); + var logger = NullLogger.Instance; + return new(options, tenantsProvider, serviceProvider, logger); + } +} diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs new file mode 100644 index 0000000000..9848816ac1 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs @@ -0,0 +1,116 @@ +using Elsa.Workflows.Activities; +using Elsa.Workflows.Models; + +namespace Elsa.Workflows.Core.UnitTests.Models; + +public class ActivityConstructionResultTests +{ + [Theory] + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(2, true)] + public void Constructor_WithVaryingExceptionCounts_SetsHasExceptionsCorrectly(int exceptionCount, bool expectedHasExceptions) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + + // Act + var result = new ActivityConstructionResult(activity, exceptions); + + // Assert + Assert.Same(activity, result.Activity); + Assert.Equal(exceptionCount, result.Exceptions.Count()); + Assert.Equal(expectedHasExceptions, result.HasExceptions); + } + + [Fact] + public void Constructor_WithNullExceptions_TreatsAsEmpty() + { + // Arrange + var activity = CreateActivity(); + + // Act + var result = new ActivityConstructionResult(activity, null); + + // Assert + Assert.Empty(result.Exceptions); + Assert.False(result.HasExceptions); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(3)] + public void Cast_PreservesActivityAndExceptions(int exceptionCount) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + var result = new ActivityConstructionResult(activity, exceptions); + + // Act + var typedResult = result.Cast(); + + // Assert + Assert.IsType>(typedResult); + Assert.Same(activity, typedResult.Activity); + Assert.Equal(exceptionCount, typedResult.Exceptions.Count()); + Assert.Equal(result.HasExceptions, typedResult.HasExceptions); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + public void GenericConstructor_CreatesTypedResult(int exceptionCount) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + + // Act + var result = new ActivityConstructionResult(activity, exceptions); + + // Assert + Assert.Same(activity, result.Activity); + Assert.Equal(exceptionCount, result.Exceptions.Count()); + Assert.Equal(exceptionCount > 0, result.HasExceptions); + Assert.IsAssignableFrom(result); + } + + [Fact] + public void Exceptions_CanBeEnumerated() + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(3); + var result = new ActivityConstructionResult(activity, exceptions); + + // Act + var enumeratedExceptions = new List(); + foreach (var ex in result.Exceptions) + { + enumeratedExceptions.Add(ex); + } + + // Assert + Assert.Equal(3, enumeratedExceptions.Count); + Assert.All(enumeratedExceptions, ex => Assert.NotNull(ex)); + } + + // Helper methods + private static WriteLine CreateActivity() => new("test"); + + private static List? CreateExceptions(int count) + { + if (count == 0) return null; + + var exceptions = new List(); + for (var i = 0; i < count; i++) + { + exceptions.Add(new InvalidOperationException($"Error {i + 1}")); + } + return exceptions; + } +} From d849aeaaa7d1640bb81380a189108805be19134a Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 18:57:45 +0100 Subject: [PATCH 09/21] Update unit tests for `ActivityConstructionResult` - Refactor test parameterization to verify `HasExceptions` property more explicitly. - Simplify exception creation logic in helper methods. - Improve test assertions by combining act and assert phases where applicable. --- .../Models/ActivityConstructionResultTests.cs | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs index 9848816ac1..aabad71ce9 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs @@ -9,7 +9,7 @@ public class ActivityConstructionResultTests [InlineData(0, false)] [InlineData(1, true)] [InlineData(2, true)] - public void Constructor_WithVaryingExceptionCounts_SetsHasExceptionsCorrectly(int exceptionCount, bool expectedHasExceptions) + public void Constructor_WithVaryingExceptionCounts_SetsPropertiesCorrectly(int exceptionCount, bool expectedHasExceptions) { // Arrange var activity = CreateActivity(); @@ -39,10 +39,10 @@ public void Constructor_WithNullExceptions_TreatsAsEmpty() } [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(3)] - public void Cast_PreservesActivityAndExceptions(int exceptionCount) + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(3, true)] + public void Cast_PreservesActivityAndExceptions(int exceptionCount, bool expectedHasExceptions) { // Arrange var activity = CreateActivity(); @@ -56,14 +56,14 @@ public void Cast_PreservesActivityAndExceptions(int exceptionCount) Assert.IsType>(typedResult); Assert.Same(activity, typedResult.Activity); Assert.Equal(exceptionCount, typedResult.Exceptions.Count()); - Assert.Equal(result.HasExceptions, typedResult.HasExceptions); + Assert.Equal(expectedHasExceptions, typedResult.HasExceptions); } [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - public void GenericConstructor_CreatesTypedResult(int exceptionCount) + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(2, true)] + public void GenericConstructor_CreatesTypedResultWithInheritance(int exceptionCount, bool expectedHasExceptions) { // Arrange var activity = CreateActivity(); @@ -75,7 +75,7 @@ public void GenericConstructor_CreatesTypedResult(int exceptionCount) // Assert Assert.Same(activity, result.Activity); Assert.Equal(exceptionCount, result.Exceptions.Count()); - Assert.Equal(exceptionCount > 0, result.HasExceptions); + Assert.Equal(expectedHasExceptions, result.HasExceptions); Assert.IsAssignableFrom(result); } @@ -87,16 +87,14 @@ public void Exceptions_CanBeEnumerated() var exceptions = CreateExceptions(3); var result = new ActivityConstructionResult(activity, exceptions); - // Act - var enumeratedExceptions = new List(); + // Act & Assert + var count = 0; foreach (var ex in result.Exceptions) { - enumeratedExceptions.Add(ex); + Assert.NotNull(ex); + count++; } - - // Assert - Assert.Equal(3, enumeratedExceptions.Count); - Assert.All(enumeratedExceptions, ex => Assert.NotNull(ex)); + Assert.Equal(3, count); } // Helper methods @@ -106,11 +104,8 @@ public void Exceptions_CanBeEnumerated() { if (count == 0) return null; - var exceptions = new List(); - for (var i = 0; i < count; i++) - { - exceptions.Add(new InvalidOperationException($"Error {i + 1}")); - } - return exceptions; + return Enumerable.Range(1, count) + .Select(i => new InvalidOperationException($"Error {i}") as Exception) + .ToList(); } } From 6c1e6fc14c93f750501ba26b6165f3fd4d0823ca Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 27 Jan 2026 15:26:31 +0100 Subject: [PATCH 10/21] Enable configuration-based multitenancy with tenant-specific settings - Introduced a configuration-based tenant provider to streamline tenant initialization and customization. - Added tenant ID handling filters to ensure tenant ID is applied and filtered automatically. - Deprecated the `CommonPersistenceFeature` in favor of modular persistence feature extension. --- src/apps/Elsa.Server.Web/Program.cs | 17 +++++++--- src/apps/Elsa.Server.Web/appsettings.json | 13 +++++++ .../Multitenancy/Contracts/ITenantResolver.cs | 2 +- .../CommonPersistenceFeature.cs | 34 +++++++++---------- .../PersistenceFeatureBase.cs | 4 +++ 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index d16503af39..a1a8de9c7e 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -4,12 +4,14 @@ using Elsa.Expressions.Helpers; using Elsa.Extensions; using Elsa.Features.Services; +using Elsa.Identity.Multitenancy; using Elsa.Persistence.EFCore.Extensions; using Elsa.Persistence.EFCore.Modules.Management; using Elsa.Persistence.EFCore.Modules.Runtime; using Elsa.Server.Web.Activities; using Elsa.Server.Web.ActivityHosts; using Elsa.Server.Web.Filters; +using Elsa.Tenants; using Elsa.Tenants.AspNetCore; using Elsa.Tenants.Extensions; using Elsa.WorkflowProviders.BlobStorage.ElsaScript.Extensions; @@ -118,10 +120,17 @@ http.ConfigureHttpOptions = options => configuration.GetSection("Http").Bind(options); http.UseCache(); }); - - if (useMultitenancy) - elsa.UseTenants(); - + + if(useMultitenancy) + { + elsa.UseTenants(tenants => + { + tenants.UseConfigurationBasedTenantsProvider(options => configuration.GetSection("Multitenancy").Bind(options)); + tenants.ConfigureMultitenancy(options => options.TenantResolverPipelineBuilder = new TenantResolverPipelineBuilder() + .Append()); + }); + } + ConfigureForTest?.Invoke(elsa); }); diff --git a/src/apps/Elsa.Server.Web/appsettings.json b/src/apps/Elsa.Server.Web/appsettings.json index 7fd3ed9157..e3cf3df520 100644 --- a/src/apps/Elsa.Server.Web/appsettings.json +++ b/src/apps/Elsa.Server.Web/appsettings.json @@ -39,6 +39,19 @@ "Sqlite": "Data Source=App_Data/elsa.sqlite.db;Cache=Shared;" } } + }, + { + "Id": "tenant-2", + "Name": "Tenant 2", + "Configuration": { + "Http": { + "Prefix": "tenant-2", + "Host": "localhost:5001" + }, + "ConnectionStrings": { + "Sqlite": "Data Source=App_Data/elsa.sqlite.db;Cache=Shared;" + } + } } ] }, diff --git a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs index 76fcb7e93a..9927e676a9 100644 --- a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs +++ b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs @@ -1,7 +1,7 @@ namespace Elsa.Common.Multitenancy; /// -/// A strategy for resolving the current tenant. This is called the tenant initializer. +/// A strategy for resolving the current tenant, called from the tenant initializer. /// public interface ITenantResolver { diff --git a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs b/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs index e040a20af4..8b18890d46 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs @@ -1,17 +1,17 @@ -using Elsa.Persistence.EFCore.EntityHandlers; -using Elsa.Features.Abstractions; -using Elsa.Features.Services; -using Microsoft.Extensions.DependencyInjection; - -namespace Elsa.Persistence.EFCore; - -/// -public class CommonPersistenceFeature(IModule module) : FeatureBase(module) -{ - /// - public override void Apply() - { - Services.AddScoped(); - Services.AddScoped(); - } -} \ No newline at end of file +// using Elsa.Persistence.EFCore.EntityHandlers; +// using Elsa.Features.Abstractions; +// using Elsa.Features.Services; +// using Microsoft.Extensions.DependencyInjection; +// +// namespace Elsa.Persistence.EFCore; +// +// /// +// public class CommonPersistenceFeature(IModule module) : FeatureBase(module) +// { +// /// +// public override void Apply() +// { +// Services.AddScoped(); +// Services.AddScoped(); +// } +// } \ No newline at end of file diff --git a/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs b/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs index 5acc709bf6..ab7bca5f1a 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs @@ -2,6 +2,7 @@ using Elsa.Extensions; using Elsa.Features.Abstractions; using Elsa.Features.Services; +using Elsa.Persistence.EFCore.EntityHandlers; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.DependencyInjection; @@ -60,6 +61,9 @@ public override void Apply() { options.RunMigrations[typeof(TDbContext)] = RunMigrations; }); + + Services.AddScoped(); + Services.AddScoped(); } protected virtual void ConfigureMigrations() From 970ca15fd9e757efa9a050e94ffd22dd6d74efa2 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 27 Jan 2026 19:59:20 +0100 Subject: [PATCH 11/21] Update database indexes to include `TenantId` for multitenancy support - Added `TenantId` to unique constraints on `Triggers` table across all EFCore providers. - Adjusted index names to reflect the updated constraints. - Updated trigger configuration to ensure uniqueness includes `TenantId`. --- .../Migrations/Runtime/20251204150235_V3_6.cs | 6 +++--- .../Migrations/Runtime/20251204150355_V3_6.cs | 6 +++--- .../Migrations/Runtime/20251204150341_V3_6.cs | 6 +++--- .../Migrations/Runtime/20251204150326_V3_6.cs | 6 +++--- .../Migrations/Runtime/20251204150006_V3_6.cs | 6 +++--- .../Modules/Runtime/Configurations.cs | 7 ++++--- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs index 6a99e43f0c..12286caeb9 100644 --- a/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs @@ -30,10 +30,10 @@ protected override void Up(MigrationBuilder migrationBuilder) .OldAnnotation("MySql:CharSet", "utf8mb4"); migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -41,7 +41,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); diff --git a/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs index da50b7b7a9..d996768b38 100644 --- a/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true, filter: "\"Hash\" IS NOT NULL"); } @@ -31,7 +31,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs index 8446ca4aa9..bce6fbd4aa 100644 --- a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -30,7 +30,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs index e59cf0b8eb..2fcbff07b8 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs @@ -28,10 +28,10 @@ protected override void Up(MigrationBuilder migrationBuilder) oldType: "nvarchar(max)"); migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true, filter: "[Hash] IS NOT NULL"); } @@ -40,7 +40,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); diff --git a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs index 1f8e85ddd9..d3246d656d 100644 --- a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -30,7 +30,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs b/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs index 7c7988bea4..230ed4f0bd 100644 --- a/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs +++ b/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs @@ -125,15 +125,16 @@ public void Configure(EntityTypeBuilder builder) builder.HasIndex(x => x.TenantId).HasDatabaseName($"IX_{nameof(StoredTrigger)}_{nameof(StoredTrigger.TenantId)}"); // Add unique constraint to prevent duplicate trigger registrations in multi-engine environments - // A trigger is uniquely identified by WorkflowDefinitionId + Hash + ActivityId + // A trigger is uniquely identified by WorkflowDefinitionId + Hash + ActivityId + TenantId builder.HasIndex(x => new { x.WorkflowDefinitionId, x.Hash, - x.ActivityId + x.ActivityId, + x.TenantId }) .IsUnique() - .HasDatabaseName($"IX_{nameof(StoredTrigger)}_Unique_{nameof(StoredTrigger.WorkflowDefinitionId)}_{nameof(StoredTrigger.Hash)}_{nameof(StoredTrigger.ActivityId)}"); + .HasDatabaseName($"IX_{nameof(StoredTrigger)}_Unique_{nameof(StoredTrigger.WorkflowDefinitionId)}_{nameof(StoredTrigger.Hash)}_{nameof(StoredTrigger.ActivityId)}_{nameof(StoredTrigger.TenantId)}"); } /// From 4395aff694723e79562f6a749be74329efe500a1 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 13:23:16 +0100 Subject: [PATCH 12/21] Add tenant filtering to `DefaultWorkflowDefinitionStorePopulator` - Introduced `ITenantAccessor` to support tenant-specific filtering of workflow definitions. - Updated logic to skip workflows not matching the current tenant. --- ...DefaultWorkflowDefinitionStorePopulator.cs | 20 ++++++++++++++++++- ...ltWorkflowDefinitionStorePopulatorTests.cs | 2 ++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs index 334b0d035e..c895e5170a 100644 --- a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs +++ b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs @@ -1,5 +1,6 @@ using Elsa.Common; using Elsa.Common.Models; +using Elsa.Common.Multitenancy; using Elsa.Workflows.Activities; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; @@ -19,6 +20,7 @@ public class DefaultWorkflowDefinitionStorePopulator : IWorkflowDefinitionStoreP private readonly IPayloadSerializer _payloadSerializer; private readonly ISystemClock _systemClock; private readonly IIdentityGraphService _identityGraphService; + private readonly ITenantAccessor _tenantAccessor; private readonly ILogger _logger; private readonly SemaphoreSlim _semaphore = new(1, 1); @@ -33,6 +35,7 @@ public DefaultWorkflowDefinitionStorePopulator( IPayloadSerializer payloadSerializer, ISystemClock systemClock, IIdentityGraphService identityGraphService, + ITenantAccessor tenantAccessor, ILogger logger) { _workflowDefinitionProviders = workflowDefinitionProviders; @@ -42,6 +45,7 @@ public DefaultWorkflowDefinitionStorePopulator( _payloadSerializer = payloadSerializer; _systemClock = systemClock; _identityGraphService = identityGraphService; + _tenantAccessor = tenantAccessor; _logger = logger; } @@ -56,6 +60,7 @@ public async Task> PopulateStoreAsync(bool index { var providers = _workflowDefinitionProviders(); var workflowDefinitions = new List(); + var currentTenantId = _tenantAccessor.Tenant?.Id; foreach (var provider in providers) { @@ -63,6 +68,17 @@ public async Task> PopulateStoreAsync(bool index foreach (var result in results) { + if (result.Workflow.Identity.TenantId.NormalizeTenantId() != currentTenantId.NormalizeTenantId()) + { + _logger.LogDebug( + "Skipping adding workflow {WorkflowId} from provider {Provider} because it belongs to tenant '{WorkflowTenantId}' but current tenant is '{CurrentTenantId}'", + result.Workflow.Identity.DefinitionId, + provider.Name, + result.Workflow.Identity.TenantId, + currentTenantId); + continue; + } + var workflowDefinition = await AddAsync(result, indexTriggers, cancellationToken); workflowDefinitions.Add(workflowDefinition); } @@ -128,7 +144,9 @@ private async Task AddOrUpdateCoreAsync(MaterializedWorkflow { // NEW WAY: OriginalSource is provided // For JSON workflows, we still need to populate StringData with the serialized root for backwards compatibility - stringData = materializedWorkflow.MaterializerName == "Json" ? _activitySerializer.Serialize(workflow.Root) : + stringData = materializedWorkflow.MaterializerName == "Json" + ? _activitySerializer.Serialize(workflow.Root) + : // For new formats (ElsaScript, YAML, etc.), only OriginalSource is needed // StringData can be null as these materializers only use OriginalSource null; diff --git a/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs b/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs index 5f960ea400..cb93169878 100644 --- a/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs +++ b/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs @@ -1,4 +1,5 @@ using Elsa.Common; +using Elsa.Common.Multitenancy; using Elsa.Workflows.Activities; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; @@ -27,6 +28,7 @@ public DefaultWorkflowDefinitionStorePopulatorTests() Substitute.For(), Substitute.For(), Substitute.For(), + Substitute.For(), Substitute.For>()); } From 42a7c49d8fb37cf4d747fdc6ea813119570b41cb Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 19:09:12 +0100 Subject: [PATCH 13/21] Update doc/adr/toc.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- doc/adr/toc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/adr/toc.md b/doc/adr/toc.md index cb3c2a4364..2256411817 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -7,4 +7,4 @@ * [5. Token-Centric Flowchart Execution Model](0005-token-centric-flowchart-execution-model.md) * [6. Tenant Deleted Event](0006-tenant-deleted-event.md) * [7. Adoption of Explicit Merge Modes for Flowchart Joins](0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md) -* [8. Empty String as Default Tenant ID](0008-empty-string-as-default-tenant-id.md)string-as-default-tenant-id.md) \ No newline at end of file +* [8. Empty String as Default Tenant ID](0008-empty-string-as-default-tenant-id.md) \ No newline at end of file From dec0f4aed632ce84c06e6cfe9aa9454dc65d333e Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 19:17:04 +0100 Subject: [PATCH 14/21] Remove `CommonPersistenceFeature` as it has been deprecated --- .../CommonPersistenceFeature.cs | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs diff --git a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs b/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs deleted file mode 100644 index 8b18890d46..0000000000 --- a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs +++ /dev/null @@ -1,17 +0,0 @@ -// using Elsa.Persistence.EFCore.EntityHandlers; -// using Elsa.Features.Abstractions; -// using Elsa.Features.Services; -// using Microsoft.Extensions.DependencyInjection; -// -// namespace Elsa.Persistence.EFCore; -// -// /// -// public class CommonPersistenceFeature(IModule module) : FeatureBase(module) -// { -// /// -// public override void Apply() -// { -// Services.AddScoped(); -// Services.AddScoped(); -// } -// } \ No newline at end of file From 9bcd6ca74d1f217b0b1e8c569be31236a634439a Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 19:22:51 +0100 Subject: [PATCH 15/21] Add tenant-specific filtering to workflow import logic in `DefaultWorkflowDefinitionStorePopulator` --- .../Services/DefaultWorkflowDefinitionStorePopulator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs index c895e5170a..b64840d9d5 100644 --- a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs +++ b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs @@ -68,6 +68,7 @@ public async Task> PopulateStoreAsync(bool index foreach (var result in results) { + // Only import workflows belonging to the current tenant. if (result.Workflow.Identity.TenantId.NormalizeTenantId() != currentTenantId.NormalizeTenantId()) { _logger.LogDebug( From 6833d5e469a90e25d8dd42f8b45ca7ff83a0c79a Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 19:37:05 +0100 Subject: [PATCH 16/21] Replace hardcoded tenant ID with `Tenant.DefaultTenantId` in integration tests --- .../Scenarios/WorkflowDefinitionStorePopulation/Tests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs index a4cdd48595..076d00b162 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs @@ -1,4 +1,5 @@ -using Elsa.Testing.Shared; +using Elsa.Common.Multitenancy; +using Elsa.Testing.Shared; using Elsa.Workflows.Activities; using Elsa.Workflows.Helpers; using Elsa.Workflows.Management; @@ -29,7 +30,7 @@ public Tests(ITestOutputHelper testOutputHelper) DefinitionId: "WorkflowWithTrigger", Version: 1, Id: "1", - TenantId: "default" + TenantId: Tenant.DefaultTenantId ), Root = new Event("Foo") { From d86dc21e58da29eca00398a1e69128a5dcb0fb6a Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 20:33:30 +0100 Subject: [PATCH 17/21] Update database indexes and migration logic to support `TenantId` for multitenancy - Added `TenantId` to unique constraints on the `Triggers` table and updated index names. - Included logic to drop outdated indexes without `TenantId` during migration. - Adjusted tests to account for `TenantId` in workflow identity and indexing scenarios. --- .../Migrations/Runtime/20251204150326_V3_6.Designer.cs | 4 ++-- .../Migrations/Runtime/20251204150326_V3_6.cs | 10 ++++++++++ .../Runtime/RuntimeElsaDbContextModelSnapshot.cs | 4 ++-- .../ConcurrentTriggerIndexingTests.cs | 3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs index e7503a1ba0..49bb3921ab 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs @@ -317,9 +317,9 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.HasIndex("WorkflowDefinitionVersionId") .HasDatabaseName("IX_StoredTrigger_WorkflowDefinitionVersionId"); - b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId") + b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId", "TenantId") .IsUnique() - .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId") + .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId") .HasFilter("[Hash] IS NOT NULL"); b.ToTable("Triggers", "Elsa"); diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs index 2fcbff07b8..e1581cfc9d 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs @@ -18,6 +18,16 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) /// protected override void Up(MigrationBuilder migrationBuilder) { + // Drop old index if it exists (before TenantId was added) + migrationBuilder.Sql($@" + IF EXISTS (SELECT * FROM sys.indexes WHERE name = 'IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId' + AND object_id = OBJECT_ID('{_schema.Schema}.Triggers')) + BEGIN + DROP INDEX [IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId] + ON [{_schema.Schema}].[Triggers] + END + "); + migrationBuilder.AlterColumn( name: "ActivityId", schema: _schema.Schema, diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs index 47d2ae7cd3..b821c873fb 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs @@ -314,9 +314,9 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("WorkflowDefinitionVersionId") .HasDatabaseName("IX_StoredTrigger_WorkflowDefinitionVersionId"); - b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId") + b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId", "TenantId") .IsUnique() - .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId") + .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId") .HasFilter("[Hash] IS NOT NULL"); b.ToTable("Triggers", "Elsa"); diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs index 1a0afd6e6f..088e22043f 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs @@ -186,7 +186,7 @@ private static Workflow CreateTestWorkflow() return new() { - Identity = new(definitionId, 1, workflowId), + Identity = new(definitionId, 1, workflowId, ""), Root = new Sequence { Activities = @@ -208,6 +208,7 @@ private async Task SaveWorkflowDefinitionAsync(Workflow work Id = workflow.Identity.Id, DefinitionId = workflow.Identity.DefinitionId, Version = workflow.Identity.Version, + TenantId = workflow.Identity.TenantId, IsLatest = true, IsPublished = true, Name = "Test Concurrent Workflow", From 7ec5bef29ff1250cb4995ea9467bf8b9b3570ab9 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 20:39:28 +0100 Subject: [PATCH 18/21] Remove `TenantId` from workflow identity construction in concurrent trigger indexing tests --- .../ConcurrentTriggerIndexingTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs index 088e22043f..1a0afd6e6f 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/ConcurrentTriggerIndexing/ConcurrentTriggerIndexingTests.cs @@ -186,7 +186,7 @@ private static Workflow CreateTestWorkflow() return new() { - Identity = new(definitionId, 1, workflowId, ""), + Identity = new(definitionId, 1, workflowId), Root = new Sequence { Activities = @@ -208,7 +208,6 @@ private async Task SaveWorkflowDefinitionAsync(Workflow work Id = workflow.Identity.Id, DefinitionId = workflow.Identity.DefinitionId, Version = workflow.Identity.Version, - TenantId = workflow.Identity.TenantId, IsLatest = true, IsPublished = true, Name = "Test Concurrent Workflow", From f4a184a54fa1e0461c23ffd729c81676877b57db Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 21:48:43 +0100 Subject: [PATCH 19/21] Introduce `SelectiveMockLockProvider` for precise lock mocking in tests - Added `SelectiveMockLockProvider` to allow targeted lock mocking without affecting unrelated background operations. - Updated test services to use `SelectiveMockLockProvider` in place of `TestDistributedLockProvider`. - Refactored `DistributedLockResilienceTests` to support selective mocking for deterministic and reliable assertions. --- .../Helpers/Fixtures/WorkflowServer.cs | 16 +-- .../DistributedLockResilienceTests.cs | 66 ++++++----- .../Mocks/SelectiveMockLockProvider.cs | 104 ++++++++++++++++++ .../Mocks/TestDistributedLockProvider.cs | 5 + 4 files changed, 154 insertions(+), 37 deletions(-) create mode 100644 test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs index ab0907a536..d95d07aafa 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs @@ -131,16 +131,17 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) builder.ConfigureTestServices(services => { - // Decorate IDistributedLockProvider with TestDistributedLockProvider so tests use it - services.Decorate(); - - // Also register TestDistributedLockProvider as itself so tests can access it directly for configuration + // Decorate IDistributedLockProvider with SelectiveMockLockProvider + // This allows tests to selectively mock specific locks without affecting background operations + services.Decorate(); + + // Register SelectiveMockLockProvider as itself so tests can access it for configuration services.AddSingleton(sp => { var provider = sp.GetRequiredService(); - if (provider is not TestDistributedLockProvider testProvider) - throw new InvalidOperationException($"Expected IDistributedLockProvider to be decorated with TestDistributedLockProvider, but got {provider.GetType().Name}"); - return testProvider; + if (provider is not SelectiveMockLockProvider selectiveProvider) + throw new InvalidOperationException($"Expected IDistributedLockProvider to be decorated with SelectiveMockLockProvider, but got {provider.GetType().Name}"); + return selectiveProvider; }); services @@ -154,7 +155,6 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) .AddWorkflowsProvider() .AddNotificationHandlersFrom() .Decorate() - .Decorate() ; }); } diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs index efa063902a..0c0b89020d 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs @@ -19,10 +19,10 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience; public class DistributedLockResilienceTests(App app) : AppComponentTest(app) { private const int MaxRetryAttempts = 3; - - // The IDistributedLockProvider is decorated with TestDistributedLockProvider in WorkflowServer.ConfigureTestServices - // This cast is safe because the decorator pattern ensures TestDistributedLockProvider wraps the actual provider - private TestDistributedLockProvider MockProvider => (TestDistributedLockProvider)Scope.ServiceProvider.GetRequiredService(); + + // Selective mock provider - only mocks specific locks, not all locks globally + private SelectiveMockLockProvider SelectiveMockProvider => Scope.ServiceProvider.GetRequiredService(); + private ITransientExceptionDetector TransientExceptionDetector => Scope.ServiceProvider.GetRequiredService(); private ILogger Logger => Scope.ServiceProvider.GetRequiredService>(); private DistributedLockingOptions LockOptions => Scope.ServiceProvider.GetRequiredService>().Value; @@ -34,22 +34,25 @@ public class DistributedLockResilienceTests(App app) : AppComponentTest(app) [InlineData(4, 4, true)] // Four failures, exhausts retries (MaxRetryAttempts = 3) public async Task AcquireLockWithRetry_AcquisitionFailures_BehavesAsExpected(int failureCount, int expectedAttemptCount, bool shouldThrow) { - // Arrange - MockProvider.Reset(); - MockProvider.FailAcquisitionTimes(failureCount); + // Arrange - Mock this specific lock only + var lockName = $"test-lock-{failureCount}"; + var mockProvider = SelectiveMockProvider.MockLock(lockName); + mockProvider.Reset(); + mockProvider.FailAcquisitionTimes(failureCount); // Act & Assert if (shouldThrow) { - await Assert.ThrowsAsync(async () => await AcquireLockWithRetryAsync($"test-lock-{failureCount}")); + await Assert.ThrowsAsync(async () => await AcquireLockWithRetryAsync(lockName, mockProvider)); } else { - await using var handle = await AcquireLockWithRetryAsync($"test-lock-{failureCount}"); + await using var handle = await AcquireLockWithRetryAsync(lockName, mockProvider); Assert.NotNull(handle); } - Assert.Equal(expectedAttemptCount, MockProvider.AcquisitionAttemptCount); + // Assert exact count - only this lock is mocked + Assert.Equal(expectedAttemptCount, mockProvider.AcquisitionAttemptCount); } [Theory] @@ -62,10 +65,11 @@ public async Task RunInstanceAsync_TransientLockFailures_RetriesCorrectly(int fa var workflowClient = await CreateWorkflowClientAsync(); var workflowInstanceId = workflowClient.WorkflowInstanceId; - // Reset and configure failures for this specific workflow instance's lock - MockProvider.Reset(); - MockProvider.FailAcquisitionTimesForLock($"workflow-instance:{workflowInstanceId}", failureCount); - var attemptCountBefore = MockProvider.AcquisitionAttemptCount; + // Configure failures for this specific workflow instance's lock only + var lockPrefix = $"workflow-instance:{workflowInstanceId}"; + var mockProvider = SelectiveMockProvider.MockLock(lockPrefix); + mockProvider.Reset(); + mockProvider.FailAcquisitionTimes(failureCount); // Now run the instance with the configured lock failures var runRequest = new RunWorkflowInstanceRequest(); @@ -82,34 +86,42 @@ await Assert.ThrowsAsync(async () => Assert.NotNull(response); } - // Verify retries occurred - check the delta from before the operation to account for background noise - var expectedAttempts = failureCount + 1; // failures + 1 success (or final failure for shouldThrow case) - AssertMinimumAttempts(MockProvider.AcquisitionAttemptCount - attemptCountBefore, expectedAttempts, "acquisition"); + // Assert exact count - only this specific workflow instance lock is mocked + // When shouldThrow=true, all attempts fail: MaxRetryAttempts+1 (initial + retries) + // When shouldThrow=false, we succeed after failures: failureCount+1 (failures + success) + var expectedAttempts = shouldThrow ? MaxRetryAttempts + 1 : failureCount + 1; + Assert.Equal(expectedAttempts, mockProvider.AcquisitionAttemptCount); } [Fact] public async Task RunInstanceAsync_TransientReleaseFailure_ShouldLogButNotThrow() { // Arrange - MockProvider.Reset(); var workflowClient = await CreateWorkflowClientAsync(createInstance: false); - // Configure failure after client creation to minimize background interference - MockProvider.FailReleaseOnce(); - var releaseCountBefore = MockProvider.ReleaseAttemptCount; + // Create and run to get the workflow instance ID, then configure release failure for that lock + var request = CreateAndRunRequest(); + + // Mock the workflow-instance lock prefix (all workflow instance locks) + var mockProvider = SelectiveMockProvider.MockLock("workflow-instance:"); + mockProvider.Reset(); + mockProvider.FailReleaseOnce(); // Act - Release failure should be caught and logged, not thrown - var response = await workflowClient.CreateAndRunInstanceAsync(CreateAndRunRequest()); + var response = await workflowClient.CreateAndRunInstanceAsync(request); // Assert Assert.NotNull(response); Assert.NotNull(response.WorkflowInstanceId); - AssertMinimumAttempts(MockProvider.ReleaseAttemptCount - releaseCountBefore, 1, "release"); + + // Verify at least one release occurred + Assert.True(mockProvider.ReleaseAttemptCount >= 1, + $"Expected at least 1 release attempt, but got {mockProvider.ReleaseAttemptCount}"); } - private async Task AcquireLockWithRetryAsync(string lockName) => + private async Task AcquireLockWithRetryAsync(string lockName, TestDistributedLockProvider mockProvider) => await RetryPipeline.ExecuteAsync(async ct => - await MockProvider.AcquireLockAsync(lockName, LockOptions.LockAcquisitionTimeout, ct), + await mockProvider.CreateLock(lockName).AcquireAsync(LockOptions.LockAcquisitionTimeout, ct), CancellationToken.None); /// @@ -138,10 +150,6 @@ private static CreateAndRunWorkflowInstanceRequest CreateAndRunRequest() => WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionId(SimpleWorkflow.DefinitionId, VersionOptions.Latest) }; - private static void AssertMinimumAttempts(int actualAttempts, int expectedAttempts, string attemptType) => - Assert.True(actualAttempts >= expectedAttempts, - $"Expected at least {expectedAttempts} {attemptType} attempts, but got {actualAttempts}"); - private static ResiliencePipeline CreateRetryPipeline(ITransientExceptionDetector transientExceptionDetector, ILogger logger) => new ResiliencePipelineBuilder() .AddRetry(new() diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs new file mode 100644 index 0000000000..cc56e9ced0 --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs @@ -0,0 +1,104 @@ +using Medallion.Threading; + +namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience.Mocks; + +/// +/// A lock provider that selectively mocks specific locks while allowing others to use the real implementation. +/// +/// WHY THIS IS NECESSARY: +/// - Workflow operations trigger background processes (trigger indexing, state persistence, etc.) +/// - These background operations acquire their own locks concurrently +/// - If we mock ALL locks globally, background operations can consume configured failures +/// - This makes test assertions unreliable and flaky +/// +/// SOLUTION: +/// - Only mock locks matching specific prefixes configured by tests +/// - Background operations use real locks (not counted, not mocked) +/// - Test operations use mocked locks (counted, failures injected) +/// - Result: Deterministic, reliable test assertions +/// +public class SelectiveMockLockProvider : IDistributedLockProvider, IDisposable +{ + private readonly IDistributedLockProvider _realProvider; + private readonly Dictionary _mockProvidersByPrefix = new(); + private readonly object _lock = new(); + + public SelectiveMockLockProvider(IDistributedLockProvider realProvider) + { + _realProvider = realProvider; + } + + /// + /// Gets the real/inner provider being wrapped. + /// + public IDistributedLockProvider RealProvider => _realProvider; + + /// + /// Configures mocking for locks matching the specified prefix. + /// Returns a test provider that allows configuring failures for these locks. + /// + public TestDistributedLockProvider MockLock(string lockNamePrefix) + { + lock (_lock) + { + if (!_mockProvidersByPrefix.TryGetValue(lockNamePrefix, out var mockProvider)) + { + mockProvider = new TestDistributedLockProvider(_realProvider); + _mockProvidersByPrefix[lockNamePrefix] = mockProvider; + } + return mockProvider; + } + } + + /// + /// Removes mocking for the specified lock prefix, allowing it to use the real provider. + /// + public void Unmock(string lockNamePrefix) + { + lock (_lock) + { + _mockProvidersByPrefix.Remove(lockNamePrefix); + } + } + + /// + /// Clears all mock configurations, resetting to real provider for all locks. + /// + public void Reset() + { + lock (_lock) + { + foreach (var mockProvider in _mockProvidersByPrefix.Values) + { + mockProvider.Reset(); + } + _mockProvidersByPrefix.Clear(); + } + } + + /// + /// Creates a lock that will be mocked if it matches a configured prefix, otherwise uses the real provider. + /// + public IDistributedLock CreateLock(string name) + { + lock (_lock) + { + // Check if this lock name matches any configured mock prefix + foreach (var (prefix, mockProvider) in _mockProvidersByPrefix) + { + if (name.StartsWith(prefix, StringComparison.Ordinal)) + { + return mockProvider.CreateLock(name); + } + } + + // No mock configured, use real provider + return _realProvider.CreateLock(name); + } + } + + public void Dispose() + { + Reset(); + } +} diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs index fa50c825ad..0ffe126da6 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs @@ -15,6 +15,11 @@ public class TestDistributedLockProvider(IDistributedLockProvider innerProvider) private int _releaseAttemptCount; private string? _targetLockPrefix; + /// + /// Gets the inner/real lock provider that this test provider wraps. + /// + public IDistributedLockProvider InnerProvider => innerProvider; + public int AcquisitionAttemptCount => _acquisitionAttemptCount; public int ReleaseAttemptCount => _releaseAttemptCount; From 0b271d6caff26b16fe2cc0774c62ae5c01bf0a30 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 28 Jan 2026 22:05:13 +0100 Subject: [PATCH 20/21] Update Elsa.sln Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Elsa.sln | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Elsa.sln b/Elsa.sln index b68e81eecd..c3f6973f5f 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -199,7 +199,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{0A04B1FD-06C doc\adr\graph.dot = doc\adr\graph.dot doc\adr\toc.md = doc\adr\toc.md doc\adr\0005-activity-execution-snapshots.md = doc\adr\0004-activity-execution-snapshots.md - doc\adr\0006-tenant-deleted-event.md = doc\adr\0005-tenant-deleted-event.md + doc\adr\0005-tenant-deleted-event.md = doc\adr\0005-tenant-deleted-event.md doc\adr\0005-token-centric-flowchart-execution-model.md = doc\adr\0005-token-centric-flowchart-execution-model.md doc\adr\0006-tenant-deleted-event.md = doc\adr\0006-tenant-deleted-event.md doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md = doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md From 2bb2fef640e0d028b83460825322addd7640d469 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 08:13:50 +0000 Subject: [PATCH 21/21] Initial plan