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 all 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
12 changes: 6 additions & 6 deletions openapi_python_client/parser/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
41 changes: 34 additions & 7 deletions openapi_python_client/parser/properties/__init__.py
Original file line number Diff line number Diff line change
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 = 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(
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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(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=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
),
schemas,
)
return PropertyError(data=data, detail=f"unknown type {data.type}"), schemas


Expand Down
1 change: 1 addition & 0 deletions openapi_python_client/parser/properties/model_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"')
Expand Down
Loading