-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Unify response validation #339
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import logging | ||
from dataclasses import dataclass | ||
from typing import ( | ||
TYPE_CHECKING, | ||
Any, | ||
|
@@ -8,9 +9,11 @@ | |
NamedTuple, | ||
Optional, | ||
TypeVar, | ||
Union, | ||
) | ||
|
||
from .._types import ModelType | ||
from .._pydantic import ValidationError, is_root_model, serialize_model_instance | ||
from .._types import JsonType, ModelType, OptionalModelType | ||
from ..config import Configuration | ||
from ..response import Response | ||
|
||
|
@@ -122,3 +125,65 @@ def get_func_operation_id(self, func: Callable, path: str, method: str): | |
if not operation_id: | ||
operation_id = f"{method.lower()}_{path.replace('/', '_')}" | ||
return operation_id | ||
|
||
|
||
@dataclass(frozen=True) | ||
class RawResponsePayload: | ||
payload: Union[JsonType, bytes] | ||
|
||
|
||
@dataclass(frozen=True) | ||
class ResponseValidationResult: | ||
validated_response_payload: Any | ||
|
||
|
||
def validate_response( | ||
skip_validation: bool, | ||
validation_model: OptionalModelType, | ||
response_payload: Any, | ||
): | ||
"""Validate a given `response_payload` against a `validation_model`. | ||
|
||
:param skip_validation: When set to true, validation is not carried out | ||
and the input `response_payload` is returned as-is. This is equivalent | ||
to not providing a `validation_model`. | ||
:param validation_model: Pydantic model used to validate the provided | ||
`response_payload`. | ||
:param response_payload: Validated response payload. A `RawResponsePayload` | ||
should be provided when the plugin view function returned an already | ||
JSON-serialized response payload. | ||
""" | ||
final_response_payload = None | ||
if isinstance(response_payload, RawResponsePayload): | ||
final_response_payload = response_payload.payload | ||
elif skip_validation or validation_model is None: | ||
final_response_payload = response_payload | ||
|
||
if not skip_validation and validation_model and not final_response_payload: | ||
if is_root_model(validation_model) and not isinstance( | ||
response_payload, validation_model | ||
): | ||
# Make it possible to return an instance of the model __root__ type | ||
# (i.e. not the root model itself). | ||
try: | ||
response_payload = validation_model(__root__=response_payload) | ||
except ValidationError: | ||
raise | ||
else: | ||
skip_validation = True | ||
final_response_payload = serialize_model_instance(response_payload) | ||
elif isinstance(response_payload, validation_model): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we use: if isinstance(response_payload, validation_model):
....
elif is_root_model(validation_model) |
||
skip_validation = True | ||
final_response_payload = serialize_model_instance(response_payload) | ||
else: | ||
final_response_payload = response_payload | ||
|
||
if validation_model and not skip_validation: | ||
validator = ( | ||
validation_model.parse_raw | ||
if isinstance(final_response_payload, bytes) | ||
else validation_model.parse_obj | ||
) | ||
validator(final_response_payload) | ||
|
||
return ResponseValidationResult(validated_response_payload=final_response_payload) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,11 @@ | |
from flask import Blueprint, abort, current_app, jsonify, make_response, request | ||
from werkzeug.routing import parse_converter_args | ||
|
||
from .._pydantic import ( | ||
BaseModel, | ||
ValidationError, | ||
is_root_model, | ||
serialize_model_instance, | ||
) | ||
from .._pydantic import ValidationError | ||
from .._types import ModelType | ||
from ..response import Response | ||
from ..utils import get_multidict_items, werkzeug_parse_rule | ||
from .base import BasePlugin, Context | ||
from .base import BasePlugin, Context, RawResponsePayload, validate_response | ||
|
||
|
||
class FlaskPlugin(BasePlugin): | ||
|
@@ -206,63 +201,38 @@ def validate( | |
|
||
status = 200 | ||
rest = [] | ||
if resp and isinstance(result, tuple) and isinstance(result[0], BaseModel): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tell me if I'm missing something, but I think we always want to get
Granted it's a bit of an edge case, but the right thing to do IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree. |
||
if resp and isinstance(result, tuple): | ||
if len(result) > 1: | ||
model, status, *rest = result | ||
response_payload, status, *rest = result | ||
else: | ||
model = result[0] | ||
response_payload = result[0] | ||
else: | ||
model = result | ||
response_payload = result | ||
|
||
if not skip_validation and resp and not isinstance(result, flask.Response): | ||
expect_model = resp.find_model(status) | ||
if resp.expect_list_result(status) and isinstance(model, list): | ||
expected_list_item_type = resp.get_expected_list_item_type(status) | ||
if all(isinstance(entry, expected_list_item_type) for entry in model): | ||
skip_validation = True | ||
result = ( | ||
[ | ||
( | ||
serialize_model_instance(entry) | ||
if isinstance(entry, BaseModel) | ||
else entry | ||
) | ||
for entry in model | ||
], | ||
try: | ||
response_validation_result = validate_response( | ||
skip_validation=skip_validation, | ||
validation_model=resp.find_model(status) if resp else None, | ||
response_payload=( | ||
RawResponsePayload(payload=response_payload.get_json()) | ||
if ( | ||
isinstance(response_payload, flask.Response) | ||
and not skip_validation | ||
) | ||
else response_payload | ||
), | ||
) | ||
except ValidationError: | ||
response = make_response( | ||
jsonify({"message": "response validation error"}), 500 | ||
) | ||
else: | ||
response = make_response( | ||
( | ||
response_validation_result.validated_response_payload, | ||
status, | ||
*rest, | ||
) | ||
elif ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll notice that this branch is no longer present in the new |
||
expect_model | ||
and is_root_model(expect_model) | ||
and not isinstance(model, expect_model) | ||
): | ||
# Make it possible to return an instance of the model __root__ type | ||
# (i.e. not the root model itself). | ||
try: | ||
model = expect_model(__root__=model) | ||
except ValidationError as err: | ||
resp_validation_error = err | ||
else: | ||
skip_validation = True | ||
result = (serialize_model_instance(model), status, *rest) | ||
elif expect_model and isinstance(model, expect_model): | ||
skip_validation = True | ||
result = (serialize_model_instance(model), status, *rest) | ||
|
||
response = make_response(result) | ||
|
||
if resp and resp.has_model() and not resp_validation_error: | ||
model = resp.find_model(response.status_code) | ||
if model and not skip_validation: | ||
try: | ||
model.parse_obj(response.get_json()) | ||
except ValidationError as err: | ||
resp_validation_error = err | ||
|
||
if resp_validation_error: | ||
response = make_response( | ||
jsonify({"message": "response validation error"}), 500 | ||
) | ||
|
||
after(request, response, resp_validation_error, None) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
payload
is enough since the outer class name already contains "validation"