Skip to content

Commit

Permalink
[Enterprise Search] Refactor MockRouter test helper to not store payl…
Browse files Browse the repository at this point in the history
…oad (#90206)

* 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
  • Loading branch information
Constance authored and cee-chen committed Feb 4, 2021
1 parent c78a5a2 commit e969d90
Show file tree
Hide file tree
Showing 13 changed files with 9 additions and 63 deletions.
16 changes: 7 additions & 9 deletions x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ type PayloadType = 'params' | 'query' | 'body';
interface IMockRouter {
method: MethodType;
path: string;
payload?: PayloadType;
}
interface IMockRouterRequest {
body?: object;
Expand All @@ -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 = () => {
Expand All @@ -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) => {
Expand Down Expand Up @@ -99,7 +98,6 @@ export class MockRouter {
// const mockRouter = new MockRouter({
// method: 'get',
// path: '/api/app_search/test',
// payload: 'body'
// });
//
// beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('analytics routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/app_search/engines/{engineName}/analytics/queries',
payload: 'query',
});

registerAnalyticsRoutes({
Expand Down Expand Up @@ -71,7 +70,6 @@ describe('analytics routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/app_search/engines/{engineName}/analytics/queries/{query}',
payload: 'query',
});

registerAnalyticsRoutes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('credentials routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/app_search/credentials',
payload: 'query',
});

registerCredentialsRoutes({
Expand Down Expand Up @@ -54,7 +53,6 @@ describe('credentials routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/app_search/credentials',
payload: 'body',
});

registerCredentialsRoutes({
Expand Down Expand Up @@ -167,7 +165,6 @@ describe('credentials routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/app_search/credentials/details',
payload: 'query',
});

registerCredentialsRoutes({
Expand All @@ -191,7 +188,6 @@ describe('credentials routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/app_search/credentials/{name}',
payload: 'body',
});

registerCredentialsRoutes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('documents routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/app_search/engines/{engineName}/documents',
payload: 'body',
});

registerDocumentsRoutes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe('engine routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/app_search/engines',
payload: 'query',
});

registerEnginesRoutes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ describe('log settings routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/app_search/log_settings',
payload: 'body',
});

registerSettingsRoutes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe('Enterprise Search Telemetry API', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/enterprise_search/stats',
payload: 'body',
});

registerTelemetryRoute({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/workplace_search/groups',
payload: 'query',
});

registerGroupsRoute({
Expand All @@ -50,7 +49,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/workplace_search/groups',
payload: 'body',
});

registerGroupsRoute({
Expand Down Expand Up @@ -85,7 +83,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/workplace_search/groups/search',
payload: 'body',
});

registerSearchGroupsRoute({
Expand Down Expand Up @@ -163,7 +160,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/workplace_search/groups/{id}',
payload: 'body',
});

registerGroupRoute({
Expand Down Expand Up @@ -246,7 +242,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/workplace_search/groups/{id}/share',
payload: 'body',
});

registerShareGroupRoute({
Expand Down Expand Up @@ -282,7 +277,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'post',
path: '/api/workplace_search/groups/{id}/assign',
payload: 'body',
});

registerAssignGroupRoute({
Expand Down Expand Up @@ -318,7 +312,6 @@ describe('groups routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/workplace_search/groups/{id}/boosts',
payload: 'body',
});

registerBoostsGroupRoute({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('Overview route', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/workplace_search/overview',
payload: 'query',
});

registerOverviewRoute({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ describe('security routes', () => {
mockRouter = new MockRouter({
method: 'get',
path: '/api/workplace_search/org/security/source_restrictions',
payload: 'body',
});

registerSecuritySourceRestrictionsRoute({
Expand All @@ -72,7 +71,6 @@ describe('security routes', () => {
mockRouter = new MockRouter({
method: 'patch',
path: '/api/workplace_search/org/security/source_restrictions',
payload: 'body',
});

registerSecuritySourceRestrictionsRoute({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ describe('settings routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/workplace_search/org/settings/customize',
payload: 'body',
});

registerOrgSettingsCustomizeRoute({
Expand Down Expand Up @@ -76,7 +75,6 @@ describe('settings routes', () => {
mockRouter = new MockRouter({
method: 'put',
path: '/api/workplace_search/org/settings/oauth_application',
payload: 'body',
});

registerOrgSettingsOauthApplicationRoute({
Expand Down
Loading

0 comments on commit e969d90

Please sign in to comment.