[isaac-uptrust]

The Generic-derived schema for polymorphic datatypes is incorrect. The schema for a datatype likedata X a = X{ field :: a } has field as a required property. This means that the schema for X (Maybe something) also has field marked as a required field. The optionality of the Maybe is forgotten. A service relying on the erroneous schema would expect a non-nullable value, but a service sending X (Maybe something) values can send null in the field propety, but a service relying on the erroneous schema will expect no null in field, leading to a runtime error.

Given data X a = X{ field :: a }, the Generic-derived schema must mark field as required, because it needs to work for all a. Therefore, instance ToSchema a => ToSchema (Maybe a) has to generate a nullable schema to be correct. Then the schema for X (Maybe something) will have required property field with a nullable schema for something.

This change changes the ToSchema instance for Maybe a. The schema for Maybe a is prefixed with Maybe_ to distinguish it from the schema for a. Without this, declareSchemaRef will generate the same reference for an a or a Maybe a. When an API uses an a and a Maybe a, only one of the two schemas will make it into the components.schemas section (because of the name conflict). This can lead to a schema that says it produces non-nullable values, but may actually produce nulls.

This problem could also be solved by changing declareSchemaRef to strip the nullable property from the declared schema and return the nullability value alongside the Referenced Schema. This would increase sharing (you wouldn't have near duplicate schemas X and Maybe_X), and I think it would be cleaner, but at the cost of forcing downstream libraries (such as servant-openapi3) to explicitly handle that nullability information. I chose the solution that's compatible with current downstream code.

I think this supercedes the previous attempts at fixing Maybe:

• Fix schema for Maybe #91
• [Duplicate] Add null to Maybe a schemas #103

[chris-martin]

Clever use of overlapping instances. Love it 🚀

[stevladimir]

AFAIR nullable property is deprecated. It is absent in the latest OAS versions starting from 3.1.0 I think.
There is some uncertainty about which OpenAPI version this library actually targets. If it is 3.0 then nullable is the right way, however, it seems that it is moving towards latest versions (see #91 (comment))

[vcombey]

any plan to fix this ?

[ysangkok]

@vcombey the maintainer is only minimally available, about once a year. I have been trying to get somebody (like e.g. Rick Owens or Simon Michael) to fork this library and make improvements, I think it should be pretty easy to make something significantly better. You could say I should do it myself, but our issues haven't been severe enough.

[BrianRS]

@stevladimir the library only accepts spec versions 3.0.0 to 3.0.3 (lowerOpenApiSpecVersion / upperOpenApiSpecVersion in Data.OpenApi.Internal), and in 3.0 nullable is how a schema says a value may be null. Moving to 3.1's type: [..., "null"] would touch every schema the library emits, so it seems better as its own change.

@maksbotan I've merged master to resolve the conflicts; the test suite and doctests pass on GHC 9.10.3. Could you review this when you have a moment?