From 74a4149a4eb2d17c653b86885755dcd4969b7dea Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 26 Aug 2020 17:53:05 -0700 Subject: [PATCH] Update handler to pass method & body requests + pass back response code - This will support future POST requests as well as support (e.g.) 404 responses Minor test refactors: - Set up new request defaults (method, empty body) + expected output & convert KibanaAuthHeader to string since they're set so close to one another - group request pass tests into a describe block, remove response body portion of their tests (since we don't really care about that for those tests) - group response tests into describe block - clarify how passed handler params arg overrides request.params --- .../enterprise_search_request_handler.test.ts | 81 ++++++++++++++----- .../lib/enterprise_search_request_handler.ts | 22 +++-- 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts index d5b507d77d771..3c7926be92f75 100644 --- a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts +++ b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts @@ -14,10 +14,9 @@ const fetchMock = require('node-fetch') as jest.Mock; const { Response } = jest.requireActual('node-fetch'); const responseMock = { - ok: jest.fn(), + custom: jest.fn(), customError: jest.fn(), }; -const KibanaAuthHeader = 'Basic 123'; describe('EnterpriseSearchRequestHandler', () => { const enterpriseSearchRequestHandler = new EnterpriseSearchRequestHandler({ @@ -50,33 +49,66 @@ describe('EnterpriseSearchRequestHandler', () => { }); EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection?type=indexed&pageIndex=1' + 'http://localhost:3002/as/credentials/collection?type=indexed&pageIndex=1', + { method: 'GET' } ); - expect(responseMock.ok).toHaveBeenCalledWith({ + expect(responseMock.custom).toHaveBeenCalledWith({ body: responseBody, + statusCode: 200, }); }); - it('allows passing custom params', async () => { - const responseBody = { - results: [{ name: 'engine1' }], - meta: { page: { total_results: 1 } }, - }; - EnterpriseSearchAPI.mockReturn(responseBody); + describe('request passing', () => { + it('passes route method', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); - const requestHandler = enterpriseSearchRequestHandler.createRequest({ - path: '/as/engines/collection', - params: '?some=custom¶ms=true', + await makeAPICall(requestHandler, { route: { method: 'POST' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'POST', + }); + + await makeAPICall(requestHandler, { route: { method: 'DELETE' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'DELETE', + }); }); - await makeAPICall(requestHandler, { query: { overriden: true } }); - EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/engines/collection?some=custom¶ms=true' - ); - expect(responseMock.ok).toHaveBeenCalledWith({ - body: responseBody, + it('passes request body', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); + await makeAPICall(requestHandler, { body: { bodacious: true } }); + + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + body: '{"bodacious":true}', + }); }); + + it('passes custom params set by the handler, which override request params', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/example', + params: '?some=custom¶ms=true', + }); + await makeAPICall(requestHandler, { query: { overriden: true } }); + + EnterpriseSearchAPI.shouldHaveBeenCalledWith( + 'http://localhost:3002/api/example?some=custom¶ms=true' + ); + }); + }); + + describe('response passing', () => { + it('returns the response status code from Enterprise Search', async () => { + EnterpriseSearchAPI.mockReturn({}, { status: 404 }); + + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); + await makeAPICall(requestHandler); + + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example'); + expect(responseMock.custom).toHaveBeenCalledWith({ body: {}, statusCode: 404 }); + }); + + // TODO: It's possible we may also pass back headers at some point + // from Enterprise Search, e.g. the x-read-only mode header }); describe('error handling', () => { @@ -136,14 +168,21 @@ describe('EnterpriseSearchRequestHandler', () => { }); const makeAPICall = (handler: Function, params = {}) => { - const request = { headers: { authorization: KibanaAuthHeader }, ...params }; + const request = { + headers: { authorization: 'Basic 123' }, + route: { method: 'GET' }, + body: {}, + ...params, + }; return handler(null, request, responseMock); }; const EnterpriseSearchAPI = { shouldHaveBeenCalledWith(expectedUrl: string, expectedParams = {}) { expect(fetchMock).toHaveBeenCalledWith(expectedUrl, { - headers: { Authorization: KibanaAuthHeader }, + headers: { Authorization: 'Basic 123' }, + method: 'GET', + body: undefined, ...expectedParams, }); }, diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts index 3efad86d62918..74c2d924a4ac8 100644 --- a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts +++ b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts @@ -56,23 +56,31 @@ export class EnterpriseSearchRequestHandler { response: KibanaResponseFactory ) => { try { + // Set up API URL params = params ?? (request.query ? `?${querystring.stringify(request.query)}` : ''); const url = encodeURI(this.enterpriseSearchUrl + path + params); - const apiResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); + // Set up API options + const { method } = request.route; + const headers = { Authorization: request.headers.authorization as string }; + const body = Object.keys(request.body as object).length + ? JSON.stringify(request.body) + : undefined; + + // Call the Enterprise Search API and pass back response to the front-end + const apiResponse = await fetch(url, { method, headers, body }); if (apiResponse.url.endsWith('/login')) { throw new Error('Cannot authenticate Enterprise Search user'); } - const body = await apiResponse.json(); + const { status } = apiResponse; + const json = await apiResponse.json(); - if (hasValidData(body)) { - return response.ok({ body }); + if (hasValidData(json)) { + return response.custom({ statusCode: status, body: json }); } else { - this.log.debug(`Invalid data received from <${url}>: ${JSON.stringify(body)}`); + this.log.debug(`Invalid data received from <${url}>: ${JSON.stringify(json)}`); throw new Error('Invalid data received'); } } catch (e) {