fix(features): support features introduced during Module.Apply() (#7966)

Module.Apply() enumerated _features.Values directly while calling
feature.Apply(). A feature whose Apply() introduces another feature —
Module.Configure<T>() directly, or via a helper such as AddActivity<T>()
which configures WorkflowManagementFeature — mutated that collection
mid-enumeration and threw "Collection was modified; enumeration
operation may not execute", naming nothing about features. Whether it
fired depended on whether the other feature happened to be installed
already, so a module built or did not based on unrelated host config.

The module already treats introduction-during-apply as supported: the
ConfigureFeature loop iterates a snapshot for exactly this reason, and
Configure<T>() has an _isApplying branch that creates, resolves and
configures a feature introduced mid-Apply. Only the final apply loop
missed the same treatment, so make it tolerant rather than diagnose a
constraint the code does not hold.

The apply loop now runs in rounds until no new features appear, each
round topologically sorted so a late feature's dependencies apply before
it. Hosted services are registered in a single pass after that loop,
then moved back to the index the block previously occupied: registering
late is needed so features contributed during Apply() are included and
ordered by priority, while keeping the position matters because features
register hosted services directly from Apply() — WorkflowRuntimeFeature
adds DrainOrchestratorHostedService that way — and module-managed
services must keep starting first, or a priority such as ActivateTenants
at -1 would silently start ordering after them.

Adds Elsa.Features.UnitTests, covering the introduced feature applying,
a three-deep introduction chain, dependency ordering, hosted service
registration and priority ordering for late arrivals, the installed-
feature registry, and no double-apply, plus guards for pre-existing
ordering behaviour.

Closes #7944

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Elsa.sln

@@ -327,6 +327,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Http.IntegrationTests"
 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
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Features.UnitTests", "test\unit\Elsa.Features.UnitTests\Elsa.Features.UnitTests.csproj", "{F77B17C4-B722-402F-A238-48CDC5AE7565}"
+EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "shells", "shells", "{FD31E565-A5D9-4F25-B484-2F27FAB99B17}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Shells.Api", "src\modules\Elsa.Shells.Api\Elsa.Shells.Api.csproj", "{23BBF2CE-D2BA-43D4-8C78-A0B7B4AD6219}"
@@ -1333,6 +1335,18 @@ Global
 		{DC476900-D836-4920-A696-CF8796668723}.Release|x64.Build.0 = Release|Any CPU
 		{DC476900-D836-4920-A696-CF8796668723}.Release|x86.ActiveCfg = Release|Any CPU
 		{DC476900-D836-4920-A696-CF8796668723}.Release|x86.Build.0 = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|x64.Build.0 = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Debug|x86.Build.0 = Debug|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|Any CPU.Build.0 = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|x64.ActiveCfg = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|x64.Build.0 = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|x86.ActiveCfg = Release|Any CPU
+		{F77B17C4-B722-402F-A238-48CDC5AE7565}.Release|x86.Build.0 = Release|Any CPU
 		{23BBF2CE-D2BA-43D4-8C78-A0B7B4AD6219}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{23BBF2CE-D2BA-43D4-8C78-A0B7B4AD6219}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{23BBF2CE-D2BA-43D4-8C78-A0B7B4AD6219}.Debug|x64.ActiveCfg = Debug|Any CPU
@@ -2382,6 +2396,7 @@ Global
 		{A3C07D5B-2A30-494E-B9BC-4B1594B31ABC} = {18453B51-25EB-4317-A4B3-B10518252E92}
 		{8C4F6A2D-1E9F-4B3C-9D8E-7F5A6B4C3D2E} = {1B8D5897-902E-4632-8698-E89CAF3DDF54}
 		{DC476900-D836-4920-A696-CF8796668723} = {18453B51-25EB-4317-A4B3-B10518252E92}
+		{F77B17C4-B722-402F-A238-48CDC5AE7565} = {18453B51-25EB-4317-A4B3-B10518252E92}
 		{FD31E565-A5D9-4F25-B484-2F27FAB99B17} = {5BA4A8FA-F7F4-45B3-AEC8-8886D35AAC79}
 		{23BBF2CE-D2BA-43D4-8C78-A0B7B4AD6219} = {FD31E565-A5D9-4F25-B484-2F27FAB99B17}
 		{85E81AC4-10EF-4A93-A7A3-930087621ACC} = {D92BEAB2-60D6-4BB4-885A-6BA681C6CCF1}

doc/wiki/module-system.md

@@ -23,6 +23,8 @@ Feature classes usually use three lifecycle methods:
 
 `Module.Apply()` topologically sorts configured features and dependencies, configures them once, filters features with missing optional dependencies, registers hosted services, applies services, and finally registers installed-feature metadata.
 
+A feature may introduce another feature from `Apply()`, for example by calling `Module.Configure<OtherFeature>()` directly or through a helper such as `AddActivity<T>()`. `Module.Apply()` keeps applying until no new features show up, so the introduced feature (and its dependencies) are configured and applied as well.
+
 ## Entry Points
 
 The common public path is:
@@ -93,4 +95,4 @@ When adding a new module, follow this shape:
 - Do not register services in extension methods when the module already has a feature class. Put service registration in `Apply()`.
 - Do not bypass dependencies with direct service provider access in unrelated modules. Add a contract and dependency if the relationship is real.
 - Use `TryAdd*` for overridable defaults and normal `Add*` for deliberate multiple registrations such as handlers, validators, and descriptors.
-- If a feature uses `Module.Configure<OtherFeature>()`, verify that the other feature is already a dependency or that optional behavior is intentional.
+- If a feature uses `Module.Configure<OtherFeature>()`, verify that the other feature is already a dependency or that optional behavior is intentional. Introducing a feature this way works from `Configure()` and from `Apply()`, but declaring `[DependsOn(typeof(OtherFeature))]` keeps the relationship visible and gives the dependency the normal ordering.

src/common/Elsa.Features/Implementations/Module.cs

@@ -106,13 +106,23 @@ public class Module : IModule
         // Filter out features that depend on other features that are not installed.
         _features = ExcludeFeaturesWithMissingDependencies(_features.Values).ToDictionary(x => x.GetType(), x => x);
 
-        // Add hosted services in order of priority.
-        foreach (var hostedServiceDescriptor in _hostedServiceDescriptors.OrderBy(x => x.Order))
-            Services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IHostedService), hostedServiceDescriptor.Type));
+        // Hosted services are registered after the features have been applied, because applying a feature can contribute more of them. They still belong
+        // at this position in the service collection though, ahead of whatever the features register from their Apply method, so remember where that is.
+        var hostedServiceIndex = Services.Count;
 
