Skip to content

Commit

Permalink
[Search Sessions] Apply awaits to avoid unhandled errors (#90593) (#9…
Browse files Browse the repository at this point in the history 
…0775)

* Apply awaits to avoid unhandled errors

* catch and ignore tracking error

* added reject test for search service

* Improve search service api test coverage

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@lizozom @kibanamachine
lizozom and kibanamachine authored Feb 9, 2021
1 parent 11ca311 commit 21e99e2
Show file tree
Hide file tree
Showing 2 changed files with 315 additions and 28 deletions.
287 changes: 280 additions & 7 deletions src/plugins/data/server/search/search_service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import type { MockedKeys } from '@kbn/utility-types/jest';
import { CoreSetup, CoreStart } from '../../../../core/server';
import { CoreSetup, CoreStart, SavedObject } from '../../../../core/server';
import { coreMock } from '../../../../core/server/mocks';

import { DataPluginStart } from '../plugin';
Expand Down Expand Up @@ -86,13 +86,22 @@ describe('Search service', () => {
describe('asScopedProvider', () => {
let mockScopedClient: IScopedSearchClient;
let searcPluginStart: ISearchStart<IEsSearchRequest, IEsSearchResponse<any>>;
let mockStrategy: jest.Mocked<ISearchStrategy>;
let mockStrategy: any;
let mockStrategyNoCancel: jest.Mocked<ISearchStrategy>;
let mockSessionService: ISearchSessionService<any>;
let mockSessionClient: jest.Mocked<IScopedSearchSessionsClient>;
const sessionId = '1234';

beforeEach(() => {
mockStrategy = { search: jest.fn().mockReturnValue(of({})) };
mockStrategy = {
search: jest.fn().mockReturnValue(of({})),
cancel: jest.fn(),
extend: jest.fn(),
};

mockStrategyNoCancel = {
search: jest.fn().mockReturnValue(of({})),
};

mockSessionClient = createSearchSessionsClientMock();
mockSessionService = {
Expand All @@ -104,6 +113,7 @@ describe('Search service', () => {
expressions: expressionsPluginMock.createSetupContract(),
});
pluginSetup.registerSearchStrategy('es', mockStrategy);
pluginSetup.registerSearchStrategy('nocancel', mockStrategyNoCancel);
pluginSetup.__enhance({
defaultStrategy: 'es',
sessionService: mockSessionService,
Expand All @@ -123,7 +133,7 @@ describe('Search service', () => {
it('searches using the original request if not restoring, trackId is not called if there is no id in the response', async () => {
const searchRequest = { params: {} };
const options = { sessionId, isStored: false, isRestore: false };
mockSessionClient.trackId = jest.fn();
mockSessionClient.trackId = jest.fn().mockResolvedValue(undefined);

mockStrategy.search.mockReturnValue(
of({
Expand Down Expand Up @@ -165,10 +175,27 @@ describe('Search service', () => {
expect(request).toStrictEqual({ ...searchRequest, id: 'my_id' });
});

it('does not fail if `trackId` throws', async () => {
const searchRequest = { params: {} };
const options = { sessionId, isStored: false, isRestore: false };
mockSessionClient.trackId = jest.fn().mockRejectedValue(undefined);

mockStrategy.search.mockReturnValue(
of({
id: 'my_id',
rawResponse: {} as any,
})
);

await mockScopedClient.search(searchRequest, options).toPromise();

expect(mockSessionClient.trackId).toBeCalledTimes(1);
});

it('calls `trackId` for every response, if the response contains an `id` and not restoring', async () => {
const searchRequest = { params: {} };
const options = { sessionId, isStored: false, isRestore: false };
mockSessionClient.trackId = jest.fn();
mockSessionClient.trackId = jest.fn().mockResolvedValue(undefined);

mockStrategy.search.mockReturnValue(
of(
Expand All @@ -195,7 +222,7 @@ describe('Search service', () => {
const searchRequest = { params: {} };
const options = { sessionId, isStored: true, isRestore: true };
mockSessionClient.getId = jest.fn().mockResolvedValueOnce('my_id');
mockSessionClient.trackId = jest.fn();
mockSessionClient.trackId = jest.fn().mockResolvedValue(undefined);

await mockScopedClient.search(searchRequest, options).toPromise();

Expand All @@ -206,12 +233,258 @@ describe('Search service', () => {
const searchRequest = { params: {} };
const options = {};
mockSessionClient.getId = jest.fn().mockResolvedValueOnce('my_id');
mockSessionClient.trackId = jest.fn();
mockSessionClient.trackId = jest.fn().mockResolvedValue(undefined);

await mockScopedClient.search(searchRequest, options).toPromise();

expect(mockSessionClient.trackId).not.toBeCalled();
});
});

describe('cancelSession', () => {
const mockSavedObject: SavedObject = {
id: 'd7170a35-7e2c-48d6-8dec-9a056721b489',
type: 'search-session',
attributes: {
name: 'my_name',
appId: 'my_app_id',
urlGeneratorId: 'my_url_generator_id',
idMapping: {},
},
references: [],
};

it('cancels a saved object with no search ids', async () => {
mockSessionClient.getSearchIdMapping = jest
.fn()
.mockResolvedValue(new Map<string, string>());
mockSessionClient.cancel = jest.fn().mockResolvedValue(mockSavedObject);
const cancelSpy = jest.spyOn(mockScopedClient, 'cancel');

await mockScopedClient.cancelSession('123');

expect(mockSessionClient.cancel).toHaveBeenCalledTimes(1);
expect(cancelSpy).not.toHaveBeenCalled();
});

it('cancels a saved object and search ids', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockStrategy.cancel = jest.fn();
mockSessionClient.cancel = jest.fn().mockResolvedValue(mockSavedObject);

await mockScopedClient.cancelSession('123');

expect(mockSessionClient.cancel).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('abc');
expect(options).toHaveProperty('strategy', 'es');
});

it('cancels a saved object with some strategies that dont support cancellation, dont throw an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'nocancel');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockStrategy.cancel = jest.fn();
mockSessionClient.cancel = jest.fn().mockResolvedValue(mockSavedObject);

await mockScopedClient.cancelSession('123');

expect(mockSessionClient.cancel).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('def');
expect(options).toHaveProperty('strategy', 'es');
});

it('cancels a saved object with some strategies that dont exist, dont throw an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'notsupported');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockStrategy.cancel = jest.fn();
mockSessionClient.cancel = jest.fn().mockResolvedValue(mockSavedObject);

await mockScopedClient.cancelSession('123');

expect(mockSessionClient.cancel).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('def');
expect(options).toHaveProperty('strategy', 'es');
});
});

describe('deleteSession', () => {
const mockSavedObject: SavedObject = {
id: 'd7170a35-7e2c-48d6-8dec-9a056721b489',
type: 'search-session',
attributes: {
name: 'my_name',
appId: 'my_app_id',
urlGeneratorId: 'my_url_generator_id',
idMapping: {},
},
references: [],
};

it('deletes a saved object with no search ids', async () => {
mockSessionClient.getSearchIdMapping = jest
.fn()
.mockResolvedValue(new Map<string, string>());
mockSessionClient.delete = jest.fn().mockResolvedValue(mockSavedObject);
const cancelSpy = jest.spyOn(mockScopedClient, 'cancel');

await mockScopedClient.deleteSession('123');

expect(mockSessionClient.delete).toHaveBeenCalledTimes(1);
expect(cancelSpy).not.toHaveBeenCalled();
});

it('deletes a saved object and search ids', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockSessionClient.delete = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.cancel = jest.fn();

await mockScopedClient.deleteSession('123');

expect(mockSessionClient.delete).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('abc');
expect(options).toHaveProperty('strategy', 'es');
});

it('deletes a saved object with some strategies that dont support cancellation, dont throw an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'nocancel');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockSessionClient.delete = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.cancel = jest.fn();

await mockScopedClient.deleteSession('123');

expect(mockSessionClient.delete).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('def');
expect(options).toHaveProperty('strategy', 'es');
});

it('deletes a saved object with some strategies that dont exist, dont throw an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'notsupported');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockStrategy.cancel = jest.fn();
mockSessionClient.delete = jest.fn().mockResolvedValue(mockSavedObject);

await mockScopedClient.deleteSession('123');

expect(mockSessionClient.delete).toHaveBeenCalledTimes(1);

const [searchId, options] = mockStrategy.cancel.mock.calls[0];
expect(mockStrategy.cancel).toHaveBeenCalledTimes(1);
expect(searchId).toBe('def');
expect(options).toHaveProperty('strategy', 'es');
});
});

describe('extendSession', () => {
const mockSavedObject: SavedObject = {
id: 'd7170a35-7e2c-48d6-8dec-9a056721b489',
type: 'search-session',
attributes: {
name: 'my_name',
appId: 'my_app_id',
urlGeneratorId: 'my_url_generator_id',
idMapping: {},
},
references: [],
};

it('extends a saved object with no search ids', async () => {
mockSessionClient.getSearchIdMapping = jest
.fn()
.mockResolvedValue(new Map<string, string>());
mockSessionClient.extend = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.extend = jest.fn();

await mockScopedClient.extendSession('123', new Date('2020-01-01'));

expect(mockSessionClient.extend).toHaveBeenCalledTimes(1);
expect(mockStrategy.extend).not.toHaveBeenCalled();
});

it('extends a saved object and search ids', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockSessionClient.extend = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.extend = jest.fn();

await mockScopedClient.extendSession('123', new Date('2020-01-01'));

expect(mockSessionClient.extend).toHaveBeenCalledTimes(1);
expect(mockStrategy.extend).toHaveBeenCalledTimes(1);
const [searchId, keepAlive, options] = mockStrategy.extend.mock.calls[0];
expect(searchId).toBe('abc');
expect(keepAlive).toContain('ms');
expect(options).toHaveProperty('strategy', 'es');
});

it('doesnt extend the saved object with some strategies that dont support cancellation, throws an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'nocancel');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockSessionClient.extend = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.extend = jest.fn().mockResolvedValue({});

const extendRes = mockScopedClient.extendSession('123', new Date('2020-01-01'));

await expect(extendRes).rejects.toThrowError(
'Failed to extend the expiration of some searches'
);

expect(mockSessionClient.extend).not.toHaveBeenCalled();
const [searchId, keepAlive, options] = mockStrategy.extend.mock.calls[0];
expect(searchId).toBe('def');
expect(keepAlive).toContain('ms');
expect(options).toHaveProperty('strategy', 'es');
});

it('doesnt extend the saved object with some strategies that dont exist, throws an error', async () => {
const mockMap = new Map<string, string>();
mockMap.set('abc', 'notsupported');
mockMap.set('def', 'es');
mockSessionClient.getSearchIdMapping = jest.fn().mockResolvedValue(mockMap);
mockSessionClient.extend = jest.fn().mockResolvedValue(mockSavedObject);
mockStrategy.extend = jest.fn().mockResolvedValue({});

const extendRes = mockScopedClient.extendSession('123', new Date('2020-01-01'));

await expect(extendRes).rejects.toThrowError(
'Failed to extend the expiration of some searches'
);

expect(mockSessionClient.extend).not.toHaveBeenCalled();
const [searchId, keepAlive, options] = mockStrategy.extend.mock.calls[0];
expect(searchId).toBe('def');
expect(keepAlive).toContain('ms');
expect(options).toHaveProperty('strategy', 'es');
});
});
});
});
Loading

0 comments on commit 21e99e2

Please sign in to comment.