From 7f39f44b7489c4ade20fe4b25d97acc89bee05a4 Mon Sep 17 00:00:00 2001 From: Forest Tong Date: Sun, 9 May 2021 09:53:10 -0400 Subject: [PATCH 1/2] Handle optional nullable deserialization --- .../my_test_api_client/models/a_model.py | 72 +++++++++++-------- .../models/model_with_any_json_properties.py | 2 - ...el_with_primitive_additional_properties.py | 6 +- .../models/model_with_property_ref.py | 6 +- .../models/model_with_union_property.py | 14 ++-- .../model_with_union_property_inlined.py | 14 ++-- .../property_macros.py.jinja | 22 +++--- .../union_property.py.jinja | 2 - 8 files changed, 79 insertions(+), 59 deletions(-) diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py index 38ce8d114..0e2c06d83 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py @@ -186,7 +186,6 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: def _parse_a_camel_date_time(data: object) -> Union[datetime.date, datetime.datetime]: try: - a_camel_date_time_type_0: datetime.datetime if not isinstance(data, str): raise TypeError() a_camel_date_time_type_0 = isoparse(data) @@ -196,7 +195,6 @@ def _parse_a_camel_date_time(data: object) -> Union[datetime.date, datetime.date pass if not isinstance(data, str): raise TypeError() - a_camel_date_time_type_1: datetime.date a_camel_date_time_type_1 = isoparse(data).date() return a_camel_date_time_type_1 @@ -209,7 +207,6 @@ def _parse_a_camel_date_time(data: object) -> Union[datetime.date, datetime.date def _parse_one_of_models(data: object) -> Union[FreeFormModel, ModelWithUnionProperty]: try: - one_of_models_type_0: FreeFormModel if not isinstance(data, dict): raise TypeError() one_of_models_type_0 = FreeFormModel.from_dict(data) @@ -219,7 +216,6 @@ def _parse_one_of_models(data: object) -> Union[FreeFormModel, ModelWithUnionPro pass if not isinstance(data, dict): raise TypeError() - one_of_models_type_1: ModelWithUnionProperty one_of_models_type_1 = ModelWithUnionProperty.from_dict(data) return one_of_models_type_1 @@ -228,9 +224,11 @@ def _parse_one_of_models(data: object) -> Union[FreeFormModel, ModelWithUnionPro model = ModelWithUnionProperty.from_dict(d.pop("model")) - an_optional_allof_enum: Union[Unset, AnAllOfEnum] = UNSET _an_optional_allof_enum = d.pop("an_optional_allof_enum", UNSET) - if not isinstance(_an_optional_allof_enum, Unset): + an_optional_allof_enum: Union[Unset, AnAllOfEnum] + if isinstance(_an_optional_allof_enum, Unset): + an_optional_allof_enum = UNSET + else: an_optional_allof_enum = AnAllOfEnum(_an_optional_allof_enum) nested_list_of_enums = [] @@ -245,14 +243,18 @@ def _parse_one_of_models(data: object) -> Union[FreeFormModel, ModelWithUnionPro nested_list_of_enums.append(nested_list_of_enums_item) - a_nullable_date = None _a_nullable_date = d.pop("a_nullable_date") - if _a_nullable_date is not None: + a_nullable_date: Optional[datetime.date] + if _a_nullable_date is None: + a_nullable_date = None + else: a_nullable_date = isoparse(_a_nullable_date).date() - a_not_required_date: Union[Unset, datetime.date] = UNSET _a_not_required_date = d.pop("a_not_required_date", UNSET) - if not isinstance(_a_not_required_date, Unset): + a_not_required_date: Union[Unset, datetime.date] + if isinstance(_a_not_required_date, Unset): + a_not_required_date = UNSET + else: a_not_required_date = isoparse(_a_not_required_date).date() attr_1_leading_digit = d.pop("1_leading_digit", UNSET) @@ -267,7 +269,6 @@ def _parse_nullable_one_of_models(data: object) -> Union[FreeFormModel, ModelWit if data is None: return data try: - nullable_one_of_models_type_0: FreeFormModel if not isinstance(data, dict): raise TypeError() nullable_one_of_models_type_0 = FreeFormModel.from_dict(data) @@ -277,7 +278,6 @@ def _parse_nullable_one_of_models(data: object) -> Union[FreeFormModel, ModelWit pass if not isinstance(data, dict): raise TypeError() - nullable_one_of_models_type_1: ModelWithUnionProperty nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict(data) return nullable_one_of_models_type_1 @@ -288,12 +288,13 @@ def _parse_not_required_one_of_models(data: object) -> Union[FreeFormModel, Mode if isinstance(data, Unset): return data try: - not_required_one_of_models_type_0: Union[Unset, FreeFormModel] if not isinstance(data, dict): raise TypeError() - not_required_one_of_models_type_0 = UNSET _not_required_one_of_models_type_0 = data - if not isinstance(_not_required_one_of_models_type_0, Unset): + not_required_one_of_models_type_0: Union[Unset, FreeFormModel] + if isinstance(_not_required_one_of_models_type_0, Unset): + not_required_one_of_models_type_0 = UNSET + else: not_required_one_of_models_type_0 = FreeFormModel.from_dict(_not_required_one_of_models_type_0) return not_required_one_of_models_type_0 @@ -301,10 +302,11 @@ def _parse_not_required_one_of_models(data: object) -> Union[FreeFormModel, Mode pass if not isinstance(data, dict): raise TypeError() - not_required_one_of_models_type_1: Union[Unset, ModelWithUnionProperty] - not_required_one_of_models_type_1 = UNSET _not_required_one_of_models_type_1 = data - if not isinstance(_not_required_one_of_models_type_1, Unset): + not_required_one_of_models_type_1: Union[Unset, ModelWithUnionProperty] + if isinstance(_not_required_one_of_models_type_1, Unset): + not_required_one_of_models_type_1 = UNSET + else: not_required_one_of_models_type_1 = ModelWithUnionProperty.from_dict(_not_required_one_of_models_type_1) return not_required_one_of_models_type_1 @@ -319,12 +321,13 @@ def _parse_not_required_nullable_one_of_models( if isinstance(data, Unset): return data try: - not_required_nullable_one_of_models_type_0: Union[Unset, FreeFormModel] if not isinstance(data, dict): raise TypeError() - not_required_nullable_one_of_models_type_0 = UNSET _not_required_nullable_one_of_models_type_0 = data - if not isinstance(_not_required_nullable_one_of_models_type_0, Unset): + not_required_nullable_one_of_models_type_0: Union[Unset, FreeFormModel] + if isinstance(_not_required_nullable_one_of_models_type_0, Unset): + not_required_nullable_one_of_models_type_0 = UNSET + else: not_required_nullable_one_of_models_type_0 = FreeFormModel.from_dict( _not_required_nullable_one_of_models_type_0 ) @@ -333,12 +336,13 @@ def _parse_not_required_nullable_one_of_models( except: # noqa: E722 pass try: - not_required_nullable_one_of_models_type_1: Union[Unset, ModelWithUnionProperty] if not isinstance(data, dict): raise TypeError() - not_required_nullable_one_of_models_type_1 = UNSET _not_required_nullable_one_of_models_type_1 = data - if not isinstance(_not_required_nullable_one_of_models_type_1, Unset): + not_required_nullable_one_of_models_type_1: Union[Unset, ModelWithUnionProperty] + if isinstance(_not_required_nullable_one_of_models_type_1, Unset): + not_required_nullable_one_of_models_type_1 = UNSET + else: not_required_nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict( _not_required_nullable_one_of_models_type_1 ) @@ -352,19 +356,27 @@ def _parse_not_required_nullable_one_of_models( d.pop("not_required_nullable_one_of_models", UNSET) ) - nullable_model = None _nullable_model = d.pop("nullable_model") - if _nullable_model is not None: + nullable_model: Optional[ModelWithUnionProperty] + if _nullable_model is None: + nullable_model = None + else: nullable_model = ModelWithUnionProperty.from_dict(_nullable_model) - not_required_model: Union[Unset, ModelWithUnionProperty] = UNSET _not_required_model = d.pop("not_required_model", UNSET) - if not isinstance(_not_required_model, Unset): + not_required_model: Union[Unset, ModelWithUnionProperty] + if isinstance(_not_required_model, Unset): + not_required_model = UNSET + else: not_required_model = ModelWithUnionProperty.from_dict(_not_required_model) - not_required_nullable_model = None _not_required_nullable_model = d.pop("not_required_nullable_model", UNSET) - if _not_required_nullable_model is not None and not isinstance(_not_required_nullable_model, Unset): + not_required_nullable_model: Union[Unset, None, ModelWithUnionProperty] + if _not_required_nullable_model is None: + not_required_nullable_model = None + elif isinstance(_not_required_nullable_model, Unset): + not_required_nullable_model = UNSET + else: not_required_nullable_model = ModelWithUnionProperty.from_dict(_not_required_nullable_model) a_model = cls( diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py index ecfa97b10..08a016dd8 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py @@ -46,7 +46,6 @@ def _parse_additional_property( data: object, ) -> Union[List[str], ModelWithAnyJsonPropertiesAdditionalPropertyType0, bool, float, int, str]: try: - additional_property_type_0: ModelWithAnyJsonPropertiesAdditionalPropertyType0 if not isinstance(data, dict): raise TypeError() additional_property_type_0 = ModelWithAnyJsonPropertiesAdditionalPropertyType0.from_dict(data) @@ -55,7 +54,6 @@ def _parse_additional_property( except: # noqa: E722 pass try: - additional_property_type_1: List[str] if not isinstance(data, list): raise TypeError() additional_property_type_1 = cast(List[str], data) diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_primitive_additional_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_primitive_additional_properties.py index 5dc264152..ee28313bd 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_primitive_additional_properties.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_primitive_additional_properties.py @@ -33,9 +33,11 @@ def to_dict(self) -> Dict[str, Any]: @classmethod def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: d = src_dict.copy() - a_date_holder: Union[Unset, ModelWithPrimitiveAdditionalPropertiesADateHolder] = UNSET _a_date_holder = d.pop("a_date_holder", UNSET) - if not isinstance(_a_date_holder, Unset): + a_date_holder: Union[Unset, ModelWithPrimitiveAdditionalPropertiesADateHolder] + if isinstance(_a_date_holder, Unset): + a_date_holder = UNSET + else: a_date_holder = ModelWithPrimitiveAdditionalPropertiesADateHolder.from_dict(_a_date_holder) model_with_primitive_additional_properties = cls( diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_property_ref.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_property_ref.py index 6ebba75a6..e28a14e91 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_property_ref.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_property_ref.py @@ -31,9 +31,11 @@ def to_dict(self) -> Dict[str, Any]: @classmethod def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: d = src_dict.copy() - inner: Union[Unset, ModelName] = UNSET _inner = d.pop("inner", UNSET) - if not isinstance(_inner, Unset): + inner: Union[Unset, ModelName] + if isinstance(_inner, Unset): + inner = UNSET + else: inner = ModelName.from_dict(_inner) model_with_property_ref = cls( diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property.py index 87034d5e7..b7fa116e3 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property.py @@ -44,12 +44,13 @@ def _parse_a_property(data: object) -> Union[AnEnum, AnIntEnum, Unset]: if isinstance(data, Unset): return data try: - a_property_type_0: Union[Unset, AnEnum] if not isinstance(data, str): raise TypeError() - a_property_type_0 = UNSET _a_property_type_0 = data - if not isinstance(_a_property_type_0, Unset): + a_property_type_0: Union[Unset, AnEnum] + if isinstance(_a_property_type_0, Unset): + a_property_type_0 = UNSET + else: a_property_type_0 = AnEnum(_a_property_type_0) return a_property_type_0 @@ -57,10 +58,11 @@ def _parse_a_property(data: object) -> Union[AnEnum, AnIntEnum, Unset]: pass if not isinstance(data, int): raise TypeError() - a_property_type_1: Union[Unset, AnIntEnum] - a_property_type_1 = UNSET _a_property_type_1 = data - if not isinstance(_a_property_type_1, Unset): + a_property_type_1: Union[Unset, AnIntEnum] + if isinstance(_a_property_type_1, Unset): + a_property_type_1 = UNSET + else: a_property_type_1 = AnIntEnum(_a_property_type_1) return a_property_type_1 diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property_inlined.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property_inlined.py index 822f45cc2..27e91a80a 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property_inlined.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property_inlined.py @@ -46,12 +46,13 @@ def _parse_fruit( if isinstance(data, Unset): return data try: - fruit_type_0: Union[Unset, ModelWithUnionPropertyInlinedFruitType0] if not isinstance(data, dict): raise TypeError() - fruit_type_0 = UNSET _fruit_type_0 = data - if not isinstance(_fruit_type_0, Unset): + fruit_type_0: Union[Unset, ModelWithUnionPropertyInlinedFruitType0] + if isinstance(_fruit_type_0, Unset): + fruit_type_0 = UNSET + else: fruit_type_0 = ModelWithUnionPropertyInlinedFruitType0.from_dict(_fruit_type_0) return fruit_type_0 @@ -59,10 +60,11 @@ def _parse_fruit( pass if not isinstance(data, dict): raise TypeError() - fruit_type_1: Union[Unset, ModelWithUnionPropertyInlinedFruitType1] - fruit_type_1 = UNSET _fruit_type_1 = data - if not isinstance(_fruit_type_1, Unset): + fruit_type_1: Union[Unset, ModelWithUnionPropertyInlinedFruitType1] + if isinstance(_fruit_type_1, Unset): + fruit_type_1 = UNSET + else: fruit_type_1 = ModelWithUnionPropertyInlinedFruitType1.from_dict(_fruit_type_1) return fruit_type_1 diff --git a/openapi_python_client/templates/property_templates/property_macros.py.jinja b/openapi_python_client/templates/property_templates/property_macros.py.jinja index 92669cdc2..d578d1d4f 100644 --- a/openapi_python_client/templates/property_templates/property_macros.py.jinja +++ b/openapi_python_client/templates/property_templates/property_macros.py.jinja @@ -1,16 +1,20 @@ {% macro construct_template(construct_function, property, source, initial_value=None) %} {% if property.required and not property.nullable %} {{ property.python_name }} = {{ construct_function(property, source) }} -{% else %} -{% if initial_value != None %} -{{ property.python_name }} = {{ initial_value }} -{% elif property.nullable %} -{{ property.python_name }} = None -{% else %} -{{ property.python_name }}: {{ property.get_type_string() }} = UNSET -{% endif %} +{% else %}{# Must be nullable OR non-required #} _{{ property.python_name }} = {{ source }} -if {% if property.nullable %}_{{ property.python_name }} is not None{% endif %}{% if property.nullable and not property.required %} and {% endif %}{% if not property.required %}not isinstance(_{{ property.python_name }}, Unset){% endif %}: +{{ property.python_name }}: {{ property.get_type_string() }} + {% if property.nullable %} +if _{{ property.python_name }} is None: + {{ property.python_name }} = {% if initial_value != None %}{{ initial_value }}{% else %}None{% endif %} + + {% endif %} + {% if not property.required %} +{% if property.nullable %}elif{% else %}if{% endif %} isinstance(_{{ property.python_name }}, Unset): + {{ property.python_name }} = {% if initial_value != None %}{{ initial_value }}{% else %}UNSET{% endif %} + + {% endif %} +else: {{ property.python_name }} = {{ construct_function(property, "_" + property.python_name) }} {% endif %} {% endmacro %} diff --git a/openapi_python_client/templates/property_templates/union_property.py.jinja b/openapi_python_client/templates/property_templates/union_property.py.jinja index 684ae942d..87ea9820f 100644 --- a/openapi_python_client/templates/property_templates/union_property.py.jinja +++ b/openapi_python_client/templates/property_templates/union_property.py.jinja @@ -11,7 +11,6 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri {% for inner_property in property.inner_properties_with_template() %} {% if not loop.last or property.has_properties_without_templates %} try: - {{ inner_property.python_name }}: {{ inner_property.get_type_string() }} {% from "property_templates/" + inner_property.template import construct, check_type_for_construct %} if not {{ check_type_for_construct(inner_property, "data") }}: raise TypeError() @@ -23,7 +22,6 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri {% from "property_templates/" + inner_property.template import construct, check_type_for_construct %} if not {{ check_type_for_construct(inner_property, "data") }}: raise TypeError() - {{ inner_property.python_name }}: {{ inner_property.get_type_string() }} {{ construct(inner_property, "data", initial_value="UNSET") | indent(4) }} return {{ inner_property.python_name }} {% endif %} From 35e81f9049e8b5c622befbe34b057e0f4c32f393 Mon Sep 17 00:00:00 2001 From: Forest Tong Date: Sun, 9 May 2021 10:09:55 -0400 Subject: [PATCH 2/2] Fix test --- .../test_date_property/optional_nullable.py | 8 ++++++-- .../test_date_property/required_nullable.py | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_templates/test_property_templates/test_date_property/optional_nullable.py b/tests/test_templates/test_property_templates/test_date_property/optional_nullable.py index 0b44852df..23208c971 100644 --- a/tests/test_templates/test_property_templates/test_date_property/optional_nullable.py +++ b/tests/test_templates/test_property_templates/test_date_property/optional_nullable.py @@ -7,9 +7,13 @@ if not isinstance(some_source, Unset): some_destination = some_source.isoformat() if some_source else None -a_prop = None _a_prop = some_destination -if _a_prop is not None and not isinstance(_a_prop, Unset): +a_prop: Union[Unset, None, datetime.date] +if _a_prop is None: + a_prop = None +elif isinstance(_a_prop, Unset): + a_prop = UNSET +else: a_prop = isoparse(_a_prop).date() diff --git a/tests/test_templates/test_property_templates/test_date_property/required_nullable.py b/tests/test_templates/test_property_templates/test_date_property/required_nullable.py index f974c3210..79dd66ba4 100644 --- a/tests/test_templates/test_property_templates/test_date_property/required_nullable.py +++ b/tests/test_templates/test_property_templates/test_date_property/required_nullable.py @@ -4,9 +4,11 @@ from dateutil.parser import isoparse some_source = date(2020, 10, 12) some_destination = some_source.isoformat() if some_source else None -a_prop = None _a_prop = some_destination -if _a_prop is not None: +a_prop: Optional[datetime.date] +if _a_prop is None: + a_prop = None +else: a_prop = isoparse(_a_prop).date()