-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow multiple constructors in System.Text.Json #76608
Comments
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsToday, multiple constructors are only supported if one of them is decorated with the The problem with that is that when you design an API, you use constructors to ensure that the developer has specified the correct combination of properties. Those combinations are different, and all properties might not be specified in each combination. We also set those properties to get-only, just for convenience, so when you want to fill in optional parameters, you do not have to see the mandatory ones. That, in turn, makes it impossible to decorate one constructor with the To remedy this, I've written a converter that matches constructors against the information available in the JSON document. It then selects the first constructor that can be used with the available data. It's included below. Note, I originally tried Feature request: Add support for multiple constructors without the need for This is only one of my converters to get the same functionality as in Json.NET. It would help a lot if it were possible to create non-generic converters. public class MultipleConstructorsConverter<T> : JsonConverter<T>
{
/// <inheritdoc />
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (typeToConvert == typeof(TypeDto))
{
Debugger.Break();
}
if (reader.TokenType == JsonTokenType.StartObject)
{
reader.Read();
}
var values = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
DeserializePropertyValues(ref reader, typeToConvert, options, values);
var instance = CreateInstanceUsingAConstructor(typeToConvert, values);
if (instance == null)
{
throw new InvalidOperationException(
$"Failed to find all parameters to any of the constructors: {typeToConvert}.");
}
FillProperties(instance, values);
return (T)instance;
}
/// <inheritdoc />
public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
{
throw new NotSupportedException("Do not use this converter for serialization.");
}
private static object? CreateInstanceUsingAConstructor(
Type typeToConvert,
IReadOnlyDictionary<string, object> values)
{
object? instance = null;
foreach (var constructor in typeToConvert.GetConstructors())
{
var parameterValues = new List<object>();
foreach (var parameter in constructor.GetParameters())
{
if (values.TryGetValue(parameter.Name, out var value))
{
parameterValues.Add(value);
}
}
if (parameterValues.Count == constructor.GetParameters().Length)
{
instance = constructor.Invoke(parameterValues.ToArray());
}
}
return instance;
}
private static void DeserializePropertyValues(
ref Utf8JsonReader reader,
IReflect typeToConvert,
JsonSerializerOptions options,
IDictionary<string, object> values)
{
while (reader.TokenType is not JsonTokenType.EndObject)
{
#pragma warning disable SA1009 // Closing parenthesis should be spaced correctly
var propertyName = reader.GetString()!;
#pragma warning restore SA1009 // Closing parenthesis should be spaced correctly
// Move past property name
reader.Read();
#pragma warning disable IDE0079
#pragma warning disable S3011
var prop = typeToConvert
.GetProperties(BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Public)
.FirstOrDefault(x => x.Name.Equals(propertyName, StringComparison.OrdinalIgnoreCase));
#pragma warning restore
if (prop == null)
{
// Skip value
reader.Skip();
// Read past value
reader.Read();
continue;
}
var value = System.Text.Json.JsonSerializer.Deserialize(ref reader, prop.PropertyType, options);
if (value != null)
{
values[prop.Name] = value;
}
if (reader.TokenType is not JsonTokenType.PropertyName)
{
reader.Read();
}
}
}
private static void FillProperties(object instance, IReadOnlyDictionary<string, object> values)
{
if (values == null)
{
throw new ArgumentNullException(nameof(values));
}
if (instance == null)
{
throw new ArgumentNullException(nameof(instance));
}
foreach (var property in instance.GetType().GetProperties())
{
if (!values.TryGetValue(property.Name, out var value))
{
continue;
}
if (property.CanWrite)
{
property.SetPropertyValue(instance, value);
}
}
}
}
|
This would be a breaking change if we added it by default, but we plan to cover that particular use case using parameterized constructor in the contract model: #71944 |
How would it be a breaking change? If the [JsonConstructor] attribute is present, select that constructor. If it isn't, try to find one that works with available data. That will not break any backward compatibility. Or add it as an option in serializer settings. |
That is a very interesting request! Please consider it sooner :) |
I also need something like that. One of the properties in my model can be a string or an integer, so i have two constructors, one for each type. I cannot change it to a single type because I'm using an external API. |
Today, multiple constructors are only supported if one of them is decorated with the
[JsonConstructor]
attribute.The problem with that is that when you design an API, you use constructors to ensure that the developer has specified the correct combination of properties. Those combinations are different, and all properties might not be specified in each combination.
We also set those properties to get-only, just for convenience, so when you want to fill in optional properties, you do not have to see the mandatory ones. And by using private setters, the user cannot unset a mandatory parameter (intentional or by mistake).
That, in turn, makes it impossible to decorate one constructor with the
[JsonConstructor]
attribute since either of them can have been used before serializing, and choosing the wrong one will result in discarded information.To remedy this, I've written a converter that matches constructors against the information available in the JSON document. It then selects the first constructor that can be used with the available data. It's included below.
Note, I originally tried
JsonConverter<object>
to be able to add it once, but that didn't work well with System.Text.Json.Feature request: Add support for multiple constructors without the need for
[JsonConstructor]
.This is only one of my converters to get the same functionality as in Json.NET. It would help a lot if it were possible to create non-generic converters.
(this solution isn't very efficient, an alternative would be to support non-public default constructor + private setters)
The text was updated successfully, but these errors were encountered: