Skip to content

Nullable array models generate failing code #928

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

Closed
kgutwin opened this issue Jan 3, 2024 · 1 comment · Fixed by #929
Closed

Nullable array models generate failing code #928

kgutwin opened this issue Jan 3, 2024 · 1 comment · Fixed by #929

Comments

@kgutwin
Copy link
Contributor

kgutwin commented Jan 3, 2024

Describe the bug
When an array is marked as nullable (in OpenAPI 3.0 or 3.1) the generated code fails type checking with the message:

error: Incompatible types in assignment (expression has type "tuple[None, bytes, str]", variable has type "list[float] | Unset | None")  [assignment]

From the end-to-end test suite, making some_array nullable (part of Body_upload_file_tests_upload_post) results in this change:

@@ -165,10 +172,17 @@ class BodyUploadFileTestsUploadPost:
             else (None, str(self.some_number).encode(), "text/plain")
         )
 
-        some_array: Union[Unset, Tuple[None, bytes, str]] = UNSET
-        if not isinstance(self.some_array, Unset):
-            _temp_some_array = self.some_array
-            some_array = (None, json.dumps(_temp_some_array).encode(), "application/json")
+        some_array: Union[List[float], None, Unset]
+        if isinstance(self.some_array, Unset):
+            some_array = UNSET
+        elif isinstance(self.some_array, list):
+            some_array = UNSET
+            if not isinstance(self.some_array, Unset):
+                _temp_some_array = self.some_array
+                some_array = (None, json.dumps(_temp_some_array).encode(), "application/json")
+
+        else:
+            some_array = self.some_array
 
         some_optional_object: Union[Unset, Tuple[None, bytes, str]] = UNSET

OpenAPI Spec File
The following patch applied the end-to-end test suite reproduces the problem:

diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json
index d21d1d5..25adeaa 100644
--- a/end_to_end_tests/baseline_openapi_3.0.json
+++ b/end_to_end_tests/baseline_openapi_3.0.json
@@ -1778,6 +1778,7 @@
           },
           "some_array": {
             "title": "Some Array",
+            "nullable": true,
             "type": "array",
             "items": {
               "type": "number"
diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml
index 03270af..4e33e68 100644
--- a/end_to_end_tests/baseline_openapi_3.1.yaml
+++ b/end_to_end_tests/baseline_openapi_3.1.yaml
@@ -1794,7 +1794,7 @@ info:
         },
         "some_array": {
           "title": "Some Array",
-          "type": "array",
+          "type": [ "array", "null" ],
           "items": {
             "type": "number"
           }

Desktop (please complete the following information):

  • openapi-python-client version 0.17.0
@kgutwin
Copy link
Contributor Author

kgutwin commented Jan 3, 2024

If the item type is an object rather than a simple type, another type checking error is found:

error: "Unset" has no attribute "append"  [attr-defined]

To reproduce in end-to-end tests:

diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json
index d21d1d5..f9a33e5 100644
--- a/end_to_end_tests/baseline_openapi_3.0.json
+++ b/end_to_end_tests/baseline_openapi_3.0.json
@@ -1778,9 +1778,10 @@
           },
           "some_array": {
             "title": "Some Array",
+            "nullable": true,
             "type": "array",
             "items": {
-              "type": "number"
+              "$ref": "#/components/schemas/AFormData"
             }
           },
           "some_object": {
diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml
index 03270af..78fa7fb 100644
--- a/end_to_end_tests/baseline_openapi_3.1.yaml
+++ b/end_to_end_tests/baseline_openapi_3.1.yaml
@@ -1794,9 +1794,9 @@ info:
         },
         "some_array": {
           "title": "Some Array",
-          "type": "array",
+          "type": [ "array", "null" ],
           "items": {
-            "type": "number"
+            "$ref": "#/components/schemas/AFormData"
           }
         },
         "some_object": {

Affected resulting generated code (end_to_end_tests/golden-record/my_test_api_client/models/body_upload_file_tests_upload_post.py):

@@ -265,7 +287,27 @@ class BodyUploadFileTestsUploadPost:
 
         some_number = d.pop("some_number", UNSET)
 
-        some_array = cast(List[float], d.pop("some_array", UNSET))
+        def _parse_some_array(data: object) -> Union[List["AFormData"], None, Unset]:
+            if data is None:
+                return data
+            if isinstance(data, Unset):
+                return data
+            try:
+                if not isinstance(data, list):
+                    raise TypeError()
+                some_array_type_0 = UNSET
+                _some_array_type_0 = data
+                for some_array_type_0_item_data in _some_array_type_0 or []:
+                    some_array_type_0_item = AFormData.from_dict(some_array_type_0_item_data)
+
+                    some_array_type_0.append(some_array_type_0_item)
+
+                return some_array_type_0
+            except:  # noqa: E722
+                pass
+            return cast(Union[List["AFormData"], None, Unset], data)
+
+        some_array = _parse_some_array(d.pop("some_array", UNSET))
 
         _some_optional_object = d.pop("some_optional_object", UNSET)
         some_optional_object: Union[Unset, BodyUploadFileTestsUploadPostSomeOptionalObject]

github-merge-queue bot pushed a commit that referenced this issue Jan 4, 2024
Resolves #928 with two simple fixes:

* Adds `get_type_string()` to `ListProperty` based on the definition in
`ModelProperty`, since both arrays and objects are JSON-serialized in
multipart mode.
* Ensures that an empty list is used when `UNSET` is the default
(initial) value when constructing a list property.

---------

Co-authored-by: Dylan Anthony <[email protected]>
dbanty added a commit that referenced this issue Jan 4, 2024
This PR was created by Knope. Merging it will create a new release

### Features

#### Export `Unset` types from generated `types.py` (#927)

#### Generate properties for some boolean enums

If a schema has both `type = "boolean"` and `enum` defined, a normal
boolean property will now be created.
Previously, the generator would error. 

Note that the generate code _will not_ correctly limit the values to the
enum values. To work around this, use the
OpenAPI 3.1 `const` instead of `enum` to generate Python `Literal`
types.

Thanks for reporting #922 @macmoritz!

### Fixes

#### Do not stop generation for invalid enum values

This generator only supports `enum` values that are strings or integers.
Previously, this was handled at the parsing level, which would cause the
generator to fail if there were any unsupported values in the document.
Now, the generator will correctly keep going, skipping only endpoints
which contained unsupported values.

Thanks for reporting #922 @macmoritz!

#### Fix lists within unions

Fixes #756 and #928. Arrays within unions (which, as of 0.17 includes
nullable arrays) would generate invalid code.

Thanks @kgutwin and @diesieben07!

#### Simplify type checks for non-required unions

Co-authored-by: GitHub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant