Skip to content

Commit 0ca21aa

Browse files
feat: Allow allOf enums to be subsets of one another or their base types [#379, #423, #461]. Thanks @forest-benchling! (#461)
Co-authored-by: Forest Tong <[email protected]>
1 parent b857848 commit 0ca21aa

File tree

12 files changed

+380
-24
lines changed

12 files changed

+380
-24
lines changed

Diff for: end_to_end_tests/golden-record/my_test_api_client/models/__init__.py

+3
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
from .a_model import AModel
55
from .a_model_with_properties_reference_that_are_not_object import AModelWithPropertiesReferenceThatAreNotObject
66
from .all_of_sub_model import AllOfSubModel
7+
from .all_of_sub_model_type_enum import AllOfSubModelTypeEnum
78
from .an_all_of_enum import AnAllOfEnum
89
from .an_enum import AnEnum
910
from .an_int_enum import AnIntEnum
1011
from .another_all_of_sub_model import AnotherAllOfSubModel
12+
from .another_all_of_sub_model_type import AnotherAllOfSubModelType
13+
from .another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
1114
from .body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
1215
from .body_upload_file_tests_upload_post_additional_property import BodyUploadFileTestsUploadPostAdditionalProperty
1316
from .body_upload_file_tests_upload_post_some_nullable_object import BodyUploadFileTestsUploadPostSomeNullableObject

Diff for: end_to_end_tests/golden-record/my_test_api_client/models/all_of_sub_model.py

+22
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import attr
44

5+
from ..models.all_of_sub_model_type_enum import AllOfSubModelTypeEnum
56
from ..types import UNSET, Unset
67

78
T = TypeVar("T", bound="AllOfSubModel")
@@ -12,16 +13,26 @@ class AllOfSubModel:
1213
""" """
1314

1415
a_sub_property: Union[Unset, str] = UNSET
16+
type: Union[Unset, str] = UNSET
17+
type_enum: Union[Unset, AllOfSubModelTypeEnum] = UNSET
1518
additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
1619

1720
def to_dict(self) -> Dict[str, Any]:
1821
a_sub_property = self.a_sub_property
22+
type = self.type
23+
type_enum: Union[Unset, int] = UNSET
24+
if not isinstance(self.type_enum, Unset):
25+
type_enum = self.type_enum.value
1926

2027
field_dict: Dict[str, Any] = {}
2128
field_dict.update(self.additional_properties)
2229
field_dict.update({})
2330
if a_sub_property is not UNSET:
2431
field_dict["a_sub_property"] = a_sub_property
32+
if type is not UNSET:
33+
field_dict["type"] = type
34+
if type_enum is not UNSET:
35+
field_dict["type_enum"] = type_enum
2536

2637
return field_dict
2738

@@ -30,8 +41,19 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
3041
d = src_dict.copy()
3142
a_sub_property = d.pop("a_sub_property", UNSET)
3243

44+
type = d.pop("type", UNSET)
45+
46+
_type_enum = d.pop("type_enum", UNSET)
47+
type_enum: Union[Unset, AllOfSubModelTypeEnum]
48+
if isinstance(_type_enum, Unset):
49+
type_enum = UNSET
50+
else:
51+
type_enum = AllOfSubModelTypeEnum(_type_enum)
52+
3353
all_of_sub_model = cls(
3454
a_sub_property=a_sub_property,
55+
type=type,
56+
type_enum=type_enum,
3557
)
3658

3759
all_of_sub_model.additional_properties = d
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
from enum import IntEnum
2+
3+
4+
class AllOfSubModelTypeEnum(IntEnum):
5+
VALUE_0 = 0
6+
VALUE_1 = 1
7+
8+
def __str__(self) -> str:
9+
return str(self.value)

Diff for: end_to_end_tests/golden-record/my_test_api_client/models/another_all_of_sub_model.py

+31
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import attr
44

5+
from ..models.another_all_of_sub_model_type import AnotherAllOfSubModelType
6+
from ..models.another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
57
from ..types import UNSET, Unset
68

79
T = TypeVar("T", bound="AnotherAllOfSubModel")
@@ -12,16 +14,29 @@ class AnotherAllOfSubModel:
1214
""" """
1315

1416
another_sub_property: Union[Unset, str] = UNSET
17+
type: Union[Unset, AnotherAllOfSubModelType] = UNSET
18+
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum] = UNSET
1519
additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
1620

