Tarmil / FSharp.SystemTextJson

System.Text.Json extensions for F# types
MIT License
329 stars 43 forks source link

How to make an option field be required for deserializing? #123

Closed reinux closed 1 year ago

reinux commented 2 years ago

Is there a way to have an option field be required?

For example, if LastName here is null, it should be set to None, but if it's missing in the json, it should throw an exception.

Reason being that I want to make sure an API consumer isn't accidentally missing a field.

I'm assuming this is why we would use Skippable with IgnoreNullValues, but the latter's been deprecated, and the new one it suggests seems to have no effect.

[<CLIMutable>]
type Person = {
  FirstName: string
  [<System.ComponentModel.DataAnnotations.Required>]
  LastName: string option // want it to throw an error if this is missing
  age: int
}

let fsConverter =
  JsonFSharpConverter(
    JsonUnionEncoding.UnwrapSingleCaseUnions
    ||| JsonUnionEncoding.UnwrapOption
    ||| JsonUnionEncoding.NamedFields
    ||| JsonUnionEncoding.UnwrapFieldlessTags)

let options = JsonSerializerOptions()
//options.DefaultIgnoreCondition <- JsonIgnoreCondition.Never // doesn't seem to have an effect for this particular scenario
options.PropertyNamingPolicy <- null
options.Converters.Add(fsConverter)
options.Converters.Add(JsonStringEnumConverter())

let x = JsonSerializer.Deserialize<Person>("""{"FirstName": "yarr", "age": 5 }""", options)
Tarmil commented 2 years ago

Right, we need to support DefaultIgnoreCondition.WhenWritingNull as an alternative to the deprecated IgnoreNullValues.

reinux commented 2 years ago

Awesome, thanks! This will help a ton.

SamuelBerger commented 2 years ago

While updating to release version, our CI hits the breaking change :( As it seems to me, it is not possible the configure the previous behaviour:

IgnoreNullValues = true or DefaultIgnoreCondition = WhenWritingNull affects serializing as well.

Am I missing a trick (without changing all option properties to Skippable)? Throwing an exception for missing non option record properties is pure gold, so allowNullFields would never be an option. I think a configuration optionFieldsSkippable would restore the previous behaviour. Would that be considered an improvement to the library?

Tarmil commented 1 year ago

The new option SkippableOptionFields was added in v1.1 for this use case. https://github.com/Tarmil/FSharp.SystemTextJson/blob/master/docs/Customizing.md#skippable-option-fields