mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 07:09:11 +02:00
fix: handle explode parameter in experimental OpenAPI parser
- Add proper support for OpenAPI parameter style and explode settings - Fix query parameter encoding for explode=false scenarios - Support form (comma-delimited), pipeDelimited, and spaceDelimited styles - Add comprehensive unit tests for all style/explode combinations - Resolves issue where arrays were always encoded as repeated parameters Co-authored-by: Jeremiah Lowin <jlowin@users.noreply.github.com>
This commit is contained in:
parent
108ad4b70d
commit
87482faf8d
2 changed files with 229 additions and 4 deletions
|
|
@ -8,11 +8,48 @@ from jsonschema_path import SchemaPath
|
|||
|
||||
from fastmcp.utilities.logging import get_logger
|
||||
|
||||
from .models import HTTPRoute
|
||||
from .models import HTTPRoute, ParameterInfo
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
|
||||
def _encode_query_parameter(param: ParameterInfo, value: Any) -> Any:
|
||||
"""
|
||||
Encode a query parameter value according to OpenAPI style and explode settings.
|
||||
|
||||
Args:
|
||||
param: ParameterInfo containing style and explode settings
|
||||
value: The parameter value to encode
|
||||
|
||||
Returns:
|
||||
The encoded parameter value suitable for httpx
|
||||
"""
|
||||
# If not an array, return as-is
|
||||
if not isinstance(value, list):
|
||||
return value
|
||||
|
||||
# Handle explode=True (default behavior) - let httpx handle repeated params
|
||||
if param.explode is not False: # None or True
|
||||
return value
|
||||
|
||||
# Handle explode=False - need to serialize based on style
|
||||
style = param.style or "form" # Default style is "form"
|
||||
|
||||
if style == "form":
|
||||
# Comma-delimited: param=val1,val2,val3
|
||||
return ",".join(str(v) for v in value)
|
||||
elif style == "pipeDelimited":
|
||||
# Pipe-delimited: param=val1|val2|val3
|
||||
return "|".join(str(v) for v in value)
|
||||
elif style == "spaceDelimited":
|
||||
# Space-delimited: param=val1 val2 val3
|
||||
return " ".join(str(v) for v in value)
|
||||
else:
|
||||
# Unknown style, fall back to form (comma-delimited)
|
||||
logger.warning(f"Unknown parameter style '{style}', falling back to form style")
|
||||
return ",".join(str(v) for v in value)
|
||||
|
||||
|
||||
class RequestDirector:
|
||||
"""Builds httpx.Request objects from HTTPRoute and arguments using openapi-core."""
|
||||
|
||||
|
|
@ -89,6 +126,9 @@ class RequestDirector:
|
|||
header_params = {}
|
||||
body_props = {}
|
||||
|
||||
# Create a lookup for parameter info by name
|
||||
param_info_by_name = {param.name: param for param in route.parameters}
|
||||
|
||||
# Use parameter map to route arguments to correct locations
|
||||
if hasattr(route, "parameter_map") and route.parameter_map:
|
||||
for arg_name, value in flat_args.items():
|
||||
|
|
@ -108,7 +148,13 @@ class RequestDirector:
|
|||
if location == "path":
|
||||
path_params[openapi_name] = value
|
||||
elif location == "query":
|
||||
query_params[openapi_name] = value
|
||||
# Apply OpenAPI style and explode encoding for query parameters
|
||||
param_info = param_info_by_name.get(openapi_name)
|
||||
if param_info:
|
||||
encoded_value = _encode_query_parameter(param_info, value)
|
||||
query_params[openapi_name] = encoded_value
|
||||
else:
|
||||
query_params[openapi_name] = value
|
||||
elif location == "header":
|
||||
header_params[openapi_name] = value
|
||||
elif location == "body":
|
||||
|
|
@ -138,7 +184,15 @@ class RequestDirector:
|
|||
if location == "path":
|
||||
path_params[base_name] = value
|
||||
elif location == "query":
|
||||
query_params[base_name] = value
|
||||
# Apply OpenAPI style and explode encoding for query parameters
|
||||
param_info = param_info_by_name.get(base_name)
|
||||
if param_info:
|
||||
encoded_value = _encode_query_parameter(
|
||||
param_info, value
|
||||
)
|
||||
query_params[base_name] = encoded_value
|
||||
else:
|
||||
query_params[base_name] = value
|
||||
elif location == "header":
|
||||
header_params[base_name] = value
|
||||
continue
|
||||
|
|
@ -149,7 +203,13 @@ class RequestDirector:
|
|||
if location == "path":
|
||||
path_params[arg_name] = value
|
||||
elif location == "query":
|
||||
query_params[arg_name] = value
|
||||
# Apply OpenAPI style and explode encoding for query parameters
|
||||
param_info = param_info_by_name.get(arg_name)
|
||||
if param_info:
|
||||
encoded_value = _encode_query_parameter(param_info, value)
|
||||
query_params[arg_name] = encoded_value
|
||||
else:
|
||||
query_params[arg_name] = value
|
||||
elif location == "header":
|
||||
header_params[arg_name] = value
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,165 @@
|
|||
"""Tests for OpenAPI parameter style and explode handling in experimental parser."""
|
||||
|
||||
import httpx
|
||||
|
||||
from fastmcp.experimental.server.openapi import FastMCPOpenAPI
|
||||
from fastmcp.experimental.utilities.openapi import convert_openapi_schema_to_json_schema
|
||||
|
||||
|
||||
def _make_server_and_capture_urls(openapi_dict: dict, args: dict) -> list[str]:
|
||||
"""Helper to create a server and capture URLs generated during tool calls."""
|
||||
calls: list[str] = []
|
||||
|
||||
async def handler(request: httpx.Request):
|
||||
calls.append(str(request.url))
|
||||
return httpx.Response(200, json={"ok": True})
|
||||
|
||||
transport = httpx.MockTransport(handler)
|
||||
client = httpx.AsyncClient(base_url="https://api.test", transport=transport)
|
||||
spec = convert_openapi_schema_to_json_schema(openapi_dict)
|
||||
server = FastMCPOpenAPI(openapi_spec=spec, client=client, name="TestServer")
|
||||
|
||||
# Use the MCP call path to exercise the generated tool
|
||||
import anyio
|
||||
|
||||
anyio.run(server._mcp_call_tool, "echo", args) # type: ignore[arg-type]
|
||||
return calls
|
||||
|
||||
|
||||
def test_query_array_form_explode_false_works_correctly():
|
||||
"""Test that form style with explode=false generates comma-delimited values."""
|
||||
# Minimal OpenAPI spec: array query param with style=form, explode=false
|
||||
openapi = {
|
||||
"openapi": "3.1.0",
|
||||
"info": {"title": "T", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/echo": {
|
||||
"get": {
|
||||
"operationId": "echo",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "ids",
|
||||
"in": "query",
|
||||
"style": "form",
|
||||
"explode": False,
|
||||
"schema": {"type": "array", "items": {"type": "string"}},
|
||||
}
|
||||
],
|
||||
"responses": {"200": {"description": "ok"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
urls = _make_server_and_capture_urls(openapi, {"ids": ["1", "2", "3"]})
|
||||
|
||||
# Expected per OpenAPI (form+explode=false): `ids=1,2,3`
|
||||
# Actual (bug): multiple entries: `ids=1&ids=2&ids=3`
|
||||
assert any(url.endswith("/echo?ids=1%2C2%2C3") for url in urls), (
|
||||
f"Expected comma-delimited value, got: {urls}"
|
||||
)
|
||||
|
||||
|
||||
def test_query_array_pipe_explode_false_works_correctly():
|
||||
"""Test that pipeDelimited style with explode=false generates pipe-delimited values."""
|
||||
# pipeDelimited example: expect ids=1|2|3 when explode=false
|
||||
openapi = {
|
||||
"openapi": "3.1.0",
|
||||
"info": {"title": "T", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/echo": {
|
||||
"get": {
|
||||
"operationId": "echo",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "ids",
|
||||
"in": "query",
|
||||
"style": "pipeDelimited",
|
||||
"explode": False,
|
||||
"schema": {"type": "array", "items": {"type": "string"}},
|
||||
}
|
||||
],
|
||||
"responses": {"200": {"description": "ok"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
urls = _make_server_and_capture_urls(openapi, {"ids": ["1", "2", "3"]})
|
||||
assert any(url.endswith("/echo?ids=1%7C2%7C3") for url in urls), (
|
||||
f"Expected pipe-delimited value, got: {urls}"
|
||||
)
|
||||
|
||||
|
||||
def test_query_array_form_explode_true_works():
|
||||
"""Test that form style with explode=true works as expected (generates repeated params)."""
|
||||
openapi = {
|
||||
"openapi": "3.1.0",
|
||||
"info": {"title": "T", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/echo": {
|
||||
"get": {
|
||||
"operationId": "echo",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "ids",
|
||||
"in": "query",
|
||||
"style": "form",
|
||||
"explode": True,
|
||||
"schema": {"type": "array", "items": {"type": "string"}},
|
||||
}
|
||||
],
|
||||
"responses": {"200": {"description": "ok"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
urls = _make_server_and_capture_urls(openapi, {"ids": ["1", "2", "3"]})
|
||||
|
||||
# With explode=true, we expect repeated parameters: ids=1&ids=2&ids=3
|
||||
# The order may vary, so we check for all possible combinations
|
||||
expected_patterns = [
|
||||
"ids=1&ids=2&ids=3",
|
||||
"ids=1&ids=3&ids=2",
|
||||
"ids=2&ids=1&ids=3",
|
||||
"ids=2&ids=3&ids=1",
|
||||
"ids=3&ids=1&ids=2",
|
||||
"ids=3&ids=2&ids=1",
|
||||
]
|
||||
|
||||
assert any(any(pattern in url for pattern in expected_patterns) for url in urls), (
|
||||
f"Expected repeated params for explode=true, got: {urls}"
|
||||
)
|
||||
|
||||
|
||||
def test_query_array_space_explode_false_works_correctly():
|
||||
"""Test that spaceDelimited style with explode=false generates space-delimited values."""
|
||||
openapi = {
|
||||
"openapi": "3.1.0",
|
||||
"info": {"title": "T", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/echo": {
|
||||
"get": {
|
||||
"operationId": "echo",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "ids",
|
||||
"in": "query",
|
||||
"style": "spaceDelimited",
|
||||
"explode": False,
|
||||
"schema": {"type": "array", "items": {"type": "string"}},
|
||||
}
|
||||
],
|
||||
"responses": {"200": {"description": "ok"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
urls = _make_server_and_capture_urls(openapi, {"ids": ["1", "2", "3"]})
|
||||
# Space delimited should be URL encoded as + (or %20)
|
||||
assert any(
|
||||
url.endswith("/echo?ids=1+2+3") or url.endswith("/echo?ids=1%202%203")
|
||||
for url in urls
|
||||
), f"Expected space-delimited value, got: {urls}"
|
||||
Loading…
Add table
Add a link
Reference in a new issue