Merge pull request #5621 from Particular/backport-startuptask-order

Start FeatureStartupTasks in feature dependency order

src/NServiceBus.AcceptanceTests/Core/Feature/When_depending_on_feature.cs

@@ -0,0 +1,143 @@
+namespace NServiceBus.AcceptanceTests.Core.Feature
+{
+    using System;
+    using System.Threading.Tasks;
+    using AcceptanceTesting;
+    using EndpointTemplates;
+    using Features;
+    using NUnit.Framework;
+
+    public class When_depending_on_feature : NServiceBusAcceptanceTest
+    {
+        [Test]
+        public async Task Should_start_startup_tasks_in_order_of_dependency()
+        {
+            var context = await Scenario.Define<Context>()
+                .WithEndpoint<EndpointWithFeatures>(b => b.CustomConfig(c =>
+                {
+                    c.EnableFeature<DependencyFeature>();
+                    c.EnableFeature<TypedDependentFeature>();
+                }))
+                .Done(c => c.EndpointsStarted)
+                .Run();
+
+            Assert.That(context.StartCalled, Is.True);
+            Assert.That(context.StopCalled, Is.True);
+        }
+
+        class Context : ScenarioContext
+        {
+            public bool StartCalled { get; set; }
+            public bool StopCalled { get; set; }
+            public bool InitializeCalled { get; set; }
+        }
+
+        public class EndpointWithFeatures : EndpointConfigurationBuilder
+        {
+            public EndpointWithFeatures()
+            {
+                EndpointSetup<DefaultServer>();
+            }
+        }
+
+        public class TypedDependentFeature : Feature
+        {
+            public TypedDependentFeature()
+            {
+                DependsOn<DependencyFeature>();
+            }
+
+            protected override void Setup(FeatureConfigurationContext context)
+            {
+                context.Container.ConfigureComponent<Runner>(DependencyLifecycle.SingleInstance);
+                context.RegisterStartupTask(b => b.Build<Runner>());
+            }
+
+            class Runner : FeatureStartupTask
+            {
+                Dependency dependency;
+
+                public Runner(Dependency dependency)
+                {
+                    this.dependency = dependency;
+                }
+                protected override Task OnStart(IMessageSession session)
+                {
+                    dependency.Start();
+                    return Task.FromResult(0);
+                }
+
+                protected override Task OnStop(IMessageSession session)
+                {
+                    dependency.Stop();
+                    return Task.FromResult(0);
+                }
+            }
+        }
+
+        public class DependencyFeature : Feature
+        {
+            protected override void Setup(FeatureConfigurationContext context)
+            {
+                context.Container.ConfigureComponent<Dependency>(DependencyLifecycle.SingleInstance);
+
+                context.Container.ConfigureComponent<Runner>(DependencyLifecycle.SingleInstance);
+                context.RegisterStartupTask(b => b.Build<Runner>());
+            }
+
+            class Runner : FeatureStartupTask
+            {
+                Dependency dependency;
+
+                public Runner(Dependency dependency)
+                {
+                    this.dependency = dependency;
+                }
+                protected override Task OnStart(IMessageSession session)
+                {
+                    dependency.Initialize();
+                    return Task.FromResult(0);
+                }
+
+                protected override Task OnStop(IMessageSession session)
+                {
+                    return Task.FromResult(0);
+                }
+            }
+        }
+
+        class Dependency
+        {
+            Context context;
+
+            public Dependency(Context context)
+            {
+                this.context = context;
+            }
+
+            public void Start()
+            {
+                if (!context.InitializeCalled)
+                {
+                    throw new InvalidOperationException("Not initialized");
+                }
+                context.StartCalled = true;
+            }
+
+            public void Stop()
+            {
+                if (!context.InitializeCalled)
+                {
+                    throw new InvalidOperationException("Not initialized");
+                }
+
+                context.StopCalled = true;
+            }
+
+            public void Initialize()
+            {
+                context.InitializeCalled = true;
+            }
+        }
+    }
+}

src/NServiceBus.Core.Tests/Features/FeatureStartupTests.cs

@@ -2,6 +2,7 @@
 {
     using System;
     using System.Collections.Generic;
+    using System.Text;
     using System.Threading.Tasks;
     using NServiceBus.Features;
     using NUnit.Framework;
@@ -34,6 +35,28 @@ public async Task Should_start_and_stop_features()
             Assert.True(feature.TaskStopped);
         }
 
