Support conflicting enum names. Closes #54

dbanty · dbanty · commit 2ef30b450afa · 2020-05-30T12:18:27.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,15 +10,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Classes generated to be included within lists will now be named like <ListName>Item. For example, if a property 
     named "statuses" is an array of enum values, previously the `Enum` class declared would be called "Statuses". Now it 
     will be called "StatusesItem". If a "title" attribute was used in the OpenAPI document, that should still be respected
-    and used instead of the generated name.
-- Clients now require httpx ^0.13.0 (up from ^0.12.1)
+    and used instead of the generated name. You can restore previous names by adding "StatusesItem" to the `class_overrides`
+    section of a config file.
+- Clients now require httpx ^0.13.0 (up from ^0.12.1). See [httpx release notes](https://github.com/encode/httpx/releases/tag/0.13.0)
+    for details.
 
 ### Additions
 - Support for binary format strings (file payloads)
 - Support for multipart/form bodies
 - Support for any supported property within a list (array), including other lists.
 - Support for Union types ("anyOf" in OpenAPI document)
 - Support for more basic response types (integer, number, boolean)
+- Support for duplicate enums. Instead of erroring, enums with the same name (title) but differing values 
+    will have a number appended to the end. So if you have two conflicting enums named `MyEnum`, one of them
+    will now be named `MyEnum1`. Note that the order in which these are processed and therefore named is entirely
+    dependent on the order they are read from the OpenAPI document, so changes to the document could result 
+    in swapping the names of conflicting Enums.
 
 ### Changes
 - The way most imports are handled was changed which *should* lead to fewer unused imports in generated files.
diff --git a/end_to_end_tests/config.yml b/end_to_end_tests/config.yml
@@ -2,3 +2,9 @@ class_overrides:
   _ABCResponse:
     class_name: ABCResponse
     module_name: abc_response
+  AnEnumValueItem:
+    class_name: AnEnumValue
+    module_name: an_enum_value
+  NestedListOfEnumsItemItem:
+    class_name: AnEnumValue
+    module_name: an_enum_value
diff --git a/end_to_end_tests/fastapi_app/__init__.py b/end_to_end_tests/fastapi_app/__init__.py
@@ -27,6 +27,11 @@ class AnEnum(Enum):
     SECOND_VALUE = "SECOND_VALUE"
 
 
+class DifferentEnum(Enum):
+    FIRST_VALUE = "DIFFERENT"
+    SECOND_VALUE = "OTHER"
+
+
 class OtherModel(BaseModel):
     """ A different model for calling from TestModel """
 
@@ -37,7 +42,7 @@ class AModel(BaseModel):
     """ A Model for testing all the ways custom objects can be used """
 
     an_enum_value: AnEnum
-    nested_list_of_enums: List[List[AnEnum]]
+    nested_list_of_enums: List[List[DifferentEnum]]
     aCamelDateTime: Union[datetime, date]
     a_date: date
 
diff --git a/end_to_end_tests/fastapi_app/openapi.json b/end_to_end_tests/fastapi_app/openapi.json
@@ -164,8 +164,8 @@
                             "type": "array",
                             "items": {
                                 "enum": [
-                                    "FIRST_VALUE",
-                                    "SECOND_VALUE"
+                                    "DIFFERENT",
+                                    "OTHER"
                                 ]
                             }
                         }
diff --git a/end_to_end_tests/golden-master/my_test_api_client/api/users.py b/end_to_end_tests/golden-master/my_test_api_client/api/users.py
@@ -7,13 +7,13 @@
 from ..client import AuthenticatedClient, Client
 from ..errors import ApiResponseError
 from ..models.a_model import AModel
-from ..models.an_enum_value_item import AnEnumValueItem
+from ..models.an_enum_value import AnEnumValue
 from ..models.body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
 from ..models.http_validation_error import HTTPValidationError
 
 
 def get_user_list(
-    *, client: Client, an_enum_value: List[AnEnumValueItem], some_date: Union[date, datetime],
+    *, client: Client, an_enum_value: List[AnEnumValue], some_date: Union[date, datetime],
 ) -> Union[
     List[AModel], HTTPValidationError,
 ]:
diff --git a/end_to_end_tests/golden-master/my_test_api_client/async_api/users.py b/end_to_end_tests/golden-master/my_test_api_client/async_api/users.py
@@ -7,13 +7,13 @@
 from ..client import AuthenticatedClient, Client
 from ..errors import ApiResponseError
 from ..models.a_model import AModel
-from ..models.an_enum_value_item import AnEnumValueItem
+from ..models.an_enum_value import AnEnumValue
 from ..models.body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
 from ..models.http_validation_error import HTTPValidationError
 
 
 async def get_user_list(
-    *, client: Client, an_enum_value: List[AnEnumValueItem], some_date: Union[date, datetime],
+    *, client: Client, an_enum_value: List[AnEnumValue], some_date: Union[date, datetime],
 ) -> Union[
     List[AModel], HTTPValidationError,
 ]:
diff --git a/end_to_end_tests/golden-master/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-master/my_test_api_client/models/__init__.py
@@ -2,9 +2,8 @@
 
 from .a_model import AModel
 from .an_enum_value import AnEnumValue
-from .an_enum_value_item import AnEnumValueItem
+from .an_enum_value1 import AnEnumValue1
 from .body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
 from .http_validation_error import HTTPValidationError
-from .nested_list_of_enums_item_item import NestedListOfEnumsItemItem
 from .types import *
 from .validation_error import ValidationError
diff --git a/end_to_end_tests/golden-master/my_test_api_client/models/a_model.py b/end_to_end_tests/golden-master/my_test_api_client/models/a_model.py
@@ -5,15 +5,15 @@
 from typing import Any, Dict, List, Union, cast
 
 from .an_enum_value import AnEnumValue
-from .nested_list_of_enums_item_item import NestedListOfEnumsItemItem
+from .an_enum_value1 import AnEnumValue1
 
 
 @dataclass
 class AModel:
     """ A Model for testing all the ways custom objects can be used  """
 
     an_enum_value: AnEnumValue
-    nested_list_of_enums: List[List[NestedListOfEnumsItemItem]]
+    nested_list_of_enums: List[List[AnEnumValue1]]
     a_camel_date_time: Union[datetime, date]
     a_date: date
 
@@ -53,7 +53,7 @@ def from_dict(d: Dict[str, Any]) -> AModel:
         for nested_list_of_enums_item_data in d["nested_list_of_enums"]:
             nested_list_of_enums_item = []
             for nested_list_of_enums_item_item_data in nested_list_of_enums_item_data:
-                nested_list_of_enums_item_item = NestedListOfEnumsItemItem(nested_list_of_enums_item_item_data)
+                nested_list_of_enums_item_item = AnEnumValue1(nested_list_of_enums_item_item_data)
 
                 nested_list_of_enums_item.append(nested_list_of_enums_item_item)
 
diff --git a/end_to_end_tests/golden-master/my_test_api_client/models/an_enum_value1.py b/end_to_end_tests/golden-master/my_test_api_client/models/an_enum_value1.py
@@ -0,0 +1,6 @@
+from enum import Enum
+
+
+class AnEnumValue1(str, Enum):
+    DIFFERENT = "DIFFERENT"
+    OTHER = "OTHER"
diff --git a/end_to_end_tests/golden-master/my_test_api_client/models/an_enum_value_item.py b/end_to_end_tests/golden-master/my_test_api_client/models/an_enum_value_item.py
diff --git a/end_to_end_tests/golden-master/my_test_api_client/models/nested_list_of_enums_item_item.py b/end_to_end_tests/golden-master/my_test_api_client/models/nested_list_of_enums_item_item.py
diff --git a/openapi_python_client/openapi_parser/openapi.py b/openapi_python_client/openapi_parser/openapi.py
@@ -220,52 +220,12 @@ class OpenAPI:
     endpoint_collections_by_tag: Dict[str, EndpointCollection]
     enums: Dict[str, EnumProperty]
 
-    @staticmethod
-    def _check_enums(schemas: Iterable[Schema], collections: Iterable[EndpointCollection]) -> Dict[str, EnumProperty]:
-        """
-        Create EnumProperties for every enum in any schema or collection.
-        Enums are deduplicated by class name.
-
-        :raises AssertionError: if two Enums with the same name but different values are detected
-        """
-        enums: Dict[str, EnumProperty] = {}
-
-        def _unpack_list_property(list_prop: ListProperty[Property]) -> Property:
-            inner = list_prop.inner_property
-            if isinstance(inner, ListProperty):
-                return _unpack_list_property(inner)
-            return inner
-
-        def _iterate_properties() -> Generator[Property, None, None]:
-            for schema in schemas:
-                yield from schema.required_properties
-                yield from schema.optional_properties
-            for collection in collections:
-                for endpoint in collection.endpoints:
-                    yield from endpoint.path_parameters
-                    yield from endpoint.query_parameters
-
-        for prop in _iterate_properties():
-            if isinstance(prop, ListProperty):
-                prop = _unpack_list_property(prop)
-            if not isinstance(prop, EnumProperty):
-                continue
-
-            if prop.reference.class_name in enums:
-                # We already have an enum with this name, make sure the values match
-                assert (
-                    prop.values == enums[prop.reference.class_name].values
-                ), f"Encountered conflicting enum named {prop.reference.class_name}"
-
-            enums[prop.reference.class_name] = prop
-        return enums
-
     @staticmethod
     def from_dict(d: Dict[str, Dict[str, Any]], /) -> OpenAPI:
         """ Create an OpenAPI from dict """
         schemas = Schema.dict(d["components"]["schemas"])
         endpoint_collections_by_tag = EndpointCollection.from_dict(d["paths"])
