Runtime: Implement a concept of "Required" properties

Created on 12 Jun 2019  Â·  10Comments  Â·  Source: dotnet/runtime

Current situation

There's no way to indicate that if an object doesn't have a particular property that serialization should fail.

For example:
```c#
public class Person
{
[Required]
public string Name { get; set; }
public int Age { get; set; }
}

```json
{
    "Age": 46
}

Trying to deserialize the above json as a Person will succeed, even though it's been indicated that the name attribute is required, and is missing.

Describe the solution you'd like

System.Text.Json should respect either System.ComponentModel.DataAnnotations.RequiredAttribute or a new attribute (likely extending System.Text.Json.JsonAttribute) and error when an attempt is made to serialize an object which doesn't have that property.

Describe alternatives you've considered

Using System.ComponentModel.DataAnnotations.RequiredAttribute might be troublesome because it could indicate Requirement in a different context. The alternative is that users implement their own "Required" functionality (and likely get it wrong or incomplete).

CC @pranavkm who I talked about this with.

area-System.Text.Json enhancement json-functionality-doc

Most helpful comment

Virtually all my model's properties have Newtonsoft Json's […, JsonProperty(Required = Required.Always)], which that input formatter honors. This is blocking my System.Text.Json adoption.

All 10 comments

.... if Name is a non-nullable reference type, wouldn't that indicate that it was required?

So:

public class Person
{
    public string Name { get; set; }
    public int Age { get; set; }
}

would fail the deserialize, but:

public class Person
{
    public string? Name { get; set; }
    public int Age { get; set; }
}

... would let it succeed.

If we implement Required we'd also want to implement all the other validation attributes. I'm not sure that System.Text.Json is the correct place to do this.

Having another look at https://github.com/dotnet/corefx/issues/37485, it looks like there's already another issue that is effectively asking for the same thing. Perhaps we should just close this and use the other issue to track work here?

I believe we should have the support for Required (similar to how we have the JsonIgnore attribute), but this will likely be something we would address in the future.

This issue talks more directly to the issue at hand, so I would prefer keeping this one as a feature request.

cc @steveharter

From @pranavkm (https://github.com/dotnet/corefx/issues/37485#issuecomment-501482970):

@JamesNK, did you mean JsonPropertyRequired

MVC users have often asked for a way to identify when a property isn't on the wire. While @steveharter's solutions are acceptable for the trivial cases, it really doesn't scale well if you want the behavior to apply in general. Plus, you're forced to manually perform the checks which seems cumbersome.

Could we consider introducing an attribute like Json.NET to enforce this behavior?

Required was discussed but didn't meet the bar for 3.0 in lieu of other features. A workaround is to create a custom converter (pending feature) and override the Read method.

Required was discussed but didn't meet the bar for 3.0 in lieu of other features

Fair enough. I think we're ok with that for this release. We can keep this issue around to track the feature work for the Future.

Virtually all my model's properties have Newtonsoft Json's […, JsonProperty(Required = Required.Always)], which that input formatter honors. This is blocking my System.Text.Json adoption.

.... if Name is a non-nullable reference type, wouldn't that indicate that it was required?

So:

public class Person
{
    public string Name { get; set; }
    public int Age { get; set; }
}

would fail the deserialize, but:

public class Person
{
    public string? Name { get; set; }
    public int Age { get; set; }
}

... would let it succeed.

I agree such behaviour would be very desirable to ensure compiler checks with nullable reference types are not undermined at runtime by incorrect JSON (defeating the purpose of "nullable enable"). However this is perhaps best handled via the separate issue https://github.com/dotnet/runtime/issues/1256.

A Required attribute would at least allow a reasonable manual workaround for this in the meantime though.

NB: I have sadly decided to roll back my conversion to System.Text.Json since as now using nullable reference types I don't want to go back to adding lots of null run-time checks.

@steveharter

Required was discussed but didn't meet the bar for 3.0 in lieu of other features. A workaround is to create a custom converter (pending feature) and override the Read method.

I see that this issue has been considered a nice to have, but I would like to point out that the workaround of creating a custom converter isn't an extensible one, as you would either need to create a custom converter for every class with required properties, or create a generic one that allows you to pass the names of all the required fields to the converter, while also handling the specifics of actually converting the values.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jzabroski picture jzabroski  Â·  3Comments

matty-hall picture matty-hall  Â·  3Comments

noahfalk picture noahfalk  Â·  3Comments

yahorsi picture yahorsi  Â·  3Comments

chunseoklee picture chunseoklee  Â·  3Comments