From c28442bd6eb41f5d4326c16ff668cb588d403c5e Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Mon, 20 Nov 2023 11:13:54 -0800 Subject: [PATCH] feat(sqllab): non-blocking persistence mode (#24539) Co-authored-by: Justin Park --- .../src/SqlLab/actions/sqlLab.js | 297 ++++-------------- .../src/SqlLab/actions/sqlLab.test.js | 156 ++------- .../EditorAutoSync/EditorAutoSync.test.tsx | 137 ++++++++ .../components/EditorAutoSync/index.tsx | 106 +++++++ .../SqlLab/components/QueryTable/index.tsx | 6 +- .../src/SqlLab/components/SqlEditor/index.tsx | 7 +- superset-frontend/src/SqlLab/fixtures.ts | 2 + .../middlewares/persistSqlLabStateEnhancer.js | 36 +++ .../SqlLab/reducers/getInitialState.test.ts | 113 ++++++- .../src/SqlLab/reducers/getInitialState.ts | 76 +++-- .../src/SqlLab/reducers/sqlLab.js | 33 +- superset-frontend/src/SqlLab/types.ts | 10 +- .../SqlLab/utils/emptyQueryResults.test.js | 7 +- .../utils/reduxStateToLocalStorageHelper.js | 1 + .../hooks/apiResources/sqlEditorTabs.test.ts | 99 ++++++ .../src/hooks/apiResources/sqlEditorTabs.ts | 70 +++++ .../src/hooks/apiResources/sqlLab.ts | 2 +- .../src/hooks/useDebounceValue.ts | 4 +- superset-frontend/src/pages/SqlLab/index.tsx | 12 +- superset-frontend/src/views/store.ts | 5 +- 20 files changed, 746 insertions(+), 433 deletions(-) create mode 100644 superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx create mode 100644 superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx create mode 100644 superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts create mode 100644 superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 44b4307a1906e..567d3383d752d 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -99,6 +99,8 @@ export const CREATE_DATASOURCE_STARTED = 'CREATE_DATASOURCE_STARTED'; 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 addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; export const addDangerToast = addDangerToastAction; @@ -160,6 +162,10 @@ export function updateQueryEditor(alterations) { return { type: UPDATE_QUERY_EDITOR, alterations }; } +export function setEditorTabLastUpdate(timestamp) { + return { type: SET_EDITOR_TAB_LAST_UPDATE, timestamp }; +} + export function scheduleQuery(query) { return dispatch => SupersetClient.post({ @@ -237,44 +243,11 @@ export function startQuery(query) { } export function querySuccess(query, results) { - return function (dispatch) { - const sqlEditorId = results?.query?.sqlEditorId; - const sync = - sqlEditorId && - !query.isDataPreview && - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${sqlEditorId}`), - postPayload: { latest_query_id: query.id }, - }) - : Promise.resolve(); - - return sync - .then(() => dispatch({ type: QUERY_SUCCESS, query, results })) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while storing the latest query id in the backend. ' + - 'Please contact your administrator if this problem persists.', - ), - ), - ), - ); - }; + return { type: QUERY_SUCCESS, query, results }; } export function queryFailed(query, msg, link, errors) { return function (dispatch) { - const sync = - !query.isDataPreview && - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${query.sqlEditorId}`), - postPayload: { latest_query_id: query.id }, - }) - : Promise.resolve(); - const eventData = { has_err: true, start_offset: query.startDttm, @@ -295,22 +268,7 @@ export function queryFailed(query, msg, link, errors) { }); }); - return ( - sync - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while storing the latest query id in the backend. ' + - 'Please contact your administrator if this problem persists.', - ), - ), - ), - ) - // We should always show the error message, even if we couldn't sync the - // state to the backend - .then(() => dispatch({ type: QUERY_FAILED, query, msg, link, errors })) - ); + dispatch({ type: QUERY_FAILED, query, msg, link, errors }); }; } @@ -557,14 +515,15 @@ export function addQueryEditor(queryEditor) { ? SupersetClient.post({ endpoint: '/tabstateview/', postPayload: { queryEditor }, - }) - : Promise.resolve({ json: { id: shortid.generate() } }); + }).then(({ json }) => ({ ...json, loaded: true })) + : Promise.resolve({ id: shortid.generate() }); return sync - .then(({ json }) => { + .then(({ id, loaded }) => { const newQueryEditor = { ...queryEditor, - id: json.id.toString(), + id: id.toString(), + loaded, }; return dispatch({ type: ADD_QUERY_EDITOR, @@ -736,11 +695,6 @@ export function switchQueryEditor(queryEditor, displayLimit) { schema: json.schema, queryLimit: json.query_limit, remoteId: json.saved_query?.id, - validationResult: { - id: null, - errors: [], - completed: false, - }, hideLeftBar: json.hide_left_bar, }; dispatch(loadQueryEditor(loadedQueryEditor)); @@ -770,31 +724,10 @@ export function setActiveSouthPaneTab(tabId) { export function toggleLeftBar(queryEditor) { const hideLeftBar = !queryEditor.hideLeftBar; - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { hide_left_bar: hideLeftBar }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_TOGGLE_LEFT_BAR, - queryEditor, - hideLeftBar, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while hiding the left bar. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_TOGGLE_LEFT_BAR, + queryEditor, + hideLeftBar, }; } @@ -856,110 +789,26 @@ export function removeQuery(query) { } export function queryEditorSetDb(queryEditor, dbId) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { database_id: dbId }, - }) - : Promise.resolve(); - - return sync - .then(() => dispatch({ type: QUERY_EDITOR_SETDB, queryEditor, dbId })) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab database ID. Please contact your administrator.', - ), - ), - ), - ); - }; + return { type: QUERY_EDITOR_SETDB, queryEditor, dbId }; } export function queryEditorSetSchema(queryEditor, schema) { - return function (dispatch) { - const sync = - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && - typeof queryEditor === 'object' - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { schema }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_SCHEMA, - queryEditor: queryEditor || {}, - schema, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab schema. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_SCHEMA, + queryEditor: queryEditor || {}, + schema, }; } export function queryEditorSetAutorun(queryEditor, autorun) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { autorun }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ type: QUERY_EDITOR_SET_AUTORUN, queryEditor, autorun }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab autorun. Please contact your administrator.', - ), - ), - ), - ); - }; + return { type: QUERY_EDITOR_SET_AUTORUN, queryEditor, autorun }; } export function queryEditorSetTitle(queryEditor, name, id) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${id}`), - postPayload: { label: name }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_TITLE, - queryEditor: { ...queryEditor, id }, - name, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab name. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_TITLE, + queryEditor: { ...queryEditor, id }, + name, }; } @@ -1029,32 +878,19 @@ export function updateSavedQuery(query, clientId) { .then(() => dispatch(updateQueryEditor(query))); } -export function queryEditorSetSql(queryEditor, sql) { - return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql }; +export function queryEditorSetSql(queryEditor, sql, queryId) { + return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql, queryId }; } -export function formatQuery(queryEditor) { - return function (dispatch, getState) { - const { sql } = getUpToDateQuery(getState(), queryEditor); - return SupersetClient.post({ - endpoint: `/api/v1/sqllab/format_sql/`, - body: JSON.stringify({ sql }), - headers: { 'Content-Type': 'application/json' }, - }).then(({ json }) => { - dispatch(queryEditorSetSql(queryEditor, json.result)); - }); - }; -} - -export function queryEditorSetAndSaveSql(targetQueryEditor, sql) { +export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) { return function (dispatch, getState) { const queryEditor = getUpToDateQuery(getState(), targetQueryEditor); // saved query and set tab state use this action - dispatch(queryEditorSetSql(queryEditor, sql)); + dispatch(queryEditorSetSql(queryEditor, sql, queryId)); if (isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { return SupersetClient.put({ endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { sql, latest_query_id: queryEditor.latestQueryId }, + postPayload: { sql, latest_query_id: queryId }, }).catch(() => dispatch( addDangerToast( @@ -1071,59 +907,32 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql) { }; } -export function queryEditorSetQueryLimit(queryEditor, queryLimit) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { query_limit: queryLimit }, - }) - : Promise.resolve(); +export function formatQuery(queryEditor) { + return function (dispatch, getState) { + const { sql } = getUpToDateQuery(getState(), queryEditor); + return SupersetClient.post({ + endpoint: `/api/v1/sqllab/format_sql/`, + body: JSON.stringify({ sql }), + headers: { 'Content-Type': 'application/json' }, + }).then(({ json }) => { + dispatch(queryEditorSetSql(queryEditor, json.result)); + }); + }; +} - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_QUERY_LIMIT, - queryEditor, - queryLimit, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab name. Please contact your administrator.', - ), - ), - ), - ); +export function queryEditorSetQueryLimit(queryEditor, queryLimit) { + return { + type: QUERY_EDITOR_SET_QUERY_LIMIT, + queryEditor, + queryLimit, }; } export function queryEditorSetTemplateParams(queryEditor, templateParams) { - return function (dispatch) { - dispatch({ - type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, - queryEditor, - templateParams, - }); - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { template_params: templateParams }, - }) - : Promise.resolve(); - - return sync.catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab template parameters. ' + - 'Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, + queryEditor, + templateParams, }; } diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index dbf4e8a5c55f1..175ea06ec3ecf 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -32,7 +32,6 @@ import { initialState, queryId, } from 'src/SqlLab/fixtures'; -import { QueryState } from '@superset-ui/core'; const middlewares = [thunk]; const mockStore = configureMockStore(middlewares); @@ -531,88 +530,6 @@ describe('async actions', () => { afterEach(fetchMock.resetHistory); - describe('querySuccess', () => { - it('updates the tab state in the backend', () => { - expect.assertions(2); - - const store = mockStore({}); - const results = { query: { sqlEditorId: 'abcd' } }; - const expectedActions = [ - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.querySuccess(query, results)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); - }); - }); - - describe('fetchQueryResults', () => { - it('updates the tab state in the backend', () => { - expect.assertions(2); - - const results = { - data: mockBigNumber, - query: { sqlEditorId: 'abcd' }, - status: QueryState.SUCCESS, - query_id: 'efgh', - }; - fetchMock.get(fetchQueryEndpoint, JSON.stringify(results), { - overwriteRoutes: true, - }); - const store = mockStore({}); - const expectedActions = [ - { - type: actions.REQUEST_QUERY_RESULTS, - query, - }, - // missing below - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.fetchQueryResults(query)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); - }); - - it("doesn't update the tab state in the backend on stoppped query", () => { - expect.assertions(2); - - const results = { - status: QueryState.STOPPED, - query_id: 'efgh', - }; - fetchMock.get(fetchQueryEndpoint, JSON.stringify(results), { - overwriteRoutes: true, - }); - const store = mockStore({}); - const expectedActions = [ - { - type: actions.REQUEST_QUERY_RESULTS, - query, - }, - // missing below - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.fetchQueryResults(query)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); - }); - }); - }); - describe('addQueryEditor', () => { it('updates the tab state in the backend', () => { expect.assertions(2); @@ -621,7 +538,7 @@ describe('async actions', () => { const expectedActions = [ { type: actions.ADD_QUERY_EDITOR, - queryEditor: { ...queryEditor, id: '1' }, + queryEditor: { ...queryEditor, id: '1', loaded: true }, }, ]; return store.dispatch(actions.addQueryEditor(queryEditor)).then(() => { @@ -673,7 +590,7 @@ describe('async actions', () => { describe('queryEditorSetDb', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const dbId = 42; const store = mockStore({}); @@ -684,18 +601,14 @@ describe('async actions', () => { dbId, }, ]; - return store - .dispatch(actions.queryEditorSetDb(queryEditor, dbId)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetDb(queryEditor, dbId)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetSchema', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const schema = 'schema'; const store = mockStore({}); @@ -706,18 +619,14 @@ describe('async actions', () => { schema, }, ]; - return store - .dispatch(actions.queryEditorSetSchema(queryEditor, schema)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetSchema(queryEditor, schema)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetAutorun', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const autorun = true; const store = mockStore({}); @@ -728,18 +637,14 @@ describe('async actions', () => { autorun, }, ]; - return store - .dispatch(actions.queryEditorSetAutorun(queryEditor, autorun)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetAutorun(queryEditor, autorun)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetTitle', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const name = 'name'; const store = mockStore({}); @@ -750,14 +655,10 @@ describe('async actions', () => { name, }, ]; - return store - .dispatch( - actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), - ) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), + ); + expect(store.getActions()).toEqual(expectedActions); }); }); @@ -803,7 +704,7 @@ describe('async actions', () => { describe('queryEditorSetQueryLimit', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const queryLimit = 10; const store = mockStore({}); @@ -814,18 +715,16 @@ describe('async actions', () => { queryLimit, }, ]; - return store - .dispatch(actions.queryEditorSetQueryLimit(queryEditor, queryLimit)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetQueryLimit(queryEditor, queryLimit), + ); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetTemplateParams', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const templateParams = '{"foo": "bar"}'; const store = mockStore({}); @@ -836,14 +735,11 @@ describe('async actions', () => { templateParams, }, ]; - return store - .dispatch( - actions.queryEditorSetTemplateParams(queryEditor, templateParams), - ) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetTemplateParams(queryEditor, templateParams), + ); + + expect(store.getActions()).toEqual(expectedActions); }); }); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx new file mode 100644 index 0000000000000..52e1d44b247f8 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -0,0 +1,137 @@ +/** + * 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. + */ +/** + * 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. + */ +import React from 'react'; +import fetchMock from 'fetch-mock'; +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 '.'; + +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + logging: { + warn: jest.fn(), + }, +})); + +const editorTabLastUpdatedAt = Date.now(); +const unsavedSqlLabState = { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + name: 'updated tab name', + updatedAt: editorTabLastUpdatedAt + 100, + }, + editorTabLastUpdatedAt, +}; +beforeAll(() => { + jest.useFakeTimers(); +}); + +afterAll(() => { + jest.useRealTimers(); +}); + +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); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: unsavedSqlLabState, + }, + }); + await waitFor(() => jest.runAllTimers()); + expect(fetchMock.calls(updateEditorTabState)).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); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + name: 'updated tab name', + updatedAt: editorTabLastUpdatedAt - 100, + }, + editorTabLastUpdatedAt, + }, + }, + }); + await waitFor(() => jest.runAllTimers()); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + fetchMock.restore(); +}); + +test('renders an error toast when the sync failed', async () => { + const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; + fetchMock.put(updateEditorTabState, { + throws: new Error('errorMessage'), + }); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render( + <> + + + , + { + useRedux: true, + initialState: { + ...initialState, + sqlLab: unsavedSqlLabState, + }, + }, + ); + await waitFor(() => jest.runAllTimers()); + + expect(logging.warn).toHaveBeenCalledTimes(1); + expect(logging.warn).toHaveBeenCalledWith( + 'An error occurred while saving your editor state.', + expect.anything(), + ); + fetchMock.restore(); +}); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx new file mode 100644 index 0000000000000..51399753e95b7 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -0,0 +1,106 @@ +/** + * 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. + */ + +import React, { useRef, useEffect } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { logging } from '@superset-ui/core'; +import { + SqlLabRootState, + QueryEditor, + UnsavedQueryEditor, +} from 'src/SqlLab/types'; +import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs'; +import { useDebounceValue } from 'src/hooks/useDebounceValue'; +import { setEditorTabLastUpdate } from 'src/SqlLab/actions/sqlLab'; + +const INTERVAL = 5000; + +function hasUnsavedChanges( + queryEditor: QueryEditor, + lastSavedTimestamp: number, +) { + return ( + queryEditor.inLocalStorage || + (queryEditor.updatedAt && queryEditor.updatedAt > lastSavedTimestamp) + ); +} + +export function filterUnsavedQueryEditorList( + queryEditors: QueryEditor[], + unsavedQueryEditor: UnsavedQueryEditor, + lastSavedTimestamp: number, +) { + return queryEditors + .map(queryEditor => ({ + ...queryEditor, + ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), + })) + .filter(queryEditor => hasUnsavedChanges(queryEditor, lastSavedTimestamp)); +} + +const EditorAutoSync: React.FC = () => { + const queryEditors = useSelector( + state => state.sqlLab.queryEditors, + ); + const unsavedQueryEditor = useSelector( + state => state.sqlLab.unsavedQueryEditor, + ); + const editorTabLastUpdatedAt = useSelector( + state => state.sqlLab.editorTabLastUpdatedAt, + ); + const dispatch = useDispatch(); + const lastSavedTimestampRef = useRef(editorTabLastUpdatedAt); + const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation(); + + const debouncedUnsavedQueryEditor = useDebounceValue( + unsavedQueryEditor, + INTERVAL, + ); + + useEffect(() => { + const unsaved = filterUnsavedQueryEditorList( + queryEditors, + debouncedUnsavedQueryEditor, + lastSavedTimestampRef.current, + ); + + Promise.all( + unsaved + // TODO: Migrate migrateQueryEditorFromLocalStorage + // in TabbedSqlEditors logic by addSqlEditor mutation later + .filter(({ inLocalStorage }) => !inLocalStorage) + .map(queryEditor => updateSqlEditor({ queryEditor })), + ).then(resolvers => { + if (!resolvers.some(result => 'error' in result)) { + lastSavedTimestampRef.current = Date.now(); + dispatch(setEditorTabLastUpdate(lastSavedTimestampRef.current)); + } + }); + }, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]); + + useEffect(() => { + if (error) { + logging.warn('An error occurred while saving your editor state.', error); + } + }, [dispatch, error]); + + return null; +}; + +export default EditorAutoSync; diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index 6ddae08e68520..5dc8a43c19310 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -25,7 +25,7 @@ import { t, useTheme, QueryResponse } from '@superset-ui/core'; import { useDispatch, useSelector } from 'react-redux'; import { - queryEditorSetAndSaveSql, + queryEditorSetSql, cloneQueryToNewTab, fetchQueryResults, clearQueryResults, @@ -109,7 +109,9 @@ const QueryTable = ({ const data = useMemo(() => { const restoreSql = (query: QueryResponse) => { - dispatch(queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql)); + dispatch( + queryEditorSetSql({ id: query.sqlEditorId }, query.sql, query.id), + ); }; const openQueryInNewTab = (query: QueryResponse) => { diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index 609cb917b6f20..73941fbc79c6d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -557,10 +557,9 @@ const SqlEditor: React.FC = ({ [setQueryEditorAndSaveSql], ); - const onSqlChanged = (sql: string) => { + const onSqlChanged = useEffectEvent((sql: string) => { dispatch(queryEditorSetSql(queryEditor, sql)); - setQueryEditorAndSaveSqlWithDebounce(sql); - }; + }); // Return the heights for the ace editor and the south pane as an object // given the height of the sql editor, north pane percent and south pane percent. @@ -785,7 +784,7 @@ const SqlEditor: React.FC = ({ )} 1.0 for typescript support import persistState from 'redux-localstorage'; +import { pickBy } from 'lodash'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; +import { filterUnsavedQueryEditorList } from 'src/SqlLab/components/EditorAutoSync'; import { emptyTablePersistData, emptyQueryResults, @@ -38,6 +41,39 @@ const sqlLabPersistStateConfig = { slicer: paths => state => { const subset = {}; paths.forEach(path => { + if (isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { + const { + queryEditors, + editorTabLastUpdatedAt, + unsavedQueryEditor, + tables, + queries, + tabHistory, + } = state.sqlLab; + const unsavedQueryEditors = filterUnsavedQueryEditorList( + queryEditors, + unsavedQueryEditor, + editorTabLastUpdatedAt, + ); + if (unsavedQueryEditors.length > 0) { + const hasFinishedMigrationFromLocalStorage = + unsavedQueryEditors.every( + ({ inLocalStorage }) => !inLocalStorage, + ); + subset.sqlLab = { + queryEditors: unsavedQueryEditors, + ...(!hasFinishedMigrationFromLocalStorage && { + tabHistory, + tables: tables.filter(table => table.inLocalStorage), + queries: pickBy( + queries, + query => query.inLocalStorage && !query.isDataPreview, + ), + }), + }; + } + return; + } // this line is used to remove old data from browser localStorage. // we used to persist all redux state into localStorage, but // it caused configurations passed from server-side got override. diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index aca11e2cca10f..1dd3220fcc467 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -54,6 +54,10 @@ const apiDataWithTabState = { }, }; describe('getInitialState', () => { + afterEach(() => { + localStorage.clear(); + }); + it('should output the user that is passed in', () => { expect(getInitialState(apiData).user?.userId).toEqual(1); }); @@ -134,10 +138,6 @@ describe('getInitialState', () => { }); describe('dedupe tables schema', () => { - afterEach(() => { - localStorage.clear(); - }); - it('should dedupe the table schema', () => { localStorage.setItem( 'redux', @@ -245,4 +245,109 @@ describe('getInitialState', () => { ); }); }); + + describe('restore unsaved changes for PERSISTENCE mode', () => { + const lastUpdatedTime = Date.now(); + const expectedValue = 'updated editor value'; + beforeEach(() => { + localStorage.setItem( + 'redux', + JSON.stringify({ + sqlLab: { + queryEditors: [ + { + // restore cached value since updates are after server update time + id: '1', + name: expectedValue, + updatedAt: lastUpdatedTime + 100, + }, + { + // no update required given that last updated time comes before server update time + id: '2', + name: expectedValue, + updatedAt: lastUpdatedTime - 100, + }, + { + // no update required given that there's no updatedAt + id: '3', + name: expectedValue, + }, + ], + }, + }), + ); + }); + + it('restore unsaved changes for PERSISTENCE mode', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + ...apiDataWithTabState.active_tab, + id: 1, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 1, label: '' }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[0], + ).toEqual( + expect.objectContaining({ + id: '1', + name: expectedValue, + }), + ); + }); + + it('skip unsaved changes for expired data', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + ...apiDataWithTabState.active_tab, + id: 2, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 2, label: '' }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[1], + ).toEqual( + expect.objectContaining({ + id: '2', + name: apiDataWithLocalStorage.active_tab.label, + }), + ); + }); + + it('skip unsaved changes for legacy cache data', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + ...apiDataWithTabState.active_tab, + id: 3, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 3, label: '' }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[2], + ).toEqual( + expect.objectContaining({ + id: '3', + name: apiDataWithLocalStorage.active_tab.label, + }), + ); + }); + }); }); diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index e2aa1d4688738..8d72a313b2716 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -20,11 +20,13 @@ import { t } from '@superset-ui/core'; import getToastsFromPyFlashMessages from 'src/components/MessageToasts/getToastsFromPyFlashMessages'; import type { BootstrapData } from 'src/types/bootstrapTypes'; import type { InitialState } from 'src/hooks/apiResources/sqlLab'; -import type { +import { QueryEditor, UnsavedQueryEditor, SqlLabRootState, Table, + LatestQueryEditorVersion, + QueryEditorVersion, } from 'src/SqlLab/types'; export function dedupeTabHistory(tabHistory: string[]) { @@ -53,6 +55,7 @@ export default function getInitialState({ */ let queryEditors: Record = {}; const defaultQueryEditor = { + version: LatestQueryEditorVersion, loaded: true, name: t('Untitled query'), sql: 'SELECT *\nFROM\nWHERE', @@ -73,6 +76,7 @@ export default function getInitialState({ let queryEditor: QueryEditor; if (activeTab && activeTab.id === id) { queryEditor = { + version: activeTab.extra_json?.version ?? QueryEditorVersion.v1, id: id.toString(), loaded: true, name: activeTab.label, @@ -88,6 +92,7 @@ export default function getInitialState({ schema: activeTab.schema, queryLimit: activeTab.query_limit, hideLeftBar: activeTab.hide_left_bar, + updatedAt: activeTab.extra_json?.updatedAt, }; } else { // dummy state, actual state will be loaded on tab switch @@ -103,11 +108,12 @@ export default function getInitialState({ [queryEditor.id]: queryEditor, }; }); - const tabHistory = activeTab ? [activeTab.id.toString()] : []; let tables = {} as Record; - const editorTabLastUpdatedAt = Date.now(); + let editorTabLastUpdatedAt = Date.now(); if (activeTab) { + editorTabLastUpdatedAt = + activeTab.extra_json?.updatedAt || editorTabLastUpdatedAt; activeTab.table_schemas .filter(tableSchema => tableSchema.description !== null) .forEach(tableSchema => { @@ -153,37 +159,57 @@ export default function getInitialState({ // add query editors and tables to state with a special flag so they can // be migrated if the `SQLLAB_BACKEND_PERSISTENCE` feature flag is on sqlLab.queryEditors.forEach(qe => { + const hasConflictFromBackend = Boolean(queryEditors[qe.id]); + const unsavedUpdatedAt = queryEditors[qe.id]?.updatedAt; + const hasUnsavedUpdateSinceLastSave = + qe.updatedAt && + (!unsavedUpdatedAt || qe.updatedAt > unsavedUpdatedAt); + const cachedQueryEditor: UnsavedQueryEditor = + !hasConflictFromBackend || hasUnsavedUpdateSinceLastSave ? qe : {}; queryEditors = { ...queryEditors, [qe.id]: { ...queryEditors[qe.id], - ...qe, - name: qe.title || qe.name, - ...(unsavedQueryEditor.id === qe.id && unsavedQueryEditor), - inLocalStorage: true, + ...cachedQueryEditor, + name: + cachedQueryEditor.title || + cachedQueryEditor.name || + queryEditors[qe.id]?.name, + ...(cachedQueryEditor.id && + unsavedQueryEditor.id === qe.id && + unsavedQueryEditor), + inLocalStorage: !hasConflictFromBackend, loaded: true, }, }; }); const expandedTables = new Set(); - tables = sqlLab.tables.reduce((merged, table) => { - const expanded = !expandedTables.has(table.queryEditorId); - if (expanded) { - expandedTables.add(table.queryEditorId); - } - return { - ...merged, - [table.id]: { - ...tables[table.id], - ...table, - expanded, - }, - }; - }, tables); - Object.values(sqlLab.queries).forEach(query => { - queries[query.id] = { ...query, inLocalStorage: true }; - }); - tabHistory.push(...sqlLab.tabHistory); + + if (sqlLab.tables) { + tables = sqlLab.tables.reduce((merged, table) => { + const expanded = !expandedTables.has(table.queryEditorId); + if (expanded) { + expandedTables.add(table.queryEditorId); + } + return { + ...merged, + [table.id]: { + ...tables[table.id], + ...table, + expanded, + inLocalStorage: true, + }, + }; + }, tables); + } + if (sqlLab.queries) { + Object.values(sqlLab.queries).forEach(query => { + queries[query.id] = { ...query, inLocalStorage: true }; + }); + } + if (sqlLab.tabHistory) { + tabHistory.push(...sqlLab.tabHistory); + } } } } catch (error) { diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index 278109564f96a..59bd0558a1f1c 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -29,7 +29,7 @@ import { extendArr, } from '../../reduxUtils'; -function alterUnsavedQueryEditorState(state, updatedState, id) { +function alterUnsavedQueryEditorState(state, updatedState, id, silent = false) { if (state.tabHistory[state.tabHistory.length - 1] !== id) { const { queryEditors } = alterInArr( state, @@ -45,6 +45,7 @@ function alterUnsavedQueryEditorState(state, updatedState, id) { unsavedQueryEditor: { ...(state.unsavedQueryEditor.id === id && state.unsavedQueryEditor), ...(id ? { id, ...updatedState } : state.unsavedQueryEditor), + ...(!silent && { updatedAt: new Date().getTime() }), }, }; } @@ -64,7 +65,10 @@ export default function sqlLabReducer(state = {}, action) { ...mergeUnsavedState, tabHistory: [...state.tabHistory, action.queryEditor.id], }; - return addToArr(newState, 'queryEditors', action.queryEditor); + return addToArr(newState, 'queryEditors', { + ...action.queryEditor, + updatedAt: new Date().getTime(), + }); }, [actions.QUERY_EDITOR_SAVED]() { const { query, result, clientId } = action; @@ -308,6 +312,7 @@ export default function sqlLabReducer(state = {}, action) { latestQueryId: action.query.id, }, action.query.sqlEditorId, + action.query.isDataPreview, ), }; }, @@ -378,14 +383,12 @@ export default function sqlLabReducer(state = {}, action) { qeIds.indexOf(action.queryEditor?.id) > -1 && state.tabHistory[state.tabHistory.length - 1] !== action.queryEditor.id ) { - const mergeUnsavedState = alterInArr( - state, - 'queryEditors', - state.unsavedQueryEditor, - { + const mergeUnsavedState = { + ...alterInArr(state, 'queryEditors', state.unsavedQueryEditor, { ...state.unsavedQueryEditor, - }, - ); + }), + unsavedQueryEditor: {}, + }; return { ...(action.queryEditor.id === state.unsavedQueryEditor.id ? alterInArr( @@ -522,12 +525,20 @@ export default function sqlLabReducer(state = {}, action) { }; }, [actions.QUERY_EDITOR_SET_SQL]() { + const { unsavedQueryEditor } = state; + if ( + unsavedQueryEditor?.id === action.queryEditor.id && + unsavedQueryEditor.sql === action.sql + ) { + return state; + } return { ...state, ...alterUnsavedQueryEditorState( state, { sql: action.sql, + ...(action.queryId && { latestQueryId: action.queryId }), }, action.queryEditor.id, ), @@ -566,6 +577,7 @@ export default function sqlLabReducer(state = {}, action) { selectedText: action.sql, }, action.queryEditor.id, + true, ), }; }, @@ -708,6 +720,9 @@ export default function sqlLabReducer(state = {}, action) { [actions.CREATE_DATASOURCE_FAILED]() { return { ...state, isDatasourceLoading: false, errorMessage: action.err }; }, + [actions.SET_EDITOR_TAB_LAST_UPDATE]() { + return { ...state, editorTabLastUpdatedAt: action.timestamp }; + }, }; if (action.type in actionHandlers) { return actionHandlers[action.type](); diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 5ecd69293ca8b..6eb42718f0c70 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -29,7 +29,14 @@ export type QueryDictionary = { [id: string]: QueryResponse; }; +export enum QueryEditorVersion { + v1 = 1, +} + +export const LatestQueryEditorVersion = QueryEditorVersion.v1; + export interface QueryEditor { + version: QueryEditorVersion; id: string; dbId?: number; name: string; @@ -48,6 +55,7 @@ export interface QueryEditor { inLocalStorage?: boolean; northPercent?: number; southPercent?: number; + updatedAt?: number; } export type toastState = { @@ -86,7 +94,7 @@ export type SqlLabRootState = { errorMessage: string | null; unsavedQueryEditor: UnsavedQueryEditor; queryCostEstimates?: Record; - editorTabLastUpdatedAt?: number; + editorTabLastUpdatedAt: number; }; localStorageUsageInKilobytes: number; messageToasts: toastState[]; diff --git a/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js b/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js index 9984e1efcaf06..f08fccbef7a53 100644 --- a/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js +++ b/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js @@ -83,10 +83,11 @@ describe('reduxStateToLocalStorageHelper', () => { }); it('should only return selected keys for query editor', () => { - const queryEditors = [defaultQueryEditor]; - expect(Object.keys(queryEditors[0])).toContain('schema'); + const queryEditors = [{ ...defaultQueryEditor, dummy: 'value' }]; + expect(Object.keys(queryEditors[0])).toContain('dummy'); const clearedQueryEditors = clearQueryEditors(queryEditors); - expect(Object.keys(clearedQueryEditors)[0]).not.toContain('schema'); + expect(Object.keys(clearedQueryEditors[0])).toContain('version'); + expect(Object.keys(clearedQueryEditors[0])).not.toContain('dummy'); }); }); diff --git a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js index 281f08bcb366f..f82711362ddbf 100644 --- a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js +++ b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js @@ -26,6 +26,7 @@ import { } from '../constants'; const PERSISTENT_QUERY_EDITOR_KEYS = new Set([ + 'version', 'remoteId', 'autorun', 'dbId', diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts new file mode 100644 index 0000000000000..d0f2230f13d90 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts @@ -0,0 +1,99 @@ +/** + * 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. + */ +import fetchMock from 'fetch-mock'; +import { act, renderHook } from '@testing-library/react-hooks'; +import { + createWrapper, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { LatestQueryEditorVersion } from 'src/SqlLab/types'; +import { useUpdateSqlEditorTabMutation } from './sqlEditorTabs'; + +const expectedQueryEditor = { + version: LatestQueryEditorVersion, + id: '123', + dbId: 456, + name: 'tab 1', + sql: 'SELECT * from example_table', + schema: 'my_schema', + templateParams: '{"a": 1, "v": "str"}', + queryLimit: 1000, + remoteId: null, + autorun: false, + hideLeftBar: false, + updatedAt: Date.now(), +}; + +afterEach(() => { + fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); +}); + +test('puts api request with formData', async () => { + const tabStateMutationApiRoute = `glob:*/tabstateview/${expectedQueryEditor.id}`; + fetchMock.put(tabStateMutationApiRoute, 200); + const { result, waitFor } = renderHook( + () => useUpdateSqlEditorTabMutation(), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + act(() => { + result.current[0]({ + queryEditor: expectedQueryEditor, + }); + }); + await waitFor(() => + expect(fetchMock.calls(tabStateMutationApiRoute).length).toBe(1), + ); + const formData = fetchMock.calls(tabStateMutationApiRoute)[0][1] + ?.body as FormData; + expect(formData.get('database_id')).toBe(`${expectedQueryEditor.dbId}`); + expect(formData.get('schema')).toBe( + JSON.stringify(`${expectedQueryEditor.schema}`), + ); + expect(formData.get('sql')).toBe( + JSON.stringify(`${expectedQueryEditor.sql}`), + ); + expect(formData.get('label')).toBe( + JSON.stringify(`${expectedQueryEditor.name}`), + ); + expect(formData.get('query_limit')).toBe(`${expectedQueryEditor.queryLimit}`); + expect(formData.has('latest_query_id')).toBe(false); + expect(formData.get('template_params')).toBe( + JSON.stringify(`${expectedQueryEditor.templateParams}`), + ); + expect(formData.get('hide_left_bar')).toBe( + `${expectedQueryEditor.hideLeftBar}`, + ); + expect(formData.get('extra_json')).toBe( + JSON.stringify( + JSON.stringify({ + updatedAt: expectedQueryEditor.updatedAt, + version: LatestQueryEditorVersion, + }), + ), + ); +}); diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts new file mode 100644 index 0000000000000..71e0cf2936e5a --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -0,0 +1,70 @@ +/** + * 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. + */ +import { pickBy } from 'lodash'; +import { QueryEditor, LatestQueryEditorVersion } from 'src/SqlLab/types'; +import { api, JsonResponse } from './queryApi'; + +export type EditorMutationParams = { + queryEditor: QueryEditor; + extra?: Record; +}; + +const sqlEditorApi = api.injectEndpoints({ + endpoints: builder => ({ + updateSqlEditorTab: builder.mutation({ + query: ({ + queryEditor: { + version = LatestQueryEditorVersion, + id, + dbId, + schema, + queryLimit, + sql, + name, + latestQueryId, + hideLeftBar, + templateParams, + autorun, + updatedAt, + }, + extra, + }) => ({ + method: 'PUT', + endpoint: encodeURI(`/tabstateview/${id}`), + postPayload: pickBy( + { + database_id: dbId, + schema, + sql, + label: name, + query_limit: queryLimit, + latest_query_id: latestQueryId, + template_params: templateParams, + hide_left_bar: hideLeftBar, + autorun, + extra_json: JSON.stringify({ updatedAt, version, ...extra }), + }, + value => value !== undefined, + ), + }), + }), + }), +}); + +export const { useUpdateSqlEditorTabMutation } = sqlEditorApi; diff --git a/superset-frontend/src/hooks/apiResources/sqlLab.ts b/superset-frontend/src/hooks/apiResources/sqlLab.ts index 123db414e2681..16e8ffde6c609 100644 --- a/superset-frontend/src/hooks/apiResources/sqlLab.ts +++ b/superset-frontend/src/hooks/apiResources/sqlLab.ts @@ -50,7 +50,7 @@ export type InitialState = { template_params: string | null; hide_left_bar?: boolean; saved_query: { id: number } | null; - extra_json?: object; + extra_json?: Record; }; databases: object[]; queries: Record< diff --git a/superset-frontend/src/hooks/useDebounceValue.ts b/superset-frontend/src/hooks/useDebounceValue.ts index 711b2dbd5a98c..862c83770779d 100644 --- a/superset-frontend/src/hooks/useDebounceValue.ts +++ b/superset-frontend/src/hooks/useDebounceValue.ts @@ -19,8 +19,8 @@ import { useState, useEffect } from 'react'; import { FAST_DEBOUNCE } from 'src/constants'; -export function useDebounceValue(value: string, delay = FAST_DEBOUNCE) { - const [debouncedValue, setDebouncedValue] = useState(value); +export function useDebounceValue(value: T, delay = FAST_DEBOUNCE) { + const [debouncedValue, setDebouncedValue] = useState(value); useEffect(() => { const handler: NodeJS.Timeout = setTimeout(() => { diff --git a/superset-frontend/src/pages/SqlLab/index.tsx b/superset-frontend/src/pages/SqlLab/index.tsx index e9f84f1b1d646..3f19b54c29511 100644 --- a/superset-frontend/src/pages/SqlLab/index.tsx +++ b/superset-frontend/src/pages/SqlLab/index.tsx @@ -18,7 +18,7 @@ */ import React, { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { css } from '@superset-ui/core'; +import { css, isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; import { useSqlLabInitialState } from 'src/hooks/apiResources/sqlLab'; import type { InitialState } from 'src/hooks/apiResources/sqlLab'; import { resetState } from 'src/SqlLab/actions/sqlLab'; @@ -27,16 +27,17 @@ import type { SqlLabRootState } from 'src/SqlLab/types'; import { SqlLabGlobalStyles } from 'src/SqlLab//SqlLabGlobalStyles'; import App from 'src/SqlLab/components/App'; import Loading from 'src/components/Loading'; +import EditorAutoSync from 'src/SqlLab/components/EditorAutoSync'; import useEffectEvent from 'src/hooks/useEffectEvent'; import { LocationProvider } from './LocationContext'; export default function SqlLab() { - const editorTabLastUpdatedAt = useSelector( - state => state.sqlLab.editorTabLastUpdatedAt || 0, + const lastInitializedAt = useSelector( + state => state.sqlLab.queriesLastUpdate || 0, ); const { data, isLoading, isError, error, fulfilledTimeStamp } = useSqlLabInitialState(); - const shouldInitialize = editorTabLastUpdatedAt <= (fulfilledTimeStamp || 0); + const shouldInitialize = lastInitializedAt <= (fulfilledTimeStamp || 0); const dispatch = useDispatch(); const initBootstrapData = useEffectEvent( @@ -72,6 +73,9 @@ export default function SqlLab() { > + {isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && ( + + )} ); diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts index 55df81c588b5f..a9c3a9eb13d81 100644 --- a/superset-frontend/src/views/store.ts +++ b/superset-frontend/src/views/store.ts @@ -38,7 +38,6 @@ import logger from 'src/middleware/loggerMiddleware'; import saveModal from 'src/explore/reducers/saveModalReducer'; import explore from 'src/explore/reducers/exploreReducer'; import exploreDatasources from 'src/explore/reducers/datasourcesReducer'; -import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; import { persistSqlLabStateEnhancer } from 'src/SqlLab/middlewares/persistSqlLabStateEnhancer'; import sqlLabReducer from 'src/SqlLab/reducers/sqlLab'; @@ -167,9 +166,7 @@ export function setupStore({ }, middleware: getMiddleware, devTools: process.env.WEBPACK_MODE === 'development' && !disableDebugger, - ...(!isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && { - enhancers: [persistSqlLabStateEnhancer as StoreEnhancer], - }), + enhancers: [persistSqlLabStateEnhancer as StoreEnhancer], ...overrides, }); }