Import Keyed injects wrong instance

Multiple Keyed (Named?) Instances. Constructor wants a specific key. Grace injects the wrong instance. Am I doing something wrong? See //returns Mongo below...

.Net Core (NetStandar 2.0)
Grace 6.4.0

using System;
using Grace.DependencyInjection;
using Grace.DependencyInjection.Attributes;

namespace test
{
class Program
{
static void Main(string[] args)
{
DatabaseFactory SqlServer = new DatabaseFactory() {name = "SqlServer"};
DatabaseFactory Oracle = new DatabaseFactory() {name = "Oracle"};
DatabaseFactory Mongo = new DatabaseFactory() {name = "Mongo"};

        var container = new DependencyInjectionContainer();
        container.Configure(c =>
        {
            c.ExportInstance(SqlServer).As<IDatabaseFactory>().AsKeyed<IDatabaseFactory>(SqlServer.name);
            c.ExportInstance(Oracle).As<IDatabaseFactory>().AsKeyed<IDatabaseFactory>(Oracle.name);
            c.ExportInstance(Mongo).As<IDatabaseFactory>().AsKeyed<IDatabaseFactory>(Mongo.name);
            c.Export<TestDal>().As<ITestDal>();
        });

        IDatabaseFactory dbFactory = container.Locate<IDatabaseFactory>(withKey: "SqlServer");
        if (dbFactory.name != "SqlServer") //works
            throw new Exception("wrong dbFactory");


        var testDal = container.Locate<ITestDal>();

        if (testDal.DatabaseFactory.name != "SqlServer") //returns Mongo
            throw new Exception("wrong database in Dal");

    }
}

public interface IDatabaseFactory
{
    string name { get; set; }
}

public class DatabaseFactory : IDatabaseFactory
{
    public string name { get; set; }
}

public interface ITestDal
{
    IDatabaseFactory DatabaseFactory { get; }
}

public class TestDal : ITestDal
{
    private IDatabaseFactory _databaseFactory;

    public TestDal([Import(Key = "SqlServer")] IDatabaseFactory techXpress)
    {
        _databaseFactory = techXpress;
    }

    public IDatabaseFactory DatabaseFactory
    {
        get { return _databaseFactory; }
    }
}

}

@scovel

Hi @scovel

The reason it's returning the mongo is instance is because it's not actually processing the import attribute and picking the last registered instance of IDatabaseFactory. This could be argued to be a bug because it seems like it should throw an export not found rather than picking a keyed export. I'm hesitant to change the behavior in a non major release because even though it's not technically correct it seems like a major behavior shift to introduce in a minor version release.

The assumption I made when creating the container is that people would use either the fluent interface or the attributes for configuration. I hadn't thought of the idea of using fluent for part of the registration and attributes for the other.

That said you're the second person recently that ran into problems with this exact use case. I think maybe the answer is to add a flag to force the container to process Import attributes on parameters even when doing fluent validation.

Ian, Thanks yet again for your quick response! This is a pattern that is used in MS Unity, “named instances.” I suspect the other guy was a Unity user like myself in a past life… Can you give me an example of how to make it work just using attributes? I’d give it a try. I’d prefer to use the code I sent you, but if there is a better/working way, I’d give it a go. I’d definitely vote for adding that feature to Grace. I agree, it should either fail with an error, or work properly. As it stands it does inject something, but the wrong something. Luckily I’m a suspicious guy and wrote a test app with unit tests to prove out using Grace, so the behavior showed up in a unit test. Again, thanks for the quick response, and all your hard work on Grace. Sean P.S. My son’s name is Ian, which Scottish for John, and Sean is Irish for John. From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Wednesday, August 1, 2018 at 8:16 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) Hi @scovel<https://github.com/scovel> The reason it's returning the mongo is instance is because it's not actually processing the import attribute and picking the last registered instance of IDatabaseFactory. This could be argued to be a bug because it seems like it should throw an export not found rather than picking a keyed export. I'm hesitant to change the behavior in a non major release because even though it's not technically correct it seems like a major behavior shift to introduce in a minor version release. The assumption I made when creating the container is that people would use either the fluent interface or the attributes for configuration. I hadn't thought of the idea of using fluent for part of the registration and attributes for the other. That said you're the second person recently that ran into problems with this exact use case. I think maybe the answer is to add a flag to force the container to process Import attributes on parameters even when doing fluent validation. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXo2H6QRpEujLmcwSL7yjkxPQ3VHpks5uMZwWgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

