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

[BUG] Using Nested Pydantic models and params: MyModel = Depends() forces OpenAPI docs GET methods to require a request body. #11037

Open
8 of 9 tasks
TrevorBenson opened this issue Jan 27, 2024 · 7 comments

Comments

@TrevorBenson
Copy link
Sponsor

TrevorBenson commented Jan 27, 2024

First check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the FastAPI documentation, with the integrated search.
    • Not familiar with the "integrated search".
  • I already searched in Google "How to X in FastAPI" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to FastAPI but to Pydantic.
  • I already checked if it is not related to FastAPI but to Swagger UI.
  • I already checked if it is not related to FastAPI but to ReDoc.
  • After submitting this, I commit to:
    • Read open issues with questions until I find 2 issues where I can help someone and add a comment to help there.
    • Or, I already hit the "watch" button in this repository to receive notifications and I commit to help at least 2 people that ask questions in the future.
    • Implement a Pull Request for a confirmed bug.

Example

Here's a self-contained minimal, reproducible, example with my use case:

from fastapi import APIRouter, Depends, FastAPI, Query
from fastapi.responses import JSONResponse
from typing import List, Optional
from pydantic import BaseModel, Field
import uvicorn

class MyModelData1(BaseModel):
    archive: Optional[str] = Field(None, description="Archive name")
    archive_type: Optional[str] = Field(None, description="Archive type")

class MetadataGet(BaseModel):
    id: Optional[str] = Field(None, alias="_id")
    foreign_key: Optional[str] = Field(None, alias="_foreign_key")

class DeepNestedModelGet(BaseModel):
    name: Optional[str]
    version: Optional[str]

class DetailsModelGet(BaseModel):
    some_data: Optional[List[Optional[DeepNestedModelGet]]]
    some_data2: Optional[List[Optional[str]]]

class MyModelData2(BaseModel):
    id: Optional[str] = Field(None, alias="_id")
    details: Optional[DetailsModelGet]
    meta: Optional[MetadataGet]

def get_documents(collection, sort_by, sort_order, page, page_size, **additional_filters):
    return {
        "collection": collection,
    }

app = FastAPI()
router1 = APIRouter(prefix="/data1", tags=["Data1"])
router2 = APIRouter(prefix="/data2", tags=["Data2"])

@router1.get("/", description="Retrieve all documents.")
def get_data1(
    sort_by: str = Query(None, description="Sort by this field"),
    sort_order: str = Query(None, description="Sort order"),
    page: int = Query(1, description="Page number"),
    page_size: int = Query(100, description="Number of documents per page"),
    params: MyModelData1 = Depends(),
):
    document = get_documents(
        "data1",
        sort_by,
        sort_order,
        page,
        page_size,
        **params.model_dump()
    )
    return JSONResponse(content=document)

@router2.get("/")
def get_data2(
    sort_by: str = Query(None, description="Sort by this field"),
    sort_order: str = Query(None, description="Sort order"),
    page: int = Query(1, description="Page number"),
    page_size: int = Query(100, description="Number of documents per page"),
    params: MyModelData2 = Depends(),
):
    """Get all software."""
    document = get_documents(
        "data2",
        sort_by,
        sort_order,
        page,
        page_size,
        **params.model_dump(),
    )
    return JSONResponse(content=document)

app.include_router(router1)
app.include_router(router2)



if __name__ == "__main__":
    uvicorn.run(app, host="127.0.0.1", port=5000)

Description

  • Open the browser and call the GET endpoint /data1/.
  • No request body is required for the GET method
  • Open the browser and call the GET endpoint /data2/.
  • A request body is required for the GET method and provided
  • If the user does not delete the request body since it is invalid a TypeError: Failed to execute 'fetch' on 'Window': Request with GET/HEAD method cannot have body. is returned.

The solution you would like

The request body is not a required part of the GET /data2/ UI documentation when using an identical Depends() structure so dump_model() method can be used to pass very large models to the documentation as query parameters for the GET method which are optional.

Environment

  • OS: Linux
  • FastAPI Version: 0.109.0
  • Python version: 3.9.18

Additional context

I've gone so far as to take List[str] which became Optional[List[str] in the Get version of the model, to an extreme of Optional[List[Optional[str]]] and List[DetailsModelGet] which was already all optional fields into Optional[List[Optional[DetailsModelGet]]]. Pushing Optional down to the last type and making "deeply optional" fields in case somehow this would resolve the issue. I cannot seem to find any instance of how to get the nested models not to result in a required request body in the docs GET method except to remove the nested (optional) models entirely and use a single-layer Pydantic model.

