You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/09/18 18:16:46 UTC
(superset) branch master updated: fix(datasets): return 400, not 500, for a body-less purge POST; point the database-delete refusal at the purge flow (#44264)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new ae5d8d89a38 fix(datasets): return 400, not 500, for a body-less purge POST; point the database-delete refusal at the purge flow (#44264)
ae5d8d89a38 is described below
commit ae5d8d89a3812be752c839da83fd39864fe8aa54
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Fri Sep 18 12:16:38 2026 -0600
fix(datasets): return 400, not 500, for a body-less purge POST; point the database-delete refusal at the purge flow (#44264)
Co-authored-by: Claude Fable 5 <no...@anthropic.com>
---
superset/commands/database/exceptions.py | 10 ++--
superset/translations/messages.pot | 10 ++--
superset/views/base_api.py | 18 ++++++--
tests/integration_tests/datasets/api_tests.py | 53 ++++++++++++++++++++++
tests/unit_tests/databases/commands/delete_test.py | 21 +++++++--
5 files changed, 97 insertions(+), 15 deletions(-)
diff --git a/superset/commands/database/exceptions.py b/superset/commands/database/exceptions.py
index 3002696a42e..0e2347a292b 100644
--- a/superset/commands/database/exceptions.py
+++ b/superset/commands/database/exceptions.py
@@ -170,10 +170,12 @@ class DatabaseDeleteSoftDeletedDatasetsExistFailedError(
# are hidden (soft-deleted) rows even though their dataset list looks empty.
message = _(
"Cannot delete a database whose only remaining datasets are "
- "soft-deleted. Restore them (POST /api/v1/dataset/<uuid>/restore) "
- "and delete them permanently once a purge capability ships, or "
- "remove the underlying rows out-of-band, before deleting the "
- "database."
+ "soft-deleted. Purge each one first — GET "
+ "/api/v1/dataset/<uuid>/purge-impact for its impact token, then "
+ "POST /api/v1/dataset/<uuid>/purge with "
+ '{"confirmed_impact_token": <token>} — before deleting the '
+ "database. Restoring them (POST /api/v1/dataset/<uuid>/restore) "
+ "keeps the datasets, but the database still cannot be deleted."
)
diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot
index c0b02c2529b..20825a1d9f4 100644
--- a/superset/translations/messages.pot
+++ b/superset/translations/messages.pot
@@ -2683,10 +2683,12 @@ msgid "Cannot delete a database that has datasets attached"
msgstr ""
msgid ""
-"Cannot delete a database whose only remaining datasets are soft-deleted. "
-"Restore them (POST /api/v1/dataset/<uuid>/restore) and delete them "
-"permanently once a purge capability ships, or remove the underlying rows "
-"out-of-band, before deleting the database."
+"Cannot delete a database whose only remaining datasets are soft-deleted."
+" Purge each one first — GET /api/v1/dataset/<uuid>/purge-impact for its "
+"impact token, then POST /api/v1/dataset/<uuid>/purge with "
+"{\"confirmed_impact_token\": <token>} — before deleting the database. "
+"Restoring them (POST /api/v1/dataset/<uuid>/restore) keeps the datasets, "
+"but the database still cannot be deleted."
msgstr ""
msgid "Cannot delete system themes"
diff --git a/superset/views/base_api.py b/superset/views/base_api.py
index 28fe984af3b..63c069da39f 100644
--- a/superset/views/base_api.py
+++ b/superset/views/base_api.py
@@ -48,7 +48,7 @@ from superset.schemas import error_payload_content
from superset.sql_lab import Query as SqllabQuery
from superset.superset_typing import FlaskResponse
from superset.utils.core import get_user_id, time_function
-from superset.views.error_handling import handle_api_exception
+from superset.views.error_handling import handle_api_exception, json_error_response
logger = logging.getLogger(__name__)
get_related_schema = {
@@ -90,12 +90,24 @@ class DistincResponseSchema(Schema):
def requires_json(f: Callable[..., Any]) -> Callable[..., Any]:
"""
- Require JSON-like formatted request to the REST API
+ Require JSON-like formatted request to the REST API.
+
+ Returns the structured 400 RESPONSE directly instead of raising:
+ most call sites stack FAB's ``@safe`` outside this decorator, and
+ ``safe`` converts any non-``BadRequest`` exception — including a
+ status-400 ``SupersetErrorException`` — into a generic 500 "Fatal
+ error" before the app-level error handler can render it (sc-120966:
+ every body-less POST to such an endpoint 500'd). Building the
+ response with the same serializer the app handler uses keeps the
+ error envelope byte-identical for the call sites without ``@safe``.
"""
def wraps(self: BaseSupersetModelRestApi, *args: Any, **kwargs: Any) -> Response:
if not request.is_json:
- raise InvalidPayloadFormatError(message="Request is not JSON")
+ ex: InvalidPayloadFormatError = InvalidPayloadFormatError(
+ message="Request is not JSON"
+ )
+ return json_error_response([ex.error], status=ex.status)
return f(self, *args, **kwargs)
return functools.update_wrapper(wraps, f)
diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py
index b73894c4c07..d5fa10902a6 100644
--- a/tests/integration_tests/datasets/api_tests.py
+++ b/tests/integration_tests/datasets/api_tests.py
@@ -32,6 +32,7 @@ from sqlalchemy import text
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm import joinedload
from sqlalchemy.sql import func
+from werkzeug.test import TestResponse
from superset.commands.dataset.exceptions import DatasetCreateFailedError
from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
@@ -3967,3 +3968,55 @@ class TestDatasetApi(SupersetTestCase):
assert rv.status_code == 403
self.items_to_delete = [dash, chart, dataset, dashboard_dataset]
+
+
+class TestRequiresJsonReturns400(SupersetTestCase):
+ """sc-120966: a body-less POST to a @safe + @requires_json endpoint
+ must be the structured 400, not FAB safe's generic 500 "Fatal error".
+
+ ``requires_json`` returns the response directly (built with the same
+ serializer as the app-level SupersetErrorException handler), so the
+ surrounding ``@safe`` never sees an exception to mangle. The dataset
+ purge endpoint is the reported instance; the chart POST pin proves
+ the fix covers every stacked endpoint, not one route.
+ """
+
+ def test_body_less_dataset_purge_is_a_structured_400(self) -> None:
+ self.login(ADMIN_USERNAME)
+ import uuid as uuidlib
+
+ from superset.daos.dataset import DatasetDAO
+
+ table: SqlaTable = SqlaTable(
+ table_name=f"sc120966_{uuidlib.uuid4().hex[:8]}",
+ database=get_main_database(),
+ schema=None,
+ )
+ db.session.add(table)
+ db.session.commit()
+ ds_uuid: str = str(table.uuid)
+ DatasetDAO.soft_delete([table])
+ db.session.commit()
+ try:
+ rv: TestResponse = self.client.post(f"/api/v1/dataset/{ds_uuid}/purge")
+ assert rv.status_code == 400, rv.data
+ body: dict[str, Any] = rv.get_json()
+ assert body["errors"][0]["error_type"] == "INVALID_PAYLOAD_FORMAT_ERROR"
+ assert body["errors"][0]["message"] == "Request is not JSON"
+ finally:
+ from superset.models.helpers import skip_visibility_filter
+
+ with skip_visibility_filter(db.session, SqlaTable):
+ row: SqlaTable | None = (
+ db.session.query(SqlaTable).filter_by(uuid=table.uuid).first()
+ )
+ if row is not None:
+ db.session.delete(row)
+ db.session.commit()
+
+ def test_body_less_chart_post_is_a_structured_400(self) -> None:
+ self.login(ADMIN_USERNAME)
+ rv: TestResponse = self.client.post("/api/v1/chart/")
+ assert rv.status_code == 400, rv.data
+ body: dict[str, Any] = rv.get_json()
+ assert body["errors"][0]["error_type"] == "INVALID_PAYLOAD_FORMAT_ERROR"
diff --git a/tests/unit_tests/databases/commands/delete_test.py b/tests/unit_tests/databases/commands/delete_test.py
index 4636c726b26..6270191f482 100644
--- a/tests/unit_tests/databases/commands/delete_test.py
+++ b/tests/unit_tests/databases/commands/delete_test.py
@@ -36,6 +36,7 @@ def test_delete_database_blocked_by_soft_deleted_dataset(
from superset import db
from superset.commands.database.delete import DeleteDatabaseCommand
from superset.commands.database.exceptions import (
+ DatabaseDeleteDatasetsExistFailedError,
DatabaseDeleteSoftDeletedDatasetsExistFailedError,
)
from superset.connectors.sqla.models import SqlaTable
@@ -45,8 +46,8 @@ def test_delete_database_blocked_by_soft_deleted_dataset(
SqlaTable.metadata.create_all(session.get_bind())
- database = Database(database_name="del_db", sqlalchemy_uri="sqlite://")
- soft_deleted = SqlaTable(
+ database: Database = Database(database_name="del_db", sqlalchemy_uri="sqlite://")
+ soft_deleted: SqlaTable = SqlaTable(
table_name="gone",
database=database,
deleted_at=datetime(2026, 1, 1, 12, 0, 0),
@@ -59,11 +60,23 @@ def test_delete_database_blocked_by_soft_deleted_dataset(
mocker.patch.object(DatabaseDAO, "find_by_id", return_value=database)
mocker.patch.object(ReportScheduleDAO, "find_by_database_id", return_value=[])
- command = DeleteDatabaseCommand(database.id)
+ command: DeleteDatabaseCommand = DeleteDatabaseCommand(database.id)
# The soft-deleted-only branch raises the *specific* subclass so the
# message tells the operator the blockers are hidden rows.
- with pytest.raises(DatabaseDeleteSoftDeletedDatasetsExistFailedError):
+ exc_info: pytest.ExceptionInfo[DatabaseDeleteSoftDeletedDatasetsExistFailedError]
+ with pytest.raises(DatabaseDeleteSoftDeletedDatasetsExistFailedError) as exc_info:
command.validate()
+ assert "database still cannot be deleted" in str(exc_info.value.message)
+
+ # Restore preserves the reference: it changes the refusal, not eligibility.
+ soft_deleted.restore()
+ db.session.flush()
+ live_error: pytest.ExceptionInfo[DatabaseDeleteDatasetsExistFailedError]
+ with pytest.raises(DatabaseDeleteDatasetsExistFailedError) as live_error:
+ command.validate()
+ assert not isinstance(
+ live_error.value, DatabaseDeleteSoftDeletedDatasetsExistFailedError
+ )
def test_delete_database_blocked_by_live_dataset(