chiliu commited on
Commit
85b13ea
·
unverified ·
1 Parent(s): 8d16d26

Fix OpenAPI allOf handling at requestBody top level (#1378) (#1425)

Browse files
src/fastmcp/experimental/utilities/openapi/schemas.py CHANGED
@@ -237,6 +237,32 @@ def _combine_schemas_and_map_params(
237
  route.request_body.content_schema[content_type].copy(),
238
  route.request_body.description,
239
  )
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
240
  body_props = body_schema.get("properties", {})
241
 
242
  # Detect collisions: parameters that exist in both body and path/query/header
 
237
  route.request_body.content_schema[content_type].copy(),
238
  route.request_body.description,
239
  )
240
+
241
+ # Handle allOf at the top level by merging all schemas
242
+ if "allOf" in body_schema and isinstance(body_schema["allOf"], list):
243
+ merged_props = {}
244
+ merged_required = []
245
+
246
+ for sub_schema in body_schema["allOf"]:
247
+ if isinstance(sub_schema, dict):
248
+ # Merge properties
249
+ if "properties" in sub_schema:
250
+ merged_props.update(sub_schema["properties"])
251
+ # Merge required fields
252
+ if "required" in sub_schema:
253
+ merged_required.extend(sub_schema["required"])
254
+
255
+ # Update body_schema with merged properties
256
+ body_schema["properties"] = merged_props
257
+ if merged_required:
258
+ # Remove duplicates while preserving order
259
+ seen = set()
260
+ body_schema["required"] = [
261
+ x for x in merged_required if not (x in seen or seen.add(x))
262
+ ]
263
+ # Remove the allOf since we've merged it
264
+ body_schema.pop("allOf", None)
265
+
266
  body_props = body_schema.get("properties", {})
267
 
268
  # Detect collisions: parameters that exist in both body and path/query/header
