Tarmil / FSharp.SystemTextJson

System.Text.Json extensions for F# types
MIT License
323 stars 44 forks source link

String can be null on serialization but throws on deserialization #144

Open jannikbuschke opened 1 year ago

jannikbuschke commented 1 year ago
    type A = { Value: string }

    let serialized = serializer.Serialize { Value = null }     // works => { "Value": null }
    let desieralized = serializer.Deserializ<A> ("""{"Value":null}""")     // throws

It seems this is by design and I am currently investigating why this is so. In general I think it would be nice to prevent all null assignments to strings, as the more proper way to implement a missing value would be string option.

Am I right that these defaults from the System.Text.Json https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to?pivots=dotnet-5-0#handle-null-values

By default, the serializer handles null values as follows: For reference types and Nullable types:

  • It does not pass null to custom converters on serialization. ...

make the converters from this library never see null values for string and therefore do not fail, or does this lib totally ignores strings?

Tarmil commented 1 year ago

When serializing a field type that it doesn't explicitly handle, this library just hands it over to System.Text.Json. So that's indeed why null strings are serialized as null.

The deserializer is an input layer for the application, so it does data validation. Whereas the serializer is an output layer, it doesn't make much sense to do validation at this point.

jannikbuschke commented 1 year ago

Interesting. I'm using your lib to store data in a postgresql database (https://jannikbuschke.de/blog/fsharp-marten if you are interested). So the data stays internal to my application and I need to make sure serialization and deserialization works bidirectionally.

My current solution is to add a custom converter for strings to prevent null strings to be serialized

type NullStringConverter() =
  inherit JsonConverter<string>()
  with
    override this.HandleNull = true
    override this.Read(reader, typeToConvert, options) = reader.GetString ()
    override this.Write(writer, value, options) =
      if value = null then
        raise (NotSupportedException ("Value null is not allowed for type String."))
      else
        writer.WriteStringValue value

Do you think there is a better approach and/or should this be documented? It doesn't seem that someone else stumbled upon this, but it could bite people in the future.

Tarmil commented 1 year ago

I don't think there's anything better to do if you want to handle this within the serialization layer.