fix!: Normalize generated module names to allow more tags [#428 & #448]. Thanks @iamnoah & @forest-benchling!

* fix!: Turn all tags (endpoints) into valid Python identifiers [WIP]

* test: Fix all failing unit tests

* test: Add e2e test for tag beginning with number

* chore(deps): Update deps, add dataclasses types for Python 3.6

* refactor: Switch _PythonIdentifier to a public custom class in `utils`

Author: dbanty

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

@@ -4,6 +4,7 @@
 
 from my_test_api_client.api.default import DefaultEndpoints
 from my_test_api_client.api.parameters import ParametersEndpoints
+from my_test_api_client.api.tag1 import Tag1Endpoints
 from my_test_api_client.api.tests import TestsEndpoints
 
 
@@ -19,3 +20,7 @@ def default(cls) -> Type[DefaultEndpoints]:
     @classmethod
     def parameters(cls) -> Type[ParametersEndpoints]:
         return ParametersEndpoints
+
+    @classmethod
+    def tag1(cls) -> Type[Tag1Endpoints]:
+        return Tag1Endpoints

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

@@ -0,0 +1,11 @@
+""" Contains methods for accessing the API Endpoints """
+
+import types
+
+from my_test_api_client.api.tag1 import get_tag_with_number
+
+
+class Tag1Endpoints:
+    @classmethod
+    def get_tag_with_number(cls) -> types.ModuleType:
+        return get_tag_with_number

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

@@ -0,0 +1,61 @@
+from typing import Any, Dict
+
+import httpx
+
+from ...client import Client
+from ...types import Response
+
+
+def _get_kwargs(
+    *,
+    client: Client,
+) -> Dict[str, Any]:
+    url = "{}/tag_with_number".format(client.base_url)
+
+    headers: Dict[str, Any] = client.get_headers()
+    cookies: Dict[str, Any] = client.get_cookies()
+
+    return {
+        "url": url,
+        "headers": headers,
+        "cookies": cookies,
+        "timeout": client.get_timeout(),
+    }
+
+
+def _build_response(*, response: httpx.Response) -> Response[Any]:
+    return Response(
+        status_code=response.status_code,
+        content=response.content,
+        headers=response.headers,
+        parsed=None,
+    )
+
+
+def sync_detailed(
+    *,
+    client: Client,
+) -> Response[Any]:
+    kwargs = _get_kwargs(
+        client=client,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    *,
+    client: Client,
+) -> Response[Any]:
+    kwargs = _get_kwargs(
+        client=client,
+    )
+
+    async with httpx.AsyncClient() as _client:
+        response = await _client.get(**kwargs)
+
+    return _build_response(response=response)

end_to_end_tests/golden-record/my_test_api_client/api/tag1/get_tag_with_number.py

@@ -823,6 +823,16 @@
           }
         }
       }
