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

Keyed Services Support in dotnet 8 #112

Closed
alsami opened this issue Jul 26, 2023 · 25 comments · Fixed by #115
Closed

Keyed Services Support in dotnet 8 #112

alsami opened this issue Jul 26, 2023 · 25 comments · Fixed by #115

Comments

@alsami
Copy link
Member

alsami commented Jul 26, 2023

Just came across a Twitter post from David Fowler. Looks like the next version of the dotnet SP will support keyed services.

https://twitter.com/davidfowl/status/1683956501570879489?t=EzKZPsabAvVngUAZeCXUAA&s=19

Not sure how it works under the hood but looks to me like it's an entire new mechanism which might need to be supported here.

If you already looked into it and know it will work, please close.

@tillig
Copy link
Member

tillig commented Jul 26, 2023

I've been following the development of that since the initial proposal and have a branch started to work on it. I'm currently blocked by a runtime issue in unit testing multiple frameworks that I think will be fixed in preview 7, but that's stopping the build from targeting net8 right now.

I'm not sure what work will be involved to support the new feature. I know there's an attribute based injection for constructor filtering by key so there may need to be a core Autofac change to accommodate that. But... stuck on runtime at the moment.

@tillig
Copy link
Member

tillig commented Sep 4, 2023

Finally got to a computer where I could install preview 7 and, sure enough, it does fix the unit test problems I was seeing so I can actually get back to this now.

@tillig
Copy link
Member

tillig commented Oct 5, 2023

I'm continuing to work on this in my branch, but there are some interesting concepts that got added to the Microsoft registrations that I'm not 100% sure how to bridge for Autofac.

Null keys: Microsoft added a thing where you can do services.AddKeyedTransient<IService, Service>(null) - where the key is null. This results in the ability to both resolve with a null key like provider.GetKeyedService<IService>(null) and resolve without a key like provider.GetService<IService>(). Autofac doesn't support null as a key, so the adapter logic there will have to use a sort of static object "placeholder" that represents a null key.

Any keys: There's a placeholder/wildcard concept ServiceKey.AnyKey where if you register using that, like services.AddKeyedTransient<IService, Service>(ServiceKey.AnyKey), it means you can pass anything in as the key and get the value back - provider.GetKeyedService<IService>(10) or provider.GetKeyedService<IService>("foo") both work.

Keyed service factories: If you register a factory delegate, it takes in the key as a parameter and it generates keyed services. I'm not totally clear on whether that means it's a factory that generates keyed things or whether it's a keyed factory, or maybe both. I gather you could combine this with the ServiceKey.AnyKey to combine the concepts.

I'm discovering this stuff through the spec tests and I'm working on trying to figure out how these concepts adapt to Autofac. I'm not totally sure they all do, at least, not easily.

@tillig
Copy link
Member

tillig commented Nov 2, 2023

Progress has been made, but not much. The AnyKey construct is really problematic. It responds to any request for a keyed service of the given type, but it thinks it's responding to the provided key. Getting this to mesh with the Autofac world where that kind of mechanism isn't a thing is somewhat challenging.

There's a notion where the keyed service registration is passed, as a parameter, the key used to resolve the thing, which, also, is not a construct Autofac natively supports. I have this sort of figured out for reflection registrations, I still have no real good idea of what to do for delegates.

It does not help that my day job continues to eat more of my time and mental capacity, so working long hours and then switching from staring at a screen to staring at a different screen is not really something I'm motivated to do.

@tillig
Copy link
Member

tillig commented Nov 6, 2023

The AnyKey construct has brought me to an impasse. I'm not sure we can support it without some changes to core Autofac, which I have yet to be able to quantify.

  • Things registered with AnyKey respond to any resolution for the given type regardless of the key value.
  • Singletons registered with a reflection activator with AnyKey return a different singleton per key. It's like a tiny singleton factory.
  • Singletons registered with an instance with AnyKey do not return a different singleton per key.
  • Registration order for AnyKey gets maintained in the same way it would if you'd registered with a key, so resolving an IEnumerable<T> of the keyed services needs to obey that registration order. This is hard to do in Autofac if we bridge the gap using a RegistrationSource because sources get handled slightly differently than individually registered services.
  • In all non-provided-instance cases - reflection registrations, factory registrations - the key used to resolve the thing gets passed in as a constructor parameter. If it's AnyKey, the key provided is the key used during resolution, not AnyKey itself.

All of this adds up to not really being able to patch this onto our adapter using only existing Autofac extension points. It means we actually need to support this AnyKey sort of feature down into core Autofac.

