search

autofac / Autofac.ServiceFabric

Autofac integration for Azure Service Fabric. Provides service factory implementations for Actors, Stateful Services and Stateless Services.
MIT License
26 stars 27 forks source link

ServiceFabricModule is internal, visible to Autofac RegisterAssemblyModules, but cannot be constructed #48

Closed wasabii closed 5 years ago

wasabii commented 5 years ago

The Autofac Module in the project is set up in such a way that it's hit by the Autofac RegisterAssemblyModules method, which tries to construct it. However, since it has no default constructor, this fails.

What this means is the mere existence of the module within a project that is doing dynamic assembly loading (and loading AF modules) breaks it.

It looks like the intention is to call it from RegisterServiceFabricSupport. That's fine. But it would be nice if the module could either be unreachable by Autofac in some way.

This is new in 2.1.0.

2.0.0 did not have this issue.

alexmg commented 5 years ago

@wasabii Yes, making the module internal was intentional.

Do you have an exception that you can provide or a quick reproduction? I don't think RegisterAssemblyModules includes internal types by default.

alexmg commented 5 years ago

It looks like internal types are considered. I guess what makes this different from regular cases is the non-default constructor. This is actually an issue for the core library more than this integration. I might open a separate issue in core for this.

tillig commented 5 years ago

Be careful if you change this in the core - had a similar issue about assembly type scanning and people have come to rely on the behavior, considering it a breaking change when I tried to make it public-only by default.

alexmg commented 5 years ago

I was just about to link that that very issue. 😄

alexmg commented 5 years ago

It is actually the internal constructor that is causing the problem in this case. The internal class is located in RegisterAssemblyTypes but the instance cannot be created because the DefaultConstructorFinder excludes the internal constructor.

From a backwards compatibility perspective changing anything in core around this will be difficult, especially since it sits on top of something that we have already decided is hard to change. A similar "public only" option is something that could be considered.

I would certainly recommend when assembly scanning to try and reduce what you include. Using a base type for your modules is a reasonable choice. I'll have a think about what else we might be able to do.

tillig commented 5 years ago

Internal type, public constructor?

alexmg commented 5 years ago

That does prevent the exception but still includes the module in the scan. I actually added code in the RegisterServiceFabricSupport method to prevent the registrations in the module from being added twice. It might not matter that much, but it does highlight an issue with overreach in the module registration path.

alexmg commented 5 years ago

I think I'll just convert the module code into a static helper method removing the module from the picture all together.