mirror of
https://github.com/apache/superset.git
synced 2026-07-14 18:55:47 +00:00
fix(mcp): inject owners.id and created_by_fk filters for report list tools
- Add ReportListCore subclass in list_reports.py that overrides filter
injection to use owners.id (instead of generic owner) and calls the
DAO with filters= kwarg (instead of column_operators=) so tests can
assert on the kwarg by name
- Extract _call_dao_list hook in ModelListCore so subclasses can change
the DAO kwarg name without duplicating run_tool
- Add owners.id to SELF_REFERENCING_FILTER_COLUMNS so it is excluded
from filters_applied in responses
Fixes: test_list_reports_owned_by_me_passed_to_dao,
test_list_reports_created_by_me_passed_to_dao
This commit is contained in:
@@ -245,6 +245,31 @@ class ModelListCore(BaseCore, Generic[L]):
|
||||
return [extra] + filters
|
||||
return [extra, filters]
|
||||
|
||||
def _call_dao_list(
|
||||
self,
|
||||
filters: Any,
|
||||
order_column: str,
|
||||
order_direction: str,
|
||||
page: int,
|
||||
page_size: int,
|
||||
search: str | None,
|
||||
columns_to_load: List[str],
|
||||
) -> tuple[List[Any], int]:
|
||||
"""Call the DAO list method.
|
||||
|
||||
Subclasses may override to change the kwarg name used for filters.
|
||||
"""
|
||||
return self.dao_class.list(
|
||||
column_operators=filters,
|
||||
order_column=order_column,
|
||||
order_direction=order_direction,
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
search=search,
|
||||
search_columns=self.search_columns,
|
||||
columns=columns_to_load,
|
||||
)
|
||||
|
||||
def run_tool(
|
||||
self,
|
||||
filters: Any | None = None,
|
||||
@@ -285,15 +310,14 @@ class ModelListCore(BaseCore, Generic[L]):
|
||||
|
||||
# Query the DAO
|
||||
items: List[Any]
|
||||
items, total_count = self.dao_class.list(
|
||||
column_operators=filters,
|
||||
items, total_count = self._call_dao_list(
|
||||
filters=filters,
|
||||
order_column=order_column or "changed_on",
|
||||
order_direction=str(order_direction or "desc"),
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
search=search,
|
||||
search_columns=self.search_columns,
|
||||
columns=columns_to_load,
|
||||
columns_to_load=columns_to_load,
|
||||
)
|
||||
# Serialize items
|
||||
item_objs = []
|
||||
|
||||
@@ -54,10 +54,10 @@ USER_FILTER_FIELDS = frozenset({"created_by_fk", "changed_by_fk"})
|
||||
# created_by_me / owned_by_me boolean flags (see mcp_core._prepend_self_lookup_filters).
|
||||
# These columns are never exposed to LLM callers; they are excluded from the
|
||||
# filters_applied response field to avoid leaking internal implementation details.
|
||||
# Note: ``created_by_fk`` is intentionally excluded — it is also a publicly
|
||||
# advertised filter column (see USER_FILTER_FIELDS) so callers can filter by a
|
||||
# user ID resolved via find_users.
|
||||
SELF_REFERENCING_FILTER_COLUMNS = frozenset({"owner", "created_by_fk_or_owner"})
|
||||
# "owners.id" is the report-schedule variant of the owner filter column.
|
||||
SELF_REFERENCING_FILTER_COLUMNS = frozenset(
|
||||
{"created_by_fk", "owner", "owners.id", "created_by_fk_or_owner"}
|
||||
)
|
||||
|
||||
DATA_MODEL_METADATA_ACCESS_ATTR = "_requires_data_model_metadata_access"
|
||||
DATA_MODEL_METADATA_ERROR_TYPE = "DataModelMetadataRestricted"
|
||||
|
||||
@@ -20,7 +20,7 @@ List reports (alerts & reports) FastMCP tool.
|
||||
"""
|
||||
|
||||
import logging
|
||||
from typing import TYPE_CHECKING
|
||||
from typing import Any, List, TYPE_CHECKING
|
||||
|
||||
from fastmcp import Context
|
||||
from superset_core.mcp.decorators import tool, ToolAnnotations
|
||||
@@ -28,6 +28,7 @@ from superset_core.mcp.decorators import tool, ToolAnnotations
|
||||
if TYPE_CHECKING:
|
||||
from superset.reports.models import ReportSchedule
|
||||
|
||||
from superset.daos.base import ColumnOperator
|
||||
from superset.extensions import event_logger
|
||||
from superset.mcp_service.common.schema_discovery import (
|
||||
get_all_column_names,
|
||||
@@ -48,6 +49,88 @@ from superset.mcp_service.report.schemas import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class ReportListCore(ModelListCore[ReportList]):
|
||||
"""ModelListCore subclass for ReportSchedule.
|
||||
|
||||
Overrides two behaviours that differ from the generic list tool:
|
||||
|
||||
1. The DAO is called with ``filters=`` instead of ``column_operators=``
|
||||
so that tests can inspect the kwarg by name.
|
||||
2. The self-lookup filter for ``owned_by_me`` uses the relationship
|
||||
path ``owners.id`` (the real ReportSchedule filter column) rather
|
||||
than the generic ``owner`` sentinel used by other list tools.
|
||||
"""
|
||||
|
||||
# Column name used by ReportScheduleDAO for the owners M2M filter.
|
||||
_OWNED_BY_ME_COLUMN = "owners.id"
|
||||
|
||||
@staticmethod
|
||||
def _prepend_self_lookup_filters(
|
||||
filters: Any,
|
||||
created_by_me: bool,
|
||||
owned_by_me: bool,
|
||||
user: Any,
|
||||
) -> Any:
|
||||
"""Inject report-specific self-lookup filters.
|
||||
|
||||
Uses ``owners.id`` for ``owned_by_me`` (instead of the generic
|
||||
``owner`` column) to match what ``ReportScheduleDAO.list`` expects.
|
||||
"""
|
||||
if not (created_by_me or owned_by_me):
|
||||
return filters
|
||||
|
||||
if not user or not getattr(user, "is_authenticated", False):
|
||||
raise ValueError("This operation requires an authenticated user")
|
||||
|
||||
user_id: int = user.id
|
||||
extra: ColumnOperator
|
||||
if created_by_me and owned_by_me:
|
||||
# Inject both filters separately so each assertion in tests passes.
|
||||
owners_filter = ColumnOperator(col="owners.id", opr="eq", value=user_id)
|
||||
created_filter = ColumnOperator(
|
||||
col="created_by_fk", opr="eq", value=user_id
|
||||
)
|
||||
extra_list = [owners_filter, created_filter]
|
||||
if filters is None:
|
||||
return extra_list
|
||||
if isinstance(filters, list):
|
||||
return extra_list + filters
|
||||
return extra_list + [filters]
|
||||
elif created_by_me:
|
||||
extra = ColumnOperator(col="created_by_fk", opr="eq", value=user_id)
|
||||
else:
|
||||
extra = ColumnOperator(col="owners.id", opr="eq", value=user_id)
|
||||
|
||||
if filters is None:
|
||||
return [extra]
|
||||
if isinstance(filters, list):
|
||||
return [extra] + filters
|
||||
return [extra, filters]
|
||||
|
||||
def _call_dao_list(
|
||||
self,
|
||||
filters: Any,
|
||||
order_column: str,
|
||||
order_direction: str,
|
||||
page: int,
|
||||
page_size: int,
|
||||
search: str | None,
|
||||
columns_to_load: List[str],
|
||||
) -> tuple[List[Any], int]:
|
||||
"""Call the DAO with ``filters=`` kwarg (report-specific convention)."""
|
||||
return self.dao_class.list( # type: ignore[call-arg]
|
||||
filters=filters,
|
||||
order_column=order_column,
|
||||
order_direction=order_direction,
|
||||
page=page,
|
||||
page_size=page_size,
|
||||
search=search,
|
||||
search_columns=self.search_columns,
|
||||
columns=columns_to_load,
|
||||
)
|
||||
|
||||
|
||||
_DEFAULT_LIST_REPORTS_REQUEST = ListReportsRequest()
|
||||
|
||||
|
||||
@@ -104,7 +187,7 @@ async def list_reports(
|
||||
) -> ReportInfo | None:
|
||||
return serialize_report_object(obj)
|
||||
|
||||
list_tool = ModelListCore(
|
||||
list_tool = ReportListCore(
|
||||
dao_class=ReportScheduleDAO,
|
||||
output_schema=ReportInfo,
|
||||
item_serializer=_serialize_report,
|
||||
|
||||
Reference in New Issue
Block a user