Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Refactor imports using 'elasticsearch' to '@elastic/elasticsearch'. Extend 'isPopulatedOjbect()'. #95651

Merged
merged 8 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion x-pack/plugins/ml/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
* 2.0.
*/

export { HitsTotalRelation, SearchResponse7, HITS_TOTAL_RELATION } from './types/es_client';
export { ES_CLIENT_TOTAL_HITS_RELATION } from './types/es_client';
export { ChartData } from './types/field_histograms';
export { ANOMALY_SEVERITY, ANOMALY_THRESHOLD, SEVERITY_COLORS } from './constants/anomalies';
export { getSeverityColor, getSeverityType } from './util/anomaly_utils';
export { isPopulatedObject } from './util/object_utils';
export { isRuntimeMappings } from './util/runtime_field_utils';
export { composeValidators, patternValidator } from './util/validators';
export { extractErrorMessage } from './util/errors';
33 changes: 12 additions & 21 deletions x-pack/plugins/ml/common/types/es_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,24 @@
* 2.0.
*/

import type { SearchResponse, ShardsResponse } from 'elasticsearch';
import { estypes } from '@elastic/elasticsearch';

import { buildEsQuery } from '../../../../../src/plugins/data/common/es_query/es_query';
import type { DslQuery } from '../../../../../src/plugins/data/common/es_query/kuery';
import type { JsonObject } from '../../../../../src/plugins/kibana_utils/common';

export const HITS_TOTAL_RELATION = {
import { isPopulatedObject } from '../util/object_utils';

export function isMultiBucketAggregate(arg: unknown): arg is estypes.MultiBucketAggregate {
return isPopulatedObject(arg, ['buckets']);
}

export const ES_CLIENT_TOTAL_HITS_RELATION: Record<
Uppercase<estypes.TotalHitsRelation>,
estypes.TotalHitsRelation
> = {
EQ: 'eq',
GTE: 'gte',
} as const;
export type HitsTotalRelation = typeof HITS_TOTAL_RELATION[keyof typeof HITS_TOTAL_RELATION];

// The types specified in `@types/elasticsearch` are out of date and still have `total: number`.
interface SearchResponse7Hits<T> {
hits: SearchResponse<T>['hits']['hits'];
max_score: number;
total: {
value: number;
relation: HitsTotalRelation;
};
}
export interface SearchResponse7<T = any> {
took: number;
timed_out: boolean;
_scroll_id?: string;
_shards: ShardsResponse;
hits: SearchResponse7Hits<T>;
aggregations?: any;
}

export type InfluencersFilterQuery = ReturnType<typeof buildEsQuery> | DslQuery | JsonObject;
8 changes: 2 additions & 6 deletions x-pack/plugins/ml/common/types/feature_importance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,11 @@ export function isRegressionTotalFeatureImportance(
export function isClassificationFeatureImportanceBaseline(
baselineData: any
): baselineData is ClassificationFeatureImportanceBaseline {
return (
isPopulatedObject(baselineData) &&
baselineData.hasOwnProperty('classes') &&
Array.isArray(baselineData.classes)
);
return isPopulatedObject(baselineData, ['classes']) && Array.isArray(baselineData.classes);
}

export function isRegressionFeatureImportanceBaseline(
baselineData: any
): baselineData is RegressionFeatureImportanceBaseline {
return isPopulatedObject(baselineData) && baselineData.hasOwnProperty('baseline');
return isPopulatedObject(baselineData, ['baseline']);
}
50 changes: 50 additions & 0 deletions x-pack/plugins/ml/common/util/object_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { isPopulatedObject } from './object_utils';

describe('object_utils', () => {
describe('isPopulatedObject()', () => {
it('does not allow numbers', () => {
expect(isPopulatedObject(0)).toBe(false);
});
it('does not allow strings', () => {
expect(isPopulatedObject('')).toBe(false);
});
it('does not allow null', () => {
expect(isPopulatedObject(null)).toBe(false);
});
it('does not allow an empty object', () => {
expect(isPopulatedObject({})).toBe(false);
});
it('allows an object with an attribute', () => {
expect(isPopulatedObject({ attribute: 'value' })).toBe(true);
});
it('does not allow an object with a non-existing required attribute', () => {
expect(isPopulatedObject({ attribute: 'value' }, ['otherAttribute'])).toBe(false);
});
it('allows an object with an existing required attribute', () => {
expect(isPopulatedObject({ attribute: 'value' }, ['attribute'])).toBe(true);
});
it('allows an object with two existing required attributes', () => {
expect(
isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
'attribute1',
'attribute2',
])
).toBe(true);
});
it('does not allow an object with two required attributes where one does not exist', () => {
expect(
isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
'attribute1',
'otherAttribute',
])
).toBe(false);
});
});
});
30 changes: 28 additions & 2 deletions x-pack/plugins/ml/common/util/object_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,32 @@
* 2.0.
*/

