Skip to content

feat: Allow allOf enums to be subsets of one another or their base types [#379, #423, #461]. Thanks @forest-benchling! #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 1, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
from .a_model import AModel
from .a_model_with_properties_reference_that_are_not_object import AModelWithPropertiesReferenceThatAreNotObject
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_enum import AnEnum
from .an_int_enum import AnIntEnum
from .another_all_of_sub_model import AnotherAllOfSubModel
from .another_all_of_sub_model_type import AnotherAllOfSubModelType
from .another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
from .body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
from .body_upload_file_tests_upload_post_additional_property import BodyUploadFileTestsUploadPostAdditionalProperty
from .body_upload_file_tests_upload_post_some_nullable_object import BodyUploadFileTestsUploadPostSomeNullableObject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import attr

from ..models.all_of_sub_model_type_enum import AllOfSubModelTypeEnum
from ..types import UNSET, Unset

T = TypeVar("T", bound="AllOfSubModel")
Expand All @@ -12,16 +13,26 @@ class AllOfSubModel:
""" """

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

def to_dict(self) -> Dict[str, Any]:
a_sub_property = self.a_sub_property
type = self.type
type_enum: Union[Unset, int] = UNSET
if not isinstance(self.type_enum, Unset):
type_enum = self.type_enum.value

field_dict: Dict[str, Any] = {}
field_dict.update(self.additional_properties)
field_dict.update({})
if a_sub_property is not UNSET:
field_dict["a_sub_property"] = a_sub_property
if type is not UNSET:
field_dict["type"] = type
if type_enum is not UNSET:
field_dict["type_enum"] = type_enum

return field_dict

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

type = d.pop("type", UNSET)

_type_enum = d.pop("type_enum", UNSET)
type_enum: Union[Unset, AllOfSubModelTypeEnum]
if isinstance(_type_enum, Unset):
type_enum = UNSET
else:
type_enum = AllOfSubModelTypeEnum(_type_enum)

all_of_sub_model = cls(
a_sub_property=a_sub_property,
type=type,
type_enum=type_enum,
)

all_of_sub_model.additional_properties = d
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from enum import IntEnum


class AllOfSubModelTypeEnum(IntEnum):
VALUE_0 = 0
VALUE_1 = 1

def __str__(self) -> str:
return str(self.value)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import attr

from ..models.another_all_of_sub_model_type import AnotherAllOfSubModelType
from ..models.another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
from ..types import UNSET, Unset

T = TypeVar("T", bound="AnotherAllOfSubModel")
Expand All @@ -12,16 +14,29 @@ class AnotherAllOfSubModel:
""" """

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

def to_dict(self) -> Dict[str, Any]:
another_sub_property = self.another_sub_property
type: Union[Unset, str] = UNSET
if not isinstance(self.type, Unset):
type = self.type.value

type_enum: Union[Unset, int] = UNSET
if not isinstance(self.type_enum, Unset):
type_enum = self.type_enum.value

field_dict: Dict[str, Any] = {}
field_dict.update(self.additional_properties)
field_dict.update({})
if another_sub_property is not UNSET:
field_dict["another_sub_property"] = another_sub_property
if type is not UNSET:
field_dict["type"] = type
if type_enum is not UNSET:
field_dict["type_enum"] = type_enum

return field_dict

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

_type = d.pop("type", UNSET)
type: Union[Unset, AnotherAllOfSubModelType]
if isinstance(_type, Unset):
type = UNSET
else:
type = AnotherAllOfSubModelType(_type)

_type_enum = d.pop("type_enum", UNSET)
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum]
if isinstance(_type_enum, Unset):
type_enum = UNSET
else:
type_enum = AnotherAllOfSubModelTypeEnum(_type_enum)

another_all_of_sub_model = cls(
another_sub_property=another_sub_property,
type=type,
type_enum=type_enum,
)

another_all_of_sub_model.additional_properties = d
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from enum import Enum


class AnotherAllOfSubModelType(str, Enum):
SUBMODEL = "submodel"

def __str__(self) -> str:
return str(self.value)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from enum import IntEnum


class AnotherAllOfSubModelTypeEnum(IntEnum):
VALUE_0 = 0

def __str__(self) -> str:
return str(self.value)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import attr

from ..models.another_all_of_sub_model_type import AnotherAllOfSubModelType
from ..models.another_all_of_sub_model_type_enum import AnotherAllOfSubModelTypeEnum
from ..types import UNSET, Unset

T = TypeVar("T", bound="ModelFromAllOf")
Expand All @@ -12,18 +14,32 @@ class ModelFromAllOf:
""" """

a_sub_property: Union[Unset, str] = UNSET
type: Union[Unset, AnotherAllOfSubModelType] = UNSET
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum] = UNSET
another_sub_property: Union[Unset, str] = UNSET
additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)

def to_dict(self) -> Dict[str, Any]:
a_sub_property = self.a_sub_property
type: Union[Unset, str] = UNSET
if not isinstance(self.type, Unset):
type = self.type.value

type_enum: Union[Unset, int] = UNSET
if not isinstance(self.type_enum, Unset):
type_enum = self.type_enum.value

another_sub_property = self.another_sub_property

field_dict: Dict[str, Any] = {}
field_dict.update(self.additional_properties)
field_dict.update({})
if a_sub_property is not UNSET:
field_dict["a_sub_property"] = a_sub_property
if type is not UNSET:
field_dict["type"] = type
if type_enum is not UNSET:
field_dict["type_enum"] = type_enum
if another_sub_property is not UNSET:
field_dict["another_sub_property"] = another_sub_property

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

