Skip to content

Commit ee052fd

Browse files
validate value of 'const' properties (helps with discriminated unions) (#1024)
For 'const' properties, the parsed value will now be checked against the value in the schema at runtime and a ValueError will be raised if they do not match. Until now, the values would just be `cast` to a `Literal` without checking. This helps with with parsing `oneOf` when matching a const property is the only way to resolve which of the types should be returned. i.e. if a `oneOf` contains two schemas, but the properties in one are a subset of the other, then the only way to know which one to return is to validate the `const` value. I know that for the most part this library parses but doesn't validate, but I don't see a way around it in this case. The spec itself mentions validation when talking about `oneOf`: ``` In OAS 3.0, a response payload MAY be described to be exactly one of any number of types: [...] which means the payload MUST, by validation, match exactly one of the schemas described by Cat, Dog, or Lizard. ``` Note that this also addresses some of the use cases for discriminated unions. Per the spec, the `a discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema.` so implementing discriminators would be a speedup but with this change we'll at least construct the correct type. I'm happy to take any feedback, or abandon this if you don't like fact that it is validation. I originally went to try to implement discriminated unions fully, but this seemed like a much easier step towards that that gets the same result. --------- Co-authored-by: Dylan Anthony <[email protected]>
1 parent 5d138fd commit ee052fd

File tree

5 files changed

+34
-4
lines changed

5 files changed

+34
-4
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
default: major
3+
---
4+
5+
# `const` values in responses are now validated at runtime
6+
7+
Prior to this version, `const` values returned from servers were assumed to always be correct. Now, if a server returns
8+
an unexpected value, the client will raise a `ValueError`. This should enable better usage with `oneOf`.
9+
10+
PR #1024. Thanks @peter-greenatlas!

end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py

+4
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ def _parse_response(
4646
) -> Optional[Literal["Why have a fixed response? I dunno"]]:
4747
if response.status_code == HTTPStatus.OK:
4848
response_200 = cast(Literal["Why have a fixed response? I dunno"], response.json())
49+
if response_200 != "Why have a fixed response? I dunno":
50+
raise ValueError(
51+
f"response_200 must match const 'Why have a fixed response? I dunno', got '{response_200}'"
52+
)
4953
return response_200
5054
if client.raise_on_unexpected_status:
5155
raise errors.UnexpectedStatus(response.status_code, response.content)

end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/models/post_const_path_body.py

+12-2
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,26 @@ def to_dict(self) -> Dict[str, Any]:
4646
@classmethod
4747
def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
4848
d = src_dict.copy()
49-
required = d.pop("required")
49+
required = cast(Literal["this always goes in the body"], d.pop("required"))
50+
if required != "this always goes in the body":
51+
raise ValueError(f"required must match const 'this always goes in the body', got '{required}'")
5052

5153
def _parse_nullable(data: object) -> Union[Literal["this or null goes in the body"], None]:
5254
if data is None:
5355
return data
56+
nullable_type_1 = cast(Literal["this or null goes in the body"], data)
57+
if nullable_type_1 != "this or null goes in the body":
58+
raise ValueError(
59+
f"nullable_type_1 must match const 'this or null goes in the body', got '{nullable_type_1}'"
60+
)
61+
return nullable_type_1
5462
return cast(Union[Literal["this or null goes in the body"], None], data)
5563

5664
nullable = _parse_nullable(d.pop("nullable"))
5765

58-
optional = d.pop("optional", UNSET)
66+
optional = cast(Union[Literal["this sometimes goes in the body"], Unset], d.pop("optional", UNSET))
67+
if optional != "this sometimes goes in the body" and not isinstance(optional, Unset):
68+
raise ValueError(f"optional must match const 'this sometimes goes in the body', got '{optional}'")
5969

6070
post_const_path_body = cls(
6171
required=required,

openapi_python_client/parser/properties/const.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from typing import Any, overload
3+
from typing import Any, ClassVar, overload
44

55
from attr import define
66

@@ -12,7 +12,7 @@
1212

1313
@define
1414
class ConstProperty(PropertyProtocol):
15-
"""A property representing a Union (anyOf) of other properties"""
15+
"""A property representing a const value"""
1616

1717
name: str
1818
required: bool
@@ -21,6 +21,7 @@ class ConstProperty(PropertyProtocol):
2121
python_name: PythonIdentifier
2222
description: str | None
2323
example: None
24+
template: ClassVar[str] = "const_property.py.jinja"
2425

2526
@classmethod
2627
def build(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{% macro construct(property, source) %}
2+
{{ property.python_name }} = cast({{ property.get_type_string() }} , {{ source }})
3+
if {{ property.python_name }} != {{ property.value }}{% if not property.required %}and not isinstance({{ property.python_name }}, Unset){% endif %}:
4+
raise ValueError(f"{{ property.name }} must match const {{ property.value }}, got '{{'{' + property.python_name + '}' }}'")
5+
{%- endmacro %}

0 commit comments

Comments
 (0)