Skip to content

Commit fad98f8

Browse files
danielmarbachDavidBoikeCopilot
authored
Feature redesign to facilitate AOT and trimming support (#7413)
* Spiking around * More fiddling around * Allow overriding feature name internally * Use construct * Track enabled features * Avoid multiple creation * Fix depends on and make feature name convention internal * Fix tests * Express enable by default * Feature activation order * Cosmetics * Reference core features (order still waky) * Remove unneeded method * Fix incorrect acceptance test that only worked due to a specific order being in place * Preserve previous scanning order for now * Remove EnableDefault for now from the features in core * Core should no longer scan itself * Remove EnableDefault from acceptance tests * Rename to registry for now * Try relying less on settings with some hackery * Further decouple states * Further simplify state checks * Seggregate into local function * Filter out empty dependencies * Reorder * Use feature info more consistently * Press dependencies flatt * AddCore * Bring back previous ctor * Approve temporary API changes * Small fixes * Use AddCore * Handle weak dependencies better * Use feature info name consistently * Move a layer up * Use EnableFeature * Leaner feature component (might die) * Remove features from scanning * Cleanup * Correct state * Move todo because otherwise it trips up the documentation tests * Add some state coverage * Feature Component * Fix saga * Avoid unbounded recursion * Preserve default state * Simplify tests * Seperate dependencies * Temporary workaround for readonly struct records * Move sort out * Rename * Move feature info out * Split into settings and component like other pieces in Core * EnableByDefault in persistence to stay backward compatible * KeyedCollection makes things more expressive * Use feature settings directly * Apply things consistently * Obsolete the ability to enable features by default on the settings holder * Remove now unnecessary settings methods and add not generic public methods * Obsolete non-generic Is methods * Obsolete type based APIs on endpoint configuration * Get rid of overloads that are no longer needed * Remove another type based overload * Rename method * Spelling * Cleanup FeatureSettingsTests * Get rid of some EnabledByDefault in the dependency tests * Remove EnableByDefault where possible in FeatureStartupTests * Remove EnableByDefault from FeatureDifferingOnlyByNamespaceTests * Remove EnableByDefault from FeatureDefaultsTests * Remove EnableByDefault from FeatureSettingsTests * Remove unsupported scenario (also fails on master and is not possible to achieve because defaults run always * Remove remaining EnabledByDefault from dependency tests * Obsolete Feature EnableByDefault * Add a subscription storage instead * Obsolete DependsOnOptionaly with type * Cleanup feature base class * Correct version number * Change EnableByDefault obsolete message * Simplify code flow a bit * Isolate obsolete EnableByDefault tests with pragma suppressions (#7446) * Initial plan * Isolate EnableByDefault tests into partial classes with suppressions Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> * Restore global.json (accidentally renamed in previous commit) Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> * Collapse When_features_are_scanned into single file with pragma at top Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> * Add pragma warning restore at bottom of files with pragma disable Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> --------- Co-authored-by: Daniel Marbach <danielmarbach@users.noreply.github.com> Co-authored-by: David Boike <david.boike@gmail.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com>
1 parent ccc0d70 commit fad98f8

File tree

79 files changed

+1337
-1129
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+1337
-1129
lines changed

‎src/NServiceBus.AcceptanceTesting/AcceptanceTestingPersistence/AcceptanceTestingTransactionalStorageFeature.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ class AcceptanceTestingTransactionalStorageFeature : Feature
88
{
99
public AcceptanceTestingTransactionalStorageFeature() => DependsOn<SynchronizedStorage>();
1010

11-
protected internal override void Setup(FeatureConfigurationContext context)
11+
protected override void Setup(FeatureConfigurationContext context)
1212
=> context.Services.AddScoped<ICompletableSynchronizedStorageSession, AcceptanceTestingSynchronizedStorageSession>();
1313
}

‎src/NServiceBus.AcceptanceTesting/AcceptanceTestingPersistence/Outbox/AcceptanceTestingOutboxPersistence.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ class AcceptanceTestingOutboxPersistence : Feature
1111
{
1212
public AcceptanceTestingOutboxPersistence()
1313
{
14+
EnableByDefault<AcceptanceTestingTransactionalStorageFeature>();
15+
1416
DependsOn<Outbox>();
1517
DependsOn<AcceptanceTestingTransactionalStorageFeature>();
16-
Defaults(s => s.EnableFeatureByDefault<AcceptanceTestingTransactionalStorageFeature>());
1718
}
1819

19-
protected internal override void Setup(FeatureConfigurationContext context)
20+
protected override void Setup(FeatureConfigurationContext context)
2021
{
2122
var outboxStorage = new AcceptanceTestingOutboxStorage();
2223

‎src/NServiceBus.AcceptanceTesting/AcceptanceTestingPersistence/SagaPersister/AcceptanceTestingSagaPersistence.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ public AcceptanceTestingSagaPersistence()
1111
DependsOn<Sagas>();
1212
DependsOn<AcceptanceTestingTransactionalStorageFeature>();
1313

14-
Defaults(s => s.EnableFeatureByDefault<AcceptanceTestingTransactionalStorageFeature>());
14+
EnableByDefault<AcceptanceTestingTransactionalStorageFeature>();
1515
}
1616

17-
protected internal override void Setup(FeatureConfigurationContext context) => context.Services.AddSingleton<ISagaPersister, AcceptanceTestingSagaPersister>();
17+
protected override void Setup(FeatureConfigurationContext context) => context.Services.AddSingleton<ISagaPersister, AcceptanceTestingSagaPersister>();
1818
}

‎src/NServiceBus.AcceptanceTesting/AcceptanceTestingPersistence/SubscriptionStorage/AcceptanceTestingSubscriptionPersistence.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public AcceptanceTestingSubscriptionPersistence()
1111
DependsOn("NServiceBus.Features.MessageDrivenSubscriptions");
1212
}
1313

14-
protected internal override void Setup(FeatureConfigurationContext context)
14+
protected override void Setup(FeatureConfigurationContext context)
1515
{
1616
context.Services.AddSingleton<ISubscriptionStorage, AcceptanceTestingSubscriptionStorage>();
1717
}

‎src/NServiceBus.AcceptanceTesting/Customization/EndpointConfigurationExtensions.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ namespace NServiceBus.AcceptanceTesting.Customization;
44
using System.Collections.Generic;
55
using System.Linq;
66
using System.Reflection;
7+
using Features;
78
using Hosting.Helpers;
89
using Support;
910

@@ -31,12 +32,12 @@ public static void ScanTypesForTest(this EndpointConfiguration config,
3132
var assemblyScanner = new AssemblyScanner { ScanFileSystemAssemblies = false };
3233

3334
config.TypesToIncludeInScan(
34-
assemblyScanner.GetScannableAssemblies().Types
35+
[.. assemblyScanner.GetScannableAssemblies().Types
3536
.Except(customizationConfiguration.BuilderType.Assembly.GetTypes()) // exclude all types from test assembly by default
3637
.Union(GetNestedTypeRecursive(customizationConfiguration.BuilderType.DeclaringType, customizationConfiguration.BuilderType))
38+
.Where(t => !t.IsAssignableTo(typeof(Feature))) //remove once we default the tests to not use scanning
3739
.Union(customizationConfiguration.TypesToInclude)
38-
.Except(customizationConfiguration.TypesToExclude)
39-
.ToList());
40+
.Except(customizationConfiguration.TypesToExclude)]);
4041

4142
IEnumerable<Type> GetNestedTypeRecursive(Type rootType, Type builderType)
4243
{

‎src/NServiceBus.AcceptanceTesting/Support/FeatureStartupTaskRunner.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@
88
/// <summary>
99
/// Simple feature that allows registration of <see cref="FeatureStartupTask"/> without having to define a <see cref="Feature"/> beforehand.
1010
/// </summary>
11-
class FeatureStartupTaskRunner : Feature
11+
sealed class FeatureStartupTaskRunner : Feature
1212
{
1313
public const string ConfigKey = "FeatureStartupTaskRunner.StartupTasks";
14-
public FeatureStartupTaskRunner() => EnableByDefault();
1514

16-
protected internal override void Setup(FeatureConfigurationContext context)
15+
protected override void Setup(FeatureConfigurationContext context)
1716
{
1817
if (context.Settings.TryGet<List<Func<IServiceProvider, FeatureStartupTask>>>(ConfigKey, out var startupTasks))
1918
{

‎src/NServiceBus.AcceptanceTests/Core/DependencyInjection/When_overriding_services_in_registercomponents.cs‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,19 @@ public class Context : ScenarioContext
5151
public class EndpointWithOverrides : EndpointConfigurationBuilder
5252
{
5353
public EndpointWithOverrides() =>
54-
EndpointSetup<DefaultServer>(c => c.RegisterComponents(s =>
54+
EndpointSetup<DefaultServer>(c =>
5555
{
56-
s.AddSingleton<IDependencyFromFeature, OverridenDependency>();
57-
s.AddSingleton<IDependencyBeforeEndpointConfiguration, OverridenDependency>();
58-
s.AddSingleton<IDependencyBeforeEndpointStart, OverridenDependency>();
59-
}));
56+
c.EnableFeature<StartupFeature>();
57+
c.RegisterComponents(s =>
58+
{
59+
s.AddSingleton<IDependencyFromFeature, OverridenDependency>();
60+
s.AddSingleton<IDependencyBeforeEndpointConfiguration, OverridenDependency>();
61+
s.AddSingleton<IDependencyBeforeEndpointStart, OverridenDependency>();
62+
});
63+
});
6064

6165
public class StartupFeature : Feature
6266
{
63-
public StartupFeature() => EnableByDefault();
64-
6567
protected override void Setup(FeatureConfigurationContext context)
6668
{
6769
context.Services.AddSingleton<IDependencyFromFeature, OriginallyDefinedDependency>();
@@ -92,29 +94,19 @@ protected override Task OnStop(IMessageSession session, CancellationToken cancel
9294
}
9395
}
9496

95-
public interface IDependencyFromFeature
96-
{
97-
}
97+
public interface IDependencyFromFeature;
9898

99-
public interface IDependencyBeforeEndpointConfiguration
100-
{
101-
}
99+
public interface IDependencyBeforeEndpointConfiguration;
102100

103-
public interface IDependencyBeforeEndpointStart
104-
{
105-
}
101+
public interface IDependencyBeforeEndpointStart;
106102

107103
public class OriginallyDefinedDependency :
108104
IDependencyFromFeature,
109105
IDependencyBeforeEndpointConfiguration,
110-
IDependencyBeforeEndpointStart
111-
{
112-
}
106+
IDependencyBeforeEndpointStart;
113107

114108
public class OverridenDependency :
115109
IDependencyFromFeature,
116110
IDependencyBeforeEndpointConfiguration,
117-
IDependencyBeforeEndpointStart
118-
{
119-
}
111+
IDependencyBeforeEndpointStart;
120112
}

‎src/NServiceBus.AcceptanceTests/Core/Feature/When_features_are_scanned.cs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// This file contains tests that use the obsolete Feature.EnableByDefault() method.
2+
// EnableByDefault is obsolete and will be treated as an error in NServiceBus 11.0.
3+
// This file and its tests should be deleted when NServiceBus 11.0 is released.
4+
#pragma warning disable CS0618 // Type or member is obsolete
5+
6+
namespace NServiceBus.AcceptanceTests.Core.Feature;
7+
8+
using System.Threading.Tasks;
9+
using AcceptanceTesting;
10+
using EndpointTemplates;
11+
using Features;
12+
using NUnit.Framework;
13+
14+
public class When_features_are_scanned : NServiceBusAcceptanceTest
15+
{
16+
[Test]
17+
public async Task Should_run_features()
18+
{
19+
var context = await Scenario.Define<Context>()
20+
.WithEndpoint<EndpointWithFeature>()
21+
.Done(c => c.EndpointsStarted)
22+
.Run();
23+
24+
using (Assert.EnterMultipleScope())
25+
{
26+
Assert.That(context.RootFeatureCalled, Is.True);
27+
Assert.That(context.DependentFeatureCalled, Is.True);
28+
}
29+
}
30+
31+
class Context : ScenarioContext
32+
{
33+
public bool RootFeatureCalled { get; set; }
34+
public bool DependentFeatureCalled { get; set; }
35+
}
36+
37+
class EndpointWithFeature : EndpointConfigurationBuilder
38+
{
39+
public EndpointWithFeature() =>
40+
EndpointSetup<DefaultServer>()
41+
.IncludeType<FeatureDiscoveredByScanning>(); //simulate that the feature is included in scanning
42+
43+
sealed class FeatureDiscoveredByScanning : Feature
44+
{
45+
FeatureDiscoveredByScanning()
46+
{
47+
EnableByDefault();
48+
EnableByDefault<DependentFeature>();
49+
50+
DependsOn<DependentFeature>();
51+
}
52+
53+
protected override void Setup(FeatureConfigurationContext context) => context.Settings.Get<Context>().RootFeatureCalled = true;
54+
}
55+
56+
sealed class DependentFeature : Feature
57+
{
58+
DependentFeature() { }
59+
protected override void Setup(FeatureConfigurationContext context) => context.Settings.Get<Context>().DependentFeatureCalled = true;
60+
}
61+
}
62+
}
63+
64+
#pragma warning restore CS0618 // Type or member is obsolete

‎src/NServiceBus.AcceptanceTests/Core/Feature/When_registering_a_startup_task.cs‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,46 +32,26 @@ public class Context : ScenarioContext
3232

3333
public class SendOnlyEndpoint : EndpointConfigurationBuilder
3434
{
35-
public SendOnlyEndpoint()
36-
{
35+
public SendOnlyEndpoint() =>
3736
EndpointSetup<DefaultServer>(c =>
3837
{
3938
c.SendOnly();
4039
c.EnableFeature<Bootstrapper>();
4140
});
42-
}
4341

4442
public class Bootstrapper : Feature
4543
{
46-
public Bootstrapper()
47-
{
48-
EnableByDefault();
49-
}
50-
51-
protected override void Setup(FeatureConfigurationContext context)
52-
{
53-
context.RegisterStartupTask(b => new MyTask(b.GetService<Context>()));
54-
}
44+
protected override void Setup(FeatureConfigurationContext context) => context.RegisterStartupTask(b => new MyTask(b.GetService<Context>()));
5545

56-
public class MyTask : FeatureStartupTask
46+
public class MyTask(Context scenarioContext) : FeatureStartupTask
5747
{
58-
public MyTask(Context scenarioContext)
59-
{
60-
this.scenarioContext = scenarioContext;
61-
}
62-
6348
protected override Task OnStart(IMessageSession session, CancellationToken cancellationToken = default)
6449
{
6550
scenarioContext.SendOnlyEndpointWasStarted = true;
6651
return Task.CompletedTask;
6752
}
6853

69-
protected override Task OnStop(IMessageSession session, CancellationToken cancellationToken = default)
70-
{
71-
return Task.CompletedTask;
72-
}
73-
74-
readonly Context scenarioContext;
54+
protected override Task OnStop(IMessageSession session, CancellationToken cancellationToken = default) => Task.CompletedTask;
7555
}
7656
}
7757
}

‎src/NServiceBus.AcceptanceTests/Core/Hosting/When_custom_host_id_is_configured.cs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public async Task Should_not_apply_default_to_be_FIPS_compliant()
1717
var context = await Scenario.Define<Context>()
1818
.WithEndpoint<MyEndpoint>(builder => builder.CustomConfig((endpointConfig, ctx) =>
1919
{
20+
endpointConfig.EnableFeature<MyFeatureThatOverridesHostInformationDefaults>();
2021
endpointConfig.UniquelyIdentifyRunningInstance().UsingCustomIdentifier(ctx.CustomHostId);
2122
}))
2223
.Done(c => c.EndpointsStarted)
@@ -31,17 +32,12 @@ public async Task Should_not_apply_default_to_be_FIPS_compliant()
3132

3233
public class MyEndpoint : EndpointConfigurationBuilder
3334
{
34-
public MyEndpoint()
35-
{
36-
EndpointSetup<DefaultServer>();
37-
}
35+
public MyEndpoint() => EndpointSetup<DefaultServer>();
3836
}
3937

4038
public class MyFeatureThatOverridesHostInformationDefaults : Feature
4139
{
42-
public MyFeatureThatOverridesHostInformationDefaults()
43-
{
44-
EnableByDefault();
40+
public MyFeatureThatOverridesHostInformationDefaults() =>
4541
Defaults(s =>
4642
{
4743
var fieldInfo = s.GetType().GetField("Defaults", BindingFlags.Instance | BindingFlags.NonPublic);
@@ -52,7 +48,6 @@ public MyFeatureThatOverridesHostInformationDefaults()
5248
testContext.HostIdDefaultApplied = defaults.ContainsKey("NServiceBus.HostInformation.HostId");
5349
testContext.HostIdObserved = s.Get<Guid>("NServiceBus.HostInformation.HostId");
5450
});
55-
}
5651

5752
protected override void Setup(FeatureConfigurationContext context)
5853
{

0 commit comments

Comments
 (0)