Enable binding of IConfiguration/IConfigurationSection

[Kieranties]

Enable binding of IConfigurationSection when calling ConfigurationBinder.Bind()

• Treats IConfigurationSection as a unique case when binding a property
• Normal property name -> config section mapping applies

Addresses #274 and aspnet/Options#60

[dnfclas]

Hi @Kieranties, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

[dnfclas]

@Kieranties, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[HaoK]

This looks correct, but still looks kind of weird to me, @divega thoughts?

[HaoK]

Can you add one more test where the DerviedOptions that you bind from the first toplevel section, itself has another IConfigurationSection inside of it?

[Kieranties]

Sure I'll get this done shortly.

[Kieranties]

Updated with additional tests as requested. /cc @HaoK

[divega]

Nit: I think this could be made slightly simpler if we moved the logic into BindInstance() E.g. at the start it could do:

private static object BindInstance(Type type, object instance, IConfiguration config)
{
    if (type == typeof(IConfigurationSection))
    {
        return config;
    } 
...

Then BindProperty() would remain untouched.

[Kieranties]

I'll make this change. It makes it much cleaner :-)

[divega]

@HaoK I think we can take this PR now. I made a comment on a way I think we could simplify it a bit, but no big deal. We could also consider supporting IConfiguration as well as IConfigurationSection.

[Kieranties]

@divega I did originally have the ability to return IConfiguration but it seemed unnecessary as IConfigurationSection inherits from there. It also greatly simplifies the type checking.

[HaoK]

Hrm this feedback sounds familiar :)

@divegahttps://github.com/divega I did originally have the ability to return IConfiguration but it seemed unnecessary as IConfigurationSection inherits from there. It also greatly simplifies the type checking.

[divega]

Any IConfiguration we will ever try to bind to a property will be an IConfigurationSection so requiring the property to be of that type works. However, if the user defines a property of type IConfiguration and that property doesn't get populated it isn't great. Maybe I am missing something but I am also not sure how supporting IConfiguration would complicate the type checking. Wouldn't be a matter of checking for it too?

if (type == typeof(IConfigurationSection)  || type == typeof(IConfiguration))  
{  
    return config;  
}  

[Kieranties]

I like the clear message that when binding you can optionally directly bind child sections as IConfigurationSection. That's it, a simple 'bonus' feature to binding logic.

I don't see the benefit in having the type checking as any consumer of the bound property can always treat it as an IConfiguration if they need to, but to the binding logic its just noise.

But hey, you guys are the owners here! I'll happily drop it in if needed ;-)

[Kieranties]

If I get this merged up with the latest branch is this acceptable or would you like the additional type checking?

[HaoK]

@divega are you happy with this PR as is for merging?

[divega]

I am fine with it as is.

[HaoK]

@Kieranties can you rebase and squash your commits into one? I'll merge this after that's done...Thanks!

[Kieranties]

@HaoK Will get it done this evening

[Kieranties]

Rebased/Squashed //@HaoK

[HaoK]

Merged thanks!

3480095

[Kieranties]

👍