Skip to content

Commit

Permalink
use isSearchStored param instead of server-side checkId (#135036)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dosant authored Jul 11, 2022
1 parent 3729617 commit a985fb9
Show file tree
Hide file tree
Showing 13 changed files with 187 additions and 57 deletions.
13 changes: 12 additions & 1 deletion src/plugins/data/common/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ export interface ISearchOptions {
*/
isStored?: boolean;

/**
* Whether the search was successfully polled after session was saved. Search was added to a session saved object and keepAlive extended.
*/
isSearchStored?: boolean;

/**
* Whether the session is restored (i.e. search requests should re-use the stored search IDs,
* rather than starting from scratch)
Expand All @@ -155,5 +160,11 @@ export interface ISearchOptions {
*/
export type ISearchOptionsSerializable = Pick<
ISearchOptions,
'strategy' | 'legacyHitsTotal' | 'sessionId' | 'isStored' | 'isRestore' | 'executionContext'
| 'strategy'
| 'legacyHitsTotal'
| 'sessionId'
| 'isStored'
| 'isSearchStored'
| 'isRestore'
| 'executionContext'
>;
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ describe('SearchInterceptor', () => {
sessionId,
isStored: true,
isRestore: true,
isSearchStored: false,
strategy: 'ese',
},
})
Expand Down Expand Up @@ -736,7 +737,7 @@ describe('SearchInterceptor', () => {
sessionService.trackSearch.mockImplementation(() => ({
complete: trackSearchComplete,
error: () => {},
polled: () => {},
beforePoll: () => [{ isSearchStored: false }, () => {}],
}));

const response = searchInterceptor.search({}, { pollInterval: 0, sessionId });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ export class SearchInterceptor {
serializableOptions.legacyHitsTotal = combined.legacyHitsTotal;
if (combined.strategy !== undefined) serializableOptions.strategy = combined.strategy;
if (combined.isStored !== undefined) serializableOptions.isStored = combined.isStored;
if (combined.isSearchStored !== undefined)
serializableOptions.isSearchStored = combined.isSearchStored;
if (combined.executionContext !== undefined) {
serializableOptions.executionContext = combined.executionContext;
}
Expand All @@ -229,15 +231,27 @@ export class SearchInterceptor {
const { sessionId, strategy } = options;

const search = () => {
searchTracker?.polled();
const [{ isSearchStored }, afterPoll] = searchTracker?.beforePoll() ?? [
{ isSearchStored: false },
({ isSearchStored: boolean }) => {},
];
return this.runSearch(
{ id, ...request },
{
...options,
...this.deps.session.getSearchOptions(sessionId),
abortSignal: searchAbortController.getSignal(),
isSearchStored,
}
);
)
.then((result) => {
afterPoll({ isSearchStored: result.isStored ?? false });
return result;
})
.catch((err) => {
afterPoll({ isSearchStored: false });
throw err;
});
};

const searchTracker = this.deps.session.isCurrentSession(sessionId)
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/data/public/search/session/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ export function getSessionServiceMock(): jest.Mocked<ISessionService> {
trackSearch: jest.fn((searchDescriptor) => ({
complete: jest.fn(),
error: jest.fn(),
polled: jest.fn(),
beforePoll: jest.fn(() => {
return [{ isSearchStored: false }, () => {}];
}),
})),
destroy: jest.fn(),
cancel: jest.fn(),
Expand Down
21 changes: 17 additions & 4 deletions src/plugins/data/public/search/session/session_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,13 @@ interface TrackSearchHandler {
error(): void;

/**
* Call to notify when search was polled
* Call to notify when search is about to be polled to get current search state to build `searchOptions` from (mainly isSearchStored),
* When poll completes or errors, call `afterPoll` callback and confirm is search was successfully stored
*/
polled(): void;
beforePoll(): [
currentSearchState: { isSearchStored: boolean },
afterPoll: (newSearchState: { isSearchStored: boolean }) => void
];
}

/**
Expand Down Expand Up @@ -331,11 +335,20 @@ export class SessionService {
error: () => {
this.state.transitions.errorSearch(searchDescriptor);
},
polled: () => {
beforePoll: () => {
const search = this.state.selectors.getSearch(searchDescriptor);
this.state.transitions.updateSearchMeta(searchDescriptor, {
lastPollingTime: new Date(),
isStored: this.isStored(),
});

return [
{ isSearchStored: search?.searchMeta?.isStored ?? false },
({ isSearchStored }) => {
this.state.transitions.updateSearchMeta(searchDescriptor, {
isStored: isSearchStored,
});
},
];
},
};
}
Expand Down
31 changes: 21 additions & 10 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,18 +392,29 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
!(options.isRestore && searchRequest.id) // and not restoring already tracked search
) {
// then track this search inside the search-session saved object
return from(deps.searchSessionsClient.trackId(request, response.id, options)).pipe(
map(() => ({

// check if search was already tracked and extended, don't track again in this case
if (options.isSearchStored) {
return of({
...response,
isStored: true,
})),
catchError((e) => {
this.logger.error(
`Error while trying to track search id: ${e?.message}. This might lead to untracked long-running search.`
);
return of(response);
})
);
});
} else {
return from(
deps.searchSessionsClient.trackId(request, response.id, options)
).pipe(
map(() => ({
...response,
isStored: true,
})),
catchError((e) => {
this.logger.error(
`Error while trying to track search id: ${e?.message}. This might lead to untracked long-running search.`
);
return of(response);
})
);
}
} else {
return of(response);
}
Expand Down
37 changes: 0 additions & 37 deletions src/plugins/data/server/search/session/session_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,19 +392,6 @@ export class SearchSessionService
if (!this.sessionConfig.enabled || !sessionId || !searchId) return;
this.logger.debug(`trackId | ${sessionId} | ${searchId}`);

let isIdTracked = false;
try {
const hasId = await this.checkId(deps, user, searchRequest, options);
isIdTracked = hasId;
} catch (e) {
this.logger.warn(
`trackId | Error while checking if search is already tracked by a session object: "${e?.message}". Continue assuming not tracked.`
);
}

// no need to update search saved object if id is already tracked in a session object
if (isIdTracked) return;

let idMapping: Record<string, SearchSessionRequestInfo> = {};

if (searchRequest.params) {
Expand Down Expand Up @@ -465,30 +452,6 @@ export class SearchSessionService
return session.attributes.idMapping[requestHash].id;
};

/**
* Look up an existing search ID that matches the given request in the given session
* @internal
*/
public checkId = async (
deps: SearchSessionDependencies,
user: AuthenticatedUser | null,
searchRequest: IKibanaSearchRequest,
{ sessionId, isStored }: ISearchOptions
): Promise<boolean> => {
if (!this.sessionConfig.enabled) {
throw new Error('Search sessions are disabled');
} else if (!sessionId) {
throw new Error('Session ID is required');
} else if (!isStored) {
throw new Error('Cannot check search ID from a session that is not stored');
}

const session = await this.get(deps, user, sessionId);
const requestHash = createRequestHash(searchRequest.params);

return session.attributes.idMapping.hasOwnProperty(requestHash);
};

public asScopedProvider = ({ savedObjects, elasticsearch }: CoreStart) => {
return (request: KibanaRequest) => {
const user = this.security?.authc.getCurrentUser(request) ?? null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const RequestCodeViewer = ({ indexPattern, json }: RequestCodeViewerProps
)}
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={true}>
<EuiFlexItem grow={true} data-test-subj="inspectorRequestCodeViewerContainer">
<CodeEditor
languageId={XJsonLang.ID}
value={json}
Expand Down
8 changes: 8 additions & 0 deletions test/functional/services/dashboard/panel_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ export class DashboardPanelActionsService extends FtrService {
return searchSessionId;
}

async getSearchResponseByTitle(title: string) {
await this.openInspectorByTitle(title);
await this.inspector.openInspectorRequestsView();
const response = await this.inspector.getResponse();
await this.inspector.close();
return response;
}

async openInspector(parent?: WebElementWrapper) {
await this.openContextMenu(parent);
const exists = await this.testSubjects.exists(OPEN_INSPECTOR_TEST_SUBJ);
Expand Down
10 changes: 10 additions & 0 deletions test/functional/services/inspector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class InspectorService extends FtrService {
private readonly testSubjects = this.ctx.getService('testSubjects');
private readonly find = this.ctx.getService('find');
private readonly comboBox = this.ctx.getService('comboBox');
private readonly monacoEditor = this.ctx.getService('monacoEditor');

private async getIsEnabled(): Promise<boolean> {
const ariaDisabled = await this.testSubjects.getAttribute('openInspectorButton', 'disabled');
Expand Down Expand Up @@ -253,6 +254,15 @@ export class InspectorService extends FtrService {
return this.testSubjects.find('inspectorRequestDetailResponse');
}

public async getResponse(): Promise<Record<string, any>> {
await (await this.getOpenRequestDetailResponseButton()).click();

await this.monacoEditor.waitCodeEditorReady('inspectorRequestCodeViewerContainer');
const responseString = await this.monacoEditor.getCodeEditorValue();
this.log.debug('Response string from inspector:', responseString);
return JSON.parse(responseString);
}

/**
* Returns true if the value equals the combobox options list
* @param value default combobox single option text
Expand Down
11 changes: 11 additions & 0 deletions x-pack/test/functional/services/search_sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class SearchSessionsService extends FtrService {
private readonly retry = this.ctx.getService('retry');
private readonly browser = this.ctx.getService('browser');
private readonly supertest = this.ctx.getService('supertest');
private readonly es = this.ctx.getService('es');

public async find(): Promise<WebElementWrapper> {
return this.testSubjects.find(SEARCH_SESSION_INDICATOR_TEST_SUBJ);
Expand Down Expand Up @@ -174,4 +175,14 @@ export class SearchSessionsService extends FtrService {
this.browser.removeLocalStorageItem(TOUR_RESTORE_STEP_KEY),
]);
}

public async getAsyncSearchStatus(asyncSearchId: string) {
const asyncSearchStatus = await this.es.asyncSearch.status({ id: asyncSearchId });
return asyncSearchStatus;
}

public async getAsyncSearchExpirationTime(asyncSearchId: string): Promise<number> {
const asyncSearchStatus = await this.getAsyncSearchStatus(asyncSearchId);
return Number(asyncSearchStatus.expiration_time_in_millis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default function ({ loadTestFile, getService, getPageObjects }: FtrProvid
});

loadTestFile(require.resolve('./async_search'));
loadTestFile(require.resolve('./session_searches_integration'));
loadTestFile(require.resolve('./save_search_session'));
loadTestFile(require.resolve('./save_search_session_relative_time'));
loadTestFile(require.resolve('./search_sessions_tour'));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import expect from '@kbn/expect';
import { FtrProviderContext } from '../../../../ftr_provider_context';

export default function ({ getService, getPageObjects }: FtrProviderContext) {
const es = getService('es');
const log = getService('log');
const PageObjects = getPageObjects([
'common',
'header',
'dashboard',
'visChart',
'searchSessionsManagement',
]);
const dashboardPanelActions = getService('dashboardPanelActions');
const searchSessions = getService('searchSessions');
const retry = getService('retry');

describe('Session and searches integration', () => {
before(async function () {
const body = await es.info();
if (!body.version.number.includes('SNAPSHOT')) {
log.debug('Skipping because this build does not have the required shard_delay agg');
this.skip();
}
await PageObjects.common.navigateToApp('dashboard');
});

after(async function () {
await searchSessions.deleteAllSearchSessions();
});

it('until session is saved search keepAlive is short, when it is saved, keepAlive is extended and search is saved into the session saved object', async () => {
await PageObjects.dashboard.loadSavedDashboard('Not Delayed');
await PageObjects.dashboard.waitForRenderComplete();
await searchSessions.expectState('completed');

const searchResponse = await dashboardPanelActions.getSearchResponseByTitle(
'Sum of Bytes by Extension'
);

const asyncSearchId = searchResponse.id;
expect(typeof asyncSearchId).to.be('string');

const asyncExpirationTimeBeforeSessionWasSaved =
await searchSessions.getAsyncSearchExpirationTime(asyncSearchId);
expect(asyncExpirationTimeBeforeSessionWasSaved).to.be.lessThan(
Date.now() + 1000 * 60,
'expiration time should be less then a minute from now'
);

await searchSessions.save();
await searchSessions.expectState('backgroundCompleted');

await retry.waitFor('async search keepAlive is extended', async () => {
const asyncExpirationTimeAfterSessionWasSaved =
await searchSessions.getAsyncSearchExpirationTime(asyncSearchId);

return (
asyncExpirationTimeAfterSessionWasSaved > asyncExpirationTimeBeforeSessionWasSaved &&
asyncExpirationTimeAfterSessionWasSaved > Date.now() + 1000 * 60
);
});

const savedSessionId = await dashboardPanelActions.getSearchSessionIdByTitle(
'Sum of Bytes by Extension'
);

// check that search saved into the session

await searchSessions.openPopover();
await searchSessions.viewSearchSessions();

const searchSessionList = await PageObjects.searchSessionsManagement.getList();
const searchSessionItem = searchSessionList.find((session) => session.id === savedSessionId)!;
expect(searchSessionItem.searchesCount).to.be(1);
});
});
}

0 comments on commit a985fb9

Please sign in to comment.