Skip to content

Commit

Permalink
[8.14] [HTTP/OAS] Lazy response schemas (#181622) (#181949)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.14`:
- [[HTTP/OAS] Lazy response schemas
(#181622)](#181622)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-04-29T09:22:44Z","message":"[HTTP/OAS]
Lazy response schemas (#181622)\n\n## Summary\r\n\r\nBased on the
introduction of new response schemas for OAS generation we\r\nare going
to start the long tail of introducing missing response
(`joi`)\r\nschemas. We have roughly 520 known public APIs, most of which
do not\r\nhave response schemas defined. We expected a fairly large
increase in\r\n`@kbn/config-schema` definitions in the coming
weeks/months. Regardless\r\nof actual outcome and given how slow schema
instantiation is, this\r\npresents a slight concern for startup
time.\r\n\r\n## Proposed changes\r\n\r\nGive consumers guidance and a
way to pass in validation lazily. Under\r\nthe hood we make sure that
the lazy schemas only get called once.\r\n\r\n```ts\r\n\r\n/**\r\n * A
validation schema factory.\r\n *\r\n * @note Used to lazily create
schemas that are otherwise not needed\r\n * @note Assume this function
will only be called once\r\n *\r\n * @return A @kbn/config-schema
schema\r\n * @public\r\n */\r\nexport type LazyValidator = () =>
Type<unknown>;\r\n\r\n/** @public */\r\nexport interface
VersionedRouteCustomResponseBodyValidation {\r\n /** A custom validation
function */\r\n custom:
RouteValidationFunction<unknown>;\r\n}\r\n\r\n/** @public */\r\nexport
type VersionedResponseBodyValidation =\r\n | LazyValidator\r\n |
VersionedRouteCustomResponseBodyValidation;\r\n\r\n/**\r\n * Map of
response status codes to response schemas\r\n *\r\n * @note
Instantiating response schemas is expensive, especially when it is\r\n *
not needed in most cases. See example below to ensure this is lazily\r\n
* provided.\r\n *\r\n * @note The {@link TypeOf} type utility from
@kbn/config-schema can extract\r\n * types from lazily created
schemas\r\n *\r\n * @example\r\n * ```ts\r\n * // Avoid this:\r\n *
const badResponseSchema = schema.object({ foo: foo.string() });\r\n * //
Do this:\r\n * const goodResponseSchema = () => schema.object({ foo:
foo.string() });\r\n *\r\n * type ResponseType = TypeOf<typeof
goodResponseSchema>;\r\n * ...\r\n * .addVersion(\r\n * { ...
validation: { response: { 200: { body: goodResponseSchema } } } },\r\n *
handlerFn\r\n * )\r\n * ...\r\n * ```\r\n * @public\r\n */\r\nexport
interface VersionedRouteResponseValidation {\r\n [statusCode: number]:
{\r\n body: VersionedResponseBodyValidation;\r\n };\r\n unsafe?: {
body?: boolean };\r\n}\r\n```\r\n\r\n## Notes\r\n\r\n* Expected (worst
case) in low resource environments is an additional\r\n1.5 seconds to
start up time and additional ~70MB to memory pressure\r\nwhich is not a
great trade-off for functionality that is only used when\r\nOAS
generation is on.\r\n\r\nRelated
https://github.com/elastic/kibana/pull/181277","sha":"97e1d9f4b8b816d45b31a9caa0feca9bbb9c3bd8","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:http","Team:Core","release_note:skip","ci:project-deploy-observability","v8.14.0","v8.15.0"],"title":"[HTTP/OAS]
Lazy response
schemas","number":181622,"url":"https://github.com/elastic/kibana/pull/181622","mergeCommit":{"message":"[HTTP/OAS]
Lazy response schemas (#181622)\n\n## Summary\r\n\r\nBased on the
introduction of new response schemas for OAS generation we\r\nare going
to start the long tail of introducing missing response
(`joi`)\r\nschemas. We have roughly 520 known public APIs, most of which
do not\r\nhave response schemas defined. We expected a fairly large
increase in\r\n`@kbn/config-schema` definitions in the coming
weeks/months. Regardless\r\nof actual outcome and given how slow schema
instantiation is, this\r\npresents a slight concern for startup
time.\r\n\r\n## Proposed changes\r\n\r\nGive consumers guidance and a
way to pass in validation lazily. Under\r\nthe hood we make sure that
the lazy schemas only get called once.\r\n\r\n```ts\r\n\r\n/**\r\n * A
validation schema factory.\r\n *\r\n * @note Used to lazily create
schemas that are otherwise not needed\r\n * @note Assume this function
will only be called once\r\n *\r\n * @return A @kbn/config-schema
schema\r\n * @public\r\n */\r\nexport type LazyValidator = () =>
Type<unknown>;\r\n\r\n/** @public */\r\nexport interface
VersionedRouteCustomResponseBodyValidation {\r\n /** A custom validation
function */\r\n custom:
RouteValidationFunction<unknown>;\r\n}\r\n\r\n/** @public */\r\nexport
type VersionedResponseBodyValidation =\r\n | LazyValidator\r\n |
VersionedRouteCustomResponseBodyValidation;\r\n\r\n/**\r\n * Map of
response status codes to response schemas\r\n *\r\n * @note
Instantiating response schemas is expensive, especially when it is\r\n *
not needed in most cases. See example below to ensure this is lazily\r\n
* provided.\r\n *\r\n * @note The {@link TypeOf} type utility from
@kbn/config-schema can extract\r\n * types from lazily created
schemas\r\n *\r\n * @example\r\n * ```ts\r\n * // Avoid this:\r\n *
const badResponseSchema = schema.object({ foo: foo.string() });\r\n * //
Do this:\r\n * const goodResponseSchema = () => schema.object({ foo:
foo.string() });\r\n *\r\n * type ResponseType = TypeOf<typeof
goodResponseSchema>;\r\n * ...\r\n * .addVersion(\r\n * { ...
validation: { response: { 200: { body: goodResponseSchema } } } },\r\n *
handlerFn\r\n * )\r\n * ...\r\n * ```\r\n * @public\r\n */\r\nexport
interface VersionedRouteResponseValidation {\r\n [statusCode: number]:
{\r\n body: VersionedResponseBodyValidation;\r\n };\r\n unsafe?: {
body?: boolean };\r\n}\r\n```\r\n\r\n## Notes\r\n\r\n* Expected (worst
case) in low resource environments is an additional\r\n1.5 seconds to
start up time and additional ~70MB to memory pressure\r\nwhich is not a
great trade-off for functionality that is only used when\r\nOAS
generation is on.\r\n\r\nRelated
https://github.com/elastic/kibana/pull/181277","sha":"97e1d9f4b8b816d45b31a9caa0feca9bbb9c3bd8"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181622","number":181622,"mergeCommit":{"message":"[HTTP/OAS]
Lazy response schemas (#181622)\n\n## Summary\r\n\r\nBased on the
introduction of new response schemas for OAS generation we\r\nare going
to start the long tail of introducing missing response
(`joi`)\r\nschemas. We have roughly 520 known public APIs, most of which
do not\r\nhave response schemas defined. We expected a fairly large
increase in\r\n`@kbn/config-schema` definitions in the coming
weeks/months. Regardless\r\nof actual outcome and given how slow schema
instantiation is, this\r\npresents a slight concern for startup
time.\r\n\r\n## Proposed changes\r\n\r\nGive consumers guidance and a
way to pass in validation lazily. Under\r\nthe hood we make sure that
the lazy schemas only get called once.\r\n\r\n```ts\r\n\r\n/**\r\n * A
validation schema factory.\r\n *\r\n * @note Used to lazily create
schemas that are otherwise not needed\r\n * @note Assume this function
will only be called once\r\n *\r\n * @return A @kbn/config-schema
schema\r\n * @public\r\n */\r\nexport type LazyValidator = () =>
Type<unknown>;\r\n\r\n/** @public */\r\nexport interface
VersionedRouteCustomResponseBodyValidation {\r\n /** A custom validation
function */\r\n custom:
RouteValidationFunction<unknown>;\r\n}\r\n\r\n/** @public */\r\nexport
type VersionedResponseBodyValidation =\r\n | LazyValidator\r\n |
VersionedRouteCustomResponseBodyValidation;\r\n\r\n/**\r\n * Map of
response status codes to response schemas\r\n *\r\n * @note
Instantiating response schemas is expensive, especially when it is\r\n *
not needed in most cases. See example below to ensure this is lazily\r\n
* provided.\r\n *\r\n * @note The {@link TypeOf} type utility from
@kbn/config-schema can extract\r\n * types from lazily created
schemas\r\n *\r\n * @example\r\n * ```ts\r\n * // Avoid this:\r\n *
const badResponseSchema = schema.object({ foo: foo.string() });\r\n * //
Do this:\r\n * const goodResponseSchema = () => schema.object({ foo:
foo.string() });\r\n *\r\n * type ResponseType = TypeOf<typeof
goodResponseSchema>;\r\n * ...\r\n * .addVersion(\r\n * { ...
validation: { response: { 200: { body: goodResponseSchema } } } },\r\n *
handlerFn\r\n * )\r\n * ...\r\n * ```\r\n * @public\r\n */\r\nexport
interface VersionedRouteResponseValidation {\r\n [statusCode: number]:
{\r\n body: VersionedResponseBodyValidation;\r\n };\r\n unsafe?: {
body?: boolean };\r\n}\r\n```\r\n\r\n## Notes\r\n\r\n* Expected (worst
case) in low resource environments is an additional\r\n1.5 seconds to
start up time and additional ~70MB to memory pressure\r\nwhich is not a
great trade-off for functionality that is only used when\r\nOAS
generation is on.\r\n\r\nRelated
https://github.com/elastic/kibana/pull/181277","sha":"97e1d9f4b8b816d45b31a9caa0feca9bbb9c3bd8"}}]}]
BACKPORT-->

Co-authored-by: Jean-Louis Leysens <[email protected]>
  • Loading branch information
kibanamachine and jloleysens authored Apr 29, 2024
1 parent efe1198 commit 7701e73
Show file tree
Hide file tree
Showing 56 changed files with 713 additions and 248 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export class CoreAppsService {
body: schema.recordOf(schema.string(), schema.any()),
},
response: {
'200': { body: schema.object({ ok: schema.boolean() }) },
'200': { body: () => schema.object({ ok: schema.boolean() }) },
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export {
versionHandlerResolvers,
CoreVersionedRouter,
ALLOWED_PUBLIC_VERSION,
unwrapVersionedResponseBodyValidation,
type VersionedRouterRoute,
type HandlerResolutionStrategy,
} from './src/versioned_router';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@

import { Router, type RouterOptions } from './router';
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
import { schema } from '@kbn/config-schema';
import { isConfigSchema, schema } from '@kbn/config-schema';
import { createFooValidation } from './router.test.util';
import { createRequestMock } from '@kbn/hapi-mocks/src/request';
import type { RouteValidatorRequestAndResponses } from '@kbn/core-http-server';

const mockResponse: any = {
code: jest.fn().mockImplementation(() => mockResponse),
Expand All @@ -32,6 +33,10 @@ const routerOptions: RouterOptions = {
};

describe('Router', () => {
let testValidation: ReturnType<typeof createFooValidation>;
beforeEach(() => {
testValidation = createFooValidation();
});
afterEach(() => jest.clearAllMocks());
describe('#getRoutes', () => {
it('returns expected route metadata', () => {
Expand Down Expand Up @@ -81,12 +86,10 @@ describe('Router', () => {
});
});

const { fooValidation, validateBodyFn, validateOutputFn, validateParamsFn, validateQueryFn } =
createFooValidation();

it.each([['static' as const], ['lazy' as const]])(
'runs %s route validations',
async (staticOrLazy) => {
const { fooValidation } = testValidation;
const router = new Router('', logger, enhanceWithContext, routerOptions);
router.post(
{
Expand All @@ -104,6 +107,8 @@ describe('Router', () => {
}),
mockResponseToolkit
);
const { validateBodyFn, validateParamsFn, validateQueryFn, validateOutputFn } =
testValidation;
expect(validateBodyFn).toHaveBeenCalledTimes(1);
expect(validateParamsFn).toHaveBeenCalledTimes(1);
expect(validateQueryFn).toHaveBeenCalledTimes(1);
Expand All @@ -113,6 +118,16 @@ describe('Router', () => {

it('constructs lazily provided validations once (idempotency)', async () => {
const router = new Router('', logger, enhanceWithContext, routerOptions);
const { fooValidation } = testValidation;

const response200 = fooValidation.response[200].body;
const lazyResponse200 = jest.fn(() => response200());
fooValidation.response[200].body = lazyResponse200;

const response404 = fooValidation.response[404].body;
const lazyResponse404 = jest.fn(() => response404());
fooValidation.response[404].body = lazyResponse404;

const lazyValidation = jest.fn(() => fooValidation);
router.post(
{
Expand All @@ -121,7 +136,7 @@ describe('Router', () => {
},
(context, req, res) => res.ok()
);
const [{ handler }] = router.getRoutes();
const [{ handler, validationSchemas }] = router.getRoutes();
for (let i = 0; i < 10; i++) {
await handler(
createRequestMock({
Expand All @@ -131,8 +146,25 @@ describe('Router', () => {
}),
mockResponseToolkit
);

expect(
isConfigSchema(
(
validationSchemas as () => RouteValidatorRequestAndResponses<unknown, unknown, unknown>
)().response![200].body()
)
).toBe(true);
expect(
isConfigSchema(
(
validationSchemas as () => RouteValidatorRequestAndResponses<unknown, unknown, unknown>
)().response![404].body()
)
).toBe(true);
}
expect(lazyValidation).toHaveBeenCalledTimes(1);
expect(lazyResponse200).toHaveBeenCalledTimes(1);
expect(lazyResponse404).toHaveBeenCalledTimes(1);
});

it('registers pluginId if provided', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,18 @@ export function createFooValidation() {
},
response: {
200: {
body: schema.object({
foo: schema.number({
validate: validateOutputFn,
body: () =>
schema.object({
foo: schema.number({
validate: validateOutputFn,
}),
}),
},
404: {
body: () =>
schema.object({
error: schema.string(),
}),
}),
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import type { Request, ResponseToolkit } from '@hapi/hapi';
import { once } from 'lodash';
import apm from 'elastic-apm-node';
import { isConfigSchema } from '@kbn/config-schema';
import type { Logger } from '@kbn/logging';
Expand Down Expand Up @@ -35,6 +34,7 @@ import { kibanaResponseFactory } from './response';
import { HapiResponseAdapter } from './response_adapter';
import { wrapErrors } from './error_wrapper';
import { Method } from './versioned_router/types';
import { prepareRouteConfigValidation } from './util';

export type ContextEnhancer<
P,
Expand Down Expand Up @@ -187,9 +187,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
handler: RequestHandler<P, Q, B, Context, Method>,
internalOptions: { isVersioned: boolean } = { isVersioned: false }
) => {
if (typeof route.validate === 'function') {
route = { ...route, validate: once(route.validate) };
}
route = prepareRouteConfigValidation(route);
const routeSchemas = routeSchemasFromRouteConfig(route, method);

this.routes.push({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { schema } from '@kbn/config-schema';
import { RouteValidator } from '@kbn/core-http-server';
import { prepareResponseValidation } from './util';

describe('prepareResponseValidation', () => {
it('wraps only expected values in "once"', () => {
const validation: RouteValidator<unknown, unknown, unknown> = {
request: {},
response: {
200: {
body: jest.fn(() => schema.string()),
},
404: {
body: jest.fn(() => schema.string()),
},
unsafe: {
body: true,
},
},
};

const prepared = prepareResponseValidation(validation.response!);

expect(prepared).toEqual({
200: { body: expect.any(Function) },
404: { body: expect.any(Function) },
unsafe: { body: true },
});

[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);
});
});
66 changes: 66 additions & 0 deletions packages/core/http/core-http-router-server-internal/src/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { once } from 'lodash';
import {
isFullValidatorContainer,
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 {
const responses = Object.entries(validation).map(([key, value]) => {
if (isStatusCode(key)) {
return [key, { body: once(value.body) }];
}
return [key, value];
});

return Object.fromEntries(responses);
}

function prepareValidation<P, Q, B>(validator: RouteValidator<P, Q, B>) {
if (isFullValidatorContainer(validator) && validator.response) {
return {
...validator,
response: prepareResponseValidation(validator.response),
};
}
return validator;
}

// Integration tested in ./routes.test.ts
export function prepareRouteConfigValidation<P, Q, B>(
config: RouteConfig<P, Q, B, RouteMethod>
): RouteConfig<P, Q, B, RouteMethod> {
// Calculating schema validation can be expensive so when it is provided lazily
// we only want to instantiate it once. This also provides idempotency guarantees
if (typeof config.validate === 'function') {
const validate = config.validate;
return {
...config,
validate: once(() => prepareValidation(validate())),
};
} else if (typeof config.validate === 'object' && typeof config.validate !== null) {
return {
...config,
validate: prepareValidation(config.validate),
};
}
return config;
}
Loading

0 comments on commit 7701e73

Please sign in to comment.