I'm unclear where this leaves things. I can't finish it in this repo alone, but I also currently have neither the time nor motivation to work on the core Autofac changes.

I can't help but feel a bit of tail-wagging-the-dog on the Microsoft side of things, with their conforming container now adding features we already mentioned were going to be non-trivial to bridge and implement. Yet here we are. I don't have an answer. Don't bother pinging for status, if I get to it, you'll see it show up; if I don't, you won't. And that's pretty much it.

If you want to do the work and submit the PR for it, awesome. We'd love to see it.

@grendizeras
Copy link

Hello, do you have any updates on this issue? How can we help to push this problem? We used Autofac provider for all our services, and moving to .net 8 causes error: "This service descriptor is keyed. Your service provider may not support keyed services"

@tillig
Copy link
Member

tillig commented Dec 22, 2023

Generally if the are updates to issues you'll see them in the issue or as a release. Since there are neither, you can assume no update.

We determined that something needs to change in core Autofac to support this new construct. Keyed services in MEDI are quite different than keyed services in Autofac. While we have the develop branch in core Autofac updated to net8, the work to get core keyed services to understand things like AnyKey has not been done. If you want to "push the issue," feel free to provide the PR that changes all the core bits to understand the new constructs.

I wish I could offer more, but, as mentioned, the features Microsoft added to the conforming container are, in some cases, very different than the features in Autofac to support keyed services. It means a lot of work to change how Autofac does things and it'll likely result in since breaking changes in core Autofac to make this happen.

For future commenters: updates here will be provided if there are updates. If you don't see any, it's because there's nothing to add. The work will not be fast, and I can't lie, between unpaid maintainer burnout and my paid day job ramping up hard to eat my free time and motivation, I'm not personally actively cranking hard to solve this.

@srogovtsev
Copy link

srogovtsev commented Dec 26, 2023

@tillig, I'm thinking of maybe picking this up for my vacation pet-project (don't want to give you any false hopes though), and have a couple of questions:

  1. where should I pick up from, is this still the latest-and-greatest?
  2. is there a set of tests we can use to verify MEDI expected behavior? Maybe something from MEDI's repository?
  3. have you investigated the route where we inject middleware into Autofac's pipeline (instead of modifying core), as I used that previously for some of my weird needs, and have found it incredibly powerful?

@tillig
Copy link
Member

tillig commented Dec 26, 2023

@srogovtsev We'd absolutely welcome the help. Let me see if I can lay out where things are and the issues I've run into that pushed me toward modifying core Autofac.

The feature/keyed-services branch you found here in AEDI is as far as I've gotten things. I have not updated that branch for the release of .NET 8 (it's still on RC) but that shouldn't really impact anything.

