From commits-return-2802-archive-asf-public=cust-asf.ponee.io@superset.incubator.apache.org Fri Jun 7 21:28:15 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 0730318067E for ; Fri, 7 Jun 2019 23:28:14 +0200 (CEST) Received: (qmail 69330 invoked by uid 500); 7 Jun 2019 21:28:14 -0000 Mailing-List: contact commits-help@superset.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@superset.incubator.apache.org Delivered-To: mailing list commits@superset.incubator.apache.org Received: (qmail 69321 invoked by uid 99); 7 Jun 2019 21:28:14 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Jun 2019 21:28:14 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 434EB87A6C; Fri, 7 Jun 2019 21:28:09 +0000 (UTC) Date: Fri, 07 Jun 2019 21:28:08 +0000 To: "commits@superset.apache.org" Subject: [incubator-superset] branch master updated: [SQL Lab] Show warning when user used up localStorage (#7572) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155994288866.23368.18076161292925596114@gitbox.apache.org> From: graceguo@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-superset X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 883a02ae19457fa24bcd8a0c747746a4f8fb063d X-Git-Newrev: 39d67cbc5901995e7f45e806d163131db18887df X-Git-Rev: 39d67cbc5901995e7f45e806d163131db18887df X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. graceguo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-superset.git The following commit(s) were added to refs/heads/master by this push: new 39d67cb [SQL Lab] Show warning when user used up localStorage (#7572) 39d67cb is described below commit 39d67cbc5901995e7f45e806d163131db18887df Author: Grace Guo AuthorDate: Fri Jun 7 14:27:57 2019 -0700 [SQL Lab] Show warning when user used up localStorage (#7572) --- .../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 | 10 ++++++ .../assets/src/SqlLab/reducers/getInitialState.js | 1 + superset/assets/src/SqlLab/reducers/index.js | 2 ++ .../reducers/{index.js => localStorageUsage.js} | 14 ++------ .../index.js => utils/emptyQueryResults.js} | 26 +++++++++----- 11 files changed, 137 insertions(+), 29 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 17bb4d8..dcf2609 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -40,7 +40,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 = ( { + const { startDttm, results } = queries[key]; + const query = { + ...queries[key], + results: Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ? + {} : results, + }; -export default combineReducers({ - sqlLab, - messageToasts, - common, -}); + const updatedQueries = { + ...accu, + [key]: query, + }; + return updatedQueries; + }, {}); +}