Particular / NServiceBus.Extensions.Hosting

NServiceBus extension to support Microsoft.Extensions.Hosting
Other
14 stars 14 forks source link

Using NServiceBus.Extensions.Hosting with Microsoft.AspNetCore.Mvc.Testing #323

Open pardahlman opened 1 year ago

pardahlman commented 1 year ago

We are running ASP.NET Core applications that uses NServiceBus "send only" endpoints to publish events and send commands to RabbitMQ. NServiceBus is configured and started using this package.

We want to use ASP.NET Core integration test framework Microsoft.AspNetCore.Mvc.Testing to test these endpoints. The test framework uses the host configured in the entry point, but allows for updating DI configuration in the test before starting the application. This is useful when replacing unwanted integrations (for example database calls, ...) with mocks.

In these test we do not want to use RabbitMQ, but instead a more feasible transport (preferably some kind of in-memory transport). One important aspect that we want to test is that command routing has been correctly configured in the application.

To my knowledge, it is not possible to re-configure NServiceBus endpoint in the test: The EndpointConfiguration is never added to DI, HostAwareMessageSession is internal and also explicitly injected in NServiceBusHostedService so there is no way to intercept the startup process.

Do you have suggestions to how we can solve this issue? One suggestion that follows Microsoft conventions is to use Microsoft.Extensions.Options and "configure" EndpointConfiguration (services.Configure<EndpointConfiguration>(e => ...)). This would allow us to configure production transport in the entry point, and then override it with a new call to Configure in the test project.

SzymonPobiega commented 1 year ago

I think the best way would be to create the hosting code in such a way that it refers to configuration objects when setting up the EndpointConfiguration e.g. depending on the environment variable use RabbitMQ transport or LearningTransport. The LearningTransport is file-based so you can have an assertions based on the contents on the folder that holds the outgoing messages.

If doing this one must be very careful to not introduce any bugs that would cause the testing configuration to be used for production.

pardahlman commented 1 year ago

I understand what you are saying, but I think it is sub-optimal to let the production application take dependency on the learning transport or for that matter introduce a new risk (that I now see that you already pointed out): if the application is misconfigured to use learning transport in the production environment, it would start up as expected but not consume messages from RabbitMQ.

What arguments are there for not allowing the application to reconfigure the EndpointConfiguration before starting the application (and thereby starting the endpoint)? If you don't like the idea of using IOptions, the same behavior can be achieved by exposing a ReconfigureNServiceBus(EndpointConfiguration) API.

SzymonPobiega commented 1 year ago

services.Configure<EndpointConfiguration> would not work because the EndpointConfiguration is not resolved from the container. You would have to use some way of passing a callback that sets up the EndpointConfiguration to the code that configures the services collection.

pardahlman commented 1 year ago

You are correct that the approach that uses Configure would require the EndpointConfiguration (or IOption<EndpointConfiguration>) to be added to DI. Is that a problem?

SzymonPobiega commented 1 year ago

Unfortunately yes. This is because the EndpointConfiguration needs to be resolved when NServiceBus extensions set up the service collection here https://github.com/Particular/NServiceBus.Extensions.Hosting/blob/master/src/NServiceBus.Extensions.Hosting/HostBuilderExtensions.cs#L23. At that point there is no ServiceProvider yet to provider the EndpointConfiguration.

You can see if using IHostBuilder.Properties to pass the EndpointConfiguration works in your scenario:

var endpointConfiguration = new EndpointConfiguration("Samples.Hosting.GenericHost");
endpointConfiguration.UseTransport<LearningTransport>();
endpointConfiguration.DefineCriticalErrorAction(OnCriticalError);

var builder = Host.CreateDefaultBuilder(args);
builder.UseWindowsService();
builder.Properties.Add("EndpointConfig", endpointConfiguration);

builder.UseNServiceBus(ctx =>
{
    var config = (EndpointConfiguration)ctx.Properties["EndpointConfig"];

    return config;
});
pardahlman commented 1 year ago

Let me see if I understand this correctly. NServiceBus uses the host's IServiceCollection to add NServiceBus-specific services to it in this call. I want that very same call to use EndpointConfiguration resolved from the applications IServiceProvider, but it is not feasible to have one argument be the DI "before build" and the other argument resolved from the built DI.

I think that the design on these APIs are... not perfect, but I see where you are coming from 👍 It would make sense to have scenarios like the one I described in mind for future iterations on this package, as it is reasonable to support this widely used test strategy.

I'll look into your proposal using the HostBuilder's properties. Perhaps it suffices for now - but it feels like an unexpected work-around at best.

SzymonPobiega commented 1 year ago

@pardahlman Unfortunately the fact that the Microsoft DI container build process is single-staged prevents any other design. If there was more stages we could resolve the configuration from the DI and in another step set up NServiceBus components. The fact that the ServiceCollection is made a ServiceProvider in a single atomic step prevents this.