From 174f090ebbf3966091653d56eea580f4c343b40a Mon Sep 17 00:00:00 2001 From: maz808 Date: Mon, 24 Jan 2022 23:42:30 -0600 Subject: [PATCH 1/2] Add support for recursive/circular refs This commit adds support for recursive and circular references in object properties and additionalProperties, but not in allOf. The changes include: -Delayed processing of schema model properties -Cascading removal of invalid schema reference dependencies -Prevention of self import in ModelProperty relative imports -Prevention of forward recursive type reference errors in generated modules -Logging for detection of recursive references in allOf --- .../my_test_api_client/models/__init__.py | 6 + .../models/model_with_circular_ref_a.py | 65 +++++ .../models/model_with_circular_ref_b.py | 65 +++++ ...circular_ref_in_additional_properties_a.py | 54 ++++ ...circular_ref_in_additional_properties_b.py | 54 ++++ .../models/model_with_recursive_ref.py | 64 +++++ ..._recursive_ref_in_additional_properties.py | 52 ++++ end_to_end_tests/openapi.json | 42 +++ .../parser/properties/__init__.py | 139 +++++++++- .../parser/properties/model_property.py | 168 +++++++++-- .../parser/properties/property.py | 26 +- .../parser/properties/schemas.py | 31 ++- .../templates/model.py.jinja | 4 +- tests/conftest.py | 11 +- .../test_parser/test_properties/test_init.py | 262 +++++++++++++++++- .../test_properties/test_model_property.py | 260 ++++++++++++++--- 16 files changed, 1178 insertions(+), 125 deletions(-) create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_a.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_b.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_a.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_b.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref_in_additional_properties.py diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 85e5243ec..51a2a60a4 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -31,10 +31,16 @@ from .model_with_additional_properties_refed import ModelWithAdditionalPropertiesRefed from .model_with_any_json_properties import ModelWithAnyJsonProperties from .model_with_any_json_properties_additional_property_type_0 import ModelWithAnyJsonPropertiesAdditionalPropertyType0 +from .model_with_circular_ref_a import ModelWithCircularRefA +from .model_with_circular_ref_b import ModelWithCircularRefB +from .model_with_circular_ref_in_additional_properties_a import ModelWithCircularRefInAdditionalPropertiesA +from .model_with_circular_ref_in_additional_properties_b import ModelWithCircularRefInAdditionalPropertiesB from .model_with_date_time_property import ModelWithDateTimeProperty from .model_with_primitive_additional_properties import ModelWithPrimitiveAdditionalProperties from .model_with_primitive_additional_properties_a_date_holder import ModelWithPrimitiveAdditionalPropertiesADateHolder from .model_with_property_ref import ModelWithPropertyRef +from .model_with_recursive_ref import ModelWithRecursiveRef +from .model_with_recursive_ref_in_additional_properties import ModelWithRecursiveRefInAdditionalProperties from .model_with_union_property import ModelWithUnionProperty from .model_with_union_property_inlined import ModelWithUnionPropertyInlined from .model_with_union_property_inlined_fruit_type_0 import ModelWithUnionPropertyInlinedFruitType0 diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_a.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_a.py new file mode 100644 index 000000000..f9adb4bb1 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_a.py @@ -0,0 +1,65 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..models.model_with_circular_ref_b import ModelWithCircularRefB +from ..types import UNSET, Unset + +T = TypeVar("T", bound="ModelWithCircularRefA") + + +@attr.s(auto_attribs=True) +class ModelWithCircularRefA: + """ + Attributes: + circular (Union[Unset, ModelWithCircularRefB]): + """ + + circular: Union[Unset, ModelWithCircularRefB] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + circular: Union[Unset, Dict[str, Any]] = UNSET + if not isinstance(self.circular, Unset): + circular = self.circular.to_dict() + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if circular is not UNSET: + field_dict["circular"] = circular + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + _circular = d.pop("circular", UNSET) + circular: Union[Unset, ModelWithCircularRefB] + if isinstance(_circular, Unset): + circular = UNSET + else: + circular = ModelWithCircularRefB.from_dict(_circular) + + model_with_circular_ref_a = cls( + circular=circular, + ) + + model_with_circular_ref_a.additional_properties = d + return model_with_circular_ref_a + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_b.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_b.py new file mode 100644 index 000000000..25fa39b5f --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_b.py @@ -0,0 +1,65 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..models.model_with_circular_ref_a import ModelWithCircularRefA +from ..types import UNSET, Unset + +T = TypeVar("T", bound="ModelWithCircularRefB") + + +@attr.s(auto_attribs=True) +class ModelWithCircularRefB: + """ + Attributes: + circular (Union[Unset, ModelWithCircularRefA]): + """ + + circular: Union[Unset, ModelWithCircularRefA] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + circular: Union[Unset, Dict[str, Any]] = UNSET + if not isinstance(self.circular, Unset): + circular = self.circular.to_dict() + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if circular is not UNSET: + field_dict["circular"] = circular + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + _circular = d.pop("circular", UNSET) + circular: Union[Unset, ModelWithCircularRefA] + if isinstance(_circular, Unset): + circular = UNSET + else: + circular = ModelWithCircularRefA.from_dict(_circular) + + model_with_circular_ref_b = cls( + circular=circular, + ) + + model_with_circular_ref_b.additional_properties = d + return model_with_circular_ref_b + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_a.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_a.py new file mode 100644 index 000000000..3349d1429 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_a.py @@ -0,0 +1,54 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +from ..models.model_with_circular_ref_in_additional_properties_b import ModelWithCircularRefInAdditionalPropertiesB + +T = TypeVar("T", bound="ModelWithCircularRefInAdditionalPropertiesA") + + +@attr.s(auto_attribs=True) +class ModelWithCircularRefInAdditionalPropertiesA: + """ """ + + additional_properties: Dict[str, ModelWithCircularRefInAdditionalPropertiesB] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = prop.to_dict() + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + model_with_circular_ref_in_additional_properties_a = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = ModelWithCircularRefInAdditionalPropertiesB.from_dict(prop_dict) + + additional_properties[prop_name] = additional_property + + model_with_circular_ref_in_additional_properties_a.additional_properties = additional_properties + return model_with_circular_ref_in_additional_properties_a + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> ModelWithCircularRefInAdditionalPropertiesB: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: ModelWithCircularRefInAdditionalPropertiesB) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_b.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_b.py new file mode 100644 index 000000000..99a9c5ed2 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_circular_ref_in_additional_properties_b.py @@ -0,0 +1,54 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +from ..models.model_with_circular_ref_in_additional_properties_a import ModelWithCircularRefInAdditionalPropertiesA + +T = TypeVar("T", bound="ModelWithCircularRefInAdditionalPropertiesB") + + +@attr.s(auto_attribs=True) +class ModelWithCircularRefInAdditionalPropertiesB: + """ """ + + additional_properties: Dict[str, ModelWithCircularRefInAdditionalPropertiesA] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = prop.to_dict() + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + model_with_circular_ref_in_additional_properties_b = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = ModelWithCircularRefInAdditionalPropertiesA.from_dict(prop_dict) + + additional_properties[prop_name] = additional_property + + model_with_circular_ref_in_additional_properties_b.additional_properties = additional_properties + return model_with_circular_ref_in_additional_properties_b + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> ModelWithCircularRefInAdditionalPropertiesA: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: ModelWithCircularRefInAdditionalPropertiesA) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref.py new file mode 100644 index 000000000..b60e5a100 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref.py @@ -0,0 +1,64 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..types import UNSET, Unset + +T = TypeVar("T", bound="ModelWithRecursiveRef") + + +@attr.s(auto_attribs=True) +class ModelWithRecursiveRef: + """ + Attributes: + recursive (Union[Unset, ModelWithRecursiveRef]): + """ + + recursive: Union[Unset, "ModelWithRecursiveRef"] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + recursive: Union[Unset, Dict[str, Any]] = UNSET + if not isinstance(self.recursive, Unset): + recursive = self.recursive.to_dict() + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if recursive is not UNSET: + field_dict["recursive"] = recursive + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + _recursive = d.pop("recursive", UNSET) + recursive: Union[Unset, ModelWithRecursiveRef] + if isinstance(_recursive, Unset): + recursive = UNSET + else: + recursive = ModelWithRecursiveRef.from_dict(_recursive) + + model_with_recursive_ref = cls( + recursive=recursive, + ) + + model_with_recursive_ref.additional_properties = d + return model_with_recursive_ref + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref_in_additional_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref_in_additional_properties.py new file mode 100644 index 000000000..64d327ee6 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_recursive_ref_in_additional_properties.py @@ -0,0 +1,52 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +T = TypeVar("T", bound="ModelWithRecursiveRefInAdditionalProperties") + + +@attr.s(auto_attribs=True) +class ModelWithRecursiveRefInAdditionalProperties: + """ """ + + additional_properties: Dict[str, "ModelWithRecursiveRefInAdditionalProperties"] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = prop.to_dict() + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + model_with_recursive_ref_in_additional_properties = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = ModelWithRecursiveRefInAdditionalProperties.from_dict(prop_dict) + + additional_properties[prop_name] = additional_property + + model_with_recursive_ref_in_additional_properties.additional_properties = additional_properties + return model_with_recursive_ref_in_additional_properties + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> "ModelWithRecursiveRefInAdditionalProperties": + return self.additional_properties[key] + + def __setitem__(self, key: str, value: "ModelWithRecursiveRefInAdditionalProperties") -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index b958e530b..a058567cb 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -1916,6 +1916,48 @@ "model.reference.with.Periods": { "type": "object", "description": "A Model with periods in its reference" + }, + "ModelWithRecursiveRef": { + "type": "object", + "properties": { + "recursive": { + "$ref": "#/components/schemas/ModelWithRecursiveRef" + } + } + }, + "ModelWithCircularRefA": { + "type": "object", + "properties": { + "circular": { + "$ref": "#/components/schemas/ModelWithCircularRefB" + } + } + }, + "ModelWithCircularRefB": { + "type": "object", + "properties": { + "circular": { + "$ref": "#/components/schemas/ModelWithCircularRefA" + } + } + }, + "ModelWithRecursiveRefInAdditionalProperties": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ModelWithRecursiveRefInAdditionalProperties" + } + }, + "ModelWithCircularRefInAdditionalPropertiesA": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ModelWithCircularRefInAdditionalPropertiesB" + } + }, + "ModelWithCircularRefInAdditionalPropertiesB": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ModelWithCircularRefInAdditionalPropertiesA" + } } } } diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index 524ff5ba0..c4c6a903e 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -20,9 +20,9 @@ from ..errors import ParseError, PropertyError, ValidationError from .converter import convert, convert_chain from .enum_property import EnumProperty -from .model_property import ModelProperty, build_model_property +from .model_property import ModelProperty, build_model_property, process_model from .property import Property -from .schemas import Class, Schemas, parse_reference_path, update_schemas_with_data +from .schemas import Class, ReferencePath, Schemas, parse_reference_path, update_schemas_with_data @attr.s(auto_attribs=True, frozen=True) @@ -246,7 +246,13 @@ def get_type_strings_in_union(self, no_optional: bool = False, json: bool = Fals type_strings.add("Unset") return type_strings - def get_type_string(self, no_optional: bool = False, json: bool = False) -> str: + def get_type_string( + self, + no_optional: bool = False, + json: bool = False, + *, + model_parent: Optional[ModelProperty] = None, # pylint:disable=unused-argument + ) -> str: """ Get a string representation of type that should be used when declaring this property. This implementation differs slightly from `Property.get_type_string` in order to collapse @@ -484,7 +490,14 @@ def build_union_property( def build_list_property( - *, data: oai.Schema, name: str, required: bool, schemas: Schemas, parent_name: str, config: Config + *, + data: oai.Schema, + name: str, + required: bool, + schemas: Schemas, + parent_name: str, + config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[ListProperty[Any], PropertyError], Schemas]: """ Build a ListProperty the right way, use this instead of the normal constructor. @@ -504,7 +517,13 @@ def build_list_property( if data.items is None: return PropertyError(data=data, detail="type array must have items defined"), schemas inner_prop, schemas = property_from_data( - name=f"{name}_item", required=True, data=data.items, schemas=schemas, parent_name=parent_name, config=config + name=f"{name}_item", + required=True, + data=data.items, + schemas=schemas, + parent_name=parent_name, + config=config, + roots=roots, ) if isinstance(inner_prop, PropertyError): inner_prop.header = f'invalid data in items of array named "{name}"' @@ -532,6 +551,7 @@ def _property_from_ref( data: oai.Reference, schemas: Schemas, config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[Property, PropertyError], Schemas]: ref_path = parse_reference_path(data.ref) if isinstance(ref_path, ParseError): @@ -554,6 +574,7 @@ def _property_from_ref( return default, schemas prop = attr.evolve(prop, default=default) + schemas.add_dependencies(ref_path=ref_path, roots=roots) return prop, schemas @@ -565,17 +586,20 @@ def _property_from_data( schemas: Schemas, parent_name: str, config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[Property, PropertyError], Schemas]: """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, config=config) + return _property_from_ref( + name=name, required=required, parent=None, data=data, schemas=schemas, config=config, roots=roots + ) sub_data: List[Union[oai.Schema, oai.Reference]] = data.allOf + data.anyOf + data.oneOf # A union of a single reference should just be passed through to that reference (don't create copy class) 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, config=config + name=name, required=required, parent=data, data=sub_data[0], schemas=schemas, config=config, roots=roots ) if data.enum: @@ -635,11 +659,23 @@ def _property_from_data( ) if data.type == oai.DataType.ARRAY: return build_list_property( - data=data, name=name, required=required, schemas=schemas, parent_name=parent_name, config=config + data=data, + name=name, + required=required, + schemas=schemas, + parent_name=parent_name, + config=config, + roots=roots, ) if data.type == oai.DataType.OBJECT or data.allOf: return build_model_property( - data=data, name=name, schemas=schemas, required=required, parent_name=parent_name, config=config + data=data, + name=name, + schemas=schemas, + required=required, + parent_name=parent_name, + config=config, + roots=roots, ) return ( AnyProperty( @@ -663,6 +699,7 @@ def property_from_data( schemas: Schemas, parent_name: str, config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]] = None, ) -> Tuple[Union[Property, PropertyError], Schemas]: """ Build a Property from an OpenAPI schema or reference. This Property represents a single input or output for a @@ -682,23 +719,30 @@ def property_from_data( of duplication. config: Contains the parsed config that the user provided to tweak generation settings. Needed to apply class name overrides for generated classes. - + roots: The set of `ReferencePath`s and `ClassName`s to remove from the schemas if a child reference becomes + invalid Returns: A tuple containing either the parsed Property or a PropertyError (if something went wrong) and the updated Schemas (including any new classes that should be generated). """ + roots = roots or set() try: return _property_from_data( - name=name, required=required, data=data, schemas=schemas, parent_name=parent_name, config=config + name=name, + required=required, + data=data, + schemas=schemas, + parent_name=parent_name, + config=config, + roots=roots, ) except ValidationError: return PropertyError(detail="Failed to validate default value", data=data), schemas -def build_schemas( +def _create_schemas( *, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas, config: Config ) -> Schemas: - """Get a list of Schemas from an OpenAPI dict""" to_process: Iterable[Tuple[str, Union[oai.Reference, oai.Schema]]] = components.items() still_making_progress = True errors: List[PropertyError] = [] @@ -727,4 +771,73 @@ def build_schemas( to_process = next_round schemas.errors.extend(errors) + object.__setattr__(schemas, "schemas_created", True) + return schemas + + +def _propogate_removal(*, root: Union[ReferencePath, utils.ClassName], schemas: Schemas, error: PropertyError) -> None: + if isinstance(root, utils.ClassName): + schemas.classes_by_name.pop(root, None) + return + if root in schemas.classes_by_reference: + error.detail = error.detail or "" + error.detail += f"\n{root}" + del schemas.classes_by_reference[root] + for child in schemas.dependencies.get(root, set()): + _propogate_removal(root=child, schemas=schemas, error=error) + + +def _process_model_errors( + model_errors: List[Tuple[ModelProperty, PropertyError]], *, schemas: Schemas +) -> List[PropertyError]: + for model, error in model_errors: + error.detail = error.detail or "" + error.detail += "\n\nFailure to process schema has resulted in the removal of:" + for root in model.roots: + _propogate_removal(root=root, schemas=schemas, error=error) + return [error for _, error in model_errors] + + +def _process_models(*, schemas: Schemas, config: Config) -> Schemas: + to_process = (prop for prop in schemas.classes_by_reference.values() if isinstance(prop, ModelProperty)) + still_making_progress = True + final_model_errors: List[Tuple[ModelProperty, PropertyError]] = [] + latest_model_errors: List[Tuple[ModelProperty, PropertyError]] = [] + + # Models which refer to other models in their allOf must be processed after their referenced models + while still_making_progress: + still_making_progress = False + # Only accumulate errors from the last round, since we might fix some along the way + latest_model_errors = [] + next_round = [] + for model_prop in to_process: + schemas_or_err = process_model(model_prop, schemas=schemas, config=config) + if isinstance(schemas_or_err, PropertyError): + schemas_or_err.header = f"\nUnable to process schema {model_prop.name}:" + if isinstance(schemas_or_err.data, oai.Reference) and schemas_or_err.data.ref.endswith( + f"/{model_prop.class_info.name}" + ): + schemas_or_err.detail = schemas_or_err.detail or "" + schemas_or_err.detail += "\n\nRecursive allOf reference found" + final_model_errors.append((model_prop, schemas_or_err)) + continue + latest_model_errors.append((model_prop, schemas_or_err)) + next_round.append(model_prop) + continue + schemas = schemas_or_err + still_making_progress = True + to_process = (prop for prop in next_round) + + final_model_errors.extend(latest_model_errors) + errors = _process_model_errors(final_model_errors, schemas=schemas) + schemas.errors.extend(errors) + return schemas + + +def build_schemas( + *, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas, config: Config +) -> Schemas: + """Get a list of Schemas from an OpenAPI dict""" + schemas = _create_schemas(components=components, schemas=schemas, config=config) + schemas = _process_models(schemas=schemas, config=config) return schemas diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 6e68a8f8e..01a756d5a 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -9,7 +9,7 @@ from ..errors import ParseError, PropertyError from .enum_property import EnumProperty from .property import Property -from .schemas import Class, Schemas, parse_reference_path +from .schemas import Class, ReferencePath, Schemas, parse_reference_path @attr.s(auto_attribs=True, frozen=True) @@ -17,17 +17,28 @@ class ModelProperty(Property): """A property which refers to another Schema""" class_info: Class - required_properties: List[Property] - optional_properties: List[Property] + data: oai.Schema description: str - relative_imports: Set[str] - additional_properties: Union[bool, Property] + roots: Set[Union[ReferencePath, utils.ClassName]] + required_properties: Optional[List[Property]] + optional_properties: Optional[List[Property]] + relative_imports: Optional[Set[str]] + additional_properties: Optional[Union[bool, Property]] _json_type_string: ClassVar[str] = "Dict[str, Any]" template: ClassVar[str] = "model_property.py.jinja" json_is_dict: ClassVar[bool] = True is_multipart_body: bool = False + def __attrs_post_init__(self) -> None: + if self.relative_imports: + self.set_relative_imports(self.relative_imports) + + @property + def self_import(self) -> str: + """Constructs a self import statement from this ModelProperty's attributes""" + return f"models.{self.class_info.module_name} import {self.class_info.name}" + def get_base_type_string(self) -> str: return self.class_info.name @@ -42,13 +53,21 @@ def get_imports(self, *, prefix: str) -> Set[str]: imports = super().get_imports(prefix=prefix) imports.update( { - f"from {prefix}models.{self.class_info.module_name} import {self.class_info.name}", + f"from {prefix}{self.self_import}", "from typing import Dict", "from typing import cast", } ) return imports + def set_relative_imports(self, relative_imports: Set[str]) -> None: + """Set the relative imports set for this ModelProperty, filtering out self imports + + Args: + relative_imports: The set of relative import strings + """ + object.__setattr__(self, "relative_imports", {ri for ri in relative_imports if self.self_import not in ri}) + def _values_are_subset(first: EnumProperty, second: EnumProperty) -> bool: return set(first.values.items()) <= set(second.values.items()) @@ -111,9 +130,14 @@ class _PropertyData(NamedTuple): schemas: Schemas -# pylint: disable=too-many-locals,too-many-branches +# pylint: disable=too-many-locals,too-many-branches,too-many-return-statements def _process_properties( - *, data: oai.Schema, schemas: Schemas, class_name: str, config: Config + *, + data: oai.Schema, + schemas: Schemas, + class_name: utils.ClassName, + config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Union[_PropertyData, PropertyError]: from . import property_from_data @@ -145,10 +169,16 @@ def _add_if_no_conflict(new_prop: Property) -> Optional[PropertyError]: return PropertyError(f"Reference {sub_prop.ref} not found") if not isinstance(sub_model, ModelProperty): return PropertyError("Cannot take allOf a non-object") + # Properties of allOf references first should be processed first + if not ( + isinstance(sub_model.required_properties, list) and isinstance(sub_model.optional_properties, list) + ): + return PropertyError(f"Reference {sub_model.name} in allOf was not processed", data=sub_prop) for prop in chain(sub_model.required_properties, sub_model.optional_properties): err = _add_if_no_conflict(prop) if err is not None: return err + schemas.add_dependencies(ref_path=ref_path, roots=roots) else: unprocessed_props.update(sub_prop.properties or {}) required_set.update(sub_prop.required or []) @@ -157,7 +187,13 @@ def _add_if_no_conflict(new_prop: Property) -> Optional[PropertyError]: prop_required = key in required_set prop_or_error: Union[Property, PropertyError, None] prop_or_error, schemas = property_from_data( - name=key, required=prop_required, data=value, schemas=schemas, parent_name=class_name, config=config + name=key, + required=prop_required, + data=value, + schemas=schemas, + parent_name=class_name, + config=config, + roots=roots, ) if isinstance(prop_or_error, Property): prop_or_error = _add_if_no_conflict(prop_or_error) @@ -185,8 +221,9 @@ def _get_additional_properties( *, schema_additional: Union[None, bool, oai.Reference, oai.Schema], schemas: Schemas, - class_name: str, + class_name: utils.ClassName, config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[bool, Property, PropertyError], Schemas]: from . import property_from_data @@ -207,12 +244,79 @@ def _get_additional_properties( schemas=schemas, parent_name=class_name, config=config, + roots=roots, ) return additional_properties, schemas +def _process_property_data( + *, + data: oai.Schema, + schemas: Schemas, + class_info: Class, + config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], +) -> Tuple[Union[Tuple[_PropertyData, Union[bool, Property]], PropertyError], Schemas]: + property_data = _process_properties( + data=data, schemas=schemas, class_name=class_info.name, config=config, roots=roots + ) + if isinstance(property_data, PropertyError): + return property_data, schemas + schemas = property_data.schemas + + additional_properties, schemas = _get_additional_properties( + schema_additional=data.additionalProperties, + schemas=schemas, + class_name=class_info.name, + config=config, + roots=roots, + ) + if isinstance(additional_properties, Property): + property_data.relative_imports.update(additional_properties.get_imports(prefix="..")) + elif isinstance(additional_properties, PropertyError): + return additional_properties, schemas + + return (property_data, additional_properties), schemas + + +def process_model(model_prop: ModelProperty, *, schemas: Schemas, config: Config) -> Union[Schemas, PropertyError]: + """Populate a ModelProperty instance's property data + Args: + model_prop: The ModelProperty to build property data for + schemas: Existing Schemas + config: Config data for this run of the generator, used to modifying names + Returns: + Either the updated `schemas` input or a `PropertyError` if something went wrong. + """ + data_or_err, schemas = _process_property_data( + data=model_prop.data, + schemas=schemas, + class_info=model_prop.class_info, + config=config, + roots=model_prop.roots, + ) + if isinstance(data_or_err, PropertyError): + return data_or_err + + property_data, additional_properties = data_or_err + + object.__setattr__(model_prop, "required_properties", property_data.required_props) + object.__setattr__(model_prop, "optional_properties", property_data.optional_props) + model_prop.set_relative_imports(property_data.relative_imports) + object.__setattr__(model_prop, "additional_properties", additional_properties) + return schemas + + +# pylint: disable=too-many-locals def build_model_property( - *, data: oai.Schema, name: str, schemas: Schemas, required: bool, parent_name: Optional[str], config: Config + *, + data: oai.Schema, + name: str, + schemas: Schemas, + required: bool, + parent_name: Optional[str], + config: Config, + roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[ModelProperty, PropertyError], Schemas]: """ A single ModelProperty from its OAI data @@ -230,31 +334,39 @@ def build_model_property( if parent_name: class_string = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_string)}" class_info = Class.from_string(string=class_string, config=config) - - property_data = _process_properties(data=data, schemas=schemas, class_name=class_info.name, config=config) - if isinstance(property_data, PropertyError): - return property_data, schemas - schemas = property_data.schemas - - additional_properties, schemas = _get_additional_properties( - schema_additional=data.additionalProperties, schemas=schemas, class_name=class_info.name, config=config - ) - if isinstance(additional_properties, Property): - property_data.relative_imports.update(additional_properties.get_imports(prefix="..")) - elif isinstance(additional_properties, PropertyError): - return additional_properties, schemas + model_roots = {*roots, class_info.name} + required_properties: Optional[List[Property]] = None + optional_properties: Optional[List[Property]] = None + relative_imports: Optional[Set[str]] = None + additional_properties: Optional[Union[bool, Property]] = None + if schemas.schemas_created: + data_or_err, schemas = _process_property_data( + data=data, schemas=schemas, class_info=class_info, config=config, roots=model_roots + ) + if isinstance(data_or_err, PropertyError): + return data_or_err, schemas + property_data, additional_properties = data_or_err + required_properties = property_data.required_props + optional_properties = property_data.optional_props + relative_imports = property_data.relative_imports + for root in roots: + if isinstance(root, utils.ClassName): + continue + schemas.add_dependencies(root, {class_info.name}) prop = ModelProperty( class_info=class_info, - required_properties=property_data.required_props, - optional_properties=property_data.optional_props, - relative_imports=property_data.relative_imports, + data=data, + roots=model_roots, + required_properties=required_properties, + optional_properties=optional_properties, + relative_imports=relative_imports, + additional_properties=additional_properties, description=data.description or "", default=None, nullable=data.nullable, required=required, name=name, - additional_properties=additional_properties, python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix), example=data.example, ) diff --git a/openapi_python_client/parser/properties/property.py b/openapi_python_client/parser/properties/property.py index bcedfc3d9..9814faeb6 100644 --- a/openapi_python_client/parser/properties/property.py +++ b/openapi_python_client/parser/properties/property.py @@ -1,6 +1,6 @@ __all__ = ["Property"] -from typing import ClassVar, Optional, Set +from typing import TYPE_CHECKING, ClassVar, Optional, Set import attr @@ -9,6 +9,11 @@ from ...utils import PythonIdentifier from ..errors import ParseError +if TYPE_CHECKING: # pragma: no cover + from .model_property import ModelProperty +else: + ModelProperty = "ModelProperty" # pylint: disable=invalid-name + @attr.s(auto_attribs=True, frozen=True) class Property: @@ -68,7 +73,9 @@ def get_base_json_type_string(self) -> str: """Get the string describing the JSON type of this property.""" return self._json_type_string - def get_type_string(self, no_optional: bool = False, json: bool = False) -> str: + def get_type_string( + self, no_optional: bool = False, json: bool = False, *, model_parent: Optional[ModelProperty] = None + ) -> str: """ Get a string representation of type that should be used when declaring this property @@ -81,6 +88,9 @@ def get_type_string(self, no_optional: bool = False, json: bool = False) -> str: else: type_string = self.get_base_type_string() + if model_parent and type_string == model_parent.class_info.name: + type_string = f"'{type_string}'" + if no_optional or (self.required and not self.nullable): return type_string if self.required and self.nullable: @@ -111,8 +121,12 @@ def get_imports(self, *, prefix: str) -> Set[str]: imports.add(f"from {prefix}types import UNSET, Unset") return imports - def to_string(self) -> str: - """How this should be declared in a dataclass""" + def to_string(self, *, model_parent: Optional[ModelProperty] = None) -> str: + """How this should be declared in a dataclass + + Args: + model_parent: The ModelProperty which contains this Property (used for template type annotations) + """ default: Optional[str] if self.default is not None: default = self.default @@ -122,8 +136,8 @@ def to_string(self) -> str: default = None if default is not None: - return f"{self.python_name}: {self.get_type_string()} = {default}" - return f"{self.python_name}: {self.get_type_string()}" + return f"{self.python_name}: {self.get_type_string(model_parent=model_parent)} = {default}" + return f"{self.python_name}: {self.get_type_string(model_parent=model_parent)}" def to_docstring(self) -> str: """Returns property docstring""" diff --git a/openapi_python_client/parser/properties/schemas.py b/openapi_python_client/parser/properties/schemas.py index 9951f149f..e41225f2f 100644 --- a/openapi_python_client/parser/properties/schemas.py +++ b/openapi_python_client/parser/properties/schemas.py @@ -1,6 +1,6 @@ __all__ = ["Class", "Schemas", "parse_reference_path", "update_schemas_with_data"] -from typing import TYPE_CHECKING, Dict, List, NewType, Union, cast +from typing import TYPE_CHECKING, Dict, List, NewType, Set, Union, cast from urllib.parse import urlparse import attr @@ -16,10 +16,10 @@ Property = "Property" # pylint: disable=invalid-name -_ReferencePath = NewType("_ReferencePath", str) +ReferencePath = NewType("ReferencePath", str) -def parse_reference_path(ref_path_raw: str) -> Union[_ReferencePath, ParseError]: +def parse_reference_path(ref_path_raw: str) -> Union[ReferencePath, ParseError]: """ Takes a raw string provided in a `$ref` and turns it into a validated `_ReferencePath` or a `ParseError` if validation fails. @@ -30,7 +30,7 @@ def parse_reference_path(ref_path_raw: str) -> Union[_ReferencePath, ParseError] parsed = urlparse(ref_path_raw) if parsed.scheme or parsed.path: return ParseError(detail=f"Remote references such as {ref_path_raw} are not supported yet.") - return cast(_ReferencePath, parsed.fragment) + return cast(ReferencePath, parsed.fragment) @attr.s(auto_attribs=True, frozen=True) @@ -63,13 +63,25 @@ def from_string(*, string: str, config: Config) -> "Class": class Schemas: """Structure for containing all defined, shareable, and reusable schemas (attr classes and Enums)""" - classes_by_reference: Dict[_ReferencePath, Property] = attr.ib(factory=dict) + classes_by_reference: Dict[ReferencePath, Property] = attr.ib(factory=dict) + schemas_created: bool = False + dependencies: Dict[ReferencePath, Set[Union[ReferencePath, ClassName]]] = attr.ib(factory=dict) classes_by_name: Dict[ClassName, Property] = attr.ib(factory=dict) errors: List[ParseError] = attr.ib(factory=list) + def add_dependencies(self, ref_path: ReferencePath, roots: Set[Union[ReferencePath, ClassName]]) -> None: + """Record new dependencies on the given ReferencePath + + Args: + ref_path: The ReferencePath being referenced + roots: A set of identifiers for the objects dependent on the object corresponding to `ref_path` + """ + self.dependencies.setdefault(ref_path, set()) + self.dependencies[ref_path].update(roots) + def update_schemas_with_data( - *, ref_path: _ReferencePath, data: oai.Schema, schemas: Schemas, config: Config + *, ref_path: ReferencePath, data: oai.Schema, schemas: Schemas, config: Config ) -> Union[Schemas, PropertyError]: """ Update a `Schemas` using some new reference. @@ -90,17 +102,12 @@ def update_schemas_with_data( prop: Union[PropertyError, Property] prop, schemas = property_from_data( - data=data, name=ref_path, schemas=schemas, required=True, parent_name="", config=config + data=data, name=ref_path, schemas=schemas, required=True, parent_name="", config=config, roots={ref_path} ) if isinstance(prop, PropertyError): prop.detail = f"{prop.header}: {prop.detail}" prop.header = f"Unable to parse schema {ref_path}" - if isinstance(prop.data, oai.Reference) and prop.data.ref.endswith(ref_path): # pragma: nocover - prop.detail += ( - "\n\nRecursive and circular references are not supported. " - "See https://github.com/openapi-generators/openapi-python-client/issues/466" - ) return prop schemas = attr.evolve(schemas, classes_by_reference={ref_path: prop, **schemas.classes_by_reference}) diff --git a/openapi_python_client/templates/model.py.jinja b/openapi_python_client/templates/model.py.jinja index 07f929d66..1a5ce1879 100644 --- a/openapi_python_client/templates/model.py.jinja +++ b/openapi_python_client/templates/model.py.jinja @@ -18,7 +18,7 @@ from ..types import UNSET, Unset {% if model.additional_properties %} -{% set additional_property_type = 'Any' if model.additional_properties == True else model.additional_properties.get_type_string() %} +{% set additional_property_type = 'Any' if model.additional_properties == True else model.additional_properties.get_type_string(model_parent=model) %} {% endif %} {% set class_name = model.class_info.name %} @@ -57,7 +57,7 @@ class {{ class_name }}: {% endfor %} {% for property in model.required_properties + model.optional_properties %} {% if property.default is not none or not property.required %} - {{ property.to_string() }} + {{ property.to_string(model_parent=model) }} {% endif %} {% endfor %} {% if model.additional_properties %} diff --git a/tests/conftest.py b/tests/conftest.py index 2a683f102..ea0e71367 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ import pytest +from openapi_python_client import schema as oai from openapi_python_client.parser.properties import ( AnyProperty, DateProperty, @@ -32,10 +33,12 @@ def _factory(**kwargs): kwargs = { "description": "", "class_info": Class(name="MyClass", module_name="my_module"), - "required_properties": [], - "optional_properties": [], - "relative_imports": set(), - "additional_properties": False, + "data": oai.Schema.construct(), + "roots": set(), + "required_properties": None, + "optional_properties": None, + "relative_imports": None, + "additional_properties": None, "python_name": "", "description": "", "example": "", diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index 3d2de6519..faa8e1b4e 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/tests/test_parser/test_properties/test_init.py @@ -505,6 +505,29 @@ def test_property_from_data_ref_not_found(self, mocker): parse_reference_path.assert_called_once_with(data.ref) assert prop == PropertyError(data=data, detail="Could not find reference in parsed models or enums") assert schemas == new_schemas + assert schemas.dependencies == {} + + @pytest.mark.parametrize("references_exist", (True, False)) + def test_property_from_data_ref(self, property_factory, references_exist): + from openapi_python_client.parser.properties import Schemas, property_from_data + + name = "new_name" + required = False + ref_path = "/components/schemas/RefName" + data = oai.Reference.construct(ref=f"#{ref_path}") + roots = {"new_root"} + + existing_property = property_factory(name="old_name") + references = {ref_path: {"old_root"}} if references_exist else {} + schemas = Schemas(classes_by_reference={ref_path: existing_property}, dependencies=references) + + prop, new_schemas = property_from_data( + name=name, required=required, data=data, schemas=schemas, parent_name="", config=Config(), roots=roots + ) + + assert prop == property_factory(name=name, required=required) + assert schemas == new_schemas + assert schemas.dependencies == {ref_path: {*roots, *references.get(ref_path, set())}} def test_property_from_data_invalid_ref(self, mocker): from openapi_python_client.parser.properties import PropertyError, Schemas, property_from_data @@ -595,14 +618,21 @@ def test_property_from_data_array(self, mocker): mocker.patch("openapi_python_client.utils.remove_string_escapes", return_value=name) schemas = Schemas() config = MagicMock() + roots = {"root"} response = property_from_data( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config + name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots ) assert response == build_list_property.return_value build_list_property.assert_called_once_with( - data=data, name=name, required=required, schemas=schemas, parent_name="parent", config=config + data=data, + name=name, + required=required, + schemas=schemas, + parent_name="parent", + config=config, + roots=roots, ) def test_property_from_data_object(self, mocker): @@ -617,14 +647,21 @@ def test_property_from_data_object(self, mocker): mocker.patch("openapi_python_client.utils.remove_string_escapes", return_value=name) schemas = Schemas() config = MagicMock() + roots = {"root"} response = property_from_data( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config + name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots ) assert response == build_model_property.return_value build_model_property.assert_called_once_with( - data=data, name=name, required=required, schemas=schemas, parent_name="parent", config=config + data=data, + name=name, + required=required, + schemas=schemas, + parent_name="parent", + config=config, + roots=roots, ) def test_property_from_data_union(self, mocker): @@ -715,7 +752,13 @@ def test_build_list_property_no_items(self, mocker): schemas = properties.Schemas() p, new_schemas = properties.build_list_property( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=MagicMock() + name=name, + required=required, + data=data, + schemas=schemas, + parent_name="parent", + config=MagicMock(), + roots={"root"}, ) assert p == PropertyError(data=data, detail="type array must have items defined") @@ -737,9 +780,10 @@ def test_build_list_property_invalid_items(self, mocker): properties, "property_from_data", return_value=(properties.PropertyError(data="blah"), second_schemas) ) config = MagicMock() + roots = {"root"} p, new_schemas = properties.build_list_property( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config + name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots ) assert isinstance(p, PropertyError) @@ -748,7 +792,13 @@ def test_build_list_property_invalid_items(self, mocker): assert new_schemas == second_schemas assert schemas != new_schemas, "Schema was mutated" property_from_data.assert_called_once_with( - name=f"{name}_item", required=True, data=data.items, schemas=schemas, parent_name="parent", config=config + name=f"{name}_item", + required=True, + data=data.items, + schemas=schemas, + parent_name="parent", + config=config, + roots=roots, ) def test_build_list_property(self, any_property_factory): @@ -763,7 +813,7 @@ def test_build_list_property(self, any_property_factory): config = Config() p, new_schemas = properties.build_list_property( - name=name, required=True, data=data, schemas=schemas, parent_name="parent", config=config + name=name, required=True, data=data, schemas=schemas, parent_name="parent", config=config, roots={"root"} ) assert isinstance(p, properties.ListProperty) @@ -923,17 +973,18 @@ def test__string_based_property_unsupported_format(self, string_property_factory assert p == string_property_factory(name=name, required=required, nullable=nullable) -class TestBuildSchemas: +class TestCreateSchemas: def test_skips_references_and_keeps_going(self, mocker): - from openapi_python_client.parser.properties import Schemas, build_schemas + from openapi_python_client.parser.properties import Schemas, _create_schemas from openapi_python_client.schema import Reference, Schema components = {"a_ref": Reference.construct(), "a_schema": Schema.construct()} update_schemas_with_data = mocker.patch(f"{MODULE_NAME}.update_schemas_with_data") parse_reference_path = mocker.patch(f"{MODULE_NAME}.parse_reference_path") config = Config() + schemas = Schemas() - result = build_schemas(components=components, schemas=Schemas(), config=config) + result = _create_schemas(components=components, schemas=schemas, config=config) # Should not even try to parse a path for the Reference parse_reference_path.assert_called_once_with("#/components/schemas/a_schema") update_schemas_with_data.assert_called_once_with( @@ -945,9 +996,10 @@ def test_skips_references_and_keeps_going(self, mocker): ), ) assert result == update_schemas_with_data.return_value + assert result.schemas_created def test_records_bad_uris_and_keeps_going(self, mocker): - from openapi_python_client.parser.properties import Schemas, build_schemas + from openapi_python_client.parser.properties import Schemas, _create_schemas from openapi_python_client.schema import Schema components = {"first": Schema.construct(), "second": Schema.construct()} @@ -956,8 +1008,9 @@ def test_records_bad_uris_and_keeps_going(self, mocker): f"{MODULE_NAME}.parse_reference_path", side_effect=[PropertyError(detail="some details"), "a_path"] ) config = Config() + schemas = Schemas() - result = build_schemas(components=components, schemas=Schemas(), config=config) + result = _create_schemas(components=components, schemas=schemas, config=config) parse_reference_path.assert_has_calls( [ call("#/components/schemas/first"), @@ -971,9 +1024,10 @@ def test_records_bad_uris_and_keeps_going(self, mocker): schemas=Schemas(errors=[PropertyError(detail="some details", data=components["first"])]), ) assert result == update_schemas_with_data.return_value + assert result.schemas_created def test_retries_failing_properties_while_making_progress(self, mocker): - from openapi_python_client.parser.properties import Schemas, build_schemas + from openapi_python_client.parser.properties import Schemas, _create_schemas from openapi_python_client.schema import Schema components = {"first": Schema.construct(), "second": Schema.construct()} @@ -982,8 +1036,9 @@ def test_retries_failing_properties_while_making_progress(self, mocker): ) parse_reference_path = mocker.patch(f"{MODULE_NAME}.parse_reference_path") config = Config() + schemas = Schemas() - result = build_schemas(components=components, schemas=Schemas(), config=config) + result = _create_schemas(components=components, schemas=schemas, config=config) parse_reference_path.assert_has_calls( [ call("#/components/schemas/first"), @@ -993,6 +1048,165 @@ def test_retries_failing_properties_while_making_progress(self, mocker): ) assert update_schemas_with_data.call_count == 3 assert result.errors == [PropertyError()] + assert result.schemas_created + + +class TestProcessModels: + def test_retries_failing_models_while_making_progress(self, mocker, model_property_factory, property_factory): + from openapi_python_client.parser.properties import _process_models + + first_model = model_property_factory() + schemas = Schemas( + classes_by_reference={ + "first": first_model, + "second": model_property_factory(), + "non-model": property_factory(), + } + ) + process_model = mocker.patch( + f"{MODULE_NAME}.process_model", side_effect=[PropertyError(), Schemas(), PropertyError()] + ) + process_model_errors = mocker.patch(f"{MODULE_NAME}._process_model_errors", return_value=["error"]) + config = Config() + + result = _process_models(schemas=schemas, config=config) + + process_model.assert_has_calls( + [ + call(first_model, schemas=schemas, config=config), + call(schemas.classes_by_reference["second"], schemas=schemas, config=config), + call(first_model, schemas=result, config=config), + ] + ) + assert process_model_errors.was_called_once_with([(first_model, PropertyError())]) + assert all(error in result.errors for error in process_model_errors.return_value) + + def test_detect_recursive_allof_reference_no_retry(self, mocker, model_property_factory): + from openapi_python_client.parser.properties import Class, _process_models + from openapi_python_client.schema import Reference + + class_name = "class_name" + recursive_model = model_property_factory(class_info=Class(name=class_name, module_name="module_name")) + schemas = Schemas( + classes_by_reference={ + "recursive": recursive_model, + "second": model_property_factory(), + } + ) + recursion_error = PropertyError(data=Reference.construct(ref=f"#/{class_name}")) + process_model = mocker.patch(f"{MODULE_NAME}.process_model", side_effect=[recursion_error, Schemas()]) + process_model_errors = mocker.patch(f"{MODULE_NAME}._process_model_errors", return_value=["error"]) + config = Config() + + result = _process_models(schemas=schemas, config=config) + + process_model.assert_has_calls( + [ + call(recursive_model, schemas=schemas, config=config), + call(schemas.classes_by_reference["second"], schemas=schemas, config=config), + ] + ) + assert process_model_errors.was_called_once_with([(recursive_model, recursion_error)]) + assert all(error in result.errors for error in process_model_errors.return_value) + assert "\n\nRecursive allOf reference found" in recursion_error.detail + + +class TestPropogateRemoval: + def test_propogate_removal_class_name(self): + from openapi_python_client.parser.properties import ReferencePath, _propogate_removal + from openapi_python_client.utils import ClassName + + root = ClassName("ClassName", "") + ref_path = ReferencePath("/reference") + other_class_name = ClassName("OtherClassName", "") + schemas = Schemas( + classes_by_name={root: None, other_class_name: None}, + classes_by_reference={ref_path: None}, + dependencies={ref_path: {other_class_name}, root: {ref_path}}, + ) + error = PropertyError() + + _propogate_removal(root=root, schemas=schemas, error=error) + + assert schemas.classes_by_name == {other_class_name: None} + assert schemas.classes_by_reference == {ref_path: None} + assert not error.detail + + def test_propogate_removal_ref_path(self): + from openapi_python_client.parser.properties import ReferencePath, _propogate_removal + from openapi_python_client.utils import ClassName + + root = ReferencePath("/root/reference") + class_name = ClassName("ClassName", "") + ref_path = ReferencePath("/ref/path") + schemas = Schemas( + classes_by_name={class_name: None}, + classes_by_reference={root: None, ref_path: None}, + dependencies={root: {ref_path, class_name}}, + ) + error = PropertyError() + + _propogate_removal(root=root, schemas=schemas, error=error) + + assert schemas.classes_by_name == {} + assert schemas.classes_by_reference == {} + assert error.detail == f"\n{root}\n{ref_path}" + + def test_propogate_removal_ref_path_no_refs(self): + from openapi_python_client.parser.properties import ReferencePath, _propogate_removal + from openapi_python_client.utils import ClassName + + root = ReferencePath("/root/reference") + class_name = ClassName("ClassName", "") + ref_path = ReferencePath("/ref/path") + schemas = Schemas(classes_by_name={class_name: None}, classes_by_reference={root: None, ref_path: None}) + error = PropertyError() + + _propogate_removal(root=root, schemas=schemas, error=error) + + assert schemas.classes_by_name == {class_name: None} + assert schemas.classes_by_reference == {ref_path: None} + assert error.detail == f"\n{root}" + + def test_propogate_removal_ref_path_already_removed(self): + from openapi_python_client.parser.properties import ReferencePath, _propogate_removal + from openapi_python_client.utils import ClassName + + root = ReferencePath("/root/reference") + class_name = ClassName("ClassName", "") + ref_path = ReferencePath("/ref/path") + schemas = Schemas( + classes_by_name={class_name: None}, + classes_by_reference={ref_path: None}, + dependencies={root: {ref_path, class_name}}, + ) + error = PropertyError() + + _propogate_removal(root=root, schemas=schemas, error=error) + + assert schemas.classes_by_name == {class_name: None} + assert schemas.classes_by_reference == {ref_path: None} + assert not error.detail + + +def test_process_model_errors(mocker, model_property_factory): + from openapi_python_client.parser.properties import _process_model_errors + + propogate_removal = mocker.patch(f"{MODULE_NAME}._propogate_removal") + model_errors = [ + (model_property_factory(roots={"root1", "root2"}), PropertyError(detail="existing detail")), + (model_property_factory(roots=set()), PropertyError()), + (model_property_factory(roots={"root1", "root3"}), PropertyError(detail="other existing detail")), + ] + schemas = Schemas() + + result = _process_model_errors(model_errors, schemas=schemas) + + propogate_removal.assert_has_calls( + [call(root=root, schemas=schemas, error=error) for model, error in model_errors for root in model.roots] + ) + assert result == [error for _, error in model_errors] + assert all("\n\nFailure to process schema has resulted in the removal of:" in error.detail for error in result) def test_build_enum_property_conflict(): @@ -1038,3 +1252,21 @@ def test_build_enum_property_bad_default(): assert schemas == schemas assert err == PropertyError(detail="B is an invalid default for enum Existing", data=data) + + +def test_build_schemas(mocker): + from openapi_python_client.parser.properties import Schemas, build_schemas + from openapi_python_client.schema import Reference, Schema + + create_schemas = mocker.patch(f"{MODULE_NAME}._create_schemas") + process_models = mocker.patch(f"{MODULE_NAME}._process_models") + + components = {"a_ref": Reference.construct(), "a_schema": Schema.construct()} + schemas = Schemas() + config = Config() + + result = build_schemas(components=components, schemas=schemas, config=config) + + create_schemas.assert_called_once_with(components=components, schemas=schemas, config=config) + process_models.assert_called_once_with(schemas=create_schemas.return_value, config=config) + assert result == process_models.return_value diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index 7b96cb687..12a2eef82 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -7,6 +7,8 @@ from openapi_python_client.parser.errors import PropertyError from openapi_python_client.parser.properties import StringProperty +MODULE_NAME = "openapi_python_client.parser.properties.model_property" + @pytest.mark.parametrize( "no_optional,nullable,required,json,expected", @@ -75,7 +77,13 @@ def test_additional_schemas(self, additional_properties_schema, expected_additio ) model, _ = build_model_property( - data=data, name="prop", schemas=Schemas(), required=True, parent_name="parent", config=Config() + data=data, + name="prop", + schemas=Schemas(schemas_created=True), + required=True, + parent_name="parent", + config=Config(), + roots={"root"}, ) assert model.additional_properties == expected_additional_properties @@ -97,10 +105,14 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_ description="A class called MyModel", nullable=nullable, ) - schemas = Schemas(classes_by_reference={"OtherModel": None}, classes_by_name={"OtherModel": None}) + schemas = Schemas( + classes_by_reference={"OtherModel": None}, classes_by_name={"OtherModel": None}, schemas_created=True + ) + class_info = Class(name="ParentMyModel", module_name="parent_my_model") + roots = {"root"} model, new_schemas = build_model_property( - data=data, name=name, schemas=schemas, required=required, parent_name="parent", config=Config() + data=data, name=name, schemas=schemas, required=required, parent_name="parent", config=Config(), roots=roots ) assert new_schemas != schemas @@ -111,11 +123,14 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_ assert new_schemas.classes_by_reference == { "OtherModel": None, } + assert new_schemas.dependencies == {"root": {class_info.name}} assert model == model_property_factory( name=name, required=required, nullable=nullable, - class_info=Class(name="ParentMyModel", module_name="parent_my_model"), + roots={*roots, class_info.name}, + data=data, + class_info=class_info, required_properties=[string_property_factory(name="req", required=True)], optional_properties=[date_time_property_factory(name="opt", required=False)], description=data.description, @@ -136,7 +151,13 @@ def test_model_name_conflict(self): schemas = Schemas(classes_by_name={"OtherModel": None}) err, new_schemas = build_model_property( - data=data, name="OtherModel", schemas=schemas, required=True, parent_name=None, config=Config() + data=data, + name="OtherModel", + schemas=schemas, + required=True, + parent_name=None, + config=Config(), + roots={"root"}, ) assert new_schemas == schemas @@ -151,7 +172,13 @@ def test_model_bad_properties(self): }, ) result = build_model_property( - data=data, name="prop", schemas=Schemas(), required=True, parent_name="parent", config=Config() + data=data, + name="prop", + schemas=Schemas(schemas_created=True), + required=True, + parent_name="parent", + config=Config(), + roots={"root"}, )[0] assert isinstance(result, PropertyError) @@ -166,10 +193,59 @@ def test_model_bad_additional_properties(self): ) data = oai.Schema(additionalProperties=additional_properties) result = build_model_property( - data=data, name="prop", schemas=Schemas(), required=True, parent_name="parent", config=Config() + data=data, + name="prop", + schemas=Schemas(schemas_created=True), + required=True, + parent_name="parent", + config=Config(), + roots={"root"}, )[0] assert isinstance(result, PropertyError) + def test_schemas_not_created(self, model_property_factory): + from openapi_python_client.parser.properties import Class, Schemas, build_model_property + + name = "prop" + nullable = False + required = True + + data = oai.Schema.construct( + required=["req"], + title="MyModel", + properties={ + "req": oai.Schema.construct(type="string"), + "opt": oai.Schema(type="string", format="date-time"), + }, + description="A class called MyModel", + nullable=nullable, + ) + schemas = Schemas(classes_by_reference={"OtherModel": None}, classes_by_name={"OtherModel": None}) + roots = {"root"} + class_info = Class(name="ParentMyModel", module_name="parent_my_model") + + model, new_schemas = build_model_property( + data=data, name=name, schemas=schemas, required=required, parent_name="parent", config=Config(), roots=roots + ) + + assert new_schemas != schemas + assert new_schemas.classes_by_name == { + "OtherModel": None, + "ParentMyModel": model, + } + assert new_schemas.classes_by_reference == { + "OtherModel": None, + } + assert model == model_property_factory( + name=name, + required=required, + nullable=nullable, + class_info=class_info, + data=data, + description=data.description, + roots={*roots, class_info.name}, + ) + class TestProcessProperties: def test_conflicting_properties_different_types( @@ -183,12 +259,16 @@ def test_conflicting_properties_different_types( ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[string_property_factory()]), - "/Second": model_property_factory(optional_properties=[date_time_property_factory()]), + "/First": model_property_factory( + required_properties=[], optional_properties=[string_property_factory()] + ), + "/Second": model_property_factory( + required_properties=[], optional_properties=[date_time_property_factory()] + ), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) @@ -202,18 +282,31 @@ def test_process_properties_reference_not_exist(self): }, ) - result = _process_properties(data=data, class_name="", schemas=Schemas(), config=Config()) + result = _process_properties(data=data, class_name="", schemas=Schemas(), config=Config(), roots={"root"}) assert isinstance(result, PropertyError) - def test_invalid_reference(self, model_property_factory): + def test_process_properties_model_property_roots(self, model_property_factory): + from openapi_python_client.parser.properties import Schemas + from openapi_python_client.parser.properties.model_property import _process_properties + + roots = {"root"} + data = oai.Schema(properties={"test_model_property": oai.Schema.construct(type="object")}) + + result = _process_properties( + data=data, class_name="", schemas=Schemas(schemas_created=True), config=Config(), roots=roots + ) + + assert all(root in result.optional_props[0].roots for root in roots) + + def test_invalid_reference(self): from openapi_python_client.parser.properties import Schemas from openapi_python_client.parser.properties.model_property import _process_properties data = oai.Schema.construct(allOf=[oai.Reference.construct(ref="ThisIsNotGood")]) schemas = Schemas() - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) @@ -228,7 +321,22 @@ def test_non_model_reference(self, enum_property_factory): } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) + + assert isinstance(result, PropertyError) + + def test_reference_not_processed(self, model_property_factory): + from openapi_python_client.parser.properties import Schemas + from openapi_python_client.parser.properties.model_property import _process_properties + + data = oai.Schema.construct(allOf=[oai.Reference.construct(ref="#/Unprocessed")]) + schemas = Schemas( + classes_by_reference={ + "/Unprocessed": model_property_factory(), + } + ) + + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) @@ -241,12 +349,16 @@ def test_conflicting_properties_same_types(self, model_property_factory, string_ ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[string_property_factory(default="abc")]), - "/Second": model_property_factory(optional_properties=[string_property_factory()]), + "/First": model_property_factory( + required_properties=[], optional_properties=[string_property_factory(default="abc")] + ), + "/Second": model_property_factory( + required_properties=[], optional_properties=[string_property_factory()] + ), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) @@ -263,13 +375,14 @@ def test_allof_string_and_string_enum(self, model_property_factory, enum_propert schemas = Schemas( classes_by_reference={ "/First": model_property_factory( - optional_properties=[string_property_factory(required=False, nullable=True)] + required_properties=[], + optional_properties=[string_property_factory(required=False, nullable=True)], ), - "/Second": model_property_factory(optional_properties=[enum_property]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.required_props[0] == enum_property def test_allof_string_enum_and_string(self, model_property_factory, enum_property_factory, string_property_factory): @@ -286,14 +399,15 @@ def test_allof_string_enum_and_string(self, model_property_factory, enum_propert ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[enum_property]), + "/First": model_property_factory(required_properties=[], optional_properties=[enum_property]), "/Second": model_property_factory( - optional_properties=[string_property_factory(required=False, nullable=True)] + required_properties=[], + optional_properties=[string_property_factory(required=False, nullable=True)], ), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.optional_props[0] == enum_property def test_allof_int_and_int_enum(self, model_property_factory, enum_property_factory, int_property_factory): @@ -309,12 +423,12 @@ def test_allof_int_and_int_enum(self, model_property_factory, enum_property_fact ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[int_property_factory()]), - "/Second": model_property_factory(optional_properties=[enum_property]), + "/First": model_property_factory(required_properties=[], optional_properties=[int_property_factory()]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.required_props[0] == enum_property def test_allof_enum_incompatible_type(self, model_property_factory, enum_property_factory, int_property_factory): @@ -330,12 +444,12 @@ def test_allof_enum_incompatible_type(self, model_property_factory, enum_propert ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[int_property_factory()]), - "/Second": model_property_factory(optional_properties=[enum_property]), + "/First": model_property_factory(required_properties=[], optional_properties=[int_property_factory()]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) def test_allof_string_enums(self, model_property_factory, enum_property_factory): @@ -357,12 +471,12 @@ def test_allof_string_enums(self, model_property_factory, enum_property_factory) ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[enum_property1]), - "/Second": model_property_factory(optional_properties=[enum_property2]), + "/First": model_property_factory(required_properties=[], optional_properties=[enum_property1]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property2]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.required_props[0] == enum_property1 def test_allof_int_enums(self, model_property_factory, enum_property_factory): @@ -384,12 +498,12 @@ def test_allof_int_enums(self, model_property_factory, enum_property_factory): ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[enum_property1]), - "/Second": model_property_factory(optional_properties=[enum_property2]), + "/First": model_property_factory(required_properties=[], optional_properties=[enum_property1]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property2]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.required_props[0] == enum_property2 def test_allof_enums_are_not_subsets(self, model_property_factory, enum_property_factory): @@ -411,12 +525,12 @@ def test_allof_enums_are_not_subsets(self, model_property_factory, enum_property ) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[enum_property1]), - "/Second": model_property_factory(optional_properties=[enum_property2]), + "/First": model_property_factory(required_properties=[], optional_properties=[enum_property1]), + "/Second": model_property_factory(required_properties=[], optional_properties=[enum_property2]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert isinstance(result, PropertyError) def test_duplicate_properties(self, model_property_factory, string_property_factory): @@ -429,12 +543,12 @@ def test_duplicate_properties(self, model_property_factory, string_property_fact prop = string_property_factory(nullable=True) schemas = Schemas( classes_by_reference={ - "/First": model_property_factory(optional_properties=[prop]), - "/Second": model_property_factory(optional_properties=[prop]), + "/First": model_property_factory(required_properties=[], optional_properties=[prop]), + "/Second": model_property_factory(required_properties=[], optional_properties=[prop]), } ) - result = _process_properties(data=data, schemas=schemas, class_name="", config=Config()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=Config(), roots={"root"}) assert result.optional_props == [prop], "There should only be one copy of duplicate properties" @@ -460,15 +574,18 @@ def test_mixed_requirements( schemas = Schemas( classes_by_reference={ "/First": model_property_factory( - optional_properties=[string_property_factory(required=first_required, nullable=first_nullable)] + required_properties=[], + optional_properties=[string_property_factory(required=first_required, nullable=first_nullable)], ), "/Second": model_property_factory( - optional_properties=[string_property_factory(required=second_required, nullable=second_nullable)] + required_properties=[], + optional_properties=[string_property_factory(required=second_required, nullable=second_nullable)], ), } ) + roots = {"root"} - result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock(), roots=roots) nullable = first_nullable and second_nullable required = first_required or second_required @@ -477,6 +594,7 @@ def test_mixed_requirements( required=required, ) + assert result.schemas.dependencies == {"/First": roots, "/Second": roots} if nullable or not required: assert result.optional_props == [expected_prop] else: @@ -499,7 +617,59 @@ def test_direct_properties_non_ref(self, string_property_factory): ) schemas = Schemas() - result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock()) + result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock(), roots={"root"}) assert result.optional_props == [string_property_factory(name="second", required=False, nullable=False)] assert result.required_props == [string_property_factory(name="first", required=True, nullable=False)] + + +class TestProcessModel: + def test_process_model_error(self, mocker, model_property_factory): + from openapi_python_client.parser.properties import Schemas + from openapi_python_client.parser.properties.model_property import process_model + + model_prop = model_property_factory() + schemas = Schemas() + process_property_data = mocker.patch(f"{MODULE_NAME}._process_property_data") + process_property_data.return_value = (PropertyError(), schemas) + + result = process_model(model_prop=model_prop, schemas=schemas, config=Config()) + + assert result == PropertyError() + assert model_prop.required_properties is None + assert model_prop.optional_properties is None + assert model_prop.relative_imports is None + assert model_prop.additional_properties is None + + def test_process_model(self, mocker, model_property_factory): + from openapi_python_client.parser.properties import Schemas + from openapi_python_client.parser.properties.model_property import _PropertyData, process_model + + model_prop = model_property_factory() + schemas = Schemas() + property_data = _PropertyData( + required_props=["required"], optional_props=["optional"], relative_imports={"relative"}, schemas=schemas + ) + additional_properties = True + process_property_data = mocker.patch(f"{MODULE_NAME}._process_property_data") + process_property_data.return_value = ((property_data, additional_properties), schemas) + + result = process_model(model_prop=model_prop, schemas=schemas, config=Config()) + + assert result == schemas + assert model_prop.required_properties == property_data.required_props + assert model_prop.optional_properties == property_data.optional_props + assert model_prop.relative_imports == property_data.relative_imports + assert model_prop.additional_properties == additional_properties + + +def test_set_relative_imports(model_property_factory): + from openapi_python_client.parser.properties import Class + from openapi_python_client.parser.properties.model_property import ModelProperty + + class_info = Class("ClassName", module_name="module_name") + relative_imports = {"from typing import List", f"from ..models.{class_info.module_name} import {class_info.name}"} + + model_property = model_property_factory(class_info=class_info, relative_imports=relative_imports) + + assert model_property.relative_imports == {"from typing import List"} From d9cb65d189fd617d43fd0126bc4d2dec736b8f71 Mon Sep 17 00:00:00 2001 From: maz808 Date: Sat, 29 Jan 2022 15:07:24 -0600 Subject: [PATCH 2/2] Fix array schema object item issues Previous changes prevented models defined in array schemas from being built properly. This commit fixes that issue and adds support for recursive and circular references in array schema item objects, a behavior that was previously expected but not functioning correctly. This does not add support for recursive and circular references directly in an array schema's 'items' section. However, this feature would be functionally useless, so a warning is logged on detection. --- .../my_test_api_client/models/__init__.py | 12 +++ ...h_a_circular_ref_in_items_object_a_item.py | 75 +++++++++++++++++ ...ems_object_additional_properties_a_item.py | 83 +++++++++++++++++++ ...ems_object_additional_properties_b_item.py | 83 +++++++++++++++++++ ...h_a_circular_ref_in_items_object_b_item.py | 75 +++++++++++++++++ ...items_object_additional_properties_item.py | 79 ++++++++++++++++++ ...th_a_recursive_ref_in_items_object_item.py | 74 +++++++++++++++++ end_to_end_tests/openapi.json | 60 ++++++++++++++ .../parser/properties/__init__.py | 12 ++- .../parser/properties/model_property.py | 5 +- .../parser/properties/property.py | 7 +- .../parser/properties/schemas.py | 15 +++- .../test_parser/test_properties/test_init.py | 54 +++++++++--- .../test_properties/test_model_property.py | 38 ++++++--- 14 files changed, 642 insertions(+), 30 deletions(-) create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_a_item.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_a_item.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_b_item.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_b_item.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_additional_properties_item.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_item.py diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 51a2a60a4..ff5f15941 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -6,6 +6,18 @@ from .all_of_sub_model import AllOfSubModel from .all_of_sub_model_type_enum import AllOfSubModelTypeEnum from .an_all_of_enum import AnAllOfEnum +from .an_array_with_a_circular_ref_in_items_object_a_item import AnArrayWithACircularRefInItemsObjectAItem +from .an_array_with_a_circular_ref_in_items_object_additional_properties_a_item import ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem, +) +from .an_array_with_a_circular_ref_in_items_object_additional_properties_b_item import ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem, +) +from .an_array_with_a_circular_ref_in_items_object_b_item import AnArrayWithACircularRefInItemsObjectBItem +from .an_array_with_a_recursive_ref_in_items_object_additional_properties_item import ( + AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem, +) +from .an_array_with_a_recursive_ref_in_items_object_item import AnArrayWithARecursiveRefInItemsObjectItem from .an_enum import AnEnum from .an_enum_with_null import AnEnumWithNull from .an_int_enum import AnIntEnum diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_a_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_a_item.py new file mode 100644 index 000000000..ee802e42d --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_a_item.py @@ -0,0 +1,75 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..models.an_array_with_a_circular_ref_in_items_object_b_item import AnArrayWithACircularRefInItemsObjectBItem +from ..types import UNSET, Unset + +T = TypeVar("T", bound="AnArrayWithACircularRefInItemsObjectAItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithACircularRefInItemsObjectAItem: + """ + Attributes: + circular (Union[Unset, List[AnArrayWithACircularRefInItemsObjectBItem]]): + """ + + circular: Union[Unset, List[AnArrayWithACircularRefInItemsObjectBItem]] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + circular: Union[Unset, List[Dict[str, Any]]] = UNSET + if not isinstance(self.circular, Unset): + circular = [] + for componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item_data in self.circular: + componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item = ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item_data.to_dict() + ) + + circular.append(componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item) + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if circular is not UNSET: + field_dict["circular"] = circular + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + circular = [] + _circular = d.pop("circular", UNSET) + for componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item_data in _circular or []: + componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item = ( + AnArrayWithACircularRefInItemsObjectBItem.from_dict( + componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item_data + ) + ) + + circular.append(componentsschemas_an_array_with_a_circular_ref_in_items_object_b_item) + + an_array_with_a_circular_ref_in_items_object_a_item = cls( + circular=circular, + ) + + an_array_with_a_circular_ref_in_items_object_a_item.additional_properties = d + return an_array_with_a_circular_ref_in_items_object_a_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_a_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_a_item.py new file mode 100644 index 000000000..125e2a8be --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_a_item.py @@ -0,0 +1,83 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +from ..models.an_array_with_a_circular_ref_in_items_object_additional_properties_b_item import ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem, +) + +T = TypeVar("T", bound="AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem: + """ """ + + additional_properties: Dict[str, List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem]] = attr.ib( + init=False, factory=dict + ) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = [] + for ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item_data + ) in prop: + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item = ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item_data.to_dict() + ) + + field_dict[prop_name].append( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item + ) + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + an_array_with_a_circular_ref_in_items_object_additional_properties_a_item = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = [] + _additional_property = prop_dict + for ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item_data + ) in _additional_property: + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item = ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem.from_dict( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item_data + ) + ) + + additional_property.append( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_b_item + ) + + additional_properties[prop_name] = additional_property + + an_array_with_a_circular_ref_in_items_object_additional_properties_a_item.additional_properties = ( + additional_properties + ) + return an_array_with_a_circular_ref_in_items_object_additional_properties_a_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem]: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem]) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_b_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_b_item.py new file mode 100644 index 000000000..b984f2d12 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_additional_properties_b_item.py @@ -0,0 +1,83 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +from ..models.an_array_with_a_circular_ref_in_items_object_additional_properties_a_item import ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem, +) + +T = TypeVar("T", bound="AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithACircularRefInItemsObjectAdditionalPropertiesBItem: + """ """ + + additional_properties: Dict[str, List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem]] = attr.ib( + init=False, factory=dict + ) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = [] + for ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item_data + ) in prop: + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item = ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item_data.to_dict() + ) + + field_dict[prop_name].append( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item + ) + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + an_array_with_a_circular_ref_in_items_object_additional_properties_b_item = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = [] + _additional_property = prop_dict + for ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item_data + ) in _additional_property: + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item = ( + AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem.from_dict( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item_data + ) + ) + + additional_property.append( + componentsschemas_an_array_with_a_circular_ref_in_items_object_additional_properties_a_item + ) + + additional_properties[prop_name] = additional_property + + an_array_with_a_circular_ref_in_items_object_additional_properties_b_item.additional_properties = ( + additional_properties + ) + return an_array_with_a_circular_ref_in_items_object_additional_properties_b_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem]: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: List[AnArrayWithACircularRefInItemsObjectAdditionalPropertiesAItem]) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_b_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_b_item.py new file mode 100644 index 000000000..6224bbc1e --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_circular_ref_in_items_object_b_item.py @@ -0,0 +1,75 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..models.an_array_with_a_circular_ref_in_items_object_a_item import AnArrayWithACircularRefInItemsObjectAItem +from ..types import UNSET, Unset + +T = TypeVar("T", bound="AnArrayWithACircularRefInItemsObjectBItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithACircularRefInItemsObjectBItem: + """ + Attributes: + circular (Union[Unset, List[AnArrayWithACircularRefInItemsObjectAItem]]): + """ + + circular: Union[Unset, List[AnArrayWithACircularRefInItemsObjectAItem]] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + circular: Union[Unset, List[Dict[str, Any]]] = UNSET + if not isinstance(self.circular, Unset): + circular = [] + for componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item_data in self.circular: + componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item = ( + componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item_data.to_dict() + ) + + circular.append(componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item) + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if circular is not UNSET: + field_dict["circular"] = circular + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + circular = [] + _circular = d.pop("circular", UNSET) + for componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item_data in _circular or []: + componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item = ( + AnArrayWithACircularRefInItemsObjectAItem.from_dict( + componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item_data + ) + ) + + circular.append(componentsschemas_an_array_with_a_circular_ref_in_items_object_a_item) + + an_array_with_a_circular_ref_in_items_object_b_item = cls( + circular=circular, + ) + + an_array_with_a_circular_ref_in_items_object_b_item.additional_properties = d + return an_array_with_a_circular_ref_in_items_object_b_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_additional_properties_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_additional_properties_item.py new file mode 100644 index 000000000..52341b8bc --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_additional_properties_item.py @@ -0,0 +1,79 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +T = TypeVar("T", bound="AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem: + """ """ + + additional_properties: Dict[str, List["AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem"]] = attr.ib( + init=False, factory=dict + ) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + for prop_name, prop in self.additional_properties.items(): + field_dict[prop_name] = [] + for componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item_data in prop: + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item = ( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item_data.to_dict() + ) + + field_dict[prop_name].append( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item + ) + + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + an_array_with_a_recursive_ref_in_items_object_additional_properties_item = cls() + + additional_properties = {} + for prop_name, prop_dict in d.items(): + additional_property = [] + _additional_property = prop_dict + for ( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item_data + ) in _additional_property: + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item = ( + AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem.from_dict( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item_data + ) + ) + + additional_property.append( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_additional_properties_item + ) + + additional_properties[prop_name] = additional_property + + an_array_with_a_recursive_ref_in_items_object_additional_properties_item.additional_properties = ( + additional_properties + ) + return an_array_with_a_recursive_ref_in_items_object_additional_properties_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> List["AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem"]: + return self.additional_properties[key] + + def __setitem__( + self, key: str, value: List["AnArrayWithARecursiveRefInItemsObjectAdditionalPropertiesItem"] + ) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_item.py b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_item.py new file mode 100644 index 000000000..7f6641985 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/an_array_with_a_recursive_ref_in_items_object_item.py @@ -0,0 +1,74 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +import attr + +from ..types import UNSET, Unset + +T = TypeVar("T", bound="AnArrayWithARecursiveRefInItemsObjectItem") + + +@attr.s(auto_attribs=True) +class AnArrayWithARecursiveRefInItemsObjectItem: + """ + Attributes: + recursive (Union[Unset, List[AnArrayWithARecursiveRefInItemsObjectItem]]): + """ + + recursive: Union[Unset, List["AnArrayWithARecursiveRefInItemsObjectItem"]] = UNSET + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + recursive: Union[Unset, List[Dict[str, Any]]] = UNSET + if not isinstance(self.recursive, Unset): + recursive = [] + for componentsschemas_an_array_with_a_recursive_ref_in_items_object_item_data in self.recursive: + componentsschemas_an_array_with_a_recursive_ref_in_items_object_item = ( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_item_data.to_dict() + ) + + recursive.append(componentsschemas_an_array_with_a_recursive_ref_in_items_object_item) + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if recursive is not UNSET: + field_dict["recursive"] = recursive + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + recursive = [] + _recursive = d.pop("recursive", UNSET) + for componentsschemas_an_array_with_a_recursive_ref_in_items_object_item_data in _recursive or []: + componentsschemas_an_array_with_a_recursive_ref_in_items_object_item = ( + AnArrayWithARecursiveRefInItemsObjectItem.from_dict( + componentsschemas_an_array_with_a_recursive_ref_in_items_object_item_data + ) + ) + + recursive.append(componentsschemas_an_array_with_a_recursive_ref_in_items_object_item) + + an_array_with_a_recursive_ref_in_items_object_item = cls( + recursive=recursive, + ) + + an_array_with_a_recursive_ref_in_items_object_item.additional_properties = d + return an_array_with_a_recursive_ref_in_items_object_item + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index a058567cb..6a924b8a0 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -1958,6 +1958,66 @@ "additionalProperties": { "$ref": "#/components/schemas/ModelWithCircularRefInAdditionalPropertiesA" } + }, + "AnArrayWithARecursiveRefInItemsObject": { + "type": "array", + "items": { + "type": "object", + "properties": { + "recursive": { + "$ref": "#/components/schemas/AnArrayWithARecursiveRefInItemsObject" + } + } + } + }, + "AnArrayWithACircularRefInItemsObjectA": { + "type": "array", + "items": { + "type": "object", + "properties": { + "circular": { + "$ref": "#/components/schemas/AnArrayWithACircularRefInItemsObjectB" + } + } + } + }, + "AnArrayWithACircularRefInItemsObjectB": { + "type": "array", + "items": { + "type": "object", + "properties": { + "circular": { + "$ref": "#/components/schemas/AnArrayWithACircularRefInItemsObjectA" + } + } + } + }, + "AnArrayWithARecursiveRefInItemsObjectAdditionalProperties": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/AnArrayWithARecursiveRefInItemsObjectAdditionalProperties" + } + } + }, + "AnArrayWithACircularRefInItemsObjectAdditionalPropertiesA": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/AnArrayWithACircularRefInItemsObjectAdditionalPropertiesB" + } + } + }, + "AnArrayWithACircularRefInItemsObjectAdditionalPropertiesB": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/AnArrayWithACircularRefInItemsObjectAdditionalPropertiesA" + } + } } } } diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index c4c6a903e..1167ddf0b 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -497,6 +497,7 @@ def build_list_property( schemas: Schemas, parent_name: str, config: Config, + process_properties: bool, roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[ListProperty[Any], PropertyError], Schemas]: """ @@ -523,6 +524,7 @@ def build_list_property( schemas=schemas, parent_name=parent_name, config=config, + process_properties=process_properties, roots=roots, ) if isinstance(inner_prop, PropertyError): @@ -586,6 +588,7 @@ def _property_from_data( schemas: Schemas, parent_name: str, config: Config, + process_properties: bool, roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[Property, PropertyError], Schemas]: """Generate a Property from the OpenAPI dictionary representation of it""" @@ -665,6 +668,7 @@ def _property_from_data( schemas=schemas, parent_name=parent_name, config=config, + process_properties=process_properties, roots=roots, ) if data.type == oai.DataType.OBJECT or data.allOf: @@ -675,6 +679,7 @@ def _property_from_data( required=required, parent_name=parent_name, config=config, + process_properties=process_properties, roots=roots, ) return ( @@ -699,6 +704,7 @@ def property_from_data( schemas: Schemas, parent_name: str, config: Config, + process_properties: bool = True, roots: Set[Union[ReferencePath, utils.ClassName]] = None, ) -> Tuple[Union[Property, PropertyError], Schemas]: """ @@ -719,6 +725,8 @@ def property_from_data( of duplication. config: Contains the parsed config that the user provided to tweak generation settings. Needed to apply class name overrides for generated classes. + process_properties: If the new property is a ModelProperty, determines whether it will be initialized with + property data roots: The set of `ReferencePath`s and `ClassName`s to remove from the schemas if a child reference becomes invalid Returns: @@ -734,6 +742,7 @@ def property_from_data( schemas=schemas, parent_name=parent_name, config=config, + process_properties=process_properties, roots=roots, ) except ValidationError: @@ -771,7 +780,6 @@ def _create_schemas( to_process = next_round schemas.errors.extend(errors) - object.__setattr__(schemas, "schemas_created", True) return schemas @@ -799,7 +807,7 @@ def _process_model_errors( def _process_models(*, schemas: Schemas, config: Config) -> Schemas: - to_process = (prop for prop in schemas.classes_by_reference.values() if isinstance(prop, ModelProperty)) + to_process = (prop for prop in schemas.classes_by_name.values() if isinstance(prop, ModelProperty)) still_making_progress = True final_model_errors: List[Tuple[ModelProperty, PropertyError]] = [] latest_model_errors: List[Tuple[ModelProperty, PropertyError]] = [] diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 01a756d5a..b4f51297b 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -316,6 +316,7 @@ def build_model_property( required: bool, parent_name: Optional[str], config: Config, + process_properties: bool, roots: Set[Union[ReferencePath, utils.ClassName]], ) -> Tuple[Union[ModelProperty, PropertyError], Schemas]: """ @@ -329,6 +330,8 @@ def build_model_property( required: Whether or not this property is required by the parent (affects typing) parent_name: The name of the property that this property is inside of (affects class naming) config: Config data for this run of the generator, used to modifying names + roots: Set of strings that identify schema objects on which the new ModelProperty will depend + process_properties: Determines whether the new ModelProperty will be initialized with property data """ class_string = data.title or name if parent_name: @@ -339,7 +342,7 @@ def build_model_property( optional_properties: Optional[List[Property]] = None relative_imports: Optional[Set[str]] = None additional_properties: Optional[Union[bool, Property]] = None - if schemas.schemas_created: + if process_properties: data_or_err, schemas = _process_property_data( data=data, schemas=schemas, class_info=class_info, config=config, roots=model_roots ) diff --git a/openapi_python_client/parser/properties/property.py b/openapi_python_client/parser/properties/property.py index 9814faeb6..ff0817cb3 100644 --- a/openapi_python_client/parser/properties/property.py +++ b/openapi_python_client/parser/properties/property.py @@ -88,8 +88,11 @@ def get_type_string( else: type_string = self.get_base_type_string() - if model_parent and type_string == model_parent.class_info.name: - type_string = f"'{type_string}'" + if model_parent: + if type_string == model_parent.class_info.name: + type_string = f"'{type_string}'" + if type_string == f"List[{model_parent.class_info.name}]": + type_string = f"List['{model_parent.class_info.name}']" if no_optional or (self.required and not self.nullable): return type_string diff --git a/openapi_python_client/parser/properties/schemas.py b/openapi_python_client/parser/properties/schemas.py index e41225f2f..6ac396008 100644 --- a/openapi_python_client/parser/properties/schemas.py +++ b/openapi_python_client/parser/properties/schemas.py @@ -64,7 +64,6 @@ class Schemas: """Structure for containing all defined, shareable, and reusable schemas (attr classes and Enums)""" classes_by_reference: Dict[ReferencePath, Property] = attr.ib(factory=dict) - schemas_created: bool = False dependencies: Dict[ReferencePath, Set[Union[ReferencePath, ClassName]]] = attr.ib(factory=dict) classes_by_name: Dict[ClassName, Property] = attr.ib(factory=dict) errors: List[ParseError] = attr.ib(factory=list) @@ -102,12 +101,24 @@ def update_schemas_with_data( prop: Union[PropertyError, Property] prop, schemas = property_from_data( - data=data, name=ref_path, schemas=schemas, required=True, parent_name="", config=config, roots={ref_path} + data=data, + name=ref_path, + schemas=schemas, + required=True, + parent_name="", + config=config, + # Don't process ModelProperty properties because schemas are still being created + process_properties=False, + roots={ref_path}, ) if isinstance(prop, PropertyError): prop.detail = f"{prop.header}: {prop.detail}" prop.header = f"Unable to parse schema {ref_path}" + if isinstance(prop.data, oai.Reference) and prop.data.ref.endswith(ref_path): # pragma: nocover + prop.detail += ( + "\n\nRecursive and circular references are not supported directly in an array schema's 'items' section" + ) return prop schemas = attr.evolve(schemas, classes_by_reference={ref_path: prop, **schemas.classes_by_reference}) diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index faa8e1b4e..73a6c08ae 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/tests/test_parser/test_properties/test_init.py @@ -619,9 +619,17 @@ def test_property_from_data_array(self, mocker): schemas = Schemas() config = MagicMock() roots = {"root"} + process_properties = False response = property_from_data( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots + name=name, + required=required, + data=data, + schemas=schemas, + parent_name="parent", + config=config, + roots=roots, + process_properties=process_properties, ) assert response == build_list_property.return_value @@ -632,6 +640,7 @@ def test_property_from_data_array(self, mocker): schemas=schemas, parent_name="parent", config=config, + process_properties=process_properties, roots=roots, ) @@ -648,9 +657,17 @@ def test_property_from_data_object(self, mocker): schemas = Schemas() config = MagicMock() roots = {"root"} + process_properties = False response = property_from_data( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots + name=name, + required=required, + data=data, + schemas=schemas, + parent_name="parent", + config=config, + process_properties=process_properties, + roots=roots, ) assert response == build_model_property.return_value @@ -661,6 +678,7 @@ def test_property_from_data_object(self, mocker): schemas=schemas, parent_name="parent", config=config, + process_properties=process_properties, roots=roots, ) @@ -758,6 +776,7 @@ def test_build_list_property_no_items(self, mocker): schemas=schemas, parent_name="parent", config=MagicMock(), + process_properties=True, roots={"root"}, ) @@ -780,10 +799,18 @@ def test_build_list_property_invalid_items(self, mocker): properties, "property_from_data", return_value=(properties.PropertyError(data="blah"), second_schemas) ) config = MagicMock() + process_properties = False roots = {"root"} p, new_schemas = properties.build_list_property( - name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=config, roots=roots + name=name, + required=required, + data=data, + schemas=schemas, + parent_name="parent", + config=config, + roots=roots, + process_properties=process_properties, ) assert isinstance(p, PropertyError) @@ -798,6 +825,7 @@ def test_build_list_property_invalid_items(self, mocker): schemas=schemas, parent_name="parent", config=config, + process_properties=process_properties, roots=roots, ) @@ -813,7 +841,14 @@ def test_build_list_property(self, any_property_factory): config = Config() p, new_schemas = properties.build_list_property( - name=name, required=True, data=data, schemas=schemas, parent_name="parent", config=config, roots={"root"} + name=name, + required=True, + data=data, + schemas=schemas, + parent_name="parent", + config=config, + roots={"root"}, + process_properties=True, ) assert isinstance(p, properties.ListProperty) @@ -996,7 +1031,6 @@ def test_skips_references_and_keeps_going(self, mocker): ), ) assert result == update_schemas_with_data.return_value - assert result.schemas_created def test_records_bad_uris_and_keeps_going(self, mocker): from openapi_python_client.parser.properties import Schemas, _create_schemas @@ -1024,7 +1058,6 @@ def test_records_bad_uris_and_keeps_going(self, mocker): schemas=Schemas(errors=[PropertyError(detail="some details", data=components["first"])]), ) assert result == update_schemas_with_data.return_value - assert result.schemas_created def test_retries_failing_properties_while_making_progress(self, mocker): from openapi_python_client.parser.properties import Schemas, _create_schemas @@ -1048,7 +1081,6 @@ def test_retries_failing_properties_while_making_progress(self, mocker): ) assert update_schemas_with_data.call_count == 3 assert result.errors == [PropertyError()] - assert result.schemas_created class TestProcessModels: @@ -1057,7 +1089,7 @@ def test_retries_failing_models_while_making_progress(self, mocker, model_proper first_model = model_property_factory() schemas = Schemas( - classes_by_reference={ + classes_by_name={ "first": first_model, "second": model_property_factory(), "non-model": property_factory(), @@ -1074,7 +1106,7 @@ def test_retries_failing_models_while_making_progress(self, mocker, model_proper process_model.assert_has_calls( [ call(first_model, schemas=schemas, config=config), - call(schemas.classes_by_reference["second"], schemas=schemas, config=config), + call(schemas.classes_by_name["second"], schemas=schemas, config=config), call(first_model, schemas=result, config=config), ] ) @@ -1088,7 +1120,7 @@ def test_detect_recursive_allof_reference_no_retry(self, mocker, model_property_ class_name = "class_name" recursive_model = model_property_factory(class_info=Class(name=class_name, module_name="module_name")) schemas = Schemas( - classes_by_reference={ + classes_by_name={ "recursive": recursive_model, "second": model_property_factory(), } @@ -1103,7 +1135,7 @@ def test_detect_recursive_allof_reference_no_retry(self, mocker, model_property_ process_model.assert_has_calls( [ call(recursive_model, schemas=schemas, config=config), - call(schemas.classes_by_reference["second"], schemas=schemas, config=config), + call(schemas.classes_by_name["second"], schemas=schemas, config=config), ] ) assert process_model_errors.was_called_once_with([(recursive_model, recursion_error)]) diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index 12a2eef82..24db65976 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -79,11 +79,12 @@ def test_additional_schemas(self, additional_properties_schema, expected_additio model, _ = build_model_property( data=data, name="prop", - schemas=Schemas(schemas_created=True), + schemas=Schemas(), required=True, parent_name="parent", config=Config(), roots={"root"}, + process_properties=True, ) assert model.additional_properties == expected_additional_properties @@ -105,14 +106,19 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_ description="A class called MyModel", nullable=nullable, ) - schemas = Schemas( - classes_by_reference={"OtherModel": None}, classes_by_name={"OtherModel": None}, schemas_created=True - ) + schemas = Schemas(classes_by_reference={"OtherModel": None}, classes_by_name={"OtherModel": None}) class_info = Class(name="ParentMyModel", module_name="parent_my_model") roots = {"root"} model, new_schemas = build_model_property( - data=data, name=name, schemas=schemas, required=required, parent_name="parent", config=Config(), roots=roots + data=data, + name=name, + schemas=schemas, + required=required, + parent_name="parent", + config=Config(), + roots=roots, + process_properties=True, ) assert new_schemas != schemas @@ -158,6 +164,7 @@ def test_model_name_conflict(self): parent_name=None, config=Config(), roots={"root"}, + process_properties=True, ) assert new_schemas == schemas @@ -174,11 +181,12 @@ def test_model_bad_properties(self): result = build_model_property( data=data, name="prop", - schemas=Schemas(schemas_created=True), + schemas=Schemas(), required=True, parent_name="parent", config=Config(), roots={"root"}, + process_properties=True, )[0] assert isinstance(result, PropertyError) @@ -195,15 +203,16 @@ def test_model_bad_additional_properties(self): result = build_model_property( data=data, name="prop", - schemas=Schemas(schemas_created=True), + schemas=Schemas(), required=True, parent_name="parent", config=Config(), roots={"root"}, + process_properties=True, )[0] assert isinstance(result, PropertyError) - def test_schemas_not_created(self, model_property_factory): + def test_process_properties_false(self, model_property_factory): from openapi_python_client.parser.properties import Class, Schemas, build_model_property name = "prop" @@ -225,7 +234,14 @@ def test_schemas_not_created(self, model_property_factory): class_info = Class(name="ParentMyModel", module_name="parent_my_model") model, new_schemas = build_model_property( - data=data, name=name, schemas=schemas, required=required, parent_name="parent", config=Config(), roots=roots + data=data, + name=name, + schemas=schemas, + required=required, + parent_name="parent", + config=Config(), + roots=roots, + process_properties=False, ) assert new_schemas != schemas @@ -293,9 +309,7 @@ def test_process_properties_model_property_roots(self, model_property_factory): roots = {"root"} data = oai.Schema(properties={"test_model_property": oai.Schema.construct(type="object")}) - result = _process_properties( - data=data, class_name="", schemas=Schemas(schemas_created=True), config=Config(), roots=roots - ) + result = _process_properties(data=data, class_name="", schemas=Schemas(), config=Config(), roots=roots) assert all(root in result.optional_props[0].roots for root in roots)