@muratg / @davidfowl - can you take a look?

For posterity of "how do other containers handle this?":

• Autofac
• Castle Windsor
• StructureMap
• LightInject
• DryIoc
• JasperFx-Lamar
• SimpleInjector

SimpleInjector is the odd one out here but @dotnetjunkie would know more I suppose about the approach he took.

Unity is the only ASP.NET DI-supporting container I could find that does not support this feature. I'm not sure the impact, that package has 2k NuGet downloads and Autofac has >1M.

What about this case?

public interface I<T> {}
public interface X<A, B> where A : I<B> {}
public class Y<A> : X<A, String> where A : I<String> {}

collection.AddTransient(typeof(X<,>), typeof(Y<>);

Would existing containers even be affected? If they didn't support it before, they blew up with exceptions. Unity for example just blows up with that ArgumentException. They'd only see the blow up during service resolution, not configuration, so for those containers that don't support filtering services based on generic constraints, they'd continue to blow up today.

From my understanding of the extensions, they either replace IServiceCollection, or dump its contents into their own container configuration. So AFAIK, containers that supply their own IServiceProviderEngine won't ever hit the code in this PR. Unity will continue to break :)

Perfect then... just wasn't wrapping my head around the full use case.

Would existing containers even be affected? If they didn't support it before, they blew up with exceptions. Uni

It doesn't exactly work like that.

As soon as a feature is added to DI.Specification AspNetCore might take a dependency on it and would start exploding with containers that used to run fine on.

@jbogard did you validate that all mentioned containers actually pass new spec test?

@ENikS what are your thought on this feature? Do you think it's worth adding? Would you be willing/able to support it in unity?

How does this change affect time to first service benchmark (https://github.com/aspnet/DependencyInjection/blob/dev/benchmarks/DI.Performance/TimeToFirstServiceBenchmark.cs)?

What's the implication of moving it out of the DI.Specification? I just want the feature in this container, I'm less concerned if every container does. Though I wasn't sure if features are added to this container just for ASP.NET Core, or other libraries that also want to use it.

I'll update with benchmark #'s.

I've also opened an issue with Unity here: unitycontainer/container#81

I can also open a PR with them.

Benchmarks after: https://gist.github.com/jbogard/20f026885a79fb5f0227f577d16397f0

Benchmarks on dev: https://gist.github.com/jbogard/82db983fe5d1518bc8bd230211422db7

I would like to implement this feature but I am not sure what is correct behavior. Is there a concise description of how it should behave?

Correct me if I am wrong. I am assuming it applies to either resolving collection or single service. When resolving collection it should return only these that satisfy constraint and when resolving single service it should blow?

@ENikS it mainly concerns collection behavior. If I'm resolving IEnumerable<IService<...>> then only the IService implementations that can close the generic parameters should be returned. Then based on the generic parameters and what fits constraints I may have a different list of services returned.

It really isn't for single services, but most containers I've checked just let it apply to both, and let the parent service's needs dictate the behavior (if I require a service, and none exists, blow up).

The real-world scenarios are in cases where I still need the generic parameter for a further dependency. I want the generic parameter to "flow" down, instead of terminating with a base type. Keeping IFooService<T> : IService<T> where T: Foo instead of subclassing to IFooService : IService<Foo>. If the implementer then has dependencies, it's lost the T parameter to then get other dependencies that may depend on that T parameter.

In terms of what it means to support this behavior, all containers just put a try-catch around closing an open generic type (some try to catch the simple cases without just catch-first) since the methods to actually test a closed type aren't public in the BCL. See the links above for how other containers do it. It can be pretty simple to support, just as simple as try-catch-don't match.

I will add this to next patch release of Unity.

Correction regarding DryIoc.

It does the filtering when registering by walking down the constraints.

The try-catch in the link above is for resolution phase to wrap the unlikely failed match in container exception.

As of v5.8.6 Unity supports constrained generics

@jbogard Wanted to make sure to ask this. If you are adding support for generics, my assumption is that the following example would work out using the Lazy<T> class:

public interface ILogger
{
    void Log(string message);
}

public class ConsoleLogger : ILogger
{
    public void Log(string message)
    {
        Console.WriteLine(message);
    }
}

public class TestClass
{
    private readonly Lazy<ILogger> lazyLogger;
    private ILogger Logger => this.lazyLogger.Value;

    public SomeClass(Lazy<ILogger> logger)
    {
        this.lazyLogger = logger;
    }

    public void SomeMethod()
    {
        var logMessage = "Test Message to log";
        this.Logger.Log(logMessage);
    }
}

And I can just register my type as below:
.AddTransient<ILogger, ConsoleLogger>()

I apologize as I haven't read the entire conversation.
I was about to start work on a PR for that use case.
Let me know if my assumption is correct or not.
Thanks.

Not sure who you asking but in Unity it is supported scenario.

@ENikS I was actually asking @jbogard but that doesn't matter.

I have used Unity in all of my ASP.NET apps. So I am familiar with it.
I use Lazy as much as possible for objects that may or may not be used in a service. No need to resolve if it isn't used in an instance.

I was trying to go with the new built-in container included in ASP.NET Core for any new work.
That is when I recognized that my use case wouldn't work.

But if sticking to Unity is the only viable solution, then I will stick with it using the Unity.Microsoft.DependencyInjection package.

Resolving Lazy<T> out of the box won't work, no. You'd have to register it using something like services.AddTransient<Lazy<ILogger>>(p => new Lazy<ILogger>(() => p.GetService<ILogger>())); 😕

@muratg lets put this into the 2.2 milestone for consideration.

@muratg So we're in the 2.2 window now...thoughts?

bump

@davidfowl @pakrym please take a look.

@davidfowl could you either merge or close this? @natemcmaster is going to move this to the mondo repo and moving PRs are tricky. Thanks.

We would like to get this feature in for 3.0 but in a bit different shape (I managed to convince @davidfowl to catch first chance exceptions). It means, unfortunately, this PR would have to be closed and reopened against new repo.

Thanks @jbogard!

first chance exceptions are the 👿. We should do what we can in ns2.0 and ask for new APIs in corefx to make it possible to avoid throwing.

@davidfowl I agree but this is what every container developer has had to deal with. May be easier now to expose the APIs now.