diff --git a/Elsa.sln b/Elsa.sln index 4827949841..c3f6973f5f 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -196,12 +196,14 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{0A04B1FD-06C doc\adr\0001-record-architecture-decisions.md = doc\adr\0001-record-architecture-decisions.md doc\adr\0002-fault-propagation-from-child-to-parent-activities.md = doc\adr\0002-fault-propagation-from-child-to-parent-activities.md doc\adr\0003-direct-bookmark-management-in-workflowexecutioncontext.md = doc\adr\0003-direct-bookmark-management-in-workflowexecutioncontext.md - doc\adr\0004-token-centric-flowchart-execution-model.md = doc\adr\0004-token-centric-flowchart-execution-model.md doc\adr\graph.dot = doc\adr\graph.dot doc\adr\toc.md = doc\adr\toc.md doc\adr\0005-activity-execution-snapshots.md = doc\adr\0004-activity-execution-snapshots.md - doc\adr\0006-tenant-deleted-event.md = doc\adr\0005-tenant-deleted-event.md - doc\adr\0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md = doc\adr\0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md + doc\adr\0005-tenant-deleted-event.md = doc\adr\0005-tenant-deleted-event.md + doc\adr\0005-token-centric-flowchart-execution-model.md = doc\adr\0005-token-centric-flowchart-execution-model.md + doc\adr\0006-tenant-deleted-event.md = doc\adr\0006-tenant-deleted-event.md + doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md = doc\adr\0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md + doc\adr\0008-empty-string-as-default-tenant-id.md = doc\adr\0008-empty-string-as-default-tenant-id.md EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bounty", "bounty", "{9B80A705-2E31-4012-964A-83963DCDB384}" @@ -327,6 +329,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Resilience.Core.UnitTe EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Common.UnitTests", "test\unit\Elsa.Common.UnitTests\Elsa.Common.UnitTests.csproj", "{A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Tenants.UnitTests", "test\unit\Elsa.Tenants.UnitTests\Elsa.Tenants.UnitTests.csproj", "{DC476900-D836-4920-A696-CF8796668723}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -591,6 +595,10 @@ Global {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Debug|Any CPU.Build.0 = Debug|Any CPU {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Release|Any CPU.ActiveCfg = Release|Any CPU {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC}.Release|Any CPU.Build.0 = Release|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Debug|Any CPU.Build.0 = Debug|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Release|Any CPU.ActiveCfg = Release|Any CPU + {DC476900-D836-4920-A696-CF8796668723}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -694,6 +702,7 @@ Global {874F5A44-DB06-47AB-A18C-2D13942E0147} = {477C2416-312D-46AE-BCD6-8FA1FAB43624} {B8006D70-1630-43DB-A043-FA89FAC70F37} = {18453B51-25EB-4317-A4B3-B10518252E92} {A3C07D5B-2A30-494E-B9BC-4B1594B31ABC} = {18453B51-25EB-4317-A4B3-B10518252E92} + {DC476900-D836-4920-A696-CF8796668723} = {18453B51-25EB-4317-A4B3-B10518252E92} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {D4B5CEAA-7D70-4FCB-A68E-B03FBE5E0E5E} diff --git a/doc/adr/0004-token-centric-flowchart-execution-model.md b/doc/adr/0005-token-centric-flowchart-execution-model.md similarity index 98% rename from doc/adr/0004-token-centric-flowchart-execution-model.md rename to doc/adr/0005-token-centric-flowchart-execution-model.md index 4c01e65366..c633c4bb6f 100644 --- a/doc/adr/0004-token-centric-flowchart-execution-model.md +++ b/doc/adr/0005-token-centric-flowchart-execution-model.md @@ -1,4 +1,4 @@ -# 4. Token-Centric Flowchart Execution Model +# 5. Token-Centric Flowchart Execution Model Date: 2025-05-06 diff --git a/doc/adr/0005-tenant-deleted-event.md b/doc/adr/0006-tenant-deleted-event.md similarity index 98% rename from doc/adr/0005-tenant-deleted-event.md rename to doc/adr/0006-tenant-deleted-event.md index d0d2fa9f13..0fd7a8d244 100644 --- a/doc/adr/0005-tenant-deleted-event.md +++ b/doc/adr/0006-tenant-deleted-event.md @@ -1,4 +1,4 @@ -# 5. Tenant Deleted Event +# 6. Tenant Deleted Event Date: 2025-08-05 diff --git a/doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md b/doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md similarity index 99% rename from doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md rename to doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md index f6f279a50c..66ece0e875 100644 --- a/doc/adr/0006-adoption-of-explicit-merge-modes-for-flowchart-joins.md +++ b/doc/adr/0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md @@ -1,4 +1,4 @@ -# 6. Adoption of Explicit Merge Modes for Flowchart Joins +# 7. Adoption of Explicit Merge Modes for Flowchart Joins Date: 2025-09-30 diff --git a/doc/adr/0008-empty-string-as-default-tenant-id.md b/doc/adr/0008-empty-string-as-default-tenant-id.md new file mode 100644 index 0000000000..fedbedd36d --- /dev/null +++ b/doc/adr/0008-empty-string-as-default-tenant-id.md @@ -0,0 +1,48 @@ +# 8. Empty String as Default Tenant ID + +Date: 2026-01-27 + +## Status + +Accepted + +## Context + +The multitenancy system in Elsa supports an optional mode where, when multitenancy is disabled, the system assumes a single tenant. When enabled, there's still a default tenant involved. The convention has been to use `null` as the tenant ID for the default tenant. + +However, this convention created several issues: + +1. **Dictionary compatibility**: The `DefaultTenantResolverPipelineInvoker` attempts to build a dictionary of tenants by their ID using `ToDictionary(x => x.Id)`, which throws an exception because dictionaries do not support null keys. +2. **Inconsistency**: The codebase used `null`, empty string (`""`), and string literal `"default"` interchangeably to refer to the default tenant across different parts of the system (e.g., in configuration files and database records). +3. **Code clarity**: Using `null` as a sentinel value for "default" is implicit and can be unclear to developers reading the code. + +## Decision + +We will standardize on using an **empty string** (`""`) as the tenant ID for the default tenant instead of `null`. This decision includes: + +1. **Define a constant**: Add `Tenant.DefaultTenantId = ""` to explicitly document the convention. +2. **Update Tenant.Default**: Change `Tenant.Default.Id` from `null!` to use the `DefaultTenantId` constant. +3. **Add normalization helper**: Create a `NormalizeTenantId()` extension method that converts `null` to empty string, ensuring backwards compatibility with code that still uses null. +4. **Apply normalization consistently**: Use the normalization method in: + - Dictionary creation in `DefaultTenantResolverPipelineInvoker` + - Tenant lookups in `TenantResolverContext` + - Any other places where tenant IDs are compared or used as dictionary keys + +## Consequences + +### Positive + +- **No more exceptions**: Empty string is a valid dictionary key, eliminating the runtime exception in `DefaultTenantResolverPipelineInvoker`. +- **Backwards compatible**: The `NormalizeTenantId()` extension method ensures that existing code using `null` or empty string will work correctly. +- **Explicit convention**: The `DefaultTenantId` constant makes the convention clear and self-documenting. +- **Simplified logic**: Reduces the need for null-checking throughout the multitenancy code. +- **Consistency**: Aligns with parts of the codebase that were already using empty string (e.g., in configuration files). + +### Negative + +- **Migration consideration**: Existing data stores that have `null` tenant IDs will need to be normalized to empty strings, though the normalization helper provides a runtime solution. +- **String vs null semantics**: Some developers may find using empty string less intuitive than null for representing "no tenant", though this is mitigated by the explicit constant. + +### Neutral + +- The empty string convention is common in multitenancy systems and aligns with string-based identifier patterns used elsewhere in the codebase. diff --git a/doc/adr/graph.dot b/doc/adr/graph.dot index 4818df2a23..b659a835a3 100644 --- a/doc/adr/graph.dot +++ b/doc/adr/graph.dot @@ -8,7 +8,13 @@ _3 [label="3. Direct Bookmark Management in WorkflowExecutionContext"; URL="0003 _2 -> _3 [style="dotted", weight=1]; _4 [label="4. Activity Execution Snapshots"; URL="0004-activity-execution-snapshots.html"]; _3 -> _4 [style="dotted", weight=1]; -_5 [label="5. Tenant Deleted Event"; URL="0005-tenant-deleted-event.html"]; +_5 [label="5. Token-Centric Flowchart Execution Model"; URL="0005-token-centric-flowchart-execution-model.html"]; _4 -> _5 [style="dotted", weight=1]; +_6 [label="6. Tenant Deleted Event"; URL="0006-tenant-deleted-event.html"]; +_5 -> _6 [style="dotted", weight=1]; +_7 [label="7. Adoption of Explicit Merge Modes for Flowchart Joins"; URL="0007-adoption-of-explicit-merge-modes-for-flowchart-joins.html"]; +_6 -> _7 [style="dotted", weight=1]; +_8 [label="8. Empty String as Default Tenant ID"; URL="0008-empty-string-as-default-tenant-id.html"]; +_7 -> _8 [style="dotted", weight=1]; } } \ No newline at end of file diff --git a/doc/adr/toc.md b/doc/adr/toc.md index 26b15683d9..2256411817 100644 --- a/doc/adr/toc.md +++ b/doc/adr/toc.md @@ -4,4 +4,7 @@ * [2. Fault Propagation from Child to Parent Activities](0002-fault-propagation-from-child-to-parent-activities.md) * [3. Direct Bookmark Management in WorkflowExecutionContext](0003-direct-bookmark-management-in-workflowexecutioncontext.md) * [4. Activity Execution Snapshots](0004-activity-execution-snapshots.md) -* [5. Tenant Deleted Event](0005-tenant-deleted-event.md) \ No newline at end of file +* [5. Token-Centric Flowchart Execution Model](0005-token-centric-flowchart-execution-model.md) +* [6. Tenant Deleted Event](0006-tenant-deleted-event.md) +* [7. Adoption of Explicit Merge Modes for Flowchart Joins](0007-adoption-of-explicit-merge-modes-for-flowchart-joins.md) +* [8. Empty String as Default Tenant ID](0008-empty-string-as-default-tenant-id.md) \ 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 45c7a17931..a1a8de9c7e 100644 --- a/src/apps/Elsa.Server.Web/Program.cs +++ b/src/apps/Elsa.Server.Web/Program.cs @@ -4,12 +4,14 @@ using Elsa.Expressions.Helpers; using Elsa.Extensions; using Elsa.Features.Services; +using Elsa.Identity.Multitenancy; using Elsa.Persistence.EFCore.Extensions; using Elsa.Persistence.EFCore.Modules.Management; using Elsa.Persistence.EFCore.Modules.Runtime; using Elsa.Server.Web.Activities; using Elsa.Server.Web.ActivityHosts; using Elsa.Server.Web.Filters; +using Elsa.Tenants; using Elsa.Tenants.AspNetCore; using Elsa.Tenants.Extensions; using Elsa.WorkflowProviders.BlobStorage.ElsaScript.Extensions; @@ -29,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; @@ -118,6 +120,17 @@ http.ConfigureHttpOptions = options => configuration.GetSection("Http").Bind(options); http.UseCache(); }); + + if(useMultitenancy) + { + elsa.UseTenants(tenants => + { + tenants.UseConfigurationBasedTenantsProvider(options => configuration.GetSection("Multitenancy").Bind(options)); + tenants.ConfigureMultitenancy(options => options.TenantResolverPipelineBuilder = new TenantResolverPipelineBuilder() + .Append()); + }); + } + ConfigureForTest?.Invoke(elsa); }); diff --git a/src/apps/Elsa.Server.Web/appsettings.json b/src/apps/Elsa.Server.Web/appsettings.json index 7fd3ed9157..e3cf3df520 100644 --- a/src/apps/Elsa.Server.Web/appsettings.json +++ b/src/apps/Elsa.Server.Web/appsettings.json @@ -39,6 +39,19 @@ "Sqlite": "Data Source=App_Data/elsa.sqlite.db;Cache=Shared;" } } + }, + { + "Id": "tenant-2", + "Name": "Tenant 2", + "Configuration": { + "Http": { + "Prefix": "tenant-2", + "Host": "localhost:5001" + }, + "ConnectionStrings": { + "Sqlite": "Data Source=App_Data/elsa.sqlite.db;Cache=Shared;" + } + } } ] }, diff --git a/src/modules/Elsa.Common/Multitenancy/Abstractions/TenantResolverBase.cs b/src/modules/Elsa.Common/Multitenancy/Abstractions/TenantResolverBase.cs index db0966a5db..1b60dfc42d 100644 --- a/src/modules/Elsa.Common/Multitenancy/Abstractions/TenantResolverBase.cs +++ b/src/modules/Elsa.Common/Multitenancy/Abstractions/TenantResolverBase.cs @@ -29,13 +29,13 @@ protected virtual TenantResolverResult Resolve(TenantResolverContext context) /// /// Creates a new instance of representing a resolved tenant. /// - protected TenantResolverResult Resolved(string tenantId) => new(tenantId); - + protected TenantResolverResult Resolved(string? tenantId) => TenantResolverResult.Resolved(tenantId); + /// /// Creates a new instance of representing an unresolved tenant. /// - protected TenantResolverResult Unresolved() => new(null); - + protected TenantResolverResult Unresolved() => TenantResolverResult.Unresolved(); + /// /// Automatically resolves the tenant if the tenant ID is not null. /// diff --git a/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs b/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs index 72b4619fbf..eea5ab3d51 100644 --- a/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs +++ b/src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs @@ -30,9 +30,10 @@ public TenantResolverContext(IDictionary tenants, CancellationTo /// /// The tenant ID. /// The found tenant or null if no tenant with the provided ID exists. - public Tenant? FindTenant(string tenantId) + public Tenant? FindTenant(string? tenantId) { - return _tenantsDictionary.TryGetValue(tenantId, out var tenant) ? tenant : null; + var normalizedId = tenantId.NormalizeTenantId(); + return _tenantsDictionary.TryGetValue(normalizedId, out var tenant) ? tenant : null; } /// diff --git a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs index 76fcb7e93a..9927e676a9 100644 --- a/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs +++ b/src/modules/Elsa.Common/Multitenancy/Contracts/ITenantResolver.cs @@ -1,7 +1,7 @@ namespace Elsa.Common.Multitenancy; /// -/// A strategy for resolving the current tenant. This is called the tenant initializer. +/// A strategy for resolving the current tenant, called from the tenant initializer. /// public interface ITenantResolver { diff --git a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs index cba4b371bc..6d3632d515 100644 --- a/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs +++ b/src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs @@ -10,6 +10,11 @@ namespace Elsa.Common.Multitenancy; [UsedImplicitly] public class Tenant : Entity { + /// + /// The ID used for the default tenant. + /// + public const string DefaultTenantId = ""; + /// /// Gets or sets the name. /// @@ -19,10 +24,10 @@ public class Tenant : Entity /// Gets or sets the configuration. /// public IConfiguration Configuration { get; set; } = new ConfigurationBuilder().Build(); - + public static readonly Tenant Default = new() { - Id = null!, + Id = DefaultTenantId, Name = "Default" }; } diff --git a/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs b/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs index a60e1f9c35..0e5a66b217 100644 --- a/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs +++ b/src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs @@ -5,6 +5,11 @@ namespace Elsa.Common.Multitenancy; [UsedImplicitly] public static class TenantsProviderExtensions { + /// + /// Normalizes a tenant ID by converting null to empty string, ensuring consistency with the default tenant convention. + /// + public static string NormalizeTenantId(this string? tenantId) => tenantId ?? Tenant.DefaultTenantId; + public static async Task FindByIdAsync(this ITenantsProvider tenantsProvider, string id, CancellationToken cancellationToken = default) { var filter = new TenantFilter diff --git a/src/modules/Elsa.Common/Multitenancy/Results/TenantResolverResult.cs b/src/modules/Elsa.Common/Multitenancy/Results/TenantResolverResult.cs index 76c06ceac3..d3034a9783 100644 --- a/src/modules/Elsa.Common/Multitenancy/Results/TenantResolverResult.cs +++ b/src/modules/Elsa.Common/Multitenancy/Results/TenantResolverResult.cs @@ -3,26 +3,38 @@ namespace Elsa.Common.Multitenancy; /// /// Represents the result of a tenant resolution. /// -/// The resolved tenant. -public record TenantResolverResult(string? TenantId) +public record TenantResolverResult { + private readonly bool _isResolved; + + private TenantResolverResult(string? tenantId, bool isResolved) + { + TenantId = tenantId; + _isResolved = isResolved; + } + + /// + /// The normalized tenant ID. Returns null if unresolved. + /// + public string? TenantId => _isResolved ? field.NormalizeTenantId() : null; + /// /// Creates a new instance of representing a resolved tenant. /// /// The resolved tenant. /// A new instance of representing a resolved tenant. - public static TenantResolverResult Resolved(string tenantId) => new(tenantId); - + public static TenantResolverResult Resolved(string? tenantId) => new(tenantId, true); + /// /// Creates a new instance of representing an unresolved tenant. /// /// A new instance of representing an unresolved tenant. - public static TenantResolverResult Unresolved() => new(default(string?)); - + public static TenantResolverResult Unresolved() => new(null, false); + /// /// Gets a value indicating whether the tenant has been resolved. /// - public bool IsResolved => TenantId != null; - + public bool IsResolved => _isResolved; + public string ResolveTenantId() => TenantId ?? throw new InvalidOperationException("Tenant has not been resolved."); } \ No newline at end of file diff --git a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs b/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs deleted file mode 100644 index e040a20af4..0000000000 --- a/src/modules/Elsa.Persistence.EFCore.Common/CommonPersistenceFeature.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Elsa.Persistence.EFCore.EntityHandlers; -using Elsa.Features.Abstractions; -using Elsa.Features.Services; -using Microsoft.Extensions.DependencyInjection; - -namespace Elsa.Persistence.EFCore; - -/// -public class CommonPersistenceFeature(IModule module) : FeatureBase(module) -{ - /// - public override void Apply() - { - Services.AddScoped(); - Services.AddScoped(); - } -} \ No newline at end of file diff --git a/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs b/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs index 5acc709bf6..ab7bca5f1a 100644 --- a/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs +++ b/src/modules/Elsa.Persistence.EFCore.Common/PersistenceFeatureBase.cs @@ -2,6 +2,7 @@ using Elsa.Extensions; using Elsa.Features.Abstractions; using Elsa.Features.Services; +using Elsa.Persistence.EFCore.EntityHandlers; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.DependencyInjection; @@ -60,6 +61,9 @@ public override void Apply() { options.RunMigrations[typeof(TDbContext)] = RunMigrations; }); + + Services.AddScoped(); + Services.AddScoped(); } protected virtual void ConfigureMigrations() diff --git a/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs index 6a99e43f0c..12286caeb9 100644 --- a/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Runtime/20251204150235_V3_6.cs @@ -30,10 +30,10 @@ protected override void Up(MigrationBuilder migrationBuilder) .OldAnnotation("MySql:CharSet", "utf8mb4"); migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -41,7 +41,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); diff --git a/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs index da50b7b7a9..d996768b38 100644 --- a/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Runtime/20251204150355_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true, filter: "\"Hash\" IS NOT NULL"); } @@ -31,7 +31,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs index 8446ca4aa9..bce6fbd4aa 100644 --- a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -30,7 +30,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs index e7503a1ba0..49bb3921ab 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs @@ -317,9 +317,9 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.HasIndex("WorkflowDefinitionVersionId") .HasDatabaseName("IX_StoredTrigger_WorkflowDefinitionVersionId"); - b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId") + b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId", "TenantId") .IsUnique() - .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId") + .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId") .HasFilter("[Hash] IS NOT NULL"); b.ToTable("Triggers", "Elsa"); diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs index e59cf0b8eb..e1581cfc9d 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.cs @@ -18,6 +18,16 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) /// protected override void Up(MigrationBuilder migrationBuilder) { + // Drop old index if it exists (before TenantId was added) + migrationBuilder.Sql($@" + IF EXISTS (SELECT * FROM sys.indexes WHERE name = 'IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId' + AND object_id = OBJECT_ID('{_schema.Schema}.Triggers')) + BEGIN + DROP INDEX [IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId] + ON [{_schema.Schema}].[Triggers] + END + "); + migrationBuilder.AlterColumn( name: "ActivityId", schema: _schema.Schema, @@ -28,10 +38,10 @@ protected override void Up(MigrationBuilder migrationBuilder) oldType: "nvarchar(max)"); migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true, filter: "[Hash] IS NOT NULL"); } @@ -40,7 +50,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); diff --git a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs index 47d2ae7cd3..b821c873fb 100644 --- a/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs +++ b/src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/RuntimeElsaDbContextModelSnapshot.cs @@ -314,9 +314,9 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("WorkflowDefinitionVersionId") .HasDatabaseName("IX_StoredTrigger_WorkflowDefinitionVersionId"); - b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId") + b.HasIndex("WorkflowDefinitionId", "Hash", "ActivityId", "TenantId") .IsUnique() - .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId") + .HasDatabaseName("IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId") .HasFilter("[Hash] IS NOT NULL"); b.ToTable("Triggers", "Elsa"); diff --git a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs index 1f8e85ddd9..d3246d656d 100644 --- a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs @@ -19,10 +19,10 @@ public V3_6(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers", - columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId" }, + columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); } @@ -30,7 +30,7 @@ protected override void Up(MigrationBuilder migrationBuilder) protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropIndex( - name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId", + name: "IX_StoredTrigger_Unique_WorkflowDefinitionId_Hash_ActivityId_TenantId", schema: _schema.Schema, table: "Triggers"); } diff --git a/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs b/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs index 7c7988bea4..230ed4f0bd 100644 --- a/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs +++ b/src/modules/Elsa.Persistence.EFCore/Modules/Runtime/Configurations.cs @@ -125,15 +125,16 @@ public void Configure(EntityTypeBuilder builder) builder.HasIndex(x => x.TenantId).HasDatabaseName($"IX_{nameof(StoredTrigger)}_{nameof(StoredTrigger.TenantId)}"); // Add unique constraint to prevent duplicate trigger registrations in multi-engine environments - // A trigger is uniquely identified by WorkflowDefinitionId + Hash + ActivityId + // A trigger is uniquely identified by WorkflowDefinitionId + Hash + ActivityId + TenantId builder.HasIndex(x => new { x.WorkflowDefinitionId, x.Hash, - x.ActivityId + x.ActivityId, + x.TenantId }) .IsUnique() - .HasDatabaseName($"IX_{nameof(StoredTrigger)}_Unique_{nameof(StoredTrigger.WorkflowDefinitionId)}_{nameof(StoredTrigger.Hash)}_{nameof(StoredTrigger.ActivityId)}"); + .HasDatabaseName($"IX_{nameof(StoredTrigger)}_Unique_{nameof(StoredTrigger.WorkflowDefinitionId)}_{nameof(StoredTrigger.Hash)}_{nameof(StoredTrigger.ActivityId)}_{nameof(StoredTrigger.TenantId)}"); } /// diff --git a/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs b/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs index b8c03eaa0e..2c453baf7e 100644 --- a/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs +++ b/src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs @@ -1,5 +1,6 @@ using Elsa.Features.Services; using Elsa.Tenants.Features; +using JetBrains.Annotations; // ReSharper disable once CheckNamespace namespace Elsa.Tenants.Extensions; @@ -7,32 +8,39 @@ namespace Elsa.Tenants.Extensions; /// /// Extensions for that installs the feature. /// +[UsedImplicitly] public static class ModuleExtensions { /// /// Installs and configures the feature. /// - public static IModule UseTenants(this IModule module, Action? configure = default) + [UsedImplicitly] + public static IModule UseTenants(this IModule module, Action? configure = null) { module.Configure(configure); return module; } - /// - /// Installs and configures the feature. - /// - public static TenantsFeature UseTenantManagementEndpoints(this TenantsFeature feature, Action? configure = default) + extension(TenantsFeature feature) { - feature.Module.Configure(configure); - return feature; - } + /// + /// Installs and configures the feature. + /// + [UsedImplicitly] + public TenantsFeature UseTenantManagementEndpoints(Action? configure = null) + { + feature.Module.Configure(configure); + return feature; + } - /// - /// Installs and configures the feature. - /// - public static TenantsFeature UseTenantManagement(this TenantsFeature feature, Action? configure = default) - { - feature.Module.Configure(configure); - return feature; + /// + /// Installs and configures the feature. + /// + [UsedImplicitly] + public TenantsFeature UseTenantManagement(Action? configure = null) + { + feature.Module.Configure(configure); + return feature; + } } } \ No newline at end of file diff --git a/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs b/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs index c0b832f329..4074ac5968 100644 --- a/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs +++ b/src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs @@ -17,7 +17,8 @@ public class DefaultTenantResolverPipelineInvoker( public async Task InvokePipelineAsync(CancellationToken cancellationToken = default) { var resolutionPipeline = options.Value.TenantResolverPipelineBuilder.Build(serviceProvider); - var tenantsDictionary = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id); + var tenants = await tenantsProvider.ListAsync(cancellationToken); + var tenantsDictionary = tenants.ToDictionary(x => x.Id.NormalizeTenantId()); var context = new TenantResolverContext(tenantsDictionary, cancellationToken); foreach (var resolver in resolutionPipeline) diff --git a/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs b/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs index 1f5eda689d..757df21f4a 100644 --- a/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs +++ b/src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs @@ -47,7 +47,7 @@ private async Task BuildWorkflowAsync(Func _logger; private readonly SemaphoreSlim _semaphore = new(1, 1); @@ -33,6 +35,7 @@ public DefaultWorkflowDefinitionStorePopulator( IPayloadSerializer payloadSerializer, ISystemClock systemClock, IIdentityGraphService identityGraphService, + ITenantAccessor tenantAccessor, ILogger logger) { _workflowDefinitionProviders = workflowDefinitionProviders; @@ -42,6 +45,7 @@ public DefaultWorkflowDefinitionStorePopulator( _payloadSerializer = payloadSerializer; _systemClock = systemClock; _identityGraphService = identityGraphService; + _tenantAccessor = tenantAccessor; _logger = logger; } @@ -56,6 +60,7 @@ public async Task> PopulateStoreAsync(bool index { var providers = _workflowDefinitionProviders(); var workflowDefinitions = new List(); + var currentTenantId = (_tenantAccessor.Tenant?.Id).NormalizeTenantId(); foreach (var provider in providers) { @@ -63,6 +68,18 @@ 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) + { + _logger.LogDebug( + "Skipping adding workflow {WorkflowId} from provider {Provider} because it belongs to tenant '{WorkflowTenantId}' but current tenant is '{CurrentTenantId}'", + result.Workflow.Identity.DefinitionId, + provider.Name, + result.Workflow.Identity.TenantId, + currentTenantId); + continue; + } + var workflowDefinition = await AddAsync(result, indexTriggers, cancellationToken); workflowDefinitions.Add(workflowDefinition); } @@ -128,7 +145,9 @@ private async Task AddOrUpdateCoreAsync(MaterializedWorkflow { // NEW WAY: OriginalSource is provided // For JSON workflows, we still need to populate StringData with the serialized root for backwards compatibility - stringData = materializedWorkflow.MaterializerName == "Json" ? _activitySerializer.Serialize(workflow.Root) : + stringData = materializedWorkflow.MaterializerName == "Json" + ? _activitySerializer.Serialize(workflow.Root) + : // For new formats (ElsaScript, YAML, etc.), only OriginalSource is needed // StringData can be null as these materializers only use OriginalSource null; diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs index ab0907a536..d95d07aafa 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs @@ -131,16 +131,17 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) builder.ConfigureTestServices(services => { - // Decorate IDistributedLockProvider with TestDistributedLockProvider so tests use it - services.Decorate(); - - // Also register TestDistributedLockProvider as itself so tests can access it directly for configuration + // Decorate IDistributedLockProvider with SelectiveMockLockProvider + // This allows tests to selectively mock specific locks without affecting background operations + services.Decorate(); + + // Register SelectiveMockLockProvider as itself so tests can access it for configuration services.AddSingleton(sp => { var provider = sp.GetRequiredService(); - if (provider is not TestDistributedLockProvider testProvider) - throw new InvalidOperationException($"Expected IDistributedLockProvider to be decorated with TestDistributedLockProvider, but got {provider.GetType().Name}"); - return testProvider; + if (provider is not SelectiveMockLockProvider selectiveProvider) + throw new InvalidOperationException($"Expected IDistributedLockProvider to be decorated with SelectiveMockLockProvider, but got {provider.GetType().Name}"); + return selectiveProvider; }); services @@ -154,7 +155,6 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) .AddWorkflowsProvider() .AddNotificationHandlersFrom() .Decorate() - .Decorate() ; }); } diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs index efa063902a..0c0b89020d 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs @@ -19,10 +19,10 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience; public class DistributedLockResilienceTests(App app) : AppComponentTest(app) { private const int MaxRetryAttempts = 3; - - // The IDistributedLockProvider is decorated with TestDistributedLockProvider in WorkflowServer.ConfigureTestServices - // This cast is safe because the decorator pattern ensures TestDistributedLockProvider wraps the actual provider - private TestDistributedLockProvider MockProvider => (TestDistributedLockProvider)Scope.ServiceProvider.GetRequiredService(); + + // Selective mock provider - only mocks specific locks, not all locks globally + private SelectiveMockLockProvider SelectiveMockProvider => Scope.ServiceProvider.GetRequiredService(); + private ITransientExceptionDetector TransientExceptionDetector => Scope.ServiceProvider.GetRequiredService(); private ILogger Logger => Scope.ServiceProvider.GetRequiredService>(); private DistributedLockingOptions LockOptions => Scope.ServiceProvider.GetRequiredService>().Value; @@ -34,22 +34,25 @@ public class DistributedLockResilienceTests(App app) : AppComponentTest(app) [InlineData(4, 4, true)] // Four failures, exhausts retries (MaxRetryAttempts = 3) public async Task AcquireLockWithRetry_AcquisitionFailures_BehavesAsExpected(int failureCount, int expectedAttemptCount, bool shouldThrow) { - // Arrange - MockProvider.Reset(); - MockProvider.FailAcquisitionTimes(failureCount); + // Arrange - Mock this specific lock only + var lockName = $"test-lock-{failureCount}"; + var mockProvider = SelectiveMockProvider.MockLock(lockName); + mockProvider.Reset(); + mockProvider.FailAcquisitionTimes(failureCount); // Act & Assert if (shouldThrow) { - await Assert.ThrowsAsync(async () => await AcquireLockWithRetryAsync($"test-lock-{failureCount}")); + await Assert.ThrowsAsync(async () => await AcquireLockWithRetryAsync(lockName, mockProvider)); } else { - await using var handle = await AcquireLockWithRetryAsync($"test-lock-{failureCount}"); + await using var handle = await AcquireLockWithRetryAsync(lockName, mockProvider); Assert.NotNull(handle); } - Assert.Equal(expectedAttemptCount, MockProvider.AcquisitionAttemptCount); + // Assert exact count - only this lock is mocked + Assert.Equal(expectedAttemptCount, mockProvider.AcquisitionAttemptCount); } [Theory] @@ -62,10 +65,11 @@ public async Task RunInstanceAsync_TransientLockFailures_RetriesCorrectly(int fa var workflowClient = await CreateWorkflowClientAsync(); var workflowInstanceId = workflowClient.WorkflowInstanceId; - // Reset and configure failures for this specific workflow instance's lock - MockProvider.Reset(); - MockProvider.FailAcquisitionTimesForLock($"workflow-instance:{workflowInstanceId}", failureCount); - var attemptCountBefore = MockProvider.AcquisitionAttemptCount; + // Configure failures for this specific workflow instance's lock only + var lockPrefix = $"workflow-instance:{workflowInstanceId}"; + var mockProvider = SelectiveMockProvider.MockLock(lockPrefix); + mockProvider.Reset(); + mockProvider.FailAcquisitionTimes(failureCount); // Now run the instance with the configured lock failures var runRequest = new RunWorkflowInstanceRequest(); @@ -82,34 +86,42 @@ await Assert.ThrowsAsync(async () => Assert.NotNull(response); } - // Verify retries occurred - check the delta from before the operation to account for background noise - var expectedAttempts = failureCount + 1; // failures + 1 success (or final failure for shouldThrow case) - AssertMinimumAttempts(MockProvider.AcquisitionAttemptCount - attemptCountBefore, expectedAttempts, "acquisition"); + // Assert exact count - only this specific workflow instance lock is mocked + // When shouldThrow=true, all attempts fail: MaxRetryAttempts+1 (initial + retries) + // When shouldThrow=false, we succeed after failures: failureCount+1 (failures + success) + var expectedAttempts = shouldThrow ? MaxRetryAttempts + 1 : failureCount + 1; + Assert.Equal(expectedAttempts, mockProvider.AcquisitionAttemptCount); } [Fact] public async Task RunInstanceAsync_TransientReleaseFailure_ShouldLogButNotThrow() { // Arrange - MockProvider.Reset(); var workflowClient = await CreateWorkflowClientAsync(createInstance: false); - // Configure failure after client creation to minimize background interference - MockProvider.FailReleaseOnce(); - var releaseCountBefore = MockProvider.ReleaseAttemptCount; + // Create and run to get the workflow instance ID, then configure release failure for that lock + var request = CreateAndRunRequest(); + + // Mock the workflow-instance lock prefix (all workflow instance locks) + var mockProvider = SelectiveMockProvider.MockLock("workflow-instance:"); + mockProvider.Reset(); + mockProvider.FailReleaseOnce(); // Act - Release failure should be caught and logged, not thrown - var response = await workflowClient.CreateAndRunInstanceAsync(CreateAndRunRequest()); + var response = await workflowClient.CreateAndRunInstanceAsync(request); // Assert Assert.NotNull(response); Assert.NotNull(response.WorkflowInstanceId); - AssertMinimumAttempts(MockProvider.ReleaseAttemptCount - releaseCountBefore, 1, "release"); + + // Verify at least one release occurred + Assert.True(mockProvider.ReleaseAttemptCount >= 1, + $"Expected at least 1 release attempt, but got {mockProvider.ReleaseAttemptCount}"); } - private async Task AcquireLockWithRetryAsync(string lockName) => + private async Task AcquireLockWithRetryAsync(string lockName, TestDistributedLockProvider mockProvider) => await RetryPipeline.ExecuteAsync(async ct => - await MockProvider.AcquireLockAsync(lockName, LockOptions.LockAcquisitionTimeout, ct), + await mockProvider.CreateLock(lockName).AcquireAsync(LockOptions.LockAcquisitionTimeout, ct), CancellationToken.None); /// @@ -138,10 +150,6 @@ private static CreateAndRunWorkflowInstanceRequest CreateAndRunRequest() => WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionId(SimpleWorkflow.DefinitionId, VersionOptions.Latest) }; - private static void AssertMinimumAttempts(int actualAttempts, int expectedAttempts, string attemptType) => - Assert.True(actualAttempts >= expectedAttempts, - $"Expected at least {expectedAttempts} {attemptType} attempts, but got {actualAttempts}"); - private static ResiliencePipeline CreateRetryPipeline(ITransientExceptionDetector transientExceptionDetector, ILogger logger) => new ResiliencePipelineBuilder() .AddRetry(new() diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs new file mode 100644 index 0000000000..cc56e9ced0 --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/SelectiveMockLockProvider.cs @@ -0,0 +1,104 @@ +using Medallion.Threading; + +namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience.Mocks; + +/// +/// A lock provider that selectively mocks specific locks while allowing others to use the real implementation. +/// +/// WHY THIS IS NECESSARY: +/// - Workflow operations trigger background processes (trigger indexing, state persistence, etc.) +/// - These background operations acquire their own locks concurrently +/// - If we mock ALL locks globally, background operations can consume configured failures +/// - This makes test assertions unreliable and flaky +/// +/// SOLUTION: +/// - Only mock locks matching specific prefixes configured by tests +/// - Background operations use real locks (not counted, not mocked) +/// - Test operations use mocked locks (counted, failures injected) +/// - Result: Deterministic, reliable test assertions +/// +public class SelectiveMockLockProvider : IDistributedLockProvider, IDisposable +{ + private readonly IDistributedLockProvider _realProvider; + private readonly Dictionary _mockProvidersByPrefix = new(); + private readonly object _lock = new(); + + public SelectiveMockLockProvider(IDistributedLockProvider realProvider) + { + _realProvider = realProvider; + } + + /// + /// Gets the real/inner provider being wrapped. + /// + public IDistributedLockProvider RealProvider => _realProvider; + + /// + /// Configures mocking for locks matching the specified prefix. + /// Returns a test provider that allows configuring failures for these locks. + /// + public TestDistributedLockProvider MockLock(string lockNamePrefix) + { + lock (_lock) + { + if (!_mockProvidersByPrefix.TryGetValue(lockNamePrefix, out var mockProvider)) + { + mockProvider = new TestDistributedLockProvider(_realProvider); + _mockProvidersByPrefix[lockNamePrefix] = mockProvider; + } + return mockProvider; + } + } + + /// + /// Removes mocking for the specified lock prefix, allowing it to use the real provider. + /// + public void Unmock(string lockNamePrefix) + { + lock (_lock) + { + _mockProvidersByPrefix.Remove(lockNamePrefix); + } + } + + /// + /// Clears all mock configurations, resetting to real provider for all locks. + /// + public void Reset() + { + lock (_lock) + { + foreach (var mockProvider in _mockProvidersByPrefix.Values) + { + mockProvider.Reset(); + } + _mockProvidersByPrefix.Clear(); + } + } + + /// + /// Creates a lock that will be mocked if it matches a configured prefix, otherwise uses the real provider. + /// + public IDistributedLock CreateLock(string name) + { + lock (_lock) + { + // Check if this lock name matches any configured mock prefix + foreach (var (prefix, mockProvider) in _mockProvidersByPrefix) + { + if (name.StartsWith(prefix, StringComparison.Ordinal)) + { + return mockProvider.CreateLock(name); + } + } + + // No mock configured, use real provider + return _realProvider.CreateLock(name); + } + } + + public void Dispose() + { + Reset(); + } +} diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs index fa50c825ad..0ffe126da6 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs @@ -15,6 +15,11 @@ public class TestDistributedLockProvider(IDistributedLockProvider innerProvider) private int _releaseAttemptCount; private string? _targetLockPrefix; + /// + /// Gets the inner/real lock provider that this test provider wraps. + /// + public IDistributedLockProvider InnerProvider => innerProvider; + public int AcquisitionAttemptCount => _acquisitionAttemptCount; public int ReleaseAttemptCount => _releaseAttemptCount; diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs index 06b5d600c7..838ed8bbad 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Multitenancy/MultitenancyTests.cs @@ -1,4 +1,5 @@ using Elsa.Common.Models; +using Elsa.Common.Multitenancy; using Elsa.Workflows.ComponentTests.Abstractions; using Elsa.Workflows.ComponentTests.Fixtures; using Elsa.Workflows.Management; @@ -7,18 +8,114 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.Multitenancy; +/// +/// Tests for multitenancy tenant ID normalization. +/// public class MultitenancyTests(App app) : AppComponentTest(app) { - [Fact(Skip = "Multitenancy disabled. This test doesn't work because not all workflows are assigned the Tenant1 tenant.")] - public async Task LoadingWorkflows_ShouldReturnWorkflows_FromCurrentTenant() + [Fact] + public void DefaultTenant_ShouldUseEmptyStringAsId() { + // Assert + Assert.Equal(string.Empty, Tenant.DefaultTenantId); + Assert.Equal(Tenant.DefaultTenantId, Tenant.Default.Id); + } + + [Fact] + public void NormalizeTenantId_WithNull_ShouldReturnEmptyString() + { + // Arrange + string? tenantId = null; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, normalizedId); + Assert.Equal(string.Empty, normalizedId); + } + + [Fact] + public void NormalizeTenantId_WithEmptyString_ShouldReturnEmptyString() + { + // Arrange + var tenantId = string.Empty; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, normalizedId); + } + + [Fact] + public void NormalizeTenantId_WithValidTenantId_ShouldReturnSameValue() + { + // Arrange + var tenantId = "tenant-123"; + + // Act + var normalizedId = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal("tenant-123", normalizedId); + } + + [Fact] + public async Task WorkflowDefinitionStore_ShouldWorkWithTenantNormalization() + { + // Arrange var store = Scope.ServiceProvider.GetRequiredService(); var filter = new WorkflowDefinitionFilter { IsSystem = false, VersionOptions = VersionOptions.Latest }; + + // Act & Assert - Should not throw exceptions related to tenant ID handling var workflows = await store.FindManyAsync(filter); - Assert.All(workflows, workflow => Assert.Equal("Tenant1", workflow.TenantId)); + Assert.NotNull(workflows); + } + + [Fact] + public void TenantResolverContext_FindTenant_WithNull_ShouldNormalize() + { + // Arrange + var defaultTenant = new Tenant { Id = Tenant.DefaultTenantId, Name = "Default" }; + var tenant1 = new Tenant { Id = "tenant1", Name = "Tenant 1" }; + var tenantsDictionary = new Dictionary + { + { defaultTenant.Id, defaultTenant }, + { tenant1.Id, tenant1 } + }; + var context = new TenantResolverContext(tenantsDictionary, CancellationToken.None); + + // Act + string? nullTenantId = null; + var result = context.FindTenant(nullTenantId); + + // Assert + Assert.NotNull(result); + Assert.Equal(Tenant.DefaultTenantId, result.Id); + } + + [Fact] + public void TenantResolverContext_FindTenant_WithEmptyString_ShouldFindDefaultTenant() + { + // Arrange + var defaultTenant = new Tenant { Id = Tenant.DefaultTenantId, Name = "Default" }; + var tenantsDictionary = new Dictionary + { + { defaultTenant.Id, defaultTenant } + }; + var context = new TenantResolverContext(tenantsDictionary, CancellationToken.None); + + // Act + var result = context.FindTenant(string.Empty); + + // Assert + Assert.NotNull(result); + Assert.Equal(Tenant.DefaultTenantId, result.Id); + Assert.Equal("Default", result.Name); } -} \ No newline at end of file +} diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs index a4cdd48595..076d00b162 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionStorePopulation/Tests.cs @@ -1,4 +1,5 @@ -using Elsa.Testing.Shared; +using Elsa.Common.Multitenancy; +using Elsa.Testing.Shared; using Elsa.Workflows.Activities; using Elsa.Workflows.Helpers; using Elsa.Workflows.Management; @@ -29,7 +30,7 @@ public Tests(ITestOutputHelper testOutputHelper) DefinitionId: "WorkflowWithTrigger", Version: 1, Id: "1", - TenantId: "default" + TenantId: Tenant.DefaultTenantId ), Root = new Event("Foo") { diff --git a/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs new file mode 100644 index 0000000000..8d02b77de7 --- /dev/null +++ b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantIdNormalizationTests.cs @@ -0,0 +1,49 @@ +using Elsa.Common.Multitenancy; + +namespace Elsa.Common.UnitTests.Multitenancy; + +public class TenantIdNormalizationTests +{ + [Theory] + [InlineData(null)] + [InlineData("")] + public void NormalizeTenantId_WithNullOrEmpty_ReturnsDefaultTenantId(string? tenantId) + { + // Act + var result = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(Tenant.DefaultTenantId, result); + Assert.Equal(string.Empty, result); + } + + [Theory] + [InlineData("tenant1")] + [InlineData("tenant-abc-123")] + [InlineData("DEFAULT")] + [InlineData("my-custom-tenant")] + [InlineData(" ")] // Whitespace is not normalized + public void NormalizeTenantId_WithNonNullString_ReturnsOriginalValue(string tenantId) + { + // Act + var result = tenantId.NormalizeTenantId(); + + // Assert + Assert.Equal(tenantId, result); + } + + [Fact] + public void DefaultTenantId_IsEmptyString() + { + // Assert + Assert.Equal(Tenant.DefaultTenantId, string.Empty); + } + + [Fact] + public void DefaultTenant_UsesDefaultTenantId() + { + // Assert + Assert.Equal(Tenant.DefaultTenantId, Tenant.Default.Id); + Assert.Equal(string.Empty, Tenant.Default.Id); + } +} diff --git a/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs new file mode 100644 index 0000000000..94cb6d86f0 --- /dev/null +++ b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantResolverContextTests.cs @@ -0,0 +1,155 @@ +using Elsa.Common.Multitenancy; + +namespace Elsa.Common.UnitTests.Multitenancy; + +public class TenantResolverContextTests +{ + [Theory] + [InlineData(null, "Default")] + [InlineData("", "Default")] + [InlineData("tenant1", "Tenant 1")] + [InlineData("tenant2", "Tenant 2")] + public void FindTenant_ById_FindsCorrectTenant(string? tenantId, string expectedName) + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant(tenantId!); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedName, result.Name); + } + + [Fact] + public void FindTenant_WithNonExistentId_ReturnsNull() + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant("non-existent"); + + // Assert + Assert.Null(result); + } + + [Theory] + [InlineData("Alpha", "tenant1", "Tenant Alpha")] + [InlineData("Beta", "tenant2", "Tenant Beta")] + public void FindTenant_WithPredicate_FindsMatchingTenant(string searchTerm, string expectedId, string expectedName) + { + // Arrange + var context = CreateContextWithNamedTenants(); + + // Act + var result = context.FindTenant(t => t.Name.Contains(searchTerm)); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedId, result.Id); + Assert.Equal(expectedName, result.Name); + } + + [Fact] + public void FindTenant_WithPredicate_NoMatch_ReturnsNull() + { + // Arrange + var context = CreateContext(); + + // Act + var result = context.FindTenant(t => t.Name == "NonExistent"); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Constructor_StoresCancellationToken() + { + // Arrange + using var cts = new CancellationTokenSource(); + + // Act + var context = new TenantResolverContext(new Dictionary(), cts.Token); + + // Assert + Assert.Equal(cts.Token, context.CancellationToken); + } + + [Fact] + public void FindTenant_NormalizesNullAndEmptyStringToSameValue() + { + // Arrange + var context = CreateContext(); + + // Act + var resultFromNull = context.FindTenant((string?)null); + var resultFromEmptyString = context.FindTenant(string.Empty); + + // Assert + Assert.NotNull(resultFromNull); + Assert.NotNull(resultFromEmptyString); + Assert.Same(resultFromNull, resultFromEmptyString); + } + + // Helper methods + private static TenantResolverContext CreateContext() + { + var tenants = new Dictionary + { + { + Tenant.DefaultTenantId, new() + { + Id = Tenant.DefaultTenantId, + Name = "Default" + } + }, + { + "tenant1", new() + { + Id = "tenant1", + Name = "Tenant 1" + } + }, + { + "tenant2", new() + { + Id = "tenant2", + Name = "Tenant 2" + } + } + }; + return new(tenants, CancellationToken.None); + } + + private static TenantResolverContext CreateContextWithNamedTenants() + { + var tenants = new Dictionary + { + { + Tenant.DefaultTenantId, new() + { + Id = Tenant.DefaultTenantId, + Name = "Default" + } + }, + { + "tenant1", new() + { + Id = "tenant1", + Name = "Tenant Alpha" + } + }, + { + "tenant2", new() + { + Id = "tenant2", + Name = "Tenant Beta" + } + } + }; + return new(tenants, CancellationToken.None); + } +} \ No newline at end of file diff --git a/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj b/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj new file mode 100644 index 0000000000..057369bbd1 --- /dev/null +++ b/test/unit/Elsa.Tenants.UnitTests/Elsa.Tenants.UnitTests.csproj @@ -0,0 +1,14 @@ + + + + [Elsa.Tenants]* + 0 + + + + + + + + + diff --git a/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs b/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs new file mode 100644 index 0000000000..2e886f9c9a --- /dev/null +++ b/test/unit/Elsa.Tenants.UnitTests/Services/DefaultTenantResolverPipelineInvokerTests.cs @@ -0,0 +1,181 @@ +using Elsa.Common.Multitenancy; +using Elsa.Tenants.Options; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; + +namespace Elsa.Tenants.UnitTests.Services; + +public class DefaultTenantResolverPipelineInvokerTests +{ + [Fact] + public async Task InvokePipelineAsync_WithEmptyStringTenantId_FindsDefaultTenant() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("Default", result.Name); + } + + [Fact] + public async Task InvokePipelineAsync_WithValidTenantId_FindsCorrectTenant() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("tenant1")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("Tenant 1", result.Name); + } + + [Fact] + public async Task InvokePipelineAsync_WithNonExistentTenantId_ReturnsNull() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("non-existent")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithNullTenantIdsInList_DoesNotThrowDictionaryException() + { + // Arrange - Simulates legacy data with null IDs + var tenants = new List + { + new() { Id = null!, Name = "Legacy Null Tenant" }, + new() { Id = "tenant1", Name = "Tenant 1" } + }; + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Unresolved()); + + // Act & Assert - Should not throw + var result = await invoker.InvokePipelineAsync(); + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithUnresolvedResult_ReturnsNull() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Unresolved()); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task InvokePipelineAsync_WithMultipleResolvers_UsesFirstResolvedResult() + { + // Arrange + var tenants = CreateDefaultTenantList(); + var mockResolver1 = CreateMockResolver(TenantResolverResult.Resolved("tenant1")); + var mockResolver2 = CreateMockResolver(TenantResolverResult.Resolved("tenant2")); + var invoker = CreateInvokerWithMultipleResolvers(tenants, mockResolver1, mockResolver2); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert + Assert.NotNull(result); + Assert.Equal("tenant1", result.Id); + await mockResolver2.DidNotReceive().ResolveAsync(Arg.Any()); + } + + [Fact] + public async Task InvokePipelineAsync_WithLegacyNullTenantIds_NormalizesAndFindsDefaultTenant() + { + // Arrange - Simulates legacy data with null ID that gets normalized + var tenants = new List + { + new() { Id = null!, Name = "Legacy" }, // Will be normalized to "" + new() { Id = "tenant1", Name = "Tenant 1" } + }; + var (invoker, _) = CreateInvoker(tenants, TenantResolverResult.Resolved("")); + + // Act + var result = await invoker.InvokePipelineAsync(); + + // Assert - The null tenant ID gets normalized to "" in dictionary, so it should be found + Assert.NotNull(result); + Assert.Equal("Legacy", result.Name); // Should find the Legacy tenant (normalized from null) + } + + // Helper methods + private static List CreateDefaultTenantList() => new() + { + new() { Id = Tenant.DefaultTenantId, Name = "Default" }, + new() { Id = "tenant1", Name = "Tenant 1" }, + new() { Id = "tenant2", Name = "Tenant 2" } + }; + + private static ITenantResolver CreateMockResolver(TenantResolverResult result) + { + var mockResolver = Substitute.For(); + mockResolver.ResolveAsync(Arg.Any()).Returns(result); + return mockResolver; + } + + private static (DefaultTenantResolverPipelineInvoker Invoker, ITenantResolver Resolver) CreateInvoker( + List tenants, + TenantResolverResult resolverResult) + { + var tenantsProvider = Substitute.For(); + tenantsProvider.ListAsync(Arg.Any()).Returns(tenants); + + var mockResolver = CreateMockResolver(resolverResult); + + // Use a mock pipeline builder that directly returns our mock resolver + var pipelineBuilder = Substitute.For(); + pipelineBuilder.Build(Arg.Any()).Returns(new[] { mockResolver }); + + var options = Microsoft.Extensions.Options.Options.Create(new MultitenancyOptions + { + TenantResolverPipelineBuilder = pipelineBuilder + }); + + var serviceProvider = Substitute.For(); + var logger = NullLogger.Instance; + var invoker = new DefaultTenantResolverPipelineInvoker(options, tenantsProvider, serviceProvider, logger); + + return (invoker, mockResolver); + } + + private static DefaultTenantResolverPipelineInvoker CreateInvokerWithMultipleResolvers( + List tenants, + params ITenantResolver[] resolvers) + { + var tenantsProvider = Substitute.For(); + tenantsProvider.ListAsync(Arg.Any()).Returns(tenants); + + // Use a mock pipeline builder that directly returns our mock resolvers + var pipelineBuilder = Substitute.For(); + pipelineBuilder.Build(Arg.Any()).Returns(resolvers); + + var options = Microsoft.Extensions.Options.Options.Create(new MultitenancyOptions + { + TenantResolverPipelineBuilder = pipelineBuilder + }); + + var serviceProvider = Substitute.For(); + var logger = NullLogger.Instance; + return new(options, tenantsProvider, serviceProvider, logger); + } +} diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs new file mode 100644 index 0000000000..aabad71ce9 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Models/ActivityConstructionResultTests.cs @@ -0,0 +1,111 @@ +using Elsa.Workflows.Activities; +using Elsa.Workflows.Models; + +namespace Elsa.Workflows.Core.UnitTests.Models; + +public class ActivityConstructionResultTests +{ + [Theory] + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(2, true)] + public void Constructor_WithVaryingExceptionCounts_SetsPropertiesCorrectly(int exceptionCount, bool expectedHasExceptions) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + + // Act + var result = new ActivityConstructionResult(activity, exceptions); + + // Assert + Assert.Same(activity, result.Activity); + Assert.Equal(exceptionCount, result.Exceptions.Count()); + Assert.Equal(expectedHasExceptions, result.HasExceptions); + } + + [Fact] + public void Constructor_WithNullExceptions_TreatsAsEmpty() + { + // Arrange + var activity = CreateActivity(); + + // Act + var result = new ActivityConstructionResult(activity, null); + + // Assert + Assert.Empty(result.Exceptions); + Assert.False(result.HasExceptions); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(3, true)] + public void Cast_PreservesActivityAndExceptions(int exceptionCount, bool expectedHasExceptions) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + var result = new ActivityConstructionResult(activity, exceptions); + + // Act + var typedResult = result.Cast(); + + // Assert + Assert.IsType>(typedResult); + Assert.Same(activity, typedResult.Activity); + Assert.Equal(exceptionCount, typedResult.Exceptions.Count()); + Assert.Equal(expectedHasExceptions, typedResult.HasExceptions); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(2, true)] + public void GenericConstructor_CreatesTypedResultWithInheritance(int exceptionCount, bool expectedHasExceptions) + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(exceptionCount); + + // Act + var result = new ActivityConstructionResult(activity, exceptions); + + // Assert + Assert.Same(activity, result.Activity); + Assert.Equal(exceptionCount, result.Exceptions.Count()); + Assert.Equal(expectedHasExceptions, result.HasExceptions); + Assert.IsAssignableFrom(result); + } + + [Fact] + public void Exceptions_CanBeEnumerated() + { + // Arrange + var activity = CreateActivity(); + var exceptions = CreateExceptions(3); + var result = new ActivityConstructionResult(activity, exceptions); + + // Act & Assert + var count = 0; + foreach (var ex in result.Exceptions) + { + Assert.NotNull(ex); + count++; + } + Assert.Equal(3, count); + } + + // Helper methods + private static WriteLine CreateActivity() => new("test"); + + private static List? CreateExceptions(int count) + { + if (count == 0) return null; + + return Enumerable.Range(1, count) + .Select(i => new InvalidOperationException($"Error {i}") as Exception) + .ToList(); + } +} diff --git a/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs b/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs index 5f960ea400..158e9055b9 100644 --- a/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs +++ b/test/unit/Elsa.Workflows.Runtime.UnitTests/Services/DefaultWorkflowDefinitionStorePopulatorTests.cs @@ -1,10 +1,11 @@ using Elsa.Common; -using Elsa.Workflows.Activities; +using Elsa.Common.Multitenancy; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; using Elsa.Workflows.Management.Filters; using Elsa.Workflows.Models; using Microsoft.Extensions.Logging; +using Open.Linq.AsyncExtensions; using NSubstitute; namespace Elsa.Workflows.Runtime.UnitTests.Services; @@ -18,25 +19,25 @@ public class DefaultWorkflowDefinitionStorePopulatorTests public DefaultWorkflowDefinitionStorePopulatorTests() { _storeMock = Substitute.For(); - _storeMock.FindManyAsync(Arg.Any(), Arg.Any()) - .Returns(_workflowDefinitionsInStore); - _populator = new DefaultWorkflowDefinitionStorePopulator(() => new List(), + _storeMock.FindManyAsync(Arg.Any(), Arg.Any()).Returns(_workflowDefinitionsInStore); + _populator = new(() => new List(), Substitute.For(), _storeMock, Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), + Substitute.For(), Substitute.For>()); } [Fact(DisplayName = "When adding a new workflow it needs to be saved")] public async Task AddOrUpdateCoreAsync_NewWorkflowDefinition_AddsWorkflowDefinition() { - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 7, "1"), - Publication = new WorkflowPublication(true, true) + Identity = new("a", 7, "1"), + Publication = new(true, true) }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -62,9 +63,9 @@ public async Task AddOrUpdateCoreAsync_ExistingWorkflowDefinition_KeepsExistingW } }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 1, "1"), + Identity = new("a", 1, "1"), Inputs = new List { new() @@ -94,10 +95,10 @@ public async Task AddOrUpdateCoreAsync_NewWorkflowDefinitionVersion_AddsWorkflow IsLatest = true, IsPublished = true }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 2, "2"), - Publication = new WorkflowPublication(workflowAddedIsLatest, workflowAddedIsPublished) + Identity = new("a", 2, "2"), + Publication = new(workflowAddedIsLatest, workflowAddedIsPublished) }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -119,11 +120,11 @@ public async Task AddOrUpdateCoreAsync_NewWorkflowDefinitionVersion_IgnoreRootVe IsPublished = true }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 3, "1"), + Identity = new("a", 3, "1"), Version = 1, - Publication = new WorkflowPublication(true, true) + Publication = new(true, true) }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -143,9 +144,9 @@ public async Task AddOrUpdateCoreAsync_NewWorkflowDefinitionVersionWithSameId_Is Version = 1, }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 2, "1") + Identity = new("a", 2, "1") }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -166,10 +167,10 @@ public async Task AddOrUpdateCoreAsync_OlderVersionAsLatest_ExistingVersionShoul IsLatest = true }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 1, "1"), - Publication = new WorkflowPublication(true, true) + Identity = new("a", 1, "1"), + Publication = new(true, true) }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -195,10 +196,10 @@ public async Task AddOrUpdateCoreAsync_OlderVersionAsPublished_ShouldNotBeUpdate } }); - var workflow = new MaterializedWorkflow(new Workflow + var workflow = new MaterializedWorkflow(new() { - Identity = new WorkflowIdentity("a", 1, "1"), - Publication = new WorkflowPublication(true, true) + Identity = new("a", 1, "1"), + Publication = new(true, true) }, "Test", "Test"); await _populator.AddAsync(workflow); @@ -227,4 +228,91 @@ private async Task CheckAmountOfWorkflowsSaved(int count) await _storeMock.Received(count) .SaveManyAsync(Arg.Any>(), Arg.Any()); } + + [Fact(DisplayName = "PopulateStoreAsync imports workflows from current tenant")] + public async Task PopulateStoreAsync_CurrentTenantWorkflows_ImportsWorkflows() + { + var currentTenantId = "tenant-1"; + + var workflow1 = CreateMaterializedWorkflow("workflow-1", "id-1", currentTenantId); + var workflow2 = CreateMaterializedWorkflow("workflow-2", "id-2", currentTenantId); + + var populator = CreatePopulatorWithTenant(currentTenantId, workflow1, workflow2); + var result = await populator.PopulateStoreAsync(); + + Assert.Equal(2, result.Count()); + await _storeMock.Received(2).SaveManyAsync(Arg.Any>(), Arg.Any()); + } + + [Fact(DisplayName = "PopulateStoreAsync skips workflows from other tenants")] + public async Task PopulateStoreAsync_OtherTenantWorkflows_SkipsWorkflows() + { + var currentTenantId = "tenant-1"; + var otherTenantId = "tenant-2"; + + var workflowCurrentTenant = CreateMaterializedWorkflow("workflow-1", "id-1", currentTenantId); + var workflowOtherTenant = CreateMaterializedWorkflow("workflow-2", "id-2", otherTenantId); + + var populator = CreatePopulatorWithTenant(currentTenantId, workflowCurrentTenant, workflowOtherTenant); + var result = await populator.PopulateStoreAsync().ToList(); + + Assert.Single(result); + Assert.Equal("workflow-1", result.First().DefinitionId); + await _storeMock.Received(1).SaveManyAsync(Arg.Any>(), Arg.Any()); + } + + [Theory(DisplayName = "PopulateStoreAsync handles null/empty tenant IDs correctly")] + [InlineData(null, null, true)] // Both null - should import + [InlineData("", "", true)] // Both empty - should import + [InlineData(null, "", true)] // Normalized as same - should import + [InlineData("tenant-1", null, false)] // Different tenants - should skip + [InlineData("tenant-1", "", false)] // Different tenants - should skip + public async Task PopulateStoreAsync_NullOrEmptyTenantIds_HandlesCorrectly(string? currentTenantId, string? workflowTenantId, bool shouldImport) + { + var workflow = CreateMaterializedWorkflow("workflow-1", "id-1", workflowTenantId); + var populator = CreatePopulatorWithTenant(currentTenantId, workflow); + var result = await populator.PopulateStoreAsync(); + + if (shouldImport) + { + Assert.Single(result); + await _storeMock.Received(1).SaveManyAsync(Arg.Any>(), Arg.Any()); + } + else + { + Assert.Empty(result); + await _storeMock.DidNotReceive().SaveManyAsync(Arg.Any>(), Arg.Any()); + } + } + + private MaterializedWorkflow CreateMaterializedWorkflow(string definitionId, string id, string? tenantId) + { + return new(new() + { + Identity = new(definitionId, 1, id, tenantId), + Publication = new(true, true) + }, "Test", "TestProvider"); + } + + private DefaultWorkflowDefinitionStorePopulator CreatePopulatorWithTenant(string? tenantId, params MaterializedWorkflow[] workflows) + { + var tenantAccessor = Substitute.For(); + tenantAccessor.Tenant.Returns(tenantId != null ? new Tenant { Id = tenantId } : null); + + var provider = Substitute.For(); + provider.Name.Returns("TestProvider"); + provider.GetWorkflowsAsync(Arg.Any()) + .Returns(new ValueTask>(workflows)); + + return new( + () => new List { provider }, + Substitute.For(), + _storeMock, + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + tenantAccessor, + Substitute.For>()); + } } \ No newline at end of file