_type = d.pop("type", UNSET)
type: Union[Unset, AnotherAllOfSubModelType]
if isinstance(_type, Unset):
type = UNSET
else:
type = AnotherAllOfSubModelType(_type)

_type_enum = d.pop("type_enum", UNSET)
type_enum: Union[Unset, AnotherAllOfSubModelTypeEnum]
if isinstance(_type_enum, Unset):
type_enum = UNSET
else:
type_enum = AnotherAllOfSubModelTypeEnum(_type_enum)

another_sub_property = d.pop("another_sub_property", UNSET)

model_from_all_of = cls(
a_sub_property=a_sub_property,
type=type,
type_enum=type_enum,
another_sub_property=another_sub_property,
)

Expand Down
15 changes: 15 additions & 0 deletions end_to_end_tests/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,13 @@
"properties": {
"a_sub_property": {
"type": "string"
},
"type": {
"type": "string"
},
"type_enum": {
"type": "int",
"enum": [0, 1]
}
}
},
Expand All @@ -1391,6 +1398,14 @@
"properties": {
"another_sub_property": {
"type": "string"
},
"type": {
"type": "string",
"enum": ["submodel"]
},
"type_enum": {
"type": "int",
"enum": [0]
}
}
},
Expand Down
85 changes: 64 additions & 21 deletions openapi_python_client/parser/properties/model_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from ... import schema as oai
from ... import utils
from ..errors import ParseError, PropertyError
from .enum_property import EnumProperty
from .property import Property
from .schemas import Class, Schemas, parse_reference_path

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


def _values_are_subset(first: EnumProperty, second: EnumProperty) -> bool:
return set(first.values.items()) <= set(second.values.items())


def _types_are_subset(first: EnumProperty, second: Property) -> bool:
from . import IntProperty, StringProperty

if first.value_type == int and isinstance(second, IntProperty):
return True
if first.value_type == str and isinstance(second, StringProperty):
return True
return False


def _enum_subset(first: Property, second: Property) -> Optional[EnumProperty]:
"""Return the EnumProperty that is the subset of the other, if possible."""

if isinstance(first, EnumProperty):
if isinstance(second, EnumProperty):
if _values_are_subset(first, second):
return first
if _values_are_subset(second, first):
return second
return None
return first if _types_are_subset(first, second) else None
if isinstance(second, EnumProperty) and _types_are_subset(second, first):
return second
return None


def _merge_properties(first: Property, second: Property) -> Union[Property, PropertyError]:
if first.__class__ != second.__class__:
return PropertyError(header="Cannot merge properties", detail="Properties are two different types")
nullable = first.nullable and second.nullable
required = first.required or second.required
first = attr.evolve(first, nullable=nullable, required=required)
second = attr.evolve(second, nullable=nullable, required=required)
if first != second:
return PropertyError(header="Cannot merge properties", detail="Properties has conflicting values")
return first

err = None

if first.__class__ == second.__class__:
first = attr.evolve(first, nullable=nullable, required=required)
second = attr.evolve(second, nullable=nullable, required=required)
if first == second:
return first
err = PropertyError(header="Cannot merge properties", detail="Properties has conflicting values")

enum_subset = _enum_subset(first, second)
if enum_subset is not None:
return attr.evolve(enum_subset, nullable=nullable, required=required)

return err or PropertyError(
header="Cannot merge properties",
detail=f"{first.__class__}, {second.__class__}Properties have incompatible types",
)


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

def _check_existing(prop: Property) -> Union[Property, PropertyError]:
def _add_if_no_conflict(new_prop: Property) -> Optional[PropertyError]:
nonlocal properties

existing = properties.get(prop.name)
prop_or_error = _merge_properties(existing, prop) if existing else prop
if isinstance(prop_or_error, PropertyError):
prop_or_error.header = f"Found conflicting properties named {prop.name} when creating {class_name}"
return prop_or_error
properties[prop_or_error.name] = prop_or_error
return prop_or_error
existing = properties.get(new_prop.name)
merged_prop_or_error = _merge_properties(existing, new_prop) if existing else new_prop
if isinstance(merged_prop_or_error, PropertyError):
merged_prop_or_error.header = (
f"Found conflicting properties named {new_prop.name} when creating {class_name}"
)
return merged_prop_or_error
properties[merged_prop_or_error.name] = merged_prop_or_error
return None

unprocessed_props = data.properties or {}
for sub_prop in data.allOf or []:
Expand All @@ -100,25 +144,24 @@ def _check_existing(prop: Property) -> Union[Property, PropertyError]:
if not isinstance(sub_model, ModelProperty):
return PropertyError("Cannot take allOf a non-object")
for prop in chain(sub_model.required_properties, sub_model.optional_properties):
prop_or_error = _check_existing(prop)
if isinstance(prop_or_error, PropertyError):
return prop_or_error
err = _add_if_no_conflict(prop)
if err is not None:
return err
else:
unprocessed_props.update(sub_prop.properties or {})
required_set.update(sub_prop.required or [])

for key, value in unprocessed_props.items():
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
)
if isinstance(prop_or_error, Property):
prop_or_error = _check_existing(prop_or_error)
prop_or_error = _add_if_no_conflict(prop_or_error)
if isinstance(prop_or_error, PropertyError):
return prop_or_error

properties[prop_or_error.name] = prop_or_error

required_properties = []
optional_properties = []
for prop in properties.values():
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ check = """
isort .\
&& black .\
&& flake8 openapi_python_client\
&& safety check --bare\
&& poetry export -f requirements.txt | poetry run safety check --bare --stdin\
&& mypy openapi_python_client\
&& pytest --cov openapi_python_client tests --cov-report=term-missing\
"""
Expand Down
Loading