This is an automated email from the ASF dual-hosted git repository. michellet pushed a commit to branch release--0.33 in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit 5c607c16ad8e66499420de35805c67bff7afee3a Author: Grace Guo <[email protected]> AuthorDate: Fri Jun 7 14:27:57 2019 -0700 [SQL Lab] Show warning when user used up localStorage (#7572) (cherry picked from commit 39d67cbc5901995e7f45e806d163131db18887df) --- .../spec/javascripts/sqllab/SouthPane_spec.jsx | 8 ++--- .../spec/javascripts/sqllab/SqlEditor_spec.jsx | 1 - .../sqllab/utils/emptyQueryResults_spec.js | 42 ++++++++++++++++++++++ superset/assets/src/SqlLab/App.jsx | 18 +++++++++- superset/assets/src/SqlLab/components/App.jsx | 39 +++++++++++++++++++- .../assets/src/SqlLab/components/SouthPane.jsx | 5 +-- superset/assets/src/SqlLab/constants.js | 15 ++++++++ .../assets/src/SqlLab/reducers/getInitialState.js | 1 + superset/assets/src/SqlLab/reducers/index.js | 2 ++ .../reducers/{index.js => localStorageUsage.js} | 14 ++------ .../{constants.js => utils/emptyQueryResults.js} | 42 +++++++++------------- 11 files changed, 142 insertions(+), 45 deletions(-) diff --git a/superset/assets/spec/javascripts/sqllab/SouthPane_spec.jsx b/superset/assets/spec/javascripts/sqllab/SouthPane_spec.jsx index 7baba2b..b67b2e6 100644 --- a/superset/assets/spec/javascripts/sqllab/SouthPane_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SouthPane_spec.jsx @@ -34,10 +34,10 @@ describe('SouthPane', () => { const mockedProps = { editorQueries: [ - { cached: false, changedOn: 1559238552333, db: 'main', dbId: 1, id: 'LCly_kkIN' }, - { cached: false, changedOn: 1559238500401, db: 'main', dbId: 1, id: 'lXJa7F9_r' }, - { cached: false, changedOn: 1559238506925, db: 'main', dbId: 1, id: '2g2_iRFMl' }, - { cached: false, changedOn: 1559238516395, db: 'main', dbId: 1, id: 'erWdqEWPm' }, + { cached: false, changedOn: Date.now(), db: 'main', dbId: 1, id: 'LCly_kkIN', startDttm: Date.now() }, + { cached: false, changedOn: 1559238500401, db: 'main', dbId: 1, id: 'lXJa7F9_r', startDttm: 1559238500401 }, + { cached: false, changedOn: 1559238506925, db: 'main', dbId: 1, id: '2g2_iRFMl', startDttm: 1559238506925 }, + { cached: false, changedOn: 1559238516395, db: 'main', dbId: 1, id: 'erWdqEWPm', startDttm: 1559238516395 }, ], latestQueryId: 'LCly_kkIN', dataPreviewQueries: [], diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx index 046b2e6..c2500f5 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -31,7 +31,6 @@ describe('SqlEditor', () => { queryEditor: initialState.sqlLab.queryEditors[0], latestQuery: queries[0], tables: [table], - queries, getHeight: () => ('100px'), editorQueries: [], dataPreviewQueries: [], diff --git a/superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults_spec.js b/superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults_spec.js new file mode 100644 index 0000000..f202430 --- /dev/null +++ b/superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults_spec.js @@ -0,0 +1,42 @@ +/** + * 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 emptyQueryResults from '../../../../src/SqlLab/utils/emptyQueryResults'; +import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../../../../src/SqlLab/constants'; +import { queries } from '../fixtures'; + +describe('emptyQueryResults', () => { + const queriesObj = {}; + beforeEach(() => { + queries.forEach((q) => { + queriesObj[q.id] = q; + }); + }); + + it('should empty query.results if query.startDttm is > LOCALSTORAGE_MAX_QUERY_AGE_MS', () => { + // make sure sample data contains old query + const oldQuery = queries[0]; + const { id, startDttm } = oldQuery; + expect(Date.now() - startDttm).toBeGreaterThan(LOCALSTORAGE_MAX_QUERY_AGE_MS); + expect(Object.keys(oldQuery.results)).toContain('data'); + + const emptiedQuery = emptyQueryResults(queriesObj); + expect(emptiedQuery[id].startDttm).toBe(startDttm); + expect(emptiedQuery[id].results).toEqual({}); + }); +}); diff --git a/superset/assets/src/SqlLab/App.jsx b/superset/assets/src/SqlLab/App.jsx index 359cd73..3883591 100644 --- a/superset/assets/src/SqlLab/App.jsx +++ b/superset/assets/src/SqlLab/App.jsx @@ -27,6 +27,8 @@ import getInitialState from './reducers/getInitialState'; import rootReducer from './reducers/index'; import { initEnhancer } from '../reduxUtils'; import App from './components/App'; +import emptyQueryResults from './utils/emptyQueryResults'; +import { BYTES_PER_CHAR, KB_STORAGE } from './constants'; import setupApp from '../setup/setupApp'; import './main.less'; @@ -50,8 +52,22 @@ const sqlLabPersistStateConfig = { // it caused configurations passed from server-side got override. // see PR 6257 for details delete state[path].common; // eslint-disable-line no-param-reassign - subset[path] = state[path]; + + if (path === 'sqlLab') { + subset[path] = { + ...state[path], + queries: emptyQueryResults(state[path].queries), + }; + } }); + + const data = JSON.stringify(subset); + // 2 digit precision + const currentSize = Math.round(data.length * BYTES_PER_CHAR / KB_STORAGE * 100) / 100; + if (state.localStorageUsageInKilobytes !== currentSize) { + state.localStorageUsageInKilobytes = currentSize; // eslint-disable-line no-param-reassign + } + return subset; }, }, diff --git a/superset/assets/src/SqlLab/components/App.jsx b/superset/assets/src/SqlLab/components/App.jsx index e4e516a..c4f2960 100644 --- a/superset/assets/src/SqlLab/components/App.jsx +++ b/superset/assets/src/SqlLab/components/App.jsx @@ -21,11 +21,18 @@ import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; import $ from 'jquery'; +import { t } from '@superset-ui/translation'; +import throttle from 'lodash/throttle'; import TabbedSqlEditors from './TabbedSqlEditors'; import QueryAutoRefresh from './QueryAutoRefresh'; import QuerySearch from './QuerySearch'; import ToastPresenter from '../../messageToasts/containers/ToastPresenter'; +import { + LOCALSTORAGE_MAX_USAGE_KB, + LOCALSTORAGE_WARNING_THRESHOLD, + LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS, +} from '../constants'; import * as Actions from '../actions/sqlLab'; class App extends React.PureComponent { @@ -35,6 +42,12 @@ class App extends React.PureComponent { hash: window.location.hash, contentHeight: '0px', }; + + this.showLocalStorageUsageWarning = throttle( + this.showLocalStorageUsageWarning, + LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS, + { trailing: false }, + ); } componentDidMount() { /* eslint-disable react/no-did-mount-set-state */ @@ -42,6 +55,13 @@ class App extends React.PureComponent { window.addEventListener('hashchange', this.onHashChanged.bind(this)); window.addEventListener('resize', this.handleResize.bind(this)); } + componentDidUpdate() { + if (this.props.localStorageUsageInKilobytes >= + LOCALSTORAGE_WARNING_THRESHOLD * LOCALSTORAGE_MAX_USAGE_KB + ) { + this.showLocalStorageUsageWarning(this.props.localStorageUsageInKilobytes); + } + } componentWillUnmount() { window.removeEventListener('hashchange', this.onHashChanged.bind(this)); window.removeEventListener('resize', this.handleResize.bind(this)); @@ -65,6 +85,15 @@ class App extends React.PureComponent { const alertHeight = alertEl.length > 0 ? alertEl.outerHeight() : 0; return `${window.innerHeight - headerHeight - tabsHeight - warningHeight - alertHeight}px`; } + showLocalStorageUsageWarning(currentUsage) { + this.props.actions.addDangerToast( + t('SQL Lab uses your browser\'s local storage to store queries and results.' + + `\n Currently, you are using ${currentUsage.toFixed(2)} KB out of ${LOCALSTORAGE_MAX_USAGE_KB} KB. storage space.` + + '\n To keep SQL Lab from crashing, please delete some query tabs.' + + '\n You can re-access these queries by using the Save feature before you delete the tab. ' + + 'Note that you will need to close other SQL Lab windows before you do this.'), + ); + } handleResize() { this.setState({ contentHeight: this.getHeight() }); } @@ -91,8 +120,16 @@ class App extends React.PureComponent { App.propTypes = { actions: PropTypes.object, + localStorageUsageInKilobytes: PropTypes.number.isRequired, }; +function mapStateToProps(state) { + const { localStorageUsageInKilobytes } = state; + return { + localStorageUsageInKilobytes, + }; +} + function mapDispatchToProps(dispatch) { return { actions: bindActionCreators(Actions, dispatch), @@ -101,6 +138,6 @@ function mapDispatchToProps(dispatch) { export { App }; export default connect( - null, + mapStateToProps, mapDispatchToProps, )(App); diff --git a/superset/assets/src/SqlLab/components/SouthPane.jsx b/superset/assets/src/SqlLab/components/SouthPane.jsx index 68321f3..6da48ab 100644 --- a/superset/assets/src/SqlLab/components/SouthPane.jsx +++ b/superset/assets/src/SqlLab/components/SouthPane.jsx @@ -27,7 +27,7 @@ import { t } from '@superset-ui/translation'; import * as Actions from '../actions/sqlLab'; import QueryHistory from './QueryHistory'; import ResultSet from './ResultSet'; -import { STATUS_OPTIONS, STATE_BSSTYLE_MAP } from '../constants'; +import { STATUS_OPTIONS, STATE_BSSTYLE_MAP, LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants'; const TAB_HEIGHT = 44; @@ -87,7 +87,8 @@ export class SouthPane extends React.PureComponent { latestQuery = props.editorQueries.find(q => q.id === this.props.latestQueryId); } let results; - if (latestQuery) { + if (latestQuery && + (Date.now() - latestQuery.startDttm) <= LOCALSTORAGE_MAX_QUERY_AGE_MS) { results = ( <ResultSet showControls diff --git a/superset/assets/src/SqlLab/constants.js b/superset/assets/src/SqlLab/constants.js index 3bf8ce0..c030ce2 100644 --- a/superset/assets/src/SqlLab/constants.js +++ b/superset/assets/src/SqlLab/constants.js @@ -43,3 +43,18 @@ export const TIME_OPTIONS = [ '90 days ago', '1 year ago', ]; + +// SqlEditor layout constants +export const SQL_EDITOR_GUTTER_HEIGHT = 5; +export const SQL_EDITOR_GUTTER_MARGIN = 3; +export const SQL_TOOLBAR_HEIGHT = 51; + +// kilobyte storage +export const KB_STORAGE = 1024; +export const BYTES_PER_CHAR = 2; + +// browser's localStorage max usage constants +export const LOCALSTORAGE_MAX_QUERY_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours +export const LOCALSTORAGE_MAX_USAGE_KB = 5 * 1024; // 5M +export const LOCALSTORAGE_WARNING_THRESHOLD = 0.9; +export const LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS = 8000; // danger type toast duration diff --git a/superset/assets/src/SqlLab/reducers/getInitialState.js b/superset/assets/src/SqlLab/reducers/getInitialState.js index adb3db4..dbeeb18 100644 --- a/superset/assets/src/SqlLab/reducers/getInitialState.js +++ b/superset/assets/src/SqlLab/reducers/getInitialState.js @@ -47,6 +47,7 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) { messageToasts: getToastsFromPyFlashMessages( (restBootstrapData.common || {}).flash_messages || [], ), + localStorageUsageInKilobytes: 0, common: { flash_messages: restBootstrapData.common.flash_messages, conf: restBootstrapData.common.conf, diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/src/SqlLab/reducers/index.js index 9bea4f1..904b306 100644 --- a/superset/assets/src/SqlLab/reducers/index.js +++ b/superset/assets/src/SqlLab/reducers/index.js @@ -19,11 +19,13 @@ import { combineReducers } from 'redux'; import sqlLab from './sqlLab'; +import localStorageUsageInKilobytes from './localStorageUsage'; import messageToasts from '../../messageToasts/reducers/index'; import common from './common'; export default combineReducers({ sqlLab, + localStorageUsageInKilobytes, messageToasts, common, }); diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/src/SqlLab/reducers/localStorageUsage.js similarity index 76% copy from superset/assets/src/SqlLab/reducers/index.js copy to superset/assets/src/SqlLab/reducers/localStorageUsage.js index 9bea4f1..eafbb07 100644 --- a/superset/assets/src/SqlLab/reducers/index.js +++ b/superset/assets/src/SqlLab/reducers/localStorageUsage.js @@ -16,14 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { combineReducers } from 'redux'; - -import sqlLab from './sqlLab'; -import messageToasts from '../../messageToasts/reducers/index'; -import common from './common'; - -export default combineReducers({ - sqlLab, - messageToasts, - common, -}); +export default function localStorageUsageReducer(state = 0) { + return state; +} diff --git a/superset/assets/src/SqlLab/constants.js b/superset/assets/src/SqlLab/utils/emptyQueryResults.js similarity index 61% copy from superset/assets/src/SqlLab/constants.js copy to superset/assets/src/SqlLab/utils/emptyQueryResults.js index 3bf8ce0..2798168 100644 --- a/superset/assets/src/SqlLab/constants.js +++ b/superset/assets/src/SqlLab/utils/emptyQueryResults.js @@ -16,30 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -export const STATE_BSSTYLE_MAP = { - offline: 'danger', - failed: 'danger', - pending: 'info', - fetching: 'info', - running: 'warning', - stopped: 'danger', - success: 'success', -}; +import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants'; -export const STATUS_OPTIONS = { - success: 'success', - failed: 'failed', - running: 'running', - offline: 'offline', - pending: 'pending', -}; +export default function emptyQueryResults(queries) { + return Object.keys(queries) + .reduce((accu, key) => { + const { startDttm, results } = queries[key]; + const query = { + ...queries[key], + results: Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ? + {} : results, + }; -export const TIME_OPTIONS = [ - 'now', - '1 hour ago', - '1 day ago', - '7 days ago', - '28 days ago', - '90 days ago', - '1 year ago', -]; + const updatedQueries = { + ...accu, + [key]: query, + }; + return updatedQueries; + }, {}); +}
