You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2020/07/13 14:21:24 UTC
[incubator-superset] branch master updated: fix(chart-data-api):
case insensitive evaluation of filter op (#10299)
This is an automated email from the ASF dual-hosted git repository.
villebro 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 b316f72 fix(chart-data-api): case insensitive evaluation of filter op (#10299)
b316f72 is described below
commit b316f723a17f12052d075a5e68d7f0c87afa42b3
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Mon Jul 13 17:21:02 2020 +0300
fix(chart-data-api): case insensitive evaluation of filter op (#10299)
* fix(chart-data-api): case insensitive evaluation of filter op
* fix(chart-data-api): case insensitive evaluation of filter op
* mypy
* remove print statement
* add test
---
superset/charts/schemas.py | 21 ++++++------------
superset/utils/schema.py | 54 ++++++++++++++++++++++++++++++++++++++++++++++
tests/charts/api_tests.py | 14 ++++++++++++
tests/utils_tests.py | 21 ++++++++++++++++++
4 files changed, 96 insertions(+), 14 deletions(-)
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index 891dde7..9e5e663 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -14,15 +14,15 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Any, Dict, Union
+from typing import Any, Dict
from flask_babel import gettext as _
-from marshmallow import fields, post_load, Schema, validate, ValidationError
+from marshmallow import fields, post_load, Schema, validate
from marshmallow.validate import Length, Range
from superset.common.query_context import QueryContext
-from superset.exceptions import SupersetException
-from superset.utils import core as utils
+from superset.utils import schema as utils
+from superset.utils.core import FilterOperator
#
# RISON/JSON schemas for query parameters
@@ -101,13 +101,6 @@ openapi_spec_methods_override = {
}
-def validate_json(value: Union[bytes, bytearray, str]) -> None:
- try:
- utils.validate_json(value)
- except SupersetException:
- raise ValidationError("JSON not valid")
-
-
class ChartPostSchema(Schema):
"""
Schema to add a new chart.
@@ -124,7 +117,7 @@ class ChartPostSchema(Schema):
)
owners = fields.List(fields.Integer(description=owners_description))
params = fields.String(
- description=params_description, allow_none=True, validate=validate_json
+ description=params_description, allow_none=True, validate=utils.validate_json
)
cache_timeout = fields.Integer(
description=cache_timeout_description, allow_none=True
@@ -573,8 +566,8 @@ class ChartDataFilterSchema(Schema):
)
op = fields.String( # pylint: disable=invalid-name
description="The comparison operator.",
- validate=validate.OneOf(
- choices=[filter_op.value for filter_op in utils.FilterOperator]
+ validate=utils.OneOfCaseInsensitive(
+ choices=[filter_op.value for filter_op in FilterOperator]
),
required=True,
example="IN",
diff --git a/superset/utils/schema.py b/superset/utils/schema.py
new file mode 100644
index 0000000..2384e28
--- /dev/null
+++ b/superset/utils/schema.py
@@ -0,0 +1,54 @@
+# 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 typing import Any, Union
+
+from marshmallow import validate, ValidationError
+
+from superset.exceptions import SupersetException
+from superset.utils import core as utils
+
+
+class OneOfCaseInsensitive(validate.OneOf):
+ """
+ Marshmallow validator that's based on the built-in `OneOf`, but performs
+ validation case insensitively.
+ """
+
+ def __call__(self, value: Any) -> str:
+ try:
+ if (value.lower() if isinstance(value, str) else value) not in [
+ choice.lower() if isinstance(choice, str) else choice
+ for choice in self.choices
+ ]:
+ raise ValidationError(self._format_error(value))
+ except TypeError as error:
+ raise ValidationError(self._format_error(value)) from error
+
+ return value
+
+
+def validate_json(value: Union[bytes, bytearray, str]) -> None:
+ """
+ JSON Validator that can be passed to a Marshmallow `Field`'s validate argument.
+
+ :raises ValidationError: if value is not serializable to JSON
+ :param value: an object that should be parseable to JSON
+ """
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py
index c3e3b50..78461cf 100644
--- a/tests/charts/api_tests.py
+++ b/tests/charts/api_tests.py
@@ -708,6 +708,20 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
result = response_payload["result"][0]
self.assertEqual(result["rowcount"], 5)
+ def test_chart_data_mixed_case_filter_op(self):
+ """
+ Chart data API: Ensure mixed case filter operator generates valid result
+ """
+ self.login(username="admin")
+ table = self.get_table_by_name("birth_names")
+ request_payload = get_query_context(table.name, table.id, table.type)
+ request_payload["queries"][0]["filters"][0]["op"] = "In"
+ request_payload["queries"][0]["row_limit"] = 10
+ rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
+ response_payload = json.loads(rv.data.decode("utf-8"))
+ result = response_payload["result"][0]
+ self.assertEqual(result["rowcount"], 10)
+
def test_chart_data_with_invalid_datasource(self):
"""Chart data API: Test chart data query with invalid schema
"""
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index 25ad02f..4c2092a 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -27,6 +27,7 @@ from unittest.mock import Mock, patch
import numpy
from flask import Flask, g
from flask_caching import Cache
+import marshmallow
from sqlalchemy.exc import ArgumentError
import tests.test_app
@@ -60,6 +61,7 @@ from superset.utils.core import (
zlib_compress,
zlib_decompress,
)
+from superset.utils import schema
from superset.views.utils import (
build_extra_filters,
get_form_data,
@@ -582,6 +584,8 @@ class TestUtils(SupersetTestCase):
self.assertEqual(jsonObj.process_result_value(val, "dialect"), obj)
def test_validate_json(self):
+ valid = '{"a": 5, "b": [1, 5, ["g", "h"]]}'
+ self.assertIsNone(validate_json(valid))
invalid = '{"a": 5, "b": [1, 5, ["g", "h]]}'
with self.assertRaises(SupersetException):
validate_json(invalid)
@@ -1344,3 +1348,20 @@ class TestUtils(SupersetTestCase):
json.loads(record.json)["form_data"]["viz_type"],
slc.viz.form_data["viz_type"],
)
+
+ def test_schema_validate_json(self):
+ valid = '{"a": 5, "b": [1, 5, ["g", "h"]]}'
+ self.assertIsNone(schema.validate_json(valid))
+ invalid = '{"a": 5, "b": [1, 5, ["g", "h]]}'
+ self.assertRaises(marshmallow.ValidationError, schema.validate_json, invalid)
+
+ def test_schema_one_of_case_insensitive(self):
+ validator = schema.OneOfCaseInsensitive(choices=[1, 2, 3, "FoO", "BAR", "baz"])
+ self.assertEqual(1, validator(1))
+ self.assertEqual(2, validator(2))
+ self.assertEqual("FoO", validator("FoO"))
+ self.assertEqual("FOO", validator("FOO"))
+ self.assertEqual("bar", validator("bar"))
+ self.assertEqual("BaZ", validator("BaZ"))
+ self.assertRaises(marshmallow.ValidationError, validator, "qwerty")
+ self.assertRaises(marshmallow.ValidationError, validator, 4)