fastapi icon indicating copy to clipboard operation
fastapi copied to clipboard

Generated schemas do not follow the OpenAPI specification (FastAPI v0.66+)

Open ml-evs opened this issue 3 years ago • 18 comments

First Check

  • [X] I added a very descriptive title to this issue.
  • [X] I used the GitHub search to find a similar issue and didn't find it.
  • [X] I searched the FastAPI documentation, with the integrated search.
  • [X] I already searched in Google "How to X in FastAPI" and didn't find any information.
  • [X] I already read and followed all the tutorial in the docs and didn't find an answer.
  • [X] I already checked if it is not related to FastAPI but to Pydantic.
  • [X] I already checked if it is not related to FastAPI but to Swagger UI.
  • [X] I already checked if it is not related to FastAPI but to ReDoc.

Commit to Help

  • [X] I commit to help with one of those options 👆

Example Code

import json
from pydantic import BaseModel, Field
from fastapi import FastAPI
from fastapi import __version__ as fastapi_version

class Model(BaseModel):
    field: str = Field("foo", const=True)


app = FastAPI(title=f"Created with FastAPI v{fastapi_version}")


@app.get("/", response_model=Model)
def root():
    return Model(field="foo")

print(json.dumps(Model.schema(), indent=2))
print(json.dumps(app.openapi(), indent=2))

Description

With fastapi==0.68.0 and pydantic==1.8.2, the above MWE produces an invalid OpenAPI schema due to the handling of extra keys introduced in the last release. The inclusion of the const key is not supported in OpenAPI 3 (see Supported Keywords).

Here is a diff between the schema created for v0.65.2 vs v0.68 (also tested with 0.66, 0.67 with the same results).

