sadpandajoe commented on code in PR #43575:
URL: https://github.com/apache/superset/pull/43575#discussion_r3931092106
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,113 @@ def position(self) -> dict[str, Any]:
return {}
@property
- def tabs(self) -> dict[str, Any]:
+ def tabs(self) -> dict[str, Any]: # noqa: C901
+ # Callers index ``all_tabs`` and iterate ``tab_tree``, so every exit
+ # returns that shape. A bare ``{}`` reaches the API as a payload with
+ # neither key, which a caller cannot iterate.
+ no_tabs: dict[str, Any] = {"all_tabs": {}, "tab_tree": []}
+ if not isinstance(self.position, dict):
+ logger.warning("Dashboard %s: layout is not a mapping", self.id)
+ return no_tabs
if self.position == {}:
- return {}
+ return no_tabs
- def get_node(node_id: str) -> dict[str, Any]:
+ def get_node(node_id: str) -> Optional[dict[str, Any]]:
"""
Helper function for getting a node from the position_data
"""
- return self.position[node_id]
+ return self.position.get(node_id)
+
+ def register_tab(node: dict[str, Any]) -> None:
+ """
+ Helper function for titling a TAB node and adding it to all_tabs
+ """
+ meta = node.get("meta")
+ title = meta.get("text") if isinstance(meta, dict) else None
+ if not isinstance(title, str):
+ logger.warning(
+ "Dashboard %s: tab node %s has no title in the layout",
+ self.id,
+ node.get("id"),
+ )
+ title = ""
+ node["title"] = title
+ node_id = node.get("id")
+ if not isinstance(node_id, str):
+ logger.warning(
+ "Dashboard %s: skipping tab node with no usable id in the
layout",
+ self.id,
+ )
+ return
+ node["value"] = node_id
+ all_tabs[node_id] = title
def build_tab_tree(
node: dict[str, Any], children: list[dict[str, Any]]
) -> None:
"""
Function for building the tab tree structure and list of all tabs
"""
-
+ if "type" not in node:
+ logger.warning(
+ "Dashboard %s: skipping untyped layout node %s",
+ self.id,
+ node.get("id"),
+ )
+ return
+
+ # A node whose type is not one of the four below is walked through
+ # without contributing to the tree, exactly as an untabbed layout
+ # element always has been.
+ node_type = node["type"]
+ child_ids = node.get("children", [])
+ if not isinstance(child_ids, list):
+ logger.warning(
+ "Dashboard %s: layout node %s has malformed children",
+ self.id,
+ node.get("id"),
+ )
+ child_ids = []
new_children: list[dict[str, Any]] = []
# new children to overwrite parent's children
- for child_id in node.get("children", []):
- child = get_node(child_id)
- if node["type"] == "TABS":
- # if TABS add create a new list and append children to it
- # new_children.append(child)
- children.append(child)
+ for child_id in child_ids:
+ child = get_node(child_id) if isinstance(child_id, str) else
None
+ if not isinstance(child, dict):
+ logger.warning(
+ "Dashboard %s: skipping layout node %s, missing or
malformed",
+ self.id,
+ child_id,
+ )
+ continue
+ if node_type == "TABS":
+ # Only a node that will register as a tab belongs in the
+ # tree. Anything else -- an untyped node, or a tab whose id
+ # cannot key ``all_tabs`` -- is rejected later and would be
+ # left in the tree with no ``value`` and no ``title``. It
is
+ # still walked, so tabs stored below it are not lost.
+ if child.get("type") == "TAB" and
isinstance(child.get("id"), str):
+ children.append(child)
+ else:
+ logger.warning(
+ "Dashboard %s: keeping layout node %s out of the "
+ "tab tree, it is not a usable tab",
+ self.id,
+ child_id,
+ )
queue.append((child, new_children))
Review Comment:
The defensive walk still has no visited-node guard. A JSON-valid stored
layout with `ROOT_ID.children = ["ROOT_ID"]` re-enqueues the root forever here,
so `/tabs` hangs and an update that supplies a corrective layout also blocks
while `process_tab_diff` reads the stored tabs. Could we detect already-visited
node IDs (or otherwise reject cycles) before enqueueing?
--
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]
