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

✨ Add support for adding multiple examples in request bodies and path, query, cookie, and header params #1267

Merged
merged 11 commits into from May 5, 2021

Conversation

austinorr
Copy link
Contributor

@austinorr austinorr commented Apr 15, 2020

This PR enables multiple examples in a pick-list style dropdown in the OpenApi Docs a la the 'Request Body Examples' here

For more information and discussion see the issue this PR related to #822

Note, this still needs tests to cover the 3 new lines. Docs and full coverage tests added Sept 4 with 8138ff6


Edit by @tiangolo πŸ€“ πŸ‘‡

Added support for arguments example and examples in Body(), Path(), Query(), Cookie(), Header(), handling compatibility (and incompatibilities) between OpenAPI, JSON Schema, and OpenAPI's 3.0.x custom version of JSON Schema πŸ€ͺ πŸ’«

More details about the standards dizzy mix in a comment below, near the end.

@codecov
Copy link

codecov bot commented Apr 15, 2020

Codecov Report

Merging #1267 (a77098f) into master (b3f139c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            master     #1267    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          243       245     +2     
  Lines         7419      7539   +120     
==========================================
+ Hits          7419      7539   +120     
Impacted Files Coverage Ξ”
fastapi/openapi/utils.py 100.00% <100.00%> (ΓΈ)
fastapi/param_functions.py 100.00% <100.00%> (ΓΈ)
fastapi/params.py 100.00% <100.00%> (ΓΈ)
tests/test_schema_extra_examples.py 100.00% <100.00%> (ΓΈ)
...rial/test_schema_extra_example/test_tutorial004.py 100.00% <100.00%> (ΓΈ)

Continue to review full report at Codecov.

Legend - Click here to learn more
Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data
Powered by Codecov. Last update 3e32eb5...a77098f. Read the comment docs.

fastapi/openapi/utils.py Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2020

πŸ“ Docs preview for commit 8138ff61822a5d944b284e436ecc8a88475e893c at: https://5f52f468e319ee0ebf5d7aad--fastapi.netlify.app

@austinorr
Copy link
Contributor Author

The docs built for this PR have a screenshot of the new multi-example capabilities here.

@kbakk
Copy link

kbakk commented Nov 19, 2020

@austinorr The description mentions the need for more coverage but you seem to have added tests - is it ready to be merged perhaps? It would be a really useful feature! πŸ˜ƒ

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Nov 19, 2020

@kbakk docs_src is not being covered, maybe that's why...

check this: #1904

@austinorr
Copy link
Contributor Author

@kbakk and @Kludex, I was thinking this PR is ready to go. Is there some reason merging this PR should wait on #1904? If not, I think this is fully tested (relative to the master branch).

I could also push up an empty commit to try to force a re-run of the tests/docs/coverage if that'd be useful.

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Nov 20, 2020

@austinorr The goal of my message was to explain one of the possible reasons for what he saw. I didn't mean that we should wait. πŸ˜…

I could also push up an empty commit to try to force a re-run of the tests/docs/coverage if that'd be useful.

I don't think this is necessary... @tiangolo will eventually see this PR, dw πŸ‘

@mattnotmitt
Copy link

@tiangolo Is there any chance of seeing this merged soon?

@austinorr
Copy link
Contributor Author

For easier review of the capability this PR enables see the (draft) docs I edited here: https://5f52f468e319ee0ebf5d7aad--fastapi.netlify.app/tutorial/schema-extra-example/#body-with-multiple-examples

@kbakk
Copy link

kbakk commented Jan 11, 2021

Note, this still needs tests to cover the 3 new lines.

If this isn't the case anymore, maybe it should be removed from the description. :)

@austinorr
Copy link
Contributor Author

austinorr commented Jan 11, 2021

If this isn't the case anymore, maybe it should be removed from the description. :)

Hey sorry -- we use git comments differently. I will update the original comment to indicate that this has been covered since the commits on Sept 4. The Coverage bot is current (it updates live).

@adriangb
Copy link
Contributor

adriangb commented Jan 19, 2021

Coming over from #822 (comment).

I tried the implementation in this PR with Pydantic models as described in #822 (comment) and it did not seem to work. That said, there is a good chance I made a mistake somewhere along the way.

Maybe this example can be updated/extended and it can be tested that way? I'd be happy to try to submit a PR to this PR with this (but I'm not very familiar with the internals, so I can't promise I'll get it to work).

