dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.33k stars 4.74k forks source link

Add DI compatibility test #50029

Open aarrgard opened 5 years ago

aarrgard commented 5 years ago

We are currently using Unity in some of our projects and want to use it with asp.net core. To enable this feature we use the "Unity.Microsoft.DependencyInjection - 2.1.1". We also register some types in the default ServiceCollection provided here that then resolves types from the Unity registrations. There seems to be an issue how unity resolves object by using lambda functions. Since all the DI tests runs successfully using the Unity bridge perhaps the "Microsoft.Extensions.DependencyInjection.Specification.Tests" needs to address this.

The issue is related to scoped containers.

  1. Create and register a transient type that takes a scoped type as parameter.
  2. Create service provider
  3. Create scoped provider
  4. Make sure that the scoped type comes from the child container when getting the transient type.

This works in Unity if you register the type with

serviceCollection.AddTransient<ITransientService, TransientService>()

but not if you register it with

serviceCollection.AddTransient<ITransientService>(sp=>new TransientService(sp.GetRequiredService<IScopedService>()))

The lamda function receives the root container when resolving types in the scoped container.

Attaching test case. Not XUnit :(

I can rewrite the test in XUnit and perhaps reuse some of the existing test structures if this is an issue that you feel should be addressed.

// Andreas UnitTestScopes.txt

pakrym commented 5 years ago

I'll add a spec test. You might want to file a similar issue in https://github.com/unitycontainer/microsoft-dependency-injection/issues.

cc @ENikS

ENikS commented 5 years ago

@aarrgard @pakrym I will take a look

ENikS commented 5 years ago

I fixed it in Unity.DI. xUnit tests are available here

aarrgard commented 5 years ago

Looks good. Just out of curiosity. Is this test intended for just th unity container is is is something that should be tested i the generic test suite where the container is created using an abstract method. Since the https://github.com/unitycontainer/microsoft-dependency-injection passes all the tests i thougth it would be a good idea to add one that addresses the problem for all the IoC frameworks that want to be compatible. Perhaps I´m missing something. Just my 2 cents.

pakrym commented 5 years ago

We should add a spec test too. This scenario was implied but not tested so it should pass on all supported containers.

ghost commented 3 years ago

Tagging subscribers to this area: @eerhardt, @maryamariyan See info in area-owners.md if you want to be subscribed.

Issue Details
We are currently using Unity in some of our projects and want to use it with asp.net core. To enable this feature we use the "Unity.Microsoft.DependencyInjection - 2.1.1". We also register some types in the default ServiceCollection provided here that then resolves types from the Unity registrations. There seems to be an issue how unity resolves object by using lambda functions. Since all the DI tests runs successfully using the Unity bridge perhaps the "Microsoft.Extensions.DependencyInjection.Specification.Tests" needs to address this. The issue is related to scoped containers. 1. Create and register a transient type that takes a scoped type as parameter. 2. Create service provider 3. Create scoped provider 3. Make sure that the scoped type comes from the child container when getting the transient type. This works in Unity if you register the type with serviceCollection.AddTransient() but not if you register it with serviceCollection.AddTransient(sp=>new TransientService(sp.GetRequiredService())) The lamda function receives the root container when resolving types in the scoped container. Attaching test case. Not XUnit :( I can rewrite the test in XUnit and perhaps reuse some of the existing test structures if this is an issue that you feel should be addressed. // Andreas [UnitTestScopes.txt](https://github.com/aspnet/Extensions/files/2609193/UnitTestScopes.txt)
Author: aarrgard
Assignees: -
Labels: `area-Extensions-DependencyInjection`, `untriaged`
Milestone: -
ENikS commented 3 years ago

serviceCollection.AddTransient(sp=>new TransientService(sp.GetRequiredService()))

You are resolving service from root inside your lambda, how sp should know about the scope?

aarrgard commented 3 years ago

When I tested this 2,5 years ago this registration works:

serviceCollection.AddTransient<ITransientService, TransientService>()

How does the service provider know about the scope in that case when resolving instances?