Skip to content

Enumerate duplicate model names #1212

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
13 changes: 13 additions & 0 deletions .changeset/enumerate_duplicate_model_names.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
default: minor
---

# Enumerate duplicate model names

#1212 by @tjb346

This addresses: https://github.com/openapi-generators/openapi-python-client/issues/652

Even with `use_path_prefixes_for_title_model_names` set to `true`, duplicate model class names can occur. By default, when duplicates are encountered they will be skipped. This can cause error when they are referenced later.

This enables setting `enumerate_duplicate_model_names` to `true` (`false` by default) in the config file which will result in a number being added to duplicate names starting with 1. For instance, if there are multiple occurrences in the schema of `MyModelName`, the initial occurrence will remain `MyModelName` and subsequent occurrences will be named `MyModelName1`, `MyModelName2` and so on.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ If you are carefully curating your `title` properties already to ensure no dupli

If this option results in conflicts, you will need to manually override class names instead via the `class_overrides` option.

### enumerate_duplicate_model_names

Even with `use_path_prefixes_for_title_model_names` set to `true`, duplicate model class names can occur. By default, when duplicates are encountered they will be skipped.

Setting `enumerate_duplicate_model_names` to `true` in your config file will result in a number being added to duplicate names starting with 1. For instance, if there are multiple occurrences in the schema of `MyModelName`, the initial occurrence will remain `MyModelName` and subsequent occurrences will be named `MyModelName1`, `MyModelName2` and so on.

### http_timeout

By default, the timeout for retrieving the schema file via HTTP is 5 seconds. In case there is an error when retrieving the schema, you might try and increase this setting to a higher value.
Expand Down
3 changes: 3 additions & 0 deletions openapi_python_client/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ConfigFile(BaseModel):
package_name_override: Optional[str] = None
package_version_override: Optional[str] = None
use_path_prefixes_for_title_model_names: bool = True
enumerate_duplicate_model_names: bool = False
post_hooks: Optional[list[str]] = None
docstrings_on_attributes: bool = False
field_prefix: str = "field_"
Expand Down Expand Up @@ -70,6 +71,7 @@ class Config:
package_name_override: Optional[str]
package_version_override: Optional[str]
use_path_prefixes_for_title_model_names: bool
enumerate_duplicate_model_names: bool
post_hooks: list[str]
docstrings_on_attributes: bool
field_prefix: str
Expand Down Expand Up @@ -112,6 +114,7 @@ def from_sources(
package_name_override=config_file.package_name_override,
package_version_override=config_file.package_version_override,
use_path_prefixes_for_title_model_names=config_file.use_path_prefixes_for_title_model_names,
enumerate_duplicate_model_names=config_file.enumerate_duplicate_model_names,
post_hooks=post_hooks,
docstrings_on_attributes=config_file.docstrings_on_attributes,
field_prefix=config_file.field_prefix,
Expand Down
5 changes: 5 additions & 0 deletions openapi_python_client/parser/properties/model_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ def build(
else:
class_string = title
class_info = Class.from_string(string=class_string, config=config)
if config.enumerate_duplicate_model_names:
suffix = 1
while class_info.name in schemas.classes_by_name:
class_info = Class.from_string(string=class_string + str(suffix), config=config)
suffix += 1
model_roots = {*roots, class_info.name}
required_properties: list[Property] | None = None
optional_properties: list[Property] | None = None
Expand Down
41 changes: 34 additions & 7 deletions tests/test_parser/test_properties/test_model_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,36 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_
additional_properties=ANY_ADDITIONAL_PROPERTY,
)

def test_model_name_conflict(self, config):
@pytest.mark.parametrize(
"existing_names, new_name, enumerate_duplicate_model_names, should_raise, expected",
ids=(
"name without duplicate suffix",
"name with duplicate suffix",
"name with duplicate suffix and matching existing name",
),
argvalues=(
(["OtherModel"], "OtherModel", None, True, 'Attempted to generate duplicate models with name "OtherModel"'),
(["OtherModel"], "OtherModel", True, False, "OtherModel1"),
(["OtherModel", "OtherModel1"], "OtherModel", True, False, "OtherModel2"),
),
)
def test_model_name_conflict(
self,
existing_names: str,
new_name: str,
enumerate_duplicate_model_names: Optional[str],
should_raise: bool,
expected: str,
config,
):
from openapi_python_client.parser.properties import ModelProperty

data = oai.Schema.model_construct()
schemas = Schemas(classes_by_name={"OtherModel": None})

err, new_schemas = ModelProperty.build(
schemas = Schemas(classes_by_name={name: None for name in existing_names})
config = evolve(config, enumerate_duplicate_model_names=enumerate_duplicate_model_names)
result, new_schemas = ModelProperty.build(
data=data,
name="OtherModel",
name=new_name,
schemas=schemas,
required=True,
parent_name=None,
Expand All @@ -182,8 +203,14 @@ def test_model_name_conflict(self, config):
process_properties=True,
)

assert new_schemas == schemas
assert err == PropertyError(detail='Attempted to generate duplicate models with name "OtherModel"', data=data)
if should_raise:
assert isinstance(result, PropertyError)
assert new_schemas == schemas
assert result.detail == expected
else:
assert isinstance(result, ModelProperty)
assert result.class_info.name in new_schemas.classes_by_name
assert result.class_info.name == expected

@pytest.mark.parametrize(
"name, title, parent_name, use_title_prefixing, expected",
Expand Down