django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

[BUG] Query parameters typed as Optional[List[...]] fail when at least one value provided

Open OtherBarry opened this issue 3 years ago • 5 comments
trafficstars

Problem NB: A very similar issue was reported in #181, however it appears it was only resolved for non-optional types.

When using an query parameter typed as an optional list, e.g. Optional[List[str]], any values provided give the error:

{
  "detail": [
    {
      "loc": [
        "query",
        "<param name>"
      ],
      "msg": "value is not a valid list",
      "type": "type_error.list"
    }
  ]
}

Example

@router.get("/test")
def test(request, values: Optional[List[str]] = Query(None)) -> str:
    if values is None:
        return "No values"
    return "|".join(values)

A GET request to /test will return "No values" as expected, however a GET request to /test?values=foo or /test?values=foo&values=bar will result in a 422 response, with the error listed above.

Workaround There is a workaround that I found, however it is not an ideal solution.

class QueryParams(Schema):
    values: Optional[List[str]] = Field(None)


@router.get("/test")
def test(request, params: QueryParams = Query(...)) -> str:
    if params.values is None:
        return "No values"
    return "|".join(params.values)

Versions

  • Python version: 3.10.4
  • Django version: 3.2.14
  • Django-Ninja version: 0.19.1

OtherBarry avatar Aug 26 '22 02:08 OtherBarry

Hi @OtherBarry

I think it some general issue with Optional marking - for you now you can skip it by passing [] empty list as default value:

@router.get("/test")
def test(request, values: list[str] = Query([])) -> str:
    if not values:
        return "No values"
    return "|".join(values)

vitalik avatar Aug 26 '22 07:08 vitalik

I have a similar problem

@router.get("/")
def get_many(request: WSGIRequest, id: Optional[int] = None) -> Iterator[Hero]:
    pass

GET /id=

django ninja responds with

{"detail": [{"loc": ["query", "id"], "msg": "value is not a valid integer", "type": "type_error.integer"}]}

I assume the problem is that the query param id= is read as a string and django ninja tries to convert "" to int. However, that makes no sense as it should be converted to None.

As far as I can think, there exist no logical reasons for an client to input a blank query int param, and not want it to interpreted as None.

Andrioden avatar Aug 28 '22 14:08 Andrioden

@vitalik yeah that's a cleaner way to do it. I always forget you can use mutable defaults with pydantic.

@Andrioden not including the id param at all would be the standard way to indicate None I would think. You can probably create a custom type as a workaround, see here.

OtherBarry avatar Aug 28 '22 23:08 OtherBarry

@Andrioden

try change : Optional[int] = None to : int = None (Remove the optional - I think generally Optional is currently have some bug)

vitalik avatar Aug 29 '22 06:08 vitalik

@Andrioden not including the id param at all would be the standard way to indicate None I would think. You can probably create a custom type as a workaround, see https://github.com/pydantic/pydantic/discussions/2687.

@OtherBarry - Thanks for this tip

try change : Optional[int] = None to : int = None (Remove the optional - I think generally Optional is currently have some bug)

@vitalik - I tried this, and several other variations, farily certain there is no way around it with normal types.


All things considered, i do still suggest the default behavior of django ninja is that a blank int typed input query param is interpreted as None. If this is not possible or wanted due to django ninja using native pydantic type conversion, then i suggest documenting the behavior here.

I did end up solving this for my case by filtering this out on the client side, but i still generally believe backends should be as flexible as possible as long as there is no chance of confusion. = P

Andrioden avatar Aug 29 '22 07:08 Andrioden

The latest ninja beta made this a problem for us. This used to be fine, but now throws an error. Seems like the issue might be in how is_collection_type works? __ninja_collection_fields__ is simply empty for this schema.

    state: Optional[List[int]] = None

shughes-uk avatar Sep 18 '23 07:09 shughes-uk