Skip to content

Commit

Permalink
[ES|QL] Clears the history component cache correctly (#204891)
Browse files Browse the repository at this point in the history
## Summary

Clears the cache when there are no items in the localStorage (which
means that the user has cleared it). It can create unpredictable bugs.

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Vadim Kibana <[email protected]>
  • Loading branch information
stratoula and vadimkibana authored Dec 19, 2024
1 parent b321646 commit 9562a19
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,28 @@
*/
import { addQueriesToCache, getCachedQueries } from './history_local_storage';

class LocalStorageMock {
public store: Record<string, unknown>;
constructor(defaultStore: Record<string, unknown>) {
this.store = defaultStore;
}
clear() {
this.store = {};
}
getItem(key: string) {
return this.store[key] || null;
}
setItem(key: string, value: unknown) {
this.store[key] = String(value);
}
}

describe('history local storage', function () {
const mockGetItem = jest.fn();
const mockSetItem = jest.fn();
const storage = new LocalStorageMock({}) as unknown as Storage;
Object.defineProperty(window, 'localStorage', {
value: {
getItem: (...args: string[]) => mockGetItem(...args),
setItem: (...args: string[]) => mockSetItem(...args),
},
value: storage,
});

it('should add queries to cache correctly ', function () {
addQueriesToCache({
queryString: 'from kibana_sample_data_flights | limit 10',
Expand All @@ -37,11 +50,6 @@ describe('history local storage', function () {
expect(historyItems.length).toBe(2);
expect(historyItems[1].timeRan).toBeDefined();
expect(historyItems[1].status).toBe('success');

expect(mockSetItem).toHaveBeenCalledWith(
'QUERY_HISTORY_ITEM_KEY',
JSON.stringify(historyItems)
);
});

it('should update queries to cache correctly if they are the same with different format', function () {
Expand All @@ -54,11 +62,6 @@ describe('history local storage', function () {
expect(historyItems.length).toBe(2);
expect(historyItems[1].timeRan).toBeDefined();
expect(historyItems[1].status).toBe('success');

expect(mockSetItem).toHaveBeenCalledWith(
'QUERY_HISTORY_ITEM_KEY',
JSON.stringify(historyItems)
);
});

it('should allow maximum x queries ', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export const addQueriesToCache = (
// the cachedQueries Map might not contain all
// the localStorage queries
const queries = getHistoryItems('desc');
cachedQueries.clear();
queries.forEach((queryItem) => {
const trimmedQueryString = getTrimmedQuery(queryItem.queryString);
cachedQueries.set(trimmedQueryString, queryItem);
Expand All @@ -87,6 +88,7 @@ export const addQueriesToCache = (

// queries to store in the localstorage
allQueries = sortedByDate.slice(0, maxQueriesAllowed);

// clear and reset the queries in the cache
cachedQueries.clear();
allQueries.forEach((queryItem) => {
Expand Down

0 comments on commit 9562a19

Please sign in to comment.