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

Allow accepting missing fields of other types than Maybe #646

Closed
lortabac opened this issue Jun 12, 2018 · 22 comments
Closed

Allow accepting missing fields of other types than Maybe #646

lortabac opened this issue Jun 12, 2018 · 22 comments

Comments

@lortabac
Copy link

When deriving FromJSON instances generically, there is no way to provide a default value to a missing field of a record if the field is not a Maybe.

This seems related to #614, in that the proposed solution may help fixing both issues.

@tolysz
Copy link
Contributor

tolysz commented Jun 28, 2018

I would say it is related to: #323
To make aeson full flexible we need 2 extra contructors: one for missing, one for rawNumber.

@lortabac
Copy link
Author

@tolysz Thanks. It seems that your proposal could fix this issue. However I am not sure it is a good idea to modify Value.

Since the problem is specific to records, what I would like to have is some way to specify whether a record field is allowed to have a default type or not (and what the default value should be) according to its type.

@tolysz
Copy link
Contributor

tolysz commented Jun 29, 2018

I can only say it is a good idea ;) It is transparent to virtually all applications.

I am porting my old patches to the new version of aeson and it will just work (The generic is almost done(example work), the TH is a bit pain, but it will be done soon).
Adding the extra constructors does not change the output, just allow to work with JSON more easily.

@lortabac
Copy link
Author

lortabac commented Jun 29, 2018

@tolysz It works, but I am not sure it is the most appropriate solution. Since this issue only concerns records, I would prefer a solution that only concerns records.

If you add new constructors to Value you end up with values that do not fit into any JSON type. For ex. how would you encode Missing in isolation? Or in a list? Would it make sense?

@Lysxia
Copy link
Collaborator

Lysxia commented Jun 29, 2018

genericToJSON/genericParseJSON could also take a list of field encoders/decoders to customize the behavior on various fields (referred to by name or by type). I have an example of this approach to generic deriving in generic-random (customizing random generators instead of JSON parsers). It is technically a breaking change (aeson's generic type classes would need to carry an additional parameter) but seems feasible without breaking too much in practice.

@tolysz
Copy link
Contributor

tolysz commented Jun 29, 2018

@lortabac I prefer a situation where I can encode all possible scenarios, rather than a limited subset, even if some would be invalid (a stricter validation would remove those invalid cases).
I would encode Missing as mempty or null or error "you should never encode it" (but I love robustness and hate errors, so I chose the first or the second. The 2nd produces a valid JSON ;) but you should never see it).

My proposed approach composes well and it is really useful, but I do get that people are afraid of changing the holly library. Good thing is that I do not see any other useful constructors ;) but the new one is essential if we go via the Value.
Direct encoding solvers problems with how we encode floating point numbers (as you can choose anything you want for your custom type ~ including an invalid string), but I like to fix the Value as well.
Missing is essential, signal the level up to remove the object key ~ there is no (negative) encoding.

Anyway, this will be coming back, let me update the old patches. You can look at them. Each time I improve something there.

@torgeirsh
Copy link

What's the latest news on this? Is it possible to derive FromJSON and have the instance allow Maybe fields to be missing, or do such instances have to be written manually?

@lortabac
Copy link
Author

lortabac commented Nov 8, 2019

@torgeirsh You can already do that with Generic instances. If a Maybe field is missing, it defaults to Nothing.

In this issue I propose to extend this behavior to other types than Maybe, which is currently not possible.

@torgeirsh
Copy link

@lortabac I can't get it working:

data MyType a = MyType {
        field1 :: Int,
        field2 :: a
} deriving (Show, Generic, FromJSON)
*Main> parseEither (parseJSON :: Value -> Parser (MyType (Maybe Int))) (Object $ fromList ["field1" .= 1])
Left "Error in $: parsing Main.MyType(MyType) failed, key \"field2\" not present"

Do you mean something else by "Generic instances"?

@Lysxia
Copy link
Collaborator

Lysxia commented Nov 8, 2019

@torgeirsh That's a somewhat different issue, see also #690 and the extra documentation added here. Don't hesitate to ask for further clarification though!

@torgeirsh
Copy link

@Lysxia Thanks, I can see that I have the FromJSON version of that issue.

@georgefst
Copy link

