From 070e8412c096ab1208c20f57b1f8ede11ca115c4 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Mon, 27 Jan 2025 20:36:10 +0000 Subject: [PATCH] refactor: standardize resource response format Introduce ReadResourceContents type to properly handle MIME types in resource responses. Breaking change in FastMCP read_resource() return type. Github-Issue:#152 --- src/mcp/server/fastmcp/server.py | 7 +- src/mcp/server/lowlevel/helper_types.py | 9 ++ src/mcp/server/lowlevel/server.py | 40 +++---- tests/issues/test_152_resource_mime_type.py | 7 +- .../fastmcp/servers/test_file_server.py | 14 +-- tests/server/fastmcp/test_server.py | 4 +- tests/server/test_read_resource.py | 109 ++++++++++++++++++ 7 files changed, 155 insertions(+), 35 deletions(-) create mode 100644 src/mcp/server/lowlevel/helper_types.py create mode 100644 tests/server/test_read_resource.py diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index e935dd8..e8b311e 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -20,6 +20,7 @@ from mcp.server.fastmcp.tools import ToolManager from mcp.server.fastmcp.utilities.logging import configure_logging, get_logger from mcp.server.fastmcp.utilities.types import Image from mcp.server.lowlevel import Server as MCPServer +from mcp.server.lowlevel.helper_types import ReadResourceContents from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.shared.context import RequestContext @@ -197,7 +198,7 @@ class FastMCP: for template in templates ] - async def read_resource(self, uri: AnyUrl | str) -> tuple[str | bytes, str]: + async def read_resource(self, uri: AnyUrl | str) -> ReadResourceContents: """Read a resource by URI.""" resource = await self._resource_manager.get_resource(uri) @@ -206,7 +207,7 @@ class FastMCP: try: content = await resource.read() - return (content, resource.mime_type) + return ReadResourceContents(content=content, mime_type=resource.mime_type) except Exception as e: logger.error(f"Error reading resource {uri}: {e}") raise ResourceError(str(e)) @@ -608,7 +609,7 @@ class Context(BaseModel): progress_token=progress_token, progress=progress, total=total ) - async def read_resource(self, uri: str | AnyUrl) -> tuple[str | bytes, str]: + async def read_resource(self, uri: str | AnyUrl) -> ReadResourceContents: """Read a resource by URI. Args: diff --git a/src/mcp/server/lowlevel/helper_types.py b/src/mcp/server/lowlevel/helper_types.py new file mode 100644 index 0000000..3d09b25 --- /dev/null +++ b/src/mcp/server/lowlevel/helper_types.py @@ -0,0 +1,9 @@ +from dataclasses import dataclass + + +@dataclass +class ReadResourceContents: + """Contents returned from a read_resource call.""" + + content: str | bytes + mime_type: str | None = None diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 753590d..13d4fd9 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -74,6 +74,7 @@ from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStre from pydantic import AnyUrl import mcp.types as types +from mcp.server.lowlevel.helper_types import ReadResourceContents from mcp.server.models import InitializationOptions from mcp.server.session import ServerSession from mcp.server.stdio import stdio_server as stdio_server @@ -253,20 +254,20 @@ class Server: def read_resource(self): def decorator( - func: Callable[[AnyUrl], Awaitable[str | bytes | tuple[str | bytes, str]]], + func: Callable[[AnyUrl], Awaitable[str | bytes | ReadResourceContents]], ): logger.debug("Registering handler for ReadResourceRequest") async def handler(req: types.ReadResourceRequest): result = await func(req.params.uri) - def create_content(data: str | bytes, mime_type: str): + def create_content(data: str | bytes, mime_type: str | None): match data: case str() as data: return types.TextResourceContents( uri=req.params.uri, text=data, - mimeType=mime_type, + mimeType=mime_type or "text/plain", ) case bytes() as data: import base64 @@ -274,34 +275,31 @@ class Server: return types.BlobResourceContents( uri=req.params.uri, blob=base64.urlsafe_b64encode(data).decode(), - mimeType=mime_type, + mimeType=mime_type or "application/octet-stream", ) match result: case str() | bytes() as data: - default_mime = ( - "text/plain" - if isinstance(data, str) - else "application/octet-stream" - ) - content = create_content(data, default_mime) - return types.ServerResult( - types.ReadResourceResult( - contents=[content], - ) - ) - case (data, mime_type): - content = create_content(data, mime_type) - return types.ServerResult( - types.ReadResourceResult( - contents=[content], - ) + warnings.warn( + "Returning str or bytes from read_resource is deprecated. " + "Use ReadResourceContents instead.", + DeprecationWarning, + stacklevel=2, ) + content = create_content(data, None) + case ReadResourceContents() as contents: + content = create_content(contents.content, contents.mime_type) case _: raise ValueError( f"Unexpected return type from read_resource: {type(result)}" ) + return types.ServerResult( + types.ReadResourceResult( + contents=[content], + ) + ) + self.request_handlers[types.ReadResourceRequest] = handler return func diff --git a/tests/issues/test_152_resource_mime_type.py b/tests/issues/test_152_resource_mime_type.py index 0e655c8..7a1b660 100644 --- a/tests/issues/test_152_resource_mime_type.py +++ b/tests/issues/test_152_resource_mime_type.py @@ -6,6 +6,7 @@ from pydantic import AnyUrl from mcp import types from mcp.server.fastmcp import FastMCP from mcp.server.lowlevel import Server +from mcp.server.lowlevel.helper_types import ReadResourceContents from mcp.shared.memory import ( create_connected_server_and_client_session as client_session, ) @@ -98,9 +99,11 @@ async def test_lowlevel_resource_mime_type(): @server.read_resource() async def handle_read_resource(uri: AnyUrl): if str(uri) == "test://image": - return (base64_string, "image/png") + return ReadResourceContents(content=base64_string, mime_type="image/png") elif str(uri) == "test://image_bytes": - return (bytes(image_bytes), "image/png") + return ReadResourceContents( + content=bytes(image_bytes), mime_type="image/png" + ) raise Exception(f"Resource not found: {uri}") # Test that resources are listed with correct mime type diff --git a/tests/server/fastmcp/servers/test_file_server.py b/tests/server/fastmcp/servers/test_file_server.py index ccf2580..edaaa15 100644 --- a/tests/server/fastmcp/servers/test_file_server.py +++ b/tests/server/fastmcp/servers/test_file_server.py @@ -88,10 +88,10 @@ async def test_list_resources(mcp: FastMCP): @pytest.mark.anyio async def test_read_resource_dir(mcp: FastMCP): - files, mime_type = await mcp.read_resource("dir://test_dir") - assert mime_type == "text/plain" + res = await mcp.read_resource("dir://test_dir") + assert res.mime_type == "text/plain" - files = json.loads(files) + files = json.loads(res.content) assert sorted([Path(f).name for f in files]) == [ "config.json", @@ -102,8 +102,8 @@ async def test_read_resource_dir(mcp: FastMCP): @pytest.mark.anyio async def test_read_resource_file(mcp: FastMCP): - result, _ = await mcp.read_resource("file://test_dir/example.py") - assert result == "print('hello world')" + res = await mcp.read_resource("file://test_dir/example.py") + assert res.content == "print('hello world')" @pytest.mark.anyio @@ -119,5 +119,5 @@ async def test_delete_file_and_check_resources(mcp: FastMCP, test_dir: Path): await mcp.call_tool( "delete_file", arguments=dict(path=str(test_dir / "example.py")) ) - result, _ = await mcp.read_resource("file://test_dir/example.py") - assert result == "File not found" + res = await mcp.read_resource("file://test_dir/example.py") + assert res.content == "File not found" diff --git a/tests/server/fastmcp/test_server.py b/tests/server/fastmcp/test_server.py index 61f122c..d90e993 100644 --- a/tests/server/fastmcp/test_server.py +++ b/tests/server/fastmcp/test_server.py @@ -581,8 +581,8 @@ class TestContextInjection: @mcp.tool() async def tool_with_resource(ctx: Context) -> str: - data, mime_type = await ctx.read_resource("test://data") - return f"Read resource: {data} with mime type {mime_type}" + r = await ctx.read_resource("test://data") + return f"Read resource: {r.content} with mime type {r.mime_type}" async with client_session(mcp._mcp_server) as client: result = await client.call_tool("tool_with_resource", {}) diff --git a/tests/server/test_read_resource.py b/tests/server/test_read_resource.py new file mode 100644 index 0000000..de00bc3 --- /dev/null +++ b/tests/server/test_read_resource.py @@ -0,0 +1,109 @@ +from pathlib import Path +from tempfile import NamedTemporaryFile + +import pytest +from pydantic import AnyUrl, FileUrl + +import mcp.types as types +from mcp.server.lowlevel.server import ReadResourceContents, Server + + +@pytest.fixture +def temp_file(): + """Create a temporary file for testing.""" + with NamedTemporaryFile(mode="w", delete=False) as f: + f.write("test content") + path = Path(f.name).resolve() + yield path + try: + path.unlink() + except FileNotFoundError: + pass + + +@pytest.mark.anyio +async def test_read_resource_text(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents(content="Hello World", mime_type="text/plain") + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.TextResourceContents) + assert content.text == "Hello World" + assert content.mimeType == "text/plain" + + +@pytest.mark.anyio +async def test_read_resource_binary(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents( + content=b"Hello World", mime_type="application/octet-stream" + ) + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.BlobResourceContents) + assert content.mimeType == "application/octet-stream" + + +@pytest.mark.anyio +async def test_read_resource_default_mime(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents( + content="Hello World", + # No mime_type specified, should default to text/plain + ) + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.TextResourceContents) + assert content.text == "Hello World" + assert content.mimeType == "text/plain"