-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
List[...]
as type for a Form
field doesn't work as expected
#3532
Comments
Then why does the Swagger UI produce curl requests which clearly don't work? |
You've got me! I don't know what is causing it. 😢 |
Found a new related issue, when using
Request made using the Swagger UI:
Response:
Again: is the implementation wrong? Is Swagger wrong? If the implementation is bad why isn't it catched as an issue? Would really like to get some kind of feedback on this or even an acknowledgement that somebody is remotely planning to look into it, seems like nobody cares (@tiangolo ?) |
Bump |
This is also a problem with |
Bump on this, still experiencing this with lists of form params |
Why hasn't it been solved? I am experiencing it too. @tiangolo any comment about this? Thank you in advance The service: @router.post(
path="/solicitud-inscripcion/registrar",
tags=["identificacion"]
)
async def registrar_solicitud_inscripcion(
imagenes: UploadFile = File(...),
esParcial: bool = Form(
...,
example=True
),
idSolicitante: int = Form(
...,
gt=0,
example=1
),
idSede: int = Form(
...,
gt=0,
example=1
),
idPropietario: int = Form(
...,
gt=0,
example=1
),
bienes: List[BienOutModel] = Form(...)
):
return {"resp": "it works"}
and the output is this
|
EDIT - this is a bug in swagger-ui see swagger-api/swagger-js#2713 That linked doc talks about query parameters but there is a mention of it when describing request bodies too. Looks like the same rules apply. |
@phillipuniverse Do you override the OpenAPI generation to fix this? Any chance you'd submit a PR to fix this here or in pydantic? |
I'd like to do a bit more with this, but it's getting late here, and wanted to put something down because I did make a tiny bit of progress with this and wanted to share. This page describes where the It goes under the content / multipart-form (or x-www-form-urlencoded) section in it's own @phillipuniverse is correct -- once adding the As an example of what the openapi schema should look like ... if you had a param that was a form list ( "requestBody": {
"content": {
"multipart/form-data": {
"schema": {
"$ref": "#/components/schemas/Body_update_movie_movie__movie_id__patch"
},
// this is the section to add:
"encoding": {
"my_list_param": {
"style": "form",
"explode": true
}
}
}
}
} |
@falkben looking at this again, I think that there is a bug in Swagger UI. According to the 'Form Data' section of describing request bodies, it says:
So the default should work the exact same as query parameters. If there are changes to code, it should be within Swagger UI, not in FastAPI or Pydantic (although perhaps FastAPI could make it easier to extend the I messed around with a very rough way to override this with monkeypatching: import fastapi.openapi.utils
orig_get_request_body = fastapi.openapi.utils.get_openapi_operation_request_body
from typing import Optional
from pydantic.fields import ModelField
from typing import Dict
from typing import Union
from typing import Type
from pydantic import BaseModel
from enum import Enum
from typing import Any
def get_request_body_with_explode(*,
body_field: Optional[ModelField],
model_name_map: Dict[Union[Type[BaseModel], Type[Enum]], str],
) -> Optional[Dict[str, Any]]:
original = orig_get_request_body(body_field=body_field, model_name_map=model_name_map)
if not original:
return original
content = original.get("content", {})
form_patch = content.get("application/x-www-form-urlencoded", {})
if not form_patch:
form_patch = content.get("multipart/form-data", {})
if form_patch:
form_patch["encoding"] = {
"vals": { # TODO: change to be more dynamic
"style": "form",
"explode": True
}
}
return original
fastapi.openapi.utils.get_openapi_operation_request_body = get_request_body_with_explode You would monkeypatch that in the same sort of place you instantiate This works great with an endpoint defined like: @router.post("/withlistparams")
async def with_list_params(vals: list[str] = Form()):
raise NotImplementedError() The annoying part in the spec is that you have to specify the "encoding" on a per-request-property basis. But you could use the given Hope that helps! |
Thanks @phillipuniverse, that is very helpful. The code I had was directly modifying the schema dictionary, so I think this would be an improvement, although I think I want to come up with a way to generalize it to all List/form params. Unfortunately, that typing information doesn't seem to be available in the
I'm curious why you say that. It seems that what SwaggerUI produces is fine, generally speaking -- it's just that FastAPI doesn't parse lists of params with CSV so it cannot use the default that SwaggerUI produces. Since that is the case, I would think the fix would naturally go into FastAPI where it generates the openapi scheme. Since it knows that Re: easier extending into the encoding section, I did see some interesting options there. For example, I know that FastAPI can't easily convert complex, nested Pydantic models to Form fields. However, there is an option in openapi called "Complex Serialization in Form Data" (here) where a form param will parse a JSON payload. This could be a "natural" way to enable nested models in Forms. |
@falkben whether or not FastAPI should or should not parse CSV values is a bit separate from this discussion about Swagger UI.
I have read a bit more into the OpenAPI spec and opened a bug ticket in swagger-js. In the bug ticket I included a bit more information and relevant parts of the OpenAPI spec, specifically that if not provided, the If you look at the bug ticket that should convince you that the default the SwaggerUI produces is inconsistent with the OpenAPI specification. Let me know if I'm misreading! If I'm not misreading then we should close this ticket in FastAPI as effectively a duplicate of #50 (comment). |
ah, that would explain some of my confusion when reading through the spec and then comparing to the behavior of SwaggerUI. The spec does state that form data should have a default of I'm definitely in favor of this being fixed in SwaggerUI if they agree it's a bug. Otherwise, I think it should be possible to look into the model when creating the schema to determine if it needs an encoding section. It probably won't be pretty, but I suspect there's a way to get the information we need through the |
@falkben I couldn't help myself, here's the fully dynamic version of my monkeypatch!! import fastapi.openapi.utils
orig_get_request_body = fastapi.openapi.utils.get_openapi_operation_request_body
from typing import Optional
from pydantic.fields import ModelField
from pydantic.schema import field_schema
from typing import Any, Dict, Union, Type
from pydantic import BaseModel
from fastapi.openapi.constants import REF_PREFIX
from enum import Enum
def get_request_body_with_explode(*,
body_field: Optional[ModelField],
model_name_map: Dict[Union[Type[BaseModel], Type[Enum]], str],
) -> Optional[Dict[str, Any]]:
original = orig_get_request_body(body_field=body_field, model_name_map=model_name_map)
if not original:
return original
content = original.get("content", {})
form_patch = content.get("application/x-www-form-urlencoded", {})
if not form_patch:
form_patch = content.get("multipart/form-data", {})
if form_patch:
schema_reference, schemas, _ = field_schema(
body_field, model_name_map=model_name_map, ref_prefix=REF_PREFIX
)
array_props = []
for schema in schemas.values(): # type: Dict[str, Any]
for prop, prop_schema in schema.get("properties", {}).items():
if prop_schema.get("type") == "array":
array_props.append(prop)
form_patch["encoding"] = {prop: {"style": "form"} for prop in array_props} # could include "explode": True but not necessary in swagger-ui
return original
fastapi.openapi.utils.get_openapi_operation_request_body = get_request_body_with_explode
Yup, exactly! The key part to my above code is to get the actual OpenAPI schema from Pydantic, not just a reference to it. The schema comes back in the 2nd part of the tuple in the schema_reference, schemas, _ = field_schema(
body_field, model_name_map=model_name_map, ref_prefix=REF_PREFIX
) Once you've got that it's trivial to determine the array properties. Shouldn't be too bad to refactor that to a recursive version either. Let me know if that works for you! |
This worked perfectly @phillipuniverse! I modified it slightly to use the walrus operator (my first time using the walrus operator, in fact) if form_patch := (
content.get("application/x-www-form-urlencoded")
or content.get("multipart/form-data")
): |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Opening a new issue since no action has been taken on #842 for more than a month.
Looks like we have an issue on how fastapi handles lists of elements passed as
Form
parameter (so anything that is type hintedList[...] = Form(...)
). The issue doesn't arise when they are defined asQuery
parameters.After some digging I managed to get to the root of the problem, which is how we pass the parameters when making an api call. fastapi indeed correctly understands that the input needs to be a list of elements, but doesn't parse the list if it comes from a single field.
Example using
Query
, works as expectedApi request generated using the swagger UI
note how the passed values are
l=1&l=2
Example using
Form
, doesn't work as expectedApi request generated using the swagger UI
note how the passed values are
l=1,2
Manual api request
Now the values are passed as
-d 'l=1'
and-d 'l=2'
I am fairly sure that
l=1,2
should be an accepted way of passing a list of values as parameter, but fastapi doesn't seem to like it. Also, if this isn't the case, the Swagger UI doesn't produce the correct curl requests for lists given asForm
parameters.Packages:
fastapi
:0.66.0
python
:3.7.4
Let me know if you need other details!
Thank you for coming to my TED talk
The text was updated successfully, but these errors were encountered: