This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-report-put-database-500 in repository https://gitbox.apache.org/repos/asf/superset.git
commit e6fa96878ef6b396fc2b606141e919952a9c98d7 Author: Joe Li <[email protected]> AuthorDate: Wed Feb 18 18:05:54 2026 -0800 test(reports): add database validation edge case tests - No-mutation-on-422: verify rejected PUT does not change model state - POST parity guard: confirm POST with Report+database still returns 400 - Bad DB on transition: Report→Alert with nonexistent DB returns 422 - Command-level unit matrix: 12 tests covering all effective type/database combinations (payload absent vs None vs int, model DB present/absent, type transitions in both directions) Co-Authored-By: Claude Opus 4.6 <[email protected]> --- tests/integration_tests/reports/api_tests.py | 95 ++++++++++ tests/unit_tests/commands/report/update_test.py | 234 ++++++++++++++++++++++++ 2 files changed, 329 insertions(+) diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 9a7b5cf7b20..48ccb6f18ad 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -1770,6 +1770,101 @@ class TestReportSchedulesApi(SupersetTestCase): data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"database": "Database is required for alerts"}} + @pytest.mark.usefixtures("create_report_schedules") + def test_update_report_schedule_422_does_not_mutate(self): + """ + ReportSchedule API: Test that a rejected PUT does not mutate the model + """ + self.login(ADMIN_USERNAME) + + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.name == "name2") + .one_or_none() + ) + assert report_schedule.type == ReportScheduleType.ALERT + original_type = report_schedule.type + original_database_id = report_schedule.database_id + assert original_database_id is not None + uri = f"api/v1/report/{report_schedule.id}" + + # Alert→Report without clearing database → 422 + rv = self.put_assert_metric(uri, {"type": ReportScheduleType.REPORT}, "put") + assert rv.status_code == 422 + + # Re-query and verify no mutation + db.session.expire(report_schedule) + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.id == report_schedule.id) + .one_or_none() + ) + assert report_schedule.type == original_type + assert report_schedule.database_id == original_database_id + + @pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", "create_report_schedules" + ) + def test_create_report_schedule_database_not_allowed(self): + """ + ReportSchedule API: Test POST rejects database on Report type at schema level + """ + self.login(ADMIN_USERNAME) + + chart = db.session.query(Slice).first() + example_db = get_example_database() + report_schedule_data = { + "type": ReportScheduleType.REPORT, + "name": "report_with_db", + "description": "should fail", + "crontab": "0 9 * * *", + "creation_method": ReportCreationMethod.ALERTS_REPORTS, + "chart": chart.id, + "database": example_db.id, + } + uri = "api/v1/report/" + rv = self.post_assert_metric(uri, report_schedule_data, "post") + assert rv.status_code == 400 + data = json.loads(rv.data.decode("utf-8")) + assert "database" in data.get("message", {}) + + @pytest.mark.usefixtures("create_report_schedules") + def test_update_report_to_alert_nonexistent_database(self): + """ + ReportSchedule API: Test Report→Alert with nonexistent database returns 422 + """ + self.login(ADMIN_USERNAME) + + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.name == "name4") + .one_or_none() + ) + assert report_schedule.type == ReportScheduleType.ALERT + uri = f"api/v1/report/{report_schedule.id}" + + # First transition to Report (clearing database) + rv = self.put_assert_metric( + uri, + {"type": ReportScheduleType.REPORT, "database": None}, + "put", + ) + assert rv.status_code == 200 + + # Now transition back to Alert with nonexistent database + database_max_id = db.session.query(func.max(Database.id)).scalar() + rv = self.put_assert_metric( + uri, + { + "type": ReportScheduleType.ALERT, + "database": database_max_id + 1, + }, + "put", + ) + assert rv.status_code == 422 + data = json.loads(rv.data.decode("utf-8")) + assert data == {"message": {"database": "Database does not exist"}} + @pytest.mark.usefixtures( "load_birth_names_dashboard_with_slices", "create_report_schedules" ) diff --git a/tests/unit_tests/commands/report/update_test.py b/tests/unit_tests/commands/report/update_test.py new file mode 100644 index 00000000000..c2694fefb84 --- /dev/null +++ b/tests/unit_tests/commands/report/update_test.py @@ -0,0 +1,234 @@ +# 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 UpdateReportScheduleCommand.validate() database invariants.""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +import pytest +from pytest_mock import MockerFixture + +from superset.commands.report.exceptions import ( + ReportScheduleInvalidError, +) +from superset.commands.report.update import UpdateReportScheduleCommand +from superset.reports.models import ReportScheduleType + + +def _make_model( + mocker: MockerFixture, + *, + model_type: str, + database_id: int | None, +) -> MagicMock: + model = mocker.Mock() + model.type = model_type + model.database_id = database_id + model.name = "test_schedule" + model.crontab = "0 9 * * *" + model.last_state = "noop" + model.owners = [] + return model + + +def _setup_mocks(mocker: MockerFixture, model: MagicMock) -> None: + mocker.patch( + "superset.commands.report.update.ReportScheduleDAO.find_by_id", + return_value=model, + ) + mocker.patch( + "superset.commands.report.update.ReportScheduleDAO.validate_update_uniqueness", + return_value=True, + ) + mocker.patch( + "superset.commands.report.update.security_manager.raise_for_ownership", + ) + mocker.patch( + "superset.commands.report.update.DatabaseDAO.find_by_id", + return_value=mocker.Mock(), + ) + mocker.patch.object( + UpdateReportScheduleCommand, + "validate_chart_dashboard", + ) + mocker.patch.object( + UpdateReportScheduleCommand, + "validate_report_frequency", + ) + mocker.patch.object( + UpdateReportScheduleCommand, + "compute_owners", + return_value=[], + ) + + +def _get_validation_messages( + exc_info: pytest.ExceptionInfo[ReportScheduleInvalidError], +) -> dict[str, str]: + """Extract field→first message string from ReportScheduleInvalidError.""" + raw = exc_info.value.normalized_messages() + result = {} + for field, msgs in raw.items(): + if isinstance(msgs, list): + result[field] = str(msgs[0]) + else: + result[field] = str(msgs) + return result + + +# --- Report type: database must NOT be set --- + + +def test_report_with_database_in_payload_rejected(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={"database": 5}) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "not allowed" in messages["database"].lower() + + +def test_report_with_database_none_in_payload_accepted(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={"database": None}) + cmd.validate() # should not raise + + +def test_report_no_database_in_payload_model_has_db_rejected( + mocker: MockerFixture, +) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=5) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={}) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "not allowed" in messages["database"].lower() + + +def test_report_no_database_anywhere_accepted(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={}) + cmd.validate() # should not raise + + +# --- Alert type: database MUST be set --- + + +def test_alert_with_database_in_payload_accepted(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={"database": 5}) + cmd.validate() # should not raise + + +def test_alert_with_database_none_in_payload_rejected(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=5) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={"database": None}) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "required" in messages["database"].lower() + + +def test_alert_no_database_in_payload_model_has_db_accepted( + mocker: MockerFixture, +) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=5) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={}) + cmd.validate() # should not raise + + +def test_alert_no_database_anywhere_rejected(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand(model_id=1, data={}) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "required" in messages["database"].lower() + + +# --- Type transitions --- + + +def test_alert_to_report_without_clearing_db_rejected(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=5) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand( + model_id=1, data={"type": ReportScheduleType.REPORT} + ) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "not allowed" in messages["database"].lower() + + +def test_alert_to_report_with_db_cleared_accepted(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=5) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand( + model_id=1, + data={"type": ReportScheduleType.REPORT, "database": None}, + ) + cmd.validate() # should not raise + + +def test_report_to_alert_without_db_rejected(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand( + model_id=1, data={"type": ReportScheduleType.ALERT} + ) + with pytest.raises(ReportScheduleInvalidError) as exc_info: + cmd.validate() + messages = _get_validation_messages(exc_info) + assert "database" in messages + assert "required" in messages["database"].lower() + + +def test_report_to_alert_with_db_accepted(mocker: MockerFixture) -> None: + model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None) + _setup_mocks(mocker, model) + + cmd = UpdateReportScheduleCommand( + model_id=1, + data={"type": ReportScheduleType.ALERT, "database": 5}, + ) + cmd.validate() # should not raise
