Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.15] [OAS/HTTP] Empty response bodies (status only) and descriptions for responses (#188632) #190545

Merged
merged 1 commit into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions oas_docs/bundle.json
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@
"description": "Kibana's operational status. A minimal response is sent for unauthorized users."
}
}
}
},
"description": "Overall status is OK and Kibana should be functioning normally."
},
"503": {
"content": {
Expand All @@ -516,7 +517,8 @@
"description": "Kibana's operational status. A minimal response is sent for unauthorized users."
}
}
}
},
"description": "Kibana or some of it's essential services are unavailable. Kibana may be degraded or unavailable."
}
},
"summary": "Get Kibana's current status",
Expand Down
6 changes: 4 additions & 2 deletions oas_docs/bundle.serverless.json
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@
"description": "Kibana's operational status. A minimal response is sent for unauthorized users."
}
}
}
},
"description": "Overall status is OK and Kibana should be functioning normally."
},
"503": {
"content": {
Expand All @@ -516,7 +517,8 @@
"description": "Kibana's operational status. A minimal response is sent for unauthorized users."
}
}
}
},
"description": "Kibana or some of it's essential services are unavailable. Kibana may be degraded or unavailable."
}
},
"summary": "Get Kibana's current status",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,14 @@ describe('Router', () => {
isConfigSchema(
(
validationSchemas as () => RouteValidatorRequestAndResponses<unknown, unknown, unknown>
)().response![200].body()
)().response![200].body!()
)
).toBe(true);
expect(
isConfigSchema(
(
validationSchemas as () => RouteValidatorRequestAndResponses<unknown, unknown, unknown>
)().response![404].body()
)().response![404].body!()
)
).toBe(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ describe('prepareResponseValidation', () => {
404: {
body: jest.fn(() => schema.string()),
},
500: {
description: 'just a description',
body: undefined,
},
unsafe: {
body: true,
},
Expand All @@ -32,13 +36,15 @@ describe('prepareResponseValidation', () => {
expect(prepared).toEqual({
200: { body: expect.any(Function) },
404: { body: expect.any(Function) },
500: { description: 'just a description', body: undefined },
unsafe: { body: true },
});

[1, 2, 3].forEach(() => prepared[200].body());
[1, 2, 3].forEach(() => prepared[404].body());
[1, 2, 3].forEach(() => prepared[200].body!());
[1, 2, 3].forEach(() => prepared[404].body!());

expect(validation.response![200].body).toHaveBeenCalledTimes(1);
expect(validation.response![404].body).toHaveBeenCalledTimes(1);
expect(validation.response![500].body).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,22 @@
import { once } from 'lodash';
import {
isFullValidatorContainer,
type RouteValidatorFullConfigResponse,
type RouteConfig,
type RouteMethod,
type RouteValidator,
} from '@kbn/core-http-server';
import type { ObjectType, Type } from '@kbn/config-schema';

function isStatusCode(key: string) {
return !isNaN(parseInt(key, 10));
}

interface ResponseValidation {
[statusCode: number]: { body: () => ObjectType | Type<unknown> };
}

export function prepareResponseValidation(validation: ResponseValidation): ResponseValidation {
export function prepareResponseValidation(
validation: RouteValidatorFullConfigResponse
): RouteValidatorFullConfigResponse {
const responses = Object.entries(validation).map(([key, value]) => {
if (isStatusCode(key)) {
return [key, { body: once(value.body) }];
return [key, { ...value, ...(value.body ? { body: once(value.body) } : {}) }];
}
return [key, value];
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,12 @@ describe('Versioned route', () => {
] = route.handlers;

const res200 = (validate as () => VersionedRouteValidation<unknown, unknown, unknown>)()
.response![200].body;
.response![200].body!;

expect(isConfigSchema(unwrapVersionedResponseBodyValidation(res200))).toBe(true);

const res404 = (validate as () => VersionedRouteValidation<unknown, unknown, unknown>)()
.response![404].body;
.response![404].body!;

expect(isConfigSchema(unwrapVersionedResponseBodyValidation(res404))).toBe(true);

Expand Down Expand Up @@ -301,6 +301,33 @@ describe('Versioned route', () => {
expect(validateOutputFn).toHaveBeenCalledTimes(1);
});

it('handles "undefined" response schemas', async () => {
let handler: RequestHandler;

(router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn));
const versionedRouter = CoreVersionedRouter.from({ router, isDev: true });
versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion(
{
version: '1',
validate: { response: { 500: { description: 'jest description', body: undefined } } },
},
async (ctx, req, res) => res.custom({ statusCode: 500 })
);

await expect(
handler!(
{} as any,
createRequest({
version: '1',
body: { foo: 1 },
params: { foo: 1 },
query: { foo: 1 },
}),
responseFactory
)
).resolves.not.toThrow();
});

it('runs custom response validations', async () => {
let handler: RequestHandler;
const { fooValidation, validateBodyFn, validateOutputFn, validateParamsFn, validateQueryFn } =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,13 @@ export class CoreVersionedRoute implements VersionedRoute {

const response = await handler.fn(ctx, req, res);

if (this.router.isDev && validation?.response?.[response.status]) {
if (this.router.isDev && validation?.response?.[response.status]?.body) {
const { [response.status]: responseValidation, unsafe } = validation.response;
try {
validate(
{ body: response.payload },
{
body: unwrapVersionedResponseBodyValidation(responseValidation.body),
body: unwrapVersionedResponseBodyValidation(responseValidation.body!),
unsafe: { body: unsafe?.body },
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
*/

import { schema } from '@kbn/config-schema';
import { VersionedRouteResponseValidation } from '@kbn/core-http-server';
import { isCustomValidation, unwrapVersionedResponseBodyValidation } from './util';
import type { VersionedRouteResponseValidation } from '@kbn/core-http-server';
import {
isCustomValidation,
unwrapVersionedResponseBodyValidation,
prepareVersionedRouteValidation,
} from './util';

test.each([
[() => schema.object({}), false],
Expand All @@ -17,6 +21,43 @@ test.each([
expect(isCustomValidation(input)).toBe(result);
});

describe('prepareVersionedRouteValidation', () => {
it('wraps only expected values', () => {
const validate = {
request: {},
response: {
200: {
body: jest.fn(() => schema.string()),
},
404: {
body: jest.fn(() => schema.string()),
},
500: {
description: 'just a description',
body: undefined,
},
},
};

const prepared = prepareVersionedRouteValidation({
version: '1',
validate,
});

expect(prepared).toEqual({
version: '1',
validate: {
request: {},
response: {
200: { body: expect.any(Function) },
404: { body: expect.any(Function) },
500: { description: 'just a description', body: undefined },
},
},
});
});
});

test('unwrapVersionedResponseBodyValidation', () => {
const mySchema = schema.object({});
const custom = () => ({ value: 'ok' });
Expand All @@ -29,6 +70,6 @@ test('unwrapVersionedResponseBodyValidation', () => {
},
};

expect(unwrapVersionedResponseBodyValidation(validation[200].body)).toBe(mySchema);
expect(unwrapVersionedResponseBodyValidation(validation[404].body)).toBe(custom);
expect(unwrapVersionedResponseBodyValidation(validation[200].body!)).toBe(mySchema);
expect(unwrapVersionedResponseBodyValidation(validation[404].body!)).toBe(custom);
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function isCustomValidation(
* @internal
*/
export function unwrapVersionedResponseBodyValidation(
validation: VersionedRouteResponseValidation[number]['body']
validation: VersionedResponseBodyValidation
): RouteValidationSpec<unknown> {
if (isCustomValidation(validation)) {
return validation.custom;
Expand All @@ -43,8 +43,15 @@ function prepareValidation(validation: VersionedRouteValidation<unknown, unknown
const { unsafe, ...responseValidations } = validation.response;
const result: VersionedRouteResponseValidation = {};

for (const [key, { body }] of Object.entries(responseValidations)) {
result[key as unknown as number] = { body: isCustomValidation(body) ? body : once(body) };
for (const [key, value] of Object.entries(responseValidations)) {
result[key as unknown as number] = {
...value,
};
if (value.body) {
result[key as unknown as number].body = isCustomValidation(value.body)
? value.body
: once(value.body);
}
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,15 @@ export type RouteValidatorFullConfigRequest<P, Q, B> = RouteValidatorConfig<P, Q
*/
export interface RouteValidatorFullConfigResponse {
[statusCode: number]: {
/**
* A description of the response. This is required input for complete OAS documentation.
*/
description?: string;
/**
* A string representing the mime type of the response body.
*/
bodyContentType?: string;
body: LazyValidator;
body?: LazyValidator;
};
unsafe?: {
body?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,15 @@ export type VersionedResponseBodyValidation =
*/
export interface VersionedRouteResponseValidation {
[statusCode: number]: {
/**
* A description of the response. This is required input for complete OAS documentation.
*/
description?: string;
/**
* A string representing the mime type of the response body.
*/
bodyContentType?: string;
body: VersionedResponseBodyValidation;
body?: VersionedResponseBodyValidation;
};
unsafe?: { body?: boolean };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ export const registerStatusRoute = ({
},
response: {
200: {
description: 'Overall status is OK and Kibana should be functioning normally.',
body: statusResponse,
},
503: {
description: `Kibana or some of it's essential services are unavailable. Kibana may be degraded or unavailable.`,
body: statusResponse,
},
},
Expand Down
Loading
Loading