Skip to content

Allow Keyed Composites #1458

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Allow Keyed Composites #1458

wants to merge 6 commits into from

Conversation

syko9000
Copy link

@syko9000 syko9000 commented May 4, 2025

I encountered a situation where multiple implementations of a service were registered with the same keys. A composite currently only wraps all implementations of that service that do not have a key.

The workaround involves creating the composite manually, instead of resolving it.

var comp = new MyComposite(container.ResolveKeyed<IEnumerable<I1>>("2")) as I1;
var comp = container.ResolveKeyed<I1>("1");

Currently the above line resolves only the last instance of I1 registered with "1". I needed this to resolve a composite containing just the registrations with a specific key.

This PR allows for that functionality. All behavior remains the same unless the composite is additionally registered with .Keyed<I1>(...), then the above line will resolve an appropriate composite.

Example from added unit test.

var builder = new ContainerBuilder();
builder.Register(ctx => new S1()).As<I1>().Keyed<I1>("1");
builder.Register(ctx => new S2()).As<I1>().Keyed<I1>("1");
builder.Register(ctx => new S3()).As<I1>().Keyed<I1>("2");
builder.Register(ctx => new S4()).As<I1>().Keyed<I1>("2");

builder.RegisterComposite<MyComposite, I1>()
    .Keyed<I1>("1")
    .Keyed<I1>("2");

var container = builder.Build();

var comp = container.Resolve<I1>(); // gets composite with all I1 without a key (all)
var comp1 = container.ResolveKeyed<I1>("1"); // gets composite with only I1 keyed to "1"
var comp2 = container.ResolveKeyed<I1>("2"); // gets composite with only I1 keyed to "2"

Still only one non-keyed service registration is allowed. Keyed registrations must be added to enable this feature.

It works by determining: the resolving service is a composite, it is resolving a parameter of an IEnumerable<>, IList<>, or ICollection<> of the same service type, or the service type is nested in relationship types. Then it does a keyed service resolution using the composites key, else a normal typed resolution.

builder.RegisterComposite<MyComplexComposite, I1>()
    .Keyed<I1>("1")
    .Keyed<I1>("2");

var comp2 = container.ResolveKeyed<I1>("2"); // gets composite with only I1 keyed to "2"

private class MyComplexComposite : I1
{
    public MyComplexComposite(IEnumerable<Lazy<Func<int, Owned<Meta<I1>>>>> implementations)
    {
    }
}

Will resolve Lazies of a Function that takes an int argument (for demonstration only in this example) that returns an Owned Meta object for S3 or S4 only.

Copy link

codecov bot commented May 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.99%. Comparing base (cf88148) to head (9c861a4).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1458      +/-   ##
===========================================
+ Coverage    78.94%   78.99%   +0.04%     
===========================================
  Files          200      200              
  Lines         5681     5693      +12     
  Branches      1145     1147       +2     
===========================================
+ Hits          4485     4497      +12     
  Misses         699      699              
  Partials       497      497              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@tillig tillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's some logic that can be pulled/adapted/reused from CollectionRegistrationSource to aid in perf/caching as well as collection type detection logic.

if ((ctx.Registration.Options & Registration.RegistrationOptions.Composite) == Registration.RegistrationOptions.Composite
&& ctx.Service is KeyedService keyedService
&& pi.ParameterType.IsGenericType
&& (pi.ParameterType.GetGenericTypeDefinition() == typeof(IEnumerable<>)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out CollectionRegistrationSource to see how we detect generic collections. There's an extension method IsGenericListOrCollectionInterfaceType that can check but also caches the result so we don't have to look it up every time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth looking at that for detection of things like array parameters or collection types that are derived from an open generic (like public class MyCollection : IEnumerable<MyType>).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the info. I moved to using the extension method that included the IEnumerable<> lookup. I looked through the rest of them to see if any of them already did my recursive lookup, there was not. I moved my lookup into the extension class as well, which feels better. I didn't know there was caching as I looked at the code for the first time on Saturday. I wrapped my lookup into it's own cache as well. Will look into the derived types.

…neric of service type code to InternalTypeExtensions, created cache for result.
@syko9000 syko9000 requested a review from tillig May 6, 2025 04:21
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

Successfully merging this pull request may close these issues.

2 participants