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

Deserializing null values to Option #87

Open
pnobre opened this issue Jun 7, 2020 · 3 comments
Open

Deserializing null values to Option #87

pnobre opened this issue Jun 7, 2020 · 3 comments

Comments

@pnobre
Copy link
Contributor

pnobre commented Jun 7, 2020

Trying to deserialize a null value into an option produces an obj expected value (json type mismatch). The behavior should be consistent whether the value is null or doesn't exist, but that's not the case. It correctly returns a None when the value is absent, and and error when the value is present, but set to null.

Repro

type Address = { 
    country: string }
with 
    static member OfJson json = 
        match json with 
        | JObject o -> monad { 
            let! country = o .@ "country"

            return { country = country }}
        | x -> Decode.Fail.objExpected x
        
type Customer = { 
    id: int 
    name: string
    address: Address option} 
with 
    static member OfJson json = 
        match json with 
        | JObject o -> monad { 
            let! id = o .@ "id"
            let! name = o .@ "name"
            let! address = o .@? "address"
            return { 
                id = id
                name = name
                address = address } }
        | x -> Decode.Fail.objExpected x

Succeds - Returns Some { id = 1; name = "Joe"; address = None}

let json = """{"id":1, "name": "Joe"}"""
let (customer:Customer ParseResult) = parseJson json

Fails - Should return Some { id = 1; name = "Joe"; address = None}

let json' = """{"id":1, "name": "Joe", "address": null}"""
let (customer':Customer ParseResult) = parseJson json 

a potential solution for this issue would be to modify jgetOptionWith function like:

let inline jgetOptWith ofJson (o: IReadOnlyDictionary<string, JsonValue>) key =
        match o.TryGetValue key with
        | true, JNull -> Success None
        | true, value -> ofJson value |> map Some
        | _ -> Success None

//edited to update the test cases and confirm the solution works for other libraries as well.

@pnobre
Copy link
Contributor Author

pnobre commented Jun 7, 2020

just an update, just tried aeson, and what I'm reporting here is aeson's behavior

{-# LANGUAGE OverloadedStrings #-}
import Control.Applicative
import Data.Aeson
import Data.Text (Text)
data Customer = Customer
                { id      :: Integer
                , name    :: String
                , address :: Maybe Address }
                deriving Show
data Address = Address
               { country :: String }
               deriving Show
instance FromJSON Address where 
    parseJSON (Object v) = Address <$> v .: "country"
    parseJSON _ = empty
instance FromJSON Customer where 
    parseJSON (Object v) = Customer <$> v .: "id" <*> v .: "name" <*> v .:? "address"
    parseJSON _ = empty
instance ToJSON Address where 
    toJSON (Address country) = object ["country" .= country]
instance ToJSON Customer where 
    toJSON (Customer id name address) = object ["id" .= id, "name" .= name, "address" .= address]

@pnobre
Copy link
Contributor Author

pnobre commented Jun 7, 2020

from what I see here, the problem is when some folks might actually want to get nulls back.

//edit
actually, giving a better though on this, there's no way someone would actually get nulls. .@? will return a None if the value isn't present, but it'll still fail for most cases (unless there's a particular OfJson implementation) when the json value is null.

@gusty
Copy link
Member

gusty commented Jun 18, 2022

Note that as from v 0.10.0 jopt and friends, support Nullables, ValueOption and anything that has the zero value (even lists).

Not sure if this solves your specific problem, I think if they supply nulls and the field is Nullable it should work now. Otherwise please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants