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 <forest@benchling.com>

Author: dbanty

end_to_end_tests/golden-record/my_test_api_client/models/__init__.py

@@ -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

end_to_end_tests/golden-record/my_test_api_client/models/all_of_sub_model.py

@@ -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")
@@ -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
 
@@ -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

end_to_end_tests/golden-record/my_test_api_client/models/all_of_sub_model_type_enum.py

@@ -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)

end_to_end_tests/golden-record/my_test_api_client/models/another_all_of_sub_model.py

@@ -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")
@@ -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
 
@@ -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

end_to_end_tests/golden-record/my_test_api_client/models/another_all_of_sub_model_type.py

@@ -0,0 +1,8 @@
+from enum import Enum
+
+
+class AnotherAllOfSubModelType(str, Enum):
+    SUBMODEL = "submodel"
+
+    def __str__(self) -> str:
+        return str(self.value)

end_to_end_tests/golden-record/my_test_api_client/models/another_all_of_sub_model_type_enum.py

@@ -0,0 +1,8 @@
+from enum import IntEnum
+
+
+class AnotherAllOfSubModelTypeEnum(IntEnum):
+    VALUE_0 = 0
+
+    def __str__(self) -> str:
+        return str(self.value)

end_to_end_tests/golden-record/my_test_api_client/models/model_from_all_of.py

@@ -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")
@@ -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
 
@@ -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,
         )
 

end_to_end_tests/openapi.json

@@ -1382,6 +1382,13 @@
         "properties": {
           "a_sub_property": {
             "type": "string"
+          },
+          "type": {
+            "type": "string"
+          },
+          "type_enum": {
+            "type": "int",
+            "enum": [0, 1]
           }
         }
       },
@@ -1391,6 +1398,14 @@
         "properties": {
           "another_sub_property": {
             "type": "string"
+          },
+          "type": {
+            "type": "string",
+            "enum": ["submodel"]
+          },
+          "type_enum": {
+            "type": "int",
+            "enum": [0]
           }
         }
       },

openapi_python_client/parser/properties/model_property.py

@@ -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
 
@@ -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):
@@ -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 []:
@@ -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():

pyproject.toml

@@ -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\
 """