-        // Make sure to use the complete list of features when applying them.
-        foreach (var feature in _features.Values)
-            feature.Apply();
+        // Make sure to use the complete list of features when applying them. Applying a feature can introduce additional features (a feature calling
+        // Module.Configure<T>() from its Apply method, directly or through a helper such as AddActivity<T>()), so keep going until nothing new shows up.
+        var appliedFeatures = new HashSet<IFeature>();
+        while (GetFeaturesPendingApply(appliedFeatures) is { Count: > 0 } pendingFeatures)
+        {
+            appliedFeatures.UnionWith(pendingFeatures);
+
+            foreach (var feature in pendingFeatures)
+                feature.Apply();
+        }
+
+        // Add hosted services in order of priority.
+        RegisterHostedServices(hostedServiceIndex);
 
         // Add a registry of enabled features to the service collection for client applications to reflect on what features are installed.
         var registry = new InstalledFeatureRegistry();
@@ -141,6 +151,40 @@ public class Module : IModule
             select feature;
     }
 
+    // Registers the configured hosted services in order of priority, starting at the specified index.
+    private void RegisterHostedServices(int index)
+    {
+        var appendIndex = Services.Count;
+
+        foreach (var hostedServiceDescriptor in _hostedServiceDescriptors.OrderBy(x => x.Order))
+            Services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IHostedService), hostedServiceDescriptor.Type));
+
+        // TryAddEnumerable appends, so move what it added back to where hosted services configured through this module belong. Doing it this way rather
+        // than inserting directly keeps the de-duplication behaviour of TryAddEnumerable, which also considers what the features registered themselves.
+        var appendedDescriptors = Services.Skip(appendIndex).ToList();
+
+        for (var i = 0; i < appendedDescriptors.Count; i++)
+            Services.RemoveAt(appendIndex);
+
+        for (var i = 0; i < appendedDescriptors.Count; i++)
+            Services.Insert(index + i, appendedDescriptors[i]);
+    }
+
+    // Returns the features that have not been applied yet, sorted so that dependencies are applied before the features that depend on them.
+    private List<IFeature> GetFeaturesPendingApply(IReadOnlySet<IFeature> appliedFeatures)
+    {
+        var pendingFeatureTypes = _features.Where(x => !appliedFeatures.Contains(x.Value)).Select(x => x.Key).ToList();
+        var pendingFeatureTypeLookup = pendingFeatureTypes.ToHashSet();
+
+        // Sorting pulls in dependencies that are not pending themselves, so filter those out again.
+        return pendingFeatureTypes
+            .TSort(GetDeclaredDependencyTypes)
+            .Where(pendingFeatureTypeLookup.Contains)
+            .Select(x => _features[x])
+            .Distinct()
+            .ToList();
+    }
+
     private void ConfigureFeature(IFeature feature)
     {
         if (_configuredFeatures.Contains(feature))
@@ -161,13 +205,18 @@ public class Module : IModule
     {
         var featureTypes = _features.Keys.ToHashSet();
         var featureTypesWithDependencies = featureTypes.Concat(featureTypes.SelectMany(GetDependencyTypes)).ToHashSet();
-        return featureTypesWithDependencies.TSort(x => x.GetCustomAttributes<DependsOnAttribute>(true).Select(dependsOn => dependsOn.Type)).ToHashSet();
+        return featureTypesWithDependencies.TSort(GetDeclaredDependencyTypes).ToHashSet();
+    }
+
+    private static IEnumerable<Type> GetDeclaredDependencyTypes(Type type)
+    {
+        return type.GetCustomAttributes<DependsOnAttribute>(true).Select(dependsOn => dependsOn.Type);
     }
 
     // Recursively get dependency types.
     private IEnumerable<Type> GetDependencyTypes(Type type)
     {
-        var dependencies = type.GetCustomAttributes<DependsOnAttribute>(true).Select(dependsOn => dependsOn.Type).ToList();
+        var dependencies = GetDeclaredDependencyTypes(type).ToList();
         return dependencies.Concat(dependencies.SelectMany(GetDependencyTypes));
     }
 }

