Null value option roundtrip

[mausch]

Roundtripping a string option doesn't work. Problem is, serializing Some null outputs null which then gets deserialized as None. This problem can be generalized to all reference types within an option.

This doesn't happen in Haskell/Aeson because strings are not nullable.

[mausch]

One solution would be changing the option representation: instead of using null for None, which causes the ambiguity, use "None". And probably Some x should be also represented in JSON instead of being "transparent" as in Aeson.

However this would probably be annoying for other consumers.

[dtchepak]

Example:

open FsCheck.Xunit
open Fleece
open Fleece.Operators
open FSharpx.Choice
open Xunit
open Swensen.Unquote


type Foo = { maybeStr : string option }
    with
    static member Create a = {maybeStr = a}
    static member ToJSON (x : Foo) =
        jobj [ "maybeStr" .= x.maybeStr ]
    static member FromJSON (_ : Foo) =
        function 
        | JObject o -> Foo.Create <!> o.@?"maybeStr"
        | x -> sprintf "Foo %A" x |> Choice2Of2

[<Fact>]
let ``none to json`` () =
    let foo = {maybeStr = None}
    let x = toJSON foo  |> string
    test <@ x = """{"maybeStr":null}""" @>
   (* PASSED *)

[<Fact>]
let ``null in json to none`` () =
    let src = """{"maybeStr":null}"""
    let x = System.Json.JsonValue.Parse(src);
    let foo : Choice<Foo,string> = fromJSON x
    test <@ foo = Choice1Of2 { maybeStr = None } @>
    (* FAILED
Choice1Of2 {maybeStr = Some null;} = Choice1Of2(NewRecord (Foo, NewUnionCase (None)))
Choice1Of2 {maybeStr = Some null;} = Choice1Of2({maybeStr = null;})
false
    *)

See also: https://gist.github.com/dtchepak/cf498a1f44b59a6a4dce

[mausch]

Actually there's no need to wrap the string option in a separate type to reproduce this. Simply adding yield testProperty "string option" (roundtrip<string option>) to https://github.com/mausch/Fleece/blob/master/Tests/Tests.fs#L207 fails.

[dtchepak]

Not sure if it makes a difference, but our actual case is more like this:

type Bar = A | B
    with
    static member ToJSON (x : Bar) =
        match x with | A -> JString "A" | B -> JString "B"
    static member FromJSON (_ : Bar) =
        function 
        | JString "A" -> Choice1Of2 A
        | JString "B" -> Choice1Of2 B
        | _ -> Choice2Of2 "could not parse Bar"

type Foo = { bar : Bar option }
    with
    static member Create a = {bar = a}
    static member ToJSON (x : Foo) =
        jobj [ "bar" .= x.bar ]
    static member FromJSON (_ : Foo) =
        function 
        | JObject o -> Foo.Create <!> o.@?"bar"
        | x -> sprintf "Foo %A" x |> Choice2Of2

[mausch]

The last case you mention is slightly different: when using .@?, None corresponds to a missing field and Some x corresponds to a present field with value x.

It can't roundtrip like that because of its semantics.

If you change that to .@ it will roundtrip.

[mausch]

BTW you might want to use Success / Failure instead of Choice1Of2 / Choice2Of2: easier to type and easier to tell from each other when reading it.

[ScottSedgwick]

I've been working with Dave, we figured out a possible solution.

We created these functions:

let inline (.=?) key value = 
    let nullstr : string = null
    match value with
    | Some(v) -> jpair key value
    | None    -> jpair nullstr nullstr

let filteredJobj pairs = pairs |> Seq.filter (fun (k,_) -> k <> null) |> jobj

Then we create objects with optional elements like this:

static member ToJSON (x : AClass) =
      [
        "MandatoryValue" .=  x.mandatoryValue
        "OptionalValue"  .=? x.optionalValue
      ] |> filteredJobj

Dave wondered if the jobj function could be modified to filter out pairs with null keys?

[mausch]

Yes, I think that makes sense. It's either that or throwing an exception. Implemented that change in b8964b019f185c8ed3932f9c9dac837a07522423 .

As for optional serialization elements, personally I'd rather be explicit about it, even if it's a bit more verbose, e.g.:

static member ToJSON (x : AClass) =
  jobj [
    yield "MandatoryValue" .= x.mandatoryValue
    if x.optionalValue <> null then
      yield "OptionalValue"  .= x.optionalValue
  ]

Also, none of this seems to be related to the original description of this issue...

[gusty]

This problem can be generalized to all reference types within an option. This doesn't happen in Haskell/Aeson because strings are not nullable.

@mausch I think the decision of not encoding the "Someness" of an option cause not only this issue with reference types but with any other option-like nesting, including nesting with option itself:

#r "nuget: Fleece.NewtonsoftJson"

open FSharpPlus
open Fleece.Newtonsoft
open Fleece.Newtonsoft.Operators

let x: int option option = (Some None : int option option) |> toJson |> ofJson |> Result.get ;;

gives val x: int option option = None

and ~probably~ Aeson has the same problem ~if~ as they took the same road https://github.com/haskell/aeson/issues/376

[omcnoe]

Recently was bitten by this issue, luckily was caught in testing.

Fundamental issue is that there are more values for a type like int option option than can be fit in a single encoded value. So any fix to the issue will have to compromise somewhere: more verbose encoding, or losing information.

Would it be possible to special case the encoder/decoder for a Some outer option to include the case in the encoded form? eg. Some (Some 7) -> "Some 7" Some None -> "Some null" None -> "null"

Going fully verbose with option case names in all cases is probably a pain for many other consumers of the data, but if you choose to use complex nested optional types then you can deal with the verbosity?