Flowminder
API schema is invalid for fields.Nested with allow_none=True
In the flowmachine query schemas, there are several optional query parameters ("hours", "sampling" and "bounds", in several query schemas) which are validated by a nested schema (via marshmallow's fields.Nested), but whose values can be null (or default to None if not provided).
This is handled fine by the marshmallow schemas, but the API spec generated in these cases looks like
"SomeQuerySchema": {
"properties": {
"hours": {
"type": [
"null"
],
"allOf": [
{
"$ref": "#/components/schemas/Hours"
}
],
"default": null,
"nullable": true
}
},
"type": "object"
}
which looks fine, but actually says that the hours value still has to match the Hours schema, even if it's null (see discussion in https://github.com/OAI/OpenAPI-Specification/issues/1368) - and null is not a valid Hours object, so this API spec is not valid. It appears there's actually no way to express this marshmallow schema in an openapi 3.0 API spec, without defining a custom "null type" schema. We got away with this until recently, because openapi-schema-validator interpreted this spec as meaning "can be null OR Hours" rather than "can be null but must still be Hours", but as of v0.4.0, openapi-schema-validator has been fixed to interpret such a schema as per the discussion linked above, so upgrading to openapi-schema-validator>=0.4.0 breaks our integration tests.
In OpenAPI version 3.1, "nullable" was removed in favour of a new "type": "null". This means that our schema can be correctly expressed as
"SomeQuerySchema": {
"properties": {
"hours": {
"type": [
"null"
],
"anyOf": [
{
"$ref": "#/components/schemas/Hours"
},
{"type": "null"}
],
"default": null,
"nullable": true
}
},
"type": "object"
}
However, apispec currently doesn't handle this correctly (https://github.com/marshmallow-code/apispec/issues/833), so as things stand we'd still end up with a broken API spec if we bump the openapi version to 3.1.
It's not strictly necessary to handle optional parameters by allowing null input. We could just leave them optional without a default value, and handle missing parameters the way we currently handle null ones. I believe this would work fine in the API spec. The difficulty with this approach is that we explicitly add all parameters as attributes on the exposed query objects to aid serialisation, and we'd therefore have to add in extra logic for every optional parameter to skip adding it as an attribute if it's not provided. So if we make this change, it may be beneficial to re-work the query schema / exposed query setup at the same time.
https://github.com/marshmallow-code/apispec/issues/833 is now fixed in apispec v6.6.1, so we should now be able to bump the openapi version to 3.1 and unpin some FlowAPI dependencies.
