From 78afddebbff3b0b95e04ca467064772369113c02 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Fri, 12 May 2023 15:42:04 +0100 Subject: [PATCH] Search: Fix Search returning results out of order (#68370) * Search: Fix Search returning results out of order * test --- .../search/state/SearchStateManager.test.ts | 51 ++++++++++++++++++- .../search/state/SearchStateManager.ts | 39 +++++++------- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/public/app/features/search/state/SearchStateManager.test.ts b/public/app/features/search/state/SearchStateManager.test.ts index 78267fadde4..702c1fba586 100644 --- a/public/app/features/search/state/SearchStateManager.test.ts +++ b/public/app/features/search/state/SearchStateManager.test.ts @@ -7,6 +7,15 @@ import * as utils from '../utils'; import { getSearchStateManager } from './SearchStateManager'; +jest.mock('lodash', () => { + const orig = jest.requireActual('lodash'); + + return { + ...orig, + debounce: (d: Function) => d, + }; +}); + jest.mock('@grafana/runtime', () => { const originalModule = jest.requireActual('@grafana/runtime'); return { @@ -15,7 +24,8 @@ jest.mock('@grafana/runtime', () => { }); describe('SearchStateManager', () => { - jest.spyOn(getGrafanaSearcher(), 'search').mockResolvedValue({ + const searcher = getGrafanaSearcher(); + jest.spyOn(searcher, 'search').mockResolvedValue({ isItemLoaded: jest.fn(), loadMoreItems: jest.fn(), totalRows: 0, @@ -69,5 +79,44 @@ describe('SearchStateManager', () => { expect(stm.state.sort).toBe(undefined); expect(stm.state.folderUid).toBe('abc'); }); + + it('updates search results in order', async () => { + const stm = getSearchStateManager(); + + jest.spyOn(searcher, 'search').mockReturnValueOnce( + new Promise(async (resolve) => { + await wait(100); + + resolve({ + isItemLoaded: jest.fn(), + loadMoreItems: jest.fn(), + totalRows: 100, + view: new DataFrameView({ fields: [], length: 0 }), + }); + }) + ); + stm.onQueryChange('d'); + + jest.spyOn(searcher, 'search').mockReturnValueOnce( + new Promise(async (resolve) => { + await wait(50); + + resolve({ + isItemLoaded: jest.fn(), + loadMoreItems: jest.fn(), + totalRows: 10, + view: new DataFrameView({ fields: [], length: 0 }), + }); + }) + ); + + stm.onQueryChange('debugging'); + + await wait(150); + + expect(stm.state.result?.totalRows).toEqual(10); + }); }); }); + +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); diff --git a/public/app/features/search/state/SearchStateManager.ts b/public/app/features/search/state/SearchStateManager.ts index 180aa23920f..570181818c6 100644 --- a/public/app/features/search/state/SearchStateManager.ts +++ b/public/app/features/search/state/SearchStateManager.ts @@ -39,6 +39,7 @@ export class SearchStateManager extends StateManagerBase { updateLocation = debounce((query) => locationService.partial(query, true), 300); doSearchWithDebounce = debounce(() => this.doSearch(), 300); lastQuery?: SearchQuery; + lastSearchPromise: Promise = Promise.resolve(); initStateFromUrl(folderUid?: string, doInitialSearch = true) { const stateFromUrl = parseRouteParams(locationService.getSearchObject()); @@ -224,29 +225,23 @@ export class SearchStateManager extends StateManagerBase { this.setState({ loading: true }); - if (this.state.starred) { - getGrafanaSearcher() - .starred(this.lastQuery) - .then((result) => this.setState({ result, loading: false })) - .catch((error) => { - reportSearchFailedQueryInteraction(this.state.eventTrackingNamespace, { - ...trackingInfo, - error: error?.message, - }); - this.setState({ loading: false }); + const searcher = getGrafanaSearcher(); + + // Issue this search now, but wait until previous searches have resolved to update it in the state + const searchPromise = this.state.starred ? searcher.starred(this.lastQuery) : searcher.search(this.lastQuery); + + this.lastSearchPromise = this.lastSearchPromise + .then(() => searchPromise) + .then((result) => this.setState({ result, loading: false })) + .catch((error) => { + reportSearchFailedQueryInteraction(this.state.eventTrackingNamespace, { + ...trackingInfo, + error: error?.message, }); - } else { - getGrafanaSearcher() - .search(this.lastQuery) - .then((result) => this.setState({ result, loading: false })) - .catch((error) => { - reportSearchFailedQueryInteraction(this.state.eventTrackingNamespace, { - ...trackingInfo, - error: error?.message, - }); - this.setState({ loading: false }); - }); - } + this.setState({ loading: false }); + + return Promise.resolve(); // make sure this.lastSearchPromise is always resolved + }); } // This gets the possible tags from within the query results