1721
def to_dict(self) -> Dict[str, Any]:
1822
another_sub_property = self.another_sub_property
23+
type: Union[Unset, str] = UNSET
24+
if not isinstance(self.type, Unset):
25+
type = self.type.value
26+
27+
type_enum: Union[Unset, int] = UNSET
28+
if not isinstance(self.type_enum, Unset):
29+
type_enum = self.type_enum.value
1930

2031
field_dict: Dict[str, Any] = {}
2132
field_dict.update(self.additional_properties)
2233
field_dict.update({})
2334
if another_sub_property is not UNSET:
2435
field_dict["another_sub_property"] = another_sub_property
36+
if type is not UNSET:
37+
field_dict["type"] = type
38+
if type_enum is not UNSET:
39+
field_dict["type_enum"] = type_enum
2540

2641
return field_dict
2742

@@ -30,8 +45,24 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
3045
d = src_dict.copy()
3146
another_sub_property = d.pop("another_sub_property", UNSET)
3247

48+
_type = d.pop("type", UNSET)
49+
type: Union[Unset, AnotherAllOfSubModelType]
50+
if isinstance(_type, Unset):
51+
type = UNSET
52+
else:
53+
type = AnotherAllOfSubModelType(_type)
54+
55+
_type_enum = d.pop("type_enum", UNSET)
56+
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum]
57+
if isinstance(_type_enum, Unset):
58+
type_enum = UNSET
59+
else:
60+
type_enum = AnotherAllOfSubModelTypeEnum(_type_enum)
61+
3362
another_all_of_sub_model = cls(
3463
another_sub_property=another_sub_property,
64+
type=type,
65+
type_enum=type_enum,
3566
)
3667

3768
another_all_of_sub_model.additional_properties = d
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
from enum import Enum
2+
3+
4+
class AnotherAllOfSubModelType(str, Enum):
5+
SUBMODEL = "submodel"
6+
7+
def __str__(self) -> str:
8+
return str(self.value)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
from enum import IntEnum
2+
3+
4+
class AnotherAllOfSubModelTypeEnum(IntEnum):
5+
VALUE_0 = 0
6+
7+
def __str__(self) -> str:
8+
return str(self.value)

Diff for: end_to_end_tests/golden-record/my_test_api_client/models/model_from_all_of.py

+32
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import attr
44

5+
from ..models.another_all_of_sub_model_type import AnotherAllOfSubModelType
6+
from ..models.another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
57
from ..types import UNSET, Unset
68

79
T = TypeVar("T", bound="ModelFromAllOf")
@@ -12,18 +14,32 @@ class ModelFromAllOf:
1214
""" """
1315

1416
a_sub_property: Union[Unset, str] = UNSET
17+
type: Union[Unset, AnotherAllOfSubModelType] = UNSET
18+
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum] = UNSET
1519
another_sub_property: Union[Unset, str] = UNSET
1620
additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
1721

1822
def to_dict(self) -> Dict[str, Any]:
1923
a_sub_property = self.a_sub_property
24+
type: Union[Unset, str] = UNSET
25+
if not isinstance(self.type, Unset):
26+
type = self.type.value
27+
28+
type_enum: Union[Unset, int] = UNSET
29+
if not isinstance(self.type_enum, Unset):
30+
type_enum = self.type_enum.value
31+
2032
another_sub_property = self.another_sub_property
2133

2234
field_dict: Dict[str, Any] = {}
2335
field_dict.update(self.additional_properties)
2436
field_dict.update({})
2537
if a_sub_property is not UNSET:
2638
field_dict["a_sub_property"] = a_sub_property
39+
if type is not UNSET:
40+
field_dict["type"] = type
41+
if type_enum is not UNSET:
42+
field_dict["type_enum"] = type_enum
2743
if another_sub_property is not UNSET:
2844
field_dict["another_sub_property"] = another_sub_property
2945

