DefaultValueHandling.Populate doesn't set default value when deserializing

[ychuzevi]

While migrating from JSON.Net 6 to latest version (9.0.1), I noticed a regression regarding how DefaultValueHandling.Populate & DefaultValueHandling.IgnoreAndPopulate are handled during deserialization. With latest version, the Json property is initialized to null instead of its default value after deserialization.

Here is a simple test to reproduce the issue:

        private class MyTestClass
        {
            public const string DefaultText = "...";

            [DefaultValue(DefaultText)]
            [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
            public readonly string Text;

            public MyTestClass(string text = DefaultText)
            {
                Text = text;
            }
        }

        [Test]
        public void DumbTest()
        {
            MyTestClass myObject = JsonConvert.DeserializeObject<MyTestClass>("{}"); // Fail with version 9.0.1
            Assert.AreEqual(MyTestClass.DefaultText, myObject.Text);
        }

I tried with an int property and the default value was also ignored.

[ychuzevi]

It seems the issue is that if the JsonConverter finds a parameter's name in the constructor which matches one of the property, then it will consider it initialized by the constructor and will not apply the property's "default value" rules.

For example if I rename either MyTestClass's constructor "test" parameter or MyTestClass "Text" property then it will work fine.

I don't want to remove the constructor because I need to create the object from another source than JSON. And renaming the constructor's argument doesn't seem like a clean solution.

Do you confirm is it a bug? Is there some better way around?

[dbc2]

There's some discussion of the issue on stackoverflow, here.

It appears that JsonSerializerInternalReader.CreateObjectUsingCreatorWithParameters() now checks for the presence of the [DefaultValue(DefaultText)] and [JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate)] attributes on the constructor parameter rather than the corresponding property. This check is made around line 1979:

if (HasFlag(constructorProperty.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate))
{
    context.Value = EnsureType(
        reader,
        constructorProperty.GetResolvedDefaultValue(),
        CultureInfo.InvariantCulture,
        constructorProperty.PropertyContract,
        constructorProperty.PropertyType);
}

The line context.Value = EnsureType(...) is where the default value gets applied, and as you can see it's generated using constructorProperty rather than context.Property.GetResolvedDefaultValue() and context.Property.DefaultValueHandling.

Thus a workaround is to mark the constructor parameters with the necessary attributes:

class MyTestClass
{
    public const string DefaultText = "...";

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public readonly string Text;

    public MyTestClass([JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate), DefaultValue(DefaultText)] string text = DefaultText)
    {
        Text = text;
    }
}

It's definitely a breaking change from 6.0.8 as the constructor attributes are not required in that version to get the default value properly initialized. Not sure if the change is intentional or a bug.

[dbc2]

For comparison, in 6.0.8 there was a method JsonSerializerInternalReader.EndObject() that would, after deserialization, set properties not set from JSON to default values:

                                if (resolvedRequired == Required.AllowNull || resolvedRequired == Required.Always)
                                    throw JsonSerializationException.Create(reader, "Required property '{0}' not found in JSON.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName));

                                if (property.PropertyContract == null)
                                    property.PropertyContract = GetContractSafe(property.PropertyType);

                                if (HasFlag(property.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate) && property.Writable && !property.Ignored)
                                    property.ValueProvider.SetValue(newObject, EnsureType(reader, property.GetResolvedDefaultValue(), CultureInfo.InvariantCulture, property.PropertyContract, property.PropertyType));
                                break;

This is what causes the public readonly string Text; field in the issue to be correctly initialized; even in 6.0.8 the correct default value is not passed to the constructor. If in 6.0.8 I make Text be a get-only property rather than a read-only field or property with a private setter, then Text is not correctly initialized in that version also, since a property without any setter at all cannot be set even by reflection:

class MyTestClass
{
    public const string DefaultText = "...";

    readonly string _text;

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public string Text
    {
        get
        {
            return _text;
        }
    }

    MyTestClass()
        : this(DefaultText)
    {
    }

    [JsonConstructor]
    public MyTestClass(string text /*= DefaultText*/)
    {
        Console.WriteLine("{0}: Constructor called with text = \"{1}\"", GetType(), text);
        _text = text;
    }
}

[ychuzevi]

Thanks for the feedback! This definitely looks like a bug to me even if there was some loopholes in the JSon.Net 6.0.8 version.

I'm not so keen to duplicate the default value attributes in the constructor parameters because it won't be easy to maintain and read.

Do you see any issue with the idea to rename the constructor parameters to avoid the match?

It seems like a possible fix could be to look into the property attributes before the constructor attributes when initializing default values then?

[dbc2]

I don't recommend renaming the constructor argument. Another developer might come along and rename it back, and break serialization. Instead you could introduce a private parameterless constructor like so:

class MyTestClass
{
    public const string DefaultText = "...";

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public readonly string Text;

    [JsonConstructor]
    MyTestClass()
        : this(DefaultText)
    {
    }

    public MyTestClass(string text = DefaultText)
    {
        Text = text;
    }
}

Json.NET will now call the private parameterless constructor, then correctly set the value for the readonly field Text through reflection, since it has been marked with [JsonProperty]. (Or even make the parameterless constructor public and make the parameter to the second constructor non-optional.)

[ychuzevi]

Yes, I do agree it looks like the best solution with current implementation.

I tested it and it works fine although some comments are also required to make sure someone will not come and remove the "useless" constructor

[JamesNK]

That is intended behavior. The constructor param name is text and the property name is myText. Place an attribute on the param to change its name so the two are linked up.

private class MyTestClass
{
    public const string DefaultText = "...";

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public readonly string Text;

    public MyTestClass([JsonProperty(PropertyName = "myText")]string text = DefaultText)
    {
        Text = text;
    }
}

[ychuzevi]

Hi JamesNK,

Thanks for the update but I'm a bit confused. Indeed the solution you are proposing is working. But it is also working if I choose a random name for my constructor argument :

private class MyTestClass
{
    public const string DefaultText = "...";

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public readonly string Text;

    public MyTestClass(string makeSureIDontMatchAProperty = DefaultText)
    {
        Text = makeSureIDontMatchAProperty;
    }
}

The serializer is matching the constructor's parameter to the property when its name ("text") matches the name of the C# property ("Text"). And indeed in the implementation it will set the flag "context.used" to true. But this match is only partial and it will not trigger the application of the property's attribute unless I define myself the link or use the exact name used for the JSON property.

If this is the intended behavior, wouldn't it better to match constructor's argument name only with its JSON property name? I.e. accept only below example if there no specific attribute on the constructor argument to match a property :

private class MyTestClass
{
    public const string DefaultText = "...";

    [DefaultValue(DefaultText)]
    [JsonProperty(PropertyName = "myText", DefaultValueHandling = DefaultValueHandling.Populate)]
    public readonly string Text;

    public MyTestClass(string myText= DefaultText)
    {
        Text = myText;
    }
}