You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2026/05/29 16:26:15 UTC
(superset) 08/23: refactor(activity-view): extract resolve_endpoint_path_entity helper
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch sc-107283-versioning-activity-view
in repository https://gitbox.apache.org/repos/asf/superset.git
commit a412f1d9e6ab93c49a859adcd163d4bff36f14ce
Author: Mike Bridge <mi...@ext.preset.io>
AuthorDate: Tue May 26 15:46:20 2026 -0600
refactor(activity-view): extract resolve_endpoint_path_entity helper
Make the change easy, then make the easy change. Phase 4 (T028 chart
/activity/ endpoint, T033 dataset endpoint) duplicates the UUID-parse
+ find_active_by_uuid + raise_for_ownership dance from the existing
DashboardRestApi.activity. Extracting now keeps the next two endpoints
single-line affairs.
* New PathEntityResponseError carries a pre-built error Response.
Subclassing Exception (not ValueError) so the endpoint's catch is
precise and the standard hierarchy stays unpolluted.
* New resolve_endpoint_path_entity(api, model_cls, uuid_str) runs the
three-step preflight and raises PathEntityResponseError with the
right 400/404/403 response carried inside.
* DashboardRestApi.activity refactored to use the helper. 12 lines of
imports + try/except boilerplate collapse to a single try/except
block that maps every preflight failure to its response.
No behaviour change: 10/10 dashboard activity integration tests still
green. Pure structural change, per Beck's tidy-first discipline (the
T028 endpoint code follows in the next commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <no...@anthropic.com>
---
superset/dashboards/api.py | 22 +++++------------
superset/versioning/activity.py | 55 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index db051b5ee08..576a799fcca 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -2500,32 +2500,22 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi):
$ref: '#/components/responses/404'
"""
# pylint: disable=import-outside-toplevel
- from uuid import UUID
-
- from superset.daos.version import VersionDAO
- from superset.exceptions import SupersetSecurityException
from superset.versioning import activity as activity_module
from superset.versioning.schemas import ActivityResponseSchema
try:
- entity_uuid = UUID(uuid_str)
- except ValueError:
- return self.response_400(message="Invalid UUID")
+ entity = activity_module.resolve_endpoint_path_entity(
+ self, Dashboard, uuid_str
+ )
+ except activity_module.PathEntityResponseError as exc:
+ return exc.response
try:
params = activity_module.parse_activity_query_params(request.args)
except activity_module.ActivityParamsError as exc:
return self.response_400(message=str(exc))
- entity = VersionDAO.find_active_by_uuid(Dashboard, entity_uuid)
- if entity is None:
- return self.response_404()
- try:
- security_manager.raise_for_ownership(entity)
- except SupersetSecurityException:
- return self.response_403()
-
- records, count = activity_module.get_activity(Dashboard, entity_uuid, **params)
+ records, count = activity_module.get_activity(Dashboard, entity.uuid, **params)
payload = ActivityResponseSchema().dump({"result": records, "count": count})
return self.response(200, **payload)
diff --git a/superset/versioning/activity.py b/superset/versioning/activity.py
index 07719b21089..73b319924d7 100644
--- a/superset/versioning/activity.py
+++ b/superset/versioning/activity.py
@@ -881,6 +881,61 @@ class ActivityParamsError(ValueError):
no other callers should depend on the exception type."""
+class PathEntityResponseError(Exception):
+ """Carries a pre-built error ``Response`` from
+ :func:`resolve_endpoint_path_entity`. The endpoint catches this and
+ returns the carried response directly. The shape exists so the
+ UUID-parse + find-by-uuid + ownership-check dance can live in one
+ place across the three activity-view endpoint families."""
+
+ def __init__(self, response: Any) -> None:
+ super().__init__("PathEntityResponseError")
+ self.response = response
+
+
+def resolve_endpoint_path_entity(api: Any, model_cls: type, uuid_str: str) -> Any:
+ """Run the standard path-entity preflight for an activity endpoint:
+
+ 1. Parse *uuid_str* into a UUID (or raise → 400).
+ 2. Look up the live entity via ``VersionDAO.find_active_by_uuid``
+ (or raise → 404).
+ 3. Run ``security_manager.raise_for_ownership`` (or raise → 403).
+
+ Returns the live entity on success. Raises
+ :class:`PathEntityResponseError` carrying the appropriate error
+ Response on any failure; the endpoint method should::
+
+ try:
+ entity = resolve_endpoint_path_entity(self, Dashboard, uuid_str)
+ except PathEntityResponseError as exc:
+ return exc.response
+
+ *api* is the FAB ``ModelRestApi`` instance — we call
+ ``api.response_400`` / ``api.response_403`` / ``api.response_404``
+ on it. Pass ``self`` from the endpoint method.
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset import security_manager
+ from superset.daos.version import VersionDAO
+ from superset.exceptions import SupersetSecurityException
+
+ try:
+ entity_uuid = UUID(uuid_str)
+ except ValueError as exc:
+ raise PathEntityResponseError(api.response_400(message="Invalid UUID")) from exc
+
+ entity = VersionDAO.find_active_by_uuid(model_cls, entity_uuid)
+ if entity is None:
+ raise PathEntityResponseError(api.response_404())
+
+ try:
+ security_manager.raise_for_ownership(entity)
+ except SupersetSecurityException as exc:
+ raise PathEntityResponseError(api.response_403()) from exc
+
+ return entity
+
+
def parse_activity_query_params(args: Any) -> dict[str, Any]:
"""Parse the ``since`` / ``until`` / ``include`` / ``page`` / ``page_size``
query parameters into the kwargs ``get_activity`` accepts.