@scovel

@scovel for the moment I think it's much easier to make a slight change to the fluent registration to specify the key than set up registration using attributes.

c.Export<TestDal>().As<ITestDal>().WithCtorParam<IDatabaseFactory>().LocateWithKey("SqlServer");

I'll see if I can find some time this weekend to add the flag as it's shouldn't much work and I'll release a beta for you to try.

That's very cool about your son, very good name :)

That kinda works… If I add just the one registration, it works. c.Export<TestDal>().As<ITestDal>().WithCtorParam<IDatabaseFactory>().LocateWithKey("SqlServer"); If I add all 3, it goes back to picking Mongo. c.Export<TestDal>().As<ITestDal>().WithCtorParam<IDatabaseFactory>().LocateWithKey("Oracle"); c.Export<TestDal>().As<ITestDal>().WithCtorParam<IDatabaseFactory>().LocateWithKey("Mongo"); Let me describe the use-case so we’re on the same page. We have a “framework” that is used by our developers to handle most cross-cutting concerns. (we have a lot of less experienced developers, in a different time-zone…). One thing the framework does is read a config file for database connection information and add them to the container. DATASOURCE_CONFIG=[\ {"ConnectionType":"MsSqlServer","ConnectionName":"SqlServer","Properties":[{"key":"ProviderName","value":"System.Data.SqlClient"},{"key":"ConnectionString","value":"xxxx"}]},\ {"ConnectionType":"Oracle","ConnectionName":"Oracle","Properties":[{"key":"ProviderName","value":"Oracle.ManagedDataAccess.Client"},{"key":"ConnectionString","value":"xxxx", "value": “CONFIG_DV"}]},\ {"ConnectionType":"MongoDb","ConnectionName":"Mongo","Properties":[{"key": "server","value": "xxx"}]}] We then inject named instances of a database factory into the container. When the developers create their business Dal, they specify which database factory they need in their constructor, and it gets injected automagically. Mjultiple connections will be registered. Multiple business Dals will get registered (again, automatically by the framework). Actually we scan their components and add them to the container, they don’t even specify the registration themselves. What this syntax meant to me: public TestDal([Import(Key = "SqlServer")] IDatabaseFactory databaseFactory) Is the container should find the named instance of the IDatabaseFactory called “Oracle” and inject it. If there were multiple Dals… public TestDal2([Import(Key = "Oracle")] IDatabaseFactory databaseFactory) the container should be able to find the correct named instance and inject it at run-time. Does that make sense? Let me know if you need any more details. Sean From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Wednesday, August 1, 2018 at 8:44 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) @scovel<https://github.com/scovel> for the moment I think it's much easier to make a slight change to the fluent registration to specify the key than set up registration using attributes. c.Export<TestDal>().As<ITestDal>().WithCtorParam<IDatabaseFactory>().LocateWithKey("SqlServer"); I'll see if I can find some time this weekend to add the flag as it's shouldn't much work and I'll release a beta for you to try. That's very cool about your son, very good name :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXswRjGDC_BK0kDqBqtzApVn2QEDyks5uMaKUgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

That makes complete sense. I’ll write a more in depth reply this evening but the short version is I’ll add a flag to process the import attribute automatically and it should solve your issue.

Awesome! Thanks From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Wednesday, August 1, 2018 at 10:47 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) That makes complete sense. I’ll write a more in depth reply this evening but the short version is I’ll add a flag to process the import attribute automatically and it should solve your issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXtFCGjJzNMFpXvb-D3mtWwwzUKeYks5uMb9bgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

adding flag to process import attributes for constructors issue #181

@scovel

@scovel I've added a flag to process import attributes on constructor parameters. I set it to true by default because it seems like if you went the length to attach an attribute you want it processed.

Would it be possible for you to test it off the nightly nuget feed? I'd prefer to test it before I release an RC to nuget.org.

https://ci.appveyor.com/nuget/grace-master

I was just looking through your dev branch when this email came in. Sure, I can test it. I’ll get back to you today. Sean From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Friday, August 3, 2018 at 8:22 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) @scovel<https://github.com/scovel> I've added a flag to process import attributes on constructor parameters. I set it to true by default because it seems like if you went the length to attach an attribute you want it processed. Would it be possible for you to test it off the nightly nuget feed? I'd prefer to test it before I release an RC to nuget.org. https://ci.appveyor.com/nuget/grace-master — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXlGlJM2kJQZu-nll5_0RWycthlFKks5uNEB4gaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

@scovel

@scovel one thing I did realize when I was working this is that your registration should be tweaked a little.

I think you want

c.ExportInstance(SqlServer).AsKeyed<IDatabaseFactory>(SqlServer.name);

I don't think you want the extra As<IDatabaseFactory> , I think you want only the keyed export.

That would explain why I had each item in the container 2X… Where do I get the nightly build? From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Friday, August 3, 2018 at 8:40 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) @scovel<https://github.com/scovel> one thing I did realize when I was working this is that your registration should be tweaked a little. I think you want c.ExportInstance(SqlServer).AsKeyed<IDatabaseFactory>(SqlServer.name); I don't think you want the extra As<IDatabaseFactory> , I think you want only the keyed export. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXp5QdbCbkqOzkYpmQC_zlry_TZbiks5uNESqgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

I pulled the dev branch and built it… It works as expected in my test project. AWESOME! Thanks, Sean From: Sean Covel <SCOVEL@travelers.com> Date: Friday, August 3, 2018 at 8:41 AM To: ipjohnson/Grace <reply@reply.github.com>, ipjohnson/Grace <Grace@noreply.github.com> Cc: Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) That would explain why I had each item in the container 2X… Where do I get the nightly build? From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Friday, August 3, 2018 at 8:40 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) @scovel<https://github.com/scovel> one thing I did realize when I was working this is that your registration should be tweaked a little. I think you want c.ExportInstance(SqlServer).AsKeyed<IDatabaseFactory>(SqlServer.name); I don't think you want the extra As<IDatabaseFactory> , I think you want only the keyed export. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXp5QdbCbkqOzkYpmQC_zlry_TZbiks5uNESqgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