-        enums = OpenAPI._check_enums(schemas.values(), endpoint_collections_by_tag.values())
+        enums = EnumProperty.get_all_enums()
 
         return OpenAPI(
             title=d["info"]["title"],
diff --git a/openapi_python_client/openapi_parser/properties.py b/openapi_python_client/openapi_parser/properties.py
@@ -1,4 +1,6 @@
-from dataclasses import dataclass, field
+from __future__ import annotations
+
+from dataclasses import InitVar, dataclass, field
 from typing import Any, ClassVar, Dict, Generic, List, Optional, Set, TypeVar, Union
 
 from openapi_python_client import utils
@@ -222,20 +224,40 @@ def get_imports(self, *, prefix: str) -> Set[str]:
         return imports
 
 
+_existing_enums: Dict[str, EnumProperty] = {}
+
+
 @dataclass
 class EnumProperty(Property):
     """ A property that should use an enum """
 
     values: Dict[str, str]
-    reference: Reference
+    reference: Reference = field(init=False)
+    title: InitVar[str]
 
     template: ClassVar[str] = "enum_property.pyi"
 
-    def __post_init__(self) -> None:
+    def __post_init__(self, title: str) -> None:  # type: ignore
         super().__post_init__()
+        reference = Reference.from_ref(title)
+        dedup_counter = 0
+        while reference.class_name in _existing_enums:
+            existing = _existing_enums[reference.class_name]
+            if self.values == existing.values:
+                break  # This is the same Enum, we're good
+            dedup_counter += 1
+            reference = Reference.from_ref(f"{reference.class_name}{dedup_counter}")
+
+        self.reference = reference
         inverse_values = {v: k for k, v in self.values.items()}
         if self.default is not None:
             self.default = f"{self.reference.class_name}.{inverse_values[self.default]}"
+        _existing_enums[self.reference.class_name] = self
+
+    @staticmethod
+    def get_all_enums() -> Dict[str, EnumProperty]:
+        """ Get all the EnumProperties that have been registered keyed by class name """
+        return _existing_enums
 
     def get_type_string(self) -> str:
         """ Get a string representation of type that should be used when declaring this property """
@@ -345,7 +367,7 @@ def property_from_dict(name: str, required: bool, data: Dict[str, Any]) -> Prope
             name=name,
             required=required,
             values=EnumProperty.values_from_list(data["enum"]),
-            reference=Reference.from_ref(data.get("title", name)),
+            title=data.get("title", name),
             default=data.get("default"),
         )
     if "$ref" in data:
diff --git a/openapi_python_client/templates/async_endpoint_module.pyi b/openapi_python_client/templates/async_endpoint_module.pyi
@@ -11,7 +11,7 @@ from ..errors import ApiResponseError
 {% endfor %}
 {% for endpoint in collection.endpoints %}
 
-{% from "endpoint_macros.py.jinja" import query_params, json_body, return_type %}
+{% from "endpoint_macros.pyi" import query_params, json_body, return_type %}
 
 async def {{ endpoint.name | snakecase }}(
     *,
diff --git a/openapi_python_client/templates/endpoint_macros.pyi b/openapi_python_client/templates/endpoint_macros.pyi
diff --git a/openapi_python_client/templates/endpoint_module.pyi b/openapi_python_client/templates/endpoint_module.pyi
@@ -11,7 +11,7 @@ from ..errors import ApiResponseError
 {% endfor %}
 {% for endpoint in collection.endpoints %}
 
-{% from "endpoint_macros.py.jinja" import query_params, json_body, return_type %}
+{% from "endpoint_macros.pyi" import query_params, json_body, return_type %}
 
 def {{ endpoint.name | snakecase }}(
     *,
diff --git a/tests/test_openapi_parser/test_openapi.py b/tests/test_openapi_parser/test_openapi.py
diff --git a/tests/test_openapi_parser/test_properties.py b/tests/test_openapi_parser/test_properties.py

Original file line number	Diff line number	Diff line change
`@@ -164,8 +164,8 @@`
`164`	`164`	`"type": "array",`
`165`	`165`	`"items": {`
`166`	`166`	`"enum": [`
`167`		`- "FIRST_VALUE",`
`168`		`- "SECOND_VALUE"`
	`167`	`+ "DIFFERENT",`
	`168`	`+ "OTHER"`
`169`	`169`	`]`
`170`	`170`	`}`
`171`	`171`	`}`