+        [Test]
+        public async Task Should_start_and_stop_features_in_dependency_order()
+        {
+            var orderBuilder = new StringBuilder();
+
+            featureSettings.Add(new FeatureWithStartupTaskWithDependency(orderBuilder));
+            featureSettings.Add(new FeatureWithStartupThatAnotherFeatureDependsOn(orderBuilder));
+
+            featureSettings.SetupFeatures(new FakeFeatureConfigurationContext());
+
+            await featureSettings.StartFeatures(null, null);
+            await featureSettings.StopFeatures();
+
+            var expectedOrderBuilder = new StringBuilder();
+            expectedOrderBuilder.AppendLine("FeatureWithStartupThatAnotherFeatureDependsOn.Start");
+            expectedOrderBuilder.AppendLine("FeatureWithStartupTaskWithDependency.Start");
+            expectedOrderBuilder.AppendLine("FeatureWithStartupThatAnotherFeatureDependsOn.Stop");
+            expectedOrderBuilder.AppendLine("FeatureWithStartupTaskWithDependency.Stop");
+
+            Assert.AreEqual(expectedOrderBuilder.ToString(), orderBuilder.ToString());
+        }
+
         [Test]
         public async Task Should_dispose_feature_startup_tasks_when_they_implement_IDisposable()
         {
@@ -99,6 +122,85 @@ public async Task Should_dispose_feature_task_even_when_stop_throws()
         FeatureActivator featureSettings;
         SettingsHolder settings;
 
+        class FeatureWithStartupTaskWithDependency : TestFeature
+        {
+            public FeatureWithStartupTaskWithDependency(StringBuilder orderBuilder)
+            {
+                EnableByDefault();
+                DependsOn<FeatureWithStartupThatAnotherFeatureDependsOn>();
+
+                this.orderBuilder = orderBuilder;
+            }
+
+            protected internal override void Setup(FeatureConfigurationContext context)
+            {
+                context.RegisterStartupTask(new Runner(orderBuilder));
+            }
+
+            class Runner : FeatureStartupTask
+            {
+                public Runner(StringBuilder orderBuilder)
+                {
+                    this.orderBuilder = orderBuilder;
+                }
+
+                protected override Task OnStart(IMessageSession session)
+                {
+                    orderBuilder.AppendLine($"{nameof(FeatureWithStartupTaskWithDependency)}.Start");
+                    return TaskEx.CompletedTask;
+                }
+
+                protected override Task OnStop(IMessageSession session)
+                {
+                    orderBuilder.AppendLine($"{nameof(FeatureWithStartupTaskWithDependency)}.Stop");
+                    return TaskEx.CompletedTask;
+                }
+
+                StringBuilder orderBuilder;
+            }
+
+            readonly StringBuilder orderBuilder;
+        }
+
+        class FeatureWithStartupThatAnotherFeatureDependsOn : TestFeature
+        {
+            public FeatureWithStartupThatAnotherFeatureDependsOn(StringBuilder orderBuilder)
+            {
+                EnableByDefault();
+
+                this.orderBuilder = orderBuilder;
+            }
+
+            protected internal override void Setup(FeatureConfigurationContext context)
+            {
+                context.RegisterStartupTask(new Runner(orderBuilder));
+            }
+
+            class Runner : FeatureStartupTask
+            {
+                public Runner(StringBuilder orderBuilder)
+                {
+                    this.orderBuilder = orderBuilder;
+                }
+
+                protected override Task OnStart(IMessageSession session)
+                {
+                    orderBuilder.AppendLine($"{nameof(FeatureWithStartupThatAnotherFeatureDependsOn)}.Start");
+                    return TaskEx.CompletedTask;
+                }
+
+                protected override Task OnStop(IMessageSession session)
+                {
+                    orderBuilder.AppendLine($"{nameof(FeatureWithStartupThatAnotherFeatureDependsOn)}.Stop");
+                    return TaskEx.CompletedTask;
+                }
+
+                StringBuilder orderBuilder;
+            }
+
+            readonly StringBuilder orderBuilder;
+        }
+
         class FeatureWithStartupTask : TestFeature
         {
             public FeatureWithStartupTask()

src/NServiceBus.Core/Features/FeatureActivator.cs

@@ -40,7 +40,6 @@ public FeatureDiagnosticData[] SetupFeatures(FeatureConfigurationContext feature
             // featuresToActivate is enumerated twice because after setting defaults some new features might got activated.
             var sourceFeatures = Sort(features);
 
-            var enabledFeatures = new List<FeatureInfo>();
             while (true)
             {
                 var featureToActivate = sourceFeatures.FirstOrDefault(x => settings.IsFeatureEnabled(x.Feature.GetType()));
@@ -63,7 +62,8 @@ public FeatureDiagnosticData[] SetupFeatures(FeatureConfigurationContext feature
 
         public async Task StartFeatures(IBuilder builder, IMessageSession session)
         {
-            foreach (var feature in features.Where(f => f.Feature.IsActive))
+            // sequential starting of startup tasks is intended, introducing concurrency here could break a lot of features.
+            foreach (var feature in enabledFeatures.Where(f => f.Feature.IsActive))
             {
                 foreach (var taskController in feature.TaskControllers)
                 {
@@ -74,7 +74,7 @@ public async Task StartFeatures(IBuilder builder, IMessageSession session)
 
         public Task StopFeatures()
         {
-            var featureStopTasks = features.Where(f => f.Feature.IsActive)
+            var featureStopTasks = enabledFeatures.Where(f => f.Feature.IsActive)
                 .SelectMany(f => f.TaskControllers)
                 .Select(task => task.Stop());
 
@@ -208,6 +208,7 @@ static bool HasAllPrerequisitesSatisfied(Feature feature, FeatureDiagnosticData
         }
 
         List<FeatureInfo> features = new List<FeatureInfo>();
+        List<FeatureInfo> enabledFeatures = new List<FeatureInfo>();
         SettingsHolder settings;
 
         class FeatureInfo