codeant-ai-for-open-source[bot] commented on code in PR #36076:
URL: https://github.com/apache/superset/pull/36076#discussion_r3524936425
##########
superset/utils/schema.py:
##########
@@ -83,3 +84,35 @@ def validate_external_url(value: Optional[str]) -> None:
)
if not parsed.netloc:
raise ValidationError("URL must be absolute and include a host.")
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str, None])
-> None:
+ """
+ Validator for query_context field to ensure it contains required metadata.
+
+ Validates that the query_context JSON contains the required 'datasource'
and
+ 'queries' fields needed for chart data retrieval.
+
+ :raises ValidationError: if value is not valid JSON or missing required
fields
+ :param value: a JSON string that should contain datasource and queries
metadata
+ """
+ if value is None or value == "":
+ return # Allow None values and empty strings
+
+ # Reuse existing JSON validation logic
+ validate_json(value)
+
+ # Parse and validate the structure
+ parsed_data = json.loads(value)
+
+ # Validate required fields exist in the query_context
+ if not isinstance(parsed_data, dict):
+ error_msg = "Query context must be a valid JSON object"
+ raise ValidationError(error_msg)
+
+ # When query_context is provided (not None), validate it has required
fields
+ required_fields = {"datasource", "queries"}
Review Comment:
**Suggestion:** Add a concrete collection type annotation for this
required-fields variable to satisfy the type-hint requirement on newly
introduced relevant variables. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly introduced variable is a concrete collection that can be
annotated, but no type hint is provided. That is a real violation of the
type-hint rule for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=65b2b20d468d49249010697450e17f6a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=65b2b20d468d49249010697450e17f6a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/schema.py
**Line:** 114:114
**Comment:**
*Custom Rule: Add a concrete collection type annotation for this
required-fields variable to satisfy the type-hint requirement on newly
introduced relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=ffe44dd44a975180388f0351a0a1328a43449b54fe4575cca0d1ea0877d7de46&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=ffe44dd44a975180388f0351a0a1328a43449b54fe4575cca0d1ea0877d7de46&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_schema.py:
##########
@@ -0,0 +1,327 @@
+# 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.
+
+"""Unit tests for schema validation utilities."""
+
+import pytest
+from marshmallow import ValidationError
+
+from superset.utils import json
+from superset.utils.schema import (
+ OneOfCaseInsensitive,
+ validate_json,
+ validate_query_context_metadata,
+)
+
+
+def test_validate_json_valid() -> None:
+ """Test validate_json with valid JSON string."""
+ valid_json = '{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_valid_bytes() -> None:
+ """Test validate_json with valid JSON bytes."""
+ valid_json = b'{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_invalid() -> None:
+ """Test validate_json with invalid JSON."""
+ invalid_json = '{"key": "value", "number": 123'
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(invalid_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_empty_string() -> None:
+ """Test validate_json with empty string - empty strings are allowed."""
+ # Empty strings do not raise an error because validate_json has early
return
+ validate_json("") # Should not raise any exception
+
+
+def test_validate_json_whitespace_only() -> None:
+ """Test validate_json with whitespace-only string."""
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(" ")
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_not_json() -> None:
+ """Test validate_json with non-JSON string."""
+ not_json = "this is not json"
Review Comment:
**Suggestion:** Add a concrete type annotation for this string variable to
satisfy the rule requiring typed relevant variables. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The variable is a newly added local string in Python code and could be
annotated; leaving it untyped matches the type-hint rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2bc1f7172fd045c9af0bc415b24e7047&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2bc1f7172fd045c9af0bc415b24e7047&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_schema.py
**Line:** 68:68
**Comment:**
*Custom Rule: Add a concrete type annotation for this string variable
to satisfy the rule requiring typed relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=709a2caedd9a5514783d87cfafe7b1ccb07e29d6b8ba38c298b547b91a328e02&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=709a2caedd9a5514783d87cfafe7b1ccb07e29d6b8ba38c298b547b91a328e02&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_schema.py:
##########
@@ -0,0 +1,327 @@
+# 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.
+
+"""Unit tests for schema validation utilities."""
+
+import pytest
+from marshmallow import ValidationError
+
+from superset.utils import json
+from superset.utils.schema import (
+ OneOfCaseInsensitive,
+ validate_json,
+ validate_query_context_metadata,
+)
+
+
+def test_validate_json_valid() -> None:
+ """Test validate_json with valid JSON string."""
+ valid_json = '{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_valid_bytes() -> None:
+ """Test validate_json with valid JSON bytes."""
+ valid_json = b'{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_invalid() -> None:
+ """Test validate_json with invalid JSON."""
+ invalid_json = '{"key": "value", "number": 123'
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(invalid_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_empty_string() -> None:
+ """Test validate_json with empty string - empty strings are allowed."""
+ # Empty strings do not raise an error because validate_json has early
return
+ validate_json("") # Should not raise any exception
+
+
+def test_validate_json_whitespace_only() -> None:
+ """Test validate_json with whitespace-only string."""
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(" ")
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_not_json() -> None:
+ """Test validate_json with non-JSON string."""
+ not_json = "this is not json"
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(not_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_none() -> None:
+ """Test validate_query_context_metadata allows None values."""
+ # Should not raise any exception for None
+ validate_query_context_metadata(None)
+
+
+def test_validate_query_context_metadata_valid() -> None:
+ """Test validate_query_context_metadata with valid query context."""
+ valid_query_context = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": [{"metrics": ["count"], "columns": []}],
+ },
+ )
+ # Should not raise any exception
+ validate_query_context_metadata(valid_query_context)
+
+
+def test_validate_query_context_metadata_valid_bytes() -> None:
+ """Test validate_query_context_metadata with valid query context as
bytes."""
+ valid_query_context = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": [{"metrics": ["count"], "columns": []}],
+ },
+ ).encode("utf-8")
+ # Should not raise any exception
+ validate_query_context_metadata(valid_query_context)
+
+
+def test_validate_query_context_metadata_invalid_json() -> None:
+ """Test validate_query_context_metadata with invalid JSON."""
+ invalid_json = '{"datasource": {"type": "table"'
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(invalid_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_not_dict() -> None:
+ """Test validate_query_context_metadata with non-dict JSON."""
+ not_dict = json.dumps(["array", "values"])
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(not_dict)
+ assert "Query context must be a valid JSON object" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_missing_datasource() -> None:
+ """Test validate_query_context_metadata with missing datasource field."""
+ missing_datasource = json.dumps(
+ {"queries": [{"metrics": ["count"], "columns": []}]},
+ )
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(missing_datasource)
+ error_message = str(exc_info.value)
+ assert "Query context is missing required fields" in error_message
+ assert "datasource" in error_message
+
+
+def test_validate_query_context_metadata_missing_queries() -> None:
+ """Test validate_query_context_metadata with missing queries field."""
+ missing_queries = json.dumps({"datasource": {"type": "table", "id": 1}})
Review Comment:
**Suggestion:** Add an explicit type annotation for this serialized
query-context variable. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This local serialized JSON variable is untyped even though it can be
annotated, so it falls under the rule requiring type hints for relevant
variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=263afcc5e91642d7824a79338ae278be&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=263afcc5e91642d7824a79338ae278be&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_schema.py
**Line:** 134:134
**Comment:**
*Custom Rule: Add an explicit type annotation for this serialized
query-context variable.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=56e815c36f3f6ce391b1aaa6320fe77413873bade7010e755070a87bdc567b18&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=56e815c36f3f6ce391b1aaa6320fe77413873bade7010e755070a87bdc567b18&reaction=dislike'>👎</a>
##########
superset/utils/schema.py:
##########
@@ -83,3 +84,35 @@ def validate_external_url(value: Optional[str]) -> None:
)
if not parsed.netloc:
raise ValidationError("URL must be absolute and include a host.")
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str, None])
-> None:
+ """
+ Validator for query_context field to ensure it contains required metadata.
+
+ Validates that the query_context JSON contains the required 'datasource'
and
+ 'queries' fields needed for chart data retrieval.
+
+ :raises ValidationError: if value is not valid JSON or missing required
fields
+ :param value: a JSON string that should contain datasource and queries
metadata
+ """
+ if value is None or value == "":
+ return # Allow None values and empty strings
+
+ # Reuse existing JSON validation logic
+ validate_json(value)
+
+ # Parse and validate the structure
+ parsed_data = json.loads(value)
Review Comment:
**Suggestion:** Add an explicit type annotation for this parsed JSON
variable so the new validation logic keeps full type-hint coverage.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is newly added Python code and the local variable can be annotated. It
currently lacks an explicit type hint, so it matches the rule requiring type
hints on relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7cbc66c812ab42f79288ae81d13b0db9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7cbc66c812ab42f79288ae81d13b0db9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/schema.py
**Line:** 106:106
**Comment:**
*Custom Rule: Add an explicit type annotation for this parsed JSON
variable so the new validation logic keeps full type-hint coverage.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=24224cd224b4fa9c0acf4f1fa484a7bdcf98599db8b6265b871b91d0aac16bfa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=24224cd224b4fa9c0acf4f1fa484a7bdcf98599db8b6265b871b91d0aac16bfa&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_schema.py:
##########
@@ -0,0 +1,327 @@
+# 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.
+
+"""Unit tests for schema validation utilities."""
+
+import pytest
+from marshmallow import ValidationError
+
+from superset.utils import json
+from superset.utils.schema import (
+ OneOfCaseInsensitive,
+ validate_json,
+ validate_query_context_metadata,
+)
+
+
+def test_validate_json_valid() -> None:
+ """Test validate_json with valid JSON string."""
+ valid_json = '{"key": "value", "number": 123}'
Review Comment:
**Suggestion:** Add an explicit type annotation for this local test input
variable to comply with the type-hint requirement. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is new Python code in a test file and the local variable is untyped
even though it can be annotated; this matches the type-hint requirement for
relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=346511e55e624fc69dfcbf92b5bb8c0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=346511e55e624fc69dfcbf92b5bb8c0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_schema.py
**Line:** 33:33
**Comment:**
*Custom Rule: Add an explicit type annotation for this local test input
variable to comply with the type-hint requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=a742ec4711ef2f733d893ce6dfd4f1c7fb8a3df75650eca54a275b34c3baba20&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=a742ec4711ef2f733d893ce6dfd4f1c7fb8a3df75650eca54a275b34c3baba20&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_schema.py:
##########
@@ -0,0 +1,327 @@
+# 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.
+
+"""Unit tests for schema validation utilities."""
+
+import pytest
+from marshmallow import ValidationError
+
+from superset.utils import json
+from superset.utils.schema import (
+ OneOfCaseInsensitive,
+ validate_json,
+ validate_query_context_metadata,
+)
+
+
+def test_validate_json_valid() -> None:
+ """Test validate_json with valid JSON string."""
+ valid_json = '{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_valid_bytes() -> None:
+ """Test validate_json with valid JSON bytes."""
+ valid_json = b'{"key": "value", "number": 123}'
Review Comment:
**Suggestion:** Add an explicit bytes type annotation for this variable
instead of leaving it untyped. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added local variable is a bytes value and is left without a type
annotation, which violates the rule requiring type hints for annotatable Python
variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2341b51a384f4aa28231e236d4b97938&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2341b51a384f4aa28231e236d4b97938&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_schema.py
**Line:** 40:40
**Comment:**
*Custom Rule: Add an explicit bytes type annotation for this variable
instead of leaving it untyped.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=055a348dec50ab15fca988e6f9d555e272b73762d42439c4f7de5142faba41c2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=055a348dec50ab15fca988e6f9d555e272b73762d42439c4f7de5142faba41c2&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_schema.py:
##########
@@ -0,0 +1,327 @@
+# 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.
+
+"""Unit tests for schema validation utilities."""
+
+import pytest
+from marshmallow import ValidationError
+
+from superset.utils import json
+from superset.utils.schema import (
+ OneOfCaseInsensitive,
+ validate_json,
+ validate_query_context_metadata,
+)
+
+
+def test_validate_json_valid() -> None:
+ """Test validate_json with valid JSON string."""
+ valid_json = '{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_valid_bytes() -> None:
+ """Test validate_json with valid JSON bytes."""
+ valid_json = b'{"key": "value", "number": 123}'
+ # Should not raise any exception
+ validate_json(valid_json)
+
+
+def test_validate_json_invalid() -> None:
+ """Test validate_json with invalid JSON."""
+ invalid_json = '{"key": "value", "number": 123'
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(invalid_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_empty_string() -> None:
+ """Test validate_json with empty string - empty strings are allowed."""
+ # Empty strings do not raise an error because validate_json has early
return
+ validate_json("") # Should not raise any exception
+
+
+def test_validate_json_whitespace_only() -> None:
+ """Test validate_json with whitespace-only string."""
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(" ")
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_json_not_json() -> None:
+ """Test validate_json with non-JSON string."""
+ not_json = "this is not json"
+ with pytest.raises(ValidationError) as exc_info:
+ validate_json(not_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_none() -> None:
+ """Test validate_query_context_metadata allows None values."""
+ # Should not raise any exception for None
+ validate_query_context_metadata(None)
+
+
+def test_validate_query_context_metadata_valid() -> None:
+ """Test validate_query_context_metadata with valid query context."""
+ valid_query_context = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": [{"metrics": ["count"], "columns": []}],
+ },
+ )
+ # Should not raise any exception
+ validate_query_context_metadata(valid_query_context)
+
+
+def test_validate_query_context_metadata_valid_bytes() -> None:
+ """Test validate_query_context_metadata with valid query context as
bytes."""
+ valid_query_context = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": [{"metrics": ["count"], "columns": []}],
+ },
+ ).encode("utf-8")
+ # Should not raise any exception
+ validate_query_context_metadata(valid_query_context)
+
+
+def test_validate_query_context_metadata_invalid_json() -> None:
+ """Test validate_query_context_metadata with invalid JSON."""
+ invalid_json = '{"datasource": {"type": "table"'
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(invalid_json)
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_not_dict() -> None:
+ """Test validate_query_context_metadata with non-dict JSON."""
+ not_dict = json.dumps(["array", "values"])
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(not_dict)
+ assert "Query context must be a valid JSON object" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_missing_datasource() -> None:
+ """Test validate_query_context_metadata with missing datasource field."""
+ missing_datasource = json.dumps(
+ {"queries": [{"metrics": ["count"], "columns": []}]},
+ )
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(missing_datasource)
+ error_message = str(exc_info.value)
+ assert "Query context is missing required fields" in error_message
+ assert "datasource" in error_message
+
+
+def test_validate_query_context_metadata_missing_queries() -> None:
+ """Test validate_query_context_metadata with missing queries field."""
+ missing_queries = json.dumps({"datasource": {"type": "table", "id": 1}})
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(missing_queries)
+ error_message = str(exc_info.value)
+ assert "Query context is missing required fields" in error_message
+ assert "queries" in error_message
+
+
+def test_validate_query_context_metadata_missing_both_fields() -> None:
+ """Test validate_query_context_metadata with both required fields
missing."""
+ empty_context = json.dumps({})
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(empty_context)
+ error_message = str(exc_info.value)
+ assert "Query context is missing required fields" in error_message
+ assert "datasource" in error_message
+ assert "queries" in error_message
+
+
+def test_validate_query_context_metadata_extra_fields() -> None:
+ """Test validate_query_context_metadata allows extra fields."""
+ context_with_extras = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": [{"metrics": ["count"], "columns": []}],
+ "extra_field": "extra_value",
+ "another_field": 123,
+ },
+ )
+ # Should not raise any exception - extra fields are allowed
+ validate_query_context_metadata(context_with_extras)
+
+
+def test_validate_query_context_metadata_empty_values() -> None:
+ """Test validate_query_context_metadata with empty but present values."""
+ context_with_empty = json.dumps(
+ {
+ "datasource": {},
+ "queries": [],
+ },
+ )
+ # Should not raise any exception - fields exist even if empty
+ validate_query_context_metadata(context_with_empty)
+
+
+def test_validate_query_context_metadata_null_datasource() -> None:
+ """Test validate_query_context_metadata with null datasource value."""
+ context_with_null = json.dumps(
+ {
+ "datasource": None,
+ "queries": [{"metrics": ["count"]}],
+ },
+ )
+ # Should not raise any exception - field exists even if null
+ validate_query_context_metadata(context_with_null)
+
+
+def test_validate_query_context_metadata_null_queries() -> None:
+ """Test validate_query_context_metadata with null queries value."""
+ context_with_null = json.dumps(
+ {
+ "datasource": {"type": "table", "id": 1},
+ "queries": None,
+ },
+ )
+ # Should not raise any exception - field exists even if null
+ validate_query_context_metadata(context_with_null)
+
+
+def test_validate_query_context_metadata_empty_string() -> None:
+ """Test validate_query_context_metadata with empty string."""
+ # Empty string should be treated as None and not raise error
+ validate_query_context_metadata("")
+
+
+def test_validate_query_context_metadata_whitespace() -> None:
+ """Test validate_query_context_metadata with whitespace-only string."""
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(" ")
+ assert "JSON not valid" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_string_value() -> None:
+ """Test validate_query_context_metadata with plain string instead of JSON
object."""
+ plain_string = json.dumps("just a string")
+ with pytest.raises(ValidationError) as exc_info:
+ validate_query_context_metadata(plain_string)
+ assert "Query context must be a valid JSON object" in str(exc_info.value)
+
+
+def test_validate_query_context_metadata_number_value() -> None:
+ """Test validate_query_context_metadata with number instead of JSON
object."""
+ number_value = json.dumps(12345)
Review Comment:
**Suggestion:** Add an explicit type annotation for this variable holding
serialized numeric JSON data. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is another newly added untyped local variable in Python test code, and
it can be annotated directly, so the type-hint rule applies.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1e13419cee4444d5a4d93f7f383b062a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1e13419cee4444d5a4d93f7f383b062a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_schema.py
**Line:** 226:226
**Comment:**
*Custom Rule: Add an explicit type annotation for this variable holding
serialized numeric JSON data.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=f2be1117b15c2791fe91da9d60424163ba34d89877642a8eaca56e3052da05d8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36076&comment_hash=f2be1117b15c2791fe91da9d60424163ba34d89877642a8eaca56e3052da05d8&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
