Compare commits

..

6 Commits

Author SHA1 Message Date
Joe Li
8b62bf0935 fix(embedded-e2e): strengthen assertion signal-to-noise from review
Address /review-code findings — the previous round's hardening fixed
flake but a few assertions still gave weak signals:

- The chart-rendered selector matched a still-loading chart cell, since
  Superset's `Loading` spinner itself renders an SVG. Exclude the spinner
  via `:not(:has([data-test="loading-indicator"]))` and centralize the
  selector as `EmbeddedPage.RENDERED_CHART_SELECTOR`.
- The "dashboard renders" test only proved iframe/header chrome, not the
  dashboard. Add `waitForChartRendered()` so the test name matches what
  it asserts.
- The `hideTitle` test passed for the wrong reason if the locator
  drifted (`toBeHidden()` succeeds for absent elements). Add an explicit
  `toHaveCount(0)` so the contrast against the baseline visibility check
  in test 1 is load-bearing.
- `tokenCallCount` was a `>=1` check that any rendered dashboard would
  satisfy. Tighten to `=== 1` to actually exercise the SDK's caching
  contract.
- Drop the redundant `appUrl` shadow of `appServer.url`.
- Move `import os` to module top in the docker-light config; document
  the strict `"true"`-only env-var truthiness convention.

Pre-commit clean (type-check, prettier, oxlint, ruff, mypy). Local
re-verification blocked by an unrelated worktree env issue (semantic
layers feature has incomplete state — the docker-compose-light stack
doesn't bind-mount superset-core/, so the image's stale copy lacks the
new submodule); CI on the chromium-embedded project will validate.
Changes are strictly stronger assertions and refactors so they cannot
turn a previously-passing test into a false positive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 16:11:17 -07:00
Joe Li
dc136e8898 feat(docker-light): enable embedded-dashboard support in local stack
Three additions to the lightweight local config so the embedded-dashboard
flow works against docker-compose-light without manually patching state:

- Read SUPERSET_FEATURE_<NAME> env vars into FEATURE_FLAGS so a docker
  .env-local can toggle features without editing tracked config.
- Disable Talisman so /embedded/<uuid> doesn't serve X-Frame-Options:
  SAMEORIGIN, which otherwise blocks cross-origin iframe embedding.
- Mirror Public to Gamma via PUBLIC_ROLE_LIKE so guest tokens can hit
  /api/v1/me/roles/ (CI does this implicitly via load_test_users; the
  light stack does not).

