fix: Problems with enum defaults in allOf (#363). Thanks @csymeonides-mf

* test: allOf should work without "type": "object"

* test: allOf should work with enums, and get default from outer schema

* fix: Get enum default from parent schema

Co-authored-by: Constantinos Symeonides <constantinos.symeonides@mindfoundry.ai>

Author: csymeonides-mf

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

@@ -2,6 +2,7 @@
 
 from .a_model import AModel
 from .all_of_sub_model import AllOfSubModel
+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

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

@@ -4,6 +4,7 @@
 import attr
 from dateutil.parser import isoparse
 
+from ..models.an_all_of_enum import AnAllOfEnum
 from ..models.an_enum import AnEnum
 from ..models.different_enum import DifferentEnum
 from ..models.free_form_model import FreeFormModel
@@ -27,6 +28,8 @@ class AModel:
     required_nullable: Optional[str]
     nullable_one_of_models: Union[FreeFormModel, ModelWithUnionProperty, None]
     nullable_model: Optional[ModelWithUnionProperty]
+    an_allof_enum_with_overridden_default: AnAllOfEnum = AnAllOfEnum.OVERRIDDEN_DEFAULT
+    an_optional_allof_enum: Union[Unset, AnAllOfEnum] = UNSET
     nested_list_of_enums: Union[Unset, List[List[DifferentEnum]]] = UNSET
     a_not_required_date: Union[Unset, datetime.date] = UNSET
     attr_1_leading_digit: Union[Unset, str] = UNSET
@@ -40,6 +43,8 @@ class AModel:
     def to_dict(self) -> Dict[str, Any]:
         an_enum_value = self.an_enum_value.value
 
+        an_allof_enum_with_overridden_default = self.an_allof_enum_with_overridden_default.value
+
         if isinstance(self.a_camel_date_time, datetime.datetime):
             a_camel_date_time = self.a_camel_date_time.isoformat()
 
@@ -56,6 +61,10 @@ def to_dict(self) -> Dict[str, Any]:
 
         model = self.model.to_dict()
 
+        an_optional_allof_enum: Union[Unset, str] = UNSET
+        if not isinstance(self.an_optional_allof_enum, Unset):
+            an_optional_allof_enum = self.an_optional_allof_enum.value
+
         nested_list_of_enums: Union[Unset, List[List[str]]] = UNSET
         if not isinstance(self.nested_list_of_enums, Unset):
             nested_list_of_enums = []
@@ -133,6 +142,7 @@ def to_dict(self) -> Dict[str, Any]:
         field_dict.update(
             {
                 "an_enum_value": an_enum_value,
+                "an_allof_enum_with_overridden_default": an_allof_enum_with_overridden_default,
                 "aCamelDateTime": a_camel_date_time,
                 "a_date": a_date,
                 "required_not_nullable": required_not_nullable,
@@ -144,6 +154,8 @@ def to_dict(self) -> Dict[str, Any]:
                 "nullable_model": nullable_model,
             }
         )
+        if an_optional_allof_enum is not UNSET:
+            field_dict["an_optional_allof_enum"] = an_optional_allof_enum
         if nested_list_of_enums is not UNSET:
             field_dict["nested_list_of_enums"] = nested_list_of_enums
         if a_not_required_date is not UNSET:
@@ -170,6 +182,8 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         d = src_dict.copy()
         an_enum_value = AnEnum(d.pop("an_enum_value"))
 
+        an_allof_enum_with_overridden_default = AnAllOfEnum(d.pop("an_allof_enum_with_overridden_default"))
+
         def _parse_a_camel_date_time(data: object) -> Union[datetime.date, datetime.datetime]:
             try:
                 a_camel_date_time_type0: datetime.datetime
@@ -214,6 +228,11 @@ def _parse_one_of_models(data: object) -> Union[FreeFormModel, ModelWithUnionPro
 
         model = ModelWithUnionProperty.from_dict(d.pop("model"))
 
+        an_optional_allof_enum: Union[Unset, AnAllOfEnum] = UNSET
+        _an_optional_allof_enum = d.pop("an_optional_allof_enum", UNSET)
+        if not isinstance(_an_optional_allof_enum, Unset):
+            an_optional_allof_enum = AnAllOfEnum(_an_optional_allof_enum)
+
         nested_list_of_enums = []
         _nested_list_of_enums = d.pop("nested_list_of_enums", UNSET)
         for nested_list_of_enums_item_data in _nested_list_of_enums or []:
@@ -350,11 +369,13 @@ def _parse_not_required_nullable_one_of_models(
 
         a_model = cls(
             an_enum_value=an_enum_value,
+            an_allof_enum_with_overridden_default=an_allof_enum_with_overridden_default,
             a_camel_date_time=a_camel_date_time,
             a_date=a_date,
             required_not_nullable=required_not_nullable,
             one_of_models=one_of_models,
             model=model,
+            an_optional_allof_enum=an_optional_allof_enum,
             nested_list_of_enums=nested_list_of_enums,
             a_nullable_date=a_nullable_date,
             a_not_required_date=a_not_required_date,

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

@@ -0,0 +1,11 @@
+from enum import Enum
+
+
+class AnAllOfEnum(str, Enum):
+    FOO = "foo"
+    BAR = "bar"
+    A_DEFAULT = "a_default"
+    OVERRIDDEN_DEFAULT = "overridden_default"
+
+    def __str__(self) -> str:
+        return str(self.value)

end_to_end_tests/openapi.json

@@ -747,6 +747,7 @@
         "title": "AModel",
         "required": [
           "an_enum_value",
+          "an_allof_enum_with_overridden_default",
           "aCamelDateTime",
           "a_date",
           "a_nullable_date",
@@ -762,6 +763,21 @@
           "an_enum_value": {
             "$ref": "#/components/schemas/AnEnum"
           },
+          "an_allof_enum_with_overridden_default": {
+            "allOf": [
+              {
+                "$ref": "#/components/schemas/AnAllOfEnum"
+              }
+            ],
+            "default": "overridden_default"
+          },
+          "an_optional_allof_enum": {
+            "allOf": [
+              {
+                "$ref": "#/components/schemas/AnAllOfEnum"
+              }
+            ]
+          },
           "nested_list_of_enums": {
             "title": "Nested List Of Enums",
             "type": "array",
@@ -874,7 +890,6 @@
             "nullable": true
           },
           "model": {
-            "type": "object",
             "allOf": [
               {
                 "ref": "#/components/schemas/ModelWithUnionProperty"
@@ -883,7 +898,6 @@
             "nullable": false
           },
           "nullable_model": {
-            "type": "object",
             "allOf": [
               {
                 "ref": "#/components/schemas/ModelWithUnionProperty"
@@ -892,7 +906,6 @@
             "nullable": true
           },
           "not_required_model": {
-            "type": "object",
             "allOf": [
               {
                 "ref": "#/components/schemas/ModelWithUnionProperty"
@@ -901,7 +914,6 @@
             "nullable": false
           },
           "not_required_nullable_model": {
-            "type": "object",
             "allOf": [
               {
                 "ref": "#/components/schemas/ModelWithUnionProperty"
@@ -918,6 +930,11 @@
         "enum": ["FIRST_VALUE", "SECOND_VALUE"],
         "description": "For testing Enums in all the ways they can be used "
       },
+      "AnAllOfEnum": {
+        "title": "AnAllOfEnum",
+        "enum": ["foo", "bar", "a_default", "overridden_default"],
+        "default": "a_default"
+      },
       "AnIntEnum": {
         "title": "AnIntEnum",
         "enum": [-1, 1, 2],

end_to_end_tests/regen_golden_record.py

@@ -1,6 +1,5 @@
 """ Regenerate golden-record """
 import shutil
-import sys
 from pathlib import Path
 
 from typer.testing import CliRunner

openapi_python_client/parser/properties/__init__.py

@@ -318,30 +318,36 @@ def build_enum_property(
     else:
         return PropertyError(data=data, detail="No values provided for Enum"), schemas
 
-    default = None
-    if data.default is not None:
-        inverse_values = {v: k for k, v in values.items()}
-        try:
-            default = f"{class_info.name}.{inverse_values[data.default]}"
-        except KeyError:
-            return (
-                PropertyError(detail=f"{data.default} is an invalid default for enum {class_info.name}", data=data),
-                schemas,
-            )
-
     prop = EnumProperty(
         name=name,
         required=required,
-        default=default,
         nullable=data.nullable,
         class_info=class_info,
         values=values,
         value_type=value_type,
+        default=None,
     )
+
+    default = get_enum_default(prop, data)
+    if isinstance(default, PropertyError):
+        return default, schemas
+    prop = attr.evolve(prop, default=default)
+
     schemas = attr.evolve(schemas, classes_by_name={**schemas.classes_by_name, class_info.name: prop})
     return prop, schemas
 
 
+def get_enum_default(prop: EnumProperty, data: oai.Schema) -> Union[Optional[Any], PropertyError]:
+    if data.default is None:
+        return None
+
+    inverse_values = {v: k for k, v in prop.values.items()}
+    try:
+        return f"{prop.class_info.name}.{inverse_values[data.default]}"
+    except KeyError:
+        return PropertyError(detail=f"{data.default} is an invalid default for enum {prop.class_info.name}", data=data)
+
+
 def build_union_property(
     *, data: oai.Schema, name: str, required: bool, schemas: Schemas, parent_name: str, config: Config
 ) -> Tuple[Union[UnionProperty, PropertyError], Schemas]:
@@ -397,20 +403,27 @@ def build_list_property(
 def _property_from_ref(
     name: str,
     required: bool,
-    nullable: bool,
+    parent: Union[oai.Schema, None],
     data: oai.Reference,
     schemas: Schemas,
 ) -> Tuple[Union[Property, PropertyError], Schemas]:
     ref_path = parse_reference_path(data.ref)
     if isinstance(ref_path, ParseError):
         return PropertyError(data=data, detail=ref_path.detail), schemas
     existing = schemas.classes_by_reference.get(ref_path)
-    if existing:
-        return (
-            attr.evolve(existing, required=required, name=name, nullable=nullable),
-            schemas,
-        )
-    return PropertyError(data=data, detail="Could not find reference in parsed models or enums"), schemas
+    if not existing:
+        return PropertyError(data=data, detail="Could not find reference in parsed models or enums"), schemas
+
+    prop = attr.evolve(existing, required=required, name=name)
+    if parent:
+        prop = attr.evolve(prop, nullable=parent.nullable)
+        if isinstance(prop, EnumProperty):
+            default = get_enum_default(prop, parent)
+            if isinstance(default, PropertyError):
+                return default, schemas
+            prop = attr.evolve(prop, default=default)
+
+    return prop, schemas
 
 
 def _property_from_data(
@@ -424,14 +437,12 @@ def _property_from_data(
     """ 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, nullable=False, data=data, schemas=schemas)
+        return _property_from_ref(name=name, required=required, parent=None, data=data, schemas=schemas)
 
     # A union of a single reference should just be passed through to that reference (don't create copy class)
     sub_data = (data.allOf or []) + data.anyOf + data.oneOf
     if len(sub_data) == 1 and isinstance(sub_data[0], oai.Reference):
-        return _property_from_ref(
-            name=name, required=required, nullable=data.nullable, data=sub_data[0], schemas=schemas
-        )
+        return _property_from_ref(name=name, required=required, parent=data, data=sub_data[0], schemas=schemas)
 
     if data.enum:
         return build_enum_property(
@@ -443,11 +454,11 @@ def _property_from_data(
             parent_name=parent_name,
             config=config,
         )
-    if data.anyOf or data.oneOf:
+    elif data.anyOf or data.oneOf:
         return build_union_property(
             data=data, name=name, required=required, schemas=schemas, parent_name=parent_name, config=config
         )
-    if data.type == "string":
+    elif data.type == "string":
         return _string_based_property(name=name, required=required, data=data), schemas
     elif data.type == "number":
         return (

tests/test_parser/test_properties/test_init.py

@@ -555,6 +555,60 @@ def test_property_from_data_ref_enum(self):
         )
         assert schemas == new_schemas
 
+    def test_property_from_data_ref_enum_with_overridden_default(self):
+        from openapi_python_client.parser.properties import Class, EnumProperty, Schemas, property_from_data
+
+        name = "some_enum"
+        data = oai.Schema.construct(default="b", allOf=[oai.Reference.construct(ref="#/components/schemas/MyEnum")])
+        existing_enum = EnumProperty(
+            name="an_enum",
+            required=True,
+            nullable=False,
+            default="MyEnum.A",
+            values={"A": "a", "B": "b"},
+            value_type=str,
+            class_info=Class(name="MyEnum", module_name="my_enum"),
+        )
+        schemas = Schemas(classes_by_reference={"/components/schemas/MyEnum": existing_enum})
+
+        prop, new_schemas = property_from_data(
+            name=name, required=False, data=data, schemas=schemas, parent_name="", config=Config()
+        )
+
+        assert prop == EnumProperty(
+            name="some_enum",
+            required=False,
+            nullable=False,
+            default="MyEnum.B",
+            values={"A": "a", "B": "b"},
+            value_type=str,
+            class_info=Class(name="MyEnum", module_name="my_enum"),
+        )
+        assert schemas == new_schemas
+
+    def test_property_from_data_ref_enum_with_invalid_default(self):
+        from openapi_python_client.parser.properties import Class, EnumProperty, Schemas, property_from_data
+
+        name = "some_enum"
+        data = oai.Schema.construct(default="x", allOf=[oai.Reference.construct(ref="#/components/schemas/MyEnum")])
+        existing_enum = EnumProperty(
+            name="an_enum",
+            required=True,
+            nullable=False,
+            default="MyEnum.A",
+            values={"A": "a", "B": "b"},
+            value_type=str,
+            class_info=Class(name="MyEnum", module_name="my_enum"),
+        )
+        schemas = Schemas(classes_by_reference={"/components/schemas/MyEnum": existing_enum})
+
+        prop, new_schemas = property_from_data(
+            name=name, required=False, data=data, schemas=schemas, parent_name="", config=Config()
+        )
+
+        assert schemas == new_schemas
+        assert prop == PropertyError(data=data, detail="x is an invalid default for enum MyEnum")
+
     def test_property_from_data_ref_model(self):
         from openapi_python_client.parser.properties import Class, ModelProperty, Schemas, property_from_data