iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(sqllab): nonblocking new query editor (#28795)

This commit is contained in:
JUST.in DO IT 2024-06-04 18:56:50 -07:00 committed by GitHub
parent 1a52c6a3b8
commit 8a8ce16a1f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 212 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
superset-frontend/src/SqlLab/actions/sqlLab.js
View File

@ -467,12 +467,24 @@ function migrateQuery(queryId, queryEditorId, dispatch) {
);
}
export function migrateQueryEditorFromLocalStorage(
queryEditor,
tables,
queries,
) {
return function (dispatch) {
/**
* Persist QueryEditor from local storage to backend tab state.
* This ensures that when new tabs are created, query editors are
* asynchronously stored in local storage and periodically synchronized
* with the backend.
* When switching to persistence mode, the QueryEditors previously
* stored in local storage will also be synchronized to the backend
* through syncQueryEditor.
*/
export function syncQueryEditor(queryEditor) {
return function (dispatch, getState) {
const { tables, queries } = getState().sqlLab;
const localStorageTables = tables.filter(
table => table.inLocalStorage && table.queryEditorId === queryEditor.id,
);
const localStorageQueries = Object.values(queries).filter(
query => query.inLocalStorage && query.sqlEditorId === queryEditor.id,
);
return SupersetClient.post({
endpoint: '/tabstateview/',
postPayload: { queryEditor },
@ -482,6 +494,7 @@ export function migrateQueryEditorFromLocalStorage(
...queryEditor,
id: json.id.toString(),
inLocalStorage: false,
loaded: true,
};
dispatch({
type: MIGRATE_QUERY_EDITOR,
@ -494,10 +507,10 @@ export function migrateQueryEditorFromLocalStorage(
newId: newQueryEditor.id,
});
return Promise.all([
...tables.map(table =>
...localStorageTables.map(table =>
migrateTable(table, newQueryEditor.id, dispatch),
),
...queries.map(query =>
...localStorageQueries.map(query =>
migrateQuery(query.id, newQueryEditor.id, dispatch),
),
]);
@ -516,35 +529,15 @@ export function migrateQueryEditorFromLocalStorage(
}
export function addQueryEditor(queryEditor) {
return function (dispatch) {
const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
? SupersetClient.post({
endpoint: '/tabstateview/',
postPayload: { queryEditor },
}).then(({ json }) => ({ ...json, loaded: true }))
: Promise.resolve({ id: shortid.generate() });
return sync
.then(({ id, loaded }) => {
const newQueryEditor = {
...queryEditor,
id: id.toString(),
loaded,
};
return dispatch({
type: ADD_QUERY_EDITOR,
queryEditor: newQueryEditor,
});
})
.catch(() =>
dispatch(
addDangerToast(
t(
'Unable to add a new tab to the backend. Please contact your administrator.',
),
),
),
);
const newQueryEditor = {
...queryEditor,
id: shortid.generate().toString(),
loaded: true,
inLocalStorage: true,
};
return {
type: ADD_QUERY_EDITOR,
queryEditor: newQueryEditor,
};
}

101
superset-frontend/src/SqlLab/actions/sqlLab.test.js
View File

@ -427,13 +427,15 @@ describe('async actions', () => {
maxRow: undefined,
id: 'abcd',
templateParams: undefined,
inLocalStorage: true,
loaded: true,
},
},
];
const request = actions.cloneQueryToNewTab(query, true);
return request(store.dispatch, store.getState).then(() => {
expect(store.getActions()).toEqual(expectedActions);
});
request(store.dispatch, store.getState);
expect(store.getActions()).toEqual(expectedActions);
});
});
@ -453,13 +455,16 @@ describe('async actions', () => {
const expectedActions = [
{
type: actions.ADD_QUERY_EDITOR,
queryEditor,
queryEditor: {
...queryEditor,
inLocalStorage: true,
loaded: true,
},
},
];
const request = actions.addQueryEditor(defaultQueryEditor);
return request(store.dispatch, store.getState).then(() => {
expect(store.getActions()).toEqual(expectedActions);
});
store.dispatch(actions.addQueryEditor(defaultQueryEditor));
expect(store.getActions()).toEqual(expectedActions);
});
describe('addNewQueryEditor', () => {
@ -488,13 +493,14 @@ describe('async actions', () => {
queryLimit:
defaultQueryEditor.queryLimit ||
initialState.common.conf.DEFAULT_SQLLAB_LIMIT,
inLocalStorage: true,
loaded: true,
},
},
];
const request = actions.addNewQueryEditor();
return request(store.dispatch, store.getState).then(() => {
expect(store.getActions()).toEqual(expectedActions);
});
request(store.dispatch, store.getState);
expect(store.getActions()).toEqual(expectedActions);
});
});
});
@ -534,20 +540,33 @@ describe('async actions', () => {
afterEach(fetchMock.resetHistory);
describe('addQueryEditor', () => {
it('updates the tab state in the backend', () => {
let stub;
beforeEach(() => {
stub = sinon.stub(shortid, 'generate').returns('abcd');
});
afterEach(() => {
stub.restore();
});
it('creates the tab state in the local storage', () => {
expect.assertions(2);
const store = mockStore({});
const expectedActions = [
{
type: actions.ADD_QUERY_EDITOR,
queryEditor: { ...queryEditor, id: '1', loaded: true },
queryEditor: {
...queryEditor,
id: 'abcd',
loaded: true,
inLocalStorage: true,
},
},
];
return store.dispatch(actions.addQueryEditor(queryEditor)).then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1);
});
store.dispatch(actions.addQueryEditor(queryEditor));
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
});
});
@ -965,7 +984,7 @@ describe('async actions', () => {
});
});
describe('migrateQueryEditorFromLocalStorage', () => {
describe('syncQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(3);
@ -978,16 +997,41 @@ describe('async actions', () => {
overwriteRoutes: true,
});
const oldQueryEditor = { ...queryEditor, inLocalStorage: true };
const tables = [
{ id: 'one', dataPreviewQueryId: 'previewOne' },
{ id: 'two', dataPreviewQueryId: 'previewTwo' },
{
id: 'one',
dataPreviewQueryId: 'previewOne',
queryEditorId: oldQueryEditor.id,
inLocalStorage: true,
},
{
id: 'two',
dataPreviewQueryId: 'previewTwo',
queryEditorId: oldQueryEditor.id,
inLocalStorage: true,
},
];
const queries = [
{ ...query, id: 'previewOne' },
{ ...query, id: 'previewTwo' },
{
...query,
id: 'previewOne',
sqlEditorId: oldQueryEditor.id,
inLocalStorage: true,
},
{
...query,
id: 'previewTwo',
sqlEditorId: oldQueryEditor.id,
inLocalStorage: true,
},
];
const store = mockStore({});
const oldQueryEditor = { ...queryEditor, inLocalStorage: true };
const store = mockStore({
sqlLab: {
queries,
tables,
},
});
const expectedActions = [
{
type: actions.MIGRATE_QUERY_EDITOR,
@ -997,6 +1041,7 @@ describe('async actions', () => {
...oldQueryEditor,
id: '1',
inLocalStorage: false,
loaded: true,
},
},
{
@ -1028,13 +1073,7 @@ describe('async actions', () => {
},
];
return store
.dispatch(
actions.migrateQueryEditorFromLocalStorage(
oldQueryEditor,
tables,
queries,
),
)
.dispatch(actions.syncQueryEditor(oldQueryEditor))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(3);

34
superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
View File

@ -40,7 +40,7 @@ import { render, waitFor } from 'spec/helpers/testing-library';
import ToastContainer from 'src/components/MessageToasts/ToastContainer';
import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures';
import { logging } from '@superset-ui/core';
import EditorAutoSync from '.';
import EditorAutoSync, { INTERVAL } from '.';
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
@ -78,11 +78,37 @@ test('sync the unsaved editor tab state when there are new changes since the las
sqlLab: unsavedSqlLabState,
},
});
await waitFor(() => jest.runAllTimers());
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateEditorTabState)).toHaveLength(1);
fetchMock.restore();
});
test('sync the unsaved NEW editor state when there are new in local storage', async () => {
const createEditorTabState = `glob:*/tabstateview/`;
fetchMock.post(createEditorTabState, { id: 123 });
expect(fetchMock.calls(createEditorTabState)).toHaveLength(0);
render(<EditorAutoSync />, {
useRedux: true,
initialState: {
...initialState,
sqlLab: {
...initialState.sqlLab,
queryEditors: [
...initialState.sqlLab.queryEditors,
{
id: 'rnd-new-id',
name: 'new tab name',
inLocalStorage: true,
},
],
},
},
});
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(createEditorTabState)).toHaveLength(1);
fetchMock.restore();
});
test('skip syncing the unsaved editor tab state when the updates are already synced', async () => {
const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`;
fetchMock.put(updateEditorTabState, 200);
@ -102,7 +128,7 @@ test('skip syncing the unsaved editor tab state when the updates are already syn
},
},
});
await waitFor(() => jest.runAllTimers());
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0);
fetchMock.restore();
});
@ -126,7 +152,7 @@ test('renders an error toast when the sync failed', async () => {
},
},
);
await waitFor(() => jest.runAllTimers());
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(logging.warn).toHaveBeenCalledTimes(1);
expect(logging.warn).toHaveBeenCalledWith(

48
superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
View File

@ -27,9 +27,13 @@ import {
} from 'src/SqlLab/types';
import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
import { setEditorTabLastUpdate } from 'src/SqlLab/actions/sqlLab';
import {
syncQueryEditor,
setEditorTabLastUpdate,
} from 'src/SqlLab/actions/sqlLab';
import useEffectEvent from 'src/hooks/useEffectEvent';
const INTERVAL = 5000;
export const INTERVAL = 5000;
function hasUnsavedChanges(
queryEditor: QueryEditor,
@ -73,17 +77,43 @@ const EditorAutoSync: React.FC = () => {
INTERVAL,
);
useEffect(() => {
const unsaved = filterUnsavedQueryEditorList(
const getUnsavedItems = useEffectEvent(unsavedQE =>
filterUnsavedQueryEditorList(
queryEditors,
debouncedUnsavedQueryEditor,
unsavedQE,
lastSavedTimestampRef.current,
);
),
);
const getUnsavedNewQueryEditor = useEffectEvent(() =>
filterUnsavedQueryEditorList(
queryEditors,
unsavedQueryEditor,
lastSavedTimestampRef.current,
).find(({ inLocalStorage }) => Boolean(inLocalStorage)),
);
useEffect(() => {
let timer: NodeJS.Timeout;
function saveUnsavedQueryEditor() {
const firstUnsavedQueryEditor = getUnsavedNewQueryEditor();
if (firstUnsavedQueryEditor) {
dispatch(syncQueryEditor(firstUnsavedQueryEditor));
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
return () => {
clearTimeout(timer);
};
}, [getUnsavedNewQueryEditor, dispatch]);
useEffect(() => {
const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor);
Promise.all(
unsaved
// TODO: Migrate migrateQueryEditorFromLocalStorage
// in TabbedSqlEditors logic by addSqlEditor mutation later
.filter(({ inLocalStorage }) => !inLocalStorage)
.map(queryEditor => updateSqlEditor({ queryEditor })),
).then(resolvers => {
@ -92,7 +122,7 @@ const EditorAutoSync: React.FC = () => {
dispatch(setEditorTabLastUpdate(lastSavedTimestampRef.current));
}
});
}, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]);
}, [debouncedUnsavedQueryEditor, getUnsavedItems, dispatch, updateSqlEditor]);
useEffect(() => {
if (error) {

13
superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx
View File

@ -165,8 +165,8 @@ test('should disable new tab when offline', () => {
});
expect(queryAllByLabelText('Add tab').length).toEqual(0);
});
test('should have an empty state when query editors is empty', () => {
const { getByText } = setup(undefined, {
test('should have an empty state when query editors is empty', async () => {
const { getByText, getByRole } = setup(undefined, {
...initialState,
sqlLab: {
...initialState.sqlLab,
@ -174,5 +174,12 @@ test('should have an empty state when query editors is empty', () => {
tabHistory: [],
},
});
expect(getByText('Add a new tab to create SQL Query')).toBeInTheDocument();
// Clear the new tab applied in componentDidMount and check the state of the empty tab
const removeTabButton = getByRole('button', { name: 'remove' });
fireEvent.click(removeTabButton);
await waitFor(() =>
expect(getByText('Add a new tab to create SQL Query')).toBeInTheDocument(),
);
});

27
superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
View File

@ -70,32 +70,6 @@ class TabbedSqlEditors extends React.PureComponent<TabbedSqlEditorsProps> {
}
componentDidMount() {
// migrate query editor and associated tables state to server
if (isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)) {
const localStorageTables = this.props.tables.filter(
table => table.inLocalStorage,
);
const localStorageQueries = Object.values(this.props.queries).filter(
query => query.inLocalStorage,
);
this.props.queryEditors
.filter(qe => qe.inLocalStorage)
.forEach(qe => {
// get all queries associated with the query editor
const queries = localStorageQueries.filter(
query => query.sqlEditorId === qe.id,
);
const tables = localStorageTables.filter(
table => table.queryEditorId === qe.id,
);
this.props.actions.migrateQueryEditorFromLocalStorage(
qe,
tables,
queries,
);
});
}
// merge post form data with GET search params
// Hack: this data should be coming from getInitialState
// but for some reason this data isn't being passed properly through
@ -322,7 +296,6 @@ export function mapStateToProps({ sqlLab, common }: SqlLabRootState) {
queryEditors: sqlLab.queryEditors ?? DEFAULT_PROPS.queryEditors,
queries: sqlLab.queries,
tabHistory: sqlLab.tabHistory,
tables: sqlLab.tables,
defaultDbId: common.conf.SQLLAB_DEFAULT_DBID,
displayLimit: common.conf.DISPLAY_MAX_ROW,
offline: sqlLab.offline ?? DEFAULT_PROPS.offline,

20
superset-frontend/src/SqlLab/reducers/sqlLab.js
View File

@ -442,9 +442,10 @@ export default function sqlLabReducer(state = {}, action) {
// continue regardless of error
}
// replace localStorage query editor with the server backed one
return addToArr(
removeFromArr(state, 'queryEditors', action.oldQueryEditor),
return alterInArr(
state,
'queryEditors',
action.oldQueryEditor,
action.newQueryEditor,
);
},
@ -468,20 +469,9 @@ export default function sqlLabReducer(state = {}, action) {
);
},
[actions.MIGRATE_TAB_HISTORY]() {
try {
// remove migrated tab from localStorage tabHistory
const { sqlLab } = JSON.parse(localStorage.getItem('redux'));
sqlLab.tabHistory = sqlLab.tabHistory.filter(
tabId => tabId !== action.oldId,
);
localStorage.setItem('redux', JSON.stringify({ sqlLab }));
} catch (error) {
// continue regardless of error
}
const tabHistory = state.tabHistory.filter(
tabId => tabId !== action.oldId,
const tabHistory = state.tabHistory.map(tabId =>
tabId === action.oldId ? action.newId : tabId,
);
tabHistory.push(action.newId);
return { ...state, tabHistory };
},
[actions.MIGRATE_QUERY]() {

28
superset-frontend/src/SqlLab/reducers/sqlLab.test.js
View File

@ -239,6 +239,34 @@ describe('sqlLabReducer', () => {
interceptedAction.northPercent,
);
});
it('should migrate query editor by new query editor id', () => {
const index = newState.queryEditors.findIndex(({ id }) => id === qe.id);
const newQueryEditor = {
...qe,
id: 'updatedNewId',
schema: 'updatedSchema',
};
const action = {
type: actions.MIGRATE_QUERY_EDITOR,
oldQueryEditor: qe,
newQueryEditor,
};
newState = sqlLabReducer(newState, action);
expect(newState.queryEditors[index].id).toEqual('updatedNewId');
expect(newState.queryEditors[index]).toEqual(newQueryEditor);
});
it('should migrate tab history by new query editor id', () => {
expect(newState.tabHistory).toContain(qe.id);
const action = {
type: actions.MIGRATE_TAB_HISTORY,
oldId: qe.id,
newId: 'updatedNewId',
};
newState = sqlLabReducer(newState, action);
expect(newState.tabHistory).toContain('updatedNewId');
expect(newState.tabHistory).not.toContain(qe.id);
});
});
describe('Tables', () => {
let newState;