Skip to content

Commit e428569

Browse files
fix: Apply required overrides from allOf schemas (#1384)
### Summary Fixes an issue where `required` field overrides in `allOf` schemas were being ignored when the base schema was referenced. Properties inherited from a base schema can now be correctly marked as required in derived schemas. ### Problem When using `allOf` with a reference to a base schema and then marking fields as required in a second schema, the generator was ignoring the required override. For example: ```yaml components: schemas: FooBase: type: object properties: bar: type: string baz: type: string FooCreate: allOf: - $ref: '#/components/schemas/FooBase' - type: object required: - bar ``` ### New behavior A field will be `required` if it was marked `required` in the base schema **or** the derived schema. There is no way to remove a `required` restriction in a derived schema. This is because for a message to be valid under an `allOf`, it must be valid under all of the parent schemas.
1 parent dea19a1 commit e428569

File tree

7 files changed

+232
-0
lines changed

7 files changed

+232
-0
lines changed

end_to_end_tests/baseline_openapi_3.0.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1918,6 +1918,30 @@
19181918
}
19191919
]
19201920
},
1921+
"AllOfRequiredBase": {
1922+
"type": "object",
1923+
"properties": {
1924+
"bar": {
1925+
"type": "string",
1926+
"description": "The bar property"
1927+
},
1928+
"baz": {
1929+
"type": "string",
1930+
"description": "The baz property"
1931+
}
1932+
}
1933+
},
1934+
"AllOfRequiredDerived": {
1935+
"allOf": [
1936+
{
1937+
"$ref": "#/components/schemas/AllOfRequiredBase"
1938+
},
1939+
{
1940+
"type": "object",
1941+
"required": ["bar"]
1942+
}
1943+
]
1944+
},
19211945
"AModel": {
19221946
"title": "AModel",
19231947
"required": [

end_to_end_tests/baseline_openapi_3.1.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1871,6 +1871,27 @@ info:
18711871
}
18721872
]
18731873
},
1874+
"AllOfRequiredBase": {
1875+
"type": "object",
1876+
"properties": {
1877+
"bar": {
1878+
"type": "string",
1879+
"description": "The bar property"
1880+
},
1881+
"baz": {
1882+
"type": "string",
1883+
"description": "The baz property"
1884+
}
1885+
}
1886+
},
1887+
"AllOfRequiredDerived": {
1888+
"allOf": [
1889+
{ "$ref": "#/components/schemas/AllOfRequiredBase" },
1890+
{ "type": "object",
1891+
"required": ["bar"]
1892+
}
1893+
]
1894+
},
18741895
"AModel": {
18751896
"title": "AModel",
18761897
"required": [

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
from .a_model_with_properties_reference_that_are_not_object import AModelWithPropertiesReferenceThatAreNotObject
88
from .all_of_has_properties_but_no_type import AllOfHasPropertiesButNoType
99
from .all_of_has_properties_but_no_type_type_enum import AllOfHasPropertiesButNoTypeTypeEnum
10+
from .all_of_required_base import AllOfRequiredBase
11+
from .all_of_required_derived import AllOfRequiredDerived
1012
from .all_of_sub_model import AllOfSubModel
1113
from .all_of_sub_model_type_enum import AllOfSubModelTypeEnum
1214
from .an_all_of_enum import AnAllOfEnum
@@ -100,6 +102,8 @@
100102
"AFormData",
101103
"AllOfHasPropertiesButNoType",
102104
"AllOfHasPropertiesButNoTypeTypeEnum",
105+
"AllOfRequiredBase",
106+
"AllOfRequiredDerived",
103107
"AllOfSubModel",
104108
"AllOfSubModelTypeEnum",
105109
"AModel",
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
from __future__ import annotations
2+
3+
from collections.abc import Mapping
4+
from typing import Any, TypeVar
5+
6+
from attrs import define as _attrs_define
7+
from attrs import field as _attrs_field
8+
9+
from ..types import UNSET, Unset
10+
11+
T = TypeVar("T", bound="AllOfRequiredBase")
12+
13+
14+
@_attrs_define
15+
class AllOfRequiredBase:
16+
"""
17+
Attributes:
18+
bar (str | Unset): The bar property
19+
baz (str | Unset): The baz property
20+
"""
21+
22+
bar: str | Unset = UNSET
23+
baz: str | Unset = UNSET
24+
additional_properties: dict[str, Any] = _attrs_field(init=False, factory=dict)
25+
26+
def to_dict(self) -> dict[str, Any]:
27+
bar = self.bar
28+
29+
baz = self.baz
30+
31+
field_dict: dict[str, Any] = {}
32+
field_dict.update(self.additional_properties)
33+
field_dict.update({})
34+
if bar is not UNSET:
35+
field_dict["bar"] = bar
36+
if baz is not UNSET:
37+
field_dict["baz"] = baz
38+
39+
return field_dict
40+
41+
@classmethod
42+
def from_dict(cls: type[T], src_dict: Mapping[str, Any]) -> T:
43+
d = dict(src_dict)
44+
bar = d.pop("bar", UNSET)
45+
46+
baz = d.pop("baz", UNSET)
47+
48+
all_of_required_base = cls(
49+
bar=bar,
50+
baz=baz,
51+
)
52+
53+
all_of_required_base.additional_properties = d
54+
return all_of_required_base
55+
56+
@property
57+
def additional_keys(self) -> list[str]:
58+
return list(self.additional_properties.keys())
59+
60+
def __getitem__(self, key: str) -> Any:
61+
return self.additional_properties[key]
62+
63+
def __setitem__(self, key: str, value: Any) -> None:
64+
self.additional_properties[key] = value
65+
66+
def __delitem__(self, key: str) -> None:
67+
del self.additional_properties[key]
68+
69+
def __contains__(self, key: str) -> bool:
70+
return key in self.additional_properties
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
from __future__ import annotations
2+
3+
from collections.abc import Mapping
4+
from typing import Any, TypeVar
5+
6+
from attrs import define as _attrs_define
7+
from attrs import field as _attrs_field
8+
9+
from ..types import UNSET, Unset
10+
11+
T = TypeVar("T", bound="AllOfRequiredDerived")
12+
13+
14+
@_attrs_define
15+
class AllOfRequiredDerived:
16+
"""
17+
Attributes:
18+
bar (str): The bar property
19+
baz (str | Unset): The baz property
20+
"""
21+
22+
bar: str
23+
baz: str | Unset = UNSET
24+
additional_properties: dict[str, Any] = _attrs_field(init=False, factory=dict)
25+
26+
def to_dict(self) -> dict[str, Any]:
27+
bar = self.bar
28+
29+
baz = self.baz
30+
31+
field_dict: dict[str, Any] = {}
32+
field_dict.update(self.additional_properties)
33+
field_dict.update(
34+
{
35+
"bar": bar,
36+
}
37+
)
38+
if baz is not UNSET:
39+
field_dict["baz"] = baz
40+
41+
return field_dict
42+
43+
@classmethod
44+
def from_dict(cls: type[T], src_dict: Mapping[str, Any]) -> T:
45+
d = dict(src_dict)
46+
bar = d.pop("bar")
47+
48+
baz = d.pop("baz", UNSET)
49+
50+
all_of_required_derived = cls(
51+
bar=bar,
52+
baz=baz,
53+
)
54+
55+
all_of_required_derived.additional_properties = d
56+
return all_of_required_derived
57+
58+
@property
59+
def additional_keys(self) -> list[str]:
60+
return list(self.additional_properties.keys())
61+
62+
def __getitem__(self, key: str) -> Any:
63+
return self.additional_properties[key]
64+
65+
def __setitem__(self, key: str, value: Any) -> None:
66+
self.additional_properties[key] = value
67+
68+
def __delitem__(self, key: str) -> None:
69+
del self.additional_properties[key]
70+
71+
def __contains__(self, key: str) -> bool:
72+
return key in self.additional_properties

openapi_python_client/parser/properties/model_property.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,11 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
292292
unprocessed_props.extend(sub_prop.properties.items() if sub_prop.properties else [])
293293
required_set.update(sub_prop.required or [])
294294

295+
# Update properties that are marked as required in the schema
296+
for prop_name in required_set:
297+
if prop_name in properties and not properties[prop_name].required:
298+
properties[prop_name] = evolve(properties[prop_name], required=True)
299+
295300
for key, value in unprocessed_props:
296301
prop_required = key in required_set
297302
prop_or_error: Property | (PropertyError | None)

tests/test_parser/test_properties/test_model_property.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,42 @@ def test_duplicate_properties(self, model_property_factory, string_property_fact
543543

544544
assert result.optional_props == [prop], "There should only be one copy of duplicate properties"
545545

546+
def test_allof_required_override(self, model_property_factory, string_property_factory, config):
547+
"""Test that required field can be overridden in allOf schemas"""
548+
# Simulates:
549+
# FooBase:
550+
# type: object
551+
# properties:
552+
# bar: {type: string}
553+
# baz: {type: string}
554+
# FooCreate:
555+
# allOf:
556+
# - $ref: '#/components/schemas/FooBase'
557+
# - type: object
558+
# required: [bar]
559+
bar_prop = string_property_factory(name="bar", required=False)
560+
baz_prop = string_property_factory(name="baz", required=False)
561+
562+
data = oai.Schema.model_construct(
563+
allOf=[
564+
oai.Reference.model_construct(ref="#/FooBase"),
565+
oai.Schema.model_construct(type="object", required=["bar"]),
566+
]
567+
)
568+
schemas = Schemas(
569+
classes_by_reference={
570+
"/FooBase": model_property_factory(required_properties=[], optional_properties=[bar_prop, baz_prop]),
571+
}
572+
)
573+
574+
result = _process_properties(data=data, schemas=schemas, class_name="FooCreate", config=config, roots={"root"})
575+
576+
# bar should now be required, baz should remain optional
577+
assert len(result.required_props) == 1
578+
assert result.required_props[0].name == "bar"
579+
assert len(result.optional_props) == 1
580+
assert result.optional_props[0].name == "baz"
581+
546582
@pytest.mark.parametrize("first_required", [True, False])
547583
@pytest.mark.parametrize("second_required", [True, False])
548584
def test_mixed_requirements(

0 commit comments

Comments
 (0)