Skip to content

Commit

Permalink
apply fixes from Gidi's PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
pmuellr committed Jul 15, 2020
1 parent 4387f29 commit d1ab30e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 11 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/event_log/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class Plugin implements CorePlugin<IEventLogService, IEventLogClientServi
// Routes
const router = core.http.createRouter();
// Register routes
findRoute(router);
findRoute(router, this.systemLogger);

return this.eventLogService;
}
Expand Down
31 changes: 29 additions & 2 deletions x-pack/plugins/event_log/server/routes/find.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import { findRoute } from './find';
import { httpServiceMock } from 'src/core/server/mocks';
import { mockHandlerArguments, fakeEvent } from './_mock_handler_arguments';
import { eventLogClientMock } from '../event_log_client.mock';
import { loggingSystemMock } from 'src/core/server/mocks';

const eventLogClient = eventLogClientMock.create();
const systemLogger = loggingSystemMock.createLogger();

beforeEach(() => {
jest.resetAllMocks();
Expand All @@ -19,7 +21,7 @@ describe('find', () => {
it('finds events with proper parameters', async () => {
const router = httpServiceMock.createRouter();

findRoute(router);
findRoute(router, systemLogger);

const [config, handler] = router.get.mock.calls[0];

Expand Down Expand Up @@ -58,7 +60,7 @@ describe('find', () => {
it('supports optional pagination parameters', async () => {
const router = httpServiceMock.createRouter();

findRoute(router);
findRoute(router, systemLogger);

const [, handler] = router.get.mock.calls[0];
eventLogClient.findEventsBySavedObject.mockResolvedValueOnce({
Expand Down Expand Up @@ -95,4 +97,29 @@ describe('find', () => {
},
});
});

it('logs a warning when the query throws an error', async () => {
const router = httpServiceMock.createRouter();

findRoute(router, systemLogger);

const [, handler] = router.get.mock.calls[0];
eventLogClient.findEventsBySavedObject.mockRejectedValueOnce(new Error('oof!'));

const [context, req, res] = mockHandlerArguments(
eventLogClient,
{
params: { id: '1', type: 'action' },
query: { page: 3, per_page: 10 },
},
['ok']
);

await handler(context, req, res);

expect(systemLogger.debug).toHaveBeenCalledTimes(1);
expect(systemLogger.debug).toHaveBeenCalledWith(
'error calling eventLog findEventsBySavedObject(action, 1, {"page":3,"per_page":10}): oof!'
);
});
});
13 changes: 7 additions & 6 deletions x-pack/plugins/event_log/server/routes/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ import {
KibanaRequest,
IKibanaResponse,
KibanaResponseFactory,
Logger,
} from 'src/core/server';

import { BASE_EVENT_LOG_API_PATH } from '../../common';
import { findOptionsSchema, FindOptionsType } from '../event_log_client';
import { QueryEventsBySavedObjectResult } from '../es/cluster_client_adapter';

const paramSchema = schema.object({
type: schema.string(),
id: schema.string(),
});

export const findRoute = (router: IRouter) => {
export const findRoute = (router: IRouter, systemLogger: Logger) => {
router.get(
{
path: `${BASE_EVENT_LOG_API_PATH}/{type}/{id}/_find`,
Expand All @@ -45,14 +45,15 @@ export const findRoute = (router: IRouter) => {
query,
} = req;

let result: QueryEventsBySavedObjectResult;
try {
result = await eventLogClient.findEventsBySavedObject(type, id, query);
return res.ok({
body: await eventLogClient.findEventsBySavedObject(type, id, query),
});
} catch (err) {
const call = `findEventsBySavedObject(${type}, ${id}, ${JSON.stringify(query)})`;
systemLogger.debug(`error calling eventLog ${call}: ${err.message}`);
return res.notFound();
}

return res.ok({ body: result });
})
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

// import { IEvent } from '../../../../../../plugins/event_log/server';

import { IValidatedEvent } from '../../../../plugins/event_log/server';
import { getUrlPrefix } from '.';
import { FtrProviderContext } from '../ftr_provider_context';
Expand Down

0 comments on commit d1ab30e

Please sign in to comment.