fix(template): Fix deserialization of non-required properties (#334)

forest-benchling · web-flow · commit 351478f62ea9 · 2021-02-10T12:13:55.000-07:00
Also unifies None and Unset checking in deserialization to prevent future similar bugs.
diff --git a/end_to_end_tests/golden-record-custom/custom_e2e/api/tests/upload_file_tests_upload_post.py b/end_to_end_tests/golden-record-custom/custom_e2e/api/tests/upload_file_tests_upload_post.py
@@ -24,6 +24,8 @@ def _parse_response(*, response: httpx.Response) -> Optional[Union[
     if response.status_code == 422:
         response_422 = HTTPValidationError.from_dict(response.json())
 
+
+
         return response_422
     return None
 
diff --git a/end_to_end_tests/golden-record-custom/custom_e2e/models/a_model.py b/end_to_end_tests/golden-record-custom/custom_e2e/models/a_model.py
@@ -1,5 +1,5 @@
 import datetime
-from typing import Any, Dict, List, Optional, Type, TypeVar, Union, cast
+from typing import Any, Dict, List, Optional, Type, TypeVar, Union
 
 import attr
 from dateutil.parser import isoparse
@@ -28,6 +28,7 @@ class AModel:
     required_nullable: Optional[str]
     nullable_model: Optional[AModelNullableModel]
     nested_list_of_enums: Union[Unset, List[List[DifferentEnum]]] = UNSET
+    a_not_required_date: Union[Unset, datetime.date] = UNSET
     attr_1_leading_digit: Union[Unset, str] = UNSET
     not_required_nullable: Union[Unset, Optional[str]] = UNSET
     not_required_not_nullable: Union[Unset, str] = UNSET
@@ -60,6 +61,10 @@ def to_dict(self) -> Dict[str, Any]:
                 nested_list_of_enums.append(nested_list_of_enums_item)
 
         a_nullable_date = self.a_nullable_date.isoformat() if self.a_nullable_date else None
+        a_not_required_date: Union[Unset, str] = UNSET
+        if not isinstance(self.a_not_required_date, Unset):
+            a_not_required_date = self.a_not_required_date.isoformat()
+
         attr_1_leading_digit = self.attr_1_leading_digit
         required_nullable = self.required_nullable
         not_required_nullable = self.not_required_nullable
@@ -91,6 +96,8 @@ def to_dict(self) -> Dict[str, Any]:
         )
         if nested_list_of_enums is not UNSET:
             field_dict["nested_list_of_enums"] = nested_list_of_enums
+        if a_not_required_date is not UNSET:
+            field_dict["a_not_required_date"] = a_not_required_date
         if attr_1_leading_digit is not UNSET:
             field_dict["1_leading_digit"] = attr_1_leading_digit
         if not_required_nullable is not UNSET:
@@ -145,7 +152,12 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
         a_nullable_date = None
         _a_nullable_date = d.pop("a_nullable_date")
         if _a_nullable_date is not None:
-            a_nullable_date = isoparse(cast(str, _a_nullable_date)).date()
+            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 = isoparse(_a_not_required_date).date()
 
         attr_1_leading_digit = d.pop("1_leading_digit", UNSET)
 
@@ -158,19 +170,17 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
         nullable_model = None
         _nullable_model = d.pop("nullable_model")
         if _nullable_model is not None:
-            nullable_model = AModelNullableModel.from_dict(cast(Dict[str, Any], _nullable_model))
+            nullable_model = AModelNullableModel.from_dict(_nullable_model)
 
         not_required_model: Union[AModelNotRequiredModel, Unset] = UNSET
         _not_required_model = d.pop("not_required_model", UNSET)
         if not isinstance(_not_required_model, Unset):
-            not_required_model = AModelNotRequiredModel.from_dict(cast(Dict[str, Any], _not_required_model))
+            not_required_model = AModelNotRequiredModel.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 = AModelNotRequiredNullableModel.from_dict(
-                cast(Dict[str, Any], _not_required_nullable_model)
-            )
+            not_required_nullable_model = AModelNotRequiredNullableModel.from_dict(_not_required_nullable_model)
 
         a_model = cls(
             an_enum_value=an_enum_value,
@@ -180,6 +190,7 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
             model=model,
             nested_list_of_enums=nested_list_of_enums,
             a_nullable_date=a_nullable_date,
+            a_not_required_date=a_not_required_date,
             attr_1_leading_digit=attr_1_leading_digit,
             required_nullable=required_nullable,
             not_required_nullable=not_required_nullable,
diff --git a/end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_primitive_additional_properties.py b/end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_primitive_additional_properties.py
@@ -1,4 +1,4 @@
-from typing import Any, Dict, List, Type, TypeVar, Union, cast
+from typing import Any, Dict, List, Type, TypeVar, Union
 
 import attr
 
@@ -36,9 +36,7 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         a_date_holder: Union[ModelWithPrimitiveAdditionalPropertiesADateHolder, Unset] = UNSET
         _a_date_holder = d.pop("a_date_holder", UNSET)
         if not isinstance(_a_date_holder, Unset):
-            a_date_holder = ModelWithPrimitiveAdditionalPropertiesADateHolder.from_dict(
-                cast(Dict[str, Any], _a_date_holder)
-            )
+            a_date_holder = ModelWithPrimitiveAdditionalPropertiesADateHolder.from_dict(_a_date_holder)
 
         model_with_primitive_additional_properties = cls(
             a_date_holder=a_date_holder,
diff --git a/end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_union_property.py b/end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_union_property.py
@@ -46,15 +46,15 @@ def _parse_a_property(data: Any) -> Union[Unset, AnEnum, AnIntEnum]:
             try:
                 a_property = UNSET
                 _a_property = data
-                if _a_property is not None and _a_property is not UNSET:
+                if not isinstance(_a_property, Unset):
                     a_property = AnEnum(_a_property)
 
                 return a_property
             except:  # noqa: E722
                 pass
             a_property = UNSET
             _a_property = data
-            if _a_property is not None and _a_property is not UNSET:
+            if not isinstance(_a_property, Unset):
                 a_property = AnIntEnum(_a_property)
 
             return a_property
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
@@ -1,5 +1,5 @@
 import datetime
-from typing import Any, Dict, List, Optional, Type, TypeVar, Union, cast
+from typing import Any, Dict, List, Optional, Type, TypeVar, Union
 
 import attr
 from dateutil.parser import isoparse
@@ -28,6 +28,7 @@ class AModel:
     required_nullable: Optional[str]
     nullable_model: Optional[AModelNullableModel]
     nested_list_of_enums: Union[Unset, List[List[DifferentEnum]]] = UNSET
+    a_not_required_date: Union[Unset, datetime.date] = UNSET
     attr_1_leading_digit: Union[Unset, str] = UNSET
     not_required_nullable: Union[Unset, Optional[str]] = UNSET
     not_required_not_nullable: Union[Unset, str] = UNSET
@@ -60,6 +61,10 @@ def to_dict(self) -> Dict[str, Any]:
                 nested_list_of_enums.append(nested_list_of_enums_item)
 
         a_nullable_date = self.a_nullable_date.isoformat() if self.a_nullable_date else None
+        a_not_required_date: Union[Unset, str] = UNSET
+        if not isinstance(self.a_not_required_date, Unset):
+            a_not_required_date = self.a_not_required_date.isoformat()
+
         attr_1_leading_digit = self.attr_1_leading_digit
         required_nullable = self.required_nullable
         not_required_nullable = self.not_required_nullable
@@ -91,6 +96,8 @@ def to_dict(self) -> Dict[str, Any]:
         )
         if nested_list_of_enums is not UNSET:
             field_dict["nested_list_of_enums"] = nested_list_of_enums
+        if a_not_required_date is not UNSET:
+            field_dict["a_not_required_date"] = a_not_required_date
         if attr_1_leading_digit is not UNSET:
             field_dict["1_leading_digit"] = attr_1_leading_digit
         if not_required_nullable is not UNSET:
@@ -145,7 +152,12 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
         a_nullable_date = None
         _a_nullable_date = d.pop("a_nullable_date")
         if _a_nullable_date is not None:
-            a_nullable_date = isoparse(cast(str, _a_nullable_date)).date()
+            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 = isoparse(_a_not_required_date).date()
 
         attr_1_leading_digit = d.pop("1_leading_digit", UNSET)
 
@@ -158,19 +170,17 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
         nullable_model = None
         _nullable_model = d.pop("nullable_model")
         if _nullable_model is not None:
-            nullable_model = AModelNullableModel.from_dict(cast(Dict[str, Any], _nullable_model))
+            nullable_model = AModelNullableModel.from_dict(_nullable_model)
 
         not_required_model: Union[AModelNotRequiredModel, Unset] = UNSET
         _not_required_model = d.pop("not_required_model", UNSET)
         if not isinstance(_not_required_model, Unset):
-            not_required_model = AModelNotRequiredModel.from_dict(cast(Dict[str, Any], _not_required_model))
+            not_required_model = AModelNotRequiredModel.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 = AModelNotRequiredNullableModel.from_dict(
-                cast(Dict[str, Any], _not_required_nullable_model)
-            )
+            not_required_nullable_model = AModelNotRequiredNullableModel.from_dict(_not_required_nullable_model)
 
         a_model = cls(
             an_enum_value=an_enum_value,
@@ -180,6 +190,7 @@ def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.dat
             model=model,
             nested_list_of_enums=nested_list_of_enums,
             a_nullable_date=a_nullable_date,
+            a_not_required_date=a_not_required_date,
             attr_1_leading_digit=attr_1_leading_digit,
             required_nullable=required_nullable,
             not_required_nullable=not_required_nullable,
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
@@ -1,4 +1,4 @@
-from typing import Any, Dict, List, Type, TypeVar, Union, cast
+from typing import Any, Dict, List, Type, TypeVar, Union
 
 import attr
 
@@ -36,9 +36,7 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         a_date_holder: Union[ModelWithPrimitiveAdditionalPropertiesADateHolder, Unset] = UNSET
         _a_date_holder = d.pop("a_date_holder", UNSET)
         if not isinstance(_a_date_holder, Unset):
-            a_date_holder = ModelWithPrimitiveAdditionalPropertiesADateHolder.from_dict(
-                cast(Dict[str, Any], _a_date_holder)
-            )
+            a_date_holder = ModelWithPrimitiveAdditionalPropertiesADateHolder.from_dict(_a_date_holder)
 
         model_with_primitive_additional_properties = cls(
             a_date_holder=a_date_holder,
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
@@ -46,15 +46,15 @@ def _parse_a_property(data: Any) -> Union[Unset, AnEnum, AnIntEnum]:
             try:
                 a_property = UNSET
                 _a_property = data
-                if _a_property is not None and _a_property is not UNSET:
+                if not isinstance(_a_property, Unset):
                     a_property = AnEnum(_a_property)
 
                 return a_property
             except:  # noqa: E722
                 pass
             a_property = UNSET
             _a_property = data
-            if _a_property is not None and _a_property is not UNSET:
+            if not isinstance(_a_property, Unset):
                 a_property = AnIntEnum(_a_property)
 
             return a_property
diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json
@@ -663,6 +663,11 @@
             "format": "date",
             "nullable": true
           },
+          "a_not_required_date": {
+            "title": "A Nullable Date",
+            "type": "string",
+            "format": "date",
+          },
           "1_leading_digit": {
             "title": "Leading Digit",
             "type": "string"
diff --git a/openapi_python_client/templates/property_templates/date_property.py.jinja b/openapi_python_client/templates/property_templates/date_property.py.jinja
@@ -1,12 +1,11 @@
-{% macro construct(property, source, initial_value="None") %}
-{% if property.required and not property.nullable %}
-{{ property.python_name }} = isoparse({{ source }}).date()
-{% else %}
-{{ property.python_name }} = {{ initial_value }}
-_{{ property.python_name }} = {{ source }}
-if _{{ property.python_name }} is not None:
-    {{ property.python_name }} = isoparse(cast(str, _{{ property.python_name }})).date()
-{% endif %}
+{% macro construct_function(property, source) %}
+isoparse({{ source }}).date()
+{% endmacro %}
+
+{% from "property_templates/property_macros.py.jinja" import construct_template %}
+
+{% macro construct(property, source, initial_value=None) %}
+{{ construct_template(construct_function, property, source, initial_value=initial_value) }}
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True) %}
diff --git a/openapi_python_client/templates/property_templates/datetime_property.py.jinja b/openapi_python_client/templates/property_templates/datetime_property.py.jinja
@@ -1,17 +1,11 @@
-{% macro construct(property, source, initial_value="None") %}
-{% if property.required %}
-{% if property.nullable %}
-{{ property.python_name }} = {{ source }}
-{{ property.python_name }} = isoparse({{ property.python_name }}) if {{ property.python_name }} else None
-{% else %}
-{{ property.python_name }} = isoparse({{ source }})
-{% endif %}
-{% else %}
-{{ property.python_name }} = {{ initial_value }}
-_{{ property.python_name }} = {{ source }}
-if _{{ property.python_name }} is not None:
-    {{ property.python_name }} = isoparse(cast(str, _{{ property.python_name }}))
-{% endif %}
+{% macro construct_function(property, source) %}
+isoparse({{ source }})
+{% endmacro %}
+
+{% from "property_templates/property_macros.py.jinja" import construct_template %}
+
+{% macro construct(property, source, initial_value=None) %}
+{{ construct_template(construct_function, property, source, initial_value=initial_value) }}
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True) %}
diff --git a/openapi_python_client/templates/property_templates/enum_property.py.jinja b/openapi_python_client/templates/property_templates/enum_property.py.jinja
@@ -1,12 +1,11 @@
-{% macro construct(property, source, initial_value="None") %}
-{% if property.required %}
-{{ property.python_name }} = {{ property.reference.class_name }}({{ source }})
-{% else %}
-{{ property.python_name }} = {{ initial_value }}
-_{{ property.python_name }} = {{ source }}
-if _{{ property.python_name }} is not None and _{{ property.python_name }} is not UNSET:
-    {{ property.python_name }} = {{ property.reference.class_name }}(_{{ property.python_name }})
-{% endif %}
+{% macro construct_function(property, source) %}
+{{ property.reference.class_name }}({{ source }})
+{% endmacro %}
+
+{% from "property_templates/property_macros.py.jinja" import construct_template %}
+
+{% macro construct(property, source, initial_value=None) %}
+{{ construct_template(construct_function, property, source, initial_value=initial_value) }}
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True) %}
diff --git a/openapi_python_client/templates/property_templates/file_property.py.jinja b/openapi_python_client/templates/property_templates/file_property.py.jinja
@@ -1,9 +1,15 @@
-{% macro construct(property, source, initial_value=None) %}
-{{ property.python_name }} = File(
+{% macro construct_function(property, source) %}
+File(
      payload = BytesIO({{ source }})
 )
 {% endmacro %}
 
+{% from "property_templates/property_macros.py.jinja" import construct_template %}
+
+{% macro construct(property, source, initial_value=None) %}
+{{ construct_template(construct_function, property, source, initial_value=initial_value) }}
+{% endmacro %}
+
 {% macro transform(property, source, destination, declare_type=True) %}
 {% if property.required %}
 {% if property.nullable %}
diff --git a/openapi_python_client/templates/property_templates/list_property.py.jinja b/openapi_python_client/templates/property_templates/list_property.py.jinja
@@ -4,7 +4,7 @@
 {% set inner_source = inner_property.python_name + "_data" %}
 {{ property.python_name }} = {{ initial_value }}
 _{{ property.python_name }} = {{ source }}
-{% if property.required %}
+{% if property.required and not property.nullable %}
 for {{ inner_source }} in (_{{ property.python_name }}):
 {% else %}
 for {{ inner_source }} in (_{{ property.python_name }} or []):
diff --git a/openapi_python_client/templates/property_templates/model_property.py.jinja b/openapi_python_client/templates/property_templates/model_property.py.jinja
@@ -1,18 +1,11 @@
+{% macro construct_function(property, source) %}
+{{ property.reference.class_name }}.from_dict({{ source }})
+{% endmacro %}
+
+{% from "property_templates/property_macros.py.jinja" import construct_template %}
+
 {% macro construct(property, source, initial_value=None) %}
-{% if property.required and not property.nullable %}
-{{ property.python_name }} = {{ property.reference.class_name }}.from_dict({{ 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 %}
-_{{ 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.reference.class_name }}.from_dict(cast(Dict[str, Any], _{{ property.python_name }}))
-{% endif %}
+{{ construct_template(construct_function, property, source, initial_value=initial_value) }}
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True) %}
diff --git a/openapi_python_client/templates/property_templates/property_macros.py.jinja b/openapi_python_client/templates/property_templates/property_macros.py.jinja
@@ -0,0 +1,16 @@
+{% 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 %}
+_{{ 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 }} = {{ construct_function(property, "_" + property.python_name) }}
+{% endif %}
+{% endmacro %}
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
diff --git a/tests/test_templates/test_property_templates/test_date_property/required_not_null.py b/tests/test_templates/test_property_templates/test_date_property/required_not_null.py
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
diff --git a/tests/test_templates/test_property_templates/test_date_property/test_date_property.py b/tests/test_templates/test_property_templates/test_date_property/test_date_property.py
