🐛 Fix get_model_fields to support Annotated types#14805
🐛 Fix get_model_fields to support Annotated types#14805ilovemesomeramen wants to merge 4 commits intofastapi:masterfrom
get_model_fields to support Annotated types#14805Conversation
|
@ilovemesomeramen, thank you for your interest and efforts! |
|
@YuriiMotov What do you mean exactly? The provided example is exactly the issue which this PR solved, this example worked in 0.124.0 but does not in 0.124.1 A little bit of context why this is needed: the provided test case is based on this test: tests/test_arbitrary_types.py The Problem is the Union with a single value in it this worked before and no longer works since the type is not properly extracted. |
|
This pull request has a merge conflict that needs to be resolved. |
get_model_fields to support Annotated types
There was a problem hiding this comment.
LGTM!
We can implement the same (*almost) logic without RootModel (with MyModel: TypeAlias = Annotated[Union[Base], Field(discriminator="type")]), but openapi schema looks different - it doesn't create dedicated schema in components, but just embeds schema into response object:
Details
from typing import Annotated, Literal, TypeAlias, Union
from fastapi import FastAPI
from pydantic import BaseModel, Field
class Base(BaseModel):
type: Literal["BASE"] = "BASE"
value: str
MyModel: TypeAlias = Annotated[Union[Base], Field(discriminator="type")]
app = FastAPI()
@app.get("/")
def test() -> MyModel:
pass{
"openapi": "3.1.0",
"info": {
"title": "FastAPI",
"version": "0.1.0"
},
"paths": {
"/": {
"get": {
"summary": "Test",
"operationId": "test__get",
"responses": {
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/Base"
}
],
"title": "Response Test Get",
"discriminator": {
"propertyName": "type",
"mapping": {
"BASE": "#/components/schemas/Base"
}
}
}
}
}
}
}
}
}
},
"components": {
"schemas": {
"Base": {
"properties": {
"type": {
"type": "string",
"const": "BASE",
"title": "Type",
"default": "BASE"
},
"value": {
"type": "string",
"title": "Value"
}
},
"type": "object",
"required": [
"value"
],
"title": "Base"
}
}
}
}Serialization logic works the same, only schema is different.
I attempted to come up with code example without RootModel that causes the same issue, but failed - seems that regular BaseModel unwraps type annotations for its fields.
So, even though I don't really like the test, I still think we should merge this fix - it looks logically to also check for Annotated[BaseModel, ..]
@ilovemesomeramen, thanks for clarifications and for working on this!
|
@YuriiMotov Yes exactly, i know the test looks a little wonky but it makes sense in my opinion. I think it is logical that the plain annotated type does not create a new component but the root model creates one, as defining a RootModel implicates a new "Model" (component) while the plain Type Annotation does not (in my opinion). So this should be fine. Regarding the additional test, would something like this work for you? Detailsfrom typing import Annotated, Literal, Union
import pytest
from fastapi import FastAPI
from fastapi.testclient import TestClient
from inline_snapshot import snapshot
from pydantic import BaseModel, Field, RootModel
class Base(BaseModel):
type: Literal["BASE"] = "BASE"
value: str
class MyModelRoot(RootModel[Annotated[Union[Base], Field(discriminator="type")]]):
pass
class MyModelBase(RootModel[Annotated[Base, Field(discriminator="type")]]):
pass
app = FastAPI()
@app.get("/root")
def get_root() -> MyModelRoot:
return MyModelRoot.model_validate(Base(value="test"))
@app.get("/base")
def get_base() -> MyModelBase:
return MyModelBase.model_validate(Base(value="test"))
client = TestClient(app)
@pytest.mark.parametrize("path", ["/root", "/base"])
def test_get(path: str):
response = client.get(path)
assert response.json() == {"value": "test", "type": "BASE"}
def test_openapi_schema():
response = client.get("openapi.json")
assert response.json() == snapshot(
{
"openapi": "3.1.0",
"info": {"title": "FastAPI", "version": "0.1.0"},
"paths": {
"/root": {
"get": {
"summary": "Get Root",
"operationId": "get_root_root_get",
"responses": {
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/MyModelRoot"
}
}
},
}
},
}
},
"/base": {
"get": {
"summary": "Get Base",
"operationId": "get_base_base_get",
"responses": {
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/MyModelBase"
}
}
},
}
},
}
},
},
"components": {
"schemas": {
"Base": {
"properties": {
"type": {
"type": "string",
"const": "BASE",
"title": "Type",
"default": "BASE",
},
"value": {"type": "string", "title": "Value"},
},
"type": "object",
"required": ["value"],
"title": "Base",
},
"MyModelBase": {
"oneOf": [{"$ref": "#/components/schemas/Base"}],
"title": "MyModelBase",
"discriminator": {
"propertyName": "type",
"mapping": {"BASE": "#/components/schemas/Base"},
},
},
"MyModelRoot": {
"oneOf": [{"$ref": "#/components/schemas/Base"}],
"title": "MyModelRoot",
"discriminator": {
"propertyName": "type",
"mapping": {"BASE": "#/components/schemas/Base"},
},
},
}
},
}
|
| GetJsonSchemaHandler as GetJsonSchemaHandler, | ||
| ) | ||
| from pydantic._internal._typing_extra import eval_type_lenient | ||
| from pydantic._internal._typing_extra import annotated_type, eval_type_lenient |
There was a problem hiding this comment.
Would be great if FastAPI did not rely on more Pydantic internals than it does currently. eval_type_lenient is a clear example, I had to "soft-deprecate" it (i.e. only wrap it around @typing_extensions.deprecated without raising any runtime warning) solely because FastAPI was using it. annotated_type might be deleted at any point (and in fact has a good chance of being removed at some point).
This can easily be implemented directly in FastAPI.
There was a problem hiding this comment.
@Viicos Good point, but in general this is a weird set of circumstances in the first place.
Since this entire code
type_ = annotated_type(field_info.annotation) or field_info.annotation
if lenient_issubclass(type_, (BaseModel, dict)) or is_dataclass(type_):
model_config = None
else:
model_config = model.model_configis just a duplication of this pydantic internal function anyway
def _type_has_config(type_: Any) -> bool:
"""Returns whether the type has config."""
type_ = _typing_extra.annotated_type(type_) or type_
try:
return issubclass(type_, BaseModel) or is_dataclass(type_) or is_typeddict(type_)
except TypeError:
# type is not a class
return Falsesince otherwise the pydantic TypeAdapter raises an error. So i guess we could just copy this function to fastapi but then when Pydantic decides to remove the function and in the process maybe change the check in the TypeAdapter the two checks are out of sync again just waiting for a new bug to be introduced.
i think depending directly on the internal pydantic function will make it clear that this code needs to be looked at again if the pydantic function is removed and ported to the new pydantic way of handling it.
As far as I can see the difference is only |
|
I'll also note that |
#14482 added the model_config to the ModelField to support arbitrary types, however the if statement did not check for all the correct types.
This PR brings the check inline with the internal pydantic TypeAdapter check which would throw an error in some cases, e.g. Union fields with single Values.