JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
563 stars 118 forks source link

Registering the same interface for two types, one of them named and one without name resolves alway the last registered one. #312

Closed rizi closed 2 years ago

rizi commented 2 years ago

When I register the same interface for two different classes and one registration is named an one is not then the last registration wins even when this registration is a named registration.

Is this a bug, that seems to be really dangerous, because you can't resolve the instance which is registered without a name. I would expect to get the instance that is not registered with a name (when I resolve without a name). And I would expect to get the instance that is registered with a name when I resolve with a name.

Here is an example:

   configuration
     .For<ICalculator>()
     .Use<SimpleCalculator>();

    configuration
       .For<ICalculator>()
        .Use<ScientificCalculator>()
        .Named("namedRegistration");

ICalculator calculator = _container.GetInstance<ICalculator>(); //this resolves ScientificCalculator, but I would expect that SimpleCalculator is resolved

Is my registration wrong or is this by intention?

How would I be able to resolve SimpleCalculator ?

Maybe I'm just not used to this behavior because Unity and Autofac behave different (they would resolve SimpleCalculator just as I expect)

I also noticed that all registrations become named registrations, either explicitly via the Named("...") method or implicitly by Lamar (there is some kind of algorithm to determine the name).

Is there a way to distinguish between explicit and implicit named registrations?

It would also help if I can override how the default name is "calculated" (ConstructorInstance --> Ctor() --> this.Name = Variable.DefaultArgName(implementationType); --> It would help if I can add a postfix or prefix --> so I know that is a default name. --> with this knowledge I'm able to check if an explicit named registrations occurs before or after and implicit named registration.

I think a better way instead of adding a postfix/prefix would be if we could add a bool property (e.g IsNamedRegistration) to the ConstructorInstance or the instance class ( set it to false in the constructor, at the same place where the name is currently set) and in the extension method Named(...) I would set the property IsNamedRegistration to true --> that shouldn't be to much effort and wouldn't be a breaking change.

@jeremydmiller what do you think?

br

jeremydmiller commented 2 years ago

So, last registration wins as the "default", and that behavior is mandated by our ASP.Net Core overlords for compliance with how containers are supposed to work, so that's all intended behavior and should be well documented in the docs.

The automatic name is just based on the implementation name, with a numeric postfix to disambiguate as necessary.

I'd have to dig into the code, but there's a method somewhere on Instance that tells you if the name is explicit or the default. Not following why that matters here.

You can always resolve a specific instance by doing Container.GetInstance<T>(string name) (the docs are fubar on that, just noticed).

And if you really need to or care, you could add an instance policy to overwrite names to your heart's content. Think there might be an attribute you could stick on the concrete type to set the name explicitly if you wanted, but then you're bleeding IoC goop into your real code.

rizi commented 2 years ago

@jeremydmiller

I'd have to dig into the code, but there's a method somewhere on Instance that tells you if the name is explicit or the default. Not following why that matters here

That would be great, thx

jeremydmiller commented 2 years ago

https://github.com/JasperFx/lamar/blob/master/src/Lamar/IoC/Instances/Instance.cs#L26-L37

rizi commented 2 years ago

https://github.com/JasperFx/lamar/blob/master/src/Lamar/IoC/Instances/Instance.cs#L26-L37

@jeremydmiller but if someone use .Named(...) with the same value( by coincidence) as the "DefaultArgName" I would not be able to distinguish, right?

Br

jeremydmiller commented 2 years ago

What are you doing where any of this matters?

rizi commented 2 years ago

What are you doing where any of this matters?

@jeremydmiller Sometimes we have a default registration (without name) and a specific registration with name, so when we resolve without a name we get the default registration and when we resolve with name we get the registration we registered with the name (the order of the registrations do not matter). That's how unity and Autofac works, at the moment we are moving away from unity because it's abandoned.

And now I have to ensure that the correct instance is resolved, with Lamar the order of the registration is important --> therefore I will check that a not named registration have to occur after a named registration --> so the behaviour of the IOC container stays the same. We have a lot of applications that depend on this behaviour.

Therefore it would be very helpful if there would be a mechanism (e.g a property) that helps me to distinguish between a named and a not named registration.

Of course the "DefaultArgName" helps but as I mentioned in the post above it's possible that a named registration uses the same name a Lamar would calculate and then my check would fail silently and the wrong instance would be resolved.

That would be a bug that's very hard to identify.

Any ideas/suggestions?

Br