adding extra null check for issue #181

@scovel

@scovel I'll push an RC to nuget this weekend and plan to do an official release later this month

I released an RC version today

The RC is working perfectly. Looking forward to the official release. Thanks for your hard work and quick response. Sean From: Ian Johnson <notifications@github.com> Reply-To: ipjohnson/Grace <reply@reply.github.com> Date: Monday, August 6, 2018 at 11:55 AM To: ipjohnson/Grace <Grace@noreply.github.com> Cc: Sean Covel <SCOVEL@travelers.com>, Mention <mention@noreply.github.com> Subject: Re: [ipjohnson/Grace] Import Keyed injects wrong instance (#181) I released an RC version today — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#181 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYMQXhU9eGwTiue_vYpEKYE6YM3Hg9eKks5uOGbrgaJpZM4Voi_a>.

…

________________________________ This message (including any attachments) may contain confidential, proprietary, privileged and/or private information. The information is intended to be for the use of the individual or entity designated above. If you are not the intended recipient of this message, please notify the sender immediately, and delete the message and any attachments. Any disclosure, reproduction, distribution or other use of this message or any attachments by an individual or entity other than the intended recipient is prohibited. TRVDiscDefault::1201

@scovel

@scovel sorry had to publish another preview for another issue. I'm shooting for middle of next week for an official release.

Navigation Menu

Repository navigation

Import Keyed injects wrong instance #181

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Import Keyed injects wrong instance #181

Description

Activity

ipjohnson commented on Aug 1, 2018

scovel commented on Aug 1, 2018

ipjohnson commented on Aug 1, 2018

scovel commented on Aug 1, 2018

ipjohnson commented on Aug 1, 2018

scovel commented on Aug 1, 2018

ipjohnson commented on Aug 3, 2018

scovel commented on Aug 3, 2018

ipjohnson commented on Aug 3, 2018

scovel commented on Aug 3, 2018

scovel commented on Aug 3, 2018

ipjohnson commented on Aug 3, 2018

ipjohnson commented on Aug 6, 2018

scovel commented on Aug 14, 2018

ipjohnson commented on Aug 16, 2018

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions