Handle bool enums better (#923)

TODO:

- [x] Cover error cases with new unit tests

---------

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

Author: dbanty

.changeset/do_not_stop_generation_for_invalid_enum_values.md

@@ -0,0 +1,11 @@
+---
+default: patch
+---
+
+# 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!

.changeset/generate_properties_for_some_boolean_enums.md

@@ -0,0 +1,13 @@
+---
+default: minor
+---
+
+# 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!

end_to_end_tests/baseline_openapi_3.0.json

@@ -61,7 +61,9 @@
     },
     "/bodies/json-like": {
       "post": {
-        "tags": ["bodies"],
+        "tags": [
+          "bodies"
+        ],
         "description": "A content type that works like json but isn't application/json",
         "operationId": "json-like",
         "requestBody": {
@@ -799,11 +801,11 @@
           }
         }
       }
-    },
-    "/tests/int_enum": {
+      },
+    "/enum/int": {
       "post": {
         "tags": [
-          "tests"
+          "enums"
         ],
         "summary": "Int Enum",
         "operationId": "int_enum_tests_int_enum_post",
@@ -825,14 +827,37 @@
                 "schema": {}
               }
             }
-          },
-          "422": {
-            "description": "Validation Error",
+          }
+        }
+      }
+    },
+    "/enum/bool": {
+      "post": {
+        "tags": [
+          "enums"
+        ],
+        "summary": "Bool Enum",
+        "operationId": "bool_enum_tests_bool_enum_post",
+        "parameters": [
+          {
+            "required": true,
+            "schema": {
+              "type": "boolean",
+              "enum": [
+                true,
+                false
+              ]
+            },
+            "name": "bool_enum",
+            "in": "query"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Successful Response",
             "content": {
               "application/json": {
-                "schema": {
-                  "$ref": "#/components/schemas/HTTPValidationError"
-                }
+                "schema": {}
               }
             }
           }

end_to_end_tests/baseline_openapi_3.1.yaml

@@ -794,10 +794,10 @@ info:
       }
     }
   },
