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);