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

## Summary

* Adds the ability to exclude a response schema when declaring route
schemas
* Adds the ability to provide a description of a the response

See code comments for more info.

## Example

You can declare a response with no validation to imply an empty object
in OAS

```
router.versioned.post({ version: '2023-10-31', access: 'public', path: '...' })
  .addVersion({
    validation: {
      responses: {
        201: { description: 'Resource created!' }
      }
    }
  }, () => {})
```

Will result in

```jsonc
{
 //...
  201: { description: 'Resource created!' }
 //...
}
```

## Risks

No notable risks
This commit is contained in:
Jean-Louis Leysens 2024-07-22 15:29:15 +02:00 committed by GitHub
parent 06b111d993
commit a8091ab0ac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 332 additions and 110 deletions

View file

@ -395,7 +395,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": {
@ -412,7 +413,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",

View file

@ -395,7 +395,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": {
@ -412,7 +413,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",

View file

@ -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);
}

View file

@ -21,6 +21,10 @@ describe('prepareResponseValidation', () => {
404: {
body: jest.fn(() => schema.string()),
},
500: {
description: 'just a description',
body: undefined,
},
unsafe: {
body: true,
},
@ -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();
});
});

View file

@ -9,25 +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';
import type { ZodEsque } from '@kbn/zod';
function isStatusCode(key: string) {
return !isNaN(parseInt(key, 10));
}
interface ResponseValidation {
[statusCode: number]: { body: () => ObjectType | Type<unknown> | ZodEsque<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];
});

View file

@ -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);
@ -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 } =

View file

@ -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 },
}
);

View file

@ -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],
@ -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' });
@ -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);
});

View file

@ -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;
@ -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 {

View file

@ -173,11 +173,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;

View file

@ -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 };
}

View file

@ -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,
},
},

View file

@ -89,6 +89,7 @@ Object {
},
},
},
"description": undefined,
},
},
"summary": "",
@ -215,6 +216,8 @@ Object {
},
},
},
"description": "OK response oas-test-version-1
OK response oas-test-version-2",
},
},
"summary": "versioned route",
@ -224,6 +227,34 @@ Object {
},
},
"/foo/{id}/{path*}": Object {
"delete": Object {
"description": "route description",
"operationId": "/foo/{id}/{path*}#2",
"parameters": Array [
Object {
"description": "The version of the API to use",
"in": "header",
"name": "elastic-api-version",
"schema": Object {
"default": "2023-10-31",
"enum": Array [
"2023-10-31",
],
"type": "string",
},
},
],
"requestBody": undefined,
"responses": Object {
"200": Object {
"description": "good response",
},
},
"summary": "route summary",
"tags": Array [
"bar",
],
},
"get": Object {
"description": "route description",
"operationId": "/foo/{id}/{path*}#0",
@ -362,6 +393,7 @@ Object {
},
},
},
"description": undefined,
},
},
"summary": "route summary",
@ -507,6 +539,7 @@ Object {
},
},
},
"description": undefined,
},
},
"summary": "route summary",
@ -615,6 +648,7 @@ Object {
},
},
},
"description": undefined,
},
},
"summary": "",
@ -691,6 +725,7 @@ Object {
"schema": Object {},
},
},
"description": undefined,
},
},
"summary": "",
@ -728,6 +763,7 @@ Object {
"schema": Object {},
},
},
"description": undefined,
},
},
"summary": "",

View file

