fix: ensure metric_macro expands templates (apache#32344)

superset/jinja_context.py

@@ -27,8 +27,7 @@
 import dateutil
 from flask import current_app, g, has_request_context, request
 from flask_babel import gettext as _
-from jinja2 import DebugUndefined, Environment, nodes
-from jinja2.nodes import Call, Node
+from jinja2 import DebugUndefined, Environment
 from jinja2.sandbox import SandboxedEnvironment
 from sqlalchemy.engine.interfaces import Dialect
 from sqlalchemy.sql.expression import bindparam
@@ -602,7 +601,12 @@ def process_template(self, sql: str, **kwargs: Any) -> str:
         kwargs.update(self._context)
 
         context = validate_template_context(self.engine, kwargs)
-        return template.render(context)
+        try:
+            return template.render(context)
+        except RecursionError as ex:
+            raise SupersetTemplateException(
+                "Infinite recursion detected in template"
+            ) from ex
 
 
 class JinjaTemplateProcessor(BaseTemplateProcessor):
@@ -659,11 +663,18 @@ def set_context(self, **kwargs: Any) -> None:
                 "filter_values": partial(safe_proxy, extra_cache.filter_values),
                 "get_filters": partial(safe_proxy, extra_cache.get_filters),
                 "dataset": partial(safe_proxy, dataset_macro_with_context),
-                "metric": partial(safe_proxy, metric_macro),
                 "get_time_filter": partial(safe_proxy, extra_cache.get_time_filter),
             }
         )
 
+        # The `metric` filter needs the full context, in order to expand other filters
+        self._context["metric"] = partial(
+            safe_proxy,
+            metric_macro,
+            self.env,
+            self._context,
+        )
+
 
 class NoOpTemplateProcessor(BaseTemplateProcessor):
     def process_template(self, sql: str, **kwargs: Any) -> str:
@@ -889,27 +900,12 @@ def get_dataset_id_from_context(metric_key: str) -> int:
     raise SupersetTemplateException(exc_message)
 
 
-def has_metric_macro(template_string: str, env: Environment) -> bool:
-    """
-    Checks if a template string contains a metric macro.
-
-        >>> has_metric_macro("{{ metric('my_metric') }}")
-        True
-
-    """
-    ast = env.parse(template_string)
-
-    def visit_node(node: Node) -> bool:
-        return (
-            isinstance(node, Call)
-            and isinstance(node.node, nodes.Name)
-            and node.node.name == "metric"
-        ) or any(visit_node(child) for child in node.iter_child_nodes())
-
-    return visit_node(ast)
-
-
-def metric_macro(metric_key: str, dataset_id: Optional[int] = None) -> str:
+def metric_macro(
+    env: Environment,
+    context: dict[str, Any],
+    metric_key: str,
+    dataset_id: Optional[int] = None,
+) -> str:
     """
     Given a metric key, returns its syntax.
 
@@ -943,18 +939,7 @@ def metric_macro(metric_key: str, dataset_id: Optional[int] = None) -> str:
         )
 
     definition = metrics[metric_key]
-
-    env = SandboxedEnvironment(undefined=DebugUndefined)
-    context = {"metric": partial(safe_proxy, metric_macro)}
-    while has_metric_macro(definition, env):
-        old_definition = definition
-        template = env.from_string(definition)
-        try:
-            definition = template.render(context)
-        except RecursionError as ex:
-            raise SupersetTemplateException("Cyclic metric macro detected") from ex
-
-        if definition == old_definition:
-            break
+    template = env.from_string(definition)
+    definition = template.render(context)
 
     return definition