allow merge of str with date/file/etc., add end-to-end tests

eli-bl · eli-bl · commit db90641b4f16 · 2024-08-26T12:40:25.000-07:00
diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json
@@ -2152,6 +2152,56 @@
           "additionalProperties": {}
         }
       },
+      "ModelWithMergedProperties": {
+        "title": "ModelWithMergedProperties",
+        "allOf": [
+          {
+            "type": "object",
+            "properties": {
+              "simpleString": {
+                "type": "string",
+                "description": "base simpleString description"
+              },
+              "stringToEnum": {
+                "type": "string",
+                "default": "a"
+              },
+              "stringToDate": {
+                "type": "string"
+              },
+              "numberToInt": {
+                "type": "number"
+              },
+              "anyToString": {}
+            }
+          },
+          {
+            "type": "object",
+            "properties": {
+              "simpleString": {
+                "type": "string",
+                "description": "extended simpleString description",
+                "default": "new default"
+              },
+              "stringToEnum": {
+                "type": "string",
+                "enum": ["a", "b"]
+              },
+              "stringToDate": {
+                "type": "string",
+                "format": "date"
+              },
+              "numberToInt": {
+                "type": "integer"
+              },
+              "anyToString": {
+                "type": "string",
+                "default": "x"
+              }
+            }
+          }
+        ]
+      },
       "ModelWithPrimitiveAdditionalProperties": {
         "title": "ModelWithPrimitiveAdditionalProperties",
         "type": "object",
diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml
@@ -2146,6 +2146,56 @@ info:
         "additionalProperties": { }
       }
     },
+    "ModelWithMergedProperties": {
+      "title": "ModelWithMergedProperties",
+      "allOf": [
+        {
+          "type": "object",
+          "properties": {
+            "simpleString": {
+              "type": "string",
+              "description": "base simpleString description"
+            },
+            "stringToEnum": {
+              "type": "string",
+              "default": "a"
+            },
+            "stringToDate": {
+              "type": "string"
+            },
+            "numberToInt": {
+              "type": "number"
+            },
+            "anyToString": {}
+          }
+        },
+        {
+          "type": "object",
+          "properties": {
+            "simpleString": {
+              "type": "string",
+              "description": "extended simpleString description",
+              "default": "new default"
+            },
+            "stringToEnum": {
+              "type": "string",
+              "enum": ["a", "b"]
+            },
+            "stringToDate": {
+              "type": "string",
+              "format": "date"
+            },
+            "numberToInt": {
+              "type": "integer"
+            },
+            "anyToString": {
+              "type": "string",
+              "default": "x"
+            }
+          }
+        }
+      ]
+    },
     "ModelWithPrimitiveAdditionalProperties": {
       "title": "ModelWithPrimitiveAdditionalProperties",
       "type": "object",
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
@@ -60,6 +60,8 @@
 from .model_with_circular_ref_in_additional_properties_b import ModelWithCircularRefInAdditionalPropertiesB
 from .model_with_date_time_property import ModelWithDateTimeProperty
 from .model_with_discriminated_union import ModelWithDiscriminatedUnion
+from .model_with_merged_properties import ModelWithMergedProperties
+from .model_with_merged_properties_string_to_enum import ModelWithMergedPropertiesStringToEnum
 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
@@ -137,6 +139,8 @@
     "ModelWithCircularRefInAdditionalPropertiesB",
     "ModelWithDateTimeProperty",
     "ModelWithDiscriminatedUnion",
+    "ModelWithMergedProperties",
+    "ModelWithMergedPropertiesStringToEnum",
     "ModelWithPrimitiveAdditionalProperties",
     "ModelWithPrimitiveAdditionalPropertiesADateHolder",
     "ModelWithPropertyRef",
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
@@ -0,0 +1,111 @@
+import datetime
+from typing import Any, Dict, List, Type, TypeVar, Union
+
+from attrs import define as _attrs_define
+from attrs import field as _attrs_field
+from dateutil.parser import isoparse
+
+from ..models.model_with_merged_properties_string_to_enum import ModelWithMergedPropertiesStringToEnum
+from ..types import UNSET, Unset
+
+T = TypeVar("T", bound="ModelWithMergedProperties")
+
+
+@_attrs_define
+class ModelWithMergedProperties:
+    """
+    Attributes:
+        simple_string (Union[Unset, str]): extended simpleString description Default: 'new default'.
+        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):
+        string_to_date (Union[Unset, datetime.date]):
+        number_to_int (Union[Unset, int]):
+        any_to_string (Union[Unset, str]):  Default: 'x'.
+    """
+
+    simple_string: Union[Unset, str] = "new default"
+    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = UNSET
+    string_to_date: Union[Unset, datetime.date] = UNSET
+    number_to_int: Union[Unset, int] = UNSET
+    any_to_string: Union[Unset, str] = "x"
+    additional_properties: Dict[str, Any] = _attrs_field(init=False, factory=dict)
+
+    def to_dict(self) -> Dict[str, Any]:
+        simple_string = self.simple_string
+
+        string_to_enum: Union[Unset, str] = UNSET
+        if not isinstance(self.string_to_enum, Unset):
+            string_to_enum = self.string_to_enum.value
+
+        string_to_date: Union[Unset, str] = UNSET
+        if not isinstance(self.string_to_date, Unset):
+            string_to_date = self.string_to_date.isoformat()
+
+        number_to_int = self.number_to_int
+
+        any_to_string = self.any_to_string
+
+        field_dict: Dict[str, Any] = {}
+        field_dict.update(self.additional_properties)
+        field_dict.update({})
+        if simple_string is not UNSET:
+            field_dict["simpleString"] = simple_string
+        if string_to_enum is not UNSET:
+            field_dict["stringToEnum"] = string_to_enum
+        if string_to_date is not UNSET:
+            field_dict["stringToDate"] = string_to_date
+        if number_to_int is not UNSET:
+            field_dict["numberToInt"] = number_to_int
+        if any_to_string is not UNSET:
+            field_dict["anyToString"] = any_to_string
+
+        return field_dict
+
+    @classmethod
+    def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
+        d = src_dict.copy()
+        simple_string = d.pop("simpleString", UNSET)
+
+        _string_to_enum = d.pop("stringToEnum", UNSET)
+        string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum]
+        if isinstance(_string_to_enum, Unset):
+            string_to_enum = UNSET
+        else:
+            string_to_enum = ModelWithMergedPropertiesStringToEnum(_string_to_enum)
+
+        _string_to_date = d.pop("stringToDate", UNSET)
+        string_to_date: Union[Unset, datetime.date]
+        if isinstance(_string_to_date, Unset):
+            string_to_date = UNSET
+        else:
+            string_to_date = isoparse(_string_to_date).date()
+
+        number_to_int = d.pop("numberToInt", UNSET)
+
+        any_to_string = d.pop("anyToString", UNSET)
+
+        model_with_merged_properties = cls(
+            simple_string=simple_string,
+            string_to_enum=string_to_enum,
+            string_to_date=string_to_date,
+            number_to_int=number_to_int,
+            any_to_string=any_to_string,
+        )
+
+        model_with_merged_properties.additional_properties = d
+        return model_with_merged_properties
+
+    @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_merged_properties_string_to_enum.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties_string_to_enum.py
@@ -0,0 +1,9 @@
+from enum import Enum
+
+
+class ModelWithMergedPropertiesStringToEnum(str, Enum):
+    A = "a"
+    B = "b"
+
+    def __str__(self) -> str:
+        return str(self.value)
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,5 +1,9 @@
 from __future__ import annotations
 
+from openapi_python_client.parser.properties.date import DateProperty
+from openapi_python_client.parser.properties.datetime import DateTimeProperty
+from openapi_python_client.parser.properties.file import FileProperty
+
 __all__ = ["merge_properties"]
 
 from typing import TypeVar, cast
@@ -19,7 +23,10 @@
 PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
 
 
