diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index a0b03e9ccc4..7e08b45cfea 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -41,6 +41,7 @@ import { import { fDuration, extendedDayjs } from '@superset-ui/core/utils/dates'; import { SqlLabRootState } from 'src/SqlLab/types'; import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes'; +import { makeUrl } from 'src/utils/pathUtils'; import ResultSet from '../ResultSet'; import HighlightedSql from '../HighlightedSql'; import { StaticPosition, StyledTooltip } from './styles'; @@ -68,7 +69,7 @@ interface QueryTableProps { } const openQuery = (id: number) => { - const url = `/sqllab?queryId=${id}`; + const url = makeUrl(`/sqllab?queryId=${id}`); window.open(url); }; diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index 4df11f4ca00..8198bd25873 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -85,8 +85,8 @@ import { Icons } from '@superset-ui/core/components/Icons'; import { findPermission } from 'src/utils/findPermission'; import { StreamingExportModal } from 'src/components/StreamingExportModal'; import { useStreamingExport } from 'src/components/StreamingExportModal/useStreamingExport'; -import { ensureAppRoot } from 'src/utils/pathUtils'; import { useConfirmModal } from 'src/hooks/useConfirmModal'; +import { makeUrl } from 'src/utils/pathUtils'; import ExploreCtasResultsButton from '../ExploreCtasResultsButton'; import ExploreResultsButton from '../ExploreResultsButton'; import HighlightedSql from '../HighlightedSql'; @@ -314,7 +314,7 @@ const ResultSet = ({ }; const getExportCsvUrl = (clientId: string) => - ensureAppRoot(`/api/v1/sqllab/export/${clientId}/`); + makeUrl(`/api/v1/sqllab/export/${clientId}/`); const handleCloseStreamingModal = () => { cancelExport(); diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx index 30d5a824281..56be62634fe 100644 --- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx +++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx @@ -72,6 +72,7 @@ import { } from 'src/database/actions'; import Mousetrap from 'mousetrap'; import { clearDatasetCache } from 'src/utils/cachedSupersetGet'; +import { makeUrl } from 'src/utils/pathUtils'; import { DatabaseSelector } from '../../../DatabaseSelector'; import CollectionTable from '../CollectionTable'; import Fieldset from '../Fieldset'; @@ -787,7 +788,7 @@ class DatasourceEditor extends PureComponent { autorun: true, isDataset: true, }); - return `/sqllab/?${queryParams.toString()}`; + return makeUrl(`/sqllab/?${queryParams.toString()}`); } openOnSqlLab() { diff --git a/superset-frontend/src/explore/components/controls/ViewQuery.tsx b/superset-frontend/src/explore/components/controls/ViewQuery.tsx index 20ef5450e06..86f51d97a75 100644 --- a/superset-frontend/src/explore/components/controls/ViewQuery.tsx +++ b/superset-frontend/src/explore/components/controls/ViewQuery.tsx @@ -39,6 +39,7 @@ import { import { CopyToClipboard } from 'src/components'; import { RootState } from 'src/dashboard/types'; import { findPermission } from 'src/utils/findPermission'; +import { makeUrl } from 'src/utils/pathUtils'; import CodeSyntaxHighlighter, { SupportedLanguage, preloadLanguages, @@ -137,7 +138,9 @@ const ViewQuery: FC = props => { if (domEvent.metaKey || domEvent.ctrlKey) { domEvent.preventDefault(); window.open( - `/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`, + makeUrl( + `/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`, + ), '_blank', ); } else { diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx index ee713591de8..da3d432dc7d 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx @@ -33,6 +33,7 @@ import { CheckboxChangeEvent } from '@superset-ui/core/components/Checkbox/types import { useHistory } from 'react-router-dom'; import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; +import { makeUrl } from 'src/utils/pathUtils'; import Tabs from '@superset-ui/core/components/Tabs'; import { Button, @@ -1736,7 +1737,7 @@ const DatabaseModal: FunctionComponent = ({ onClick={() => { setLoading(true); fetchAndSetDB(); - redirectURL(`/sqllab?db=true`); + redirectURL(makeUrl(`/sqllab?db=true`)); }} > {t('Query data in SQL Lab')} diff --git a/superset-frontend/src/features/home/EmptyState.tsx b/superset-frontend/src/features/home/EmptyState.tsx index 34c8ac58574..35a7050fa15 100644 --- a/superset-frontend/src/features/home/EmptyState.tsx +++ b/superset-frontend/src/features/home/EmptyState.tsx @@ -24,6 +24,7 @@ import { TableTab } from 'src/views/CRUD/types'; import { t } from '@superset-ui/core'; import { styled } from '@apache-superset/core/ui'; import { navigateTo } from 'src/utils/navigationUtils'; +import { makeUrl } from 'src/utils/pathUtils'; import { WelcomeTable } from './types'; const EmptyContainer = styled.div` @@ -58,7 +59,7 @@ const REDIRECTS = { create: { [WelcomeTable.Charts]: '/chart/add', [WelcomeTable.Dashboards]: '/dashboard/new', - [WelcomeTable.SavedQueries]: '/sqllab?new=true', + [WelcomeTable.SavedQueries]: makeUrl('/sqllab?new=true'), }, viewAll: { [WelcomeTable.Charts]: '/chart/list', diff --git a/superset-frontend/src/features/home/RightMenu.tsx b/superset-frontend/src/features/home/RightMenu.tsx index 6222df3d04f..fda352a0ce3 100644 --- a/superset-frontend/src/features/home/RightMenu.tsx +++ b/superset-frontend/src/features/home/RightMenu.tsx @@ -33,7 +33,7 @@ import { TelemetryPixel, } from '@superset-ui/core/components'; import type { ItemType, MenuItem } from '@superset-ui/core/components/Menu'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot, makeUrl } from 'src/utils/pathUtils'; import { findPermission } from 'src/utils/findPermission'; import { isUserAdmin } from 'src/dashboard/util/permissionUtils'; import { @@ -201,7 +201,7 @@ const RightMenu = ({ }, { label: t('SQL query'), - url: '/sqllab?new=true', + url: makeUrl('/sqllab?new=true'), icon: , perm: 'can_sqllab', view: 'Superset', diff --git a/superset-frontend/src/pages/SavedQueryList/index.tsx b/superset-frontend/src/pages/SavedQueryList/index.tsx index a66848c46cf..9057ec34b2c 100644 --- a/superset-frontend/src/pages/SavedQueryList/index.tsx +++ b/superset-frontend/src/pages/SavedQueryList/index.tsx @@ -65,6 +65,7 @@ import copyTextToClipboard from 'src/utils/copy'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal'; import { findPermission } from 'src/utils/findPermission'; +import { makeUrl } from 'src/utils/pathUtils'; const PAGE_SIZE = 25; const PASSWORDS_NEEDED_MESSAGE = t( @@ -222,7 +223,7 @@ function SavedQueryList({ name: t('Query'), buttonStyle: 'primary', onClick: () => { - history.push('/sqllab?new=true'); + history.push(makeUrl('/sqllab?new=true')); }, }); @@ -231,7 +232,9 @@ function SavedQueryList({ // Action methods const openInSqlLab = (id: number, openInNewWindow: boolean) => { copyTextToClipboard(() => - Promise.resolve(`${window.location.origin}/sqllab?savedQueryId=${id}`), + Promise.resolve( + `${window.location.origin}${makeUrl(`/sqllab?savedQueryId=${id}`)}`, + ), ) .then(() => { addSuccessToast(t('Link Copied!')); @@ -240,9 +243,9 @@ function SavedQueryList({ addDangerToast(t('Sorry, your browser does not support copying.')); }); if (openInNewWindow) { - window.open(`/sqllab?savedQueryId=${id}`); + window.open(makeUrl(`/sqllab?savedQueryId=${id}`)); } else { - history.push(`/sqllab?savedQueryId=${id}`); + history.push(makeUrl(`/sqllab?savedQueryId=${id}`)); } }; @@ -335,7 +338,9 @@ function SavedQueryList({ row: { original: { id, label }, }, - }: any) => {label}, + }: any) => ( + {label} + ), id: 'label', }, { diff --git a/superset-frontend/src/utils/pathUtils.test.ts b/superset-frontend/src/utils/pathUtils.test.ts new file mode 100644 index 00000000000..bf24053e671 --- /dev/null +++ b/superset-frontend/src/utils/pathUtils.test.ts @@ -0,0 +1,160 @@ +/** + * 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. + */ + +afterEach(() => { + // Clean up the DOM + document.body.innerHTML = ''; + jest.resetModules(); +}); + +test('ensureAppRoot should add application root prefix to path with default root', async () => { + document.body.innerHTML = ''; + jest.resetModules(); + + const { ensureAppRoot } = await import('./pathUtils'); + expect(ensureAppRoot('/sqllab')).toBe('/sqllab'); + expect(ensureAppRoot('/api/v1/chart')).toBe('/api/v1/chart'); +}); + +test('ensureAppRoot should add application root prefix to path with custom subdirectory', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + // Import getBootstrapData first to initialize the cache + await import('./getBootstrapData'); + const { ensureAppRoot } = await import('./pathUtils'); + + expect(ensureAppRoot('/sqllab')).toBe('/superset/sqllab'); + expect(ensureAppRoot('/api/v1/chart')).toBe('/superset/api/v1/chart'); +}); + +test('ensureAppRoot should handle paths without leading slash', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { ensureAppRoot } = await import('./pathUtils'); + + expect(ensureAppRoot('sqllab')).toBe('/superset/sqllab'); + expect(ensureAppRoot('api/v1/chart')).toBe('/superset/api/v1/chart'); +}); + +test('ensureAppRoot should handle paths with query strings', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { ensureAppRoot } = await import('./pathUtils'); + + expect(ensureAppRoot('/sqllab?new=true')).toBe('/superset/sqllab?new=true'); + expect(ensureAppRoot('/api/v1/chart/export/123/')).toBe( + '/superset/api/v1/chart/export/123/', + ); +}); + +test('makeUrl should create URL with default application root', async () => { + document.body.innerHTML = ''; + jest.resetModules(); + + const { makeUrl } = await import('./pathUtils'); + expect(makeUrl('/sqllab')).toBe('/sqllab'); + expect(makeUrl('/api/v1/chart')).toBe('/api/v1/chart'); +}); + +test('makeUrl should create URL with subdirectory prefix', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { makeUrl } = await import('./pathUtils'); + + expect(makeUrl('/sqllab')).toBe('/superset/sqllab'); + expect(makeUrl('/sqllab?new=true')).toBe('/superset/sqllab?new=true'); + expect(makeUrl('/api/v1/sqllab/export/123/')).toBe( + '/superset/api/v1/sqllab/export/123/', + ); +}); + +test('makeUrl should handle paths without leading slash', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { makeUrl } = await import('./pathUtils'); + + expect(makeUrl('sqllab?queryId=123')).toBe('/superset/sqllab?queryId=123'); +}); + +test('makeUrl should work with different subdirectory paths', async () => { + const customData = { + common: { + application_root: '/my-app/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { makeUrl } = await import('./pathUtils'); + + expect(makeUrl('/sqllab')).toBe('/my-app/superset/sqllab'); + expect(makeUrl('/dashboard/list')).toBe('/my-app/superset/dashboard/list'); +}); + +test('makeUrl should handle URLs with anchors', async () => { + const customData = { + common: { + application_root: '/superset/', + }, + }; + document.body.innerHTML = `
`; + jest.resetModules(); + + await import('./getBootstrapData'); + const { makeUrl } = await import('./pathUtils'); + + expect(makeUrl('/dashboard/123#anchor')).toBe( + '/superset/dashboard/123#anchor', + ); +}); diff --git a/superset-frontend/src/utils/pathUtils.ts b/superset-frontend/src/utils/pathUtils.ts index db4b94d69b7..0e6cac429f1 100644 --- a/superset-frontend/src/utils/pathUtils.ts +++ b/superset-frontend/src/utils/pathUtils.ts @@ -26,3 +26,19 @@ import { applicationRoot } from 'src/utils/getBootstrapData'; export function ensureAppRoot(path: string): string { return `${applicationRoot()}${path.startsWith('/') ? path : `/${path}`}`; } + +/** + * Creates a URL with the proper application root prefix for subdirectory deployments. + * Use this when constructing URLs for navigation, API calls, or file downloads. + * + * @param path - The path to convert to a full URL (e.g., '/sqllab', '/api/v1/chart/123') + * @returns The path prefixed with the application root (e.g., '/superset/sqllab') + * + * @example + * // In a subdirectory deployment at /superset + * makeUrl('/sqllab?new=true') // returns '/superset/sqllab?new=true' + * makeUrl('/api/v1/chart/export/123/') // returns '/superset/api/v1/chart/export/123/' + */ +export function makeUrl(path: string): string { + return ensureAppRoot(path); +} diff --git a/superset/app.py b/superset/app.py index 54f1b79baea..cf89171a570 100644 --- a/superset/app.py +++ b/superset/app.py @@ -68,6 +68,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 diff --git a/superset/utils/urls.py b/superset/utils/urls.py index 9e672eb9441..ef92b7de376 100644 --- a/superset/utils/urls.py +++ b/superset/utils/urls.py @@ -15,7 +15,6 @@ # specific language governing permissions and limitations # under the License. import urllib -from contextlib import nullcontext from typing import Any from urllib.parse import urlparse @@ -33,13 +32,21 @@ def headless_url(path: str, user_friendly: bool = False) -> str: def get_url_path(view: str, user_friendly: bool = False, **kwargs: Any) -> str: - if has_request_context(): - request_context = nullcontext - else: - request_context = app.test_request_context + in_request_context = has_request_context() - with request_context(): - return headless_url(url_for(view, **kwargs), user_friendly=user_friendly) + # When already in a request context, Flask's url_for respects SCRIPT_NAME from + # the WSGI environment, so the prefix is already included. Only add APPLICATION_ROOT + # prefix when creating a new request context. + if in_request_context: + url = url_for(view, **kwargs) + else: + with app.test_request_context(): + url = url_for(view, **kwargs) + app_root = app.config.get("APPLICATION_ROOT", "/") + if app_root != "/" and not url.startswith(app_root): + url = app_root.rstrip("/") + url + + return headless_url(url, user_friendly=user_friendly) def modify_url_query(url: str, **kwargs: Any) -> str: diff --git a/tests/integration_tests/test_subdirectory_deployments.py b/tests/integration_tests/test_subdirectory_deployments.py new file mode 100644 index 00000000000..c7676288538 --- /dev/null +++ b/tests/integration_tests/test_subdirectory_deployments.py @@ -0,0 +1,101 @@ +# 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. +"""Tests for subdirectory deployment features.""" + +from unittest.mock import MagicMock + +from werkzeug.test import EnvironBuilder + +from superset.app import AppRootMiddleware +from tests.integration_tests.base_tests import SupersetTestCase + + +class TestSubdirectoryDeployments(SupersetTestCase): + """Test subdirectory deployment features including middleware.""" + + def setUp(self): + super().setUp() + + # AppRootMiddleware tests (core subdirectory deployment functionality) + + def test_app_root_middleware_path_handling(self): + """Test middleware correctly handles path prefixes.""" + # Create a mock WSGI app + mock_app = MagicMock() + mock_app.return_value = [b"response"] + + middleware = AppRootMiddleware(mock_app, "/superset") + + # Test with correct prefix + environ = EnvironBuilder("/superset/dashboard").get_environ() + start_response = MagicMock() + + result = list(middleware(environ, start_response)) + + # Should call the wrapped app + mock_app.assert_called_once() + called_environ = mock_app.call_args[0][0] + + # PATH_INFO should be stripped of prefix + assert called_environ["PATH_INFO"] == "/dashboard" + # SCRIPT_NAME should be set to the prefix + assert called_environ["SCRIPT_NAME"] == "/superset" + assert result == [b"response"] + + def test_app_root_middleware_wrong_path_returns_404(self): + """Test middleware returns 404 for incorrect paths.""" + # Create a mock WSGI app + mock_app = MagicMock() + + middleware = AppRootMiddleware(mock_app, "/superset") + + # Test with incorrect prefix + environ = EnvironBuilder("/wrong/path").get_environ() + start_response = MagicMock() + + list(middleware(environ, start_response)) + + # Should not call the wrapped app + mock_app.assert_not_called() + + # Should return 404 response + start_response.assert_called_once() + status = start_response.call_args[0][0] + assert "404" in status + + def test_app_root_middleware_root_path_handling(self): + """Test middleware handles root path correctly.""" + # Create a mock WSGI app + mock_app = MagicMock() + mock_app.return_value = [b"response"] + + middleware = AppRootMiddleware(mock_app, "/superset") + + # Test with exact prefix path + environ = EnvironBuilder("/superset").get_environ() + start_response = MagicMock() + + list(middleware(environ, start_response)) + + # Should call the wrapped app + mock_app.assert_called_once() + called_environ = mock_app.call_args[0][0] + + # PATH_INFO should be empty + assert called_environ["PATH_INFO"] == "" + # SCRIPT_NAME should be set to the prefix + assert called_environ["SCRIPT_NAME"] == "/superset"