Originally posted by @stevesuh in #7275

@TrevorBenson TrevorBenson changed the title ### Using Nested Pydantic models and params: MyModel = Depends() seems to force swagger ui docs to require a request body for GET methods. ### Using Nested Pydantic models and params: MyModel = Depends() seems to force OpenAPI docs to require a request body for GET methods. Jan 29, 2024
@TrevorBenson TrevorBenson changed the title ### Using Nested Pydantic models and params: MyModel = Depends() seems to force OpenAPI docs to require a request body for GET methods. ### Using Nested Pydantic models and params: MyModel = Depends() forces OpenAPI docs GET methods to require a request body. Jan 29, 2024
@juampivallejo
Copy link

I faced a similar issue, the only way I found to make it work in the way you described was to explicitly pass a None default for the Optional nested model.
Like this:

class MyModelData1(BaseModel):
    archive: Optional[str] = Field(None, description="Archive name")
    archive_type: Optional[str] = Field(None, description="Archive type")

class MetadataGet(BaseModel):
    id: Optional[str] = Field(None, alias="_id")
    foreign_key: Optional[str] = Field(None, alias="_foreign_key")

class DeepNestedModelGet(BaseModel):
    name: Optional[str] = None
    version: Optional[str] = None

class DetailsModelGet(BaseModel):
    some_data: Optional[List[DeepNestedModelGet]] = None
    some_data2: Optional[List[Optional[str]]] = None

class MyModelData2(BaseModel):
    id: Optional[str] = Field(None, alias="_id")
    details: DetailsModelGet = Depends()
    meta: MetadataGet = Depends()

In this case, the request body is not required for the GET call.

@TrevorBenson
Copy link
Sponsor Author

I stumbled onto this (or something very similar) the night I first posted this. However, I didn't update the issue because

  1. While the body was no longer required, the docs then contained 2 Required fields, args and kwargs.
    Screenshot from 2024-02-07 13-15-00
  2. The Try it out option seems to be broken, as clicking Execute does not return a TypeError, however no response is received and a LOADING is constantly spinning, even after clicking the cancel button to try and back out.
    Screenshot from 2024-02-07 13-16-25

The args and kwargs instead of actual params for the nested models (preferred) would be acceptable if not required or breaking the functionality for testing the API. I would prefer some form of the dump_model() defining the params as optional since that is how they are listed. I suppose not documenting nested models as queryable params may be an intentional choice. 🤷

@TrevorBenson TrevorBenson changed the title ### Using Nested Pydantic models and params: MyModel = Depends() forces OpenAPI docs GET methods to require a request body. [BUG] Using Nested Pydantic models and params: MyModel = Depends() forces OpenAPI docs GET methods to require a request body. Feb 7, 2024
@sheluchin
Copy link

I believe using Pydantic models for GET params is not officially supported, and whatever of it works is only the case by accident. Using nested Pydantic models to define GET params is particularly problematic; there are many mentions of this if you search the issues and discussions. See this comment by @tiangolo.

The roadmap has plans for official support:

Support for Pydantic models for Query(), Form(), etc.

#10556 (comment)
#318 (comment)

@TrevorBenson
Copy link
Sponsor Author

I believe using Pydantic models for GET params is not officially supported, and whatever of it works is only the case by accident. Using nested Pydantic models to define GET params is particularly problematic; there are many mentions of this if you search the issues and discussions. See this comment by @tiangolo.

The roadmap has plans for official support:

Support for Pydantic models for Query(), Form(), etc.

#10556 (comment) #318 (comment)

That does appear to be the case. Thank you for the links, I had overlooked the comment in the closed discussion when it was marked answered w/ Pydantic v2 support, as well as the Roadmap as it did not explicitly mention Depends() for get methods or the resulting request body.

@sheluchin
Copy link

Since this comes up so often (judging by the number of issues and discussions), maybe it would be good to explicitly mention in the Query Parameters docs that using Pydantic models is not yet supported?

@sonnix85
Copy link

sonnix85 commented Mar 3, 2024

Hi, if you want to store data in a class, consider using dataclasses with query parameters. For the body, if there are only optional fields and any data will be sent, you should use Depends().

@j-carson
Copy link

@TrevorBenson

Here is a way to think about what you are trying to do in your code and what the workaround might look like:
It is important to understand that you are not
specifying a pydantic model as an HTTP query parameter here: Your backend function is receiving a Pydantic model as an
argument that was generated via a dependency injection.

