rusackas commented on code in PR #43499:
URL: https://github.com/apache/superset/pull/43499#discussion_r4372257501
##########
superset/security/manager.py:
##########
@@ -4682,6 +4687,175 @@ def semantic_view_before_update(
.values(perm=new_perm)
)
+ def saved_query_before_update(
+ self,
+ mapper: Mapper,
+ connection: Connection,
+ target: "SavedQuery",
+ ) -> None:
+ """
+ Keep a saved query's dependent charts' denormalized permission
+ columns in sync when its database, catalog, or schema changes.
+
+ ``SavedQuery.perm``/``catalog_perm``/``schema_perm`` are computed
+ properties, not persisted columns (unlike ``SqlaTable`` and
+ ``SemanticView``, which each get a PVM-rename dance in
+ ``dataset_before_update``/``semantic_view_before_update``), so there
+ is no FAB ViewMenu here to rename -- saved queries never get a
+ ``datasource_access`` PVM of their own. But ``Slice``'s
+ ``before_insert``/``before_update`` listener (``set_related_perm``)
+ copies those computed strings onto the chart at save time, and
+ nothing re-copies them if the saved query moves afterwards: a chart
+ would otherwise keep matching stale schema/catalog grants
+ indefinitely. This is the dependent-chart half of
+ ``semantic_view_before_update``, without the PVM-rename half.
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset.models.sql_lab import SavedQuery
+
+ sq_table = SavedQuery.__table__ # pylint: disable=no-member
+ current = connection.execute(
+ sq_table.select().where(sq_table.c.id == target.id)
+ ).one()
+
+ if (
+ current.db_id == target.db_id
+ and current.catalog == target.catalog
+ and current.schema == target.schema
+ ):
+ return
+
+ if target.database is None:
+ return
+
+ # Skip the perm computation (which may hit the warehouse via
+ # ``get_default_catalog``) when no chart references this saved query.
+ # pylint: disable=import-outside-toplevel
+ from superset.models.slice import Slice
+
+ chart_table = Slice.__table__ # pylint: disable=no-member
+ has_charts = connection.execute(
+ select(chart_table.c.id)
+ .where(
+ chart_table.c.datasource_type == DatasourceType.SAVEDQUERY,
+ chart_table.c.datasource_id == target.id,
+ )
+ .limit(1)
+ ).first()
+ if has_charts is None:
+ return
+
+ self._sync_saved_query_chart_perms(connection, target, target.database)
+
+ def _saved_query_chart_perm_values(
+ self,
+ saved_query: "SavedQuery",
+ database: "Database",
+ ) -> dict[str, Optional[str]]:
+ """
+ Compute the denormalized ``perm``/``catalog_perm``/``schema_perm`` a
+ chart backed by ``saved_query`` should carry, given ``database``.
+
+ ``database`` is passed explicitly rather than read off the saved query
+ because callers run inside a flush: ``SavedQuery.before_update`` may
+ have just reloaded it from a changed ``db_id``, and on a database
+ rename (``database_after_update``) the new ``database_name`` only
+ exists on the in-memory target.
+ """
+ database_name = database.database_name
+ return {
+ "perm":
f"[{database_name}].[{saved_query.name}](id:{saved_query.id})",
+ "catalog_perm": self.get_catalog_perm(database_name,
saved_query.catalog),
+ "schema_perm": self.get_schema_perm(
+ database_name,
+ saved_query.catalog or database.get_default_catalog(),
Review Comment:
@sadpandajoe added a test with `catalog=None` on a database with a default
catalog, plus a second saved query whose chart must stay 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]
