Nservicebus: Can container test that allows updating registrations be removed?

[WilliamBZA]

The container tests has a test that validates that a registration can be updated after it has originally been created.

This is not supported by Spring, yet we support Spring as a container. Since it is not supported by Spring, the tests continue to fail within the NServiceBus.Spring repository but these failures are ignored.

Ignoring test failures is a huge alarm bell for me. If we can safely ignore a test but still support the library, is the test actually valid?

Surely if the test can be ignored, the test is not testing anything worthwhile. If this is the case, I propose that we remove the A_registration_should_be_allowed_to_be_updated test.

The same discussion applies to Should_be_able_to_build_components_registered_after_first_build.

[andreasohlund]

Agreed.

Shouldn't we "invert" the test and check that the core (at least) never re-registers any types since that would break spring?

Side note: Have we checked if the recent version of spring might support updating registration?

[WilliamBZA]

That's an excellent idea. I'm not sure how we would test that we _don't_ call a line of code, unless we throw an exception on the Configure methods - which could have far bigger consequences too.

As to your side note: I tried a couple months back to get these tests to work and could not find a way to get them to pass. Once the Spring container is intialized it appears to be immutable. The only workaround I could come up with was to recreate a new container and copy all the registrations over. _(I think... this was a while ago)_

[bording]

I recently took a look at adding support for SimpleInjector, and it also doesn't allow registrations to occur after something has been resolved.

As a side note, I couldn't get it working, because our container interface makes some additional assumptions that aren't valid for SimpleInjector, primarily around registering collections.

[WilliamBZA]

As a side note, I couldn't get it working, because our container interface makes some additional assumptions that aren't valid for SimpleInjector, primarily around registering collections.

Which is the real problem with containers. I don't even want to think about what impact the .NET Core container contract are going to have on _all_ containers going forward - which we will have to respond to. The sooner we can move away from containers the better IMO.

[bording]

The sooner we can move away from containers the better IMO.

I agree. One of the reasons I was looking at SimpleInjector is that I've been doing some performance profiling and the Autofac methods have been showing up as rather expensive. I know SimpleInjector is a lot faster than Autofac, so I had been hoping to wire it in real quick to do a perf comparison.

[WilliamBZA]

What about moving those tests out of the acceptance tests package, and making them specific tests within the containers that support that feature?

[bording]

What about moving those tests out of the acceptance tests package, and making them specific tests within the containers that support that feature?

I'm not sure I see the value in that. If we aren't enforcing this for all containers,, then what benefit is there in testing that some of them do let you update the registration?

[danielmarbach]

@bording about SimpleInjector: I've had a few chats with @dotnetjunkie about simple injector, how our container abstraction is bad. He was also trying to get SimpleInjector running with NSB

[andreasohlund]

The conclusion seems to be to remove the tests from the container test suite?

[dotnetjunkie]

I'm sorry for my late response. In my chats with @danielmarbach we concluded that the container abstraction leaks and the general concept of having such abstraction is bad. I had multiple failed attempts in producing an adapter for Simple Injector and got help from @kijanawoodard and daniel, but failed with every try. It is impossible to create an adapter, since the abstraction is incompatible to how Simple Injector views the world. For instance, register-resolve-register is not allowed.

The solution is to provide users with the right extension points they can override to intercept the creation of so called 'root types' (mainly message handlers in NSB). This allows anyone with any container to plugin its container and even allows developers who wish to apply DI without the use of a container (a practice commonly known as Pure DI).

Problem however is that NSB currently lacks the proper abstractions to do so. This makes it hard to use Simple Injector as container for your application. It also makes it hard to use application-specific abstractions instead of NSB provided abstractions for core parts of the application. A design that follows SOLID and Hexagonal Architecture tells us to move external libraries to the outer layers of the application, instead of letting core layers have a direct dependency on the external library.

This becomes even more relevant with the async changes for NSB v6. With the current design, async is forced upon every user, while not every application actually needs the performance benefits. For some (or perhaps even many) applications, the developement overhead caused by async does not outweigh the benefits of the performance improvement.

A proper abstraction can solve both problems; it allows developers to pick their own application-specific abstractions and it allows them to make them synchronous (if needed). The interception point will allow them to make the mapping from the async NSB API to an (synchronous) application API.

In other words, good interception points will give more flexibility to the user in choosing the design and tools that suit their specific application. This will be a win for everybody. Having such interception point will not hinder users that use NSB with its OOTB configuration.