mikebridge commented on code in PR #44905:
URL: https://github.com/apache/superset/pull/44905#discussion_r4198813227
##########
superset/security/manager.py:
##########
@@ -4334,6 +4335,74 @@ def semantic_layer_before_update(
.values(perm=new_view_perm)
)
+ def semantic_layer_before_delete(
+ self,
+ mapper: Mapper,
+ connection: Connection,
+ target: "SemanticLayer",
+ ) -> None:
+ """
+ Remove child view permissions before the layer row is deleted.
+
+ Views the session has not loaded are deleted by the database
+ ``ON DELETE CASCADE`` (``passive_deletes=True``), so their ORM
+ ``after_delete`` hook never runs. Read their perms through the
+ connection while the rows still exist; views the ORM deletes itself
+ are already gone by now and clean up in ``semantic_view_after_delete``.
+ """
+ from superset.semantic_layers.models import ( # pylint:
disable=import-outside-toplevel
+ SemanticView,
+ )
+
+ sv_table = SemanticView.__table__ # pylint: disable=no-member
+ views: Sequence[Row[Any]] = connection.execute(
+ sv_table.select().where(sv_table.c.semantic_layer_uuid ==
target.uuid)
Review Comment:
Thanks, good catch. The later batch-cleanup commit already changed the
layer-deletion hook to select only `perm`, so it no longer loads each view's
`configuration`.
I added a regression assertion in 837a7410437ec125bcae17cdb0ab07a0c00ed7e5
that keeps all of the ownership SELECTs narrow; the focused models and security
tests pass. The separate update hook is unchanged, since it is outside this
deletion-path change.
--
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]
