Newtonsoft.Json icon indicating copy to clipboard operation
Newtonsoft.Json copied to clipboard

JsonRequiredAttribute populating object throws error even if field has a value

Open BigBadE opened this issue 2 years ago • 1 comments

Expected behavior

If the field does have a value, even if nothing is serialized, it shouldn't throw an exception. If this isn't wanted for whatever reason, the documentation should be updated to match. I'd personally like to be able to populate an object with JSON values to be a feature and not break in weird edge-cases like this, for example I've made a template system using JSON to reduce copy-pasting and this breaks it.

Actual behavior

When populating an object with a field that has the JsonRequired attribute with a value in it, an exception will be thrown even though the documentation says exactly: "Instructs the JsonSerializer to always serialize the member, and to require that the member has a value".

Steps to reproduce

using System;
using Newtonsoft.Json;

Console.Write(JsonConvert.DeserializeObject<Testing>("{}").testValue);

class Testing
{
    [JsonRequired] public float testValue = 2f;
}

BigBadE avatar Sep 09 '22 15:09 BigBadE

Hmm, what should happen in a case like this for example?

class Testing
{
    [JsonRequired] public float testValue = 0f;
}

Clearly, a value has been explicitly set on the field -- you can say the developer intentionally initialized the field with a value. But here it gets ambiguous: Every field has always a value. There are no fields without a value. If a field is not explicitly initialized with some assigned value, the field value will be the default value of the field's type. Basically, my example above is practically equivalent to:

class Testing
{
    [JsonRequired] public float testValue;
}

So, what now? I agree 100% with what you say about the documentation. But i don't see how the behavior you expect could be generally realized in a way that does not lead to confusion with respect to the example case i laid out here. This example here is precisely what would really be called a weird edge-case.

The current behavior, albeit poorly or incorrectly described in the documentation, makes sense. The attribute's name is descriptive with respect to the current behavior: the json is required to have the property, a behavior (or rule, if you will) that is unambiguous and straightforward. In that sense, there are no weird edge cases i know of with the current behavior. It's just that the documentation sucks, sometimes...

elgonzo avatar Sep 11 '22 15:09 elgonzo

Hmm, what should happen in a case like this for example?

class Testing
{
    [JsonRequired] public float testValue = 0f;
}

Maybe just a simple check if it's not equal to the default value? Or even a way to specify in the field that it just must not be the default value that's disabled by default?

I don't know, but it would be nice to have in some form, even if most projects don't use it.

BigBadE avatar Sep 27 '22 19:09 BigBadE

Maybe just a simple check if it's not equal to the default value? Or even a way to specify in the field that it just must not be the default value that's disabled by default?

Now we are piling on rules on top of rules and make everything even more complicated. Uh...

I don't know, but it would be nice to have in some form, even if most projects don't use it.

If most projects don't use it, then how could it be justified for the author to spend effort on writing, testing, and maintaining such a feature?

it would be nice to have in some form

You can achieve the desired behavior with a custom ContractResolver or a custom JsonConverter. Yes, writing such a custom ContractResolver or JsonConverter is probably not a work item you want to have on your plate, but at least the library provides you with such facility to customize the (de)serialization process to your desires...

elgonzo avatar Sep 27 '22 20:09 elgonzo