Use HTTP request schemas to create types, use those types in the client (#59340)

* wip

* wip

* wip

* will this work?

* wip but it works

* pedro

* remove thing

* remove TODOs

* fix type issue

* add tests to check that alert index api works

* Revert "add tests to check that alert index api works"

This reverts commit 5d40ca18337cf8deb63a0291150780ec094db016.

* Moved schema

* undoing my evils

* fix comments. fix incorrect import

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

src/core/public/http/types.ts

@@ -205,7 +205,12 @@ export interface HttpRequestInit {
 
 /** @public */
 export interface HttpFetchQuery {
-  [key: string]: string | number | boolean | undefined;
+  [key: string]:
+    | string
+    | number
+    | boolean
+    | undefined
+    | Array<string | number | boolean | undefined>;
 }
 
 /**

src/core/public/public.api.md

@@ -610,7 +610,7 @@ export interface HttpFetchOptionsWithPath extends HttpFetchOptions {
 // @public (undocumented)
 export interface HttpFetchQuery {
     // (undocumented)
-    [key: string]: string | number | boolean | undefined;
+    [key: string]: string | number | boolean | undefined | Array<string | number | boolean | undefined>;
 }
 
 // @public

x-pack/plugins/endpoint/common/schema/README.md

@@ -0,0 +1,6 @@
+# Schemas
+
+These schemas are used to validate, coerce, and provide types for the comms between the client, server, and ES.
+
+# Future work
+In the future, we may be able to locate these under 'server'. 

x-pack/plugins/endpoint/common/schema/alert_index.ts

@@ -3,13 +3,17 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { decode } from 'rison-node';
-import { i18n } from '@kbn/i18n';
-import { schema } from '@kbn/config-schema';
-import { esKuery } from '../../../../../../../src/plugins/data/server';
-import { EndpointAppConstants } from '../../../../common/types';
 
-export const alertListReqSchema = schema.object(
+import { schema, Type } from '@kbn/config-schema';
+import { i18n } from '@kbn/i18n';
+import { decode } from 'rison-node';
+import { fromKueryExpression } from '../../../../../src/plugins/data/common';
+import { EndpointAppConstants } from '../types';
+
+/**
+ * Used to validate GET requests against the index of the alerting APIs.
+ */
+export const alertingIndexGetQuerySchema = schema.object(
   {
     page_size: schema.maybe(
       schema.number({
@@ -26,31 +30,21 @@ export const alertListReqSchema = schema.object(
       schema.arrayOf(schema.string(), {
         minSize: 2,
         maxSize: 2,
-      })
+      }) as Type<[string, string]> // Cast this to a string tuple. `@kbn/config-schema` doesn't do this automatically
     ),
     before: schema.maybe(
       schema.arrayOf(schema.string(), {
         minSize: 2,
         maxSize: 2,
-      })
+      }) as Type<[string, string]> // Cast this to a string tuple. `@kbn/config-schema` doesn't do this automatically
     ),
     sort: schema.maybe(schema.string()),
-    order: schema.maybe(
-      schema.string({
-        validate(value) {
-          if (value !== 'asc' && value !== 'desc') {
-            return i18n.translate('xpack.endpoint.alerts.errors.bad_sort_direction', {
-              defaultMessage: 'must be `asc` or `desc`',
-            });
-          }
-        },
-      })
-    ),
+    order: schema.maybe(schema.oneOf([schema.literal('asc'), schema.literal('desc')])),
     query: schema.maybe(
       schema.string({
         validate(value) {
           try {
-            esKuery.fromKueryExpression(value);
+            fromKueryExpression(value);
           } catch (err) {
             return i18n.translate('xpack.endpoint.alerts.errors.bad_kql', {
               defaultMessage: 'must be valid KQL',

x-pack/plugins/endpoint/common/types.ts

@@ -5,6 +5,9 @@
  */
 
 import { SearchResponse } from 'elasticsearch';
+import { TypeOf } from '@kbn/config-schema';
+import * as kbnConfigSchemaTypes from '@kbn/config-schema/target/types/types';
+import { alertingIndexGetQuerySchema } from './schema/alert_index';
 
 /**
  * A deep readonly type that will make all children of a given object readonly recursively
@@ -24,10 +27,7 @@ export type ImmutableMap<K, V> = ReadonlyMap<Immutable<K>, Immutable<V>>;
 export type ImmutableSet<T> = ReadonlySet<Immutable<T>>;
 export type ImmutableObject<T> = { readonly [K in keyof T]: Immutable<T[K]> };
 
-export enum Direction {
-  asc = 'asc',
-  desc = 'desc',
-}
+export type Direction = 'asc' | 'desc';
 
 export class EndpointAppConstants {
   static BASE_API_URL = '/api/endpoint';
@@ -45,7 +45,6 @@ export class EndpointAppConstants {
    **/
   static ALERT_LIST_DEFAULT_PAGE_SIZE = 10;
   static ALERT_LIST_DEFAULT_SORT = '@timestamp';
-  static ALERT_LIST_DEFAULT_ORDER = Direction.desc;
 }
 
 export interface AlertResultList {
@@ -336,3 +335,72 @@ export type ResolverEvent = EndpointEvent | LegacyEndpointEvent;
  * The PageId type is used for the payload when firing userNavigatedToPage actions
  */
 export type PageId = 'alertsPage' | 'managementPage' | 'policyListPage';
+
+/**
+ * Takes a @kbn/config-schema 'schema' type and returns a type that represents valid inputs.
+ * Similar to `TypeOf`, but allows strings as input for `schema.number()` (which is inline
+ * with the behavior of the validator.) Also, for `schema.object`, when a value is a `schema.maybe`
+ * the key will be marked optional (via `?`) so that you can omit keys for optional values.
+ *
+ * Use this when creating a value that will be passed to the schema.
+ * e.g.
+ * ```ts
+ * const input: KbnConfigSchemaInputTypeOf<typeof schema> = value
+ * schema.validate(input) // should be valid
+ * ```
+ */
+type KbnConfigSchemaInputTypeOf<
+  T extends kbnConfigSchemaTypes.Type<unknown>
+> = T extends kbnConfigSchemaTypes.ObjectType
+  ? KbnConfigSchemaInputObjectTypeOf<
+      T
+    > /** `schema.number()` accepts strings, so this type should accept them as well. */
+  : kbnConfigSchemaTypes.Type<number> extends T
+  ? TypeOf<T> | string
+  : TypeOf<T>;
+
+/**
+ * Works like ObjectResultType, except that 'maybe' schema will create an optional key.
+ * This allows us to avoid passing 'maybeKey: undefined' when constructing such an object.
+ *
+ * Instead of using this directly, use `InputTypeOf`.
+ */
+type KbnConfigSchemaInputObjectTypeOf<
+  T extends kbnConfigSchemaTypes.ObjectType
+> = T extends kbnConfigSchemaTypes.ObjectType<infer P>
+  ? {
+      /** Use ? to make the field optional if the prop accepts undefined.
+       * This allows us to avoid writing `field: undefined` for optional fields.
+       */
+      [K in Exclude<
+        keyof P,
+        keyof KbnConfigSchemaNonOptionalProps<P>
+      >]?: KbnConfigSchemaInputTypeOf<P[K]>;
+    } &
+      { [K in keyof KbnConfigSchemaNonOptionalProps<P>]: KbnConfigSchemaInputTypeOf<P[K]> }
+  : never;
+
+/**
+ * Takes the props of a schema.object type, and returns a version that excludes
+ * optional values. Used by `InputObjectTypeOf`.
+ *
+ * Instead of using this directly, use `InputTypeOf`.
+ */
+type KbnConfigSchemaNonOptionalProps<Props extends kbnConfigSchemaTypes.Props> = Pick<
+  Props,
+  {
+    [Key in keyof Props]: undefined extends TypeOf<Props[Key]> ? never : Key;
+  }[keyof Props]
+>;
+
+/**
+ * Query params to pass to the alert API when fetching new data.
+ */
+export type AlertingIndexGetQueryInput = KbnConfigSchemaInputTypeOf<
+  typeof alertingIndexGetQuerySchema
+>;
+
+/**
+ * Result of the validated query params when handling alert index requests.
+ */
+export type AlertingIndexGetQueryResult = TypeOf<typeof alertingIndexGetQuerySchema>;

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts

@@ -8,6 +8,7 @@ import { AlertResultList, AlertData } from '../../../../../common/types';
 import { AppAction } from '../action';
 import { MiddlewareFactory, AlertListState } from '../../types';
 import { isOnAlertPage, apiQueryParams, hasSelectedAlert, uiQueryParams } from './selectors';
+import { cloneHttpFetchQuery } from '../../../../common/clone_http_fetch_query';
 
 export const alertMiddlewareFactory: MiddlewareFactory<AlertListState> = coreStart => {
   return api => next => async (action: AppAction) => {
@@ -15,7 +16,7 @@ export const alertMiddlewareFactory: MiddlewareFactory<AlertListState> = coreSta
     const state = api.getState();
     if (action.type === 'userChangedUrl' && isOnAlertPage(state)) {
       const response: AlertResultList = await coreStart.http.get(`/api/endpoint/alerts`, {
-        query: apiQueryParams(state),
+        query: cloneHttpFetchQuery(apiQueryParams(state)),
       });
       api.dispatch({ type: 'serverReturnedAlertsData', payload: response });
     }

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts

@@ -9,15 +9,11 @@ import {
   createSelector,
   createStructuredSelector as createStructuredSelectorWithBadType,
 } from 'reselect';
-import {
-  AlertListState,
-  AlertingIndexUIQueryParams,
-  AlertsAPIQueryParams,
-  CreateStructuredSelector,
-} from '../../types';
-import { Immutable } from '../../../../../common/types';
+import { AlertListState, AlertingIndexUIQueryParams, CreateStructuredSelector } from '../../types';
+import { Immutable, AlertingIndexGetQueryInput } from '../../../../../common/types';
 
 const createStructuredSelector: CreateStructuredSelector = createStructuredSelectorWithBadType;
+
 /**
  * Returns the Alert Data array from state
  */
@@ -82,7 +78,7 @@ export const uiQueryParams: (
  */
 export const apiQueryParams: (
   state: AlertListState
-) => Immutable<AlertsAPIQueryParams> = createSelector(
+) => Immutable<AlertingIndexGetQueryInput> = createSelector(
   uiQueryParams,
   ({ page_size, page_index }) => ({
     page_size,
@@ -90,6 +86,10 @@ export const apiQueryParams: (
   })
 );
 
+/**
+ * True if the user has selected an alert to see details about.
+ * Populated via the browsers query params.
+ */
 export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector(
   uiQueryParams,
   ({ selected_alert: selectedAlert }) => selectedAlert !== undefined

x-pack/plugins/endpoint/public/applications/endpoint/types.ts

@@ -5,7 +5,6 @@
  */
 
 import { Dispatch, MiddlewareAPI } from 'redux';
-import { CoreStart } from 'kibana/public';
 import {
   EndpointMetadata,
   AlertData,
@@ -14,6 +13,7 @@ import {
   ImmutableArray,
 } from '../../../common/types';
 import { AppAction } from './store/action';
+import { CoreStart } from '../../../../../../src/core/public';
 
 export { AppAction };
 export type MiddlewareFactory<S = GlobalState> = (
@@ -140,17 +140,3 @@ export interface AlertingIndexUIQueryParams {
    */
   selected_alert?: string;
 }
-
-/**
- * Query params to pass to the alert API when fetching new data.
- */
-export interface AlertsAPIQueryParams {
-  /**
-   * Number of results to return.
-   */
-  page_size?: string;
-  /**
-   * 0-based index of 'page' to return.
-   */
-  page_index?: string;
-}

x-pack/plugins/endpoint/public/common/clone_http_fetch_query.test.ts

@@ -0,0 +1,32 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { cloneHttpFetchQuery } from './clone_http_fetch_query';
+import { Immutable } from '../../common/types';
+import { HttpFetchQuery } from '../../../../../src/core/public';
+
+describe('cloneHttpFetchQuery', () => {
+  it('can clone complex queries', () => {
+    const query: Immutable<HttpFetchQuery> = {
+      a: 'a',
+      '1': 1,
+      undefined,
+      array: [1, 2, undefined],
+    };
+    expect(cloneHttpFetchQuery(query)).toMatchInlineSnapshot(`
+      Object {
+        "1": 1,
+        "a": "a",
+        "array": Array [
+          1,
+          2,
+          undefined,
+        ],
+        "undefined": undefined,
+      }
+    `);
+  });
+});

x-pack/plugins/endpoint/public/common/clone_http_fetch_query.ts

@@ -0,0 +1,22 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { Immutable } from '../../common/types';
+
+import { HttpFetchQuery } from '../../../../../src/core/public';
+
+export function cloneHttpFetchQuery(query: Immutable<HttpFetchQuery>): HttpFetchQuery {
+  const clone: HttpFetchQuery = {};
+  for (const [key, value] of Object.entries(query)) {
+    if (Array.isArray(value)) {
+      clone[key] = [...value];
+    } else {
+      // Array.isArray is not removing ImmutableArray from the union.
+      clone[key] = value as string | number | boolean;
+    }
+  }
+  return clone;
+}

x-pack/plugins/endpoint/server/routes/alerts/alerts.test.ts

@@ -9,9 +9,9 @@ import {
   httpServiceMock,
   loggingServiceMock,
 } from '../../../../../../src/core/server/mocks';
-import { alertListReqSchema } from './list/schemas';
 import { registerAlertRoutes } from './index';
 import { EndpointConfigSchema } from '../../config';
+import { alertingIndexGetQuerySchema } from '../../../common/schema/alert_index';
 
 describe('test alerts route', () => {
   let routerMock: jest.Mocked<IRouter>;
@@ -31,7 +31,7 @@ describe('test alerts route', () => {
 
   it('should fail to validate when `page_size` is not a number', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         page_size: 'abc',
       });
     };
@@ -40,7 +40,7 @@ describe('test alerts route', () => {
 
   it('should validate when `page_size` is a number', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         page_size: 25,
       });
     };
@@ -49,7 +49,7 @@ describe('test alerts route', () => {
 
   it('should validate when `page_size` can be converted to a number', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         page_size: '50',
       });
     };
@@ -58,7 +58,7 @@ describe('test alerts route', () => {
 
   it('should allow either `page_index` or `after`, but not both', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         page_index: 1,
         after: [123, 345],
       });
@@ -68,7 +68,7 @@ describe('test alerts route', () => {
 
   it('should allow either `page_index` or `before`, but not both', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         page_index: 1,
         before: 'abc',
       });
@@ -78,7 +78,7 @@ describe('test alerts route', () => {
 
   it('should allow either `before` or `after`, but not both', async () => {
     const validate = () => {
-      alertListReqSchema.validate({
+      alertingIndexGetQuerySchema.validate({
         before: ['abc', 'def'],
         after: [123, 345],
       });

x-pack/plugins/endpoint/server/routes/alerts/details/lib/pagination.ts

@@ -5,7 +5,6 @@
  */
 
 import { GetResponse, SearchResponse } from 'elasticsearch';
-import { RequestHandlerContext } from 'src/core/server';
 import {
   AlertEvent,
   AlertHits,
@@ -16,6 +15,7 @@ import { EndpointConfigType } from '../../../../config';
 import { searchESForAlerts, Pagination } from '../../lib';
 import { AlertSearchQuery, SearchCursor, AlertDetailsRequestParams } from '../../types';
 import { BASE_ALERTS_ROUTE } from '../..';
+import { RequestHandlerContext } from '../../../../../../../../src/core/server';
 
 /**
  * Pagination class for alert details.
@@ -40,10 +40,10 @@ export class AlertDetailsPagination extends Pagination<
     const reqData: AlertSearchQuery = {
       pageSize: 1,
       sort: EndpointAppConstants.ALERT_LIST_DEFAULT_SORT,
-      order: EndpointAppConstants.ALERT_LIST_DEFAULT_ORDER,
+      order: 'desc',
     };
 
-    if (direction === Direction.asc) {
+    if (direction === 'asc') {
       reqData.searchAfter = cursor;
     } else {
       reqData.searchBefore = cursor;
@@ -67,7 +67,7 @@ export class AlertDetailsPagination extends Pagination<
    * Gets the next alert after this one.
    */
   async getNextUrl(): Promise<string | null> {
-    const response = await this.doSearch(Direction.asc, [
+    const response = await this.doSearch('asc', [
       this.data._source['@timestamp'].toString(),
       this.data._source.event.id,
     ]);
@@ -78,7 +78,7 @@ export class AlertDetailsPagination extends Pagination<
    * Gets the alert before this one.
    */
   async getPrevUrl(): Promise<string | null> {
-    const response = await this.doSearch(Direction.desc, [
+    const response = await this.doSearch('desc', [
       this.data._source['@timestamp'].toString(),
       this.data._source.event.id,
     ]);

x-pack/plugins/endpoint/server/routes/alerts/index.ts

@@ -6,8 +6,9 @@
 import { IRouter } from 'kibana/server';
 import { EndpointAppContext } from '../../types';
 import { EndpointAppConstants } from '../../../common/types';
-import { alertListHandlerWrapper, alertListReqSchema } from './list';
+import { alertListHandlerWrapper } from './list';
 import { alertDetailsHandlerWrapper, alertDetailsReqSchema } from './details';
+import { alertingIndexGetQuerySchema } from '../../../common/schema/alert_index';
 
 export const BASE_ALERTS_ROUTE = `${EndpointAppConstants.BASE_API_URL}/alerts`;
 
@@ -16,7 +17,7 @@ export function registerAlertRoutes(router: IRouter, endpointAppContext: Endpoin
     {
       path: BASE_ALERTS_ROUTE,
       validate: {
-        query: alertListReqSchema,
+        query: alertingIndexGetQuerySchema,
       },
       options: { authRequired: true },
     },

x-pack/plugins/endpoint/server/routes/alerts/lib/index.ts

@@ -18,10 +18,10 @@ import {
 export { Pagination } from './pagination';
 
 function reverseSortDirection(order: Direction): Direction {
-  if (order === Direction.asc) {
-    return Direction.desc;
+  if (order === 'asc') {
+    return 'desc';
   }
-  return Direction.asc;
+  return 'asc';
 }
 
 function buildQuery(query: AlertSearchQuery): JsonObject {

x-pack/plugins/endpoint/server/routes/alerts/lib/pagination.ts

@@ -4,8 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { RequestHandlerContext } from 'src/core/server';
 import { EndpointConfigType } from '../../../config';
+import { RequestHandlerContext } from '../../../../../../../src/core/server';
 
 /**
  * Abstract Pagination class for determining next/prev urls,

x-pack/plugins/endpoint/server/routes/alerts/list/handlers.ts

@@ -3,18 +3,18 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { KibanaRequest, RequestHandler } from 'kibana/server';
+import { RequestHandler } from 'kibana/server';
 import { EndpointAppContext } from '../../../types';
 import { searchESForAlerts } from '../lib';
 import { getRequestData, mapToAlertResultList } from './lib';
-import { AlertListRequestQuery } from '../types';
+import { AlertingIndexGetQueryResult } from '../../../../common/types';
 
 export const alertListHandlerWrapper = function(
   endpointAppContext: EndpointAppContext
-): RequestHandler<unknown, AlertListRequestQuery, unknown> {
-  const alertListHandler: RequestHandler<unknown, AlertListRequestQuery, unknown> = async (
+): RequestHandler<unknown, AlertingIndexGetQueryResult, unknown> {
+  const alertListHandler: RequestHandler<unknown, AlertingIndexGetQueryResult, unknown> = async (
     ctx,
-    req: KibanaRequest<unknown, AlertListRequestQuery, unknown>,
+    req,
     res
   ) => {
     try {

x-pack/plugins/endpoint/server/routes/alerts/list/index.ts

@@ -4,5 +4,4 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { alertListReqSchema } from './schemas';
 export { alertListHandlerWrapper } from './handlers';

x-pack/plugins/endpoint/server/routes/alerts/list/lib/index.ts

@@ -15,13 +15,14 @@ import {
   AlertHits,
   EndpointAppConstants,
   ESTotal,
+  AlertingIndexGetQueryResult,
 } from '../../../../../common/types';
 import { EndpointAppContext } from '../../../../types';
-import { AlertSearchQuery, AlertListRequestQuery } from '../../types';
+import { AlertSearchQuery } from '../../types';
 import { AlertListPagination } from './pagination';
 
 export const getRequestData = async (
-  request: KibanaRequest<unknown, AlertListRequestQuery, unknown>,
+  request: KibanaRequest<unknown, AlertingIndexGetQueryResult, unknown>,
   endpointAppContext: EndpointAppContext
 ): Promise<AlertSearchQuery> => {
   const config = await endpointAppContext.config();
@@ -29,7 +30,7 @@ export const getRequestData = async (
     // Defaults not enforced by schema
     pageSize: request.query.page_size || EndpointAppConstants.ALERT_LIST_DEFAULT_PAGE_SIZE,
     sort: request.query.sort || EndpointAppConstants.ALERT_LIST_DEFAULT_SORT,
-    order: request.query.order || EndpointAppConstants.ALERT_LIST_DEFAULT_ORDER,
+    order: request.query.order || 'desc',
     dateRange: ((request.query.date_range !== undefined
       ? decode(request.query.date_range)
       : config.alertResultListDefaultDateRange) as unknown) as TimeRange,