From 50374bed734e22e6da34a7332009ec3bd3a263dc Mon Sep 17 00:00:00 2001 From: juspence <87657842+juspence@users.noreply.github.com> Date: Tue, 12 Oct 2021 13:01:43 -0400 Subject: [PATCH 1/3] fix: Allow None in enum properties Needed for some OpenAPI schemas generated by django-rest-framework / drf-spectacular. --- openapi_python_client/parser/properties/__init__.py | 2 +- openapi_python_client/parser/properties/enum_property.py | 9 +++++---- .../schema/openapi_schema_pydantic/schema.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index ac9c0f4d5..c2328454b 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -296,7 +296,7 @@ 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]: diff --git a/openapi_python_client/parser/properties/enum_property.py b/openapi_python_client/parser/properties/enum_property.py index 8cb2f0327..0ae1c9ab4 100644 --- a/openapi_python_client/parser/properties/enum_property.py +++ b/openapi_python_client/parser/properties/enum_property.py @@ -8,7 +8,7 @@ from .property import Property from .schemas import Class -ValueType = Union[str, int] +ValueType = Union[str, int, None] @attr.s(auto_attribs=True, frozen=True) @@ -41,8 +41,8 @@ def get_imports(self, *, prefix: str) -> Set[str]: return imports @staticmethod - def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType]: - """Convert a list of values into dict of {name: value}""" + def values_from_list(values: Union[List[Optional[str]], List[Optional[int]]]) -> Dict[str, ValueType]: + """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): @@ -60,5 +60,6 @@ def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType if key in output: raise ValueError(f"Duplicate key {key} in Enum") sanitized_key = utils.snake_case(key).upper() - output[sanitized_key] = utils.remove_string_escapes(value) + output[sanitized_key] = utils.remove_string_escapes(value) if isinstance(value, str) else value + # If value is the string "null", this becomes Python's None instead of a str, so must special-case it return output diff --git a/openapi_python_client/schema/openapi_schema_pydantic/schema.py b/openapi_python_client/schema/openapi_schema_pydantic/schema.py index bf8dcd2d0..bfd0b2719 100644 --- a/openapi_python_client/schema/openapi_schema_pydantic/schema.py +++ b/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"]] = [] From 70886ba292b4ab3b2aafc7be3d3ea803ea953232 Mon Sep 17 00:00:00 2001 From: juspence <87657842+juspence@users.noreply.github.com> Date: Fri, 15 Oct 2021 10:47:30 -0400 Subject: [PATCH 2/3] Handle enum properties which have None / null as a possible value For enums which only allow None, add new null_enum.py.jinja template For enums with mixed values, add logic to remove None from list Mark enum property as nullable if None isn't the only value Add tests for enums with mixed values and only null values TODO: Make the type checker stop failing, but PTO next week --- .../api/tests/get_user_list.py | 36 +++++++ .../my_test_api_client/models/__init__.py | 2 + .../models/an_enum_with_null.py | 10 ++ .../models/an_enum_with_only_null.py | 8 ++ end_to_end_tests/openapi.json | 40 ++++++++ openapi_python_client/__init__.py | 4 + .../parser/properties/__init__.py | 8 ++ .../templates/null_enum.py.jinja | 9 ++ .../test_parser/test_properties/test_init.py | 94 +++++++++++++++++++ 9 files changed, 211 insertions(+) create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py create mode 100644 openapi_python_client/templates/null_enum.py.jinja diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py index accd2378b..1a64d8814 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py @@ -6,6 +6,8 @@ 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.an_enum_with_only_null import AnEnumWithOnlyNull from ...models.http_validation_error import HTTPValidationError from ...types import UNSET, Response @@ -14,6 +16,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[AnEnumWithOnlyNull], some_date: Union[datetime.date, datetime.datetime], ) -> Dict[str, Any]: url = "{}/tests/".format(client.base_url) @@ -27,6 +31,20 @@ 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 = [] + for an_enum_value_with_only_null_item_data in an_enum_value_with_only_null: + an_enum_value_with_only_null_item = an_enum_value_with_only_null_item_data.value + + json_an_enum_value_with_only_null.append(an_enum_value_with_only_null_item) + if isinstance(some_date, datetime.date): json_some_date = some_date.isoformat() else: @@ -34,6 +52,8 @@ def _get_kwargs( 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 +102,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[AnEnumWithOnlyNull], 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,6 +125,8 @@ def sync( *, client: Client, an_enum_value: List[AnEnum], + an_enum_value_with_null: List[Optional[AnEnumWithNull]], + an_enum_value_with_only_null: List[AnEnumWithOnlyNull], some_date: Union[datetime.date, datetime.datetime], ) -> Optional[Union[HTTPValidationError, List[AModel]]]: """Get a list of things""" @@ -108,6 +134,8 @@ def sync( 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 +144,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[AnEnumWithOnlyNull], 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 +166,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[AnEnumWithOnlyNull], some_date: Union[datetime.date, datetime.datetime], ) -> Optional[Union[HTTPValidationError, List[AModel]]]: """Get a list of things""" @@ -142,6 +176,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 diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 63c5dd10d..6b9b0a330 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -7,6 +7,8 @@ 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_enum_with_only_null import AnEnumWithOnlyNull from .an_int_enum import AnIntEnum from .another_all_of_sub_model import AnotherAllOfSubModel from .another_all_of_sub_model_type import AnotherAllOfSubModelType diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py new file mode 100644 index 000000000..d583f3186 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py @@ -0,0 +1,10 @@ +from enum import Enum + + +class AnEnumWithNull(str, Enum): + FIRST_VALUE = "FIRST_VALUE" + SECOND_VALUE = "SECOND_VALUE" + + def __str__(self) -> str: + return str(self.value) + \ No newline at end of file diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py new file mode 100644 index 000000000..e9cc77087 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py @@ -0,0 +1,8 @@ +from enum import Enum + + +class AnEnumWithOnlyNull(Enum): + VALUE_0 = None + + def __str__(self) -> str: + return str(self.value) diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index 7a4a593c6..e03b60d56 100644 --- a/end_to_end_tests/openapi.json +++ b/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": [ diff --git a/openapi_python_client/__init__.py b/openapi_python_client/__init__.py index 85e83dbfd..a28595e1b 100644 --- a/openapi_python_client/__init__.py +++ b/openapi_python_client/__init__.py @@ -227,6 +227,7 @@ def _build_models(self) -> None: models_dir.mkdir() models_init = models_dir / "__init__.py" imports = [] + NoneType = type(None) model_template = self.env.get_template("model.py.jinja") for model in self.openapi.models: @@ -236,11 +237,14 @@ def _build_models(self) -> None: # Generate enums str_enum_template = self.env.get_template("str_enum.py.jinja") + null_enum_template = self.env.get_template("null_enum.py.jinja") int_enum_template = self.env.get_template("int_enum.py.jinja") for enum in self.openapi.enums: module_path = models_dir / f"{enum.class_info.module_name}.py" if enum.value_type is int: module_path.write_text(int_enum_template.render(enum=enum), encoding=self.file_encoding) + elif enum.value_type is NoneType: + module_path.write_text(null_enum_template.render(enum=enum), encoding=self.file_encoding) else: module_path.write_text(str_enum_template.render(enum=enum), encoding=self.file_encoding) imports.append(import_string_from_class(enum.class_info)) diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index c2328454b..34607e1b2 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -332,6 +332,14 @@ def build_enum_property( schemas, ) + # Remove None from str / int list, if present, and mark property as nullable + # If list only has None, with no str or int, make special None enum instead + keys_to_remove = [key for key, value in values.items() if value is None] + if keys_to_remove and len(keys_to_remove) < len(values.items()): + data.nullable = True + for key in keys_to_remove: + values.pop(key) + for value in values.values(): value_type = type(value) break diff --git a/openapi_python_client/templates/null_enum.py.jinja b/openapi_python_client/templates/null_enum.py.jinja new file mode 100644 index 000000000..f123e75e9 --- /dev/null +++ b/openapi_python_client/templates/null_enum.py.jinja @@ -0,0 +1,9 @@ +from enum import Enum + +class {{ enum.class_info.name }}(Enum): + {% for key, value in enum.values.items() %} + {{ key }} = {{ value }} + {% endfor %} + + def __str__(self) -> str: + return str(self.value) diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index 8a6dd26a4..4c55dbe76 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/tests/test_parser/test_properties/test_init.py @@ -222,6 +222,8 @@ class TestEnumProperty: (True, False, "{}"), (False, True, "Union[Unset, None, {}]"), (True, True, "Optional[{}]"), + (True, False, "Optional[{}]"), + (True, False, "{}"), ), ) def test_get_type_string(self, mocker, enum_property_factory, required, nullable, expected): @@ -273,6 +275,37 @@ def test_values_from_list_duplicate(self): with pytest.raises(ValueError): EnumProperty.values_from_list(data) + def test_values_from_list_with_null(self): + from openapi_python_client.parser.properties import EnumProperty + + data = ["abc", "123", "a23", "1bc", 4, -3, "a Thing WIth spaces", "", "null"] + + result = EnumProperty.values_from_list(data) + + # None / null is removed from result, and result is now Optional[{}] + assert result == { + "ABC": "abc", + "VALUE_1": "123", + "A23": "a23", + "VALUE_3": "1bc", + "VALUE_4": 4, + "VALUE_NEGATIVE_3": -3, + "A_THING_WITH_SPACES": "a Thing WIth spaces", + "VALUE_7": "", + } + + def test_values_from_list_with_only_null(self): + from openapi_python_client.parser.properties import EnumProperty + + data = ["null"] + + result = EnumProperty.values_from_list(data) + + # None / null is not removed from result since it's the only value + assert result == { + "VALUE_0": None, + } + class TestPropertyFromData: def test_property_from_data_str_enum(self, enum_property_factory): @@ -304,6 +337,67 @@ 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="AnEnumWithNull", enum=["A", "B", "C", "null"], 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, + 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 == { + "AnEnumWithNull": 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 + + existing = enum_property_factory() + data = Schema(title="AnEnumWithOnlyNull", enum=["null"], nullable=False, default=None) + 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() + ) + + assert prop == enum_property_factory( + name=name, + required=required, + values={"VALUE_0": None}, + class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"), + value_type=type(None), + default=None, + ) + assert prop.nullable is False + assert schemas != new_schemas, "Provided Schemas was mutated" + assert new_schemas.classes_by_name == { + "AnEnumWithOnlyNull": existing, + "ParentAnEnum": prop, + } + 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.schema import Schema From 60b5ac9081429084ff4c86d55a7b93cb8417d92d Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sat, 16 Oct 2021 20:52:20 -0600 Subject: [PATCH 3/3] refactor: Change null-only enums to simply be `None` --- .../api/tests/get_user_list.py | 17 ++--- .../my_test_api_client/models/__init__.py | 1 - .../models/an_enum_with_null.py | 1 - .../models/an_enum_with_only_null.py | 8 --- openapi_python_client/__init__.py | 4 -- .../parser/properties/__init__.py | 49 +++++++++---- .../parser/properties/enum_property.py | 7 +- .../templates/null_enum.py.jinja | 9 --- .../test_parser/test_properties/test_init.py | 71 ++++--------------- 9 files changed, 58 insertions(+), 109 deletions(-) delete mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py delete mode 100644 openapi_python_client/templates/null_enum.py.jinja diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py index 1a64d8814..fa6d12bb4 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py @@ -7,7 +7,6 @@ from ...models.a_model import AModel from ...models.an_enum import AnEnum from ...models.an_enum_with_null import AnEnumWithNull -from ...models.an_enum_with_only_null import AnEnumWithOnlyNull from ...models.http_validation_error import HTTPValidationError from ...types import UNSET, Response @@ -17,7 +16,7 @@ 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[AnEnumWithOnlyNull], + an_enum_value_with_only_null: List[None], some_date: Union[datetime.date, datetime.datetime], ) -> Dict[str, Any]: url = "{}/tests/".format(client.base_url) @@ -39,11 +38,7 @@ def _get_kwargs( json_an_enum_value_with_null.append(an_enum_value_with_null_item) - json_an_enum_value_with_only_null = [] - for an_enum_value_with_only_null_item_data in an_enum_value_with_only_null: - an_enum_value_with_only_null_item = an_enum_value_with_only_null_item_data.value - - json_an_enum_value_with_only_null.append(an_enum_value_with_only_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() @@ -103,7 +98,7 @@ 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[AnEnumWithOnlyNull], + an_enum_value_with_only_null: List[None], some_date: Union[datetime.date, datetime.datetime], ) -> Response[Union[HTTPValidationError, List[AModel]]]: kwargs = _get_kwargs( @@ -126,7 +121,7 @@ def sync( client: Client, an_enum_value: List[AnEnum], an_enum_value_with_null: List[Optional[AnEnumWithNull]], - an_enum_value_with_only_null: List[AnEnumWithOnlyNull], + 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""" @@ -145,7 +140,7 @@ 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[AnEnumWithOnlyNull], + an_enum_value_with_only_null: List[None], some_date: Union[datetime.date, datetime.datetime], ) -> Response[Union[HTTPValidationError, List[AModel]]]: kwargs = _get_kwargs( @@ -167,7 +162,7 @@ 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[AnEnumWithOnlyNull], + 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""" diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 6b9b0a330..40baca7b9 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -8,7 +8,6 @@ from .an_all_of_enum import AnAllOfEnum from .an_enum import AnEnum from .an_enum_with_null import AnEnumWithNull -from .an_enum_with_only_null import AnEnumWithOnlyNull from .an_int_enum import AnIntEnum from .another_all_of_sub_model import AnotherAllOfSubModel from .another_all_of_sub_model_type import AnotherAllOfSubModelType diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py index d583f3186..b1d6611e0 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_null.py @@ -7,4 +7,3 @@ class AnEnumWithNull(str, Enum): def __str__(self) -> str: return str(self.value) - \ No newline at end of file diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py deleted file mode 100644 index e9cc77087..000000000 --- a/end_to_end_tests/golden-record/my_test_api_client/models/an_enum_with_only_null.py +++ /dev/null @@ -1,8 +0,0 @@ -from enum import Enum - - -class AnEnumWithOnlyNull(Enum): - VALUE_0 = None - - def __str__(self) -> str: - return str(self.value) diff --git a/openapi_python_client/__init__.py b/openapi_python_client/__init__.py index a28595e1b..85e83dbfd 100644 --- a/openapi_python_client/__init__.py +++ b/openapi_python_client/__init__.py @@ -227,7 +227,6 @@ def _build_models(self) -> None: models_dir.mkdir() models_init = models_dir / "__init__.py" imports = [] - NoneType = type(None) model_template = self.env.get_template("model.py.jinja") for model in self.openapi.models: @@ -237,14 +236,11 @@ def _build_models(self) -> None: # Generate enums str_enum_template = self.env.get_template("str_enum.py.jinja") - null_enum_template = self.env.get_template("null_enum.py.jinja") int_enum_template = self.env.get_template("int_enum.py.jinja") for enum in self.openapi.enums: module_path = models_dir / f"{enum.class_info.module_name}.py" if enum.value_type is int: module_path.write_text(int_enum_template.render(enum=enum), encoding=self.file_encoding) - elif enum.value_type is NoneType: - module_path.write_text(null_enum_template.render(enum=enum), encoding=self.file_encoding) else: module_path.write_text(str_enum_template.render(enum=enum), encoding=self.file_encoding) imports.append(import_string_from_class(enum.class_info)) diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index 34607e1b2..b85c7c74a 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/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""" @@ -299,7 +307,7 @@ def build_enum_property( 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,19 +363,7 @@ def build_enum_property( schemas, ) - # Remove None from str / int list, if present, and mark property as nullable - # If list only has None, with no str or int, make special None enum instead - keys_to_remove = [key for key, value in values.items() if value is None] - if keys_to_remove and len(keys_to_remove) < len(values.items()): - data.nullable = True - for key in keys_to_remove: - values.pop(key) - - 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, diff --git a/openapi_python_client/parser/properties/enum_property.py b/openapi_python_client/parser/properties/enum_property.py index 0ae1c9ab4..fe704eefd 100644 --- a/openapi_python_client/parser/properties/enum_property.py +++ b/openapi_python_client/parser/properties/enum_property.py @@ -8,7 +8,7 @@ from .property import Property from .schemas import Class -ValueType = Union[str, int, None] +ValueType = Union[str, int] @attr.s(auto_attribs=True, frozen=True) @@ -41,7 +41,7 @@ def get_imports(self, *, prefix: str) -> Set[str]: return imports @staticmethod - def values_from_list(values: Union[List[Optional[str]], List[Optional[int]]]) -> Dict[str, ValueType]: + def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType]: """Convert a list of values into dict of {name: value}, where value can sometimes be None""" output: Dict[str, ValueType] = {} @@ -60,6 +60,5 @@ def values_from_list(values: Union[List[Optional[str]], List[Optional[int]]]) -> if key in output: raise ValueError(f"Duplicate key {key} in Enum") sanitized_key = utils.snake_case(key).upper() - output[sanitized_key] = utils.remove_string_escapes(value) if isinstance(value, str) else value - # If value is the string "null", this becomes Python's None instead of a str, so must special-case it + output[sanitized_key] = utils.remove_string_escapes(value) return output diff --git a/openapi_python_client/templates/null_enum.py.jinja b/openapi_python_client/templates/null_enum.py.jinja deleted file mode 100644 index f123e75e9..000000000 --- a/openapi_python_client/templates/null_enum.py.jinja +++ /dev/null @@ -1,9 +0,0 @@ -from enum import Enum - -class {{ enum.class_info.name }}(Enum): - {% for key, value in enum.values.items() %} - {{ key }} = {{ value }} - {% endfor %} - - def __str__(self) -> str: - return str(self.value) diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index 4c55dbe76..246493619 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/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" @@ -222,8 +222,6 @@ class TestEnumProperty: (True, False, "{}"), (False, True, "Union[Unset, None, {}]"), (True, True, "Optional[{}]"), - (True, False, "Optional[{}]"), - (True, False, "{}"), ), ) def test_get_type_string(self, mocker, enum_property_factory, required, nullable, expected): @@ -275,37 +273,6 @@ def test_values_from_list_duplicate(self): with pytest.raises(ValueError): EnumProperty.values_from_list(data) - def test_values_from_list_with_null(self): - from openapi_python_client.parser.properties import EnumProperty - - data = ["abc", "123", "a23", "1bc", 4, -3, "a Thing WIth spaces", "", "null"] - - result = EnumProperty.values_from_list(data) - - # None / null is removed from result, and result is now Optional[{}] - assert result == { - "ABC": "abc", - "VALUE_1": "123", - "A23": "a23", - "VALUE_3": "1bc", - "VALUE_4": 4, - "VALUE_NEGATIVE_3": -3, - "A_THING_WITH_SPACES": "a Thing WIth spaces", - "VALUE_7": "", - } - - def test_values_from_list_with_only_null(self): - from openapi_python_client.parser.properties import EnumProperty - - data = ["null"] - - result = EnumProperty.values_from_list(data) - - # None / null is not removed from result since it's the only value - assert result == { - "VALUE_0": None, - } - class TestPropertyFromData: def test_property_from_data_str_enum(self, enum_property_factory): @@ -342,7 +309,7 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory): from openapi_python_client.schema import Schema existing = enum_property_factory() - data = Schema(title="AnEnumWithNull", enum=["A", "B", "C", "null"], nullable=False, default="B") + data = Schema(title="AnEnum", enum=["A", "B", "C", None], nullable=False, default="B") name = "my_enum" required = True @@ -356,6 +323,7 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory): 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, @@ -364,7 +332,7 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory): assert prop.nullable is True assert schemas != new_schemas, "Provided Schemas was mutated" assert new_schemas.classes_by_name == { - "AnEnumWithNull": existing, + "AnEnum": existing, "ParentAnEnum": prop, } @@ -372,34 +340,22 @@ 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 - existing = enum_property_factory() - data = Schema(title="AnEnumWithOnlyNull", enum=["null"], nullable=False, default=None) + data = Schema(title="AnEnumWithOnlyNull", enum=[None], nullable=False, default=None) name = "my_enum" required = True - schemas = Schemas(classes_by_name={"AnEnum": existing}) + schemas = Schemas() prop, new_schemas = property_from_data( name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config() ) - assert prop == enum_property_factory( - name=name, - required=required, - values={"VALUE_0": None}, - class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"), - value_type=type(None), - default=None, + assert prop == NoneProperty( + name="my_enum", required=required, nullable=False, default="None", python_name="my_enum" ) - assert prop.nullable is False - assert schemas != new_schemas, "Provided Schemas was mutated" - assert new_schemas.classes_by_name == { - "AnEnumWithOnlyNull": existing, - "ParentAnEnum": prop, - } 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" @@ -1017,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