mirror of https://github.com/apache/superset.git
feat: Add a remove filter_flag to jinja filter_values function (#14507)
Implementation issue 13943 Co-authored-by: cccs-jc <cccs-jc@cyber.gc.ca>
This commit is contained in:
parent
88de4109e7
commit
590fe20a45
|
@ -33,6 +33,7 @@ These features are considered **unfinished** and should only be used on developm
|
|||
- REMOVE_SLICE_LEVEL_LABEL_COLORS
|
||||
- SHARE_QUERIES_VIA_KV_STORE
|
||||
- TAGGING_SYSTEM
|
||||
- ENABLE_TEMPLATE_REMOVE_FILTERS
|
||||
|
||||
## In Testing
|
||||
These features are **finished** but currently being tested. They are usable, but may still contain some bugs.
|
||||
|
|
|
@ -334,6 +334,7 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
|
|||
# See `PR 7935 <https://github.com/apache/superset/pull/7935>`_ for more details.
|
||||
"ENABLE_EXPLORE_JSON_CSRF_PROTECTION": False,
|
||||
"ENABLE_TEMPLATE_PROCESSING": False,
|
||||
"ENABLE_TEMPLATE_REMOVE_FILTERS": False,
|
||||
"KV_STORE": False,
|
||||
# When this feature is enabled, nested types in Presto will be
|
||||
# expanded into extra columns and/or arrays. This is experimental,
|
||||
|
|
|
@ -985,6 +985,8 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
|
|||
template_kwargs.update(self.template_params_dict)
|
||||
extra_cache_keys: List[Any] = []
|
||||
template_kwargs["extra_cache_keys"] = extra_cache_keys
|
||||
removed_filters: List[str] = []
|
||||
template_kwargs["removed_filters"] = removed_filters
|
||||
template_processor = self.get_template_processor(**template_kwargs)
|
||||
db_engine_spec = self.db_engine_spec
|
||||
prequeries: List[str] = []
|
||||
|
@ -1168,6 +1170,12 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
|
|||
val = flt.get("val")
|
||||
op = flt["op"].upper()
|
||||
col_obj = columns_by_name.get(col)
|
||||
|
||||
if is_feature_enabled("ENABLE_TEMPLATE_REMOVE_FILTERS"):
|
||||
if col in removed_filters:
|
||||
# Skip generating SQLA filter when the jinja template handles it.
|
||||
continue
|
||||
|
||||
if col_obj:
|
||||
col_spec = db_engine_spec.get_column_spec(col_obj.type)
|
||||
is_list_target = op in (
|
||||
|
|
|
@ -18,12 +18,23 @@
|
|||
import json
|
||||
import re
|
||||
from functools import partial
|
||||
from typing import Any, Callable, cast, Dict, List, Optional, Tuple, TYPE_CHECKING
|
||||
from typing import (
|
||||
Any,
|
||||
Callable,
|
||||
cast,
|
||||
Dict,
|
||||
List,
|
||||
Optional,
|
||||
Tuple,
|
||||
TYPE_CHECKING,
|
||||
Union,
|
||||
)
|
||||
|
||||
from flask import current_app, g, request
|
||||
from flask_babel import gettext as _
|
||||
from jinja2 import DebugUndefined
|
||||
from jinja2.sandbox import SandboxedEnvironment
|
||||
from typing_extensions import TypedDict
|
||||
|
||||
from superset.exceptions import SupersetTemplateException
|
||||
from superset.extensions import feature_flag_manager
|
||||
|
@ -60,56 +71,10 @@ def context_addons() -> Dict[str, Any]:
|
|||
return current_app.config.get("JINJA_CONTEXT_ADDONS", {})
|
||||
|
||||
|
||||
def filter_values(column: str, default: Optional[str] = None) -> List[str]:
|
||||
"""Gets a values for a particular filter as a list
|
||||
|
||||
This is useful if:
|
||||
- you want to use a filter box to filter a query where the name of filter box
|
||||
column doesn't match the one in the select statement
|
||||
- you want to have the ability for filter inside the main query for speed
|
||||
purposes
|
||||
|
||||
Usage example::
|
||||
|
||||
SELECT action, count(*) as times
|
||||
FROM logs
|
||||
WHERE action in ( {{ "'" + "','".join(filter_values('action_type')) + "'" }} )
|
||||
GROUP BY action
|
||||
|
||||
:param column: column/filter name to lookup
|
||||
:param default: default value to return if there's no matching columns
|
||||
:return: returns a list of filter values
|
||||
"""
|
||||
|
||||
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)
|
||||
|
||||
return_val = [
|
||||
comparator
|
||||
for filter in form_data.get("adhoc_filters", [])
|
||||
for comparator in (
|
||||
filter["comparator"]
|
||||
if isinstance(filter["comparator"], list)
|
||||
else [filter["comparator"]]
|
||||
)
|
||||
if (
|
||||
filter.get("expressionType") == "SIMPLE"
|
||||
and filter.get("clause") == "WHERE"
|
||||
and filter.get("subject") == column
|
||||
and filter.get("comparator")
|
||||
)
|
||||
]
|
||||
|
||||
if return_val:
|
||||
return return_val
|
||||
|
||||
if default:
|
||||
return [default]
|
||||
|
||||
return []
|
||||
class Filter(TypedDict):
|
||||
op: str # pylint: disable=C0103
|
||||
col: str
|
||||
val: Union[None, Any, List[Any]]
|
||||
|
||||
|
||||
class ExtraCache:
|
||||
|
@ -129,8 +94,13 @@ class ExtraCache:
|
|||
r").*\}\}"
|
||||
)
|
||||
|
||||
def __init__(self, extra_cache_keys: Optional[List[Any]] = None):
|
||||
def __init__(
|
||||
self,
|
||||
extra_cache_keys: Optional[List[Any]] = None,
|
||||
removed_filters: Optional[List[str]] = None,
|
||||
):
|
||||
self.extra_cache_keys = extra_cache_keys
|
||||
self.removed_filters = removed_filters if removed_filters is not None else []
|
||||
|
||||
def current_user_id(self, add_to_cache_keys: bool = True) -> Optional[int]:
|
||||
"""
|
||||
|
@ -213,6 +183,142 @@ class ExtraCache:
|
|||
self.cache_key_wrapper(result)
|
||||
return result
|
||||
|
||||
def filter_values(
|
||||
self, column: str, default: Optional[str] = None, remove_filter: bool = False
|
||||
) -> List[Any]:
|
||||
"""Gets a values for a particular filter as a list
|
||||
|
||||
This is useful if:
|
||||
- you want to use a filter component to filter a query where the name of
|
||||
filter component column doesn't match the one in the select statement
|
||||
- you want to have the ability for filter inside the main query for speed
|
||||
purposes
|
||||
|
||||
Usage example::
|
||||
|
||||
SELECT action, count(*) as times
|
||||
FROM logs
|
||||
WHERE
|
||||
action in ({{ "'" + "','".join(filter_values('action_type')) + "'" }})
|
||||
GROUP BY action
|
||||
|
||||
:param column: column/filter name to lookup
|
||||
:param default: default value to return if there's no matching columns
|
||||
:param remove_filter: When set to true, mark the filter as processed,
|
||||
removing it from the outer query. Useful when a filter should
|
||||
only apply to the inner query
|
||||
:return: returns a list of filter values
|
||||
"""
|
||||
return_val: List[Any] = []
|
||||
filters = self.get_filters(column, remove_filter)
|
||||
for flt in filters:
|
||||
val = flt.get("val")
|
||||
if isinstance(val, list):
|
||||
return_val.extend(val)
|
||||
elif val:
|
||||
return_val.append(val)
|
||||
|
||||
if (not return_val) and default:
|
||||
# If no values are found, return the default provided.
|
||||
return_val = [default]
|
||||
|
||||
return return_val
|
||||
|
||||
def get_filters(self, column: str, remove_filter: bool = False) -> List[Filter]:
|
||||
"""Get the filters applied to the given column. In addition
|
||||
to returning values like the filter_values function
|
||||
the get_filters function returns the operator specified in the explorer UI.
|
||||
|
||||
This is useful if:
|
||||
- you want to handle more than the IN operator in your SQL clause
|
||||
- you want to handle generating custom SQL conditions for a filter
|
||||
- you want to have the ability for filter inside the main query for speed
|
||||
purposes
|
||||
|
||||
Usage example::
|
||||
|
||||
|
||||
WITH RECURSIVE
|
||||
superiors(employee_id, manager_id, full_name, level, lineage) AS (
|
||||
SELECT
|
||||
employee_id,
|
||||
manager_id,
|
||||
full_name,
|
||||
1 as level,
|
||||
employee_id as lineage
|
||||
FROM
|
||||
employees
|
||||
WHERE
|
||||
1=1
|
||||
{# Render a blank line #}
|
||||
{%- for filter in get_filters('full_name', remove_filter=True) -%}
|
||||
{%- if filter.get('op') == 'IN' -%}
|
||||
AND
|
||||
full_name IN ( {{ "'" + "', '".join(filter.get('val')) + "'" }} )
|
||||
{%- endif -%}
|
||||
{%- if filter.get('op') == 'LIKE' -%}
|
||||
AND
|
||||
full_name LIKE {{ "'" + filter.get('val') + "'" }}
|
||||
{%- endif -%}
|
||||
{%- endfor -%}
|
||||
UNION ALL
|
||||
SELECT
|
||||
e.employee_id,
|
||||
e.manager_id,
|
||||
e.full_name,
|
||||
s.level + 1 as level,
|
||||
s.lineage
|
||||
FROM
|
||||
employees e,
|
||||
superiors s
|
||||
WHERE s.manager_id = e.employee_id
|
||||
)
|
||||
|
||||
|
||||
SELECT
|
||||
employee_id, manager_id, full_name, level, lineage
|
||||
FROM
|
||||
superiors
|
||||
order by lineage, level
|
||||
|
||||
:param column: column/filter name to lookup
|
||||
:param remove_filter: When set to true, mark the filter as processed,
|
||||
removing it from the outer query. Useful when a filter should
|
||||
only apply to the inner query
|
||||
:return: returns a list of filters
|
||||
"""
|
||||
from superset.utils.core import FilterOperator
|
||||
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)
|
||||
|
||||
filters: List[Filter] = []
|
||||
|
||||
for flt in form_data.get("adhoc_filters", []):
|
||||
val: Union[str, List[str]] = flt.get("comparator")
|
||||
op: str = flt["operator"].upper() if "operator" in flt else None
|
||||
# fltOpName: str = flt.get("filterOptionName")
|
||||
if (
|
||||
flt.get("expressionType") == "SIMPLE"
|
||||
and flt.get("clause") == "WHERE"
|
||||
and flt.get("subject") == column
|
||||
and val
|
||||
):
|
||||
if remove_filter:
|
||||
if column not in self.removed_filters:
|
||||
self.removed_filters.append(column)
|
||||
if op in (
|
||||
FilterOperator.IN.value,
|
||||
FilterOperator.NOT_IN.value,
|
||||
) and not isinstance(val, list):
|
||||
val = [val]
|
||||
|
||||
filters.append({"op": op, "col": column, "val": val})
|
||||
|
||||
return filters
|
||||
|
||||
|
||||
def safe_proxy(func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
|
||||
return_value = func(*args, **kwargs)
|
||||
|
@ -280,12 +386,14 @@ class BaseTemplateProcessor: # pylint: disable=too-few-public-methods
|
|||
|
||||
engine: Optional[str] = None
|
||||
|
||||
# pylint: disable=too-many-arguments
|
||||
def __init__(
|
||||
self,
|
||||
database: "Database",
|
||||
query: Optional["Query"] = None,
|
||||
table: Optional["SqlaTable"] = None,
|
||||
extra_cache_keys: Optional[List[Any]] = None,
|
||||
removed_filters: Optional[List[str]] = None,
|
||||
**kwargs: Any,
|
||||
) -> None:
|
||||
self._database = database
|
||||
|
@ -296,6 +404,7 @@ class BaseTemplateProcessor: # pylint: disable=too-few-public-methods
|
|||
elif table:
|
||||
self._schema = table.schema
|
||||
self._extra_cache_keys = extra_cache_keys
|
||||
self._removed_filters = removed_filters
|
||||
self._context: Dict[str, Any] = {}
|
||||
self._env = SandboxedEnvironment(undefined=DebugUndefined)
|
||||
self.set_context(**kwargs)
|
||||
|
@ -321,14 +430,15 @@ class BaseTemplateProcessor: # pylint: disable=too-few-public-methods
|
|||
class JinjaTemplateProcessor(BaseTemplateProcessor):
|
||||
def set_context(self, **kwargs: Any) -> None:
|
||||
super().set_context(**kwargs)
|
||||
extra_cache = ExtraCache(self._extra_cache_keys)
|
||||
extra_cache = ExtraCache(self._extra_cache_keys, self._removed_filters)
|
||||
self._context.update(
|
||||
{
|
||||
"url_param": partial(safe_proxy, extra_cache.url_param),
|
||||
"current_user_id": partial(safe_proxy, extra_cache.current_user_id),
|
||||
"current_username": partial(safe_proxy, extra_cache.current_username),
|
||||
"cache_key_wrapper": partial(safe_proxy, extra_cache.cache_key_wrapper),
|
||||
"filter_values": partial(safe_proxy, filter_values),
|
||||
"filter_values": partial(safe_proxy, extra_cache.filter_values),
|
||||
"get_filters": partial(safe_proxy, extra_cache.get_filters),
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
@ -24,12 +24,7 @@ import pytest
|
|||
import tests.test_app
|
||||
from superset import app
|
||||
from superset.exceptions import SupersetTemplateException
|
||||
from superset.jinja_context import (
|
||||
ExtraCache,
|
||||
filter_values,
|
||||
get_template_processor,
|
||||
safe_proxy,
|
||||
)
|
||||
from superset.jinja_context import ExtraCache, get_template_processor, safe_proxy
|
||||
from superset.utils import core as utils
|
||||
from tests.base_tests import SupersetTestCase
|
||||
|
||||
|
@ -37,11 +32,26 @@ from tests.base_tests import SupersetTestCase
|
|||
class TestJinja2Context(SupersetTestCase):
|
||||
def test_filter_values_default(self) -> None:
|
||||
with app.test_request_context():
|
||||
self.assertEqual(filter_values("name", "foo"), ["foo"])
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name", "foo"), ["foo"])
|
||||
self.assertEqual(cache.removed_filters, list())
|
||||
|
||||
def test_filter_values_remove_not_present(self) -> None:
|
||||
with app.test_request_context():
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name", remove_filter=True), [])
|
||||
self.assertEqual(cache.removed_filters, list())
|
||||
|
||||
def test_get_filters_remove_not_present(self) -> None:
|
||||
with app.test_request_context():
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.get_filters("name", remove_filter=True), [])
|
||||
self.assertEqual(cache.removed_filters, list())
|
||||
|
||||
def test_filter_values_no_default(self) -> None:
|
||||
with app.test_request_context():
|
||||
self.assertEqual(filter_values("name"), [])
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name"), [])
|
||||
|
||||
def test_filter_values_adhoc_filters(self) -> None:
|
||||
with app.test_request_context(
|
||||
|
@ -61,7 +71,8 @@ class TestJinja2Context(SupersetTestCase):
|
|||
)
|
||||
}
|
||||
):
|
||||
self.assertEqual(filter_values("name"), ["foo"])
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name"), ["foo"])
|
||||
|
||||
with app.test_request_context(
|
||||
data={
|
||||
|
@ -80,7 +91,80 @@ class TestJinja2Context(SupersetTestCase):
|
|||
)
|
||||
}
|
||||
):
|
||||
self.assertEqual(filter_values("name"), ["foo", "bar"])
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name"), ["foo", "bar"])
|
||||
|
||||
def test_get_filters_adhoc_filters(self) -> None:
|
||||
with app.test_request_context(
|
||||
data={
|
||||
"form_data": json.dumps(
|
||||
{
|
||||
"adhoc_filters": [
|
||||
{
|
||||
"clause": "WHERE",
|
||||
"comparator": "foo",
|
||||
"expressionType": "SIMPLE",
|
||||
"operator": "in",
|
||||
"subject": "name",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
}
|
||||
):
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(
|
||||
cache.get_filters("name"), [{"op": "IN", "col": "name", "val": ["foo"]}]
|
||||
)
|
||||
self.assertEqual(cache.removed_filters, list())
|
||||
|
||||
with app.test_request_context(
|
||||
data={
|
||||
"form_data": json.dumps(
|
||||
{
|
||||
"adhoc_filters": [
|
||||
{
|
||||
"clause": "WHERE",
|
||||
"comparator": ["foo", "bar"],
|
||||
"expressionType": "SIMPLE",
|
||||
"operator": "in",
|
||||
"subject": "name",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
}
|
||||
):
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(
|
||||
cache.get_filters("name"),
|
||||
[{"op": "IN", "col": "name", "val": ["foo", "bar"]}],
|
||||
)
|
||||
self.assertEqual(cache.removed_filters, list())
|
||||
|
||||
with app.test_request_context(
|
||||
data={
|
||||
"form_data": json.dumps(
|
||||
{
|
||||
"adhoc_filters": [
|
||||
{
|
||||
"clause": "WHERE",
|
||||
"comparator": ["foo", "bar"],
|
||||
"expressionType": "SIMPLE",
|
||||
"operator": "in",
|
||||
"subject": "name",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
}
|
||||
):
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(
|
||||
cache.get_filters("name", remove_filter=True),
|
||||
[{"op": "IN", "col": "name", "val": ["foo", "bar"]}],
|
||||
)
|
||||
self.assertEqual(cache.removed_filters, ["name"])
|
||||
|
||||
def test_filter_values_extra_filters(self) -> None:
|
||||
with app.test_request_context(
|
||||
|
@ -90,25 +174,30 @@ class TestJinja2Context(SupersetTestCase):
|
|||
)
|
||||
}
|
||||
):
|
||||
self.assertEqual(filter_values("name"), ["foo"])
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.filter_values("name"), ["foo"])
|
||||
|
||||
def test_url_param_default(self) -> None:
|
||||
with app.test_request_context():
|
||||
self.assertEqual(ExtraCache().url_param("foo", "bar"), "bar")
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.url_param("foo", "bar"), "bar")
|
||||
|
||||
def test_url_param_no_default(self) -> None:
|
||||
with app.test_request_context():
|
||||
self.assertEqual(ExtraCache().url_param("foo"), None)
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.url_param("foo"), None)
|
||||
|
||||
def test_url_param_query(self) -> None:
|
||||
with app.test_request_context(query_string={"foo": "bar"}):
|
||||
self.assertEqual(ExtraCache().url_param("foo"), "bar")
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.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.assertEqual(ExtraCache().url_param("foo"), "bar")
|
||||
cache = ExtraCache()
|
||||
self.assertEqual(cache.url_param("foo"), "bar")
|
||||
|
||||
def test_safe_proxy_primitive(self) -> None:
|
||||
def func(input: Any) -> Any:
|
||||
|
|
Loading…
Reference in New Issue