tien238lnd commented on code in PR #44338:
URL: https://github.com/apache/superset/pull/44338#discussion_r4033097060
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -86,6 +91,106 @@ def _build_unsaved_chart_info(form_data_key: str) ->
ChartInfo | ChartError:
)
+def _get_explore_permalink(
+ permalink_key: str,
+) -> ExplorePermalinkValue | ChartError:
+ """Read an Explore permalink, enforcing the same access checks as Explore.
+
+ ``GetExplorePermalinkCommand`` checks access to the permalink's datasource
+ and, when it references a saved chart, to that chart.
+ """
+ from superset.commands.explore.permalink.get import
GetExplorePermalinkCommand
+
+ try:
+ value = GetExplorePermalinkCommand(permalink_key).run()
+ except ForbiddenError:
+ return ChartError(
+ error="You do not have access to the chart or dataset in this
permalink.",
+ error_type="PermalinkAccessDenied",
+ )
+ except (CommandException, SQLAlchemyError, ValidationError, ValueError) as
ex:
+ # ValidationError: the permalink's datasource no longer exists or has
an
+ # invalid type (raised by the access check).
+ logger.warning("Failed to read explore permalink: %s", ex)
+ return ChartError(
+ error="The explore permalink could not be read. Check the key.",
+ error_type="InvalidPermalink",
+ )
+ if not value:
+ return ChartError(
+ error="No explore permalink found for permalink_key.",
+ error_type="NotFound",
+ )
+ return value
+
+
+def _permalink_chart_id(permalink: ExplorePermalinkValue) -> int | None:
+ """Return the saved chart a permalink was created from, if any.
+
+ ``chartId`` is copied from the client-supplied ``formData.slice_id``, so
+ it is not guaranteed to be an int.
+ """
+ try:
+ return int(permalink.get("chartId") or 0) or None
+ except (TypeError, ValueError):
+ return None
+
+
+def _permalink_form_data(permalink: ExplorePermalinkValue) -> dict[str, Any]:
+ state = permalink.get("state")
+ form_data = state.get("formData") if isinstance(state, dict) else None
+ return dict(form_data) if isinstance(form_data, dict) else {}
+
+
+def _permalink_datasource(
+ permalink: ExplorePermalinkValue,
+) -> tuple[str | None, str | None]:
+ """Return the (name, type) of the datasource a permalink was built on.
+
+ A permalink's form_data carries the datasource as an opaque "<id>__<type>"
+ string, so the name has to be resolved from the ids the permalink stores
+ alongside it. Access to that datasource was already checked by
+ ``GetExplorePermalinkCommand``.
+ """
+ datasource_type = permalink.get("datasourceType") or
DatasourceType.TABLE.value
+ datasource_id = permalink.get("datasourceId") or permalink.get("datasetId")
+ if not datasource_id:
+ return None, str(datasource_type)
+ try:
+ from superset.daos.datasource import DatasourceDAO
+
+ datasource = DatasourceDAO.get_datasource(
+ datasource_type=DatasourceType(datasource_type),
+ database_id_or_uuid=datasource_id,
+ )
+ except Exception: # noqa: BLE001
+ # A deleted or unsupported datasource must not sink the whole read;
+ # the rest of the permalink state is still worth returning.
+ logger.warning(
+ "Could not resolve permalink datasource %s of type %s",
+ datasource_id,
+ datasource_type,
+ )
+ return None, str(datasource_type)
+ return datasource.datasource_name, str(datasource_type)
Review Comment:
Fixed in 222adb3d14 with your fallback, unchanged.
`test_permalink_without_saved_chart_names_its_sql_lab_query` builds the
permalink on `5__query` and has `get_datasource` return a real
`Query(tab_name="Untitled Query")`; it fails on the previous commit and passes
now. The existing table-name test also uses a real `SqlaTable` now instead of a
`MagicMock`, for the same reason.
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -86,6 +91,106 @@ def _build_unsaved_chart_info(form_data_key: str) ->
ChartInfo | ChartError:
)
+def _get_explore_permalink(
+ permalink_key: str,
+) -> ExplorePermalinkValue | ChartError:
+ """Read an Explore permalink, enforcing the same access checks as Explore.
+
+ ``GetExplorePermalinkCommand`` checks access to the permalink's datasource
+ and, when it references a saved chart, to that chart.
+ """
+ from superset.commands.explore.permalink.get import
GetExplorePermalinkCommand
+
+ try:
+ value = GetExplorePermalinkCommand(permalink_key).run()
+ except ForbiddenError:
+ return ChartError(
+ error="You do not have access to the chart or dataset in this
permalink.",
+ error_type="PermalinkAccessDenied",
+ )
+ except (CommandException, SQLAlchemyError, ValidationError, ValueError) as
ex:
Review Comment:
Fixed in 222adb3d14: `SupersetSecurityException` now maps to
`PermalinkAccessDenied`, next to `ForbiddenError`. Covered by
`test_permalink_query_access_denied`.
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -86,6 +91,106 @@ def _build_unsaved_chart_info(form_data_key: str) ->
ChartInfo | ChartError:
)
+def _get_explore_permalink(
+ permalink_key: str,
+) -> ExplorePermalinkValue | ChartError:
+ """Read an Explore permalink, enforcing the same access checks as Explore.
+
+ ``GetExplorePermalinkCommand`` checks access to the permalink's datasource
+ and, when it references a saved chart, to that chart.
+ """
+ from superset.commands.explore.permalink.get import
GetExplorePermalinkCommand
+
+ try:
+ value = GetExplorePermalinkCommand(permalink_key).run()
+ except ForbiddenError:
+ return ChartError(
+ error="You do not have access to the chart or dataset in this
permalink.",
+ error_type="PermalinkAccessDenied",
+ )
+ except (CommandException, SQLAlchemyError, ValidationError, ValueError) as
ex:
Review Comment:
Fixed in 222adb3d14: `SupersetTemplateException` now maps to
`InvalidPermalink`, and the error message does not echo the template. Covered
by `test_permalink_query_with_template_error`.
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -86,6 +91,106 @@ def _build_unsaved_chart_info(form_data_key: str) ->
ChartInfo | ChartError:
)
+def _get_explore_permalink(
+ permalink_key: str,
+) -> ExplorePermalinkValue | ChartError:
+ """Read an Explore permalink, enforcing the same access checks as Explore.
+
+ ``GetExplorePermalinkCommand`` checks access to the permalink's datasource
+ and, when it references a saved chart, to that chart.
+ """
+ from superset.commands.explore.permalink.get import
GetExplorePermalinkCommand
+
+ try:
+ value = GetExplorePermalinkCommand(permalink_key).run()
+ except ForbiddenError:
+ return ChartError(
+ error="You do not have access to the chart or dataset in this
permalink.",
+ error_type="PermalinkAccessDenied",
+ )
+ except (CommandException, SQLAlchemyError, ValidationError, ValueError) as
ex:
Review Comment:
Same as the Copilot thread on this line: handled in 222adb3d14,
`SupersetTemplateException` maps to `InvalidPermalink`.
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -86,6 +91,106 @@ def _build_unsaved_chart_info(form_data_key: str) ->
ChartInfo | ChartError:
)
+def _get_explore_permalink(
+ permalink_key: str,
+) -> ExplorePermalinkValue | ChartError:
+ """Read an Explore permalink, enforcing the same access checks as Explore.
+
+ ``GetExplorePermalinkCommand`` checks access to the permalink's datasource
+ and, when it references a saved chart, to that chart.
+ """
+ from superset.commands.explore.permalink.get import
GetExplorePermalinkCommand
+
+ try:
+ value = GetExplorePermalinkCommand(permalink_key).run()
+ except ForbiddenError:
+ return ChartError(
+ error="You do not have access to the chart or dataset in this
permalink.",
+ error_type="PermalinkAccessDenied",
+ )
+ except (CommandException, SQLAlchemyError, ValidationError, ValueError) as
ex:
+ # ValidationError: the permalink's datasource no longer exists or has
an
+ # invalid type (raised by the access check).
+ logger.warning("Failed to read explore permalink: %s", ex)
+ return ChartError(
+ error="The explore permalink could not be read. Check the key.",
+ error_type="InvalidPermalink",
+ )
+ if not value:
+ return ChartError(
+ error="No explore permalink found for permalink_key.",
+ error_type="NotFound",
+ )
+ return value
+
+
+def _permalink_chart_id(permalink: ExplorePermalinkValue) -> int | None:
+ """Return the saved chart a permalink was created from, if any.
+
+ ``chartId`` is copied from the client-supplied ``formData.slice_id``, so
+ it is not guaranteed to be an int.
+ """
+ try:
+ return int(permalink.get("chartId") or 0) or None
+ except (TypeError, ValueError):
+ return None
+
+
+def _permalink_form_data(permalink: ExplorePermalinkValue) -> dict[str, Any]:
+ state = permalink.get("state")
+ form_data = state.get("formData") if isinstance(state, dict) else None
+ return dict(form_data) if isinstance(form_data, dict) else {}
Review Comment:
Left out on purpose. `get_chart_info` describes a chart through its
`form_data`, and that is the part of the permalink that replaces the saved
chart's. `urlParams` are extra query-string parameters Explore puts back on its
URL, and `chartState` is viz-specific interactive state (for example table
sorting) that the frontend turns into `ownState` through `chartStateConverter`.
Neither is part of the chart definition. If a caller turns out to need them,
they can be exposed as separate fields in a follow-up.
--
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]
