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

🐛 Fix using Annotated in routers or path operations decorated multiple times #9315

Merged

Conversation

sharonyogev
Copy link
Contributor

@sharonyogev sharonyogev commented Mar 26, 2023

We need to deep clone the field_info to prevent ourself from mutating it. This allows multiple path or nested routers ,etc.

Should solve those discussions:
#9279
#9309
#9306

@sharonyogev sharonyogev marked this pull request as ready for review March 26, 2023 11:47
@github-actions
Copy link
Contributor

📝 Docs preview for commit 49c965c at: https://642030d2e2f56d1fc2f7a3e6--fastapi.netlify.app

@sharonyogev sharonyogev force-pushed the fix/deepcoy_fieldinfo_of_annotated_args branch from 49c965c to c21ac1f Compare March 27, 2023 06:14
We need to copy the field_info to prevent ourselves from
mutating it.  This allows multiple path or nested routers ,etc.
@sharonyogev sharonyogev force-pushed the fix/deepcoy_fieldinfo_of_annotated_args branch from c21ac1f to 069e485 Compare March 27, 2023 06:15
@@ -47,4 +47,4 @@ def test_websocket():
def test_websocket_invalid_path_doesnt_match():
with pytest.raises(WebSocketDisconnect):
with client.websocket_connect("/itemsx/portal-gun"):
pass
pass # pragma: nocover
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, locally I need this to have 100% coverage
I can revert this to keep the PR cleaner if needed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass never gets executed. I've found the same thing before.

Might client.websocket_connect("/itemsx/portal-gun") work, instead?

@github-actions
Copy link
Contributor

📝 Docs preview for commit 069e485 at: https://64213563bf51d1544b1883b2--fastapi.netlify.app

@sharonyogev sharonyogev changed the title Fix: deepclone FieldInfo from Annotated arguments Fix: copy FieldInfo from Annotated arguments Mar 27, 2023
@jhamman
Copy link

jhamman commented Mar 30, 2023

I ran into this issue today so decided to give this PR a test drive. Seems to solve the issue nicely. Thanks @sharonyogev!

@Darkheir
Copy link

This PR also fixed this issue on one of my projects !

@bkis
Copy link

bkis commented Mar 31, 2023

Same here: This fixed the issue I had with Annotated and APIRouter endpoints:

`Query` default value cannot be set in `Annotated` for 'some_param'. Set the default value with `=` instead.

... although set up as

@router.get("/", ...)
async def some_function(
    some_param: Annotated[
        list[str],
        Query(description="This is some parameter"),
    ] = []
)  # ...

@VikalpRusia
Copy link

Please have it merge ASAP

@PotatoPope
Copy link

This solved my issue, great work!

Copy link
Contributor

@yezz123 yezz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻

@hochstibe
Copy link

As stated in the discussion (actually a real issue), this fix solves my problem

Copy link
Contributor

@nzig nzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This fixes a bug I introduced in #4871.

fastapi/dependencies/utils.py Show resolved Hide resolved
@JordanZimmitti
Copy link

This will fix my issue as well! Any idea when this can get merged in?

@jesse-r-s-hines
Copy link

A monkey patch workaround for until this gets merged:

fastapi_analyze_param = fastapi.dependencies.utils.analyze_param
def fastapi_analyze_param_monkey_patch(*, annotation, **kwargs):
    """
    Workaround for until fastapi!9315 is merged.
    This needs to be patched in before you import/call any APIRouters
    """
    from typing import Annotated, get_args, get_origin
    from copy import copy
    if get_origin(annotation) is Annotated:
        annotation = Annotated[tuple(copy(arg) for arg in get_args(annotation))]
    return fastapi_analyze_param(annotation = annotation, **kwargs)
fastapi.dependencies.utils.analyze_param = fastapi_analyze_param_monkey_patch

@tiangolo tiangolo changed the title Fix: copy FieldInfo from Annotated arguments 🐛 Fix using Annotated in routers or path operations decorated multiple times Apr 13, 2023
@github-actions
Copy link
Contributor

📝 Docs preview for commit e11eb50 at: https://643840014d5e810cf59c606f--fastapi.netlify.app

@tiangolo tiangolo merged commit fdf66c8 into tiangolo:master Apr 13, 2023
8 checks passed
@tiangolo
Copy link
Owner

Awesome, great job @sharonyogev! 🙇

Thanks everyone for the comments and for testing it locally to confirm that it solved it for you. 👏

This will be released in the next hour or so in FastAPI 0.95.1 🎉

@sharonyogev sharonyogev deleted the fix/deepcoy_fieldinfo_of_annotated_args branch April 13, 2023 20:41
@sharonyogev
Copy link
Contributor Author

Thanks @tiangolo, glad I could help 😃

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

Successfully merging this pull request may close these issues.

None yet