0b01001001 / spectree

API spec validator and OpenAPI document generator for Python web frameworks.
https://0b01001001.github.io/spectree/
Apache License 2.0
318 stars 74 forks source link

Support returning lists of unserialized models #335

Closed jean-edouard-boulanger closed 1 year ago

jean-edouard-boulanger commented 1 year ago

Description

Currently, spectree only partially supports validation of list responses. For example:

class User(BaseModel):
    user_id: str

@app.route()
@api.validate(
    resp=Response(HTTP_200=List[User])
)
def get_users():
    return [
        User(user_id="user1").dict(),  # pre-serialization of individual entries is required, or spectree crashes
        User(user_id="user2").dict()
    ]

You'll notice in particular that individual entries must currently be serialized before returning or spectree crashes. This is inconsistent with the rest of the interface, because it's perfectly okay to return models (without serializing them) - so why not list of models?

The change basically updates individual plugins to serialize individual list entries (when needed) before the final response is created. The change is conservative and the above logic is only applied when all of the below applies:

I have not changed any of the existing validation logic in this case: we still rely on the generated root model for validation.

Change summary:

jean-edouard-boulanger commented 1 year ago

Hi @jean-edouard-boulanger, if I understand it correctly, your expectation is to return a list like [User(user_id="1"), User(user_id="2")] directly. I think it's possible but will need to use some tricks.

Yes this is absolutely correct! I don't particularly like the alternative (using gen_list_model in the view) and I think it'd be much nicer if spectree allowed returning a list of models directly.

kemingy commented 1 year ago

Yes this is absolutely correct! I don't particularly like the alternative (using gen_list_model in the view) and I think it'd be much nicer if spectree allowed returning a list of models directly.

Yes, returning a list directly is more convenient.

As for the skip_validation part, if it can return both BaseModel instances and dictionaries, it might be hard to decide whether to validate it or not.

jean-edouard-boulanger commented 1 year ago

With the commit I have just pushed, we're getting much closer to your suggestion:

Couple of examples:

Example 1: all item types have the expected/specified type (User), validation is skipped

@app.route("/api/example1", methods=["GET"])
@api.validate(resp=Response(HTTP_200=List[User]))
def return_list():
    return [User(id=1), User(id=2)]

Example 2: some items are serialized, some items are not serialized, validation is delegated to Pydantic

@app.route("/api/example2", methods=["GET"])
@api.validate(resp=Response(HTTP_200=List[User]))
def return_list():
    return [User(id=1), {"id": 2}]

Example 3: all items are serialized, validation is delegated to Pydantic (existing use case)

@app.route("/api/example3", methods=["GET"])
@api.validate(resp=Response(HTTP_200=List[User]))
def return_list():
    return [{"id": 1}, {"id": 2}]

Example 4: none of the items have the expected type, validation is delegated to Pydantic (same as example 3)

@app.route("/api/example4", methods=["GET"])
@api.validate(resp=Response(HTTP_200=List[User]))
def return_list():
    return [OtherUserModel(id=1), OtherUserModel(id=2)]
jean-edouard-boulanger commented 1 year ago

@kemingy Common validation logic now implemented for both Falcon sync/async implementations (see FalconPlugin.response_validation). Tests were updated accordingly.

jean-edouard-boulanger commented 1 year ago

Thanks for the help merging this @kemingy 🙌

Btw, I have noticed that returning Pydantic root models from views is also not supported. Would you be interested in a fix for this too?

kemingy commented 1 year ago

Btw, I have noticed that returning Pydantic root models from views is also not supported. Would you be interested in a fix for this too?

I'll take #329. You can fix the views if you're interested. Thanks.