mirror of
https://github.com/apache/superset.git
synced 2026-04-28 12:34:23 +00:00
Compare commits
1 Commits
fix/postgr
...
fix-sql-la
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b253006ecb |
@@ -0,0 +1,254 @@
|
||||
/**
|
||||
* 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 SQL Lab immediate backend persistence functionality
|
||||
*
|
||||
* This file tests the addQueryEditorWithBackendSync function that
|
||||
* immediately creates backend tabs when SqllabBackendPersistence is enabled,
|
||||
* ensuring consistent state management and preventing race conditions
|
||||
* between frontend tab creation and backend synchronization.
|
||||
*
|
||||
* Related to issue #34997: Prevents metadata update failures by ensuring
|
||||
* tabs have valid backend IDs before users can modify them.
|
||||
*/
|
||||
|
||||
import configureMockStore from 'redux-mock-store';
|
||||
import thunk from 'redux-thunk';
|
||||
import fetchMock from 'fetch-mock';
|
||||
|
||||
import { isFeatureEnabled } from '@superset-ui/core';
|
||||
import * as actions from './sqlLab';
|
||||
|
||||
// Mock feature flags
|
||||
jest.mock('@superset-ui/core', () => ({
|
||||
...jest.requireActual('@superset-ui/core'),
|
||||
isFeatureEnabled: jest.fn(),
|
||||
}));
|
||||
|
||||
const mockStore = configureMockStore([thunk]);
|
||||
|
||||
describe('SQL Lab immediate backend persistence', () => {
|
||||
beforeEach(() => {
|
||||
fetchMock.reset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchMock.reset();
|
||||
isFeatureEnabled.mockRestore();
|
||||
});
|
||||
|
||||
describe('with backend persistence enabled', () => {
|
||||
beforeEach(() => {
|
||||
isFeatureEnabled.mockImplementation(
|
||||
feature => feature === 'SQLLAB_BACKEND_PERSISTENCE',
|
||||
);
|
||||
});
|
||||
|
||||
test('creates backend tab immediately and returns tabViewId', async () => {
|
||||
const store = mockStore({});
|
||||
const queryEditor = { name: 'Test Tab', sql: 'SELECT 1' };
|
||||
|
||||
// Mock successful backend creation
|
||||
fetchMock.post('glob:*/tabstateview/', { id: 42 });
|
||||
|
||||
const result = await store.dispatch(
|
||||
actions.addQueryEditorWithBackendSync(queryEditor),
|
||||
);
|
||||
|
||||
// Verify backend call was made
|
||||
const postCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'POST',
|
||||
);
|
||||
expect(postCalls).toHaveLength(1);
|
||||
|
||||
// Verify action was dispatched with backend ID
|
||||
const addActions = store
|
||||
.getActions()
|
||||
.filter(action => action.type === 'ADD_QUERY_EDITOR');
|
||||
expect(addActions).toHaveLength(1);
|
||||
expect(addActions[0].queryEditor.tabViewId).toBe('42');
|
||||
expect(addActions[0].queryEditor.inLocalStorage).toBe(false);
|
||||
expect(addActions[0].queryEditor.loaded).toBe(true);
|
||||
|
||||
// Verify returned query editor
|
||||
expect(result.tabViewId).toBe('42');
|
||||
expect(result.inLocalStorage).toBe(false);
|
||||
});
|
||||
|
||||
test('falls back to local storage on backend failure', async () => {
|
||||
const store = mockStore({});
|
||||
const queryEditor = { name: 'Test Tab', sql: 'SELECT 1' };
|
||||
|
||||
// Mock backend failure
|
||||
fetchMock.post('glob:*/tabstateview/', { status: 500 });
|
||||
|
||||
const result = await store.dispatch(
|
||||
actions.addQueryEditorWithBackendSync(queryEditor),
|
||||
);
|
||||
|
||||
// Verify fallback action was dispatched
|
||||
const actionsDispatched = store.getActions();
|
||||
const addActions = actionsDispatched.filter(
|
||||
action => action.type === 'ADD_QUERY_EDITOR',
|
||||
);
|
||||
const toastActions = actionsDispatched.filter(
|
||||
action => action.type === 'ADD_TOAST',
|
||||
);
|
||||
|
||||
expect(addActions).toHaveLength(1);
|
||||
expect(addActions[0].queryEditor.tabViewId).toBeUndefined();
|
||||
expect(addActions[0].queryEditor.inLocalStorage).toBe(true);
|
||||
|
||||
// Should have error toast
|
||||
expect(toastActions).toHaveLength(1);
|
||||
|
||||
// Verify returned query editor (fallback)
|
||||
expect(result.tabViewId).toBeUndefined();
|
||||
expect(result.inLocalStorage).toBe(true);
|
||||
});
|
||||
|
||||
test('SQL updates work immediately after tab creation', async () => {
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock backend tab creation
|
||||
fetchMock.post('glob:*/tabstateview/', { id: 123 });
|
||||
// Mock SQL update
|
||||
fetchMock.put('glob:*/tabstateview/123', { status: 200 });
|
||||
|
||||
// Create tab with backend sync
|
||||
const queryEditor = await store.dispatch(
|
||||
actions.addQueryEditorWithBackendSync({
|
||||
name: 'Test',
|
||||
sql: 'SELECT 1',
|
||||
}),
|
||||
);
|
||||
|
||||
// Clear previous actions
|
||||
store.clearActions();
|
||||
|
||||
// Update state for the SQL update call
|
||||
const updatedStore = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [queryEditor],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Now SQL updates should work immediately
|
||||
await updatedStore.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(queryEditor, 'SELECT 2'),
|
||||
);
|
||||
|
||||
// Verify PUT call was made with correct integer ID
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'PUT',
|
||||
);
|
||||
expect(putCalls).toHaveLength(1);
|
||||
expect(putCalls[0][0]).toContain('/tabstateview/123');
|
||||
});
|
||||
});
|
||||
|
||||
describe('with backend persistence disabled', () => {
|
||||
beforeEach(() => {
|
||||
isFeatureEnabled.mockImplementation(() => false);
|
||||
});
|
||||
|
||||
test('creates local tab without backend call', async () => {
|
||||
const store = mockStore({});
|
||||
const queryEditor = { name: 'Test Tab', sql: 'SELECT 1' };
|
||||
|
||||
const result = await store.dispatch(
|
||||
actions.addQueryEditorWithBackendSync(queryEditor),
|
||||
);
|
||||
|
||||
// Verify no backend calls were made
|
||||
const calls = fetchMock.calls();
|
||||
expect(calls).toHaveLength(0);
|
||||
|
||||
// Verify local action was dispatched
|
||||
const addActions = store
|
||||
.getActions()
|
||||
.filter(action => action.type === 'ADD_QUERY_EDITOR');
|
||||
expect(addActions).toHaveLength(1);
|
||||
expect(addActions[0].queryEditor.tabViewId).toBeUndefined();
|
||||
expect(addActions[0].queryEditor.inLocalStorage).toBe(true);
|
||||
|
||||
// Verify returned query editor
|
||||
expect(result.tabViewId).toBeUndefined();
|
||||
expect(result.inLocalStorage).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('integration with addNewQueryEditor', () => {
|
||||
test('addNewQueryEditor uses backend sync', async () => {
|
||||
isFeatureEnabled.mockImplementation(
|
||||
feature => feature === 'SQLLAB_BACKEND_PERSISTENCE',
|
||||
);
|
||||
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [],
|
||||
unsavedQueryEditor: {},
|
||||
tabHistory: [],
|
||||
databases: {},
|
||||
},
|
||||
common: {
|
||||
conf: {
|
||||
SQLLAB_DEFAULT_DBID: 1,
|
||||
DEFAULT_SQLLAB_LIMIT: 1000,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock backend creation
|
||||
fetchMock.post('glob:*/tabstateview/', { id: 999 });
|
||||
|
||||
await store.dispatch(actions.addNewQueryEditor());
|
||||
|
||||
// Verify backend call was made
|
||||
const postCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'POST',
|
||||
);
|
||||
expect(postCalls).toHaveLength(1);
|
||||
|
||||
// Verify resulting tab has backend ID
|
||||
const addActions = store
|
||||
.getActions()
|
||||
.filter(action => action.type === 'ADD_QUERY_EDITOR');
|
||||
expect(addActions).toHaveLength(1);
|
||||
expect(addActions[0].queryEditor.tabViewId).toBe('999');
|
||||
expect(addActions[0].queryEditor.inLocalStorage).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,305 @@
|
||||
/**
|
||||
* 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 SQL Lab tab state management and validation
|
||||
*
|
||||
* This file validates the behavior of SQL Lab tab state management,
|
||||
* particularly the interaction between frontend-generated IDs and backend
|
||||
* persistence. Tests edge cases and race conditions that can occur when
|
||||
* the SqllabBackendPersistence feature flag is enabled.
|
||||
*
|
||||
* Related to issue #34997: Ensures metadata updates work correctly
|
||||
* when users modify queries in newly created tabs.
|
||||
*/
|
||||
|
||||
import configureMockStore from 'redux-mock-store';
|
||||
import thunk from 'redux-thunk';
|
||||
import fetchMock from 'fetch-mock';
|
||||
|
||||
import { isFeatureEnabled } from '@superset-ui/core';
|
||||
import * as actions from './sqlLab';
|
||||
import { defaultQueryEditor } from '../fixtures';
|
||||
|
||||
// Mock feature flags
|
||||
jest.mock('@superset-ui/core', () => ({
|
||||
...jest.requireActual('@superset-ui/core'),
|
||||
isFeatureEnabled: jest.fn(),
|
||||
}));
|
||||
|
||||
const mockStore = configureMockStore([thunk]);
|
||||
|
||||
describe('SQL Lab tab state validation', () => {
|
||||
const updateTabStateEndpoint = 'glob:*/tabstateview/*';
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock.reset();
|
||||
// Enable backend persistence for these tests
|
||||
isFeatureEnabled.mockImplementation(
|
||||
feature => feature === 'SQLLAB_BACKEND_PERSISTENCE',
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchMock.reset();
|
||||
isFeatureEnabled.mockRestore();
|
||||
});
|
||||
|
||||
describe('New tab creation with string IDs', () => {
|
||||
test('addQueryEditor creates tabs with string nanoid IDs', () => {
|
||||
const queryEditor = { name: 'Test Tab', sql: 'SELECT 1' };
|
||||
const action = actions.addQueryEditor(queryEditor);
|
||||
|
||||
// Verify string ID was generated
|
||||
expect(typeof action.queryEditor.id).toBe('string');
|
||||
expect(action.queryEditor.id).toMatch(/^[a-zA-Z0-9_-]{11}$/);
|
||||
expect(action.queryEditor.inLocalStorage).toBe(true);
|
||||
expect(action.queryEditor.tabViewId).toBeUndefined();
|
||||
});
|
||||
|
||||
test('multiple new tabs get unique string IDs', () => {
|
||||
const editor1 = actions.addQueryEditor({ name: 'Tab 1' });
|
||||
const editor2 = actions.addQueryEditor({ name: 'Tab 2' });
|
||||
const editor3 = actions.addQueryEditor({ name: 'Tab 3' });
|
||||
|
||||
// All should have different string IDs
|
||||
expect(editor1.queryEditor.id).not.toBe(editor2.queryEditor.id);
|
||||
expect(editor2.queryEditor.id).not.toBe(editor3.queryEditor.id);
|
||||
expect(editor1.queryEditor.id).not.toBe(editor3.queryEditor.id);
|
||||
|
||||
// All should be strings
|
||||
expect(typeof editor1.queryEditor.id).toBe('string');
|
||||
expect(typeof editor2.queryEditor.id).toBe('string');
|
||||
expect(typeof editor3.queryEditor.id).toBe('string');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Backend persistence edge cases', () => {
|
||||
test('queryEditorSetAndSaveSql sends string ID to backend', async () => {
|
||||
// Create new tab with string ID (simulating real nanoid output)
|
||||
const newQueryEditor = {
|
||||
...defaultQueryEditor,
|
||||
id: 'FRRULMQgWHa', // Realistic nanoid(11) output
|
||||
inLocalStorage: true,
|
||||
// No tabViewId - simulates new tab before sync
|
||||
};
|
||||
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [newQueryEditor],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock the endpoint to track what ID is sent
|
||||
fetchMock.put(updateTabStateEndpoint, { status: 404 });
|
||||
|
||||
// This should attempt to use the string ID
|
||||
const sql = 'SELECT * FROM test';
|
||||
await store.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(newQueryEditor, sql),
|
||||
);
|
||||
|
||||
// Verify the string ID was sent to backend
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'PUT',
|
||||
);
|
||||
|
||||
expect(putCalls).toHaveLength(1);
|
||||
expect(putCalls[0][0]).toContain('/tabstateview/FRRULMQgWHa');
|
||||
});
|
||||
|
||||
test('string IDs cause backend endpoint mismatch errors', async () => {
|
||||
const newQueryEditor = {
|
||||
...defaultQueryEditor,
|
||||
id: 'abc123def456', // String ID
|
||||
inLocalStorage: true,
|
||||
};
|
||||
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [newQueryEditor],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock 404 response for string IDs (simulating real backend behavior)
|
||||
fetchMock.put('glob:*/tabstateview/abc123def456', { status: 404 });
|
||||
|
||||
// The function should handle the 404 but dispatch a danger toast
|
||||
await store.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(newQueryEditor, 'SELECT 1'),
|
||||
);
|
||||
|
||||
// Verify error handling occurred (the function catches and handles 404 silently)
|
||||
const actionsDispatched = store.getActions();
|
||||
console.log(
|
||||
'Actions dispatched:',
|
||||
actionsDispatched.map(a => a.type),
|
||||
);
|
||||
|
||||
// Check that the PUT call was attempted with string ID
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'PUT',
|
||||
);
|
||||
expect(putCalls).toHaveLength(1);
|
||||
expect(putCalls[0][0]).toContain('/abc123def456');
|
||||
});
|
||||
|
||||
test('existing tabs with tabViewId work correctly', async () => {
|
||||
// Existing tab with proper backend ID
|
||||
const existingQueryEditor = {
|
||||
...defaultQueryEditor,
|
||||
id: 'frontend-id-123',
|
||||
tabViewId: '42', // Integer ID from backend (as string)
|
||||
inLocalStorage: false,
|
||||
};
|
||||
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [existingQueryEditor],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock successful response for integer IDs
|
||||
fetchMock.put('glob:*/tabstateview/42', { status: 200 });
|
||||
|
||||
await store.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(existingQueryEditor, 'SELECT 1'),
|
||||
);
|
||||
|
||||
// Verify correct endpoint was called with integer ID
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'PUT',
|
||||
);
|
||||
|
||||
expect(putCalls).toHaveLength(1);
|
||||
expect(putCalls[0][0]).toContain('/tabstateview/42');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Race condition scenarios', () => {
|
||||
test('rapid tab creation and SQL editing triggers string ID usage', async () => {
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [],
|
||||
unsavedQueryEditor: {},
|
||||
tabHistory: [],
|
||||
databases: {},
|
||||
},
|
||||
common: {
|
||||
conf: {
|
||||
SQLLAB_DEFAULT_DBID: 1,
|
||||
DEFAULT_SQLLAB_LIMIT: 1000,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Mock endpoints for this specific test
|
||||
fetchMock.put('glob:*/tabstateview/*', { status: 404 });
|
||||
fetchMock.post('glob:*/tabstateview/', { id: 999 });
|
||||
|
||||
// Simulate rapid user actions: create tab then immediately edit SQL
|
||||
await store.dispatch(actions.addNewQueryEditor());
|
||||
const addAction = store
|
||||
.getActions()
|
||||
.find(a => a.type === 'ADD_QUERY_EDITOR');
|
||||
const newTab = addAction.queryEditor;
|
||||
|
||||
// User immediately starts typing (before EditorAutoSync runs)
|
||||
await store.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(newTab, 'SELECT NOW()'),
|
||||
);
|
||||
|
||||
// Should attempt to use string ID
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(
|
||||
call =>
|
||||
call[0].includes('/tabstateview/') && call[1].method === 'PUT',
|
||||
);
|
||||
expect(putCalls).toHaveLength(1);
|
||||
expect(putCalls[0][0]).toMatch(/\/tabstateview\/[a-zA-Z0-9_-]{11}$/);
|
||||
});
|
||||
|
||||
test('feature flag disabled should not attempt backend updates', async () => {
|
||||
// Disable backend persistence
|
||||
isFeatureEnabled.mockImplementation(() => false);
|
||||
|
||||
const newQueryEditor = {
|
||||
...defaultQueryEditor,
|
||||
id: 'string-id-123',
|
||||
inLocalStorage: true,
|
||||
};
|
||||
|
||||
const store = mockStore({
|
||||
sqlLab: {
|
||||
queryEditors: [newQueryEditor],
|
||||
unsavedQueryEditor: {},
|
||||
},
|
||||
});
|
||||
|
||||
await store.dispatch(
|
||||
actions.queryEditorSetAndSaveSql(newQueryEditor, 'SELECT 1'),
|
||||
);
|
||||
|
||||
// Should not make any backend calls
|
||||
const putCalls = fetchMock
|
||||
.calls()
|
||||
.filter(call => call[0].includes('/tabstateview/'));
|
||||
expect(putCalls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Comparison: old vs new behavior', () => {
|
||||
test('old addQueryEditor still creates local tabs', () => {
|
||||
// The original function still works for backward compatibility
|
||||
const action = actions.addQueryEditor({ name: 'New Tab' });
|
||||
|
||||
expect(action.queryEditor.tabViewId).toBeUndefined();
|
||||
expect(action.queryEditor.inLocalStorage).toBe(true);
|
||||
});
|
||||
|
||||
test('FIXED: addQueryEditorWithBackendSync creates backend tabs immediately', async () => {
|
||||
// The new function fixes the race condition
|
||||
const store = mockStore({});
|
||||
|
||||
// Mock successful tab creation
|
||||
fetchMock.post('glob:*/tabstateview/', { id: 123 });
|
||||
|
||||
const queryEditor = await store.dispatch(
|
||||
actions.addQueryEditorWithBackendSync({ name: 'New Tab' }),
|
||||
);
|
||||
|
||||
// After fix: should have tabViewId set immediately
|
||||
expect(queryEditor.tabViewId).toBe('123');
|
||||
expect(queryEditor.inLocalStorage).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user