Required for the chromium-embedded Playwright project to run locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 16:11:17 -07:00
Joe Li
fdf8525c5d fix(embedded-e2e): harden Playwright suite against flake
Replaces racy one-shot checks with auto-retrying assertions, asserts the
referrer-block test against the deterministic 403 response (not iframe
content), uses an OS-allocated port for the static test app with
connection-tracked teardown, caches the JWT access token across tests,
sends CSRF on the guest-token call (page.request always carries the
storageState cookie, so JWT-only doesn't actually skip CSRF), and waits
for a real viz element inside chart containers rather than a class that
doesn't exist. Verified with --repeat-each=5 (25/25 passing).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 16:11:17 -07:00
Joe Li
f7b5de25e9 fix(embedded-e2e): use route allowlist in static test server
The test app server only ever serves /, /index.html, and /sdk/index.js,
so replace dynamic path joining with a fixed allowlist. This eliminates
the data flow from req.url to readFileSync that CodeQL flagged as a
path-traversal sink — the previous resolve+startsWith containment check
was correct but not recognized as a sanitizer by the analyzer.
2026-05-11 16:11:17 -07:00
Joe Li
918143fe90 ci(embedded-e2e): build SDK and configure test environment
- Add a build-embedded-sdk step to bashlib.sh and wire it into the
  superset-playwright and superset-e2e workflows so the SDK bundle is
  compiled before Playwright runs.
- Set SUPERSET_FEATURE_EMBEDDED_SUPERSET=true via workflow env so the
  feature flag only affects Playwright jobs. Setting it in the shared
  integration test config breaks unrelated Python tests because the
  security manager's guest-user paths access g.user through paths that
  most tests don't mock.
- Add CORS for localhost:9000 and TALISMAN_ENABLED=False to the
  integration test config. Talisman defaults to X-Frame-Options:
  SAMEORIGIN, which blocks the embedded dashboard from rendering
  inside an iframe hosted on a different port.
2026-05-11 16:11:17 -07:00
Joe Li
335a08a81b feat(embedded-e2e): add Playwright E2E tests for embedded dashboards
Adds five tests covering the embedded dashboard flow against the
world_health example: render, hideTitle UI config, chart rendering,
allowed_domains referrer check, and guest-token data access. Includes:

- A chromium-embedded Playwright project, excluded from the main
  project via testIgnore so it can be opted into separately.
- An EmbeddedPage page object and API helpers for embedding/guest
  tokens plus dashboard lookup by slug.
- A static test app (embedded-app/index.html) loaded from a minimal
  Node static server. Playwright bridges the guest-token fetch from
  Node into the browser via page.exposeFunction.
- EMBEDDED timeout/config constants.

Workflow integration and test-environment configuration land in a
follow-up commit.
2026-05-11 16:11:17 -07:00
46 changed files with 1031 additions and 657 deletions

View File

@@ -59,6 +59,15 @@ build-assets() {
say "::endgroup::"
}
build-embedded-sdk() {
cd "$GITHUB_WORKSPACE/superset-embedded-sdk"
say "::group::Build embedded SDK bundle for E2E tests"
npm ci
npm run build
say "::endgroup::"
}
build-instrumented-assets() {
cd "$GITHUB_WORKSPACE/superset-frontend"

View File

@@ -17,12 +17,13 @@ jobs:
steps:
- name: Check if user is allowed
id: check
env:
COMMENTER: ${{ github.event.comment.user.login }}
run: |
# List of allowed users
ALLOWED_USERS="mistercrunch,rusackas"
# Get the commenter's username
COMMENTER="${{ github.event.comment.user.login }}"
echo "Checking permissions for user: $COMMENTER"
# Check if user is in allowed list
@@ -44,12 +45,9 @@ jobs:
steps:
- name: Comment access denied
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
COMMENTER_LOGIN: ${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}
with:
script: |
const commenter = process.env.COMMENTER_LOGIN;
const message = `👋 Hi @${commenter}!
const message = `👋 Hi @${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}!
Thanks for trying to use Claude Code, but currently only certain team members have access to this feature.

View File

@@ -41,7 +41,7 @@ jobs:
# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
uses: github/codeql-action/init@v4
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -53,6 +53,6 @@ jobs:
- name: Perform CodeQL Analysis
if: steps.check.outputs.python || steps.check.outputs.frontend
uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
uses: github/codeql-action/analyze@v4
with:
category: "/language:${{matrix.language}}"

View File

@@ -169,6 +169,7 @@ jobs:
PYTHONPATH: ${{ github.workspace }}
REDIS_PORT: 16379
GITHUB_TOKEN: ${{ github.token }}
SUPERSET_FEATURE_EMBEDDED_SUPERSET: "true"
services:
postgres:
image: postgres:17-alpine
@@ -239,6 +240,11 @@ jobs:
uses: ./.github/actions/cached-dependencies
with:
run: build-instrumented-assets
- name: Build embedded SDK
if: steps.check.outputs.python || steps.check.outputs.frontend
uses: ./.github/actions/cached-dependencies
with:
run: build-embedded-sdk
- name: Install Playwright
if: steps.check.outputs.python || steps.check.outputs.frontend
uses: ./.github/actions/cached-dependencies

View File

@@ -43,6 +43,7 @@ jobs:
PYTHONPATH: ${{ github.workspace }}
REDIS_PORT: 16379
GITHUB_TOKEN: ${{ github.token }}
SUPERSET_FEATURE_EMBEDDED_SUPERSET: "true"
services:
postgres:
image: postgres:17-alpine
@@ -113,6 +114,11 @@ jobs:
uses: ./.github/actions/cached-dependencies
with:
run: build-instrumented-assets
- name: Build embedded SDK
if: steps.check.outputs.python || steps.check.outputs.frontend
uses: ./.github/actions/cached-dependencies
with:
run: build-embedded-sdk
- name: Install Playwright
if: steps.check.outputs.python || steps.check.outputs.frontend
uses: ./.github/actions/cached-dependencies

View File

@@ -18,6 +18,8 @@
# Configuration for docker-compose-light.yml - disables Redis and uses minimal services
# Import all settings from the main config first
import os
from flask_caching.backends.filesystemcache import FileSystemCache
from superset_config import * # noqa: F403
@@ -36,3 +38,31 @@ THUMBNAIL_CACHE_CONFIG = CACHE_CONFIG
# Disable Celery entirely for lightweight mode
CELERY_CONFIG = None # type: ignore[assignment,misc]
# Honor SUPERSET_FEATURE_<NAME> env vars on top of any flags inherited from
# superset_config. Lets local dev/e2e enable features (e.g. EMBEDDED_SUPERSET)
# without editing shipped config files. Only the literal string "true"
# (case-insensitive) is treated as enabled — "1"/"yes"/"on" are not, matching
# the strict-string convention used elsewhere in Superset's env parsing.
FEATURE_FLAGS = {
**FEATURE_FLAGS, # noqa: F405
**{
name[len("SUPERSET_FEATURE_") :]: value.strip().lower() == "true"
for name, value in os.environ.items()
if name.startswith("SUPERSET_FEATURE_")
},
}
# Disable Talisman so /embedded/<uuid> doesn't return X-Frame-Options:SAMEORIGIN.
# Without this, browsers refuse to render Superset inside an iframe from a
# different origin (i.e. the embedded SDK use case). Production/CI configures
# Talisman with explicit `frame-ancestors`; for the lightweight local stack we
# just turn it off.
TALISMAN_ENABLED = False
# Guest tokens (used by the embedded SDK) inherit the "Public" role's perms.
# Out of the box Public has zero perms, so embedded dashboards immediately fail
# their first call (`/api/v1/me/roles/`) with 403. Mirror Public to Gamma —
# the standard read-only viewer role — so the embedded flow can authenticate
# and load dashboard data in local dev.
PUBLIC_ROLE_LIKE = "Gamma"

View File

@@ -17,109 +17,59 @@
* under the License.
*/
import { ReactElement, useEffect, useRef, useState } from 'react';
import Loadable from 'react-loadable';
import { ComponentClass } from 'react';
export type LoadableRendererProps = {
onRenderFailure?: (error: Error) => void;
onRenderSuccess?: () => void;
onRenderFailure?: Function;
onRenderSuccess?: Function;
};
type LoaderMap<Exports> = {
[K in keyof Exports]: () => Promise<Exports[K]> | Exports[K];
const defaultProps = {
onRenderFailure() {},
onRenderSuccess() {},
};
export interface LoadingProps {
error?: { toString(): string };
}
export interface LoadableRenderer<Props>
extends
ComponentClass<Props & LoadableRendererProps>,
Loadable.LoadableComponent {}
export interface LoadableOptions<Props, Exports> {
loader: LoaderMap<Exports>;
loading: (loadingProps: LoadingProps) => ReactElement | null;
render: (loaded: Exports, props: Props) => ReactElement;
}
export default function createLoadableRenderer<
Props,
Exports extends { [key: string]: any },
>(options: Loadable.OptionsWithMap<Props, Exports>): LoadableRenderer<Props> {
const LoadableRenderer = Loadable.Map<Props, Exports>(
options,
) as LoadableRenderer<Props>;
export interface LoadableRenderer<Props> {
(props: Props & LoadableRendererProps): ReactElement | null;
preload: () => Promise<unknown>;
displayName?: string;
}
// Extends the behavior of LoadableComponent to provide post-render listeners
class CustomLoadableRenderer extends LoadableRenderer {
static defaultProps: object;
export default function createLoadableRenderer<Props, Exports>(
options: LoadableOptions<Props, Exports>,
): LoadableRenderer<Props> {
let promise: Promise<Exports> | null = null;
let cachedResult: Exports | null = null;
let cachedError: Error | null = null;
componentDidMount() {
this.afterRender();
}
const load = (): Promise<Exports> => {
if (promise) return promise;
const keys = Object.keys(options.loader) as (keyof Exports)[];
promise = Promise.all(
keys.map(key => Promise.resolve(options.loader[key]())),
).then(
values => {
const loaded = {} as Exports;
keys.forEach((key, i) => {
loaded[key] = values[i] as Exports[typeof key];
});
cachedResult = loaded;
return loaded;
},
err => {
cachedError = err instanceof Error ? err : new Error(String(err));
throw cachedError;
},
);
return promise;
};
componentDidUpdate() {
this.afterRender();
}
const Renderer: LoadableRenderer<Props> = props => {
const [state, setState] = useState<{
loaded: Exports | null;
error: Error | null;
}>(() => ({ loaded: cachedResult, error: cachedError }));
useEffect(() => {
if (state.loaded || state.error) return undefined;
let cancelled = false;
load().then(
loaded => {
if (!cancelled) setState({ loaded, error: null });
},
err => {
if (!cancelled) setState({ loaded: null, error: err });
},
);
return () => {
cancelled = true;
};
}, [state.loaded, state.error]);
// Keep callback refs current without retriggering the post-load effect on
// every prop update.
const onRenderSuccessRef = useRef(props.onRenderSuccess);
const onRenderFailureRef = useRef(props.onRenderFailure);
onRenderSuccessRef.current = props.onRenderSuccess;
onRenderFailureRef.current = props.onRenderFailure;
useEffect(() => {
if (state.error) {
onRenderFailureRef.current?.(state.error);
} else if (state.loaded && Object.keys(state.loaded).length > 0) {
onRenderSuccessRef.current?.();
afterRender() {
const { loaded, loading, error } = this.state;
const { onRenderFailure, onRenderSuccess } = this.props;
if (!loading) {
if (error) {
(onRenderFailure as Function)(error);
} else if (loaded && Object.keys(loaded).length > 0) {
(onRenderSuccess as Function)();
}
}
}, [state.loaded, state.error]);
if (state.error) {
return options.loading({ error: state.error });
}
if (!state.loaded) {
return options.loading({});
}
return options.render(state.loaded, props as Props);
};
}
Renderer.preload = load;
CustomLoadableRenderer.defaultProps = defaultProps;
CustomLoadableRenderer.preload = LoadableRenderer.preload;
return Renderer;
return CustomLoadableRenderer;
}

View File

@@ -38,7 +38,7 @@ export function Label(props: LabelProps) {
} = props;
const baseColor = getColorVariants(theme, type);
const color = baseColor.text;
const color = baseColor.active;
const borderColor = baseColor.border;
const backgroundColor = baseColor.bg;

View File

@@ -95,6 +95,7 @@ export default defineConfig({
testIgnore: [
'**/tests/auth/**/*.spec.ts',
'**/tests/sqllab/**/*.spec.ts',
'**/tests/embedded/**/*.spec.ts',
...(process.env.INCLUDE_EXPERIMENTAL ? [] : ['**/experimental/**']),
],
use: {
@@ -132,6 +133,22 @@ export default defineConfig({
// No storageState = clean browser with no cached cookies
},
},
{
// Embedded dashboard tests - validates the full embedding flow:
// external app -> SDK -> iframe -> guest token -> dashboard render.
// Each spec file mutates per-dashboard embedding state (UUID,
// allowed_domains) on a single shared Superset, so files must not
// run in parallel even if more are added later.
name: 'chromium-embedded',
testMatch: '**/tests/embedded/**/*.spec.ts',
fullyParallel: false,
use: {
browserName: 'chromium',
testIdAttribute: 'data-test',
// Uses admin auth for API calls to configure embedding and get guest tokens
storageState: 'playwright/.auth/user.json',
},
},
],
// Web server setup - disabled in CI (Flask started separately in workflow)

View File

@@ -0,0 +1,96 @@
<!--
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.
-->
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Embedded Dashboard Test App</title>
<style>
html, body { margin: 0; padding: 0; height: 100%; }
#superset-container { width: 100%; height: 100vh; }
#superset-container iframe { width: 100%; height: 100%; border: none; }
#error { color: red; padding: 20px; display: none; }
#status { padding: 10px; font-family: monospace; font-size: 12px; }
</style>
</head>
<body>
<div id="status">Initializing embedded dashboard...</div>
<div id="error"></div>
<div id="superset-container" data-test="embedded-container"></div>
<script src="/sdk/index.js"></script>
<script>
(async function () {
const params = new URLSearchParams(window.location.search);
const uuid = params.get('uuid');
const supersetDomain = params.get('supersetDomain');
if (!uuid || !supersetDomain) {
document.getElementById('error').style.display = 'block';
document.getElementById('error').textContent =
'Missing required query params: uuid, supersetDomain';
return;
}
const statusEl = document.getElementById('status');
// fetchGuestToken is injected by Playwright via page.exposeFunction()
// Falls back to window.__guestToken for simple/static token injection
async function fetchGuestToken() {
if (typeof window.__fetchGuestToken === 'function') {
statusEl.textContent = 'Fetching guest token...';
const token = await window.__fetchGuestToken();
statusEl.textContent = 'Guest token received, loading dashboard...';
return token;
}
if (window.__guestToken) {
return window.__guestToken;
}
throw new Error('No guest token source available');
}
try {
// Parse optional UI config from query params
const uiConfig = {};
if (params.get('hideTitle') === 'true') uiConfig.hideTitle = true;
if (params.get('hideTab') === 'true') uiConfig.hideTab = true;
if (params.get('hideChartControls') === 'true') uiConfig.hideChartControls = true;
const dashboard = await supersetEmbeddedSdk.embedDashboard({
id: uuid,
supersetDomain: supersetDomain,
mountPoint: document.getElementById('superset-container'),
fetchGuestToken: fetchGuestToken,
dashboardUiConfig: Object.keys(uiConfig).length > 0 ? uiConfig : undefined,
debug: params.get('debug') === 'true',
});
statusEl.textContent = 'Dashboard embedded successfully';
// Expose dashboard API on window for Playwright assertions
window.__embeddedDashboard = dashboard;
} catch (err) {
document.getElementById('error').style.display = 'block';
document.getElementById('error').textContent = 'Embed failed: ' + err.message;
statusEl.textContent = 'Error';
}
})();
</script>
</body>
</html>

View File

@@ -132,26 +132,14 @@ export interface DashboardResult {
published?: boolean;
}
/**
* Get a dashboard by its title
* @param page - Playwright page instance (provides authentication context)
* @param title - The dashboard_title to search for
* @returns Dashboard object if found, null if not found
*/
export async function getDashboardByName(
async function getDashboardByFilter(
page: Page,
title: string,
col: 'dashboard_title' | 'slug',
value: string,
): Promise<DashboardResult | null> {
const filter = {
filters: [
{
col: 'dashboard_title',
opr: 'eq',
value: title,
},
],
};
const queryParam = rison.encode(filter);
const queryParam = rison.encode({
filters: [{ col, opr: 'eq', value }],
});
const response = await apiGet(
page,
`${ENDPOINTS.DASHBOARD}?q=${queryParam}`,
@@ -169,3 +157,29 @@ export async function getDashboardByName(
return null;
}
/**
* Get a dashboard by its title
* @param page - Playwright page instance (provides authentication context)
* @param title - The dashboard_title to search for
* @returns Dashboard object if found, null if not found
*/
export async function getDashboardByName(
page: Page,
title: string,
): Promise<DashboardResult | null> {
return getDashboardByFilter(page, 'dashboard_title', title);
}
/**
* Get a dashboard by its slug
* @param page - Playwright page instance (provides authentication context)
* @param slug - The slug to search for
* @returns Dashboard object if found, null if not found
*/
export async function getDashboardBySlug(
page: Page,
slug: string,
): Promise<DashboardResult | null> {
return getDashboardByFilter(page, 'slug', slug);
}

View File

@@ -0,0 +1,136 @@
/**
* 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.
*/
import { Page } from '@playwright/test';
import { apiPost, apiPut, getCsrfToken } from './requests';
import { ENDPOINTS as DASHBOARD_ENDPOINTS } from './dashboard';
export const ENDPOINTS = {
SECURITY_LOGIN: 'api/v1/security/login',
GUEST_TOKEN: 'api/v1/security/guest_token/',
} as const;
export interface EmbeddedConfig {
uuid: string;
allowed_domains: string[];
dashboard_id: number;
}
/**
* Enable embedding on a dashboard and return the embedded UUID.
* Uses PUT (upsert) to preserve UUID across repeated calls.
* @param page - Playwright page instance (provides authentication context)
* @param dashboardIdOrSlug - Numeric dashboard id or slug
* @param allowedDomains - Domains allowed to embed; empty array allows all
* @returns Embedded config with UUID, allowed_domains, and dashboard_id
*/
export async function apiEnableEmbedding(
page: Page,
dashboardIdOrSlug: number | string,
allowedDomains: string[] = [],
): Promise<EmbeddedConfig> {
const response = await apiPut(
page,
`${DASHBOARD_ENDPOINTS.DASHBOARD}${dashboardIdOrSlug}/embedded`,
{ allowed_domains: allowedDomains },
);
const body = await response.json();
return body.result as EmbeddedConfig;
}
/**
* Login as admin and return the JWT access token used by the guest_token
* endpoint. Cache the result at suite level (`beforeAll`) and pass it into
* `getGuestToken` to avoid a fresh login on every test.
*
* Defaults match `playwright/global-setup.ts` so credentials come from one
* source (env vars). Overrides via `options` win.
*/
export async function getAccessToken(
page: Page,
options?: { username?: string; password?: string },
): Promise<string> {
const username =
options?.username ?? process.env.PLAYWRIGHT_ADMIN_USERNAME ?? 'admin';
const password =
options?.password ?? process.env.PLAYWRIGHT_ADMIN_PASSWORD ?? 'general';
const loginResponse = await apiPost(
page,
ENDPOINTS.SECURITY_LOGIN,
{
username,
password,
provider: 'db',
refresh: true,
},
{ allowMissingCsrf: true },
);
const loginBody = await loginResponse.json();
return loginBody.access_token;
}
/**
* Get a guest token for an embedded dashboard.
* If `accessToken` is provided, the login round-trip is skipped — preferred
* for tests that fetch many tokens from a single suite.
* @returns Signed guest token string
*/
export async function getGuestToken(
page: Page,
dashboardId: number | string,
options?: {
accessToken?: string;
username?: string;
password?: string;
rls?: Array<{ dataset: number; clause: string }>;
},
): Promise<string> {
const accessToken =
options?.accessToken ??
(await getAccessToken(page, {
username: options?.username,
password: options?.password,
}));
const rls = options?.rls ?? [];
// The guest_token endpoint authenticates via JWT Bearer, but if the
// request also carries a session cookie (which page.request inherits from
// storageState), Flask-WTF still requires a matching X-CSRFToken. Send it
// unconditionally so this works whether the caller is authenticated via
// session, JWT, or both.
const { token: csrfToken } = await getCsrfToken(page);
const guestResponse = await page.request.post(ENDPOINTS.GUEST_TOKEN, {
data: {
user: {
username: 'embedded_test_user',
first_name: 'Embedded',
last_name: 'TestUser',
},
resources: [{ type: 'dashboard', id: String(dashboardId) }],
rls,
},
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${accessToken}`,
...(csrfToken ? { 'X-CSRFToken': csrfToken } : {}),
},
});
const guestBody = await guestResponse.json();
return guestBody.token;
}

View File

@@ -39,7 +39,7 @@ function getBaseUrl(): string {
return url.endsWith('/') ? url : `${url}/`;
}
interface CsrfResult {
export interface CsrfResult {
token: string;
error?: string;
}
@@ -49,7 +49,7 @@ interface CsrfResult {
* Superset provides a CSRF token via api/v1/security/csrf_token/
* The session cookie is automatically included by page.request
*/
async function getCsrfToken(page: Page): Promise<CsrfResult> {
export async function getCsrfToken(page: Page): Promise<CsrfResult> {
try {
const response = await page.request.get('api/v1/security/csrf_token/', {
failOnStatusCode: false,

View File

@@ -0,0 +1,172 @@
/**
* 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.
*/
import { Page, FrameLocator, Locator, expect } from '@playwright/test';
import { EMBEDDED } from '../utils/constants';
/**
* Page object for the embedded dashboard test app.
*
* The test app runs on a separate origin (its origin is assigned per-suite
* via an OS-allocated port) and uses the @superset-ui/embedded-sdk to render
* a Superset dashboard in an iframe. Playwright's page.exposeFunction()
* bridges the guest token from Node.js into the browser page.
*/
export class EmbeddedPage {
private readonly page: Page;
private static readonly SELECTORS = {
CONTAINER: '[data-test="embedded-container"]',
IFRAME: 'iframe[title="Embedded Dashboard"]',
STATUS: '#status',
ERROR: '#error',
} as const;
constructor(page: Page) {
this.page = page;
}
/**
* Set up the guest token bridge before navigating.
* Must be called BEFORE goto() since embedDashboard() calls fetchGuestToken
* immediately on page load.
*/
async exposeTokenFetcher(tokenFn: () => Promise<string>): Promise<void> {
await this.page.exposeFunction('__fetchGuestToken', tokenFn);
}
/**
* Navigate to the embedded test app with the given parameters.
* `appUrl` is the origin of the static test app (assigned dynamically by
* the spec's beforeAll fixture so workers don't collide on a fixed port).
*/
async goto(params: {
appUrl: string;
uuid: string;
supersetDomain: string;
hideTitle?: boolean;
hideTab?: boolean;
hideChartControls?: boolean;
debug?: boolean;
}): Promise<void> {
const searchParams = new URLSearchParams({
uuid: params.uuid,
supersetDomain: params.supersetDomain,
});
if (params.hideTitle) searchParams.set('hideTitle', 'true');
if (params.hideTab) searchParams.set('hideTab', 'true');
if (params.hideChartControls) searchParams.set('hideChartControls', 'true');
if (params.debug) searchParams.set('debug', 'true');
await this.page.goto(`${params.appUrl}/?${searchParams.toString()}`);
}
/**
* FrameLocator for the embedded dashboard iframe.
*/
get iframe(): FrameLocator {
return this.page.frameLocator(EmbeddedPage.SELECTORS.IFRAME);
}
/**
* Wait for the iframe to appear in the DOM AND have its src set.
* The SDK appends the iframe element before assigning src, so a bare
* `state: 'attached'` wait races the src read.
*/
async waitForIframe(options?: { timeout?: number }): Promise<void> {
const locator = this.page.locator(EmbeddedPage.SELECTORS.IFRAME);
await locator.waitFor({
state: 'attached',
timeout: options?.timeout ?? EMBEDDED.IFRAME_LOAD,
});
await expect(locator).toHaveAttribute('src', /.+/, {
timeout: options?.timeout ?? EMBEDDED.IFRAME_LOAD,
});
}
/**
* Wait for dashboard content to render inside the iframe.
* Looks for the grid-container which indicates charts are loading/loaded.
*/
async waitForDashboardContent(options?: { timeout?: number }): Promise<void> {
const frame = this.iframe;
await frame
.locator('.grid-container, [data-test="grid-container"]')
.first()
.waitFor({
state: 'visible',
timeout: options?.timeout ?? EMBEDDED.DASHBOARD_RENDER,
});
}
/**
* Matches a chart cell that has finished loading: it contains a real viz
* element (svg, canvas, table) AND no longer hosts the `Loading` spinner
* (`data-test="loading-indicator"`). Excluding the spinner matters —
* the spinner itself renders an SVG, so a `:has(svg)`-only check can match
* a still-loading chart for the wrong reason.
*/
static readonly RENDERED_CHART_SELECTOR =
'[data-test="chart-container"]:has(svg, canvas, table):not(:has([data-test="loading-indicator"]))';
/**
* Wait for at least one chart to finish rendering — viz drawn AND no
* loading spinner in that cell.
*/
async waitForChartRendered(options?: { timeout?: number }): Promise<void> {
await this.iframe
.locator(EmbeddedPage.RENDERED_CHART_SELECTOR)
.first()
.waitFor({
state: 'visible',
timeout: options?.timeout ?? EMBEDDED.CHART_RENDER,
});
}
/**
* Locator for the dashboard title input inside the iframe.
* Returned as a `Locator` so callers can use `expect(...).toBeVisible()` /
* `.toBeHidden()` with auto-retry instead of one-shot `.isVisible()`.
*/
get titleLocator(): Locator {
return this.iframe.locator(
'[data-test="dashboard-header-container"] [data-test="editable-title-input"]',
);
}
/**
* Get the status text from the test app.
*/
async getStatus(): Promise<string> {
return (
(await this.page.locator(EmbeddedPage.SELECTORS.STATUS).textContent()) ??
''
);
}
/**
* Get the error text, if any.
*/
async getError(): Promise<string> {
const errorEl = this.page.locator(EmbeddedPage.SELECTORS.ERROR);
const display = await errorEl.evaluate(el => getComputedStyle(el).display);
if (display === 'none') return '';
return (await errorEl.textContent()) ?? '';
}
}

View File

@@ -0,0 +1,356 @@
/**
* 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.
*/
import { test, expect, Browser, BrowserContext, Page } from '@playwright/test';
import { createServer, IncomingMessage, ServerResponse, Server } from 'http';
import { AddressInfo, Socket } from 'net';
import { readFileSync, existsSync } from 'fs';
import { join } from 'path';
import {
apiEnableEmbedding,
getAccessToken,
getGuestToken,
} from '../../helpers/api/embedded';
import { getDashboardBySlug } from '../../helpers/api/dashboard';
import { EmbeddedPage } from '../../pages/EmbeddedPage';
/**
* Superset domain (Flask server) — set by CI or defaults to local dev
*/
const SUPERSET_DOMAIN = (() => {
const url = process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8088';
return url.replace(/\/+$/, '');
})();
const SUPERSET_BASE_URL = SUPERSET_DOMAIN.endsWith('/')
? SUPERSET_DOMAIN
: `${SUPERSET_DOMAIN}/`;
/**
* Path to the SDK bundle built from superset-embedded-sdk/
*/
const SDK_BUNDLE_PATH = join(
__dirname,
'../../../../superset-embedded-sdk/bundle/index.js',
);
/**
* Path to the embedded test app static files
*/
const EMBED_APP_DIR = join(__dirname, '../../embedded-app');
/**
* Create a minimal static file server for the embedded test app.
* Serves only a fixed allowlist of routes — the test app references just
* its index.html and the SDK bundle, so anything else is 404.
*/
const INDEX_HTML_PATH = join(EMBED_APP_DIR, 'index.html');
interface EmbedAppServer {
server: Server;
url: string;
close: () => Promise<void>;
}
/**
* Start the static test app on an OS-assigned ephemeral port. Tracks open
* sockets so close() doesn't hang on iframe keep-alive connections, and so
* different workers/retries never collide on a fixed port.
*/
async function startEmbedAppServer(): Promise<EmbedAppServer> {
const sockets = new Set<Socket>();
const server = createServer((req: IncomingMessage, res: ServerResponse) => {
const urlPath = req.url?.split('?')[0] || '/';
if (urlPath === '/sdk/index.js') {
if (!existsSync(SDK_BUNDLE_PATH)) {
res.writeHead(404);
res.end(
'SDK bundle not found. Run: cd superset-embedded-sdk && npm ci && npm run build',
);
return;
}
res.writeHead(200, { 'Content-Type': 'text/javascript' });
res.end(readFileSync(SDK_BUNDLE_PATH));
return;
}
if (urlPath === '/' || urlPath === '/index.html') {
res.writeHead(200, { 'Content-Type': 'text/html' });
res.end(readFileSync(INDEX_HTML_PATH));
return;
}
res.writeHead(404);
res.end('Not found');
});
server.on('connection', socket => {
sockets.add(socket);
socket.once('close', () => sockets.delete(socket));
});
await new Promise<void>((resolve, reject) => {
server.once('error', reject);
server.listen(0, '127.0.0.1', () => {
server.removeListener('error', reject);
resolve();
});
});
const address = server.address() as AddressInfo;
const url = `http://127.0.0.1:${address.port}`;
return {
server,
url,
close: () =>
new Promise<void>(resolve => {
for (const socket of sockets) socket.destroy();
sockets.clear();
server.close(() => resolve());
}),
};
}
/**
* Create a browser context authenticated as admin for API-only work
* (enabling embedding, restoring config). Caller is responsible for closing.
*/
function createAdminContext(browser: Browser): Promise<BrowserContext> {
return browser.newContext({
storageState: 'playwright/.auth/user.json',
baseURL: SUPERSET_BASE_URL,
});
}
// ─── Test Suite ────────────────────────────────────────────────────────────
// Describe wrapper is needed for shared server state and serial execution:
// all tests share a static file server and must not run in parallel.
test.describe('Embedded Dashboard E2E', () => {
test.describe.configure({ mode: 'serial' });
// The full embedded chain (login → guest token → iframe → dashboard render
// → chart render) routinely exceeds the 30s default on cold CI starts.
test.setTimeout(60000);
let appServer: EmbedAppServer;
let accessToken: string;
let embedUuid: string;
let dashboardId: number;
/**
* Set up a page to render the default embedded dashboard.
* Tests that need a different UUID or UI config should not use this helper.
*/
async function setupEmbeddedPage(page: Page): Promise<EmbeddedPage> {
const embeddedPage = new EmbeddedPage(page);
await embeddedPage.exposeTokenFetcher(async () =>
getGuestToken(page, dashboardId, { accessToken }),
);
await embeddedPage.goto({
appUrl: appServer.url,
uuid: embedUuid,
supersetDomain: SUPERSET_DOMAIN,
});
await embeddedPage.waitForIframe();
await embeddedPage.waitForDashboardContent();
return embeddedPage;
}
test.beforeAll(async ({ browser }) => {
// Skip all tests if the SDK bundle hasn't been built
test.skip(
!existsSync(SDK_BUNDLE_PATH),
'Embedded SDK bundle not found. Build it with: cd superset-embedded-sdk && npm ci && npm run build',
);
appServer = await startEmbedAppServer();
// Use a fresh context with auth to set up test data via API
const context = await createAdminContext(browser);
const setupPage = await context.newPage();
try {
const dashboard = await getDashboardBySlug(setupPage, 'world_health');
if (!dashboard) {
throw new Error(
'Dashboard "world_health" not found. Ensure load_examples ran in CI setup.',
);
}
dashboardId = dashboard.id;
// Enable embedding on the dashboard (empty allowed_domains = allow all)
const embedded = await apiEnableEmbedding(setupPage, dashboardId);
embedUuid = embedded.uuid;
// Cache the JWT access token so tests don't re-login per guest token.
accessToken = await getAccessToken(setupPage);
} finally {
await context.close();
}
});
test.afterAll(async ({ browser }) => {
// Defensive restore in case the allowed_domains test failed mid-flight.
if (dashboardId !== undefined) {
const context = await createAdminContext(browser);
try {
const setupPage = await context.newPage();
await apiEnableEmbedding(setupPage, dashboardId, []);
} catch {
// Best-effort cleanup — never fail teardown.
} finally {
await context.close();
}
}
if (appServer) await appServer.close();
});
test('dashboard renders in embedded iframe', async ({ page }) => {
const embeddedPage = await setupEmbeddedPage(page);
// Verify the iframe src points to Superset's /embedded/ endpoint
await expect(
page.locator('iframe[title="Embedded Dashboard"]'),
).toHaveAttribute('src', new RegExp(`/embedded/${embedUuid}`));
// Verify no errors in the test app
expect(await embeddedPage.getError()).toBe('');
// Baseline: title should be visible when hideTitle is not set. This
// doubles as a positive existence check the `hideTitle` test relies on
// for distinguishing "title was hidden" from "selector is wrong".
await expect(embeddedPage.titleLocator).toBeVisible();
// Prove the dashboard actually renders, not just the chrome.
await embeddedPage.waitForChartRendered();
});
test('UI config hideTitle hides dashboard title', async ({ page }) => {
const embeddedPage = new EmbeddedPage(page);
await embeddedPage.exposeTokenFetcher(async () =>
getGuestToken(page, dashboardId, { accessToken }),
);
await embeddedPage.goto({
appUrl: appServer.url,
uuid: embedUuid,
supersetDomain: SUPERSET_DOMAIN,
hideTitle: true,
});
await embeddedPage.waitForIframe();
await embeddedPage.waitForDashboardContent();
// The iframe URL should include uiConfig parameter
await expect(
page.locator('iframe[title="Embedded Dashboard"]'),
).toHaveAttribute('src', /uiConfig=/);
// hideTitle removes the header from the DOM (rather than CSS-hiding it),
// so toBeHidden + toHaveCount(0) together assert: not visible AND
// confirmed-removed (so the test can't pass for the wrong reason if the
// selector ever drifts — the baseline test asserts the selector matches
// when hideTitle is off).
await expect(embeddedPage.titleLocator).toBeHidden();
await expect(embeddedPage.titleLocator).toHaveCount(0);
});
test('charts render inside embedded iframe', async ({ page }) => {
const embeddedPage = await setupEmbeddedPage(page);
await embeddedPage.waitForChartRendered();
const renderedCharts = embeddedPage.iframe.locator(
EmbeddedPage.RENDERED_CHART_SELECTOR,
);
expect(await renderedCharts.count()).toBeGreaterThan(0);
});
test('allowed_domains blocks unauthorized referrer', async ({
page,
browser,
}) => {
const context = await createAdminContext(browser);
const setupPage = await context.newPage();
try {
// Restrict to a domain that is NOT the test app's origin
const restrictedEmbed = await apiEnableEmbedding(setupPage, dashboardId, [
'https://allowed.example.com',
]);
const embeddedPage = new EmbeddedPage(page);
await embeddedPage.exposeTokenFetcher(async () =>
getGuestToken(page, dashboardId, { accessToken }),
);
// The deterministic signal that the referrer check fired is the HTTP
// status of the /embedded/<uuid> response — assert that directly rather
// than racing against cross-origin iframe rendering.
const embeddedResponsePromise = page.waitForResponse(
resp =>
resp.url().includes(`/embedded/${restrictedEmbed.uuid}`) &&
resp.request().resourceType() === 'document',
);
await embeddedPage.goto({
appUrl: appServer.url,
uuid: restrictedEmbed.uuid,
supersetDomain: SUPERSET_DOMAIN,
});
const response = await embeddedResponsePromise;
expect(response.status()).toBe(403);
} finally {
// Restore the open embedding config for other tests in this file.
await apiEnableEmbedding(setupPage, dashboardId, []);
await context.close();
}
});
test('guest token enables dashboard data access', async ({ page }) => {
const embeddedPage = new EmbeddedPage(page);
let tokenCallCount = 0;
await embeddedPage.exposeTokenFetcher(async () => {
tokenCallCount += 1;
return getGuestToken(page, dashboardId, { accessToken });
});
await embeddedPage.goto({
appUrl: appServer.url,
uuid: embedUuid,
supersetDomain: SUPERSET_DOMAIN,
});
await embeddedPage.waitForIframe();
await embeddedPage.waitForDashboardContent();
await embeddedPage.waitForChartRendered();
// The SDK fetches the token exactly once per embed (caching is the
// SDK's responsibility, not ours) — assert the stronger invariant.
expect(tokenCallCount).toBe(1);
// Confirm at least one chart actually rendered with data, not just its shell
const renderedCharts = embeddedPage.iframe.locator(
EmbeddedPage.RENDERED_CHART_SELECTOR,
);
expect(await renderedCharts.count()).toBeGreaterThan(0);
});
});

View File

@@ -75,3 +75,16 @@ export const TIMEOUT = {
*/
SLOW_TEST: 60000, // 60s for tests that chain multiple slow operations
} as const;
/**
* Embedded dashboard test app configuration.
* The test app is served by a Node.js http server started in the test fixture.
*/
export const EMBEDDED = {
/** Timeout for iframe to appear in the DOM */
IFRAME_LOAD: 15000, // 15s
/** Timeout for dashboard content to render inside the iframe */
DASHBOARD_RENDER: 30000, // 30s
/** Timeout for individual chart cells to finish rendering */
CHART_RENDER: 30000, // 30s
} as const;

View File

@@ -1,83 +0,0 @@
/**
* 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.
*/
import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
import htmlTextFilterValueGetter, {
htmlTextComparator,
} from './htmlTextFilterValueGetter';
const makeParams = (value: unknown): ValueGetterParams =>
({
data: { foo: value },
colDef: { field: 'foo' },
}) as unknown as ValueGetterParams;
test('htmlTextFilterValueGetter extracts visible text from HTML anchor', () => {
expect(
htmlTextFilterValueGetter(
makeParams(
'<a href="https://jira.example.com/123/S18_3232">S18_3232</a>',
),
),
).toBe('S18_3232');
});
test('htmlTextFilterValueGetter strips nested HTML markup', () => {
expect(
htmlTextFilterValueGetter(
makeParams('<div><strong>Hello</strong> <em>World</em></div>'),
),
).toBe('Hello World');
});
test('htmlTextFilterValueGetter passes plain strings through', () => {
expect(htmlTextFilterValueGetter(makeParams('plain value'))).toBe(
'plain value',
);
});
test('htmlTextFilterValueGetter passes non-string values through', () => {
expect(htmlTextFilterValueGetter(makeParams(42))).toBe(42);
expect(htmlTextFilterValueGetter(makeParams(null))).toBeNull();
expect(htmlTextFilterValueGetter(makeParams(undefined))).toBeUndefined();
});
test('htmlTextComparator orders by visible text, not raw HTML', () => {
// URL prefixes (zzz vs bbb) would flip the order under raw-HTML sort,
// but the visible labels (S700_4002 vs S72_3212) sort the other way.
const left = '<a href="https://jira.example.com/zzz/S700_4002">S700_4002</a>';
const right = '<a href="https://jira.example.com/bbb/S72_3212">S72_3212</a>';
expect(htmlTextComparator(left, right)).toBeLessThan(0);
});
test('htmlTextComparator handles nulls and numbers', () => {
expect(htmlTextComparator(null, null)).toBe(0);
expect(htmlTextComparator(null, 'x')).toBeLessThan(0);
expect(htmlTextComparator('x', null)).toBeGreaterThan(0);
expect(htmlTextComparator(1, 2)).toBeLessThan(0);
expect(htmlTextComparator(2, 1)).toBeGreaterThan(0);
});
test('htmlTextComparator preserves default codepoint ordering for plain strings', () => {
// AG Grid's default string comparator orders by codepoint, so 'Z' (90)
// sorts before 'a' (97). A locale-aware comparator would flip this —
// verify we match the default so plain string columns are unaffected.
expect(htmlTextComparator('Z', 'a')).toBeLessThan(0);
expect(htmlTextComparator('a', 'Z')).toBeGreaterThan(0);
expect(htmlTextComparator('apple', 'banana')).toBeLessThan(0);
});

View File

@@ -1,74 +0,0 @@
/**
* 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.
*/
import { isProbablyHTML, sanitizeHtml } from '@superset-ui/core';
import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
const stripHtmlToText = (html: string): string => {
const doc = new DOMParser().parseFromString(sanitizeHtml(html), 'text/html');
return (doc.body.textContent || '').trim();
};
// Cache the comparator-ready form per raw string. Both the HTML-detection
// step (`isProbablyHTML`, which itself invokes DOMParser for HTML-looking
// values) and the extraction step (`stripHtmlToText`, also DOMParser) are
// expensive; sort runs `O(n log n)` comparator calls against the same set
// of cell values. Memoizing the combined detection + extraction means each
// unique cell value pays the cost once per session. Module-level scope;
// bounded by the count of unique string cell values seen.
const comparableTextCache = new Map<string, string>();
const toComparableText = (raw: string): string => {
const cached = comparableTextCache.get(raw);
if (cached !== undefined) return cached;
const normalized = isProbablyHTML(raw) ? stripHtmlToText(raw) : raw;
comparableTextCache.set(raw, normalized);
return normalized;
};
/**
* Returns the visible-text representation of an HTML cell value so AG Grid
* filters and sort operate on what the user sees, not the underlying markup.
* Pass-through for non-HTML values.
*/
const htmlTextFilterValueGetter = (params: ValueGetterParams) => {
const raw = params.data?.[params.colDef.field as string];
return typeof raw === 'string' ? toComparableText(raw) : raw;
};
/**
* Comparator that mirrors AG Grid's default string comparator (codepoint
* order, nulls first), but extracts visible text from HTML values first
* so HTML cells sort by their displayed label. Plain (non-HTML) values
* pass through unchanged, preserving default ordering — e.g. 'Z' still
* sorts before 'a' as it does under the default comparator.
*/
export const htmlTextComparator = (a: unknown, b: unknown): number => {
const toText = (v: unknown) =>
typeof v === 'string' ? toComparableText(v) : v;
const aT = toText(a);
const bT = toText(b);
if (aT == null && bT == null) return 0;
if (aT == null) return -1;
if (bT == null) return 1;
if (typeof aT === 'number' && typeof bT === 'number') return aT - bT;
if (aT === bT) return 0;
return aT < bT ? -1 : 1;
};
export default htmlTextFilterValueGetter;

View File

@@ -32,9 +32,6 @@ import {
} from '../types';
import getCellClass from './getCellClass';
import filterValueGetter from './filterValueGetter';
import htmlTextFilterValueGetter, {
htmlTextComparator,
} from './htmlTextFilterValueGetter';
import dateFilterComparator from './dateFilterComparator';
import DateWithFormatter from './DateWithFormatter';
import { getAggFunc } from './getAggFunc';
@@ -320,24 +317,6 @@ export const useColDefs = ({
...(isPercentMetric && {
filterValueGetter,
}),
...(dataType === GenericDataType.String &&
!serverPagination && {
// HTML cells (e.g. anchor markup) are rendered by TextCellRenderer
// via dangerouslySetInnerHTML; without these the filter and sort
// operate on raw HTML so the URL inside the markup dictates order
// and the "Contains" filter matches against the raw HTML string.
//
// Gated on !serverPagination: in server-pagination mode sort and
// filter are both delegated to the backend (which sees raw HTML
// in the database), so applying the visible-text getter only on
// the client would create a mismatch where the typed filter
// value is stripped client-side but the server query still
// operates on the raw HTML. Server-paginated tables with HTML
// columns are out of scope for this fix and would require
// server-side handling.
filterValueGetter: htmlTextFilterValueGetter,
comparator: htmlTextComparator,
}),
...(dataType === GenericDataType.Temporal && {
// Use dateFilterValueGetter so AG Grid correctly identifies null dates for blank filter
filterValueGetter: dateFilterValueGetter,

View File

@@ -285,6 +285,8 @@ export default function transformProps(
}
const labelProps = {
color: theme.colorText,
textBorderColor: theme.colorBgBase,
textBorderWidth: 1,
};
const traverse = (
treeNodes: TreeNode[],

View File

@@ -1,53 +0,0 @@
/**
* 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.
*/
import { ChartProps } from '@superset-ui/core';
import { supersetTheme } from '@apache-superset/core/theme';
import { EchartsSunburstChartProps } from '../../src/Sunburst/types';
import transformProps from '../../src/Sunburst/transformProps';
const formData = {
colorScheme: 'bnbColors',
datasource: '3__table',
groupby: ['category'],
metric: 'sum__value',
};
const chartProps = new ChartProps({
formData,
width: 800,
height: 600,
queriesData: [
{
data: [
{ category: 'A', sum__value: 10 },
{ category: 'B', sum__value: 20 },
],
},
],
theme: supersetTheme,
});
test('series label has no textBorderColor or textBorderWidth', () => {
const { echartOptions } = transformProps(
chartProps as EchartsSunburstChartProps,
);
const series = (echartOptions as any).series[0];
expect(series.label).not.toHaveProperty('textBorderColor');
expect(series.label).not.toHaveProperty('textBorderWidth');
});

View File

@@ -781,16 +781,6 @@ export function exploreJSON(
handleChartDataResponse(response, json, useLegacyApi),
)
.then(queriesResponse => {
// Drop stale responses: if a newer query has started for this chart,
// its controller will have replaced ours in state, so ignore this
// response to avoid clobbering newer data with older results.
if (key != null) {
const currentController =
getState().charts?.[key]?.queryController;
if (currentController && currentController !== controller) {
return undefined;
}
}
(queriesResponse as QueryData[]).forEach(
(resultItem: QueryData & { applied_filters?: JsonObject[] }) =>
dispatch(
@@ -836,16 +826,6 @@ export function exploreJSON(
);
}
// Drop stale failures the same way we drop stale successes,
// so a slow earlier request can't mark a newer one as failed.
if (key != null) {
const currentController =
getState().charts?.[key]?.queryController;
if (currentController && currentController !== controller) {
return undefined;
}
}
if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
// In async mode we just pass the raw error response through
return dispatch(

View File

@@ -156,78 +156,6 @@ describe('chart actions', () => {
.mockImplementation((data: unknown) => Promise.resolve(data));
});
test('should drop stale success dispatches when a newer controller has replaced ours in state', async () => {
const chartKey = 'stale_success_test';
const formData: Partial<QueryFormData> = {
slice_id: 456,
datasource: 'table__1',
viz_type: 'table',
};
// A controller belonging to a *newer* in-flight request, already stored
// in state by the time this thunk's response resolves.
const newerController = new AbortController();
const state: MockState = {
charts: {
[chartKey]: {
queryController: newerController,
},
},
common: {
conf: {
SUPERSET_WEBSERVER_TIMEOUT: 60,
},
},
};
const getState = jest.fn(() => state);
const dispatchMock = jest.fn();
const getChartDataRequestSpy = jest
.spyOn(actions, 'getChartDataRequest')
.mockResolvedValue({
response: { status: 200 } as Response,
json: { result: [{ data: [{ stale: true }] }] },
});
const handleChartDataResponseSpy = jest
.spyOn(actions, 'handleChartDataResponse')
.mockResolvedValue([{ data: [{ stale: true }] }]);
const updateDataMaskSpy = jest
.spyOn(dataMaskActions, 'updateDataMask')
.mockReturnValue({ type: 'UPDATE_DATA_MASK' } as ReturnType<
typeof dataMaskActions.updateDataMask
>);
const getQuerySettingsStub = jest
.spyOn(exploreUtils, 'getQuerySettings')
.mockReturnValue([false, () => {}] as unknown as ReturnType<
typeof exploreUtils.getQuerySettings
>);
try {
const thunkAction = actions.exploreJSON(
formData as QueryFormData,
false,
undefined,
chartKey,
);
await thunkAction(
dispatchMock as unknown as actions.ChartThunkDispatch,
getState as unknown as () => actions.RootState,
undefined,
);
// CHART_UPDATE_STARTED is fine (it ran before the gate),
// but CHART_UPDATE_SUCCEEDED must NOT have fired with the stale data.
const dispatchedTypes = dispatchMock.mock.calls.map(
([action]) => action?.type,
);
expect(dispatchedTypes).toContain(actions.CHART_UPDATE_STARTED);
expect(dispatchedTypes).not.toContain(actions.CHART_UPDATE_SUCCEEDED);
} finally {
getChartDataRequestSpy.mockRestore();
handleChartDataResponseSpy.mockRestore();
updateDataMaskSpy.mockRestore();
getQuerySettingsStub.mockRestore();
}
});
test('should defer abort of previous controller to avoid Redux state mutation', async () => {
jest.useFakeTimers();
const chartKey = 'defer_abort_test';

View File

@@ -18,7 +18,7 @@
*/
import 'src/public-path';
import { lazy, Suspense, useEffect } from 'react';
import { lazy, StrictMode, Suspense, useEffect } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { BrowserRouter as Router, Route } from 'react-router-dom';
import { Global } from '@emotion/react';
@@ -197,7 +197,11 @@ function start() {
if (!root) {
root = createRoot(appMountPoint);
}
root.render(<EmbeddedApp />);
root.render(
<StrictMode>
<EmbeddedApp />
</StrictMode>,
);
},
err => {
// something is most likely wrong with the guest token; reset the guard

View File

@@ -22,18 +22,12 @@ export const ExplorePopoverContent = styled.div`
.edit-popover-resize {
transform: scaleX(-1);
float: right;
margin-top: ${({ theme }) => theme.margin}px;
margin-right: ${({ theme }) => theme.marginXXS * -1}px;
margin-top: ${({ theme }) => theme.sizeUnit * 4}px;
margin-right: ${({ theme }) => theme.sizeUnit * -1}px;
color: ${({ theme }) => theme.colorIcon};
cursor: nwse-resize;
}
.filter-sql-editor {
border: ${({ theme }) => theme.colorBorder} solid thin;
}
&& .ant-tabs-nav {
margin-bottom: ${({ theme }) => theme.marginSM}px;
}
&& .ant-form-item {
margin-bottom: ${({ theme }) => theme.marginXS}px;
}
`;

View File

@@ -78,6 +78,14 @@ interface AdhocFilterEditPopoverState {
}
const FilterPopoverContentContainer = styled.div`
.adhoc-filter-edit-tabs > .nav-tabs {
margin-bottom: ${({ theme }) => theme.sizeUnit * 2}px;
& > li > a {
padding: ${({ theme }) => theme.sizeUnit}px;
}
}
#filter-edit-popover {
max-width: none;
}
@@ -89,17 +97,21 @@ const FilterPopoverContentContainer = styled.div`
.filter-edit-clause-section {
display: flex;
flex-direction: row;
gap: ${({ theme }) => theme.marginMD}px;
gap: ${({ theme }) => theme.sizeUnit * 5}px;
}
.adhoc-filter-simple-column-dropdown {
margin-top: ${({ theme }) => theme.sizeUnit * 5}px;
}
`;
const FilterActionsContainer = styled.div`
margin-top: ${({ theme }) => theme.marginXS}px;
margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
`;
const LayerSelectContainer = styled.div`
margin-top: ${({ theme }) => theme.marginXS}px;
margin-bottom: ${({ theme }) => theme.marginXXL}px;
margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
margin-bottom: ${({ theme }) => theme.sizeUnit * 12}px;
`;
export default class AdhocFilterEditPopover extends Component<

View File

@@ -523,7 +523,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
const subjectComponent = (
<Select
css={{
marginBottom: theme.marginXS,
marginTop: theme.sizeUnit * 4,
marginBottom: theme.sizeUnit * 4,
}}
data-test="select-element"
options={columns.map(column => ({
@@ -564,7 +565,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
>
<SelectWithLabel
css={css`
margin-top: ${theme.marginXS}px;
margin-top: ${theme.sizeUnit * 4}px;
`}
labelText={labelText}
options={suggestions}
@@ -580,7 +581,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
>
<div
css={css`
margin-top: ${theme.marginXS}px;
margin-top: ${theme.sizeUnit * 4}px;
`}
/>
<Input

View File

@@ -64,6 +64,7 @@ export const StyledCloseButton = styled(Button)`
color: ${theme.colorPrimaryText};
font-size: ${theme.fontSizeSM}px;
font-weight: ${theme.fontWeightStrong};
text-transform: uppercase;
min-width: ${theme.sizeUnit * 36};
min-height: ${theme.sizeUnit * 8};
box-shadow: none;
@@ -112,6 +113,7 @@ export const StyledSaveButton = styled(Button)`
color: ${theme.colorTextLightSolid};
font-size: ${theme.fontSizeSM}px;
font-weight: ${theme.fontWeightStrong};
text-transform: uppercase;
min-width: ${theme.sizeUnit * 36};
min-height: ${theme.sizeUnit * 8};
box-shadow: none;

View File

@@ -36,6 +36,7 @@ export const StyledExtentButton = styled(Button)`
color: ${theme.colorPrimaryText};
font-size: ${theme.fontSizeSM}px;
font-weight: ${theme.fontWeightStrong};
text-transform: uppercase;
min-width: ${theme.sizeUnit * 36};
min-height: ${theme.sizeUnit * 8};
box-shadow: none;

View File

@@ -18,6 +18,7 @@
*/
import 'src/public-path';
import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import { logging } from '@apache-superset/core/utils';
import initPreamble from 'src/preamble';
@@ -31,7 +32,11 @@ if (appMountPoint) {
await initPreamble();
} finally {
const { default: App } = await import(/* webpackMode: "eager" */ './App');
root.render(<App />);
root.render(
<StrictMode>
<App />
</StrictMode>,
);
}
})().catch(err => {
logging.error('Unhandled error during app initialization', err);

View File

@@ -20,6 +20,7 @@ import 'src/public-path';
// Menu App. Used in views that do not already include the Menu component in the layout.
// eg, backend rendered views
import { StrictMode } from 'react';
import { Provider } from 'react-redux';
import { createRoot } from 'react-dom/client';
import { BrowserRouter } from 'react-router-dom';
@@ -45,24 +46,26 @@ const emotionCache = createCache({
});
const app = (
<CacheProvider value={emotionCache}>
<ThemeProvider theme={theme}>
<Provider store={store}>
<BrowserRouter>
<QueryParamProvider
adapter={ReactRouter5Adapter}
options={{
searchStringToObject: querystring.parse,
objectToSearchString: (object: Record<string, any>) =>
querystring.stringify(object, { encode: false }),
}}
>
<Menu data={menu} />
</QueryParamProvider>
</BrowserRouter>
</Provider>
</ThemeProvider>
</CacheProvider>
<StrictMode>
<CacheProvider value={emotionCache}>
<ThemeProvider theme={theme}>
<Provider store={store}>
<BrowserRouter>
<QueryParamProvider
adapter={ReactRouter5Adapter}
options={{
searchStringToObject: querystring.parse,
objectToSearchString: (object: Record<string, any>) =>
querystring.stringify(object, { encode: false }),
}}
>
<Menu data={menu} />
</QueryParamProvider>
</BrowserRouter>
</Provider>
</ThemeProvider>
</CacheProvider>
</StrictMode>
);
const menuMountPoint = document.getElementById('app-menu');

View File

@@ -71,6 +71,22 @@ def create_app(
# value of app_root so things work out of the box
if not app.config["STATIC_ASSETS_PREFIX"]:
app.config["STATIC_ASSETS_PREFIX"] = app_root
# Prefix APP_ICON path with subdirectory root for subdirectory deployments
if (
app.config.get("APP_ICON", "").startswith("/static/")
and app_root != "/"
):
app.config["APP_ICON"] = f"{app_root}{app.config['APP_ICON']}"
# Also update theme tokens for subdirectory deployments
for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
theme = app.config[theme_key]
token = theme.get("token", {})
# Update brandLogoUrl if it points to /static/
if token.get("brandLogoUrl", "").startswith("/static/"):
token["brandLogoUrl"] = f"{app_root}{token['brandLogoUrl']}"
# Update brandLogoHref if it's the default "/"
if token.get("brandLogoHref") == "/":
token["brandLogoHref"] = app_root
if app.config["APPLICATION_ROOT"] == "/":
app.config["APPLICATION_ROOT"] = app_root

View File

@@ -516,47 +516,6 @@ def _cleanup_session_on_error() -> None:
logger.warning("Error cleaning up session after exception: %s", e)
def _remove_session_safe() -> None:
"""Remove the scoped SQLAlchemy session, tolerating SSL/connection errors.
Thread-pool workers reuse threads across requests. Before each tool call
the session is removed to prevent a prior request's thread-local session
from leaking into the next one. If the underlying DBAPI connection died
between requests (e.g. RDS SSL idle-timeout or max-connection-age), the
rollback implicit in ``session.close()`` raises a ``DBAPIError`` subclass
(``OperationalError`` for psycopg2, ``InterfaceError`` for some other
drivers).
When that happens:
1. Invalidate the dead connection so the pool discards it (rather than
returning a broken connection to the next caller).
2. Retry ``remove()`` to deregister the session from the scoped registry.
The tool call still proceeds because a fresh connection will be obtained
on the next DB access.
"""
from sqlalchemy.exc import DBAPIError
from superset.extensions import db
try:
db.session.remove()
except DBAPIError as exc:
logger.warning(
"Connection error during pre-call session cleanup "
"(likely SSL/idle timeout); invalidating connection and retrying: %s",
exc,
)
try:
db.session.invalidate()
except Exception as invalidate_exc:
logger.debug(
"Could not invalidate session after connection error: %s",
invalidate_exc,
)
db.session.remove() # retry: session deregisters cleanly after invalidation
def mcp_auth_hook(tool_func: F) -> F: # noqa: C901
"""
Authentication and authorization decorator for MCP tools.
@@ -679,7 +638,9 @@ def mcp_auth_hook(tool_func: F) -> F: # noqa: C901
# still be bound to a different tenant's DB engine. Removing it here
# ensures the next DB access creates a fresh session bound to the
# correct engine for the current request.
_remove_session_safe()
from superset.extensions import db
db.session.remove()
user = _setup_user_context()
# No Flask context - this is a FastMCP internal operation

View File

@@ -46,10 +46,7 @@ from superset.mcp_service.chart.schemas import (
GetChartDataRequest,
PerformanceMetadata,
)
from superset.mcp_service.utils import (
escape_llm_context_delimiters,
sanitize_for_llm_context,
)
from superset.mcp_service.utils import sanitize_for_llm_context
from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
from superset.mcp_service.utils.oauth2_utils import (
build_oauth2_redirect_message,
@@ -202,12 +199,8 @@ async def get_chart_data( # noqa: C901
if not chart:
await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
return ChartError(
error=(
f"No chart found with identifier: {safe_id}."
" Use list_charts to get valid chart IDs."
),
error=f"No chart found with identifier: {request.identifier}",
error_type="NotFound",
)

View File

@@ -47,10 +47,7 @@ from superset.mcp_service.chart.schemas import (
URLPreview,
VegaLitePreview,
)
from superset.mcp_service.utils import (
escape_llm_context_delimiters,
sanitize_for_llm_context,
)
from superset.mcp_service.utils import sanitize_for_llm_context
from superset.mcp_service.utils.oauth2_utils import (
build_oauth2_redirect_message,
OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -1204,22 +1201,8 @@ async def _get_chart_preview_internal( # noqa: C901
if not chart:
await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
is_form_data_key = (
isinstance(request.identifier, str)
and len(request.identifier) > 8
and not request.identifier.isdigit()
)
if is_form_data_key:
recovery = (
"If using a form_data_key, it may have expired — "
"use generate_explore_link to get a fresh key, "
"or use list_charts to find a saved chart by ID."
)
else:
recovery = "Use list_charts to get valid chart IDs."
safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
return ChartError(
error=f"No chart found with identifier: {safe_id}. {recovery}",
error=f"No chart found with identifier: {request.identifier}",
error_type="NotFound",
)

View File

@@ -47,7 +47,6 @@ from superset.mcp_service.chart.schemas import (
PerformanceMetadata,
UpdateChartRequest,
)
from superset.mcp_service.utils import escape_llm_context_delimiters
from superset.mcp_service.utils.oauth2_utils import (
build_oauth2_redirect_message,
OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -338,18 +337,17 @@ async def update_chart( # noqa: C901
chart = find_chart_by_identifier(request.identifier)
if not chart:
safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
not_found_msg = (
f"No chart found with identifier: {safe_id}."
" Use list_charts to get valid chart IDs."
)
return GenerateChartResponse.model_validate(
{
"chart": None,
"error": {
"error_type": "NotFound",
"message": not_found_msg,
"details": not_found_msg,
"message": (
f"No chart found with identifier: {request.identifier}"
),
"details": (
f"No chart found with identifier: {request.identifier}"
),
},
"success": False,
"schema_version": "2.0",

View File

@@ -334,10 +334,7 @@ def _find_and_authorize_dashboard(
dashboard=None,
dashboard_url=None,
position=None,
error=(
f"Dashboard with ID {dashboard_id} not found."
" Use list_dashboards to get valid dashboard IDs."
),
error=f"Dashboard with ID {dashboard_id} not found",
)
try:
@@ -395,10 +392,7 @@ def add_chart_to_existing_dashboard(
dashboard=None,
dashboard_url=None,
position=None,
error=(
f"Chart with ID {request.chart_id} not found."
" Use list_charts to get valid chart IDs."
),
error=f"Chart with ID {request.chart_id} not found",
)
# Validate dataset access for the chart.

View File

@@ -230,10 +230,7 @@ def generate_dashboard( # noqa: C901
return GenerateDashboardResponse(
dashboard=None,
dashboard_url=None,
error=(
f"Charts not found: {sorted(missing_chart_ids)}."
" Use list_charts to get valid chart IDs."
),
error=f"Charts not found: {list(missing_chart_ids)}",
)
# Validate dataset access for each chart.

View File

@@ -183,10 +183,7 @@ async def query_dataset( # noqa: C901
if dataset is None:
await ctx.error("Dataset not found: identifier=%s" % (request.dataset_id,))
return DatasetError.create(
error=(
f"No dataset found with identifier: {request.dataset_id}."
" Use list_datasets to get valid dataset IDs."
),
error=f"No dataset found with identifier: {request.dataset_id}",
error_type="NotFound",
)

View File

@@ -100,10 +100,7 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: Context) -> ExecuteSqlRes
)
return ExecuteSqlResponse(
success=False,
error=(
f"Database with ID {request.database_id} not found."
" Use list_databases to get valid database IDs."
),
error=f"Database with ID {request.database_id} not found",
error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR.value,
)

View File

@@ -103,8 +103,7 @@ def open_sql_lab_with_context(
database = DatabaseDAO.find_by_id(request.database_connection_id)
if not database:
error_message = (
f"Database with ID {request.database_connection_id} not found."
" Use list_databases to get valid database IDs."
f"Database with ID {request.database_connection_id} not found"
)
return _sanitize_sql_lab_response_for_llm_context(
SqlLabResponse(

View File

@@ -99,29 +99,14 @@ class SecretsMigrator:
def discover_encrypted_fields(self) -> dict[str, dict[str, EncryptedType]]:
"""
Iterates over ORM-mapped tables, looking for EncryptedType columns
along the way. Builds up a dict of
Iterates over SqlAlchemy's metadata, looking for EncryptedType
columns along the way. Builds up a dict of
table_name -> dict of col_name: enc type instance
Superset's ORM models inherit from Flask-AppBuilder's declarative base
(`flask_appbuilder.Model`), whose MetaData is distinct from
`db.metadata`. We combine both sources so encrypted columns are found
regardless of which base a model uses. FAB's metadata takes precedence
when a table name appears in both registries.
:return: mapping of table name to a dict of {column name: EncryptedType}
:return:
"""
from flask_appbuilder import ( # pylint: disable=import-outside-toplevel
Model as FABModel,
)
meta_info: dict[str, Any] = {}
tables: dict[str, Any] = dict(FABModel.metadata.tables)
for table_name, table in self._db.metadata.tables.items():
tables.setdefault(table_name, table)
for table_name, table in tables.items():
for col_name, col in table.columns.items():
if isinstance(col.type, EncryptedType):
cols = meta_info.get(table_name, {})

View File

@@ -78,6 +78,15 @@ FEATURE_FLAGS = {
WEBDRIVER_BASEURL = "http://0.0.0.0:8081/"
# Enable CORS for embedded dashboard E2E tests (test app on port 9000)
ENABLE_CORS = True
CORS_OPTIONS: dict = {
"origins": [
"http://localhost:9000",
],
"supports_credentials": True,
}
def GET_FEATURE_FLAGS_FUNC(ff): # noqa: N802
ff_copy = copy(ff)
@@ -86,6 +95,7 @@ def GET_FEATURE_FLAGS_FUNC(ff): # noqa: N802
TESTING = True
TALISMAN_ENABLED = False
WTF_CSRF_ENABLED = False
FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_access"]]}

View File

@@ -89,21 +89,6 @@ class EncryptedFieldTest(SupersetTestCase):
" encrypted_field_factory"
)
def test_discover_encrypted_fields_finds_dbs_table(self):
"""
Ensure discover_encrypted_fields finds the encrypted columns on the
dbs table (password, encrypted_extra, server_cert). This guards
against db.metadata diverging from db.Model.metadata.
"""
migrator = SecretsMigrator("")
encrypted_fields = migrator.discover_encrypted_fields()
assert "dbs" in encrypted_fields, (
"dbs table not found in encrypted fields — "
"discover_encrypted_fields may be using the wrong MetaData instance"
)
dbs_cols = set(encrypted_fields["dbs"].keys())
assert {"password", "encrypted_extra", "server_cert"}.issubset(dbs_cols)
def test_lazy_key_resolution(self):
"""
Verify that the encryption key is resolved lazily at runtime,

View File

@@ -298,8 +298,7 @@ class TestOpenSqlLabWithContext:
field_path=("title",),
)
assert response.error == sanitize_for_llm_context(
"Database with ID 404 not found."
" Use list_databases to get valid database IDs.",
"Database with ID 404 not found",
field_path=("error",),
)
finally:

View File

@@ -409,50 +409,6 @@ def test_mcp_auth_hook_removes_stale_db_session_in_sync_wrapper(app) -> None:
assert result == "fresh"
def test_sync_wrapper_handles_ssl_error_on_pre_call_remove(app) -> None:
"""sync_wrapper tolerates OperationalError from db.session.remove() before the call.
If the underlying DBAPI connection died between requests (e.g. RDS SSL
idle-timeout), the rollback implicit in session.close() raises
OperationalError. _remove_session_safe() should:
- Log a warning
- Call session.invalidate() to mark the dead connection for pool discard
- Retry session.remove() so the registry is clean
- Allow the tool to run successfully
"""
from sqlalchemy.exc import OperationalError as SAOperationalError
fresh_user = _make_mock_user("fresh")
def dummy_tool() -> str:
"""Dummy sync tool."""
return g.user.username
wrapped = mcp_auth_hook(dummy_tool)
with app.test_request_context():
g.user = fresh_user
with patch("superset.extensions.db") as mock_db:
mock_db.session.remove.side_effect = [
SAOperationalError(
"SSL connection has been closed unexpectedly", None, None
),
None, # second call succeeds
]
with patch(
"superset.mcp_service.auth.get_user_from_request",
return_value=fresh_user,
):
result = wrapped()
assert result == "fresh"
assert mock_db.session.invalidate.called, "invalidate() must be called on SSL error"
assert mock_db.session.remove.call_count == 2, (
"remove() must be retried after SSL error"
)
# -- default_user_resolver --