Skip to content

Commit

Permalink
Use HTTP request schemas to create types, use those types in the clie…
Browse files Browse the repository at this point in the history
…nt (#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 <[email protected]>
  • Loading branch information
Robert Austin and elasticmachine committed Mar 11, 2020
1 parent 655f23b commit 193926f
Show file tree
Hide file tree
Showing 18 changed files with 192 additions and 77 deletions.
7 changes: 6 additions & 1 deletion src/core/public/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/endpoint/common/schema/README.md
Original file line number Diff line number Diff line change
@@ -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'.
Original file line number Diff line number Diff line change
Expand Up @@ -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 { schema, Type } from '@kbn/config-schema';
import { i18n } from '@kbn/i18n';
import { schema } from '@kbn/config-schema';
import { esKuery } from '../../../../../../../src/plugins/data/server';
import { EndpointAppConstants } from '../../../../common/types';
import { decode } from 'rison-node';
import { fromKueryExpression } from '../../../../../src/plugins/data/common';
import { EndpointAppConstants } from '../types';

export const alertListReqSchema = schema.object(
/**
* Used to validate GET requests against the index of the alerting APIs.
*/
export const alertingIndexGetQuerySchema = schema.object(
{
page_size: schema.maybe(
schema.number({
Expand All @@ -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',
Expand Down
78 changes: 73 additions & 5 deletions x-pack/plugins/endpoint/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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';
Expand All @@ -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 {
Expand Down Expand Up @@ -345,3 +344,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>;
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ 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) => {
next(action);
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 });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -82,14 +78,18 @@ export const uiQueryParams: (
*/
export const apiQueryParams: (
state: AlertListState
) => Immutable<AlertsAPIQueryParams> = createSelector(
) => Immutable<AlertingIndexGetQueryInput> = createSelector(
uiQueryParams,
({ page_size, page_index }) => ({
page_size,
page_index,
})
);

/**
* 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
Expand Down
16 changes: 1 addition & 15 deletions x-pack/plugins/endpoint/public/applications/endpoint/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import { Dispatch, MiddlewareAPI } from 'redux';
import { CoreStart } from 'kibana/public';
import {
EndpointMetadata,
AlertData,
Expand All @@ -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> = (
Expand Down Expand Up @@ -128,17 +128,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;
}
Original file line number Diff line number Diff line change
@@ -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,
}
`);
});
});
22 changes: 22 additions & 0 deletions x-pack/plugins/endpoint/public/common/clone_http_fetch_query.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Loading

0 comments on commit 193926f

Please sign in to comment.