+    },
+    "/tag_with_number": {
+      "get": {
+        "tags": [1],
+        "responses": {
+          "200": {
+            "description": "Success"
+          }
+        }
+      }
     }
   },
   "components": {

end_to_end_tests/openapi.json

@@ -1,3 +1,4 @@
+import os
 import shutil
 from filecmp import cmpfiles, dircmp
 from pathlib import Path
@@ -101,18 +102,25 @@ def test_end_to_end():
 
 def test_custom_templates():
     expected_differences = {}  # key: path relative to generated directory, value: expected generated content
+    api_dir = Path("my_test_api_client").joinpath("api")
+    golden_tpls_root_dir = Path(__file__).parent.joinpath("custom-templates-golden-record")
+
     expected_difference_paths = [
         Path("README.md"),
-        Path("my_test_api_client").joinpath("api", "__init__.py"),
-        Path("my_test_api_client").joinpath("api", "tests", "__init__.py"),
-        Path("my_test_api_client").joinpath("api", "default", "__init__.py"),
-        Path("my_test_api_client").joinpath("api", "parameters", "__init__.py"),
+        api_dir.joinpath("__init__.py"),
     ]
 
-    golden_tpls_root_dir = Path(__file__).parent.joinpath("custom-templates-golden-record")
     for expected_difference_path in expected_difference_paths:
         expected_differences[expected_difference_path] = (golden_tpls_root_dir / expected_difference_path).read_text()
 
+    # Each API module (defined by tag) has a custom __init__.py in it now.
+    for endpoint_mod in golden_tpls_root_dir.joinpath(api_dir).iterdir():
+        if not endpoint_mod.is_dir():
+            continue
+        relative_path = api_dir.joinpath(endpoint_mod.name, "__init__.py")
+        expected_text = endpoint_mod.joinpath("__init__.py").read_text()
+        expected_differences[relative_path] = expected_text
+
     run_e2e_test(
         extra_args=["--custom-template-path=end_to_end_tests/test_custom_templates/"],
         expected_differences=expected_differences,

end_to_end_tests/test_end_to_end.py

@@ -20,9 +20,6 @@ class Config(BaseModel):
     @staticmethod
     def load_from_path(path: Path) -> "Config":
         """Creates a Config from provided JSON or YAML file and sets a bunch of globals from it"""
-        from . import utils
-
         config_data = yaml.safe_load(path.read_text())
         config = Config(**config_data)
-        utils.FIELD_PREFIX = config.field_prefix
         return config

openapi_python_client/config.py

@@ -30,9 +30,9 @@ class EndpointCollection:
     @staticmethod
     def from_data(
         *, data: Dict[str, oai.PathItem], schemas: Schemas, config: Config
-    ) -> Tuple[Dict[str, "EndpointCollection"], Schemas]:
+    ) -> Tuple[Dict[utils.PythonIdentifier, "EndpointCollection"], Schemas]:
         """Parse the openapi paths data to get EndpointCollections by tag"""
-        endpoints_by_tag: Dict[str, EndpointCollection] = {}
+        endpoints_by_tag: Dict[utils.PythonIdentifier, EndpointCollection] = {}
 
         methods = ["get", "put", "post", "delete", "options", "head", "patch", "trace"]
 
@@ -41,7 +41,7 @@ def from_data(
                 operation: Optional[oai.Operation] = getattr(path_data, method)
                 if operation is None:
                     continue
-                tag = utils.snake_case((operation.tags or ["default"])[0])
+                tag = utils.PythonIdentifier(value=(operation.tags or ["default"])[0], prefix="tag")
                 collection = endpoints_by_tag.setdefault(tag, EndpointCollection(tag=tag))
                 endpoint, schemas = Endpoint.from_data(
                     data=operation, path=path, method=method, tag=tag, schemas=schemas, config=config
@@ -261,8 +261,8 @@ def _add_parameters(
             if prop.python_name in used_python_names:
                 duplicate, duplicate_location = used_python_names[prop.python_name]
                 if duplicate.python_name == prop.python_name:  # Existing should be converted too for consistency
-                    duplicate.set_python_name(f"{duplicate.python_name}_{duplicate_location}")
-                prop.set_python_name(f"{prop.python_name}_{param.param_in}")
+                    duplicate.set_python_name(f"{duplicate.python_name}_{duplicate_location}", config=config)
+                prop.set_python_name(f"{prop.python_name}_{param.param_in}", config=config)
             else:
                 used_python_names[prop.python_name] = (prop, param.param_in)
 
@@ -340,7 +340,7 @@ class GeneratorData:
     version: str
     models: Iterator[ModelProperty]
     errors: List[ParseError]
-    endpoint_collections_by_tag: Dict[str, EndpointCollection]
+    endpoint_collections_by_tag: Dict[utils.PythonIdentifier, EndpointCollection]
     enums: Iterator[EnumProperty]
 
     @staticmethod

openapi_python_client/parser/openapi.py

@@ -177,7 +177,6 @@ class UnionProperty(Property):
     has_properties_without_templates: bool = attr.ib(init=False)
 
     def __attrs_post_init__(self) -> None:
-        super().__attrs_post_init__()
         object.__setattr__(
             self, "has_properties_without_templates", any(prop.template is None for prop in self.inner_properties)
         )
@@ -235,30 +234,34 @@ def inner_properties_with_template(self) -> Iterator[Property]:
 
 
 def _string_based_property(
-    name: str, required: bool, data: oai.Schema
+    name: str, required: bool, data: oai.Schema, config: Config
 ) -> Union[StringProperty, DateProperty, DateTimeProperty, FileProperty]:
     """Construct a Property from the type "string" """
     string_format = data.schema_format
+    python_name = utils.PythonIdentifier(value=name, prefix=config.field_prefix)
     if string_format == "date-time":
         return DateTimeProperty(
             name=name,
             required=required,
             default=convert("datetime.datetime", data.default),
             nullable=data.nullable,
+            python_name=python_name,
         )
     elif string_format == "date":
         return DateProperty(
             name=name,
             required=required,
             default=convert("datetime.date", data.default),
             nullable=data.nullable,
+            python_name=python_name,
         )
     elif string_format == "binary":
         return FileProperty(
             name=name,
             required=required,
             default=None,
             nullable=data.nullable,
+            python_name=python_name,
         )
     else:
         return StringProperty(
@@ -267,6 +270,7 @@ def _string_based_property(
             required=required,
             pattern=data.pattern,
             nullable=data.nullable,
+            python_name=python_name,
         )
 
 
@@ -326,6 +330,7 @@ def build_enum_property(
         values=values,
         value_type=value_type,
         default=None,
+        python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
     )
 
     default = get_enum_default(prop, data)
@@ -373,6 +378,7 @@ def build_union_property(
             default=default,
             inner_properties=sub_properties,
             nullable=data.nullable,
+            python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
         ),
         schemas,
     )
@@ -395,6 +401,7 @@ def build_list_property(
             default=None,
             inner_property=inner_prop,
             nullable=data.nullable,
+            python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
         ),
         schemas,
     )
@@ -406,6 +413,7 @@ def _property_from_ref(
     parent: Union[oai.Schema, None],
     data: oai.Reference,
     schemas: Schemas,
+    config: Config,
 ) -> Tuple[Union[Property, PropertyError], Schemas]:
     ref_path = parse_reference_path(data.ref)
     if isinstance(ref_path, ParseError):
@@ -414,7 +422,12 @@ def _property_from_ref(
     if not existing:
         return PropertyError(data=data, detail="Could not find reference in parsed models or enums"), schemas
 
-    prop = attr.evolve(existing, required=required, name=name)
+    prop = attr.evolve(
+        existing,
+        required=required,
+        name=name,
+        python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+    )
     if parent:
         prop = attr.evolve(prop, nullable=parent.nullable)
         if isinstance(prop, EnumProperty):
@@ -437,12 +450,14 @@ def _property_from_data(
     """Generate a Property from the OpenAPI dictionary representation of it"""
     name = utils.remove_string_escapes(name)
     if isinstance(data, oai.Reference):
-        return _property_from_ref(name=name, required=required, parent=None, data=data, schemas=schemas)
+        return _property_from_ref(name=name, required=required, parent=None, data=data, schemas=schemas, config=config)
 
     # A union of a single reference should just be passed through to that reference (don't create copy class)
     sub_data = (data.allOf or []) + data.anyOf + data.oneOf
     if len(sub_data) == 1 and isinstance(sub_data[0], oai.Reference):
-        return _property_from_ref(name=name, required=required, parent=data, data=sub_data[0], schemas=schemas)
+        return _property_from_ref(
+            name=name, required=required, parent=data, data=sub_data[0], schemas=schemas, config=config
+        )
 
     if data.enum:
         return build_enum_property(
@@ -459,14 +474,15 @@ def _property_from_data(
             data=data, name=name, required=required, schemas=schemas, parent_name=parent_name, config=config
         )
     elif data.type == "string":
-        return _string_based_property(name=name, required=required, data=data), schemas
+        return _string_based_property(name=name, required=required, data=data, config=config), schemas
     elif data.type == "number":
         return (
             FloatProperty(
                 name=name,
                 default=convert("float", data.default),
                 required=required,
                 nullable=data.nullable,
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
             ),
             schemas,
         )
@@ -477,6 +493,7 @@ def _property_from_data(
                 default=convert("int", data.default),
                 required=required,
                 nullable=data.nullable,
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
             ),
             schemas,
         )
@@ -487,6 +504,7 @@ def _property_from_data(
                 required=required,
                 default=convert("bool", data.default),
                 nullable=data.nullable,
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
             ),
             schemas,
         )
@@ -499,7 +517,16 @@ def _property_from_data(
             data=data, name=name, schemas=schemas, required=required, parent_name=parent_name, config=config
         )
     elif not data.type:
-        return AnyProperty(name=name, required=required, nullable=False, default=None), schemas
+        return (
+            AnyProperty(
+                name=name,
+                required=required,
+                nullable=False,
+                default=None,
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+            ),
+            schemas,
+        )
     return PropertyError(data=data, detail=f"unknown type {data.type}"), schemas
 
 

openapi_python_client/parser/properties/__init__.py

@@ -210,6 +210,7 @@ def build_model_property(
         required=required,
         name=name,
         additional_properties=additional_properties,
+        python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
     )
     if class_info.name in schemas.classes_by_name:
         error = PropertyError(data=data, detail=f'Attempted to generate duplicate models with name "{class_info.name}"')