sadpandajoe commented on code in PR #45357: URL: https://github.com/apache/superset/pull/45357#discussion_r4350375232
########## superset/mcp_service/dashboard/filter_values.py: ########## @@ -0,0 +1,1251 @@ +# 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. +"""Authorized, bounded native dropdown discovery, independent of chart scope.""" + +from __future__ import annotations + +import logging +import math +import re +import time +from collections.abc import Iterator +from contextlib import contextmanager +from copy import deepcopy +from dataclasses import dataclass, field +from decimal import Decimal +from numbers import Integral, Real +from typing import Any, cast, Literal, TYPE_CHECKING + +from fastmcp.exceptions import ToolError +from flask import current_app, g +from marshmallow import ValidationError +from pandas import DataFrame, NA +from sqlalchemy.exc import CompileError, UnsupportedCompilationError + +from superset.charts.data.form_data import set_query_context_form_data +from superset.charts.schemas import ChartDataQueryContextSchema +from superset.commands.chart.data.get_data_command import ChartDataCommand +from superset.constants import EMPTY_FILTER_SQL_EXPRESSION, NO_TIME_RANGE +from superset.daos.dataset import DatasetDAO +from superset.dashboards.filter_scope import _is_divider +from superset.dashboards.permalink.schemas import DashboardPermalinkStateSchema +from superset.exceptions import ( + OAuth2Error, + OAuth2RedirectError, + SupersetSecurityException, +) +from superset.mcp_service.chart.chart_helpers import ( + build_query_dicts_from_form_data, + merge_extra_form_data, +) +from superset.mcp_service.dashboard.schemas import ( + _extract_native_filters, + FilterValueDiscovery, + FilterValueReason, + FilterValueRequest, + FilterValueScalar, + GetDashboardInfoRequest, + NativeFilterAvailableValues, + NativeFilterValueSummary, + ProposedFilterValues, +) +from superset.mcp_service.dashboard.tool.manage_native_filters import ( + _empty_select_data_mask, + _FilterValidationError, + _select_data_mask, + current_native_filter_config, +) +from superset.mcp_service.privacy import user_can_view_data_model_metadata +from superset.utils import json +from superset.utils.core import apply_max_row_limit, FilterOperator, GenericDataType + +if TYPE_CHECKING: + from superset.connectors.sqla.models import SqlaTable, TableColumn + from superset.models.dashboard import Dashboard + +_MAX_QUERIES = 20 +_SCHEDULING_SECONDS = 10 +_PAGE_SIZE = 20 +_MAX_SAFE_INTEGER = 2**53 - 1 +# Mirrors the dashboard's extractUrlParams('regular') exclusions. +_RESERVED_URL_PARAMS = frozenset({"standalone", "edit"}) +logger = logging.getLogger(__name__) + + +class _DiscoveryUnavailableError(Exception): + """A bounded, non-sensitive failure for a single dropdown.""" + + def __init__(self, reason: FilterValueReason) -> None: + super().__init__(reason) + self.reason = reason + + +@dataclass +class _Target: + """A configured table target; this is metadata, not an access grant.""" + + dataset: SqlaTable + column: TableColumn + + +@dataclass +class _Discovery: + """Call-local configuration, hydrated state, and execution accounting.""" + + dashboard: Dashboard + configs: dict[str, dict[str, Any]] + masks: dict[str, dict[str, Any]] + started: float = field(default_factory=time.monotonic) + query_count: int = 0 + proposal_errors: dict[str, FilterValueReason] = field(default_factory=dict) + targets: dict[ + tuple[int, str], + _Target | _DiscoveryUnavailableError | SupersetSecurityException, + ] = field(default_factory=dict) + mask_error_reasons: dict[str, FilterValueReason] = field(default_factory=dict) + normalized_masks: set[str] = field(default_factory=set) + url_params: dict[str, str] = field(default_factory=dict) + + +def _check_budget(context: _Discovery) -> None: + """Stop preparation and authorization after a call reaches either budget.""" + if context.query_count >= _MAX_QUERIES: + raise _DiscoveryUnavailableError("query_budget") + if time.monotonic() - context.started >= _SCHEDULING_SECONDS: + raise _DiscoveryUnavailableError("scheduling_budget") + + +def _configured_filters(dashboard: Dashboard) -> list[dict[str, Any]]: + """Read saved definitions in order, excluding visual dividers.""" + return [c for c in current_native_filter_config(dashboard) if not _is_divider(c)] + + +def _select_target(conf: dict[str, Any], context: _Discovery) -> _Target: + """Memoize a native target's metadata preflight, never its final query gate.""" + _check_budget(context) + controls = conf.get("controlValues") or {} + if ( + conf.get("filterType") != "filter_select" + or not isinstance(controls, dict) + or controls.get("inverseSelection") + or controls.get("operatorType", "exact") != "exact" + ): + raise _DiscoveryUnavailableError("unsupported_filter") + targets = conf.get("targets") + if not isinstance(targets, list) or len(targets) != 1: + raise _DiscoveryUnavailableError("unsupported_target") + target = targets[0] + if ( + not isinstance(target, dict) + or target.get("datasourceType") not in (None, "table") + or target.get("semantic_selection_version") + or type(target.get("datasetId")) is not int + or target["datasetId"] <= 0 + or not isinstance(target.get("column"), dict) + or not isinstance(target["column"].get("name"), str) + or not target["column"]["name"] + ): + raise _DiscoveryUnavailableError("unsupported_target") + key = (target["datasetId"], conf["id"]) + cached = context.targets.get(key) + if isinstance(cached, Exception): + raise cached + if cached is not None: + return cached + dataset = DatasetDAO.find_by_id(target["datasetId"], skip_base_filter=True) + if dataset is None: + raise _DiscoveryUnavailableError("unsupported_target") + column = next( + (col for col in dataset.columns if col.column_name == target["column"]["name"]), + None, + ) + if column is None: + raise _DiscoveryUnavailableError("unsupported_target") + # Native authorization preserves dashboard-only grants and guest limits. + # This memoized context is never executed. Every final query is authorized + # separately with its complete predicates before dataframe cache access. + access_context = ChartDataQueryContextSchema().load( + { + "datasource": {"id": dataset.id, "type": "table"}, + "queries": [ + {"columns": [column.column_name], "metrics": [], "row_limit": 1} + ], + "form_data": { + "type": "NATIVE_FILTER", + "dashboardId": context.dashboard.id, + "native_filter_id": conf["id"], + }, + "result_type": "full", + "result_format": "json", + } + ) + _check_budget(context) + try: + ChartDataCommand(access_context).validate() + except SupersetSecurityException as ex: + context.targets[key] = ex + raise + if column.type_generic not in ( + GenericDataType.STRING, + GenericDataType.NUMERIC, + GenericDataType.BOOLEAN, + ): + error = _DiscoveryUnavailableError("unsupported_target") + context.targets[key] = error + raise error + resolved = _Target(dataset, column) + context.targets[key] = resolved + return resolved + + +def _compatible(value: FilterValueScalar, target: _Target) -> bool: + """Compare JSON scalar families, keeping booleans distinct from numbers.""" + if value is None: + return True + if type(value) not in (str, bool, int, float) or ( + isinstance(value, float) and not math.isfinite(value) + ): + return False + if isinstance(value, str): + return target.column.type_generic == GenericDataType.STRING + if type(value) is bool: + return target.column.type_generic == GenericDataType.BOOLEAN + return target.column.type_generic == GenericDataType.NUMERIC + + +def _ancestors(filter_id: str, configs: dict[str, dict[str, Any]]) -> list[str]: + """Resolve ancestors furthest first and reject missing or cyclic graphs.""" + ordered: list[str] = [] + active: set[str] = set() + visited: set[str] = set() + + def visit(current: str) -> None: + """Walk one configuration, retaining stable parent ordering.""" + if current in active or current not in configs: + raise _DiscoveryUnavailableError("parent_unresolved") + if current in visited: + return + active.add(current) + parents = configs[current].get("cascadeParentIds") or [] + if not isinstance(parents, list) or any( + not isinstance(p, str) for p in parents + ): + raise _DiscoveryUnavailableError("parent_unresolved") + for parent in parents: + visit(parent) + active.remove(current) + visited.add(current) + if current != filter_id: + ordered.append(current) + + visit(filter_id) + return ordered + + +def _requests( + configs: list[dict[str, Any]], + request: GetDashboardInfoRequest, +) -> list[FilterValueRequest]: + """Choose explicit IDs or a bounded page before querying any datasource.""" + if request.filter_value_requests is not None: + return request.filter_value_requests + return [ + FilterValueRequest(filter_id=c["id"]) + for c in configs[ + request.filter_values_offset : request.filter_values_offset + _PAGE_SIZE + ] + ] + + +def validate_discovery_configuration( # noqa: C901 + dashboard: Dashboard, + request: GetDashboardInfoRequest, + *, + started_at: float | None = None, +) -> _Discovery: + """Reject invalid IDs, proposed ancestors, and scalar types before querying.""" + configured = _configured_filters(dashboard) + configs: dict[str, dict[str, Any]] = { + c["id"]: c for c in configured if isinstance(c.get("id"), str) + } + if any(not isinstance(c.get("id"), str) or not c["id"] for c in configured) or len( + configs + ) != len(configured): + raise ValueError("Native filter configuration has invalid or duplicate IDs") + context = _Discovery( + dashboard, + configs, + {}, + started=time.monotonic() if started_at is None else started_at, + ) + requests = _requests(configured, request) + for item in requests: + if item.filter_id not in configs: + raise ValueError(f"Unknown native filter ID: {item.filter_id}") + if "exact_value" in item.model_fields_set: + try: + target = _select_target(configs[item.filter_id], context) + except (_DiscoveryUnavailableError, SupersetSecurityException): + continue + if not _compatible(item.exact_value, target): + raise ValueError( + f"Incompatible exact value for native filter {item.filter_id}" + ) + allowed: set[str] = set() + for item in requests: + try: + allowed.update(_ancestors(item.filter_id, configs)) + except _DiscoveryUnavailableError: + pending = [item.filter_id] + visited: set[str] = set() + while pending: + current = pending.pop() + if current in visited or current not in configs: + continue + visited.add(current) + parents = configs[current].get("cascadeParentIds") + if isinstance(parents, list): + pending.extend(p for p in parents if isinstance(p, str)) + allowed.update(visited - {item.filter_id}) + for proposal in request.proposed_filter_values or []: + if proposal.filter_id not in configs or proposal.filter_id not in allowed: + raise ValueError( + "Proposed filters must be configured ancestors of requested filters" + ) + conf = configs[proposal.filter_id] + controls = conf.get("controlValues") or {} + if ( + isinstance(controls, dict) + and controls.get("multiSelect") is False + and len(proposal.values) > 1 + ): + raise ValueError("A single-select proposal accepts at most one value") + try: + target = _select_target(conf, context) + except (_DiscoveryUnavailableError, SupersetSecurityException): + continue + if any(not _compatible(value, target) for value in proposal.values): + raise ValueError( + f"Incompatible proposed value for native filter {proposal.filter_id}" + ) + return context + + +def _real_value(value: Any) -> bool: + """Mirror fillNativeFilters' empty/all-null-array restoration test.""" + return value is not None and not ( + isinstance(value, list) and all(v is None for v in value) + ) + + +def _explicit_clear(mask: dict[str, Any]) -> bool: + """Recognize only the frontend's canonical match-nothing clear evidence.""" + extra = mask.get("extraFormData") or {} + if not isinstance(extra, dict): + return False + predicates = extra.get("adhoc_filters") + return isinstance(predicates, list) and any( + isinstance(p, dict) + and p.get("expressionType") == "SQL" + and p.get("clause") == "WHERE" + and p.get("sqlExpression") == EMPTY_FILTER_SQL_EXPRESSION + for p in predicates + ) + + +def _validate_snapshot_value(conf: dict[str, Any], value: Any) -> None: # noqa: C901 + """Reject malformed known native selection shapes before any option query.""" + if value is None: + return + kind = conf.get("filterType") + if kind in ("filter_select", "filter_range", "filter_timegrain") and not isinstance( + value, list + ): + raise _DiscoveryUnavailableError("state_invalid") + if kind == "filter_range": + if len(value) != 2 or any( + item is not None and type(item) not in (int, float) for item in value + ): + raise _DiscoveryUnavailableError("state_invalid") + if all(item is not None for item in value) and value[0] > value[1]: + raise _DiscoveryUnavailableError("state_invalid") + elif kind == "filter_timegrain": + if len(value) > 1 or any(not isinstance(item, str) for item in value): + raise _DiscoveryUnavailableError("state_invalid") + elif kind == "filter_time" and not isinstance(value, str): + raise _DiscoveryUnavailableError("state_invalid") + + +def _normalize_select_mask( # noqa: C901 + context: _Discovery, + conf: dict[str, Any], + mask: dict[str, Any], +) -> dict[str, Any]: + """Validate effective selections and recover lossless browser numeric strings.""" + invalid_reason = context.mask_error_reasons[conf["id"]] + state = mask.get("filterState") + controls = conf.get("controlValues") or {} + if not isinstance(state, dict) or not isinstance(controls, dict): + return {"invalid_reason": invalid_reason} + values = state.get("value") + if conf.get("filterType") != "filter_select": + try: + _validate_snapshot_value(conf, values) + except _DiscoveryUnavailableError: + return {"invalid_reason": invalid_reason} + return mask + # Empty selections need no parent column metadata. Predicate reconstruction + # still applies required-clear and unresolved-state semantics downstream. + if values is None or values == []: + return mask + try: + target = _select_target(conf, context) + except SupersetSecurityException: + return {"invalid_reason": "permission_denied"} + except _DiscoveryUnavailableError as ex: + return ( + mask + if ex.reason.startswith("unsupported_") + else {"invalid_reason": ex.reason} + ) + if not isinstance(values, list): + return {"invalid_reason": invalid_reason} + if controls.get("multiSelect") is False and len(values) > 1: + return {"invalid_reason": invalid_reason} + normalized = [] + try: + for value in values: + if ( + target.column.type_generic == GenericDataType.NUMERIC + and isinstance(value, str) + and re.fullmatch( + r"-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?", value + ) + ): + value = _scalar(Decimal(value)) + if ( + target.column.type_generic == GenericDataType.BOOLEAN + and type(value) in (int, float) + and value in (0, 1) + ): + value = bool(value) + if not _compatible(value, target): + return {"invalid_reason": invalid_reason} + if not _safe_number(value): + return {"invalid_reason": "invalid_scalar"} + normalized.append(value) + except _DiscoveryUnavailableError as ex: + return {"invalid_reason": ex.reason} + except ArithmeticError: + return {"invalid_reason": "invalid_scalar"} + state["value"] = normalized + return mask + + +def _url_params(pairs: Any) -> dict[str, str]: + """Rebuild the dashboard's query url_params from snapshot key-value pairs. + + The dashboard sends every non-reserved URL parameter with each dropdown + query, so ``url_param`` macros in virtual dataset SQL render against them. + """ + if pairs is None: + return {} + if not isinstance(pairs, list): + raise _DiscoveryUnavailableError("state_invalid") + params: dict[str, str] = {} + for pair in pairs: + if pair is None: + # The permalink schema permits null entries; they carry no parameter. + continue + if ( + not isinstance(pair, (list, tuple)) + or len(pair) != 2 + or any(part is not None and not isinstance(part, str) for part in pair) + ): + raise _DiscoveryUnavailableError("state_invalid") + key, value = pair + if key is None or value is None or key in _RESERVED_URL_PARAMS: + continue + # URLSearchParams keeps the last value for a repeated key. + params[key] = value + return params + + +def _hydrate_masks( # noqa: C901 + context: _Discovery, + snapshot_state: dict[str, Any] | None, +) -> dict[str, dict[str, Any]]: + """Shallow-merge initial/default/loaded masks and restore required defaults.""" + configs = context.configs + loaded_masks: dict[str, Any] = {} + if snapshot_state is not None: + if not isinstance(snapshot_state, dict): + raise _DiscoveryUnavailableError("state_invalid") + supplied_masks = snapshot_state.get("dataMask") + if isinstance(supplied_masks, dict): + # Explicit masks are authoritative even with extra permalink metadata. + loaded_masks = supplied_masks + elif supplied_masks is not None: + raise _DiscoveryUnavailableError("state_invalid") + else: + try: + state = DashboardPermalinkStateSchema().load(snapshot_state) + except ValidationError as ex: + raise _DiscoveryUnavailableError("state_invalid") from ex + if any( + pair and pair[0] in ("native_filters", "native_filters_key") + for pair in state.get("urlParams") or [] + ): + # URL-encoded selections are not native dataMask state. + raise _DiscoveryUnavailableError("state_invalid") + context.url_params = _url_params(snapshot_state.get("urlParams")) + for filter_id, mask in loaded_masks.items(): + if not isinstance(filter_id, str) or not isinstance(mask, dict): + raise _DiscoveryUnavailableError("state_invalid") + if filter_id not in configs: + continue + if any( + key in mask and not isinstance(mask[key], dict) + for key in ("filterState", "extraFormData", "ownState") + ): + raise _DiscoveryUnavailableError("state_invalid") + value = mask.get("filterState", {}).get("value") + values = value if isinstance(value, list) else [value] + for scalar in values: + if scalar is not None and ( + type(scalar) not in (str, int, float, bool) + or (isinstance(scalar, float) and not math.isfinite(scalar)) + ): + raise _DiscoveryUnavailableError("state_invalid") + hydrated: dict[str, dict[str, Any]] = {} + for filter_id, conf in configs.items(): + default = conf.get("defaultDataMask") or {} + loaded = loaded_masks.get(filter_id, {}) + context.mask_error_reasons[filter_id] = ( + "state_invalid" if "filterState" in loaded else "parent_unresolved" + ) + if not isinstance(default, dict): + hydrated[filter_id] = {"invalid": True} + continue + mask = { + "filterState": {}, + "extraFormData": {}, + **deepcopy(default), + **deepcopy(loaded), + } + controls = conf.get("controlValues") or {} + loaded_state = loaded.get("filterState") or {} + if not isinstance(loaded_state, dict) or not isinstance(controls, dict): + hydrated[filter_id] = {"invalid": True} + continue + has_value = _real_value(loaded_state.get("value")) + explicit_clear = ( + conf.get("filterType") == "filter_select" + and not has_value + and _explicit_clear(loaded) + ) + if ( + controls.get("enableEmptyFilter") + and default + and not explicit_clear + and ( + not has_value + or (not loaded.get("extraFormData") and default.get("extraFormData")) + ) + ): + mask["filterState"] = deepcopy(default.get("filterState") or {}) + mask["extraFormData"] = deepcopy(default.get("extraFormData") or {}) + context.mask_error_reasons[filter_id] = "parent_unresolved" + hydrated[filter_id] = mask + return hydrated + + +def _selected_extra( # noqa: C901 + context: _Discovery, conf: dict[str, Any], mask: dict[str, Any] +) -> dict[str, Any]: + """Rebuild parent predicates from typed values, never executing snapshot SQL.""" + from superset.mcp_service.dashboard.tool.apply_dashboard_filters import ( + _range_data_mask, + _time_data_mask, + ) + + state = mask.get("filterState") + if mask.get("invalid_reason"): + raise _DiscoveryUnavailableError(mask["invalid_reason"]) + if mask.get("invalid") or not isinstance(state, dict): + raise _DiscoveryUnavailableError("parent_unresolved") + controls = conf.get("controlValues") or {} + value = state.get("value") + if controls.get("defaultToFirstItem") and "value" not in state: + raise _DiscoveryUnavailableError("parent_unresolved") + kind = conf.get("filterType") + if kind != "filter_select" and value is None and mask.get("extraFormData"): + # As with Select, predicates without a typed value cannot be rebuilt, + # and reading them as unfiltered would overstate the child's choices. + raise _DiscoveryUnavailableError("parent_unresolved") + try: + if kind == "filter_select": + values = [] if value is None else value + if values == [] and not ( + value is None + and mask.get("extraFormData") + and not _explicit_clear(mask) + ): + return cast( + dict[str, Any], _empty_select_data_mask(conf)["extraFormData"] + ) + target = _select_target(conf, context) + if not isinstance(values, list) or any( + not _compatible(v, target) for v in values + ): + raise _DiscoveryUnavailableError("parent_unresolved") + if any(not _safe_number(v) for v in values): + raise _DiscoveryUnavailableError("invalid_scalar") + # SQL-only state is not a selected value. A canonical clear is the + # sole SQL-shaped evidence we recognize and regenerate ourselves. + if ( + value is None + and mask.get("extraFormData") + and not _explicit_clear(mask) + ): + raise _DiscoveryUnavailableError("parent_unresolved") + return cast( + dict[str, Any], _select_data_mask(conf, values)["extraFormData"] + ) + if kind == "filter_time": + if value is not None and not isinstance(value, str): + raise _DiscoveryUnavailableError("parent_unresolved") + return cast( + dict[str, Any], + _time_data_mask(conf, value or NO_TIME_RANGE)["extraFormData"], + ) + if kind == "filter_range": + values = [None, None] if value is None else value + if ( + not isinstance(values, list) + or len(values) != 2 + or any( + v is not None + and (type(v) not in (int, float) or not math.isfinite(v)) + for v in values + ) + ): + raise _DiscoveryUnavailableError("parent_unresolved") + if all(v is not None for v in values) and values[0] > values[1]: + raise _DiscoveryUnavailableError("parent_unresolved") + return cast(dict[str, Any], _range_data_mask(conf, values)["extraFormData"]) + if kind == "filter_timegrain": + values = [] if value is None else value + if ( + not isinstance(values, list) + or len(values) > 1 + or any(not isinstance(v, str) for v in values) + ): + raise _DiscoveryUnavailableError("parent_unresolved") + if not values: + if controls.get("enableEmptyFilter"): + raise _DiscoveryUnavailableError("parent_unresolved") + return {} + targets = conf.get("targets") + if ( + not isinstance(targets, list) + or len(targets) != 1 + or not isinstance(targets[0], dict) + ): + raise _DiscoveryUnavailableError("parent_unresolved") + grain_target = targets[0] + if ( + grain_target.get("datasourceType") not in (None, "table") + or type(grain_target.get("datasetId")) is not int + ): + raise _DiscoveryUnavailableError("parent_unresolved") + dataset = DatasetDAO.find_by_id( + grain_target["datasetId"], skip_base_filter=True + ) + allowed = conf.get("time_grains") + if ( + dataset is None + or (allowed and values[0] not in allowed) + or values[0] + not in [duration for duration, _ in dataset.time_grain_sqla] + ): + raise _DiscoveryUnavailableError("parent_unresolved") + return {"time_grain_sqla": values[0]} + except _DiscoveryUnavailableError as ex: + if ex.reason in ("invalid_scalar", "query_budget", "scheduling_budget"): + raise + raise _DiscoveryUnavailableError("parent_unresolved") from ex + except ArithmeticError as ex: + raise _DiscoveryUnavailableError("invalid_scalar") from ex + except (TypeError, ValueError, _FilterValidationError) as ex: + raise _DiscoveryUnavailableError("parent_unresolved") from ex + raise _DiscoveryUnavailableError("parent_unresolved") + + +def _parent_extra(context: _Discovery, filter_id: str) -> dict[str, Any]: + """Merge transitive parents in native order, excluding self and nonparents.""" + extra: dict[str, Any] = {} + for parent in _ancestors(filter_id, context.configs): + if parent in context.proposal_errors: + raise _DiscoveryUnavailableError(context.proposal_errors[parent]) + if parent not in context.normalized_masks: + context.masks[parent] = _normalize_select_mask( + context, context.configs[parent], context.masks[parent] + ) + context.normalized_masks.add(parent) + extra = merge_extra_form_data( + extra, + _selected_extra(context, context.configs[parent], context.masks[parent]), + ) + return extra + + +def _native_query( # noqa: C901 + context: _Discovery, + conf: dict[str, Any], + target: _Target, + item: FilterValueRequest, + proposal: ProposedFilterValues | None = None, +) -> tuple[ChartDataCommand, int]: + """Build the Select dropdown QueryContext and authorize it before cache use.""" + column = target.column.column_name + controls = conf.get("controlValues") or {} + metric = conf.get("sortMetric") or controls.get("sortMetric") + if metric is not None and not isinstance(metric, str): + raise _DiscoveryUnavailableError("unsupported_target") + configured_limit = current_app.config.get("NATIVE_FILTER_DEFAULT_ROW_LIMIT", 1000) + server_limit = apply_max_row_limit(item.limit + 1) + if type(configured_limit) is not int or configured_limit <= 0 or server_limit <= 0: + raise _DiscoveryUnavailableError("query_budget") + limit = min(item.limit + 1, server_limit, configured_limit) + parents = _parent_extra(context, conf["id"]) + parent_filters = parents.pop("filters", []) + form_data = { + **deepcopy(controls), + "datasource": f"{target.dataset.id}__table", + "groupby": [column], + "metrics": [], + "row_limit": limit, Review Comment: Discovery hands its bounded `limit + 1` to the dataset as the Jinja `row_limit`, while the dashboard's own dropdown renders it with the native row limit (1000 by default). A virtual dataset such as `SELECT country FROM sales ORDER BY country LIMIT {{ row_limit }}` with 30 `A` rows followed by `Z` then returns `complete: ["A"]` for a default listing and `complete: []` for `exact_value: "Z"`, because the inner SQL is cut at 26 rows before `_run` sees the outer cap and the template-filter guard never fires. Should the native row limit be kept for template rendering separately from the output bound, or should completeness be refused when the dataset consumes `row_limit`? ########## superset/models/helpers.py: ########## @@ -220,6 +221,53 @@ def build_like_predicate( return sa.func.lower(expr).contains(search.lower(), autoescape=True) +def build_native_like_predicate( + expr: ColumnElement[Any], search: str, engine_spec: type["BaseEngineSpec"] +) -> ColumnElement[Any]: + """Build the internal native search predicate with dialect-specific literals.""" + from superset.db_engine_specs.mssql import MssqlEngineSpec + + lowered = sa.func.lower(expr) + if engine_spec.engine == "bigquery": + # STRPOS has no LIKE escape layer: trailing backslashes remain plain data. + return sa.func.strpos(lowered, sa.func.lower(sa.literal(search))) > 0 + escaped = search.replace("/", "//").replace("%", "/%").replace("_", "/_") + if issubclass(engine_spec, MssqlEngineSpec): + # SQL Server and its derivatives treat '[' as a wildcard-class opener. + escaped = escaped.replace("[", "/[") + return lowered.like(sa.func.lower(sa.literal("%" + escaped + "%")), escape="/") Review Comment: This helper always passes `escape="/"` to `like()`, and the ClickHouse SQLAlchemy compiler (`clickhouse_connect` `visit_like_op_binary`) raises `CompileError: ClickHouse does not support the ESCAPE clause on LIKE` when an escape is set. A `search` on any ClickHouse-backed filter, e.g. `{"filter_id": "country", "search": "Fran"}`, therefore fails to compile, while the dashboard's own dropdown (escape-free ILIKE) works. Should the literal-contains predicate move into an engine-spec hook, with ClickHouse using a substring function such as `positionCaseInsensitive`, like the BigQuery branch above? ########## tests/unit_tests/mcp_service/dashboard/tool/test_get_dashboard_filter_values.py: ########## @@ -0,0 +1,3211 @@ +# 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. +"""Native choice queries preserve their authorization, scope, and exact values.""" + +from collections.abc import Iterator +from copy import deepcopy +from decimal import Decimal +from typing import Any, Literal +from unittest.mock import Mock + +import pytest +from flask import current_app, g +from pytest_mock import MockerFixture +from sqlalchemy import column, create_engine, select, String, text +from sqlalchemy.dialects import mysql, postgresql, sqlite + +from superset.commands.chart.data.get_data_command import ChartDataCommand +from superset.connectors.sqla.models import SqlaTable, TableColumn +from superset.constants import EMPTY_FILTER_SQL_EXPRESSION +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import SupersetSecurityException +from superset.mcp_service.dashboard import filter_values as choices +from superset.mcp_service.dashboard.schemas import ( + GetDashboardInfoRequest, + NativeFilterAvailableValues, + NativeFilterValueSummary, +) +from superset.models.core import Database +from superset.models.dashboard import Dashboard +from superset.models.helpers import build_like_predicate +from superset.utils import json +from tests.unit_tests.conftest import with_feature_flags + + [email protected] +def native_query(mocker: MockerFixture, app_context: None) -> dict[str, Any]: + """Use real QueryContext construction with query execution at a test boundary.""" + from superset.db_engine_specs.sqlite import SqliteEngineSpec + + mocker.patch.object(Database, "get_db_engine_spec", return_value=SqliteEngineSpec) + dataset = SqlaTable( + id=5, + table_name="sales", + database=Database(id=1, database_name="test", sqlalchemy_uri="sqlite://"), + ) + dataset.columns = [ + TableColumn(column_name="region", type="VARCHAR"), + TableColumn(column_name="country", type="VARCHAR"), + TableColumn(column_name="amount", type="INTEGER"), + TableColumn(column_name="enabled", type="BOOLEAN"), + ] + mocker.patch.object(choices.DatasetDAO, "find_by_id", return_value=dataset) + mocker.patch( + "superset.daos.datasource.DatasourceDAO.get_datasource", return_value=dataset + ) + mocker.patch( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + return_value="sqlite", + ) + mocker.patch.object( + choices, "user_can_view_data_model_metadata", return_value=False + ) + authorize = mocker.patch("superset.security_manager.raise_for_access") + query_contexts: list[Any] = [] + rows: list[dict[str, Any]] = [{"region": "Bikes"}] + + def execute(command: ChartDataCommand, **kwargs: Any) -> dict[str, Any]: + """Record the authorized context before returning warehouse rows.""" + query_contexts.append(command.query_context) + return {"queries": [{"data": deepcopy(rows)}]} + + run = mocker.patch.object(choices, "_execute", autospec=True, side_effect=execute) + return { + "dataset": dataset, + "contexts": query_contexts, + "rows": rows, + "run": run, + "authorize": authorize, + } + + +def _filter(filter_id: str = "region", **kwargs: Any) -> dict[str, Any]: + """Build a normal saved table select with no chosen value.""" + return { + "id": filter_id, + "name": filter_id, + "filterType": "filter_select", + "type": "NATIVE_FILTER", + "targets": [{"datasetId": 5, "column": {"name": filter_id}}], + "controlValues": {"multiSelect": True}, + "defaultDataMask": {"filterState": {"value": None}, "extraFormData": {}}, + **kwargs, + } + + +def _dashboard(*configs: dict[str, Any]) -> Dashboard: + """Build the authorized dashboard passed to the discovery helper.""" + return Dashboard( + id=1, + dashboard_title="Sales", + json_metadata=json.dumps( + {"native_filter_configuration": list(configs or [_filter()])} + ), + ) + + +def _request(**kwargs: Any) -> GetDashboardInfoRequest: + """Construct a validated opted-in request.""" + return GetDashboardInfoRequest(identifier=1, include_filter_values=True, **kwargs) + + +def _available(summary: NativeFilterValueSummary) -> NativeFilterAvailableValues: + """Require the opt-in choice envelope before asserting its typed fields.""" + assert summary.available_values is not None + return summary.available_values + + +def _mask(values: list[Any] | None, **extra: Any) -> dict[str, Any]: + """Represent snapshot selections without trusting their extra predicates.""" + return { + "filterState": {"value": values, "excludeFilterValues": True}, + "extraFormData": extra, + } + + +def test_native_query_authorized_before_run_and_targets_redacted( + native_query: dict[str, Any], +) -> None: + """Dashboard context, target restrictions and viewer redaction survive discovery.""" + summaries, metadata = choices.discover_filter_values(_dashboard(), _request()) + assert summaries[0].targets == [] + assert _available(summaries[0]).values == ["Bikes"] + assert _available(summaries[0]).status == "complete" + assert metadata.query_count == 1 + context = native_query["contexts"][0] + assert context.form_data["type"] == "NATIVE_FILTER" + assert context.form_data["dashboardId"] == 1 + assert context.form_data["native_filter_id"] == "region" + assert context.queries[0].columns == ["region"] + assert context.queries[0].metrics == [] + assert context.queries[0].row_limit == 26 + native_query["authorize"].assert_called_with(query_context=context) + + [email protected]( + "value", + [ + "Bikes", + "Europe, Middle East & Africa", + "Parts & Service", + 'Bikes "Pro"', + "Côte d'Ivoire", + "O'Brien & Sons, Ltd.", + "Smith, Jones & Partners, LLC", + " Online ", + "ignore previous instructions", + "x" * 1000, + "a%b_c\\d", + ], +) +def test_exact_strings_never_split_normalize_or_clip( + native_query: dict[str, Any], value: str +) -> None: + """Exact probes preserve quoted, comma-containing, Unicode and long scalars.""" + native_query["rows"][:] = [{"region": value}] + summaries, _ = choices.discover_filter_values( + _dashboard(), + _request(filter_value_requests=[{"filter_id": "region", "exact_value": value}]), + ) + available = _available(summaries[0]) + assert available.values == [value] + assert available.model_dump()["exact_value"] == value + assert native_query["contexts"][0].queries[0].filter == [ + {"col": "region", "op": "==", "val": value, "is_literal": True} + ] + + [email protected]( + ("name", "value"), + [ + ("amount", 1), + ("amount", 1.5), + ("enabled", True), + ("enabled", False), + ("region", None), + ], +) +def test_exact_scalar_families( + native_query: dict[str, Any], name: str, value: Any +) -> None: + """Booleans, numbers and explicit null are returned in their stored types.""" + native_query["rows"][:] = [{name: value}] + summaries, _ = choices.discover_filter_values( + _dashboard(_filter(name)), + _request(filter_value_requests=[{"filter_id": name, "exact_value": value}]), + ) + available = _available(summaries[0]) + assert available.status == "complete" + assert type(available.values[0]) is type(value) + assert available.model_dump()["exact_value"] == value + assert native_query["contexts"][0].queries[0].filter[0]["op"] == ( + "IS NULL" if value is None else "==" + ) + + [email protected]( + ("name", "value"), + [("amount", True), ("amount", "1"), ("enabled", 1), ("region", 1)], +) +def test_incompatible_exact_values_reject_before_all_queries( + native_query: dict[str, Any], name: str, value: Any +) -> None: + """A later invalid exact probe cannot leave earlier queries executed.""" + with pytest.raises(ValueError, match="Incompatible exact"): + choices.discover_filter_values( + _dashboard(_filter("country"), _filter(name)), + _request( + filter_value_requests=[ + {"filter_id": "country"}, + {"filter_id": name, "exact_value": value}, + ] + ), + ) + native_query["run"].assert_not_called() + + +def test_search_filters_domain_before_limit_with_literal_wildcards( + native_query: dict[str, Any], +) -> None: + """Search is a literal escaped SQL predicate, never a scan of the sample.""" + native_query["rows"][:] = [{"region": "later 10%_O'Brien/\\"}] + summaries, _ = choices.discover_filter_values( + _dashboard(), + _request( + filter_value_requests=[ + {"filter_id": "region", "search": "10%_O'Brien/\\", "limit": 1} + ] + ), + ) + assert _available(summaries[0]).status == "complete" + context = native_query["contexts"][0] + assert context.queries[0].filter == [ + {"col": "region", "op": "ILIKE", "val": "10%_O'Brien/\\", "is_literal": True} + ] + assert context.queries[0].orderby == [] + assert context.queries[0].row_limit == 2 + + [email protected]( + "dialect", [sqlite.dialect(), postgresql.dialect(), mysql.dialect()] +) +def test_literal_search_compiles_with_engine_escaping(dialect: Any) -> None: + """Supported SQLAlchemy dialects keep wildcard/escape characters literal.""" + expression = build_like_predicate(column("region", String), "100%_O'Brien/\\") + compiled = str( + expression.compile(dialect=dialect, compile_kwargs={"literal_binds": True}) + ) + assert "ESCAPE '/'" in compiled + assert "/_o''brien//" in compiled + + +def test_literal_search_executes_exact_substring_on_sqlite() -> None: + """A wildcard-bearing request matches only the literal substring.""" + engine = create_engine("sqlite://") + with engine.connect() as connection: + connection.execute(text("create table sales (region text)")) + connection.execute( + text("insert into sales values (:value)"), + [{"value": v} for v in ["10%_O'Brien/\\", "10xx_O'Brien/\\", "other"]], + ) + expression = build_like_predicate(column("region", String), "10%_O'Brien/\\") + values = ( + connection.execute( + select(column("region")).select_from(text("sales")).where(expression) + ) + .scalars() + .all() + ) + assert values == ["10%_O'Brien/\\"] + + +def test_search_numeric_is_unsupported_without_query( + native_query: dict[str, Any], +) -> None: + """Search never casts numeric domains to string.""" + summaries, meta = choices.discover_filter_values( + _dashboard(_filter("amount")), + _request(filter_value_requests=[{"filter_id": "amount", "search": "1"}]), + ) + assert _available(summaries[0]).reason == "unsupported_search" + assert meta.query_count == 0 + + +def test_saved_predicates_sort_and_transitive_parents( + native_query: dict[str, Any], +) -> None: + """Only configured ancestors filter the dropdown; self and nonparents do not.""" + parent = _filter(defaultDataMask=_mask(["Americas"])) + middle = _filter( + "country", cascadeParentIds=["region"], defaultDataMask=_mask(["France"]) + ) + target = _filter( + "amount", + cascadeParentIds=["country"], + time_range="2020-01-01 : 2021-01-01", + granularity_sqla="date", + adhoc_filters=[ + { + "expressionType": "SIMPLE", + "clause": "WHERE", + "subject": "enabled", + "operator": "==", + "comparator": True, + } + ], + controlValues={"sortMetric": "count", "sortAscending": False}, + ) + native_query["rows"][:] = [{"amount": 9}] + snapshot = {"dataMask": {"amount": _mask([1]), "enabled": _mask([False])}} + summaries, _ = choices.discover_filter_values( + _dashboard(parent, middle, target, _filter("enabled")), + _request(filter_value_requests=[{"filter_id": "amount"}]), + snapshot_state=snapshot, + ) + assert _available(summaries[0]).status == "complete" + query = native_query["contexts"][0].queries[0] + assert query.filter == [ + {"col": "enabled", "op": "==", "val": True}, + {"col": "region", "op": "IN", "val": ["Americas"]}, + {"col": "country", "op": "IN", "val": ["France"]}, + ] + assert query.time_range == "2020-01-01 : 2021-01-01" + assert query.granularity == "date" + assert query.metrics[0] == "count" + assert query.metrics[1]["aggregate"] == "COUNT" + assert query.orderby == [("count", False)] + assert snapshot["dataMask"]["amount"]["filterState"]["value"] == [1] + + [email protected]( + ("required", "loaded", "expected"), + [ + (False, None, ["Default"]), + (False, _mask([]), []), + (True, _mask([]), ["Default"]), + (True, _mask([None]), ["Default"]), + (True, _mask(["Loaded"]), ["Default"]), + (True, _mask(["Loaded"], filters=[{"untrusted": True}]), ["Loaded"]), + ], +) +def test_snapshot_hydration_matches_required_default_restoration( + native_query: dict[str, Any], + required: bool, + loaded: dict[str, Any] | None, + expected: list[str], +) -> None: + """Required incomplete states restore defaults; optional explicit clears remain.""" + parent = _filter( + controlValues={"enableEmptyFilter": required}, + defaultDataMask=_mask( + ["Default"], filters=[{"col": "region", "op": "IN", "val": ["Default"]}] + ), + ) + child = _filter("country", cascadeParentIds=["region"]) + native_query["rows"][:] = [{"country": "France"}] + snapshot = {"dataMask": {"region": loaded} if loaded is not None else {}} + summaries, _ = choices.discover_filter_values( + _dashboard(parent, child), + _request(filter_value_requests=[{"filter_id": "country"}]), + snapshot_state=snapshot, + ) + assert _available(summaries[0]).status == "complete" + assert native_query["contexts"][0].queries[0].filter == ( + [{"col": "region", "op": "IN", "val": expected}] if expected else [] + ) + + +def test_required_explicit_clear_is_regenerated_and_caller_sql_not_copied( + native_query: dict[str, Any], +) -> None: + """Canonical match-nothing evidence survives hydration without trusting SQL.""" + parent = _filter( + controlValues={"enableEmptyFilter": True}, defaultDataMask=_mask(["Default"]) + ) + child = _filter("country", cascadeParentIds=["region"]) + clear = _mask( + [], + adhoc_filters=[ + { + "expressionType": "SQL", + "clause": "WHERE", + "sqlExpression": EMPTY_FILTER_SQL_EXPRESSION, + }, + { + "expressionType": "SQL", + "clause": "WHERE", + "sqlExpression": "dangerous()", + }, + ], + ) + native_query["rows"][:] = [] + summaries, _ = choices.discover_filter_values( + _dashboard(parent, child), + _request(filter_value_requests=[{"filter_id": "country"}]), + snapshot_state={"dataMask": {"region": clear}}, + ) + assert _available(summaries[0]).status == "complete" + query = native_query["contexts"][0].queries[0] + assert EMPTY_FILTER_SQL_EXPRESSION in query.extras["where"] + assert "dangerous" not in query.extras["where"] + + [email protected]("reason", ["state_missing", "state_invalid"]) +def test_bad_whole_snapshot_never_falls_back( + native_query: dict[str, Any], reason: Literal["state_missing", "state_invalid"] +) -> None: + """An invalid trusted/supplied snapshot yields no default-backed queries.""" + summaries, meta = choices.discover_filter_values( + _dashboard(), _request(), state_error=reason + ) + assert _available(summaries[0]).reason == reason + assert meta.query_count == 0 + native_query["run"].assert_not_called() + + [email protected]("parents", [["missing"], ["country"], "region"]) +def test_broken_parent_graph_is_unavailable( + native_query: dict[str, Any], parents: Any +) -> None: + """Missing nodes, cycles and malformed parent lists fail closed.""" + summaries, meta = choices.discover_filter_values( + _dashboard(_filter("country", cascadeParentIds=parents)), _request() + ) + assert _available(summaries[0]).reason == "parent_unresolved" + assert meta.query_count == 0 + + +def test_first_item_parent_without_resolved_state_is_unavailable( + native_query: dict[str, Any], +) -> None: + """The server does not guess a dynamic default-to-first parent.""" + summaries, _ = choices.discover_filter_values( + _dashboard( + _filter( + controlValues={"defaultToFirstItem": True}, + defaultDataMask={"filterState": {}}, + ), + _filter("country", cascadeParentIds=["region"]), + ), + _request(filter_value_requests=[{"filter_id": "country"}]), + ) + assert _available(summaries[0]).reason == "parent_unresolved" + + +def test_proposed_parent_preview_is_validated_and_read_only( Review Comment: `_validate_proposals` orders proposals by ancestry so each one is checked against its already-validated parents, but every proposal test here supplies a single ancestor, so replacing that ordering with request order would still pass. With `region -> country -> amount`, a snapshot of Americas/US and `proposed_filter_values=[country=France, region=EMEA]`, request-order validation would reject France against Americas, while dependency order validates both and lets `amount` come back `complete`. Could a test cover that case (assert both proposals validate, three queries run, and the snapshot is unchanged)? -- 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]
