From b43695c90dd58cba3be27d6059775c53e348ff97 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 31 Jan 2026 02:59:50 +0100 Subject: [PATCH 1/7] Add ADR for Null Tenant ID, implement tenant-agnostic logic Introduce ADR-0009 to document the use of `null` for tenant-agnostic entities, enhancing multitenancy handling. Update multitenancy features across the codebase, including EF Core query filters and ActivityRegistry, to handle null as a tenant ID, ensuring tenant-agnostic entities are accessible across all tenants. --- Elsa.sln | 1 + ...-tenant-id-for-tenant-agnostic-entities.md | 59 +++++++++++++++ doc/adr/graph.dot | 38 +++++----- doc/adr/toc.md | 3 +- src/apps/Elsa.Server.Web/Program.cs | 2 +- .../Multitenancy/Contracts/ITenantAccessor.cs | 2 + .../Implementations/DefaultTenantAccessor.cs | 2 + .../EntityHandlers/SetTenantIdFilter.cs | 6 +- .../Models/ActivityDescriptor.cs | 2 + .../Services/ActivityRegistry.cs | 72 ++++++++++++------- ...flowDefinitionActivityDescriptorFactory.cs | 8 +-- .../Providers/ClrWorkflowsProvider.cs | 12 +--- .../Helpers/Abstractions/AppComponentTest.cs | 24 +++++-- .../Helpers/Fixtures/WorkflowServer.cs | 13 ++++ .../Services/ComponentTestTenantResolver.cs | 15 ++++ .../WorkflowActivities/DeleteWorkflowTests.cs | 8 +++ .../Workflows/DeleteWorkflow.cs | 1 - 17 files changed, 202 insertions(+), 66 deletions(-) create mode 100644 doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md create mode 100644 test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs diff --git a/Elsa.sln b/Elsa.sln index 0127f2dc4e..2643b6c9c4 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -203,6 +203,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{0A04B1FD-06C 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 + doc\adr\0009-null-tenant-id-for-tenant-agnostic-entities.md = doc\adr\0009-null-tenant-id-for-tenant-agnostic-entities.md EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bounty", "bounty", "{9B80A705-2E31-4012-964A-83963DCDB384}" diff --git a/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md b/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md new file mode 100644 index 0000000000..f165bccba9 --- /dev/null +++ b/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md @@ -0,0 +1,59 @@ +# 9. Null Tenant ID for Tenant-Agnostic Entities + +Date: 2026-01-31 + +## Status + +Accepted + +## Context + +The multitenancy system in Elsa supports both tenant-specific and tenant-agnostic entities. The convention established in ADR-0008 uses an empty string (`""`) as the tenant ID for the default tenant. However, we also need a way to represent entities that are **tenant-agnostic** - entities that should be visible and accessible across all tenants. + +Previously, the system did not properly distinguish between: +1. **Default tenant entities** (should only be visible to the default tenant with `TenantId = ""`) +2. **Tenant-agnostic entities** (should be visible to all tenants regardless of their tenant context) + +This caused issues where: +- Activity descriptors for workflow definitions with empty/null tenant IDs were not accessible when a different tenant context was active +- EF Core query filters only matched records with an exact tenant ID match, excluding tenant-agnostic records +- The `ActivityRegistry` methods did not properly handle tenant-agnostic descriptors + +## Decision + +We will use `null` as the tenant ID to represent **tenant-agnostic entities** - entities that should be accessible across all tenants. This decision includes: + +1. **Convention**: + - `null` = tenant-agnostic (visible to all tenants) + - Empty string `""` = default tenant (visible only to default tenant) + - Any other string = specific tenant (visible only to that tenant) + +2. **EF Core Query Filter**: Update `SetTenantIdFilter` to return records where: + - `TenantId == dbContext.TenantId` (tenant-specific match), OR + - `TenantId == null` (tenant-agnostic records) + +3. **Activity Registry**: Update all query methods to include descriptors where: + - `TenantId == tenantAccessor.TenantId` (tenant-specific match), OR + - `TenantId == null` (tenant-agnostic descriptors) + +4. **Composite Keys**: Make `TenantId` nullable in composite keys (e.g., `(string? TenantId, string Type, int Version)`) + +## Consequences + +### Positive + +- **Tenant-agnostic entities**: System-level entities (like built-in activities) can be shared across all tenants without duplication +- **Proper isolation**: Tenant-specific entities remain isolated to their respective tenants +- **Clear semantics**: `null` explicitly means "no tenant restriction" while empty string means "default tenant" +- **Database efficiency**: Tenant-agnostic entities are stored once and queried once, not duplicated per tenant +- **Consistent behavior**: Both EF Core queries and in-memory collections (like `ActivityRegistry`) follow the same tenant filtering rules + +### Negative + +- **Two conventions**: Developers must understand the distinction between `null` (agnostic) and `""` (default tenant) +- **Nullable handling**: Code must properly handle nullable `TenantId` fields in composite keys and comparisons + +### Neutral + +- This convention aligns with common multitenancy patterns where `null` represents "global" or "shared" resources +- The distinction between default tenant and tenant-agnostic is necessary for proper multitenancy architecture diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index b659a835a3..3ef5641d0e 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -1,20 +1,22 @@ 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. 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]; -} + 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]; + _9 [label="9. Null Tenant ID for Tenant-Agnostic Entities"; URL="0009-null-tenant-id-for-tenant-agnostic-entities.html"]; + _8 -> _9 [style="dotted", weight=1]; + } } \ No newline at end of file diff --git a/doc/adr/toc.md b/doc/adr/toc.md index 2256411817..ac8baa38c9 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -7,4 +7,5 @@ * [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) \ No newline at end of file +* [8. Empty String as Default Tenant ID](0008-empty-string-as-default-tenant-id.md) +* [9. Null Tenant ID for Tenant-Agnostic Entities](0009-null-tenant-id-for-tenant-agnostic-entities.md) \ No newline at end of file diff --git a/src/apps/Elsa.Server.Web/Program.cs b/src/apps/Elsa.Server.Web/Program.cs index 43fd4bce37..a1a8de9c7e 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -31,7 +31,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; diff --git a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantAccessor.cs b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantAccessor.cs index 66dab5b996..133dd2c301 100644 --- a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantAccessor.cs +++ b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantAccessor.cs @@ -2,6 +2,8 @@ namespace Elsa.Common.Multitenancy; public interface ITenantAccessor { + string TenantId { get; } + /// /// Get the current . /// diff --git a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs index 146dfd451b..921d77aa90 100644 --- a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs +++ b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs @@ -7,6 +7,8 @@ public class DefaultTenantAccessor : ITenantAccessor { private static readonly AsyncLocal CurrentTenantField = new(); + public string TenantId => (Tenant?.TenantId).NormalizeTenantId(); + /// public Tenant? Tenant { diff --git a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs index 7ae5eafb30..3992582cfc 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs @@ -25,7 +25,7 @@ private LambdaExpression CreateTenantFilterExpression(ElsaDbContextBase dbContex { var parameter = Expression.Parameter(clrType, "e"); - // e => EF.Property(e, "TenantId") == this.TenantId + // e => EF.Property(e, "TenantId") == this.TenantId || EF.Property(e, "TenantId") == null var tenantIdProperty = Expression.Call( typeof(EF), nameof(EF.Property), @@ -37,7 +37,9 @@ private LambdaExpression CreateTenantFilterExpression(ElsaDbContextBase dbContex Expression.Constant(dbContext), nameof(ElsaDbContextBase.TenantId)); - var body = Expression.Equal(tenantIdProperty, tenantIdOnContext); + var equalityCheck = Expression.Equal(tenantIdProperty, tenantIdOnContext); + var nullCheck = Expression.Equal(tenantIdProperty, Expression.Constant(null, typeof(string))); + var body = Expression.OrElse(equalityCheck, nullCheck); return Expression.Lambda(body, parameter); } diff --git a/src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs b/src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs index 9505667954..ef4856c184 100644 --- a/src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs +++ b/src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs @@ -10,6 +10,8 @@ namespace Elsa.Workflows.Models; [DebuggerDisplay("{TypeName}")] public class ActivityDescriptor { + public string? TenantId { get; set; } // Null means tenant-agnostic. + /// /// The fully qualified name of the activity type. /// diff --git a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs index f5b4482ca1..52bf4c1ad3 100644 --- a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs +++ b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs @@ -1,5 +1,6 @@ using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; +using Elsa.Common.Multitenancy; using Elsa.Workflows.Helpers; using Elsa.Workflows.Models; using Microsoft.Extensions.Logging; @@ -7,11 +8,11 @@ namespace Elsa.Workflows; /// -public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable modifiers, ILogger logger) : IActivityRegistry +public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable modifiers, ITenantAccessor tenantAccessor, ILogger logger) : IActivityRegistry { private readonly ISet _manualActivityDescriptors = new HashSet(); private ConcurrentDictionary> _providedActivityDescriptors = new(); - private ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> _activityDescriptors = new(); + private ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor> _activityDescriptors = new(); /// public void Add(Type providerType, ActivityDescriptor descriptor) => Add(descriptor, GetOrCreateDescriptors(providerType)); @@ -20,26 +21,30 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< public void Remove(Type providerType, ActivityDescriptor descriptor) { _providedActivityDescriptors[providerType].Remove(descriptor); - _activityDescriptors.Remove((descriptor.TypeName, descriptor.Version), out _); + _activityDescriptors.Remove((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out _); } /// - public IEnumerable ListAll() => _activityDescriptors.Values; + public IEnumerable ListAll() => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null); /// - public IEnumerable ListByProvider(Type providerType) => _providedActivityDescriptors.TryGetValue(providerType, out var descriptors) ? descriptors : ArraySegment.Empty; + public IEnumerable ListByProvider(Type providerType) + { + var list = _providedActivityDescriptors.TryGetValue(providerType, out var descriptors) ? descriptors : ArraySegment.Empty; + return list.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null); + } /// - public ActivityDescriptor? Find(string type) => _activityDescriptors.Values.Where(x => x.TypeName == type).MaxBy(x => x.Version); + public ActivityDescriptor? Find(string type) => _activityDescriptors.Values.Where(x => (x.TenantId == tenantAccessor.TenantId || x.TenantId == null) && x.TypeName == type).MaxBy(x => x.Version); /// - public ActivityDescriptor? Find(string type, int version) => _activityDescriptors.TryGetValue((type, version), out var descriptor) ? descriptor : null; + public ActivityDescriptor? Find(string type, int version) => _activityDescriptors.GetValueOrDefault((tenantAccessor.TenantId, type, version)) ?? _activityDescriptors.GetValueOrDefault((null, type, version)); /// - public ActivityDescriptor? Find(Func predicate) => _activityDescriptors.Values.FirstOrDefault(predicate); + public ActivityDescriptor? Find(Func predicate) => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null).FirstOrDefault(predicate); /// - public IEnumerable FindMany(Func predicate) => _activityDescriptors.Values.Where(predicate); + public IEnumerable FindMany(Func predicate) => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null).Where(predicate); /// public void Register(ActivityDescriptor descriptor) @@ -74,13 +79,24 @@ public async Task RegisterAsync(IEnumerable activityTypes, CancellationTok /// public async Task RefreshDescriptorsAsync(IEnumerable activityProviders, CancellationToken cancellationToken = default) { - var providersDictionary = new ConcurrentDictionary>(); - var activityDescriptors = new ConcurrentDictionary<(string Type, int Version), ActivityDescriptor>(_activityDescriptors); + var providersDictionary = new ConcurrentDictionary>(_providedActivityDescriptors); + var activityDescriptors = new ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor>(_activityDescriptors); + foreach (var activityProvider in activityProviders) { + var providerType = activityProvider.GetType(); + + // Remove old descriptors for THIS provider + if (providersDictionary.TryGetValue(providerType, out var oldDescriptors)) + { + foreach (var oldDescriptor in oldDescriptors) + activityDescriptors.TryRemove((oldDescriptor.TenantId, oldDescriptor.TypeName, oldDescriptor.Version), out _); + } + + // Add new descriptors for THIS provider var descriptors = (await activityProvider.GetDescriptorsAsync(cancellationToken)).ToList(); var providerDescriptors = new List(); - providersDictionary[activityProvider.GetType()] = providerDescriptors; + providersDictionary[providerType] = providerDescriptors; foreach (var descriptor in descriptors) { Add(descriptor, activityDescriptors, providerDescriptors); @@ -93,17 +109,25 @@ public async Task RefreshDescriptorsAsync(IEnumerable activit public async Task RefreshDescriptorsAsync(IActivityProvider activityProvider, CancellationToken cancellationToken = default) { - var providersDictionary = new ConcurrentDictionary>(_providedActivityDescriptors); - var activityDescriptors = new ConcurrentDictionary<(string Type, int Version), ActivityDescriptor>(_activityDescriptors); + var providerType = activityProvider.GetType(); + + // Remove ALL old descriptors for this provider from _activityDescriptors + if (_providedActivityDescriptors.TryGetValue(providerType, out var oldDescriptors)) + { + foreach (var oldDescriptor in oldDescriptors) + _activityDescriptors.TryRemove((oldDescriptor.TenantId, oldDescriptor.TypeName, oldDescriptor.Version), out _); + } + + // Get new descriptors from provider var descriptors = (await activityProvider.GetDescriptorsAsync(cancellationToken)).ToList(); - var providerDescriptors = new List(); - providersDictionary[activityProvider.GetType()] = providerDescriptors; + // Add new descriptors + var providerDescriptors = new List(); foreach (var descriptor in descriptors) - Add(descriptor, activityDescriptors, providerDescriptors); + Add(descriptor, _activityDescriptors, providerDescriptors); - Interlocked.Exchange(ref _activityDescriptors, activityDescriptors); - Interlocked.Exchange(ref _providedActivityDescriptors, providersDictionary); + // Update the provider's descriptor list + _providedActivityDescriptors[providerType] = providerDescriptors; } private void Add(ActivityDescriptor descriptor, ICollection target) @@ -111,7 +135,7 @@ private void Add(ActivityDescriptor descriptor, ICollection Add(descriptor, _activityDescriptors, target); } - private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> activityDescriptors, ICollection providerDescriptors) + private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor> activityDescriptors, ICollection providerDescriptors) { if (descriptor is null) { @@ -123,7 +147,7 @@ private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string Ty modifier.Modify(descriptor); // If the descriptor already exists, replace it. But log a warning. - if (activityDescriptors.TryGetValue((descriptor.TypeName, descriptor.Version), out var existingDescriptor)) + if (activityDescriptors.TryGetValue((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out var existingDescriptor)) { // Remove the existing descriptor from the providerDescriptors collection. providerDescriptors.Remove(existingDescriptor); @@ -132,7 +156,7 @@ private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string Ty logger.LogWarning("Activity descriptor {ActivityType} v{ActivityVersion} was already registered. Replacing with new descriptor", descriptor.TypeName, descriptor.Version); } - activityDescriptors[(descriptor.TypeName, descriptor.Version)] = descriptor; + activityDescriptors[(descriptor.TenantId, descriptor.TypeName, descriptor.Version)] = descriptor; providerDescriptors.Add(descriptor); } @@ -149,7 +173,7 @@ public void ClearProvider(Type providerType) var descriptors = ListByProvider(providerType).ToList(); foreach (var descriptor in descriptors) - _activityDescriptors.Remove((descriptor.TypeName, descriptor.Version), out _); + _activityDescriptors.Remove((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out _); _providedActivityDescriptors.Remove(providerType, out _); } @@ -164,4 +188,4 @@ private ICollection GetOrCreateDescriptors(Type provider) return descriptors; } -} +} \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs index 0051c293ac..51f13d7adf 100644 --- a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs @@ -10,11 +10,8 @@ public class WorkflowDefinitionActivityDescriptorFactory { public ActivityDescriptor CreateDescriptor(WorkflowDefinition definition, WorkflowDefinition? latestPublishedDefinition = null) { - var baseName = definition.Name!.Pascalize(); - var tenantId = definition.TenantId.NormalizeTenantId(); - - // Include tenant ID in type name for non-default tenants to ensure uniqueness across tenants - var typeName = string.IsNullOrEmpty(tenantId) ? baseName : $"{tenantId}:{baseName}"; + var typeName = definition.Name!.Pascalize(); + var tenantId = definition.TenantId; var ports = definition.Outcomes.Select(outcome => new Port { @@ -36,6 +33,7 @@ public ActivityDescriptor CreateDescriptor(WorkflowDefinition definition, Workfl return new() { + TenantId = tenantId, TypeName = typeName, ClrType = typeof(WorkflowDefinitionActivity), Name = typeName, diff --git a/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs b/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs index 757df21f4a..debe17fb28 100644 --- a/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs +++ b/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs @@ -1,5 +1,3 @@ -using Elsa.Common.Multitenancy; -using Elsa.Extensions; using Elsa.Workflows.Management.Materializers; using Elsa.Workflows.Runtime.Features; using Elsa.Workflows.Runtime.Options; @@ -15,7 +13,6 @@ namespace Elsa.Workflows.Runtime.Providers; public class ClrWorkflowsProvider( IOptions options, IWorkflowBuilderFactory workflowBuilderFactory, - ITenantAccessor tenantAccessor, IServiceProvider serviceProvider) : IWorkflowsProvider { /// @@ -34,20 +31,17 @@ private async Task BuildWorkflowAsync(Func(); + _tenantScope = tenantAccessor.PushContext(new Tenant { Id = string.Empty, Name = "Default" }); + } void IDisposable.Dispose() { + _tenantScope.Dispose(); Scope.Dispose(); OnDispose(); } diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs index d95d07aafa..d702eba865 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs @@ -17,6 +17,10 @@ using Elsa.Workflows.ComponentTests.WorkflowProviders; using Elsa.Workflows.Management; using Elsa.Workflows.Runtime.Distributed.Extensions; +using Elsa.Tenants; +using Elsa.Tenants.Extensions; +using Elsa.Common.Features; +using Elsa.Workflows.ComponentTests.Services; using FluentStorage; using JetBrains.Annotations; using Medallion.Threading; @@ -126,6 +130,15 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) { http.UseCache(); }); + + // Ensure a consistent tenant context for tests. + elsa.Configure(feature => feature.UseTenantsProvider(_ => new TestTenantsProvider(string.Empty, "Tenant1", "Tenant2", "Tenant3"))); + elsa.UseTenants(tenants => + { + tenants.ConfigureMultitenancy(options => + options.TenantResolverPipelineBuilder = new TenantResolverPipelineBuilder() + .Append()); + }); }; } diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs new file mode 100644 index 0000000000..c7e6fb911b --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs @@ -0,0 +1,15 @@ +using Elsa.Common.Multitenancy; + +namespace Elsa.Workflows.ComponentTests.Services; + +/// +/// A tenant resolver for component tests that resolves to the default/empty tenant. +/// +public class ComponentTestTenantResolver : TenantResolverBase +{ + protected override TenantResolverResult Resolve(TenantResolverContext context) + { + // Resolve to empty string (default tenant) to match workflow definitions without explicit tenants + return AutoResolve(string.Empty); + } +} diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/DeleteWorkflowTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/DeleteWorkflowTests.cs index aa9d9d64c9..2b111e8d43 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/DeleteWorkflowTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/DeleteWorkflowTests.cs @@ -39,6 +39,14 @@ public async Task DeleteWorkflow() var workflowDefinitionManager = _scope1.ServiceProvider.GetRequiredService(); var deletedCount = await workflowDefinitionManager.DeleteByDefinitionIdAsync(Workflows.DeleteWorkflow.DefinitionId); Assert.True(deletedCount > 0, "Expected workflow definition to be deleted."); + + var store = _scope1.ServiceProvider.GetRequiredService(); + var t1 = await store.FindAsync(new WorkflowDefinitionFilter + { + DefinitionId = Workflows.DeleteWorkflow.DefinitionId + }); + + Assert.Null(t1); // Force a refresh of the activity registry to ensure it reflects the deletion var activityRegistry = _scope1.ServiceProvider.GetRequiredService(); diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/Workflows/DeleteWorkflow.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/Workflows/DeleteWorkflow.cs index 12344e26f2..8a9d1e2f03 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/Workflows/DeleteWorkflow.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/Workflows/DeleteWorkflow.cs @@ -12,7 +12,6 @@ protected override void Build(IWorkflowBuilder builder) { builder.Name = Type; builder.WithDefinitionId(DefinitionId); - builder.WithTenantId("Tenant1"); // Use Tenant1 to match the test environment tenant builder.WorkflowOptions.UsableAsActivity = true; builder.Root = new Sequence { From 4c8c8414091a3e7d92cde7cd9b86a864c1778bb1 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 31 Jan 2026 03:02:41 +0100 Subject: [PATCH 2/7] Add multitenancy support in `ActivityTestFixture` by registering `ITenantAccessor`. --- src/common/Elsa.Testing.Shared/ActivityTestFixture.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/Elsa.Testing.Shared/ActivityTestFixture.cs b/src/common/Elsa.Testing.Shared/ActivityTestFixture.cs index bd0399b5e2..77d1c88e43 100644 --- a/src/common/Elsa.Testing.Shared/ActivityTestFixture.cs +++ b/src/common/Elsa.Testing.Shared/ActivityTestFixture.cs @@ -1,4 +1,5 @@ using Elsa.Common; +using Elsa.Common.Multitenancy; using Elsa.Expressions.Contracts; using Elsa.Expressions.Services; using Elsa.Extensions; @@ -180,5 +181,6 @@ private static void AddCoreWorkflowServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); } } \ No newline at end of file From fcb8b972be320635fa1b5fd6d60d25ecbf3f6ba6 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 31 Jan 2026 03:16:52 +0100 Subject: [PATCH 3/7] Update src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Multitenancy/Implementations/DefaultTenantAccessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs index 921d77aa90..6c591d5390 100644 --- a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs +++ b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs @@ -7,7 +7,7 @@ public class DefaultTenantAccessor : ITenantAccessor { private static readonly AsyncLocal CurrentTenantField = new(); - public string TenantId => (Tenant?.TenantId).NormalizeTenantId(); + public string TenantId => (Tenant?.Id).NormalizeTenantId(); /// public Tenant? Tenant From c81dc4408e3afe94418acb19b2d21e11c01b802b Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 31 Jan 2026 13:00:09 +0100 Subject: [PATCH 4/7] Remove unused `using Elsa.Common.Multitenancy;` from WorkflowDefinitionActivityDescriptorFactory (#7230) * Initial plan * Remove unused using Elsa.Common.Multitenancy statement Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> --- .../WorkflowDefinitionActivityDescriptorFactory.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs index 51f13d7adf..224b8dd425 100644 --- a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs @@ -1,4 +1,3 @@ -using Elsa.Common.Multitenancy; using Elsa.Extensions; using Elsa.Workflows.Management.Entities; using Elsa.Workflows.Models; From 55460bd9d090cabbf9fd5e0ee69b945cdf9a2d41 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 31 Jan 2026 14:57:30 +0100 Subject: [PATCH 5/7] Optimize ActivityRegistry.Find to prefer tenant-specific descriptors without performance regression (#7227) * Initial plan * Optimize Find(string type) to prefer tenant-specific descriptors with single-pass iteration Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> * Apply review feedback: combine if statements and add comprehensive unit tests Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> * Refactor tests for DRYness using theories and helper methods Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> * Clean up extra whitespace in test file Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> --- .../Services/ActivityRegistry.cs | 22 ++- .../Services/ActivityRegistryTests.cs | 160 ++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs diff --git a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs index 52bf4c1ad3..5e6bc1b7f0 100644 --- a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs +++ b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs @@ -35,7 +35,27 @@ public IEnumerable ListByProvider(Type providerType) } /// - public ActivityDescriptor? Find(string type) => _activityDescriptors.Values.Where(x => (x.TenantId == tenantAccessor.TenantId || x.TenantId == null) && x.TypeName == type).MaxBy(x => x.Version); + public ActivityDescriptor? Find(string type) + { + var tenantId = tenantAccessor.TenantId; + ActivityDescriptor? tenantSpecific = null; + ActivityDescriptor? tenantAgnostic = null; + + // Single-pass iteration to find both tenant-specific and tenant-agnostic descriptors + foreach (var descriptor in _activityDescriptors.Values) + { + if (descriptor.TypeName != type) + continue; + + if (descriptor.TenantId == tenantId && (tenantSpecific == null || descriptor.Version > tenantSpecific.Version)) + tenantSpecific = descriptor; + else if (descriptor.TenantId == null && (tenantAgnostic == null || descriptor.Version > tenantAgnostic.Version)) + tenantAgnostic = descriptor; + } + + // Prefer tenant-specific over tenant-agnostic + return tenantSpecific ?? tenantAgnostic; + } /// public ActivityDescriptor? Find(string type, int version) => _activityDescriptors.GetValueOrDefault((tenantAccessor.TenantId, type, version)) ?? _activityDescriptors.GetValueOrDefault((null, type, version)); diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs new file mode 100644 index 0000000000..0214b5eb08 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs @@ -0,0 +1,160 @@ +using Elsa.Common.Multitenancy; +using Elsa.Workflows; +using Elsa.Workflows.Models; +using Microsoft.Extensions.Logging; +using NSubstitute; + +namespace Elsa.Workflows.Core.UnitTests.Services; + +/// +/// Unit tests for ActivityRegistry, specifically testing multi-tenant descriptor resolution logic. +/// +public class ActivityRegistryTests +{ + private const string TestActivityType = "TestActivity"; + private const string CurrentTenant = "tenant1"; + + private readonly ITenantAccessor _tenantAccessor; + private readonly IActivityDescriber _activityDescriber; + private readonly ILogger _logger; + private readonly ActivityRegistry _registry; + + public ActivityRegistryTests() + { + _tenantAccessor = Substitute.For(); + _activityDescriber = Substitute.For(); + _logger = Substitute.For>(); + _registry = new ActivityRegistry(_activityDescriber, Array.Empty(), _tenantAccessor, _logger); + + // Set default tenant for all tests + _tenantAccessor.TenantId.Returns(CurrentTenant); + } + + private ActivityDescriptor CreateDescriptor(string typeName, int version, string? tenantId) => + new() + { + TypeName = typeName, + Version = version, + TenantId = tenantId, + Kind = ActivityKind.Action + }; + + private void RegisterDescriptors(params ActivityDescriptor[] descriptors) + { + foreach (var descriptor in descriptors) + _registry.Register(descriptor); + } + + private static void AssertDescriptor(ActivityDescriptor? result, string? expectedTenantId, int expectedVersion) + { + Assert.NotNull(result); + Assert.Equal(expectedTenantId, result.TenantId); + Assert.Equal(expectedVersion, result.Version); + } + + [Fact] + public void Find_TenantSpecificPreferredOverTenantAgnostic_WhenBothExist() + { + // Arrange + var tenantSpecific = CreateDescriptor(TestActivityType, 1, CurrentTenant); + var tenantAgnostic = CreateDescriptor(TestActivityType, 2, null); // Higher version + RegisterDescriptors(tenantSpecific, tenantAgnostic); + + // Act + var result = _registry.Find(TestActivityType); + + // Assert - tenant-specific should be preferred even though it has a lower version + AssertDescriptor(result, CurrentTenant, 1); + } + + [Fact] + public void Find_ReturnsTenantAgnostic_WhenNoTenantSpecificExists() + { + // Arrange + var tenantAgnostic = CreateDescriptor(TestActivityType, 1, null); + RegisterDescriptors(tenantAgnostic); + + // Act + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, null, 1); + } + + [Theory] + [InlineData(1, 2, 3, 3)] // Multiple versions, expect highest + [InlineData(3, 1, 2, 3)] // Out of order registration + [InlineData(1, 1, 1, 1)] // Same version multiple times + public void Find_ReturnsHighestVersionTenantSpecific_WhenMultipleTenantSpecificExist(int v1, int v2, int v3, int expectedVersion) + { + // Arrange + var descriptors = new[] + { + CreateDescriptor(TestActivityType, v1, CurrentTenant), + CreateDescriptor(TestActivityType, v2, CurrentTenant), + CreateDescriptor(TestActivityType, v3, CurrentTenant) + }; + RegisterDescriptors(descriptors); + + // Act + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, CurrentTenant, expectedVersion); + } + + [Theory] + [InlineData(1, 2, 3, 3)] // Multiple versions, expect highest + [InlineData(3, 1, 2, 3)] // Out of order registration + [InlineData(1, 1, 1, 1)] // Same version multiple times + public void Find_ReturnsHighestVersionTenantAgnostic_WhenMultipleTenantAgnosticExist(int v1, int v2, int v3, int expectedVersion) + { + // Arrange + var descriptors = new[] + { + CreateDescriptor(TestActivityType, v1, null), + CreateDescriptor(TestActivityType, v2, null), + CreateDescriptor(TestActivityType, v3, null) + }; + RegisterDescriptors(descriptors); + + // Act + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, null, expectedVersion); + } + + [Fact] + public void Find_ReturnsNull_WhenNoMatchingDescriptorsExist() + { + // Arrange + var otherDescriptor = CreateDescriptor("OtherActivity", 1, CurrentTenant); + RegisterDescriptors(otherDescriptor); + + // Act + var result = _registry.Find("NonExistentActivity"); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Find_IgnoresOtherTenantDescriptors_OnlyReturnsCurrentTenantOrAgnostic() + { + // Arrange + var descriptors = new[] + { + CreateDescriptor(TestActivityType, 1, CurrentTenant), + CreateDescriptor(TestActivityType, 5, "tenant2"), // Much higher version but wrong tenant + CreateDescriptor(TestActivityType, 2, null) + }; + RegisterDescriptors(descriptors); + + // Act + var result = _registry.Find(TestActivityType); + + // Assert - should return tenant1 descriptor (not tenant2, even though it has higher version) + AssertDescriptor(result, CurrentTenant, 1); + } +} From ddffc29bd89a8b5c7ff3975d367de561fc1c819b Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 22:11:54 +0100 Subject: [PATCH 6/7] Fix default tenant data visibility leak by removing NullIfEmpty conversion (#7229) * Initial plan * Remove NullIfEmpty conversion to align with ADR-0008 and ADR-0009 - Updated ElsaDbContextBase to use empty string for default tenant - Updated ApplyTenantId to stop converting empty string to null - Updated TenantAwareDbContextFactory to preserve empty string for default tenant - Updated Store.cs to preserve empty string for default tenant - This ensures: null = tenant-agnostic (visible to all), "" = default tenant Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> * Add database migration to convert null TenantId to empty string for SqlServer - Added Management migration to convert null to "" for WorkflowDefinitions and WorkflowInstances - Added Runtime migration to convert null to "" for all runtime entities - This ensures existing default tenant data is properly migrated per ADR-0008 - Note: Similar migrations needed for PostgreSql, MySql, Sqlite, and Oracle providers Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> * Clarify tenant handling logic in `ElsaDbContextBase` with new ADR references. * Add tenant ID convention analysis documents and quick reference guide * Implement tenant-agnostic functionality across modules - Introduce `AgnosticTenantId` constant to manage tenant-agnostic entities. - Modify entity handling logic to respect tenant-agnostic designations. - Adjust workflow processing to include tenant-agnostic workflows. - Update caching and activity descriptor logic to accommodate the `AgnosticTenantId`. * Refactor tenant management and registry logic in `ActivityRegistry` for improved clarity and separation of tenant-specific and tenant-agnostic activity descriptors. Remove `TestTenantResolver` and update workflow definition handling for tenant support. * Refactor `ActivityRegistry`: prioritize tenant-specific descriptors over tenant-agnostic and simplify descriptor retrieval logic. * Improve async handling in `CommandHandlerInvokerMiddleware` to await tasks without blocking * Update ADR to use asterisk as sentinel value for tenant-agnostic entities Replace the previous convention of using `null` for tenant-agnostic entities with an asterisk (`"*"`) for improved clarity and system architecture. Updated ADR documentation, TOC, and dependency graph accordingly. * Remove migration `ConvertNullTenantIdToEmptyString` and its associated designer file to clean up the codebase. * Refactor `ActivityRegistry`: streamline activity descriptor removal logic and simplify tenant ID checks. * Update Elsa.sln Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Simplify `RefreshDescriptorsAsync` by removing unnecessary local variable `currentTenantId`. * Remove unused `currentTenantId` variable from `ActivityRegistry`. * Add detailed semantic flow and key points to ADR 0009 Document the tenant ID flow from entity creation to query, emphasizing normalization and tenant-agnostic workflows. Update semantic flow diagrams and provide testing considerations for preserving `"*"` values in multi-tenant scenarios. * Remove outdated Tenant ID Analysis and associated documents * Add security-by-default design for tenant-agnostic entities in ADR Enhance Architecture Decision Record to detail explicit requirements for tenant-agnostic database entities, highlighting differences between in-memory activity descriptors and persistent entities. Emphasize importance of setting `TenantId = "*"` to prevent accidental data leakage. * Normalize tenant ID grouping in `ActivityRegistry` to unify null and agnostic IDs, reducing redundant processing. * Refactor `SignalManager`: improve timeout handling and streamline signal task cancellation. * Update src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Refactor tests to use `Tenant.AgnosticTenantId` instead of `null` for tenant-agnostic descriptors. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> Co-authored-by: Sipke Schoorstra Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Elsa.sln | 4 +- ...inel-value-for-tenant-agnostic-entities.md | 224 ++++++++++++++ ...-tenant-id-for-tenant-agnostic-entities.md | 59 ---- doc/adr/graph.dot | 2 +- doc/adr/toc.md | 2 +- ...binding-issue_enumerable-type-converter.md | 0 ...-20_trigger-deletion-exception-handling.md | 0 ...-instance-deletion_runtime-coordination.md | 0 .../CommandHandlerInvokerMiddleware.cs | 4 +- .../Services/SignalManager.cs | 17 +- .../Services/TestTenantResolver.cs | 11 - .../Multitenancy/Entities/Tenant.cs | 5 + .../ElsaDbContextBase.cs | 6 +- .../EntityHandlers/ApplyTenantId.cs | 14 +- .../EntityHandlers/SetTenantIdFilter.cs | 7 +- .../Elsa.Persistence.EFCore.Common/Store.cs | 12 +- .../TenantAwareDbContextFactory.cs | 2 +- ...nvertNullTenantIdToEmptyString.Designer.cs | 235 +++++++++++++++ ...023442_ConvertNullTenantIdToEmptyString.cs | 57 ++++ .../Models/TenantRegistryData.cs | 43 +++ .../Services/ActivityRegistry.cs | 279 ++++++++++++------ .../Stores/CachingWorkflowDefinitionStore.cs | 2 +- ...DefaultWorkflowDefinitionStorePopulator.cs | 15 +- .../Services/ComponentTestTenantResolver.cs | 2 +- .../Services/ActivityRegistryTests.cs | 16 +- 25 files changed, 823 insertions(+), 195 deletions(-) create mode 100644 doc/adr/0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md delete mode 100644 doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md rename {agent-logs => doc/agent-logs}/7019/2025-11-20_configuration-binding-issue_enumerable-type-converter.md (100%) rename {agent-logs => doc/agent-logs}/7077/2025-11-20_trigger-deletion-exception-handling.md (100%) rename {agent-logs => doc/agent-logs}/7077/2025-11-20_workflow-instance-deletion_runtime-coordination.md (100%) delete mode 100644 src/common/Elsa.Testing.Shared.Component/Services/TestTenantResolver.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.cs create mode 100644 src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs diff --git a/Elsa.sln b/Elsa.sln index 2643b6c9c4..4e1d4b622e 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -22,6 +22,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "solution", "solution", "{7D EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "doc", "doc", "{0354F050-3992-4DD4-B0EE-5FBA04AC72B6}" + ProjectSection(SolutionItems) = preProject + EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "modules", "modules", "{5BA4A8FA-F7F4-45B3-AEC8-8886D35AAC79}" EndProject @@ -203,7 +205,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{0A04B1FD-06C 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 - doc\adr\0009-null-tenant-id-for-tenant-agnostic-entities.md = doc\adr\0009-null-tenant-id-for-tenant-agnostic-entities.md + doc\adr\0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md = doc\adr\0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bounty", "bounty", "{9B80A705-2E31-4012-964A-83963DCDB384}" diff --git a/doc/adr/0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md b/doc/adr/0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md new file mode 100644 index 0000000000..3a38a047d7 --- /dev/null +++ b/doc/adr/0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md @@ -0,0 +1,224 @@ +# 9. Asterisk Sentinel Value for Tenant-Agnostic Entities + +Date: 2026-01-31 + +## Status + +Accepted + +## Context + +The multitenancy system in Elsa supports both tenant-specific and tenant-agnostic entities. The convention established in ADR-0008 uses an empty string (`""`) as the tenant ID for the default tenant. However, we also need a way to represent entities that are **tenant-agnostic** - entities that should be visible and accessible across all tenants. + +Previously, the system did not properly distinguish between: +1. **Default tenant entities** (should only be visible to the default tenant with `TenantId = ""`) +2. **Tenant-agnostic entities** (should be visible to all tenants regardless of their tenant context) + +This caused issues where: +- Activity descriptors for built-in activities were being wiped out when different tenants were activated +- The `ActivityRegistry` used a single global dictionary that was replaced during tenant activation via `Interlocked.Exchange`, causing race conditions +- EF Core query filters only matched records with an exact tenant ID match, excluding tenant-agnostic records +- Workflows with no explicit tenant ID could not be found when a specific tenant context was active + +### Why Use a Sentinel Value Instead of Null? + +We considered using `null` as the marker for tenant-agnostic entities, but chose an explicit sentinel value (`"*"`) instead for several reasons: + +1. **Explicit Intent**: A sentinel value makes it crystal clear in code and database queries that an entity is intentionally tenant-agnostic, not accidentally missing a tenant assignment +2. **Simpler Composite Keys**: Avoids nullable handling complexity in composite keys like `(string TenantId, string Type, int Version)` which would become `(string? TenantId, ...)` +3. **Clearer SQL Queries**: Database queries with `TenantId = '*'` are more explicit than `TenantId IS NULL` +4. **Better Logging**: Seeing `"*"` in logs immediately signals tenant-agnostic behavior +5. **Architecture Alignment**: Works seamlessly with the three-dictionary ActivityRegistry architecture where agnostic entities have their own dedicated registry + +## Decision + +We will use the asterisk character (`"*"`) as a sentinel value to represent **tenant-agnostic entities** - entities that should be accessible across all tenants. This decision includes: + +### 1. Convention + +- `"*"` (represented by constant `Tenant.AgnosticTenantId`) = tenant-agnostic (visible to all tenants) +- `""` (represented by constant `Tenant.DefaultTenantId`) = default tenant (visible only to default tenant) +- Any other non-null string = specific tenant (visible only to that tenant) +- `null` = not yet assigned (will be normalized to either agnostic or current tenant by handlers) + +### 2. Activity Registry Architecture + +Implement a **three-dictionary architecture** in `ActivityRegistry` to properly isolate tenant-specific and tenant-agnostic descriptors: + +- **`_tenantRegistries`**: `ConcurrentDictionary` - Per-tenant activity descriptors (e.g., workflow-as-activities) +- **`_agnosticRegistry`**: `TenantRegistryData` - Shared tenant-agnostic descriptors (e.g., built-in activities) +- **`_manualActivityDescriptors`**: `ISet` - Legacy support for manually registered activities + +Key behaviors: +- Descriptors with `TenantId = null` or `TenantId = "*"` are stored in `_agnosticRegistry` +- Descriptors with any other `TenantId` are stored in the corresponding tenant's registry in `_tenantRegistries` +- `RefreshDescriptorsAsync()` updates only the affected tenant's registry, not the entire global dictionary +- Find methods **always prefer tenant-specific descriptors over agnostic ones**, even if agnostic has a higher version number + +### 3. EF Core Query Filter + +Update `SetTenantIdFilter` to return records where: +- `TenantId == dbContext.TenantId` (tenant-specific match), OR +- `TenantId == "*"` (tenant-agnostic records) + +### 4. Entity Handlers + +Update `ApplyTenantId` handler to: +- Preserve `TenantId = "*"` (don't overwrite tenant-agnostic entities) +- Only apply current tenant ID to entities with `TenantId = null` + +**Important: This is a security-by-default design.** Entities with `null` tenant ID are **never** automatically converted to tenant-agnostic (`"*"`). They are always assigned to the current tenant context. To create tenant-agnostic database entities, developers **must explicitly** set `TenantId = "*"`. This prevents accidental data leakage across tenants. + +### 5. Reserved Character Constraint + +The asterisk character `"*"` is **reserved** and cannot be used as an actual tenant ID. Tenant creation and validation logic should reject any attempt to create a tenant with ID `"*"`. + +## Consequences + +### Positive + +- **Explicit tenant-agnostic marking**: The `"*"` sentinel makes intent clear in code, logs, and database +- **Proper tenant isolation**: The three-dictionary architecture prevents tenant activation from wiping out other tenants' descriptors +- **No nullable handling**: Composite keys remain `(string TenantId, ...)` instead of `(string? TenantId, ...)` +- **Tenant precedence**: Tenant-specific descriptors always take precedence over agnostic ones, allowing tenants to override built-in activities +- **Dynamic tenant management**: Tenants can be activated and deactivated at runtime without affecting each other +- **Database efficiency**: Tenant-agnostic entities are stored once and accessible to all tenants +- **Clear SQL queries**: `WHERE TenantId = current_tenant OR TenantId = '*'` is more explicit than null checks +- **Thread safety**: Per-tenant dictionaries eliminate the need for `Interlocked.Exchange` and its race conditions + +### Negative + +- **Reserved character**: The `"*"` character cannot be used as an actual tenant ID (low impact, as tenant IDs are typically alphanumeric) +- **Two conventions**: Developers must understand the distinction between `"*"` (agnostic) and `""` (default tenant) +- **Migration complexity**: Existing systems using `null` for agnostic entities would need data migration + +### Neutral + +- Using a sentinel value for special cases is a common pattern in software architecture +- The distinction between default tenant and tenant-agnostic is fundamental to proper multitenancy design +- The three-dictionary architecture adds complexity but is necessary for correct tenant isolation + +## Implementation Notes + +### Semantic Flow: From Entity Creation to Query + +Understanding how tenant IDs flow through the system is critical: + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Entity Creation / Deserialization │ +├─────────────────────────────────────────────────────────────┤ +│ TenantId = null → Not yet assigned │ +│ TenantId = "*" → Explicitly agnostic │ +│ TenantId = "" → Default tenant │ +│ TenantId = "foo" → Specific tenant "foo" │ +└─────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ ApplyTenantId Handler (before DB save) │ +├─────────────────────────────────────────────────────────────┤ +│ TenantId = "*" → PRESERVED (agnostic) │ +│ TenantId = null → SET to current tenant from context │ +│ TenantId = "" → PRESERVED (default tenant) │ +│ TenantId = "foo" → PRESERVED (specific tenant) │ +└─────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ Database Storage │ +├─────────────────────────────────────────────────────────────┤ +│ TenantId = "*" → Stored as "*" (agnostic) │ +│ TenantId = "" → Stored as "" (default tenant) │ +│ TenantId = "foo" → Stored as "foo" (specific tenant) │ +│ NOTE: No null values in DB after ApplyTenantId handler │ +└─────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ SetTenantIdFilter (EF Core Query) │ +├─────────────────────────────────────────────────────────────┤ +│ Returns: TenantId == current_tenant OR TenantId == "*" │ +│ Result: Tenant-specific records + agnostic records │ +└─────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ ActivityRegistry (In-Memory) │ +├─────────────────────────────────────────────────────────────┤ +│ null or "*" → _agnosticRegistry (shared) │ +│ TenantId="" → _tenantRegistries[""] (default) │ +│ TenantId=X → _tenantRegistries[X] (specific tenant X) │ +└─────────────────────────────────────────────────────────────┘ +``` + +**Key Points:** +- **`null` is transient**: It only exists during entity creation/deserialization before `ApplyTenantId` runs +- **`"*"` is permanent**: Once set, it's preserved and stored in the database as-is +- **`NormalizeTenantId()` converts `null` → `""`**: This ensures `null` becomes the default tenant, NOT agnostic +- **Database has no nulls**: After `ApplyTenantId` handler, all entities have non-null tenant IDs +- **EF Core filters check for `"*"`**: The query filter explicitly compares against the string `"*"`, not null +- **ActivityRegistry accepts both**: For flexibility, in-memory registry treats both `null` and `"*"` as agnostic + +### ActivityRegistry Behavior + +When `Find(string type)` is called: +1. First check the current tenant's registry for matching descriptors +2. If found, return the highest version from the tenant-specific registry +3. Only if no tenant-specific descriptor exists, fall back to the agnostic registry +4. This ensures tenant-specific customizations always take precedence + +The `GetOrCreateRegistry()` method treats both `null` and `"*"` as agnostic: +```csharp +if (tenantId is null or Tenant.AgnosticTenantId) + return _agnosticRegistry; +``` + +This provides flexibility for in-memory operations where activity descriptors might temporarily have `null` tenant IDs before normalization. + +### Activity Descriptors vs Database Entities: Different Rules + +The system treats **in-memory activity descriptors** and **persistent database entities** differently for security and architectural reasons: + +#### In-Memory Activity Descriptors (Ephemeral) +- Created on startup by activity providers +- **Built-in activities** (WriteLine, SetVariable, etc.): Created with `TenantId = null` by `ActivityDescriber` +- **Workflow-as-activities**: Created with `TenantId = definition.TenantId` by `WorkflowDefinitionActivityDescriptorFactory` +- `null` is acceptable here because descriptors are recreated on each startup and mapped to `_agnosticRegistry` +- No security risk: descriptors don't contain sensitive data, just metadata about activity types + +#### Persistent Database Entities (WorkflowDefinition, etc.) +- Stored permanently in the database +- **Must explicitly set `TenantId = "*"`** to be tenant-agnostic +- `TenantId = null` is **never** converted to `"*"` - always assigned to current tenant +- **Security-by-default**: Prevents accidental data leakage across tenants +- A developer who forgets to set `TenantId` creates a tenant-specific entity, not a global one + +**Example - Creating Tenant-Agnostic Workflow:** +```json +{ + "tenantId": "*", + "name": "GlobalApprovalWorkflow", + "description": "Shared across all tenants", + "root": { ... } +} +``` + +**Why This Asymmetry Is Important:** +1. **Safety**: Database entities with null tenant ID default to current tenant (safe) +2. **Explicitness**: Tenant-agnostic entities must be intentional (require `"*"`) +3. **Different lifecycles**: Descriptors are ephemeral, entities are persistent +4. **Backward compatibility**: Built-in activities work without modification + +### Workflow Import Behavior + +When workflows are imported from providers (e.g., blob storage): +- Workflows without an explicit `tenantId` field in their JSON have `TenantId = null` +- During import, these are normalized to the current tenant ID via `NormalizeTenantId()` extension +- When saved to database, `ApplyTenantId` handler assigns the current tenant from context +- To create truly tenant-agnostic workflows, explicitly set `"tenantId": "*"` in the workflow JSON +- The `"*"` value will be preserved through import, save, and query operations + +### Testing Considerations + +- Component tests use the default tenant (`""`) +- Built-in activities use the agnostic marker (`"*"`) +- Tenant-specific tests should create explicit tenant contexts to verify proper isolation +- Unit tests should verify that `"*"` is preserved through save operations +- Integration tests should verify that `"*"` entities are returned for all tenant contexts diff --git a/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md b/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md deleted file mode 100644 index f165bccba9..0000000000 --- a/doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md +++ /dev/null @@ -1,59 +0,0 @@ -# 9. Null Tenant ID for Tenant-Agnostic Entities - -Date: 2026-01-31 - -## Status - -Accepted - -## Context - -The multitenancy system in Elsa supports both tenant-specific and tenant-agnostic entities. The convention established in ADR-0008 uses an empty string (`""`) as the tenant ID for the default tenant. However, we also need a way to represent entities that are **tenant-agnostic** - entities that should be visible and accessible across all tenants. - -Previously, the system did not properly distinguish between: -1. **Default tenant entities** (should only be visible to the default tenant with `TenantId = ""`) -2. **Tenant-agnostic entities** (should be visible to all tenants regardless of their tenant context) - -This caused issues where: -- Activity descriptors for workflow definitions with empty/null tenant IDs were not accessible when a different tenant context was active -- EF Core query filters only matched records with an exact tenant ID match, excluding tenant-agnostic records -- The `ActivityRegistry` methods did not properly handle tenant-agnostic descriptors - -## Decision - -We will use `null` as the tenant ID to represent **tenant-agnostic entities** - entities that should be accessible across all tenants. This decision includes: - -1. **Convention**: - - `null` = tenant-agnostic (visible to all tenants) - - Empty string `""` = default tenant (visible only to default tenant) - - Any other string = specific tenant (visible only to that tenant) - -2. **EF Core Query Filter**: Update `SetTenantIdFilter` to return records where: - - `TenantId == dbContext.TenantId` (tenant-specific match), OR - - `TenantId == null` (tenant-agnostic records) - -3. **Activity Registry**: Update all query methods to include descriptors where: - - `TenantId == tenantAccessor.TenantId` (tenant-specific match), OR - - `TenantId == null` (tenant-agnostic descriptors) - -4. **Composite Keys**: Make `TenantId` nullable in composite keys (e.g., `(string? TenantId, string Type, int Version)`) - -## Consequences - -### Positive - -- **Tenant-agnostic entities**: System-level entities (like built-in activities) can be shared across all tenants without duplication -- **Proper isolation**: Tenant-specific entities remain isolated to their respective tenants -- **Clear semantics**: `null` explicitly means "no tenant restriction" while empty string means "default tenant" -- **Database efficiency**: Tenant-agnostic entities are stored once and queried once, not duplicated per tenant -- **Consistent behavior**: Both EF Core queries and in-memory collections (like `ActivityRegistry`) follow the same tenant filtering rules - -### Negative - -- **Two conventions**: Developers must understand the distinction between `null` (agnostic) and `""` (default tenant) -- **Nullable handling**: Code must properly handle nullable `TenantId` fields in composite keys and comparisons - -### Neutral - -- This convention aligns with common multitenancy patterns where `null` represents "global" or "shared" resources -- The distinction between default tenant and tenant-agnostic is necessary for proper multitenancy architecture diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index 3ef5641d0e..c4a9a1b1d0 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -16,7 +16,7 @@ digraph { _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]; - _9 [label="9. Null Tenant ID for Tenant-Agnostic Entities"; URL="0009-null-tenant-id-for-tenant-agnostic-entities.html"]; + _9 [label="9. Asterisk Sentinel Value for Tenant-Agnostic Entities"; URL="0009-asterisk-sentinel-value-for-tenant-agnostic-entities.html"]; _8 -> _9 [style="dotted", weight=1]; } } \ No newline at end of file diff --git a/doc/adr/toc.md b/doc/adr/toc.md index ac8baa38c9..fc0275e880 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -8,4 +8,4 @@ * [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) -* [9. Null Tenant ID for Tenant-Agnostic Entities](0009-null-tenant-id-for-tenant-agnostic-entities.md) \ No newline at end of file +* [9. Asterisk Sentinel Value for Tenant-Agnostic Entities](0009-asterisk-sentinel-value-for-tenant-agnostic-entities.md) \ No newline at end of file diff --git a/agent-logs/7019/2025-11-20_configuration-binding-issue_enumerable-type-converter.md b/doc/agent-logs/7019/2025-11-20_configuration-binding-issue_enumerable-type-converter.md similarity index 100% rename from agent-logs/7019/2025-11-20_configuration-binding-issue_enumerable-type-converter.md rename to doc/agent-logs/7019/2025-11-20_configuration-binding-issue_enumerable-type-converter.md diff --git a/agent-logs/7077/2025-11-20_trigger-deletion-exception-handling.md b/doc/agent-logs/7077/2025-11-20_trigger-deletion-exception-handling.md similarity index 100% rename from agent-logs/7077/2025-11-20_trigger-deletion-exception-handling.md rename to doc/agent-logs/7077/2025-11-20_trigger-deletion-exception-handling.md diff --git a/agent-logs/7077/2025-11-20_workflow-instance-deletion_runtime-coordination.md b/doc/agent-logs/7077/2025-11-20_workflow-instance-deletion_runtime-coordination.md similarity index 100% rename from agent-logs/7077/2025-11-20_workflow-instance-deletion_runtime-coordination.md rename to doc/agent-logs/7077/2025-11-20_workflow-instance-deletion_runtime-coordination.md diff --git a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs index 645181b95c..647fe4b58e 100644 --- a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs +++ b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs @@ -41,8 +41,10 @@ public async ValueTask InvokeAsync(CommandContext context) // Execute command. var task = executeMethodWithReturnType.Invoke(strategy, [strategyContext]); - // Get the result of the task. + // Await the task to get the result without blocking. var taskWithReturnType = typeof(Task<>).MakeGenericType(resultType); + var taskInstance = (Task)task!; + await taskInstance.ConfigureAwait(false); var resultProperty = taskWithReturnType.GetProperty(nameof(Task.Result))!; context.Result = resultProperty.GetValue(task); diff --git a/src/common/Elsa.Testing.Shared.Component/Services/SignalManager.cs b/src/common/Elsa.Testing.Shared.Component/Services/SignalManager.cs index b9b585f666..d7e44f7aac 100644 --- a/src/common/Elsa.Testing.Shared.Component/Services/SignalManager.cs +++ b/src/common/Elsa.Testing.Shared.Component/Services/SignalManager.cs @@ -20,17 +20,18 @@ public async Task WaitAsync(object signal, int millisecondsTimeout = 60000 { var taskCompletionSource = GetOrCreate(signal); using var cancellationTokenSource = new CancellationTokenSource(millisecondsTimeout); - try + var delayTask = Task.Delay(millisecondsTimeout, cancellationTokenSource.Token); + var completedTask = await Task.WhenAny(taskCompletionSource.Task, delayTask); + + if (completedTask == delayTask) { - await Task.WhenAny(taskCompletionSource.Task, Task.Delay(millisecondsTimeout, cancellationTokenSource.Token)); - cancellationTokenSource.Token.ThrowIfCancellationRequested(); _signals.TryRemove(signal, out _); - return await taskCompletionSource.Task; - } - catch (OperationCanceledException) - { throw new TimeoutException($"Signal '{signal}' timed out after {millisecondsTimeout} milliseconds."); } + + cancellationTokenSource.Cancel(); + _signals.TryRemove(signal, out _); + return await taskCompletionSource.Task; } public void Trigger(object signal, object? result = null) @@ -45,6 +46,6 @@ public void Trigger(object signal, object? result = null) private TaskCompletionSource GetOrCreate(object eventName) { - return _signals.GetOrAdd(eventName, _ => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + return _signals.GetOrAdd(eventName, _ => new(TaskCreationOptions.RunContinuationsAsynchronously)); } } \ No newline at end of file diff --git a/src/common/Elsa.Testing.Shared.Component/Services/TestTenantResolver.cs b/src/common/Elsa.Testing.Shared.Component/Services/TestTenantResolver.cs deleted file mode 100644 index 042db6ef65..0000000000 --- a/src/common/Elsa.Testing.Shared.Component/Services/TestTenantResolver.cs +++ /dev/null @@ -1,11 +0,0 @@ -using Elsa.Common.Multitenancy; - -namespace Elsa.Testing.Shared.Services; - -public class TestTenantResolver : TenantResolverBase -{ - protected override TenantResolverResult Resolve(TenantResolverContext context) - { - return AutoResolve("Tenant1"); - } -} \ No newline at end of file diff --git a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs index 6d3632d515..a70bbeeecd 100644 --- a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs +++ b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs @@ -15,6 +15,11 @@ public class Tenant : Entity /// public const string DefaultTenantId = ""; + /// + /// The ID used for tenant-agnostic entities that are available to all tenants. + /// + public const string AgnosticTenantId = "*"; + /// /// Gets or sets the name. /// diff --git a/src/modules/Elsa.Persistence.EFCore.Common/ElsaDbContextBase.cs b/src/modules/Elsa.Persistence.EFCore.Common/ElsaDbContextBase.cs index ce6e6d2994..ea43a8ff32 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/ElsaDbContextBase.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/ElsaDbContextBase.cs @@ -47,10 +47,8 @@ protected ElsaDbContextBase(DbContextOptions options, IServiceProvider servicePr Schema = !string.IsNullOrWhiteSpace(_elsaDbContextOptions?.SchemaName) ? _elsaDbContextOptions.SchemaName : ElsaSchema; var tenantAccessor = serviceProvider.GetService(); - var tenantId = tenantAccessor?.Tenant?.Id; - - if (!string.IsNullOrWhiteSpace(tenantId)) - TenantId = tenantId.NullIfEmpty(); + var tenantId = (tenantAccessor?.TenantId).NormalizeTenantId(); + TenantId ??= tenantId; } /// diff --git a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/ApplyTenantId.cs b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/ApplyTenantId.cs index 3d0fb77ecc..24a6ca7a28 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/ApplyTenantId.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/ApplyTenantId.cs @@ -1,5 +1,5 @@ using Elsa.Common.Entities; -using Elsa.Extensions; +using Elsa.Common.Multitenancy; using Microsoft.EntityFrameworkCore.ChangeTracking; namespace Elsa.Persistence.EFCore.EntityHandlers; @@ -12,8 +12,16 @@ public class ApplyTenantId : IEntitySavingHandler /// public ValueTask HandleAsync(ElsaDbContextBase dbContext, EntityEntry entry, CancellationToken cancellationToken = default) { - if (entry.Entity is Entity entity) - entity.TenantId = dbContext.TenantId.NullIfEmpty(); + if (entry.Entity is Entity entity) + { + // Don't touch tenant-agnostic entities (marked with "*") + if (entity.TenantId == Tenant.AgnosticTenantId) + return default; + + // Apply current tenant ID to entities without one + if (entity.TenantId == null && dbContext.TenantId != null) + entity.TenantId = dbContext.TenantId; + } return default; } diff --git a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs index 3992582cfc..eeb4741c53 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs @@ -1,5 +1,6 @@ using System.Linq.Expressions; using Elsa.Common.Entities; +using Elsa.Common.Multitenancy; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Metadata; @@ -25,7 +26,7 @@ private LambdaExpression CreateTenantFilterExpression(ElsaDbContextBase dbContex { var parameter = Expression.Parameter(clrType, "e"); - // e => EF.Property(e, "TenantId") == this.TenantId || EF.Property(e, "TenantId") == null + // e => EF.Property(e, "TenantId") == this.TenantId || EF.Property(e, "TenantId") == "*" var tenantIdProperty = Expression.Call( typeof(EF), nameof(EF.Property), @@ -38,8 +39,8 @@ private LambdaExpression CreateTenantFilterExpression(ElsaDbContextBase dbContex nameof(ElsaDbContextBase.TenantId)); var equalityCheck = Expression.Equal(tenantIdProperty, tenantIdOnContext); - var nullCheck = Expression.Equal(tenantIdProperty, Expression.Constant(null, typeof(string))); - var body = Expression.OrElse(equalityCheck, nullCheck); + var agnosticCheck = Expression.Equal(tenantIdProperty, Expression.Constant(Tenant.AgnosticTenantId, typeof(string))); + var body = Expression.OrElse(equalityCheck, agnosticCheck); return Expression.Lambda(body, parameter); } diff --git a/src/modules/Elsa.Persistence.EFCore.Common/Store.cs b/src/modules/Elsa.Persistence.EFCore.Common/Store.cs index 0ac58c34ba..680e2ca7a8 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/Store.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/Store.cs @@ -182,11 +182,19 @@ public async Task SaveManyAsync( } // When doing a custom SQL query (Bulk Upsert), none of the installed query filters will be applied. Hence, we are assigning the current tenant ID explicitly. - var tenantId = serviceProvider.GetRequiredService().Tenant?.Id.NullIfEmpty(); + var tenantId = serviceProvider.GetRequiredService().Tenant?.Id; foreach (var entity in entityList) { if (entity is Entity entityWithTenant) - entityWithTenant.TenantId = tenantId; + { + // Don't touch tenant-agnostic entities (marked with "*") + if (entityWithTenant.TenantId == Tenant.AgnosticTenantId) + continue; + + // Apply current tenant ID to entities without one + if (entityWithTenant.TenantId == null && tenantId != null) + entityWithTenant.TenantId = tenantId; + } } try diff --git a/src/modules/Elsa.Persistence.EFCore.Common/TenantAwareDbContextFactory.cs b/src/modules/Elsa.Persistence.EFCore.Common/TenantAwareDbContextFactory.cs index b0aacfe444..8128553ce3 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/TenantAwareDbContextFactory.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/TenantAwareDbContextFactory.cs @@ -32,6 +32,6 @@ public async Task CreateDbContextAsync(CancellationToken cancellatio private void SetTenantId(TDbContext context) { if (context is ElsaDbContextBase elsaContext) - elsaContext.TenantId = tenantAccessor.Tenant?.Id.NullIfEmpty(); + elsaContext.TenantId = tenantAccessor.Tenant?.Id; } } \ No newline at end of file diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.Designer.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.Designer.cs new file mode 100644 index 0000000000..5884c74c8a --- /dev/null +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.Designer.cs @@ -0,0 +1,235 @@ +// +using System; +using Elsa.Persistence.EFCore.Modules.Management; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Elsa.Persistence.EFCore.SqlServer.Migrations.Management +{ + [DbContext(typeof(ManagementElsaDbContext))] + [Migration("20260131023442_ConvertNullTenantIdToEmptyString")] + partial class ConvertNullTenantIdToEmptyString + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "9.0.10") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder); + + modelBuilder.Entity("Elsa.Workflows.Management.Entities.WorkflowDefinition", b => + { + b.Property("Id") + .HasColumnType("nvarchar(450)"); + + b.Property("BinaryData") + .HasColumnType("varbinary(max)"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Data") + .HasColumnType("nvarchar(max)"); + + b.Property("DefinitionId") + .IsRequired() + .HasColumnType("nvarchar(450)"); + + b.Property("Description") + .HasColumnType("nvarchar(max)"); + + b.Property("IsLatest") + .HasColumnType("bit"); + + b.Property("IsPublished") + .HasColumnType("bit"); + + b.Property("IsReadonly") + .HasColumnType("bit"); + + b.Property("IsSystem") + .HasColumnType("bit"); + + b.Property("MaterializerContext") + .HasColumnType("nvarchar(max)"); + + b.Property("MaterializerName") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.Property("Name") + .HasColumnType("nvarchar(450)"); + + b.Property("OriginalSource") + .HasColumnType("nvarchar(max)"); + + b.Property("ProviderName") + .HasColumnType("nvarchar(max)"); + + b.Property("StringData") + .HasColumnType("nvarchar(max)"); + + b.Property("TenantId") + .HasColumnType("nvarchar(450)"); + + b.Property("ToolVersion") + .HasColumnType("nvarchar(max)"); + + b.Property("UsableAsActivity") + .HasColumnType("bit"); + + b.Property("Version") + .HasColumnType("int"); + + b.HasKey("Id"); + + b.HasIndex("IsLatest") + .HasDatabaseName("IX_WorkflowDefinition_IsLatest"); + + b.HasIndex("IsPublished") + .HasDatabaseName("IX_WorkflowDefinition_IsPublished"); + + b.HasIndex("IsSystem") + .HasDatabaseName("IX_WorkflowDefinition_IsSystem"); + + b.HasIndex("Name") + .HasDatabaseName("IX_WorkflowDefinition_Name"); + + b.HasIndex("TenantId") + .HasDatabaseName("IX_WorkflowDefinition_TenantId"); + + b.HasIndex("UsableAsActivity") + .HasDatabaseName("IX_WorkflowDefinition_UsableAsActivity"); + + b.HasIndex("Version") + .HasDatabaseName("IX_WorkflowDefinition_Version"); + + b.HasIndex("DefinitionId", "Version") + .IsUnique() + .HasDatabaseName("IX_WorkflowDefinition_DefinitionId_Version"); + + b.ToTable("WorkflowDefinitions", "Elsa"); + }); + + modelBuilder.Entity("Elsa.Workflows.Management.Entities.WorkflowInstance", b => + { + b.Property("Id") + .HasColumnType("nvarchar(450)"); + + b.Property("CorrelationId") + .HasColumnType("nvarchar(450)"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Data") + .HasColumnType("nvarchar(max)"); + + b.Property("DataCompressionAlgorithm") + .HasColumnType("nvarchar(max)"); + + b.Property("DefinitionId") + .IsRequired() + .HasColumnType("nvarchar(450)"); + + b.Property("DefinitionVersionId") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.Property("FinishedAt") + .HasColumnType("datetimeoffset"); + + b.Property("IncidentCount") + .HasColumnType("int"); + + b.Property("IsExecuting") + .HasColumnType("bit"); + + b.Property("IsSystem") + .HasColumnType("bit"); + + b.Property("Name") + .HasColumnType("nvarchar(450)"); + + b.Property("ParentWorkflowInstanceId") + .HasColumnType("nvarchar(max)"); + + b.Property("Status") + .IsRequired() + .HasColumnType("nvarchar(450)"); + + b.Property("SubStatus") + .IsRequired() + .HasColumnType("nvarchar(450)"); + + b.Property("TenantId") + .HasColumnType("nvarchar(450)"); + + b.Property("UpdatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Version") + .HasColumnType("int"); + + b.HasKey("Id"); + + b.HasIndex("CorrelationId") + .HasDatabaseName("IX_WorkflowInstance_CorrelationId"); + + b.HasIndex("CreatedAt") + .HasDatabaseName("IX_WorkflowInstance_CreatedAt"); + + b.HasIndex("DefinitionId") + .HasDatabaseName("IX_WorkflowInstance_DefinitionId"); + + b.HasIndex("FinishedAt") + .HasDatabaseName("IX_WorkflowInstance_FinishedAt"); + + b.HasIndex("IsExecuting") + .HasDatabaseName("IX_WorkflowInstance_IsExecuting"); + + b.HasIndex("IsSystem") + .HasDatabaseName("IX_WorkflowInstance_IsSystem"); + + b.HasIndex("Name") + .HasDatabaseName("IX_WorkflowInstance_Name"); + + b.HasIndex("Status") + .HasDatabaseName("IX_WorkflowInstance_Status"); + + b.HasIndex("SubStatus") + .HasDatabaseName("IX_WorkflowInstance_SubStatus"); + + b.HasIndex("TenantId") + .HasDatabaseName("IX_WorkflowInstance_TenantId"); + + b.HasIndex("UpdatedAt") + .HasDatabaseName("IX_WorkflowInstance_UpdatedAt"); + + b.HasIndex("Status", "DefinitionId") + .HasDatabaseName("IX_WorkflowInstance_Status_DefinitionId"); + + b.HasIndex("Status", "SubStatus") + .HasDatabaseName("IX_WorkflowInstance_Status_SubStatus"); + + b.HasIndex("SubStatus", "DefinitionId") + .HasDatabaseName("IX_WorkflowInstance_SubStatus_DefinitionId"); + + b.HasIndex("Status", "SubStatus", "DefinitionId", "Version") + .HasDatabaseName("IX_WorkflowInstance_Status_SubStatus_DefinitionId_Version"); + + b.ToTable("WorkflowInstances", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.cs new file mode 100644 index 0000000000..6f55c97144 --- /dev/null +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.cs @@ -0,0 +1,57 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Persistence.EFCore.SqlServer.Migrations.Management +{ + /// + public partial class ConvertNullTenantIdToEmptyString : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public ConvertNullTenantIdToEmptyString(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + // Convert null TenantId values to empty string for default tenant entities + // This aligns with ADR-0008 (empty string = default tenant) and ADR-0009 (null = tenant-agnostic) + // All existing null values are assumed to be default tenant data from before the tenant-agnostic feature was introduced + + migrationBuilder.Sql($@" + UPDATE [{_schema.Schema}].[WorkflowDefinitions] + SET TenantId = '' + WHERE TenantId IS NULL + "); + + migrationBuilder.Sql($@" + UPDATE [{_schema.Schema}].[WorkflowInstances] + SET TenantId = '' + WHERE TenantId IS NULL + "); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + // Revert empty string TenantId values back to null + // Note: This may cause issues with the tenant-agnostic feature if run after new tenant-agnostic entities are created + + migrationBuilder.Sql($@" + UPDATE [{_schema.Schema}].[WorkflowDefinitions] + SET TenantId = NULL + WHERE TenantId = '' + "); + + migrationBuilder.Sql($@" + UPDATE [{_schema.Schema}].[WorkflowInstances] + SET TenantId = NULL + WHERE TenantId = '' + "); + } + } +} diff --git a/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs b/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs new file mode 100644 index 0000000000..cc42574e14 --- /dev/null +++ b/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs @@ -0,0 +1,43 @@ +using System.Collections.Concurrent; + +namespace Elsa.Workflows.Models; + +/// +/// Holds the per-tenant activity descriptor dictionaries that back the activity registry. +/// +/// +/// +/// This model represents the value stored in the tenant-level registry dictionary described in ADR-0009's +/// three-dictionary architecture. The outermost dictionary typically maps a tenant identifier (or a value +/// representing tenant-agnostic scope) to an instance of . Within this class, +/// the and properties form the +/// inner dictionaries that index activity descriptors for that specific tenant or for tenant-agnostic activities. +/// +/// +/// By encapsulating these dictionaries, the registry can manage activity descriptors per tenant while maintaining +/// a consistent lookup and invalidation strategy across the entire system. +/// +/// +public class TenantRegistryData +{ + /// + /// Primary index of activity descriptors for this tenant (or for tenant-agnostic scope). + /// + /// + /// The key is a composite of the activity Type (a logical activity type identifier) and its + /// Version. This allows efficient lookup of a specific activity descriptor by type and version, + /// which is the most common access pattern when compiling or executing workflows. + /// + public ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> ActivityDescriptors { get; } = new(); + + /// + /// Secondary index of activity descriptors grouped by their provider type. + /// + /// + /// This dictionary maps a provider (for example, an activity provider implementation) + /// to the collection of instances contributed by that provider for this + /// tenant. It complements by enabling provider-centric operations such as + /// refreshing, removing, or re-registering all descriptors originating from a given provider. + /// + public ConcurrentDictionary> ProvidedActivityDescriptors { get; } = new(); +} diff --git a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs index 5e6bc1b7f0..3357e9140e 100644 --- a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs +++ b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs @@ -10,66 +10,148 @@ namespace Elsa.Workflows; /// public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable modifiers, ITenantAccessor tenantAccessor, ILogger logger) : IActivityRegistry { + // Legacy support for manually registered activities private readonly ISet _manualActivityDescriptors = new HashSet(); - private ConcurrentDictionary> _providedActivityDescriptors = new(); - private ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor> _activityDescriptors = new(); + + // Per-tenant activity descriptors (workflow-as-activities, tenant-specific providers, etc.) + private readonly ConcurrentDictionary _tenantRegistries = new(); + + // Tenant-agnostic activity descriptors (built-in activities, manually registered, etc.) + private readonly TenantRegistryData _agnosticRegistry = new(); /// - public void Add(Type providerType, ActivityDescriptor descriptor) => Add(descriptor, GetOrCreateDescriptors(providerType)); + public void Add(Type providerType, ActivityDescriptor descriptor) + { + var registry = GetOrCreateRegistry(descriptor.TenantId); + var providerDescriptors = GetOrCreateProviderDescriptors(registry, providerType); + Add(descriptor, registry.ActivityDescriptors, providerDescriptors); + } /// public void Remove(Type providerType, ActivityDescriptor descriptor) { - _providedActivityDescriptors[providerType].Remove(descriptor); - _activityDescriptors.Remove((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out _); + var registry = GetOrCreateRegistry(descriptor.TenantId); + if (registry.ProvidedActivityDescriptors.TryGetValue(providerType, out var providerDescriptors)) + { + providerDescriptors.Remove(descriptor); + registry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); + } } /// - public IEnumerable ListAll() => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null); + public IEnumerable ListAll() + { + var currentTenantId = tenantAccessor.TenantId; + + // Get descriptors from current tenant's registry + var tenantDescriptors = _tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry) + ? tenantRegistry.ActivityDescriptors.Values + : Enumerable.Empty(); + + // Get descriptors from agnostic registry + var agnosticDescriptors = _agnosticRegistry.ActivityDescriptors.Values; + + return tenantDescriptors.Concat(agnosticDescriptors); + } /// public IEnumerable ListByProvider(Type providerType) { - var list = _providedActivityDescriptors.TryGetValue(providerType, out var descriptors) ? descriptors : ArraySegment.Empty; - return list.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null); + var currentTenantId = tenantAccessor.TenantId; + + // Get descriptors from current tenant's registry + var tenantDescriptors = _tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry) && + tenantRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var tenantProviderDescriptors) + ? tenantProviderDescriptors + : Enumerable.Empty(); + + // Get descriptors from agnostic registry + var agnosticDescriptors = _agnosticRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var agnosticProviderDescriptors) + ? agnosticProviderDescriptors + : Enumerable.Empty(); + + return tenantDescriptors.Concat(agnosticDescriptors); } /// public ActivityDescriptor? Find(string type) { - var tenantId = tenantAccessor.TenantId; - ActivityDescriptor? tenantSpecific = null; - ActivityDescriptor? tenantAgnostic = null; + var currentTenantId = tenantAccessor.TenantId; - // Single-pass iteration to find both tenant-specific and tenant-agnostic descriptors - foreach (var descriptor in _activityDescriptors.Values) + // Always prefer tenant-specific descriptors over tenant-agnostic ones + // Get highest version from current tenant's registry + if (_tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry)) { - if (descriptor.TypeName != type) - continue; + var tenantDescriptor = tenantRegistry.ActivityDescriptors.Values + .Where(x => x.TypeName == type) + .MaxBy(x => x.Version); - if (descriptor.TenantId == tenantId && (tenantSpecific == null || descriptor.Version > tenantSpecific.Version)) - tenantSpecific = descriptor; - else if (descriptor.TenantId == null && (tenantAgnostic == null || descriptor.Version > tenantAgnostic.Version)) - tenantAgnostic = descriptor; + if (tenantDescriptor != null) + return tenantDescriptor; } - // Prefer tenant-specific over tenant-agnostic - return tenantSpecific ?? tenantAgnostic; + // Fall back to agnostic registry only if no tenant-specific descriptor exists + return _agnosticRegistry.ActivityDescriptors.Values + .Where(x => x.TypeName == type) + .MaxBy(x => x.Version); } /// - public ActivityDescriptor? Find(string type, int version) => _activityDescriptors.GetValueOrDefault((tenantAccessor.TenantId, type, version)) ?? _activityDescriptors.GetValueOrDefault((null, type, version)); + public ActivityDescriptor? Find(string type, int version) + { + var currentTenantId = tenantAccessor.TenantId; + + // Check current tenant's registry first + if (_tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry) && + tenantRegistry.ActivityDescriptors.TryGetValue((type, version), out var tenantDescriptor)) + { + return tenantDescriptor; + } + + // Fall back to agnostic registry + return _agnosticRegistry.ActivityDescriptors.TryGetValue((type, version), out var agnosticDescriptor) + ? agnosticDescriptor + : null; + } /// - public ActivityDescriptor? Find(Func predicate) => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null).FirstOrDefault(predicate); + public ActivityDescriptor? Find(Func predicate) + { + var currentTenantId = tenantAccessor.TenantId; + + // Check current tenant's registry first + if (_tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry)) + { + var tenantMatch = tenantRegistry.ActivityDescriptors.Values.FirstOrDefault(predicate); + if (tenantMatch != null) return tenantMatch; + } + + // Fall back to agnostic registry + return _agnosticRegistry.ActivityDescriptors.Values.FirstOrDefault(predicate); + } /// - public IEnumerable FindMany(Func predicate) => _activityDescriptors.Values.Where(x => x.TenantId == tenantAccessor.TenantId || x.TenantId == null).Where(predicate); + public IEnumerable FindMany(Func predicate) + { + var currentTenantId = tenantAccessor.TenantId; + + // Get descriptors from current tenant's registry + var tenantDescriptors = _tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry) + ? tenantRegistry.ActivityDescriptors.Values.Where(predicate) + : Enumerable.Empty(); + + // Get descriptors from agnostic registry + var agnosticDescriptors = _agnosticRegistry.ActivityDescriptors.Values.Where(predicate); + + return tenantDescriptors.Concat(agnosticDescriptors); + } /// public void Register(ActivityDescriptor descriptor) { - Add(GetType(), descriptor); + var registry = GetOrCreateRegistry(descriptor.TenantId); + var providerDescriptors = GetOrCreateProviderDescriptors(registry, GetType()); + Add(descriptor, registry.ActivityDescriptors, providerDescriptors); } /// @@ -77,12 +159,14 @@ public async Task RegisterAsync([DynamicallyAccessedMembers(DynamicallyAccessedM { var activityTypeName = ActivityTypeNameHelper.GenerateTypeName(activityType); - if (_activityDescriptors.Values.Any(x => x.TypeName == activityTypeName)) + // Check if already registered in any registry + if (ListAll().Any(x => x.TypeName == activityTypeName)) return; var activityDescriptor = await activityDescriber.DescribeActivityAsync(activityType, cancellationToken); - Add(activityDescriptor, _activityDescriptors, _manualActivityDescriptors); + var registry = GetOrCreateRegistry(activityDescriptor.TenantId); + Add(activityDescriptor, registry.ActivityDescriptors, _manualActivityDescriptors); _manualActivityDescriptors.Add(activityDescriptor); } @@ -99,63 +183,48 @@ public async Task RegisterAsync(IEnumerable activityTypes, CancellationTok /// public async Task RefreshDescriptorsAsync(IEnumerable activityProviders, CancellationToken cancellationToken = default) { - var providersDictionary = new ConcurrentDictionary>(_providedActivityDescriptors); - var activityDescriptors = new ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor>(_activityDescriptors); - - foreach (var activityProvider in activityProviders) - { - var providerType = activityProvider.GetType(); - - // Remove old descriptors for THIS provider - if (providersDictionary.TryGetValue(providerType, out var oldDescriptors)) - { - foreach (var oldDescriptor in oldDescriptors) - activityDescriptors.TryRemove((oldDescriptor.TenantId, oldDescriptor.TypeName, oldDescriptor.Version), out _); - } - - // Add new descriptors for THIS provider - var descriptors = (await activityProvider.GetDescriptorsAsync(cancellationToken)).ToList(); - var providerDescriptors = new List(); - providersDictionary[providerType] = providerDescriptors; - foreach (var descriptor in descriptors) - { - Add(descriptor, activityDescriptors, providerDescriptors); - } - } - - Interlocked.Exchange(ref _activityDescriptors, activityDescriptors); - Interlocked.Exchange(ref _providedActivityDescriptors, providersDictionary); + foreach (var activityProvider in activityProviders) + await RefreshDescriptorsAsync(activityProvider, cancellationToken); } public async Task RefreshDescriptorsAsync(IActivityProvider activityProvider, CancellationToken cancellationToken = default) { var providerType = activityProvider.GetType(); - // Remove ALL old descriptors for this provider from _activityDescriptors - if (_providedActivityDescriptors.TryGetValue(providerType, out var oldDescriptors)) - { - foreach (var oldDescriptor in oldDescriptors) - _activityDescriptors.TryRemove((oldDescriptor.TenantId, oldDescriptor.TypeName, oldDescriptor.Version), out _); - } - // Get new descriptors from provider var descriptors = (await activityProvider.GetDescriptorsAsync(cancellationToken)).ToList(); - // Add new descriptors - var providerDescriptors = new List(); - foreach (var descriptor in descriptors) - Add(descriptor, _activityDescriptors, providerDescriptors); + // Group descriptors by normalized tenant ID + // Normalize null to "*" so both map to the same agnostic group, avoiding redundant processing + var descriptorsByTenant = descriptors.GroupBy(d => NormalizeTenantIdForGrouping(d.TenantId)); - // Update the provider's descriptor list - _providedActivityDescriptors[providerType] = providerDescriptors; - } + foreach (var group in descriptorsByTenant) + { + var tenantId = group.Key; + var registry = GetOrCreateRegistry(tenantId); - private void Add(ActivityDescriptor descriptor, ICollection target) - { - Add(descriptor, _activityDescriptors, target); + // Remove old descriptors for this provider from this tenant's registry + if (registry.ProvidedActivityDescriptors.TryGetValue(providerType, out var oldDescriptors)) + { + foreach (var oldDescriptor in oldDescriptors.ToList()) + { + registry.ActivityDescriptors.TryRemove((oldDescriptor.TypeName, oldDescriptor.Version), out _); + } + } + + // Add new descriptors for this tenant + var providerDescriptors = new List(); + foreach (var descriptor in group) + { + Add(descriptor, registry.ActivityDescriptors, providerDescriptors); + } + + // Update the provider's descriptor list in this registry + registry.ProvidedActivityDescriptors[providerType] = providerDescriptors; + } } - private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string? TenantId, string Type, int Version), ActivityDescriptor> activityDescriptors, ICollection providerDescriptors) + private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> activityDescriptors, ICollection providerDescriptors) { if (descriptor is null) { @@ -167,45 +236,83 @@ private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string? T modifier.Modify(descriptor); // If the descriptor already exists, replace it. But log a warning. - if (activityDescriptors.TryGetValue((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out var existingDescriptor)) + if (activityDescriptors.TryGetValue((descriptor.TypeName, descriptor.Version), out var existingDescriptor)) { // Remove the existing descriptor from the providerDescriptors collection. providerDescriptors.Remove(existingDescriptor); // Log a warning. - logger.LogWarning("Activity descriptor {ActivityType} v{ActivityVersion} was already registered. Replacing with new descriptor", descriptor.TypeName, descriptor.Version); + logger.LogWarning("Activity descriptor {ActivityType} v{ActivityVersion} was already registered for tenant {TenantId}. Replacing with new descriptor", descriptor.TypeName, descriptor.Version, descriptor.TenantId); } - activityDescriptors[(descriptor.TenantId, descriptor.TypeName, descriptor.Version)] = descriptor; + activityDescriptors[(descriptor.TypeName, descriptor.Version)] = descriptor; providerDescriptors.Add(descriptor); } /// public void Clear() { - _activityDescriptors.Clear(); - _providedActivityDescriptors.Clear(); + _tenantRegistries.Clear(); + _agnosticRegistry.ActivityDescriptors.Clear(); + _agnosticRegistry.ProvidedActivityDescriptors.Clear(); } /// public void ClearProvider(Type providerType) { - var descriptors = ListByProvider(providerType).ToList(); + var currentTenantId = tenantAccessor.TenantId; + + // Clear from current tenant's registry + if (_tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry) + && tenantRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var descriptors)) + { + foreach (var descriptor in descriptors.ToList()) + tenantRegistry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); + + tenantRegistry.ProvidedActivityDescriptors.TryRemove(providerType, out _); + } + + // Clear from agnostic registry + if (_agnosticRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var agnosticDescriptors)) + { + foreach (var descriptor in agnosticDescriptors.ToList()) + _agnosticRegistry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); - foreach (var descriptor in descriptors) - _activityDescriptors.Remove((descriptor.TenantId, descriptor.TypeName, descriptor.Version), out _); + _agnosticRegistry.ProvidedActivityDescriptors.TryRemove(providerType, out _); + } + } - _providedActivityDescriptors.Remove(providerType, out _); + /// + /// Clears all activity descriptors for a specific tenant. Useful when a tenant is deactivated. + /// + internal void ClearTenant(string tenantId) + { + _tenantRegistries.TryRemove(tenantId, out _); } - private ICollection GetOrCreateDescriptors(Type provider) + private TenantRegistryData GetOrCreateRegistry(string? tenantId) { - if (_providedActivityDescriptors.TryGetValue(provider, out var descriptors)) - return descriptors; + // Null or agnostic tenant ID goes to agnostic registry + if (tenantId is null or Tenant.AgnosticTenantId) + return _agnosticRegistry; + + // Get or create tenant-specific registry + return _tenantRegistries.GetOrAdd(tenantId, _ => new()); + } - descriptors = new List(); - _providedActivityDescriptors[provider] = descriptors; + private ICollection GetOrCreateProviderDescriptors(TenantRegistryData registry, Type providerType) + { + return registry.ProvidedActivityDescriptors.GetOrAdd(providerType, _ => new List()); + } - return descriptors; + /// + /// Normalizes tenant ID for grouping purposes. + /// Converts null to "*" so that both null and "*" descriptors are grouped together, + /// avoiding redundant processing of the agnostic registry. + /// + private static string? NormalizeTenantIdForGrouping(string? tenantId) + { + // Normalize null to "*" so both map to the same group + return tenantId ?? Tenant.AgnosticTenantId; } } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Management/Stores/CachingWorkflowDefinitionStore.cs b/src/modules/Elsa.Workflows.Management/Stores/CachingWorkflowDefinitionStore.cs index 796a55755a..7753064a8b 100644 --- a/src/modules/Elsa.Workflows.Management/Stores/CachingWorkflowDefinitionStore.cs +++ b/src/modules/Elsa.Workflows.Management/Stores/CachingWorkflowDefinitionStore.cs @@ -137,7 +137,7 @@ public async Task GetIsNameUnique(string name, string? definitionId = defa private async Task GetOrCreateAsync(string key, Func> factory) { - var tenantId = tenantAccessor.Tenant?.Id; + var tenantId = tenantAccessor.TenantId; var tenantIdPrefix = !string.IsNullOrEmpty(tenantId) ? $"{tenantId}:" : string.Empty; var internalKey = $"{tenantIdPrefix}{typeof(T).Name}:{key}"; return await cacheManager.GetOrCreateAsync(internalKey, async entry => diff --git a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs index 6260820410..7650fea41d 100644 --- a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs +++ b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs @@ -68,8 +68,11 @@ 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) + // Normalize tenant IDs for comparison (null becomes empty string) + var definitionTenantId = result.Workflow.Identity.TenantId.NormalizeTenantId(); + + // Only import workflows belonging to the current tenant or tenant-agnostic workflows (TenantId = "*"). + if (definitionTenantId != currentTenantId && definitionTenantId != Tenant.AgnosticTenantId) { _logger.LogDebug( "Skipping adding workflow {WorkflowId} from provider {Provider} because it belongs to tenant '{WorkflowTenantId}' but current tenant is '{CurrentTenantId}'", @@ -187,14 +190,18 @@ private async Task AddOrUpdateCoreAsync(MaterializedWorkflow await UpdateIsLatest(); await UpdateIsPublished(); + // Determine the tenant ID for the workflow definition + // If the workflow has no tenant ID, use the current tenant (normalized to handle null -> "") + var workflowTenantId = workflow.Identity.TenantId ?? (_tenantAccessor.Tenant?.Id).NormalizeTenantId(); + var workflowDefinition = existingDefinitionVersion ?? new WorkflowDefinition { DefinitionId = workflow.Identity.DefinitionId, Id = workflow.Identity.Id, Version = workflow.Identity.Version, - TenantId = workflow.Identity.TenantId, + TenantId = workflowTenantId, }; - + workflowDefinition.Description = workflow.WorkflowMetadata.Description; workflowDefinition.Name = workflow.WorkflowMetadata.Name; workflowDefinition.ToolVersion = workflow.WorkflowMetadata.ToolVersion; diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs index c7e6fb911b..ec9624b37f 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs @@ -10,6 +10,6 @@ public class ComponentTestTenantResolver : TenantResolverBase protected override TenantResolverResult Resolve(TenantResolverContext context) { // Resolve to empty string (default tenant) to match workflow definitions without explicit tenants - return AutoResolve(string.Empty); + return AutoResolve(Tenant.DefaultTenantId); } } diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs index 0214b5eb08..b2cd863ba6 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs @@ -57,7 +57,7 @@ public void Find_TenantSpecificPreferredOverTenantAgnostic_WhenBothExist() { // Arrange var tenantSpecific = CreateDescriptor(TestActivityType, 1, CurrentTenant); - var tenantAgnostic = CreateDescriptor(TestActivityType, 2, null); // Higher version + var tenantAgnostic = CreateDescriptor(TestActivityType, 2, Tenant.AgnosticTenantId); // Higher version RegisterDescriptors(tenantSpecific, tenantAgnostic); // Act @@ -71,14 +71,14 @@ public void Find_TenantSpecificPreferredOverTenantAgnostic_WhenBothExist() public void Find_ReturnsTenantAgnostic_WhenNoTenantSpecificExists() { // Arrange - var tenantAgnostic = CreateDescriptor(TestActivityType, 1, null); + var tenantAgnostic = CreateDescriptor(TestActivityType, 1, Tenant.AgnosticTenantId); RegisterDescriptors(tenantAgnostic); // Act var result = _registry.Find(TestActivityType); // Assert - AssertDescriptor(result, null, 1); + AssertDescriptor(result, Tenant.AgnosticTenantId, 1); } [Theory] @@ -112,9 +112,9 @@ public void Find_ReturnsHighestVersionTenantAgnostic_WhenMultipleTenantAgnosticE // Arrange var descriptors = new[] { - CreateDescriptor(TestActivityType, v1, null), - CreateDescriptor(TestActivityType, v2, null), - CreateDescriptor(TestActivityType, v3, null) + CreateDescriptor(TestActivityType, v1, Tenant.AgnosticTenantId), + CreateDescriptor(TestActivityType, v2, Tenant.AgnosticTenantId), + CreateDescriptor(TestActivityType, v3, Tenant.AgnosticTenantId) }; RegisterDescriptors(descriptors); @@ -122,7 +122,7 @@ public void Find_ReturnsHighestVersionTenantAgnostic_WhenMultipleTenantAgnosticE var result = _registry.Find(TestActivityType); // Assert - AssertDescriptor(result, null, expectedVersion); + AssertDescriptor(result, Tenant.AgnosticTenantId, expectedVersion); } [Fact] @@ -147,7 +147,7 @@ public void Find_IgnoresOtherTenantDescriptors_OnlyReturnsCurrentTenantOrAgnosti { CreateDescriptor(TestActivityType, 1, CurrentTenant), CreateDescriptor(TestActivityType, 5, "tenant2"), // Much higher version but wrong tenant - CreateDescriptor(TestActivityType, 2, null) + CreateDescriptor(TestActivityType, 2, Tenant.AgnosticTenantId) }; RegisterDescriptors(descriptors); From 08a46745beb72fa67c373c4c5d4924daaf9bbffb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 22:15:55 +0000 Subject: [PATCH 7/7] Initial plan