rusackas commented on code in PR #44549:
URL: https://github.com/apache/superset/pull/44549#discussion_r4271674023
##########
superset/utils/schema.py:
##########
@@ -56,6 +57,63 @@ def validate_json(value: Union[bytes, bytearray, str]) ->
None:
raise ValidationError("JSON not valid") from ex
+def is_query_context_metadata_complete(value: Any) -> bool:
+ """
+ Whether a parsed ``query_context`` payload carries the ``datasource`` and
+ ``queries`` keys ``QueryContextFactory.create()`` requires (apache/superset
+ #35774): both are required keyword-only arguments there, and a chart saved
+ without them fails every subsequent read with a raw ``TypeError`` instead
+ of a clear error at save time. ``datasource`` must be a mapping carrying
+ the ``id``/``type`` keys ``DatasourceDict`` requires --
``_convert_to_model``
+ indexes both directly and raises ``KeyError`` if either is absent, and
+ passes ``type`` straight to ``DatasourceType(...)``, which raises
+ ``ValueError`` for a value outside that enum -- but ``queries`` may
+ legitimately be an empty list (a chart with no query objects yet still
+ round-trips through ``create()`` without error), so it is only required
+ to be a list of mappings, not to be non-empty.
+
+ :param value: the object obtained by JSON-decoding a query_context string
+ """
+ if not isinstance(value, dict) or not isinstance(value.get("queries"),
list):
+ return False
+ if not all(isinstance(query, dict) for query in value["queries"]):
+ return False
+ datasource = value.get("datasource")
+ if not isinstance(datasource, dict) or not datasource.get("id"):
+ return False
+ datasource_type = datasource.get("type")
+ if not isinstance(datasource_type, str):
+ return False
+ try:
+ DatasourceType(datasource_type)
+ except ValueError:
+ return False
+ return True
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str]) ->
None:
+ """
+ Validator for a chart's ``query_context`` field: beyond being well-formed
+ JSON (see ``validate_json``), it must be a JSON object carrying a
+ non-empty ``datasource`` key and a ``queries`` key that is a list
+ (apache/superset#35774).
+
+ :raises ValidationError: if value is not valid JSON, or is valid JSON that
+ is not an object with a non-empty ``datasource`` key and a ``queries``
+ key holding a list
+ :param value: an object that should be parseable to a query_context JSON
object
+ """
+ try:
+ parsed = json.loads(value)
+ except json.JSONDecodeError as ex:
+ raise ValidationError("JSON not valid") from ex
Review Comment:
Good catch, applied: empty string is skipped again, with a test.
##########
superset/utils/schema.py:
##########
@@ -56,6 +57,63 @@ def validate_json(value: Union[bytes, bytearray, str]) ->
None:
raise ValidationError("JSON not valid") from ex
+def is_query_context_metadata_complete(value: Any) -> bool:
+ """
+ Whether a parsed ``query_context`` payload carries the ``datasource`` and
+ ``queries`` keys ``QueryContextFactory.create()`` requires (apache/superset
+ #35774): both are required keyword-only arguments there, and a chart saved
+ without them fails every subsequent read with a raw ``TypeError`` instead
+ of a clear error at save time. ``datasource`` must be a mapping carrying
+ the ``id``/``type`` keys ``DatasourceDict`` requires --
``_convert_to_model``
+ indexes both directly and raises ``KeyError`` if either is absent, and
+ passes ``type`` straight to ``DatasourceType(...)``, which raises
+ ``ValueError`` for a value outside that enum -- but ``queries`` may
+ legitimately be an empty list (a chart with no query objects yet still
+ round-trips through ``create()`` without error), so it is only required
+ to be a list of mappings, not to be non-empty.
+
+ :param value: the object obtained by JSON-decoding a query_context string
+ """
+ if not isinstance(value, dict) or not isinstance(value.get("queries"),
list):
+ return False
+ if not all(isinstance(query, dict) for query in value["queries"]):
+ return False
+ datasource = value.get("datasource")
+ if not isinstance(datasource, dict) or not datasource.get("id"):
+ return False
+ datasource_type = datasource.get("type")
+ if not isinstance(datasource_type, str):
+ return False
+ try:
+ DatasourceType(datasource_type)
+ except ValueError:
+ return False
+ return True
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str]) ->
None:
+ """
+ Validator for a chart's ``query_context`` field: beyond being well-formed
+ JSON (see ``validate_json``), it must be a JSON object carrying a
+ non-empty ``datasource`` key and a ``queries`` key that is a list
+ (apache/superset#35774).
+
+ :raises ValidationError: if value is not valid JSON, or is valid JSON that
+ is not an object with a non-empty ``datasource`` key and a ``queries``
+ key holding a list
+ :param value: an object that should be parseable to a query_context JSON
object
+ """
+ try:
+ parsed = json.loads(value)
+ except json.JSONDecodeError as ex:
+ raise ValidationError("JSON not valid") from ex
+ if not is_query_context_metadata_complete(parsed):
+ raise ValidationError(
+ "query_context must be a JSON object with a non-empty "
+ "'datasource' key and a 'queries' key holding a list"
+ )
Review Comment:
Applied, the error now names the `id`/`type` and list-of-objects
requirements.
--
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]