It's important to keep the concepts of "query parameters" and "results of a dependency injection function" separate
in your head: Query parameters at the HTTP level can only be types allowed by OpenAPI which is the specification underneath FastAPI. Dependency injection, on the other hand, can create whatever complicated object you want.

What's happening here is the default dependency injection function is not doing what you expected for deeply nested
models. What is that dependency function doing? It's basically just looking at the type hint and seeing what fields the constructor needs
and pulling them out of the HTTP query parameters. But, those query args that the constructor is receiving can only be types that the OpenAPI standard allows! So, you can't really send a Pydantic model in easily that way.

I would actually say that your "working" endpoint 1 is only working partially - If you look at the Swagger docs,
you didn't get the description strings you specified for archive_name and archive_type, for example.
FastAPI is working as you expected, but the
Swagger UI (and any application trying to use your openapi.json schema info directly) doesn't have the quite
same picture as the one expressed in your code.

You can correct this by writing your own dependency injection function, rather than using what you get with a bare Depends(), which is what I'm going to do below.
One of the key things to keep in mind when writing your own dependency injection: FastAPI figures things out by
looking at type hints, but Swagger/OpenAPI figures things out by looking at the JSON schema of your API, which is generated by FastAPI using Pydantic, the glue between the highest and lowest levels. Basically, I'll be explicitly gluing things together by grabbing JSON schema for Swagger and arguments for FastAPI functions by pulling info
out of your Pydantic classes.

Here's a handy glue function: This function takes info out of a particular field in a
a Pydantic class object and and populates an arguments dictionary that we're going to want to
pass to FastAPI's Query() API to make dependency magic happen.

def query_kwargs(
    class_: type[BaseModel],  # The Pydantic class we're making in the dependency injection
    field: str                # The name of the field we're trying to pull out of the query string
):              
    field_info =  class_.model_fields[field]
    is_required = field_info.is_required()
    description = field_info.description
    default = field_info.get_default()
    
    result = {
        'default': default,
        'required': is_required,
        'description': description,
    }

    if alias := field_info.serialization_alias:
        json_schema = class_.model_json_schema()['properties'][alias]
    else:
        json_schema = class_.model_json_schema()['properties'][field]

    result['json_schema_extra'] = json_schema
    return result