-def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyError:
+STRING_WITH_FORMAT_TYPES = (DateProperty, DateTimeProperty, FileProperty)
+
+
+def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyError:  # noqa: PLR0911
     """Attempt to create a new property that incorporates the behavior of both.
 
     This is used when merging schemas with allOf, when two schemas define a property with the same name.
@@ -52,6 +59,9 @@ def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyErr
     if (merged := _merge_numeric(prop1, prop2)) is not None:
         return merged
 
+    if (merged := _merge_string_with_format(prop1, prop2)) is not None:
+        return merged
+
     return PropertyError(
         detail=f"{prop1.get_type_string(no_optional=True)} can't be merged with {prop2.get_type_string(no_optional=True)}"
     )
@@ -65,9 +75,6 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
         # It's always OK to redefine a property with everything exactly the same
         return prop1
 
-    if isinstance(prop1, StringProperty) and isinstance(prop2, StringProperty):
-        return _merge_string(prop1, prop2)
-
     if isinstance(prop1, ListProperty) and isinstance(prop2, ListProperty):
         # There's no clear way to represent the intersection of two different list types. Fail in this case.
         inner_property = merge_properties(prop1.inner_property, prop2.inner_property)  # type: ignore
@@ -80,8 +87,17 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
     return _merge_common_attributes(prop1, prop2)
 
 
-def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty | PropertyError:
-    return _merge_common_attributes(prop1, prop2)
+def _merge_string_with_format(prop1: Property, prop2: Property) -> Property | None | PropertyError:
+    """Merge a string that has no format with a string that has a format"""
+    # Here we need to use the DateProperty/DateTimeProperty/FileProperty as the base so that we preserve
+    # its class, but keep the correct override order for merging the attributes.
+    if isinstance(prop1, StringProperty) and isinstance(prop2, STRING_WITH_FORMAT_TYPES):
+        # Use the more specific class as a base, but keep the correct override order
+        return _merge_common_attributes(prop2, prop1, prop2)
+    elif isinstance(prop2, StringProperty) and isinstance(prop1, STRING_WITH_FORMAT_TYPES):
+        return _merge_common_attributes(prop1, prop2)
+    else:
+        return None
 
 
 def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None | PropertyError:
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
@@ -193,3 +193,29 @@ def test_merge_with_incompatible_enum(
         if not isinstance(prop, IntProperty):
             assert isinstance(merge_properties(prop, int_enum_prop), PropertyError)
             assert isinstance(merge_properties(int_enum_prop, prop), PropertyError)
+
+
+def test_merge_string_with_formatted_string(
+    date_property_factory,
+    date_time_property_factory,
+    file_property_factory,
+    string_property_factory,
+):
+    string_prop = string_property_factory(description="a plain string")
+    string_prop_with_invalid_default = string_property_factory(default="plain string value")
+    formatted_props = [
+        date_property_factory(description="a date"),
+        date_time_property_factory(description="a datetime"),
+        file_property_factory(description="a file"),
+    ]
+    for formatted_prop in formatted_props:
+        merged1 = merge_properties(string_prop, formatted_prop)
+        assert isinstance(merged1, formatted_prop.__class__)
+        assert merged1.description == formatted_prop.description
+
+        merged2 = merge_properties(formatted_prop, string_prop)
+        assert isinstance(merged2, formatted_prop.__class__)
+        assert merged2.description == string_prop.description
+
+        assert isinstance(merge_properties(string_prop_with_invalid_default, formatted_prop), PropertyError)
+        assert isinstance(merge_properties(formatted_prop, string_prop_with_invalid_default), PropertyError)
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
@@ -325,7 +325,7 @@ def test_process_properties_false(self, model_property_factory, config):
 
 class TestProcessProperties:
     def test_conflicting_properties_different_types(
-        self, model_property_factory, string_property_factory, date_time_property_factory, config
+        self, model_property_factory, string_property_factory, int_property_factory, config
     ):
         data = oai.Schema.model_construct(
             allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")]
@@ -335,9 +335,7 @@ def test_conflicting_properties_different_types(
                 "/First": model_property_factory(
                     required_properties=[], optional_properties=[string_property_factory()]
                 ),
-                "/Second": model_property_factory(
-                    required_properties=[], optional_properties=[date_time_property_factory()]
-                ),
+                "/Second": model_property_factory(required_properties=[], optional_properties=[int_property_factory()]),
             }
         )
 
