From dba3fdfadf2cfafbfce83c4e966bb9cbffed4a44 Mon Sep 17 00:00:00 2001 From: Antonio Rivero <38889534+Antonio-RiveroMartnez@users.noreply.github.com> Date: Thu, 11 Dec 2025 13:08:09 +0100 Subject: [PATCH] feat(mcp): Caching uses in-memory store by default when no external store is configured (#36527) --- superset/mcp_service/caching.py | 33 +++++---- superset/mcp_service/mcp_config.py | 71 ++++++++++++++++--- .../mcp_service/test_mcp_caching.py | 66 ++++++++++++++--- 3 files changed, 140 insertions(+), 30 deletions(-) diff --git a/superset/mcp_service/caching.py b/superset/mcp_service/caching.py index d50f2761ab4..e8b309543f7 100644 --- a/superset/mcp_service/caching.py +++ b/superset/mcp_service/caching.py @@ -79,10 +79,11 @@ def _build_caching_settings(cache_config: Dict[str, Any]) -> Dict[str, Any]: def create_response_caching_middleware() -> Any | None: """ - Create ResponseCachingMiddleware with RedisStore backend. + Create ResponseCachingMiddleware with optional RedisStore backend. - Uses MCP_CACHE_CONFIG for caching settings and prefix. - Uses get_mcp_store() factory for store creation. + Uses MCP_CACHE_CONFIG for caching settings. + When MCP_STORE_CONFIG is enabled, uses Redis store with prefix. + Otherwise, uses FastMCP's default in-memory store (no prefix needed). Returns: ResponseCachingMiddleware instance or None if not configured/disabled @@ -95,22 +96,12 @@ def create_response_caching_middleware() -> Any | None: def _create_middleware() -> Any | None: cache_config = flask_app.config.get("MCP_CACHE_CONFIG", {}) + store_config = flask_app.config.get("MCP_STORE_CONFIG", {}) if not cache_config.get("enabled", False): logger.debug("MCP response caching disabled") return None - # Get cache-specific prefix from MCP_CACHE_CONFIG - cache_prefix = cache_config.get("CACHE_KEY_PREFIX") - if not cache_prefix: - logger.warning("MCP caching enabled but no CACHE_KEY_PREFIX configured") - return None - - # Create store with cache-specific prefix - store = get_mcp_store(prefix=cache_prefix) - if store is None: - return None - try: from fastmcp.server.middleware.caching import ResponseCachingMiddleware except ImportError: @@ -119,9 +110,23 @@ def create_response_caching_middleware() -> Any | None: ) return None + # Determine which store to use + store = None + if store_config.get("enabled", False): + # Redis store requires a prefix + cache_prefix = cache_config.get("CACHE_KEY_PREFIX") + if not cache_prefix: + logger.warning( + "MCP_STORE_CONFIG enabled but no CACHE_KEY_PREFIX configured - " + "falling back to in-memory store" + ) + else: + store = get_mcp_store(prefix=cache_prefix) + # Build per-operation settings from config settings = _build_caching_settings(cache_config) + # Create middleware (store=None uses FastMCP's default in-memory store) middleware = ResponseCachingMiddleware( cache_storage=store, **settings, diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py index 624c60e47af..c8e9435eac7 100644 --- a/superset/mcp_service/mcp_config.py +++ b/superset/mcp_service/mcp_config.py @@ -73,18 +73,73 @@ MCP_FACTORY_CONFIG = { "config": None, # No additional config } -# MCP Store Configuration - shared infrastructure for all MCP storage needs -# (caching, auth, events, etc.) +# ============================================================================= +# MCP Storage and Caching Configuration +# ============================================================================= +# +# Overview: +# --------- +# MCP caching uses FastMCP's ResponseCachingMiddleware to cache tool responses. +# By default, caching is DISABLED. When enabled, it can use either: +# 1. In-memory store (default) - no additional configuration needed +# 2. Redis store - requires MCP_STORE_CONFIG to be enabled +# +# Configuration Flow: +# ------------------- +# - MCP_CACHE_CONFIG controls whether caching is enabled and its TTL settings +# - MCP_STORE_CONFIG controls the Redis store (optional) +# +# Scenarios: +# ---------- +# 1. Caching disabled (default): +# MCP_CACHE_CONFIG["enabled"] = False +# → No caching, MCP_STORE_CONFIG is ignored +# +# 2. Caching with in-memory store: +# MCP_CACHE_CONFIG["enabled"] = True +# MCP_STORE_CONFIG["enabled"] = False (or not configured) +# → Caching uses FastMCP's default in-memory store, no Prefix wrapper used +# +# 3. Caching with Redis store: +# MCP_CACHE_CONFIG["enabled"] = True +# MCP_STORE_CONFIG["enabled"] = True +# MCP_STORE_CONFIG["CACHE_REDIS_URL"] = "redis://..." +# → Caching uses Redis with PrefixKeysWrapper +# +# Redis Store Details: +# -------------------- +# When MCP_STORE_CONFIG is enabled, it creates a RedisStore wrapped with +# PrefixKeysWrapper (configurable via WRAPPER_TYPE). The wrapper prepends a +# prefix to all keys, allowing multiple features to share the same Redis +# instance with isolated key namespaces. The prefix comes from the consumer +# (e.g., MCP_CACHE_CONFIG["CACHE_KEY_PREFIX"] for caching). +# +# Advanced Usage: +# --------------- +# The store factory (get_mcp_store) can be used independently for custom +# purposes like auth token storage or event logging. Import and call: +# +# from superset.mcp_service.storage import get_mcp_store +# my_store = get_mcp_store(prefix="my_feature_v1_") +# +# Currently, the store is only used by the caching layer when enabled. +# ============================================================================= + +# MCP Store Configuration - shared Redis infrastructure for all MCP storage needs +# (caching, auth, events, etc.). Only used when a consumer explicitly requests it. MCP_STORE_CONFIG: Dict[str, Any] = { - "enabled": False, # Disabled by default in OSS - "CACHE_REDIS_URL": None, # Must be configured to enable + "enabled": False, # Disabled by default - caching uses in-memory store + "CACHE_REDIS_URL": None, # Redis URL, e.g., "redis://localhost:6379/0" + # Wrapper class that prefixes all keys. Each consumer provides their own prefix. "WRAPPER_TYPE": "key_value.aio.wrappers.prefix_keys.PrefixKeysWrapper", } -# MCP Response Caching Configuration - feature-specific settings +# MCP Response Caching Configuration - controls caching behavior and TTLs +# When enabled without MCP_STORE_CONFIG, uses in-memory store. +# When enabled with MCP_STORE_CONFIG, uses Redis store. MCP_CACHE_CONFIG: Dict[str, Any] = { - "enabled": False, # Disabled by default in OSS - "CACHE_KEY_PREFIX": "mcp_cache_v1_", # Static prefix for OSS + "enabled": False, # Disabled by default + "CACHE_KEY_PREFIX": None, # Only needed when using the store "list_tools_ttl": 60 * 5, # 5 minutes "list_resources_ttl": 60 * 5, # 5 minutes "list_prompts_ttl": 60 * 5, # 5 minutes @@ -92,7 +147,7 @@ MCP_CACHE_CONFIG: Dict[str, Any] = { "get_prompt_ttl": 60 * 60, # 1 hour "call_tool_ttl": 60 * 60, # 1 hour "max_item_size": 1024 * 1024, # 1MB - "excluded_tools": [ + "excluded_tools": [ # Tools that should never be cached (side effects, dynamic) "execute_sql", "generate_dashboard", "generate_chart", diff --git a/tests/unit_tests/mcp_service/test_mcp_caching.py b/tests/unit_tests/mcp_service/test_mcp_caching.py index ed1d33345ae..ff02f94e7d2 100644 --- a/tests/unit_tests/mcp_service/test_mcp_caching.py +++ b/tests/unit_tests/mcp_service/test_mcp_caching.py @@ -85,24 +85,74 @@ def test_create_response_caching_middleware_returns_none_when_disabled(): assert result is None -def test_create_response_caching_middleware_returns_none_when_no_prefix(): - """Caching middleware returns None when CACHE_KEY_PREFIX is not set.""" +def test_create_response_caching_middleware_falls_back_to_memory_when_no_prefix(): + """Caching middleware uses in-memory store when CACHE_KEY_PREFIX is not set.""" mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = { - "enabled": True, - "CACHE_KEY_PREFIX": None, + mock_configs = { + "MCP_CACHE_CONFIG": {"enabled": True, "list_tools_ttl": 300}, + "MCP_STORE_CONFIG": {"enabled": True}, # Store enabled but no CACHE_KEY_PREFIX } + mock_flask_app.config.get.side_effect = lambda key, default=None: mock_configs.get( + key, default + ) + + mock_middleware = MagicMock() with patch( "superset.mcp_service.flask_singleton.get_flask_app", return_value=mock_flask_app, ): with patch("flask.has_app_context", return_value=True): - from superset.mcp_service.caching import create_response_caching_middleware + with patch( + "fastmcp.server.middleware.caching.ResponseCachingMiddleware", + return_value=mock_middleware, + ) as mock_middleware_class: + from superset.mcp_service.caching import ( + create_response_caching_middleware, + ) - result = create_response_caching_middleware() + result = create_response_caching_middleware() - assert result is None + # Middleware should be created with cache_storage=None (in-memory) + assert result is mock_middleware + mock_middleware_class.assert_called_once() + call_kwargs = mock_middleware_class.call_args[1] + assert call_kwargs["cache_storage"] is None + + +def test_create_response_caching_middleware_uses_memory_store_when_store_disabled(): + """Caching middleware uses in-memory store when MCP_STORE_CONFIG is disabled.""" + mock_flask_app = MagicMock() + mock_configs = { + "MCP_CACHE_CONFIG": {"enabled": True, "list_tools_ttl": 300}, + "MCP_STORE_CONFIG": {"enabled": False}, + } + mock_flask_app.config.get.side_effect = lambda key, default=None: mock_configs.get( + key, default + ) + + mock_middleware = MagicMock() + + with patch( + "superset.mcp_service.flask_singleton.get_flask_app", + return_value=mock_flask_app, + ): + with patch("flask.has_app_context", return_value=True): + with patch( + "fastmcp.server.middleware.caching.ResponseCachingMiddleware", + return_value=mock_middleware, + ) as mock_middleware_class: + from superset.mcp_service.caching import ( + create_response_caching_middleware, + ) + + result = create_response_caching_middleware() + + # Middleware should be created with cache_storage=None (in-memory) + assert result is mock_middleware + mock_middleware_class.assert_called_once() + call_kwargs = mock_middleware_class.call_args[1] + assert call_kwargs["cache_storage"] is None def test_create_response_caching_middleware_creates_middleware():