@@ -34,10 +50,26 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
3450
d = src_dict.copy()
3551
a_sub_property = d.pop("a_sub_property", UNSET)
3652

53+
_type = d.pop("type", UNSET)
54+
type: Union[Unset, AnotherAllOfSubModelType]
55+
if isinstance(_type, Unset):
56+
type = UNSET
57+
else:
58+
type = AnotherAllOfSubModelType(_type)
59+
60+
_type_enum = d.pop("type_enum", UNSET)
61+
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum]
62+
if isinstance(_type_enum, Unset):
63+
type_enum = UNSET
64+
else:
65+
type_enum = AnotherAllOfSubModelTypeEnum(_type_enum)
66+
3767
another_sub_property = d.pop("another_sub_property", UNSET)
3868

3969
model_from_all_of = cls(
4070
a_sub_property=a_sub_property,
71+
type=type,
72+
type_enum=type_enum,
4173
another_sub_property=another_sub_property,
4274
)
4375

Diff for: end_to_end_tests/openapi.json

+15
Original file line numberDiff line numberDiff line change
@@ -1382,6 +1382,13 @@
13821382
"properties": {
13831383
"a_sub_property": {
13841384
"type": "string"
1385+
},
1386+
"type": {
1387+
"type": "string"
1388+
},
1389+
"type_enum": {
1390+
"type": "int",
1391+
"enum": [0, 1]
13851392
}
13861393
}
13871394
},
@@ -1391,6 +1398,14 @@
13911398
"properties": {
13921399
"another_sub_property": {
13931400
"type": "string"
1401+
},
1402+
"type": {
1403+
"type": "string",
1404+
"enum": ["submodel"]
1405+
},
1406+
"type_enum": {
1407+
"type": "int",
1408+
"enum": [0]
13941409
}
13951410
}
13961411
},

Diff for: openapi_python_client/parser/properties/model_property.py

+64-21
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from ... import schema as oai
88
from ... import utils
99
from ..errors import ParseError, PropertyError
10+
from .enum_property import EnumProperty
1011
from .property import Property
1112
from .schemas import Class, Schemas, parse_reference_path
1213

@@ -49,16 +50,57 @@ def get_imports(self, *, prefix: str) -> Set[str]:
4950
return imports
5051

5152

53+
def _values_are_subset(first: EnumProperty, second: EnumProperty) -> bool:
54+
return set(first.values.items()) <= set(second.values.items())
55+
56+
57+
def _types_are_subset(first: EnumProperty, second: Property) -> bool:
58+
from . import IntProperty, StringProperty
59+
60+
if first.value_type == int and isinstance(second, IntProperty):
61+
return True
62+
if first.value_type == str and isinstance(second, StringProperty):
63+
return True
64+
return False
65+
66+
67+
def _enum_subset(first: Property, second: Property) -> Optional[EnumProperty]:
68+
"""Return the EnumProperty that is the subset of the other, if possible."""
69+
70+
if isinstance(first, EnumProperty):
71+
if isinstance(second, EnumProperty):
72+
if _values_are_subset(first, second):
73+
return first
74+
if _values_are_subset(second, first):
75+
return second
76+
return None
77+
return first if _types_are_subset(first, second) else None
78+
if isinstance(second, EnumProperty) and _types_are_subset(second, first):
79+
return second
80+
return None
81+
82+
5283
def _merge_properties(first: Property, second: Property) -> Union[Property, PropertyError]:
53-
if first.__class__ != second.__class__:
54-
return PropertyError(header="Cannot merge properties", detail="Properties are two different types")
5584
nullable = first.nullable and second.nullable
5685
required = first.required or second.required
57-
first = attr.evolve(first, nullable=nullable, required=required)
58-
second = attr.evolve(second, nullable=nullable, required=required)
59-
if first != second:
60-
return PropertyError(header="Cannot merge properties", detail="Properties has conflicting values")
61-
return first
86+
87+
err = None
88+
89+
if first.__class__ == second.__class__:
90+
first = attr.evolve(first, nullable=nullable, required=required)
91+
second = attr.evolve(second, nullable=nullable, required=required)
92+
if first == second:
93+
return first
94+
err = PropertyError(header="Cannot merge properties", detail="Properties has conflicting values")
95+
96+
enum_subset = _enum_subset(first, second)
97+
if enum_subset is not None:
98+
return attr.evolve(enum_subset, nullable=nullable, required=required)
99+
100+
return err or PropertyError(
101+
header="Cannot merge properties",
102+
detail=f"{first.__class__}, {second.__class__}Properties have incompatible types",
103+
)
62104

