alexandrusoare commented on code in PR #45022: URL: https://github.com/apache/superset/pull/45022#discussion_r4401927838
########## superset/mcp_service/canvas/tool/preview_widget.py: ########## @@ -0,0 +1,258 @@ +# 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. + +"""MCP tool: preview_widget""" + +from __future__ import annotations + +import logging +from typing import Any + +import requests +from fastmcp.tools import ToolResult +from flask import current_app +from mcp.types import ImageContent, TextContent +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.canvas.data import run_binding, RuntimeFilter +from superset.canvas.definition.registry import get_widget_resolver +from superset.canvas.definition.render import render_context +from superset.canvas.definition.versions import DefinitionVersionError +from superset.commands.canvas.draft import load_draft +from superset.commands.canvas.exceptions import ( + CanvasDraftNotFoundError, + CanvasForbiddenError, + CanvasNotFoundError, +) +from superset.daos.canvas import CanvasDAO +from superset.extensions import security_manager +from superset.mcp_service.canvas.schemas import PreviewWidgetResponse +from superset.mcp_service.canvas.tool.canvas_draft_common import NOT_FOUND +from superset.utils import json + +logger = logging.getLogger(__name__) + +SAMPLE_ROWS = 10 +RENDER_TIMEOUT_SECONDS = 15 +# The canvas page's content width and grid, for sizing a placement in pixels. +PAGE_WIDTH = 1360 +GAP = 16 + + +def _pixel_size(definition: dict[str, Any], placement_id: str) -> tuple[int, int]: + root_layout = definition["root"].get("layout", {}) + columns = root_layout.get("columns", 24) + row_unit = root_layout.get("rowUnit", 40) + gap = root_layout.get("gap", GAP) + placement = render_context(definition)["placements"].get(placement_id, {}) + col_span = placement.get("colSpan", columns // 2) + row_span = placement.get("rowSpan", 8) + column_width = (PAGE_WIDTH - gap * (columns - 1)) / columns + width = col_span * column_width + gap * (col_span - 1) + height = row_span * row_unit + gap * (row_span - 1) + return round(width), round(height) + + +def _render( + url: str, + props: dict[str, Any], + rows: list[dict[str, Any]], + definition: dict[str, Any], + size: tuple[int, int], + dark: bool, +) -> dict[str, Any]: + theme = current_app.config["THEME_DARK"] if dark else None + theme = theme or current_app.config["THEME_DEFAULT"] + colors = definition.get("settings", {}).get("colors", {}) + response = requests.post( + f"{url.rstrip('/')}/render/echarts", + json={ + "props": props, + "rows": rows, + "theme": theme, + "scheme": colors.get("scheme"), + "labelColors": colors.get("labelColors", {}), + "width": size[0], + "height": size[1], + }, + timeout=RENDER_TIMEOUT_SECONDS, + ) + response.raise_for_status() + return response.json() + + +def _with_rows( + result: dict[str, Any], + props: dict[str, Any], + runtime_filters: list[RuntimeFilter] | None = None, +) -> list[dict[str, Any]]: + """Run the widget's query, if it has one, and record what came back.""" + binding = props.get("dataBinding") + if not isinstance(binding, dict) or not binding.get("datasetId"): + return [] + data = run_binding(binding, runtime_filters) + rows = data.rows + result["row_count"] = len(rows) + result["columns"] = list(rows[0]) if rows else [] + result["sample_rows"] = rows[:SAMPLE_ROWS] + # Classic charts gate "View query" on the same permission. + if data.query and security_manager.can_access("can_view_query", "Dashboard"): + result["query"] = data.query + result["query_language"] = data.language + if not rows: + result["warnings"] = ["The widget's query returned no rows"] + return rows + + +def _with_render( + result: dict[str, Any], + url: str, + props: dict[str, Any], + rows: list[dict[str, Any]], + definition: dict[str, Any], + size: tuple[int, int], + dark: bool, +) -> str | None: + """Render the widget, record the option and warnings, return the PNG.""" + warnings = result.setdefault("warnings", []) + try: + rendered = _render(url, props, rows, definition, size, dark) + except requests.RequestException as ex: + logger.warning("Canvas widget renderer failed: %s", ex) + result["rendered"] = False + warnings.append( + f"No image: the renderer failed ({ex}). The chart was not checked " + "visually; don't report it as verified." + ) + return None + result["rendered"] = True + result["option"] = rendered.get("option") + warnings.extend(rendered.get("warnings", [])) + if rendered.get("error"): + result["error"] = rendered["error"] + image = rendered.get("png") + if image: + width, height = rendered.get("width"), rendered.get("height") + result["image"] = f"attached, {width}x{height} PNG" + return image + + +def _definition(canvas_id: int | str, draft_token: str | None) -> dict[str, Any] | str: + """The canvas's definition, or its draft's, or an error message.""" + if draft_token: + try: + draft = load_draft(draft_token) + except (CanvasDraftNotFoundError, CanvasNotFoundError, CanvasForbiddenError): + return NOT_FOUND + return draft.definition + canvas = CanvasDAO.find_by_id_or_uuid(str(canvas_id)) + if canvas is None: + return f"Canvas {canvas_id} not found, or no access to it" + try: + return CanvasDAO.load(canvas) + except DefinitionVersionError as ex: + return str(ex) + + +def _preview_widget_impl( + canvas_id: int | str, + placement_id: str, + dark: bool, + draft_token: str | None = None, + runtime_filters: list[RuntimeFilter] | None = None, +) -> tuple[dict[str, Any], str | None]: + definition = _definition(canvas_id, draft_token) + if isinstance(definition, str): + return {"error": definition}, None + node = definition["nodes"].get(placement_id) + if node is None: + return {"error": f"No placement {placement_id!r} on this canvas"}, None + widget_type = node.get("widgetType") + props = node.get("props") or {} + if widget_id := node.get("widgetId"): + resolver = ( + load_draft(draft_token).resolver() if draft_token else get_widget_resolver() + ) + widget = resolver.widgets({widget_id}).get(widget_id) + if widget is None: + return { + "error": f"The widget {placement_id!r} holds no longer exists" + }, None + widget_type, props = widget.widget_type, widget.props + result: dict[str, Any] = { + "placement_id": placement_id, + "widget_type": widget_type, + } + try: + rows = _with_rows(result, props, runtime_filters) + except Exception as ex: # pylint: disable=broad-except Review Comment: This `except` is wide enough to swallow `SupersetSecurityException` (whose message names the datasource) and driver errors, which tend to echo the failing SQL back — a bit awkward two lines under a `query` field that's carefully gated on `can_view_query`. Worth giving `SupersetSecurityException` its own branch and returning an actual refusal instead? ########## superset/commands/canvas/create.py: ########## @@ -0,0 +1,105 @@ +# 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. +"""Create a canvas.""" + +from __future__ import annotations + +from functools import partial +from typing import Any + +from marshmallow import ValidationError + +from superset.canvas.definition.registry import get_widget_resolver +from superset.canvas.definition.schemas import empty_definition +from superset.canvas.definition.validation import ( + DefinitionValidationError, + normalize_definition, +) +from superset.canvas.definition.versions import ( + check_definition_version, + DefinitionVersionError, +) +from superset.commands.base import BaseCommand, CreateMixin +from superset.commands.canvas.exceptions import ( + CanvasCreateFailedError, + CanvasInvalidError, +) +from superset.commands.canvas.utils import validate_metadata +from superset.commands.utils import populate_subjects +from superset.daos.canvas import CanvasDAO +from superset.models.canvas import Canvas +from superset.utils import json +from superset.utils.decorators import on_error, transaction + + +class CreateCanvasCommand(CreateMixin, BaseCommand): + def __init__(self, data: dict[str, Any]) -> None: + self._properties = data.copy() + + @transaction(on_error=partial(on_error, reraise=CanvasCreateFailedError)) + def run(self) -> Canvas: + self.validate() + return CanvasDAO.create(attributes=self._properties) + + def validate(self) -> None: + exceptions: list[ValidationError] = [] + # Defaults the editors to the requesting user, and resolves viewers. + populate_subjects(self._properties, exceptions) + validate_metadata(self._properties, exceptions) + self._validate_definition(exceptions) + if exceptions: + raise CanvasInvalidError(exceptions=exceptions) + + def _validate_definition(self, exceptions: list[ValidationError]) -> None: + raw = self._properties.pop("definition", None) or empty_definition() + try: + definition = normalize_definition(check_definition_version(raw)) Review Comment: Ordering looks off here: `normalize_definition` bails out on line 82 before we ever reach `placeable()` on line 88. Since validation resolves `widgetId` through the unfiltered resolver, the error ends up naming the widget's type — a known uuid dropped into a `filter.bar` gives back `"a echarts widget cannot be placed in filter.bar"` for a widget you can't read, where a uuid that doesn't exist just gives `"unknown widget, or no access to it"`. So existing-but-forbidden is distinguishable from missing. `apply_ops.py:216-228` and `draft.py:209-213` already check first — running `placeable()` over the raw `widgetId` set before `normalize_definition` would line this up with them. ########## superset/canvas/api.py: ########## @@ -0,0 +1,1057 @@ +# 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. +"""REST API for canvases.""" + +from __future__ import annotations + +import logging +from collections.abc import Callable +from functools import wraps +from typing import Any + +from flask import request, Response +from flask_appbuilder.api import expose, protect, rison as parse_rison, safe +from flask_appbuilder.hooks import before_request +from flask_appbuilder.models.sqla.interface import SQLAInterface +from flask_babel import ngettext +from marshmallow import ValidationError +from pydantic import TypeAdapter, ValidationError as PydanticValidationError +from superset_core.canvas import GridPlacement +from superset_core.widgets import Widget + +from superset import is_feature_enabled +from superset.canvas.data import PlacementDataRequest +from superset.canvas.definition.registry import get_widget_types +from superset.canvas.definition.render import render_context +from superset.canvas.definition.schemas import ( + ApplyOperationsRequest, + CanvasDefinition, + CommitDraftRequest, + DEFINITION_VERSION, + DraftOperationsRequest, + Operation, +) +from superset.canvas.definition.scopes import resolve_scopes +from superset.canvas.definition.validation import request_pointer +from superset.canvas.definition.versions import DefinitionVersionError +from superset.canvas.filters import ( + CanvasAccessFilter, + CanvasAllTextFilter, + CanvasEditableFilter, +) +from superset.canvas.schemas import ( + CanvasPostSchema, + CanvasPutSchema, + get_delete_ids_schema, +) +from superset.commands.canvas.apply_ops import ApplyCanvasOperationsCommand +from superset.commands.canvas.create import CreateCanvasCommand +from superset.commands.canvas.delete import DeleteCanvasCommand +from superset.commands.canvas.draft import ( + ApplyCanvasDraftOperationsCommand, + CommitCanvasDraftCommand, + CreateCanvasDraftCommand, + DeleteCanvasDraftCommand, + draft_payload, + load_draft, +) +from superset.commands.canvas.exceptions import ( + CanvasCreateFailedError, + CanvasDeleteFailedError, + CanvasDraftNotFoundError, + CanvasForbiddenError, + CanvasInvalidError, + CanvasNotFoundError, + CanvasPlacementNotFoundError, + CanvasPlacementUnboundError, + CanvasUpdateFailedError, + DefinitionConflictError, + DefinitionInvalidError, + DraftConflictError, +) +from superset.commands.canvas.placement_data import GetPlacementDataCommand +from superset.commands.canvas.update import UpdateCanvasCommand +from superset.commands.chart.exceptions import ChartDataQueryFailedError +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod +from superset.daos.canvas import CanvasDAO +from superset.daos.exceptions import DatasourceNotFound +from superset.exceptions import QueryObjectValidationError, SupersetSecurityException +from superset.extensions import event_logger, security_manager +from superset.models.canvas import Canvas +from superset.subjects.filters import FilterRelatedSubjects, subject_type_filter +from superset.utils import json +from superset.views.base_api import ( + BaseSupersetModelRestApi, + RelatedFieldFilter, + statsd_metrics, +) +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedUsers + +logger = logging.getLogger(__name__) + + +def _handle_canvas_errors( + f: Callable[..., Response], +) -> Callable[..., Response]: + @wraps(f) + def wrapped(self: CanvasRestApi, *args: Any, **kwargs: Any) -> Response: + try: + return f(self, *args, **kwargs) + except ( + CanvasNotFoundError, + CanvasDraftNotFoundError, + CanvasPlacementNotFoundError, + ): + return self.response_404() + except CanvasForbiddenError: + return self.response_403() + except CanvasInvalidError as ex: + return self.response_422(message=ex.normalized_messages()) + except ( + DefinitionConflictError, + DefinitionInvalidError, + DraftConflictError, + ) as ex: + return self.response(ex.status, **ex.to_payload()) + except CanvasPlacementUnboundError as ex: + return self.response_422(message=str(ex.message)) + except DefinitionVersionError as ex: + return self.response_422(message=str(ex)) + except ( + CanvasCreateFailedError, + CanvasUpdateFailedError, + CanvasDeleteFailedError, + ) as ex: + logger.exception("Canvas write failed") + return self.response_422(message=str(ex)) + + return wrapped + + +# Draft tokens travel in a header, never in a URL, so they stay out of logs. +DRAFT_TOKEN_HEADER = "X-Canvas-Draft" # noqa: S105 + + +def _draft_token() -> str: + token = request.headers.get(DRAFT_TOKEN_HEADER, "") + if not token: + raise CanvasDraftNotFoundError() + return token + + +def _pydantic_errors(ex: PydanticValidationError) -> list[dict[str, Any]]: + """Request errors as JSON-pointer paths into the request body.""" + return [ + {"path": request_pointer(error["loc"]), "message": error["msg"]} + for error in ex.errors() + ] + + +def describe_widget(widget: type[Widget]) -> dict[str, Any]: + """How a widget takes part in a canvas: nesting, child layout, roles.""" + + def names(types: frozenset[str] | None) -> list[str] | None: + return sorted(types) if types is not None else None + + behavior, ui = widget.behavior, widget.ui + return { + "id": widget.widget_type, + "name": widget.name, + "schemaVersion": widget.schema_version, + "container": behavior.container, + "structural": behavior.structural, + "acceptedChildren": names(behavior.accepted_children), + "allowedParents": names(behavior.allowed_parents), + "gridColumns": behavior.grid_columns, + "childLayout": behavior.child_layout_model.model_json_schema(by_alias=True) + if behavior.child_layout_model is not None + else None, + "filter": behavior.filter, + "emitsFilters": behavior.emits_filters, + "customization": behavior.customization, + "filterable": behavior.filterable, + "boundsFilterScope": behavior.bounds_filter_scope, + "defaultSize": list(ui.default_size) if ui.default_size else None, + "colSpan": {"min": ui.min_col_span, "max": ui.max_col_span}, + "rowSpan": {"min": ui.min_row_span, "max": ui.max_row_span}, + } + + +def _get_canvas(id_or_uuid: str) -> Canvas: + canvas = CanvasDAO.find_by_id_or_uuid(id_or_uuid) + if canvas is None: + raise CanvasNotFoundError() + return canvas + + +class CanvasRestApi(BaseSupersetModelRestApi): + datamodel = SQLAInterface(Canvas) + + @before_request + def ensure_canvas_enabled(self) -> Response | None: + if not is_feature_enabled("CANVAS"): + return self.response_404() + return None + + include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | { + RouteMethod.RELATED, + "bulk_delete", + "get_definition", + "get_definition_changes", + "apply_definition_operations", + "get_schema", + "create_draft", + "get_draft", + "apply_draft_operations", + "commit_draft", + "delete_draft", + "get_placement_data", + } + resource_name = "canvas" + allow_browser_login = True + class_permission_name = "Canvas" + method_permission_name = { + **MODEL_API_RW_METHOD_PERMISSION_MAP, + "get_definition": "read", + "get_definition_changes": "read", + "apply_definition_operations": "write", + "get_schema": "read", + "create_draft": "write", + "get_draft": "write", + "apply_draft_operations": "write", + "commit_draft": "write", + "delete_draft": "write", + "get_placement_data": "read", + } + openapi_spec_tag = "Canvases" + + base_filters = [["id", CanvasAccessFilter, lambda: []]] + + _subject_columns = ["editors.id", "editors.label", "editors.type"] + _viewer_columns = ["viewers.id", "viewers.label", "viewers.type"] + _audit_columns = [ + "changed_by.first_name", + "changed_by.id", + "changed_by.last_name", + "changed_on_delta_humanized", + "created_by.first_name", + "created_by.id", + "created_by.last_name", + ] + _metadata_columns = [ + "slug", + "css", + "theme_id", + "certified_by", + "certification_details", + "is_managed_externally", + "external_url", + ] + show_columns = [ + "id", + "uuid", + "title", + "description", + "revision", + "url", + *_metadata_columns, + "theme.id", + "theme.theme_name", + "theme.json_data", + *_subject_columns, + *_viewer_columns, + *_audit_columns, + ] + list_columns = [ + "id", + "uuid", + "title", + "description", + "url", + "slug", + "certified_by", + "certification_details", + "changed_on", + *_subject_columns, + *_audit_columns, + ] + list_select_columns = list_columns + ["changed_by_fk", "created_on"] + add_columns = [ + "title", + "description", + "editors", + "viewers", + *_metadata_columns, + "definition", + ] + edit_columns = ["title", "description", "editors", "viewers", *_metadata_columns] + order_columns = ["title", "changed_on"] + search_columns = ["title", "id", "slug"] + search_filters = {"title": [CanvasAllTextFilter], "id": [CanvasEditableFilter]} + + add_model_schema = CanvasPostSchema() + edit_model_schema = CanvasPutSchema() + apispec_parameter_schemas = {"get_delete_ids_schema": get_delete_ids_schema} + + allowed_rel_fields = {"created_by", "changed_by", "editors", "viewers"} + order_rel_fields = {"editors": ("label", "asc"), "viewers": ("label", "asc")} + text_field_rel_fields = {"editors": "label", "viewers": "label"} + extra_fields_rel_fields = { + "editors": ["type", "active", "secondary_label", "img"], + "viewers": ["type", "active", "secondary_label", "img"], + } + related_field_filters = { Review Comment: Small thing: `created_by` is listed in `allowed_rel_fields` on line 310 but missing from both of these dicts, so `base_api.py:582` finds no filter and `BaseFilterRelatedUsers` never runs — `related/created_by` ends up returning all of `ab_user`, `EXCLUDE_USERS_FROM_LISTS` included. `changed_by` just below is fine and `WidgetRestApi` has both, so I'm fairly sure it's just a slip. The two `created_by` lines from `widgets/api.py:150,156` should cover it, and it'll fix the typeahead on that field too. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
