diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 506f140fca46..972b2b52a690 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -119,7 +119,6 @@ "react-jsonschema-form": "^1.8.1", "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", - "react-query": "^3.39.2", "react-redux": "^7.2.8", "react-resize-detector": "^7.1.2", "react-reverse-portal": "^2.1.1", @@ -40836,11 +40835,6 @@ "node": ">=0.10.0" } }, - "node_modules/js-sha3": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/js-sha3/-/js-sha3-0.8.0.tgz", - "integrity": "sha512-gF1cRrHhIzNfToc802P800N8PpXS+evLLXfsVpowqmAFR9uwbi89WvXg2QspOmXL8QL86J4T1EpFu+yUkwJY3Q==" - }, "node_modules/js-string-escape": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/js-string-escape/-/js-string-escape-1.0.1.tgz", @@ -50440,55 +50434,6 @@ "react-dom": "^16.6.0 || ^17.0.0" } }, - "node_modules/react-query": { - "version": "3.39.2", - "resolved": "https://registry.npmjs.org/react-query/-/react-query-3.39.2.tgz", - "integrity": "sha512-F6hYDKyNgDQfQOuR1Rsp3VRzJnWHx6aRnnIZHMNGGgbL3SBgpZTDg8MQwmxOgpCAoqZJA+JSNCydF1xGJqKOCA==", - "dependencies": { - "@babel/runtime": "^7.5.5", - "broadcast-channel": "^3.4.1", - "match-sorter": "^6.0.2" - }, - "funding": { - "type": "github", - "url": "https://github.com/sponsors/tannerlinsley" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0" - }, - "peerDependenciesMeta": { - "react-dom": { - "optional": true - }, - "react-native": { - "optional": true - } - } - }, - "node_modules/react-query/node_modules/broadcast-channel": { - "version": "3.7.0", - "resolved": "https://registry.npmjs.org/broadcast-channel/-/broadcast-channel-3.7.0.tgz", - "integrity": "sha512-cIAKJXAxGJceNZGTZSBzMxzyOn72cVgPnKx4dc6LRjQgbaJUQqhy5rzL3zbMxkMWsGKkv2hSFkPRMEXfoMZ2Mg==", - "dependencies": { - "@babel/runtime": "^7.7.2", - "detect-node": "^2.1.0", - "js-sha3": "0.8.0", - "microseconds": "0.2.0", - "nano-time": "1.0.0", - "oblivious-set": "1.0.0", - "rimraf": "3.0.2", - "unload": "2.2.0" - } - }, - "node_modules/react-query/node_modules/unload": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/unload/-/unload-2.2.0.tgz", - "integrity": "sha512-B60uB5TNBLtN6/LsgAf3udH9saB5p7gqJwcFfbOEZ8BcBHnGwCf6G/TGiEqkRAxX7zAFIUtzdrXQSdL3Q/wqNA==", - "dependencies": { - "@babel/runtime": "^7.6.2", - "detect-node": "^2.0.4" - } - }, "node_modules/react-redux": { "version": "7.2.8", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.8.tgz", @@ -94229,11 +94174,6 @@ "resolved": "https://registry.npmjs.org/js-levenshtein/-/js-levenshtein-1.1.6.tgz", "integrity": "sha512-X2BB11YZtrRqY4EnQcLX5Rh373zbK4alC1FW7D7MBhL2gtcC17cTnr6DmfHZeS0s2rTHjUTMMHfG7gO8SSdw+g==" }, - "js-sha3": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/js-sha3/-/js-sha3-0.8.0.tgz", - "integrity": "sha512-gF1cRrHhIzNfToc802P800N8PpXS+evLLXfsVpowqmAFR9uwbi89WvXg2QspOmXL8QL86J4T1EpFu+yUkwJY3Q==" - }, "js-string-escape": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/js-string-escape/-/js-string-escape-1.0.1.tgz", @@ -101549,42 +101489,6 @@ "react-popper": "^2.2.4" } }, - "react-query": { - "version": "3.39.2", - "resolved": "https://registry.npmjs.org/react-query/-/react-query-3.39.2.tgz", - "integrity": "sha512-F6hYDKyNgDQfQOuR1Rsp3VRzJnWHx6aRnnIZHMNGGgbL3SBgpZTDg8MQwmxOgpCAoqZJA+JSNCydF1xGJqKOCA==", - "requires": { - "@babel/runtime": "^7.5.5", - "broadcast-channel": "^3.4.1", - "match-sorter": "^6.0.2" - }, - "dependencies": { - "broadcast-channel": { - "version": "3.7.0", - "resolved": "https://registry.npmjs.org/broadcast-channel/-/broadcast-channel-3.7.0.tgz", - "integrity": "sha512-cIAKJXAxGJceNZGTZSBzMxzyOn72cVgPnKx4dc6LRjQgbaJUQqhy5rzL3zbMxkMWsGKkv2hSFkPRMEXfoMZ2Mg==", - "requires": { - "@babel/runtime": "^7.7.2", - "detect-node": "^2.1.0", - "js-sha3": "0.8.0", - "microseconds": "0.2.0", - "nano-time": "1.0.0", - "oblivious-set": "1.0.0", - "rimraf": "3.0.2", - "unload": "2.2.0" - } - }, - "unload": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/unload/-/unload-2.2.0.tgz", - "integrity": "sha512-B60uB5TNBLtN6/LsgAf3udH9saB5p7gqJwcFfbOEZ8BcBHnGwCf6G/TGiEqkRAxX7zAFIUtzdrXQSdL3Q/wqNA==", - "requires": { - "@babel/runtime": "^7.6.2", - "detect-node": "^2.0.4" - } - } - } - }, "react-redux": { "version": "7.2.8", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.8.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index c84a259a2332..098e01f6109a 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -184,7 +184,6 @@ "react-jsonschema-form": "^1.8.1", "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", - "react-query": "^3.39.2", "react-redux": "^7.2.8", "react-resize-detector": "^7.1.2", "react-reverse-portal": "^2.1.1", diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 4790b4b718ab..0c7b6a0a66a5 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -26,26 +26,38 @@ import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import reducerIndex from 'spec/helpers/reducerIndex'; import { QueryParamProvider } from 'use-query-params'; -import QueryProvider from 'src/views/QueryProvider'; import { configureStore, Store } from '@reduxjs/toolkit'; +import { api } from 'src/hooks/apiResources/queryApi'; type Options = Omit & { useRedux?: boolean; useDnd?: boolean; useQueryParams?: boolean; useRouter?: boolean; - useQuery?: boolean; initialState?: {}; reducers?: {}; store?: Store; }; +const createStore = (initialState: object = {}, reducers: object = {}) => + configureStore({ + preloadedState: initialState, + reducer: { + ...reducers, + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), + devTools: false, + }); + +export const defaultStore = createStore(); + export function createWrapper(options?: Options) { const { useDnd, useRedux, useQueryParams, - useQuery = true, useRouter, initialState, reducers, @@ -63,13 +75,7 @@ export function createWrapper(options?: Options) { if (useRedux) { const mockStore = - store ?? - configureStore({ - preloadedState: initialState || {}, - devTools: false, - reducer: reducers || reducerIndex, - }); - + store ?? createStore(initialState, reducers || reducerIndex); result = {result}; } @@ -81,10 +87,6 @@ export function createWrapper(options?: Options) { result = {result}; } - if (useQuery) { - result = {result}; - } - return result; }; } diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx index 86b981ae76df..be0a982a4951 100644 --- a/superset-frontend/src/SqlLab/App.jsx +++ b/superset-frontend/src/SqlLab/App.jsx @@ -22,7 +22,6 @@ import { Provider } from 'react-redux'; import { hot } from 'react-hot-loader/root'; import { FeatureFlag, ThemeProvider } from '@superset-ui/core'; import { GlobalStyles } from 'src/GlobalStyles'; -import QueryProvider from 'src/views/QueryProvider'; import { initFeatureFlags, isFeatureEnabled } from 'src/featureFlags'; import { setupStore } from 'src/views/store'; import setupExtensions from 'src/setup/setupExtensions'; @@ -133,15 +132,13 @@ if (sqlLabMenu) { } const Application = () => ( - - - - - - - - - + + + + + + + ); export default hot(Application); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx index 99551b9d41e8..d8a1aa260a7d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx @@ -17,25 +17,12 @@ * under the License. */ import React from 'react'; -import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; import { fireEvent, render, waitFor } from 'spec/helpers/testing-library'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; -import { Provider } from 'react-redux'; -import thunk from 'redux-thunk'; -import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; -import { - SQL_EDITOR_GUTTER_HEIGHT, - SQL_EDITOR_GUTTER_MARGIN, - SQL_TOOLBAR_HEIGHT, -} from 'src/SqlLab/constants'; -import AceEditorWrapper from 'src/SqlLab/components/AceEditorWrapper'; -import SouthPane from 'src/SqlLab/components/SouthPane'; +import { reducers } from 'src/SqlLab/reducers'; import SqlEditor from 'src/SqlLab/components/SqlEditor'; import { setupStore } from 'src/views/store'; -import { reducers } from 'src/SqlLab/reducers'; -import QueryProvider from 'src/views/QueryProvider'; -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import { initialState, queries, @@ -43,6 +30,7 @@ import { defaultQueryEditor, } from 'src/SqlLab/fixtures'; import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar'; +import { api } from 'src/hooks/apiResources/queryApi'; jest.mock('src/components/AsyncAceEditor', () => ({ ...jest.requireActual('src/components/AsyncAceEditor'), @@ -51,27 +39,24 @@ jest.mock('src/components/AsyncAceEditor', () => ({ data-test="react-ace" onChange={evt => onChange(evt.target.value)} onBlur={onBlur} - > - {value} - + value={value} + /> ), })); jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn()); -const MOCKED_SQL_EDITOR_HEIGHT = 500; - fetchMock.get('glob:*/api/v1/database/*', { result: [] }); fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] }); fetchMock.post('glob:*/sqllab/execute/*', { result: [] }); -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); +let store; +let actions; const mockInitialState = { ...initialState, sqlLab: { ...initialState.sqlLab, databases: { - dbid1: { + 1991: { allow_ctas: false, allow_cvas: false, allow_dml: false, @@ -86,11 +71,10 @@ const mockInitialState = { }, unsavedQueryEditor: { id: defaultQueryEditor.id, - dbId: 'dbid1', + dbId: 1991, }, }, }; -const store = mockStore(mockInitialState); const setup = (props = {}, store) => render(, { @@ -98,6 +82,23 @@ const setup = (props = {}, store) => ...(store && { store }), }); +const logAction = () => next => action => { + if (typeof action === 'function') { + return next(action); + } + actions.push(action); + return next(action); +}; + +const createStore = initState => + setupStore({ + disableDegugger: true, + initialState: initState, + rootReducers: reducers, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware, logAction), + }); + describe('SqlEditor', () => { const mockedProps = { queryEditor: initialState.sqlLab.queryEditors[0], @@ -112,24 +113,20 @@ describe('SqlEditor', () => { }; beforeEach(() => { + store = createStore(mockInitialState); + actions = []; + SqlEditorLeftBar.mockClear(); SqlEditorLeftBar.mockImplementation(() => (
)); }); - const buildWrapper = (props = {}) => - mount( - - - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, - }, - ); + afterEach(() => { + act(() => { + store.dispatch(api.util.resetApiState()); + }); + }); it('does not render SqlEditor if no db selected', async () => { const queryEditor = initialState.sqlLab.queryEditors[1]; @@ -152,11 +149,7 @@ describe('SqlEditor', () => { }); it('avoids rerendering EditorLeftBar while typing', async () => { - const sqlLabStore = setupStore({ - initialState: mockInitialState, - rootReducers: reducers, - }); - const { findByTestId } = setup(mockedProps, sqlLabStore); + const { findByTestId } = setup(mockedProps, store); const editor = await findByTestId('react-ace'); const sql = 'select *'; const renderCount = SqlEditorLeftBar.mock.calls.length; @@ -168,34 +161,32 @@ describe('SqlEditor', () => { it('renders sql from unsaved change', async () => { const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE'; - const { findByTestId } = setup( - mockedProps, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - databases: { - dbid1: { - allow_ctas: false, - allow_cvas: false, - allow_dml: false, - allow_file_upload: false, - allow_run_async: false, - backend: 'postgresql', - database_name: 'examples', - expose_in_sqllab: true, - force_ctas_schema: null, - id: 1, - }, - }, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - dbId: 'dbid1', - sql: expectedSql, + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + databases: { + 2023: { + allow_ctas: false, + allow_cvas: false, + allow_dml: false, + allow_file_upload: false, + allow_run_async: false, + backend: 'postgresql', + database_name: 'examples', + expose_in_sqllab: true, + force_ctas_schema: null, + id: 1, }, }, - }), - ); + unsavedQueryEditor: { + id: defaultQueryEditor.id, + dbId: 2023, + sql: expectedSql, + }, + }, + }); + const { findByTestId } = setup(mockedProps, store); const editor = await findByTestId('react-ace'); expect(editor).toHaveValue(expectedSql); @@ -209,7 +200,7 @@ describe('SqlEditor', () => { }); it('runs query action with ctas false', async () => { - const expectedStore = mockStore({ + store = createStore({ ...initialState, sqlLab: { ...initialState.sqlLab, @@ -234,11 +225,11 @@ describe('SqlEditor', () => { }, }, }); - const { findByTestId } = setup(mockedProps, expectedStore); + const { findByTestId } = setup(mockedProps, store); const runButton = await findByTestId('run-query-action'); fireEvent.click(runButton); await waitFor(() => - expect(expectedStore.getActions()).toContainEqual({ + expect(actions).toContainEqual({ type: 'START_QUERY', query: expect.objectContaining({ ctas: false, @@ -248,34 +239,6 @@ describe('SqlEditor', () => { ); }); - // TODO eschutho convert tests to RTL - // eslint-disable-next-line jest/no-disabled-tests - it.skip('does not overflow the editor window', async () => { - const wrapper = buildWrapper(); - await waitForComponentToPaint(wrapper); - const totalSize = - parseFloat(wrapper.find(AceEditorWrapper).props().height) + - wrapper.find(SouthPane).props().height + - SQL_TOOLBAR_HEIGHT + - SQL_EDITOR_GUTTER_MARGIN * 2 + - SQL_EDITOR_GUTTER_HEIGHT; - expect(totalSize).toEqual(MOCKED_SQL_EDITOR_HEIGHT); - }); - - // eslint-disable-next-line jest/no-disabled-tests - it.skip('does not overflow the editor window after resizing', async () => { - const wrapper = buildWrapper(); - wrapper.setState({ height: 450 }); - await waitForComponentToPaint(wrapper); - const totalSize = - parseFloat(wrapper.find(AceEditorWrapper).props().height) + - wrapper.find(SouthPane).props().height + - SQL_TOOLBAR_HEIGHT + - SQL_EDITOR_GUTTER_MARGIN * 2 + - SQL_EDITOR_GUTTER_HEIGHT; - expect(totalSize).toEqual(450); - }); - it('render a Limit Dropdown', async () => { const defaultQueryLimit = 101; const updatedProps = { ...mockedProps, defaultQueryLimit }; diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index b3ef02743ffe..d12938a23508 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -17,15 +17,16 @@ * under the License. */ import React from 'react'; -import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Provider } from 'react-redux'; import '@testing-library/jest-dom/extend-expect'; -import thunk from 'redux-thunk'; import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar'; import { table, initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { setupStore } from 'src/views/store'; +import { reducers } from 'src/SqlLab/reducers'; const mockedProps = { tables: [table], @@ -37,11 +38,30 @@ const mockedProps = { }, height: 0, }; -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); -const store = mockStore(initialState); + +let store; +let actions; + +const logAction = () => next => action => { + if (typeof action === 'function') { + return next(action); + } + actions.push(action); + return next(action); +}; + +const createStore = initState => + setupStore({ + disableDegugger: true, + initialState: initState, + rootReducers: reducers, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware, logAction), + }); beforeEach(() => { + store = createStore(initialState); + actions = []; fetchMock.get('glob:*/api/v1/database/?*', { result: [] }); fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { count: 2, @@ -60,6 +80,8 @@ beforeEach(() => { afterEach(() => { fetchMock.restore(); + store.dispatch(api.util.resetApiState()); + jest.clearAllMocks(); }); const renderAndWait = (props, store) => @@ -112,21 +134,29 @@ test('table should be visible when expanded is true', async () => { }); test('should toggle the table when the header is clicked', async () => { - const store = mockStore(initialState); await renderAndWait(mockedProps, store); const header = (await screen.findAllByText(/ab_user/))[1]; expect(header).toBeInTheDocument(); + userEvent.click(header); await waitFor(() => { - expect(store.getActions()[store.getActions().length - 1].type).toEqual( - 'COLLAPSE_TABLE', - ); + expect(actions[actions.length - 1].type).toEqual('COLLAPSE_TABLE'); }); }); test('When changing database the table list must be updated', async () => { + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + schema: 'new_schema', + }, + }, + }); const { rerender } = await renderAndWait(mockedProps, store); expect(screen.getAllByText(/main/i)[0]).toBeInTheDocument(); @@ -145,16 +175,7 @@ test('When changing database the table list must be updated', async () => { />, { useRedux: true, - store: mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - schema: 'new_schema', - }, - }, - }), + store, }, ); expect(await screen.findByText(/new_db/i)).toBeInTheDocument(); @@ -163,22 +184,19 @@ test('When changing database the table list must be updated', async () => { test('ignore schema api when current schema is deprecated', async () => { const invalidSchemaName = 'None'; - const { rerender } = await renderAndWait( - mockedProps, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - schema: invalidSchemaName, - }, + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + schema: invalidSchemaName, }, - }), - ); + }, + }); + const { rerender } = await renderAndWait(mockedProps, store); expect(await screen.findByText(/Database/i)).toBeInTheDocument(); - expect(screen.queryByText(/None/i)).toBeInTheDocument(); expect(fetchMock.calls()).not.toContainEqual( expect.arrayContaining([ expect.stringContaining( diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx index d826a5a9b055..90d1de2528a6 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx @@ -32,7 +32,6 @@ import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors'; import SqlEditor from 'src/SqlLab/components/SqlEditor'; import { initialState } from 'src/SqlLab/fixtures'; import { newQueryTabName } from 'src/SqlLab/utils/newQueryTabName'; -import QueryProvider from 'src/views/QueryProvider'; fetchMock.get('glob:*/api/v1/database/*', {}); fetchMock.get('glob:*/api/v1/saved_query/*', {}); @@ -77,11 +76,9 @@ describe('TabbedSqlEditors', () => { const mountWithAct = async () => act(async () => { mount( - - - - - , + + + , { wrappingComponent: ThemeProvider, wrappingComponentProps: { theme: supersetTheme }, diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index a684e6d837a7..7635361d8933 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -18,10 +18,16 @@ */ import React from 'react'; +import { act } from 'react-dom/test-utils'; import fetchMock from 'fetch-mock'; -import { render, screen, waitFor } from 'spec/helpers/testing-library'; -import { queryClient } from 'src/views/QueryProvider'; +import { + render, + screen, + waitFor, + defaultStore as store, +} from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; +import { api } from 'src/hooks/apiResources/queryApi'; import DatabaseSelector, { DatabaseSelectorProps } from '.'; import { EmptyStateSmall } from '../EmptyState'; @@ -163,24 +169,26 @@ function setupFetchMock() { } beforeEach(() => { - queryClient.clear(); setupFetchMock(); }); afterEach(() => { fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('Should render', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); expect(await screen.findByTestId('DatabaseSelector')).toBeInTheDocument(); }); test('Refresh should work', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); expect(fetchMock.calls(schemaApiRoute).length).toBe(0); @@ -212,7 +220,7 @@ test('Refresh should work', async () => { test('Should database select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -233,7 +241,7 @@ test('should show empty state if there are no options', async () => { db={undefined} emptyState={} />, - { useRedux: true }, + { useRedux: true, store }, ); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', @@ -246,7 +254,7 @@ test('should show empty state if there are no options', async () => { test('Should schema select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -262,7 +270,7 @@ test('Should schema select display options', async () => { test('Sends the correct db when changing the database', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -282,7 +290,7 @@ test('Sends the correct db when changing the database', async () => { test('Sends the correct schema when changing the schema', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index cbac98968873..8516017b20e6 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -223,14 +223,13 @@ export default function DatabaseSelector({ const { data, isFetching: loadingSchemas, - isFetched, refetch, } = useSchemas({ dbId: currentDb?.value, - onSuccess: data => { - if (data.length === 1) { - changeSchema(data[0]); - } else if (!data.find(schemaOption => schema === schemaOption.value)) { + onSuccess: (schemas, isFetched) => { + if (schemas.length === 1) { + changeSchema(schemas[0]); + } else if (!schemas.find(schemaOption => schema === schemaOption.value)) { changeSchema(undefined); } diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 7d378902e6e9..d5619c552792 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -18,24 +18,21 @@ */ import React from 'react'; import { act } from 'react-dom/test-utils'; -import configureStore from 'redux-mock-store'; import { mount } from 'enzyme'; import { Provider } from 'react-redux'; import fetchMock from 'fetch-mock'; -import thunk from 'redux-thunk'; import sinon from 'sinon'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { defaultStore as store } from 'spec/helpers/testing-library'; import Modal from 'src/components/Modal'; import { DatasourceModal } from 'src/components/Datasource'; import DatasourceEditor from 'src/components/Datasource/DatasourceEditor'; import * as featureFlags from 'src/featureFlags'; import mockDatasource from 'spec/fixtures/mockDatasource'; -import QueryProvider from 'src/views/QueryProvider'; +import { api } from 'src/hooks/apiResources/queryApi'; -const mockStore = configureStore([thunk]); -const store = mockStore({}); const datasource = mockDatasource['7__table']; const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7'; @@ -55,11 +52,9 @@ const mockedProps = { async function mountAndWait(props = mockedProps) { const mounted = mount( - - - - - , + + + , { wrappingComponent: ThemeProvider, wrappingComponentProps: { theme: supersetTheme }, @@ -81,6 +76,9 @@ describe('DatasourceModal', () => { afterAll(() => { isFeatureEnabledMock.restore(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); it('renders', () => { diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 1e6083e1b46f..f6107d4cacb2 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -18,8 +18,15 @@ */ import React from 'react'; -import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; -import { queryClient } from 'src/views/QueryProvider'; +import { act } from 'react-dom/test-utils'; +import { + render, + screen, + waitFor, + within, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import fetchMock from 'fetch-mock'; import userEvent from '@testing-library/user-event'; import TableSelector, { TableSelectorMultiple } from '.'; @@ -56,11 +63,13 @@ const getSelectItemContainer = (select: HTMLElement) => ); beforeEach(() => { - queryClient.clear(); fetchMock.get(databaseApiRoute, { result: [] }); }); afterEach(() => { + act(() => { + store.dispatch(api.util.resetApiState()); + }); fetchMock.reset(); }); @@ -69,7 +78,7 @@ test('renders with default props', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -91,7 +100,7 @@ test('renders table options', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -109,7 +118,10 @@ test('renders disabled without schema', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { + useRedux: true, + store, + }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -128,7 +140,7 @@ test('table select retain value if not in SQL Lab mode', async () => { sqlLabMode: false, }); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', @@ -168,7 +180,7 @@ test('table multi select retain all the values selected', async () => { onTableSelectChange: callback, }); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 3886a86fd20a..f2a06c56b297 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -175,17 +175,16 @@ const TableSelector: FunctionComponent = ({ const { data, isFetching: loadingTables, - isFetched, refetch, } = useTables({ dbId: database?.id, schema: currentSchema, - onSuccess: () => { + onSuccess: (data, isFetched) => { if (isFetched) { addSuccessToast(t('List updated')); } }, - onError: (err: Response) => { + onError: err => { getClientErrorObject(err).then(clientError => { handleError( getClientErrorMessage( diff --git a/superset-frontend/src/hooks/apiResources/queryApi.test.ts b/superset-frontend/src/hooks/apiResources/queryApi.test.ts new file mode 100644 index 000000000000..4fb71387752d --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/queryApi.test.ts @@ -0,0 +1,100 @@ +/** + * 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 configureStore, { MockStore } from 'redux-mock-store'; +import rison from 'rison'; +import { JsonResponse } from '@superset-ui/core'; +import { supersetClientQuery } from './queryApi'; + +const getBaseQueryApiMock = (store: MockStore) => ({ + ...new AbortController(), + dispatch: store.dispatch, + getState: store.getState, + extra: undefined, + endpoint: 'endpoint', + type: 'query' as const, +}); + +const mockStore = configureStore(); +const store = mockStore(); + +afterEach(() => { + fetchMock.reset(); +}); + +test('supersetClientQuery should build the endpoint with rison encoded query string and return data when successful', async () => { + const expectedData = { id: 1, name: 'Test' }; + const expectedUrl = '/api/v1/get-endpoint/'; + const expectedPostUrl = '/api/v1/post-endpoint/'; + const urlParams = { str: 'string', num: 123, bool: true }; + const getEndpoint = `glob:*${expectedUrl}?q=${rison.encode(urlParams)}`; + const postEndpoint = `glob:*${expectedPostUrl}?q=${rison.encode(urlParams)}`; + fetchMock.get(getEndpoint, { result: expectedData }); + fetchMock.post(postEndpoint, { result: expectedData }); + const result = await supersetClientQuery( + { + endpoint: expectedUrl, + urlParams, + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(0); + expect((result.data as JsonResponse).json.result).toEqual(expectedData); + await supersetClientQuery( + { + method: 'post', + endpoint: expectedPostUrl, + urlParams, + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(1); +}); + +test('supersetClientQuery should return error when unsuccessful', async () => { + const expectedError = 'Request failed'; + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + fetchMock.get(endpoint, { throws: new Error(expectedError) }); + const result = await supersetClientQuery( + { endpoint }, + getBaseQueryApiMock(store), + {}, + ); + expect(result.error).toEqual({ error: expectedError }); +}); + +test('supersetClientQuery should return parsed response by parseMethod', async () => { + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + const bitIntVal = '9223372036854775807'; + const expectedData = `{ "id": ${bitIntVal} }`; + fetchMock.get(endpoint, expectedData); + const result = await supersetClientQuery( + { endpoint, parseMethod: 'json-bigint' }, + getBaseQueryApiMock(store), + {}, + ); + expect(`${(result.data as JsonResponse).json.id}`).toEqual(bitIntVal); +}); diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts new file mode 100644 index 000000000000..9147173d7918 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -0,0 +1,71 @@ +/** + * 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 rison from 'rison'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { createApi, BaseQueryFn } from '@reduxjs/toolkit/query/react'; +import { + SupersetClient, + ParseMethod, + SupersetClientResponse, + JsonValue, + RequestBase, +} from '@superset-ui/core'; + +export { JsonResponse, TextResponse } from '@superset-ui/core'; + +export const supersetClientQuery: BaseQueryFn< + Pick & { + endpoint: string; + parseMethod?: ParseMethod; + transformResponse?: (response: SupersetClientResponse) => JsonValue; + urlParams?: Record; + } +> = ( + { + endpoint, + urlParams, + transformResponse, + method = 'GET', + parseMethod = 'json', + ...rest + }, + api, +) => + SupersetClient.request({ + ...rest, + endpoint: `${endpoint}${urlParams ? `?q=${rison.encode(urlParams)}` : ''}`, + method, + parseMethod, + signal: api.signal, + }) + .then(data => ({ + data: transformResponse?.(data) ?? data, + })) + .catch(response => + getClientErrorObject(response).then(errorObj => ({ + error: errorObj, + })), + ); + +export const api = createApi({ + reducerPath: 'queryApi', + tagTypes: ['Schemas', 'Tables'], + endpoints: () => ({}), + baseQuery: supersetClientQuery, +}); diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 59d00a5dc71b..70c1289d576a 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -19,28 +19,37 @@ import rison from 'rison'; import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; -import QueryProvider, { queryClient } from 'src/views/QueryProvider'; +import { + createWrapper, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import { useSchemas } from './schemas'; const fakeApiResult = { result: ['test schema 1', 'test schema b'], }; +const fakeApiResult2 = { + result: ['test schema 2', 'test schema a'], +}; const expectedResult = fakeApiResult.result.map((value: string) => ({ value, label: value, title: value, })); +const expectedResult2 = fakeApiResult2.result.map((value: string) => ({ + value, + label: value, + title: value, +})); describe('useSchemas hook', () => { - beforeEach(() => { - queryClient.clear(); - jest.useFakeTimers(); - }); - afterEach(() => { fetchMock.reset(); - jest.useRealTimers(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('returns api response mapping json result', async () => { @@ -48,19 +57,22 @@ describe('useSchemas hook', () => { const forceRefresh = false; const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; fetchMock.get(schemaApiRoute, fakeApiResult); - const { result } = renderHook( + const onSuccess = jest.fn(); + const { result, waitFor } = renderHook( () => useSchemas({ dbId: expectDbId, + onSuccess, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); + expect(result.current.data).toEqual(expectedResult); expect( fetchMock.calls( `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ @@ -68,11 +80,11 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); - expect(result.current.data).toEqual(expectedResult); - await act(async () => { + expect(onSuccess).toHaveBeenCalledTimes(1); + act(() => { result.current.refetch(); }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2)); expect( fetchMock.calls( `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ @@ -80,6 +92,7 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); + expect(onSuccess).toHaveBeenCalledTimes(2); expect(result.current.data).toEqual(expectedResult); }); @@ -87,52 +100,68 @@ describe('useSchemas hook', () => { const expectDbId = 'db1'; const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; fetchMock.get(schemaApiRoute, fakeApiResult); - const { result, rerender } = renderHook( + const { result, rerender, waitFor } = renderHook( () => useSchemas({ dbId: expectDbId, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); rerender(); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - expect(result.current.data).toEqual(expectedResult); }); it('returns refreshed data after expires', async () => { const expectDbId = 'db1'; - const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; - fetchMock.get(schemaApiRoute, fakeApiResult); - const { result, rerender } = renderHook( - () => + const schemaApiRoute = `glob:*/api/v1/database/*/schemas/*`; + fetchMock.get(schemaApiRoute, url => + url.includes(expectDbId) ? fakeApiResult : fakeApiResult2, + ); + const onSuccess = jest.fn(); + const { result, rerender, waitFor } = renderHook( + ({ dbId }) => useSchemas({ - dbId: expectDbId, + dbId, + onSuccess, }), { - wrapper: QueryProvider, + initialProps: { dbId: expectDbId }, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - queryClient.clear(); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + expect(onSuccess).toHaveBeenCalledTimes(1); + + rerender({ dbId: 'db2' }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult2)); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); - expect(result.current.data).toEqual(expectedResult); + expect(onSuccess).toHaveBeenCalledTimes(2); + + rerender({ dbId: expectDbId }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + expect(onSuccess).toHaveBeenCalledTimes(3); + + // clean up cache + act(() => { + store.dispatch(api.util.invalidateTags(['Schemas'])); + }); + + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(3)); + expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); }); }); diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 34a50ca4e989..22fd2cf38b09 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -16,20 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { useRef } from 'react'; -import { useQuery, UseQueryOptions } from 'react-query'; -import rison from 'rison'; -import { SupersetClient } from '@superset-ui/core'; - -export type FetchSchemasQueryParams = { - dbId?: string | number; - forceRefresh?: boolean; -}; - -type QueryData = { - json: { result: string[] }; - response: Response; -}; +import { useCallback, useEffect, useRef } from 'react'; +import useEffectEvent from 'src/hooks/useEffectEvent'; +import { api, JsonResponse } from './queryApi'; export type SchemaOption = { value: string; @@ -37,44 +26,98 @@ export type SchemaOption = { title: string; }; -export function fetchSchemas({ dbId, forceRefresh }: FetchSchemasQueryParams) { - const queryParams = rison.encode({ force: forceRefresh }); - // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. - const endpoint = `/api/v1/database/${dbId}/schemas/?q=${queryParams}`; - return SupersetClient.get({ endpoint }) as Promise; -} +export type FetchSchemasQueryParams = { + dbId?: string | number; + forceRefresh: boolean; + onSuccess?: (data: SchemaOption[], isRefetched: boolean) => void; + onError?: () => void; +}; -type Params = FetchSchemasQueryParams & - Pick, 'onSuccess' | 'onError'>; +type Params = Omit; + +const schemaApi = api.injectEndpoints({ + endpoints: builder => ({ + schemas: builder.query({ + providesTags: [{ type: 'Schemas', id: 'LIST' }], + query: ({ dbId, forceRefresh }) => ({ + endpoint: `/api/v1/database/${dbId}/schemas/`, + // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. + urlParams: { + force: forceRefresh, + }, + transformResponse: ({ json }: JsonResponse) => + json.result.map((value: string) => ({ + value, + label: value, + title: value, + })), + }), + serializeQueryArgs: ({ queryArgs: { dbId } }) => ({ + dbId, + }), + }), + }), +}); + +export const { useLazySchemasQuery, useSchemasQuery } = schemaApi; + +const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { + const isMountedRef = useRef(false); const { dbId, onSuccess, onError } = options || {}; - const forceRefreshRef = useRef(false); - const params = { dbId }; - const result = useQuery( - ['schemas', { dbId }], - () => fetchSchemas({ ...params, forceRefresh: forceRefreshRef.current }), + const [trigger] = useLazySchemasQuery(); + const result = useSchemasQuery( + { dbId, forceRefresh: false }, { - select: ({ json }) => - json.result.map((value: string) => ({ - value, - label: value, - title: value, - })), - enabled: Boolean(dbId), - onSuccess, - onError, - onSettled: () => { - forceRefreshRef.current = false; - }, + skip: !dbId, + }, + ); + + const handleOnSuccess = useEffectEvent( + (data: SchemaOption[], isRefetched: boolean) => { + onSuccess?.(data, isRefetched); }, ); + const handleOnError = useEffectEvent(() => { + onError?.(); + }); + + const refetch = useCallback(() => { + if (dbId) { + trigger({ dbId, forceRefresh: true }).then( + ({ isSuccess, isError, data }) => { + if (isSuccess) { + handleOnSuccess(data || EMPTY_SCHEMAS, true); + } + if (isError) { + handleOnError(); + } + }, + ); + } + }, [dbId, handleOnError, handleOnSuccess, trigger]); + + useEffect(() => { + if (isMountedRef.current) { + const { requestId, isSuccess, isError, isFetching, data, originalArgs } = + result; + if (!originalArgs?.forceRefresh && requestId && !isFetching) { + if (isSuccess) { + handleOnSuccess(data || EMPTY_SCHEMAS, false); + } + if (isError) { + handleOnError(); + } + } + } else { + isMountedRef.current = true; + } + }, [result, handleOnSuccess, handleOnError]); + return { ...result, - refetch: () => { - forceRefreshRef.current = true; - return result.refetch(); - }, + refetch, }; } diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 8cc0791d5069..e461b4f86b2b 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -19,7 +19,11 @@ import rison from 'rison'; import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; -import QueryProvider, { queryClient } from 'src/views/QueryProvider'; +import { + createWrapper, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import { useTables } from './tables'; const fakeApiResult = { @@ -67,14 +71,11 @@ const expectedHasMoreData = { }; describe('useTables hook', () => { - beforeEach(() => { - queryClient.clear(); - jest.useFakeTimers(); - }); - afterEach(() => { fetchMock.reset(); - jest.useRealTimers(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('returns api response mapping json options', async () => { @@ -86,19 +87,20 @@ describe('useTables hook', () => { fetchMock.get(schemaApiRoute, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); expect( fetchMock.calls( @@ -108,19 +110,20 @@ describe('useTables hook', () => { })}`, ).length, ).toBe(1); - expect(result.current.data).toEqual(expectedData); - await act(async () => { + act(() => { result.current.refetch(); }); + await waitFor(() => + expect( + fetchMock.calls( + `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ + force: true, + schema_name: expectedSchema, + })}`, + ).length, + ).toBe(1), + ); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - expect( - fetchMock.calls( - `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ - force: true, - schema_name: expectedSchema, - })}`, - ).length, - ).toBe(1); expect(result.current.data).toEqual(expectedData); }); @@ -133,20 +136,20 @@ describe('useTables hook', () => { fetchMock.get(schemaApiRoute, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: unexpectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); expect(result.current.data).toEqual(undefined); expect( fetchMock.calls( @@ -166,20 +169,20 @@ describe('useTables hook', () => { fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(1)); expect(result.current.data).toEqual(expectedHasMoreData); }); @@ -191,58 +194,79 @@ describe('useTables hook', () => { fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result, rerender } = renderHook( + const { result, rerender, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(1)); rerender(); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(tableApiRoute).length).toBe(1); - expect(result.current.data).toEqual(expectedData); }); test('returns refreshed data after expires', async () => { const expectDbId = 'db1'; const expectedSchema = 'schema1'; const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; - fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(tableApiRoute, url => + url.includes(expectedSchema) ? fakeApiResult : fakeHasMoreApiResult, + ); fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result, rerender } = renderHook( - () => + const { result, rerender, waitFor } = renderHook( + ({ schema }) => useTables({ dbId: expectDbId, - schema: expectedSchema, + schema, }), { - wrapper: QueryProvider, + initialProps: { schema: expectedSchema }, + wrapper: createWrapper({ + useRedux: true, + store, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(tableApiRoute).length).toBe(1); - queryClient.clear(); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + rerender({ schema: 'schema2' }); + await waitFor(() => + expect(result.current.data).toEqual(expectedHasMoreData), + ); expect(fetchMock.calls(tableApiRoute).length).toBe(2); - expect(result.current.data).toEqual(expectedData); + + rerender({ schema: expectedSchema }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + expect(fetchMock.calls(tableApiRoute).length).toBe(2); + + // clean up cache + act(() => { + store.dispatch(api.util.invalidateTags(['Tables'])); + }); + + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(3)); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + + rerender({ schema: 'schema2' }); + await waitFor(() => + expect(result.current.data).toEqual(expectedHasMoreData), + ); + expect(fetchMock.calls(tableApiRoute).length).toBe(4); + + rerender({ schema: expectedSchema }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + expect(fetchMock.calls(tableApiRoute).length).toBe(4); }); }); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 34d286c9456a..5d28cba4880f 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -16,18 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { useRef, useMemo } from 'react'; -import { useQuery, UseQueryOptions } from 'react-query'; -import rison from 'rison'; -import { SupersetClient } from '@superset-ui/core'; +import { useCallback, useMemo, useEffect, useRef } from 'react'; +import useEffectEvent from 'src/hooks/useEffectEvent'; +import { api } from './queryApi'; import { useSchemas } from './schemas'; -export type FetchTablesQueryParams = { - dbId?: string | number; - schema?: string; - forceRefresh?: boolean; -}; export interface Table { label: string; value: string; @@ -41,7 +35,7 @@ export interface Table { }; } -type QueryData = { +type QueryResponse = { json: { count: number; result: Table[]; @@ -54,28 +48,44 @@ export type Data = { hasMore: boolean; }; -export function fetchTables({ - dbId, - schema, - forceRefresh, -}: FetchTablesQueryParams) { - const encodedSchema = schema ? encodeURIComponent(schema) : ''; - const params = rison.encode({ - force: forceRefresh, - schema_name: encodedSchema, - }); +export type FetchTablesQueryParams = { + dbId?: string | number; + schema?: string; + forceRefresh?: boolean; + onSuccess?: (data: Data, isRefetched: boolean) => void; + onError?: (error: Response) => void; +}; - // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. - const endpoint = `/api/v1/database/${ - dbId ?? 'undefined' - }/tables/?q=${params}`; - return SupersetClient.get({ endpoint }) as Promise; -} +type Params = Omit; + +const tableApi = api.injectEndpoints({ + endpoints: builder => ({ + tables: builder.query({ + providesTags: ['Tables'], + query: ({ dbId, schema, forceRefresh }) => ({ + endpoint: `/api/v1/database/${dbId ?? 'undefined'}/tables/`, + // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. + urlParams: { + force: forceRefresh, + schema_name: schema ? encodeURIComponent(schema) : '', + }, + transformResponse: ({ json }: QueryResponse) => ({ + options: json.result, + hasMore: json.count > json.result.length, + }), + }), + serializeQueryArgs: ({ queryArgs: { dbId, schema } }) => ({ + dbId, + schema, + }), + }), + }), +}); -type Params = FetchTablesQueryParams & - Pick, 'onSuccess' | 'onError'>; +export const { useLazyTablesQuery, useTablesQuery } = tableApi; export function useTables(options: Params) { + const isMountedRef = useRef(false); const { data: schemaOptions, isFetching } = useSchemas({ dbId: options.dbId, }); @@ -84,32 +94,68 @@ export function useTables(options: Params) { [schemaOptions], ); const { dbId, schema, onSuccess, onError } = options || {}; - const forceRefreshRef = useRef(false); - const params = { dbId, schema }; - const result = useQuery( - ['tables', { dbId, schema }], - () => fetchTables({ ...params, forceRefresh: forceRefreshRef.current }), + + const enabled = Boolean( + dbId && schema && !isFetching && schemaOptionsMap.has(schema), + ); + + const result = useTablesQuery( + { dbId, schema, forceRefresh: false }, { - select: ({ json }) => ({ - options: json.result, - hasMore: json.count > json.result.length, - }), - enabled: Boolean( - dbId && schema && !isFetching && schemaOptionsMap.has(schema), - ), - onSuccess, - onError, - onSettled: () => { - forceRefreshRef.current = false; - }, + skip: !enabled, }, ); + const [trigger] = useLazyTablesQuery(); + + const handleOnSuccess = useEffectEvent((data: Data, isRefetched: boolean) => { + onSuccess?.(data, isRefetched); + }); + + const handleOnError = useEffectEvent((error: Response) => { + onError?.(error); + }); + + const refetch = useCallback(() => { + if (enabled) { + trigger({ dbId, schema, forceRefresh: true }).then( + ({ isSuccess, isError, data, error }) => { + if (isSuccess && data) { + handleOnSuccess(data, true); + } + if (isError) { + handleOnError(error as Response); + } + }, + ); + } + }, [dbId, schema, enabled, handleOnSuccess, handleOnError, trigger]); + + useEffect(() => { + if (isMountedRef.current) { + const { + requestId, + isSuccess, + isError, + isFetching, + data, + error, + originalArgs, + } = result; + if (!originalArgs?.forceRefresh && requestId && !isFetching) { + if (isSuccess && data) { + handleOnSuccess(data, false); + } + if (isError) { + handleOnError(error as Response); + } + } + } else { + isMountedRef.current = true; + } + }, [result, handleOnSuccess, handleOnError]); return { ...result, - refetch: () => { - forceRefreshRef.current = true; - return result.refetch(); - }, + refetch, }; } diff --git a/superset-frontend/src/views/App.tsx b/superset-frontend/src/views/App.tsx index 3443e336e526..a3c0715f39ea 100644 --- a/superset-frontend/src/views/App.tsx +++ b/superset-frontend/src/views/App.tsx @@ -40,7 +40,6 @@ import { logEvent } from 'src/logger/actions'; import { store } from 'src/views/store'; import { RootContextProviders } from './RootContextProviders'; import { ScrollToTop } from './ScrollToTop'; -import QueryProvider from './QueryProvider'; setupApp(); setupPlugins(); @@ -70,31 +69,29 @@ const LocationPathnameLogger = () => { }; const App = () => ( - - - - - - - - - {routes.map(({ path, Component, props = {}, Fallback = Loading }) => ( - - }> - - - - - - ))} - - - - - + + + + + + + + {routes.map(({ path, Component, props = {}, Fallback = Loading }) => ( + + }> + + + + + + ))} + + + + ); export default hot(App); diff --git a/superset-frontend/src/views/QueryProvider.tsx b/superset-frontend/src/views/QueryProvider.tsx deleted file mode 100644 index 9fef2022d4c3..000000000000 --- a/superset-frontend/src/views/QueryProvider.tsx +++ /dev/null @@ -1,43 +0,0 @@ -/** - * 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 { QueryClient, QueryClientProvider } from 'react-query'; - -export const queryClient = new QueryClient({ - defaultOptions: { - queries: { - staleTime: Infinity, - retry: false, - retryOnMount: false, - refetchOnMount: false, - refetchOnReconnect: false, - refetchOnWindowFocus: false, - }, - }, -}); - -type Props = { - children: React.ReactNode; -}; - -const Queryprovider: React.FC = ({ children }) => ( - {children} -); - -export default Queryprovider; diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts index 9fe1443bf952..78d5a99714f0 100644 --- a/superset-frontend/src/views/store.ts +++ b/superset-frontend/src/views/store.ts @@ -17,6 +17,7 @@ * under the License. */ import { configureStore, ConfigureStoreOptions, Store } from '@reduxjs/toolkit'; +import { api } from 'src/hooks/apiResources/queryApi'; import messageToastReducer from 'src/components/MessageToasts/reducers'; import charts from 'src/components/Chart/chartReducer'; import dataMask from 'src/dataMask/reducer'; @@ -133,6 +134,7 @@ export function setupStore({ return configureStore({ preloadedState: initialState, reducer: { + [api.reducerPath]: api.reducer, ...rootReducers, }, middleware: getDefaultMiddleware => @@ -146,7 +148,7 @@ export function setupStore({ ignoredPaths: [/queryController/g], warnAfter: 200, }, - }).concat(logger), + }).concat(logger, api.middleware), devTools: process.env.WEBPACK_MODE === 'development' && !disableDebugger, ...overrides, });