Skip to content

Commit

Permalink
[ML] Refactor imports using 'elasticsearch' to '@elastic/elasticsearc…
Browse files Browse the repository at this point in the history
…h'. Extend 'isPopulatedOjbect()'. (elastic#95651)

- Gets rid of imports from 'elasticsearch' and replaces them with '@elastic/elasticsearch'.
- Extends isPopulatedObject() to allow an optional array of attributes to check if they exist. Allows us to get rid of the manual and inconsistent usages of hasOwnProperty().
  • Loading branch information
walterra authored Apr 1, 2021
1 parent 07a3f9e commit a1c36e7
Show file tree
Hide file tree
Showing 50 changed files with 383 additions and 300 deletions.
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

0 comments on commit a1c36e7

Please sign in to comment.