fix: Allow None in enum properties [#504, #512, #516]. Thanks @juspence!

Co-authored-by: juspence <[email protected]>

Author: dbanty

end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py

@@ -6,6 +6,7 @@
 from ...client import Client
 from ...models.a_model import AModel
 from ...models.an_enum import AnEnum
+from ...models.an_enum_with_null import AnEnumWithNull
 from ...models.http_validation_error import HTTPValidationError
 from ...types import UNSET, Response
 
@@ -14,6 +15,8 @@ def _get_kwargs(
     *,
     client: Client,
     an_enum_value: List[AnEnum],
+    an_enum_value_with_null: List[Optional[AnEnumWithNull]],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Dict[str, Any]:
     url = "{}/tests/".format(client.base_url)
@@ -27,13 +30,25 @@ def _get_kwargs(
 
         json_an_enum_value.append(an_enum_value_item)
 
+    json_an_enum_value_with_null = []
+    for an_enum_value_with_null_item_data in an_enum_value_with_null:
+        an_enum_value_with_null_item = (
+            an_enum_value_with_null_item_data.value if an_enum_value_with_null_item_data else None
+        )
+
+        json_an_enum_value_with_null.append(an_enum_value_with_null_item)
+
+    json_an_enum_value_with_only_null = an_enum_value_with_only_null
+
     if isinstance(some_date, datetime.date):
         json_some_date = some_date.isoformat()
     else:
         json_some_date = some_date.isoformat()
 
     params: Dict[str, Any] = {
         "an_enum_value": json_an_enum_value,
+        "an_enum_value_with_null": json_an_enum_value_with_null,
+        "an_enum_value_with_only_null": json_an_enum_value_with_only_null,
         "some_date": json_some_date,
     }
     params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
@@ -82,11 +97,15 @@ def sync_detailed(
     *,
     client: Client,
     an_enum_value: List[AnEnum],
+    an_enum_value_with_null: List[Optional[AnEnumWithNull]],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Response[Union[HTTPValidationError, List[AModel]]]:
     kwargs = _get_kwargs(
         client=client,
         an_enum_value=an_enum_value,
+        an_enum_value_with_null=an_enum_value_with_null,
+        an_enum_value_with_only_null=an_enum_value_with_only_null,
         some_date=some_date,
     )
 
@@ -101,13 +120,17 @@ def sync(
     *,
     client: Client,
     an_enum_value: List[AnEnum],
+    an_enum_value_with_null: List[Optional[AnEnumWithNull]],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Optional[Union[HTTPValidationError, List[AModel]]]:
     """Get a list of things"""
 
     return sync_detailed(
         client=client,
         an_enum_value=an_enum_value,
+        an_enum_value_with_null=an_enum_value_with_null,
+        an_enum_value_with_only_null=an_enum_value_with_only_null,
         some_date=some_date,
     ).parsed
 
@@ -116,11 +139,15 @@ async def asyncio_detailed(
     *,
     client: Client,
     an_enum_value: List[AnEnum],
+    an_enum_value_with_null: List[Optional[AnEnumWithNull]],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Response[Union[HTTPValidationError, List[AModel]]]:
     kwargs = _get_kwargs(
         client=client,
         an_enum_value=an_enum_value,
+        an_enum_value_with_null=an_enum_value_with_null,
+        an_enum_value_with_only_null=an_enum_value_with_only_null,
         some_date=some_date,
     )
 
@@ -134,6 +161,8 @@ async def asyncio(
     *,
     client: Client,
     an_enum_value: List[AnEnum],
+    an_enum_value_with_null: List[Optional[AnEnumWithNull]],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Optional[Union[HTTPValidationError, List[AModel]]]:
     """Get a list of things"""
@@ -142,6 +171,8 @@ async def asyncio(
         await asyncio_detailed(
             client=client,
             an_enum_value=an_enum_value,
+            an_enum_value_with_null=an_enum_value_with_null,
+            an_enum_value_with_only_null=an_enum_value_with_only_null,
             some_date=some_date,
         )
     ).parsed

end_to_end_tests/golden-record/my_test_api_client/models/__init__.py

@@ -7,6 +7,7 @@
 from .all_of_sub_model_type_enum import AllOfSubModelTypeEnum
 from .an_all_of_enum import AnAllOfEnum
 from .an_enum import AnEnum
+from .an_enum_with_null import AnEnumWithNull
 from .an_int_enum import AnIntEnum
 from .another_all_of_sub_model import AnotherAllOfSubModel
 from .another_all_of_sub_model_type import AnotherAllOfSubModelType

end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py

@@ -0,0 +1,9 @@
+from enum import Enum
+
+
+class AnEnumWithNull(str, Enum):
+    FIRST_VALUE = "FIRST_VALUE"
+    SECOND_VALUE = "SECOND_VALUE"
+
+    def __str__(self) -> str:
+        return str(self.value)

end_to_end_tests/openapi.json

@@ -27,6 +27,30 @@
             "name": "an_enum_value",
             "in": "query"
           },
+          {
+            "required": true,
+            "schema": {
+              "title": "An Enum Value With Null And String Values",
+              "type": "array",
+              "items": {
+                "$ref": "#/components/schemas/AnEnumWithNull"
+              }
+            },
+            "name": "an_enum_value_with_null",
+            "in": "query"
+          },
+          {
+            "required": true,
+            "schema": {
+              "title": "An Enum Value With Only Null Values",
+              "type": "array",
+              "items": {
+                "$ref": "#/components/schemas/AnEnumWithOnlyNull"
+              }
+            },
+            "name": "an_enum_value_with_only_null",
+            "in": "query"
+          },
           {
             "required": true,
             "schema": {
@@ -1164,6 +1188,22 @@
         ],
         "description": "For testing Enums in all the ways they can be used "
       },
+      "AnEnumWithNull": {
+        "title": "AnEnumWithNull",
+        "enum": [
+          "FIRST_VALUE",
+          "SECOND_VALUE",
+          null
+        ],
+        "description": "For testing Enums with mixed string / null values "
+      },
+      "AnEnumWithOnlyNull": {
+        "title": "AnEnumWithOnlyNull",
+        "enum": [
+          null
+        ],
+        "description": "For testing Enums with only null values "
+      },
       "AnAllOfEnum": {
         "title": "AnAllOfEnum",
         "enum": [

openapi_python_client/parser/properties/__init__.py

@@ -34,6 +34,14 @@ class AnyProperty(Property):
     template: ClassVar[Optional[str]] = "any_property.py.jinja"
 
 
+@attr.s(auto_attribs=True, frozen=True)
+class NoneProperty(Property):
+    """A property that can only be None"""
+
+    _type_string: ClassVar[str] = "None"
+    _json_type_string: ClassVar[str] = "None"
+
+
 @attr.s(auto_attribs=True, frozen=True)
 class StringProperty(Property):
     """A property of type str"""
@@ -296,10 +304,10 @@ def build_enum_property(
     name: str,
     required: bool,
     schemas: Schemas,
-    enum: Union[List[str], List[int]],
+    enum: Union[List[Optional[str]], List[Optional[int]]],
     parent_name: Optional[str],
     config: Config,
-) -> Tuple[Union[EnumProperty, PropertyError], Schemas]:
+) -> Tuple[Union[EnumProperty, NoneProperty, PropertyError], Schemas]:
     """
     Create an EnumProperty from schema data.
 
@@ -316,11 +324,34 @@ def build_enum_property(
         A tuple containing either the created property or a PropertyError describing what went wrong AND update schemas.
     """
 
+    if len(enum) == 0:
+        return PropertyError(detail="No values provided for Enum", data=data), schemas
+
     class_name = data.title or name
     if parent_name:
         class_name = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_name)}"
     class_info = Class.from_string(string=class_name, config=config)
-    values = EnumProperty.values_from_list(enum)
+
+    # 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: Union[List[str], List[int]] = [value for value in enum if value is not None]  # type: ignore
+    if len(value_list) < len(enum):
+        data.nullable = True
+
+    # It's legal to have an enum that only contains null as a value, we don't bother constructing an enum in that case
+    if len(value_list) == 0:
+        return (
+            NoneProperty(
+                name=name,
+                required=required,
+                nullable=False,
+                default="None",
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+            ),
+            schemas,
+        )
+    values = EnumProperty.values_from_list(value_list)
 
     if class_info.name in schemas.classes_by_name:
         existing = schemas.classes_by_name[class_info.name]
@@ -332,11 +363,7 @@ def build_enum_property(
                 schemas,
             )
 
-    for value in values.values():
-        value_type = type(value)
-        break
-    else:
-        return PropertyError(data=data, detail="No values provided for Enum"), schemas
+    value_type = type(next(iter(values.values())))
 
     prop = EnumProperty(
         name=name,

openapi_python_client/parser/properties/enum_property.py

@@ -42,7 +42,7 @@ def get_imports(self, *, prefix: str) -> Set[str]:
 
     @staticmethod
     def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType]:
-        """Convert a list of values into dict of {name: value}"""
+        """Convert a list of values into dict of {name: value}, where value can sometimes be None"""
         output: Dict[str, ValueType] = {}
 
         for i, value in enumerate(values):

openapi_python_client/schema/openapi_schema_pydantic/schema.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_items=1)
-    enum: Union[None, List[StrictInt], List[StrictStr]] = Field(default=None, min_items=1)
+    enum: Union[None, List[Optional[StrictInt]], List[Optional[StrictStr]]] = Field(default=None, min_items=1)
     type: Optional[DataType] = Field(default=None)
     allOf: Optional[List[Union[Reference, "Schema"]]] = None
     oneOf: List[Union[Reference, "Schema"]] = []

tests/test_parser/test_properties/test_init.py

@@ -6,7 +6,7 @@
 import openapi_python_client.schema as oai
 from openapi_python_client import Config
 from openapi_python_client.parser.errors import PropertyError, ValidationError
-from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, Schemas
+from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, NoneProperty, Schemas
 
 MODULE_NAME = "openapi_python_client.parser.properties"
 
@@ -304,8 +304,58 @@ def test_property_from_data_str_enum(self, enum_property_factory):
             "ParentAnEnum": prop,
         }
 
+    def test_property_from_data_str_enum_with_null(self, enum_property_factory):
+        from openapi_python_client.parser.properties import Class, Schemas, property_from_data
+        from openapi_python_client.schema import Schema
+
+        existing = enum_property_factory()
+        data = Schema(title="AnEnum", enum=["A", "B", "C", None], nullable=False, default="B")
+        name = "my_enum"
+        required = True
+
+        schemas = Schemas(classes_by_name={"AnEnum": existing})
+
+        prop, new_schemas = property_from_data(
+            name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config()
+        )
+
+        # None / null is removed from enum, and property is now nullable
+        assert prop == enum_property_factory(
+            name=name,
+            required=required,
+            nullable=True,
+            values={"A": "A", "B": "B", "C": "C"},
+            class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"),
+            value_type=str,
+            default="ParentAnEnum.B",
+        )
+        assert prop.nullable is True
+        assert schemas != new_schemas, "Provided Schemas was mutated"
+        assert new_schemas.classes_by_name == {
+            "AnEnum": existing,
+            "ParentAnEnum": prop,
+        }
+
+    def test_property_from_data_null_enum(self, enum_property_factory):
+        from openapi_python_client.parser.properties import Class, Schemas, property_from_data
+        from openapi_python_client.schema import Schema
+
+        data = Schema(title="AnEnumWithOnlyNull", enum=[None], nullable=False, default=None)
+        name = "my_enum"
+        required = True
+
+        schemas = Schemas()
+
+        prop, new_schemas = property_from_data(
+            name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config()
+        )
+
+        assert prop == NoneProperty(
+            name="my_enum", required=required, nullable=False, default="None", python_name="my_enum"
+        )
+
     def test_property_from_data_int_enum(self, enum_property_factory):
-        from openapi_python_client.parser.properties import Class, EnumProperty, Schemas, property_from_data
+        from openapi_python_client.parser.properties import Class, Schemas, property_from_data
         from openapi_python_client.schema import Schema
 
         name = "my_enum"
@@ -923,14 +973,17 @@ def test_retries_failing_properties_while_making_progress(self, mocker):
         assert result.errors == [PropertyError()]
 
 
-def test_build_enum_property_conflict(mocker):
+def test_build_enum_property_conflict():
     from openapi_python_client.parser.properties import Schemas, build_enum_property
 
     data = oai.Schema()
-    schemas = Schemas(classes_by_name={"Existing": mocker.MagicMock()})
+    schemas = Schemas()
 
+    _, schemas = build_enum_property(
+        data=data, name="Existing", required=True, schemas=schemas, enum=["a"], parent_name=None, config=Config()
+    )
     err, schemas = build_enum_property(
-        data=data, name="Existing", required=True, schemas=schemas, enum=[], parent_name=None, config=Config()
+        data=data, name="Existing", required=True, schemas=schemas, enum=["a", "b"], parent_name=None, config=Config()
     )
 
     assert schemas == schemas