63105

64106
class _PropertyData(NamedTuple):
@@ -77,16 +119,18 @@ def _process_properties(
77119
relative_imports: Set[str] = set()
78120
required_set = set(data.required or [])
79121

80-
def _check_existing(prop: Property) -> Union[Property, PropertyError]:
122+
def _add_if_no_conflict(new_prop: Property) -> Optional[PropertyError]:
81123
nonlocal properties
82124

83-
existing = properties.get(prop.name)
84-
prop_or_error = _merge_properties(existing, prop) if existing else prop
85-
if isinstance(prop_or_error, PropertyError):
86-
prop_or_error.header = f"Found conflicting properties named {prop.name} when creating {class_name}"
87-
return prop_or_error
88-
properties[prop_or_error.name] = prop_or_error
89-
return prop_or_error
125+
existing = properties.get(new_prop.name)
126+
merged_prop_or_error = _merge_properties(existing, new_prop) if existing else new_prop
127+
if isinstance(merged_prop_or_error, PropertyError):
128+
merged_prop_or_error.header = (
129+
f"Found conflicting properties named {new_prop.name} when creating {class_name}"
130+
)
131+
return merged_prop_or_error
132+
properties[merged_prop_or_error.name] = merged_prop_or_error
133+
return None
90134

91135
unprocessed_props = data.properties or {}
92136
for sub_prop in data.allOf or []:
@@ -100,25 +144,24 @@ def _check_existing(prop: Property) -> Union[Property, PropertyError]:
100144
if not isinstance(sub_model, ModelProperty):
101145
return PropertyError("Cannot take allOf a non-object")
102146
for prop in chain(sub_model.required_properties, sub_model.optional_properties):
103-
prop_or_error = _check_existing(prop)
104-
if isinstance(prop_or_error, PropertyError):
105-
return prop_or_error
147+
err = _add_if_no_conflict(prop)
148+
if err is not None:
149+
return err
106150
else:
107151
unprocessed_props.update(sub_prop.properties or {})
108152
required_set.update(sub_prop.required or [])
109153

110154
for key, value in unprocessed_props.items():
111155
prop_required = key in required_set
156+
prop_or_error: Union[Property, PropertyError, None]
112157
prop_or_error, schemas = property_from_data(
113158
name=key, required=prop_required, data=value, schemas=schemas, parent_name=class_name, config=config
114159
)
115160
if isinstance(prop_or_error, Property):
116-
prop_or_error = _check_existing(prop_or_error)
161+
prop_or_error = _add_if_no_conflict(prop_or_error)
117162
if isinstance(prop_or_error, PropertyError):
118163
return prop_or_error
119164

120-
properties[prop_or_error.name] = prop_or_error
121-
122165
required_properties = []
123166
optional_properties = []
124167
for prop in properties.values():

Diff for: pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ check = """
5757
isort .\
5858
&& black .\
5959
&& flake8 openapi_python_client\
60-
&& safety check --bare\
60+
&& poetry export -f requirements.txt | poetry run safety check --bare --stdin\
6161
&& mypy openapi_python_client\
6262
&& pytest --cov openapi_python_client tests --cov-report=term-missing\
6363
"""

0 commit comments

Comments
 (0)