Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -19,3 +20,7 @@ def default(cls) -> Type[DefaultEndpoints]:
@classmethod
def parameters(cls) -> Type[ParametersEndpoints]:
return ParametersEndpoints

@classmethod
def tag1(cls) -> Type[Tag1Endpoints]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! cc @bowenwr @GitOnUp on this feature, of using the tags to define class methods on the client. This is just like what we've done on our wrapper client with the "services".

return Tag1Endpoints
Original file line number Diff line number Diff line change
@@ -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
Empty file.
Original file line number Diff line number Diff line change
@@ -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)
10 changes: 10 additions & 0 deletions end_to_end_tests/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,16 @@
}
}
}
},
"/tag_with_number": {
"get": {
"tags": [1],
"responses": {
"200": {
"description": "Success"
}
}
}
}
},
"components": {
Expand Down
18 changes: 13 additions & 5 deletions end_to_end_tests/test_end_to_end.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import shutil
from filecmp import cmpfiles, dircmp
from pathlib import Path
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions openapi_python_client/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 7 additions & 6 deletions openapi_python_client/parser/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ..config import Config
from .errors import GeneratorError, ParseError, PropertyError
from .properties import Class, EnumProperty, ModelProperty, Property, Schemas, build_schemas, property_from_data
from .properties.property import _PythonIdentifier, to_valid_python_identifier
from .responses import Response, response_from_data


Expand All @@ -30,9 +31,9 @@ class EndpointCollection:
@staticmethod
def from_data(
*, data: Dict[str, oai.PathItem], schemas: Schemas, config: Config
) -> Tuple[Dict[str, "EndpointCollection"], Schemas]:
) -> Tuple[Dict[_PythonIdentifier, "EndpointCollection"], Schemas]:
"""Parse the openapi paths data to get EndpointCollections by tag"""
endpoints_by_tag: Dict[str, EndpointCollection] = {}
endpoints_by_tag: Dict[_PythonIdentifier, EndpointCollection] = {}

methods = ["get", "put", "post", "delete", "options", "head", "patch", "trace"]

Expand All @@ -41,7 +42,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 = to_valid_python_identifier(value=(operation.tags or ["default"])[0], prefix="tag")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be too much work to make "tag" configurable, like field prefix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be pretty easy to add in a future PR if anyone wants it.

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
Expand Down Expand Up @@ -261,8 +262,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)

Expand Down Expand Up @@ -340,7 +341,7 @@ class GeneratorData:
version: str
models: Iterator[ModelProperty]
errors: List[ParseError]
endpoint_collections_by_tag: Dict[str, EndpointCollection]
endpoint_collections_by_tag: Dict[_PythonIdentifier, EndpointCollection]
enums: Iterator[EnumProperty]

@staticmethod
Expand Down
43 changes: 35 additions & 8 deletions openapi_python_client/parser/properties/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from .converter import convert, convert_chain
from .enum_property import EnumProperty
from .model_property import ModelProperty, build_model_property
from .property import Property
from .property import Property, to_valid_python_identifier
from .schemas import Class, Schemas, parse_reference_path, update_schemas_with_data


Expand Down Expand Up @@ -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)
)
Expand Down Expand Up @@ -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 = to_valid_python_identifier(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(
Expand All @@ -267,6 +270,7 @@ def _string_based_property(
required=required,
pattern=data.pattern,
nullable=data.nullable,
python_name=python_name,
)


Expand Down Expand Up @@ -326,6 +330,7 @@ def build_enum_property(
values=values,
value_type=value_type,
default=None,
python_name=to_valid_python_identifier(value=name, prefix=config.field_prefix),
)

default = get_enum_default(prop, data)
Expand Down Expand Up @@ -373,6 +378,7 @@ def build_union_property(
default=default,
inner_properties=sub_properties,
nullable=data.nullable,
python_name=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
Expand All @@ -395,6 +401,7 @@ def build_list_property(
default=None,
inner_property=inner_prop,
nullable=data.nullable,
python_name=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
Expand All @@ -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):
Expand All @@ -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=to_valid_python_identifier(value=name, prefix=config.field_prefix),
)
if parent:
prop = attr.evolve(prop, nullable=parent.nullable)
if isinstance(prop, EnumProperty):
Expand All @@ -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(
Expand All @@ -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=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
Expand All @@ -477,6 +493,7 @@ def _property_from_data(
default=convert("int", data.default),
required=required,
nullable=data.nullable,
python_name=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
Expand All @@ -487,6 +504,7 @@ def _property_from_data(
required=required,
default=convert("bool", data.default),
nullable=data.nullable,
python_name=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
Expand All @@ -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=to_valid_python_identifier(value=name, prefix=config.field_prefix),
),
schemas,
)
return PropertyError(data=data, detail=f"unknown type {data.type}"), schemas


Expand Down
3 changes: 2 additions & 1 deletion openapi_python_client/parser/properties/model_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from ... import schema as oai
from ... import utils
from ..errors import ParseError, PropertyError
from .property import Property
from .property import Property, to_valid_python_identifier
from .schemas import Class, Schemas, parse_reference_path


Expand Down Expand Up @@ -210,6 +210,7 @@ def build_model_property(
required=required,
name=name,
additional_properties=additional_properties,
python_name=to_valid_python_identifier(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}"')
Expand Down
Loading