[Versioned HTTP] Add response runtime and type-level validation #153011

jloleysens · 2023-03-09T13:25:25Z

Summary

This PR restructures the validation object on the .addVersion method by:

Adding an in object for the body, params and query validations
Adding out so that we can have both runtime and TS type checking our responses

To reviewers: easiest way to interpret these changes is to read the example.ts file.

…nd TS type checking

jloleysens · 2023-03-09T13:43:09Z

packages/core/http/core-http-server/src/router/response_factory.ts

@@ -27,14 +27,18 @@ export interface KibanaSuccessResponseFactory {
   * Status code: `200`.
   * @param options - {@link HttpResponseOptions} configures HTTP response body & headers.
   */
-  ok(options?: HttpResponseOptions): IKibanaResponse;
+  ok<T extends HttpResponsePayload = HttpResponsePayload>(


Needed for properly type checking responses. Assuming we only want this for ok. But I can easily see this also being checkable for non-OK responses like 400.

jloleysens · 2023-03-09T13:43:19Z

packages/core/versioning/core-version-http-server/src/version_http_toolkit.ts

+interface FullValidationConfig<P, Q, B, R> {
+  /** Validation to run against route inputs: params, query and body */
+  in?: InValidation<P, Q, B>;
+  /** Validation to run against route output */
+  out?: Type<R>;
+}


Adding something like this gives us a few new capabilities:

TS checking enforcing shape of return type

Runtime checks against the shape of return type

A clearer path to true HTTP API introspection since we have the types for the return value of the endpoint...

…fix'

TinaHeiligers

I like where we're heading here. I left a few nits as naming conventions.

packages/core/versioning/core-version-http-server/src/version_http_toolkit.ts

src/plugins/data_views/server/rest_api_routes/runtime_fields/response_formatter.ts

x-pack/test/plugin_api_perf/plugins/task_manager_performance/server/init_routes.ts

…fix'

packages/core/versioning/core-version-http-server/src/version_http_toolkit.ts

elasticmachine · 2023-03-10T12:48:59Z

Pinging @elastic/kibana-core (Team:Core)

ymao1

Response Ops changes LGTM

TinaHeiligers

LGTM

TinaHeiligers · 2023-03-10T15:30:29Z

packages/core/versioning/core-version-http-server/src/version_http_toolkit.ts

-    opts: AddVersionOpts<P, Q, B>,
-    handler: RequestHandler<P, Q, B, Ctx>
+  addVersion<P, Q, B, R>(
+    options: AddVersionOpts<P, Q, B, R>,


TinaHeiligers · 2023-03-10T15:33:04Z

packages/kbn-utility-types/index.ts

+ * interface Foo { bar?: string }
+ * const foo: WithRequiredProperty<Foo, 'bar'> = { bar: 'baz' }
+ */
+export type WithRequiredProperty<Type, Key extends keyof Type> = Omit<Type, Key> & {


We'll need to add a test if the type starts getting used a lot.

kibana-ci · 2023-03-10T19:32:14Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 002519f

Failed CI Steps

Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution

Test Failures

[job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution / Rules selection should correctly update the selection label when rules are bulk selected and then unselected via the table select all checkbox

Metrics [docs]

Unknown metric groups

API count

id	before	after	diff
`@kbn/utility-types`	35	36	+1

ESLint disabled line counts

id	before	after	diff
`securitySolution`	434	437	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	514	517	+3

History

💚 Build #113279 succeeded 7f5165a
💚 Build #113043 succeeded 116ac30
💔 Build #113024 failed d782bce
💔 Build #113021 failed a677581
💔 Build #113013 failed b1d4084

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @rudolf

davismcphee

response_formatter.ts explicit return type change LGTM!

updated experimental versioned route types to include "out" runtime a…

08353d5

…nd TS type checking

jloleysens assigned rudolf Mar 9, 2023

make "in" and "out" optional rather than | false

b1d4084

jloleysens commented Mar 9, 2023

View reviewed changes

jloleysens and others added 5 commits March 9, 2023 14:45

do not use "undefined" type

397cfe5

sigh, should probably allow route validation function

a677581

fix ts

d782bce

some more TS fixes to narrow down types for inferences

7cc5347

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

116ac30

…fix'

jloleysens changed the title ~~[Versioned routes] Add response runtime and type-level validation~~ [Versioned HTTP] Add response runtime and type-level validation Mar 9, 2023

TinaHeiligers reviewed Mar 9, 2023

View reviewed changes

jloleysens and others added 3 commits March 10, 2023 12:52

address nits

4c625c3

minor refactor: move type utility to shared package

4bdfb87

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

705b7e8

…fix'

rudolf approved these changes Mar 10, 2023

View reviewed changes

packages/core/versioning/core-version-http-server/src/version_http_toolkit.ts Outdated Show resolved Hide resolved

jloleysens and others added 2 commits March 10, 2023 13:42

refactor in=>request and out=>response

23faaff

Merge branch 'main' into add-response-validation-to-versioned-router

7f5165a

jloleysens marked this pull request as ready for review March 10, 2023 12:44

jloleysens requested review from a team as code owners March 10, 2023 12:44

jloleysens added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.8.0 labels Mar 10, 2023

Merge branch 'main' into add-response-validation-to-versioned-router

d9ff379

jloleysens enabled auto-merge (squash) March 10, 2023 15:13

jloleysens added 3 commits March 10, 2023 16:26

added doc comment

7a60b06

input=>request output=>response

1bb6cee

added @experimental

002519f

ymao1 approved these changes Mar 10, 2023

View reviewed changes

TinaHeiligers approved these changes Mar 10, 2023

View reviewed changes

davismcphee approved these changes Mar 13, 2023

View reviewed changes

jloleysens merged commit 3c7bf58 into elastic:main Mar 13, 2023

kibanamachine added the backport:skip This commit does not require backporting label Mar 13, 2023

jloleysens deleted the add-response-validation-to-versioned-router branch March 23, 2023 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Versioned HTTP] Add response runtime and type-level validation #153011

[Versioned HTTP] Add response runtime and type-level validation #153011

jloleysens commented Mar 9, 2023 •

edited

Loading

jloleysens Mar 9, 2023

jloleysens Mar 9, 2023

TinaHeiligers left a comment

elasticmachine commented Mar 10, 2023

ymao1 left a comment

TinaHeiligers left a comment

TinaHeiligers Mar 10, 2023

TinaHeiligers Mar 10, 2023

kibana-ci commented Mar 10, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

davismcphee left a comment

[Versioned HTTP] Add response runtime and type-level validation #153011

[Versioned HTTP] Add response runtime and type-level validation #153011

Conversation

jloleysens commented Mar 9, 2023 • edited Loading

Summary

jloleysens Mar 9, 2023

Choose a reason for hiding this comment

jloleysens Mar 9, 2023

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 10, 2023

ymao1 left a comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

TinaHeiligers Mar 10, 2023

Choose a reason for hiding this comment

TinaHeiligers Mar 10, 2023

Choose a reason for hiding this comment

kibana-ci commented Mar 10, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

API count

ESLint disabled line counts

Total ESLint disabled count

History

davismcphee left a comment

Choose a reason for hiding this comment

jloleysens commented Mar 9, 2023 •

edited

Loading