dadhi / DryIoc

DryIoc is fast, small, full-featured IoC Container for .NET
MIT License
1.03k stars 122 forks source link

[question/feature request] keyed services in delegate resolution #636

Closed kicsiede closed 8 months ago

kicsiede commented 8 months ago

is it possible to use keyed services in delegate resolution?

what i mean is like this:

var c = new DryIoc.Container();
c.RegisterInstance("firstInstance", serviceKey: "firstInstance");
c.RegisterInstance("secondInstance", serviceKey: "secondInstance");
c.RegisterDelegate(([Keyed /* key resolved from parameter name */]string firstInstance, 
    [Keyed("secondInstance")]string sec) => firstInstance, serviceKey: "test");

Assert.AreEqual("firstInstance", c.Resolve<string>("test"));

passing parameters could go along the lines of wrapping values like "myparameter".WithKey("key") that would create a key-value tuple.

i understand that this would create some runtime overhead, however that's a price the user could choose to pay.

being able to resolve dependencies from parameters would also be nice. (see ninject parameters)

dadhi commented 8 months ago

@kicsiede Looking...

dadhi commented 8 months ago

I did a small change to enable this scenario you may look at my commit, especially into the test. I still need to fix one regression. But acshually the change simplifies and speedups the Delegate registration. So thanks for bringing my attention to it.

kicsiede commented 8 months ago

i guess in the changed version i get the ParameterInfo instances of the original delegate.method now, so i can default to par.Name like this:

  var c = new Container(Rules.Default.With(parameters: static request => static par /* note in v5.4.3 par.Name == "par1" instead of "first" */ =>
      {
          var attr = par.GetCustomAttribute<KeyedAttribute>();
          return attr != null ? ParameterServiceInfo.Of(par, ServiceDetails.Of(serviceKey: attr.ServiceKey ?? par.Name)) : null;
      }));

how would you go about passing instances / keyed instances when resolving? for single resolutions should i create a copy of the container and add new registrations there, or is there a more efficient way?

dadhi commented 8 months ago

how would you go about passing instances / keyed instances when resolving? for single resolutions should i create a copy of the container and add new registrations there, or is there a more efficient way?

What do you mean?, provide the example please.

kicsiede commented 8 months ago
var c = new Container(Rules.Default.With(parameters: static request => static par =>
{
    var attr = par.GetCustomAttribute<KeyedAttribute>();

    return attr != null ? ParameterServiceInfo.Of(par, ServiceDetails.Of(serviceKey: attr.ServiceKey ?? par.Name).RequiredServiceType, ServiceDetails.Of(serviceKey: attr.ServiceKey ?? par.Name)) : null;
}));
//c.RegisterInstance("firstInstance", serviceKey: "first");
//c.RegisterInstance("secondInstance", serviceKey: "second");
c.RegisterDelegate(static ([Keyed /* key resolved from parameter name */]string first, [Keyed("second")]string sec) => first, serviceKey: "test");

c.Resolve<string>("test", args: ["firstInstance".WithKey("first"), "secondInstance".WithKey("second"), /* some keyless service */42]);
dadhi commented 8 months ago

@kicsiede Key resolved from the parameter name does not look so robust to me. Maybe you can do [Keyed(nameof(first)]string first...

c.Resolve<string>("test", args: ["firstInstance".WithKey("first"), "secondInstance".WithKey("second"), /* some keyless service */42]);

For this feature I have an issue opened: https://github.com/dadhi/DryIoc/issues/222 Now I need to think if I need to do that for the upcoming release or stop adding the features :)

kicsiede commented 8 months ago

regarding the parameterless keyed attribute: yeah, that was my first thought as well, but then i realized that i cannot reference the name at the registration site anyway, so a proper error message is needed for failed binding in either case. (as a side note the key-type coupling in itself is also a kind of fragile construct imho)

dadhi commented 8 months ago

I have cleanup the fix for all things related and will close the issue. I likely will look into #222 before release, because I stumble on tests for #191 with passing the arguments, and it happens to be related to the other stability fixes I am working on.