zcz527 / autofac

Automatically exported from code.google.com/p/autofac
Other
0 stars 0 forks source link

No warning/error when constructor selection policy produces an ambiguous match #314

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
If a registered type has multiple constructors, one of which is an 
IEnumerable<T>, that constructor will be selected over a more specific 
constructor even if named parameters are specified.

I would expect a service resolution to pick the most specific constructor based 
on the available arguments. That doesn't seem to happen if one of the 
constructors has an IEnumerable argument; in that case, the IEnumerable is 
selected and set to empty rather than selection of the most specific 
constructor.

The following NUnit fixture demonstrates the issue:

[TestFixture]
public class DemoResolutionFixture
{
  [Test]
  public void Visit_ConstantValue()
  {
    var autofacParam = new NamedParameter("input", "abc");
    var builder = new ContainerBuilder();
    builder.RegisterType<RegisteredComponent>().WithParameter(autofacParam);
    var container = builder.Build();
    var resolved = container.Resolve<RegisteredComponent>();
    // THIS FAILS:
    Assert.AreEqual("abc", resolved.CtorParameter);
  }

  private interface ISampleService
  {
  }

  private class RegisteredComponent
  {
    public object CtorParameter { get; private set; }
    public RegisteredComponent(ISampleService service)
    {
      Console.WriteLine("Service CTOR");
      this.CtorParameter = service;
    }
    public RegisteredComponent(IEnumerable<ISampleService> services)
    {
      // THIS CTOR WILL BE SELECTED
      // EVEN THOUGH A STRING PARAMETER WAS PROVIDED.
      // COMMENT THIS CTOR OUT AND THE TEST PASSES.
      Console.WriteLine("IEnumerable CTOR");
      this.CtorParameter = services;
    }
    public RegisteredComponent(string input)
    {
      // THIS CTOR SHOULD BE SELECTED BUT ISN'T.
      Console.WriteLine("String CTOR");
      this.CtorParameter = input;
    }
  }
}

Original issue reported on code.google.com by travis.illig on 31 Mar 2011 at 11:13

GoogleCodeExporter commented 8 years ago
Using Autofac 2.4.5.724 with .NET 4.

Original comment by travis.illig on 31 Mar 2011 at 11:14

GoogleCodeExporter commented 8 years ago
Interesting - might need to give this one some thought.

Autofac currently doesn't distinguish between supplied and resolved parameters, 
so it really just sees that two constructors can be satisfied and rolls a dice 
to pick one.

Given the parameter architecture I don't think this could be changed very 
easily, but I see how behaviour in this situation is tricky. Perhaps we should 
just throw when multiple constructors with the same number of parameters match? 
The extra line of configuration with UsingConstructor(x) would be a small pain, 
but the unpredictable choice is probably worse...

Original comment by nicholas...@gmail.com on 4 Apr 2011 at 10:20

GoogleCodeExporter commented 8 years ago
I must just be getting lucky then, or maybe I'm not running into this as often 
as I think I am. I always just assumed the behavior was something like:

* Get all the possible constructors.
* Get all the container-supplied arguments.
* Get all the manually-supplied parameters.
* From all the constructors, see which one matches the most manually-supplied 
parameters.
* Failing that, fall back from most-available-parameters to 
least-available-parameters. (That is, "most specific to least specific.")

The need to use UsingConstructor wasn't clear from 
http://code.google.com/p/autofac/wiki/Autowiring - it looked like that was only 
necessary to override the default behavior of "most to least specific 
constructor." My misunderstanding.

In the short term, I'd propose two things:
1) Enhance the docs to explain the logic of which constructor is chosen, 
specifically when there's a "tie" and one gets randomly chosen.
2) Throw an exception when there's an ambiguous constructor selection to force 
folks to use UsingConstructor to choose.

Longer term, it'd be nice to have behavior like I described - where manually 
supplied parameters take precedence over container parameters and influence the 
logic that infers which constructor to select. I'm guessing folks wouldn't go 
to the trouble of manually specifying parameters if they didn't intend for them 
to be used.

Original comment by travis.illig on 4 Apr 2011 at 2:56

GoogleCodeExporter commented 8 years ago
Sounds good. I'm looking at #2 now - in the process optimising a bit of the hot 
path through constructor selection, so worth spending some time in there anyway.

Original comment by nicholas...@gmail.com on 10 Apr 2011 at 6:16

GoogleCodeExporter commented 8 years ago
Exception now thrown in the case of ambiguity.

Original comment by nicholas...@gmail.com on 10 Apr 2011 at 10:34