Skip to content

Commit

Permalink
refactor(sqllab): nonblocking switch query editor (apache#29108)
Browse files Browse the repository at this point in the history
  • Loading branch information
@justinpark
justinpark authored Jun 12, 2024
1 parent a889796 commit 31afb62
Show file tree
Hide file tree
Showing 14 changed files with 188 additions and 101 deletions.
110 changes: 45 additions & 65 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const CREATE_DATASOURCE_SUCCESS = 'CREATE_DATASOURCE_SUCCESS';
export const CREATE_DATASOURCE_FAILED = 'CREATE_DATASOURCE_FAILED';

export const SET_EDITOR_TAB_LAST_UPDATE = 'SET_EDITOR_TAB_LAST_UPDATE';
export const SET_LAST_UPDATED_ACTIVE_TAB = 'SET_LAST_UPDATED_ACTIVE_TAB';

export const addInfoToast = addInfoToastAction;
export const addSuccessToast = addSuccessToastAction;
Expand Down Expand Up @@ -611,29 +612,17 @@ export function cloneQueryToNewTab(query, autorun) {
};
}

export function setActiveQueryEditor(queryEditor) {
return function (dispatch) {
const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
? SupersetClient.post({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}/activate`),
})
: Promise.resolve();
export function setLastUpdatedActiveTab(queryEditorId) {
return {
type: SET_LAST_UPDATED_ACTIVE_TAB,
queryEditorId,
};
}

return sync
.then(() => dispatch({ type: SET_ACTIVE_QUERY_EDITOR, queryEditor }))
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(
t(
'An error occurred while setting the active tab. Please contact ' +
'your administrator.',
),
),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
export function setActiveQueryEditor(queryEditor) {
return {
type: SET_ACTIVE_QUERY_EDITOR,
queryEditor,
};
}

Expand Down Expand Up @@ -674,51 +663,42 @@ export function setTables(tableSchemas) {
return { type: SET_TABLES, tables };
}

export function switchQueryEditor(queryEditor, displayLimit) {
export function fetchQueryEditor(queryEditor, displayLimit) {
return function (dispatch) {
if (
isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) &&
queryEditor &&
!queryEditor.loaded
) {
SupersetClient.get({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}`),
SupersetClient.get({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}`),
})
.then(({ json }) => {
const loadedQueryEditor = {
id: json.id.toString(),
loaded: true,
name: json.label,
sql: json.sql,
selectedText: null,
latestQueryId: json.latest_query?.id,
autorun: json.autorun,
dbId: json.database_id,
templateParams: json.template_params,
catalog: json.catalog,
schema: json.schema,
queryLimit: json.query_limit,
remoteId: json.saved_query?.id,
hideLeftBar: json.hide_left_bar,
};
dispatch(loadQueryEditor(loadedQueryEditor));
dispatch(setTables(json.table_schemas || []));
if (json.latest_query && json.latest_query.resultsKey) {
dispatch(fetchQueryResults(json.latest_query, displayLimit));
}
})
.then(({ json }) => {
const loadedQueryEditor = {
id: json.id.toString(),
loaded: true,
name: json.label,
sql: json.sql,
selectedText: null,
latestQueryId: json.latest_query?.id,
autorun: json.autorun,
dbId: json.database_id,
templateParams: json.template_params,
catalog: json.catalog,
schema: json.schema,
queryLimit: json.query_limit,
remoteId: json.saved_query?.id,
hideLeftBar: json.hide_left_bar,
};
dispatch(loadQueryEditor(loadedQueryEditor));
dispatch(setTables(json.table_schemas || []));
dispatch(setActiveQueryEditor(loadedQueryEditor));
if (json.latest_query && json.latest_query.resultsKey) {
dispatch(fetchQueryResults(json.latest_query, displayLimit));
}
})
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(t('An error occurred while fetching tab state')),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
} else {
dispatch(setActiveQueryEditor(queryEditor));
}
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(t('An error occurred while fetching tab state')),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
};
}

Expand Down
35 changes: 15 additions & 20 deletions superset-frontend/src/SqlLab/actions/sqlLab.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,21 @@ describe('async actions', () => {
});
});

it('set current query editor', () => {
expect.assertions(1);

const store = mockStore(initialState);
const expectedActions = [
{
type: actions.SET_ACTIVE_QUERY_EDITOR,
queryEditor: defaultQueryEditor,
},
];
store.dispatch(actions.setActiveQueryEditor(defaultQueryEditor));

expect(store.getActions()).toEqual(expectedActions);
});

describe('backend sync', () => {
const updateTabStateEndpoint = 'glob:*/tabstateview/*';
fetchMock.put(updateTabStateEndpoint, {});
Expand Down Expand Up @@ -593,26 +608,6 @@ describe('async actions', () => {
});
});

describe('setActiveQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(2);

const store = mockStore({});
const expectedActions = [
{
type: actions.SET_ACTIVE_QUERY_EDITOR,
queryEditor,
},
];
return store
.dispatch(actions.setActiveQueryEditor(queryEditor))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1);
});
});
});

describe('removeQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(2);
Expand Down
38 changes: 38 additions & 0 deletions superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const unsavedSqlLabState = {
},
editorTabLastUpdatedAt,
};

beforeAll(() => {
jest.useFakeTimers();
});
Expand All @@ -66,6 +67,16 @@ afterAll(() => {
jest.useRealTimers();
});

const updateActiveEditorTabState = `glob:*/tabstateview/*/activate`;

beforeEach(() => {
fetchMock.post(updateActiveEditorTabState, {});
});

afterEach(() => {
fetchMock.reset();
});

test('sync the unsaved editor tab state when there are new changes since the last update', async () => {
const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`;
fetchMock.put(updateEditorTabState, 200);
Expand Down Expand Up @@ -108,6 +119,33 @@ test('sync the unsaved NEW editor state when there are new in local storage', as
fetchMock.restore();
});

test('sync the active editor id when there are updates in tab history', async () => {
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(0);
render(<EditorAutoSync />, {
useRedux: true,
initialState: {
...initialState,
sqlLab: {
...initialState.sqlLab,
lastUpdatedActiveTab: 'old-tab-id',
queryEditors: [
...initialState.sqlLab.queryEditors,
{
id: 'rnd-new-id12',
name: 'new tab name',
inLocalStorage: false,
},
],
tabHistory: ['old-tab-id', 'rnd-new-id12'],
},
},
});
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1);
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1);
});

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);
Expand Down
39 changes: 33 additions & 6 deletions superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ import {
QueryEditor,
UnsavedQueryEditor,
} from 'src/SqlLab/types';
import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs';
import {
useUpdateCurrentSqlEditorTabMutation,
useUpdateSqlEditorTabMutation,
} from 'src/hooks/apiResources/sqlEditorTabs';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
import {
syncQueryEditor,
setEditorTabLastUpdate,
setLastUpdatedActiveTab,
} from 'src/SqlLab/actions/sqlLab';
import useEffectEvent from 'src/hooks/useEffectEvent';

Expand Down Expand Up @@ -71,7 +75,15 @@ const EditorAutoSync: FC = () => {
);
const dispatch = useDispatch();
const lastSavedTimestampRef = useRef<number>(editorTabLastUpdatedAt);

const currentQueryEditorId = useSelector<SqlLabRootState, string>(
({ sqlLab }) => sqlLab.tabHistory.slice(-1)[0] || '',
);
const lastUpdatedActiveTab = useSelector<SqlLabRootState, string>(
({ sqlLab }) => sqlLab.lastUpdatedActiveTab,
);
const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation();
const [updateCurrentSqlEditor] = useUpdateCurrentSqlEditorTabMutation();

const debouncedUnsavedQueryEditor = useDebounceValue(
unsavedQueryEditor,
Expand All @@ -94,21 +106,36 @@ const EditorAutoSync: FC = () => {
).find(({ inLocalStorage }) => Boolean(inLocalStorage)),
);

const syncCurrentQueryEditor = useEffectEvent(() => {
if (
currentQueryEditorId &&
currentQueryEditorId !== lastUpdatedActiveTab &&
!queryEditors.find(({ id }) => id === currentQueryEditorId)
?.inLocalStorage
) {
updateCurrentSqlEditor(currentQueryEditorId).then(() => {
dispatch(setLastUpdatedActiveTab(currentQueryEditorId));
});
}
});

useEffect(() => {
let timer: NodeJS.Timeout;
let saveTimer: NodeJS.Timeout;
function saveUnsavedQueryEditor() {
const firstUnsavedQueryEditor = getUnsavedNewQueryEditor();

if (firstUnsavedQueryEditor) {
dispatch(syncQueryEditor(firstUnsavedQueryEditor));
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
const syncTimer = setInterval(syncCurrentQueryEditor, INTERVAL);
saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
return () => {
clearTimeout(timer);
clearTimeout(saveTimer);
clearInterval(syncTimer);
};
}, [getUnsavedNewQueryEditor, dispatch]);
}, [getUnsavedNewQueryEditor, syncCurrentQueryEditor, dispatch]);

useEffect(() => {
const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor);
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ describe('SqlEditor', () => {
await waitFor(() =>
expect(fetchMock.calls('glob:*/tabstateview/*').length).toBe(1),
);
expect(fetchMock.calls(switchTabApi).length).toBe(1);
// it will be called from EditorAutoSync
expect(fetchMock.calls(switchTabApi).length).toBe(0);
});
});
});
4 changes: 2 additions & 2 deletions superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ import {
setActiveSouthPaneTab,
updateSavedQuery,
formatQuery,
switchQueryEditor,
fetchQueryEditor,
} from 'src/SqlLab/actions/sqlLab';
import {
STATE_TYPE_MAP,
Expand Down Expand Up @@ -506,7 +506,7 @@ const SqlEditor: FC<Props> = ({

const loadQueryEditor = useEffectEvent(() => {
if (shouldLoadQueryEditor) {
dispatch(switchQueryEditor(queryEditor, displayLimit));
dispatch(fetchQueryEditor(queryEditor, displayLimit));
}
});

Expand Down
5 changes: 1 addition & 4 deletions superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,7 @@ class TabbedSqlEditors extends PureComponent<TabbedSqlEditorsProps> {
if (!queryEditor) {
return;
}
this.props.actions.switchQueryEditor(
queryEditor,
this.props.displayLimit,
);
this.props.actions.setActiveQueryEditor(queryEditor);
}
}

Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ const sqlLabPersistStateConfig = {
tables,
queries,
tabHistory,
lastUpdatedActiveTab,
} = state.sqlLab;
const unsavedQueryEditors = filterUnsavedQueryEditorList(
queryEditors,
unsavedQueryEditor,
editorTabLastUpdatedAt,
);
if (unsavedQueryEditors.length > 0) {
const hasUnsavedActiveTabState =
tabHistory.slice(-1)[0] !== lastUpdatedActiveTab;
if (unsavedQueryEditors.length > 0 || hasUnsavedActiveTabState) {
const hasFinishedMigrationFromLocalStorage =
unsavedQueryEditors.every(
({ inLocalStorage }) => !inLocalStorage,
Expand All @@ -70,6 +73,9 @@ const sqlLabPersistStateConfig = {
query => query.inLocalStorage && !query.isDataPreview,
),
}),
...(hasUnsavedActiveTabState && {
tabHistory,
}),
};
}
return;
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/SqlLab/reducers/getInitialState.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ describe('getInitialState', () => {
name: expectedValue,
}),
);
expect(
getInitialState(apiDataWithLocalStorage).sqlLab.lastUpdatedActiveTab,
).toEqual(apiDataWithTabState.active_tab.id.toString());
});

it('skip unsaved changes for expired data', () => {
Expand Down
Loading

0 comments on commit 31afb62

Please sign in to comment.