// ...
  },
  "components": {
    "schemas": {
      "Model": {
        "title": "Model",
        "type": "object",
        "properties": {
          "field": {
            "title": "Field",
            "type": "string",
+           "const": "foo"
          }
// ...

This regression is caused by the addition of extra = "allow" to fastapi.openapi.models.Schema in (#1429) (see also the comment https://github.com/tiangolo/fastapi/pull/1429#issuecomment-889256569). This is something we ran into when updating FastAPI at https://github.com/Materials-Consortia/optimade-python-tools/pull/887.

While I would also like to be able to use extension keys (with the appropriate x- prefix) too, this change now makes it hard to use some standard pydantic features. My suggestion would be to enumerate the keys from JSON schema that OpenAPI does not support (linked above) and handle those separately in the Config for Schema. Perhaps any "extra" key that is not in this list could be prepended with a x-my-app prefix defined on the init of the FastAPI instance. I'm happy to have a go at implementing this, if desired.

Operating System

Linux

Operating System Details

No response

FastAPI Version

0.68.0

Python Version

3.8.5

Additional Context

I've made two gists for the OpenAPI schemas from v0.68.0 and v0.65.2:

  • 0.65.2: https://gist.github.com/ml-evs/99b33f68b74a0c197e970f9222faa2bc
  • 0.68.0: https://gist.github.com/ml-evs/824f9ac510f92d4e180a4e984d1da38d

You can pass the raw json through the online swagger validator to verify:

  • 0.65.2: https://validator.swagger.io/validator/debug?url=https://gist.githubusercontent.com/ml-evs/99b33f68b74a0c197e970f9222faa2bc/raw/71d056de37a942bb9bc26943151c477a97aa18f4/fastapi_v0.65.2_openapi.json

    $ curl -s https://validator.swagger.io/validator/debug\?url\=https://gist.githubusercontent.com/ml-evs/99b33f68b74a0c197e970f9222faa2bc/raw/71d056de37a942bb9bc26943151c477a97aa18f4/fastapi_v0.65.2_openapi.json | jq
    {}
    
  • 0.68.0: https://validator.swagger.io/validator/debug?url=https://gist.githubusercontent.com/ml-evs/824f9ac510f92d4e180a4e984d1da38d/raw/d25b9c3a7802702ced6e3f7a1aa60e017ac9b339/fastapi_v0.68.0_openapi.json

    $ curl -s https://validator.swagger.io/validator/debug\?url\=https://gist.githubusercontent.com/ml-evs/824f9ac510f92d4e180a4e984d1da38d/raw/d25b9c3a7802702ced6e3f7a1aa60e017ac9b339/fastapi_v0.68.0_openapi.json | jq
    {
      "messages": [
        "attribute components.schemas.Model.const is unexpected"
      ],
      "schemaValidationMessages": [
        {
          "level": "error",
          "domain": "validation",
          "keyword": "oneOf",
          "message": "instance failed to match exactly one schema (matched 0 out of 2)",
          "schema": {
            "loadingURI": "#",
            "pointer": "/definitions/Components/properties/schemas/patternProperties/^[a-zA-Z0-9\\.\\-_]+$"
          },
          "instance": {
            "pointer": "/components/schemas/Model"
          }
        }
      ]
    }
    

ml-evs avatar Aug 21 '21 12:08 ml-evs

This addition of extra: str = "allow" has completely broken nearly every Model that I have across dozens of apps. All of my models leverage pydantics Field() which errors completely when viewing the OpenAPI docs

/fastapi/encoders.py", line 142, in jsonable_encoder raise ValueError(errors)

ValueError: [TypeError("'ModelField' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')]

This is obviously because all of pydantics __fields__ which are translated to ModelField are using __slots__ not standard dicts so vars(obj) or dict(obj) fail completely. All of this because extra was set to allow so all sorts of unwanted items are trying to be pumped out to OpenAPI. Seems like a massive breaking change to me. I guess no one else uses Field() in their models?

mreschke avatar Sep 22 '21 04:09 mreschke

Turns out the custom BaseModel was adding some stuff to 'extra' in FieldInfo that it was using for its own internals. Now that extra is defaulted to allow those all show up in OpenAPI during FastAPIs processing of the models which if not JSON serializable will cause issues. Not FastAPI's issue, but how the BaseModel was being tweaked for internals.

mreschke avatar Sep 22 '21 15:09 mreschke

Turns out the custom BaseModel was adding some stuff to 'extra' in FieldInfo that it was using for its own internals. Now that extra is defaulted to allow those all show up in OpenAPI during FastAPIs processing of the models which if not JSON serializable will cause issues. Not FastAPI's issue, but how the BaseModel was being tweaked for internals.

Could you elaborate please? Which custom BaseModel?

This sounds like a different (but related) issue to what I am facing (where the OpenAPI is generated without error but the field names themselves are not valid OpenAPI).

ml-evs avatar Sep 22 '21 15:09 ml-evs

It was a custom BaseModel and Field() override that added a few things to extra. In older FastAPI versions those were ignored, which was the desired effect. Now they show up due to extra=allow. Your issue is more complex because const is a valid parameter in pydantic/fields.py both in the def Field() and in class FieldInfo and now it shows in the OpenAPI spec, which is definitely a invalid field according to https://swagger.io/docs/specification/data-models/keywords/. The larger issue is that pydantic Field() (and therefore class FieldInfo) have that extra kwargs which pipes any undefined param right into OpenAPI spec creating a potentially invalid .json spec. The only valid extra kwargs that Field() could possibly take without breaking the spec validator is those that begin with x-, although you cannot add a dash as a parameter name, ie: name: string = Field(x-other='goodies'). I'm sure FastAPI had a good reason to add extra=allow by default, but it sure makes things more breakable.

mreschke avatar Sep 22 '21 15:09 mreschke

Right, I'm with you now.

The only valid extra kwargs that Field() could possibly take without breaking the spec validator is those that begin with x-, although you cannot add a dash as a parameter name, ie: name: string = Field(x-other='goodies').

My suggested fix was something like this, i.e. allowing any x- prefixed fields through, but your custom FieldInfo (and also the custom schema_extra described in the PR that added this https://github.com/tiangolo/fastapi/pull/1429) seem to add another constraint that might require some introspection.

I'm sure FastAPI had a good reason to add extra=allow by default, but it sure makes things more breakable.

I would've thought this would be quite important to fix for anyone relying on FastAPI for OpenAPI schema generation, but it doesn't seem like many other users are running into this.

Due to the lack of uptake on this issue, perhaps it would not be too rude to attempt to summon @tiangolo to get his thoughts...

ml-evs avatar Sep 22 '21 15:09 ml-evs

My guess is, if people are using extra and pumping in invalid parameters that get piped to extra, their OpenAPI spec IS invalid, they just don't know it. I didn't until you mentioned the swagger validator. I ran my .json through https://apitools.dev/swagger-parser/online/ and it was definitely invalid because of my extra stuff not using x-. The point is, the OpenAPI docs site did not break or complain. It works just fine with invalid spec. I bet many a peoples specs broke validation since FastAPI 0.65.0+ they just don't know it yet.

mreschke avatar Sep 22 '21 16:09 mreschke

Not sure if this helps with your particular use case...as a workaround. Obviously defaulting extra=allow will still be a pervasive issue. But you could make your own class Field(PydanticFieldInfo) (extending Pydantics FieldInfo class) and handle any extra kwargs and placing them inside a x-tra parameter before calling super. Then your Field("foo", const=True) would result in a

{
  "x-tra": {
    "const": True
  },
}

That's basically what I did with my custom Field() class.

mreschke avatar Sep 22 '21 20:09 mreschke

When using custom extra attributes on fields, which are used internally for different kinds of data processing, these values are now exposed in the api schema. In my case additionally it causes an infinite recursion.

To give some context: I'm using fastapi with django. To map pydantic model fields to a django model field, an argument orm_field is set on the pydantic model's field pointing to the django model field.

from django.db import models
from pydantic import BaseModel

class MyDBModel(models.Model):
    id = models.CharField(primary_key=True, max_length=16)

class MyModel(BaseModel):
    id: str = Field(orm_field=MyDBModel.id)

Without patching fastapi.openapi.models.Schema's Config setting extra = 'ignore' (as it was pre 0.66.0), a infinite recursion ocurrs when rendering the openapi.json

  File ".../fastapi/encoders.py", line 101 in jsonable_encoder
  File ".../fastapi/encoders.py", line 145 in jsonable_encoder
  File ".../fastapi/encoders.py", line 115 in jsonable_encoder
  File ".../fastapi/encoders.py", line 101 in jsonable_encoder
  File ".../fastapi/encoders.py", line 145 in jsonable_encoder
  File ".../fastapi/encoders.py", line 101 in jsonable_encoder
  File ".../fastapi/encoders.py", line 145 in jsonable_encoder
  File ".../fastapi/encoders.py", line 145 in jsonable_encoder
  File ".../fastapi/encoders.py", line 101 in jsonable_encoder
  File ".../fastapi/encoders.py", line 145 in jsonable_encoder
  File ".../fastapi/encoders.py", line 115 in jsonable_encoder
  File ".../fastapi/encoders.py", line 101 in jsonable_encoder

As the OpenAPI Spec only allows custom attributes with a x- prefix, I suggest to revert the change, as it is possible to declare a custom schema definition on the pydantic model using a custom config.

mstingl avatar Feb 26 '22 23:02 mstingl

Just tested again and this bug is still affecting v0.75.1. It would be great to get some confirmation that a fix to FastAPI would be useful, perhaps following @mreschke's workaround above. I fear that many users are producing invalid OpenAPI schemas without realising.

ml-evs avatar Apr 14 '22 15:04 ml-evs

Is there any chance you could confirm this is a bug @tiangolo, 1 year on? If not, I am happy to prepare a docs PR that tries to explain the issue for other confused users. Given the (deserved) popularity of FastAPI I think this is unfortunately poisoning the OpenAPI ecosystem with invalid schemas!

Whilst @mreschke's fix above does generate the correct OpenAPI spec, I think that it ends up disabling the pydantic validation (of e.g. const) without much extra boilerplate that redefines the entire class hierarchy down to fastapi.openapi.models.Schema.

A nice fix would be to capture this at the pydantic level with a new extra = 'prefix' handling of extra fields (or even provide a callable for how to handle them), but until then this functionality might have to be implemented in the Config for fastapi.openapi.models.Schema directly.

Unfortunately, as we are 1 year on, this may break things for people relying on extra fields in their non-standard OpenAPI schemas.

ml-evs avatar Apr 25 '22 11:04 ml-evs

Any progress on this one?

ramilmsh avatar May 10 '22 17:05 ramilmsh

Any progress on this one?

I made a pull request to the docs warning people about this (#4846) but it hasn't gotten any traction either, I wonder if anyone else who has encountered this problem has feedback for that PR or better suggestions for how to deal with it?

ml-evs avatar May 11 '22 11:05 ml-evs

Hey all! Some additional info: Pydantic generates JSON Schema, and that is re-used by OpenAPI.

But OpenAPI 3.0.x is based on an old version of JSON Schema that didn't have const and also had some additional incompatibilities. OpenAPI 3.1.x is based on JSON Schema v7, which has const.

So, about const, the ideal would be to be able to upgrade OpenAPI to 3.1.x. In fact, the currently generated OpenAPI schema is a bit more compatible with 3.1.x than 3.0.x. But I haven't been able to update the generated version by FastAPI because Swagger UI is not compatible with 3.1.x, it doesn't even try to render, it just explodes. So I can't upgrade that resulting version yet. The best way to move forward for that would be to improve/add support in Swagger UI to OpenAPI 3.1.x.

Now, about extra fields included by default in the output JSON Schema, I hope a future version of Pydantic will have something like Field(schema_extra={}) instead of just including any extra args there. That would allow explicitly adding custom stuff that should be included in the output JSON Schema and avoid side effects for other use cases.

tiangolo avatar May 11 '22 21:05 tiangolo

Thanks for breaking it down @tiangolo!

ml-evs avatar May 13 '22 16:05 ml-evs

any movement on this? We're also stuck with an autogenerated openapi spec that seems invalid. From the comment above it's not clear to me how what users should do. What are the mitigation steps? Downgrade fastapi if needing a valid openapi spec?

trondhindenes avatar Aug 08 '22 18:08 trondhindenes

Here's the slightly kludgey way we got around it @trondhindenes, in case it is useful to you: https://github.com/Materials-Consortia/optimade-python-tools/pull/1131/files

Basically:

  • stop using const and use a regex that matches the constant value instead
  • wrap/reimplement pydantic.Field with our own class that itself wraps FieldInfo, which strips or prefixes invalid OpenAPI keys.

ml-evs avatar Aug 08 '22 19:08 ml-evs

awesome, thanks for that @ml-evs !

trondhindenes avatar Aug 09 '22 13:08 trondhindenes

Just a quick note that Pydantic v2 will have its own independent field json_schema_extra instead of taking everything else and putting it in the JSON Schema output. :tada:

That will at least help understand and debug these use cases better. :nerd_face:

tiangolo avatar Nov 05 '22 10:11 tiangolo

Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

github-actions[bot] avatar Nov 16 '22 00:11 github-actions[bot]