This is an automated email from the ASF dual-hosted git repository.
asoare pushed a commit to branch alexandrusoare/fix/dashboard-imports-fail
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to
refs/heads/alexandrusoare/fix/dashboard-imports-fail by this push:
new ea4b4e890b4 small changes
ea4b4e890b4 is described below
commit ea4b4e890b4402d13aeaa52f27a3f19c6b486e39
Author: alexandrusoare <[email protected]>
AuthorDate: Fri Jan 30 16:43:18 2026 +0200
small changes
---
superset/explorables/base.py | 12 +++++++++
superset/security/manager.py | 3 +--
tests/unit_tests/security/manager_test.py | 42 -------------------------------
3 files changed, 13 insertions(+), 44 deletions(-)
diff --git a/superset/explorables/base.py b/superset/explorables/base.py
index 2d534b72099..b611e25eb4b 100644
--- a/superset/explorables/base.py
+++ b/superset/explorables/base.py
@@ -106,6 +106,18 @@ class Explorable(Protocol):
# Identity & Metadata
# =========================================================================
+ @property
+ def id(self) -> int:
+ """
+ Primary key identifier for this explorable.
+
+ Used for database lookups such as row-level security filter resolution.
+ Must be accessible without triggering expensive operations like
+ database engine connections.
+
+ :return: Integer primary key
+ """
+
@property
def uid(self) -> str:
"""
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 9ac48277567..ab297b0f3d6 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -2749,9 +2749,8 @@ class SupersetSecurityManager( # pylint:
disable=too-many-public-methods
)
.filter(RLSFilterRoles.c.role_id.in_(user_roles))
)
- table_id = table.id if hasattr(table, "id") else table.data["id"]
filter_tables =
self.session.query(RLSFilterTables.c.rls_filter_id).filter(
- RLSFilterTables.c.table_id == table_id
+ RLSFilterTables.c.table_id == table.id
)
query = (
self.session.query(
diff --git a/tests/unit_tests/security/manager_test.py
b/tests/unit_tests/security/manager_test.py
index 2b4ff191c78..db34a8edb98 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -1186,45 +1186,3 @@ def test_get_catalogs_accessible_by_user_schema_access(
catalogs = {"catalog1", "catalog2"}
assert sm.get_catalogs_accessible_by_user(database, catalogs) ==
{"catalog2"}
-
-
-def test_get_rls_filters_uses_table_id_not_data(
- mocker: MockerFixture,
- app_context: None,
-) -> None:
- """
- Test that get_rls_filters() uses table.id directly instead of
table.data["id"].
-
- Accessing table.data triggers the full data property chain including
select_star,
- which requires a live database engine connection. When the DB is
unreachable, this
- causes the entire dashboard GET endpoint to fail with a 500 error.
-
- Regression introduced in #36245 (Explorable protocol) which changed
table.id to
- table.data["id"]. This test ensures we use the direct .id attribute for
- BaseDatasource objects.
- """
- sm = SupersetSecurityManager(appbuilder)
-
- table = mocker.MagicMock()
- table.id = 42
- # Make .data raise an exception to prove it is never accessed
- type(table).data = mocker.PropertyMock(
- side_effect=Exception("should not access table.data")
- )
-
- mock_user = mocker.MagicMock()
- mock_user.roles = [mocker.MagicMock(id=1)]
- mocker.patch("superset.security.manager.g", user=mock_user)
- mocker.patch.object(sm, "get_user_roles", return_value=mock_user.roles)
-
- # The method issues DB queries via self.session; mock them to return empty
results
- mock_query = mocker.MagicMock()
- mock_query.join.return_value = mock_query
- mock_query.filter.return_value = mock_query
- mock_query.all.return_value = []
- mocker.patch.object(sm, "session", mocker.MagicMock())
- sm.session.query.return_value = mock_query
-
- result = sm.get_rls_filters(table)
- assert result == []
- # Verify .data was never accessed (the PropertyMock would have raised)