@ -121,6 +121,7 @@ export const sharedOas = {
},
responses: {
'200': {
description: 'OK response oas-test-version-1\nOK response oas-test-version-2',
content: {
'application/json; Elastic-Api-Version=oas-test-version-1': {
schema: {

View file

@ -45,7 +45,21 @@ describe('generateOpenApiDocument', () => {
it('generates the expected OpenAPI document', () => {
const [routers, versionedRouters] = createTestRouters({
routers: { testRouter: { routes: [{ method: 'get' }, { method: 'post' }] } },
routers: {
testRouter: {
routes: [
{ method: 'get' },
{ method: 'post' },
{
method: 'delete',
validationSchemas: {
request: {},
response: { [200]: { description: 'good response' } },
},
},
],
},
},
versionedRouters: { testVersionedRouter: { routes: [{}] } },
});
expect(

View file

@ -83,6 +83,7 @@ export const getVersionedRouterDefaults = (bodySchema?: RuntimeSchema) => ({
},
response: {
[200]: {
description: 'OK response oas-test-version-1',
body: () =>
schema.object(
{ fooResponseWithDescription: schema.string() },
@ -101,6 +102,7 @@ export const getVersionedRouterDefaults = (bodySchema?: RuntimeSchema) => ({
request: { body: schema.object({ foo: schema.string() }) },
response: {
[200]: {
description: 'OK response oas-test-version-2',
body: () => schema.stream({ meta: { description: 'stream response' } }),
bodyContentType: 'application/octet-stream',
},

View file

@ -33,10 +33,12 @@ describe('extractResponses', () => {
response: {
200: {
bodyContentType: 'application/test+json',
description: 'OK response',
body: () => schema.object({ bar: schema.number({ min: 1, max: 99 }) }),
},
404: {
bodyContentType: 'application/test2+json',
description: 'Not Found response',
body: () => schema.object({ ok: schema.literal(false) }),
},
unsafe: { body: false },
@ -45,6 +47,7 @@ describe('extractResponses', () => {
};
expect(extractResponses(route, oasConverter)).toEqual({
200: {
description: 'OK response',
content: {
'application/test+json; Elastic-Api-Version=2023-10-31': {
schema: {
@ -59,6 +62,7 @@ describe('extractResponses', () => {
},
},
404: {
description: 'Not Found response',
content: {
'application/test2+json; Elastic-Api-Version=2023-10-31': {
schema: {

View file

@ -19,6 +19,7 @@ import {
getPathParameters,
getVersionedContentTypeString,
getVersionedHeaderParam,
mergeResponseContent,
prepareRoutes,
} from './util';
import type { OperationIdCounter } from './operation_id_counter';
@ -102,18 +103,23 @@ export const extractResponses = (route: InternalRouterRoute, converter: OasConve
const contentType = extractContentType(route.options?.body);
return Object.entries(validationSchemas).reduce<OpenAPIV3.ResponsesObject>(
(acc, [statusCode, schema]) => {
const oasSchema = converter.convert(schema.body());
const newContent = schema.body
? {
[getVersionedContentTypeString(
SERVERLESS_VERSION_2023_10_31,
schema.bodyContentType ? [schema.bodyContentType] : contentType
)]: {
schema: converter.convert(schema.body()),
},
}
: undefined;
acc[statusCode] = {
...acc[statusCode],
content: {
...((acc[statusCode] ?? {}) as OpenAPIV3.ResponseObject).content,
[getVersionedContentTypeString(
SERVERLESS_VERSION_2023_10_31,
schema.bodyContentType ? [schema.bodyContentType] : contentType
)]: {
schema: oasSchema,
},
},
description: schema.description!,
...mergeResponseContent(
((acc[statusCode] ?? {}) as OpenAPIV3.ResponseObject).content,
newContent
),
};
return acc;
},

View file

@ -20,60 +20,6 @@ import {
extractVersionedRequestBodies,
} from './process_versioned_router';
const route: VersionedRouterRoute = {
path: '/foo',
method: 'get',
options: {
access: 'public',
options: { body: { access: ['application/test+json'] } as any },
},
handlers: [
{
fn: jest.fn(),
options: {
version: '2023-10-31',
validate: () => ({
request: {
body: schema.object({ foo: schema.string() }),
},
response: {
200: {
bodyContentType: 'application/test+json',
body: () => schema.object({ bar: schema.number({ min: 1, max: 99 }) }),
},
404: {
bodyContentType: 'application/test2+json',
body: () => schema.object({ ok: schema.literal(false) }),
},
unsafe: { body: false },
},
}),
},
},
{
fn: jest.fn(),
options: {
version: '2024-12-31',
validate: () => ({
request: {
body: schema.object({ foo2: schema.string() }),
},
response: {
200: {
bodyContentType: 'application/test+json',
body: () => schema.object({ bar2: schema.number({ min: 1, max: 99 }) }),
},
500: {
bodyContentType: 'application/test2+json',
body: () => schema.object({ ok: schema.literal(false) }),
},
unsafe: { body: false },
},
}),
},
},
],
};
let oasConverter: OasConverter;
beforeEach(() => {
oasConverter = new OasConverter();
@ -81,7 +27,9 @@ beforeEach(() => {
describe('extractVersionedRequestBodies', () => {
test('handles full request config as expected', () => {
expect(extractVersionedRequestBodies(route, oasConverter, ['application/json'])).toEqual({
expect(
extractVersionedRequestBodies(createTestRoute(), oasConverter, ['application/json'])
).toEqual({
'application/json; Elastic-Api-Version=2023-10-31': {
schema: {
additionalProperties: false,
@ -112,8 +60,11 @@ describe('extractVersionedRequestBodies', () => {
describe('extractVersionedResponses', () => {
test('handles full response config as expected', () => {
expect(extractVersionedResponses(route, oasConverter, ['application/test+json'])).toEqual({
expect(
extractVersionedResponses(createTestRoute(), oasConverter, ['application/test+json'])
).toEqual({
200: {
description: 'OK response 2023-10-31\nOK response 2024-12-31', // merge multiple version descriptions
content: {
'application/test+json; Elastic-Api-Version=2023-10-31': {
schema: {
@ -138,6 +89,7 @@ describe('extractVersionedResponses', () => {
},
},
404: {
description: 'Not Found response 2023-10-31',
content: {
'application/test2+json; Elastic-Api-Version=2023-10-31': {
schema: {
@ -172,7 +124,7 @@ describe('extractVersionedResponses', () => {
describe('processVersionedRouter', () => {
it('correctly extracts the version based on the version filter', () => {
const baseCase = processVersionedRouter(
{ getRoutes: () => [route] } as unknown as CoreVersionedRouter,
{ getRoutes: () => [createTestRoute()] } as unknown as CoreVersionedRouter,
new OasConverter(),
createOperationIdCounter(),
{}
@ -184,7 +136,7 @@ describe('processVersionedRouter', () => {
]);
const filteredCase = processVersionedRouter(
{ getRoutes: () => [route] } as unknown as CoreVersionedRouter,
{ getRoutes: () => [createTestRoute()] } as unknown as CoreVersionedRouter,
new OasConverter(),
createOperationIdCounter(),
{ version: '2023-10-31' }
@ -194,3 +146,61 @@ describe('processVersionedRouter', () => {
]);
});
});
const createTestRoute: () => VersionedRouterRoute = () => ({
path: '/foo',
method: 'get',
options: {
access: 'public',
options: { body: { access: ['application/test+json'] } as any },
},
handlers: [
{
fn: jest.fn(),
options: {
version: '2023-10-31',
validate: () => ({
request: {
body: schema.object({ foo: schema.string() }),
},
response: {
200: {
description: 'OK response 2023-10-31',
bodyContentType: 'application/test+json',
body: () => schema.object({ bar: schema.number({ min: 1, max: 99 }) }),
},
404: {
description: 'Not Found response 2023-10-31',
bodyContentType: 'application/test2+json',
body: () => schema.object({ ok: schema.literal(false) }),
},
unsafe: { body: false },
},
}),
},
},
{
fn: jest.fn(),
options: {
version: '2024-12-31',
validate: () => ({
request: {
body: schema.object({ foo2: schema.string() }),
},
response: {
200: {
description: 'OK response 2024-12-31',
bodyContentType: 'application/test+json',
body: () => schema.object({ bar2: schema.number({ min: 1, max: 99 }) }),
},
500: {
bodyContentType: 'application/test2+json',
body: () => schema.object({ ok: schema.literal(false) }),
},
unsafe: { body: false },
},
}),
},
},
],
});

View file

@ -15,6 +15,7 @@ import {
import type { OpenAPIV3 } from 'openapi-types';
import type { GenerateOpenApiDocumentOptionsFilters } from './generate_oas';
import type { OasConverter } from './oas_converter';
import { isReferenceObject } from './oas_converter/common';
import type { OperationIdCounter } from './operation_id_counter';
import {
prepareRoutes,
@ -24,6 +25,7 @@ import {
getVersionedHeaderParam,
getVersionedContentTypeString,
extractTags,
mergeResponseContent,
} from './util';
export const processVersionedRouter = (
@ -153,31 +155,49 @@ export const extractVersionedResponse = (
const result: OpenAPIV3.ResponsesObject = {};
const { unsafe, ...responses } = schemas.response;
for (const [statusCode, responseSchema] of Object.entries(responses)) {
const maybeSchema = unwrapVersionedResponseBodyValidation(responseSchema.body);
const schema = converter.convert(maybeSchema);
const contentTypeString = getVersionedContentTypeString(
handler.options.version,
responseSchema.bodyContentType ? [responseSchema.bodyContentType] : contentType
);
result[statusCode] = {
...result[statusCode],
content: {
...((result[statusCode] ?? {}) as OpenAPIV3.ResponseObject).content,
let newContent: OpenAPIV3.ResponseObject['content'];
if (responseSchema.body) {
const maybeSchema = unwrapVersionedResponseBodyValidation(responseSchema.body);
const schema = converter.convert(maybeSchema);
const contentTypeString = getVersionedContentTypeString(
handler.options.version,
responseSchema.bodyContentType ? [responseSchema.bodyContentType] : contentType
);
newContent = {
[contentTypeString]: {
schema,
},
},
};
}
result[statusCode] = {
...result[statusCode],
description: responseSchema.description!,
...mergeResponseContent(
((result[statusCode] ?? {}) as OpenAPIV3.ResponseObject).content,
newContent
),
};
}
return result;
};
const mergeDescriptions = (
existing: undefined | string,
toAppend: OpenAPIV3.ResponsesObject[string]
): string | undefined => {
if (!isReferenceObject(toAppend) && toAppend.description) {
return existing?.length ? `${existing}\n${toAppend.description}` : toAppend.description;
}
return existing;
};
const mergeVersionedResponses = (a: OpenAPIV3.ResponsesObject, b: OpenAPIV3.ResponsesObject) => {
const result: OpenAPIV3.ResponsesObject = Object.assign({}, a);
for (const [statusCode, responseContent] of Object.entries(b)) {
const existing = (result[statusCode] as OpenAPIV3.ResponseObject) ?? {};
result[statusCode] = {
...result[statusCode],
description: mergeDescriptions(existing.description, responseContent)!,
content: Object.assign(
{},
existing.content,

View file

@ -7,7 +7,7 @@
*/
import { OpenAPIV3 } from 'openapi-types';
import { buildGlobalTags, prepareRoutes } from './util';
import { buildGlobalTags, mergeResponseContent, prepareRoutes } from './util';
import { assignToPaths, extractTags } from './util';
describe('extractTags', () => {
@ -159,3 +159,29 @@ describe('prepareRoutes', () => {
expect(prepareRoutes(input, filters)).toEqual(output);
});
});
describe('mergeResponseContent', () => {
it('returns an empty object if no content is provided', () => {
expect(mergeResponseContent(undefined, undefined)).toEqual({});
expect(mergeResponseContent({}, {})).toEqual({});
});
it('merges content objects', () => {
expect(
mergeResponseContent(
{
['application/json+v1']: { encoding: {} },
},
{
['application/json+v1']: { example: 'overridden' },
['application/json+v2']: {},
}
)
).toEqual({
content: {
['application/json+v1']: { example: 'overridden' },
['application/json+v2']: {},
},
});
});
});

View file

@ -131,3 +131,14 @@ export const assignToPaths = (
const pathName = path.replace('?', '');
paths[pathName] = { ...paths[pathName], ...pathObject };
};
export const mergeResponseContent = (
a: OpenAPIV3.ResponseObject['content'],
b: OpenAPIV3.ResponseObject['content']
) => {
const mergedContent = {
...(a ?? {}),
...(b ?? {}),
};
return { ...(Object.keys(mergedContent).length ? { content: mergedContent } : {}) };
};