You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2019/06/07 21:28:08 UTC

[incubator-superset] branch master updated: [SQL Lab] Show warning when user used up localStorage (#7572)

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 <gr...@airbnb.com>
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 = (
         <ResultSet
           showControls
diff --git a/superset/assets/src/SqlLab/constants.js b/superset/assets/src/SqlLab/constants.js
index 4dd2118..c030ce2 100644
--- a/superset/assets/src/SqlLab/constants.js
+++ b/superset/assets/src/SqlLab/constants.js
@@ -48,3 +48,13 @@ export const TIME_OPTIONS = [
 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 535cb3d..5aaf39b 100644
--- a/superset/assets/src/SqlLab/reducers/getInitialState.js
+++ b/superset/assets/src/SqlLab/reducers/getInitialState.js
@@ -52,6 +52,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/reducers/index.js b/superset/assets/src/SqlLab/utils/emptyQueryResults.js
similarity index 61%
copy from superset/assets/src/SqlLab/reducers/index.js
copy to superset/assets/src/SqlLab/utils/emptyQueryResults.js
index 9bea4f1..2798168 100644
--- a/superset/assets/src/SqlLab/reducers/index.js
+++ b/superset/assets/src/SqlLab/utils/emptyQueryResults.js
@@ -16,14 +16,22 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { combineReducers } from 'redux';
+import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants';
 
-import sqlLab from './sqlLab';
-import messageToasts from '../../messageToasts/reducers/index';
-import common from './common';
+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 default combineReducers({
-  sqlLab,
-  messageToasts,
-  common,
-});
+      const updatedQueries = {
+        ...accu,
+        [key]: query,
+      };
+      return updatedQueries;
+    }, {});
+}