This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3fa06db85b6 fix(mcp): include feature_availability in
instance://metadata resource (#44891)
3fa06db85b6 is described below
commit 3fa06db85b6320364d3a4747aca0447ede179020
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Sat Oct 3 09:50:28 2026 +1000
fix(mcp): include feature_availability in instance://metadata resource
(#44891)
---
.../system/resources/instance_metadata.py | 37 +++---
superset/mcp_service/system/system_utils.py | 33 ++++-
.../mcp_service/system/tool/get_instance_info.py | 38 +-----
.../system/resources/test_instance_metadata.py | 144 +++++++++++++++++++++
.../system/tool/test_get_current_user.py | 6 +-
5 files changed, 202 insertions(+), 56 deletions(-)
diff --git a/superset/mcp_service/system/resources/instance_metadata.py
b/superset/mcp_service/system/resources/instance_metadata.py
index 29e223f06e1..7fe593aa210 100644
--- a/superset/mcp_service/system/resources/instance_metadata.py
+++ b/superset/mcp_service/system/resources/instance_metadata.py
@@ -56,13 +56,12 @@ def get_instance_metadata_resource() -> str:
from superset.daos.tag import TagDAO
from superset.daos.user import UserDAO
from superset.mcp_service.mcp_core import InstanceInfoCore
+ from superset.mcp_service.privacy import
user_can_view_data_model_metadata
from superset.mcp_service.system.schemas import InstanceInfo
from superset.mcp_service.system.system_utils import (
- calculate_dashboard_breakdown,
- calculate_database_breakdown,
- calculate_instance_summary,
- calculate_popular_content,
- calculate_recent_activity,
+ INSTANCE_INFO_METRIC_CALCULATORS,
+ INSTANCE_INFO_TIME_WINDOWS,
+ redact_data_model_metadata,
)
from superset.utils import json
@@ -76,29 +75,29 @@ def get_instance_metadata_resource() -> str:
"tags": cast(Type[BaseDAO[Any]], TagDAO),
},
output_schema=InstanceInfo,
- metric_calculators={
- "instance_summary": calculate_instance_summary,
- "recent_activity": calculate_recent_activity,
- "dashboard_breakdown": calculate_dashboard_breakdown,
- "database_breakdown": calculate_database_breakdown,
- "popular_content": calculate_popular_content,
- },
- time_windows={
- "recent": 7,
- "monthly": 30,
- "quarterly": 90,
- },
+ metric_calculators=INSTANCE_INFO_METRIC_CALCULATORS,
+ time_windows=INSTANCE_INFO_TIME_WINDOWS,
logger=logger,
)
- # Get base instance info
- base_result = json.loads(instance_info_core.get_resource())
+ # Get base instance info, redacting data model metadata for principals
+ # that the get_instance_info tool also hides it from.
+ can_view_data_model = user_can_view_data_model_metadata()
+ instance_info = instance_info_core.run_tool()
+ if not can_view_data_model:
+ instance_info = redact_data_model_metadata(instance_info)
+ base_result = json.loads(json.dumps(instance_info.model_dump()))
# Remove empty popular_content if it has no useful data
popular = base_result.get("popular_content", {})
if popular and not any(popular.get(k) for k in popular):
del base_result["popular_content"]
+ if not can_view_data_model:
+ base_result["available_datasets"] = []
+ base_result["available_databases"] = []
+ return json.dumps(base_result, indent=2)
+
# Add available datasets (top 20 by most recent modification)
dataset_dao = instance_info_core.dao_classes["datasets"]
try:
diff --git a/superset/mcp_service/system/system_utils.py
b/superset/mcp_service/system/system_utils.py
index 75fe81af5f8..292dd809c92 100644
--- a/superset/mcp_service/system/system_utils.py
+++ b/superset/mcp_service/system/system_utils.py
@@ -23,12 +23,13 @@ instance metrics, dashboard breakdowns, database
breakdowns, and activity summar
"""
import logging
-from typing import Any, Dict
+from typing import Any, Callable, Dict
from superset.mcp_service.system.schemas import (
DashboardBreakdown,
DatabaseBreakdown,
FeatureAvailability,
+ InstanceInfo,
InstanceSummary,
PopularContent,
RecentActivity,
@@ -223,3 +224,33 @@ def calculate_feature_availability(
return FeatureAvailability(
accessible_menus=accessible_menus,
)
+
+
+def redact_data_model_metadata(result: InstanceInfo) -> InstanceInfo:
+ """Remove dataset/database counts and activity from instance overview."""
+ data = result.model_copy(deep=True)
+ data.instance_summary.total_datasets = 0
+ data.instance_summary.total_databases = 0
+ data.recent_activity.datasets_created_last_30_days = 0
+ data.recent_activity.datasets_modified_last_7_days = 0
+ data.database_breakdown.by_type = {}
+ data.data_model_metadata_redacted = True
+ return data
+
+
+# Shared by the get_instance_info tool and the instance://metadata resource so
a
+# newly required InstanceInfo field only needs a calculator registered once.
+INSTANCE_INFO_METRIC_CALCULATORS: Dict[str, Callable[..., Any]] = {
+ "instance_summary": calculate_instance_summary,
+ "recent_activity": calculate_recent_activity,
+ "dashboard_breakdown": calculate_dashboard_breakdown,
+ "database_breakdown": calculate_database_breakdown,
+ "popular_content": calculate_popular_content,
+ "feature_availability": calculate_feature_availability,
+}
+
+INSTANCE_INFO_TIME_WINDOWS: Dict[str, int] = {
+ "recent": 7,
+ "monthly": 30,
+ "quarterly": 90,
+}
diff --git a/superset/mcp_service/system/tool/get_instance_info.py
b/superset/mcp_service/system/tool/get_instance_info.py
index bf6ff243036..ff9a499a0eb 100644
--- a/superset/mcp_service/system/tool/get_instance_info.py
+++ b/superset/mcp_service/system/tool/get_instance_info.py
@@ -35,12 +35,9 @@ from superset.mcp_service.system.schemas import (
serialize_user_object,
)
from superset.mcp_service.system.system_utils import (
- calculate_dashboard_breakdown,
- calculate_database_breakdown,
- calculate_feature_availability,
- calculate_instance_summary,
- calculate_popular_content,
- calculate_recent_activity,
+ INSTANCE_INFO_METRIC_CALCULATORS,
+ INSTANCE_INFO_TIME_WINDOWS,
+ redact_data_model_metadata,
)
logger = logging.getLogger(__name__)
@@ -57,19 +54,8 @@ _instance_info_core = InstanceInfoCore(
"tags": None, # type: ignore[dict-item]
},
output_schema=InstanceInfo,
- metric_calculators={
- "instance_summary": calculate_instance_summary,
- "recent_activity": calculate_recent_activity,
- "dashboard_breakdown": calculate_dashboard_breakdown,
- "database_breakdown": calculate_database_breakdown,
- "popular_content": calculate_popular_content,
- "feature_availability": calculate_feature_availability,
- },
- time_windows={
- "recent": 7,
- "monthly": 30,
- "quarterly": 90,
- },
+ metric_calculators=INSTANCE_INFO_METRIC_CALCULATORS,
+ time_windows=INSTANCE_INFO_TIME_WINDOWS,
logger=logger,
)
@@ -77,18 +63,6 @@ _instance_info_core = InstanceInfoCore(
_DEFAULT_INSTANCE_INFO_REQUEST = GetSupersetInstanceInfoRequest()
-def _redact_data_model_metadata(result: InstanceInfo) -> InstanceInfo:
- """Remove dataset/database counts and activity from instance overview."""
- data = result.model_copy(deep=True)
- data.instance_summary.total_datasets = 0
- data.instance_summary.total_databases = 0
- data.recent_activity.datasets_created_last_30_days = 0
- data.recent_activity.datasets_modified_last_7_days = 0
- data.database_breakdown.by_type = {}
- data.data_model_metadata_redacted = True
- return data
-
-
@tool(
tags=["core"],
annotations=ToolAnnotations(
@@ -176,7 +150,7 @@ def _run_instance_info() -> InstanceInfo:
result = _instance_info_core.run_tool()
if not user_can_view_data_model_metadata():
- result = _redact_data_model_metadata(result)
+ result = redact_data_model_metadata(result)
if (user := getattr(g, "user", None)) is not None:
result.current_user = serialize_user_object(user)
diff --git
a/tests/unit_tests/mcp_service/system/resources/test_instance_metadata.py
b/tests/unit_tests/mcp_service/system/resources/test_instance_metadata.py
new file mode 100644
index 00000000000..0e7972affd7
--- /dev/null
+++ b/tests/unit_tests/mcp_service/system/resources/test_instance_metadata.py
@@ -0,0 +1,144 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for the instance://metadata MCP resource."""
+
+from collections.abc import Iterator
+from types import SimpleNamespace
+from typing import Any
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.system.schemas import FeatureAvailability,
InstanceInfo
+from superset.mcp_service.system.tool.get_instance_info import
_instance_info_core
+from superset.utils import json
+
+INSTANCE_METADATA_URI = "instance://metadata"
+
+
[email protected](autouse=True)
+def mock_auth() -> Iterator[Mock]:
+ """Mock authentication for all tests."""
+ with patch("superset.mcp_service.auth.get_user_from_request") as
mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield mock_get_user
+
+
[email protected](autouse=True)
+def can_view() -> bool:
+ """Whether the mocked user may inspect data model metadata."""
+ return True
+
+
[email protected](autouse=True)
+def mock_data_access(can_view: bool) -> Iterator[tuple[Mock, Mock]]:
+ """Avoid hitting the metadata database while generating the resource."""
+ dataset = SimpleNamespace(
+ id=7,
+ table_name="orders",
+ schema="public",
+ database_id=3,
+ changed_on=None,
+ )
+ database = SimpleNamespace(id=3, database_name="examples",
backend="sqlite")
+ with (
+ patch(
+
"superset.mcp_service.mcp_core.InstanceInfoCore._calculate_basic_counts",
+ return_value={"total_datasets": 7, "total_databases": 3},
+ ),
+ patch(
+ "superset.mcp_service.mcp_core.InstanceInfoCore"
+ "._calculate_time_based_metrics",
+ return_value={},
+ ),
+ patch(
+ "superset.daos.dataset.DatasetDAO.find_all", return_value=[dataset]
+ ) as find_datasets,
+ patch(
+ "superset.daos.database.DatabaseDAO.find_all",
return_value=[database]
+ ) as find_databases,
+ patch(
+ "superset.mcp_service.privacy.user_can_view_data_model_metadata",
+ return_value=can_view,
+ ),
+ ):
+ yield find_datasets, find_databases
+
+
+async def _read_metadata() -> dict[str, Any]:
+ async with Client(mcp) as client:
+ result = await client.read_resource(INSTANCE_METADATA_URI)
+ return json.loads(result[0].text)
+
+
[email protected]
+async def test_instance_metadata_resource_returns_valid_payload() -> None:
+ data = await _read_metadata()
+
+ assert "error" not in data
+ # The resource intentionally drops an empty popular_content block, so
restore
+ # it before validating the payload against the full InstanceInfo schema.
+ InstanceInfo.model_validate(
+ {"popular_content": {"top_tags": [], "top_creators": []}, **data}
+ )
+ assert FeatureAvailability.model_validate(data["feature_availability"])
+ assert data["instance_summary"]["total_datasets"] == 7
+ assert data["instance_summary"]["total_databases"] == 3
+ assert data["data_model_metadata_redacted"] is False
+ assert data["available_datasets"] == [
+ {"id": 7, "table_name": "orders", "schema": "public", "database_id": 3}
+ ]
+ assert data["available_databases"] == [
+ {"id": 3, "database_name": "examples", "backend": "sqlite"}
+ ]
+
+
[email protected]
[email protected]("can_view", [False])
+async def test_instance_metadata_resource_redacts_data_model_metadata(
+ mock_data_access: tuple[Mock, Mock],
+) -> None:
+ """Principals without data model access must not see dataset/database
info."""
+ find_datasets, find_databases = mock_data_access
+
+ data = await _read_metadata()
+
+ assert "error" not in data
+ assert data["data_model_metadata_redacted"] is True
+ assert data["instance_summary"]["total_datasets"] == 0
+ assert data["instance_summary"]["total_databases"] == 0
+ assert data["database_breakdown"]["by_type"] == {}
+ assert data["available_datasets"] == []
+ assert data["available_databases"] == []
+ find_datasets.assert_not_called()
+ find_databases.assert_not_called()
+ assert "accessible_menus" in data["feature_availability"]
+
+
+def test_resource_and_tool_share_metric_calculators() -> None:
+ """The tool and resource must compute the same set of InstanceInfo
metrics."""
+ from superset.mcp_service.system.system_utils import (
+ INSTANCE_INFO_METRIC_CALCULATORS,
+ )
+
+ assert _instance_info_core.metric_calculators is
INSTANCE_INFO_METRIC_CALCULATORS
diff --git a/tests/unit_tests/mcp_service/system/tool/test_get_current_user.py
b/tests/unit_tests/mcp_service/system/tool/test_get_current_user.py
index fd9acad2abc..589ff97c60c 100644
--- a/tests/unit_tests/mcp_service/system/tool/test_get_current_user.py
+++ b/tests/unit_tests/mcp_service/system/tool/test_get_current_user.py
@@ -138,9 +138,7 @@ def
test_redact_data_model_metadata_removes_dataset_and_database_summary():
InstanceSummary,
RecentActivity,
)
- from superset.mcp_service.system.tool.get_instance_info import (
- _redact_data_model_metadata,
- )
+ from superset.mcp_service.system.system_utils import
redact_data_model_metadata
instance_info = _make_instance_info(
instance_summary=InstanceSummary(
@@ -164,7 +162,7 @@ def
test_redact_data_model_metadata_removes_dataset_and_database_summary():
database_breakdown=DatabaseBreakdown(by_type={"postgresql": 2}),
)
- redacted = _redact_data_model_metadata(instance_info)
+ redacted = redact_data_model_metadata(instance_info)
assert redacted.instance_summary.total_dashboards == 2
assert redacted.instance_summary.total_charts == 4