tests/experimental/utilities/openapi/test_allof_requestbody.py ADDED
@@ -0,0 +1,227 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """Tests for allOf handling at requestBody top level."""
2
+
3
+ from fastmcp.experimental.utilities.openapi.models import (
4
+ HTTPRoute,
5
+ RequestBodyInfo,
6
+ )
7
+ from fastmcp.experimental.utilities.openapi.schemas import _combine_schemas
8
+
9
+
10
+ def test_allof_at_requestbody_top_level():
11
+ """Test that allOf schemas at requestBody top level are properly merged."""
12
+
13
+ # Create a route with allOf at the requestBody top level
14
+ route = HTTPRoute(
15
+ path="/test",
16
+ method="POST",
17
+ operation_id="testOperation",
18
+ parameters=[],
19
+ request_body=RequestBodyInfo(
20
+ required=True,
21
+ content_schema={
22
+ "application/json": {
23
+ "allOf": [
24
+ {
25
+ "type": "object",
26
+ "properties": {
27
+ "name": {"type": "string"},
28
+ "age": {"type": "integer"},
29
+ },
30
+ "required": ["name"],
31
+ },
32
+ {
33
+ "type": "object",
34
+ "properties": {
35
+ "email": {"type": "string"},
36
+ "phone": {"type": "string"},
37
+ },
38
+ "required": ["email"],
39
+ },
40
+ ]
41
+ }
42
+ },
43
+ ),
44
+ responses={},
45
+ )
46
+
47
+ # Combine schemas - this should merge allOf schemas
48
+ combined = _combine_schemas(route)
49
+
50
+ # Check that all properties from both allOf schemas are present
51
+ properties = combined.get("properties", {})
52
+ assert "name" in properties
53
+ assert "age" in properties
54
+ assert "email" in properties
55
+ assert "phone" in properties
56
+
57
+ # Check property types
58
+ assert properties["name"]["type"] == "string"
59
+ assert properties["age"]["type"] == "integer"
60
+ assert properties["email"]["type"] == "string"
61
+ assert properties["phone"]["type"] == "string"
62
+
63
+ # Check that required fields are merged correctly
64
+ required = set(combined.get("required", []))
65
+ assert "name" in required
66
+ assert "email" in required
67
+
68
+ # allOf should be removed after merging
69
+ assert "allOf" not in combined
70
+
71
+
72
+ def test_allof_with_nested_properties():
73
+ """Test allOf with nested object properties."""
74
+
75
+ route = HTTPRoute(
76
+ path="/test",
77
+ method="POST",
78
+ operation_id="testNested",
79
+ parameters=[],
80
+ request_body=RequestBodyInfo(
81
+ required=True,
82
+ content_schema={
83
+ "application/json": {
84
+ "allOf": [
85
+ {
86
+ "type": "object",
87
+ "properties": {
88
+ "user": {
89
+ "type": "object",
90
+ "properties": {
91
+ "id": {"type": "integer"},
92
+ "name": {"type": "string"},
93
+ },
94
+ }
95
+ },
96
+ "required": ["user"],
97
+ },
98
+ {
99
+ "type": "object",
100
+ "properties": {
101
+ "metadata": {
102
+ "type": "object",
103
+ "properties": {
104
+ "created": {"type": "string"},
105
+ "updated": {"type": "string"},
106
+ },
107
+ }
108
+ },
109
+ },
110
+ ]
111
+ }
112
+ },
113
+ ),
114
+ responses={},
115
+ )
116
+
117
+ combined = _combine_schemas(route)
118
+
119
+ # Check nested properties are preserved
120
+ properties = combined.get("properties", {})
121
+ assert "user" in properties
122
+ assert "metadata" in properties
123
+
124
+ # Check nested structure
125
+ assert properties["user"]["type"] == "object"
126
+ assert "id" in properties["user"]["properties"]
127
+ assert "name" in properties["user"]["properties"]
128
+
129
+ assert properties["metadata"]["type"] == "object"
130
+ assert "created" in properties["metadata"]["properties"]
131
+ assert "updated" in properties["metadata"]["properties"]
132
+
133
+ # Check required
134
+ required = set(combined.get("required", []))
135
+ assert "user" in required
136
+ assert "metadata" not in required # Not in any required array
137
+
138
+
139
+ def test_allof_with_overlapping_properties():
140
+ """Test allOf with overlapping property names (later schemas override)."""
141
+
142
+ route = HTTPRoute(
143
+ path="/test",
144
+ method="POST",
145
+ operation_id="testOverlap",
146
+ parameters=[],
147
+ request_body=RequestBodyInfo(
148
+ required=True,
149
+ content_schema={
150
+ "application/json": {
151
+ "allOf": [
152
+ {
153
+ "type": "object",
154
+ "properties": {
155
+ "name": {"type": "string", "minLength": 1},
156
+ "age": {"type": "integer"},
157
+ },
158
+ "required": ["name"],
159
+ },
160
+ {
161
+ "type": "object",
162
+ "properties": {
163
+ "name": {"type": "string", "maxLength": 50}, # Override
164
+ "email": {"type": "string"},
165
+ },
166
+ "required": ["email"],
167
+ },
168
+ ]
169
+ }
170
+ },
171
+ ),
172
+ responses={},
173
+ )
174
+
175
+ combined = _combine_schemas(route)
176
+
177
+ properties = combined.get("properties", {})
178
+
179
+ # Later schema should win for overlapping properties
180
+ assert "name" in properties
181
+ assert properties["name"]["type"] == "string"
182
+ assert "maxLength" in properties["name"] # From second schema
183
+ assert properties["name"]["maxLength"] == 50
184
+
185
+ # Check other properties
186
+ assert "age" in properties
187
+ assert "email" in properties
188
+
189
+ # Both name and email should be required
190
+ required = set(combined.get("required", []))
191
+ assert "name" in required
192
+ assert "email" in required
193
+
194
+
195
+ def test_no_allof_passthrough():
196
+ """Test that schemas without allOf pass through unchanged."""
197
+
198
+ route = HTTPRoute(
199
+ path="/test",
200
+ method="POST",
201
+ operation_id="testNoAllOf",
202
+ parameters=[],
203
+ request_body=RequestBodyInfo(
204
+ required=True,
205
+ content_schema={
206
+ "application/json": {
207
+ "type": "object",
208
+ "properties": {"simple": {"type": "string"}},
209
+ "required": ["simple"],
210
+ }
211
+ },
212
+ ),
213
+ responses={},
214
+ )
215
+
216
+ combined = _combine_schemas(route)
217
+
218
+ # Should pass through unchanged
219
+ properties = combined.get("properties", {})
220
+ assert "simple" in properties
221
+ assert properties["simple"]["type"] == "string"
222
+
223
+ required = set(combined.get("required", []))
224
+ assert "simple" in required
225
+
226
+ # No allOf in original or result
227
+ assert "allOf" not in combined