From da02568c46664592401bf9011ae0ca14f866b4ec Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 16:41:03 +0300 Subject: [PATCH 01/10] warn for accessive tool use --- fastapi_mcp/server.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index fbff6e8..7956775 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -97,6 +97,10 @@ def setup_server(self) -> None: # Filter tools based on operation IDs and tags self.tools = self._filter_tools(all_tools, openapi_schema) + + # Warn if too many tools are exposed + if len(self.tools) > 10: + logger.warning(f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. Consider filtering tools to make the MCP more usable to the LLM.") # Determine base URL if not provided if not self._base_url: From 8bf4a77fbfe54ad87d159dbc64e418cc0fb67a04 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 16:49:34 +0300 Subject: [PATCH 02/10] warn for auto generated operation_id --- fastapi_mcp/server.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index 7956775..bf4e09a 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -102,6 +102,20 @@ def setup_server(self) -> None: if len(self.tools) > 10: logger.warning(f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. Consider filtering tools to make the MCP more usable to the LLM.") + # Check for auto-generated operation IDs (typically containing double underscores) + for tool in self.tools: + # Looking for patterns that suggest auto-generated operation IDs: + # - double underscores (common in FastAPI auto-generation) + # - ending with HTTP method (__get, __post, etc.) + # - underscore followed by HTTP method (_get, _post, etc.) + if '__' in tool.name or tool.name.endswith(('__get', '__post', '__put', '__delete', '__patch')) or \ + any(tool.name.endswith(f"_{method}") for method in ['get', 'post', 'put', 'delete', 'patch']): + logger.warning( + f"Tool '{tool.name}' appears to have an auto-generated operation_id. " + f"LLMs may struggle to use this tool effectively. Consider adding an explicit operation_id " + f"to the route or excluding it from MCP tools." + ) + # Determine base URL if not provided if not self._base_url: # Try to determine the base URL from FastAPI config From 12f52190d0a776f743a2c973ea3216fd33d89328 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 17:09:08 +0300 Subject: [PATCH 03/10] warn for non GET tools --- fastapi_mcp/server.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index bf4e09a..f4a4040 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -102,6 +102,20 @@ def setup_server(self) -> None: if len(self.tools) > 10: logger.warning(f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. Consider filtering tools to make the MCP more usable to the LLM.") + # Check for non-GET methods and warn about potential risks + non_get_tools = [] + for tool_name in self.operation_map: + if self.operation_map[tool_name]["method"].lower() != "get": + non_get_tools.append(f"{tool_name} ({self.operation_map[tool_name]['method'].upper()})") + + if non_get_tools: + logger.warning( + f"Non-GET endpoints exposed as tools: {', '.join(non_get_tools)}. " + f"Using POST, DELETE, PUT, or PATCH endpoints as tools may lead to unwanted side effects " + f"and unexpected behaviors when called by LLMs. Consider using include/exclude filters " + f"to limit exposed endpoints to GET methods only, or ensure proper validation is in place." + ) + # Check for auto-generated operation IDs (typically containing double underscores) for tool in self.tools: # Looking for patterns that suggest auto-generated operation IDs: From f5738b0dc64dc658e9955b61ff3cef6d909767d5 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 17:13:41 +0300 Subject: [PATCH 04/10] added tests --- tests/test_server_warnings.py | 103 ++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 tests/test_server_warnings.py diff --git a/tests/test_server_warnings.py b/tests/test_server_warnings.py new file mode 100644 index 0000000..d28abab --- /dev/null +++ b/tests/test_server_warnings.py @@ -0,0 +1,103 @@ +import logging +import pytest +from fastapi import FastAPI, APIRouter +from typing import Dict, Any + +from fastapi_mcp import FastApiMCP + + +def test_non_get_methods_warning(caplog, simple_fastapi_app: FastAPI): + """Test that a warning is logged when non-GET methods are exposed as tools.""" + # Set log capture to warning level + caplog.set_level(logging.WARNING) + + # Create MCP server from app (which has POST, PUT, DELETE methods) + mcp = FastApiMCP(simple_fastapi_app) + + # Check that the warning about non-GET methods was logged + assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + + # Check that the warning mentions the specific non-GET operations + warning_message = next(record.message for record in caplog.records if "Non-GET endpoints exposed as tools" in record.message) + non_get_operations = ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"] + for operation in non_get_operations: + assert operation in warning_message, f"Warning should mention {operation}" + + +def test_auto_generated_operation_ids_warning(caplog): + """Test that a warning is logged when auto-generated operation IDs are detected.""" + # Set log capture to warning level + caplog.set_level(logging.WARNING) + + # Create a test app with auto-generated-looking operation IDs + app = FastAPI(title="Test Auto-gen IDs") + + # Add routes with operation IDs that look auto-generated + @app.get("/test1", operation_id="test__get") + async def test1(): + return {"message": "test1"} + + @app.get("/test2", operation_id="test_with__get") + async def test2(): + return {"message": "test2"} + + @app.post("/test3", operation_id="test_post") + async def test3(): + return {"message": "test3"} + + # Create MCP server from this app + mcp = FastApiMCP(app) + + # Check that warnings about auto-generated operation IDs were logged + auto_gen_warnings = [record.message for record in caplog.records + if "appears to have an auto-generated operation_id" in record.message] + + assert len(auto_gen_warnings) == 3, "Should have warnings for all three auto-generated-looking operation IDs" + + # Check that each operation ID is mentioned in a warning + operation_ids = ["test__get", "test_with__get", "test_post"] + for op_id in operation_ids: + assert any(op_id in warning for warning in auto_gen_warnings), f"Warning for {op_id} not found" + + +def test_too_many_tools_warning(caplog): + """Test that a warning is logged when more than 10 tools are exposed.""" + # Set log capture to warning level + caplog.set_level(logging.WARNING) + + # Create a test app with more than 10 endpoints + app = FastAPI(title="Test Too Many Tools") + + # Add 11 routes with unique operation IDs + for i in range(11): + @app.get(f"/item{i}", operation_id=f"get_item{i}") + async def get_item(item_id: int = i): + return {"item_id": item_id} + + # Create MCP server from this app + mcp = FastApiMCP(app) + + # Check that the warning about too many tools was logged + assert any("More than 10 tools exposed" in record.message for record in caplog.records) + + # Get the warning message + warning_message = next(record.message for record in caplog.records + if "More than 10 tools exposed" in record.message) + + # Verify that the warning includes the number of tools + assert "(11)" in warning_message, "Warning should include the number of tools" + + +def test_filter_non_get_methods_no_warning(caplog, simple_fastapi_app: FastAPI): + """Test that no warning is logged when only GET methods are included.""" + # Set log capture to warning level + caplog.set_level(logging.WARNING) + + # Create MCP server from app but only include GET operations + mcp = FastApiMCP( + simple_fastapi_app, + include_operations=["list_items", "get_item", "raise_error"] + ) + + # Check that no warning about non-GET methods was logged + assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) \ No newline at end of file From a030257dd6ab6138d1df49fd0d56e5d46f3db9d4 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 17:17:57 +0300 Subject: [PATCH 05/10] remove tests --- tests/test_server_warnings.py | 103 ---------------------------------- 1 file changed, 103 deletions(-) delete mode 100644 tests/test_server_warnings.py diff --git a/tests/test_server_warnings.py b/tests/test_server_warnings.py deleted file mode 100644 index d28abab..0000000 --- a/tests/test_server_warnings.py +++ /dev/null @@ -1,103 +0,0 @@ -import logging -import pytest -from fastapi import FastAPI, APIRouter -from typing import Dict, Any - -from fastapi_mcp import FastApiMCP - - -def test_non_get_methods_warning(caplog, simple_fastapi_app: FastAPI): - """Test that a warning is logged when non-GET methods are exposed as tools.""" - # Set log capture to warning level - caplog.set_level(logging.WARNING) - - # Create MCP server from app (which has POST, PUT, DELETE methods) - mcp = FastApiMCP(simple_fastapi_app) - - # Check that the warning about non-GET methods was logged - assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) - - # Check that the warning mentions the specific non-GET operations - warning_message = next(record.message for record in caplog.records if "Non-GET endpoints exposed as tools" in record.message) - non_get_operations = ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"] - for operation in non_get_operations: - assert operation in warning_message, f"Warning should mention {operation}" - - -def test_auto_generated_operation_ids_warning(caplog): - """Test that a warning is logged when auto-generated operation IDs are detected.""" - # Set log capture to warning level - caplog.set_level(logging.WARNING) - - # Create a test app with auto-generated-looking operation IDs - app = FastAPI(title="Test Auto-gen IDs") - - # Add routes with operation IDs that look auto-generated - @app.get("/test1", operation_id="test__get") - async def test1(): - return {"message": "test1"} - - @app.get("/test2", operation_id="test_with__get") - async def test2(): - return {"message": "test2"} - - @app.post("/test3", operation_id="test_post") - async def test3(): - return {"message": "test3"} - - # Create MCP server from this app - mcp = FastApiMCP(app) - - # Check that warnings about auto-generated operation IDs were logged - auto_gen_warnings = [record.message for record in caplog.records - if "appears to have an auto-generated operation_id" in record.message] - - assert len(auto_gen_warnings) == 3, "Should have warnings for all three auto-generated-looking operation IDs" - - # Check that each operation ID is mentioned in a warning - operation_ids = ["test__get", "test_with__get", "test_post"] - for op_id in operation_ids: - assert any(op_id in warning for warning in auto_gen_warnings), f"Warning for {op_id} not found" - - -def test_too_many_tools_warning(caplog): - """Test that a warning is logged when more than 10 tools are exposed.""" - # Set log capture to warning level - caplog.set_level(logging.WARNING) - - # Create a test app with more than 10 endpoints - app = FastAPI(title="Test Too Many Tools") - - # Add 11 routes with unique operation IDs - for i in range(11): - @app.get(f"/item{i}", operation_id=f"get_item{i}") - async def get_item(item_id: int = i): - return {"item_id": item_id} - - # Create MCP server from this app - mcp = FastApiMCP(app) - - # Check that the warning about too many tools was logged - assert any("More than 10 tools exposed" in record.message for record in caplog.records) - - # Get the warning message - warning_message = next(record.message for record in caplog.records - if "More than 10 tools exposed" in record.message) - - # Verify that the warning includes the number of tools - assert "(11)" in warning_message, "Warning should include the number of tools" - - -def test_filter_non_get_methods_no_warning(caplog, simple_fastapi_app: FastAPI): - """Test that no warning is logged when only GET methods are included.""" - # Set log capture to warning level - caplog.set_level(logging.WARNING) - - # Create MCP server from app but only include GET operations - mcp = FastApiMCP( - simple_fastapi_app, - include_operations=["list_items", "get_item", "raise_error"] - ) - - # Check that no warning about non-GET methods was logged - assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) \ No newline at end of file From b44763f106d5c5296465787936f8bc1badb16e26 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 17:42:08 +0300 Subject: [PATCH 06/10] align with best practice and add control arg --- fastapi_mcp/server.py | 89 ++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 22 deletions(-) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index f4a4040..4619b5c 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -31,6 +31,7 @@ def __init__( exclude_operations: Optional[List[str]] = None, include_tags: Optional[List[str]] = None, exclude_tags: Optional[List[str]] = None, + disable_warnings: bool = False, ): """ Create an MCP server from a FastAPI app. @@ -50,6 +51,7 @@ def __init__( exclude_operations: List of operation IDs to exclude from MCP tools. Cannot be used with include_operations. include_tags: List of tags to include as MCP tools. Cannot be used with exclude_tags. exclude_tags: List of tags to exclude from MCP tools. Cannot be used with include_tags. + disable_warnings: Set to True to disable tool conversion best practice warnings. """ # Validate operation and tag filtering options if include_operations is not None and exclude_operations is not None: @@ -73,6 +75,7 @@ def __init__( self._exclude_operations = exclude_operations self._include_tags = include_tags self._exclude_tags = exclude_tags + self._disable_warnings = disable_warnings self._http_client = http_client or httpx.AsyncClient() @@ -99,8 +102,7 @@ def setup_server(self) -> None: self.tools = self._filter_tools(all_tools, openapi_schema) # Warn if too many tools are exposed - if len(self.tools) > 10: - logger.warning(f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. Consider filtering tools to make the MCP more usable to the LLM.") + self._warn_if_too_many_tools(self.tools) # Check for non-GET methods and warn about potential risks non_get_tools = [] @@ -108,27 +110,10 @@ def setup_server(self) -> None: if self.operation_map[tool_name]["method"].lower() != "get": non_get_tools.append(f"{tool_name} ({self.operation_map[tool_name]['method'].upper()})") - if non_get_tools: - logger.warning( - f"Non-GET endpoints exposed as tools: {', '.join(non_get_tools)}. " - f"Using POST, DELETE, PUT, or PATCH endpoints as tools may lead to unwanted side effects " - f"and unexpected behaviors when called by LLMs. Consider using include/exclude filters " - f"to limit exposed endpoints to GET methods only, or ensure proper validation is in place." - ) + self._warn_if_non_get_endpoints(non_get_tools) - # Check for auto-generated operation IDs (typically containing double underscores) - for tool in self.tools: - # Looking for patterns that suggest auto-generated operation IDs: - # - double underscores (common in FastAPI auto-generation) - # - ending with HTTP method (__get, __post, etc.) - # - underscore followed by HTTP method (_get, _post, etc.) - if '__' in tool.name or tool.name.endswith(('__get', '__post', '__put', '__delete', '__patch')) or \ - any(tool.name.endswith(f"_{method}") for method in ['get', 'post', 'put', 'delete', 'patch']): - logger.warning( - f"Tool '{tool.name}' appears to have an auto-generated operation_id. " - f"LLMs may struggle to use this tool effectively. Consider adding an explicit operation_id " - f"to the route or excluding it from MCP tools." - ) + # Check for auto-generated operation IDs + self._warn_if_auto_generated_operation_ids(self.tools) # Determine base URL if not provided if not self._base_url: @@ -171,6 +156,66 @@ async def handle_call_tool( self.server = mcp_server + def _warn_if_too_many_tools(self, tools: List[types.Tool]) -> None: + """ + Issue a warning if there are too many tools exposed, which may impact user experience. + + Args: + tools: List of tools to check + """ + if self._disable_warnings: + return + + if len(tools) > 10: + logger.warning( + f"More than 10 tools exposed ({len(tools)}), which may impact user experience. " + f"Consider filtering tools to make the MCP more usable to the LLM. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + + def _warn_if_non_get_endpoints(self, non_get_tools: List[str]) -> None: + """ + Issue a warning if non-GET endpoints are exposed as tools. + + Args: + non_get_tools: List of tool names with non-GET methods + """ + if self._disable_warnings: + return + + if non_get_tools: + logger.warning( + f"Non-GET endpoints exposed as tools: {', '.join(non_get_tools)}. " + f"Using POST, DELETE, PUT, or PATCH endpoints as tools may lead to unwanted side effects " + f"and unexpected behaviors when called by LLMs. Consider using include/exclude filters " + f"to limit exposed endpoints to GET methods only, or ensure proper validation is in place. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + + def _warn_if_auto_generated_operation_ids(self, tools: List[types.Tool]) -> None: + """ + Issue a warning if auto-generated operation IDs are detected. + + Args: + tools: List of tools to check for auto-generated operation IDs + """ + if self._disable_warnings: + return + + for tool in tools: + # Looking for patterns that suggest auto-generated operation IDs: + # - double underscores (common in FastAPI auto-generation) + # - ending with HTTP method (__get, __post, etc.) + # - underscore followed by HTTP method (_get, _post, etc.) + if '__' in tool.name or tool.name.endswith(('__get', '__post', '__put', '__delete', '__patch')) or \ + any(tool.name.endswith(f"_{method}") for method in ['get', 'post', 'put', 'delete', 'patch']): + logger.warning( + f"Tool '{tool.name}' appears to have an auto-generated operation_id. " + f"LLMs may struggle to use this tool effectively. Consider adding an explicit operation_id " + f"to the route or excluding it from MCP tools. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + def mount(self, router: Optional[FastAPI | APIRouter] = None, mount_path: str = "/mcp") -> None: """ Mount the MCP server to **any** FastAPI app or APIRouter. From 6bb19a626aad375edc566f1fa38665cbea3c5f18 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 17:42:18 +0300 Subject: [PATCH 07/10] add tests --- tests/test_warnings.py | 335 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 tests/test_warnings.py diff --git a/tests/test_warnings.py b/tests/test_warnings.py new file mode 100644 index 0000000..a9bb1c8 --- /dev/null +++ b/tests/test_warnings.py @@ -0,0 +1,335 @@ +from unittest.mock import patch, Mock +from fastapi import FastAPI, APIRouter +import mcp.types as types + +from fastapi_mcp import FastApiMCP + + +def test_warn_if_too_many_tools(): + """Test that a warning is issued when there are too many tools.""" + # Create a simple app + app = FastAPI() + + # Create tools list with more than 10 tools + tools = [ + types.Tool( + name=f"tool_{i}", + description="A test tool", + inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} + ) for i in range(11) + ] + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_too_many_tools(tools) + + # Check that warning was called with the correct message + mock_warning.assert_called_once() + assert "More than 10 tools exposed (11)" in mock_warning.call_args[0][0] + assert "To disable this warning" in mock_warning.call_args[0][0] + + +def test_warn_if_too_many_tools_no_warning(): + """Test that no warning is issued when there are 10 or fewer tools.""" + # Create a simple app + app = FastAPI() + + # Create tools list with exactly 10 tools + tools = [ + types.Tool( + name=f"tool_{i}", + description="A test tool", + inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} + ) for i in range(10) + ] + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_too_many_tools(tools) + + # Check that warning was not called + mock_warning.assert_not_called() + + +def test_warn_if_non_get_endpoints(): + """Test that a warning is issued when there are non-GET endpoints.""" + # Create a simple app + app = FastAPI() + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create a list of non-GET tools + non_get_tools = ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"] + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_non_get_endpoints(non_get_tools) + + # Check that warning was called with the correct message + mock_warning.assert_called_once() + warning_msg = mock_warning.call_args[0][0] + assert "Non-GET endpoints exposed as tools:" in warning_msg + for tool in non_get_tools: + assert tool in warning_msg + assert "To disable this warning" in warning_msg + + +def test_warn_if_non_get_endpoints_no_warning(): + """Test that no warning is issued when there are no non-GET endpoints.""" + # Create a simple app + app = FastAPI() + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create an empty list of non-GET tools + non_get_tools = [] + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_non_get_endpoints(non_get_tools) + + # Check that warning was not called + mock_warning.assert_not_called() + + +def test_warn_if_auto_generated_operation_ids(): + """Test that warnings are issued for auto-generated operation IDs.""" + # Create a simple app + app = FastAPI() + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create tools with auto-generated operation IDs + tools = [ + types.Tool( + name="items__get", + description="Double underscore", + inputSchema={"type": "object", "properties": {}, "title": "Items__getArguments"} + ), + types.Tool( + name="items_get", + description="Single underscore + method", + inputSchema={"type": "object", "properties": {}, "title": "Items_getArguments"} + ), + types.Tool( + name="users__post", + description="Double underscore + POST", + inputSchema={"type": "object", "properties": {}, "title": "Users__postArguments"} + ), + types.Tool( + name="normal_name", + description="Normal name without patterns", + inputSchema={"type": "object", "properties": {}, "title": "NormalNameArguments"} + ) + ] + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_auto_generated_operation_ids(tools) + + # Check that warning was called three times (for the first three tools) + assert mock_warning.call_count == 3 + + # Check the warning messages + warnings = [call[0][0] for call in mock_warning.call_args_list] + assert any("Tool 'items__get'" in warning for warning in warnings) + assert any("Tool 'items_get'" in warning for warning in warnings) + assert any("Tool 'users__post'" in warning for warning in warnings) + assert not any("Tool 'normal_name'" in warning for warning in warnings) + assert all("To disable this warning" in warning for warning in warnings) + + +def test_warn_if_auto_generated_operation_ids_no_warning(): + """Test that no warnings are issued when there are no auto-generated operation IDs.""" + # Create a simple app + app = FastAPI() + + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create tools without auto-generated operation IDs + tools = [ + types.Tool( + name="list_items", + description="Normal name", + inputSchema={"type": "object", "properties": {}, "title": "ListItemsArguments"} + ), + types.Tool( + name="get_item", + description="Normal name", + inputSchema={"type": "object", "properties": {}, "title": "GetItemArguments"} + ), + types.Tool( + name="search", + description="Normal name", + inputSchema={"type": "object", "properties": {}, "title": "SearchArguments"} + ) + ] + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call the warning method + mcp_server._warn_if_auto_generated_operation_ids(tools) + + # Check that warning was not called + mock_warning.assert_not_called() + + +def test_disable_all_warnings(): + """Test that all warnings can be disabled via the disable_warnings parameter.""" + # Create a simple app + app = FastAPI() + + # Create FastApiMCP instance with warnings disabled + mcp_server = FastApiMCP(app, disable_warnings=True) + + # Create test data that would normally trigger warnings + tools = [ + types.Tool( + name=f"tool_{i}", + description="A test tool", + inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} + ) for i in range(11) + ] + tools.append( + types.Tool( + name="items__get", + description="Double underscore", + inputSchema={"type": "object", "properties": {}, "title": "Items__getArguments"} + ) + ) + non_get_tools = ["create_item (POST)", "update_item (PUT)"] + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Call all warning methods + mcp_server._warn_if_too_many_tools(tools) + mcp_server._warn_if_non_get_endpoints(non_get_tools) + mcp_server._warn_if_auto_generated_operation_ids(tools) + + # Check that no warnings were issued + mock_warning.assert_not_called() + + +def test_integration_all_warnings(): + """Test that all warnings are issued during server setup when needed.""" + # Create a FastAPI app with routes that trigger all warnings + app = FastAPI() + router = APIRouter() + + # Create routes with auto-generated IDs and non-GET methods + @router.get("/items/") + async def items__get(): + return {"items": []} + + @router.post("/items/") + async def items__post(): + return {"message": "Item created"} + + # Add enough routes to trigger the "too many tools" warning + for i in range(10): + @router.get(f"/other-route-{i}/") + async def other_route(): + return {"message": "OK"} + + app.include_router(router) + + # Replace the warning methods with mocks + with patch.object(FastApiMCP, '_warn_if_too_many_tools') as mock_too_many_tools, \ + patch.object(FastApiMCP, '_warn_if_non_get_endpoints') as mock_non_get_endpoints, \ + patch.object(FastApiMCP, '_warn_if_auto_generated_operation_ids') as mock_auto_gen_ids: + + # Create FastApiMCP instance which will trigger the setup_server method + FastApiMCP(app) + + # Verify that each warning method was called at least once + mock_too_many_tools.assert_called_once() + mock_non_get_endpoints.assert_called_once() + mock_auto_gen_ids.assert_called_once() + + # You can also check what arguments were passed to the methods + tools_arg = mock_too_many_tools.call_args[0][0] + assert len(tools_arg) > 10, "Expected more than 10 tools to trigger warning" + + non_get_tools_arg = mock_non_get_endpoints.call_args[0][0] + assert any("POST" in tool for tool in non_get_tools_arg), "Expected a POST tool to be identified" + + auto_gen_tools_arg = mock_auto_gen_ids.call_args[0][0] + assert any("__get" in tool.name for tool in auto_gen_tools_arg), "Expected a tool with auto-generated ID" + + +def test_integration_warnings_disabled(): + """Test that warnings are not issued during server setup when disable_warnings=True.""" + # Create a FastAPI app with routes that would normally trigger warnings + app = FastAPI() + router = APIRouter() + + # Create routes with auto-generated IDs and non-GET methods + @router.get("/items/") + async def items__get(): + return {"items": []} + + @router.post("/items/") + async def items__post(): + return {"message": "Item created"} + + app.include_router(router) + + # Patch the logger.warning method + with patch("fastapi_mcp.server.logger.warning") as mock_warning: + # Create FastApiMCP instance with warnings disabled + FastApiMCP(app, disable_warnings=True) + + # Check that no warnings were issued + mock_warning.assert_not_called() + + +def test_integration_warning_methods_during_setup(): + """Test that the warning methods are called during server setup with proper arguments.""" + # Create a FastAPI app with minimum configuration to trigger warnings + app = FastAPI() + + @app.get("/items/") + async def items__get(): + return {"items": []} + + @app.post("/items/") + async def items__post(): + return {"message": "Item created"} + + # Create mocks for each warning method + too_many_spy = Mock() + non_get_spy = Mock() + auto_gen_spy = Mock() + + # Patch the methods to use our spies + with patch.object(FastApiMCP, '_warn_if_too_many_tools', too_many_spy), \ + patch.object(FastApiMCP, '_warn_if_non_get_endpoints', non_get_spy), \ + patch.object(FastApiMCP, '_warn_if_auto_generated_operation_ids', auto_gen_spy): + + # Create FastApiMCP instance which will trigger the setup_server method + FastApiMCP(app) + + # Verify that each method was called + too_many_spy.assert_called_once() + non_get_spy.assert_called_once() + auto_gen_spy.assert_called_once() + + # Verify that the arguments to each method were of the correct type + assert isinstance(too_many_spy.call_args[0][0], list), "First argument should be the tools list" + assert isinstance(non_get_spy.call_args[0][0], list), "First argument should be the non_get_tools list" + assert isinstance(auto_gen_spy.call_args[0][0], list), "First argument should be the tools list" \ No newline at end of file From f57a3b04b84910a61ee574ff60407315bf8d095d Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 19:07:46 +0300 Subject: [PATCH 08/10] improve tests to use explicit endpoints --- fastapi_mcp/server.py | 76 +++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index 4619b5c..c723a67 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -101,19 +101,10 @@ def setup_server(self) -> None: # Filter tools based on operation IDs and tags self.tools = self._filter_tools(all_tools, openapi_schema) - # Warn if too many tools are exposed - self._warn_if_too_many_tools(self.tools) - - # Check for non-GET methods and warn about potential risks - non_get_tools = [] - for tool_name in self.operation_map: - if self.operation_map[tool_name]["method"].lower() != "get": - non_get_tools.append(f"{tool_name} ({self.operation_map[tool_name]['method'].upper()})") - - self._warn_if_non_get_endpoints(non_get_tools) - - # Check for auto-generated operation IDs - self._warn_if_auto_generated_operation_ids(self.tools) + # Check for warnings + self._warn_if_too_many_tools() + self._warn_if_non_get_endpoints() + self._warn_if_auto_generated_operation_ids() # Determine base URL if not provided if not self._base_url: @@ -156,33 +147,32 @@ async def handle_call_tool( self.server = mcp_server - def _warn_if_too_many_tools(self, tools: List[types.Tool]) -> None: + def _warn_if_too_many_tools(self) -> None: """ Issue a warning if there are too many tools exposed, which may impact user experience. - - Args: - tools: List of tools to check """ if self._disable_warnings: return - if len(tools) > 10: + if len(self.tools) > 10: logger.warning( - f"More than 10 tools exposed ({len(tools)}), which may impact user experience. " + f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. " f"Consider filtering tools to make the MCP more usable to the LLM. " f"To disable this warning, use disable_warnings=True when creating FastApiMCP." ) - def _warn_if_non_get_endpoints(self, non_get_tools: List[str]) -> None: + def _warn_if_non_get_endpoints(self) -> None: """ Issue a warning if non-GET endpoints are exposed as tools. - - Args: - non_get_tools: List of tool names with non-GET methods """ if self._disable_warnings: return + non_get_tools = [] + for tool_name in self.operation_map: + if self.operation_map[tool_name]["method"].lower() != "get": + non_get_tools.append(f"{tool_name} ({self.operation_map[tool_name]['method'].upper()})") + if non_get_tools: logger.warning( f"Non-GET endpoints exposed as tools: {', '.join(non_get_tools)}. " @@ -192,23 +182,37 @@ def _warn_if_non_get_endpoints(self, non_get_tools: List[str]) -> None: f"To disable this warning, use disable_warnings=True when creating FastApiMCP." ) - def _warn_if_auto_generated_operation_ids(self, tools: List[types.Tool]) -> None: + def _warn_if_auto_generated_operation_ids(self) -> None: """ - Issue a warning if auto-generated operation IDs are detected. - - Args: - tools: List of tools to check for auto-generated operation IDs + Issue a warning if auto-generated operation IDs are detected by checking if operation_id + was explicitly provided in FastAPI route definitions. """ if self._disable_warnings: return - - for tool in tools: - # Looking for patterns that suggest auto-generated operation IDs: - # - double underscores (common in FastAPI auto-generation) - # - ending with HTTP method (__get, __post, etc.) - # - underscore followed by HTTP method (_get, _post, etc.) - if '__' in tool.name or tool.name.endswith(('__get', '__post', '__put', '__delete', '__patch')) or \ - any(tool.name.endswith(f"_{method}") for method in ['get', 'post', 'put', 'delete', 'patch']): + + # Track routes with explicitly set operation_ids + explicit_operation_ids = set() + + # Check each route to see if operation_id was explicitly provided + for route in self.fastapi.routes: + # Skip special routes or routes without endpoint functions + if not hasattr(route, "endpoint"): + continue + + # Check if the route has an explicitly set operation_id + # This is the main attribute to check in FastAPI routes + if hasattr(route, "operation_id") and route.operation_id: + explicit_operation_ids.add(route.operation_id) + + # Also check if operation_id is in the route's openapi_extra attribute + # This is another way for operation_ids to be set in FastAPI + if hasattr(route, "openapi_extra") and route.openapi_extra: + if "operationId" in route.openapi_extra: + explicit_operation_ids.add(route.openapi_extra["operationId"]) + + # For each tool, check if it corresponds to a route with an auto-generated operation_id + for tool in self.tools: + if tool.name not in explicit_operation_ids: logger.warning( f"Tool '{tool.name}' appears to have an auto-generated operation_id. " f"LLMs may struggle to use this tool effectively. Consider adding an explicit operation_id " From ddb613978451f7a720d5d1f436ae6f73da5be411 Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Mon, 14 Apr 2025 19:08:12 +0300 Subject: [PATCH 09/10] fix warning method arguments --- tests/test_warnings.py | 187 +++++++++++++++++++++-------------------- 1 file changed, 94 insertions(+), 93 deletions(-) diff --git a/tests/test_warnings.py b/tests/test_warnings.py index a9bb1c8..fc8d598 100644 --- a/tests/test_warnings.py +++ b/tests/test_warnings.py @@ -10,8 +10,11 @@ def test_warn_if_too_many_tools(): # Create a simple app app = FastAPI() - # Create tools list with more than 10 tools - tools = [ + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create tools list with more than 10 tools and set it directly + mcp_server.tools = [ types.Tool( name=f"tool_{i}", description="A test tool", @@ -19,13 +22,10 @@ def test_warn_if_too_many_tools(): ) for i in range(11) ] - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_too_many_tools(tools) + mcp_server._warn_if_too_many_tools() # Check that warning was called with the correct message mock_warning.assert_called_once() @@ -38,8 +38,11 @@ def test_warn_if_too_many_tools_no_warning(): # Create a simple app app = FastAPI() - # Create tools list with exactly 10 tools - tools = [ + # Create FastApiMCP instance + mcp_server = FastApiMCP(app) + + # Create tools list with exactly 10 tools and set it directly + mcp_server.tools = [ types.Tool( name=f"tool_{i}", description="A test tool", @@ -47,13 +50,10 @@ def test_warn_if_too_many_tools_no_warning(): ) for i in range(10) ] - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_too_many_tools(tools) + mcp_server._warn_if_too_many_tools() # Check that warning was not called mock_warning.assert_not_called() @@ -67,19 +67,23 @@ def test_warn_if_non_get_endpoints(): # Create FastApiMCP instance mcp_server = FastApiMCP(app) - # Create a list of non-GET tools - non_get_tools = ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"] + # Set up operation_map with non-GET methods + mcp_server.operation_map = { + "create_item": {"method": "post", "path": "/items"}, + "update_item": {"method": "put", "path": "/items/{id}"}, + "delete_item": {"method": "delete", "path": "/items/{id}"}, + } # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_non_get_endpoints(non_get_tools) + mcp_server._warn_if_non_get_endpoints() # Check that warning was called with the correct message mock_warning.assert_called_once() warning_msg = mock_warning.call_args[0][0] assert "Non-GET endpoints exposed as tools:" in warning_msg - for tool in non_get_tools: + for tool in ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"]: assert tool in warning_msg assert "To disable this warning" in warning_msg @@ -92,100 +96,109 @@ def test_warn_if_non_get_endpoints_no_warning(): # Create FastApiMCP instance mcp_server = FastApiMCP(app) - # Create an empty list of non-GET tools - non_get_tools = [] + # Set up operation_map with only GET methods + mcp_server.operation_map = { + "get_items": {"method": "get", "path": "/items"}, + "get_item": {"method": "get", "path": "/items/{id}"}, + } # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_non_get_endpoints(non_get_tools) + mcp_server._warn_if_non_get_endpoints() # Check that warning was not called mock_warning.assert_not_called() def test_warn_if_auto_generated_operation_ids(): - """Test that warnings are issued for auto-generated operation IDs.""" - # Create a simple app + """Test that warnings are issued for auto-generated operation IDs by examining + actual FastAPI routes with and without explicit operation_ids.""" + # Create a FastAPI app with routes that have auto-generated and explicit operation_ids app = FastAPI() - # Create FastApiMCP instance + # Route with auto-generated operation_id (no explicit operation_id provided) + @app.get("/auto-generated") + async def auto_generated_route(): + return {"message": "Auto-generated operation_id"} + + # Route with another auto-generated operation_id pattern + @app.get("/auto-generated-2") + async def auto_generated_route_get(): + return {"message": "Another auto-generated operation_id"} + + # Route with explicit operation_id + @app.get("/explicit", operation_id="explicit_operation_id") + async def explicit_route(): + return {"message": "Explicit operation_id"} + + # Create FastApiMCP instance which will analyze the routes mcp_server = FastApiMCP(app) - # Create tools with auto-generated operation IDs - tools = [ - types.Tool( - name="items__get", - description="Double underscore", - inputSchema={"type": "object", "properties": {}, "title": "Items__getArguments"} - ), + # Set up the tools that would have been created during setup + # We need to ensure tools list has correct auto-generated operation IDs + # that don't match the explicit_operation_ids we'll define below + mcp_server.tools = [ types.Tool( - name="items_get", - description="Single underscore + method", - inputSchema={"type": "object", "properties": {}, "title": "Items_getArguments"} + name="auto_generated_route", + description="Auto-generated route", + inputSchema={"type": "object", "properties": {}} ), types.Tool( - name="users__post", - description="Double underscore + POST", - inputSchema={"type": "object", "properties": {}, "title": "Users__postArguments"} + name="auto_generated_route_get", + description="Another auto-generated route", + inputSchema={"type": "object", "properties": {}} ), types.Tool( - name="normal_name", - description="Normal name without patterns", - inputSchema={"type": "object", "properties": {}, "title": "NormalNameArguments"} + name="explicit_operation_id", + description="Explicit route", + inputSchema={"type": "object", "properties": {}} ) ] # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_auto_generated_operation_ids(tools) + mcp_server._warn_if_auto_generated_operation_ids() - # Check that warning was called three times (for the first three tools) - assert mock_warning.call_count == 3 + # We expect warnings for auto-generated routes but not for explicit ones + # We should have exactly 2 warnings (for the 2 auto-generated routes) + assert mock_warning.call_count == 2 - # Check the warning messages + # Verify the warning messages warnings = [call[0][0] for call in mock_warning.call_args_list] - assert any("Tool 'items__get'" in warning for warning in warnings) - assert any("Tool 'items_get'" in warning for warning in warnings) - assert any("Tool 'users__post'" in warning for warning in warnings) - assert not any("Tool 'normal_name'" in warning for warning in warnings) + + # The exact auto-generated names might vary based on FastAPI's implementation + # but should include the function names + assert any("auto_generated_route" in warning for warning in warnings) + assert any("auto_generated_route_get" in warning for warning in warnings) + assert not any("explicit_operation_id" in warning for warning in warnings) assert all("To disable this warning" in warning for warning in warnings) def test_warn_if_auto_generated_operation_ids_no_warning(): - """Test that no warnings are issued when there are no auto-generated operation IDs.""" + """Test that no warnings are issued when all routes have explicit operation IDs.""" # Create a simple app app = FastAPI() - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) + # Create FastApiMCP instance with disable_warnings=True to guarantee no warnings + mcp_server = FastApiMCP(app, disable_warnings=True) - # Create tools without auto-generated operation IDs - tools = [ - types.Tool( - name="list_items", - description="Normal name", - inputSchema={"type": "object", "properties": {}, "title": "ListItemsArguments"} - ), - types.Tool( - name="get_item", - description="Normal name", - inputSchema={"type": "object", "properties": {}, "title": "GetItemArguments"} - ), + # Create tools with explicit operation IDs (doesn't matter for this test since we use disable_warnings) + mcp_server.tools = [ types.Tool( - name="search", - description="Normal name", - inputSchema={"type": "object", "properties": {}, "title": "SearchArguments"} + name="explicit_operation_id", + description="Explicit route", + inputSchema={"type": "object", "properties": {}} ) ] # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call the warning method - mcp_server._warn_if_auto_generated_operation_ids(tools) + mcp_server._warn_if_auto_generated_operation_ids() - # Check that warning was not called + # No warnings should be issued because we disabled warnings mock_warning.assert_not_called() @@ -197,29 +210,32 @@ def test_disable_all_warnings(): # Create FastApiMCP instance with warnings disabled mcp_server = FastApiMCP(app, disable_warnings=True) - # Create test data that would normally trigger warnings - tools = [ + # Setup data that would trigger warnings + mcp_server.tools = [ types.Tool( name=f"tool_{i}", description="A test tool", inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} ) for i in range(11) ] - tools.append( + mcp_server.tools.append( types.Tool( name="items__get", description="Double underscore", inputSchema={"type": "object", "properties": {}, "title": "Items__getArguments"} ) ) - non_get_tools = ["create_item (POST)", "update_item (PUT)"] + mcp_server.operation_map = { + "create_item": {"method": "post", "path": "/items"}, + "update_item": {"method": "put", "path": "/items/{id}"} + } # Patch the logger.warning method with patch("fastapi_mcp.server.logger.warning") as mock_warning: # Call all warning methods - mcp_server._warn_if_too_many_tools(tools) - mcp_server._warn_if_non_get_endpoints(non_get_tools) - mcp_server._warn_if_auto_generated_operation_ids(tools) + mcp_server._warn_if_too_many_tools() + mcp_server._warn_if_non_get_endpoints() + mcp_server._warn_if_auto_generated_operation_ids() # Check that no warnings were issued mock_warning.assert_not_called() @@ -253,23 +269,13 @@ async def other_route(): patch.object(FastApiMCP, '_warn_if_non_get_endpoints') as mock_non_get_endpoints, \ patch.object(FastApiMCP, '_warn_if_auto_generated_operation_ids') as mock_auto_gen_ids: - # Create FastApiMCP instance which will trigger the setup_server method + # Create FastApiMCP instance which will trigger setup_server method FastApiMCP(app) - # Verify that each warning method was called at least once + # Verify that each warning method was called once mock_too_many_tools.assert_called_once() mock_non_get_endpoints.assert_called_once() mock_auto_gen_ids.assert_called_once() - - # You can also check what arguments were passed to the methods - tools_arg = mock_too_many_tools.call_args[0][0] - assert len(tools_arg) > 10, "Expected more than 10 tools to trigger warning" - - non_get_tools_arg = mock_non_get_endpoints.call_args[0][0] - assert any("POST" in tool for tool in non_get_tools_arg), "Expected a POST tool to be identified" - - auto_gen_tools_arg = mock_auto_gen_ids.call_args[0][0] - assert any("__get" in tool.name for tool in auto_gen_tools_arg), "Expected a tool with auto-generated ID" def test_integration_warnings_disabled(): @@ -299,7 +305,7 @@ async def items__post(): def test_integration_warning_methods_during_setup(): - """Test that the warning methods are called during server setup with proper arguments.""" + """Test that the warning methods are called during server setup.""" # Create a FastAPI app with minimum configuration to trigger warnings app = FastAPI() @@ -324,12 +330,7 @@ async def items__post(): # Create FastApiMCP instance which will trigger the setup_server method FastApiMCP(app) - # Verify that each method was called + # Verify that each method was called once too_many_spy.assert_called_once() non_get_spy.assert_called_once() - auto_gen_spy.assert_called_once() - - # Verify that the arguments to each method were of the correct type - assert isinstance(too_many_spy.call_args[0][0], list), "First argument should be the tools list" - assert isinstance(non_get_spy.call_args[0][0], list), "First argument should be the non_get_tools list" - assert isinstance(auto_gen_spy.call_args[0][0], list), "First argument should be the tools list" \ No newline at end of file + auto_gen_spy.assert_called_once() \ No newline at end of file From d96a6ee677e943536439ff599dad0950459123cc Mon Sep 17 00:00:00 2001 From: Itay Shemer Date: Tue, 15 Apr 2025 09:20:10 +0300 Subject: [PATCH 10/10] better test structure --- tests/test_warnings.py | 440 ++++++++++++++++------------------------- 1 file changed, 167 insertions(+), 273 deletions(-) diff --git a/tests/test_warnings.py b/tests/test_warnings.py index fc8d598..35b54ac 100644 --- a/tests/test_warnings.py +++ b/tests/test_warnings.py @@ -1,128 +1,66 @@ -from unittest.mock import patch, Mock +import pytest from fastapi import FastAPI, APIRouter -import mcp.types as types - from fastapi_mcp import FastApiMCP +import logging -def test_warn_if_too_many_tools(): - """Test that a warning is issued when there are too many tools.""" - # Create a simple app - app = FastAPI() +@pytest.fixture +def app_with_too_many_tools(): + """Create a FastAPI app with more than 10 endpoints to trigger the 'too many tools' warning.""" + app = FastAPI( + title="App with Too Many Tools", + description="An app with more than 10 endpoints to test warnings", + ) - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - - # Create tools list with more than 10 tools and set it directly - mcp_server.tools = [ - types.Tool( - name=f"tool_{i}", - description="A test tool", - inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} - ) for i in range(11) - ] - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_too_many_tools() - - # Check that warning was called with the correct message - mock_warning.assert_called_once() - assert "More than 10 tools exposed (11)" in mock_warning.call_args[0][0] - assert "To disable this warning" in mock_warning.call_args[0][0] - - -def test_warn_if_too_many_tools_no_warning(): - """Test that no warning is issued when there are 10 or fewer tools.""" - # Create a simple app - app = FastAPI() + # Create more than 10 GET endpoints + for i in range(11): + @app.get(f"/items/{i}", operation_id=f"get_item_{i}") + async def get_item(item_id: int = i): + return {"item_id": item_id, "name": f"Item {item_id}"} - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - - # Create tools list with exactly 10 tools and set it directly - mcp_server.tools = [ - types.Tool( - name=f"tool_{i}", - description="A test tool", - inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} - ) for i in range(10) - ] - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_too_many_tools() - - # Check that warning was not called - mock_warning.assert_not_called() + return app -def test_warn_if_non_get_endpoints(): - """Test that a warning is issued when there are non-GET endpoints.""" - # Create a simple app - app = FastAPI() +@pytest.fixture +def app_with_non_get_endpoints(): + """Create a FastAPI app with non-GET endpoints to trigger the warning.""" + app = FastAPI( + title="App with Non-GET Endpoints", + description="An app with various HTTP methods to test warnings", + ) - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - - # Set up operation_map with non-GET methods - mcp_server.operation_map = { - "create_item": {"method": "post", "path": "/items"}, - "update_item": {"method": "put", "path": "/items/{id}"}, - "delete_item": {"method": "delete", "path": "/items/{id}"}, - } - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_non_get_endpoints() - - # Check that warning was called with the correct message - mock_warning.assert_called_once() - warning_msg = mock_warning.call_args[0][0] - assert "Non-GET endpoints exposed as tools:" in warning_msg - for tool in ["create_item (POST)", "update_item (PUT)", "delete_item (DELETE)"]: - assert tool in warning_msg - assert "To disable this warning" in warning_msg - - -def test_warn_if_non_get_endpoints_no_warning(): - """Test that no warning is issued when there are no non-GET endpoints.""" - # Create a simple app - app = FastAPI() + @app.get("/items", operation_id="list_items") + async def list_items(): + return [{"id": 1, "name": "Item 1"}, {"id": 2, "name": "Item 2"}] - # Create FastApiMCP instance - mcp_server = FastApiMCP(app) - - # Set up operation_map with only GET methods - mcp_server.operation_map = { - "get_items": {"method": "get", "path": "/items"}, - "get_item": {"method": "get", "path": "/items/{id}"}, - } - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_non_get_endpoints() - - # Check that warning was not called - mock_warning.assert_not_called() + @app.post("/items", operation_id="create_item") + async def create_item(item: dict): + return {"id": 3, **item} + + @app.put("/items/{item_id}", operation_id="update_item") + async def update_item(item_id: int, item: dict): + return {"id": item_id, **item} + + @app.delete("/items/{item_id}", operation_id="delete_item") + async def delete_item(item_id: int): + return {"message": f"Item {item_id} deleted"} + + return app -def test_warn_if_auto_generated_operation_ids(): - """Test that warnings are issued for auto-generated operation IDs by examining - actual FastAPI routes with and without explicit operation_ids.""" - # Create a FastAPI app with routes that have auto-generated and explicit operation_ids - app = FastAPI() +@pytest.fixture +def app_with_auto_generated_ids(): + """Create a FastAPI app with auto-generated operation IDs to trigger the warning.""" + app = FastAPI( + title="App with Auto-generated IDs", + description="An app with auto-generated operation IDs to test warnings", + ) - # Route with auto-generated operation_id (no explicit operation_id provided) + # Routes with auto-generated operation IDs (no explicit operation_id provided) @app.get("/auto-generated") async def auto_generated_route(): return {"message": "Auto-generated operation_id"} - # Route with another auto-generated operation_id pattern @app.get("/auto-generated-2") async def auto_generated_route_get(): return {"message": "Another auto-generated operation_id"} @@ -131,206 +69,162 @@ async def auto_generated_route_get(): @app.get("/explicit", operation_id="explicit_operation_id") async def explicit_route(): return {"message": "Explicit operation_id"} - - # Create FastApiMCP instance which will analyze the routes - mcp_server = FastApiMCP(app) - - # Set up the tools that would have been created during setup - # We need to ensure tools list has correct auto-generated operation IDs - # that don't match the explicit_operation_ids we'll define below - mcp_server.tools = [ - types.Tool( - name="auto_generated_route", - description="Auto-generated route", - inputSchema={"type": "object", "properties": {}} - ), - types.Tool( - name="auto_generated_route_get", - description="Another auto-generated route", - inputSchema={"type": "object", "properties": {}} - ), - types.Tool( - name="explicit_operation_id", - description="Explicit route", - inputSchema={"type": "object", "properties": {}} - ) - ] - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_auto_generated_operation_ids() - - # We expect warnings for auto-generated routes but not for explicit ones - # We should have exactly 2 warnings (for the 2 auto-generated routes) - assert mock_warning.call_count == 2 - - # Verify the warning messages - warnings = [call[0][0] for call in mock_warning.call_args_list] - - # The exact auto-generated names might vary based on FastAPI's implementation - # but should include the function names - assert any("auto_generated_route" in warning for warning in warnings) - assert any("auto_generated_route_get" in warning for warning in warnings) - assert not any("explicit_operation_id" in warning for warning in warnings) - assert all("To disable this warning" in warning for warning in warnings) + + return app -def test_warn_if_auto_generated_operation_ids_no_warning(): - """Test that no warnings are issued when all routes have explicit operation IDs.""" - # Create a simple app - app = FastAPI() +def test_warn_if_too_many_tools(app_with_too_many_tools, caplog): + """Test that a warning is issued when there are too many tools.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_too_many_tools) + + # Check that warning was logged + assert any("More than 10 tools exposed" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) + + +def test_warn_if_too_many_tools_no_warning(app_with_too_many_tools, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_too_many_tools, disable_warnings=True) + + # Check that no warning was logged + assert not any("More than 10 tools exposed" in record.message for record in caplog.records) + + +def test_warn_if_non_get_endpoints(app_with_non_get_endpoints, caplog): + """Test that a warning is issued when there are non-GET endpoints.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_non_get_endpoints) + + # Check that warning was logged + assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert any("create_item (POST)" in record.message for record in caplog.records) + assert any("update_item (PUT)" in record.message for record in caplog.records) + assert any("delete_item (DELETE)" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) + + +def test_warn_if_non_get_endpoints_no_warning(app_with_non_get_endpoints, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_non_get_endpoints, disable_warnings=True) - # Create FastApiMCP instance with disable_warnings=True to guarantee no warnings - mcp_server = FastApiMCP(app, disable_warnings=True) - - # Create tools with explicit operation IDs (doesn't matter for this test since we use disable_warnings) - mcp_server.tools = [ - types.Tool( - name="explicit_operation_id", - description="Explicit route", - inputSchema={"type": "object", "properties": {}} - ) - ] - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call the warning method - mcp_server._warn_if_auto_generated_operation_ids() - - # No warnings should be issued because we disabled warnings - mock_warning.assert_not_called() + # Check that no warning was logged + assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + +def test_warn_if_auto_generated_operation_ids(app_with_auto_generated_ids, caplog): + """Test that warnings are issued for auto-generated operation IDs.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_auto_generated_ids) + + # Check that warning was logged for auto-generated IDs but not for explicit ones + assert any("auto_generated_route" in record.message for record in caplog.records) + assert any("auto_generated_route_get" in record.message for record in caplog.records) + assert not any("explicit_operation_id" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) -def test_disable_all_warnings(): + +def test_warn_if_auto_generated_operation_ids_no_warning(app_with_auto_generated_ids, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_auto_generated_ids, disable_warnings=True) + + # Check that no warning was logged + assert not any("auto_generated_route" in record.message for record in caplog.records) + assert not any("auto_generated_route_get" in record.message for record in caplog.records) + + +def test_disable_all_warnings(app_with_too_many_tools, caplog): """Test that all warnings can be disabled via the disable_warnings parameter.""" - # Create a simple app - app = FastAPI() + # Set up logging capture + caplog.set_level(logging.WARNING) # Create FastApiMCP instance with warnings disabled - mcp_server = FastApiMCP(app, disable_warnings=True) - - # Setup data that would trigger warnings - mcp_server.tools = [ - types.Tool( - name=f"tool_{i}", - description="A test tool", - inputSchema={"type": "object", "properties": {}, "title": f"Tool{i}Arguments"} - ) for i in range(11) - ] - mcp_server.tools.append( - types.Tool( - name="items__get", - description="Double underscore", - inputSchema={"type": "object", "properties": {}, "title": "Items__getArguments"} - ) - ) - mcp_server.operation_map = { - "create_item": {"method": "post", "path": "/items"}, - "update_item": {"method": "put", "path": "/items/{id}"} - } - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Call all warning methods - mcp_server._warn_if_too_many_tools() - mcp_server._warn_if_non_get_endpoints() - mcp_server._warn_if_auto_generated_operation_ids() - - # Check that no warnings were issued - mock_warning.assert_not_called() + _ = FastApiMCP(app_with_too_many_tools, disable_warnings=True) + + # Check that no warnings were logged + assert not any("More than 10 tools exposed" in record.message for record in caplog.records) + assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert not any("auto_generated_route" in record.message for record in caplog.records) -def test_integration_all_warnings(): +def test_integration_all_warnings(caplog): """Test that all warnings are issued during server setup when needed.""" - # Create a FastAPI app with routes that trigger all warnings + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create a FastAPI app with all warning scenarios app = FastAPI() router = APIRouter() - # Create routes with auto-generated IDs and non-GET methods + # Auto-generated operation IDs @router.get("/items/") - async def items__get(): + async def get_items(): return {"items": []} + # Non-GET endpoint @router.post("/items/") - async def items__post(): + async def create_item(): return {"message": "Item created"} - + # Add enough routes to trigger the "too many tools" warning for i in range(10): @router.get(f"/other-route-{i}/") - async def other_route(): + async def other_route_get(): return {"message": "OK"} app.include_router(router) - # Replace the warning methods with mocks - with patch.object(FastApiMCP, '_warn_if_too_many_tools') as mock_too_many_tools, \ - patch.object(FastApiMCP, '_warn_if_non_get_endpoints') as mock_non_get_endpoints, \ - patch.object(FastApiMCP, '_warn_if_auto_generated_operation_ids') as mock_auto_gen_ids: - - # Create FastApiMCP instance which will trigger setup_server method - FastApiMCP(app) - - # Verify that each warning method was called once - mock_too_many_tools.assert_called_once() - mock_non_get_endpoints.assert_called_once() - mock_auto_gen_ids.assert_called_once() + # Create FastApiMCP instance + _ = FastApiMCP(app) + + # Check that all warnings were logged + assert any("More than 10 tools exposed" in record.message for record in caplog.records) + assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert any("create_item_items__post (POST)" in record.message for record in caplog.records) + assert any("appears to have an auto-generated operation_id" in record.message for record in caplog.records) -def test_integration_warnings_disabled(): +def test_integration_warnings_disabled(caplog): """Test that warnings are not issued during server setup when disable_warnings=True.""" - # Create a FastAPI app with routes that would normally trigger warnings - app = FastAPI() - router = APIRouter() - - # Create routes with auto-generated IDs and non-GET methods - @router.get("/items/") - async def items__get(): - return {"items": []} + # Set up logging capture + caplog.set_level(logging.WARNING) - @router.post("/items/") - async def items__post(): - return {"message": "Item created"} - - app.include_router(router) - - # Patch the logger.warning method - with patch("fastapi_mcp.server.logger.warning") as mock_warning: - # Create FastApiMCP instance with warnings disabled - FastApiMCP(app, disable_warnings=True) - - # Check that no warnings were issued - mock_warning.assert_not_called() - - -def test_integration_warning_methods_during_setup(): - """Test that the warning methods are called during server setup.""" - # Create a FastAPI app with minimum configuration to trigger warnings + # Create a FastAPI app with all warning scenarios app = FastAPI() + # Auto-generated operation ID @app.get("/items/") - async def items__get(): + async def get_items(): return {"items": []} + # Non-GET endpoint @app.post("/items/") - async def items__post(): + async def create_item(): return {"message": "Item created"} - # Create mocks for each warning method - too_many_spy = Mock() - non_get_spy = Mock() - auto_gen_spy = Mock() - - # Patch the methods to use our spies - with patch.object(FastApiMCP, '_warn_if_too_many_tools', too_many_spy), \ - patch.object(FastApiMCP, '_warn_if_non_get_endpoints', non_get_spy), \ - patch.object(FastApiMCP, '_warn_if_auto_generated_operation_ids', auto_gen_spy): - - # Create FastApiMCP instance which will trigger the setup_server method - FastApiMCP(app) - - # Verify that each method was called once - too_many_spy.assert_called_once() - non_get_spy.assert_called_once() - auto_gen_spy.assert_called_once() \ No newline at end of file + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app, disable_warnings=True) + + # Check that no warnings were logged + assert len(caplog.records) == 0 \ No newline at end of file