From 4d4d26532646316a9ff80fab31dc3881f4b2b52d Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Fri, 29 Nov 2024 18:21:51 -0500 Subject: [PATCH] Update resource URI handling --- src/fastmcp/resources.py | 24 +++++--- src/fastmcp/server.py | 103 +++++---------------------------- tests/test_resource_manager.py | 43 +++++++++----- tests/test_server.py | 37 ++++++++++-- 4 files changed, 91 insertions(+), 116 deletions(-) diff --git a/src/fastmcp/resources.py b/src/fastmcp/resources.py index 2987cf8ea..f0f3f1b07 100644 --- a/src/fastmcp/resources.py +++ b/src/fastmcp/resources.py @@ -23,6 +23,22 @@ class Resource(BaseModel): description: Optional[str] = None mime_type: str = "text/plain" + @field_validator("uri") + @classmethod + def validate_uri_format(cls, uri: str) -> str: + """Validate URI follows [protocol]://[host]/[path] format.""" + parsed = urlparse(uri) + + # Check protocol exists and is not empty + if not parsed.scheme: + raise ValueError("URI must have a protocol (e.g., 'http://', 'file://')") + + # Check host exists and is not empty + if not parsed.netloc and not parsed.path: + raise ValueError("URI must have a host or path") + + return uri + @abc.abstractmethod async def read(self) -> str: """Read the resource content.""" @@ -39,14 +55,6 @@ class FunctionResource(Resource): func: Callable[..., Any] - @field_validator("uri") - @classmethod - def validate_uri(cls, uri: str) -> str: - """Ensure URI starts with fn://.""" - if not uri.startswith("fn://"): - raise ValueError(f"URI must start with fn://: {uri}") - return uri - def _parse_uri_params(self) -> Dict[str, str]: """Parse URI query string into kwargs.""" parsed = urlparse(self.uri) diff --git a/src/fastmcp/server.py b/src/fastmcp/server.py index 8fb4758f3..c7f5f629f 100644 --- a/src/fastmcp/server.py +++ b/src/fastmcp/server.py @@ -3,7 +3,7 @@ import base64 import functools import json -from typing import Any, Callable, Dict, Optional, Sequence, Union, Literal +from typing import Any, Callable, Optional, Sequence, Union, Literal from mcp.server import Server as MCPServer from mcp.server.stdio import stdio_server @@ -147,7 +147,12 @@ class FastMCP: self, name: Optional[str] = None, description: Optional[str] = None ) -> Callable: """Decorator to register a tool.""" - breakpoint() + # Check if user passed function directly instead of calling decorator + if callable(name): + raise TypeError( + "The @tool decorator was used incorrectly. " + "Did you forget to call it? Use @tool() instead of @tool" + ) def decorator(func: Callable) -> Callable: self.add_tool(func, name=name, description=description) @@ -163,94 +168,6 @@ class FastMCP: """ self._resource_manager.add_resource(resource) - def add_file_resource( - self, - path: str, - *, - name: Optional[str] = None, - description: Optional[str] = None, - mime_type: Optional[str] = None, - ) -> None: - """Add a file as a resource. - - This is a convenience method that constructs and adds a FileResource. - For more control, use add_resource() directly. - """ - from pathlib import Path - from .resources import FileResource - - file = Path(path) - if not file.is_absolute(): - raise ValueError(f"Path must be absolute: {path}") - if not file.is_file(): - raise FileNotFoundError(f"File does not exist: {path}") - - resource = FileResource( - uri=f"file://{str(file)}", - name=name or file.name, - description=description, - mime_type=mime_type or "text/plain", - path=file, - ) - self.add_resource(resource) - - def add_http_resource( - self, - url: str, - *, - name: Optional[str] = None, - description: Optional[str] = None, - mime_type: Optional[str] = None, - headers: Optional[Dict[str, str]] = None, - ) -> None: - """Add an HTTP endpoint as a resource. - - This is a convenience method that constructs and adds an HttpResource. - For more control, use add_resource() directly. - """ - from .resources import HttpResource - - resource = HttpResource( - uri=f"http://{url}", - name=name or url.split("/")[-1], - description=description, - mime_type=mime_type or "text/plain", - url=url, - headers=headers, - ) - self.add_resource(resource) - - def add_dir_resource( - self, - path: str, - *, - recursive: bool = False, - pattern: Optional[str] = None, - name: Optional[str] = None, - description: Optional[str] = None, - ) -> None: - """Add a directory as a resource. - - This is a convenience method that constructs and adds a DirectoryResource. - For more control, use add_resource() directly. - """ - from pathlib import Path - from .resources import DirectoryResource - - dir_path = Path(path).expanduser().resolve() - if not dir_path.is_dir(): - raise ValueError(f"Directory does not exist: {path}") - - resource = DirectoryResource( - uri=f"dir://{str(dir_path)}", - name=name or dir_path.name, - description=description, - path=dir_path, - recursive=recursive, - pattern=pattern, - ) - self.add_resource(resource) - def resource( self, name: str, @@ -275,6 +192,12 @@ class FastMCP: # Called with fn://my_func?x=1&y=2 return f"x={x}, y={y}" """ + # Check if user passed function directly instead of calling decorator + if callable(name): + raise TypeError( + "The @resource decorator was used incorrectly. " + "Did you forget to call it? Use @resource('name') instead of @resource" + ) def decorator(func: Callable) -> Callable: @functools.wraps(func) diff --git a/tests/test_resource_manager.py b/tests/test_resource_manager.py index 4ef673c35..360fa5db2 100644 --- a/tests/test_resource_manager.py +++ b/tests/test_resource_manager.py @@ -43,6 +43,36 @@ def temp_dir(): yield Path(d).resolve() +class TestResourceValidation: + def test_resource_uri_validation(self): + def dummy_func() -> str: + return "data" + + # Valid URI + resource = FunctionResource( + uri="http://example.com/data", + name="test", + func=dummy_func, + ) + assert resource.uri == "http://example.com/data" + + # Missing protocol + with pytest.raises(ValueError, match="URI must have a protocol"): + FunctionResource( + uri="invalid", + name="test", + func=dummy_func, + ) + + # Missing host + with pytest.raises(ValueError, match="URI must have a host"): + FunctionResource( + uri="http://", + name="test", + func=dummy_func, + ) + + class TestFileResource: """Test FileResource functionality.""" @@ -138,19 +168,6 @@ class TestFunctionResource: assert resource.mime_type == "text/plain" assert resource.func == my_func - def test_function_resource_invalid_uri(self): - """Test FunctionResource rejects invalid URIs.""" - - def my_func() -> str: - return "test" - - with pytest.raises(ValueError, match="URI must start with fn://"): - FunctionResource( - uri="invalid://test", - name="test", - func=my_func, - ) - async def test_function_resource_read_no_params(self): """Test reading a FunctionResource with no parameters.""" diff --git a/tests/test_server.py b/tests/test_server.py index 90c291068..fb82efd36 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2,6 +2,7 @@ from mcp.shared.memory import ( create_connected_server_and_client_session as client_session, ) from fastmcp import FastMCP +import pytest class TestServer: @@ -12,14 +13,40 @@ class TestServer: async def test_add_tool_decorator(self): mcp = FastMCP() - @mcp.tool + @mcp.tool() def add(x: int, y: int) -> int: return x + y - async with client_session(mcp._mcp_server) as client: - tools = await client.list_tools() - assert len(tools.tools) == 1 - assert tools.tools[0].name == "add" + assert len(mcp._tool_manager.list_tools()) == 1 + + async def test_add_tool_decorator_incorrect_usage(self): + mcp = FastMCP() + + with pytest.raises(TypeError, match="The @tool decorator was used incorrectly"): + + @mcp.tool # Missing parentheses + def add(x: int, y: int) -> int: + return x + y + + async def test_add_resource_decorator(self): + mcp = FastMCP() + + @mcp.resource("data") + def get_data(x: str) -> str: + return f"Data: {x}" + + assert len(mcp._resource_manager.list_resources()) == 1 + + async def test_add_resource_decorator_incorrect_usage(self): + mcp = FastMCP() + + with pytest.raises( + TypeError, match="The @resource decorator was used incorrectly" + ): + + @mcp.resource # Missing parentheses + def get_data(x: str) -> str: + return f"Data: {x}" def tool_fn(x: int, y: int) -> int: