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] AIOps: Improve cleanup of default queries. #176534

Merged
merged 14 commits into from
Mar 8, 2024
Merged
11 changes: 10 additions & 1 deletion x-pack/packages/ml/query_utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,17 @@

export { addExcludeFrozenToQuery } from './src/add_exclude_frozen_to_query';
export { buildBaseFilterCriteria } from './src/build_base_filter_criteria';
export { isDefaultQuery } from './src/default_query';
export { ES_CLIENT_TOTAL_HITS_RELATION } from './src/es_client_total_hits_relation';
export { getSafeAggregationName } from './src/get_safe_aggregation_name';
export { matchAllQuery, isMatchAllQuery } from './src/match_all_query';
export { isSimpleQuery } from './src/simple_query';
export { SEARCH_QUERY_LANGUAGE } from './src/types';
export type { SearchQueryLanguage } from './src/types';
export type {
BoolFilterBasedSimpleQuery,
SavedSearchQuery,
SearchQueryLanguage,
SearchQueryVariant,
SimpleQuery,
} from './src/types';
export { getDefaultDSLQuery } from './src/get_default_query';
10 changes: 10 additions & 0 deletions x-pack/packages/ml/query_utils/src/__mocks__/simple_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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 type { SearchQueryVariant } from '../types';

export const simpleQueryMock: SearchQueryVariant = { query_string: { query: 'airline:AAL' } };
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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 { isBoolFilterBasedSimpleQuery } from './bool_filter_based_simple_query';
import { simpleQueryMock } from './__mocks__/simple_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isBoolFilterBasedSimpleQuery', () => {
it('should identify bool filter based simple queries', () => {
expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [simpleQueryMock] },
})
).toBe(true);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [simpleQueryMock], must: [], must_not: [], should: [] },
})
).toBe(true);
});

it('should identify non-simple queries or differently structured simple queries', () => {
expect(isBoolFilterBasedSimpleQuery(defaultSimpleQuery)).toBe(false);
expect(isBoolFilterBasedSimpleQuery(matchAllQuery)).toBe(false);
expect(isBoolFilterBasedSimpleQuery(simpleQueryMock)).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [], must: [], must_not: [], should: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [matchAllQuery], must: [], must_not: [], should: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [matchAllQuery] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [] },
})
).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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 '@kbn/ml-is-populated-object';

import { isSimpleQuery } from './simple_query';
import type { BoolFilterBasedSimpleQuery } from './types';

/**
* Type guard to check if the provided argument is a boolean filter-based simple query.
*
* A valid `BoolFilterBasedSimpleQuery` must have a `bool` property, which itself
* must have a `filter` property. This `filter` must be an array with exactly
* one element, and that element must satisfy the conditions of a simple query
* as defined by `isSimpleQuery`.
*
* The type guard is useful to identify simple queries within bool filter
* queries exposed from Kibana/EUI search bars.
*
* @param arg - The argument to be checked.
* @returns `true` if `arg` meets the criteria of a `BoolFilterBasedSimpleQuery`, otherwise `false`.
*/
export function isBoolFilterBasedSimpleQuery(arg: unknown): arg is BoolFilterBasedSimpleQuery {
return (
isPopulatedObject(arg, ['bool']) &&
isPopulatedObject(arg.bool, ['filter']) &&
Array.isArray(arg.bool.filter) &&
arg.bool.filter.length === 1 &&
isSimpleQuery(arg.bool.filter[0])
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { Query } from '@kbn/es-query';

import { isDefaultQuery } from './default_query';
import { isFilterBasedDefaultQuery } from './filter_based_default_query';

/**
* Builds the base filter criteria used in queries,
* adding criteria for the time range and an optional query.
Expand Down Expand Up @@ -38,7 +41,12 @@ export function buildBaseFilterCriteria(
});
}

if (query && typeof query === 'object') {
if (
query &&
typeof query === 'object' &&
!isDefaultQuery(query) &&
!isFilterBasedDefaultQuery(query)
) {
filterCriteria.push(query);
}

Expand Down
19 changes: 19 additions & 0 deletions x-pack/packages/ml/query_utils/src/default_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 { simpleQueryMock } from './__mocks__/simple_query';
import { isDefaultQuery } from './default_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isDefaultQuery', () => {
it("should return if it's a default query", () => {
expect(isDefaultQuery(defaultSimpleQuery)).toBe(true);
expect(isDefaultQuery(matchAllQuery)).toBe(true);
expect(isDefaultQuery(simpleQueryMock)).toBe(false);
});
});
28 changes: 28 additions & 0 deletions x-pack/packages/ml/query_utils/src/default_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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 { isBoolFilterBasedSimpleQuery } from './bool_filter_based_simple_query';
import { isMatchAllQuery } from './match_all_query';
import { isSimpleDefaultQuery } from './simple_query';
import type { SearchQueryVariant } from './types';

/**
* Checks if the provided query is a default query. A default query is considered as one that matches all documents,
* either directly through a `match_all` query, a `SimpleQuery` with a wildcard query string, or a `BoolFilterBasedSimpleQuery`
* that contains a filter with a wildcard query or a `match_all` condition.
*
* @param query - The query to check.
* @returns True if the query is a default query, false otherwise.
*/
export function isDefaultQuery(query: SearchQueryVariant): boolean {
return (
isMatchAllQuery(query) ||
isSimpleDefaultQuery(query) ||
(isBoolFilterBasedSimpleQuery(query) &&
(query.bool.filter[0].query_string.query === '*' || isMatchAllQuery(query.bool.filter[0])))
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 { isFilterBasedDefaultQuery } from './filter_based_default_query';
import { simpleQueryMock } from './__mocks__/simple_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isFilterBasedDefaultQuery', () => {
it('should identify filter based default queries', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would separate should identify filter-based default queries and should identify non-default queries

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 updated in 6e6d393.

expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [], must_not: [], should: [] },
})
).toBe(true);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [matchAllQuery], must: [], must_not: [], should: [] },
})
).toBe(true);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [] },
})
).toBe(true);
});

