[fix] Ensuring cache warmup respects request form data

superset/jinja_context.py

@@ -16,7 +16,6 @@
 # under the License.
 """Defines the templating context for SQL Lab"""
 import inspect
-import json
 import re
 from typing import Any, List, Optional, Tuple
 
@@ -49,7 +48,9 @@ def filter_values(column: str, default: Optional[str] = None) -> List[str]:
     :return: returns a list of filter values
     """
 
-    form_data = json.loads(request.form.get("form_data", "{}"))
+    from superset.views.utils import get_form_data
+
+    form_data, _ = get_form_data()
     convert_legacy_filters_into_adhoc(form_data)
     merge_extra_filters(form_data)
 
@@ -168,18 +169,16 @@ def url_param(
         :returns: The URL parameters
         """
 
+        from superset.views.utils import get_form_data
+
         if request.args.get(param):
             return request.args.get(param, default)
-        # Supporting POST as well as get
-        form_data = request.form.get("form_data")
-        if isinstance(form_data, str):
-            form_data = json.loads(form_data)
-            url_params = form_data.get("url_params") or {}
-            result = url_params.get(param, default)
-            if add_to_cache_keys:
-                self.cache_key_wrapper(result)
-            return result
-        return default
+        form_data, _ = get_form_data()
+        url_params = form_data.get("url_params") or {}
+        result = url_params.get(param, default)
+        if add_to_cache_keys:
+            self.cache_key_wrapper(result)
+        return result
 
 
 class BaseTemplateProcessor:  # pylint: disable=too-few-public-methods

superset/views/core.py

@@ -1703,13 +1703,17 @@ def warm_up_cache(self):
                     form_data["extra_filters"] = get_dashboard_extra_filters(
                         slc.id, dashboard_id
                     )
+
                 obj = get_viz(
                     datasource_type=slc.datasource.type,
                     datasource_id=slc.datasource.id,
                     form_data=form_data,
                     force=True,
                 )
+
+                g.form_data = form_data
                 payload = obj.get_payload()
+                delattr(g, "form_data")
                 error = payload["error"]
                 status = payload["status"]
             except Exception as ex:

superset/views/utils.py

@@ -20,7 +20,7 @@
 from urllib import parse
 
 import simplejson as json
-from flask import request
+from flask import g, request
 
 import superset.models.core as models
 from superset import app, db, is_feature_enabled
