Loading new modules via ConfigurationRegistrar always constructs modules using the constructor with most parameters

[pnagoorkar]

Describe the Bug

When registering a module via the ConfigurationRegistrar, the underlying ModuleRegistrar always constructs the module using the constructor with the most number of parameters.

Steps to Reproduce

Please clone and run the solution available here to reproduce the issue
https://github.com/pnagoorkar/AutofacModuleConstructionIssue

Expected output:
Hello from constructor 1
Hello from constructor 2
Hello from constructor 3

Actual output:
Hello from constructor 3
Hello from constructor 3
Hello from constructor 3

Expected Behavior

I expect to be able to control which constructor (on my module) is invoked based on the parameters I define on my type.

Dependency Versions

Autofac.Configuration : 6.0.0
Microsoft.Extensions.Configuration.Json : 8.0.0

[tillig]

At the moment this is sort of functioning as designed.

There's a little weirdness around modules, because while other components are put into the container and pass through the full "constructor location based on available parameters" system, modules are not resolved - we have to do the entire thing by manual reflection, for the most part, so there's not the logic around named vs. typed sorts of parameters like you might expect in a full component resolution. It's a chicken/egg problem. So the "best guess" is to select the constructor with the most parameters (which is how Autofac core does it, but for components it's more about "most parameters _that match") and we go with it. (I'm not saying we're doing it right - stick with me. Just that it's the current logic and there was a reason for that.)

To solve the immediate problem, you have a couple of workarounds:

1. Save the constructor for things that must always be there and fill in optional things using properties. That's supported in config already. Reduce the number of constructors in the module to one.
2. Use different classes instead of different constructors. Maybe have a base abstract class with all three constructors and then individual derived module classes where you can pick the constructor. Change the module type instead of the parameter count.

Now, I did notice a weird thing that "felt unexpected": while the list of parameters is retrieved by name, I was able to change the config to something totally different...

{
  "modules": [
    {
      "type": "AutofacModuleConstructionIssue.MyModule, AutofacModuleConstructionIssue",
      "parameters": {
        "x": {
          "Str1": "String 1"
        }
      }
    }
  ]
}

...and it still worked. I'm not totally sure why this is and I'm not sure I have time to dig deep on it right now. But that's definitely strange feeling.

We would love a PR that fixes this stuff:

• Better parameter matching so it actually enforces name matching for module constructor parameter names.
• Better constructor selection based on the parameters available and how many match.

That'd be a better solution for the issue and it'd make the system behave how you want.

[pnagoorkar]

Thanks for the quick response, @tillig. I did work around the issue using a composite type that encapsulated the parameters I want to pass. But this has created additional complexity that I believe can be eliminated by the solution I am proposing.
The solution tries to find the constructor having parameters whose names match that of the parameters passed via the configuration. I tested with the sample code above and it now does what I expect. All tests pass as well.

PR 45 Created for your review. Note that this is my first open-source PR so it may have some obvious issues. But I will be happy to make any changes that would make it "pull worthy" :-)

[pnagoorkar]

As for the issue you noticed where the module would load even if you change "parameterType1" to "x" (in settings1.json). I notice that although the module loads, parameter "x" is completely ignored because it does not match (in name) any of the available parameters on the constructor with the most parameters. Even with the solution I am proposing, this issue will always exist. And, in my opinion, it is a matter of due diligence by the developers creating the .json file to make sure the parameter names match that of the names on the constructor they seek to invoke.

[tillig]

Fixed by #45 . Will be released shortly along with #46.