You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2025/12/19 22:59:43 UTC
(superset) 09/10: fix(streaming-export): improve URL prefix guard and flatten tests
This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch fix-app-prefix-export
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bbb4aaf9217d97bf5de4e573de29a34c5d60d18e
Author: Joe Li <jo...@preset.io>
AuthorDate: Fri Dec 19 10:45:41 2025 -0800
fix(streaming-export): improve URL prefix guard and flatten tests
Fixes:
- ensureUrlPrefix now correctly handles sibling paths (/app2 when appRoot is /app)
by checking url === appRoot || url.startsWith(`${appRoot}/`)
- Add early return for protocol-relative URLs (//example.com) to prevent
incorrect prefixing
Tests:
- Add tests for sibling path and protocol-relative URL edge cases
- Flatten nested describe() blocks to top-level test() per project guidelines
- Add missing jest.mock('./pathUtils') declaration in export.test.ts
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <no...@anthropic.com>
---
.../useStreamingExport.test.ts | 287 ++++++++++++---------
.../StreamingExportModal/useStreamingExport.ts | 7 +-
.../src/explore/exploreUtils/exportChart.test.ts | 251 +++++++++---------
superset-frontend/src/utils/export.test.ts | 93 ++++---
4 files changed, 345 insertions(+), 293 deletions(-)
diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts
index 5f2bbb41f5..3d4b631607 100644
--- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts
+++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts
@@ -218,172 +218,213 @@ test('sets ERROR status and calls onError when fetch rejects', async () => {
});
// URL prefix guard tests - prevent regression of missing app root prefix
-describe('URL prefix guard', () => {
- const { applicationRoot } = jest.requireMock('src/utils/getBootstrapData');
- const { makeUrl } = jest.requireMock('src/utils/pathUtils');
-
- const createMockFetch = () =>
- jest.fn().mockResolvedValue({
- ok: true,
- headers: new Headers({
- 'Content-Disposition': 'attachment; filename="export.csv"',
+const { applicationRoot } = jest.requireMock('src/utils/getBootstrapData');
+const { makeUrl } = jest.requireMock('src/utils/pathUtils');
+
+const createPrefixTestMockFetch = () =>
+ jest.fn().mockResolvedValue({
+ ok: true,
+ headers: new Headers({
+ 'Content-Disposition': 'attachment; filename="export.csv"',
+ }),
+ body: {
+ getReader: () => ({
+ read: jest.fn().mockResolvedValue({ done: true, value: undefined }),
}),
- body: {
- getReader: () => ({
- read: jest.fn().mockResolvedValue({ done: true, value: undefined }),
- }),
- },
+ },
+ });
+
+test('URL prefix guard applies prefix to unprefixed relative URL when app root is configured', async () => {
+ const appRoot = '/superset';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
+
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
+
+ const { result } = renderHook(() => useStreamingExport());
+
+ act(() => {
+ result.current.startExport({
+ url: '/api/v1/sqllab/export_streaming/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- beforeEach(() => {
- applicationRoot.mockReturnValue('');
- makeUrl.mockImplementation((path: string) => path);
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
- test('applies prefix to unprefixed relative URL when app root is configured', async () => {
- const appRoot = '/superset';
- applicationRoot.mockReturnValue(appRoot);
- makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
+ expect(mockFetch).toHaveBeenCalledWith(
+ '/superset/api/v1/sqllab/export_streaming/',
+ expect.any(Object),
+ );
+});
- const mockFetch = createMockFetch();
- global.fetch = mockFetch;
+test('URL prefix guard does not double-prefix URL that already has app root', async () => {
+ const appRoot = '/superset';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
- const { result } = renderHook(() => useStreamingExport());
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
- act(() => {
- result.current.startExport({
- url: '/api/v1/sqllab/export_streaming/',
- payload: { client_id: 'test-id' },
- exportType: 'csv',
- });
- });
+ const { result } = renderHook(() => useStreamingExport());
- await waitFor(() => {
- expect(mockFetch).toHaveBeenCalledTimes(1);
+ act(() => {
+ result.current.startExport({
+ url: '/superset/api/v1/sqllab/export_streaming/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- // Guard should have applied the prefix
- expect(mockFetch).toHaveBeenCalledWith(
- '/superset/api/v1/sqllab/export_streaming/',
- expect.any(Object),
- );
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
- test('does not double-prefix URL that already has app root', async () => {
- const appRoot = '/superset';
- applicationRoot.mockReturnValue(appRoot);
- makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
+ expect(mockFetch).toHaveBeenCalledWith(
+ '/superset/api/v1/sqllab/export_streaming/',
+ expect.any(Object),
+ );
+});
- const mockFetch = createMockFetch();
- global.fetch = mockFetch;
+test('URL prefix guard leaves URL unchanged when no app root is configured', async () => {
+ applicationRoot.mockReturnValue('');
- const { result } = renderHook(() => useStreamingExport());
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
- // URL already has prefix (caller did the right thing)
- act(() => {
- result.current.startExport({
- url: '/superset/api/v1/sqllab/export_streaming/',
- payload: { client_id: 'test-id' },
- exportType: 'csv',
- });
- });
+ const { result } = renderHook(() => useStreamingExport());
- await waitFor(() => {
- expect(mockFetch).toHaveBeenCalledTimes(1);
+ act(() => {
+ result.current.startExport({
+ url: '/api/v1/sqllab/export_streaming/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- // Should NOT double-prefix
- expect(mockFetch).toHaveBeenCalledWith(
- '/superset/api/v1/sqllab/export_streaming/',
- expect.any(Object),
- );
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
- test('leaves URL unchanged when no app root is configured', async () => {
- applicationRoot.mockReturnValue('');
+ expect(mockFetch).toHaveBeenCalledWith(
+ '/api/v1/sqllab/export_streaming/',
+ expect.any(Object),
+ );
+});
- const mockFetch = createMockFetch();
- global.fetch = mockFetch;
+test('URL prefix guard leaves relative URL without leading slash unchanged', async () => {
+ const appRoot = '/superset';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
- const { result } = renderHook(() => useStreamingExport());
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
- act(() => {
- result.current.startExport({
- url: '/api/v1/sqllab/export_streaming/',
- payload: { client_id: 'test-id' },
- exportType: 'csv',
- });
- });
+ const { result } = renderHook(() => useStreamingExport());
- await waitFor(() => {
- expect(mockFetch).toHaveBeenCalledTimes(1);
+ act(() => {
+ result.current.startExport({
+ url: 'api/v1/sqllab/export_streaming/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- // No prefix should be applied
- expect(mockFetch).toHaveBeenCalledWith(
- '/api/v1/sqllab/export_streaming/',
- expect.any(Object),
- );
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
- test('leaves relative URL without leading slash unchanged (caller responsibility)', async () => {
- // URLs without leading slash are treated as non-relative and passed through.
- // Callers should always use absolute paths starting with /
- const appRoot = '/superset';
- applicationRoot.mockReturnValue(appRoot);
- makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
+ expect(mockFetch).toHaveBeenCalledWith(
+ 'api/v1/sqllab/export_streaming/',
+ expect.any(Object),
+ );
+});
- const mockFetch = createMockFetch();
- global.fetch = mockFetch;
+test('URL prefix guard leaves absolute URLs (https) unchanged', async () => {
+ const appRoot = '/superset';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
- const { result } = renderHook(() => useStreamingExport());
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
- act(() => {
- result.current.startExport({
- url: 'api/v1/sqllab/export_streaming/',
- payload: { client_id: 'test-id' },
- exportType: 'csv',
- });
- });
+ const { result } = renderHook(() => useStreamingExport());
- await waitFor(() => {
- expect(mockFetch).toHaveBeenCalledTimes(1);
+ act(() => {
+ result.current.startExport({
+ url: 'https://external.example.com/api/export/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- // URL without leading slash is passed through unchanged - guard only handles /paths
- expect(mockFetch).toHaveBeenCalledWith(
- 'api/v1/sqllab/export_streaming/',
- expect.any(Object),
- );
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
- test('leaves absolute URLs (http/https) unchanged', async () => {
- // Absolute URLs should pass through without modification
- const appRoot = '/superset';
- applicationRoot.mockReturnValue(appRoot);
- makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
+ expect(mockFetch).toHaveBeenCalledWith(
+ 'https://external.example.com/api/export/',
+ expect.any(Object),
+ );
+});
+
+test('URL prefix guard leaves protocol-relative URLs (//host) unchanged', async () => {
+ const appRoot = '/superset';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
- const mockFetch = createMockFetch();
- global.fetch = mockFetch;
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
- const { result } = renderHook(() => useStreamingExport());
+ const { result } = renderHook(() => useStreamingExport());
- act(() => {
- result.current.startExport({
- url: 'https://external.example.com/api/export/',
- payload: { client_id: 'test-id' },
- exportType: 'csv',
- });
+ act(() => {
+ result.current.startExport({
+ url: '//external.example.com/api/export/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
+
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
+ });
+
+ expect(mockFetch).toHaveBeenCalledWith(
+ '//external.example.com/api/export/',
+ expect.any(Object),
+ );
+});
+
+test('URL prefix guard correctly handles sibling paths (prefixes /app2 when appRoot is /app)', async () => {
+ const appRoot = '/app';
+ applicationRoot.mockReturnValue(appRoot);
+ makeUrl.mockImplementation((path: string) => `${appRoot}${path}`);
- await waitFor(() => {
- expect(mockFetch).toHaveBeenCalledTimes(1);
+ const mockFetch = createPrefixTestMockFetch();
+ global.fetch = mockFetch;
+
+ const { result } = renderHook(() => useStreamingExport());
+
+ act(() => {
+ result.current.startExport({
+ url: '/app2/api/v1/export/',
+ payload: { client_id: 'test-id' },
+ exportType: 'csv',
});
+ });
- // Absolute URLs should not be modified
- expect(mockFetch).toHaveBeenCalledWith(
- 'https://external.example.com/api/export/',
- expect.any(Object),
- );
+ await waitFor(() => {
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});
+
+ // /app2 should be prefixed because it's not under /app/ - it's a sibling path
+ expect(mockFetch).toHaveBeenCalledWith(
+ '/app/app2/api/v1/export/',
+ expect.any(Object),
+ );
});
diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts
index 945827d371..9d2f3b798c 100644
--- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts
+++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts
@@ -48,6 +48,10 @@ const NEWLINE_BYTE = 10; // '\n' character code
*/
const ensureUrlPrefix = (url: string): string => {
const appRoot = applicationRoot();
+ // Protocol-relative URLs (//example.com/...) should pass through unchanged
+ if (url.startsWith('//')) {
+ return url;
+ }
// Only process relative URLs (starting with /)
if (!url.startsWith('/')) {
return url;
@@ -57,7 +61,8 @@ const ensureUrlPrefix = (url: string): string => {
return url;
}
// If URL already has the app root prefix, return as-is
- if (url.startsWith(appRoot)) {
+ // Use strict check to avoid false positives with sibling paths (e.g., /app2 when appRoot is /app)
+ if (url === appRoot || url.startsWith(`${appRoot}/`)) {
return url;
}
// Apply prefix via makeUrl
diff --git a/superset-frontend/src/explore/exploreUtils/exportChart.test.ts b/superset-frontend/src/explore/exploreUtils/exportChart.test.ts
index ae6de113a1..661e1248b3 100644
--- a/superset-frontend/src/explore/exploreUtils/exportChart.test.ts
+++ b/superset-frontend/src/explore/exploreUtils/exportChart.test.ts
@@ -40,143 +40,134 @@ jest.mock('@superset-ui/core', () => ({
}));
const { ensureAppRoot } = jest.requireMock('src/utils/pathUtils');
+const { getChartMetadataRegistry } = jest.requireMock('@superset-ui/core');
+
+// Minimal formData that won't trigger legacy API (useLegacyApi = false)
+const baseFormData = {
+ datasource: '1__table',
+ viz_type: 'table',
+};
+
+beforeEach(() => {
+ jest.clearAllMocks();
+ // Default: no prefix
+ ensureAppRoot.mockImplementation((path: string) => path);
+ // Default: v1 API (not legacy)
+ getChartMetadataRegistry.mockReturnValue({
+ get: jest.fn().mockReturnValue({ parseMethod: 'json' }),
+ });
+});
+
+// Tests for exportChart URL prefix handling in streaming export
+test('exportChart v1 API passes prefixed URL to onStartStreamingExport when app root is configured', async () => {
+ const appRoot = '/superset';
+ ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
+
+ const onStartStreamingExport = jest.fn();
-describe('exportChart URL prefix for streaming export', () => {
- beforeEach(() => {
- jest.clearAllMocks();
- // Default: no prefix
- ensureAppRoot.mockImplementation((path: string) => path);
+ await exportChart({
+ formData: baseFormData,
+ resultFormat: 'csv',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
});
- // Minimal formData that won't trigger legacy API (useLegacyApi = false)
- const baseFormData = {
- datasource: '1__table',
- viz_type: 'table',
- };
+ expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
+ const callArgs = onStartStreamingExport.mock.calls[0][0];
+ expect(callArgs.url).toBe('/superset/api/v1/chart/data');
+ expect(callArgs.exportType).toBe('csv');
+});
+
+test('exportChart v1 API passes unprefixed URL when no app root is configured', async () => {
+ ensureAppRoot.mockImplementation((path: string) => path);
- describe('v1 API endpoint', () => {
- test('passes prefixed URL to onStartStreamingExport when app root is configured', async () => {
- const appRoot = '/superset';
- ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
-
- const onStartStreamingExport = jest.fn();
-
- await exportChart({
- formData: baseFormData,
- resultFormat: 'csv',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
- const callArgs = onStartStreamingExport.mock.calls[0][0];
- expect(callArgs.url).toBe('/superset/api/v1/chart/data');
- expect(callArgs.exportType).toBe('csv');
- });
-
- test('passes unprefixed URL when no app root is configured', async () => {
- ensureAppRoot.mockImplementation((path: string) => path);
-
- const onStartStreamingExport = jest.fn();
-
- await exportChart({
- formData: baseFormData,
- resultFormat: 'csv',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
- const callArgs = onStartStreamingExport.mock.calls[0][0];
- expect(callArgs.url).toBe('/api/v1/chart/data');
- });
-
- test('passes nested prefix for deeply nested deployments', async () => {
- const appRoot = '/my-company/analytics/superset';
- ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
-
- const onStartStreamingExport = jest.fn();
-
- await exportChart({
- formData: baseFormData,
- resultFormat: 'xlsx',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
- const callArgs = onStartStreamingExport.mock.calls[0][0];
- expect(callArgs.url).toBe(
- '/my-company/analytics/superset/api/v1/chart/data',
- );
- expect(callArgs.exportType).toBe('xlsx');
- });
+ const onStartStreamingExport = jest.fn();
+
+ await exportChart({
+ formData: baseFormData,
+ resultFormat: 'csv',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
});
- describe('exportType passthrough', () => {
- test('passes csv exportType for CSV exports', async () => {
- const onStartStreamingExport = jest.fn();
-
- await exportChart({
- formData: baseFormData,
- resultFormat: 'csv',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledWith(
- expect.objectContaining({
- exportType: 'csv',
- }),
- );
- });
-
- test('passes xlsx exportType for Excel exports', async () => {
- const onStartStreamingExport = jest.fn();
-
- await exportChart({
- formData: baseFormData,
- resultFormat: 'xlsx',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledWith(
- expect.objectContaining({
- exportType: 'xlsx',
- }),
- );
- });
+ expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
+ const callArgs = onStartStreamingExport.mock.calls[0][0];
+ expect(callArgs.url).toBe('/api/v1/chart/data');
+});
+
+test('exportChart v1 API passes nested prefix for deeply nested deployments', async () => {
+ const appRoot = '/my-company/analytics/superset';
+ ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
+
+ const onStartStreamingExport = jest.fn();
+
+ await exportChart({
+ formData: baseFormData,
+ resultFormat: 'xlsx',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
});
- describe('legacy API endpoint (useLegacyApi=true)', () => {
- // Legacy API uses getExploreUrl() -> getURIDirectory() -> ensureAppRoot()
- // This test ensures the legacy path also correctly prefixes URLs
- const { getChartMetadataRegistry } = jest.requireMock('@superset-ui/core');
-
- test('passes prefixed URL for legacy viz type with app root configured', async () => {
- const appRoot = '/superset';
- ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
-
- // Configure mock to return useLegacyApi: true
- getChartMetadataRegistry.mockReturnValue({
- get: jest
- .fn()
- .mockReturnValue({ useLegacyApi: true, parseMethod: 'json' }),
- });
-
- const onStartStreamingExport = jest.fn();
- const legacyFormData = {
- datasource: '1__table',
- viz_type: 'legacy_viz',
- };
-
- await exportChart({
- formData: legacyFormData,
- resultFormat: 'csv',
- onStartStreamingExport: onStartStreamingExport as unknown as null,
- });
-
- expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
- const callArgs = onStartStreamingExport.mock.calls[0][0];
- // Legacy path uses getURIDirectory which calls ensureAppRoot
- expect(callArgs.url).toContain(appRoot);
- expect(callArgs.exportType).toBe('csv');
- });
+ expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
+ const callArgs = onStartStreamingExport.mock.calls[0][0];
+ expect(callArgs.url).toBe('/my-company/analytics/superset/api/v1/chart/data');
+ expect(callArgs.exportType).toBe('xlsx');
+});
+
+test('exportChart passes csv exportType for CSV exports', async () => {
+ const onStartStreamingExport = jest.fn();
+
+ await exportChart({
+ formData: baseFormData,
+ resultFormat: 'csv',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
});
+
+ expect(onStartStreamingExport).toHaveBeenCalledWith(
+ expect.objectContaining({
+ exportType: 'csv',
+ }),
+ );
+});
+
+test('exportChart passes xlsx exportType for Excel exports', async () => {
+ const onStartStreamingExport = jest.fn();
+
+ await exportChart({
+ formData: baseFormData,
+ resultFormat: 'xlsx',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
+ });
+
+ expect(onStartStreamingExport).toHaveBeenCalledWith(
+ expect.objectContaining({
+ exportType: 'xlsx',
+ }),
+ );
+});
+
+test('exportChart legacy API (useLegacyApi=true) passes prefixed URL with app root configured', async () => {
+ // Legacy API uses getExploreUrl() -> getURIDirectory() -> ensureAppRoot()
+ const appRoot = '/superset';
+ ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`);
+
+ // Configure mock to return useLegacyApi: true
+ getChartMetadataRegistry.mockReturnValue({
+ get: jest.fn().mockReturnValue({ useLegacyApi: true, parseMethod: 'json' }),
+ });
+
+ const onStartStreamingExport = jest.fn();
+ const legacyFormData = {
+ datasource: '1__table',
+ viz_type: 'legacy_viz',
+ };
+
+ await exportChart({
+ formData: legacyFormData,
+ resultFormat: 'csv',
+ onStartStreamingExport: onStartStreamingExport as unknown as null,
+ });
+
+ expect(onStartStreamingExport).toHaveBeenCalledTimes(1);
+ const callArgs = onStartStreamingExport.mock.calls[0][0];
+ // Legacy path uses getURIDirectory which calls ensureAppRoot
+ expect(callArgs.url).toContain(appRoot);
+ expect(callArgs.exportType).toBe('csv');
});
diff --git a/superset-frontend/src/utils/export.test.ts b/superset-frontend/src/utils/export.test.ts
index 94ccf9a9ab..4c07c4cbf9 100644
--- a/superset-frontend/src/utils/export.test.ts
+++ b/superset-frontend/src/utils/export.test.ts
@@ -33,6 +33,11 @@ jest.mock('@superset-ui/core', () => ({
jest.mock('content-disposition');
+// Mock pathUtils for double-prefix prevention tests
+jest.mock('./pathUtils', () => ({
+ ensureAppRoot: jest.fn((path: string) => path),
+}));
+
let mockBlob: Blob;
let mockResponse: Response;
let createElementSpy: jest.SpyInstance;
@@ -393,42 +398,52 @@ test('handles export with empty IDs array', async () => {
);
});
-// Test to prevent double-prefix bug: SupersetClient.getUrl() already adds
-// the appRoot prefix, so handleResourceExport should NOT pre-apply it.
-// If we use ensureAppRoot() in export.ts, SupersetClient.getUrl() adds the
-// prefix again, resulting in double-prefixed URLs like /app/app/api/v1/...
-describe('double-prefix prevention', () => {
- // Import the mocked ensureAppRoot to control its behavior per test
- const { ensureAppRoot } = jest.requireMock('./pathUtils');
-
- const prefixTestCases = [
- { name: 'subdirectory prefix', appRoot: '/superset', resource: 'dashboard', ids: [1] },
- { name: 'subdirectory prefix (dataset)', appRoot: '/superset', resource: 'dataset', ids: [1] },
- { name: 'nested prefix', appRoot: '/my-app/superset', resource: 'dataset', ids: [1, 2] },
- ];
-
- test.each(prefixTestCases)(
- 'endpoint should not include app prefix: $name',
- async ({ appRoot, resource, ids }) => {
- // Simulate real ensureAppRoot behavior: prepend the appRoot
- (ensureAppRoot as jest.Mock).mockImplementation(
- (path: string) => `${appRoot}${path}`,
- );
-
- const doneMock = jest.fn();
- await handleResourceExport(resource, ids, doneMock);
-
- // The endpoint passed to SupersetClient.get should NOT have the appRoot prefix
- // because SupersetClient.getUrl() adds it when building the full URL.
- const expectedEndpoint = `/api/v1/${resource}/export/?q=!(${ids.join(',')})`;
-
- // Explicitly verify no prefix in endpoint - this will fail if ensureAppRoot is used
- const callArgs = (SupersetClient.get as jest.Mock).mock.calls.slice(-1)[0][0];
- expect(callArgs.endpoint).not.toContain(appRoot);
- expect(callArgs.endpoint).toBe(expectedEndpoint);
-
- // Reset mock for next test
- (ensureAppRoot as jest.Mock).mockImplementation((path: string) => path);
- },
- );
-});
+const { ensureAppRoot } = jest.requireMock('./pathUtils');
+
+const doublePrefixTestCases = [
+ {
+ name: 'subdirectory prefix',
+ appRoot: '/superset',
+ resource: 'dashboard',
+ ids: [1],
+ },
+ {
+ name: 'subdirectory prefix (dataset)',
+ appRoot: '/superset',
+ resource: 'dataset',
+ ids: [1],
+ },
+ {
+ name: 'nested prefix',
+ appRoot: '/my-app/superset',
+ resource: 'dataset',
+ ids: [1, 2],
+ },
+];
+
+test.each(doublePrefixTestCases)(
+ 'handleResourceExport endpoint should not include app prefix: $name',
+ async ({ appRoot, resource, ids }) => {
+ // Simulate real ensureAppRoot behavior: prepend the appRoot
+ (ensureAppRoot as jest.Mock).mockImplementation(
+ (path: string) => `${appRoot}${path}`,
+ );
+
+ const doneMock = jest.fn();
+ await handleResourceExport(resource, ids, doneMock);
+
+ // The endpoint passed to SupersetClient.get should NOT have the appRoot prefix
+ // because SupersetClient.getUrl() adds it when building the full URL.
+ const expectedEndpoint = `/api/v1/${resource}/export/?q=!(${ids.join(',')})`;
+
+ // Explicitly verify no prefix in endpoint - this will fail if ensureAppRoot is used
+ const callArgs = (SupersetClient.get as jest.Mock).mock.calls.slice(
+ -1,
+ )[0][0];
+ expect(callArgs.endpoint).not.toContain(appRoot);
+ expect(callArgs.endpoint).toBe(expectedEndpoint);
+
+ // Reset mock for next test
+ (ensureAppRoot as jest.Mock).mockImplementation((path: string) => path);
+ },
+);