Resolutions with many TypedParameter of the same type

aydjay commented 5 months ago

Problem Statement

Just hit an unpleasant bug in production. We have a type that has a constructor that has multiple of the same type as different parameters, like the supplied people class. I have fixed it by using the named parameter but it's too easy to fall foul of this behaviour if you aren't aware.

Typed parameters work well when the types they are resolving take distict types. When more that 1 of the same type is expected. You get undesirable behaviour.

Repro steps:

[TestClass]
public class MinimalRepro
{
    public class Person
    {
        public Person(int age)
        {
            Age = age;
        }

        public int Age { get; }
    }

    public class People
    {
        public People(Person person1, Person person2)
        {
            CombinedAges = person1.Age + person2.Age;
        }

        public int CombinedAges { get; }
    }

    [TestMethod]
    public void AgesAddUp()
    {
        var builder = new ContainerBuilder();
        builder.RegisterType<People>();

        var container = builder.Build();

        var people = container.Resolve<People>(
            TypedParameter.From(new Person(10)),
            TypedParameter.From(new Person(20)));

        people.CombinedAges.Should().Be(30); //This fails as you get 20
    }
}

Desired Solution

A runtime warning that throws in debug that shows that your intent and the outcome doesn't add up.

Alternatives You've Considered

Writing a decorator to autofac to the resolve call to add our own validation.

Additional Context

Version used: Autofac V6.3.0 Autofac.Extras.Moq V6.1.0

tillig commented 5 months ago

I don't think this is something we will be pursuing.

First, this is a pretty well-known behavior. It's known enough that it's been documented on the Func<X, Y, B> section to indicate why you can't have two typed parameters of the same time. Changing it to issue a warning would be pretty breaking.

But even if we did that, there's a logic problem.

Let's say you have the class you mentioned:

public class People
{
  public People(Person person1, Person person2)
  {
    CombinedAges = person1.Age + person2.Age;
  }

  public int CombinedAges { get; }
}

Now you register things like this:

var builder = new ContainerBuilder();
builder.RegisterType<People>();
builder.RegisterInstance(new Person(10));
var container = builder.Build();

What should you get if you do this?

var people = container.Resolve<People>();

Is it...

The same Person gets injected both times? (That's what'd happen today.)
A resolution error because you don't have two different Person instances registered?

Hold on, though. Let's change the class:

public class People
{
  public People(Person person1, Person person2)
  {
    CombinedAges = person1.Age + (person2.Age * 2);
  }

  public int CombinedAges { get; }
}

Now it matters which order the Person objects get pushed to the constructor because you'll have a different result.

We still have the same question about what happens if we only have one Person registered, but what if we have three registered?

var builder = new ContainerBuilder();
builder.RegisterType<People>();
builder.RegisterInstance(new Person(10));
builder.RegisterInstance(new Person(20));
builder.RegisterInstance(new Person(30));
var container = builder.Build();

Now what do we expect if we resolve a People object?

Is it the first two Person instances registered?
Is it the last two Person instances registered?
Let's pretend it's the last two - what order? Is it last-to-first? First-to-last?

Wait - let's add a resolution parameter!

var people = container.Resolve<People>(TypedParameter.From(new Person(100)));

Now which Person objects get injected, and in which order?

For the sake of argument, let's say we could answer all that.

How do we help troubleshoot that when people don't understand why things got injected like that?
How does it end up working when we have to use Autofac to back Microsoft.Extensions.DependencyInjection? Are we going to break people there?

The long and short of it is... this isn't something we're planning on investing in.

As a workaround, if you find yourself in this position (needing to inject two things of the same type), we have an FAQ for a sort of similar issue that may give you some ideas.

For the People example specifically - which I realize is just an example, but sometimes it helps to hear what I'd do for the actual example...

...I wouldn't inject the Person objects at all. I'd make the CombinedAges thing a method. But, again, I know these things are just examples - just trying to provide additional ideas for ways to work around issues like this.

Good luck!

aydjay commented 5 months ago

Thanks for the well reasoned explanation.

autofac / Autofac