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

Resolve aggregate services using keyed registrations #14

Open
rcdailey opened this issue Oct 13, 2023 · 3 comments
Open

Resolve aggregate services using keyed registrations #14

rcdailey opened this issue Oct 13, 2023 · 3 comments

Comments

@rcdailey
Copy link

rcdailey commented Oct 13, 2023

I have an aggregate service containing interfaces that are only registered with keys (using Keyed<>()). Furthermore, each interface within the aggregate has the same number of concrete implementations, which are all mapped to the same key. Essentially it's a non-jagged 2D array of registrations.

I'd like the ability to resolve the aggregate service with IIndex, which would forward the key to the resolution of the services within it.

I have an example that demonstrates the behavior I'm looking for, which I'll provide below.

internal interface IMyService2
{
    void PrintInfo();
}

internal interface IMyService1
{
    void PrintInfo();
}

internal class MyService1Impl1 : IMyService1
{
    public void PrintInfo()
    {
        Console.WriteLine(" - MyService1Impl1");
    }
}

internal class MyService1Impl2 : IMyService1
{
    public void PrintInfo()
    {
        Console.WriteLine(" - MyService1Impl2");
    }
}

internal class MyService2Impl1 : IMyService2
{
    public void PrintInfo()
    {
        Console.WriteLine(" - MyService2Impl1");
    }
}

internal class MyService2Impl2 : IMyService2
{
    public void PrintInfo()
    {
        Console.WriteLine(" - MyService2Impl2");
    }
}

internal interface IMyAggregateServices
{
    public IMyService1 Service1 { get; init; }
    public IMyService2 Service2 { get; init; }
}

public static class Program
{
    public static void Main()
    {
        var builder = new ContainerBuilder();

        builder.RegisterType<MyService1Impl1>().Keyed<IMyService1>("one");
        builder.RegisterType<MyService1Impl2>().Keyed<IMyService1>("one");
        builder.RegisterType<MyService2Impl1>().Keyed<IMyService2>("two");
        builder.RegisterType<MyService2Impl2>().Keyed<IMyService2>("two");
        builder.RegisterAggregateService<IMyAggregateServices>();

        var container = builder.Build();

        var index = container.Resolve<IIndex<string, IMyAggregateServices>>();
        var aggregateServices = index["one"];

        Console.WriteLine("Print Service1:");
        aggregateServices.Service1.PrintInfo();

        Console.WriteLine("Print Service2:");
        aggregateServices.Service2.PrintInfo();
    }
}

Above, I expect the following output:

Print Service1:
 - MyService1Impl1
Print Service2:
 - MyService2Impl1

Obviously, the code above won't even execute. Instead, I get this exception:

Autofac.Core.Registration.ComponentNotRegisteredException: The requested service 'one (ConsoleApp1.IMyAggregateServices)' has not been registered. To avoid this exception, either register a component to provide the service, check for service registration using IsRegistered(), or use the ResolveOptional() method to resolve an optional dependency.

If there's a reasonable workaround, I'm open to that as well. I'm sure I could implement my own concrete aggregate class to simulate this somehow, but I was hoping native support for this would be added.

@tillig
Copy link
Member

tillig commented Oct 13, 2023

The workouts is to make the concrete class, as you probably guessed.

If this is a feature you're interested in, it's be good if you fleshed out how it should work. It's a non-trivial API proposal, not just a "fix this easily reproducible bug."

What should it look like to define an aggregate service that does this? Have you done a rough draft implementation or any deeper investigation to see if it could be done without any core Autofac changes?

I recognize this is probably deeper driving than you might have been hoping for, but there isn't a giant team addressing issues - we rely on community help. Plus, doing the leg-work to save is time also does how much you really want it - no time to look into it, maybe it's not so important.

In general, I think this is an interesting concept. We just need to get some "requirements" down.

@rcdailey
Copy link
Author

Based on your responses to other issues, I completely respect that you don't have resources to fulfill every request. My expectation is to get some design feedback from you, possibly some workaround, and I can decide based on that if it's worth spending time on. I'll admit that I'm already spread thin myself but I do enjoy contributing to projects I benefit from when I can.

I immediately assume this kind of feature will have corner cases. The most obvious first one is: what if a particular service in the aggregate doesn't have an implementation registered for a key mapping? I imagine that's a failure, but maybe there's a fallback system in place of some kind. Maybe jagged 2D arrays of keyed registrations has a valid use case somewhere.

I haven't thought too deeply on this. I won't lie, though, if I can get away with a workaround like a concrete class, I might just stick with that. I imagine for a quality library like Autofac, there's going to be a high bar set for changes (which it should) because it needs to be useful to a wider audience and not just my own project. It would be interesting to see if there are others out there that have wanted something similar to this.

I appreciate you taking the time to participate!

@tillig
Copy link
Member

tillig commented Oct 13, 2023

I'm happy to provide some design feedback, but I'm not sure that's going to be in the form of "I provide a design and see if it works for you." I'm kind of taking a page from the ASP.NET team on this - if you want an API change, it's not just "hey, this would be neat, thanks," but actually, "in pseudocode, here's how I think it should work - here's the registration syntax, here's what an aggregate service would look like marked up with the keys, here are cases I can think of where this could be a problem (backward compatibility issues?), here are some sample unit tests that might pass if this thing worked."

And maybe it doesn't have to be you that does it - if this is interesting to someone else in the community, maybe they can chip in on it and collaborate. That happens, too, in some of the ASP.NET design improvements. It shows the community wants it, the community has put the time in to think about the pros/cons, and that maybe it's worth being a feature that the team should maintain and support [effectively] forever.

I'm happy to leave this open for a while. We've had feature requests that have sat open for months or years. If it doesn't get any traction, maybe that's a sign that we saved some time working on a feature that very few would use.

On another note, it is interesting that you raise keyed services now because the .NET DI container is adding keyed support in .NET 8. That's actually a good example of the community working together on an API proposal. You can see the issue where it was opened here, and it shows an example of the kind of thought about a feature that I'm talking about - the proposal, the use cases, examples of how it is anticipated to work, that sort of thing.

I see the start of some of that here - the concept and code that doesn't work. Fill in some of the blanks and it starts becoming something actionable, actual API and behavior requirements.

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

No branches or pull requests

2 participants