Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Commit

Permalink
fix(sqllab): empty large query results from localStorage (apache#23302)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored Mar 13, 2023
1 parent d415eed commit 9ae81b7
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 5 deletions.
1 change: 1 addition & 0 deletions superset-frontend/src/SqlLab/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ 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_MAX_QUERY_RESULTS_KB = 1 * 1024; // 1M
export const LOCALSTORAGE_WARNING_THRESHOLD = 0.9;
export const LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS = 8000; // danger type toast duration

Expand Down
39 changes: 38 additions & 1 deletion superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import {
emptyQueryResults,
clearQueryEditors,
} from 'src/SqlLab/utils/reduxStateToLocalStorageHelper';
import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from 'src/SqlLab/constants';
import {
KB_STORAGE,
BYTES_PER_CHAR,
LOCALSTORAGE_MAX_QUERY_AGE_MS,
LOCALSTORAGE_MAX_QUERY_RESULTS_KB,
} from 'src/SqlLab/constants';
import { queries, defaultQueryEditor } from '../fixtures';

describe('reduxStateToLocalStorageHelper', () => {
Expand All @@ -45,6 +50,38 @@ describe('reduxStateToLocalStorageHelper', () => {
expect(emptiedQuery[id].results).toEqual({});
});

it('should empty query.results if query,.results size is greater than LOCALSTORAGE_MAX_QUERY_RESULTS_KB', () => {
const reasonableSizeQuery = {
...queries[0],
startDttm: Date.now(),
results: { data: [{ a: 1 }] },
};
const largeQuery = {
...queries[1],
startDttm: Date.now(),
results: {
data: [
{
jsonValue: `{"str":"${new Array(
(LOCALSTORAGE_MAX_QUERY_RESULTS_KB / BYTES_PER_CHAR) * KB_STORAGE,
)
.fill(0)
.join('')}"}`,
},
],
},
};
expect(Object.keys(largeQuery.results)).toContain('data');
const emptiedQuery = emptyQueryResults({
[reasonableSizeQuery.id]: reasonableSizeQuery,
[largeQuery.id]: largeQuery,
});
expect(emptiedQuery[largeQuery.id].results).toEqual({});
expect(emptiedQuery[reasonableSizeQuery.id].results).toEqual(
reasonableSizeQuery.results,
);
});

it('should only return selected keys for query editor', () => {
const queryEditors = [defaultQueryEditor];
expect(Object.keys(queryEditors[0])).toContain('schemaOptions');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants';
import {
BYTES_PER_CHAR,
KB_STORAGE,
LOCALSTORAGE_MAX_QUERY_AGE_MS,
LOCALSTORAGE_MAX_QUERY_RESULTS_KB,
} from '../constants';

const PERSISTENT_QUERY_EDITOR_KEYS = new Set([
'remoteId',
Expand All @@ -36,13 +41,21 @@ const PERSISTENT_QUERY_EDITOR_KEYS = new Set([
'hideLeftBar',
]);

function shouldEmptyQueryResults(query) {
const { startDttm, results } = query;
return (
Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ||
((JSON.stringify(results)?.length || 0) * BYTES_PER_CHAR) / KB_STORAGE >
LOCALSTORAGE_MAX_QUERY_RESULTS_KB
);
}

export function emptyQueryResults(queries) {
return Object.keys(queries).reduce((accu, key) => {
const { startDttm, results } = queries[key];
const { results } = queries[key];
const query = {
...queries[key],
results:
Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ? {} : results,
results: shouldEmptyQueryResults(queries[key]) ? {} : results,
};

const updatedQueries = {
Expand Down

0 comments on commit 9ae81b7

Please sign in to comment.