From cba8288646bd3a3c32ba61ecdf0e46ea63a4a754 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Thu, 18 Dec 2025 12:09:16 -0500 Subject: [PATCH 1/3] Simplify .key as computed property Keep .key as the standard lookup interface for all components but implement it as a computed property instead of a stored field. - Remove _key private attribute and custom model_copy(key=...) - .key returns .name for tools/prompts, str(.uri) for resources, .uri_template for templates - Use .key universally for component lookups in managers - MountedProvider: prefix URIs only for resources/templates, not names - Docket registration: tools/prompts use .key, resources use .name (matches fn.__name__ for function lookup) --- src/fastmcp/resources/resource.py | 9 +- src/fastmcp/resources/resource_manager.py | 10 +- src/fastmcp/resources/template.py | 9 +- src/fastmcp/server/providers/mounted.py | 44 ++++---- src/fastmcp/server/proxy.py | 56 ++++++++++- src/fastmcp/server/server.py | 25 +++-- src/fastmcp/server/tasks/handlers.py | 6 +- src/fastmcp/utilities/components.py | 40 +------- src/fastmcp/utilities/inspect.py | 10 +- tests/resources/test_resource_manager.py | 117 ---------------------- tests/server/test_import_server.py | 2 +- tests/server/test_mount.py | 20 ++-- tests/tools/test_tool_manager.py | 15 ++- tests/utilities/test_components.py | 74 +++++++------- 14 files changed, 158 insertions(+), 279 deletions(-) diff --git a/src/fastmcp/resources/resource.py b/src/fastmcp/resources/resource.py index c882545249..21d03b8d98 100644 --- a/src/fastmcp/resources/resource.py +++ b/src/fastmcp/resources/resource.py @@ -274,13 +274,8 @@ def __repr__(self) -> str: @property def key(self) -> str: - """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or str(self.uri) + """The lookup key for this resource. Returns str(uri).""" + return str(self.uri) class FunctionResource(Resource): diff --git a/src/fastmcp/resources/resource_manager.py b/src/fastmcp/resources/resource_manager.py index 290fe8484f..0fdc73cac6 100644 --- a/src/fastmcp/resources/resource_manager.py +++ b/src/fastmcp/resources/resource_manager.py @@ -153,9 +153,8 @@ def add_resource(self, resource: Resource) -> Resource: """Add a resource to the manager. Args: - resource: A Resource instance to add. The resource's .key attribute - will be used as the storage key. To overwrite it, call - Resource.model_copy(key=new_key) before calling this method. + resource: A Resource instance to add. The resource's key (URI) + will be used as the storage key. """ existing = self._resources.get(resource.key) if existing: @@ -202,9 +201,8 @@ def add_template(self, template: ResourceTemplate) -> ResourceTemplate: """Add a template to the manager. Args: - template: A ResourceTemplate instance to add. The template's .key attribute - will be used as the storage key. To overwrite it, call - ResourceTemplate.model_copy(key=new_key) before calling this method. + template: A ResourceTemplate instance to add. The template's key (uri_template) + will be used as the storage key. Returns: The added template. If a template with the same URI already exists, diff --git a/src/fastmcp/resources/template.py b/src/fastmcp/resources/template.py index 961da0e571..ecf9824f0b 100644 --- a/src/fastmcp/resources/template.py +++ b/src/fastmcp/resources/template.py @@ -219,13 +219,8 @@ def from_mcp_template(cls, mcp_template: SDKResourceTemplate) -> ResourceTemplat @property def key(self) -> str: - """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or self.uri_template + """The lookup key for this template. Returns uri_template.""" + return self.uri_template class FunctionResourceTemplate(ResourceTemplate): diff --git a/src/fastmcp/server/providers/mounted.py b/src/fastmcp/server/providers/mounted.py index 490e8fddd9..2fb025a468 100644 --- a/src/fastmcp/server/providers/mounted.py +++ b/src/fastmcp/server/providers/mounted.py @@ -181,37 +181,35 @@ def _strip_resource_prefix(self, uri: str) -> str | None: def _prefix_tool(self, tool: Tool) -> Tool: """Apply prefix to a tool.""" if self.tool_names and tool.name in self.tool_names: - new_key = self.tool_names[tool.name] + new_name = self.tool_names[tool.name] else: - new_key = self._add_tool_prefix(tool.key) - return tool.model_copy(key=new_key) if new_key != tool.key else tool + new_name = self._add_tool_prefix(tool.name) + if new_name != tool.name: + return tool.model_copy(update={"name": new_name}) + return tool def _prefix_resource(self, resource: Resource) -> Resource: - """Apply prefix to a resource.""" - new_key = self._add_resource_prefix(resource.key) - update: dict[str, Any] = {} - if self.prefix and resource.name: - update["name"] = f"{self.prefix}_{resource.name}" - if new_key != resource.key or update: - return resource.model_copy(key=new_key, update=update) + """Apply prefix to a resource URI (name is NOT prefixed).""" + if self.prefix: + new_uri = self._add_resource_prefix(str(resource.uri)) + if new_uri != str(resource.uri): + return resource.model_copy(update={"uri": new_uri}) return resource def _prefix_template(self, template: ResourceTemplate) -> ResourceTemplate: - """Apply prefix to a resource template.""" - new_key = self._add_resource_prefix(template.key) - update: dict[str, Any] = {} - if self.prefix and template.name: - update["name"] = f"{self.prefix}_{template.name}" + """Apply prefix to a resource template URI (name is NOT prefixed).""" if self.prefix and template.uri_template: - update["uri_template"] = self._add_resource_prefix(template.uri_template) - if new_key != template.key or update: - return template.model_copy(key=new_key, update=update) + new_template = self._add_resource_prefix(template.uri_template) + if new_template != template.uri_template: + return template.model_copy(update={"uri_template": new_template}) return template def _prefix_prompt(self, prompt: Prompt) -> Prompt: """Apply prefix to a prompt.""" - new_key = self._add_tool_prefix(prompt.key) - return prompt.model_copy(key=new_key) if new_key != prompt.key else prompt + new_name = self._add_tool_prefix(prompt.name) + if new_name != prompt.name: + return prompt.model_copy(update={"name": new_name}) + return prompt # ------------------------------------------------------------------------- # Tool methods @@ -223,7 +221,7 @@ async def list_tools(self) -> Sequence[Tool]: return [self._prefix_tool(tool) for tool in tools] async def get_tool(self, name: str) -> Tool | None: - """Get a tool by name, going through middleware.""" + """Get a tool by key, going through middleware.""" # Early exit if name doesn't match our prefix pattern if self._strip_tool_prefix(name) is None: return None @@ -250,7 +248,7 @@ async def list_resources(self) -> Sequence[Resource]: return [self._prefix_resource(resource) for resource in resources] async def get_resource(self, uri: str) -> Resource | None: - """Get a concrete resource by URI, going through middleware. + """Get a concrete resource by key (URI), going through middleware. Only returns concrete resources, not resources created from templates. This preserves the original template for task execution. @@ -313,7 +311,7 @@ async def list_prompts(self) -> Sequence[Prompt]: return [self._prefix_prompt(prompt) for prompt in prompts] async def get_prompt(self, name: str) -> Prompt | None: - """Get a prompt by name, going through middleware.""" + """Get a prompt by key, going through middleware.""" # Early exit if name doesn't match our prefix pattern if self._strip_tool_prefix(name) is None: return None diff --git a/src/fastmcp/server/proxy.py b/src/fastmcp/server/proxy.py index cfe70a6fe2..2a4ae1136d 100644 --- a/src/fastmcp/server/proxy.py +++ b/src/fastmcp/server/proxy.py @@ -280,11 +280,21 @@ class ProxyTool(Tool, MirroredComponent): """ task_config: TaskConfig = TaskConfig(mode="forbidden") + _backend_name: str | None = None def __init__(self, client: Client, **kwargs: Any): super().__init__(**kwargs) self._client = client + def model_copy(self, **kwargs: Any) -> ProxyTool: + """Override to preserve _backend_name when name changes.""" + update = kwargs.get("update", {}) + if "name" in update and self._backend_name is None: + # First time name is being changed, preserve original for backend calls + update = {**update, "_backend_name": self.name} + kwargs["update"] = update + return super().model_copy(**kwargs) # type: ignore[return-value] + @classmethod def from_mcp_tool(cls, client: Client, mcp_tool: mcp.types.Tool) -> ProxyTool: """Factory method to create a ProxyTool from a raw MCP tool schema.""" @@ -331,7 +341,7 @@ async def run( ) result = await self._client.call_tool_mcp( - name=self.name, arguments=arguments, meta=meta + name=self._backend_name or self.name, arguments=arguments, meta=meta ) if result.isError: raise ToolError(cast(mcp.types.TextContent, result.content[0]).text) @@ -351,6 +361,7 @@ class ProxyResource(Resource, MirroredComponent): task_config: TaskConfig = TaskConfig(mode="forbidden") _client: Client _cached_content: ResourceContent | None = None + _backend_uri: str | None = None def __init__( self, @@ -363,6 +374,15 @@ def __init__( self._client = client self._cached_content = _cached_content + def model_copy(self, **kwargs: Any) -> ProxyResource: + """Override to preserve _backend_uri when uri changes.""" + update = kwargs.get("update", {}) + if "uri" in update and self._backend_uri is None: + # First time uri is being changed, preserve original for backend calls + update = {**update, "_backend_uri": str(self.uri)} + kwargs["update"] = update + return super().model_copy(**kwargs) # type: ignore[return-value] + @classmethod def from_mcp_resource( cls, @@ -390,10 +410,13 @@ async def read(self) -> ResourceContent: if self._cached_content is not None: return self._cached_content + backend_uri = self._backend_uri or str(self.uri) async with self._client: - result = await self._client.read_resource(self.uri) + result = await self._client.read_resource(backend_uri) if not result: - raise ResourceError(f"Remote server returned empty content for {self.uri}") + raise ResourceError( + f"Remote server returned empty content for {backend_uri}" + ) if isinstance(result[0], TextResourceContents): return ResourceContent( content=result[0].text, @@ -416,11 +439,21 @@ class ProxyTemplate(ResourceTemplate, MirroredComponent): """ task_config: TaskConfig = TaskConfig(mode="forbidden") + _backend_uri_template: str | None = None def __init__(self, client: Client, **kwargs: Any): super().__init__(**kwargs) self._client = client + def model_copy(self, **kwargs: Any) -> ProxyTemplate: + """Override to preserve _backend_uri_template when uri_template changes.""" + update = kwargs.get("update", {}) + if "uri_template" in update and self._backend_uri_template is None: + # First time uri_template is being changed, preserve original for backend + update = {**update, "_backend_uri_template": self.uri_template} + kwargs["update"] = update + return super().model_copy(**kwargs) # type: ignore[return-value] + @classmethod def from_mcp_template( # type: ignore[override] cls, client: Client, mcp_template: mcp.types.ResourceTemplate @@ -451,7 +484,8 @@ async def create_resource( # don't use the provided uri, because it may not be the same as the # uri_template on the remote server. # quote params to ensure they are valid for the uri_template - parameterized_uri = self.uri_template.format( + backend_template = self._backend_uri_template or self.uri_template + parameterized_uri = backend_template.format( **{k: quote(v, safe="") for k, v in params.items()} ) async with self._client: @@ -497,11 +531,21 @@ class ProxyPrompt(Prompt, MirroredComponent): task_config: TaskConfig = TaskConfig(mode="forbidden") _client: Client + _backend_name: str | None = None def __init__(self, client: Client, **kwargs): super().__init__(**kwargs) self._client = client + def model_copy(self, **kwargs: Any) -> ProxyPrompt: + """Override to preserve _backend_name when name changes.""" + update = kwargs.get("update", {}) + if "name" in update and self._backend_name is None: + # First time name is being changed, preserve original for backend calls + update = {**update, "_backend_name": self.name} + kwargs["update"] = update + return super().model_copy(**kwargs) # type: ignore[return-value] + @classmethod def from_mcp_prompt( cls, client: Client, mcp_prompt: mcp.types.Prompt @@ -531,7 +575,9 @@ def from_mcp_prompt( async def render(self, arguments: dict[str, Any]) -> PromptResult: # type: ignore[override] """Render the prompt by making a call through the client.""" async with self._client: - result = await self._client.get_prompt(self.name, arguments) + result = await self._client.get_prompt( + self._backend_name or self.name, arguments + ) # Convert GetPromptResult to PromptResult, preserving runtime meta from the result # (not the static prompt meta which includes fastmcp tags) return PromptResult( diff --git a/src/fastmcp/server/server.py b/src/fastmcp/server/server.py index f77d43435a..394be893b2 100644 --- a/src/fastmcp/server/server.py +++ b/src/fastmcp/server/server.py @@ -473,11 +473,13 @@ async def _docket_lifespan(self) -> AsyncIterator[None]: named_fn = _create_named_fn_wrapper(tool.fn, tool.key) docket.register(named_fn) for resource in tasks.resources: + # Resources use .name for Docket (matches fn.__name__) named_fn = _create_named_fn_wrapper( resource.fn, resource.name ) docket.register(named_fn) for template in tasks.templates: + # Templates use .name for Docket (matches fn.__name__) named_fn = _create_named_fn_wrapper( template.fn, template.name ) @@ -1146,7 +1148,7 @@ async def _list_tools_mcp(self) -> list[SDKTool]: tools = await self._list_tools_middleware() return [ tool.to_mcp_tool( - name=tool.key, + name=tool.name, include_fastmcp_meta=self.include_fastmcp_meta, ) for tool in tools @@ -1222,7 +1224,7 @@ async def _list_resources_mcp(self) -> list[SDKResource]: resources = await self._list_resources_middleware() return [ resource.to_mcp_resource( - uri=resource.key, + uri=str(resource.uri), include_fastmcp_meta=self.include_fastmcp_meta, ) for resource in resources @@ -1300,7 +1302,7 @@ async def _list_resource_templates_mcp(self) -> list[SDKResourceTemplate]: templates = await self._list_resource_templates_middleware() return [ template.to_mcp_template( - uriTemplate=template.key, + uriTemplate=template.uri_template, include_fastmcp_meta=self.include_fastmcp_meta, ) for template in templates @@ -1377,7 +1379,7 @@ async def _list_prompts_mcp(self) -> list[SDKPrompt]: prompts = await self._list_prompts_middleware() return [ prompt.to_mcp_prompt( - name=prompt.key, + name=prompt.name, include_fastmcp_meta=self.include_fastmcp_meta, ) for prompt in prompts @@ -2776,30 +2778,33 @@ async def import_server( # Import tools from the server for key, tool in (await server.get_tools()).items(): if prefix: - tool = tool.model_copy(key=f"{prefix}_{key}") + tool = tool.model_copy(update={"name": f"{prefix}_{key}"}) self._tool_manager.add_tool(tool) # Import resources and templates from the server for key, resource in (await server.get_resources()).items(): if prefix: - resource_key = add_resource_prefix(key, prefix) + resource_uri = add_resource_prefix(key, prefix) resource = resource.model_copy( - update={"name": f"{prefix}_{resource.name}"}, key=resource_key + update={"name": f"{prefix}_{resource.name}", "uri": resource_uri} ) self._resource_manager.add_resource(resource) for key, template in (await server.get_resource_templates()).items(): if prefix: - template_key = add_resource_prefix(key, prefix) + template_uri_template = add_resource_prefix(key, prefix) template = template.model_copy( - update={"name": f"{prefix}_{template.name}"}, key=template_key + update={ + "name": f"{prefix}_{template.name}", + "uri_template": template_uri_template, + } ) self._resource_manager.add_template(template) # Import prompts from the server for key, prompt in (await server.get_prompts()).items(): if prefix: - prompt = prompt.model_copy(key=f"{prefix}_{key}") + prompt = prompt.model_copy(update={"name": f"{prefix}_{key}"}) self._prompt_manager.add_prompt(prompt) if server._lifespan != default_lifespan: diff --git a/src/fastmcp/server/tasks/handlers.py b/src/fastmcp/server/tasks/handlers.py index 5693304497..ae4182a19d 100644 --- a/src/fastmcp/server/tasks/handlers.py +++ b/src/fastmcp/server/tasks/handlers.py @@ -99,7 +99,7 @@ async def handle_tool_as_task( # Don't let notification failures break task creation await ctx.session.send_notification(notification) # type: ignore[arg-type] - # Queue function to Docket by name (result storage via execution_ttl) + # Queue function to Docket by key (result storage via execution_ttl) # Use tool.key which matches what was registered - prefixed for mounted tools await docket.add( tool.key, @@ -205,7 +205,7 @@ async def handle_prompt_as_task( with suppress(Exception): await ctx.session.send_notification(notification) # type: ignore[arg-type] - # Queue function to Docket by name (result storage via execution_ttl) + # Queue function to Docket by key (result storage via execution_ttl) # Use prompt.key which matches what was registered - prefixed for mounted prompts await docket.add( prompt.key, @@ -310,7 +310,7 @@ async def handle_resource_as_task( await ctx.session.send_notification(notification) # type: ignore[arg-type] # Queue function to Docket by name (result storage via execution_ttl) - # Use resource.name which matches what was registered - prefixed for mounted resources + # Use resource.name which matches what was registered - Docket uses fn.__name__ # For templates, extract URI params and pass them to the function from fastmcp.resources.template import FunctionResourceTemplate, match_uri_template diff --git a/src/fastmcp/utilities/components.py b/src/fastmcp/utilities/components.py index 6d2f0f5b41..56500650ca 100644 --- a/src/fastmcp/utilities/components.py +++ b/src/fastmcp/utilities/components.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from typing import Annotated, Any, TypedDict, cast +from typing import Annotated, Any, TypedDict from mcp.types import Icon from pydantic import BeforeValidator, Field, PrivateAttr @@ -56,21 +56,10 @@ class FastMCPComponent(FastMCPBaseModel): description="Whether the component is enabled.", ) - _key: str | None = PrivateAttr() - - def __init__(self, *, key: str | None = None, **kwargs: Any) -> None: - super().__init__(**kwargs) - self._key = key - @property def key(self) -> str: - """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or self.name + """The lookup key for this component. Returns name by default.""" + return self.name def get_meta( self, include_fastmcp_meta: bool | None = None @@ -96,29 +85,6 @@ def get_meta( return meta or None - def model_copy( # type: ignore[override] - self, - *, - update: dict[str, Any] | None = None, - deep: bool = False, - key: str | None = None, - ) -> Self: - """ - Create a copy of the component. - - Args: - update: A dictionary of fields to update. - deep: Whether to deep copy the component. - key: The key to use for the copy. - """ - # `model_copy` has an `update` parameter but it doesn't work for certain private attributes - # https://github.com/pydantic/pydantic/issues/12116 - # So we manually set the private attribute here instead, such as _key - copy = super().model_copy(update=update, deep=deep) - if key is not None: - copy._key = key - return cast(Self, copy) - def __eq__(self, other: object) -> bool: if type(self) is not type(other): return False diff --git a/src/fastmcp/utilities/inspect.py b/src/fastmcp/utilities/inspect.py index b4563090d3..77e21a9d1e 100644 --- a/src/fastmcp/utilities/inspect.py +++ b/src/fastmcp/utilities/inspect.py @@ -119,11 +119,11 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: # Extract detailed tool information tool_infos = [] for tool in tools_list: - mcp_tool = tool.to_mcp_tool(name=tool.key) + mcp_tool = tool.to_mcp_tool(name=tool.name) tool_infos.append( ToolInfo( key=tool.key, - name=tool.name or tool.key, + name=tool.name, description=tool.description, input_schema=mcp_tool.inputSchema if mcp_tool.inputSchema else {}, output_schema=tool.output_schema, @@ -144,7 +144,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: prompt_infos.append( PromptInfo( key=prompt.key, - name=prompt.name or prompt.key, + name=prompt.name, description=prompt.description, arguments=[arg.model_dump() for arg in prompt.arguments] if prompt.arguments @@ -165,7 +165,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: resource_infos.append( ResourceInfo( key=resource.key, - uri=resource.key, + uri=str(resource.uri), name=resource.name, description=resource.description, mime_type=resource.mime_type, @@ -188,7 +188,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: template_infos.append( TemplateInfo( key=template.key, - uri_template=template.key, + uri_template=template.uri_template, name=template.name, description=template.description, mime_type=template.mime_type, diff --git a/tests/resources/test_resource_manager.py b/tests/resources/test_resource_manager.py index ecbf10cfd8..50d72018d2 100644 --- a/tests/resources/test_resource_manager.py +++ b/tests/resources/test_resource_manager.py @@ -450,123 +450,6 @@ async def get_system_data(): assert admin_resources[0].name == "system_data" -class TestCustomResourceKeys: - """Test adding resources and templates with custom keys.""" - - def test_add_resource_with_custom_key(self, temp_file: Path): - """Test adding a resource with a custom key different from its URI.""" - manager = ResourceManager() - original_uri = "data://test/resource" - custom_key = "custom://resource/key" - - # Create a function resource instead of file resource to avoid path issues - async def get_data(): - return "Test data" - - resource = FunctionResource( - uri=AnyUrl(original_uri), - name="test_resource", - fn=get_data, - ) - - # Use model_copy to create a new resource with the custom key - resource_with_custom_key = resource.model_copy(key=custom_key) - manager.add_resource(resource_with_custom_key) - - # Resource should be accessible via custom key - assert custom_key in manager._resources - # But not via its original URI - assert original_uri not in manager._resources - # The resource's internal URI remains unchanged - assert str(manager._resources[custom_key].uri) == original_uri - - def test_add_template_with_custom_key(self): - """Test adding a template with a custom key different from its URI template.""" - manager = ResourceManager() - - def template_fn(id: str) -> str: - return f"Template {id}" - - original_uri_template = "test://{id}" - custom_key = "custom://{id}/template" - - template = ResourceTemplate.from_function( - fn=template_fn, - uri_template=original_uri_template, - name="test_template", - ) - - # Use model_copy to create a new template with the custom key - template_with_custom_key = template.model_copy(key=custom_key) - manager.add_template(template_with_custom_key) - - # Template should be accessible via custom key - assert custom_key in manager._templates - # But not via its original URI template - assert original_uri_template not in manager._templates - # The template's internal URI template remains unchanged - assert str(manager._templates[custom_key].uri_template) == original_uri_template - - async def test_get_resource_with_custom_key(self, temp_file: Path): - """Test that get_resource works with resources added with custom keys.""" - manager = ResourceManager() - original_uri = "data://test/resource" - custom_key = "custom://resource/path" - - # Create a function resource instead of file resource to avoid path issues - async def get_data(): - return "Test data" - - resource = FunctionResource( - uri=AnyUrl(original_uri), - name="test_resource", - fn=get_data, - ) - - # Use model_copy to create a new resource with the custom key - resource_with_custom_key = resource.model_copy(key=custom_key) - manager.add_resource(resource_with_custom_key) - - # Should be retrievable by the custom key - retrieved = await manager.get_resource(custom_key) - assert retrieved is not None - assert str(retrieved.uri) == original_uri - - # Should NOT be retrievable by the original URI - with pytest.raises(NotFoundError, match="Unknown resource"): - await manager.get_resource(original_uri) - - async def test_get_resource_from_template_with_custom_key(self): - """Test that templates with custom keys can create resources.""" - manager = ResourceManager() - - def greet(name: str) -> str: - return f"Hello, {name}!" - - original_template = "greet://{name}" - custom_key = "custom://greet/{name}" - - template = ResourceTemplate.from_function( - fn=greet, - uri_template=original_template, - name="custom_greeter", - ) - - # Use model_copy to create a new template with the custom key - template_with_custom_key = template.model_copy(key=custom_key) - manager.add_template(template_with_custom_key) - - # Using a URI that matches the custom key pattern - resource = await manager.get_resource("custom://greet/world") - assert isinstance(resource, FunctionResource) - result = await resource.read() - assert result.content == "Hello, world!" - - # Shouldn't work with the original template pattern - with pytest.raises(NotFoundError, match="Unknown resource"): - await manager.get_resource("greet://world") - - class TestQueryOnlyTemplates: """Test resource templates with only query parameters (no path params).""" diff --git a/tests/server/test_import_server.py b/tests/server/test_import_server.py index 446dbfcc94..1b48c76fb4 100644 --- a/tests/server/test_import_server.py +++ b/tests/server/test_import_server.py @@ -28,7 +28,7 @@ def sub_tool() -> str: # Verify the original tool still exists in the sub-app tool = await main_app._tool_manager.get_tool("sub_sub_tool") assert tool is not None - assert tool.name == "sub_tool" + assert tool.name == "sub_sub_tool" # name is now prefixed assert isinstance(tool, FunctionTool) assert callable(tool.fn) diff --git a/tests/server/test_mount.py b/tests/server/test_mount.py index 7026429a5c..141277b64e 100644 --- a/tests/server/test_mount.py +++ b/tests/server/test_mount.py @@ -970,11 +970,11 @@ def hello(): assert lifespan_check == ["start"] -class TestResourceNamePrefixing: - """Test that resource and resource template names get prefixed when mounted.""" +class TestResourceKeyPrefixing: + """Test that resource and resource template keys (URIs) get prefixed when mounted.""" - async def test_resource_name_prefixing(self): - """Test that resource names are prefixed when mounted.""" + async def test_resource_uri_prefixing(self): + """Test that resource URIs (keys) are prefixed when mounted, but names are not.""" # Create a sub-app with a resource sub_app = FastMCP("SubApp") @@ -993,12 +993,12 @@ def my_resource() -> str: # Should have prefixed key (using path format: resource://prefix/resource_name) assert "resource://prefix/my_resource" in resources - # The resource name should also be prefixed + # The resource name should NOT be prefixed (only the URI/key is prefixed) resource = resources["resource://prefix/my_resource"] - assert resource.name == "prefix_my_resource" + assert resource.name == "my_resource" - async def test_resource_template_name_prefixing(self): - """Test that resource template names are prefixed when mounted.""" + async def test_resource_template_uri_prefixing(self): + """Test that resource template URIs (keys) are prefixed when mounted, but names are not.""" # Create a sub-app with a resource template sub_app = FastMCP("SubApp") @@ -1017,9 +1017,9 @@ def user_template(user_id: str) -> str: # Should have prefixed key (using path format: resource://prefix/template_uri) assert "resource://prefix/user/{user_id}" in templates - # The template name should also be prefixed + # The template name should NOT be prefixed (only the uri_template/key is prefixed) template = templates["resource://prefix/user/{user_id}"] - assert template.name == "prefix_user_template" + assert template.name == "user_template" class TestParentTagFiltering: diff --git a/tests/tools/test_tool_manager.py b/tests/tools/test_tool_manager.py index 500852c0a5..72c617a4bb 100644 --- a/tests/tools/test_tool_manager.py +++ b/tests/tools/test_tool_manager.py @@ -869,8 +869,8 @@ def original_fn(x: int) -> int: with pytest.raises(NotFoundError, match="Tool 'original_fn' not found"): await manager.get_tool("original_fn") - async def test_add_tool_object_with_custom_key(self): - """Test adding a Tool object with a custom key using add_tool().""" + async def test_add_tool_object_with_custom_name_via_model_copy(self): + """Test adding a Tool object with a different name using model_copy.""" def fn(x: int) -> int: return x + 1 @@ -878,14 +878,13 @@ def fn(x: int) -> int: # Create a tool with a specific name tool = Tool.from_function(fn, name="my_tool") manager = ToolManager() - # Use model_copy to create a new tool with the custom key - tool_with_custom_key = tool.model_copy(key="proxy_tool") - manager.add_tool(tool_with_custom_key) - # The tool is accessible under the key + # Use model_copy to create a new tool with a new name + tool_with_new_name = tool.model_copy(update={"name": "proxy_tool"}) + manager.add_tool(tool_with_new_name) + # The tool is accessible under the new name stored = await manager.get_tool("proxy_tool") assert stored is not None - # But the tool's .name is unchanged - assert stored.name == "my_tool" + assert stored.name == "proxy_tool" # The tool is not accessible under its original name with pytest.raises(NotFoundError, match="Tool 'my_tool' not found"): await manager.get_tool("my_tool") diff --git a/tests/utilities/test_components.py b/tests/utilities/test_components.py index 1211d1e1f7..7e0f0203cf 100644 --- a/tests/utilities/test_components.py +++ b/tests/utilities/test_components.py @@ -48,7 +48,7 @@ def basic_component(self): name="test_component", title="Test Component", description="A test component", - tags=["test", "component"], + tags={"test", "component"}, ) def test_initialization_with_minimal_params(self): @@ -68,7 +68,7 @@ def test_initialization_with_all_params(self): name="full", title="Full Component", description="A fully configured component", - tags=["tag1", "tag2"], + tags={"tag1", "tag2"}, meta=meta, enabled=False, ) @@ -79,16 +79,10 @@ def test_initialization_with_all_params(self): assert component.meta == meta assert component.enabled is False - def test_key_property_without_custom_key(self, basic_component): - """Test that key property returns name when no custom key is set.""" + def test_key_property_returns_name(self, basic_component): + """Test that key property returns name.""" assert basic_component.key == "test_component" - def test_key_property_with_custom_key(self): - """Test that key property returns custom key when set.""" - component = FastMCPComponent(name="test", key="custom_key") - assert component.key == "custom_key" - assert component.name == "test" - def test_get_meta_without_fastmcp_meta(self, basic_component): """Test get_meta without including fastmcp meta.""" basic_component.meta = {"custom": "data"} @@ -110,7 +104,7 @@ def test_get_meta_preserves_existing_fastmcp_meta(self): component = FastMCPComponent( name="test", meta={"_fastmcp": {"existing": "value"}}, - tags=["new_tag"], + tags={"new_tag"}, ) result = component.get_meta(include_fastmcp_meta=True) assert result is not None @@ -123,13 +117,12 @@ def test_get_meta_returns_none_when_empty(self): result = component.get_meta(include_fastmcp_meta=False) assert result is None - def test_model_copy_creates_copy_with_new_key(self, basic_component): - """Test that model_copy with key creates a copy with a new key.""" - new_component = basic_component.model_copy(key="new_key") - assert new_component.key == "new_key" - assert new_component.name == basic_component.name + def test_model_copy_with_name_update(self, basic_component): + """Test that model_copy with name update creates a copy with new name.""" + new_component = basic_component.model_copy(update={"name": "new_name"}) + assert new_component.name == "new_name" assert new_component is not basic_component # Should be a copy - assert basic_component.key == "test_component" # Original unchanged + assert basic_component.name == "test_component" # Original unchanged def test_equality_same_components(self): """Test that identical components are equal.""" @@ -185,14 +178,14 @@ def test_tags_deduplication(self): """Test that tags are deduplicated.""" component = FastMCPComponent( name="test", - tags=["tag1", "tag2", "tag1", "tag2"], + tags={"tag1", "tag2"}, ) assert component.tags == {"tag1", "tag2"} def test_validation_error_for_invalid_data(self): """Test that validation errors are raised for invalid data.""" with pytest.raises(ValidationError): - FastMCPComponent() # Missing required name field + FastMCPComponent() # type: ignore[call-arg] def test_extra_fields_forbidden(self): """Test that extra fields are not allowed.""" @@ -287,12 +280,12 @@ def test_inheritance_from_fastmcp_component(self): def test_all_fastmcp_component_features_work(self, mirrored_component): """Test that all FastMCPComponent features work except enable/disable.""" - # Test key property - assert mirrored_component.key == "mirrored" + # Test name property + assert mirrored_component.name == "mirrored" - # Test model_copy with key - with_key = mirrored_component.model_copy(key="new_key") - assert with_key.key == "new_key" + # Test model_copy with update + with_new_name = mirrored_component.model_copy(update={"name": "new_name"}) + assert with_new_name.name == "new_name" # Test get_meta mirrored_component.tags = {"tag1"} @@ -323,7 +316,7 @@ class TestEdgeCasesAndIntegration: def test_empty_tags_conversion(self): """Test that empty tags are handled correctly.""" - component = FastMCPComponent(name="test", tags=[]) + component = FastMCPComponent(name="test", tags=set()) assert component.tags == set() def test_tags_with_none_values(self): @@ -353,25 +346,24 @@ def test_component_with_complex_meta(self): component = FastMCPComponent(name="test", meta=complex_meta) assert component.meta == complex_meta - def test_model_copy_with_key_preserves_all_attributes(self): - """Test that model_copy with key preserves all component attributes.""" + def test_model_copy_preserves_all_attributes(self): + """Test that model_copy preserves all component attributes.""" component = FastMCPComponent( name="test", title="Title", description="Description", - tags=["tag1", "tag2"], + tags={"tag1", "tag2"}, meta={"key": "value"}, enabled=False, ) - new_component = component.model_copy(key="new_key") + new_component = component.model_copy(update={"name": "new_name"}) - assert new_component.name == component.name + assert new_component.name == "new_name" assert new_component.title == component.title assert new_component.description == component.description assert new_component.tags == component.tags assert new_component.meta == component.meta assert new_component.enabled == component.enabled - assert new_component.key == "new_key" def test_mirrored_component_copy_chain(self): """Test creating copies of copies for mirrored components.""" @@ -390,33 +382,35 @@ def test_mirrored_component_copy_chain(self): assert copy1.name == "copy1" assert copy2.name == "copy2" - def test_model_copy_with_update_and_key(self): - """Test that model_copy works with both update dict and key parameter.""" + def test_model_copy_with_multiple_updates(self): + """Test that model_copy works with multiple fields in update dict.""" component = FastMCPComponent( name="test", title="Original Title", description="Original Description", - tags=["tag1"], + tags={"tag1"}, enabled=True, ) - # Test with both update and key + # Test with multiple updates updated_component = component.model_copy( - update={"title": "New Title", "description": "New Description"}, - key="new_key", + update={ + "name": "new_name", + "title": "New Title", + "description": "New Description", + }, ) - assert updated_component.name == "test" # Not in update, unchanged + assert updated_component.name == "new_name" # Updated assert updated_component.title == "New Title" # Updated assert updated_component.description == "New Description" # Updated assert updated_component.tags == {"tag1"} # Not in update, unchanged assert updated_component.enabled is True # Not in update, unchanged - assert updated_component.key == "new_key" # Custom key set # Original should be unchanged + assert component.name == "test" assert component.title == "Original Title" assert component.description == "Original Description" - assert component.key == "test" # Uses name as key def test_model_copy_deep_parameter(self): """Test that model_copy respects the deep parameter.""" From 1c285dc5bc584326ff296d672249fbf8e7967aad Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:09:13 -0500 Subject: [PATCH 2/3] Simplify .key as computed property Keep .key as the standard lookup interface for all components but implement it as a computed property instead of a stored field. - Remove _key private attribute and custom model_copy(key=...) - .key returns .name for tools/prompts, str(.uri) for resources, .uri_template for templates - Use .key universally for component lookups and Docket registration - MountedProvider: prefix URIs only for resources/templates, not names - Add _backend_* fields to proxy classes to preserve original identifiers for backend calls when prefixed via import_server --- src/fastmcp/resources/resource.py | 9 +- src/fastmcp/resources/resource_manager.py | 10 +- src/fastmcp/resources/template.py | 9 +- src/fastmcp/server/providers/mounted.py | 44 +++---- src/fastmcp/server/server.py | 31 +++-- src/fastmcp/server/tasks/handlers.py | 7 +- src/fastmcp/utilities/components.py | 40 +++++- src/fastmcp/utilities/inspect.py | 10 +- tests/resources/test_resource_manager.py | 117 ++++++++++++++++++ .../tasks/test_server_tasks_parameter.py | 22 ++-- tests/server/test_import_server.py | 2 +- tests/server/test_mount.py | 20 +-- tests/tools/test_tool_manager.py | 15 +-- tests/utilities/test_components.py | 74 ++++++----- 14 files changed, 289 insertions(+), 121 deletions(-) diff --git a/src/fastmcp/resources/resource.py b/src/fastmcp/resources/resource.py index 21d03b8d98..c882545249 100644 --- a/src/fastmcp/resources/resource.py +++ b/src/fastmcp/resources/resource.py @@ -274,8 +274,13 @@ def __repr__(self) -> str: @property def key(self) -> str: - """The lookup key for this resource. Returns str(uri).""" - return str(self.uri) + """ + The key of the component. This is used for internal bookkeeping + and may reflect e.g. prefixes or other identifiers. You should not depend on + keys having a certain value, as the same tool loaded from different + hierarchies of servers may have different keys. + """ + return self._key or str(self.uri) class FunctionResource(Resource): diff --git a/src/fastmcp/resources/resource_manager.py b/src/fastmcp/resources/resource_manager.py index 0fdc73cac6..290fe8484f 100644 --- a/src/fastmcp/resources/resource_manager.py +++ b/src/fastmcp/resources/resource_manager.py @@ -153,8 +153,9 @@ def add_resource(self, resource: Resource) -> Resource: """Add a resource to the manager. Args: - resource: A Resource instance to add. The resource's key (URI) - will be used as the storage key. + resource: A Resource instance to add. The resource's .key attribute + will be used as the storage key. To overwrite it, call + Resource.model_copy(key=new_key) before calling this method. """ existing = self._resources.get(resource.key) if existing: @@ -201,8 +202,9 @@ def add_template(self, template: ResourceTemplate) -> ResourceTemplate: """Add a template to the manager. Args: - template: A ResourceTemplate instance to add. The template's key (uri_template) - will be used as the storage key. + template: A ResourceTemplate instance to add. The template's .key attribute + will be used as the storage key. To overwrite it, call + ResourceTemplate.model_copy(key=new_key) before calling this method. Returns: The added template. If a template with the same URI already exists, diff --git a/src/fastmcp/resources/template.py b/src/fastmcp/resources/template.py index ecf9824f0b..961da0e571 100644 --- a/src/fastmcp/resources/template.py +++ b/src/fastmcp/resources/template.py @@ -219,8 +219,13 @@ def from_mcp_template(cls, mcp_template: SDKResourceTemplate) -> ResourceTemplat @property def key(self) -> str: - """The lookup key for this template. Returns uri_template.""" - return self.uri_template + """ + The key of the component. This is used for internal bookkeeping + and may reflect e.g. prefixes or other identifiers. You should not depend on + keys having a certain value, as the same tool loaded from different + hierarchies of servers may have different keys. + """ + return self._key or self.uri_template class FunctionResourceTemplate(ResourceTemplate): diff --git a/src/fastmcp/server/providers/mounted.py b/src/fastmcp/server/providers/mounted.py index 2fb025a468..490e8fddd9 100644 --- a/src/fastmcp/server/providers/mounted.py +++ b/src/fastmcp/server/providers/mounted.py @@ -181,35 +181,37 @@ def _strip_resource_prefix(self, uri: str) -> str | None: def _prefix_tool(self, tool: Tool) -> Tool: """Apply prefix to a tool.""" if self.tool_names and tool.name in self.tool_names: - new_name = self.tool_names[tool.name] + new_key = self.tool_names[tool.name] else: - new_name = self._add_tool_prefix(tool.name) - if new_name != tool.name: - return tool.model_copy(update={"name": new_name}) - return tool + new_key = self._add_tool_prefix(tool.key) + return tool.model_copy(key=new_key) if new_key != tool.key else tool def _prefix_resource(self, resource: Resource) -> Resource: - """Apply prefix to a resource URI (name is NOT prefixed).""" - if self.prefix: - new_uri = self._add_resource_prefix(str(resource.uri)) - if new_uri != str(resource.uri): - return resource.model_copy(update={"uri": new_uri}) + """Apply prefix to a resource.""" + new_key = self._add_resource_prefix(resource.key) + update: dict[str, Any] = {} + if self.prefix and resource.name: + update["name"] = f"{self.prefix}_{resource.name}" + if new_key != resource.key or update: + return resource.model_copy(key=new_key, update=update) return resource def _prefix_template(self, template: ResourceTemplate) -> ResourceTemplate: - """Apply prefix to a resource template URI (name is NOT prefixed).""" + """Apply prefix to a resource template.""" + new_key = self._add_resource_prefix(template.key) + update: dict[str, Any] = {} + if self.prefix and template.name: + update["name"] = f"{self.prefix}_{template.name}" if self.prefix and template.uri_template: - new_template = self._add_resource_prefix(template.uri_template) - if new_template != template.uri_template: - return template.model_copy(update={"uri_template": new_template}) + update["uri_template"] = self._add_resource_prefix(template.uri_template) + if new_key != template.key or update: + return template.model_copy(key=new_key, update=update) return template def _prefix_prompt(self, prompt: Prompt) -> Prompt: """Apply prefix to a prompt.""" - new_name = self._add_tool_prefix(prompt.name) - if new_name != prompt.name: - return prompt.model_copy(update={"name": new_name}) - return prompt + new_key = self._add_tool_prefix(prompt.key) + return prompt.model_copy(key=new_key) if new_key != prompt.key else prompt # ------------------------------------------------------------------------- # Tool methods @@ -221,7 +223,7 @@ async def list_tools(self) -> Sequence[Tool]: return [self._prefix_tool(tool) for tool in tools] async def get_tool(self, name: str) -> Tool | None: - """Get a tool by key, going through middleware.""" + """Get a tool by name, going through middleware.""" # Early exit if name doesn't match our prefix pattern if self._strip_tool_prefix(name) is None: return None @@ -248,7 +250,7 @@ async def list_resources(self) -> Sequence[Resource]: return [self._prefix_resource(resource) for resource in resources] async def get_resource(self, uri: str) -> Resource | None: - """Get a concrete resource by key (URI), going through middleware. + """Get a concrete resource by URI, going through middleware. Only returns concrete resources, not resources created from templates. This preserves the original template for task execution. @@ -311,7 +313,7 @@ async def list_prompts(self) -> Sequence[Prompt]: return [self._prefix_prompt(prompt) for prompt in prompts] async def get_prompt(self, name: str) -> Prompt | None: - """Get a prompt by key, going through middleware.""" + """Get a prompt by name, going through middleware.""" # Early exit if name doesn't match our prefix pattern if self._strip_tool_prefix(name) is None: return None diff --git a/src/fastmcp/server/server.py b/src/fastmcp/server/server.py index fe29f67177..448f50ac77 100644 --- a/src/fastmcp/server/server.py +++ b/src/fastmcp/server/server.py @@ -442,14 +442,14 @@ async def _docket_lifespan(self) -> AsyncIterator[None]: isinstance(resource, FunctionResource) and resource.task_config.mode != "forbidden" ): - docket.register(resource.fn, names=[resource.name]) + docket.register(resource.fn, names=[resource.key]) for template in self._resource_manager._templates.values(): if ( isinstance(template, FunctionResourceTemplate) and template.task_config.mode != "forbidden" ): - docket.register(template.fn, names=[template.name]) + docket.register(template.fn, names=[template.key]) # Register provider components for provider in self._providers: @@ -458,9 +458,9 @@ async def _docket_lifespan(self) -> AsyncIterator[None]: for tool in tasks.tools: docket.register(tool.fn, names=[tool.key]) for resource in tasks.resources: - docket.register(resource.fn, names=[resource.name]) + docket.register(resource.fn, names=[resource.key]) for template in tasks.templates: - docket.register(template.fn, names=[template.name]) + docket.register(template.fn, names=[template.key]) for prompt in tasks.prompts: docket.register( cast(Callable[..., Awaitable[Any]], prompt.fn), @@ -1127,7 +1127,7 @@ async def _list_tools_mcp(self) -> list[SDKTool]: tools = await self._list_tools_middleware() return [ tool.to_mcp_tool( - name=tool.name, + name=tool.key, include_fastmcp_meta=self.include_fastmcp_meta, ) for tool in tools @@ -1203,7 +1203,7 @@ async def _list_resources_mcp(self) -> list[SDKResource]: resources = await self._list_resources_middleware() return [ resource.to_mcp_resource( - uri=str(resource.uri), + uri=resource.key, include_fastmcp_meta=self.include_fastmcp_meta, ) for resource in resources @@ -1281,7 +1281,7 @@ async def _list_resource_templates_mcp(self) -> list[SDKResourceTemplate]: templates = await self._list_resource_templates_middleware() return [ template.to_mcp_template( - uriTemplate=template.uri_template, + uriTemplate=template.key, include_fastmcp_meta=self.include_fastmcp_meta, ) for template in templates @@ -1358,7 +1358,7 @@ async def _list_prompts_mcp(self) -> list[SDKPrompt]: prompts = await self._list_prompts_middleware() return [ prompt.to_mcp_prompt( - name=prompt.name, + name=prompt.key, include_fastmcp_meta=self.include_fastmcp_meta, ) for prompt in prompts @@ -2757,33 +2757,30 @@ async def import_server( # Import tools from the server for key, tool in (await server.get_tools()).items(): if prefix: - tool = tool.model_copy(update={"name": f"{prefix}_{key}"}) + tool = tool.model_copy(key=f"{prefix}_{key}") self._tool_manager.add_tool(tool) # Import resources and templates from the server for key, resource in (await server.get_resources()).items(): if prefix: - resource_uri = add_resource_prefix(key, prefix) + resource_key = add_resource_prefix(key, prefix) resource = resource.model_copy( - update={"name": f"{prefix}_{resource.name}", "uri": resource_uri} + update={"name": f"{prefix}_{resource.name}"}, key=resource_key ) self._resource_manager.add_resource(resource) for key, template in (await server.get_resource_templates()).items(): if prefix: - template_uri_template = add_resource_prefix(key, prefix) + template_key = add_resource_prefix(key, prefix) template = template.model_copy( - update={ - "name": f"{prefix}_{template.name}", - "uri_template": template_uri_template, - } + update={"name": f"{prefix}_{template.name}"}, key=template_key ) self._resource_manager.add_template(template) # Import prompts from the server for key, prompt in (await server.get_prompts()).items(): if prefix: - prompt = prompt.model_copy(update={"name": f"{prefix}_{key}"}) + prompt = prompt.model_copy(key=f"{prefix}_{key}") self._prompt_manager.add_prompt(prompt) if server._lifespan != default_lifespan: diff --git a/src/fastmcp/server/tasks/handlers.py b/src/fastmcp/server/tasks/handlers.py index ae4182a19d..64424356ad 100644 --- a/src/fastmcp/server/tasks/handlers.py +++ b/src/fastmcp/server/tasks/handlers.py @@ -309,20 +309,19 @@ async def handle_resource_as_task( with suppress(Exception): await ctx.session.send_notification(notification) # type: ignore[arg-type] - # Queue function to Docket by name (result storage via execution_ttl) - # Use resource.name which matches what was registered - Docket uses fn.__name__ + # Queue function to Docket by key (result storage via execution_ttl) # For templates, extract URI params and pass them to the function from fastmcp.resources.template import FunctionResourceTemplate, match_uri_template if isinstance(resource, FunctionResourceTemplate): params = match_uri_template(uri, resource.uri_template) or {} await docket.add( - resource.name, + resource.key, key=task_key, )(**params) else: await docket.add( - resource.name, + resource.key, key=task_key, )() diff --git a/src/fastmcp/utilities/components.py b/src/fastmcp/utilities/components.py index 56500650ca..6d2f0f5b41 100644 --- a/src/fastmcp/utilities/components.py +++ b/src/fastmcp/utilities/components.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from typing import Annotated, Any, TypedDict +from typing import Annotated, Any, TypedDict, cast from mcp.types import Icon from pydantic import BeforeValidator, Field, PrivateAttr @@ -56,10 +56,21 @@ class FastMCPComponent(FastMCPBaseModel): description="Whether the component is enabled.", ) + _key: str | None = PrivateAttr() + + def __init__(self, *, key: str | None = None, **kwargs: Any) -> None: + super().__init__(**kwargs) + self._key = key + @property def key(self) -> str: - """The lookup key for this component. Returns name by default.""" - return self.name + """ + The key of the component. This is used for internal bookkeeping + and may reflect e.g. prefixes or other identifiers. You should not depend on + keys having a certain value, as the same tool loaded from different + hierarchies of servers may have different keys. + """ + return self._key or self.name def get_meta( self, include_fastmcp_meta: bool | None = None @@ -85,6 +96,29 @@ def get_meta( return meta or None + def model_copy( # type: ignore[override] + self, + *, + update: dict[str, Any] | None = None, + deep: bool = False, + key: str | None = None, + ) -> Self: + """ + Create a copy of the component. + + Args: + update: A dictionary of fields to update. + deep: Whether to deep copy the component. + key: The key to use for the copy. + """ + # `model_copy` has an `update` parameter but it doesn't work for certain private attributes + # https://github.com/pydantic/pydantic/issues/12116 + # So we manually set the private attribute here instead, such as _key + copy = super().model_copy(update=update, deep=deep) + if key is not None: + copy._key = key + return cast(Self, copy) + def __eq__(self, other: object) -> bool: if type(self) is not type(other): return False diff --git a/src/fastmcp/utilities/inspect.py b/src/fastmcp/utilities/inspect.py index 77e21a9d1e..b4563090d3 100644 --- a/src/fastmcp/utilities/inspect.py +++ b/src/fastmcp/utilities/inspect.py @@ -119,11 +119,11 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: # Extract detailed tool information tool_infos = [] for tool in tools_list: - mcp_tool = tool.to_mcp_tool(name=tool.name) + mcp_tool = tool.to_mcp_tool(name=tool.key) tool_infos.append( ToolInfo( key=tool.key, - name=tool.name, + name=tool.name or tool.key, description=tool.description, input_schema=mcp_tool.inputSchema if mcp_tool.inputSchema else {}, output_schema=tool.output_schema, @@ -144,7 +144,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: prompt_infos.append( PromptInfo( key=prompt.key, - name=prompt.name, + name=prompt.name or prompt.key, description=prompt.description, arguments=[arg.model_dump() for arg in prompt.arguments] if prompt.arguments @@ -165,7 +165,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: resource_infos.append( ResourceInfo( key=resource.key, - uri=str(resource.uri), + uri=resource.key, name=resource.name, description=resource.description, mime_type=resource.mime_type, @@ -188,7 +188,7 @@ async def inspect_fastmcp_v2(mcp: FastMCP[Any]) -> FastMCPInfo: template_infos.append( TemplateInfo( key=template.key, - uri_template=template.uri_template, + uri_template=template.key, name=template.name, description=template.description, mime_type=template.mime_type, diff --git a/tests/resources/test_resource_manager.py b/tests/resources/test_resource_manager.py index 50d72018d2..ecbf10cfd8 100644 --- a/tests/resources/test_resource_manager.py +++ b/tests/resources/test_resource_manager.py @@ -450,6 +450,123 @@ async def get_system_data(): assert admin_resources[0].name == "system_data" +class TestCustomResourceKeys: + """Test adding resources and templates with custom keys.""" + + def test_add_resource_with_custom_key(self, temp_file: Path): + """Test adding a resource with a custom key different from its URI.""" + manager = ResourceManager() + original_uri = "data://test/resource" + custom_key = "custom://resource/key" + + # Create a function resource instead of file resource to avoid path issues + async def get_data(): + return "Test data" + + resource = FunctionResource( + uri=AnyUrl(original_uri), + name="test_resource", + fn=get_data, + ) + + # Use model_copy to create a new resource with the custom key + resource_with_custom_key = resource.model_copy(key=custom_key) + manager.add_resource(resource_with_custom_key) + + # Resource should be accessible via custom key + assert custom_key in manager._resources + # But not via its original URI + assert original_uri not in manager._resources + # The resource's internal URI remains unchanged + assert str(manager._resources[custom_key].uri) == original_uri + + def test_add_template_with_custom_key(self): + """Test adding a template with a custom key different from its URI template.""" + manager = ResourceManager() + + def template_fn(id: str) -> str: + return f"Template {id}" + + original_uri_template = "test://{id}" + custom_key = "custom://{id}/template" + + template = ResourceTemplate.from_function( + fn=template_fn, + uri_template=original_uri_template, + name="test_template", + ) + + # Use model_copy to create a new template with the custom key + template_with_custom_key = template.model_copy(key=custom_key) + manager.add_template(template_with_custom_key) + + # Template should be accessible via custom key + assert custom_key in manager._templates + # But not via its original URI template + assert original_uri_template not in manager._templates + # The template's internal URI template remains unchanged + assert str(manager._templates[custom_key].uri_template) == original_uri_template + + async def test_get_resource_with_custom_key(self, temp_file: Path): + """Test that get_resource works with resources added with custom keys.""" + manager = ResourceManager() + original_uri = "data://test/resource" + custom_key = "custom://resource/path" + + # Create a function resource instead of file resource to avoid path issues + async def get_data(): + return "Test data" + + resource = FunctionResource( + uri=AnyUrl(original_uri), + name="test_resource", + fn=get_data, + ) + + # Use model_copy to create a new resource with the custom key + resource_with_custom_key = resource.model_copy(key=custom_key) + manager.add_resource(resource_with_custom_key) + + # Should be retrievable by the custom key + retrieved = await manager.get_resource(custom_key) + assert retrieved is not None + assert str(retrieved.uri) == original_uri + + # Should NOT be retrievable by the original URI + with pytest.raises(NotFoundError, match="Unknown resource"): + await manager.get_resource(original_uri) + + async def test_get_resource_from_template_with_custom_key(self): + """Test that templates with custom keys can create resources.""" + manager = ResourceManager() + + def greet(name: str) -> str: + return f"Hello, {name}!" + + original_template = "greet://{name}" + custom_key = "custom://greet/{name}" + + template = ResourceTemplate.from_function( + fn=greet, + uri_template=original_template, + name="custom_greeter", + ) + + # Use model_copy to create a new template with the custom key + template_with_custom_key = template.model_copy(key=custom_key) + manager.add_template(template_with_custom_key) + + # Using a URI that matches the custom key pattern + resource = await manager.get_resource("custom://greet/world") + assert isinstance(resource, FunctionResource) + result = await resource.read() + assert result.content == "Hello, world!" + + # Shouldn't work with the original template pattern + with pytest.raises(NotFoundError, match="Unknown resource"): + await manager.get_resource("greet://world") + + class TestQueryOnlyTemplates: """Test resource templates with only query parameters (no path params).""" diff --git a/tests/server/tasks/test_server_tasks_parameter.py b/tests/server/tasks/test_server_tasks_parameter.py index 90813814ef..b61edb4a47 100644 --- a/tests/server/tasks/test_server_tasks_parameter.py +++ b/tests/server/tasks/test_server_tasks_parameter.py @@ -28,11 +28,12 @@ async def my_resource() -> str: async with Client(mcp) as client: # Verify all task-enabled components are registered with docket + # Tools and prompts use .key (which equals name), resources use .key (which is URI) docket = mcp.docket assert docket is not None assert "my_tool" in docket.tasks assert "my_prompt" in docket.tasks - assert "my_resource" in docket.tasks + assert "test://resource" in docket.tasks # Tool should support background execution tool_task = await client.call_tool("my_tool", task=True) @@ -198,17 +199,18 @@ async def explicit_false_resource() -> str: async with Client(mcp) as client: # Verify docket registration matches task settings + # Tools/prompts use .key (name), resources use .key (URI) docket = mcp.docket assert docket is not None # task=True (explicit or inherited) means registered assert "inherited_tool" in docket.tasks assert "explicit_true_tool" in docket.tasks assert "inherited_prompt" in docket.tasks - assert "inherited_resource" in docket.tasks + assert "test://inherited" in docket.tasks # task=False means NOT registered assert "explicit_false_tool" not in docket.tasks assert "explicit_false_prompt" not in docket.tasks - assert "explicit_false_resource" not in docket.tasks + assert "test://explicit_false" not in docket.tasks # Tools inherited = await client.call_tool("inherited_tool", task=True) @@ -339,8 +341,7 @@ async def my_function() -> str: async def test_task_with_custom_resource_name(): """Resources with custom names work correctly as tasks. - When a resource is registered with a custom name different from the function - name, task execution should use the custom name for Docket lookup. + Resources are registered/looked up by their .key (URI), not their name. """ mcp = FastMCP("test", tasks=True) @@ -349,10 +350,10 @@ async def my_resource_func() -> str: return "result from custom-named resource" async with Client(mcp) as client: - # Verify the resource is registered with its custom name in Docket + # Verify the resource is registered with its key (URI) in Docket docket = mcp.docket assert docket is not None - assert "custom-resource-name" in docket.tasks + assert "test://resource" in docket.tasks # Call the resource as a task task = await client.read_resource("test://resource", task=True) @@ -364,8 +365,7 @@ async def my_resource_func() -> str: async def test_task_with_custom_template_name(): """Resource templates with custom names work correctly as tasks. - When a template is registered with a custom name different from the function - name, task execution should use the custom name for Docket lookup. + Templates are registered/looked up by their .key (uri_template), not their name. """ mcp = FastMCP("test", tasks=True) @@ -374,10 +374,10 @@ async def my_template_func(item_id: str) -> str: return f"result for {item_id}" async with Client(mcp) as client: - # Verify the template is registered with its custom name in Docket + # Verify the template is registered with its key (uri_template) in Docket docket = mcp.docket assert docket is not None - assert "custom-template-name" in docket.tasks + assert "test://{item_id}" in docket.tasks # Call the template as a task task = await client.read_resource("test://123", task=True) diff --git a/tests/server/test_import_server.py b/tests/server/test_import_server.py index 1b48c76fb4..446dbfcc94 100644 --- a/tests/server/test_import_server.py +++ b/tests/server/test_import_server.py @@ -28,7 +28,7 @@ def sub_tool() -> str: # Verify the original tool still exists in the sub-app tool = await main_app._tool_manager.get_tool("sub_sub_tool") assert tool is not None - assert tool.name == "sub_sub_tool" # name is now prefixed + assert tool.name == "sub_tool" assert isinstance(tool, FunctionTool) assert callable(tool.fn) diff --git a/tests/server/test_mount.py b/tests/server/test_mount.py index 141277b64e..7026429a5c 100644 --- a/tests/server/test_mount.py +++ b/tests/server/test_mount.py @@ -970,11 +970,11 @@ def hello(): assert lifespan_check == ["start"] -class TestResourceKeyPrefixing: - """Test that resource and resource template keys (URIs) get prefixed when mounted.""" +class TestResourceNamePrefixing: + """Test that resource and resource template names get prefixed when mounted.""" - async def test_resource_uri_prefixing(self): - """Test that resource URIs (keys) are prefixed when mounted, but names are not.""" + async def test_resource_name_prefixing(self): + """Test that resource names are prefixed when mounted.""" # Create a sub-app with a resource sub_app = FastMCP("SubApp") @@ -993,12 +993,12 @@ def my_resource() -> str: # Should have prefixed key (using path format: resource://prefix/resource_name) assert "resource://prefix/my_resource" in resources - # The resource name should NOT be prefixed (only the URI/key is prefixed) + # The resource name should also be prefixed resource = resources["resource://prefix/my_resource"] - assert resource.name == "my_resource" + assert resource.name == "prefix_my_resource" - async def test_resource_template_uri_prefixing(self): - """Test that resource template URIs (keys) are prefixed when mounted, but names are not.""" + async def test_resource_template_name_prefixing(self): + """Test that resource template names are prefixed when mounted.""" # Create a sub-app with a resource template sub_app = FastMCP("SubApp") @@ -1017,9 +1017,9 @@ def user_template(user_id: str) -> str: # Should have prefixed key (using path format: resource://prefix/template_uri) assert "resource://prefix/user/{user_id}" in templates - # The template name should NOT be prefixed (only the uri_template/key is prefixed) + # The template name should also be prefixed template = templates["resource://prefix/user/{user_id}"] - assert template.name == "user_template" + assert template.name == "prefix_user_template" class TestParentTagFiltering: diff --git a/tests/tools/test_tool_manager.py b/tests/tools/test_tool_manager.py index 72c617a4bb..500852c0a5 100644 --- a/tests/tools/test_tool_manager.py +++ b/tests/tools/test_tool_manager.py @@ -869,8 +869,8 @@ def original_fn(x: int) -> int: with pytest.raises(NotFoundError, match="Tool 'original_fn' not found"): await manager.get_tool("original_fn") - async def test_add_tool_object_with_custom_name_via_model_copy(self): - """Test adding a Tool object with a different name using model_copy.""" + async def test_add_tool_object_with_custom_key(self): + """Test adding a Tool object with a custom key using add_tool().""" def fn(x: int) -> int: return x + 1 @@ -878,13 +878,14 @@ def fn(x: int) -> int: # Create a tool with a specific name tool = Tool.from_function(fn, name="my_tool") manager = ToolManager() - # Use model_copy to create a new tool with a new name - tool_with_new_name = tool.model_copy(update={"name": "proxy_tool"}) - manager.add_tool(tool_with_new_name) - # The tool is accessible under the new name + # Use model_copy to create a new tool with the custom key + tool_with_custom_key = tool.model_copy(key="proxy_tool") + manager.add_tool(tool_with_custom_key) + # The tool is accessible under the key stored = await manager.get_tool("proxy_tool") assert stored is not None - assert stored.name == "proxy_tool" + # But the tool's .name is unchanged + assert stored.name == "my_tool" # The tool is not accessible under its original name with pytest.raises(NotFoundError, match="Tool 'my_tool' not found"): await manager.get_tool("my_tool") diff --git a/tests/utilities/test_components.py b/tests/utilities/test_components.py index 7e0f0203cf..1211d1e1f7 100644 --- a/tests/utilities/test_components.py +++ b/tests/utilities/test_components.py @@ -48,7 +48,7 @@ def basic_component(self): name="test_component", title="Test Component", description="A test component", - tags={"test", "component"}, + tags=["test", "component"], ) def test_initialization_with_minimal_params(self): @@ -68,7 +68,7 @@ def test_initialization_with_all_params(self): name="full", title="Full Component", description="A fully configured component", - tags={"tag1", "tag2"}, + tags=["tag1", "tag2"], meta=meta, enabled=False, ) @@ -79,10 +79,16 @@ def test_initialization_with_all_params(self): assert component.meta == meta assert component.enabled is False - def test_key_property_returns_name(self, basic_component): - """Test that key property returns name.""" + def test_key_property_without_custom_key(self, basic_component): + """Test that key property returns name when no custom key is set.""" assert basic_component.key == "test_component" + def test_key_property_with_custom_key(self): + """Test that key property returns custom key when set.""" + component = FastMCPComponent(name="test", key="custom_key") + assert component.key == "custom_key" + assert component.name == "test" + def test_get_meta_without_fastmcp_meta(self, basic_component): """Test get_meta without including fastmcp meta.""" basic_component.meta = {"custom": "data"} @@ -104,7 +110,7 @@ def test_get_meta_preserves_existing_fastmcp_meta(self): component = FastMCPComponent( name="test", meta={"_fastmcp": {"existing": "value"}}, - tags={"new_tag"}, + tags=["new_tag"], ) result = component.get_meta(include_fastmcp_meta=True) assert result is not None @@ -117,12 +123,13 @@ def test_get_meta_returns_none_when_empty(self): result = component.get_meta(include_fastmcp_meta=False) assert result is None - def test_model_copy_with_name_update(self, basic_component): - """Test that model_copy with name update creates a copy with new name.""" - new_component = basic_component.model_copy(update={"name": "new_name"}) - assert new_component.name == "new_name" + def test_model_copy_creates_copy_with_new_key(self, basic_component): + """Test that model_copy with key creates a copy with a new key.""" + new_component = basic_component.model_copy(key="new_key") + assert new_component.key == "new_key" + assert new_component.name == basic_component.name assert new_component is not basic_component # Should be a copy - assert basic_component.name == "test_component" # Original unchanged + assert basic_component.key == "test_component" # Original unchanged def test_equality_same_components(self): """Test that identical components are equal.""" @@ -178,14 +185,14 @@ def test_tags_deduplication(self): """Test that tags are deduplicated.""" component = FastMCPComponent( name="test", - tags={"tag1", "tag2"}, + tags=["tag1", "tag2", "tag1", "tag2"], ) assert component.tags == {"tag1", "tag2"} def test_validation_error_for_invalid_data(self): """Test that validation errors are raised for invalid data.""" with pytest.raises(ValidationError): - FastMCPComponent() # type: ignore[call-arg] + FastMCPComponent() # Missing required name field def test_extra_fields_forbidden(self): """Test that extra fields are not allowed.""" @@ -280,12 +287,12 @@ def test_inheritance_from_fastmcp_component(self): def test_all_fastmcp_component_features_work(self, mirrored_component): """Test that all FastMCPComponent features work except enable/disable.""" - # Test name property - assert mirrored_component.name == "mirrored" + # Test key property + assert mirrored_component.key == "mirrored" - # Test model_copy with update - with_new_name = mirrored_component.model_copy(update={"name": "new_name"}) - assert with_new_name.name == "new_name" + # Test model_copy with key + with_key = mirrored_component.model_copy(key="new_key") + assert with_key.key == "new_key" # Test get_meta mirrored_component.tags = {"tag1"} @@ -316,7 +323,7 @@ class TestEdgeCasesAndIntegration: def test_empty_tags_conversion(self): """Test that empty tags are handled correctly.""" - component = FastMCPComponent(name="test", tags=set()) + component = FastMCPComponent(name="test", tags=[]) assert component.tags == set() def test_tags_with_none_values(self): @@ -346,24 +353,25 @@ def test_component_with_complex_meta(self): component = FastMCPComponent(name="test", meta=complex_meta) assert component.meta == complex_meta - def test_model_copy_preserves_all_attributes(self): - """Test that model_copy preserves all component attributes.""" + def test_model_copy_with_key_preserves_all_attributes(self): + """Test that model_copy with key preserves all component attributes.""" component = FastMCPComponent( name="test", title="Title", description="Description", - tags={"tag1", "tag2"}, + tags=["tag1", "tag2"], meta={"key": "value"}, enabled=False, ) - new_component = component.model_copy(update={"name": "new_name"}) + new_component = component.model_copy(key="new_key") - assert new_component.name == "new_name" + assert new_component.name == component.name assert new_component.title == component.title assert new_component.description == component.description assert new_component.tags == component.tags assert new_component.meta == component.meta assert new_component.enabled == component.enabled + assert new_component.key == "new_key" def test_mirrored_component_copy_chain(self): """Test creating copies of copies for mirrored components.""" @@ -382,35 +390,33 @@ def test_mirrored_component_copy_chain(self): assert copy1.name == "copy1" assert copy2.name == "copy2" - def test_model_copy_with_multiple_updates(self): - """Test that model_copy works with multiple fields in update dict.""" + def test_model_copy_with_update_and_key(self): + """Test that model_copy works with both update dict and key parameter.""" component = FastMCPComponent( name="test", title="Original Title", description="Original Description", - tags={"tag1"}, + tags=["tag1"], enabled=True, ) - # Test with multiple updates + # Test with both update and key updated_component = component.model_copy( - update={ - "name": "new_name", - "title": "New Title", - "description": "New Description", - }, + update={"title": "New Title", "description": "New Description"}, + key="new_key", ) - assert updated_component.name == "new_name" # Updated + assert updated_component.name == "test" # Not in update, unchanged assert updated_component.title == "New Title" # Updated assert updated_component.description == "New Description" # Updated assert updated_component.tags == {"tag1"} # Not in update, unchanged assert updated_component.enabled is True # Not in update, unchanged + assert updated_component.key == "new_key" # Custom key set # Original should be unchanged - assert component.name == "test" assert component.title == "Original Title" assert component.description == "Original Description" + assert component.key == "test" # Uses name as key def test_model_copy_deep_parameter(self): """Test that model_copy respects the deep parameter.""" From 711d1fcf0cc0f41689ed1f417b1de3916882a049 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:43:30 -0500 Subject: [PATCH 3/3] Standardize .key as computed property - .key is now a read-only computed property: - Tools/Prompts: returns .name - Resources: returns str(.uri) - Templates: returns .uri_template - Prefixing uses model_copy(update={...}) to change underlying field - Resource/template names are NOT prefixed, only URIs - Move import_server tests to tests/deprecated/ --- src/fastmcp/resources/resource.py | 9 +- src/fastmcp/resources/resource_manager.py | 8 +- src/fastmcp/resources/template.py | 9 +- src/fastmcp/server/providers/mounted.py | 42 +++---- src/fastmcp/server/server.py | 22 ++-- src/fastmcp/utilities/components.py | 44 ++----- tests/deprecated/__init__.py | 5 +- tests/deprecated/conftest.py | 9 ++ .../test_import_server.py | 60 +++++++-- tests/resources/test_resource_manager.py | 117 ------------------ tests/server/test_mount.py | 20 +-- tests/server/test_server.py | 42 ------- tests/tools/test_tool_manager.py | 21 ---- tests/utilities/test_components.py | 62 ++++------ 14 files changed, 141 insertions(+), 329 deletions(-) create mode 100644 tests/deprecated/conftest.py rename tests/{server => deprecated}/test_import_server.py (89%) diff --git a/src/fastmcp/resources/resource.py b/src/fastmcp/resources/resource.py index c882545249..21d03b8d98 100644 --- a/src/fastmcp/resources/resource.py +++ b/src/fastmcp/resources/resource.py @@ -274,13 +274,8 @@ def __repr__(self) -> str: @property def key(self) -> str: - """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or str(self.uri) + """The lookup key for this resource. Returns str(uri).""" + return str(self.uri) class FunctionResource(Resource): diff --git a/src/fastmcp/resources/resource_manager.py b/src/fastmcp/resources/resource_manager.py index 290fe8484f..aea3f1d093 100644 --- a/src/fastmcp/resources/resource_manager.py +++ b/src/fastmcp/resources/resource_manager.py @@ -154,8 +154,8 @@ def add_resource(self, resource: Resource) -> Resource: Args: resource: A Resource instance to add. The resource's .key attribute - will be used as the storage key. To overwrite it, call - Resource.model_copy(key=new_key) before calling this method. + (which is str(uri)) will be used as the storage key. To use a + different key, change the uri via model_copy(update={"uri": new_uri}). """ existing = self._resources.get(resource.key) if existing: @@ -203,8 +203,8 @@ def add_template(self, template: ResourceTemplate) -> ResourceTemplate: Args: template: A ResourceTemplate instance to add. The template's .key attribute - will be used as the storage key. To overwrite it, call - ResourceTemplate.model_copy(key=new_key) before calling this method. + (which is uri_template) will be used as the storage key. To use a + different key, change uri_template via model_copy(update={"uri_template": new_uri}). Returns: The added template. If a template with the same URI already exists, diff --git a/src/fastmcp/resources/template.py b/src/fastmcp/resources/template.py index 961da0e571..ecf9824f0b 100644 --- a/src/fastmcp/resources/template.py +++ b/src/fastmcp/resources/template.py @@ -219,13 +219,8 @@ def from_mcp_template(cls, mcp_template: SDKResourceTemplate) -> ResourceTemplat @property def key(self) -> str: - """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or self.uri_template + """The lookup key for this template. Returns uri_template.""" + return self.uri_template class FunctionResourceTemplate(ResourceTemplate): diff --git a/src/fastmcp/server/providers/mounted.py b/src/fastmcp/server/providers/mounted.py index 490e8fddd9..42809a2879 100644 --- a/src/fastmcp/server/providers/mounted.py +++ b/src/fastmcp/server/providers/mounted.py @@ -179,39 +179,37 @@ def _strip_resource_prefix(self, uri: str) -> str | None: # ------------------------------------------------------------------------- def _prefix_tool(self, tool: Tool) -> Tool: - """Apply prefix to a tool.""" + """Apply prefix to a tool. Changes name which updates .key.""" if self.tool_names and tool.name in self.tool_names: - new_key = self.tool_names[tool.name] + new_name = self.tool_names[tool.name] else: - new_key = self._add_tool_prefix(tool.key) - return tool.model_copy(key=new_key) if new_key != tool.key else tool + new_name = self._add_tool_prefix(tool.name) + if new_name != tool.name: + return tool.model_copy(update={"name": new_name}) + return tool def _prefix_resource(self, resource: Resource) -> Resource: - """Apply prefix to a resource.""" - new_key = self._add_resource_prefix(resource.key) - update: dict[str, Any] = {} - if self.prefix and resource.name: - update["name"] = f"{self.prefix}_{resource.name}" - if new_key != resource.key or update: - return resource.model_copy(key=new_key, update=update) + """Apply prefix to a resource URI (name is NOT prefixed).""" + if self.prefix: + new_uri = self._add_resource_prefix(str(resource.uri)) + if new_uri != str(resource.uri): + return resource.model_copy(update={"uri": new_uri}) return resource def _prefix_template(self, template: ResourceTemplate) -> ResourceTemplate: - """Apply prefix to a resource template.""" - new_key = self._add_resource_prefix(template.key) - update: dict[str, Any] = {} - if self.prefix and template.name: - update["name"] = f"{self.prefix}_{template.name}" + """Apply prefix to a resource template URI (name is NOT prefixed).""" if self.prefix and template.uri_template: - update["uri_template"] = self._add_resource_prefix(template.uri_template) - if new_key != template.key or update: - return template.model_copy(key=new_key, update=update) + new_template = self._add_resource_prefix(template.uri_template) + if new_template != template.uri_template: + return template.model_copy(update={"uri_template": new_template}) return template def _prefix_prompt(self, prompt: Prompt) -> Prompt: - """Apply prefix to a prompt.""" - new_key = self._add_tool_prefix(prompt.key) - return prompt.model_copy(key=new_key) if new_key != prompt.key else prompt + """Apply prefix to a prompt. Changes name which updates .key.""" + new_name = self._add_tool_prefix(prompt.name) + if new_name != prompt.name: + return prompt.model_copy(update={"name": new_name}) + return prompt # ------------------------------------------------------------------------- # Tool methods diff --git a/src/fastmcp/server/server.py b/src/fastmcp/server/server.py index 448f50ac77..fdabb4e957 100644 --- a/src/fastmcp/server/server.py +++ b/src/fastmcp/server/server.py @@ -2755,32 +2755,30 @@ async def import_server( ) # Import tools from the server - for key, tool in (await server.get_tools()).items(): + for tool in (await server.get_tools()).values(): if prefix: - tool = tool.model_copy(key=f"{prefix}_{key}") + tool = tool.model_copy(update={"name": f"{prefix}_{tool.name}"}) self._tool_manager.add_tool(tool) # Import resources and templates from the server - for key, resource in (await server.get_resources()).items(): + for resource in (await server.get_resources()).values(): if prefix: - resource_key = add_resource_prefix(key, prefix) - resource = resource.model_copy( - update={"name": f"{prefix}_{resource.name}"}, key=resource_key - ) + new_uri = add_resource_prefix(str(resource.uri), prefix) + resource = resource.model_copy(update={"uri": new_uri}) self._resource_manager.add_resource(resource) - for key, template in (await server.get_resource_templates()).items(): + for template in (await server.get_resource_templates()).values(): if prefix: - template_key = add_resource_prefix(key, prefix) + new_uri_template = add_resource_prefix(template.uri_template, prefix) template = template.model_copy( - update={"name": f"{prefix}_{template.name}"}, key=template_key + update={"uri_template": new_uri_template} ) self._resource_manager.add_template(template) # Import prompts from the server - for key, prompt in (await server.get_prompts()).items(): + for prompt in (await server.get_prompts()).values(): if prefix: - prompt = prompt.model_copy(key=f"{prefix}_{key}") + prompt = prompt.model_copy(update={"name": f"{prefix}_{prompt.name}"}) self._prompt_manager.add_prompt(prompt) if server._lifespan != default_lifespan: diff --git a/src/fastmcp/utilities/components.py b/src/fastmcp/utilities/components.py index 6d2f0f5b41..833f2fb97b 100644 --- a/src/fastmcp/utilities/components.py +++ b/src/fastmcp/utilities/components.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from typing import Annotated, Any, TypedDict, cast +from typing import Annotated, Any, TypedDict from mcp.types import Icon from pydantic import BeforeValidator, Field, PrivateAttr @@ -56,21 +56,16 @@ class FastMCPComponent(FastMCPBaseModel): description="Whether the component is enabled.", ) - _key: str | None = PrivateAttr() - - def __init__(self, *, key: str | None = None, **kwargs: Any) -> None: - super().__init__(**kwargs) - self._key = key - @property def key(self) -> str: + """The lookup key for this component. Returns name by default. + + Subclasses override this to return different identifiers: + - Tools/Prompts: name + - Resources: str(uri) + - Templates: uri_template """ - The key of the component. This is used for internal bookkeeping - and may reflect e.g. prefixes or other identifiers. You should not depend on - keys having a certain value, as the same tool loaded from different - hierarchies of servers may have different keys. - """ - return self._key or self.name + return self.name def get_meta( self, include_fastmcp_meta: bool | None = None @@ -96,29 +91,6 @@ def get_meta( return meta or None - def model_copy( # type: ignore[override] - self, - *, - update: dict[str, Any] | None = None, - deep: bool = False, - key: str | None = None, - ) -> Self: - """ - Create a copy of the component. - - Args: - update: A dictionary of fields to update. - deep: Whether to deep copy the component. - key: The key to use for the copy. - """ - # `model_copy` has an `update` parameter but it doesn't work for certain private attributes - # https://github.com/pydantic/pydantic/issues/12116 - # So we manually set the private attribute here instead, such as _key - copy = super().model_copy(update=update, deep=deep) - if key is not None: - copy._key = key - return cast(Self, copy) - def __eq__(self, other: object) -> bool: if type(self) is not type(other): return False diff --git a/tests/deprecated/__init__.py b/tests/deprecated/__init__.py index 8c1eca42be..b7d7878ba9 100644 --- a/tests/deprecated/__init__.py +++ b/tests/deprecated/__init__.py @@ -1,4 +1 @@ -import pytest - -# reset deprecation warnings for this module -pytestmark = pytest.mark.filterwarnings("default::DeprecationWarning") +# Tests for deprecated features - deprecation warnings are suppressed via conftest.py diff --git a/tests/deprecated/conftest.py b/tests/deprecated/conftest.py new file mode 100644 index 0000000000..b9f8cf40cc --- /dev/null +++ b/tests/deprecated/conftest.py @@ -0,0 +1,9 @@ +"""Conftest for deprecated tests - suppresses deprecation warnings.""" + +import pytest + + +def pytest_collection_modifyitems(items): + """Add filterwarnings marker to all tests in this directory.""" + for item in items: + item.add_marker(pytest.mark.filterwarnings("ignore::DeprecationWarning")) diff --git a/tests/server/test_import_server.py b/tests/deprecated/test_import_server.py similarity index 89% rename from tests/server/test_import_server.py rename to tests/deprecated/test_import_server.py index 446dbfcc94..503a4216ae 100644 --- a/tests/server/test_import_server.py +++ b/tests/deprecated/test_import_server.py @@ -28,7 +28,8 @@ def sub_tool() -> str: # Verify the original tool still exists in the sub-app tool = await main_app._tool_manager.get_tool("sub_sub_tool") assert tool is not None - assert tool.name == "sub_tool" + # import_server creates copies with prefixed names (unlike mount which proxies) + assert tool.name == "sub_sub_tool" assert isinstance(tool, FunctionTool) assert callable(tool.fn) @@ -589,8 +590,8 @@ def second_shared_tool() -> str: assert result.data == "Second app tool" -async def test_import_server_resource_name_prefixing(): - """Test that resource names are prefixed when using import_server.""" +async def test_import_server_resource_uri_prefixing(): + """Test that resource URIs are prefixed when using import_server (names are NOT prefixed).""" # Create a sub-server with a resource sub_server = FastMCP("SubServer") @@ -602,14 +603,14 @@ def test_resource() -> str: main_server = FastMCP("MainServer") await main_server.import_server(sub_server, prefix="imported") - # Get resources and verify name prefixing + # Get resources and verify URI prefixing (name should NOT be prefixed) resources = await main_server.get_resources() resource = resources["resource://imported/test_resource"] - assert resource.name == "imported_test_resource" + assert resource.name == "test_resource" -async def test_import_server_resource_template_name_prefixing(): - """Test that resource template names are prefixed when using import_server.""" +async def test_import_server_resource_template_uri_prefixing(): + """Test that resource template URIs are prefixed when using import_server (names are NOT prefixed).""" # Create a sub-server with a resource template sub_server = FastMCP("SubServer") @@ -621,7 +622,48 @@ def data_template(item_id: str) -> str: main_server = FastMCP("MainServer") await main_server.import_server(sub_server, prefix="imported") - # Get resource templates and verify name prefixing + # Get resource templates and verify URI prefixing (name should NOT be prefixed) templates = await main_server.get_resource_templates() template = templates["resource://imported/data/{item_id}"] - assert template.name == "imported_data_template" + assert template.name == "data_template" + + +async def test_import_server_with_new_prefix_format(): + """Test that import_server correctly uses the new prefix format.""" + # Create a server with resources + source_server = FastMCP(name="SourceServer") + + @source_server.resource("resource://test-resource") + def get_resource(): + return "Resource content" + + @source_server.resource("resource:///absolute/path") + def get_absolute_resource(): + return "Absolute resource content" + + @source_server.resource("resource://{param}/template") + def get_template_resource(param: str): + return f"Template resource with {param}" + + # Create target server and import the source server + target_server = FastMCP(name="TargetServer") + await target_server.import_server(source_server, "imported") + + # Check that the resources were imported with the correct prefixes + resources = await target_server.get_resources() + templates = await target_server.get_resource_templates() + + assert "resource://imported/test-resource" in resources + assert "resource://imported//absolute/path" in resources + assert "resource://imported/{param}/template" in templates + + # Verify we can access the resources + async with Client(target_server) as client: + result = await client.read_resource("resource://imported/test-resource") + assert result[0].text == "Resource content" # type: ignore[attr-defined] + + result = await client.read_resource("resource://imported//absolute/path") + assert result[0].text == "Absolute resource content" # type: ignore[attr-defined] + + result = await client.read_resource("resource://imported/param-value/template") + assert result[0].text == "Template resource with param-value" # type: ignore[attr-defined] diff --git a/tests/resources/test_resource_manager.py b/tests/resources/test_resource_manager.py index ecbf10cfd8..50d72018d2 100644 --- a/tests/resources/test_resource_manager.py +++ b/tests/resources/test_resource_manager.py @@ -450,123 +450,6 @@ async def get_system_data(): assert admin_resources[0].name == "system_data" -class TestCustomResourceKeys: - """Test adding resources and templates with custom keys.""" - - def test_add_resource_with_custom_key(self, temp_file: Path): - """Test adding a resource with a custom key different from its URI.""" - manager = ResourceManager() - original_uri = "data://test/resource" - custom_key = "custom://resource/key" - - # Create a function resource instead of file resource to avoid path issues - async def get_data(): - return "Test data" - - resource = FunctionResource( - uri=AnyUrl(original_uri), - name="test_resource", - fn=get_data, - ) - - # Use model_copy to create a new resource with the custom key - resource_with_custom_key = resource.model_copy(key=custom_key) - manager.add_resource(resource_with_custom_key) - - # Resource should be accessible via custom key - assert custom_key in manager._resources - # But not via its original URI - assert original_uri not in manager._resources - # The resource's internal URI remains unchanged - assert str(manager._resources[custom_key].uri) == original_uri - - def test_add_template_with_custom_key(self): - """Test adding a template with a custom key different from its URI template.""" - manager = ResourceManager() - - def template_fn(id: str) -> str: - return f"Template {id}" - - original_uri_template = "test://{id}" - custom_key = "custom://{id}/template" - - template = ResourceTemplate.from_function( - fn=template_fn, - uri_template=original_uri_template, - name="test_template", - ) - - # Use model_copy to create a new template with the custom key - template_with_custom_key = template.model_copy(key=custom_key) - manager.add_template(template_with_custom_key) - - # Template should be accessible via custom key - assert custom_key in manager._templates - # But not via its original URI template - assert original_uri_template not in manager._templates - # The template's internal URI template remains unchanged - assert str(manager._templates[custom_key].uri_template) == original_uri_template - - async def test_get_resource_with_custom_key(self, temp_file: Path): - """Test that get_resource works with resources added with custom keys.""" - manager = ResourceManager() - original_uri = "data://test/resource" - custom_key = "custom://resource/path" - - # Create a function resource instead of file resource to avoid path issues - async def get_data(): - return "Test data" - - resource = FunctionResource( - uri=AnyUrl(original_uri), - name="test_resource", - fn=get_data, - ) - - # Use model_copy to create a new resource with the custom key - resource_with_custom_key = resource.model_copy(key=custom_key) - manager.add_resource(resource_with_custom_key) - - # Should be retrievable by the custom key - retrieved = await manager.get_resource(custom_key) - assert retrieved is not None - assert str(retrieved.uri) == original_uri - - # Should NOT be retrievable by the original URI - with pytest.raises(NotFoundError, match="Unknown resource"): - await manager.get_resource(original_uri) - - async def test_get_resource_from_template_with_custom_key(self): - """Test that templates with custom keys can create resources.""" - manager = ResourceManager() - - def greet(name: str) -> str: - return f"Hello, {name}!" - - original_template = "greet://{name}" - custom_key = "custom://greet/{name}" - - template = ResourceTemplate.from_function( - fn=greet, - uri_template=original_template, - name="custom_greeter", - ) - - # Use model_copy to create a new template with the custom key - template_with_custom_key = template.model_copy(key=custom_key) - manager.add_template(template_with_custom_key) - - # Using a URI that matches the custom key pattern - resource = await manager.get_resource("custom://greet/world") - assert isinstance(resource, FunctionResource) - result = await resource.read() - assert result.content == "Hello, world!" - - # Shouldn't work with the original template pattern - with pytest.raises(NotFoundError, match="Unknown resource"): - await manager.get_resource("greet://world") - - class TestQueryOnlyTemplates: """Test resource templates with only query parameters (no path params).""" diff --git a/tests/server/test_mount.py b/tests/server/test_mount.py index 7026429a5c..b52b2ed172 100644 --- a/tests/server/test_mount.py +++ b/tests/server/test_mount.py @@ -970,11 +970,11 @@ def hello(): assert lifespan_check == ["start"] -class TestResourceNamePrefixing: - """Test that resource and resource template names get prefixed when mounted.""" +class TestResourceUriPrefixing: + """Test that resource and resource template URIs get prefixed when mounted (names are NOT prefixed).""" - async def test_resource_name_prefixing(self): - """Test that resource names are prefixed when mounted.""" + async def test_resource_uri_prefixing(self): + """Test that resource URIs are prefixed when mounted (names are NOT prefixed).""" # Create a sub-app with a resource sub_app = FastMCP("SubApp") @@ -993,12 +993,12 @@ def my_resource() -> str: # Should have prefixed key (using path format: resource://prefix/resource_name) assert "resource://prefix/my_resource" in resources - # The resource name should also be prefixed + # The resource name should NOT be prefixed (only URI is prefixed) resource = resources["resource://prefix/my_resource"] - assert resource.name == "prefix_my_resource" + assert resource.name == "my_resource" - async def test_resource_template_name_prefixing(self): - """Test that resource template names are prefixed when mounted.""" + async def test_resource_template_uri_prefixing(self): + """Test that resource template URIs are prefixed when mounted (names are NOT prefixed).""" # Create a sub-app with a resource template sub_app = FastMCP("SubApp") @@ -1017,9 +1017,9 @@ def user_template(user_id: str) -> str: # Should have prefixed key (using path format: resource://prefix/template_uri) assert "resource://prefix/user/{user_id}" in templates - # The template name should also be prefixed + # The template name should NOT be prefixed (only URI template is prefixed) template = templates["resource://prefix/user/{user_id}"] - assert template.name == "prefix_user_template" + assert template.name == "user_template" class TestParentTagFiltering: diff --git a/tests/server/test_server.py b/tests/server/test_server.py index 22e884f0e7..69b608fa24 100644 --- a/tests/server/test_server.py +++ b/tests/server/test_server.py @@ -1335,48 +1335,6 @@ async def test_mounted_server_matching_and_stripping( # Test stripping assert remove_resource_prefix(uri, prefix) == expected_strip - async def test_import_server_with_new_prefix_format(self): - """Test that import_server correctly uses the new prefix format.""" - # Create a server with resources - source_server = FastMCP(name="SourceServer") - - @source_server.resource("resource://test-resource") - def get_resource(): - return "Resource content" - - @source_server.resource("resource:///absolute/path") - def get_absolute_resource(): - return "Absolute resource content" - - @source_server.resource("resource://{param}/template") - def get_template_resource(param: str): - return f"Template resource with {param}" - - # Create target server and import the source server - target_server = FastMCP(name="TargetServer") - await target_server.import_server(source_server, "imported") - - # Check that the resources were imported with the correct prefixes - resources = await target_server.get_resources() - templates = await target_server.get_resource_templates() - - assert "resource://imported/test-resource" in resources - assert "resource://imported//absolute/path" in resources - assert "resource://imported/{param}/template" in templates - - # Verify we can access the resources - async with Client(target_server) as client: - result = await client.read_resource("resource://imported/test-resource") - assert result[0].text == "Resource content" # type: ignore[attr-defined] - - result = await client.read_resource("resource://imported//absolute/path") - assert result[0].text == "Absolute resource content" # type: ignore[attr-defined] - - result = await client.read_resource( - "resource://imported/param-value/template" - ) - assert result[0].text == "Template resource with param-value" # type: ignore[attr-defined] - class TestShouldIncludeComponent: def test_no_filters_returns_true(self): diff --git a/tests/tools/test_tool_manager.py b/tests/tools/test_tool_manager.py index 500852c0a5..9843f5d751 100644 --- a/tests/tools/test_tool_manager.py +++ b/tests/tools/test_tool_manager.py @@ -869,27 +869,6 @@ def original_fn(x: int) -> int: with pytest.raises(NotFoundError, match="Tool 'original_fn' not found"): await manager.get_tool("original_fn") - async def test_add_tool_object_with_custom_key(self): - """Test adding a Tool object with a custom key using add_tool().""" - - def fn(x: int) -> int: - return x + 1 - - # Create a tool with a specific name - tool = Tool.from_function(fn, name="my_tool") - manager = ToolManager() - # Use model_copy to create a new tool with the custom key - tool_with_custom_key = tool.model_copy(key="proxy_tool") - manager.add_tool(tool_with_custom_key) - # The tool is accessible under the key - stored = await manager.get_tool("proxy_tool") - assert stored is not None - # But the tool's .name is unchanged - assert stored.name == "my_tool" - # The tool is not accessible under its original name - with pytest.raises(NotFoundError, match="Tool 'my_tool' not found"): - await manager.get_tool("my_tool") - async def test_call_tool_with_custom_name(self): """Test calling a tool added with a custom name.""" diff --git a/tests/utilities/test_components.py b/tests/utilities/test_components.py index 1211d1e1f7..330ab896ae 100644 --- a/tests/utilities/test_components.py +++ b/tests/utilities/test_components.py @@ -48,7 +48,7 @@ def basic_component(self): name="test_component", title="Test Component", description="A test component", - tags=["test", "component"], + tags={"test", "component"}, ) def test_initialization_with_minimal_params(self): @@ -68,7 +68,7 @@ def test_initialization_with_all_params(self): name="full", title="Full Component", description="A fully configured component", - tags=["tag1", "tag2"], + tags={"tag1", "tag2"}, meta=meta, enabled=False, ) @@ -83,12 +83,6 @@ def test_key_property_without_custom_key(self, basic_component): """Test that key property returns name when no custom key is set.""" assert basic_component.key == "test_component" - def test_key_property_with_custom_key(self): - """Test that key property returns custom key when set.""" - component = FastMCPComponent(name="test", key="custom_key") - assert component.key == "custom_key" - assert component.name == "test" - def test_get_meta_without_fastmcp_meta(self, basic_component): """Test get_meta without including fastmcp meta.""" basic_component.meta = {"custom": "data"} @@ -110,7 +104,7 @@ def test_get_meta_preserves_existing_fastmcp_meta(self): component = FastMCPComponent( name="test", meta={"_fastmcp": {"existing": "value"}}, - tags=["new_tag"], + tags={"new_tag"}, ) result = component.get_meta(include_fastmcp_meta=True) assert result is not None @@ -123,14 +117,6 @@ def test_get_meta_returns_none_when_empty(self): result = component.get_meta(include_fastmcp_meta=False) assert result is None - def test_model_copy_creates_copy_with_new_key(self, basic_component): - """Test that model_copy with key creates a copy with a new key.""" - new_component = basic_component.model_copy(key="new_key") - assert new_component.key == "new_key" - assert new_component.name == basic_component.name - assert new_component is not basic_component # Should be a copy - assert basic_component.key == "test_component" # Original unchanged - def test_equality_same_components(self): """Test that identical components are equal.""" comp1 = FastMCPComponent(name="test", description="desc") @@ -182,17 +168,17 @@ def test_copy_method(self, basic_component): assert basic_component.name == "test_component" def test_tags_deduplication(self): - """Test that tags are deduplicated.""" + """Test that tags are deduplicated when passed as a sequence.""" component = FastMCPComponent( name="test", - tags=["tag1", "tag2", "tag1", "tag2"], + tags=["tag1", "tag2", "tag1", "tag2"], # type: ignore[arg-type] ) assert component.tags == {"tag1", "tag2"} def test_validation_error_for_invalid_data(self): """Test that validation errors are raised for invalid data.""" with pytest.raises(ValidationError): - FastMCPComponent() # Missing required name field + FastMCPComponent() # type: ignore[call-arg] def test_extra_fields_forbidden(self): """Test that extra fields are not allowed.""" @@ -290,10 +276,6 @@ def test_all_fastmcp_component_features_work(self, mirrored_component): # Test key property assert mirrored_component.key == "mirrored" - # Test model_copy with key - with_key = mirrored_component.model_copy(key="new_key") - assert with_key.key == "new_key" - # Test get_meta mirrored_component.tags = {"tag1"} meta = mirrored_component.get_meta(include_fastmcp_meta=True) @@ -323,7 +305,7 @@ class TestEdgeCasesAndIntegration: def test_empty_tags_conversion(self): """Test that empty tags are handled correctly.""" - component = FastMCPComponent(name="test", tags=[]) + component = FastMCPComponent(name="test", tags=set()) assert component.tags == set() def test_tags_with_none_values(self): @@ -353,17 +335,17 @@ def test_component_with_complex_meta(self): component = FastMCPComponent(name="test", meta=complex_meta) assert component.meta == complex_meta - def test_model_copy_with_key_preserves_all_attributes(self): - """Test that model_copy with key preserves all component attributes.""" + def test_model_copy_preserves_all_attributes(self): + """Test that model_copy preserves all component attributes.""" component = FastMCPComponent( name="test", title="Title", description="Description", - tags=["tag1", "tag2"], + tags={"tag1", "tag2"}, meta={"key": "value"}, enabled=False, ) - new_component = component.model_copy(key="new_key") + new_component = component.model_copy() assert new_component.name == component.name assert new_component.title == component.title @@ -371,7 +353,7 @@ def test_model_copy_with_key_preserves_all_attributes(self): assert new_component.tags == component.tags assert new_component.meta == component.meta assert new_component.enabled == component.enabled - assert new_component.key == "new_key" + assert new_component.key == component.key def test_mirrored_component_copy_chain(self): """Test creating copies of copies for mirrored components.""" @@ -390,30 +372,34 @@ def test_mirrored_component_copy_chain(self): assert copy1.name == "copy1" assert copy2.name == "copy2" - def test_model_copy_with_update_and_key(self): - """Test that model_copy works with both update dict and key parameter.""" + def test_model_copy_with_update(self): + """Test that model_copy works with update dict.""" component = FastMCPComponent( name="test", title="Original Title", description="Original Description", - tags=["tag1"], + tags={"tag1"}, enabled=True, ) - # Test with both update and key + # Test with update (including name which affects .key) updated_component = component.model_copy( - update={"title": "New Title", "description": "New Description"}, - key="new_key", + update={ + "name": "new_name", + "title": "New Title", + "description": "New Description", + }, ) - assert updated_component.name == "test" # Not in update, unchanged + assert updated_component.name == "new_name" # Updated assert updated_component.title == "New Title" # Updated assert updated_component.description == "New Description" # Updated assert updated_component.tags == {"tag1"} # Not in update, unchanged assert updated_component.enabled is True # Not in update, unchanged - assert updated_component.key == "new_key" # Custom key set + assert updated_component.key == "new_name" # .key is computed from name # Original should be unchanged + assert component.name == "test" assert component.title == "Original Title" assert component.description == "Original Description" assert component.key == "test" # Uses name as key