Skip to content
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

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

Closed
reinux opened this issue Jun 30, 2022 · 4 comments
Closed

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

reinux opened this issue Jun 30, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@reinux
Copy link

reinux commented Jun 30, 2022

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
Copy link
Owner

Tarmil commented Jul 4, 2022

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

@reinux
Copy link
Author

reinux commented Jul 7, 2022

Awesome, thanks! This will help a ton.

@SamuelBerger
Copy link

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:

  • serialize None / ValueNone properties
  • deserialize missing option / voption properties to None / ValueNone

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
Copy link
Owner

Tarmil commented Jan 21, 2023

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

@Tarmil Tarmil closed this as completed Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants