Support TypeConverterAttribute on bound properties

[kyschouv]

It's very difficult to add a custom TypeConverter for framework types to have them bind correctly within ConfigurationBinder. The documentation for TypeConverterAttribute specifies that the attribute can be attached to a property, and that property will then use the specified TypeConverter during serialization/deserialization. Unfortunately, ConfigurationBinder doesn't take the property's attributes into consideration during BindProperty, instead passing the binding off to BindInstance with just the type of the property.

Presently, the only way to use custom TypeConverters is to register them using TypeDescriptor.AddAttributes before any configuration binding occurs, which is inconvenient if your library requires custom TypeConverters for standard types.

[HaoK]

@ajcvickers We should retriage this, as I think TypeConverters might be a good option for how we deal with custom config binding of optoins in general.

[HaoK]

Also look into general binding for options + config

[ajcvickers]

For anyone interested in contributing this, there is a good start in the PR here: aspnet/Configuration#787

[dgioulakis]

I would personally rather see support for Json.net JsonConverter. I prototyped a sample here: https://github.com/Cephei/JsonNetConfigurationBinder

TypeConverter is a nice to have, however, not very powerful in this context considering it requires the Value of the IConfigurationSection to be present. You can't use it to parse ConfigurationSection's with children like in the link above.

[jchannon]

Are there any plans to control the deserilaization of IConfiguration, little seems to have been discussed for a while now

[maryamariyan]

We looked at the PR in aspnet/Configuration#787 again and seems like when this gets addressed again we make the fix works well with all configuration providers

[ericstj]

I can see the value in this change as it allows folks to define custom handling of types at the member level without globally registering the converter. It also allows the model to specify the converter, rather than the caller to Bind.

It looks to me like the PR added this functionality exclusively in the Configuration.Binder. Would this be used by all Configuration providers? (honest question, I don't know, but I think it would)

I do think the support for TypeConverter in Confiugration today and the PR is incomplete. I think the right way to handle would be to go wholesale through the TypeDescriptor system as a fallback. Where you get the PropertyDescriptors for the type and use those to get the converter. In this way, Configuration never ends up doing the work to find those attributes and the TypeDescriptor infra would do that (along with all of its functionality for overrides).

I can imagine that a larger dependency on metadata here can be challenging for source-generators for Configuration, so that might be something to consider. I believe for WPF they determine at design time what converter is used and bake that into generated source / baml.

All this said, I'm not sure if we want to go down a path of tighter coupling of Configuration with some other serialization tech (TypeDescriptor, JSON, or otherwise). I'd be interested to hear @davidfowl's opinion.

[KalleOlaviNiemitalo]

I wanted to bind an options class that has a TimeSpan Timeout property in which Timeout.InfiniteTimeSpan makes sense. This special value could perhaps be configured as "-00:00:00.001" but that's just too ugly. I'd much prefer being able to set a per-property TypeConverter that recognizes "Infinite" or the empty string.

A nullable property TimeSpan? Timeout doesn't work either, as the constructor of the options class stores a nonzero default value there and ConfigurationBinder does not overwrite this default value if it gets null as out object result from TryConvertValue.

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Lines 263 to 266 in 4822e3c

	if (propertyValue != null && hasSetter)
	{
	property.SetValue(instance, propertyValue);
	}

I suppose the workaround is to add a sibling bool UseTimeout property.