Spaces:
Running
Running
Jeremiah Lowin Claude commited on
Fix optional parameter validation in OpenAPI integration (#1135)
Browse files
src/fastmcp/utilities/openapi.py
CHANGED
|
@@ -1095,6 +1095,35 @@ def _replace_ref_with_defs(
|
|
| 1095 |
return schema
|
| 1096 |
|
| 1097 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1098 |
def _combine_schemas(route: HTTPRoute) -> dict[str, Any]:
|
| 1099 |
"""
|
| 1100 |
Combines parameter and request body schemas into a single schema.
|
|
@@ -1156,15 +1185,25 @@ def _combine_schemas(route: HTTPRoute) -> dict[str, Any]:
|
|
| 1156 |
else:
|
| 1157 |
param_schema["description"] = location_desc
|
| 1158 |
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1159 |
properties[suffixed_name] = param_schema
|
| 1160 |
else:
|
| 1161 |
# No collision, use original name
|
| 1162 |
if param.required:
|
| 1163 |
required.append(param.name)
|
| 1164 |
-
|
| 1165 |
param.schema_.copy(), param.description
|
| 1166 |
)
|
| 1167 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1168 |
# Add request body properties (no suffixes for body parameters)
|
| 1169 |
if route.request_body and route.request_body.content_schema:
|
| 1170 |
for prop_name, prop_schema in body_props.items():
|
|
|
|
| 1095 |
return schema
|
| 1096 |
|
| 1097 |
|
| 1098 |
+
def _make_optional_parameter_nullable(schema: dict[str, Any]) -> dict[str, Any]:
|
| 1099 |
+
"""
|
| 1100 |
+
Make an optional parameter schema nullable to allow None values.
|
| 1101 |
+
|
| 1102 |
+
For optional parameters, we need to allow null values in addition to the
|
| 1103 |
+
specified type to handle cases where None is passed for optional parameters.
|
| 1104 |
+
"""
|
| 1105 |
+
# If schema already has multiple types or is already nullable, don't modify
|
| 1106 |
+
if "anyOf" in schema or "oneOf" in schema or "allOf" in schema:
|
| 1107 |
+
return schema
|
| 1108 |
+
|
| 1109 |
+
# If it's already nullable (type includes null), don't modify
|
| 1110 |
+
if isinstance(schema.get("type"), list) and "null" in schema["type"]:
|
| 1111 |
+
return schema
|
| 1112 |
+
|
| 1113 |
+
# Create a new schema that allows null in addition to the original type
|
| 1114 |
+
if "type" in schema:
|
| 1115 |
+
original_type = schema["type"]
|
| 1116 |
+
if isinstance(original_type, str):
|
| 1117 |
+
# Single type - make it a union with null
|
| 1118 |
+
nullable_schema = schema.copy()
|
| 1119 |
+
nullable_schema["anyOf"] = [{"type": original_type}, {"type": "null"}]
|
| 1120 |
+
# Remove the original type since we're using anyOf
|
| 1121 |
+
del nullable_schema["type"]
|
| 1122 |
+
return nullable_schema
|
| 1123 |
+
|
| 1124 |
+
return schema
|
| 1125 |
+
|
| 1126 |
+
|
| 1127 |
def _combine_schemas(route: HTTPRoute) -> dict[str, Any]:
|
| 1128 |
"""
|
| 1129 |
Combines parameter and request body schemas into a single schema.
|
|
|
|
| 1185 |
else:
|
| 1186 |
param_schema["description"] = location_desc
|
| 1187 |
|
| 1188 |
+
# Make optional parameters nullable to allow None values
|
| 1189 |
+
if not param.required:
|
| 1190 |
+
param_schema = _make_optional_parameter_nullable(param_schema)
|
| 1191 |
+
|
| 1192 |
properties[suffixed_name] = param_schema
|
| 1193 |
else:
|
| 1194 |
# No collision, use original name
|
| 1195 |
if param.required:
|
| 1196 |
required.append(param.name)
|
| 1197 |
+
param_schema = _replace_ref_with_defs(
|
| 1198 |
param.schema_.copy(), param.description
|
| 1199 |
)
|
| 1200 |
|
| 1201 |
+
# Make optional parameters nullable to allow None values
|
| 1202 |
+
if not param.required:
|
| 1203 |
+
param_schema = _make_optional_parameter_nullable(param_schema)
|
| 1204 |
+
|
| 1205 |
+
properties[param.name] = param_schema
|
| 1206 |
+
|
| 1207 |
# Add request body properties (no suffixes for body parameters)
|
| 1208 |
if route.request_body and route.request_body.content_schema:
|
| 1209 |
for prop_name, prop_schema in body_props.items():
|
tests/server/openapi/test_optional_parameters.py
ADDED
|
@@ -0,0 +1,102 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Test for optional parameter handling in FastMCP OpenAPI integration."""
|
| 2 |
+
|
| 3 |
+
import pytest
|
| 4 |
+
|
| 5 |
+
from fastmcp.utilities.openapi import HTTPRoute, ParameterInfo, _combine_schemas
|
| 6 |
+
|
| 7 |
+
|
| 8 |
+
async def test_optional_parameter_schema_allows_null():
|
| 9 |
+
"""Test that optional parameters generate schemas that allow null values."""
|
| 10 |
+
# Create a minimal HTTPRoute with optional parameter
|
| 11 |
+
optional_param = ParameterInfo(
|
| 12 |
+
name="optional_param",
|
| 13 |
+
location="query",
|
| 14 |
+
required=False,
|
| 15 |
+
schema={"type": "string"},
|
| 16 |
+
description="Optional parameter",
|
| 17 |
+
)
|
| 18 |
+
|
| 19 |
+
required_param = ParameterInfo(
|
| 20 |
+
name="required_param",
|
| 21 |
+
location="query",
|
| 22 |
+
required=True,
|
| 23 |
+
schema={"type": "string"},
|
| 24 |
+
description="Required parameter",
|
| 25 |
+
)
|
| 26 |
+
|
| 27 |
+
route = HTTPRoute(
|
| 28 |
+
method="GET",
|
| 29 |
+
path="/test",
|
| 30 |
+
parameters=[required_param, optional_param],
|
| 31 |
+
request_body=None,
|
| 32 |
+
responses={},
|
| 33 |
+
summary="Test endpoint",
|
| 34 |
+
description=None,
|
| 35 |
+
schema_definitions={},
|
| 36 |
+
)
|
| 37 |
+
|
| 38 |
+
# Generate combined schema
|
| 39 |
+
schema = _combine_schemas(route)
|
| 40 |
+
|
| 41 |
+
# Verify that optional parameter allows null values
|
| 42 |
+
optional_param_schema = schema["properties"]["optional_param"]
|
| 43 |
+
|
| 44 |
+
# Should have anyOf with string and null types
|
| 45 |
+
assert "anyOf" in optional_param_schema
|
| 46 |
+
assert {"type": "string"} in optional_param_schema["anyOf"]
|
| 47 |
+
assert {"type": "null"} in optional_param_schema["anyOf"]
|
| 48 |
+
|
| 49 |
+
# Required parameter should not allow null
|
| 50 |
+
required_param_schema = schema["properties"]["required_param"]
|
| 51 |
+
assert required_param_schema["type"] == "string"
|
| 52 |
+
assert "anyOf" not in required_param_schema
|
| 53 |
+
|
| 54 |
+
# Required list should only contain required param
|
| 55 |
+
assert "required_param" in schema["required"]
|
| 56 |
+
assert "optional_param" not in schema["required"]
|
| 57 |
+
|
| 58 |
+
|
| 59 |
+
@pytest.mark.parametrize(
|
| 60 |
+
"param_schema",
|
| 61 |
+
[
|
| 62 |
+
{"type": "string"},
|
| 63 |
+
{"type": "integer"},
|
| 64 |
+
{"type": "number"},
|
| 65 |
+
{"type": "boolean"},
|
| 66 |
+
{"type": "array", "items": {"type": "string"}},
|
| 67 |
+
{"type": "object", "properties": {"name": {"type": "string"}}},
|
| 68 |
+
],
|
| 69 |
+
)
|
| 70 |
+
async def test_optional_parameter_allows_null_for_type(param_schema):
|
| 71 |
+
"""Test that optional parameters of any type allow null values."""
|
| 72 |
+
optional_param = ParameterInfo(
|
| 73 |
+
name="optional_param",
|
| 74 |
+
location="query",
|
| 75 |
+
required=False,
|
| 76 |
+
schema=param_schema,
|
| 77 |
+
description="Optional parameter",
|
| 78 |
+
)
|
| 79 |
+
|
| 80 |
+
route = HTTPRoute(
|
| 81 |
+
method="GET",
|
| 82 |
+
path="/test",
|
| 83 |
+
parameters=[optional_param],
|
| 84 |
+
request_body=None,
|
| 85 |
+
responses={},
|
| 86 |
+
summary="Test endpoint",
|
| 87 |
+
description=None,
|
| 88 |
+
schema_definitions={},
|
| 89 |
+
)
|
| 90 |
+
|
| 91 |
+
# Generate combined schema
|
| 92 |
+
schema = _combine_schemas(route)
|
| 93 |
+
optional_param_schema = schema["properties"]["optional_param"]
|
| 94 |
+
|
| 95 |
+
# Should have anyOf with the original type and null
|
| 96 |
+
assert "anyOf" in optional_param_schema
|
| 97 |
+
assert {"type": "null"} in optional_param_schema["anyOf"]
|
| 98 |
+
# Check that original schema is preserved (either simple type or complex schema)
|
| 99 |
+
if "type" in param_schema:
|
| 100 |
+
assert {"type": param_schema["type"]} in optional_param_schema["anyOf"]
|
| 101 |
+
else:
|
| 102 |
+
assert param_schema in optional_param_schema["anyOf"]
|