This is an automated email from the ASF dual-hosted git repository.
johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 7f89f12 [debug] Debugging caching issue (#9665)
7f89f12 is described below
commit 7f89f12c4a80f95282c0c5b4807b20dbd688de43
Author: John Bodley <[email protected]>
AuthorDate: Wed Apr 29 12:16:47 2020 -0700
[debug] Debugging caching issue (#9665)
Co-authored-by: John Bodley <[email protected]>
---
superset/jinja_context.py | 23 ++++-----
superset/views/core.py | 4 ++
superset/views/utils.py | 21 +++++---
tests/jinja_context_tests.py | 117 +++++++++++++++++++++++--------------------
tests/macro_tests.py | 77 ----------------------------
tests/utils_tests.py | 85 +++++++++++++++++++++++++++++--
6 files changed, 173 insertions(+), 154 deletions(-)
diff --git a/superset/jinja_context.py b/superset/jinja_context.py
index 5438f0f..28eb6e2 100644
--- a/superset/jinja_context.py
+++ b/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 @@ class ExtraCache:
: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
diff --git a/superset/views/core.py b/superset/views/core.py
index e6c9ac9..9688b89 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1707,13 +1707,17 @@ class Superset(BaseSupersetView):
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:
diff --git a/superset/views/utils.py b/superset/views/utils.py
index edb2987..5ed7e48 100644
--- a/superset/views/utils.py
+++ b/superset/views/utils.py
@@ -20,7 +20,7 @@ from typing import Any, Dict, List, Optional, Tuple
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
diff --git a/tests/jinja_context_tests.py b/tests/jinja_context_tests.py
index 91cb5e4..431d3b6 100644
--- a/tests/jinja_context_tests.py
+++ b/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")
diff --git a/tests/macro_tests.py b/tests/macro_tests.py
deleted file mode 100644
index 1b9b66e..0000000
--- a/tests/macro_tests.py
+++ /dev/null
@@ -1,77 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License. You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied. See the License for the
-# specific language governing permissions and limitations
-# under the License.
-from flask import json
-
-from superset import app, jinja_context
-from tests.base_tests import SupersetTestCase
-
-
-class MacroTestCase(SupersetTestCase):
- def test_filter_values_macro(self):
- form_data1 = {
- "extra_filters": [{"col": "my_special_filter", "op": "in", "val":
["foo"]}],
- "filters": [{"col": "my_special_filter2", "op": "in", "val":
["bar"]}],
- }
-
- form_data2 = {
- "extra_filters": [
- {"col": "my_special_filter", "op": "in", "val": ["foo", "bar"]}
- ]
- }
-
- form_data3 = {
- "extra_filters": [
- {"col": "my_special_filter", "op": "in", "val": ["foo", "bar"]}
- ],
- "filters": [{"col": "my_special_filter", "op": "in", "val":
["savage"]}],
- }
-
- form_data4 = {
- "extra_filters": [{"col": "my_special_filter", "op": "in", "val":
"foo"}],
- "filters": [{"col": "my_special_filter", "op": "in", "val":
"savage"}],
- }
-
- data1 = {"form_data": json.dumps(form_data1)}
- data2 = {"form_data": json.dumps(form_data2)}
- data3 = {"form_data": json.dumps(form_data3)}
- data4 = {"form_data": json.dumps(form_data4)}
-
- with app.test_request_context(data=data1):
- filter_values = jinja_context.filter_values("my_special_filter")
- self.assertEqual(filter_values, ["foo"])
-
- filter_values = jinja_context.filter_values("my_special_filter2")
- self.assertEqual(filter_values, ["bar"])
-
- filter_values = jinja_context.filter_values("")
- self.assertEqual(filter_values, [])
-
- with app.test_request_context(data=data2):
- filter_values = jinja_context.filter_values("my_special_filter")
- self.assertEqual(filter_values, ["foo", "bar"])
-
- with app.test_request_context(data=data3):
- filter_values = jinja_context.filter_values("my_special_filter")
- self.assertEqual(filter_values, ["savage", "foo", "bar"])
-
- with app.test_request_context():
- filter_values = jinja_context.filter_values("nonexistent_filter",
"foo")
- self.assertEqual(filter_values, ["foo"])
-
- with app.test_request_context(data=data4):
- filter_values = jinja_context.filter_values("my_special_filter")
- self.assertEqual(filter_values, ["savage", "foo"])
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index 70b77af..9f3d0f9 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -20,11 +20,12 @@ import uuid
from datetime import date, datetime, time, timedelta
from decimal import Decimal
import hashlib
+import json
import os
from unittest.mock import Mock, patch
import numpy
-from flask import Flask
+from flask import Flask, g
from flask_caching import Cache
from sqlalchemy.exc import ArgumentError
@@ -60,8 +61,11 @@ from superset.utils.core import (
zlib_compress,
zlib_decompress,
)
-from superset.views.utils import get_time_range_endpoints
-from superset.views.utils import build_extra_filters
+from superset.views.utils import (
+ build_extra_filters,
+ get_form_data,
+ get_time_range_endpoints,
+)
from tests.base_tests import SupersetTestCase
from .fixtures.certificates import ssl_certificate
@@ -1249,3 +1253,78 @@ class UtilsTestCase(SupersetTestCase):
get_email_address_list(",a@a; b@b c@c a-c@c; d@d, f@f"),
["a@a", "b@b", "c@c", "a-c@c", "d@d", "f@f"],
)
+
+ def test_get_form_data_default(self) -> None:
+ with app.test_request_context():
+ form_data, slc = get_form_data()
+
+ self.assertEqual(
+ form_data,
+ {"time_range_endpoints":
get_time_range_endpoints(form_data={}),},
+ )
+
+ self.assertEqual(slc, None)
+
+ def test_get_form_data_request_args(self) -> None:
+ with app.test_request_context(
+ query_string={"form_data": json.dumps({"foo": "bar"})}
+ ):
+ form_data, slc = get_form_data()
+
+ self.assertEqual(
+ form_data,
+ {
+ "foo": "bar",
+ "time_range_endpoints":
get_time_range_endpoints(form_data={}),
+ },
+ )
+
+ self.assertEqual(slc, None)
+
+ def test_get_form_data_request_form(self) -> None:
+ with app.test_request_context(data={"form_data": json.dumps({"foo":
"bar"})}):
+ form_data, slc = get_form_data()
+
+ self.assertEqual(
+ form_data,
+ {
+ "foo": "bar",
+ "time_range_endpoints":
get_time_range_endpoints(form_data={}),
+ },
+ )
+
+ self.assertEqual(slc, None)
+
+ def test_get_form_data_request_args_and_form(self) -> None:
+ with app.test_request_context(
+ data={"form_data": json.dumps({"foo": "bar"})},
+ query_string={"form_data": json.dumps({"baz": "bar"})},
+ ):
+ form_data, slc = get_form_data()
+
+ self.assertEqual(
+ form_data,
+ {
+ "baz": "bar",
+ "foo": "bar",
+ "time_range_endpoints":
get_time_range_endpoints(form_data={}),
+ },
+ )
+
+ self.assertEqual(slc, None)
+
+ def test_get_form_data_globals(self) -> None:
+ with app.test_request_context():
+ g.form_data = {"foo": "bar"}
+ form_data, slc = get_form_data()
+ delattr(g, "form_data")
+
+ self.assertEqual(
+ form_data,
+ {
+ "foo": "bar",
+ "time_range_endpoints":
get_time_range_endpoints(form_data={}),
+ },
+ )
+
+ self.assertEqual(slc, None)
