ThrowIfNull forces class name in property binding?

[urig]

Hi,

Thanks for making Formo so great. It's made life real easy for me on so many projects.

I have a question or possibly a bud report.

Given:

    <appSettings>
        <add key="SessionTimeout" value="20" />
    </appSettings>

and

public class WebsiteSettings
{
    public int SessionTimeout { get; set; }
}

then this

var config = new Configuration();
var settings = config.Bind<WebsiteSettings>();

yields settings.SessionTimeout == 20 which is very cool.

However, if you do this instead:

var config = new Configuration { ThrowIfNull = true };
var settings = config.Bind<WebsiteSettings>();

you get an InvalidOperationException saying:

{"Unable to locate a value for 'WebsiteSettingsSessionTimeout' from configuration file"}

Why does ThrowIfNull "force" me to include the name of the class I'm binding to as a prefix to the appSetting key? Is this by design?

[ChrisMissal]

Hey @urig, thanks for the kind words, glad you like it! 😄

Instead of using var for the assignment of config, you can achieve the behavior you want by using dynamic. Give this example a try:

dynamic config = new Configuration { ThrowIfNull = true };
var sessionTimeout = config.SessionTimeout<int>();

Let me know if that works for you.

[urig]

Hi @ChrisMissal . Thanks for the quick response!

Unfortunately I've already tried that and got the same exception.

I've taken a look at where ThrowIfNull is used:

    protected virtual string GetValue(string name)
    {
        var value = _section[name];
        if(ThrowIfNull && value == null)
        {
            throw new InvalidOperationException("Unable to locate a value for '{0}' from configuration file".FormatWith(name));
        }
        return value;
    }

It doesn't look like the difference in behavior originates there. Maybe higher up the call chain? Any ideas for me to investigate?

[urig]

Ok, I think I have the root cause: TryGetValue() (SettingsBinder.cs, line 31) tries two keys:

var keys = new[]
{
    reflectedType.Name + propertyInfo.Name,
    propertyInfo.Name,
};

When ThrowIfNull is true, the first key (in my ex: "WebsiteSettingsSessionTimeout" ) fails and throws an exception.

My expectation would be that the exception will be thrown if and only if both keys fail.

Would you consider accepting a PR that changes the flow to that effect? :)

[ChrisMissal]

Ok, I think I understand. You want to bind all your appSettings to one class?

[urig]

Yes, but why are you asking? :) I mean - How is that relevant to this proposed change?

[ChrisMissal]

Haha, it's a good question. My thought process was that you didn't need .Bind<T> if you were doing that, but I do realize that this use case should absolutely work.

If you look at the Bind_should_assign_standalone_property_from_settings test, there should be a similar test but with ThrowIfNull = true. I believe a test like that, with only one slight difference would fail, but it should not. Does that sound right to you?

If so, I would very much appreciate a PR! :)

[urig]

Great! Thanks. I'll get right on it and hope to submit PR by EOW

PS - +1 for using Shouldly in the UTs! Shouldly for assertions is as Formo is for config ;)

[ChrisMissal]

👍 😁

[urig]

Reached a point where the new UT is green, but we need another UT - One that ensures that if both keys are missing and ThrowIfNull is true, then an exception is thrown.

[ChrisMissal]

Correct, good call!

[ChrisMissal]

Closed via #44