diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index a4774cd390afa..2c816d0e8447b 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -41,15 +41,25 @@ const middlewares = [thunk]; const mockStore = configureStore(middlewares); const store = mockStore(initialState); -fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] }); -fetchMock.get('glob:*/api/v1/database/*/tables/*', { - count: 1, - result: [ - { - label: 'ab_user', - value: 'ab_user', - }, - ], +beforeEach(() => { + fetchMock.get('glob:*/api/v1/database/?*', { result: [] }); + fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { + count: 2, + result: ['main', 'new_schema'], + }); + fetchMock.get('glob:*/api/v1/database/*/tables/*', { + count: 1, + result: [ + { + label: 'ab_user', + value: 'ab_user', + }, + ], + }); +}); + +afterEach(() => { + fetchMock.restore(); }); const renderAndWait = (props, store) => @@ -110,8 +120,9 @@ test('should toggle the table when the header is clicked', async () => { userEvent.click(header); await waitFor(() => { - expect(store.getActions()).toHaveLength(4); - expect(store.getActions()[3].type).toEqual('COLLAPSE_TABLE'); + expect(store.getActions()[store.getActions().length - 1].type).toEqual( + 'COLLAPSE_TABLE', + ); }); }); @@ -129,14 +140,55 @@ test('When changing database the table list must be updated', async () => { database_name: 'new_db', backend: 'postgresql', }} - queryEditor={{ ...mockedProps.queryEditor, schema: 'new_schema' }} + queryEditorId={defaultQueryEditor.id} tables={[{ ...mockedProps.tables[0], dbId: 2, name: 'new_table' }]} />, { useRedux: true, - initialState, + store: mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + schema: 'new_schema', + }, + }, + }), }, ); expect(await screen.findByText(/new_db/i)).toBeInTheDocument(); expect(await screen.findByText(/new_table/i)).toBeInTheDocument(); }); + +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, + }, + }, + }), + ); + + expect(await screen.findByText(/Database/i)).toBeInTheDocument(); + expect(screen.queryByText(/None/i)).toBeInTheDocument(); + expect(fetchMock.calls()).not.toContainEqual( + expect.arrayContaining([ + expect.stringContaining( + `/tables/${mockedProps.database.id}/${invalidSchemaName}/`, + ), + ]), + ); + rerender(); + // Deselect the deprecated schema selection + await waitFor(() => + expect(screen.queryByText(/None/i)).not.toBeInTheDocument(), + ); +}); diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 0b2a7b521f2a3..57b6a539ae188 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -18,14 +18,13 @@ */ import React from 'react'; +import fetchMock from 'fetch-mock'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; -import { SupersetClient } from '@superset-ui/core'; +import { queryClient } from 'src/views/QueryProvider'; import userEvent from '@testing-library/user-event'; import DatabaseSelector, { DatabaseSelectorProps } from '.'; import { EmptyStateSmall } from '../EmptyState'; -const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); - const createProps = (): DatabaseSelectorProps => ({ db: { id: 1, @@ -35,7 +34,7 @@ const createProps = (): DatabaseSelectorProps => ({ formMode: false, isDatabaseSelectEnabled: true, readOnly: false, - schema: undefined, + schema: 'public', sqlLabMode: true, getDbList: jest.fn(), handleError: jest.fn(), @@ -44,124 +43,133 @@ const createProps = (): DatabaseSelectorProps => ({ onSchemasLoad: jest.fn(), }); -beforeEach(() => { - jest.resetAllMocks(); - SupersetClientGet.mockImplementation( - async ({ endpoint }: { endpoint: string }) => { - if (endpoint.includes('schemas')) { - return { - json: { result: ['information_schema', 'public'] }, - } as any; - } - if (endpoint.includes('/function_names')) { - return { - json: { function_names: [] }, - } as any; - } - return { - json: { - count: 2, - description_columns: {}, - ids: [1, 2], - label_columns: { - allow_file_upload: 'Allow Csv Upload', - allow_ctas: 'Allow Ctas', - allow_cvas: 'Allow Cvas', - allow_dml: 'Allow Dml', - allow_run_async: 'Allow Run Async', - allows_cost_estimate: 'Allows Cost Estimate', - allows_subquery: 'Allows Subquery', - allows_virtual_table_explore: 'Allows Virtual Table Explore', - disable_data_preview: 'Disables SQL Lab Data Preview', - backend: 'Backend', - changed_on: 'Changed On', - changed_on_delta_humanized: 'Changed On Delta Humanized', - 'created_by.first_name': 'Created By First Name', - 'created_by.last_name': 'Created By Last Name', - database_name: 'Database Name', - explore_database_id: 'Explore Database Id', - expose_in_sqllab: 'Expose In Sqllab', - force_ctas_schema: 'Force Ctas Schema', - id: 'Id', - }, - list_columns: [ - 'allow_file_upload', - 'allow_ctas', - 'allow_cvas', - 'allow_dml', - 'allow_run_async', - 'allows_cost_estimate', - 'allows_subquery', - 'allows_virtual_table_explore', - 'disable_data_preview', - 'backend', - 'changed_on', - 'changed_on_delta_humanized', - 'created_by.first_name', - 'created_by.last_name', - 'database_name', - 'explore_database_id', - 'expose_in_sqllab', - 'force_ctas_schema', - 'id', - ], - list_title: 'List Database', - order_columns: [ - 'allow_file_upload', - 'allow_dml', - 'allow_run_async', - 'changed_on', - 'changed_on_delta_humanized', - 'created_by.first_name', - 'database_name', - 'expose_in_sqllab', - ], - result: [ - { - allow_file_upload: false, - allow_ctas: false, - allow_cvas: false, - allow_dml: false, - allow_run_async: false, - allows_cost_estimate: null, - allows_subquery: true, - allows_virtual_table_explore: true, - disable_data_preview: false, - backend: 'postgresql', - changed_on: '2021-03-09T19:02:07.141095', - changed_on_delta_humanized: 'a day ago', - created_by: null, - database_name: 'test-postgres', - explore_database_id: 1, - expose_in_sqllab: true, - force_ctas_schema: null, - id: 1, - }, - { - allow_csv_upload: false, - allow_ctas: false, - allow_cvas: false, - allow_dml: false, - allow_run_async: false, - allows_cost_estimate: null, - allows_subquery: true, - allows_virtual_table_explore: true, - disable_data_preview: false, - backend: 'mysql', - changed_on: '2021-03-09T19:02:07.141095', - changed_on_delta_humanized: 'a day ago', - created_by: null, - database_name: 'test-mysql', - explore_database_id: 1, - expose_in_sqllab: true, - force_ctas_schema: null, - id: 2, - }, - ], - }, - } as any; +const fakeDatabaseApiResult = { + count: 2, + description_columns: {}, + ids: [1, 2], + label_columns: { + allow_file_upload: 'Allow Csv Upload', + allow_ctas: 'Allow Ctas', + allow_cvas: 'Allow Cvas', + allow_dml: 'Allow Dml', + allow_run_async: 'Allow Run Async', + allows_cost_estimate: 'Allows Cost Estimate', + allows_subquery: 'Allows Subquery', + allows_virtual_table_explore: 'Allows Virtual Table Explore', + disable_data_preview: 'Disables SQL Lab Data Preview', + backend: 'Backend', + changed_on: 'Changed On', + changed_on_delta_humanized: 'Changed On Delta Humanized', + 'created_by.first_name': 'Created By First Name', + 'created_by.last_name': 'Created By Last Name', + database_name: 'Database Name', + explore_database_id: 'Explore Database Id', + expose_in_sqllab: 'Expose In Sqllab', + force_ctas_schema: 'Force Ctas Schema', + id: 'Id', + }, + list_columns: [ + 'allow_file_upload', + 'allow_ctas', + 'allow_cvas', + 'allow_dml', + 'allow_run_async', + 'allows_cost_estimate', + 'allows_subquery', + 'allows_virtual_table_explore', + 'disable_data_preview', + 'backend', + 'changed_on', + 'changed_on_delta_humanized', + 'created_by.first_name', + 'created_by.last_name', + 'database_name', + 'explore_database_id', + 'expose_in_sqllab', + 'force_ctas_schema', + 'id', + ], + list_title: 'List Database', + order_columns: [ + 'allow_file_upload', + 'allow_dml', + 'allow_run_async', + 'changed_on', + 'changed_on_delta_humanized', + 'created_by.first_name', + 'database_name', + 'expose_in_sqllab', + ], + result: [ + { + allow_file_upload: false, + allow_ctas: false, + allow_cvas: false, + allow_dml: false, + allow_run_async: false, + allows_cost_estimate: null, + allows_subquery: true, + allows_virtual_table_explore: true, + disable_data_preview: false, + backend: 'postgresql', + changed_on: '2021-03-09T19:02:07.141095', + changed_on_delta_humanized: 'a day ago', + created_by: null, + database_name: 'test-postgres', + explore_database_id: 1, + expose_in_sqllab: true, + force_ctas_schema: null, + id: 1, }, - ); + { + allow_csv_upload: false, + allow_ctas: false, + allow_cvas: false, + allow_dml: false, + allow_run_async: false, + allows_cost_estimate: null, + allows_subquery: true, + allows_virtual_table_explore: true, + disable_data_preview: false, + backend: 'mysql', + changed_on: '2021-03-09T19:02:07.141095', + changed_on_delta_humanized: 'a day ago', + created_by: null, + database_name: 'test-mysql', + explore_database_id: 1, + expose_in_sqllab: true, + force_ctas_schema: null, + id: 2, + }, + ], +}; + +const fakeSchemaApiResult = { + count: 2, + result: ['information_schema', 'public'], +}; + +const fakeFunctionNamesApiResult = { + function_names: [], +}; + +const databaseApiRoute = 'glob:*/api/v1/database/?*'; +const schemaApiRoute = 'glob:*/api/v1/database/*/schemas/?*'; +const tablesApiRoute = 'glob:*/api/v1/database/*/tables/*'; + +function setupFetchMock() { + fetchMock.get(databaseApiRoute, fakeDatabaseApiResult); + fetchMock.get(schemaApiRoute, fakeSchemaApiResult); + fetchMock.get(tablesApiRoute, fakeFunctionNamesApiResult); +} + +beforeEach(() => { + queryClient.clear(); + setupFetchMock(); +}); + +afterEach(() => { + fetchMock.reset(); }); test('Should render', async () => { @@ -175,6 +183,8 @@ test('Refresh should work', async () => { render(, { useRedux: true }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(0); + const select = screen.getByRole('combobox', { name: 'Select schema or type schema name', }); @@ -182,23 +192,22 @@ test('Refresh should work', async () => { userEvent.click(select); await waitFor(() => { - expect(SupersetClientGet).toBeCalledTimes(2); - expect(props.getDbList).toBeCalledTimes(0); + expect(fetchMock.calls(databaseApiRoute).length).toBe(1); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); expect(props.handleError).toBeCalledTimes(0); expect(props.onDbChange).toBeCalledTimes(0); expect(props.onSchemaChange).toBeCalledTimes(0); - expect(props.onSchemasLoad).toBeCalledTimes(0); }); + // click schema reload userEvent.click(screen.getByRole('button', { name: 'refresh' })); await waitFor(() => { - expect(SupersetClientGet).toBeCalledTimes(3); - expect(props.getDbList).toBeCalledTimes(1); + expect(fetchMock.calls(databaseApiRoute).length).toBe(1); + expect(fetchMock.calls(schemaApiRoute).length).toBe(2); expect(props.handleError).toBeCalledTimes(0); expect(props.onDbChange).toBeCalledTimes(0); expect(props.onSchemaChange).toBeCalledTimes(0); - expect(props.onSchemasLoad).toBeCalledTimes(2); }); }); @@ -214,9 +223,10 @@ test('Should database select display options', async () => { }); test('should show empty state if there are no options', async () => { - SupersetClientGet.mockImplementation( - async () => ({ json: { result: [] } } as any), - ); + fetchMock.reset(); + fetchMock.get(databaseApiRoute, { result: [] }); + fetchMock.get(schemaApiRoute, { result: [] }); + fetchMock.get(tablesApiRoute, { result: [] }); const props = createProps(); render( ` @@ -86,8 +83,6 @@ export type DatabaseObject = { backend: string; }; -type SchemaValue = { label: string; value: string }; - export interface DatabaseSelectorProps { db?: DatabaseObject | null; emptyState?: ReactNode; @@ -119,6 +114,8 @@ const SelectLabel = ({ ); +const EMPTY_SCHEMA_OPTIONS: SchemaOption[] = []; + export default function DatabaseSelector({ db, formMode = false, @@ -134,13 +131,10 @@ export default function DatabaseSelector({ schema, sqlLabMode = false, }: DatabaseSelectorProps) { - const [loadingSchemas, setLoadingSchemas] = useState(false); - const [schemaOptions, setSchemaOptions] = useState([]); const [currentDb, setCurrentDb] = useState(); - const [currentSchema, setCurrentSchema] = useState( - schema ? { label: schema, value: schema } : undefined, + const [currentSchema, setCurrentSchema] = useState( + schema ? { label: schema, value: schema, title: schema } : undefined, ); - const [refresh, setRefresh] = useState(0); const { addSuccessToast } = useToasts(); const loadDatabases = useMemo( @@ -221,48 +215,37 @@ export default function DatabaseSelector({ ); }, [db]); - function changeSchema(schema: SchemaValue) { + function changeSchema(schema: SchemaOption | undefined) { setCurrentSchema(schema); if (onSchemaChange) { - onSchemaChange(schema.value); + onSchemaChange(schema?.value); } } - useEffect(() => { - if (currentDb) { - setLoadingSchemas(true); - const queryParams = rison.encode({ force: refresh > 0 }); - const endpoint = `/api/v1/database/${currentDb.value}/schemas/?q=${queryParams}`; + const { + data, + isFetching: loadingSchemas, + isFetched, + refetch, + } = useSchemas({ + dbId: currentDb?.value, + onSuccess: data => { + onSchemasLoad?.(data); - // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. - SupersetClient.get({ endpoint }) - .then(({ json }) => { - const options = json.result.map((s: string) => ({ - value: s, - label: s, - title: s, - })); - if (onSchemasLoad) { - onSchemasLoad(options); - } - setSchemaOptions(options); - setLoadingSchemas(false); - if (options.length === 1) changeSchema(options[0]); - if (refresh > 0) addSuccessToast(t('List refreshed')); - }) - .catch(err => { - setLoadingSchemas(false); - getClientErrorObject(err).then(clientError => { - handleError( - getClientErrorMessage( - t('There was an error loading the schemas'), - clientError, - ), - ); - }); - }); - } - }, [currentDb, onSchemasLoad, refresh]); + if (data.length === 1) { + changeSchema(data[0]); + } else if (!data.find(schemaOption => schema === schemaOption.value)) { + changeSchema(undefined); + } + + if (isFetched) { + addSuccessToast('List refreshed'); + } + }, + onError: () => handleError(t('There was an error loading the schemas')), + }); + + const schemaOptions = data || EMPTY_SCHEMA_OPTIONS; function changeDataBase( value: { label: string; value: number }, @@ -309,7 +292,7 @@ export default function DatabaseSelector({ function renderSchemaSelect() { const refreshIcon = !readOnly && ( setRefresh(refresh + 1)} + onClick={() => refetch()} tooltipContent={t('Force refresh schema list')} /> ); @@ -323,7 +306,7 @@ export default function DatabaseSelector({ name="select-schema" notFoundContent={t('No compatible schema found')} placeholder={t('Select schema or type schema name')} - onChange={item => changeSchema(item as SchemaValue)} + onChange={item => changeSchema(item as SchemaOption)} options={schemaOptions} showSearch value={currentSchema} diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 09cb29a3857f5..b4b67deb92dca 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -19,13 +19,12 @@ import React from 'react'; import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; -import { SupersetClient } from '@superset-ui/core'; +import { queryClient } from 'src/views/QueryProvider'; +import fetchMock from 'fetch-mock'; import { act } from 'react-dom/test-utils'; import userEvent from '@testing-library/user-event'; import TableSelector, { TableSelectorMultiple } from '.'; -const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); - const createProps = (props = {}) => ({ database: { id: 1, @@ -37,37 +36,43 @@ const createProps = (props = {}) => ({ ...props, }); -afterEach(() => { - jest.clearAllMocks(); -}); - -const getSchemaMockFunction = async () => +const getSchemaMockFunction = () => ({ - json: { - result: ['schema_a', 'schema_b'], - }, + result: ['schema_a', 'schema_b'], } as any); -const getTableMockFunction = async () => +const getTableMockFunction = () => ({ - json: { - count: 4, - result: [ - { label: 'table_a', value: 'table_a' }, - { label: 'table_b', value: 'table_b' }, - { label: 'table_c', value: 'table_c' }, - { label: 'table_d', value: 'table_d' }, - ], - }, + count: 4, + result: [ + { label: 'table_a', value: 'table_a' }, + { label: 'table_b', value: 'table_b' }, + { label: 'table_c', value: 'table_c' }, + { label: 'table_d', value: 'table_d' }, + ], } as any); +const databaseApiRoute = 'glob:*/api/v1/database/?*'; +const schemaApiRoute = 'glob:*/api/v1/database/*/schemas/?*'; +const tablesApiRoute = 'glob:*/api/v1/database/*/tables/*'; + const getSelectItemContainer = (select: HTMLElement) => select.parentElement?.parentElement?.getElementsByClassName( 'ant-select-selection-item', ); +beforeEach(() => { + queryClient.clear(); + fetchMock.get(databaseApiRoute, { result: [] }); +}); + +afterEach(() => { + fetchMock.reset(); +}); + test('renders with default props', async () => { - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(schemaApiRoute, { result: [] }); + fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); render(, { useRedux: true }); @@ -88,7 +93,8 @@ test('renders with default props', async () => { }); test('renders table options', async () => { - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(schemaApiRoute, { result: ['test_schema'] }); + fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); render(, { useRedux: true }); @@ -105,7 +111,8 @@ test('renders table options', async () => { }); test('renders disabled without schema', async () => { - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(schemaApiRoute, { result: [] }); + fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); render(, { useRedux: true }); @@ -118,7 +125,7 @@ test('renders disabled without schema', async () => { }); test('table options are notified after schema selection', async () => { - SupersetClientGet.mockImplementation(getSchemaMockFunction); + fetchMock.get(schemaApiRoute, getSchemaMockFunction()); const callback = jest.fn(); const props = createProps({ @@ -142,7 +149,7 @@ test('table options are notified after schema selection', async () => { await screen.findByRole('option', { name: 'schema_b' }), ).toBeInTheDocument(); - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(tablesApiRoute, getTableMockFunction()); act(() => { userEvent.click(screen.getAllByText('schema_a')[1]); @@ -159,7 +166,8 @@ test('table options are notified after schema selection', async () => { }); test('table select retain value if not in SQL Lab mode', async () => { - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(schemaApiRoute, { result: ['test_schema'] }); + fetchMock.get(tablesApiRoute, getTableMockFunction()); const callback = jest.fn(); const props = createProps({ @@ -182,7 +190,7 @@ test('table select retain value if not in SQL Lab mode', async () => { await screen.findByRole('option', { name: 'table_a' }), ).toBeInTheDocument(); - act(() => { + await waitFor(() => { userEvent.click(screen.getAllByText('table_a')[1]); }); @@ -199,7 +207,8 @@ test('table select retain value if not in SQL Lab mode', async () => { }); test('table multi select retain all the values selected', async () => { - SupersetClientGet.mockImplementation(getTableMockFunction); + fetchMock.get(schemaApiRoute, { result: ['test_schema'] }); + fetchMock.get(tablesApiRoute, getTableMockFunction()); const callback = jest.fn(); const props = createProps({ @@ -217,23 +226,19 @@ test('table multi select retain all the values selected', async () => { userEvent.click(tableSelect); - act(() => { - const item = screen.getAllByText('table_b'); + await waitFor(async () => { + const item = await screen.findAllByText('table_b'); userEvent.click(item[item.length - 1]); }); - act(() => { - const item = screen.getAllByText('table_c'); + await waitFor(async () => { + const item = await screen.findAllByText('table_c'); userEvent.click(item[item.length - 1]); }); - expect(screen.getByRole('option', { name: 'table_b' })).toHaveAttribute( - 'aria-selected', - 'true', - ); + const selection1 = await screen.findByRole('option', { name: 'table_b' }); + expect(selection1).toHaveAttribute('aria-selected', 'true'); - expect(screen.getByRole('option', { name: 'table_c' })).toHaveAttribute( - 'aria-selected', - 'true', - ); + const selection2 = await screen.findByRole('option', { name: 'table_c' }); + expect(selection2).toHaveAttribute('aria-selected', 'true'); }); diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 1cf65bbc688af..ffb45cc8fed9f 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -275,26 +275,6 @@ const TableSelector: FunctionComponent = ({ internalTableChange(value); }; - function renderDatabaseSelector() { - return ( - - ); - } - const handleFilterOption = useMemo( () => (search: string, option: TableOption) => { const searchValue = search.trim().toLowerCase(); @@ -346,7 +326,21 @@ const TableSelector: FunctionComponent = ({ return ( - {renderDatabaseSelector()} + {sqlLabMode && !formMode &&
} {renderTableSelect()} diff --git a/superset-frontend/src/hooks/apiResources/index.ts b/superset-frontend/src/hooks/apiResources/index.ts index 32a6418dc07c4..81d77b5d11a50 100644 --- a/superset-frontend/src/hooks/apiResources/index.ts +++ b/superset-frontend/src/hooks/apiResources/index.ts @@ -29,3 +29,4 @@ export { export * from './charts'; export * from './dashboards'; export * from './tables'; +export * from './schemas'; diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts new file mode 100644 index 0000000000000..59d00a5dc71bc --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -0,0 +1,138 @@ +/** + * 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 fetchMock from 'fetch-mock'; +import { act, renderHook } from '@testing-library/react-hooks'; +import QueryProvider, { queryClient } from 'src/views/QueryProvider'; +import { useSchemas } from './schemas'; + +const fakeApiResult = { + result: ['test schema 1', 'test schema b'], +}; + +const expectedResult = fakeApiResult.result.map((value: string) => ({ + value, + label: value, + title: value, +})); + +describe('useSchemas hook', () => { + beforeEach(() => { + queryClient.clear(); + jest.useFakeTimers(); + }); + + afterEach(() => { + fetchMock.reset(); + jest.useRealTimers(); + }); + + test('returns api response mapping json result', async () => { + const expectDbId = 'db1'; + const forceRefresh = false; + const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; + fetchMock.get(schemaApiRoute, fakeApiResult); + const { result } = renderHook( + () => + useSchemas({ + dbId: expectDbId, + }), + { + wrapper: QueryProvider, + }, + ); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + expect( + fetchMock.calls( + `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ + force: forceRefresh, + })}`, + ).length, + ).toBe(1); + expect(result.current.data).toEqual(expectedResult); + await act(async () => { + result.current.refetch(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + expect( + fetchMock.calls( + `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ + force: true, + })}`, + ).length, + ).toBe(1); + expect(result.current.data).toEqual(expectedResult); + }); + + test('returns cached data without api request', async () => { + const expectDbId = 'db1'; + const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; + fetchMock.get(schemaApiRoute, fakeApiResult); + const { result, rerender } = renderHook( + () => + useSchemas({ + dbId: expectDbId, + }), + { + wrapper: QueryProvider, + }, + ); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + rerender(); + 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( + () => + useSchemas({ + dbId: expectDbId, + }), + { + wrapper: QueryProvider, + }, + ); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + rerender(); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + queryClient.clear(); + rerender(); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + expect(result.current.data).toEqual(expectedResult); + }); +}); diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts new file mode 100644 index 0000000000000..34a50ca4e9896 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -0,0 +1,80 @@ +/** + * 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 { 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; +}; + +export type SchemaOption = { + value: string; + label: string; + 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; +} + +type Params = FetchSchemasQueryParams & + Pick, 'onSuccess' | 'onError'>; + +export function useSchemas(options: Params) { + const { dbId, onSuccess, onError } = options || {}; + const forceRefreshRef = useRef(false); + const params = { dbId }; + const result = useQuery( + ['schemas', { dbId }], + () => fetchSchemas({ ...params, forceRefresh: forceRefreshRef.current }), + { + select: ({ json }) => + json.result.map((value: string) => ({ + value, + label: value, + title: value, + })), + enabled: Boolean(dbId), + onSuccess, + onError, + onSettled: () => { + forceRefreshRef.current = false; + }, + }, + ); + + return { + ...result, + refetch: () => { + forceRefreshRef.current = true; + return result.refetch(); + }, + }; +} diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 49305f5f62ebb..8cc0791d50694 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -16,78 +16,76 @@ * specific language governing permissions and limitations * under the License. */ +import rison from 'rison'; +import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; -import { SupersetClient } from '@superset-ui/core'; import QueryProvider, { queryClient } from 'src/views/QueryProvider'; import { useTables } from './tables'; const fakeApiResult = { - json: { - count: 2, - result: [ - { - id: 1, - name: 'fake api result1', - label: 'fake api label1', - }, - { - id: 2, - name: 'fake api result2', - label: 'fake api label2', - }, - ], - }, + count: 2, + result: [ + { + id: 1, + name: 'fake api result1', + label: 'fake api label1', + }, + { + id: 2, + name: 'fake api result2', + label: 'fake api label2', + }, + ], }; const fakeHasMoreApiResult = { - json: { - count: 4, - result: [ - { - id: 1, - name: 'fake api result1', - label: 'fake api label1', - }, - { - id: 2, - name: 'fake api result2', - label: 'fake api label2', - }, - ], - }, + count: 4, + result: [ + { + id: 1, + name: 'fake api result1', + label: 'fake api label1', + }, + { + id: 2, + name: 'fake api result2', + label: 'fake api label2', + }, + ], }; +const fakeSchemaApiResult = ['schema1', 'schema2']; + const expectedData = { - options: [...fakeApiResult.json.result], + options: fakeApiResult.result, hasMore: false, }; const expectedHasMoreData = { - options: [...fakeHasMoreApiResult.json.result], + options: fakeHasMoreApiResult.result, hasMore: true, }; -jest.mock('@superset-ui/core', () => ({ - SupersetClient: { - get: jest.fn().mockResolvedValue(fakeApiResult), - }, -})); - describe('useTables hook', () => { beforeEach(() => { - (SupersetClient.get as jest.Mock).mockClear(); queryClient.clear(); jest.useFakeTimers(); }); afterEach(() => { + fetchMock.reset(); jest.useRealTimers(); }); - it('returns api response mapping json options', async () => { + test('returns api response mapping json options', async () => { const expectDbId = 'db1'; - const expectedSchema = 'schemaA'; - const forceRefresh = false; + const expectedSchema = 'schema1'; + const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; + const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; + fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(schemaApiRoute, { + result: fakeSchemaApiResult, + }); const { result } = renderHook( () => useTables({ @@ -101,29 +99,73 @@ describe('useTables hook', () => { await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); - expect(SupersetClient.get).toHaveBeenCalledWith({ - endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!${ - forceRefresh ? 't' : 'f' - },schema_name:${expectedSchema})`, - }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + expect( + fetchMock.calls( + `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ + force: false, + schema_name: expectedSchema, + })}`, + ).length, + ).toBe(1); expect(result.current.data).toEqual(expectedData); await act(async () => { result.current.refetch(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(2); - expect(SupersetClient.get).toHaveBeenCalledWith({ - endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!t,schema_name:${expectedSchema})`, - }); + 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); }); - it('returns hasMore when total is larger than result size', async () => { - (SupersetClient.get as jest.Mock).mockResolvedValueOnce( - fakeHasMoreApiResult, + test('skips the deprecated schema option', async () => { + const expectDbId = 'db1'; + const unexpectedSchema = 'invalid schema'; + const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; + const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; + fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(schemaApiRoute, { + result: fakeSchemaApiResult, + }); + const { result } = renderHook( + () => + useTables({ + dbId: expectDbId, + schema: unexpectedSchema, + }), + { + wrapper: QueryProvider, + }, ); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + expect(result.current.data).toEqual(undefined); + expect( + fetchMock.calls( + `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ + force: false, + schema_name: unexpectedSchema, + })}`, + ).length, + ).toBe(0); + }); + + test('returns hasMore when total is larger than result size', async () => { const expectDbId = 'db1'; - const expectedSchema = 'schemaA'; + const expectedSchema = 'schema2'; + const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; + fetchMock.get(tableApiRoute, fakeHasMoreApiResult); + fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { + result: fakeSchemaApiResult, + }); const { result } = renderHook( () => useTables({ @@ -137,13 +179,18 @@ describe('useTables hook', () => { await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); + expect(fetchMock.calls(tableApiRoute).length).toBe(1); expect(result.current.data).toEqual(expectedHasMoreData); }); - it('returns cached data without api request', async () => { + test('returns cached data without api request', async () => { const expectDbId = 'db1'; - const expectedSchema = 'schemaA'; + const expectedSchema = 'schema1'; + const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; + fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { + result: fakeSchemaApiResult, + }); const { result, rerender } = renderHook( () => useTables({ @@ -157,15 +204,20 @@ describe('useTables hook', () => { await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); + expect(fetchMock.calls(tableApiRoute).length).toBe(1); rerender(); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); + expect(fetchMock.calls(tableApiRoute).length).toBe(1); expect(result.current.data).toEqual(expectedData); }); - it('returns refreshed data after expires', async () => { + test('returns refreshed data after expires', async () => { const expectDbId = 'db1'; - const expectedSchema = 'schemaA'; + const expectedSchema = 'schema1'; + const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; + fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { + result: fakeSchemaApiResult, + }); const { result, rerender } = renderHook( () => useTables({ @@ -179,18 +231,18 @@ describe('useTables hook', () => { await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); + expect(fetchMock.calls(tableApiRoute).length).toBe(1); rerender(); await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(1); + expect(fetchMock.calls(tableApiRoute).length).toBe(1); queryClient.clear(); rerender(); await act(async () => { jest.runAllTimers(); }); - expect(SupersetClient.get).toHaveBeenCalledTimes(2); + expect(fetchMock.calls(tableApiRoute).length).toBe(2); expect(result.current.data).toEqual(expectedData); }); }); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index f65cdf375ad45..34d286c9456a2 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -16,11 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { useRef } from 'react'; +import { useRef, useMemo } from 'react'; import { useQuery, UseQueryOptions } from 'react-query'; import rison from 'rison'; import { SupersetClient } from '@superset-ui/core'; +import { useSchemas } from './schemas'; + export type FetchTablesQueryParams = { dbId?: string | number; schema?: string; @@ -71,9 +73,16 @@ export function fetchTables({ } type Params = FetchTablesQueryParams & - Pick; + Pick, 'onSuccess' | 'onError'>; export function useTables(options: Params) { + const { data: schemaOptions, isFetching } = useSchemas({ + dbId: options.dbId, + }); + const schemaOptionsMap = useMemo( + () => new Set(schemaOptions?.map(({ value }) => value)), + [schemaOptions], + ); const { dbId, schema, onSuccess, onError } = options || {}; const forceRefreshRef = useRef(false); const params = { dbId, schema }; @@ -85,7 +94,9 @@ export function useTables(options: Params) { options: json.result, hasMore: json.count > json.result.length, }), - enabled: Boolean(dbId && schema), + enabled: Boolean( + dbId && schema && !isFetching && schemaOptionsMap.has(schema), + ), onSuccess, onError, onSettled: () => {