src/common/Elsa.Features/Services/IModule.cs

@@ -51,5 +51,9 @@ public interface IModule
     /// <summary>
     /// Will apply all configured features, causing the <see cref="Services"/> collection to be populated. 
     /// </summary>
+    /// <remarks>
+    /// Features are allowed to introduce additional features from their own <see cref="IFeature.Apply"/> method, e.g. by calling <see cref="Configure{T}(Action{T})"/>.
+    /// Those features are configured and applied as part of the same call.
+    /// </remarks>
     void Apply();
 }

test/unit/Elsa.Features.UnitTests/Elsa.Features.UnitTests.csproj

@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+    <PropertyGroup>
+        <Include>[Elsa.Features]*</Include>
+        <Threshold>0</Threshold>
+    </PropertyGroup>
+
+    <ItemGroup>
+        <PackageReference Include="Microsoft.Extensions.DependencyInjection"/>
+    </ItemGroup>
+
+    <ItemGroup>
+        <ProjectReference Include="..\..\..\src\common\Elsa.Features\Elsa.Features.csproj"/>
+    </ItemGroup>
+
+</Project>

test/unit/Elsa.Features.UnitTests/ModuleTests.cs

@@ -0,0 +1,240 @@
+using Elsa.Features.Abstractions;
+using Elsa.Features.Attributes;
+using Elsa.Features.Contracts;
+using Elsa.Features.Implementations;
+using Elsa.Features.Services;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+
+namespace Elsa.Features.UnitTests;
+
+/// <summary>
+/// Tests <see cref="Module.Apply"/>, with an emphasis on features that introduce additional features from their own <see cref="IFeature.Apply"/> method.
+/// </summary>
+public class ModuleTests
+{
+    private const string AppliedFeaturesKey = "AppliedFeatures";
+
+    private readonly ServiceCollection _services = new();
+    private readonly Module _module;
+    private readonly List<Type> _appliedFeatures = new();
+
+    public ModuleTests()
+    {
+        _module = new(_services);
+        _module.Properties[AppliedFeaturesKey] = _appliedFeatures;
+    }
+
+    [Fact]
+    public void Apply_AppliesFeatureIntroducedFromApply()
+    {
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.Contains(typeof(IntroducedFeature), _appliedFeatures);
+        Assert.Contains(_services, x => x.ServiceType == typeof(IntroducedMarker));
+    }
+
+    [Fact]
+    public void Apply_AppliesEntireChainOfFeaturesIntroducedFromApply()
+    {
+        _module.Configure<ChainIntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.Contains(typeof(ChainMiddleFeature), _appliedFeatures);
+        Assert.Contains(typeof(ChainLeafFeature), _appliedFeatures);
+    }
+
+    [Fact]
+    public void Apply_AppliesDependenciesOfFeatureIntroducedFromApplyBeforeThatFeature()
+    {
+        _module.Configure<IntroducingDependentFeature>();
+
+        _module.Apply();
+
+        Assert.Contains(typeof(IntroducedDependencyFeature), _appliedFeatures);
+        Assert.True(_appliedFeatures.IndexOf(typeof(IntroducedDependencyFeature)) < _appliedFeatures.IndexOf(typeof(IntroducedDependentFeature)));
+    }
+
+    [Fact]
+    public void Apply_RegistersHostedServicesOfFeatureIntroducedFromApply()
+    {
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.Contains(_services, x => x.ServiceType == typeof(IHostedService) && x.ImplementationType == typeof(IntroducedHostedService));
+    }
+
+    [Fact]
+    public void Apply_ListsFeatureIntroducedFromApplyInTheInstalledFeatureRegistry()
+    {
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.NotNull(GetInstalledFeatureRegistry().Find("Elsa.Introduced"));
+    }
+
+    [Fact]
+    public void Apply_AppliesEachFeatureOnlyOnce()
+    {
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.Equal(_appliedFeatures.Distinct().Count(), _appliedFeatures.Count);
+    }
+
+    [Fact]
+    public void Apply_AppliesFeaturesInDependencyOrder()
+    {
+        _module.Configure<DependentFeature>();
+
+        _module.Apply();
+
+        Assert.Equal([typeof(DependencyFeature), typeof(DependentFeature)], _appliedFeatures);
+    }
+
+    [Fact]
+    public void Apply_RegistersHostedServicesInPriorityOrder()
+    {
+        _module.ConfigureHostedService<SecondHostedService>(2);
+        _module.ConfigureHostedService<FirstHostedService>(1);
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        Assert.Equal([typeof(FirstHostedService), typeof(SecondHostedService), typeof(IntroducedHostedService)], GetHostedServiceTypes());
+    }
+
+    [Fact]
+    public void Apply_OrdersHostedServiceOfFeatureIntroducedFromApplyByPriority()
+    {
+        _module.ConfigureHostedService<SecondHostedService>(10);
+        _module.Configure<IntroducingFeature>();
+
+        _module.Apply();
+
+        // The introduced feature configures its hosted service at priority 3, so it has to come first even though it shows up last.
+        Assert.Equal([typeof(IntroducedHostedService), typeof(SecondHostedService)], GetHostedServiceTypes());
+    }
+
+    [Fact]
+    public void Apply_RegistersConfiguredHostedServicesBeforeThoseRegisteredFromApply()
+    {
+        _module.ConfigureHostedService<FirstHostedService>();
+        _module.Configure<HostedServiceRegisteringFeature>();
+
+        _module.Apply();
+
+        Assert.Equal([typeof(FirstHostedService), typeof(SelfRegisteredHostedService)], GetHostedServiceTypes());
+    }
+
+    private List<Type?> GetHostedServiceTypes()
+    {
+        return _services.Where(x => x.ServiceType == typeof(IHostedService)).Select(x => x.ImplementationType).ToList();
+    }
+
+    private IInstalledFeatureRegistry GetInstalledFeatureRegistry()
+    {
+        return (IInstalledFeatureRegistry)_services.Single(x => x.ServiceType == typeof(IInstalledFeatureRegistry)).ImplementationInstance!;
+    }
+
+    /// <summary>
+    /// Records the order in which features are applied so that tests can assert on it.
+    /// </summary>
+    public abstract class RecordingFeature(IModule module) : FeatureBase(module)
+    {
+        public override void Apply() => ((List<Type>)Module.Properties[AppliedFeaturesKey]).Add(GetType());
+    }
+
+    public class IntroducingFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void Apply()
+        {
+            base.Apply();
+            Module.Configure<IntroducedFeature>();
+        }
+    }
+
+    public class IntroducedFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void ConfigureHostedServices() => ConfigureHostedService<IntroducedHostedService>(3);
+
+        public override void Apply()
+        {
+            base.Apply();
+            Services.AddSingleton<IntroducedMarker>();
+        }
+    }
+
+    /// <summary>
+    /// Registers a hosted service straight with the service collection, the way <c>WorkflowRuntimeFeature</c> does, rather than through the module.
+    /// </summary>
+    public class HostedServiceRegisteringFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void Apply()
+        {
+            base.Apply();
+            Services.AddHostedService<SelfRegisteredHostedService>();
+        }
+    }
+
+    public class ChainIntroducingFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void Apply()
+        {
+            base.Apply();
+            Module.Configure<ChainMiddleFeature>();
+        }
+    }
+
+    public class ChainMiddleFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void Apply()
+        {
+            base.Apply();
+            Module.Configure<ChainLeafFeature>();
+        }
+    }
+
+    public class ChainLeafFeature(IModule module) : RecordingFeature(module);
+
+    public class IntroducingDependentFeature(IModule module) : RecordingFeature(module)
+    {
+        public override void Apply()
+        {
+            base.Apply();
+            Module.Configure<IntroducedDependentFeature>();
+        }
+    }
+
+    [DependsOn(typeof(IntroducedDependencyFeature))]
+    public class IntroducedDependentFeature(IModule module) : RecordingFeature(module);
+
+    public class IntroducedDependencyFeature(IModule module) : RecordingFeature(module);
+
+    [DependsOn(typeof(DependencyFeature))]
+    public class DependentFeature(IModule module) : RecordingFeature(module);
+
+    public class DependencyFeature(IModule module) : RecordingFeature(module);
+
+    public class IntroducedMarker;
+
+    public abstract class NoopHostedService : IHostedService
+    {
+        public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+        public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+    }
+
+    public class IntroducedHostedService : NoopHostedService;
+
+    public class SelfRegisteredHostedService : NoopHostedService;
+
+    public class FirstHostedService : NoopHostedService;
+
+    public class SecondHostedService : NoopHostedService;
+}