it('should identify non-default queries', () => {
expect(isFilterBasedDefaultQuery(defaultSimpleQuery)).toBe(false);
expect(isFilterBasedDefaultQuery(matchAllQuery)).toBe(false);
expect(isFilterBasedDefaultQuery(simpleQueryMock)).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [simpleQueryMock], must: [], must_not: [], should: [] },
})
).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [], should: [simpleQueryMock] },
})
).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [simpleQueryMock] },
})
).toBe(false);
});
});
44 changes: 44 additions & 0 deletions x-pack/packages/ml/query_utils/src/filter_based_default_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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 '@kbn/ml-is-populated-object';

import { isDefaultQuery } from './default_query';

const boolRequiredAttributes = ['filter', 'must', 'must_not'];

/**
* Determines if the provided argument is a filter-based default query within a boolean filter context.
*
* A valid filter-based default query must include a `bool` property that contains
* `filter`, `must`, and `must_not` properties. These properties should either be empty arrays
* or arrays containing exactly one default query. The function checks for these conditions
* to identify variants of default queries structured within a boolean filter.
*
* Examples of valid structures include:
* - `{ bool: { filter: [{ match_all: {} }], must: [], must_not: [], should: [] } }`
* - `{ bool: { filter: [], must: [{ match_all: {} }], must_not: [] } }`
*
* Useful to identify simple queries within bool queries
* exposed from Kibana/EUI search bars.
*
* @param arg - The argument to be checked, its structure is unknown upfront.
* @returns Returns `true` if `arg` matches the expected structure of a
* filter-based default query, otherwise `false`.
*/
export function isFilterBasedDefaultQuery(arg: unknown): boolean {
return (
isPopulatedObject(arg, ['bool']) &&
isPopulatedObject(arg.bool, boolRequiredAttributes) &&
Object.values(arg.bool).every(
// should be either an empty array or an array with just 1 default query
(d) => {
return Array.isArray(d) && (d.length === 0 || (d.length === 1 && isDefaultQuery(d[0])));
}
)
);
}
16 changes: 16 additions & 0 deletions x-pack/packages/ml/query_utils/src/match_all_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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 { isMatchAllQuery, matchAllQuery } from './match_all_query';

describe('isMatchAllQuery', () => {
it("should return if it's a match_all query", () => {
expect(isMatchAllQuery(matchAllQuery)).toBe(true);
expect(isMatchAllQuery({ query_string: { query: '*' } })).toBe(false);
expect(isMatchAllQuery({ query_string: { query: 'airline:AAL' } })).toBe(false);
});
});
32 changes: 32 additions & 0 deletions x-pack/packages/ml/query_utils/src/match_all_query.ts
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
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { isPopulatedObject } from '@kbn/ml-is-populated-object';

/**
* Represents a query that matches all documents.
*/
export const matchAllQuery = {
/**
* 'match_all' property specifies a query that matches all documents.
*/
match_all: {},
};

/**
* Checks if an argument is a `match_all` query.
* @param {unknown} query - Argument to check.
* @returns {boolean} True if `query` is a `match_all` query, false otherwise.
*/
export function isMatchAllQuery(query: unknown): boolean {
return (
isPopulatedObject(query, ['match_all']) &&
typeof query.match_all === 'object' &&
query.match_all !== null &&
Object.keys(query.match_all).length === 0
);
}
26 changes: 26 additions & 0 deletions x-pack/packages/ml/query_utils/src/simple_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { simpleQueryMock } from './__mocks__/simple_query';
import { defaultSimpleQuery, isSimpleQuery, isSimpleDefaultQuery } from './simple_query';
import { matchAllQuery } from './match_all_query';

describe('isSimpleQuery', () => {
it("should return if it's a simple query", () => {
expect(isSimpleQuery(defaultSimpleQuery)).toBe(true);
expect(isSimpleQuery(matchAllQuery)).toBe(false);
expect(isSimpleQuery(simpleQueryMock)).toBe(true);
});
});

describe('isSimpleDefaultQuery', () => {
it("should return if it's a simple default query", () => {
expect(isSimpleDefaultQuery(defaultSimpleQuery)).toBe(true);
expect(isSimpleDefaultQuery(matchAllQuery)).toBe(false);
expect(isSimpleDefaultQuery(simpleQueryMock)).toBe(false);
});
});
Loading