From e1d72c63cb35b49372eef52055ee4d373a7d5ff6 Mon Sep 17 00:00:00 2001 From: Cosmos-Atom Date: Wed, 19 Aug 2026 13:33:50 +0530 Subject: [PATCH] feat: add MCP tools caching to list_mcp_tools Adds optional in-process result caching for list_mcp_tools() to avoid redundant MCP session round-trips in agentic loops where the tool list rarely changes. - New CacheOptions(ttl, max_size) dataclass accepted by list_mcp_tools(cache=...) - Results cached per filter + auth-type combination with monotonic TTL (default 600 s) - cache.evict() for caller-triggered invalidation - LRU eviction when max_size entries exceeded (default 32) - 19 new unit tests covering hit/miss, TTL expiry, LRU eviction, evict() - user-guide.md updated with usage examples and CacheOptions API reference Closes #178 --- pyproject.toml | 2 +- src/sap_cloud_sdk/agentgateway/__init__.py | 2 + src/sap_cloud_sdk/agentgateway/_models.py | 59 +++++- .../agentgateway/_tools_cache.py | 89 +++++++++ src/sap_cloud_sdk/agentgateway/agw_client.py | 46 ++++- src/sap_cloud_sdk/agentgateway/config.py | 2 + src/sap_cloud_sdk/agentgateway/user-guide.md | 50 ++++- .../core/telemetry/user-guide.md | 1 + .../outputmanagement/user-guide.md | 152 ++++++-------- tests/agentgateway/unit/test_tools_cache.py | 189 ++++++++++++++++++ uv.lock | 2 +- 11 files changed, 499 insertions(+), 95 deletions(-) create mode 100644 src/sap_cloud_sdk/agentgateway/_tools_cache.py create mode 100644 tests/agentgateway/unit/test_tools_cache.py diff --git a/pyproject.toml b/pyproject.toml index 55e8e297..f50b9557 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.44.0" +version = "0.45.0" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/src/sap_cloud_sdk/agentgateway/__init__.py b/src/sap_cloud_sdk/agentgateway/__init__.py index a8c0c850..03b0444d 100644 --- a/src/sap_cloud_sdk/agentgateway/__init__.py +++ b/src/sap_cloud_sdk/agentgateway/__init__.py @@ -54,6 +54,7 @@ from sap_cloud_sdk.agentgateway._models import ( AuthResult, + CacheOptions, MCPTool, MCPToolFilter, Agent, @@ -78,6 +79,7 @@ "ClientConfig", # Data models "AuthResult", + "CacheOptions", "MCPTool", "MCPToolFilter", "Agent", diff --git a/src/sap_cloud_sdk/agentgateway/_models.py b/src/sap_cloud_sdk/agentgateway/_models.py index 2dd56e87..b734adca 100644 --- a/src/sap_cloud_sdk/agentgateway/_models.py +++ b/src/sap_cloud_sdk/agentgateway/_models.py @@ -1,7 +1,17 @@ """Data models for Agent Gateway MCP tools.""" +from __future__ import annotations + from dataclasses import dataclass, field -from typing import Any +from typing import TYPE_CHECKING, Any + +from sap_cloud_sdk.agentgateway.config import ( + DEFAULT_MAX_MCP_TOOLS_CACHE_SIZE, + DEFAULT_MCP_TOOLS_CACHE_TTL_SECONDS, +) + +if TYPE_CHECKING: + from sap_cloud_sdk.agentgateway._tools_cache import MCPToolsCache @dataclass @@ -185,3 +195,50 @@ class MCPToolFilter: names: list[str] = field(default_factory=list) ord_ids: list[str] = field(default_factory=list) + + +class CacheOptions: + """Options for caching the result of list_mcp_tools. + + Pass an instance to list_mcp_tools(cache=...) to enable result caching. + The same instance can be reused across calls — cache state is stored on + it. Call evict() to force a fresh fetch on the next call. + + Args: + ttl: Cache lifetime in seconds. Defaults to 600 s. + max_size: Maximum number of distinct cached entries (keyed by filter + combo + auth type). Oldest entry is evicted when the limit is + exceeded. Defaults to 32. + + Example: + ```python + from sap_cloud_sdk.agentgateway import CacheOptions + + cache = CacheOptions(ttl=300) + tools = await agw_client.list_mcp_tools(cache=cache) + + # Later — force a fresh fetch (e.g. after a tool was added): + cache.evict() + tools = await agw_client.list_mcp_tools(cache=cache) + ``` + + Note: + Cache is in-process only. It is not shared across client instances, + processes, or Kubernetes pods. Two concurrent calls that both miss + the cache will both fetch independently — the last writer wins, no + data corruption occurs. + """ + + def __init__( + self, + ttl: float = DEFAULT_MCP_TOOLS_CACHE_TTL_SECONDS, + max_size: int = DEFAULT_MAX_MCP_TOOLS_CACHE_SIZE, + ) -> None: + self.ttl = ttl + self.max_size = max_size + self._cache: MCPToolsCache | None = None + + def evict(self) -> None: + """Clear all cached tool list entries. Forces a fresh fetch on the next call.""" + if self._cache is not None: + self._cache.evict() diff --git a/src/sap_cloud_sdk/agentgateway/_tools_cache.py b/src/sap_cloud_sdk/agentgateway/_tools_cache.py new file mode 100644 index 00000000..08d29355 --- /dev/null +++ b/src/sap_cloud_sdk/agentgateway/_tools_cache.py @@ -0,0 +1,89 @@ +"""Result cache for MCP tool lists. + +Caches list[MCPTool] per (filter, auth-type) key to avoid redundant MCP +session round-trips during agentic loops. Bounded by max_size with LRU +eviction; each entry has a monotonic TTL. + +Thread safety: +CPython GIL makes individual OrderedDict operations atomic, but compound +check-then-set is not. Two concurrent coroutines for the same key may both +miss and both fetch; the race produces redundant tool-list requests, not +data corruption. This matches the accepted behaviour in _token_cache.py. +""" + +import logging +import time +from collections import OrderedDict +from dataclasses import dataclass + +from sap_cloud_sdk.agentgateway._models import CacheOptions, MCPTool, MCPToolFilter + +logger = logging.getLogger(__name__) + + +@dataclass +class _CachedToolList: + tools: list[MCPTool] + expires_at: float # time.monotonic() value + + def is_valid(self) -> bool: + return time.monotonic() < self.expires_at + + +def _make_cache_key(filter: MCPToolFilter | None, user_scoped: bool) -> str: + """Build a stable string key from filter options and auth type.""" + ord_ids = "|".join(sorted(filter.ord_ids)) if filter and filter.ord_ids else "" + names = "|".join(sorted(filter.names)) if filter and filter.names else "" + auth = "user" if user_scoped else "system" + return f"{auth}:ord={ord_ids}:names={names}" + + +class MCPToolsCache: + """TTL + LRU cache for MCP tool list results. + + Keyed by (filter combo, auth type). Entries expire after `options.ttl` + seconds. When the number of entries exceeds `options.max_size`, the + least-recently-used entry is evicted. + + Callers hold a reference to their CacheOptions instance and call + evict() to invalidate all entries. + """ + + def __init__(self) -> None: + self._entries: OrderedDict[str, _CachedToolList] = OrderedDict() + + def get( + self, + filter: MCPToolFilter | None, + user_scoped: bool, + ) -> list[MCPTool] | None: + """Return cached tools for the given filter/auth combo, or None if miss/expired.""" + key = _make_cache_key(filter, user_scoped) + entry = self._entries.get(key) + if entry and entry.is_valid(): + self._entries.move_to_end(key) + return entry.tools + if entry: + del self._entries[key] + return None + + def set( + self, + tools: list[MCPTool], + filter: MCPToolFilter | None, + user_scoped: bool, + options: CacheOptions, + ) -> None: + """Store tools under the given filter/auth key, evicting LRU if at capacity.""" + key = _make_cache_key(filter, user_scoped) + expires_at = time.monotonic() + options.ttl + self._entries[key] = _CachedToolList(tools=tools, expires_at=expires_at) + self._entries.move_to_end(key) + while len(self._entries) > options.max_size: + evicted, _ = self._entries.popitem(last=False) + logger.debug("MCP tools cache full — evicted key '%s'", evicted) + + def evict(self) -> None: + """Clear all cached entries. Forces a fresh fetch on the next call.""" + self._entries.clear() + logger.debug("MCP tools cache evicted") diff --git a/src/sap_cloud_sdk/agentgateway/agw_client.py b/src/sap_cloud_sdk/agentgateway/agw_client.py index fb1d63f8..a3ba9be3 100644 --- a/src/sap_cloud_sdk/agentgateway/agw_client.py +++ b/src/sap_cloud_sdk/agentgateway/agw_client.py @@ -34,10 +34,12 @@ Agent, AgentCardFilter, AuthResult, + CacheOptions, MCPTool, MCPToolFilter, ) from sap_cloud_sdk.agentgateway._token_cache import _GatewayUrlCache, _TokenCache +from sap_cloud_sdk.agentgateway._tools_cache import MCPToolsCache from sap_cloud_sdk.agentgateway.exceptions import AgentGatewaySDKError from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics @@ -358,6 +360,7 @@ async def list_mcp_tools( self, user_token: str | Callable[[], str] | None = None, filter: MCPToolFilter | None = None, + cache: CacheOptions | None = None, ) -> list[MCPTool]: """List all MCP tools from MCP servers. @@ -378,6 +381,11 @@ async def list_mcp_tools( If provided, uses user-scoped auth instead of system auth. filter: Optional filter to narrow results by tool name or ORD ID. If None or empty, all tools are included. + cache: Optional caching options. When provided, tool lists are cached + in-process for ``cache.ttl`` seconds (default 600 s). Distinct filter + and auth-type combinations are cached independently, up to + ``cache.max_size`` entries (LRU eviction). Call ``cache.evict()`` to + clear all entries and force a fresh fetch on the next call. Returns: List of MCPTool objects from all MCP servers. @@ -402,9 +410,32 @@ async def list_mcp_tools( ord_ids=["sap.s4:apiAccess:salesOrder:v1"], ) ) + + # With caching — avoids redundant MCP round-trips: + from sap_cloud_sdk.agentgateway import CacheOptions + cache = CacheOptions(ttl=300) + tools = await agw_client.list_mcp_tools(cache=cache) + + # Force a fresh fetch (e.g. after a tool was added on the server): + cache.evict() + tools = await agw_client.list_mcp_tools(cache=cache) ``` """ try: + user_scoped = bool(user_token) + + if cache is not None: + if cache._cache is None: + cache._cache = MCPToolsCache() + tools_cache: MCPToolsCache | None = cache._cache + cache_opts: CacheOptions | None = cache + cached = tools_cache.get(filter, user_scoped) + if cached is not None: + return cached + else: + tools_cache = None + cache_opts = None + if user_token: auth = await self.get_user_auth(user_token) else: @@ -417,23 +448,29 @@ async def list_mcp_tools( "Customer agent credentials detected at '%s'", credentials_path ) credentials = load_customer_credentials(credentials_path) - return await get_mcp_tools_customer( + tools = await get_mcp_tools_customer( credentials, auth.access_token, self._config.timeout, filter=filter, ) + if tools_cache is not None and cache_opts is not None: + tools_cache.set(tools, filter, user_scoped, cache_opts) + return tools # Check for transparent mode if detect_transparent_credentials(): logger.info(_LOG_TRANSPARENT_MODE) credentials = load_customer_credentials_from_env() - return await get_mcp_tools_customer( + tools = await get_mcp_tools_customer( credentials, auth.access_token, self._config.timeout, filter=filter, ) + if tools_cache is not None and cache_opts is not None: + tools_cache.set(tools, filter, user_scoped, cache_opts) + return tools # LoB flow - requires tenant_subdomain tenant = self._resolve_tenant_subdomain() @@ -441,12 +478,15 @@ async def list_mcp_tools( auth = await self.get_user_auth(user_token) else: auth = await self.get_system_auth() - return await get_mcp_tools_lob( + tools = await get_mcp_tools_lob( tenant, auth.access_token, self._config.timeout, filter=filter, ) + if tools_cache is not None and cache_opts is not None: + tools_cache.set(tools, filter, user_scoped, cache_opts) + return tools except AgentGatewaySDKError: raise diff --git a/src/sap_cloud_sdk/agentgateway/config.py b/src/sap_cloud_sdk/agentgateway/config.py index 17495dbd..829fb463 100644 --- a/src/sap_cloud_sdk/agentgateway/config.py +++ b/src/sap_cloud_sdk/agentgateway/config.py @@ -7,6 +7,8 @@ DEFAULT_TOKEN_EXPIRY_BUFFER_SECONDS = 30.0 DEFAULT_MAX_SYSTEM_TOKEN_CACHE_SIZE = 32 DEFAULT_MAX_USER_TOKEN_CACHE_SIZE = 256 +DEFAULT_MCP_TOOLS_CACHE_TTL_SECONDS = 600.0 +DEFAULT_MAX_MCP_TOOLS_CACHE_SIZE = 32 @dataclass diff --git a/src/sap_cloud_sdk/agentgateway/user-guide.md b/src/sap_cloud_sdk/agentgateway/user-guide.md index ab3cd3f5..3f20b2f0 100644 --- a/src/sap_cloud_sdk/agentgateway/user-guide.md +++ b/src/sap_cloud_sdk/agentgateway/user-guide.md @@ -95,6 +95,36 @@ agents = await agw_client.list_agent_cards( ) ``` +### Caching Tool Lists + +In agentic loops, `list_mcp_tools()` can be called repeatedly. By default every call opens fresh MCP sessions — expensive for a tool list that rarely changes. Pass a `CacheOptions` instance to cache results in-process. + +```python +from sap_cloud_sdk.agentgateway import CacheOptions, create_client + +agw_client = create_client(tenant_subdomain="my-tenant") +cache = CacheOptions(ttl=300) # cache for 5 minutes + +# First call fetches from network and stores in cache +tools = await agw_client.list_mcp_tools(cache=cache) + +# Subsequent calls within TTL return immediately — no network round-trip +tools = await agw_client.list_mcp_tools(cache=cache) + +# Force a fresh fetch (e.g. after a tool was added on the server): +cache.evict() +tools = await agw_client.list_mcp_tools(cache=cache) +``` + +The cache is scoped to the `CacheOptions` instance — different instances don't share state. Distinct filter and auth-type combinations are cached as independent entries, up to `max_size` entries total (LRU eviction when the limit is hit). + +```python +# Custom TTL and size cap +cache = CacheOptions(ttl=600, max_size=10) +``` + +The cache is **in-process only** — not shared across client instances, processes, or Kubernetes pods. + ### LangChain Integration Convert MCP tools to LangChain `StructuredTool` objects for use with LangChain agents: @@ -221,6 +251,7 @@ class AgentGatewayClient: self, user_token: str | Callable[[], str] | None = None, filter: MCPToolFilter | None = None, + cache: CacheOptions | None = None, ) -> list[MCPTool] async def call_mcp_tool( @@ -271,9 +302,9 @@ Both fields default to empty lists. `agent_names` is applied after fetching; `or from sap_cloud_sdk.agentgateway import MCPToolFilter MCPToolFilter( - names=[], # tool names to include (matched against MCPTool.name); empty = no filter + names=[], # tool names to include (matched against MCPTool.name); empty = no filter ord_ids=[], # ORD IDs to include (extracted from fragment URL for LoB, or matched - # against IntegrationDependency.ord_id for customer agents); empty = no filter + # against IntegrationDependency.ord_id for customer agents); empty = no filter ) ``` @@ -281,6 +312,21 @@ Both fields default to empty lists. `names` is applied after fetching; `ord_ids` > Both filter classes use AND semantics: if both fields are set, a result must match all of them to be included. +### CacheOptions + +```python +from sap_cloud_sdk.agentgateway import CacheOptions + +CacheOptions( + ttl=600.0, # cache lifetime in seconds; default 600 + max_size=32, # max distinct cached entries (LRU eviction); default 32 +) +``` + +- `ttl`: How long a cached tool list is considered valid. After expiry the next call fetches fresh from the network. +- `max_size`: Cap on how many distinct entries (filter + auth-type combinations) are held in memory. When exceeded, the least-recently-used entry is evicted. +- `.evict()`: Clears all entries immediately, forcing a fresh fetch on the next call. + ### Data Models ```python diff --git a/src/sap_cloud_sdk/core/telemetry/user-guide.md b/src/sap_cloud_sdk/core/telemetry/user-guide.md index 4af91c47..d64d0535 100644 --- a/src/sap_cloud_sdk/core/telemetry/user-guide.md +++ b/src/sap_cloud_sdk/core/telemetry/user-guide.md @@ -295,6 +295,7 @@ from litellm import completion logger = logging.getLogger(__name__) + async def handle_request(query: str, user_id: str): set_tenant_id("bh7sjh...") diff --git a/src/sap_cloud_sdk/outputmanagement/user-guide.md b/src/sap_cloud_sdk/outputmanagement/user-guide.md index 99c3435c..a13b0e6f 100644 --- a/src/sap_cloud_sdk/outputmanagement/user-guide.md +++ b/src/sap_cloud_sdk/outputmanagement/user-guide.md @@ -68,9 +68,9 @@ response = client.send_email( "PurchaseOrder": { "orderId": "PO-12345", "vendor": "ACME Corp", - "total": 1500.00 + "total": 1500.00, } - } + }, ) # Check the result @@ -94,7 +94,7 @@ response = client.send_email( to=["user@example.com"], business_document={"Document": {"id": "123"}}, cc=["manager@example.com"], # Optional - template_language="en" # Optional, default: "en" + template_language="en", # Optional, default: "en" ) ``` @@ -103,18 +103,13 @@ response = client.send_email( response = client.send_email( notification_template_key="INVOICE_NOTIFICATION", to=["customer@example.com"], - business_document={ - "Invoice": { - "invoiceNumber": "INV-2024-001", - "amount": 5000.00 - } - }, + business_document={"Invoice": {"invoiceNumber": "INV-2024-001", "amount": 5000.00}}, cc=["accounting@company.com"], template_language="en", attachment_urls=[ "https://dms.example.com/browser/root?objectId=12345&cmisselector=content", - "https://dms.example.com/browser/root?objectId=67890&cmisselector=content" - ] + "https://dms.example.com/browser/root?objectId=67890&cmisselector=content", + ], ) ``` @@ -128,7 +123,7 @@ output_request = client.create_output_request( business_document={"Document": {"id": "123"}}, cc=["manager@example.com"], # Optional template_language="en", # Optional - attachment_urls=["https://dms.example.com/..."] # Optional + attachment_urls=["https://dms.example.com/..."], # Optional ) # Inspect or modify the request @@ -156,7 +151,7 @@ response = await client.send_email_with_mcp( notification_template_key="TEMPLATE_KEY", to_emails=["user@example.com"], business_document={"Document": {"id": "123"}}, - mcp_tool=mcp_tool_instance + mcp_tool=mcp_tool_instance, ) ``` @@ -167,17 +162,14 @@ response = await client.send_email_with_mcp( notification_template_key="CONTRACT_NOTIFICATION", to_emails=["legal@company.com"], business_document={ - "Contract": { - "contractId": "CNT-2024-100", - "partyName": "Partner Corp" - } + "Contract": {"contractId": "CNT-2024-100", "partyName": "Partner Corp"} }, cc_email="manager@company.com", attachment_urls=[ "https://dms.example.com/browser/root?objectId=12345&cmisselector=content", - "https://dms.example.com/browser/root?objectId=67890&cmisselector=content" + "https://dms.example.com/browser/root?objectId=67890&cmisselector=content", ], - mcp_tool=mcp_tool_instance + mcp_tool=mcp_tool_instance, ) ``` @@ -202,9 +194,9 @@ response = client.send_email( "orderId": "ORD-789", "customerName": "John Doe", "orderDate": "2024-01-15", - "totalAmount": 2500.00 + "totalAmount": 2500.00, } - } + }, ) if response.error: @@ -230,9 +222,9 @@ response = client.send_email( "Invoice": { "invoiceNumber": "INV-2024-001", "amount": 5000.00, - "dueDate": "2024-02-15" + "dueDate": "2024-02-15", } - } + }, ) ``` @@ -248,13 +240,8 @@ client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") response = client.send_email( notification_template_key="WELCOME_EMAIL", to=["user@example.com"], - business_document={ - "User": { - "userId": "U12345", - "name": "Jane Smith" - } - }, - template_language="de" # German template + business_document={"User": {"userId": "U12345", "name": "Jane Smith"}}, + template_language="de", # German template ) ``` @@ -273,14 +260,11 @@ response = client.send_email( notification_template_key="CONTRACT_NOTIFICATION", to=["legal@company.com"], business_document={ - "Contract": { - "contractId": "CNT-2024-100", - "partyName": "Partner Corp" - } + "Contract": {"contractId": "CNT-2024-100", "partyName": "Partner Corp"} }, attachment_urls=[ "https://dms.example.com/browser/root?objectId=12345&cmisselector=content" - ] + ], ) ``` @@ -297,17 +281,13 @@ response = client.send_email( notification_template_key="REPORT_PACKAGE", to=["management@company.com"], business_document={ - "Report": { - "reportId": "RPT-Q1-2024", - "quarter": "Q1", - "year": 2024 - } + "Report": {"reportId": "RPT-Q1-2024", "quarter": "Q1", "year": 2024} }, attachment_urls=[ "https://dms.example.com/browser/root?objectId=12345&cmisselector=content", "https://dms.example.com/browser/root?objectId=67890&cmisselector=content", - "https://dms.example.com/browser/root?objectId=11111&cmisselector=content" - ] + "https://dms.example.com/browser/root?objectId=11111&cmisselector=content", + ], ) ``` @@ -328,7 +308,7 @@ client = create_client() client = create_client( destination_name="ARIBA_OUTPUT_SERVICE", access_strategy="PROVIDER_ONLY", - instance="default" + instance="default", ) ``` @@ -341,14 +321,12 @@ from sap_cloud_sdk.outputmanagement import create_client # Provider-only access (default) client = create_client( - destination_name="ARIBA_OUTPUT_SERVICE", - access_strategy="PROVIDER_ONLY" + destination_name="ARIBA_OUTPUT_SERVICE", access_strategy="PROVIDER_ONLY" ) # Subscriber-only access client = create_client( - destination_name="ARIBA_OUTPUT_SERVICE", - access_strategy="SUBSCRIBER_ONLY" + destination_name="ARIBA_OUTPUT_SERVICE", access_strategy="SUBSCRIBER_ONLY" ) ``` @@ -360,8 +338,7 @@ Specify a custom destination service instance: from sap_cloud_sdk.outputmanagement import create_client client = create_client( - destination_name="ARIBA_OUTPUT_SERVICE", - instance="my-custom-instance" + destination_name="ARIBA_OUTPUT_SERVICE", instance="my-custom-instance" ) ``` @@ -378,15 +355,12 @@ client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") output_request = client.create_output_request( notification_template_key="CUSTOM_NOTIFICATION", to=["recipient@example.com"], - business_document={ - "CustomDocument": { - "id": "DOC-456", - "type": "Important" - } - }, + business_document={"CustomDocument": {"id": "DOC-456", "type": "Important"}}, cc=["supervisor@example.com"], template_language="en", - attachment_urls=["https://dms.example.com/browser/root?objectId=999&cmisselector=content"] + attachment_urls=[ + "https://dms.example.com/browser/root?objectId=999&cmisselector=content" + ], ) # Step 2: Inspect or modify the request if needed @@ -411,7 +385,7 @@ client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") response = client.send_email( notification_template_key="NOTIFICATION", to=["user@example.com"], - business_document={"Document": {"id": "123"}} + business_document={"Document": {"id": "123"}}, ) if response.error: @@ -433,7 +407,7 @@ client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") response = client.send_email( notification_template_key="", # Invalid: empty template key to=[], # Invalid: no recipients - business_document={} # Invalid: empty document + business_document={}, # Invalid: empty document ) if response.error: @@ -456,7 +430,7 @@ try: response = client.send_email( notification_template_key="NOTIFICATION", to=["user@example.com"], - business_document={"Document": {"id": "123"}} + business_document={"Document": {"id": "123"}}, ) if response.error: @@ -491,13 +465,15 @@ for order in orders: response = client.send_email( notification_template_key="ORDER_CONFIRMATION", to=[order.customer_email], - business_document={"Order": order.to_dict()} + business_document={"Order": order.to_dict()}, ) if response.error: print(f"Failed to send email for order {order.id}: {response.error.message}") else: - print(f"Email sent for order {order.id}, Request ID: {response.outputRequestId}") + print( + f"Email sent for order {order.id}, Request ID: {response.outputRequestId}" + ) ``` ### 2. Validate Input Before Sending @@ -507,6 +483,7 @@ Validate your data before calling the API: ```python from sap_cloud_sdk.outputmanagement import create_client + def send_order_confirmation(order): # Validate input if not order.customer_email: @@ -521,12 +498,7 @@ def send_order_confirmation(order): response = client.send_email( notification_template_key="ORDER_CONFIRMATION", to=[order.customer_email], - business_document={ - "Order": { - "orderId": order.order_id, - "total": order.total - } - } + business_document={"Order": {"orderId": order.order_id, "total": order.total}}, ) return response @@ -542,7 +514,7 @@ business_document = { "Invoice": { "invoiceNumber": "INV-2024-001", # Clear identifier "customerId": "CUST-12345", - "amount": 1000.00 + "amount": 1000.00, } } @@ -550,7 +522,7 @@ business_document = { business_document = { "Invoice": { "id": "123", # Too generic - "amount": 1000.00 + "amount": 1000.00, } } ``` @@ -563,6 +535,7 @@ Always handle errors and provide meaningful feedback: from sap_cloud_sdk.outputmanagement import create_client import time + def send_notification_with_retry(template_key, recipients, document, max_retries=3): client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") @@ -571,14 +544,14 @@ def send_notification_with_retry(template_key, recipients, document, max_retries response = client.send_email( notification_template_key=template_key, to=recipients, - business_document=document + business_document=document, ) if response.error: if response.error.code in ["NETWORK_ERROR", "SERVICE_UNAVAILABLE"]: if attempt < max_retries - 1: print(f"Retrying... (attempt {attempt + 1}/{max_retries})") - time.sleep(2 ** attempt) # Exponential backoff + time.sleep(2**attempt) # Exponential backoff continue print(f"Failed to send email: {response.error.message}") @@ -588,8 +561,10 @@ def send_notification_with_retry(template_key, recipients, document, max_retries except Exception as e: if attempt < max_retries - 1: - print(f"Error occurred, retrying... (attempt {attempt + 1}/{max_retries})") - time.sleep(2 ** attempt) + print( + f"Error occurred, retrying... (attempt {attempt + 1}/{max_retries})" + ) + time.sleep(2**attempt) continue raise @@ -611,7 +586,7 @@ client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") response = client.send_email( notification_template_key="NOTIFICATION", to=["user@example.com"], - business_document={"Document": {"id": "123"}} + business_document={"Document": {"id": "123"}}, ) if response.error: @@ -647,7 +622,7 @@ from sap_cloud_sdk.outputmanagement import create_client client = create_client( destination_name="ARIBA_OUTPUT_SERVICE", access_strategy="PROVIDER_ONLY", - instance="default" + instance="default", ) # Using environment variables @@ -682,7 +657,7 @@ response = client.send_email( business_document={"Document": {"id": "123"}}, cc=["manager@example.com"], template_language="en", - attachment_urls=["https://dms.example.com/..."] + attachment_urls=["https://dms.example.com/..."], ) ``` @@ -710,7 +685,7 @@ output_request = client.create_output_request( notification_template_key="NOTIFICATION", to=["user@example.com"], business_document={"Document": {"id": "123"}}, - cc=["manager@example.com"] + cc=["manager@example.com"], ) ``` @@ -760,7 +735,7 @@ response = await client.send_email_with_mcp( notification_template_key="NOTIFICATION", to_emails=["user@example.com"], business_document={"Document": {"id": "123"}}, - mcp_tool=mcp_tool_instance + mcp_tool=mcp_tool_instance, ) ``` @@ -771,6 +746,7 @@ response = await client.send_email_with_mcp( ```python from sap_cloud_sdk.outputmanagement import create_client + def send_order_confirmation(order): client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") @@ -788,12 +764,12 @@ def send_order_confirmation(order): { "productName": item.product_name, "quantity": item.quantity, - "price": float(item.price) + "price": float(item.price), } for item in order.items - ] + ], } - } + }, ) return response @@ -804,6 +780,7 @@ def send_order_confirmation(order): ```python from sap_cloud_sdk.outputmanagement import create_client + def send_invoice_with_pdf(invoice, pdf_dms_url): client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") @@ -817,10 +794,10 @@ def send_invoice_with_pdf(invoice, pdf_dms_url): "invoiceDate": invoice.date.isoformat(), "dueDate": invoice.due_date.isoformat(), "amount": float(invoice.amount), - "currency": invoice.currency + "currency": invoice.currency, } }, - attachment_urls=[pdf_dms_url] + attachment_urls=[pdf_dms_url], ) return response @@ -831,6 +808,7 @@ def send_invoice_with_pdf(invoice, pdf_dms_url): ```python from sap_cloud_sdk.outputmanagement import create_client + def send_bulk_notification(recipients, notification_data): client = create_client(destination_name="ARIBA_OUTPUT_SERVICE") @@ -842,9 +820,9 @@ def send_bulk_notification(recipients, notification_data): "notificationId": notification_data["id"], "title": notification_data["title"], "message": notification_data["message"], - "timestamp": notification_data["timestamp"] + "timestamp": notification_data["timestamp"], } - } + }, ) return response @@ -901,7 +879,7 @@ from sap_cloud_sdk.outputmanagement import ( ValidationException, NetworkException, DestinationNotFoundException, - DestinationAccessException + DestinationAccessException, ) try: @@ -910,7 +888,7 @@ try: response = client.send_email( notification_template_key="NOTIFICATION", to=["user@example.com"], - business_document={"Document": {"id": "123"}} + business_document={"Document": {"id": "123"}}, ) if response.error: diff --git a/tests/agentgateway/unit/test_tools_cache.py b/tests/agentgateway/unit/test_tools_cache.py new file mode 100644 index 00000000..0ef0f395 --- /dev/null +++ b/tests/agentgateway/unit/test_tools_cache.py @@ -0,0 +1,189 @@ +"""Unit tests for MCPToolsCache.""" + +import time +from unittest.mock import patch + +import pytest + +from sap_cloud_sdk.agentgateway._models import CacheOptions, MCPTool, MCPToolFilter +from sap_cloud_sdk.agentgateway._tools_cache import MCPToolsCache, _make_cache_key + + +def _tool(name: str) -> MCPTool: + return MCPTool( + name=name, + server_name="test-server", + description="desc", + input_schema={}, + url="https://example.com/mcp", + ) + + +TOOLS_A = [_tool("tool-a")] +TOOLS_B = [_tool("tool-b"), _tool("tool-c")] +DEFAULT_OPTIONS = CacheOptions() + + +class TestCacheKey: + def test_system_and_user_produce_different_keys(self): + assert _make_cache_key(None, False) != _make_cache_key(None, True) + + def test_filter_ord_ids_included_in_key(self): + f = MCPToolFilter(ord_ids=["sap.s4:v1", "sap.crm:v2"]) + key = _make_cache_key(f, False) + assert "sap.s4:v1" in key + assert "sap.crm:v2" in key + + def test_filter_ord_ids_sorted_for_stability(self): + f1 = MCPToolFilter(ord_ids=["b", "a"]) + f2 = MCPToolFilter(ord_ids=["a", "b"]) + assert _make_cache_key(f1, False) == _make_cache_key(f2, False) + + def test_filter_names_sorted_for_stability(self): + f1 = MCPToolFilter(names=["z", "a"]) + f2 = MCPToolFilter(names=["a", "z"]) + assert _make_cache_key(f1, False) == _make_cache_key(f2, False) + + def test_none_filter_and_empty_filter_same_key(self): + assert _make_cache_key(None, False) == _make_cache_key(MCPToolFilter(), False) + + def test_different_filters_different_keys(self): + f1 = MCPToolFilter(names=["get-order"]) + f2 = MCPToolFilter(names=["create-order"]) + assert _make_cache_key(f1, False) != _make_cache_key(f2, False) + + +class TestCacheHitAndMiss: + def test_miss_on_empty_cache(self): + c = MCPToolsCache() + assert c.get(None, False) is None + + def test_hit_after_set(self): + c = MCPToolsCache() + c.set(TOOLS_A, None, False, DEFAULT_OPTIONS) + result = c.get(None, False) + assert result == TOOLS_A + + def test_miss_for_different_filter(self): + c = MCPToolsCache() + c.set(TOOLS_A, None, False, DEFAULT_OPTIONS) + assert c.get(MCPToolFilter(names=["other"]), False) is None + + def test_miss_for_different_auth_type(self): + c = MCPToolsCache() + c.set(TOOLS_A, None, False, DEFAULT_OPTIONS) + assert c.get(None, True) is None + + def test_independent_entries_for_different_filters(self): + c = MCPToolsCache() + f1 = MCPToolFilter(names=["tool-a"]) + f2 = MCPToolFilter(names=["tool-b"]) + c.set(TOOLS_A, f1, False, DEFAULT_OPTIONS) + c.set(TOOLS_B, f2, False, DEFAULT_OPTIONS) + assert c.get(f1, False) == TOOLS_A + assert c.get(f2, False) == TOOLS_B + + +class TestTTLExpiry: + def test_expired_entry_returns_none(self): + c = MCPToolsCache() + options = CacheOptions(ttl=1.0) + c.set(TOOLS_A, None, False, options) + with patch("sap_cloud_sdk.agentgateway._tools_cache.time") as mock_time: + mock_time.monotonic.return_value = time.monotonic() + 2.0 + assert c.get(None, False) is None + + def test_valid_entry_within_ttl_is_returned(self): + c = MCPToolsCache() + options = CacheOptions(ttl=600.0) + c.set(TOOLS_A, None, False, options) + assert c.get(None, False) == TOOLS_A + + def test_expired_entry_is_removed_from_cache(self): + c = MCPToolsCache() + options = CacheOptions(ttl=1.0) + c.set(TOOLS_A, None, False, options) + with patch("sap_cloud_sdk.agentgateway._tools_cache.time") as mock_time: + mock_time.monotonic.return_value = time.monotonic() + 2.0 + c.get(None, False) + assert len(c._entries) == 0 + + +class TestLruEviction: + def test_lru_entry_evicted_when_full(self): + options = CacheOptions(max_size=2) + c = MCPToolsCache() + f1 = MCPToolFilter(names=["a"]) + f2 = MCPToolFilter(names=["b"]) + f3 = MCPToolFilter(names=["c"]) + + c.set(TOOLS_A, f1, False, options) + c.set(TOOLS_A, f2, False, options) + # f1 is now LRU — adding f3 should evict it + c.set(TOOLS_A, f3, False, options) + + assert c.get(f1, False) is None # evicted + assert c.get(f2, False) == TOOLS_A + assert c.get(f3, False) == TOOLS_A + + def test_get_promotes_entry_to_mru(self): + options = CacheOptions(max_size=2) + c = MCPToolsCache() + f1 = MCPToolFilter(names=["a"]) + f2 = MCPToolFilter(names=["b"]) + f3 = MCPToolFilter(names=["c"]) + + c.set(TOOLS_A, f1, False, options) + c.set(TOOLS_A, f2, False, options) + # Access f1 to make it MRU; f2 becomes LRU + c.get(f1, False) + c.set(TOOLS_A, f3, False, options) + + assert c.get(f1, False) == TOOLS_A # promoted — not evicted + assert c.get(f2, False) is None # evicted + + +class TestEvict: + def test_evict_clears_all_entries(self): + c = MCPToolsCache() + c.set(TOOLS_A, None, False, DEFAULT_OPTIONS) + c.set(TOOLS_B, None, True, DEFAULT_OPTIONS) + c.evict() + assert c.get(None, False) is None + assert c.get(None, True) is None + + def test_evict_on_empty_cache_is_noop(self): + c = MCPToolsCache() + c.evict() # should not raise + assert len(c._entries) == 0 + + def test_set_after_evict_works(self): + c = MCPToolsCache() + c.set(TOOLS_A, None, False, DEFAULT_OPTIONS) + c.evict() + c.set(TOOLS_B, None, False, DEFAULT_OPTIONS) + assert c.get(None, False) == TOOLS_B + + +class TestCacheOptionsEvict: + def test_evict_before_first_use_is_noop(self): + cache = CacheOptions() + cache.evict() # _cache is None — should not raise + + def test_evict_clears_entries_via_cache_options(self): + cache = CacheOptions() + cache._cache = MCPToolsCache() + cache._cache.set(TOOLS_A, None, False, cache) + cache.evict() + assert cache._cache.get(None, False) is None + + def test_cache_options_defaults(self): + cache = CacheOptions() + assert cache.ttl == 600.0 + assert cache.max_size == 32 + assert cache._cache is None + + def test_cache_options_custom_values(self): + cache = CacheOptions(ttl=120.0, max_size=5) + assert cache.ttl == 120.0 + assert cache.max_size == 5 diff --git a/uv.lock b/uv.lock index f09b53da..2c98946e 100644 --- a/uv.lock +++ b/uv.lock @@ -3925,7 +3925,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.44.0" +version = "0.45.0" source = { editable = "." } dependencies = [ { name = "cryptography" },