Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MS DI AddKeyed... variant registration fails #632

Open
detoxhby opened this issue Feb 27, 2024 · 8 comments
Open

MS DI AddKeyed... variant registration fails #632

detoxhby opened this issue Feb 27, 2024 · 8 comments

Comments

@detoxhby
Copy link

detoxhby commented Feb 27, 2024

Registering keyed services through standard ServiceCollection results in error due to forced value in specific overload:

at DryIoc.Microsoft.DependencyInjection.DryIocAdapter.RegisterDescriptor(IContainer container, ServiceDescriptor descriptor) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs

which has fixed NotKeyed call to main register method => container.RegisterDescriptor(descriptor, IfAlreadyRegistered.AppendNotKeyed); and thus causing the keyed registration to fail.

Full stack:

System.InvalidOperationException: This service descriptor is keyed. Your service provider may not support keyed services.
   at Microsoft.Extensions.DependencyInjection.ServiceDescriptor.ThrowKeyedDescriptor()
   at Microsoft.Extensions.DependencyInjection.ServiceDescriptor.get_ImplementationType()
   at DryIoc.Microsoft.DependencyInjection.DryIocAdapter.RegisterDescriptor(IContainer container, ServiceDescriptor descriptor, IfAlreadyRegistered ifAlreadyRegistered, Object serviceKey) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs:line 247
   at DryIoc.Microsoft.DependencyInjection.DryIocAdapter.RegisterDescriptor(IContainer container, ServiceDescriptor descriptor) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs:line 239
   at DryIoc.Microsoft.DependencyInjection.DryIocAdapter.Populate(IContainer container, IEnumerable`1 descriptors, Func`3 registerDescriptor) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs:line 222
   at DryIoc.Microsoft.DependencyInjection.DryIocAdapter.WithDependencyInjectionAdapter(IContainer container, IEnumerable`1 descriptors, Func`3 registerDescriptor, RegistrySharing registrySharing) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs:line 151
   at DryIoc.Microsoft.DependencyInjection.DryIocServiceProviderFactory.CreateBuilder(IServiceCollection services) in /_/src/DryIoc.Microsoft.DependencyInjection/DryIocAdapter.cs:line 96
   at Microsoft.Extensions.Hosting.Internal.ServiceFactoryAdapter`1.CreateBuilder(IServiceCollection services)
   at Microsoft.Extensions.Hosting.HostBuilder.InitializeServiceProvider()
   at Microsoft.Extensions.Hosting.HostBuilder.Build()

based on main RegisterDescriptor code it is fully supported to register keyed services so it should just reuse the appropriate type in mentioned override

@dadhi
Copy link
Owner

dadhi commented Mar 3, 2024

@detoxhby The keyed services will be supported in the DryIoc v6 version. The support is already added. The version still need the final touches.

@dadhi dadhi closed this as completed Mar 3, 2024
@detoxhby
Copy link
Author

detoxhby commented Mar 4, 2024

@dadhi thanks!

Will you publish pre-release for testing beforehand? Would like to help by checking out in real projects!

@dadhi
Copy link
Owner

dadhi commented Mar 4, 2024

@detoxhby Here it is dotnet add package DryIoc.Microsoft.DependencyInjection --version 8.0.0-preview-01

@detoxhby
Copy link
Author

detoxhby commented Mar 5, 2024

@detoxhby Here it is dotnet add package DryIoc.Microsoft.DependencyInjection --version 8.0.0-preview-01

thanks, we've done testing and apart from the change in ConfigureContainer calls (IContainer to DryIocServiceProvider) the only issue is the Request class' API parameter changes of IContainer to Container which is a problem because generally we use the interface to reference the container and casting it down is unsafe
Can we ask why is this change was necessary? and if this is mandatory then what kind of workaround should we use to gain the typed container?

@dadhi
Copy link
Owner

dadhi commented Mar 5, 2024

Could you provide the example with the casting?
What are using Request.Container for?

@detoxhby
Copy link
Author

detoxhby commented Mar 5, 2024

Could you provide the example with the casting? What are using Request.Container for?

for factory based registrations/resolutions we use this for ex.:

CurrentContainer.ResolveFactory(Request.Create(CurrentContainer, ServiceInfo.Of<T>(serviceKey: key))).FactoryID

CurrentContainer is the IContainer and with v6 we need to cast it to (Container) atm

@dadhi
Copy link
Owner

dadhi commented Mar 5, 2024

@detoxhby Got it, will check.

Btw, I am still interested why do it with such a "low level" DryIoc stuff, are you integrating with some lib?

@dadhi dadhi reopened this Mar 5, 2024
@detoxhby
Copy link
Author

detoxhby commented Mar 5, 2024

Btw, I am still interested why do it with such a "low level" DryIoc stuff, are you integrating with some lib?

we are mainly using it for factory-based scoped registrations to be able to replace them dynamically during an execution (actually, this solution was discussed with you a few years ago because it was not possible earlier :))

// registration
var factory = ReflectionFactory.Of(typeof(TImplementation), reuse, made, setup);
var id = new FactoryIdentifier<TService>(factory.FactoryID);

container.Register(typeof(TService), factory, ifAlreadyRegistered, serviceKey);
container.RegisterInstance(typeof(FactoryIdentifier<TService>), id, serviceKey: serviceKey);


// usage (context is IResolverContext)
context.CurrentScope.SetOrAdd(context.Resolve<FactoryIdentifier<T>>(), instance)

The previous ResolveFactory is used in other testing env where we need to access the main factory (due to abstraction/isolation over different DI containers) but actually not mandatory functionality, we can probably drop it if required.
If there's an alternative API to avoid this we are willing to leave low-level parts behind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants