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

Improve SearchSource SearchRequest type #186862

Merged
merged 24 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6b778cb
Improve SearchSource SearchRequest type
lukasolson Jun 24, 2024
be1bc30
Fix data view mock
lukasolson Jun 25, 2024
894c72c
Update snapshots
lukasolson Jun 25, 2024
8372376
Merge branch 'main' into search_source_search_request
lukasolson Jun 25, 2024
f4bd2e9
Update query_to_fields filters usage
lukasolson Jun 26, 2024
c86d796
Merge branch 'search_source_search_request' of github.com:lukasolson/…
lukasolson Jun 26, 2024
ba8353a
Fix filters when they are functions
lukasolson Jun 26, 2024
5f41d2a
Merge branch 'main' into search_source_search_request
lukasolson Jul 8, 2024
6fd54cf
Review feedback & update one more reference to incorrect search request
lukasolson Jul 8, 2024
ba6c639
Merge branch 'main' into search_source_search_request
lukasolson Jul 9, 2024
50b60fb
Merge branch 'main' into search_source_search_request
lukasolson Jul 10, 2024
f12ca4c
Merge branch 'main' into search_source_search_request
davismcphee Jul 10, 2024
f8ef285
Merge branch 'main' into search_source_search_request
lukasolson Jul 11, 2024
8097a77
Review feedback
lukasolson Jul 11, 2024
411297f
Merge branch 'search_source_search_request' of github.com:lukasolson/…
lukasolson Jul 11, 2024
22fde50
Merge branch 'main' into search_source_search_request
lukasolson Jul 12, 2024
704d5c0
Merge branch 'main' into search_source_search_request
lukasolson Jul 12, 2024
0073ada
Merge branch 'main' into search_source_search_request
lukasolson Jul 15, 2024
eb3e93b
Merge branch 'main' into search_source_search_request
lukasolson Jul 15, 2024
02b31ad
Merge branch 'main' into search_source_search_request
lukasolson Jul 16, 2024
c7fab54
Merge branch 'main' into search_source_search_request
lukasolson Jul 17, 2024
e34304e
Merge branch 'main' into search_source_search_request
lukasolson Jul 17, 2024
6ad68ab
Merge branch 'search_source_search_request' of github.com:lukasolson/…
lukasolson Jul 17, 2024
140b4a6
Merge branch 'main' into search_source_search_request
lukasolson Jul 22, 2024
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
1 change: 1 addition & 0 deletions packages/kbn-discover-utils/src/__mocks__/data_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export const buildDataViewMock = ({
return dataViewFields.find((field) => field.name === timeFieldName);
},
getRuntimeField: () => null,
getAllowHidden: () => false,
} as unknown as DataView;

dataView.isTimeBased = () => !!timeFieldName;
Expand Down
8 changes: 2 additions & 6 deletions packages/kbn-generate-csv/src/lib/search_cursor_pit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@
import type { estypes } from '@elastic/elasticsearch';
import type { Logger } from '@kbn/core/server';
import { lastValueFrom } from 'rxjs';
import {
ES_SEARCH_STRATEGY,
type ISearchSource,
type SearchRequest,
} from '@kbn/data-plugin/common';
import { ES_SEARCH_STRATEGY, type ISearchSource } from '@kbn/data-plugin/common';
import { SearchCursor, type SearchCursorClients, type SearchCursorSettings } from './search_cursor';
import { i18nTexts } from './i18n_texts';

Expand Down Expand Up @@ -74,7 +70,7 @@ export class SearchCursorPit extends SearchCursor {
return pitId;
}

protected async searchWithPit(searchBody: SearchRequest) {
protected async searchWithPit(searchBody: estypes.SearchRequest) {
const { maxConcurrentShardRequests, scroll, taskInstanceFields } = this.settings;

// maxConcurrentShardRequests=0 is not supported
Expand Down
8 changes: 2 additions & 6 deletions packages/kbn-generate-csv/src/lib/search_cursor_scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import type { estypes } from '@elastic/elasticsearch';
import { lastValueFrom } from 'rxjs';
import type { Logger } from '@kbn/core/server';
import type { IEsSearchResponse } from '@kbn/search-types';
import {
ES_SEARCH_STRATEGY,
type ISearchSource,
type SearchRequest,
} from '@kbn/data-plugin/common';
import { ES_SEARCH_STRATEGY, type ISearchSource } from '@kbn/data-plugin/common';
import { SearchCursor, type SearchCursorClients, type SearchCursorSettings } from './search_cursor';
import { i18nTexts } from './i18n_texts';

Expand All @@ -32,7 +28,7 @@ export class SearchCursorScroll extends SearchCursor {
// The first search query begins the scroll context in ES
public async initialize() {}

private async scan(searchBody: SearchRequest) {
private async scan(searchBody: estypes.SearchRequest) {
const { includeFrozen, maxConcurrentShardRequests, scroll, taskInstanceFields } = this.settings;

// maxConcurrentShardRequests=0 is not supported
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { Observable } from 'rxjs';

import { schema } from '@kbn/config-schema';
import { CoreSetup, ElasticsearchClient } from '@kbn/core/server';
import { SearchRequest } from '@kbn/data-plugin/common';
import { getKbnServerError, reportServerError } from '@kbn/kibana-utils-plugin/server';
import { PluginSetup as UnifiedSearchPluginSetup } from '@kbn/unified-search-plugin/server';

import type { SearchRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { OptionsListRequestBody, OptionsListResponse } from '../../common/options_list/types';
import { getValidationAggregationBuilder } from './options_list_validation_queries';
import { getSuggestionAggregationBuilder } from './suggestion_queries';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ export function getSearchParamsFromRequest(
const searchParams = getSearchParams(getConfig);
// eslint-disable-next-line @typescript-eslint/naming-convention
const { track_total_hits, ...body } = searchRequest.body;
const dataView = typeof searchRequest.index !== 'string' ? searchRequest.index : undefined;
const index = dataView?.title ?? `${searchRequest.index}`;
Copy link
Contributor

@mattkime mattkime Jul 10, 2024

Choose a reason for hiding this comment

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

It appears as though the template literal isn't needed. might leave a note in the code if it is doing something useful

Copy link
Member Author

Choose a reason for hiding this comment

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

searchRequest.index is optional and I was trying to get around the type checks while preserving the existing behavior here - that it will be treated as "undefined".

It might be more useful here to actually throw some sort of error instead - I think it is a required parameter here. I'll dig into it a little more.


return {
index: searchRequest.index.title || searchRequest.index,
index,
body,
// @ts-ignore
track_total_hits,
...(searchRequest.index?.allowHidden && { expand_wildcards: 'all' }),
...(dataView?.getAllowHidden() && { expand_wildcards: 'all' }),
...searchParams,
};
}
8 changes: 7 additions & 1 deletion src/plugins/data/common/search/search_source/fetch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* Side Public License, v 1.
*/
import type { IKibanaSearchResponse } from '@kbn/search-types';
import type { DataView } from '@kbn/data-views-plugin/common';
import type { AggregateQuery, Filter, Query } from '@kbn/es-query';
import { SearchSourceSearchOptions } from '../../..';
import { GetConfigFn } from '../../../types';

Expand All @@ -17,7 +19,11 @@ import { GetConfigFn } from '../../../types';
* where `ISearchRequestParams` is used externally instead.
* FIXME: replace with estypes.SearchRequest?
*/
export type SearchRequest = Record<string, any>;
export type SearchRequest<T extends Record<string, any> = Record<string, any>> = {
index?: DataView | string;
query?: Array<Query | AggregateQuery>;
filters?: Filter[] | (() => Filter[]);
} & Omit<T, 'index' | 'query' | 'filters'>;

export interface FetchHandlers {
getConfig: GetConfigFn;
Expand Down
21 changes: 9 additions & 12 deletions src/plugins/data/common/search/search_source/query_to_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { DataViewLazy } from '@kbn/data-views-plugin/common';
import { fromKueryExpression, getKqlFieldNames } from '@kbn/es-query';
import { fromKueryExpression, getKqlFieldNames, isFilter, isOfQueryType } from '@kbn/es-query';
import type { SearchRequest } from './fetch';
import { EsQuerySortValue } from '../..';

Expand All @@ -25,23 +25,20 @@ export async function queryToFields({
const sortArr = Array.isArray(sort) ? sort : [sort];
fields.push(...sortArr.flatMap((s) => Object.keys(s)));
}
for (const query of request.query) {
for (const query of (request.query ?? []).filter(isOfQueryType)) {
if (query.query && query.language === 'kuery') {
const nodes = fromKueryExpression(query.query);
const queryFields = getKqlFieldNames(nodes);
fields = fields.concat(queryFields);
}
}
const filters = request.filters;
if (filters) {
const filtersArr = Array.isArray(filters) ? filters : [filters];
for (const f of filtersArr) {
// unified search bar filters have meta object and key (regular filters)
// unified search bar "custom" filters ("Edit as query DSL", where meta.key is not present but meta is)
// Any other Elasticsearch query DSL filter that gets passed in by consumers (not coming from unified search, and these probably won't have a meta key at all)
if (f?.meta?.key && f.meta.disabled !== true) {
fields.push(f.meta.key);
}
const { filters = [] } = request;
for (const f of typeof filters === 'function' ? filters() : filters) {
// unified search bar filters have meta object and key (regular filters)
// unified search bar "custom" filters ("Edit as query DSL", where meta.key is not present but meta is)
// Any other Elasticsearch query DSL filter that gets passed in by consumers (not coming from unified search, and these probably won't have a meta key at all)
if (isFilter(f) && f?.meta?.key && f.meta.disabled !== true) {
fields.push(f.meta.key);
}
}

Expand Down
19 changes: 13 additions & 6 deletions src/plugins/data/common/search/search_source/search_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ const indexPattern = {
fields: [{ name: 'foo-bar' }, { name: 'field1' }, { name: 'field2' }, { name: '_id' }],
getComputedFields,
getSourceFiltering: () => mockSource,
getAllowHidden: jest.fn(),
} as unknown as DataView;

const indexPattern2 = {
title: 'foo',
getComputedFields,
getSourceFiltering: () => mockSource2,
getAllowHidden: jest.fn(),
} as unknown as DataView;

const fields3 = [{ name: 'foo-bar' }, { name: 'field1' }, { name: 'field2' }];
Expand Down Expand Up @@ -1474,10 +1476,15 @@ describe('SearchSource', () => {
return buildExpression(ast).toString();
}

beforeEach(() => {
searchSource = new SearchSource({}, searchSourceDependencies);
searchSource.setField('index', indexPattern);
});

test('should generate an expression AST', () => {
expect(toString(searchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context
| esdsl dsl=\\"{}\\""
| esdsl dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand All @@ -1486,7 +1493,7 @@ describe('SearchSource', () => {

expect(toString(searchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context q={kql q=\\"something\\"}
| esdsl dsl=\\"{}\\""
| esdsl dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand All @@ -1504,7 +1511,7 @@ describe('SearchSource', () => {
expect(toString(searchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context filters={kibanaFilter query=\\"{\\\\\\"query_string\\\\\\":{\\\\\\"query\\\\\\":\\\\\\"query1\\\\\\"}}\\"}
filters={kibanaFilter query=\\"{\\\\\\"query_string\\\\\\":{\\\\\\"query\\\\\\":\\\\\\"query2\\\\\\"}}\\"}
| esdsl dsl=\\"{}\\""
| esdsl dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand All @@ -1517,7 +1524,7 @@ describe('SearchSource', () => {

expect(toString(searchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context filters={kibanaFilter query=\\"{\\\\\\"query_string\\\\\\":{\\\\\\"query\\\\\\":\\\\\\"query\\\\\\"}}\\"}
| esdsl dsl=\\"{}\\""
| esdsl dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand All @@ -1540,7 +1547,7 @@ describe('SearchSource', () => {
expect(toString(childSearchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context q={kql q=\\"something2\\"} q={kql q=\\"something1\\"} filters={kibanaFilter query=\\"{\\\\\\"query_string\\\\\\":{\\\\\\"query\\\\\\":\\\\\\"query2\\\\\\"}}\\"}
filters={kibanaFilter query=\\"{\\\\\\"query_string\\\\\\":{\\\\\\"query\\\\\\":\\\\\\"query1\\\\\\"}}\\"}
| esdsl dsl=\\"{}\\""
| esdsl dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand All @@ -1558,7 +1565,7 @@ describe('SearchSource', () => {

expect(toString(searchSource.toExpressionAst())).toMatchInlineSnapshot(`
"kibana_context
| esdsl size=1000 dsl=\\"{}\\""
| esdsl size=1000 dsl=\\"{}\\" index=\\"1234\\""
`);
});

Expand Down
Loading