Skip to content

Commit

Permalink
[8.15] [OAS/HTTP] Empty response bodies (status only) and description…
Browse files Browse the repository at this point in the history
…s for responses (#188632) (#190545)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[OAS/HTTP] Empty response bodies (status only) and descriptions for
responses (#188632)](#188632)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"jeanlouis.leysens@elastic.co"},"sourceCommit":{"committedDate":"2024-07-22T13:29:15Z","message":"[OAS/HTTP]
Empty response bodies (status only) and descriptions for responses
(#188632)\n\n## Summary\r\n\r\n* Adds the ability to exclude a response
schema when declaring route\r\nschemas\r\n* Adds the ability to provide
a description of a the response\r\n\r\nSee code comments for more
info.\r\n\r\n## Example\r\n\r\nYou can declare a response with no
validation to imply an empty object\r\nin
OAS\r\n\r\n```\r\nrouter.versioned.post({ version: '2023-10-31', access:
'public', path: '...' })\r\n .addVersion({\r\n validation: {\r\n
responses: {\r\n 201: { description: 'Resource created!' }\r\n }\r\n
}\r\n }, () => {})\r\n```\r\n\r\nWill result in\r\n\r\n```jsonc\r\n{\r\n
//...\r\n 201: { description: 'Resource created!' }\r\n
//...\r\n}\r\n```\r\n\r\n## Risks\r\n\r\nNo notable
risks","sha":"a8091ab0acff6f8fae2cbe19c273048fb2734632","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:http","Team:Core","release_note:skip","backport:skip","v8.15.0","Feature:OAS","v8.16.0"],"number":188632,"url":"https://github.com/elastic/kibana/pull/188632","mergeCommit":{"message":"[OAS/HTTP]
Empty response bodies (status only) and descriptions for responses
(#188632)\n\n## Summary\r\n\r\n* Adds the ability to exclude a response
schema when declaring route\r\nschemas\r\n* Adds the ability to provide
a description of a the response\r\n\r\nSee code comments for more
info.\r\n\r\n## Example\r\n\r\nYou can declare a response with no
validation to imply an empty object\r\nin
OAS\r\n\r\n```\r\nrouter.versioned.post({ version: '2023-10-31', access:
'public', path: '...' })\r\n .addVersion({\r\n validation: {\r\n
responses: {\r\n 201: { description: 'Resource created!' }\r\n }\r\n
}\r\n }, () => {})\r\n```\r\n\r\nWill result in\r\n\r\n```jsonc\r\n{\r\n
//...\r\n 201: { description: 'Resource created!' }\r\n
//...\r\n}\r\n```\r\n\r\n## Risks\r\n\r\nNo notable
risks","sha":"a8091ab0acff6f8fae2cbe19c273048fb2734632"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188632","number":188632,"mergeCommit":{"message":"[OAS/HTTP]
Empty response bodies (status only) and descriptions for responses
(#188632)\n\n## Summary\r\n\r\n* Adds the ability to exclude a response
schema when declaring route\r\nschemas\r\n* Adds the ability to provide
a description of a the response\r\n\r\nSee code comments for more
info.\r\n\r\n## Example\r\n\r\nYou can declare a response with no
validation to imply an empty object\r\nin
OAS\r\n\r\n```\r\nrouter.versioned.post({ version: '2023-10-31', access:
'public', path: '...' })\r\n .addVersion({\r\n validation: {\r\n
responses: {\r\n 201: { description: 'Resource created!' }\r\n }\r\n
}\r\n }, () => {})\r\n```\r\n\r\nWill result in\r\n\r\n```jsonc\r\n{\r\n
//...\r\n 201: { description: 'Resource created!' }\r\n
//...\r\n}\r\n```\r\n\r\n## Risks\r\n\r\nNo notable
risks","sha":"a8091ab0acff6f8fae2cbe19c273048fb2734632"}}]}] BACKPORT-->

Co-authored-by: Jean-Louis Leysens <jeanlouis.leysens@elastic.co>
  • Loading branch information
lcawl and jloleysens authored Aug 15, 2024
1 parent 3a053eb commit 0b285e6
Show file tree
Hide file tree
Showing 22 changed files with 792 additions and 109 deletions.
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();
});
});
12 changes: 5 additions & 7 deletions packages/core/http/core-http-router-server-internal/src/util.ts
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
6 changes: 5 additions & 1 deletion packages/core/http/core-http-server/src/versioning/types.ts
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

0 comments on commit 0b285e6

Please sign in to comment.