export const isPopulatedObject = <T = Record<string, any>>(arg: any): arg is T => {
return typeof arg === 'object' && arg !== null && Object.keys(arg).length > 0;
/*
* A type guard to check record like object structures.
*
* Examples:
* - `isPopulatedObject({...})`
* Limits type to Record<string, unknown>
*
* - `isPopulatedObject({...}, ['attribute'])`
* Limits type to Record<'attribute', unknown>
*
* - `isPopulatedObject<keyof MyInterface>({...})`
* Limits type to a record with keys of the given interface.
* Note that you might want to add keys from the interface to the
* array of requiredAttributes to satisfy runtime requirements.
* Otherwise you'd just satisfy TS requirements but might still
* run into runtime issues.
*/
export const isPopulatedObject = <U extends string = string>(
arg: unknown,
requiredAttributes: U[] = []
): arg is Record<U, unknown> => {
return (
typeof arg === 'object' &&
arg !== null &&
Object.keys(arg).length > 0 &&
(requiredAttributes.length === 0 ||
requiredAttributes.every((d) => ({}.hasOwnProperty.call(arg, d))))
);
};
102 changes: 102 additions & 0 deletions x-pack/plugins/ml/common/util/runtime_field_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { isRuntimeField, isRuntimeMappings } from './runtime_field_utils';

describe('ML runtime field utils', () => {
describe('isRuntimeField()', () => {
it('does not allow numbers', () => {
expect(isRuntimeField(1)).toBe(false);
});
it('does not allow null', () => {
expect(isRuntimeField(null)).toBe(false);
});
it('does not allow arrays', () => {
expect(isRuntimeField([])).toBe(false);
});
it('does not allow empty objects', () => {
expect(isRuntimeField({})).toBe(false);
});
it('does not allow objects with non-matching attributes', () => {
expect(isRuntimeField({ someAttribute: 'someValue' })).toBe(false);
expect(isRuntimeField({ type: 'wrong-type' })).toBe(false);
expect(isRuntimeField({ type: 'keyword', someAttribute: 'some value' })).toBe(false);
});
it('allows objects with type attribute only', () => {
expect(isRuntimeField({ type: 'keyword' })).toBe(true);
});
it('allows objects with both type and script attributes', () => {
expect(isRuntimeField({ type: 'keyword', script: 'some script' })).toBe(true);
});
});

describe('isRuntimeMappings()', () => {
it('does not allow numbers', () => {
expect(isRuntimeMappings(1)).toBe(false);
});
it('does not allow null', () => {
expect(isRuntimeMappings(null)).toBe(false);
});
it('does not allow arrays', () => {
expect(isRuntimeMappings([])).toBe(false);
});
it('does not allow empty objects', () => {
expect(isRuntimeMappings({})).toBe(false);
});
it('does not allow objects with non-object inner structure', () => {
expect(isRuntimeMappings({ someAttribute: 'someValue' })).toBe(false);
});
it('does not allow objects with objects with unsupported inner structure', () => {
expect(isRuntimeMappings({ fieldName1: { type: 'keyword' }, fieldName2: 'someValue' })).toBe(
false
);
expect(
isRuntimeMappings({
fieldName1: { type: 'keyword' },
fieldName2: { type: 'keyword', someAttribute: 'some value' },
})
).toBe(false);
expect(
isRuntimeMappings({
fieldName: { type: 'long', script: 1234 },
})
).toBe(false);
expect(
isRuntimeMappings({
fieldName: { type: 'long', script: { someAttribute: 'some value' } },
})
).toBe(false);
expect(
isRuntimeMappings({
fieldName: { type: 'long', script: { source: 1234 } },
})
).toBe(false);
});

it('allows object with most basic runtime mapping', () => {
expect(isRuntimeMappings({ fieldName: { type: 'keyword' } })).toBe(true);
});
it('allows object with multiple most basic runtime mappings', () => {
expect(
isRuntimeMappings({ fieldName1: { type: 'keyword' }, fieldName2: { type: 'keyword' } })
).toBe(true);
});
it('allows object with runtime mappings including scripts', () => {
expect(
isRuntimeMappings({
fieldName1: { type: 'keyword' },
fieldName2: { type: 'keyword', script: 'some script as script' },
})
).toBe(true);
expect(
isRuntimeMappings({
fieldName: { type: 'long', script: { source: 'some script as source' } },
})
).toBe(true);
});
});
});
15 changes: 7 additions & 8 deletions x-pack/plugins/ml/common/util/runtime_field_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,18 @@ import { isPopulatedObject } from './object_utils';
import { RUNTIME_FIELD_TYPES } from '../../../../../src/plugins/data/common';
import type { RuntimeField, RuntimeMappings } from '../types/fields';

type RuntimeType = typeof RUNTIME_FIELD_TYPES[number];

export function isRuntimeField(arg: unknown): arg is RuntimeField {
return (
isPopulatedObject(arg) &&
((Object.keys(arg).length === 1 && arg.hasOwnProperty('type')) ||
(Object.keys(arg).length === 2 &&
arg.hasOwnProperty('type') &&
arg.hasOwnProperty('script') &&
((isPopulatedObject(arg, ['type']) && Object.keys(arg).length === 1) ||
(isPopulatedObject(arg, ['type', 'script']) &&
Object.keys(arg).length === 2 &&
(typeof arg.script === 'string' ||
(isPopulatedObject(arg.script) &&
(isPopulatedObject(arg.script, ['source']) &&
Object.keys(arg.script).length === 1 &&
arg.script.hasOwnProperty('source') &&
typeof arg.script.source === 'string')))) &&
RUNTIME_FIELD_TYPES.includes(arg.type)
RUNTIME_FIELD_TYPES.includes(arg.type as RuntimeType)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { Dispatch, SetStateAction } from 'react';

import { estypes } from '@elastic/elasticsearch';
import {
EuiDataGridCellValueElementProps,
EuiDataGridPaginationProps,
Expand All @@ -15,7 +16,6 @@ import {
} from '@elastic/eui';

import { Dictionary } from '../../../../common/types/common';
import { HitsTotalRelation } from '../../../../common/types/es_client';
import { ChartData } from '../../../../common/types/field_histograms';

import { INDEX_STATUS } from '../../data_frame_analytics/common/analytics';
Expand All @@ -27,7 +27,7 @@ export type DataGridItem = Record<string, any>;

// `undefined` is used to indicate a non-initialized state.
export type ChartsVisible = boolean | undefined;
export type RowCountRelation = HitsTotalRelation | undefined;
export type RowCountRelation = estypes.TotalHitsRelation | undefined;

export type IndexPagination = Pick<EuiDataGridPaginationProps, 'pageIndex' | 'pageSize'>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';

import { EuiDataGridSorting, EuiDataGridColumn } from '@elastic/eui';

import { HITS_TOTAL_RELATION } from '../../../../common/types/es_client';
import { ES_CLIENT_TOTAL_HITS_RELATION } from '../../../../common/types/es_client';
import { ChartData } from '../../../../common/types/field_histograms';

import { INDEX_STATUS } from '../../data_frame_analytics/common';
Expand Down Expand Up @@ -146,7 +146,7 @@ export const useDataGrid = (
if (chartsVisible === undefined && rowCount > 0 && rowCountRelation !== undefined) {
setChartsVisible(
rowCount <= COLUMN_CHART_DEFAULT_VISIBILITY_ROWS_THRESHOLED &&
rowCountRelation !== HITS_TOTAL_RELATION.GTE
rowCountRelation !== ES_CLIENT_TOTAL_HITS_RELATION.GTE
);
}
}, [chartsVisible, rowCount, rowCountRelation]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import React, { useMemo, useEffect, useState, FC } from 'react';

import { estypes } from '@elastic/elasticsearch';

import {
EuiCallOut,
EuiComboBox,
Expand All @@ -24,7 +26,6 @@ import { i18n } from '@kbn/i18n';

import { extractErrorMessage } from '../../../../common';
import { stringHash } from '../../../../common/util/string_utils';
import type { SearchResponse7 } from '../../../../common/types/es_client';
import type { ResultsSearchQuery } from '../../data_frame_analytics/common/analytics';

import { useMlApiContext } from '../../contexts/kibana';
Expand Down Expand Up @@ -184,7 +185,7 @@ export const ScatterplotMatrix: FC<ScatterplotMatrixProps> = ({
}
: searchQuery;

const resp: SearchResponse7 = await esSearch({
const resp: estypes.SearchResponse = await esSearch({
index,
body: {
fields: queryFields,
Expand All @@ -198,7 +199,7 @@ export const ScatterplotMatrix: FC<ScatterplotMatrixProps> = ({
if (!options.didCancel) {
const items = resp.hits.hits
.map((d) =>
getProcessedFields(d.fields, (key: string) =>
getProcessedFields(d.fields ?? {}, (key: string) =>
key.startsWith(`${resultsField}.feature_importance`)
)
)
Expand Down
Loading