rusackas commented on code in PR #40634:
URL: https://github.com/apache/superset/pull/40634#discussion_r3383919353
##########
superset/charts/schemas.py:
##########
@@ -518,6 +520,13 @@ class
ChartDataRollingOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
window = fields.Integer(
metadata={"description": "Size of the rolling window in days.",
"example": 7},
Review Comment:
Good call — added `"min": 1` and `"max": 10000` to the `window` field
metadata so the OpenAPI spec now reflects the actual `Range(min=1, max=10000)`
validator, matching the treatment `periods` already got. Pushed in 1de6aa1c69.
##########
tests/unit_tests/charts/test_schemas.py:
##########
@@ -91,6 +93,66 @@ def
test_chart_data_prophet_options_schema_time_grain_validation(
assert "time_grain" in exc_info.value.messages
+def test_chart_put_schema_query_context_json_validation(
+ app_context: None,
+) -> None:
+ """ChartPutSchema.query_context must reject invalid JSON (parity with
POST)."""
+ schema = ChartPutSchema()
+
+ # Valid JSON passes
+ assert schema.load({"query_context": '{"a": 1}'})["query_context"] ==
'{"a": 1}'
+
+ # None is allowed (allow_none)
+ assert schema.load({"query_context": None})["query_context"] is None
+
+ # Invalid JSON is rejected
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({"query_context": "{not valid json"})
+ assert "query_context" in exc_info.value.messages
+
+
+def test_chart_data_prophet_options_schema_periods_range(
+ app_context: None,
+) -> None:
+ """`periods` must be a bounded non-negative integer."""
+ schema = ChartDataProphetOptionsSchema()
+ base = {"time_grain": "P1D", "confidence_interval": 0.8}
+
+ # Valid value passes
+ assert schema.load({**base, "periods": 7})["periods"] == 7
+
+ # Negative value rejected
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "periods": -1})
+ assert "periods" in exc_info.value.messages
+
+ # Excessively large value rejected (resource-exhaustion guard)
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "periods": 1_000_000})
+ assert "periods" in exc_info.value.messages
Review Comment:
Added inclusive-boundary assertions for `periods=0` (min) and
`periods=10000` (max) to
`test_chart_data_prophet_options_schema_periods_range`, so an accidental
`min_inclusive=False` or off-by-one on the bound would now fail the test.
Pushed in 1de6aa1c69.
##########
tests/unit_tests/charts/test_schemas.py:
##########
@@ -91,6 +93,66 @@ def
test_chart_data_prophet_options_schema_time_grain_validation(
assert "time_grain" in exc_info.value.messages
+def test_chart_put_schema_query_context_json_validation(
+ app_context: None,
+) -> None:
+ """ChartPutSchema.query_context must reject invalid JSON (parity with
POST)."""
+ schema = ChartPutSchema()
+
+ # Valid JSON passes
+ assert schema.load({"query_context": '{"a": 1}'})["query_context"] ==
'{"a": 1}'
+
+ # None is allowed (allow_none)
+ assert schema.load({"query_context": None})["query_context"] is None
+
+ # Invalid JSON is rejected
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({"query_context": "{not valid json"})
+ assert "query_context" in exc_info.value.messages
+
+
+def test_chart_data_prophet_options_schema_periods_range(
+ app_context: None,
+) -> None:
+ """`periods` must be a bounded non-negative integer."""
+ schema = ChartDataProphetOptionsSchema()
+ base = {"time_grain": "P1D", "confidence_interval": 0.8}
+
+ # Valid value passes
+ assert schema.load({**base, "periods": 7})["periods"] == 7
+
+ # Negative value rejected
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "periods": -1})
+ assert "periods" in exc_info.value.messages
+
+ # Excessively large value rejected (resource-exhaustion guard)
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "periods": 1_000_000})
+ assert "periods" in exc_info.value.messages
+
+
+def test_chart_data_rolling_options_schema_window_range(
+ app_context: None,
+) -> None:
+ """`window` must be a bounded positive integer."""
+ schema = ChartDataRollingOptionsSchema()
+ base = {"rolling_type": "mean"}
+
+ # Valid value passes
+ assert schema.load({**base, "window": 7})["window"] == 7
+
+ # Zero window rejected (rolling requires window > 0)
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "window": 0})
+ assert "window" in exc_info.value.messages
+
+ # Excessively large window rejected
+ with pytest.raises(ValidationError) as exc_info:
+ schema.load({**base, "window": 1_000_000})
+ assert "window" in exc_info.value.messages
Review Comment:
Added inclusive-boundary assertions for `window=1` (min) and `window=10000`
(max) to `test_chart_data_rolling_options_schema_window_range`, closing the
same inclusiveness gap. Pushed in 1de6aa1c69.
--
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]
