Setting key must match property exactly, preventing use of some json styles

[dodexahedron]

Functional impact

Due to the fact that, on line 172 of ConfigurationBinder.cs, the reflected property name is used for matching properties to JSON dictionary keys, it is not possible to consume many legal JSON properties in a configuration file.

This is a problem when attempting to load files that follow a style that includes dashes or other characters that are illegal in identifiers, in C#, VB.Net, and other languages.

For example, a JSON property such as "multi-word-key": "some value" cannot be assigned to any object value and must be accessed via the configuration object's indexer, which is cumbersome and not compile-time type safe (thus static analysis is impossible).

Minimal repro steps

The following C# code and JSON file will demonstrate the problem.

C# Code:

namespace ExampleCase
{
    using System;
    using System.Runtime.Serialization;
    using Microsoft.Extensions.Configuration;

    [DataContract]
    public class MySettings
    {
        [DataMember( Name = "multi-word-setting" )]
        public string MultiWordSetting { get; set; }
    }

    public class ExampleCase
    {
        public static void Main( string[ ] args )
        {
            IConfigurationBuilder configBuilder = new ConfigurationBuilder( ).AddJsonFile( "settings.json" );
            IConfigurationRoot configRoot = configBuilder.Build( );
            MySettings config = new MySettings(  );
            configRoot.Bind( config );

            Console.WriteLine( configRoot[ "multi-word-setting" ] ); //This will output the expected value
            Console.WriteLine( config.MultiWordSetting ); //However, this will be null
        }
    }
}

JSON File settings.json:

{
    "multi-word-setting": "some string"
}

Expected result

The config.MultiWordSetting property should have the value specified by its associated JSON, as specified by the DataMemberAttribute.
Ideally, there would be a specific attribute created for exactly this case (e.g. ConfigurationElementAttribute), as well as support for respecting other common attributes, such as DataMemberAttribute.

Actual result

The property is not set, because the mentioned line of code in the binder relies on the explicit matching of the property's name to the json key.

[dodexahedron]

I implemented this as using DataMemberAttribute, but I'm wondering if it would be a better idea to just use a new custom attribute class that lives in this namespace, so that this is not a potentially breaking change.

[erwan-joly]

@dodexahedron none of the functionality of DataMemberAttribute works in configuration. So it can't really break anything(except if someone use them as a useless flag and don't want the normal behavior to be there). I think using DataMemberAttribute which is part of serialisation make sense here. Could be nice to add EmitDefaultValue Order and TypeId of this flag too as there is another pull request for the isRequired of this same attribute #36569.

[dodexahedron]

My only thought was ust that if someone has dual-purposed a class. Sure, DataMemberAttribute and DataContractAttribute may not be used in configuration, but that doesn’t mean that someone hasn’t placed those attributes on a class they're using for it, already. In that case, a new attribute would be better and have no chance of conflict. That is arguably not a great design or someone to use, but it's a non-zero chance that there's code out there doing this already. If that's not a significant enough concern, then yeah DataMemberAttribute is a simple choice. Non-zero though that chance may be, it's probably VERY close to zero, considering the age of this namespace in the first place, so I don’t have a stronger argument than that for a new attribute, if there's a desire to contain the proliferation of additional classes in the library. I, personally, don’t see a negative in a new attribute, especially if it is kept simple (and would be happy to contribute said attribute if that’s the direction chosen), but I'm not strongly for or against either method.

…

Sent from my Windows 10 phone From: Erwan JOLY Sent: Wednesday, February 28, 2018 10:43 PM To: aspnet/Configuration Cc: dodexahedron; Mention Subject: Re: [aspnet/Configuration] Setting key must match property exactly,preventing use of some json styles (#774) @dodexahedron none of the functionality of DataMemberAttribute works in configuration. So it can't really break anything. I think using DataMemberAttribute which is part of serialisation make sense here. Could be nice to add EmitDefaultValue Order and TypeId of this flag too as there is another pull request for the isRequired of this same attribute dotnet/runtime#36569. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[erwan-joly]

yes but if they already use it the expected behavior should be the behavior you implemented. Seems to be the normal behavior to me

[moonheart]

@dodexahedron Maybe we can add a PropertyResolver to BinderOptions moonheart/Extensions@983a557.

And use like this:

public class MyObject{
    [JsonProperty(PropertyName = "my_property")]
    public string MyProperty { get; set; }
}

var myObject = config.Get<MyObject>(options =>
{
    options.PropertyNameResolver = property =>
    {
        if (property.GetCustomAttribute(typeof(JsonPropertyAttribute)) is JsonPropertyAttribute jsonPropertyAttribute) 
            return jsonPropertyAttribute.PropertyName;
        return property.Name;
    };
});

[nbarbettini]

It would be great for the binder to observe DataMemberAttribute. It seems like a nice, idiomatic way to support renaming properties.

[lzandman]

This is still marked as "Open". Is this being actively worked on? We have a need for this.

All our Azure Functions rely on environment variables for configuration. We've (artificially) grouped by prefixing them with a common identifier and using a dot to separate the prefix from the variable name. Now we're migrating them to .Net Core and noticed problems when using dependency injection for our Azure Function configuration ( https://docs.microsoft.com/bs-latn-ba/azure/azure-functions/functions-dotnet-dependency-injection ).

We like to be able to keep our current variable names and have them automagically mapped to our configuration classes. Being able to use a DataMember-attribute solution would be great.

[ghost]

I agree this is very much needed for even general use cases, i.e loading specific environment variables with raw naming (I_AM_AN_ENVIRONMENT_VARIABLE) to friendly configuration name (IAmAnEnvironmentVariable).

[safern]

Thanks @dodexahedron for the proposal. I'm closing this as a dupe of: #36010 which has a formal proposal for this.