diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index eb3e1401ed9b7..9a8649a8623f4 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -39,7 +39,7 @@ type Options = Omit & { store?: Store; }; -const createStore = (initialState: object = {}, reducers: object = {}) => +export const createStore = (initialState: object = {}, reducers: object = {}) => configureStore({ preloadedState: initialState, reducer: { diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx index dbbab0d184bad..deaf17726591e 100644 --- a/superset-frontend/src/SqlLab/App.jsx +++ b/superset-frontend/src/SqlLab/App.jsx @@ -26,7 +26,7 @@ import { initFeatureFlags, isFeatureEnabled } from 'src/featureFlags'; import { setupStore } from 'src/views/store'; import setupExtensions from 'src/setup/setupExtensions'; import getBootstrapData from 'src/utils/getBootstrapData'; -import { api } from 'src/hooks/apiResources/queryApi'; +import { tableApiUtil } from 'src/hooks/apiResources/tables'; import getInitialState from './reducers/getInitialState'; import { reducers } from './reducers/index'; import App from './components/App'; @@ -127,14 +127,14 @@ initialState.sqlLab.tables.forEach( ({ name: table, schema, dbId, persistData }) => { if (dbId && schema && table && persistData?.columns) { store.dispatch( - api.util.upsertQueryData( + tableApiUtil.upsertQueryData( 'tableMetadata', { dbId, schema, table }, persistData, ), ); store.dispatch( - api.util.upsertQueryData( + tableApiUtil.upsertQueryData( 'tableExtendedMetadata', { dbId, schema, table }, {}, diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index d14c848f59870..0adf3f8e1f8fc 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -16,38 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState, useEffect, useRef, useMemo } from 'react'; -import { shallowEqual, useDispatch, useSelector } from 'react-redux'; -import { css, styled, usePrevious, t } from '@superset-ui/core'; +import React, { useState, useEffect, useRef } from 'react'; +import { useDispatch } from 'react-redux'; +import { css, styled, usePrevious } from '@superset-ui/core'; -import { areArraysShallowEqual } from 'src/reduxUtils'; -import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; -import { - queryEditorSetSelectedText, - addTable, - addDangerToast, -} from 'src/SqlLab/actions/sqlLab'; -import { - SCHEMA_AUTOCOMPLETE_SCORE, - TABLE_AUTOCOMPLETE_SCORE, - COLUMN_AUTOCOMPLETE_SCORE, - SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, -} from 'src/SqlLab/constants'; -import { - Editor, - AceCompleterKeyword, - FullSQLEditor as AceEditor, -} from 'src/components/AsyncAceEditor'; +import { queryEditorSetSelectedText } from 'src/SqlLab/actions/sqlLab'; +import { FullSQLEditor as AceEditor } from 'src/components/AsyncAceEditor'; import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; -import { - useSchemas, - useTables, - tableEndpoints, - skipToken, -} from 'src/hooks/apiResources'; -import { useDatabaseFunctionsQuery } from 'src/hooks/apiResources/databaseFunctions'; -import { RootState } from 'src/views/store'; import { useAnnotations } from './useAnnotations'; +import { useKeywords } from './useKeywords'; type HotKey = { key: string; @@ -101,68 +78,10 @@ const AceEditorWrapper = ({ 'schema', 'templateParams', ]); - const { data: schemaOptions } = useSchemas({ - ...(autocomplete && { dbId: queryEditor.dbId }), - }); - const { data: tableData } = useTables({ - ...(autocomplete && { - dbId: queryEditor.dbId, - schema: queryEditor.schema, - }), - }); - - const { data: functionNames, isError } = useDatabaseFunctionsQuery( - { dbId: queryEditor.dbId }, - { skip: !autocomplete || !queryEditor.dbId }, - ); - - useEffect(() => { - if (isError) { - dispatch( - addDangerToast(t('An error occurred while fetching function names.')), - ); - } - }, [dispatch, isError]); const currentSql = queryEditor.sql ?? ''; - // Loading schema, table and column names as auto-completable words - const { schemas, schemaWords } = useMemo( - () => ({ - schemas: schemaOptions ?? [], - schemaWords: (schemaOptions ?? []).map(s => ({ - name: s.label, - value: s.value, - score: SCHEMA_AUTOCOMPLETE_SCORE, - meta: 'schema', - })), - }), - [schemaOptions], - ); - const tables = tableData?.options ?? []; - - const columns = useSelector(state => { - const columns = new Set(); - tables.forEach(({ value }) => { - tableEndpoints.tableMetadata - .select( - queryEditor.dbId && queryEditor.schema - ? { - dbId: queryEditor.dbId, - schema: queryEditor.schema, - table: value, - } - : skipToken, - )(state) - .data?.columns?.forEach(({ name }) => { - columns.add(name); - }); - }); - return [...columns]; - }, shallowEqual); - const [sql, setSql] = useState(currentSql); - const [words, setWords] = useState([]); // The editor changeSelection is called multiple times in a row, // faster than React reconciliation process, so the selected text @@ -173,24 +92,10 @@ const AceEditorWrapper = ({ useEffect(() => { // Making sure no text is selected from previous mount dispatch(queryEditorSetSelectedText(queryEditor, null)); - setAutoCompleter(); }, []); - const prevTables = usePrevious(tables) ?? []; - const prevSchemas = usePrevious(schemas) ?? []; - const prevColumns = usePrevious(columns) ?? []; const prevSql = usePrevious(currentSql); - useEffect(() => { - if ( - !areArraysShallowEqual(tables, prevTables) || - !areArraysShallowEqual(schemas, prevSchemas) || - !areArraysShallowEqual(columns, prevColumns) - ) { - setAutoCompleter(); - } - }, [tables, schemas, columns]); - useEffect(() => { if (currentSql !== prevSql) { setSql(currentSql); @@ -243,62 +148,6 @@ const AceEditorWrapper = ({ onChange(text); }; - function setAutoCompleter() { - const tableWords = tables.map(t => { - const tableName = t.value; - - return { - name: t.label, - value: tableName, - score: TABLE_AUTOCOMPLETE_SCORE, - meta: 'table', - }; - }); - - const columnWords = columns.map(col => ({ - name: col, - value: col, - score: COLUMN_AUTOCOMPLETE_SCORE, - meta: 'column', - })); - - const functionWords = (functionNames ?? []).map(func => ({ - name: func, - value: func, - score: SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, - meta: 'function', - })); - - const completer = { - insertMatch: (editor: Editor, data: any) => { - if (data.meta === 'table') { - dispatch(addTable(queryEditor, data.value, queryEditor.schema)); - } - - let { caption } = data; - if (data.meta === 'table' && caption.includes(' ')) { - caption = `"${caption}"`; - } - - // executing https://github.com/thlorenz/brace/blob/3a00c5d59777f9d826841178e1eb36694177f5e6/ext/language_tools.js#L1448 - editor.completer.insertMatch( - `${caption}${['function', 'schema'].includes(data.meta) ? '' : ' '}`, - ); - }, - }; - - const words = schemaWords - .concat(tableWords) - .concat(columnWords) - .concat(functionWords) - .concat(sqlKeywords) - .map(word => ({ - ...word, - completer, - })); - - setWords(words); - } const { data: annotations } = useAnnotations({ dbId: queryEditor.dbId, schema: queryEditor.schema, @@ -306,9 +155,18 @@ const AceEditorWrapper = ({ templateParams: queryEditor.templateParams, }); + const keywords = useKeywords( + { + queryEditorId, + dbId: queryEditor.dbId, + schema: queryEditor.schema, + }, + !autocomplete, + ); + return ( { + act(() => { + store.dispatch( + schemaApiUtil.upsertQueryData( + 'schemas', + { + dbId: expectDbId, + forceRefresh: false, + }, + fakeSchemaApiResult.map(value => ({ + value, + label: value, + title: value, + })), + ), + ); + store.dispatch( + tableApiUtil.upsertQueryData( + 'tables', + { dbId: expectDbId, schema: expectSchema }, + { + options: fakeTableApiResult.result, + hasMore: false, + }, + ), + ); + }); +}); + +afterEach(() => { + fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); +}); + +test('returns keywords including fetched function_names data', async () => { + const dbFunctionNamesApiRoute = `glob:*/api/v1/database/${expectDbId}/function_names/`; + fetchMock.get(dbFunctionNamesApiRoute, fakeFunctionNamesApiResult); + + const { result, waitFor } = renderHook( + () => + useKeywords({ + queryEditorId: 'testqueryid', + dbId: expectDbId, + schema: expectSchema, + }), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + + await waitFor(() => + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(1), + ); + fakeSchemaApiResult.forEach(schema => { + expect(result.current).toContainEqual( + expect.objectContaining({ + name: schema, + score: SCHEMA_AUTOCOMPLETE_SCORE, + meta: 'schema', + }), + ); + }); + fakeTableApiResult.result.forEach(({ value }) => { + expect(result.current).toContainEqual( + expect.objectContaining({ + value, + score: TABLE_AUTOCOMPLETE_SCORE, + meta: 'table', + }), + ); + }); + fakeFunctionNamesApiResult.function_names.forEach(func => { + expect(result.current).toContainEqual( + expect.objectContaining({ + name: func, + value: func, + meta: 'function', + score: SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, + }), + ); + }); +}); + +test('skip fetching if autocomplete skipped', () => { + const { result } = renderHook( + () => + useKeywords( + { + queryEditorId: 'testqueryid', + dbId: expectDbId, + schema: expectSchema, + }, + true, + ), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + expect(result.current).toEqual([]); + expect(fetchMock.calls()).toEqual([]); +}); + +test('returns column keywords among selected tables', async () => { + const expectTable = 'table1'; + const expectColumn = 'column1'; + const expectQueryEditorId = 'testqueryid'; + + const unexpectedColumn = 'column2'; + const unexpectedTable = 'table2'; + + const dbFunctionNamesApiRoute = `glob:*/api/v1/database/${expectDbId}/function_names/`; + const storeWithSqlLab = createStore(initialState, reducers); + fetchMock.get(dbFunctionNamesApiRoute, fakeFunctionNamesApiResult); + + act(() => { + storeWithSqlLab.dispatch( + tableApiUtil.upsertQueryData( + 'tableMetadata', + { dbId: expectDbId, schema: expectSchema, table: expectTable }, + { + name: expectTable, + columns: [ + { + name: expectColumn, + type: 'VARCHAR', + }, + ], + }, + ), + ); + + storeWithSqlLab.dispatch( + tableApiUtil.upsertQueryData( + 'tableMetadata', + { dbId: expectDbId, schema: expectSchema, table: unexpectedTable }, + { + name: unexpectedTable, + columns: [ + { + name: unexpectedColumn, + type: 'VARCHAR', + }, + ], + }, + ), + ); + storeWithSqlLab.dispatch( + addTable({ id: expectQueryEditorId }, expectTable, expectSchema), + ); + }); + + const { result, waitFor } = renderHook( + () => + useKeywords({ + queryEditorId: expectQueryEditorId, + dbId: expectDbId, + schema: expectSchema, + }), + { + wrapper: createWrapper({ + useRedux: true, + store: storeWithSqlLab, + }), + }, + ); + + await waitFor(() => + expect(result.current).toContainEqual( + expect.objectContaining({ + name: expectColumn, + value: expectColumn, + score: COLUMN_AUTOCOMPLETE_SCORE, + meta: 'column', + }), + ), + ); + + expect(result.current).not.toContainEqual( + expect.objectContaining({ + name: unexpectedColumn, + }), + ); + + act(() => { + storeWithSqlLab.dispatch( + addTable({ id: expectQueryEditorId }, unexpectedTable, expectSchema), + ); + }); + + await waitFor(() => + expect(result.current).toContainEqual( + expect.objectContaining({ + name: unexpectedColumn, + }), + ), + ); +}); diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts new file mode 100644 index 0000000000000..e7582158f4366 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts @@ -0,0 +1,208 @@ +/** + * 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 { useEffect, useMemo, useRef } from 'react'; +import { useSelector, useDispatch, shallowEqual, useStore } from 'react-redux'; +import { t } from '@superset-ui/core'; + +import { Editor } from 'src/components/AsyncAceEditor'; +import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; +import { addTable, addDangerToast } from 'src/SqlLab/actions/sqlLab'; +import { + SCHEMA_AUTOCOMPLETE_SCORE, + TABLE_AUTOCOMPLETE_SCORE, + COLUMN_AUTOCOMPLETE_SCORE, + SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, +} from 'src/SqlLab/constants'; +import { + schemaEndpoints, + tableEndpoints, + skipToken, +} from 'src/hooks/apiResources'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { useDatabaseFunctionsQuery } from 'src/hooks/apiResources/databaseFunctions'; +import useEffectEvent from 'src/hooks/useEffectEvent'; +import { SqlLabRootState } from 'src/SqlLab/types'; + +type Params = { + queryEditorId: string | number; + dbId?: string | number; + schema?: string; +}; + +const EMPTY_LIST = [] as typeof sqlKeywords; + +const { useQueryState: useSchemasQueryState } = schemaEndpoints.schemas; +const { useQueryState: useTablesQueryState } = tableEndpoints.tables; + +export function useKeywords( + { queryEditorId, dbId, schema }: Params, + skip = false, +) { + const dispatch = useDispatch(); + const hasFetchedKeywords = useRef(false); + // skipFetch is used to prevent re-evaluating memoized keywords + // due to updated api results by skip flag + const skipFetch = hasFetchedKeywords && skip; + const { data: schemaOptions } = useSchemasQueryState( + { + dbId, + forceRefresh: false, + }, + { skip: skipFetch || !dbId }, + ); + const { data: tableData } = useTablesQueryState( + { + dbId, + schema, + forceRefresh: false, + }, + { skip: skipFetch || !dbId || !schema }, + ); + + const { data: functionNames, isError } = useDatabaseFunctionsQuery( + { dbId }, + { skip: skipFetch || !dbId }, + ); + + useEffect(() => { + if (isError) { + dispatch( + addDangerToast(t('An error occurred while fetching function names.')), + ); + } + }, [dispatch, isError]); + + const tablesForColumnMetadata = useSelector( + ({ sqlLab }) => + skip + ? [] + : (sqlLab?.tables ?? []) + .filter(table => table.queryEditorId === queryEditorId) + .map(table => table.name), + shallowEqual, + ); + + const store = useStore(); + const apiState = store.getState()[api.reducerPath]; + + const allColumns = useMemo(() => { + const columns = new Set(); + tablesForColumnMetadata.forEach(table => { + tableEndpoints.tableMetadata + .select( + dbId && schema + ? { + dbId, + schema, + table, + } + : skipToken, + )({ + [api.reducerPath]: apiState, + }) + .data?.columns?.forEach(({ name }) => { + columns.add(name); + }); + }); + return [...columns]; + }, [dbId, schema, apiState, tablesForColumnMetadata]); + + const insertMatch = useEffectEvent((editor: Editor, data: any) => { + if (data.meta === 'table') { + dispatch(addTable({ id: queryEditorId, dbId }, data.value, schema)); + } + + let { caption } = data; + if (data.meta === 'table' && caption.includes(' ')) { + caption = `"${caption}"`; + } + + // executing https://github.com/thlorenz/brace/blob/3a00c5d59777f9d826841178e1eb36694177f5e6/ext/language_tools.js#L1448 + editor.completer.insertMatch( + `${caption}${['function', 'schema'].includes(data.meta) ? '' : ' '}`, + ); + }); + + const schemaKeywords = useMemo( + () => + (schemaOptions ?? []).map(s => ({ + name: s.label, + value: s.value, + score: SCHEMA_AUTOCOMPLETE_SCORE, + meta: 'schema', + completer: { + insertMatch, + }, + })), + [schemaOptions, insertMatch], + ); + + const tableKeywords = useMemo( + () => + (tableData?.options ?? []).map(({ value, label }) => ({ + name: label, + value, + score: TABLE_AUTOCOMPLETE_SCORE, + meta: 'table', + completer: { + insertMatch, + }, + })), + [tableData?.options, insertMatch], + ); + + const columnKeywords = useMemo( + () => + allColumns.map(col => ({ + name: col, + value: col, + score: COLUMN_AUTOCOMPLETE_SCORE, + meta: 'column', + })), + [allColumns], + ); + + const functionKeywords = useMemo( + () => + (functionNames ?? []).map(func => ({ + name: func, + value: func, + score: SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, + meta: 'function', + completer: { + insertMatch, + }, + })), + [functionNames, insertMatch], + ); + + const keywords = useMemo( + () => + columnKeywords + .concat(schemaKeywords) + .concat(tableKeywords) + .concat(functionKeywords) + .concat(sqlKeywords), + [schemaKeywords, tableKeywords, columnKeywords, functionKeywords], + ); + + hasFetchedKeywords.current = !skip; + + return skip ? EMPTY_LIST : keywords; +} diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx index 464dbf9e8ca8b..ed3f3c9de25fe 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx @@ -47,6 +47,9 @@ jest.mock('src/components/AsyncAceEditor', () => ({ })); jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn()); +fetchMock.get('glob:*/api/v1/database/*/function_names/', { + function_names: [], +}); fetchMock.get('glob:*/api/v1/database/*', { result: [] }); fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] }); fetchMock.post('glob:*/sqllab/execute/*', { result: [] }); diff --git a/superset-frontend/src/components/AsyncAceEditor/index.tsx b/superset-frontend/src/components/AsyncAceEditor/index.tsx index 297ff7b552223..e4fa51f56bd77 100644 --- a/superset-frontend/src/components/AsyncAceEditor/index.tsx +++ b/superset-frontend/src/components/AsyncAceEditor/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { forwardRef } from 'react'; +import React, { forwardRef, useEffect } from 'react'; import { Editor as OrigEditor, IEditSession, @@ -28,6 +28,7 @@ import { acequire } from 'ace-builds/src-noconflict/ace'; import AsyncEsmComponent, { PlaceholderProps, } from 'src/components/AsyncEsmComponent'; +import useEffectEvent from 'src/hooks/useEffectEvent'; export interface AceCompleterKeywordData { name: string; @@ -127,27 +128,37 @@ export default function AsyncAceEditor( }, ref, ) { - if (keywords) { - const langTools = acequire('ace/ext/language_tools'); - const completer = { - getCompletions: ( - editor: AceEditor, - session: IEditSession, - pos: Position, - prefix: string, - callback: (error: null, wordList: object[]) => void, - ) => { - // If the prefix starts with a number, don't try to autocomplete - if (!Number.isNaN(parseInt(prefix, 10))) { - return; - } - if ((session.getMode() as TextMode).$id === `ace/mode/${mode}`) { - callback(null, keywords); - } - }, - }; - langTools.setCompleters([completer]); - } + const langTools = acequire('ace/ext/language_tools'); + const setCompleters = useEffectEvent( + (keywords: AceCompleterKeyword[]) => { + const completer = { + getCompletions: ( + editor: AceEditor, + session: IEditSession, + pos: Position, + prefix: string, + callback: (error: null, wordList: object[]) => void, + ) => { + // If the prefix starts with a number, don't try to autocomplete + if (!Number.isNaN(parseInt(prefix, 10))) { + return; + } + if ( + (session.getMode() as TextMode).$id === `ace/mode/${mode}` + ) { + callback(null, keywords); + } + }, + }; + langTools.setCompleters([completer]); + }, + ); + useEffect(() => { + if (keywords) { + setCompleters(keywords); + } + }, [keywords, setCompleters]); + return (