This is an automated email from the ASF dual-hosted git repository.
yongjiezhao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 718bc3062e fix: invalid metric should raise an exception (#20882)
718bc3062e is described below
commit 718bc3062e99cc44afbb57f786b5ca228c5b13fb
Author: Yongjie Zhao <[email protected]>
AuthorDate: Thu Jul 28 08:15:43 2022 +0800
fix: invalid metric should raise an exception (#20882)
---
superset/dashboards/api.py | 8 ++++++--
superset/models/sql_lab.py | 4 +++-
superset/utils/core.py | 10 ++++++----
.../fixtures/deck_geojson_form_data.json | 2 +-
.../integration_tests/fixtures/deck_path_form_data.json | 2 +-
tests/integration_tests/viz_tests.py | 16 +++++-----------
tests/unit_tests/core_tests.py | 12 ++++++++++++
7 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 5fb59a7d1d..460cfcb0ea 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -383,8 +383,12 @@ class DashboardRestApi(BaseSupersetModelRestApi):
self.dashboard_dataset_schema.dump(dataset) for dataset in
datasets
]
return self.response(200, result=result)
- except TypeError:
- return self.response_400(message=gettext("Dataset schema is
invalid."))
+ except (TypeError, ValueError) as err:
+ return self.response_400(
+ message=gettext(
+ "Dataset schema is invalid, caused by: %(error)s",
error=str(err)
+ )
+ )
except DashboardAccessDeniedError:
return self.response_403()
except DashboardNotFoundError:
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index e7f61964e4..4449b3dfa1 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -61,7 +61,9 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
-class Query(Model, ExtraJSONMixin, ExploreMixin): # pylint:
disable=abstract-method
+class Query(
+ Model, ExtraJSONMixin, ExploreMixin
+): # pylint: disable=abstract-method,too-many-public-methods
"""ORM model for SQL query
Now that SQL Lab support multi-statement execution, an entry in this
diff --git a/superset/utils/core.py b/superset/utils/core.py
index ba7237b77b..6ce5a8a831 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -1294,7 +1294,7 @@ def get_metric_name(
sql_expression = metric.get("sqlExpression")
if sql_expression:
return sql_expression
- elif expression_type == "SIMPLE":
+ if expression_type == "SIMPLE":
column: AdhocMetricColumn = metric.get("column") or {}
column_name = column.get("column_name")
aggregate = metric.get("aggregate")
@@ -1302,10 +1302,12 @@ def get_metric_name(
return f"{aggregate}({column_name})"
if column_name:
return column_name
- raise ValueError(__("Invalid metric object"))
- verbose_map = verbose_map or {}
- return verbose_map.get(metric, metric) # type: ignore
+ if isinstance(metric, str):
+ verbose_map = verbose_map or {}
+ return verbose_map.get(metric, metric)
+
+ raise ValueError(__("Invalid metric object: %(metric)s",
metric=str(metric)))
def get_column_names(
diff --git a/tests/integration_tests/fixtures/deck_geojson_form_data.json
b/tests/integration_tests/fixtures/deck_geojson_form_data.json
index 422197a285..e8258c7d44 100644
--- a/tests/integration_tests/fixtures/deck_geojson_form_data.json
+++ b/tests/integration_tests/fixtures/deck_geojson_form_data.json
@@ -43,5 +43,5 @@
"granularity_sqla": null,
"autozoom": true,
"url_params": {},
- "size": 100
+ "size": "100"
}
diff --git a/tests/integration_tests/fixtures/deck_path_form_data.json
b/tests/integration_tests/fixtures/deck_path_form_data.json
index 39cc2007f8..ac2e404d83 100644
--- a/tests/integration_tests/fixtures/deck_path_form_data.json
+++ b/tests/integration_tests/fixtures/deck_path_form_data.json
@@ -45,5 +45,5 @@
"granularity_sqla": null,
"autozoom": true,
"url_params": {},
- "size": 100
+ "size": "100"
}
diff --git a/tests/integration_tests/viz_tests.py
b/tests/integration_tests/viz_tests.py
index 6a8bda3df9..137e2a474c 100644
--- a/tests/integration_tests/viz_tests.py
+++ b/tests/integration_tests/viz_tests.py
@@ -716,7 +716,7 @@ class TestPairedTTest(SupersetTestCase):
self.assertEqual(data, expected)
def test_get_data_empty_null_keys(self):
- form_data = {"groupby": [], "metrics": ["", None]}
+ form_data = {"groupby": [], "metrics": [""]}
datasource = self.get_datasource_mock()
# Test data
raw = {}
@@ -739,19 +739,13 @@ class TestPairedTTest(SupersetTestCase):
"group": "All",
}
],
- "NULL": [
- {
- "values": [
- {"x": 100, "y": 10},
- {"x": 200, "y": 20},
- {"x": 300, "y": 30},
- ],
- "group": "All",
- }
- ],
}
self.assertEqual(data, expected)
+ form_data = {"groupby": [], "metrics": [None]}
+ with self.assertRaises(ValueError):
+ viz.viz_types["paired_ttest"](datasource, form_data)
+
class TestPartitionViz(SupersetTestCase):
@patch("superset.viz.BaseViz.query_obj")
diff --git a/tests/unit_tests/core_tests.py b/tests/unit_tests/core_tests.py
index f7a0047157..bd151011a4 100644
--- a/tests/unit_tests/core_tests.py
+++ b/tests/unit_tests/core_tests.py
@@ -105,6 +105,18 @@ def test_get_metric_name_invalid_metric():
with pytest.raises(ValueError):
get_metric_name(metric)
+ metric = deepcopy(SQL_ADHOC_METRIC)
+ del metric["expressionType"]
+ with pytest.raises(ValueError):
+ get_metric_name(metric)
+
+ with pytest.raises(ValueError):
+ get_metric_name(None)
+ with pytest.raises(ValueError):
+ get_metric_name(0)
+ with pytest.raises(ValueError):
+ get_metric_name({})
+
def test_get_metric_names():
assert get_metric_names(