Here is my test (which for me, does not show the dropdown):

Code
# monkeypatch in this implementation
from fastapi.openapi import utils

def get_openapi_operation_request_body(*, body_field, model_name_map):
    if not body_field:
        return None
    assert isinstance(body_field, utils.ModelField)
    # ignore mypy error until enum schemas are released
    body_schema, _, _ = utils.field_schema(
        body_field, model_name_map=model_name_map, ref_prefix=utils.REF_PREFIX  # type: ignore
    )
    field_info = utils.cast(utils.Body, body_field.field_info)
    request_media_type = field_info.media_type
    required = body_field.required
    request_body_oai= {}
    if required:
        request_body_oai["required"] = required
    request_body_oai["content"] = {request_media_type: {"schema": body_schema}}
    examples = body_schema.get("examples")
    if examples:
        request_body_oai["content"][request_media_type]["examples"] = examples
    return request_body_oai

utils.get_openapi_operation_request_body = get_openapi_operation_request_body

# the rest
from typing import Optional

from fastapi import FastAPI, Body
from pydantic import BaseModel

app = FastAPI()

class Item(BaseModel):
    name: str
    description: Optional[str] = None
    price: float
    tax: Optional[float] = None

    class Config:
        schema_extra = {
            "examples": {
                "Foo": {
                    "name": "Foo",
                    "description": "A very nice Item",
                    "price": 35.4,
                    "tax": 3.2,
                },
                "Bar": {
                    "name": "Bar",
                    "description": "A Bar Item",
                    "price": 35.1,
                    "tax": 3.5,
                }
            }
        }


@app.put("/items/{item_id}")
async def update_item(item_id: int, item: Item):
    results = {"item_id": item_id, "item": item}
    return results

However, if I change the function signature from:

async def update_item(item_id: int, item: Item):

to

async def update_item(item_id: int, item: Item = Body(None, examples=Item.Config.schema_extra["examples"])):

Then I get a dropdown, as described in this PR.

@austinorr
Copy link
Contributor Author

Yeah, I see what you mean. The current implementation doesn't work with the schema_extra. If we think this through a bit more, the openapi examples picklist relate 1:1 with each route, since the examples are bound to the media type. This is really different from the schema_extras and the existing example (singular) method for creating reference examples; those are 1:1 for each model.

You're right to note that my implementation is a bit narrow in that it'll only work as expected if the developer creates a route with only one Body within it. fastapi has other Body params that only work if there's 1 Body though, like the embed kwarg so my hope was that this hack could sneak through since I'd still find it useful.

The openapi.models module has a MediaType model that appears to already be configured correctly, but I don't yet see anywhere in the fastapi library that initializes this object or offers an entrypoint for a developer to initialize it with a value for examples... so in order to keep my diff down I just lifted the key off of the Body if it's there (since there is no valid plural examples key defined on a body according openapi and the fastapi docs) and I just injected into the media type dict. I'd like to avoid any solutions that require the user to define a MediaType model on a per-route basis. This examples kwarg should either be on the route or on the Body with the same limits as the embed kwarg (I've chosen the latter since the diff was just 2 lines.)

@adriangb
Copy link
Contributor

That makes sense, I think is it pretty sensible to support the simple use case (in terms of implementation and expected behavior, as you point out above). There is certainly a discrepancy between examples-per-route (OpenAPI) and examples-per-model (Pydantic), but that is somehow reconciled within FastAPI, since example in schema_extra does work.

That said, should also be made to work with examples in schema_extra, but maybe in a separate PR. I took a quick peek under the hood, the only thing I could see that stood out is that SchemaBase has example defined (here) but not examples. This made examples immediately be dropped from schema_extra.

@austinorr
Copy link
Contributor Author

@adriangb That's right, my understanding is that there's actually nothing we can do with examples on schema_base, since openapi doesn't support that key on the body, only on the media type. It's not just a different PR, it'd be a PR to the openapi repo to bring model-level dropdown pickers to the swagger docs (that'd be really cool).

The example (singular) key works within the current paradigm because we can build up our Body from multiple models and create a valid overall example as long as every model has exactly one example. Otherwise we'd have a 'tree' of examples that we'd have to prune, and the behavior would likely differ from the developers intent.

