From 1b8299ea9f422884473b7a6d8cd555a24895b87b Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Mon, 3 Mar 2025 17:00:52 -0600 Subject: [PATCH 1/7] Add enumerate_duplicate_model_names config option --- openapi_python_client/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openapi_python_client/config.py b/openapi_python_client/config.py index 1616ac785..6df6a278d 100644 --- a/openapi_python_client/config.py +++ b/openapi_python_client/config.py @@ -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_" @@ -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 @@ -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, From e37715e401c08b624f2dad75c8ebf03ea3a13378 Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Mon, 3 Mar 2025 17:01:19 -0600 Subject: [PATCH 2/7] Update names on duplicate with config option --- openapi_python_client/parser/properties/model_property.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 762624501..6813847f2 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -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 = 2 + 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 From 2c4e4e13bef078b4cef035fab72f31e1410c4fd7 Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Mon, 3 Mar 2025 17:01:34 -0600 Subject: [PATCH 3/7] Update test --- .../test_properties/test_model_property.py | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index a51fd984b..046c394e8 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -165,15 +165,28 @@ 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, expected", + ids=( + "name without duplicate suffix", + "name with duplicate suffix", + "name with duplicate suffix and matching existing name", + ), + argvalues=( + (["OtherModel"], "OtherModel", None, 'Attempted to generate duplicate models with name "OtherModel"'), + (["OtherModel"], "OtherModel", True, "OtherModel2"), + (["OtherModel", "OtherModel2"], "OtherModel", True, "OtherModel3"), + ), + ) + def test_model_name_conflict(self, existing_names: str, new_name: str, enumerate_duplicate_model_names: Optional[str], 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, @@ -182,8 +195,13 @@ 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) + assert isinstance(result, (PropertyError, ModelProperty)) + if isinstance(result, PropertyError): + assert new_schemas == schemas + assert result == PropertyError(detail=expected, data=data) + else: # 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", From 30f42ac3da08c24211a762972ac78ae9c641826f Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Mon, 3 Mar 2025 17:13:19 -0600 Subject: [PATCH 4/7] Update README --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index b07e7b29b..66fd2a400 100644 --- a/README.md +++ b/README.md @@ -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 2. For instance, if `MyModelName` already exists, then the next time a model with that name is encountered, it will be named `MyModelName2`, then `MyModelName3` 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. From 73875859d6c65dfbea6debe75dc77ae5b945da33 Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Tue, 4 Mar 2025 09:11:41 -0600 Subject: [PATCH 5/7] Start enumeration at 1 --- README.md | 2 +- .../parser/properties/model_property.py | 2 +- .../test_properties/test_model_property.py | 27 ++++++++++++------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 66fd2a400..268204e52 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ If this option results in conflicts, you will need to manually override class na 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 2. For instance, if `MyModelName` already exists, then the next time a model with that name is encountered, it will be named `MyModelName2`, then `MyModelName3` and so on. +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 occurances in the schema of `MyModelName`, the initial occurance will remain `MyModelName` and subsequent occurances will be named `MyModelName1`, `MyModelName2` and so on. ### http_timeout diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 6813847f2..c7070494a 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -75,7 +75,7 @@ def build( class_string = title class_info = Class.from_string(string=class_string, config=config) if config.enumerate_duplicate_model_names: - suffix = 2 + 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 diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index 046c394e8..904a65ad1 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -166,19 +166,27 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_ ) @pytest.mark.parametrize( - "existing_names, new_name, enumerate_duplicate_model_names, expected", + "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, 'Attempted to generate duplicate models with name "OtherModel"'), - (["OtherModel"], "OtherModel", True, "OtherModel2"), - (["OtherModel", "OtherModel2"], "OtherModel", True, "OtherModel3"), + (["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], expected: str, config): + 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() @@ -195,11 +203,12 @@ def test_model_name_conflict(self, existing_names: str, new_name: str, enumerate process_properties=True, ) - assert isinstance(result, (PropertyError, ModelProperty)) - if isinstance(result, PropertyError): + if should_raise: + assert isinstance(result, PropertyError) assert new_schemas == schemas - assert result == PropertyError(detail=expected, data=data) - else: # ModelProperty + 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 From 0b93bbde32c92f51e62f0f7dccb12c32cecdad45 Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Tue, 4 Mar 2025 09:20:27 -0600 Subject: [PATCH 6/7] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 268204e52..81415c84a 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ If this option results in conflicts, you will need to manually override class na 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 occurances in the schema of `MyModelName`, the initial occurance will remain `MyModelName` and subsequent occurances will be named `MyModelName1`, `MyModelName2` and so on. +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 From 33200625311bb10bcacd28dabfddadd07183fbcc Mon Sep 17 00:00:00 2001 From: Ty Brockhoeft Date: Tue, 4 Mar 2025 09:29:06 -0600 Subject: [PATCH 7/7] Add changeset file --- .changeset/enumerate_duplicate_model_names.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/enumerate_duplicate_model_names.md diff --git a/.changeset/enumerate_duplicate_model_names.md b/.changeset/enumerate_duplicate_model_names.md new file mode 100644 index 000000000..aad7a6ffd --- /dev/null +++ b/.changeset/enumerate_duplicate_model_names.md @@ -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.