From da7c3b430b170912018daea164b9e9a61c647c6a Mon Sep 17 00:00:00 2001 From: Constance Date: Thu, 4 Feb 2021 13:50:52 -0800 Subject: [PATCH] [Enterprise Search] Refactor MockRouter test helper to not store payload (#90206) (#90322) * Update MockRouter to not pass/set a this.payload - but instead intelligently validate payloads based on the request keys * Fix relevance tuning API routes to not need a separate mock router for validating query & body * Update all remaining tests to no longer pass a payload param to MockRouter --- .../server/__mocks__/router.mock.ts | 16 +++++++--------- .../server/routes/app_search/analytics.test.ts | 2 -- .../routes/app_search/credentials.test.ts | 4 ---- .../server/routes/app_search/documents.test.ts | 1 - .../server/routes/app_search/engines.test.ts | 1 - .../routes/app_search/search_settings.test.ts | 17 ++--------------- .../server/routes/app_search/settings.test.ts | 1 - .../routes/enterprise_search/telemetry.test.ts | 1 - .../routes/workplace_search/groups.test.ts | 7 ------- .../routes/workplace_search/overview.test.ts | 1 - .../routes/workplace_search/security.test.ts | 2 -- .../routes/workplace_search/settings.test.ts | 2 -- .../routes/workplace_search/sources.test.ts | 17 ----------------- 13 files changed, 9 insertions(+), 63 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts index 7fde7934cf7ad..88cf30bb2a549 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts @@ -23,7 +23,6 @@ type PayloadType = 'params' | 'query' | 'body'; interface IMockRouter { method: MethodType; path: string; - payload?: PayloadType; } interface IMockRouterRequest { body?: object; @@ -39,11 +38,10 @@ export class MockRouter { public payload?: PayloadType; public response = httpServerMock.createResponseFactory(); - constructor({ method, path, payload }: IMockRouter) { + constructor({ method, path }: IMockRouter) { this.createRouter(); this.method = method; this.path = path; - this.payload = payload; } public createRouter = () => { @@ -62,16 +60,17 @@ export class MockRouter { */ public validateRoute = (request: MockRouterRequest) => { - if (!this.payload) throw new Error('Cannot validate wihout a payload type specified.'); - const route = this.findRouteRegistration(); const [config] = route; const validate = config.validate as RouteValidatorConfig<{}, {}, {}>; + const payloads = Object.keys(request) as PayloadType[]; - const payloadValidation = validate[this.payload] as { validate(request: KibanaRequest): void }; - const payloadRequest = request[this.payload] as KibanaRequest; + payloads.forEach((payload: PayloadType) => { + const payloadValidation = validate[payload] as { validate(request: KibanaRequest): void }; + const payloadRequest = request[payload] as KibanaRequest; - payloadValidation.validate(payloadRequest); + payloadValidation.validate(payloadRequest); + }); }; public shouldValidate = (request: MockRouterRequest) => { @@ -99,7 +98,6 @@ export class MockRouter { // const mockRouter = new MockRouter({ // method: 'get', // path: '/api/app_search/test', -// payload: 'body' // }); // // beforeEach(() => { diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/analytics.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/analytics.test.ts index 3d63e4044e75b..8e4a7dba165b1 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/analytics.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/analytics.test.ts @@ -18,7 +18,6 @@ describe('analytics routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/app_search/engines/{engineName}/analytics/queries', - payload: 'query', }); registerAnalyticsRoutes({ @@ -71,7 +70,6 @@ describe('analytics routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/app_search/engines/{engineName}/analytics/queries/{query}', - payload: 'query', }); registerAnalyticsRoutes({ 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 7a513b1c76b4e..d9e84d3e62f28 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 @@ -18,7 +18,6 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/app_search/credentials', - payload: 'query', }); registerCredentialsRoutes({ @@ -54,7 +53,6 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/app_search/credentials', - payload: 'body', }); registerCredentialsRoutes({ @@ -167,7 +165,6 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/app_search/credentials/details', - payload: 'query', }); registerCredentialsRoutes({ @@ -191,7 +188,6 @@ describe('credentials routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/app_search/credentials/{name}', - payload: 'body', }); registerCredentialsRoutes({ diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/documents.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/documents.test.ts index fdae51444bb54..af54d340ad150 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/documents.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/documents.test.ts @@ -18,7 +18,6 @@ describe('documents routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/app_search/engines/{engineName}/documents', - payload: 'body', }); registerDocumentsRoutes({ 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 e874a188a10f7..abd26e18c7b9d 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 @@ -29,7 +29,6 @@ describe('engine routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/app_search/engines', - payload: 'query', }); registerEnginesRoutes({ diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts index 92a695af12aaa..d8f677e2f0d82 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts @@ -87,7 +87,6 @@ describe('search settings routes', () => { const mockRouter = new MockRouter({ method: 'put', path: '/api/app_search/engines/{engineName}/search_settings', - payload: 'body', }); beforeEach(() => { @@ -149,7 +148,6 @@ describe('search settings routes', () => { const mockRouter = new MockRouter({ method: 'post', path: '/api/app_search/engines/{engineName}/search_settings_search', - payload: 'body', }); beforeEach(() => { @@ -188,29 +186,18 @@ describe('search settings routes', () => { }); describe('validates query', () => { - const queryRouter = new MockRouter({ - method: 'post', - path: '/api/app_search/engines/{engineName}/search_settings_search', - payload: 'query', - }); - it('correctly', () => { - registerSearchSettingsRoutes({ - ...mockDependencies, - router: queryRouter.router, - }); - const request = { query: { query: 'foo', }, }; - queryRouter.shouldValidate(request); + mockRouter.shouldValidate(request); }); it('missing required fields', () => { const request = { query: {} }; - queryRouter.shouldThrow(request); + mockRouter.shouldThrow(request); }); }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/settings.test.ts index 5d56bbf4fcd11..6df9a4f16d710 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/settings.test.ts @@ -41,7 +41,6 @@ describe('log settings routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/app_search/log_settings', - payload: 'body', }); registerSettingsRoutes({ 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 f41ad367839c3..08c398ba3eb0d 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 @@ -29,7 +29,6 @@ describe('Enterprise Search Telemetry API', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/enterprise_search/stats', - payload: 'body', }); registerTelemetryRoute({ diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/groups.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/groups.test.ts index e67ca4c064886..68a9ae725f8a4 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/groups.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/groups.test.ts @@ -26,7 +26,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/workplace_search/groups', - payload: 'query', }); registerGroupsRoute({ @@ -50,7 +49,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/groups', - payload: 'body', }); registerGroupsRoute({ @@ -85,7 +83,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/groups/search', - payload: 'body', }); registerSearchGroupsRoute({ @@ -163,7 +160,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/groups/{id}', - payload: 'body', }); registerGroupRoute({ @@ -246,7 +242,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/groups/{id}/share', - payload: 'body', }); registerShareGroupRoute({ @@ -282,7 +277,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/groups/{id}/assign', - payload: 'body', }); registerAssignGroupRoute({ @@ -318,7 +312,6 @@ describe('groups routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/groups/{id}/boosts', - payload: 'body', }); registerBoostsGroupRoute({ 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 1afb85b389b42..bdf885648dff7 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 @@ -18,7 +18,6 @@ describe('Overview route', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/workplace_search/overview', - payload: 'query', }); registerOverviewRoute({ diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/security.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/security.test.ts index f2117a8bc948a..a1615499c56a2 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/security.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/security.test.ts @@ -45,7 +45,6 @@ describe('security routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/workplace_search/org/security/source_restrictions', - payload: 'body', }); registerSecuritySourceRestrictionsRoute({ @@ -72,7 +71,6 @@ describe('security routes', () => { mockRouter = new MockRouter({ method: 'patch', path: '/api/workplace_search/org/security/source_restrictions', - payload: 'body', }); registerSecuritySourceRestrictionsRoute({ diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/settings.test.ts index cf654918beb49..00a5b6c75df0a 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/settings.test.ts @@ -45,7 +45,6 @@ describe('settings routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/org/settings/customize', - payload: 'body', }); registerOrgSettingsCustomizeRoute({ @@ -76,7 +75,6 @@ describe('settings routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/org/settings/oauth_application', - payload: 'body', }); registerOrgSettingsOauthApplicationRoute({ diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.test.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.test.ts index 2ae10e85ea9c0..a2fbe759f1a11 100644 --- a/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.test.ts @@ -154,7 +154,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/account/create_source', - payload: 'body', }); registerAccountCreateSourceRoute({ @@ -194,7 +193,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/account/sources/{id}/documents', - payload: 'body', }); registerAccountSourceDocumentsRoute({ @@ -281,7 +279,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'patch', path: '/api/workplace_search/account/sources/{id}/settings', - payload: 'body', }); registerAccountSourceSettingsRoute({ @@ -364,7 +361,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/account/sources/{id}/searchable', - payload: 'body', }); registerAccountSourceSearchableRoute({ @@ -422,7 +418,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/account/sources/{id}/display_settings/config', - payload: 'body', }); registerAccountSourceDisplaySettingsConfig({ @@ -489,7 +484,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/account/sources/{id}/schemas', - payload: 'body', }); registerAccountSourceSchemasRoute({ @@ -667,7 +661,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/create_source', - payload: 'body', }); registerOrgCreateSourceRoute({ @@ -707,7 +700,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/sources/{id}/documents', - payload: 'body', }); registerOrgSourceDocumentsRoute({ @@ -794,7 +786,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'patch', path: '/api/workplace_search/org/sources/{id}/settings', - payload: 'body', }); registerOrgSourceSettingsRoute({ @@ -877,7 +868,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/org/sources/{id}/searchable', - payload: 'body', }); registerOrgSourceSearchableRoute({ @@ -935,7 +925,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/sources/{id}/display_settings/config', - payload: 'body', }); registerOrgSourceDisplaySettingsConfig({ @@ -1002,7 +991,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/sources/{id}/schemas', - payload: 'body', }); registerOrgSourceSchemasRoute({ @@ -1102,7 +1090,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/settings/connectors', - payload: 'body', }); registerOrgSourceOauthConfigurationsRoute({ @@ -1133,7 +1120,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/org/settings/connectors', - payload: 'body', }); registerOrgSourceOauthConfigurationsRoute({ @@ -1187,7 +1173,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'post', path: '/api/workplace_search/org/settings/connectors/{serviceType}', - payload: 'body', }); registerOrgSourceOauthConfigurationRoute({ @@ -1218,7 +1203,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'put', path: '/api/workplace_search/org/settings/connectors/{serviceType}', - payload: 'body', }); registerOrgSourceOauthConfigurationRoute({ @@ -1272,7 +1256,6 @@ describe('sources routes', () => { mockRouter = new MockRouter({ method: 'get', path: '/api/workplace_search/sources/create', - payload: 'query', }); registerOauthConnectorParamsRoute({