From f7047afa5c18d27aee90b4ee8c17cbb38cc5d256 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 15 May 2023 10:01:46 +0100 Subject: [PATCH] [v10.0.x] Search: Fix Search returning results out of order (#68387) Search: Fix Search returning results out of order (#68370) * Search: Fix Search returning results out of order * test (cherry picked from commit 78afddebbff3b0b95e04ca467064772369113c02) --- .../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 11352388e1d..a61183c9909 100644 --- a/public/app/features/search/state/SearchStateManager.test.ts +++ b/public/app/features/search/state/SearchStateManager.test.ts @@ -6,6 +6,15 @@ import { SearchLayout } from '../types'; 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 { @@ -14,7 +23,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, @@ -44,5 +54,44 @@ describe('SearchStateManager', () => { stm.initStateFromUrl(); expect(stm.state.folderUid).toBe(undefined); }); + + 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 22b45f93cb4..fe0459a1b82 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()); @@ -221,29 +222,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