mirror of
https://github.com/apache/superset.git
synced 2026-07-20 05:36:00 +00:00
1bc20f2206
Lifts `Superset.get_redirect_url` out of `views/core.py` into a module-level
`get_explore_redirect_url() -> str | None` in `views/utils.py`. Both surviving
callers (`ExploreView.root` in `views/explore.py` and the deprecated
`Superset.explore` GET branch in `views/core.py`) call the shared helper and
redirect only when it returns a URL — closing the typed-entry
`/explore/<dst>/<int:dsid>/` GET loop that the previous `isinstance(dict)`
gate missed on cache failure.
Closes:
- nit-2 (real duplication): the `?form_data=` parse-and-redirect logic is now
a single function with one set of guards.
- AF-2 (malformed datasource): `datasource.split("__")` len!=2 and invalid
`DatasourceType(...)` enum both fall through to SPA (HEAD raised 500).
- AF-3 (non-numeric slice_id): `request.args.get("slice_id", type=int)`
returns None on parse failure (HEAD raised `ValueError` from eager `int()`).
- Cache-write loop guard: narrow `try/except ValueError` around
`CreateFormDataCommand.run` falls through to SPA on cache failure.
- `(endpoint, sorted query items)` loop guard: if the would-be redirect
target matches the current request, render SPA instead of 302-looping.
Precedence preserved (round-6 pin): form_data `slice_id` wins over query
`slice_id`; only consults query when form_data omits it.
Pinned-callers invariant: `test_get_explore_redirect_url_sanctioned_callers`
greps `superset/` for `get_explore_redirect_url(` and asserts the caller set
is exactly `{superset/views/explore.py, superset/views/core.py}`. A fourth
caller fails CI until the test (and PLAN.md Slice 5 sanction list) update.
CreateFormDataCommand/CommandParameters are imported inside the helper body
(not at module top-level) to break a circular import: `views/utils.py` is
transitively imported by `commands/base.py`'s dependency graph, so a top-
level import loops back through this file before init finishes. Matches the
prior inline `from superset.views.core import Superset` pattern.
M2 follow-up: CodeQL re-scan after merge should cover `views/utils.py` (new
helper site) in addition to the surviving `redirect()` sinks at
`explore.py:47` + `core.py:436`. The mitigation remains the server-derived
`url_for("ExploreView.root")` target (C1).
Tests: 13 new tests under `tests/integration_tests/views/test_explore_redirect.py`
(one is the sanctioned-callers static-source assertion; the other 12 pin
behaviour through `self.client.get`). Existing `test_explore_redirect` and
`test_explore_no_datasource_renders_spa` in `core_tests.py` stay green
(behaviour-equivalent through the lift). Pre-commit (auto-walrus + mypy
+ ruff-format + ruff + pylint + blacklist + license headers) clean.
Local-env validation note: this worktree's docker-light stack lacks a
working `/login/` POST route (`SupersetAuthView.login` only handles GET;
`AuthDBView.login` POST 404s) — `tests/integration_tests/test_app.py::login`
cannot authenticate, which fails any SupersetTestCase that hits a permission-
gated endpoint (including the pre-existing `test_redirect_view.py` baseline).
The static-scan test passes locally; the other 12 behaviour tests are
validated by CI's properly-configured integration stack.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>