Any update here? I actually have a datatype where certain fields have a type isomorphic to (). I would like them to be populated with the same value, regardless of whether the field is there or not. (NB. in the presence of type families, this isn't as silly as it sounds.)

I've written a manual FromJSON instance, but it involves a lot of rather cumbersome boilerplate.

@bergmark
Copy link
Collaborator

Sorry, no work has been done on this as far as I know.

Seems like @Lysxia's suggestion is a promising approach:

genericToJSON/genericParseJSON could also take a list of field encoders/decoders to customize the behavior on various fields (referred to by name or by type). I have an example of this approach to generic deriving in generic-random (customizing random generators instead of JSON parsers). It is technically a breaking change (aeson's generic type classes would need to carry an additional parameter) but seems feasible without breaking too much in practice.

@anka-213
Copy link

anka-213 commented Feb 9, 2021

The workaround I've used is to transform the value with this function prior to parsing, so that it can work with any parser that can accept null (instead of just Maybe).

import qualified Data.HashMap.Strict as HM

addNullField :: Text -> Value -> Value
addNullField s (Object o) = Object $ HM.insertWith (\_new old -> old) s Null o
addNullField _ v = v

I would use it like this:

data Foo = Foo 
  { someField :: SomePossiblyNullType
  }  deriving (Show, Generic)

instance FromJSON Foo where
  parseJSON = genericParseJSON defaultOptions . addNullField "someField"

This also works for polymorphic types:

data Foo a = Foo 
  { someField :: a
  }  deriving (Show, Generic)

instance FromJSON a => FromJSON (Foo a) where
  parseJSON = genericParseJSON defaultOptions . addNullField "someField"

-- >>> decode "{}" :: Maybe (Foo (Maybe Int))
-- Just (Foo {someField = Nothing})

-- >>> decode "{\"someField\": 3}" :: Maybe (Foo (Maybe Int))
-- Just (Foo {someField = Just 3})

@anka-213
Copy link

Maybe we could add a new option called missingAsNull or something, which would interpret any missing field as null before sending it off to the next sub-parser instead of (as is currently the case) failing unless it is specifically Maybe applied uniformly?

@tolysz
Copy link
Contributor

tolysz commented Feb 18, 2021

@anka-213
Nice that people are warming up to the idea... https://github.com/tolysz/aeson/tree/Missing_1.4
my solution (.:??) and the addition of Possible type (Data, Null, Missing)...

@anka-213
Copy link

anka-213 commented Feb 18, 2021

@tolysz That was my first thought too, but I feel like that might be too much of a breaking change.
It also have the downside that there are two different representations of a missing value:

Object (fromList [("a",Missing)])

is now equivalent to

Object (fromList [])

My suggestion would be to add an option to the Option structure that regulates the behaviour of genericParseJSON so that it replaces a missing field with Null before sending it to the next parser. And also to add a corresponding operator (similar to .:?) for that behaviour.
This should be completely backwards compatible while still allowing for a fair amount of flexibility.

It could also resolve #690, since there would now be an option to treat null and missing fields equivalently for a data type.

@tolysz
Copy link
Contributor

tolysz commented Feb 18, 2021

Those changes are not breaking... you can compile the whole stack...

@anka-213
Copy link

I meant that if anyone is pattern matching on Value, it will complain about missing cases, but I guess that might be very rare in practice.

It also might not be sufficient to solve my specific problem, since it is a combination of this and #690.
The type in question has a type parameter which together with a type family which determines the specific values. If it is the custom type Empty, it should be either missing or null.

https://github.com/alanz/lsp/blob/005f828ec23f4968119d53286028203d95f0ea3b/lsp-types/src/Language/LSP/Types/Message.hs#L245-L250

@tolysz
Copy link
Contributor

tolysz commented Feb 18, 2021

I updated it to the latest aeson: https://github.com/tolysz/aeson/tree/Missing_1.5 (the same failing test)
If the aeson was fixed we easily use apis that support patch eg:
https://github.com/tolysz/google-api

This does exactly what you need... check the examples (but you need to replace Maybe with Possible in your example).

@tolysz
Copy link
Contributor

tolysz commented Feb 18, 2021

Object (fromList [("a",Missing)])

would look like {} but allowing Missing in the intermediary representation allows to compose types.

@phadej
Copy link
Collaborator

phadej commented Jun 19, 2023

Now there is. See #1023 and #1039.

@phadej phadej closed this as completed Jun 19, 2023
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

8 participants