(The above will at least do the trick for basic field types, not fields that are nested models. If you run MyModelData2.model_json_schema() you'll see that it has a whole set of sub-definitions and other things
that I'm not even checking for.)

Here is your simple class with a custom dependency function:

class MyModelData1(BaseModel):
    archive: Optional[str] = Field(None, description="Archive name")
    archive_type: Optional[str] = Field(None, description="Archive type")

    @classmethod
    def from_query(cls):
        """Class method to build an instance from the HTTP query string"""

        # Use my handy function above to generate the args to Query
        q_archive = query_kwargs(cls, 'archive')
        q_archive_type = query_kwargs(cls, 'archive_type')

        # Create a function that parses info out of the query explicitly, rather
        # that implicitly
        def dependency_to_inject(
            # Be sure these type hints match the type annotation in the class
            # field definitions or FastAPI could become confused in some cases!
            archive: Optional[str] = Query(**q_archive),
            archive_type: Optional[str] = Query(**q_archive_type),
        ) -> Self:
            return cls(archive=archive, archive_type=archive_type)
        return Depends(dependency_to_inject)

And then swap in this new function for your bare Depends() as below. (I also changed the response to echo back the params object that was built to make it easier to see what's happening.

@router1.get("/", description="Retrieve all documents.")
def get_data1(
    sort_by: str = Query(None, description="Sort by this field"),
    sort_order: str = Query(None, description="Sort order"),
    page: int = Query(1, description="Page number"),
    page_size: int = Query(100, description="Number of documents per page"),
    params: MyModelData1 = MyModelData1.from_query()  #  <---- Call the new function here!
):
    # Change the response to return the params as decoded to see what happened!
    return JSONResponse(content=params.model_dump())

Go run this new version and play with the MyModelData1 field definitions a little - update the description, default,
type, etc. and see that the Swagger docs are now responding to those changes, type errors are caught, etc.

Now, let's move on to your deeply nested example.

The first thing I wanted to know when I tried to build a dependency function for this version was:
What do you want your query-string to look like in when sending this request? Do you just want all the
leaf-level field names to be piled in there, or do you want the deeply nested models to be passed as
stringified-something (for example, to avoid problems like the name clash on the "id" field in
MyModelData2 and MetaDataGet)?

That's a domain-specific answer, and I basically just punted on it for the code below. I said, OK, the "id" in the MyModelData2 object and the "id" in the MetaDataGet were just going to be the same value pulled from the query, since they have the same name, alias, and type. I also wasn't quite sure what do to with the array of DeepNestedModelGet objects -- I just assumed the user is required to send the same number of "name" and "version" files to make each element of the array have enough data and didn't even error check it.

Even with those simplifications, the dependency function is quite a bit more complicated: I need to pull fields out of the query and build the model tree from the bottom up:

class MyModelData2(BaseModel):
    id: Optional[str] = Field(None, alias="_id")
    details: Optional[DetailsModelGet]
    meta: Optional[MetadataGet]

    @classmethod
    def from_query(cls):
        """Class method to build an instance from a query string"""
        q_id = query_kwargs(cls, "id")

        # Plain fields of member 'details'
        q_some_data2 = query_kwargs(DetailsModelGet, "some_data2")
        
        # Plain fields of member 'details.some_data' -- except! We actually
        # want a list of these objects in the next level up so patch the
        # json schema up to make these arrays that can be in query string 
        # more than once

        q_name = query_kwargs(DeepNestedModelGet, "name")
        q_version = query_kwargs(DeepNestedModelGet, "version")

        q_name['json_schema_extra'] = {
            'type': 'array',
            'items': q_name['json_schema_extra']
        }
        q_version['json_schema_extra'] = {
            'type': 'array',
            'items': q_version['json_schema_extra']
        }
        
        # Plain fields of member "meta"
        # Note: id field name is already taken for this query so ???
        q_foreign_key = query_kwargs(MetadataGet, "foreign_key")

        def dependency_to_inject(
            id: Optional[str] = Query(**q_id),
            some_data2: Optional[list[Optional[str]]]  = Query(**q_some_data2),
            # These are now lists of the original field type
            name: Optional[list[Optional[str]]] = Query(**q_name),
            version: Optional[list[Optional[str]]] = Query(**q_version),
            foreign_key: Optional[str] = Query(**q_foreign_key),
        ):
            if name is not None and version is not None:
                some_data = [
                    DeepNestedModelGet(
                        name=n,
                        version=v
                    )
                    for n,v in zip(name, version)
                ]
            else:
                some_data = None
                
            result =  cls(
                _id=id,
                details = DetailsModelGet(
                    some_data =some_data,
                    some_data2 = some_data2
                ),
                meta = MetadataGet(
                    _id = id,
                    _foreign_key = foreign_key,
                )
            )
            return result
        return Depends(dependency_to_inject)

Now, change the definition of get_data2 to mimic what we did in get_data1 -- use the new .from_query() method
instead of Depends() and change the response to dump the params argument to see what you got. If you open the
new version in your Swagger docs, you will see a form with all the fields you need to create at least some version
of your deeply nested model, and if you hit submit you'll see the JSON that the dependency injection calculated.

But, I hope you also see drawbacks of the above code:

I made a bunch of assumptions about how you wanted the HTTP query string formatted to even begin to write this function (FastAPI usually hides that). I just used the leaf-level field names as the query field names.
Something like having query parameters details__some_data2 that encapsulate the model tree path might prevent name clashes, but it also exposes class implementation details you don't necessarily want the end user of
your API to have to worry about.
Figuring out unique names for all the parts of a deeply nested tree would not be easy on the programmer either.
Or, passing "details" as a single query parameter means you have encoded parameters that need a second level
of decoding to find fields inside the sub-model, adding complexity for both the caller and the backend.
There's basically no simple surefire way to define this endpoint.

I don't have good separation of concerns between the various class definitions in part 2 - the Data2 decoder
has intimate knowledge of the fields of the classes it encloses and will have to be maintained carefully.
Changing the contents any of the above classes would quickly mess things up.

This code required full-stack understanding of OpenAPI, Pydantic, and FastAPI to get to this still half-baked result.
No joy of letting FastAPI take care of the details here, but to understand why this feature might not come, if ever:
How could those details easily get taken care of by a library?
Even if FastAPI could support deeply nested models, it probably couldn't support them magically under the covers with the code as you first wrote it. It would probably require at least some new Annotated[] directives to tell the system how you want it to behave at various important decision points, or it would have to have a lot of restrictions and throw
runtime errors whenever things start getting tricky.

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

No branches or pull requests

5 participants