@@ -99,23 +99,28 @@ def get_viz(
     return viz_obj
 
 
-def get_form_data(slice_id=None, use_slice_data=False):
+def get_form_data(
+    slice_id: Optional[int] = None, use_slice_data: bool = False
+) -> Tuple[Dict[str, Any], Optional[Slice]]:
     form_data = {}
-    post_data = request.form.get("form_data")
+    request_form_data = request.form.get("form_data")
     request_args_data = request.args.get("form_data")
-    # Supporting POST
-    if post_data:
-        form_data.update(json.loads(post_data))
-    # request params can overwrite post body
+    if request_form_data:
+        form_data.update(json.loads(request_form_data))
+    # request params can overwrite the body
     if request_args_data:
         form_data.update(json.loads(request_args_data))
 
+    # Fallback to using the Flask globals (used for cache warmup) if defined.
+    if not form_data and hasattr(g, "form_data"):
+        form_data = getattr(g, "form_data")
+
     url_id = request.args.get("r")
     if url_id:
         saved_url = db.session.query(models.Url).filter_by(id=url_id).first()
         if saved_url:
             url_str = parse.unquote_plus(
-                saved_url.url.split("?")[1][10:], encoding="utf-8", errors=None
+                saved_url.url.split("?")[1][10:], encoding="utf-8"
             )
             url_form_data = json.loads(url_str)
             # allow form_date in request override saved url

tests/jinja_context_tests.py

@@ -15,76 +15,85 @@
 # specific language governing permissions and limitations
 # under the License.
 import json
-from unittest import mock
 
-from superset.jinja_context import filter_values
+import tests.test_app
+from superset import app
+from superset.jinja_context import ExtraCache, filter_values
 from tests.base_tests import SupersetTestCase
 
 
 class Jinja2ContextTests(SupersetTestCase):
     def test_filter_values_default(self) -> None:
-        request = mock.MagicMock()
-        request.form = {}
-
-        with mock.patch("superset.jinja_context.request", request):
+        with app.test_request_context():
             self.assertEquals(filter_values("name", "foo"), ["foo"])
 
     def test_filter_values_no_default(self) -> None:
-        request = mock.MagicMock()
-        request.form = {}
-
-        with mock.patch("superset.jinja_context.request", request):
+        with app.test_request_context():
             self.assertEquals(filter_values("name"), [])
 
     def test_filter_values_adhoc_filters(self) -> None:
-        request = mock.MagicMock()
-
-        request.form = {
-            "form_data": json.dumps(
-                {
-                    "adhoc_filters": [
-                        {
-                            "clause": "WHERE",
-                            "comparator": "foo",
-                            "expressionType": "SIMPLE",
-                            "operator": "in",
-                            "subject": "name",
-                        }
-                    ],
-                }
-            )
-        }
-
-        with mock.patch("superset.jinja_context.request", request):
+        with app.test_request_context(
+            data={
+                "form_data": json.dumps(
+                    {
+                        "adhoc_filters": [
+                            {
+                                "clause": "WHERE",
+                                "comparator": "foo",
+                                "expressionType": "SIMPLE",
+                                "operator": "in",
+                                "subject": "name",
+                            }
+                        ],
+                    }
+                )
+            }
+        ):
             self.assertEquals(filter_values("name"), ["foo"])
 
-        request.form = {
-            "form_data": json.dumps(
-                {
-                    "adhoc_filters": [
-                        {
-                            "clause": "WHERE",
-                            "comparator": ["foo", "bar"],
-                            "expressionType": "SIMPLE",
-                            "operator": "in",
-                            "subject": "name",
-                        }
-                    ],
-                }
-            )
-        }
-
-        with mock.patch("superset.jinja_context.request", request):
+        with app.test_request_context(
+            data={
+                "form_data": json.dumps(
+                    {
+                        "adhoc_filters": [
+                            {
+                                "clause": "WHERE",
+                                "comparator": ["foo", "bar"],
+                                "expressionType": "SIMPLE",
+                                "operator": "in",
+                                "subject": "name",
+                            }
+                        ],
+                    }
+                )
+            }
+        ):
             self.assertEquals(filter_values("name"), ["foo", "bar"])
 
     def test_filter_values_extra_filters(self) -> None:
-        request = mock.MagicMock()
+        with app.test_request_context(
+            data={
+                "form_data": json.dumps(
+                    {"extra_filters": [{"col": "name", "op": "in", "val": "foo"}]}
+                )
+            }
+        ):
+            self.assertEquals(filter_values("name"), ["foo"])
 
-        request.form = {
-            "form_data": json.dumps(
-                {"extra_filters": [{"col": "name", "op": "in", "val": "foo"}]}
-            )
-        }
+    def test_url_param_default(self) -> None:
+        with app.test_request_context():
+            self.assertEquals(ExtraCache().url_param("foo", "bar"), "bar")
 
-        with mock.patch("superset.jinja_context.request", request):
-            self.assertEquals(filter_values("name"), ["foo"])
+    def test_url_param_no_default(self) -> None:
+        with app.test_request_context():
+            self.assertEquals(ExtraCache().url_param("foo"), None)
+
+    def test_url_param_query(self) -> None:
+        with app.test_request_context(query_string={"foo": "bar"}):
+            self.assertEquals(ExtraCache().url_param("foo"), "bar")
+
+    def test_url_param_form_data(self) -> None:
+        with app.test_request_context(
+            query_string={"form_data": json.dumps({"url_params": {"foo": "bar"}})}
+        ):
+            self.assertEquals(ExtraCache().url_param("foo"), "bar")