diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts index d622f1ed9..cb90fe6f6 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts @@ -254,15 +254,35 @@ export type QueryColumn = { is_dttm: boolean; }; -export type QueryState = - | 'stopped' - | 'failed' - | 'pending' - | 'running' - | 'scheduled' - | 'success' - | 'fetching' - | 'timed_out'; +// Possible states of a query object for processing on the server +export enum QueryState { + STARTED = 'started', + STOPPED = 'stopped', + FAILED = 'failed', + PENDING = 'pending', + RUNNING = 'running', + SCHEDULED = 'scheduled', + SUCCESS = 'success', + FETCHING = 'fetching', + TIMED_OUT = 'timed_out', +} + +// Inidcates a Query's state is still processing +export const runningQueryStateList: QueryState[] = [ + QueryState.RUNNING, + QueryState.STARTED, + QueryState.PENDING, + QueryState.FETCHING, + QueryState.SCHEDULED, +]; + +// Indicates a Query's state has completed processing regardless of success / failure +export const concludedQueryStateList: QueryState[] = [ + QueryState.STOPPED, + QueryState.FAILED, + QueryState.SUCCESS, + QueryState.TIMED_OUT, +]; export type Query = { cached: boolean; @@ -304,7 +324,7 @@ export type Query = { executedSql: string; output: string | Record; actions: Record; - type: DatasourceType.Query; + type: DatasourceType; columns: QueryColumn[]; }; @@ -335,7 +355,7 @@ export const testQuery: Query = { isDataPreview: false, progress: 0, resultsKey: null, - state: 'success', + state: QueryState.SUCCESS, tempSchema: null, trackingUrl: null, templateParams: null, diff --git a/superset-frontend/src/SqlLab/components/App/index.jsx b/superset-frontend/src/SqlLab/components/App/index.jsx index 8a0ade950..3aae12bfb 100644 --- a/superset-frontend/src/SqlLab/components/App/index.jsx +++ b/superset-frontend/src/SqlLab/components/App/index.jsx @@ -94,12 +94,17 @@ class App extends React.PureComponent { } render() { + const { queries, actions, queriesLastUpdate } = this.props; if (this.state.hash && this.state.hash === '#search') { return window.location.replace('/superset/sqllab/history/'); } return (
- +
@@ -114,10 +119,12 @@ App.propTypes = { }; function mapStateToProps(state) { - const { common, localStorageUsageInKilobytes } = state; + const { common, localStorageUsageInKilobytes, sqlLab } = state; return { common, localStorageUsageInKilobytes, + queries: sqlLab?.queries, + queriesLastUpdate: sqlLab?.queriesLastUpdate, }; } diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx deleted file mode 100644 index 06bf187e1..000000000 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx +++ /dev/null @@ -1,68 +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 { shallow } from 'enzyme'; -import sinon from 'sinon'; -import thunk from 'redux-thunk'; -import configureStore from 'redux-mock-store'; -import QueryAutoRefresh from 'src/SqlLab/components/QueryAutoRefresh'; -import { initialState, runningQuery } from 'src/SqlLab/fixtures'; - -describe('QueryAutoRefresh', () => { - const middlewares = [thunk]; - const mockStore = configureStore(middlewares); - const sqlLab = { - ...initialState.sqlLab, - queries: { - ryhMUZCGb: runningQuery, - }, - }; - const state = { - ...initialState, - sqlLab, - }; - const store = mockStore(state); - const getWrapper = () => - shallow() - .dive() - .dive(); - let wrapper; - - it('shouldCheckForQueries', () => { - wrapper = getWrapper(); - expect(wrapper.instance().shouldCheckForQueries()).toBe(true); - }); - - it('setUserOffline', () => { - wrapper = getWrapper(); - const spy = sinon.spy(wrapper.instance().props.actions, 'setUserOffline'); - - // state not changed - wrapper.setState({ - offline: false, - }); - expect(spy.called).toBe(false); - - // state is changed - wrapper.setState({ - offline: true, - }); - expect(spy.callCount).toBe(1); - }); -}); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx new file mode 100644 index 000000000..32bf401f2 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx @@ -0,0 +1,133 @@ +/** + * 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 { render } from '@testing-library/react'; +import QueryAutoRefresh, { + isQueryRunning, + shouldCheckForQueries, +} from 'src/SqlLab/components/QueryAutoRefresh'; +import { successfulQuery, runningQuery } from 'src/SqlLab/fixtures'; +import { QueryDictionary } from 'src/SqlLab/types'; + +// NOTE: The uses of @ts-ignore in this file is to enable testing of bad inputs to verify the +// function / component handles bad data elegantly +describe('QueryAutoRefresh', () => { + const runningQueries: QueryDictionary = {}; + runningQueries[runningQuery.id] = runningQuery; + + const successfulQueries: QueryDictionary = {}; + successfulQueries[successfulQuery.id] = successfulQuery; + + const refreshQueries = jest.fn(); + + const queriesLastUpdate = Date.now(); + + it('isQueryRunning returns true for valid running query', () => { + const running = isQueryRunning(runningQuery); + expect(running).toBe(true); + }); + + it('isQueryRunning returns false for valid not-running query', () => { + const running = isQueryRunning(successfulQuery); + expect(running).toBe(false); + }); + + it('isQueryRunning returns false for invalid query', () => { + // @ts-ignore + let running = isQueryRunning(null); + expect(running).toBe(false); + // @ts-ignore + running = isQueryRunning(undefined); + expect(running).toBe(false); + // @ts-ignore + running = isQueryRunning('I Should Be An Object'); + expect(running).toBe(false); + // @ts-ignore + running = isQueryRunning({ state: { badFormat: true } }); + expect(running).toBe(false); + }); + + it('shouldCheckForQueries is true for valid running query', () => { + expect(shouldCheckForQueries(runningQueries)).toBe(true); + }); + + it('shouldCheckForQueries is false for valid completed query', () => { + expect(shouldCheckForQueries(successfulQueries)).toBe(false); + }); + + it('shouldCheckForQueries is false for invalid inputs', () => { + // @ts-ignore + expect(shouldCheckForQueries(null)).toBe(false); + // @ts-ignore + expect(shouldCheckForQueries(undefined)).toBe(false); + expect( + // @ts-ignore + shouldCheckForQueries({ + // @ts-ignore + '1234': null, + // @ts-ignore + '23425': 'hello world', + // @ts-ignore + '345': [], + // @ts-ignore + '57346': undefined, + }), + ).toBe(false); + }); + + it('Attempts to refresh when given pending query', () => { + render( + , + ); + setTimeout(() => { + expect(refreshQueries).toHaveBeenCalled(); + }, 1000); + }); + + it('Does not fail and attempts to refresh when given pending query and invlaid query', () => { + render( + , + ); + setTimeout(() => { + expect(refreshQueries).toHaveBeenCalled(); + }, 1000); + }); + + it('Does NOT Attempt to refresh when given only completed queries', () => { + render( + , + ); + setTimeout(() => { + expect(refreshQueries).not.toHaveBeenCalled(); + }, 1000); + }); +}); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx deleted file mode 100644 index b54936b69..000000000 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx +++ /dev/null @@ -1,124 +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 PropTypes from 'prop-types'; -import { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; -import { SupersetClient } from '@superset-ui/core'; -import * as Actions from 'src/SqlLab/actions/sqlLab'; - -const QUERY_UPDATE_FREQ = 2000; -const QUERY_UPDATE_BUFFER_MS = 5000; -const MAX_QUERY_AGE_TO_POLL = 21600000; -const QUERY_TIMEOUT_LIMIT = 10000; - -class QueryAutoRefresh extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - offline: props.offline, - }; - } - - UNSAFE_componentWillMount() { - this.startTimer(); - } - - componentDidUpdate(prevProps) { - if (prevProps.offline !== this.state.offline) { - this.props.actions.setUserOffline(this.state.offline); - } - } - - componentWillUnmount() { - this.stopTimer(); - } - - shouldCheckForQueries() { - // if there are started or running queries, this method should return true - const { queries } = this.props; - const now = new Date().getTime(); - const isQueryRunning = q => - ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0; - - return Object.values(queries).some( - q => isQueryRunning(q) && now - q.startDttm < MAX_QUERY_AGE_TO_POLL, - ); - } - - startTimer() { - if (!this.timer) { - this.timer = setInterval(this.stopwatch.bind(this), QUERY_UPDATE_FREQ); - } - } - - stopTimer() { - clearInterval(this.timer); - this.timer = null; - } - - stopwatch() { - // only poll /superset/queries/ if there are started or running queries - if (this.shouldCheckForQueries()) { - SupersetClient.get({ - endpoint: `/superset/queries/${ - this.props.queriesLastUpdate - QUERY_UPDATE_BUFFER_MS - }`, - timeout: QUERY_TIMEOUT_LIMIT, - }) - .then(({ json }) => { - if (Object.keys(json).length > 0) { - this.props.actions.refreshQueries(json); - } - this.setState({ offline: false }); - }) - .catch(() => { - this.setState({ offline: true }); - }); - } else { - this.setState({ offline: false }); - } - } - - render() { - return null; - } -} -QueryAutoRefresh.propTypes = { - offline: PropTypes.bool.isRequired, - queries: PropTypes.object.isRequired, - actions: PropTypes.object.isRequired, - queriesLastUpdate: PropTypes.number.isRequired, -}; - -function mapStateToProps({ sqlLab }) { - return { - offline: sqlLab.offline, - queries: sqlLab.queries, - queriesLastUpdate: sqlLab.queriesLastUpdate, - }; -} - -function mapDispatchToProps(dispatch) { - return { - actions: bindActionCreators(Actions, dispatch), - }; -} - -export default connect(mapStateToProps, mapDispatchToProps)(QueryAutoRefresh); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx new file mode 100644 index 000000000..eb3e6f4c3 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx @@ -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 { useState } from 'react'; +import { isObject } from 'lodash'; +import { + SupersetClient, + Query, + runningQueryStateList, +} from '@superset-ui/core'; +import { QueryDictionary } from 'src/SqlLab/types'; +import useInterval from 'src/SqlLab/utils/useInterval'; + +const QUERY_UPDATE_FREQ = 2000; +const QUERY_UPDATE_BUFFER_MS = 5000; +const MAX_QUERY_AGE_TO_POLL = 21600000; +const QUERY_TIMEOUT_LIMIT = 10000; + +interface RefreshQueriesFunc { + (alteredQueries: any): any; +} + +export interface QueryAutoRefreshProps { + queries: QueryDictionary; + refreshQueries: RefreshQueriesFunc; + queriesLastUpdate: number; +} + +// returns true if the Query.state matches one of the specifc values indicating the query is still processing on server +export const isQueryRunning = (q: Query): boolean => + runningQueryStateList.includes(q?.state); + +// returns true if at least one query is running and within the max age to poll timeframe +export const shouldCheckForQueries = (queryList: QueryDictionary): boolean => { + let shouldCheck = false; + const now = Date.now(); + if (isObject(queryList)) { + shouldCheck = Object.values(queryList).some( + q => isQueryRunning(q) && now - q?.startDttm < MAX_QUERY_AGE_TO_POLL, + ); + } + return shouldCheck; +}; + +function QueryAutoRefresh({ + queries, + refreshQueries, + queriesLastUpdate, +}: QueryAutoRefreshProps) { + // We do not want to spam requests in the case of slow connections and potentially recieve responses out of order + // pendingRequest check ensures we only have one active http call to check for query statuses + const [pendingRequest, setPendingRequest] = useState(false); + + const checkForRefresh = () => { + if (!pendingRequest && shouldCheckForQueries(queries)) { + setPendingRequest(true); + SupersetClient.get({ + endpoint: `/superset/queries/${ + queriesLastUpdate - QUERY_UPDATE_BUFFER_MS + }`, + timeout: QUERY_TIMEOUT_LIMIT, + }) + .then(({ json }) => { + if (json) { + refreshQueries?.(json); + } + }) + .catch(() => {}) + .finally(() => { + setPendingRequest(false); + }); + } + }; + + // Solves issue where direct usage of setInterval in function components + // uses stale props / state from closure + // See comments in the useInterval.ts file for more information + useInterval(() => { + checkForRefresh(); + }, QUERY_UPDATE_FREQ); + + return null; +} + +export default QueryAutoRefresh; diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 5b12ee292..58b21edd9 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -19,6 +19,7 @@ import sinon from 'sinon'; import * as actions from 'src/SqlLab/actions/sqlLab'; import { ColumnKeyTypeType } from 'src/SqlLab/components/ColumnElement'; +import { DatasourceType, QueryResponse, QueryState } from '@superset-ui/core'; export const mockedActions = sinon.stub({ ...actions }); @@ -189,6 +190,7 @@ export const defaultQueryEditor = { }, ], }; + export const queries = [ { dbId: 1, @@ -201,7 +203,7 @@ export const queries = [ id: 'BkA1CLrJg', progress: 100, startDttm: 1476910566092.96, - state: 'success', + state: QueryState.SUCCESS, changedOn: 1476910566000, tempTable: null, userId: 1, @@ -260,7 +262,7 @@ export const queries = [ id: 'S1zeAISkx', progress: 100, startDttm: 1476910570802.2, - state: 'success', + state: QueryState.SUCCESS, changedOn: 1476910572000, tempTable: null, userId: 1, @@ -294,7 +296,7 @@ export const queryWithNoQueryLimit = { id: 'BkA1CLrJg', progress: 100, startDttm: 1476910566092.96, - state: 'success', + state: QueryState.SUCCESS, changedOn: 1476910566000, tempTable: null, userId: 1, @@ -344,6 +346,7 @@ export const queryWithNoQueryLimit = { }, }, }; + export const queryWithBadColumns = { ...queries[0], results: { @@ -407,6 +410,7 @@ export const queryWithBadColumns = { ], }, }; + export const databases = { result: [ { @@ -429,6 +433,7 @@ export const databases = { }, ], }; + export const tables = { options: [ { @@ -464,7 +469,7 @@ export const stoppedQuery = { sql: 'SELECT ...', sqlEditorId: 'rJaf5u9WZ', startDttm: 1497400851936, - state: 'stopped', + state: QueryState.STOPPED, tab: 'Untitled Query 2', tempTable: '', }; @@ -482,7 +487,7 @@ export const failedQueryWithErrorMessage = { sql: 'SELECT ...', sqlEditorId: 'rJaf5u9WZ', startDttm: 1497400851936, - state: 'failed', + state: QueryState.FAILED, tab: 'Untitled Query 2', tempTable: '', }; @@ -507,20 +512,113 @@ export const failedQueryWithErrors = { sql: 'SELECT ...', sqlEditorId: 'rJaf5u9WZ', startDttm: 1497400851936, - state: 'failed', + state: QueryState.FAILED, tab: 'Untitled Query 2', tempTable: '', }; -export const runningQuery = { +const baseQuery: QueryResponse = { + queryId: 567, + dbId: 1, + sql: 'SELECT * FROM superset.slices', + sqlEditorId: 'SJ8YO72R', + tab: 'Demo', + ctas: false, + cached: false, + id: 'BkA1CLrJg', + progress: 100, + startDttm: 1476910566092.96, + state: QueryState.SUCCESS, + tempSchema: null, + tempTable: 'temp', + userId: 1, + executedSql: 'SELECT * FROM superset.slices', + rows: 42, + started: 'started', + queryLimit: 100, + endDttm: 1476910566798, + schema: 'test_schema', + errorMessage: null, + db: { key: 'main' }, + user: { key: 'admin' }, + isDataPreview: false, + resultsKey: null, + trackingUrl: null, + templateParams: null, + limitingFactor: 'capacity', + duration: '2334645675467', + time: { key: 'value' }, + querylink: { key: 'value' }, + output: { key: 'value' }, + actions: { key: 'value' }, + extra: { + progress: null, + }, + columns: [], + type: DatasourceType.Query, + results: { + displayLimitReached: false, + query: { limit: 6 }, + columns: [ + { + is_dttm: true, + name: 'ds', + type: 'STRING', + }, + { + is_dttm: false, + name: 'gender', + type: 'STRING', + }, + ], + selected_columns: [ + { + is_dttm: true, + name: 'ds', + type: 'STRING', + }, + { + is_dttm: false, + name: 'gender', + type: 'STRING', + }, + ], + expanded_columns: [ + { + is_dttm: true, + name: 'ds', + type: 'STRING', + }, + ], + data: [ + { col1: '0', col2: '1' }, + { col1: '2', col2: '3' }, + ], + }, +}; + +export const runningQuery: QueryResponse = { + ...baseQuery, dbId: 1, cached: false, ctas: false, id: 'ryhMUZCGb', progress: 90, - state: 'running', + state: QueryState.RUNNING, startDttm: Date.now() - 500, }; + +export const successfulQuery: QueryResponse = { + ...baseQuery, + dbId: 1, + cached: false, + ctas: false, + id: 'ryhMUZCGb', + progress: 100, + state: QueryState.SUCCESS, + startDttm: Date.now() - 500, +}; + export const cachedQuery = { ...queries[0], cached: true }; export const user = { diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index bc0427705..9a6198b86 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -25,6 +25,11 @@ import { ExploreRootState } from 'src/explore/types'; export type ExploreDatasource = Dataset | QueryResponse; +// Object as Dictionary (associative array) with Query id as the key and type Query as the value +export type QueryDictionary = { + [id: string]: QueryResponse; +}; + export interface QueryEditor { dbId?: number; title: string; diff --git a/superset-frontend/src/SqlLab/utils/useInterval.ts b/superset-frontend/src/SqlLab/utils/useInterval.ts new file mode 100644 index 000000000..731e6c85c --- /dev/null +++ b/superset-frontend/src/SqlLab/utils/useInterval.ts @@ -0,0 +1,47 @@ +/** + * 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, useRef } from 'react'; + +/* + * Functional components and setTimeout with useState do not play well + * and the setTimeout callback typically has stale state from a closure + * The useInterval function solves this issue. + * more info: https://overreacted.io/making-setinterval-declarative-with-react-hooks/ + */ +function useInterval(callback: Function, delay: number | null): void { + const savedCallback = useRef(callback); + // Remember the latest function. + useEffect(() => { + savedCallback.current = callback; + }, [callback]); + + // Set up the interval. + useEffect(() => { + function tick() { + savedCallback?.current?.(); + } + if (delay !== null) { + const id = setInterval(tick, delay); + return () => clearInterval(id); + } + return () => {}; + }, [delay]); +} + +export default useInterval;