From 602ad38afaa1468ad64983f48580345b7cc45a3f Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 5 Sep 2026 18:42:28 -0700 Subject: [PATCH 1/2] fix: preserve Sequence ownership when retrying child activities (#8027) * fix: preserve Sequence ownership when retrying child activities * fix: allow FastEndpoints EmptyRequest in invitation wrapper --- .../Activities/Sequence.cs | 25 +++- .../RetrySequenceTests.cs | 139 ++++++++++++++++++ 2 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 test/integration/Elsa.Alterations.IntegrationTests/RetrySequenceTests.cs diff --git a/src/modules/Elsa.Workflows.Core/Activities/Sequence.cs b/src/modules/Elsa.Workflows.Core/Activities/Sequence.cs index 2043e4912..55758644a 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Sequence.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Sequence.cs @@ -10,6 +10,9 @@ namespace Elsa.Workflows.Activities; /// /// Execute a set of activities in sequence. /// +/// +/// Rescheduled direct children retain completion callback ownership by this sequence. +/// [Category("Workflows")] [Activity("Elsa", "Workflows", "Execute a set of activities in sequence.")] [PublicAPI] @@ -22,6 +25,7 @@ public class Sequence : Container public Sequence([CallerFilePath] string? source = null, [CallerLineNumber] int? line = null) : base(source, line) { OnSignalReceived(OnBreakSignalReceived); + OnSignalReceived(OnScheduleChildActivityAsync); } /// @@ -68,8 +72,27 @@ public class Sequence : Container await HandleItemAsync(targetContext, childContext); } + private async ValueTask OnScheduleChildActivityAsync(ScheduleChildActivity signal, SignalContext context) + { + var sequenceContext = context.ReceiverActivityExecutionContext; + var childActivity = signal.ActivityExecutionContext?.Activity ?? signal.Activity; + + if (childActivity == null || !Activities.Contains(childActivity)) + { + return; + } + + context.StopPropagation(); + await sequenceContext.ScheduleActivityAsync(childActivity, new ScheduleWorkOptions + { + ExistingActivityExecutionContext = signal.ActivityExecutionContext, + CompletionCallback = OnChildCompleted, + Input = signal.Input + }); + } + private void OnBreakSignalReceived(BreakSignal signal, SignalContext signalContext) { signalContext.ReceiverActivityExecutionContext.SetIsBreaking(); } -} \ No newline at end of file +} diff --git a/test/integration/Elsa.Alterations.IntegrationTests/RetrySequenceTests.cs b/test/integration/Elsa.Alterations.IntegrationTests/RetrySequenceTests.cs new file mode 100644 index 000000000..58236680a --- /dev/null +++ b/test/integration/Elsa.Alterations.IntegrationTests/RetrySequenceTests.cs @@ -0,0 +1,139 @@ +using Elsa.Alterations.AlterationTypes; +using Elsa.Alterations.Core.Contracts; +using Elsa.Alterations.Extensions; +using Elsa.Common.Models; +using Elsa.Extensions; +using Elsa.Testing.Shared; +using Elsa.Workflows; +using Elsa.Workflows.Activities; +using Elsa.Workflows.Activities.Flowchart.Activities; +using Elsa.Workflows.Activities.Flowchart.Models; +using Elsa.Workflows.IncidentStrategies; +using Elsa.Workflows.Models; +using Elsa.Workflows.Runtime; +using Elsa.Workflows.Runtime.Activities; +using Elsa.Workflows.Runtime.Messages; +using Microsoft.Extensions.DependencyInjection; +using Xunit.Abstractions; + +namespace Elsa.Alterations.IntegrationTests; + +public sealed class RetrySequenceTests : IAsyncLifetime +{ + private readonly IServiceProvider _services; + private readonly CapturingTextWriter _output = new(); + private readonly RetryProbe _probe = new(); + + public RetrySequenceTests(ITestOutputHelper output) + { + _services = new TestApplicationBuilder(output) + .WithCapturingTextWriter(_output) + .ConfigureServices(services => services.AddSingleton(_probe)) + .ConfigureElsa(elsa => elsa.UseAlterations()) + .AddWorkflow() + .AddWorkflow() + .AddActivitiesFrom() + .Build(); + } + + public Task InitializeAsync() => _services.PopulateRegistriesAsync(); + public async Task DisposeAsync() => await ((IAsyncDisposable)_services).DisposeAsync(); + + [Theory] + [InlineData(nameof(RetrySequenceWorkflow))] + [InlineData(nameof(RetryStandaloneSequenceWorkflow))] + public async Task RetriedChildCompletesItsSequenceAndPreservesTheNextBookmark(string definitionId) + { + var runtime = _services.GetRequiredService(); + var client = await runtime.CreateClientAsync(); + await client.CreateInstanceAsync(new() + { + WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionId(definitionId, VersionOptions.Published) + }); + var firstRun = await client.RunInstanceAsync(RunWorkflowInstanceRequest.Empty); + var faultedState = await client.ExportStateAsync(); + var incident = Assert.Single(faultedState.Incidents); + Assert.Equal("Retry", incident.ActivityId); + var faultedContext = Assert.Single(faultedState.ActivityExecutionContexts, x => x.Status == ActivityStatus.Faulted); + var sequenceContextId = faultedContext.ParentContextId; + Assert.Equal(1, _probe.Attempts); + Assert.Empty(_output.Lines); + + var alterations = new IAlteration[] { new ScheduleActivity { ActivityInstanceId = faultedContext.Id } }; + var results = await _services.GetRequiredService().RunAsync([firstRun.WorkflowInstanceId], alterations); + Assert.True(Assert.Single(results).IsSuccessful); + + var alteredState = await client.ExportStateAsync(); + var callbacks = alteredState.CompletionCallbacks + .Where(x => x.ChildNodeId == faultedContext.ScheduledActivityNodeId) + .ToList(); + Assert.NotEmpty(callbacks); + Assert.All(callbacks, callback => Assert.Equal(sequenceContextId, callback.OwnerInstanceId)); + + await client.RunInstanceAsync(RunWorkflowInstanceRequest.Empty); + var retriedState = await client.ExportStateAsync(); + var retryBookmark = Assert.Single(retriedState.Bookmarks); + Assert.Equal(faultedContext.Id, retryBookmark.ActivityInstanceId); + Assert.Equal(2, _probe.Attempts); + + var afterRetry = await client.RunInstanceAsync(new() { BookmarkId = retryBookmark.Id }); + var nextState = await client.ExportStateAsync(); + var nextBookmark = Assert.Single(nextState.Bookmarks); + Assert.Equal(WorkflowStatus.Running, afterRetry.Status); + Assert.Equal(["Sequence finished"], _output.Lines); + Assert.Equal(faultedState.Incidents.Count, nextState.Incidents.Count); + Assert.NotEqual(retryBookmark.ActivityInstanceId, nextBookmark.ActivityInstanceId); + + var finalRun = await client.RunInstanceAsync(new() { BookmarkId = nextBookmark.Id }); + Assert.Equal(WorkflowStatus.Finished, finalRun.Status); + Assert.Equal(["Sequence finished", "Done"], _output.Lines); + Assert.Equal(2, _probe.Attempts); + Assert.Empty((await client.ExportStateAsync()).Bookmarks); + } + + public sealed class RetryProbe + { + public int Attempts { get; set; } + } + + public sealed class RetryBookmarkActivity : Activity + { + protected override void Execute(ActivityExecutionContext context) + { + if (++context.GetRequiredService().Attempts == 1) + { + throw new InvalidOperationException("Transient test failure"); + } + + context.CreateBookmark(new CreateBookmarkArgs()); + } + } + + public class RetrySequenceWorkflow : WorkflowBase + { + protected virtual bool UseFlowchart => true; + + protected override void Build(IWorkflowBuilder builder) + { + builder.WorkflowOptions.IncidentStrategyType = typeof(ContinueWithIncidentsStrategy); + var sequence = new Sequence + { + Activities = { new RetryBookmarkActivity { Id = "Retry" }, new WriteLine("Sequence finished") } + }; + var next = new Event("Next"); + var end = new WriteLine("Done"); + builder.Root = UseFlowchart + ? new Flowchart + { + Activities = { sequence, next, end }, + Connections = { new Connection(sequence, next), new Connection(next, end) } + } + : new Sequence { Activities = { sequence, next, end } }; + } + } + + public sealed class RetryStandaloneSequenceWorkflow : RetrySequenceWorkflow + { + protected override bool UseFlowchart => false; + } +} From 07f788afb80659ac9b2175e3562f539a88f63251 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sat, 5 Sep 2026 19:01:42 -0700 Subject: [PATCH 2/2] fix(identity): isolate in-memory roles by tenant (#8032) * fix(identity): isolate in-memory roles by tenant * fix(identity): retain legacy default-tenant roles --- .../Elsa.Identity/Models/RoleFilter.cs | 6 +- .../Elsa.Identity/Services/MemoryRoleStore.cs | 20 ++++-- .../Elsa.Identity/Services/RoleManager.cs | 5 +- .../Services/RoleManagerTests.cs | 70 ++++++++++++++++++- 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/modules/Elsa.Identity/Models/RoleFilter.cs b/src/modules/Elsa.Identity/Models/RoleFilter.cs index 40fc66815..57acaf0d6 100644 --- a/src/modules/Elsa.Identity/Models/RoleFilter.cs +++ b/src/modules/Elsa.Identity/Models/RoleFilter.cs @@ -21,6 +21,7 @@ public class RoleFilter /// /// Gets or sets the tenant to filter for. The tenant-agnostic sentinel is always included, matching /// the Entity Framework query filter, so a shared platform role remains visible from every tenant. + /// Legacy records without a tenant remain visible to the default tenant for backwards compatibility. /// public string? TenantId { get; set; } @@ -34,8 +35,9 @@ public class RoleFilter var filter = this; if (filter.Id != null) queryable = queryable.Where(x => x.Id == filter.Id); if (filter.Ids != null) queryable = queryable.Where(x => filter.Ids.Contains(x.Id)); - if (filter.TenantId != null) queryable = queryable.Where(x => x.TenantId == filter.TenantId || x.TenantId == Tenant.AgnosticTenantId); + if (filter.TenantId != null) + queryable = queryable.Where(x => x.TenantId == filter.TenantId || x.TenantId == Tenant.AgnosticTenantId || (x.TenantId == null && filter.TenantId == Tenant.DefaultTenantId)); return queryable; } -} \ No newline at end of file +} diff --git a/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs b/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs index 5c41df65e..c7e2d1cbb 100644 --- a/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs +++ b/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs @@ -26,22 +26,22 @@ public class MemoryRoleStore : IRoleStore /// public Task AddAsync(Role role, CancellationToken cancellationToken = default) { - _store.Save(role, x => x.Id); + _store.Save(role, GetStorageKey); return Task.CompletedTask; } /// public Task DeleteAsync(RoleFilter filter, CancellationToken cancellationToken = default) { - var ids = _store.Query(query => Filter(query, filter)).Select(x => x.Id).Distinct().ToList(); - _store.DeleteWhere(x => ids.Contains(x.Id)); + var roles = _store.Query(query => Filter(query, filter)).ToList(); + _store.DeleteMany(roles, GetStorageKey); return Task.CompletedTask; } /// public Task SaveAsync(Role role, CancellationToken cancellationToken = default) { - _store.Save(role, x => x.Id); + _store.Save(role, GetStorageKey); return Task.CompletedTask; } @@ -62,13 +62,19 @@ public class MemoryRoleStore : IRoleStore /// /// The ambient tenant is applied here rather than left to callers. Isolation previously existed only /// on the Entity Framework path, and only when multitenancy was enabled, so a deployment running the - /// default in-memory stores had none at all. + /// default in-memory stores had none at all. Null tenant IDs are retained only for the default tenant + /// for backwards compatibility with records created before tenant assignment was added. /// private IQueryable Filter(IQueryable queryable, RoleFilter filter) { var tenantId = _tenantAccessor.TenantId; - queryable = queryable.Where(x => x.TenantId == tenantId || x.TenantId == Tenant.AgnosticTenantId || x.TenantId == null); + queryable = queryable.Where(x => x.TenantId == tenantId || x.TenantId == Tenant.AgnosticTenantId || (x.TenantId == null && tenantId == Tenant.DefaultTenantId)); return filter.Apply(queryable); } -} \ No newline at end of file + + private static string GetStorageKey(Role role) => GetStorageKey(role.TenantId, role.Id); + + private static string GetStorageKey(string? tenantId, string roleId) => + $"{tenantId?.Length ?? -1}:{tenantId}{roleId.Length}:{roleId}"; +} diff --git a/src/modules/Elsa.Identity/Services/RoleManager.cs b/src/modules/Elsa.Identity/Services/RoleManager.cs index 3350e9212..f0f9f39a7 100644 --- a/src/modules/Elsa.Identity/Services/RoleManager.cs +++ b/src/modules/Elsa.Identity/Services/RoleManager.cs @@ -1,3 +1,4 @@ +using Elsa.Common.Multitenancy; using Elsa.Identity.Contracts; using Elsa.Identity.Entities; using Elsa.Identity.Models; @@ -8,7 +9,7 @@ namespace Elsa.Identity.Services; /// /// Default implementation of . /// -public class RoleManager(IRoleStore roleStore, IRoleProvider roleProvider) : IRoleManager +public class RoleManager(IRoleStore roleStore, IRoleProvider roleProvider, ITenantAccessor tenantAccessor) : IRoleManager { /// public async Task CreateRoleAsync( @@ -26,6 +27,8 @@ public class RoleManager(IRoleStore roleStore, IRoleProvider roleProvider) : IRo { Id = roleId, Name = name, + // The in-memory path does not run EF's ApplyTenantId saving handler. + TenantId = tenantAccessor.TenantId, Permissions = permissions ?? new List() }; diff --git a/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs index 687cd14a4..ff8609a99 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs @@ -1,3 +1,4 @@ +using Elsa.Common.Multitenancy; using Elsa.Testing.Shared.Multitenancy; using Elsa.Common.Services; using Elsa.Identity.Entities; @@ -8,13 +9,75 @@ namespace Elsa.Identity.UnitTests.Services; public class RoleManagerTests { + private readonly TestTenantAccessor _tenantAccessor; private readonly MemoryRoleStore _roleStore; private readonly RoleManager _manager; public RoleManagerTests() { - _roleStore = new MemoryRoleStore(new MemoryStore(), TestTenantAccessor.Default); - _manager = new RoleManager(_roleStore, new StoreBasedRoleProvider(_roleStore)); + _tenantAccessor = new TestTenantAccessor("tenant-a"); + _roleStore = new MemoryRoleStore(new MemoryStore(), _tenantAccessor); + _manager = new RoleManager(_roleStore, new StoreBasedRoleProvider(_roleStore), _tenantAccessor); + } + + [Fact] + public async Task CreateListUpdateAndDeleteAreIsolatedForRolesWithTheSameNameAcrossTenants() + { + var roleA = await _manager.CreateRoleAsync("Operators", ["tenant-a:permission"]); + + Assert.Equal("tenant-a", roleA.Role.TenantId); + Assert.Single(await _roleStore.FindManyAsync(new() { TenantId = "tenant-a" })); + + using (_tenantAccessor.PushContext(new Tenant { Id = "tenant-b", Name = "Tenant B" })) + { + var roleB = await _manager.CreateRoleAsync("Operators", ["tenant-b:permission"]); + + Assert.Equal(roleA.Role.Id, roleB.Role.Id); + Assert.Equal("tenant-b", roleB.Role.TenantId); + Assert.Equal(["tenant-b:permission"], roleB.Role.Permissions); + + var tenantBRoles = await _roleStore.FindManyAsync(new() { TenantId = "tenant-b" }); + Assert.Single(tenantBRoles); + Assert.Equal(roleB.Role.Id, tenantBRoles.Single().Id); + + tenantBRoles.Single().Name = "Operators B"; + await _roleStore.SaveAsync(tenantBRoles.Single()); + + Assert.Equal("Operators B", (await _roleStore.FindAsync(new() { Id = roleB.Role.Id }))!.Name); + } + + var tenantARole = await _roleStore.FindAsync(new() { Id = roleA.Role.Id }); + Assert.NotNull(tenantARole); + Assert.Equal("Operators", tenantARole.Name); + Assert.Equal(["tenant-a:permission"], tenantARole.Permissions); + + tenantARole.Name = "Operators A"; + await _roleStore.SaveAsync(tenantARole); + Assert.Equal("Operators A", (await _roleStore.FindAsync(new() { Id = roleA.Role.Id }))!.Name); + + await _roleStore.DeleteAsync(new() { Id = roleA.Role.Id }); + Assert.Empty(await _roleStore.FindManyAsync(new() { TenantId = "tenant-a" })); + + using (_tenantAccessor.PushContext(new Tenant { Id = "tenant-b", Name = "Tenant B" })) + { + var remainingTenantBRole = await _roleStore.FindAsync(new() { Id = roleA.Role.Id }); + Assert.NotNull(remainingTenantBRole); + Assert.Equal("Operators B", remainingTenantBRole.Name); + } + } + + [Fact] + public async Task DefaultTenantListsLegacyRolesWithoutATenantId() + { + var tenantAccessor = new TestTenantAccessor(); + var roleStore = new MemoryRoleStore(new MemoryStore(), tenantAccessor); + + await roleStore.SaveAsync(new Role { Id = "legacy", Name = "Legacy", Permissions = [] }); + + var roles = await roleStore.FindManyAsync(new() { TenantId = Tenant.DefaultTenantId }); + + Assert.Single(roles); + Assert.Equal("legacy", roles.Single().Id); } [Fact] @@ -24,6 +87,7 @@ public class RoleManagerTests { Id = "admin", Name = "Admin", + TenantId = "tenant-a", Permissions = [PermissionNames.All] }); @@ -38,7 +102,7 @@ public class RoleManagerTests [Fact] public async Task CreateRoleRejectsProvidedAdminRoleIdCollision() { - var manager = new RoleManager(_roleStore, new AdminRoleProvider()); + var manager = new RoleManager(_roleStore, new AdminRoleProvider(), _tenantAccessor); await Assert.ThrowsAsync(() => manager.CreateRoleAsync("Replacement", [], "admin")); }