There is a set of specification tests in the M.E.DI.Specification.Tests assembly, referenced, which define how keyed services should work. The test fixtures should derive from KeyedDependencyInjectionSpecificationTests and I have one for using a ContainerBuilder directly as well as one for child lifetime scope "root" and one for using the service provider factory. In addition, to help me do better debugging (because even with source link stepping into methods where there's no obvious entry point is hard) I created a temporary copy of the tests called DebuggableTests. This DebuggableTests is the same thing as the tests using ContainerBuilder, they just have the tests inline instead of deriving from the MEDI fixture.

The primary set of features, high level, are:

  • Keyed services with an explicit key (builder.RegisterType<T>().Keyed<T>("key")).
  • Injection of the key used during resolution as a parameter - a constructor parameter can be marked with [FromKey] and the key being injected gets passed in as that constructor parameter; and a factory/delegate registration can also take the key as a parameter.
  • Keyed services registered with AnyKey instead of a specific key, which will respond to any key at all being passed in as long as the type matches (container.ResolveKeyed<T>("anything-goes-here")).

Handling standard keyed service registrations is fairly trivial. It's a basic conversion from their registration syntax to Autofac.

Injection of the key as a parameter gets a little more tricky, but it's not that bad, at least for reflection-based components. I already have some middleware that handles it. However, I didn't get to a point where I got the delegate registrations to inject the parameter as an input to the delegate.

About this time is when I started running into trouble, and the first issue I hit was what sort of type casting they allow for the key being injected. Maybe an example to help explain. Let's say you have a class like this:

public class MyKeyedComponent
{
  public MyKeyedComponent([FromKey]string key)
  {
  }
}

In this case, when you register the thing...

builder.RegisterType<MyKeyedComponent>().Keyed<MyKeyedComponent>("the-key");

and then resolve it...

container.ResolveKeyed<MyKeyedComponent>("the-key");

...the parameter passed into the constructor will be the-key - the key used to resolve the thing. And that works great because the key used to resolve the service is a string and the key in the constructor parameter is a string.

However, it seems like you should be able to do this:

public class MyKeyedComponent
{
  public MyKeyedComponent([FromKey]object key)
  {
  }
}

string can be converted to object, so that should be legal. And I think that does work in MEDI. But when it came to AnyKey it got more complicated. AnyKey means it could be literally anything.

// ALL of these should work.
container.ResolveKeyed<MyKeyedComponent>("the-key");
container.ResolveKeyed<MyKeyedComponent>(17);
container.ResolveKeyed<MyKeyedComponent>(new DivideByZeroException());

So if the key gets injected into the constructor as a parameter, what are the legal conversions that can happen? I started out trying to do my best to convert it to be compatible but it turns out there are tests that have some conversions being not allowed and providing InvalidOperationException.

I haven't yet really figured out which conversions are allowed and which aren't. Does it have to be exact match? Does it allow casting down from something to object? If the constructor parameter takes an integer and someone resolves with an enum based on an integer, is that allowed? I don't know.

This is where I started getting wrapped around the axle a little with AnyKey. If it can be any key, it seems the conversion should allow for something maybe more robust than what MEDI is specifying. But I then got off on a different issue:

What's the lifetime scope handling on AnyKey registrations?

In Autofac, this is easy to figure out:

builder.RegisterType<SomeType>().Keyed<SomeType>("key").Keyed<SomeType>("otherkey").SingleInstance();

If you resolve with key or otherkey you'll get the same singleton instance.

But let's look at AnyKey. (builder.RegisterType<T>().Keyed<T>(KeyedService.AnyKey))

So now we have a situation where the lifetime scope on a single service that got registered - a service registered with AnyKey - needs to result in dynamically added components that have their own lifetimes that are identical to the originally registered component. That got me looking down the road of creating a registration source for these things to dynamically add the component as it's resolved. Unfortunately, I hit a bit of a wall with that approach where the cloned component wasn't seen as unique.

I didn't even get to the point of figuring out open generic registrations (but I think it was generally working), delegate registrations, or null key support; when I got hit with AnyKey and the lifetime scope problem that didn't seem easily fixable by a registration source or by middleware, it made me realize that we'd likely need to actually add AnyKey style support into core Autofac.

And that's where it sits.

As mentioned, we'd love the help if you have the time. And if any of the stuff I mentioned needs clarification or you're curious about other aspects of what's in the branch, definitely let me know.

@tillig
Copy link
Member

tillig commented Jan 5, 2024

Thinking out loud: From a core Autofac perspective, it's like the concept of a keyed component, at least one registered with AnyKey, is something between a standard keyed component (registered with a specific key) and a registration source.

I say it's between there because:

  • It needs to somehow "generate" new components based on key - each key used sort of "clones" the component with lifetime/ownership.
  • It needs to obey registration order - something registration sources don't currently do. Sources get evaluated last right now, which is fine for things like Lazy<T> or IEnumerable<T> but probably not so much for keyed components.

Maybe that means changing the order in which registration sources are evaluated - evaluate them in the same order as components instead of a separate evaluation order.

Again, just thinking out loud/taking notes. Not recommending anything.

@srogovtsev
Copy link

The idea on top of my head is based on the question whether we even want to support the M.E.DI behavior in Autofac; I mean, how much of a cognitive problem we'd have on our hands if the AnyKey support would be limited only to this package? E.g., no option of registering keyed service with AnyKey in Autofac, and no way of resolving it via ILifetimeScope, only via IKeyedServiceProvider? This might allow us to separate the behavior and avoid affecting core Autofac.

@tillig
Copy link
Member

tillig commented Jan 5, 2024

Oh, I'm not pushing for core Autofac changes. If it's possible to limit it to this package, awesome. I'm just trying to keep track of thoughts as they pop up.

@srogovtsev
Copy link

Same here, I'm just entertaining the thought and trying to understand how weird it'd be when resolution of a keyed service in IKeyedServiceProvider works (due to AnyKey registration), but fails in ILifetimeScope.

@tillig
Copy link
Member

tillig commented Jan 5, 2024

Might be... a little weird. Maybe not enough to matter? But could be a sort of odd gotcha if someone is trying to use Autofac semantics to resolve an IEnumerable<T> of keyed instances and it isn't complete. In fact, that might be a reason it has to somehow work with core Autofac, or at least merge into the standard Autofac semantics: container.ResolveKeyed<IEnumerable<T>>("key") should work, and it should include things registered with AnyKey. I think? It'd be weird if it didn't. I think it'd get challenging if it was in the middle of a resolve chain and not working.

@grendizeras
Copy link

The idea on top of my head is based on the question whether we even want to support the M.E.DI behavior in Autofac; I mean, how much of a cognitive problem we'd have on our hands if the AnyKey support would be limited only to this package? E.g., no option of registering keyed service with AnyKey in Autofac, and no way of resolving it via ILifetimeScope, only via IKeyedServiceProvider? This might allow us to separate the behavior and avoid affecting core Autofac.

There may be a problem, when third party library uses this feature to inject it's services into container, or resolve them with AnyKey type.

@FarseerSky
Copy link

We recently upgraded both Orleans 8.0 and Abp Framework. While Abp uses Autofac, Orleans 8.0 uses KeyedService, which results in a startup error: System.InvalidOperationException: This service descriptor is keyed.

Could you please fix this issue as soon as possible or provide a temporary solution to resolve it?

@alistairjevans
Copy link
Member

@tillig, a note on registration order and collections for keyed services; the collection source in Autofac sorts by the original registration order, so I'd expect the order of generated any-key-sourced services to match, provided it copies the metadata over from the original registration.

I've spent the morning working on the keyed services branch, and all the specification tests now pass on the feature/keyed-services branch; no core changes needed.

Summary of changes from your base @tillig are as follows:

  • Do not generate an additional "AnyKey" registration when the service key being requested is already "AnyKey".
  • Do not generate collection AnyKey registration to avoid ignoring the set of actual key registrations.
  • Make sure we generate a new Guid for generated registrations instead of copying, so we get separate instance tracking (I think this one was actually my bad originally).
  • Add the keyed implementation factory support by casting to ResolveRequestContext.
  • Make IsKeyedService understand null keys.
  • Move the [FromKeyedServices] parameter out of the keyed service conditional so it is always injected.

Can you check the changes over and that they make sense and I haven't missed anything?

@tillig
Copy link
Member

tillig commented Jan 12, 2024

That's awesome! I'll look it over in the next couple days.

@tillig
Copy link
Member

tillig commented Jan 15, 2024

@alistairjevans This looks great. Thanks for getting it over the line. I'm glad I wasn't too far off with the answer, but clearly had some... uh... cognitive challenges stopping it from just getting completed. And it's totally isolated from Autofac core, so that's super sweet.

I removed the temporary debuggable tests fixture and updated the dependencies to latest releases.

Here's how I'd like to propose release here:

  1. Release core Autofac 8.0.0. There's a small breaking change in there and it includes the .NET 8 build.
  2. Update this branch to use Autofac 8.0.0. Just to verify things are still working.
  3. Update semver to 9.0.0. Since it'll start requiring the updated versions of MEDI dependencies, that could be a breaker for some folks. I'm not glued to that, though; if 8.1.0 sounds better, I'd be OK with that. It's a feature update and should be back-compat except for that dependency update.
  4. Merge this branch back to develop. Given we paired on it, I'm not sure it's worth a whole PR cycle. Of course, if it's interesting to have the merge collapsed to a single commit and/or have the PR for record-keeping purposes, I'm fine with that.
  5. Release AEDI (develop => main).

If that all sounds good, I can probably get it done tomorrow.

@alistairjevans
Copy link
Member

Sounds good, happy with the major semver change. Can I ask for that PR though for future paperwork/tie-to-issue reasons?

I can approve and merge before you wake up tomorrow.

@tillig
Copy link
Member

tillig commented Jan 15, 2024

I'll PR it with everything but the Autofac core dependency upgrade. I don't have time right now to shepherd that through, but I could do it tomorrow.

@srogovtsev
Copy link

@alistairjevans , @tillig , I just wanted to say you're amazing, and I'm sorry I wasn't able to help even while I decided to speak up for that.

@tillig
Copy link
Member

tillig commented Jan 15, 2024

@srogovtsev no worries, mate, we appreciate any help we can get and you've thrown in some great contributions in the past. I'm just glad we got this over the line!

@tillig tillig self-assigned this Jan 16, 2024
@tillig
Copy link
Member

tillig commented Jan 16, 2024

PR approved and merged. I'll do the core Autofac release and subsequent AEDI update/release tomorrow.

@tillig
Copy link
Member

tillig commented Jan 16, 2024

Autofac 8.0.0 and Autofac.Extensions.DependencyInjection 9.0.0 are published.

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

Successfully merging a pull request may close this issue.

6 participants