diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/index.ts b/x-pack/plugins/enterprise_search/server/__mocks__/index.ts index 3cca5e21ce9c3..69f37856b6f2d 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/index.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/index.ts @@ -5,4 +5,9 @@ */ export { MockRouter } from './router.mock'; -export { mockConfig, mockLogger, mockDependencies } from './routerDependencies.mock'; +export { + mockConfig, + mockLogger, + mockRequestHandler, + mockDependencies, +} from './routerDependencies.mock'; diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts index 7a244be96cfc4..b08a14b540e55 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/routerDependencies.mock.ts @@ -9,6 +9,13 @@ import { ConfigType } from '../'; export const mockLogger = loggingSystemMock.createLogger().get(); +export const mockRequestHandler = { + createRequest: jest.fn(() => () => {}), + hasValidData(data: any) { + return (this.createRequest as jest.Mock).mock.calls[0][0].hasValidData(data); + }, +}; + export const mockConfig = { enabled: true, host: 'http://localhost:3002', @@ -24,4 +31,5 @@ export const mockDependencies = { // Mock router should be handled on a per-test basis config: mockConfig, log: mockLogger, + enterpriseSearchRequestHandler: mockRequestHandler as any, }; 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 f0c003936996e..3f3f182433144 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 @@ -6,7 +6,7 @@ import { mockConfig, mockLogger } from '../__mocks__'; -import { createEnterpriseSearchRequestHandler } from './enterprise_search_request_handler'; +import { EnterpriseSearchRequestHandler } from './enterprise_search_request_handler'; jest.mock('node-fetch'); // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -14,12 +14,16 @@ 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('createEnterpriseSearchRequestHandler', () => { +describe('EnterpriseSearchRequestHandler', () => { + const enterpriseSearchRequestHandler = new EnterpriseSearchRequestHandler({ + config: mockConfig, + log: mockLogger, + }) as any; + beforeEach(() => { jest.clearAllMocks(); fetchMock.mockReset(); @@ -33,9 +37,7 @@ describe('createEnterpriseSearchRequestHandler', () => { EnterpriseSearchAPI.mockReturn(responseBody); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/as/credentials/collection', }); @@ -47,82 +49,151 @@ describe('createEnterpriseSearchRequestHandler', () => { }); 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, }); }); - describe('when an API request fails', () => { - it('should return 502 with a message', async () => { - EnterpriseSearchAPI.mockReturnError(); + describe('request passing', () => { + it('passes route method', async () => { + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/example' }); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, - path: '/as/credentials/collection', + await makeAPICall(requestHandler, { route: { method: 'POST' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'POST', }); - await makeAPICall(requestHandler); + await makeAPICall(requestHandler, { route: { method: 'DELETE' } }); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/example', { + method: 'DELETE', + }); + }); + + 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: { someQuery: true }, + }); + await makeAPICall(requestHandler, { query: { someQuery: false } }); EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection' + 'http://localhost:3002/api/example?someQuery=true' ); + }); + }); - expect(responseMock.customError).toHaveBeenCalledWith({ - body: 'Error connecting or fetching data from Enterprise Search', - statusCode: 502, - }); + 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('when `hasValidData` fails', () => { - it('should return 502 with a message', async () => { - const responseBody = { - foo: 'bar', - }; + describe('error handling', () => { + afterEach(() => { + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('Error connecting to Enterprise Search') + ); + }); + + it('returns an error when an API request fails', async () => { + EnterpriseSearchAPI.mockReturnError(); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ path: '/api/failed' }); + + await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/failed'); - EnterpriseSearchAPI.mockReturn(responseBody); + expect(responseMock.customError).toHaveBeenCalledWith({ + body: 'Error connecting to Enterprise Search: Failed', + statusCode: 502, + }); + }); - const requestHandler = createEnterpriseSearchRequestHandler({ - config: mockConfig, - log: mockLogger, - path: '/as/credentials/collection', - hasValidData: (body?: any) => - Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number', + it('returns an error when `hasValidData` fails', async () => { + EnterpriseSearchAPI.mockReturn({ results: false }); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/invalid', + hasValidData: (body?: any) => Array.isArray(body?.results), }); await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/invalid'); - EnterpriseSearchAPI.shouldHaveBeenCalledWith( - 'http://localhost:3002/as/credentials/collection' + expect(responseMock.customError).toHaveBeenCalledWith({ + body: 'Error connecting to Enterprise Search: Invalid data received', + statusCode: 502, + }); + expect(mockLogger.debug).toHaveBeenCalledWith( + 'Invalid data received from : {"results":false}' ); + }); + + it('returns an error when user authentication to Enterprise Search fails', async () => { + EnterpriseSearchAPI.mockReturn({}, { url: 'http://localhost:3002/login' }); + const requestHandler = enterpriseSearchRequestHandler.createRequest({ + path: '/api/unauthenticated', + }); + + await makeAPICall(requestHandler); + EnterpriseSearchAPI.shouldHaveBeenCalledWith('http://localhost:3002/api/unauthenticated'); expect(responseMock.customError).toHaveBeenCalledWith({ - body: 'Error connecting or fetching data from Enterprise Search', + body: 'Error connecting to Enterprise Search: Cannot authenticate Enterprise Search user', statusCode: 502, }); }); }); + + it('has a helper for checking empty objects', async () => { + expect(enterpriseSearchRequestHandler.isEmptyObj({})).toEqual(true); + expect(enterpriseSearchRequestHandler.isEmptyObj({ empty: false })).toEqual(false); + }); }); 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, }); }, - mockReturn(response: object) { + mockReturn(response: object, options?: object) { fetchMock.mockImplementation(() => { - return Promise.resolve(new Response(JSON.stringify(response))); + return Promise.resolve(new Response(JSON.stringify(response), options)); }); }, mockReturnError() { 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 11152aa651743..8f31bd9063d4a 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 @@ -7,6 +7,7 @@ import fetch from 'node-fetch'; import querystring from 'querystring'; import { + RequestHandler, RequestHandlerContext, KibanaRequest, KibanaResponseFactory, @@ -14,56 +15,90 @@ import { } from 'src/core/server'; import { ConfigType } from '../index'; -interface IEnterpriseSearchRequestParams { +interface IConstructorDependencies { config: ConfigType; log: Logger; +} +interface IRequestParams { path: string; + params?: object; hasValidData?: (body?: ResponseBody) => boolean; } +export interface IEnterpriseSearchRequestHandler { + createRequest(requestParams?: object): RequestHandler, unknown>; +} /** - * This helper function creates a single standard DRY way of handling + * This helper lib creates a single standard DRY way of handling * Enterprise Search API requests. * * This handler assumes that it will essentially just proxy the * Enterprise Search API request, so the request body and request * parameters are simply passed through. */ -export function createEnterpriseSearchRequestHandler({ - config, - log, - path, - hasValidData = () => true, -}: IEnterpriseSearchRequestParams) { - return async ( - _context: RequestHandlerContext, - request: KibanaRequest, unknown>, - response: KibanaResponseFactory - ) => { - try { - const enterpriseSearchUrl = config.host as string; - const params = request.query ? `?${querystring.stringify(request.query)}` : ''; - const url = `${encodeURI(enterpriseSearchUrl)}${path}${params}`; +export class EnterpriseSearchRequestHandler { + private enterpriseSearchUrl: string; + private log: Logger; + + constructor({ config, log }: IConstructorDependencies) { + this.log = log; + this.enterpriseSearchUrl = config.host as string; + } + + createRequest({ + path, + params = {}, + hasValidData = () => true, + }: IRequestParams) { + return async ( + _context: RequestHandlerContext, + request: KibanaRequest, unknown>, + response: KibanaResponseFactory + ) => { + try { + // Set up API URL + const queryParams = { ...request.query, ...params }; + const queryString = !this.isEmptyObj(queryParams) + ? `?${querystring.stringify(queryParams)}` + : ''; + const url = encodeURI(this.enterpriseSearchUrl + path + queryString); + + // Set up API options + const { method } = request.route; + const headers = { Authorization: request.headers.authorization as string }; + const body = !this.isEmptyObj(request.body as object) + ? 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 { status } = apiResponse; + const json = await apiResponse.json(); - const apiResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); + if (hasValidData(json)) { + return response.custom({ statusCode: status, body: json }); + } else { + this.log.debug(`Invalid data received from <${url}>: ${JSON.stringify(json)}`); + throw new Error('Invalid data received'); + } + } catch (e) { + const errorMessage = `Error connecting to Enterprise Search: ${e?.message || e.toString()}`; - const body = await apiResponse.json(); + this.log.error(errorMessage); + if (e instanceof Error) this.log.debug(e.stack as string); - if (hasValidData(body)) { - return response.ok({ body }); - } else { - throw new Error(`Invalid data received: ${JSON.stringify(body)}`); + return response.customError({ statusCode: 502, body: errorMessage }); } - } catch (e) { - log.error(`Cannot connect to Enterprise Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); + }; + } - return response.customError({ - statusCode: 502, - body: 'Error connecting or fetching data from Enterprise Search', - }); - } - }; + // Small helper + isEmptyObj(obj: object) { + return Object.keys(obj).length === 0; + } } diff --git a/x-pack/plugins/enterprise_search/server/plugin.ts b/x-pack/plugins/enterprise_search/server/plugin.ts index ef8c72f0cbca5..617210a544262 100644 --- a/x-pack/plugins/enterprise_search/server/plugin.ts +++ b/x-pack/plugins/enterprise_search/server/plugin.ts @@ -26,6 +26,11 @@ import { } from '../common/constants'; import { ConfigType } from './'; import { checkAccess } from './lib/check_access'; +import { + EnterpriseSearchRequestHandler, + IEnterpriseSearchRequestHandler, +} from './lib/enterprise_search_request_handler'; + import { registerConfigDataRoute } from './routes/enterprise_search/config_data'; import { registerTelemetryRoute } from './routes/enterprise_search/telemetry'; @@ -48,6 +53,7 @@ export interface IRouteDependencies { router: IRouter; config: ConfigType; log: Logger; + enterpriseSearchRequestHandler: IEnterpriseSearchRequestHandler; getSavedObjectsService?(): SavedObjectsServiceStart; } @@ -65,6 +71,7 @@ export class EnterpriseSearchPlugin implements Plugin { { usageCollection, security, features }: PluginsSetup ) { const config = await this.config.pipe(first()).toPromise(); + const log = this.logger; /** * Register space/feature control @@ -84,7 +91,7 @@ export class EnterpriseSearchPlugin implements Plugin { * Register user access to the Enterprise Search plugins */ capabilities.registerSwitcher(async (request: KibanaRequest) => { - const dependencies = { config, security, request, log: this.logger }; + const dependencies = { config, security, request, log }; const { hasAppSearchAccess, hasWorkplaceSearchAccess } = await checkAccess(dependencies); @@ -105,7 +112,8 @@ export class EnterpriseSearchPlugin implements Plugin { * Register routes */ const router = http.createRouter(); - const dependencies = { router, config, log: this.logger }; + const enterpriseSearchRequestHandler = new EnterpriseSearchRequestHandler({ config, log }); + const dependencies = { router, config, log, enterpriseSearchRequestHandler }; registerConfigDataRoute(dependencies); registerEnginesRoute(dependencies); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts index 682c17aea6d52..000e6d63b5999 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.test.ts @@ -4,15 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerCredentialsRoutes } from './credentials'; -jest.mock('../../lib/enterprise_search_request_handler', () => ({ - createEnterpriseSearchRequestHandler: jest.fn(), -})); -import { createEnterpriseSearchRequestHandler } from '../../lib/enterprise_search_request_handler'; - describe('credentials routes', () => { describe('GET /api/app_search/credentials', () => { let mockRouter: MockRouter; @@ -22,16 +17,13 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerCredentialsRoutes({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - it('creates a handler with createEnterpriseSearchRequestHandler', () => { - expect(createEnterpriseSearchRequestHandler).toHaveBeenCalledWith({ - config: mockConfig, - log: mockLogger, + it('creates a request handler', () => { + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ path: '/as/credentials/collection', hasValidData: expect.any(Function), }); @@ -59,11 +51,7 @@ describe('credentials routes', () => { ], }; - const { - hasValidData, - } = (createEnterpriseSearchRequestHandler as jest.Mock).mock.calls[0][0]; - - expect(hasValidData(response)).toBe(true); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); it('should correctly validate that a response does not have data', () => { @@ -71,10 +59,7 @@ describe('credentials routes', () => { foo: 'bar', }; - const hasValidData = (createEnterpriseSearchRequestHandler as jest.Mock).mock.calls[0][0] - .hasValidData; - - expect(hasValidData(response)).toBe(false); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts index d9539692069f0..432f54c8e5b1c 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts @@ -7,7 +7,6 @@ import { schema } from '@kbn/config-schema'; import { IRouteDependencies } from '../../plugin'; -import { createEnterpriseSearchRequestHandler } from '../../lib/enterprise_search_request_handler'; interface ICredential { id: string; @@ -28,7 +27,10 @@ interface ICredentialsResponse { }; } -export function registerCredentialsRoutes({ router, config, log }: IRouteDependencies) { +export function registerCredentialsRoutes({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/app_search/credentials', @@ -38,9 +40,7 @@ export function registerCredentialsRoutes({ router, config, log }: IRouteDepende }), }, }, - createEnterpriseSearchRequestHandler({ - config, - log, + enterpriseSearchRequestHandler.createRequest({ path: '/as/credentials/collection', hasValidData: (body?: ICredentialsResponse) => { return Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number'; diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts index 03edab89d1b99..cd22ff98b01ce 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts @@ -4,16 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerEnginesRoute } from './engines'; -jest.mock('node-fetch'); -const fetch = jest.requireActual('node-fetch'); -const { Response } = fetch; -// eslint-disable-next-line @typescript-eslint/no-var-requires -const fetchMock = require('node-fetch') as jest.Mocked; - describe('engine routes', () => { describe('GET /api/app_search/engines', () => { const AUTH_HEADER = 'Basic 123'; @@ -34,71 +28,51 @@ describe('engine routes', () => { mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerEnginesRoute({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - describe('when the underlying App Search API returns a 200', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturn({ - results: [{ name: 'engine1' }], - meta: { page: { total_results: 1 } }, - }); - }); - - it('should return 200 with a list of engines from the App Search API', async () => { - await mockRouter.callRoute(mockRequest); + it('creates a request handler', () => { + mockRouter.callRoute(mockRequest); - expect(mockRouter.response.ok).toHaveBeenCalledWith({ - body: { results: [{ name: 'engine1' }], meta: { page: { total_results: 1 } } }, - }); + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/collection', + params: { type: 'indexed', 'page[current]': 1, 'page[size]': 10 }, + hasValidData: expect.any(Function), }); }); - describe('when the App Search URL is invalid', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturnError(); - }); - - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); + it('passes custom parameters to enterpriseSearchRequestHandler', () => { + mockRouter.callRoute({ query: { type: 'meta', pageIndex: 99 } }); - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith('Cannot connect to App Search: Failed'); - expect(mockLogger.debug).not.toHaveBeenCalled(); + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/collection', + params: { type: 'meta', 'page[current]': 99, 'page[size]': 10 }, + hasValidData: expect.any(Function), }); }); - describe('when the App Search API returns invalid data', () => { - beforeEach(() => { - AppSearchAPI.shouldBeCalledWith( - `http://localhost:3002/as/engines/collection?type=indexed&page%5Bcurrent%5D=1&page%5Bsize%5D=10`, - { headers: { Authorization: AUTH_HEADER } } - ).andReturnInvalidData(); + describe('hasValidData', () => { + it('should correctly validate that the response has data', () => { + const response = { + meta: { + page: { + total_results: 1, + }, + }, + results: [], + }; + + mockRouter.callRoute(mockRequest); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith( - 'Cannot connect to App Search: Error: Invalid data received from App Search: {"foo":"bar"}' - ); - expect(mockLogger.debug).toHaveBeenCalled(); + it('should correctly validate that a response does not have data', () => { + const response = {}; + + mockRouter.callRoute(mockRequest); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); @@ -128,36 +102,5 @@ describe('engine routes', () => { mockRouter.shouldThrow(request); }); }); - - const AppSearchAPI = { - shouldBeCalledWith(expectedUrl: string, expectedParams: object) { - return { - andReturn(response: object) { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify(response))); - }); - }, - andReturnInvalidData() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify({ foo: 'bar' }))); - }); - }, - andReturnError() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.reject('Failed'); - }); - }, - }; - }, - }; }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts index 7190772fb92bb..49fc5b100e0d1 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts @@ -4,14 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import fetch from 'node-fetch'; -import querystring from 'querystring'; import { schema } from '@kbn/config-schema'; import { IRouteDependencies } from '../../plugin'; import { ENGINES_PAGE_SIZE } from '../../../common/constants'; -export function registerEnginesRoute({ router, config, log }: IRouteDependencies) { +interface IEnginesResponse { + results: object[]; + meta: { page: { total_results: number } }; +} + +export function registerEnginesRoute({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/app_search/engines', @@ -23,37 +29,18 @@ export function registerEnginesRoute({ router, config, log }: IRouteDependencies }, }, async (context, request, response) => { - try { - const enterpriseSearchUrl = config.host as string; - const { type, pageIndex } = request.query; + const { type, pageIndex } = request.query; - const params = querystring.stringify({ + return enterpriseSearchRequestHandler.createRequest({ + path: '/as/engines/collection', + params: { type, 'page[current]': pageIndex, 'page[size]': ENGINES_PAGE_SIZE, - }); - const url = `${encodeURI(enterpriseSearchUrl)}/as/engines/collection?${params}`; - - const enginesResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); - - const engines = await enginesResponse.json(); - const hasValidData = - Array.isArray(engines?.results) && typeof engines?.meta?.page?.total_results === 'number'; - - if (hasValidData) { - return response.ok({ body: engines }); - } else { - // Either a completely incorrect Enterprise Search host URL was configured, or App Search is returning bad data - throw new Error(`Invalid data received from App Search: ${JSON.stringify(engines)}`); - } - } catch (e) { - log.error(`Cannot connect to App Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); - - return response.customError({ statusCode: 502, body: 'cannot-connect' }); - } + }, + hasValidData: (body?: IEnginesResponse) => + Array.isArray(body?.results) && typeof body?.meta?.page?.total_results === 'number', + })(context, request, response); } ); } diff --git a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts index daf0a1e895a61..acddd3539965a 100644 --- a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/telemetry.test.ts @@ -5,7 +5,7 @@ */ import { loggingSystemMock, savedObjectsServiceMock } from 'src/core/server/mocks'; -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockLogger, mockDependencies } from '../../__mocks__'; jest.mock('../../collectors/lib/telemetry', () => ({ incrementUICounter: jest.fn(), @@ -28,10 +28,10 @@ describe('Enterprise Search Telemetry API', () => { mockRouter = new MockRouter({ method: 'put', payload: 'body' }); registerTelemetryRoute({ + ...mockDependencies, router: mockRouter.router, getSavedObjectsService: () => savedObjectsServiceMock.createStartContract(), log: mockLogger, - config: mockConfig, }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts index 69e8354e8b2f7..a9bd4020e74b7 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.test.ts @@ -4,127 +4,47 @@ * you may not use this file except in compliance with the Elastic License. */ -import { MockRouter, mockConfig, mockLogger } from '../../__mocks__'; +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; import { registerWSOverviewRoute } from './overview'; -jest.mock('node-fetch'); -const fetch = jest.requireActual('node-fetch'); -const { Response } = fetch; -// eslint-disable-next-line @typescript-eslint/no-var-requires -const fetchMock = require('node-fetch') as jest.Mocked; - -const ORG_ROUTE = 'http://localhost:3002/ws/org'; - -describe('engine routes', () => { +describe('Overview route', () => { describe('GET /api/workplace_search/overview', () => { - const AUTH_HEADER = 'Basic 123'; - const mockRequest = { - headers: { - authorization: AUTH_HEADER, - }, - query: {}, - }; - - const mockRouter = new MockRouter({ method: 'get', payload: 'query' }); + let mockRouter: MockRouter; beforeEach(() => { jest.clearAllMocks(); - mockRouter.createRouter(); + mockRouter = new MockRouter({ method: 'get', payload: 'query' }); registerWSOverviewRoute({ + ...mockDependencies, router: mockRouter.router, - log: mockLogger, - config: mockConfig, }); }); - describe('when the underlying Workplace Search API returns a 200', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturn({ accountsCount: 1 }); - }); - - it('should return 200 with a list of overview from the Workplace Search API', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.ok).toHaveBeenCalledWith({ - body: { accountsCount: 1 }, - headers: { 'content-type': 'application/json' }, - }); + it('creates a request handler', () => { + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/ws/org', + hasValidData: expect.any(Function), }); }); - describe('when the Workplace Search URL is invalid', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturnError(); - }); - - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); - - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith('Cannot connect to Workplace Search: Failed'); - expect(mockLogger.debug).not.toHaveBeenCalled(); - }); - }); + describe('hasValidData', () => { + it('should correctly validate that the response has data', () => { + const response = { + accountsCount: 1, + }; - describe('when the Workplace Search API returns invalid data', () => { - beforeEach(() => { - WorkplaceSearchAPI.shouldBeCalledWith(ORG_ROUTE, { - headers: { Authorization: AUTH_HEADER }, - }).andReturnInvalidData(); + expect(mockRequestHandler.hasValidData(response)).toBe(true); }); - it('should return 502 with a message', async () => { - await mockRouter.callRoute(mockRequest); + it('should correctly validate that a response does not have data', () => { + const response = { + foo: 'bar', + }; - expect(mockRouter.response.customError).toHaveBeenCalledWith({ - statusCode: 502, - body: 'cannot-connect', - }); - expect(mockLogger.error).toHaveBeenCalledWith( - 'Cannot connect to Workplace Search: Error: Invalid data received from Workplace Search: {"foo":"bar"}' - ); - expect(mockLogger.debug).toHaveBeenCalled(); + expect(mockRequestHandler.hasValidData(response)).toBe(false); }); }); - - const WorkplaceSearchAPI = { - shouldBeCalledWith(expectedUrl: string, expectedParams: object) { - return { - andReturn(response: object) { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify(response))); - }); - }, - andReturnInvalidData() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.resolve(new Response(JSON.stringify({ foo: 'bar' }))); - }); - }, - andReturnError() { - fetchMock.mockImplementation((url: string, params: object) => { - expect(url).toEqual(expectedUrl); - expect(params).toEqual(expectedParams); - - return Promise.reject('Failed'); - }); - }, - }; - }, - }; }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts index 9e5d94ac1b4fe..8cfd65725a23a 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/overview.ts @@ -4,43 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import fetch from 'node-fetch'; - import { IRouteDependencies } from '../../plugin'; -export function registerWSOverviewRoute({ router, config, log }: IRouteDependencies) { +export function registerWSOverviewRoute({ + router, + enterpriseSearchRequestHandler, +}: IRouteDependencies) { router.get( { path: '/api/workplace_search/overview', validate: false, }, - async (context, request, response) => { - try { - const entSearchUrl = config.host as string; - const url = `${encodeURI(entSearchUrl)}/ws/org`; - - const overviewResponse = await fetch(url, { - headers: { Authorization: request.headers.authorization as string }, - }); - - const body = await overviewResponse.json(); - const hasValidData = typeof body?.accountsCount === 'number'; - - if (hasValidData) { - return response.ok({ - body, - headers: { 'content-type': 'application/json' }, - }); - } else { - // Either a completely incorrect Enterprise Search host URL was configured, or Workplace Search is returning bad data - throw new Error(`Invalid data received from Workplace Search: ${JSON.stringify(body)}`); - } - } catch (e) { - log.error(`Cannot connect to Workplace Search: ${e.toString()}`); - if (e instanceof Error) log.debug(e.stack as string); - - return response.customError({ statusCode: 502, body: 'cannot-connect' }); - } - } + enterpriseSearchRequestHandler.createRequest({ + path: '/ws/org', + hasValidData: (body: { accountsCount: number }) => typeof body?.accountsCount === 'number', + }) ); }