Fix nullable & required properties in multipart bodies (openapi-generators#995)

WIP Fix for openapi-generators#926

---------

Co-authored-by: Dylan Anthony <[email protected]>

Author: dbanty

.changeset/fix_nullable_and_required_properties_in_multipart_bodies.md

@@ -0,0 +1,10 @@
+---
+default: patch
+---
+
+# Fix nullable and required properties in multipart bodies
+
+Fixes #926.
+
+> [!WARNING]
+> This change is likely to break custom templates. Multipart body handling has been completely split from JSON bodies.

end_to_end_tests/baseline_openapi_3.0.json

@@ -1466,7 +1466,9 @@
     },
     "/naming/mixed-case": {
       "get": {
-        "tags": ["naming"],
+        "tags": [
+          "naming"
+        ],
         "operationId": "mixed_case",
         "parameters": [
           {
@@ -1487,30 +1489,32 @@
           }
         ],
         "responses": {
-            "200": {
-              "description": "Successful response",
-              "content": {
-                "application/json": {
-                  "schema": {
-                    "type": "object",
-                    "properties": {
-                      "mixed_case": {
-                        "type": "string"
-                      },
-                      "mixedCase": {
-                        "type": "string"
-                      }
+          "200": {
+            "description": "Successful response",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "object",
+                  "properties": {
+                    "mixed_case": {
+                      "type": "string"
+                    },
+                    "mixedCase": {
+                      "type": "string"
                     }
                   }
                 }
               }
             }
+          }
         }
       }
     },
     "/naming/{hyphen-in-path}": {
       "get": {
-        "tags": ["naming"],
+        "tags": [
+          "naming"
+        ],
         "operationId": "hyphen_in_path",
         "parameters": [
           {
@@ -1914,7 +1918,8 @@
         "required": [
           "some_file",
           "some_object",
-          "some_nullable_object"
+          "some_nullable_object",
+          "some_required_number"
         ],
         "type": "object",
         "properties": {
@@ -1947,6 +1952,15 @@
             "title": "Some Number",
             "type": "number"
           },
+          "some_nullable_number": {
+            "title": "Some Nullable Number",
+            "type": "number",
+            "nullable": true
+          },
+          "some_required_number": {
+            "title": "Some Required Number",
+            "type": "number"
+          },
           "some_array": {
             "title": "Some Array",
             "nullable": true,

end_to_end_tests/baseline_openapi_3.1.yaml

@@ -1928,7 +1928,8 @@ info:
       "required": [
         "some_file",
         "some_object",
-        "some_nullable_object"
+        "some_nullable_object",
+        "some_required_number"
       ],
       "type": "object",
       "properties": {
@@ -1961,6 +1962,14 @@ info:
           "title": "Some Number",
           "type": "number"
         },
+        "some_nullable_number": {
+          "title": "Some Nullable Number",
+          "type": [ "number", "null" ]
+        },
+        "some_required_number": {
+          "title": "Some Number",
+          "type": "number"
+        },
         "some_array": {
           "title": "Some Array",
           "type": [ "array", "null" ],

openapi_python_client/parser/properties/model_property.py

@@ -7,7 +7,9 @@
 
 from ... import Config, utils
 from ... import schema as oai
+from ...utils import PythonIdentifier
 from ..errors import ParseError, PropertyError
+from .any import AnyProperty
 from .enum_property import EnumProperty
 from .protocol import PropertyProtocol, Value
 from .schemas import Class, ReferencePath, Schemas, parse_reference_path
@@ -30,7 +32,7 @@ class ModelProperty(PropertyProtocol):
     optional_properties: list[Property] | None
     relative_imports: set[str] | None
     lazy_imports: set[str] | None
-    additional_properties: bool | Property | None
+    additional_properties: Property | None
     _json_type_string: ClassVar[str] = "Dict[str, Any]"
 
     template: ClassVar[str] = "model_property.py.jinja"
@@ -78,7 +80,7 @@ def build(
         optional_properties: list[Property] | None = None
         relative_imports: set[str] | None = None
         lazy_imports: set[str] | None = None
-        additional_properties: bool | Property | None = None
+        additional_properties: Property | None = None
         if process_properties:
             data_or_err, schemas = _process_property_data(
                 data=data, schemas=schemas, class_info=class_info, config=config, roots=model_roots
@@ -386,25 +388,37 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
     )
 
 
+ANY_ADDITIONAL_PROPERTY = AnyProperty.build(
+    name="additional",
+    required=True,
+    default=None,
+    description="",
+    python_name=PythonIdentifier(value="additional", prefix=""),
+    example=None,
+)
+
+
 def _get_additional_properties(
     *,
     schema_additional: None | (bool | (oai.Reference | oai.Schema)),
     schemas: Schemas,
     class_name: utils.ClassName,
     config: Config,
     roots: set[ReferencePath | utils.ClassName],
-) -> tuple[bool | (Property | PropertyError), Schemas]:
+) -> tuple[Property | None | PropertyError, Schemas]:
     from . import property_from_data
 
     if schema_additional is None:
-        return True, schemas
+        return ANY_ADDITIONAL_PROPERTY, schemas
 
     if isinstance(schema_additional, bool):
-        return schema_additional, schemas
+        if schema_additional:
+            return ANY_ADDITIONAL_PROPERTY, schemas
+        return None, schemas
 
     if isinstance(schema_additional, oai.Schema) and not any(schema_additional.model_dump().values()):
         # An empty schema
-        return True, schemas
+        return ANY_ADDITIONAL_PROPERTY, schemas
 
     additional_properties, schemas = property_from_data(
         name="AdditionalProperty",
@@ -425,7 +439,7 @@ def _process_property_data(
     class_info: Class,
     config: Config,
     roots: set[ReferencePath | utils.ClassName],
-) -> tuple[tuple[_PropertyData, bool | Property] | PropertyError, Schemas]:
+) -> tuple[tuple[_PropertyData, Property | None] | PropertyError, Schemas]:
     property_data = _process_properties(
         data=data, schemas=schemas, class_name=class_info.name, config=config, roots=roots
     )
@@ -442,7 +456,7 @@ def _process_property_data(
     )
     if isinstance(additional_properties, PropertyError):
         return additional_properties, schemas
-    elif isinstance(additional_properties, bool):
+    elif additional_properties is None:
         pass
     else:
         property_data.relative_imports.update(additional_properties.get_imports(prefix=".."))

openapi_python_client/parser/properties/protocol.py

@@ -107,6 +107,8 @@ def get_type_string(
         """
         if json:
             type_string = self.get_base_json_type_string(quoted=quoted)
+        elif multipart:
+            type_string = "Tuple[None, bytes, str]"
         else:
             type_string = self.get_base_type_string(quoted=quoted)
 

openapi_python_client/templates/endpoint_macros.py.jinja

@@ -83,8 +83,8 @@ params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
 {% macro multipart_body(body, destination) %}
 {% set property = body.prop %}
 {% import "property_templates/" + property.template as prop_template %}
-{% if prop_template.transform_multipart %}
-{{ prop_template.transform_multipart(property, property.python_name, destination) }}
+{% if prop_template.transform_multipart_body %}
+{{ prop_template.transform_multipart_body(property, property.python_name, destination) }}
 {% endif %}
 {% endmacro %}
 

openapi_python_client/templates/property_templates/any_property.py.jinja

@@ -0,0 +1,7 @@
+{% macro transform_multipart(property, source, destination) %}
+{% if not property.required %}
+{{ destination }} = {{source}} if isinstance({{source}}, Unset) else (None, str({{source}}).encode(), "text/plain")
+{% else %}
+{{ destination }} =  (None, str({{ source }}).encode(), "text/plain")
+{% endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/boolean_property.py.jinja

@@ -1,3 +1,11 @@
 {% macro transform_header(source) %}
 "true" if {{ source }} else "false"
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% if not property.required %}
+{{ destination }} = {{source}} if isinstance({{source}}, Unset) else (None, str({{source}}).encode(), "text/plain")
+{% else %}
+{{ destination }} =  (None, str({{ source }}).encode(), "text/plain")
+{% endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/date_property.py.jinja

@@ -10,17 +10,13 @@ isoparse({{ source }}).date()
 
 {% macro check_type_for_construct(property, source) %}isinstance({{ source }}, str){% endmacro %}
 
-{% macro transform(property, source, destination, declare_type=True, multipart=False) %}
+{% macro transform(property, source, destination, declare_type=True) %}
 {% set transformed = source + ".isoformat()" %}
-{% if multipart %}{# Multipart data must be bytes, not str #}
-{% set transformed = transformed + ".encode()" %}
-{% endif %}
 {% if property.required %}
 {{ destination }} = {{ transformed }}
 {%- else %}
 {% if declare_type %}
 {% set type_annotation = property.get_type_string(json=True) %}
-{% if multipart %}{% set type_annotation = type_annotation | replace("str", "bytes") %}{% endif %}
 {{ destination }}: {{ type_annotation }} = UNSET
 {% else %}
 {{ destination }} = UNSET
@@ -29,3 +25,15 @@ if not isinstance({{ source }}, Unset):
     {{ destination }} = {{ transformed }}
 {%- endif %}
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% set transformed = source + ".isoformat().encode()" %}
+{% if property.required %}
+{{ destination }} = {{ transformed }}
+{%- else %}
+{% set type_annotation = property.get_type_string(json=True) | replace("str", "bytes") %}
+{{ destination }}: {{ type_annotation }} = UNSET
+if not isinstance({{ source }}, Unset):
+    {{ destination }} = {{ transformed }}
+{%- endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/datetime_property.py.jinja

@@ -10,17 +10,13 @@ isoparse({{ source }})
 
 {% macro check_type_for_construct(property, source) %}isinstance({{ source }}, str){% endmacro %}
 
-{% macro transform(property, source, destination, declare_type=True, multipart=False) %}
+{% macro transform(property, source, destination, declare_type=True) %}
 {% set transformed = source + ".isoformat()" %}
-{% if multipart %}{# Multipart data must be bytes, not str #}
-{% set transformed = transformed + ".encode()" %}
-{% endif %}
 {% if property.required %}
 {{ destination }} = {{ transformed }}
 {%- else %}
 {% if declare_type %}
 {% set type_annotation = property.get_type_string(json=True) %}
-{% if multipart %}{% set type_annotation = type_annotation | replace("str", "bytes") %}{% endif %}
 {{ destination }}: {{ type_annotation }} = UNSET
 {% else %}
 {{ destination }} = UNSET
@@ -29,3 +25,15 @@ if not isinstance({{ source }}, Unset):
     {{ destination }} = {{ transformed }}
 {%- endif %}
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% set transformed = source + ".isoformat().encode()" %}
+{% if property.required %}
+{{ destination }} = {{ transformed }}
+{%- else %}
+{% set type_annotation = property.get_type_string(json=True) | replace("str", "bytes") %}
+{{ destination }}: {{ type_annotation }} = UNSET
+if not isinstance({{ source }}, Unset):
+    {{ destination }} = {{ transformed }}
+{%- endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/enum_property.py.jinja

@@ -13,10 +13,6 @@
 {% macro transform(property, source, destination, declare_type=True, multipart=False) %}
 {% set transformed = source + ".value" %}
 {% set type_string = property.get_type_string(json=True) %}
-{% if multipart %}
-    {% set transformed = "(None, str(" + transformed + ").encode(), \"text/plain\")" %}
-    {% set type_string = "Union[Unset, Tuple[None, bytes, str]]" %}
-{% endif %}
 {% if property.required %}
 {{ destination }} = {{ transformed }}
 {%- else %}
@@ -26,6 +22,18 @@ if not isinstance({{ source }}, Unset):
 {% endif %}
 {% endmacro %}
 
+{% macro transform_multipart(property, source, destination) %}
+{% set transformed = "(None, str(" + source + ".value" + ").encode(), \"text/plain\")" %}
+{% set type_string = "Union[Unset, Tuple[None, bytes, str]]" %}
+{% if property.required %}
+{{ destination }} = {{ transformed }}
+{%- else %}
+{{ destination }}: {{ type_string }} = UNSET
+if not isinstance({{ source }}, Unset):
+    {{ destination }} = {{ transformed }}
+{% endif %}
+{% endmacro %}
+
 {% macro transform_header(source) %}
 str({{ source }})
 {% endmacro %}

openapi_python_client/templates/property_templates/file_property.py.jinja

@@ -12,7 +12,7 @@ File(
 
 {% macro check_type_for_construct(property, source) %}isinstance({{ source }}, bytes){% endmacro %}
 
-{% macro transform(property, source, destination, declare_type=True, multipart=False) %}
+{% macro transform(property, source, destination, declare_type=True) %}
 {% if property.required %}
 {{ destination }} = {{ source }}.to_tuple()
 {% else %}
@@ -21,3 +21,13 @@ if not isinstance({{ source }}, Unset):
     {{ destination }} = {{ source }}.to_tuple()
 {% endif %}
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% if property.required %}
+{{ destination }} = {{ source }}.to_tuple()
+{% else %}
+{{ destination }}: {{ property.get_type_string(json=True) }} = UNSET
+if not isinstance({{ source }}, Unset):
+    {{ destination }} = {{ source }}.to_tuple()
+{% endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/float_property.py.jinja

@@ -1,3 +1,11 @@
 {% macro transform_header(source) %}
 str({{ source }})
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% if not property.required %}
+{{ destination }} = {{source}} if isinstance({{source}}, Unset) else (None, str({{source}}).encode(), "text/plain")
+{% else %}
+{{ destination }} =  (None, str({{ source }}).encode(), "text/plain")
+{% endif %}
+{% endmacro %}

openapi_python_client/templates/property_templates/int_property.py.jinja

@@ -1,3 +1,11 @@
 {% macro transform_header(source) %}
 str({{ source }})
 {% endmacro %}
+
+{% macro transform_multipart(property, source, destination) %}
+{% if not property.required %}
+{{ destination }} = {{source}} if isinstance({{source}}, Unset) else (None, str({{source}}).encode(), "text/plain")
+{% else %}
+{{ destination }} =  (None, str({{ source }}).encode(), "text/plain")
+{% endif %}
+{% endmacro %}