-  "/tests/int_enum": {
+  "/enum/int": {
     "post": {
       "tags": [
-        "tests"
+        "enums"
       ],
       "summary": "Int Enum",
       "operationId": "int_enum_tests_int_enum_post",
@@ -819,14 +819,37 @@ info:
               "schema": { }
             }
           }
-        },
-        "422": {
-          "description": "Validation Error",
+        }
+      }
+    }
+  },
+  "/enum/bool": {
+    "post": {
+      "tags": [
+        "enums"
+      ],
+      "summary": "Bool Enum",
+      "operationId": "bool_enum_tests_bool_enum_post",
+      "parameters": [
+        {
+          "required": true,
+          "schema": {
+            "type": "boolean",
+            "enum": [
+              true,
+              false
+            ]
+          },
+          "name": "bool_enum",
+          "in": "query"
+        }
+      ],
+      "responses": {
+        "200": {
+          "description": "Successful Response",
           "content": {
             "application/json": {
-              "schema": {
-                "$ref": "#/components/schemas/HTTPValidationError"
-              }
+              "schema": { }
             }
           }
         }

end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py

@@ -5,6 +5,7 @@
 from .bodies import BodiesEndpoints
 from .default import DefaultEndpoints
 from .defaults import DefaultsEndpoints
+from .enums import EnumsEndpoints
 from .location import LocationEndpoints
 from .naming import NamingEndpoints
 from .parameter_references import ParameterReferencesEndpoints
@@ -28,6 +29,10 @@ def tests(cls) -> Type[TestsEndpoints]:
     def defaults(cls) -> Type[DefaultsEndpoints]:
         return DefaultsEndpoints
 
+    @classmethod
+    def enums(cls) -> Type[EnumsEndpoints]:
+        return EnumsEndpoints
+
     @classmethod
     def responses(cls) -> Type[ResponsesEndpoints]:
         return ResponsesEndpoints

end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/enums/__init__.py

@@ -0,0 +1,21 @@
+""" Contains methods for accessing the API Endpoints """
+
+import types
+
+from . import bool_enum_tests_bool_enum_post, int_enum_tests_int_enum_post
+
+
+class EnumsEndpoints:
+    @classmethod
+    def int_enum_tests_int_enum_post(cls) -> types.ModuleType:
+        """
+        Int Enum
+        """
+        return int_enum_tests_int_enum_post
+
+    @classmethod
+    def bool_enum_tests_bool_enum_post(cls) -> types.ModuleType:
+        """
+        Bool Enum
+        """
+        return bool_enum_tests_bool_enum_post

end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tests/__init__.py

@@ -10,7 +10,6 @@
     get_basic_list_of_integers,
     get_basic_list_of_strings,
     get_user_list,
-    int_enum_tests_int_enum_post,
     json_body_tests_json_body_post,
     no_response_tests_no_response_get,
     octet_stream_tests_octet_stream_get,
@@ -132,13 +131,6 @@ def unsupported_content_tests_unsupported_content_get(cls) -> types.ModuleType:
         """
         return unsupported_content_tests_unsupported_content_get
 
-    @classmethod
-    def int_enum_tests_int_enum_post(cls) -> types.ModuleType:
-        """
-        Int Enum
-        """
-        return int_enum_tests_int_enum_post
-
     @classmethod
     def test_inline_objects(cls) -> types.ModuleType:
         """

end_to_end_tests/golden-record/my_test_api_client/api/enums/__init__.py

@@ -0,0 +1,101 @@
+from http import HTTPStatus
+from typing import Any, Dict, Optional, Union
+
+import httpx
+
+from ... import errors
+from ...client import AuthenticatedClient, Client
+from ...types import UNSET, Response
+
+
+def _get_kwargs(
+    *,
+    bool_enum: bool,
+) -> Dict[str, Any]:
+    params: Dict[str, Any] = {}
+
+    params["bool_enum"] = bool_enum
+
+    params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
+
+    _kwargs: Dict[str, Any] = {
+        "method": "post",
+        "url": "/enum/bool",
+        "params": params,
+    }
+
+    return _kwargs
+
+
+def _parse_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Optional[Any]:
+    if response.status_code == HTTPStatus.OK:
+        return None
+    if client.raise_on_unexpected_status:
+        raise errors.UnexpectedStatus(response.status_code, response.content)
+    else:
+        return None
+
+
+def _build_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Response[Any]:
+    return Response(
+        status_code=HTTPStatus(response.status_code),
+        content=response.content,
+        headers=response.headers,
+        parsed=_parse_response(client=client, response=response),
+    )
+
+
+def sync_detailed(
+    *,
+    client: Union[AuthenticatedClient, Client],
+    bool_enum: bool,
+) -> Response[Any]:
+    """Bool Enum
+
+    Args:
+        bool_enum (bool):
+
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[Any]
+    """
+
+    kwargs = _get_kwargs(
+        bool_enum=bool_enum,
+    )
+
+    response = client.get_httpx_client().request(
+        **kwargs,
+    )
+
+    return _build_response(client=client, response=response)
+
+
+async def asyncio_detailed(
+    *,
+    client: Union[AuthenticatedClient, Client],
+    bool_enum: bool,
+) -> Response[Any]:
+    """Bool Enum
+
+    Args:
+        bool_enum (bool):
+
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[Any]
+    """
+
+    kwargs = _get_kwargs(
+        bool_enum=bool_enum,
+    )
+
+    response = await client.get_async_httpx_client().request(**kwargs)
+
+    return _build_response(client=client, response=response)

end_to_end_tests/golden-record/my_test_api_client/api/enums/bool_enum_tests_bool_enum_post.py

@@ -6,7 +6,6 @@
 from ... import errors
 from ...client import AuthenticatedClient, Client
 from ...models.an_int_enum import AnIntEnum
-from ...models.http_validation_error import HTTPValidationError
 from ...types import UNSET, Response
 
 
@@ -23,32 +22,23 @@ def _get_kwargs(
 
     _kwargs: Dict[str, Any] = {
         "method": "post",
-        "url": "/tests/int_enum",
+        "url": "/enum/int",
         "params": params,
     }
 
     return _kwargs
 
 
-def _parse_response(
-    *, client: Union[AuthenticatedClient, Client], response: httpx.Response
-) -> Optional[Union[Any, HTTPValidationError]]:
+def _parse_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Optional[Any]:
     if response.status_code == HTTPStatus.OK:
-        response_200 = response.json()
-        return response_200
-    if response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY:
-        response_422 = HTTPValidationError.from_dict(response.json())
-
-        return response_422
+        return None
     if client.raise_on_unexpected_status:
         raise errors.UnexpectedStatus(response.status_code, response.content)
     else:
         return None
 
 
-def _build_response(
-    *, client: Union[AuthenticatedClient, Client], response: httpx.Response
-) -> Response[Union[Any, HTTPValidationError]]:
+def _build_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Response[Any]:
     return Response(
         status_code=HTTPStatus(response.status_code),
         content=response.content,
@@ -61,7 +51,7 @@ def sync_detailed(
     *,
     client: Union[AuthenticatedClient, Client],
     int_enum: AnIntEnum,
-) -> Response[Union[Any, HTTPValidationError]]:
+) -> Response[Any]:
     """Int Enum
 
     Args:
@@ -72,7 +62,7 @@ def sync_detailed(
         httpx.TimeoutException: If the request takes longer than Client.timeout.
 
     Returns:
-        Response[Union[Any, HTTPValidationError]]
+        Response[Any]
     """
 
     kwargs = _get_kwargs(
@@ -86,35 +76,11 @@ def sync_detailed(
     return _build_response(client=client, response=response)
 
 
-def sync(
-    *,
-    client: Union[AuthenticatedClient, Client],
-    int_enum: AnIntEnum,
-) -> Optional[Union[Any, HTTPValidationError]]:
-    """Int Enum
-
-    Args:
-        int_enum (AnIntEnum): An enumeration.
-
-    Raises:
-        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
-        httpx.TimeoutException: If the request takes longer than Client.timeout.
-
-    Returns:
-        Union[Any, HTTPValidationError]
-    """
-
-    return sync_detailed(
-        client=client,
-        int_enum=int_enum,
-    ).parsed
-
-
 async def asyncio_detailed(
     *,
     client: Union[AuthenticatedClient, Client],
     int_enum: AnIntEnum,
-) -> Response[Union[Any, HTTPValidationError]]:
+) -> Response[Any]:
     """Int Enum
 
     Args:
@@ -125,7 +91,7 @@ async def asyncio_detailed(
         httpx.TimeoutException: If the request takes longer than Client.timeout.
 
     Returns:
-        Response[Union[Any, HTTPValidationError]]
+        Response[Any]
     """
 
     kwargs = _get_kwargs(
@@ -135,29 +101,3 @@ async def asyncio_detailed(
     response = await client.get_async_httpx_client().request(**kwargs)
 
     return _build_response(client=client, response=response)
-
-
-async def asyncio(
-    *,
-    client: Union[AuthenticatedClient, Client],
-    int_enum: AnIntEnum,
-) -> Optional[Union[Any, HTTPValidationError]]:
-    """Int Enum
-
-    Args:
-        int_enum (AnIntEnum): An enumeration.
-
-    Raises:
-        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
-        httpx.TimeoutException: If the request takes longer than Client.timeout.
-
-    Returns:
-        Union[Any, HTTPValidationError]
-    """
-
-    return (
-        await asyncio_detailed(
-            client=client,
-            int_enum=int_enum,
-        )
-    ).parsed

end_to_end_tests/golden-record/my_test_api_client/api/tests/int_enum_tests_int_enum_post.py renamed to end_to_end_tests/golden-record/my_test_api_client/api/enums/int_enum_tests_int_enum_post.py

@@ -162,14 +162,24 @@ def property_from_data(  # noqa: PLR0911
             config=config,
             roots=roots,
         )
-
+    if data.type == oai.DataType.BOOLEAN:
+        return (
+            BooleanProperty.build(
+                name=name,
+                required=required,
+                default=data.default,
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+                description=data.description,
+                example=data.example,
+            ),
+            schemas,
+        )
     if data.enum:
         return EnumProperty.build(
             data=data,
             name=name,
             required=required,
             schemas=schemas,
-            enum=data.enum,
             parent_name=parent_name,
             config=config,
         )
@@ -223,18 +233,6 @@ def property_from_data(  # noqa: PLR0911
             ),
             schemas,
         )
-    if data.type == oai.DataType.BOOLEAN:
-        return (
-            BooleanProperty.build(
-                name=name,
-                required=required,
-                default=data.default,
-                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
-                description=data.description,
-                example=data.example,
-            ),
-            schemas,
-        )
     if data.type == oai.DataType.NULL:
         return (
             NoneProperty(

openapi_python_client/parser/properties/__init__.py

@@ -2,7 +2,7 @@
 
 __all__ = ["EnumProperty"]
 
-from typing import Any, ClassVar, Union, cast
+from typing import Any, ClassVar, List, Union, cast
 
 from attr import evolve
 from attrs import define
@@ -43,14 +43,13 @@ class EnumProperty(PropertyProtocol):
     }
 
     @classmethod
-    def build(
+    def build(  # noqa: PLR0911
         cls,
         *,
         data: oai.Schema,
         name: str,
         required: bool,
         schemas: Schemas,
-        enum: list[str | None] | list[int | None],
         parent_name: str,
         config: Config,
     ) -> tuple[EnumProperty | NoneProperty | UnionProperty | PropertyError, Schemas]:
@@ -70,16 +69,15 @@ def build(
             A tuple containing either the created property or a PropertyError AND update schemas.
         """
 
-        if len(enum) == 0:
-            return PropertyError(detail="No values provided for Enum", data=data), schemas
+        enum = data.enum or []  # The outer function checks for this, but mypy doesn't know that
 
         # OpenAPI allows for null as an enum value, but it doesn't make sense with how enums are constructed in Python.
         # So instead, if null is a possible value, make the property nullable.
         # Mypy is not smart enough to know that the type is right though
-        value_list: list[str] | list[int] = [value for value in enum if value is not None]  # type: ignore
+        unchecked_value_list = [value for value in enum if value is not None]  # type: ignore
 
         # It's legal to have an enum that only contains null as a value, we don't bother constructing an enum for that
-        if len(value_list) == 0:
+        if len(unchecked_value_list) == 0:
             return (
                 NoneProperty.build(
                     name=name,
@@ -91,6 +89,19 @@ def build(
                 ),
                 schemas,
             )
+
+        value_types = {type(value) for value in unchecked_value_list}
+        if len(value_types) > 1:
+            return PropertyError(
+                header="Enum values must all be the same type", detail=f"Got {value_types}", data=data
+            ), schemas
+        value_type = next(iter(value_types))
+        if value_type not in (str, int):
+            return PropertyError(header=f"Unsupported enum type {value_type}", data=data), schemas
+        value_list = cast(
+            Union[List[int], List[str]], unchecked_value_list
+        )  # We checked this with all the value_types stuff
+
         if len(value_list) < len(enum):  # Only one of the values was None, that becomes a union
             data.oneOf = [
                 oai.Schema(type=DataType.NULL),
@@ -122,8 +133,6 @@ def build(
                     schemas,
                 )
 
-        value_type = type(next(iter(values.values())))
-
         prop = EnumProperty(
             name=name,
             required=required,

openapi_python_client/parser/properties/enum_property.py

@@ -35,7 +35,7 @@ class Schema(BaseModel):
     maxProperties: Optional[int] = Field(default=None, ge=0)
     minProperties: Optional[int] = Field(default=None, ge=0)
     required: Optional[List[str]] = Field(default=None, min_length=1)
-    enum: Union[None, List[Optional[StrictInt]], List[Optional[StrictStr]]] = Field(default=None, min_length=1)
+    enum: Union[None, List[Any]] = Field(default=None, min_length=1)
     const: Union[None, StrictStr, StrictInt] = None
     type: Union[DataType, List[DataType], None] = Field(default=None)
     allOf: List[Union[Reference, "Schema"]] = Field(default_factory=list)

openapi_python_client/schema/openapi_schema_pydantic/schema.py

@@ -5,57 +5,65 @@
 
 
 def test_conflict():
-    data = oai.Schema()
     schemas = Schemas()
 
     _, schemas = EnumProperty.build(
-        data=data, name="Existing", required=True, schemas=schemas, enum=["a"], parent_name="", config=Config()
+        data=oai.Schema(enum=["a"]), name="Existing", required=True, schemas=schemas, parent_name="", config=Config()
     )
     err, new_schemas = EnumProperty.build(
-        data=data,
+        data=oai.Schema(enum=["a", "b"]),
         name="Existing",
         required=True,
         schemas=schemas,
-        enum=["a", "b"],
         parent_name="",
         config=Config(),
     )
 
     assert schemas == new_schemas
-    assert err == PropertyError(detail="Found conflicting enums named Existing with incompatible values.", data=data)
+    assert err.detail == "Found conflicting enums named Existing with incompatible values."
 
 
-def test_no_values():
-    data = oai.Schema()
+def test_bad_default_value():
+    data = oai.Schema(default="B", enum=["A"])
     schemas = Schemas()
 
     err, new_schemas = EnumProperty.build(
-        data=data, name="Existing", required=True, schemas=schemas, enum=[], parent_name=None, config=Config()
+        data=data, name="Existing", required=True, schemas=schemas, parent_name="parent", config=Config()
     )
 
     assert schemas == new_schemas
-    assert err == PropertyError(detail="No values provided for Enum", data=data)
+    assert err == PropertyError(detail="Value B is not valid for enum Existing", data=data)
 
 
-def test_bad_default_value():
-    data = oai.Schema(default="B")
+def test_bad_default_type():
+    data = oai.Schema(default=123, enum=["A"])
     schemas = Schemas()
 
     err, new_schemas = EnumProperty.build(
-        data=data, name="Existing", required=True, schemas=schemas, enum=["A"], parent_name=None, config=Config()
+        data=data, name="Existing", required=True, schemas=schemas, parent_name="parent", config=Config()
     )
 
     assert schemas == new_schemas
-    assert err == PropertyError(detail="Value B is not valid for enum Existing", data=data)
+    assert isinstance(err, PropertyError)
 
 
-def test_bad_default_type():
-    data = oai.Schema(default=123)
+def test_mixed_types():
+    data = oai.Schema(enum=["A", 1])
     schemas = Schemas()
 
-    err, new_schemas = EnumProperty.build(
-        data=data, name="Existing", required=True, schemas=schemas, enum=["A"], parent_name=None, config=Config()
+    err, _ = EnumProperty.build(
+        data=data, name="Enum", required=True, schemas=schemas, parent_name="parent", config=Config()
+    )
+
+    assert isinstance(err, PropertyError)
+
+
+def test_unsupported_type():
+    data = oai.Schema(enum=[1.4, 1.5])
+    schemas = Schemas()
+
+    err, _ = EnumProperty.build(
+        data=data, name="Enum", required=True, schemas=schemas, parent_name="parent", config=Config()
     )
 
-    assert schemas == new_schemas
     assert isinstance(err, PropertyError)