I'll also admit that I'm way out over my skis here; I just use this fantastic tool, I don't know how it really does some of its magic like the core devs do. Some of this might actually be possible/easier than I thought if we can get this thread reviewed by a dev who knows openapi, fastapi and pydantic really well.

@adriangb
Copy link
Contributor

That makes a lot of sense, your explanation is excellent.

it'd be a PR to the openapi repo to bring model-level dropdown pickers to the swagger docs

That would be cool! But I'm not sure how that fits into the larger OpenAPI picture? Like you I just use this great tool πŸ˜…

we'd have a 'tree' of examples that we'd have to prune

Right, and there's not valid way to select the right leaf. I'd imagine there'd need to be some FastAPI specific way for developers to designate the example to use in building the overall examples (it could just be the first one).

Some of this might actually be possible/easier than I thought if we can get this thread reviewed by a [core] dev

Do you want to tag someone here, or would you prefer to open a separate issue? Up to you.

@adriangb
Copy link
Contributor

Looking at this a bit more, it looks like FastAPI does nothing special with example, instead, it is Swagger itself that has the logic of "if the body is just a $ref to a schema in components, and that schema in components has an example (singular), then pull that example and show it as a body example in the path description".

So indeed, supporting examples from Pydantic models would be an OpenAPI request, not a FastAPI request, and thus your PR should remain as is. I'm sorry for the diversion @austinorr .

@talarari
Copy link

will this support multiple examples for a union type body?
i'm interested in having a route be able to take body that is either a single model or a list of models, so:

POST /users

could take body

{
   "name": "john"
}

or

[{
   "name": "john"
},
{
   "name": "jack"
}]

currently the example value for union body types is just empty.
would be great to have this be more descriptive.

@talarari
Copy link

any update on this? is there a plan to merge it?

@tiangolo tiangolo changed the title enable multiple examples per route in swagger docs ✨ Add support for adding multiple examples in request bodies and path, query, cookie, and header params May 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2021

πŸ“ Docs preview for commit a77098f at: https://6092ddfea9cea100a08ad24c--fastapi.netlify.app

@tiangolo tiangolo merged commit e10a437 into tiangolo:master May 5, 2021
@tiangolo
Copy link
Owner

tiangolo commented May 5, 2021

Thanks for all the effort and work on this @austinorr ! πŸ€“ And thanks for all the clarifications and arguments here and in the issue! πŸ“ πŸš€

Also thanks for the discussion everyone! β˜•


I finally got the time to work on this for several consecutive days (mainly studying the standards) and I refactored it a bit to handle a couple of things.

This was quite tricky, as although the description and original change were not too complex, it touches one of the main friction points in the standards, between OpenAPI, JSON Schema, OpenAPI's 3.0.x custom version of JSON Schema, and the recently released (soon to be supported) OpenAPI 3.1.0, based on a recent version of JSON Schema, without the custom parts. πŸ˜…

I added internal support for the arguments example and examples in Body(), Path(), Query(), Cookie(), Header(), handling the compatibility issues between those standards.

Boring Technical Details

The update/refactor adds example and examples (handling overrides from examples over example as specified in the spec) to each path operation's section in OpenAPI, not to the JSON Schema section, as the format for OpenAPI's examples is different than the format for examples in recent versions of JSON Schema. Although the currently supported OpenAPI 3.0.x doesn't support recent versions of JSON Schema, the recently release OpenAPI 3.1.0 is based on a recent JSON Schema, which includes its own definition of examples, so OpenAPI 3.1.0 no longer customizes its own version of JSON Schema as much as before, and the custom example in OpenAPI's own JSON Schema is although still supported, now deprecated. ...nevertheless, Swagger UI still doesn't support OpenAPI 3.1.0, so I won't update FastAPI yet.

Maybe all that sounds super complex 😬 ...it's mainly that the terms are the same here and there, but they refer to slightly different things in one standard and the other which adds to the confusion. Plus, the conflicts also involve older and newer versions, so it sounds dizzy. πŸ₯΄


But in short: this now has "first-class" support, handling all the details, making sure it's future proof, for OpenAPI 3.1.0 (:crossed_fingers: ). :sparkles:

And, now it also supports the other parameters, with a bunch of tests for all that to make sure overrides work correctly, etc. πŸŽ‰

This will be included in the next release, 0.64.0, in the next days. πŸŽ‰

@adriangb
Copy link
Contributor

adriangb commented May 5, 2021

Thank you @tiangolo and @austinorr!

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

8 participants