Fix schema for Maybe

[stevladimir]

The schema for Maybe is obviously wrong. I can guess it was done that way to be able to handle optional record fields, but looks like one can have both: correct schema for Maybe and optional fields.

Note, that this is built upon #90.
Target changes: 6f4a07a and forward

[stevladimir]

@maksbotan is any explicit "ping" required for review?
(Not that it is any rush - asking just in case of)

[maksbotan]

Hi @stevladimir! I've seen this one, but I need some time to check if it's OK.

I fear that something might break... Like optional fields.

[stevladimir]

Ah, ok... As I've said "No rush".
Regarding the rest...
If you have the idea what else should be added to test cases I'll do that.
AFAICS test suites already have optional fields and I've added/enabled some more tests.
Also I've run this across real world app with rich API having validateEveryToJSON test and see no issues (except for the need to remove custom instances for few newtype's over Maybe)

[maksbotan]

Maybe I'll see if this changes openapi json at our work project.

[stevladimir]

It should change, but in expected way. Schema for Maybe should change to, conceptually {one_of: [{type : null}, old_schema]}
Previously schema for Maybe a was the same as for a, what was actually incorrect.
If you have , field :: Maybe Int all of a) missing field, b) "field" : null and c) "field" : 5 are valid values from aeson point of view (though there are open PRs making it possible to distinguish missing and null fields, but currently they are indistinguishable). But currently b) is not valid according to schema.

[stevladimir]

For more context.

Master

ghci> data Foo = Foo { foo :: Int, bar :: Maybe Char } deriving (Generic)
ghci> instance ToSchema Foo
ghci> Data.ByteString.Lazy.Char8.putStrLn $ encode $ toSchema $ Proxy @Foo
{"properties":{"bar":{"example":"?","maxLength":1,"minLength":1,"type":"string"},"foo":{"maximum":9223372036854775807,"minimum":-9223372036854775808,"type":"integer"}},"required":["foo"],"type":"object"}

This PR

ghci> data Foo = Foo { foo :: Int, bar :: Maybe Char } deriving (Generic)
ghci> instance ToSchema Foo
ghci> Data.ByteString.Lazy.Char8.putStrLn $ encode $ toSchema $ Proxy @Foo
{"properties":{"bar":{"anyOf":[{"type":"null"},{"example":"?","maxLength":1,"minLength":1,"type":"string"}]},"foo":{"maximum":9223372036854775807,"minimum":-9223372036854775808,"type":"integer"}},"required":["foo"],"type":"object"}

[maksbotan]

How does it work for omitNothingFields = False?

[stevladimir]

Per documentation

Note that this does not affect parsing: Maybe fields are optional regardless of the value of omitNothingFields. allowOmittedFieds controls parsing behavior.

So this option does not affect parsing. So far so good...
However, allowOmittedFields option and overall machinery for distincting missing and null fields has been added in the very last version released recently (surprise for me). So it will again be incoherent this or that way.
FYI I have almost finished PR supporting aeson options currently unsupported in openapi3, but it was not dealing with omitNothingFields for aforementioned reason. Now seems I need to address that too

[stevladimir]

@maksbotan friendly ping

[zlondrej]

Note that in OpenAPI 3.0.*, type: null is not a standard.

https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.0.md#data-types:~:text=null%20is%20not%20supported%20as%20a%20type%20(see%20nullable%20for%20an%20alternative%20solution).

It's only since 3.1.0 specification which is compatible JSON Schema, that you can use type: null, for prior versions a property nullable: true should be used.

[stevladimir]

Indeed. It should be feasible to rework this using nullable.
I see some things from 3.1.0 like OpenApiItemsArray are already here. And the question about migration to latest OAS version is on board: #104. So what to use depends on the answer on that question.
I'm personally in favor of the latest version.
But the decision is up to @maksbotan.