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

Boolean JSON Schemas are not supported #4

Closed
jcalero opened this issue Sep 20, 2021 · 4 comments · Fixed by #7
Closed

Boolean JSON Schemas are not supported #4

jcalero opened this issue Sep 20, 2021 · 4 comments · Fixed by #7

Comments

@jcalero
Copy link

jcalero commented Sep 20, 2021

See https://json-schema.org/draft/2020-12/json-schema-core.html#rfc.section.4.3.2 on the 2020-12 Draft, the values false and true are valid JSON Schemas.

Unless I'm mistaken, I think the Schema model (in v3.1.0, not sure if this applies to v3.0.1) should support bool as a type in all instances where Schema is allowed. E.g. change Union[Reference, "Schema"] to Union[Reference, "Schema", bool].

Without the support of boolean schemas, using Pydantic classes as schemas fails if the Pydantic schema is configured with extra = False as this sets "additionalProperties": false on the schema, but your model is only accepting Union[Reference, "Schema"].

@kuimono
Copy link
Owner

kuimono commented Sep 20, 2021

According to https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#schema-object, although it claims to follow the 2020-12 draft, the examples much assume that the Schema is an object, instead of a pure bool value.

Since the true / false value can easily be replaced by {} / {"not": {}} (as suggested in https://json-schema.org/draft/2020-12/json-schema-core.html#rfc.section.4.3.2), I would prefer NOT to allow bool type along with Schema, until the major adoptions of OpenAPI v3.1.0 reveal it to be the common usage.

This issue should have no relation to extra = Extra.forbid setting - it prohibits the appearance of extra fields, not the field types.

@jcalero
Copy link
Author

jcalero commented Sep 20, 2021

Thanks for the quick response!

According to https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#schema-object, although it claims to follow the 2020-12 draft, the examples much assume that the Schema is an object, instead of a pure bool value.

Not sure I'm reading this correctly but to me it sounds like the OpenAPI spec is referring to a schema as "objects, but also primitives and arrays", so in this case also booleans? The examples don't show this, but it's not invalid to the spec as far as I understand.

This issue should have no relation to extra = Extra.forbid setting - it prohibits the appearance of extra fields, not the field types.

The issue here is how Pydantic handles that extra = Extra.forbid field when generating a JSON Schema. Given that this library is meant to support Pydantic I thought it worth mentioning as I would qualify it as a bug. I'll clarify:

Whenever you generate a schema out of a Pydantic model in the construct_open_api_with_schema_class method, you call the schema function from Pydantic, this function will actually look at the extra = Extra.forbid value of the model and translate it to "additionalProperties": false in the resulting schema. (See these lines in Pydantic source code: https://github.com/samuelcolvin/pydantic/blob/257908628f8bd29910744c632e847fde332d35a9/pydantic/schema.py#L635-L636)

As you then try to create openapi_schema_pydantic.Schema models out of that generated schema with Schema.parse_obj(schema_dict), this will fail validation since you expect "additionalProperties" to be an object.

Here's a code snippet that would reproduce this issue:

from pydantic import BaseModel, Extra

class PyModel(BaseModel):
    some_field: str
    
    class Config:
        extra = Extra.forbid

from pydantic.schema import schema

defs = schema([PyModel])

from openapi_schema_pydantic import Schema

Schema.parse_obj(defs["definitions"]["PyModel"])

The last line will throw the following error:

ValidationError                           Traceback (most recent call last)
<ipython-input-22-a2cc7229ace8> in <module>
----> 1 Schema.parse_obj(defs["definitions"]["PyModel"])

~/.venv/r/lib/python3.8/site-packages/pydantic/main.cpython-38-x86_64-linux-gnu.so in pydantic.main.BaseModel.parse_obj()

~/.venv/r/lib/python3.8/site-packages/pydantic/main.cpython-38-x86_64-linux-gnu.so in pydantic.main.BaseModel.__init__()

ValidationError: 2 validation errors for Schema
additionalProperties
  value is not a valid dict (type=type_error.dict)
additionalProperties
  value is not a valid dict (type=type_error.dict)

If you look at the schema for the model you'll see why:

print(PyModel.schema_json(indent=2))
{
  "title": "PyModel",
  "type": "object",
  "properties": {
    "some_field": {
      "title": "Some Field",
      "type": "string"
    }
  },
  "required": [
    "some_field"
  ],
  "additionalProperties": false
}

Having said all this I completely sympathise with you, and respect that you may not want to complicate things. I have a workaround for this for now, but just thought you'd want to know that there is a perfectly valid scenario that will cause an exception.

@kuimono
Copy link
Owner

kuimono commented Sep 20, 2021

Thank you very much for the information. I didn't aware that Pydantic sets additionalProperties as false during the schema generation.

I will address this issue in upcoming release.

@jcalero
Copy link
Author

jcalero commented Sep 20, 2021

Thanks @kuimono, to be honest I debated to submit an issue to Pydantic themselves, since like you say, having an object with {} or {"not": {}} is equivalent and much easier to work with. But given that it's valid in the specs I thought it might be best to let you know instead.

Let me know if you need anything else from me!

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

Successfully merging a pull request may close this issue.

2 participants