From f14be9ab775541d8e760edbd8f2c6d95228c2ce8 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Tue, 1 Jul 2025 19:06:35 -0400 Subject: [PATCH] Fix OpenAPI array parameter explode handling --- src/fastmcp/server/openapi.py | 4 +- src/fastmcp/utilities/openapi.py | 5 + .../openapi/test_explode_integration.py | 324 ++++++++++++++++++ .../openapi/test_openapi_path_parameters.py | 4 +- 4 files changed, 333 insertions(+), 4 deletions(-) create mode 100644 tests/server/openapi/test_explode_integration.py diff --git a/src/fastmcp/server/openapi.py b/src/fastmcp/server/openapi.py index 1e8051ace..b839e7481 100644 --- a/src/fastmcp/server/openapi.py +++ b/src/fastmcp/server/openapi.py @@ -355,10 +355,10 @@ class OpenAPITool(Tool): # Format array query parameters as comma-separated strings # following OpenAPI form style (default for query parameters) if isinstance(param_value, list) and p.schema_.get("type") == "array": - # Get explode parameter from schema, default is True for query parameters + # Get explode parameter from the parameter info, default is True for query parameters # If explode is True, the array is serialized as separate parameters # If explode is False, the array is serialized as a comma-separated string - explode = p.schema_.get("explode", True) + explode = p.explode if p.explode is not None else True if explode: # When explode=True, we pass the array directly, which HTTPX will serialize diff --git a/src/fastmcp/utilities/openapi.py b/src/fastmcp/utilities/openapi.py index fd0cbed5b..076fdb0d4 100644 --- a/src/fastmcp/utilities/openapi.py +++ b/src/fastmcp/utilities/openapi.py @@ -47,6 +47,7 @@ class ParameterInfo(FastMCPBaseModel): required: bool = False schema_: JsonSchema = Field(..., alias="schema") # Target name in IR description: str | None = None + explode: bool | None = None # OpenAPI explode property for array parameters class RequestBodyInfo(FastMCPBaseModel): @@ -359,6 +360,9 @@ class OpenAPIParser( ): param_schema_dict["default"] = resolved_media_schema.default + # Extract explode property if present + explode = getattr(parameter, "explode", None) + # Create parameter info object param_info = ParameterInfo( name=parameter.name, @@ -366,6 +370,7 @@ class OpenAPIParser( required=parameter.required, schema=param_schema_dict, description=parameter.description, + explode=explode, ) extracted_params.append(param_info) except Exception as e: diff --git a/tests/server/openapi/test_explode_integration.py b/tests/server/openapi/test_explode_integration.py new file mode 100644 index 000000000..3f82a98f9 --- /dev/null +++ b/tests/server/openapi/test_explode_integration.py @@ -0,0 +1,324 @@ +"""Integration test for OpenAPI explode property handling. + +This test verifies that the explode property is correctly parsed from OpenAPI +specifications and properly applied during HTTP request serialization. +""" + +from unittest.mock import AsyncMock, MagicMock + +import httpx +import pytest + +from fastmcp.server.openapi import OpenAPITool +from fastmcp.utilities.openapi import parse_openapi_to_http_routes + + +class TestExplodeIntegration: + """Test the complete pipeline from OpenAPI spec to HTTP request parameters.""" + + def test_explode_false_parsing_from_openapi_spec(self): + """Test that explode=false is correctly parsed from OpenAPI specification.""" + # Real OpenAPI spec with explode: false + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "required": False, + "style": "form", + "explode": False, # This should be respected + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + } + ], + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": {"schema": {"type": "object"}} + }, + } + }, + } + } + }, + } + + # Parse the spec + routes = parse_openapi_to_http_routes(openapi_spec) + route = routes[0] + parameter = route.parameters[0] + + # Verify explode property was captured correctly + assert parameter.name == "tags" + assert parameter.location == "query" + assert parameter.explode is False, ( + f"Expected explode=False, got {parameter.explode}" + ) + + def test_explode_true_parsing_from_openapi_spec(self): + """Test that explode=true is correctly parsed from OpenAPI specification.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "explode": True, # Explicitly set to true + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + } + ], + "responses": {"200": {"description": "Success"}}, + } + } + }, + } + + routes = parse_openapi_to_http_routes(openapi_spec) + parameter = routes[0].parameters[0] + + assert parameter.explode is True, ( + f"Expected explode=True, got {parameter.explode}" + ) + + def test_explode_default_parsing_from_openapi_spec(self): + """Test that missing explode defaults to None during parsing.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + # No explode property specified + } + ], + "responses": {"200": {"description": "Success"}}, + } + } + }, + } + + routes = parse_openapi_to_http_routes(openapi_spec) + parameter = routes[0].parameters[0] + + assert parameter.explode is None, ( + f"Expected explode=None, got {parameter.explode}" + ) + + @pytest.mark.asyncio + async def test_explode_false_request_serialization(self): + """Test that explode=false results in comma-separated query parameters in HTTP requests. + + This is the critical integration test that would have failed before the fix. + """ + # OpenAPI spec with explode: false + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "explode": False, + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + } + ], + "responses": {"200": {"description": "Success"}}, + } + } + }, + } + + # Parse and create tool + routes = parse_openapi_to_http_routes(openapi_spec) + route = routes[0] + + # Mock HTTP client + mock_client = AsyncMock(spec=httpx.AsyncClient) + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {} + mock_response.raise_for_status.return_value = None + mock_client.request.return_value = mock_response + + # Create tool + tool = OpenAPITool( + client=mock_client, + route=route, + name="search_items", + description="Search items", + parameters={}, + ) + + # Execute tool with array parameter + await tool.run({"tags": ["red", "blue", "green"]}) + + # Verify the HTTP request was made with comma-separated parameters + mock_client.request.assert_called_once() + call_kwargs = mock_client.request.call_args.kwargs + + # Check that params contains comma-separated values, not an array + params = call_kwargs.get("params", {}) + assert "tags" in params, "tags parameter should be present" + + tags_value = params["tags"] + assert isinstance(tags_value, str), ( + f"Expected string for explode=false, got {type(tags_value)}" + ) + assert tags_value == "red,blue,green", ( + f"Expected 'red,blue,green', got '{tags_value}'" + ) + + @pytest.mark.asyncio + async def test_explode_true_request_serialization(self): + """Test that explode=true results in separate query parameters in HTTP requests.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "explode": True, + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + } + ], + "responses": {"200": {"description": "Success"}}, + } + } + }, + } + + routes = parse_openapi_to_http_routes(openapi_spec) + route = routes[0] + + mock_client = AsyncMock(spec=httpx.AsyncClient) + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {} + mock_response.raise_for_status.return_value = None + mock_client.request.return_value = mock_response + + tool = OpenAPITool( + client=mock_client, + route=route, + name="search_items", + description="Search items", + parameters={}, + ) + + await tool.run({"tags": ["red", "blue", "green"]}) + + mock_client.request.assert_called_once() + call_kwargs = mock_client.request.call_args.kwargs + + params = call_kwargs.get("params", {}) + assert "tags" in params, "tags parameter should be present" + + tags_value = params["tags"] + assert isinstance(tags_value, list), ( + f"Expected list for explode=true, got {type(tags_value)}" + ) + assert tags_value == ["red", "blue", "green"], ( + f"Expected ['red', 'blue', 'green'], got {tags_value}" + ) + + @pytest.mark.asyncio + async def test_explode_default_request_serialization(self): + """Test that default behavior (no explode) uses explode=true for query parameters.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "1.0.0"}, + "paths": { + "/search": { + "get": { + "operationId": "search_items", + "parameters": [ + { + "name": "tags", + "in": "query", + "schema": { + "type": "array", + "items": {"type": "string"}, + }, + # No explode specified - should default to true for query params + } + ], + "responses": {"200": {"description": "Success"}}, + } + } + }, + } + + routes = parse_openapi_to_http_routes(openapi_spec) + route = routes[0] + + mock_client = AsyncMock(spec=httpx.AsyncClient) + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {} + mock_response.raise_for_status.return_value = None + mock_client.request.return_value = mock_response + + tool = OpenAPITool( + client=mock_client, + route=route, + name="search_items", + description="Search items", + parameters={}, + ) + + await tool.run({"tags": ["red", "blue", "green"]}) + + mock_client.request.assert_called_once() + call_kwargs = mock_client.request.call_args.kwargs + + params = call_kwargs.get("params", {}) + tags_value = params["tags"] + + # Default behavior should be explode=true (separate parameters) + assert isinstance(tags_value, list), ( + f"Expected list for default behavior, got {type(tags_value)}" + ) + assert tags_value == ["red", "blue", "green"], ( + f"Expected ['red', 'blue', 'green'], got {tags_value}" + ) diff --git a/tests/server/openapi/test_openapi_path_parameters.py b/tests/server/openapi/test_openapi_path_parameters.py index 075ad7045..8d6f1e820 100644 --- a/tests/server/openapi/test_openapi_path_parameters.py +++ b/tests/server/openapi/test_openapi_path_parameters.py @@ -320,9 +320,9 @@ async def test_array_query_parameter_format(mock_client): name="days", location="query", # This is a query parameter required=True, + explode=False, # Set explode=False to test comma-separated formatting schema={ "type": "array", - "explode": False, # Set explode=False to test comma-separated formatting "items": { "type": "string", "enum": [ @@ -390,9 +390,9 @@ async def test_array_query_parameter_exploded_format(mock_client): name="days", location="query", # This is a query parameter required=True, + explode=True, # Set explode=True for separate parameter serialization schema={ "type": "array", - "explode": True, # Set explode=True for separate parameter serialization "items": { "type": "string", "enum": [