From bc29dd5a93495c72b326026da3aebabb8a2fdf1c Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 24 Nov 2021 15:33:27 +0100 Subject: [PATCH 1/5] SOR.find: allow to specify per-type search fields --- .../saved_objects/service/lib/repository.ts | 4 -- .../lib/search_dsl/query_params.test.ts | 64 +++++++++++++++++-- .../service/lib/search_dsl/query_params.ts | 50 +++++++++------ .../service/lib/search_dsl/search_dsl.ts | 2 +- src/core/server/saved_objects/types.ts | 10 ++- .../server/routes/find.ts | 10 ++- 6 files changed, 101 insertions(+), 39 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 9be58f1b71861..01e5d648cd887 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -892,10 +892,6 @@ export class SavedObjectsRepository { return SavedObjectsUtils.createEmptyFindResponse(options); } - if (searchFields && !Array.isArray(searchFields)) { - throw SavedObjectsErrorHelpers.createBadRequestError('options.searchFields must be an array'); - } - if (fields && !Array.isArray(fields)) { throw SavedObjectsErrorHelpers.createBadRequestError('options.fields must be an array'); } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 437e7408b945f..f65cf12df8a79 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -7,6 +7,7 @@ */ import * as esKuery from '@kbn/es-query'; + type KueryNode = any; import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; @@ -432,19 +433,27 @@ describe('#getQueryParams', () => { }); describe('`searchFields` and `rootSearchFields` parameters', () => { - const getExpectedFields = (searchFields: string[], typeOrTypes: string | string[]) => { + const getExpectedFields = ( + searchFields: string[] | Record, + typeOrTypes: string | string[] + ) => { const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; - return searchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); + const allSearchFields = Array.isArray(searchFields) + ? searchFields + : [...new Set(Object.values(searchFields).flat())]; + return allSearchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); }; const test = ({ searchFields, rootSearchFields, + typeSubsets = ALL_TYPE_SUBSETS, }: { - searchFields?: string[]; + searchFields?: string[] | Record; rootSearchFields?: string[]; + typeSubsets?: Array; }) => { - for (const typeOrTypes of ALL_TYPE_SUBSETS) { + for (const typeOrTypes of typeSubsets) { const result = getQueryParams({ registry, type: typeOrTypes, @@ -520,6 +529,10 @@ describe('#getQueryParams', () => { it('supports search fields and raw search fields', () => { test({ searchFields: ['title'], rootSearchFields: ['_id'] }); }); + + it('supports per-type searchFields', () => { + test({ searchFields: { pending: ['title'], saved: ['desc'] }, typeSubsets: [ALL_TYPES] }); + }); }); describe('`defaultSearchOperator` parameter', () => { @@ -559,7 +572,7 @@ describe('#getQueryParams', () => { type, }: { search?: string; - searchFields?: string[]; + searchFields?: string[] | Record; type?: string[]; }) => getQueryParams({ @@ -599,7 +612,7 @@ describe('#getQueryParams', () => { const mppClauses = shouldClauses.slice(1); expect(mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0])).toEqual( - ['saved.title', 'pending.title', 'saved.desc', 'pending.desc'] + ['saved.title', 'saved.desc', 'pending.title', 'pending.desc'] ); }); @@ -666,6 +679,45 @@ describe('#getQueryParams', () => { { 'saved.description': { query: 'foo', boost: 1 } }, ]); }); + + it('supports specifying per-type searchFields ', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + type: ['saved', 'pending'], + searchFields: { + saved: ['title', 'desc'], + pending: ['title'], + }, + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(4); + + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0])).toEqual( + ['saved.title', 'saved.desc', 'pending.title'] + ); + }); + + it('fallbacks to `defaultSearchField` when fields are not specified for a type', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + type: ['saved', 'pending'], + searchFields: { + saved: ['desc'], + }, + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(3); + + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0])).toEqual( + ['saved.desc', 'pending.title'] + ); + }); }); }); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index a02378390af7d..a82c7d57f4fb4 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -7,6 +7,7 @@ */ import * as esKuery from '@kbn/es-query'; + type KueryNode = any; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; @@ -132,7 +133,7 @@ interface QueryParams { typeToNamespacesMap?: Map; search?: string; defaultSearchOperator?: SearchOperator; - searchFields?: string[]; + searchFields?: string[] | Record; rootSearchFields?: string[]; hasReference?: HasReferenceQueryParams | HasReferenceQueryParams[]; hasReferenceOperator?: SearchOperator; @@ -212,6 +213,20 @@ export function getQueryParams({ hasReference = [hasReference]; } + const allSearchFields = + !searchFields || Array.isArray(searchFields) + ? searchFields + : [...new Set(Object.values(searchFields).flat())]; + + const searchFieldMap = Array.isArray(searchFields) + ? types.reduce((record, t) => { + return { + ...record, + [t]: searchFields as string[], + }; + }, {} as Record) + : searchFields; + const bool: any = { filter: [ ...(kueryNode != null ? [esKuery.toElasticsearchQuery(kueryNode)] : []), @@ -235,7 +250,7 @@ export function getQueryParams({ const simpleQueryStringClause = getSimpleQueryStringClause({ search, types, - searchFields, + searchFields: allSearchFields, rootSearchFields, defaultSearchOperator, }); @@ -243,7 +258,7 @@ export function getQueryParams({ if (useMatchPhrasePrefix) { bool.should = [ simpleQueryStringClause, - ...getMatchPhrasePrefixClauses({ search, searchFields, types, registry }), + ...getMatchPhrasePrefixClauses({ search, searchFields: searchFieldMap, types, registry }), ]; bool.minimum_should_match = 1; } else { @@ -267,7 +282,7 @@ const getMatchPhrasePrefixClauses = ({ types, }: { search: string; - searchFields?: string[]; + searchFields?: Record; types: string[]; registry: ISavedObjectTypeRegistry; }) => { @@ -292,34 +307,28 @@ interface FieldWithBoost { } const getMatchPhrasePrefixFields = ({ - searchFields = [], + searchFields = {}, types, registry, }: { - searchFields?: string[]; + searchFields?: Record; types: string[]; registry: ISavedObjectTypeRegistry; }): FieldWithBoost[] => { - const output: FieldWithBoost[] = []; - - searchFields = searchFields.filter((field) => field !== '*'); - let fields: string[]; - if (searchFields.length === 0) { - fields = types.reduce((typeFields, type) => { + let allFields: string[] = []; + for (const type of types) { + let typeFields = (searchFields[type] ?? []).filter((field) => field !== '*'); + if (typeFields.length === 0) { const defaultSearchField = registry.getType(type)?.management?.defaultSearchField; if (defaultSearchField) { - return [...typeFields, `${type}.${defaultSearchField}`]; + typeFields = [defaultSearchField]; } - return typeFields; - }, [] as string[]); - } else { - fields = []; - for (const field of searchFields) { - fields = fields.concat(types.map((type) => `${type}.${field}`)); } + allFields = [...allFields, ...typeFields.map((typeField) => `${type}.${typeField}`)]; } - fields.forEach((rawField) => { + const output: FieldWithBoost[] = []; + allFields.forEach((rawField) => { const [field, rawBoost] = rawField.split('^'); let boost: number = 1; if (rawBoost) { @@ -337,6 +346,7 @@ const getMatchPhrasePrefixFields = ({ boost, }); }); + return output; }; diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index f2cf0013dfe08..2139e1469a7c4 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -22,7 +22,7 @@ interface GetSearchDslOptions { type: string | string[]; search?: string; defaultSearchOperator?: SearchOperator; - searchFields?: string[]; + searchFields?: string[] | Record; rootSearchFields?: string[]; searchAfter?: estypes.Id[]; sortField?: string; diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 68040d9c6e003..07d31160192e8 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -90,8 +90,12 @@ export interface SavedObjectsFindOptions { fields?: string[]; /** Search documents using the Elasticsearch Simple Query String syntax. See Elasticsearch Simple Query String `query` argument for more information */ search?: string; - /** The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information */ - searchFields?: string[]; + /** + * The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information + * Can be either an array of string, in which case the fields will be used for all specified types, or a record specifying + * the search fields per type. + */ + searchFields?: string[] | Record; /** * Use the sort values from the previous page to retrieve the next page of results. */ @@ -371,6 +375,8 @@ export interface SavedObjectsTypeManagementDefinition { visibleInManagement?: boolean; /** * The default search field to use for this type. Defaults to `id`. + * + * @remarks the field must be mapped as `text` and not `keyword` */ defaultSearchField?: string; /** diff --git a/src/plugins/saved_objects_management/server/routes/find.ts b/src/plugins/saved_objects_management/server/routes/find.ts index 45fbd311c2ee0..70db68baabdbf 100644 --- a/src/plugins/saved_objects_management/server/routes/find.ts +++ b/src/plugins/saved_objects_management/server/routes/find.ts @@ -61,19 +61,17 @@ export const registerFindRoute = ( ); const client = getClient({ includedHiddenTypes }); - const searchFields = new Set(); + const searchFields: Record = {}; importAndExportableTypes.forEach((type) => { - const searchField = managementService.getDefaultSearchField(type); - if (searchField) { - searchFields.add(searchField); - } + const searchField = managementService.getDefaultSearchField(type) ?? 'id'; + searchFields[type] = [searchField]; }); const findResponse = await client.find({ ...query, fields: undefined, - searchFields: [...searchFields], + searchFields, }); const enhancedSavedObjects = findResponse.saved_objects From d0bdaa28eaa3daa1d141051eb4dc2a34b75fbbae Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 24 Nov 2021 16:05:07 +0100 Subject: [PATCH 2/5] update generated doc --- .../kibana-plugin-core-public.savedobjectsfindoptions.md | 2 +- ...lugin-core-public.savedobjectsfindoptions.searchfields.md | 4 ++-- .../kibana-plugin-core-server.savedobjectsfindoptions.md | 2 +- ...lugin-core-server.savedobjectsfindoptions.searchfields.md | 4 ++-- ...avedobjectstypemanagementdefinition.defaultsearchfield.md | 5 +++++ src/core/public/public.api.md | 2 +- src/core/server/server.api.md | 2 +- 7 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md index f429911476307..8ef1bf651513c 100644 --- a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md +++ b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md @@ -28,7 +28,7 @@ export interface SavedObjectsFindOptions | [rootSearchFields?](./kibana-plugin-core-public.savedobjectsfindoptions.rootsearchfields.md) | string\[\] | (Optional) The fields to perform the parsed query against. Unlike the searchFields argument, these are expected to be root fields and will not be modified. If used in conjunction with searchFields, both are concatenated together. | | [search?](./kibana-plugin-core-public.savedobjectsfindoptions.search.md) | string | (Optional) Search documents using the Elasticsearch Simple Query String syntax. See Elasticsearch Simple Query String query argument for more information | | [searchAfter?](./kibana-plugin-core-public.savedobjectsfindoptions.searchafter.md) | estypes.Id\[\] | (Optional) Use the sort values from the previous page to retrieve the next page of results. | -| [searchFields?](./kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md) | string\[\] | (Optional) The fields to perform the parsed query against. See Elasticsearch Simple Query String fields argument for more information | +| [searchFields?](./kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md) | string\[\] \| Record<string, string\[\]> | (Optional) The fields to perform the parsed query against. See Elasticsearch Simple Query String fields argument for more information Can be either an array of string, in which case the fields will be used for all specified types, or a record specifying the search fields per type. | | [sortField?](./kibana-plugin-core-public.savedobjectsfindoptions.sortfield.md) | string | (Optional) | | [sortOrder?](./kibana-plugin-core-public.savedobjectsfindoptions.sortorder.md) | estypes.SearchSortOrder | (Optional) | | [type](./kibana-plugin-core-public.savedobjectsfindoptions.type.md) | string \| string\[\] | | diff --git a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md index c99864ac8c046..17a3910462363 100644 --- a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md +++ b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.searchfields.md @@ -4,10 +4,10 @@ ## SavedObjectsFindOptions.searchFields property -The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information +The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information Can be either an array of string, in which case the fields will be used for all specified types, or a record specifying the search fields per type. Signature: ```typescript -searchFields?: string[]; +searchFields?: string[] | Record; ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md index 5f3bb46cc7a99..c7e87aa09bcf0 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md @@ -28,7 +28,7 @@ export interface SavedObjectsFindOptions | [rootSearchFields?](./kibana-plugin-core-server.savedobjectsfindoptions.rootsearchfields.md) | string\[\] | (Optional) The fields to perform the parsed query against. Unlike the searchFields argument, these are expected to be root fields and will not be modified. If used in conjunction with searchFields, both are concatenated together. | | [search?](./kibana-plugin-core-server.savedobjectsfindoptions.search.md) | string | (Optional) Search documents using the Elasticsearch Simple Query String syntax. See Elasticsearch Simple Query String query argument for more information | | [searchAfter?](./kibana-plugin-core-server.savedobjectsfindoptions.searchafter.md) | estypes.Id\[\] | (Optional) Use the sort values from the previous page to retrieve the next page of results. | -| [searchFields?](./kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md) | string\[\] | (Optional) The fields to perform the parsed query against. See Elasticsearch Simple Query String fields argument for more information | +| [searchFields?](./kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md) | string\[\] \| Record<string, string\[\]> | (Optional) The fields to perform the parsed query against. See Elasticsearch Simple Query String fields argument for more information Can be either an array of string, in which case the fields will be used for all specified types, or a record specifying the search fields per type. | | [sortField?](./kibana-plugin-core-server.savedobjectsfindoptions.sortfield.md) | string | (Optional) | | [sortOrder?](./kibana-plugin-core-server.savedobjectsfindoptions.sortorder.md) | estypes.SearchSortOrder | (Optional) | | [type](./kibana-plugin-core-server.savedobjectsfindoptions.type.md) | string \| string\[\] | | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md index ba1152c05eb37..14c71b72885d0 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.searchfields.md @@ -4,10 +4,10 @@ ## SavedObjectsFindOptions.searchFields property -The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information +The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information Can be either an array of string, in which case the fields will be used for all specified types, or a record specifying the search fields per type. Signature: ```typescript -searchFields?: string[]; +searchFields?: string[] | Record; ``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemanagementdefinition.defaultsearchfield.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemanagementdefinition.defaultsearchfield.md index d922a8daaac93..7d168327401b8 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemanagementdefinition.defaultsearchfield.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemanagementdefinition.defaultsearchfield.md @@ -11,3 +11,8 @@ The default search field to use for this type. Defaults to `id`. ```typescript defaultSearchField?: string; ``` + +## Remarks + +the field must be mapped as `text` and not `keyword` + diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 5dcd3422d5d86..ca74cf9e4156b 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -1428,7 +1428,7 @@ export interface SavedObjectsFindOptions { rootSearchFields?: string[]; search?: string; searchAfter?: estypes.Id[]; - searchFields?: string[]; + searchFields?: string[] | Record; // (undocumented) sortField?: string; // (undocumented) diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index fa461946d397f..26cdb01c54538 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2352,7 +2352,7 @@ export interface SavedObjectsFindOptions { rootSearchFields?: string[]; search?: string; searchAfter?: estypes.Id[]; - searchFields?: string[]; + searchFields?: string[] | Record; // (undocumented) sortField?: string; // (undocumented) From d560cb29282b7fa9187d21df2f56295ac1f1b299 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 25 Nov 2021 08:01:53 +0100 Subject: [PATCH 3/5] remove unit test --- .../server/saved_objects/service/lib/repository.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 46a532cdefef4..9af9ac1a18af3 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -3375,14 +3375,6 @@ describe('SavedObjectsRepository', () => { await test({ type: '', namespaces: ['some-ns'], typeToNamespacesMap: new Map() }); }); - it(`throws when searchFields is defined but not an array`, async () => { - await expect( - // @ts-expect-error searchFields is an array - savedObjectsRepository.find({ type, searchFields: 'string' }) - ).rejects.toThrowError('options.searchFields must be an array'); - expect(client.search).not.toHaveBeenCalled(); - }); - it(`throws when fields is defined but not an array`, async () => { // @ts-expect-error fields is an array await expect(savedObjectsRepository.find({ type, fields: 'string' })).rejects.toThrowError( From edf871937720abb4a691afb3277b628bce7a1022 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 25 Nov 2021 08:57:01 +0100 Subject: [PATCH 4/5] fix types --- x-pack/plugins/fleet/server/services/output.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/fleet/server/services/output.test.ts b/x-pack/plugins/fleet/server/services/output.test.ts index 23ee77e0f28c2..7bca064b8f327 100644 --- a/x-pack/plugins/fleet/server/services/output.test.ts +++ b/x-pack/plugins/fleet/server/services/output.test.ts @@ -95,6 +95,7 @@ function getMockedSoClient( if ( options?.defaultOutputMonitoringId && findOptions.searchFields && + Array.isArray(findOptions.searchFields) && findOptions.searchFields.includes('is_default_monitoring') && findOptions.search === 'true' ) { @@ -117,6 +118,7 @@ function getMockedSoClient( if ( options?.defaultOutputId && findOptions.searchFields && + Array.isArray(findOptions.searchFields) && findOptions.searchFields.includes('is_default') && findOptions.search === 'true' ) { From d7e5cb80f370549382921f1b3be1948ebfb79052 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 25 Nov 2021 22:30:18 +0100 Subject: [PATCH 5/5] fix other types --- .../server/lib/timeline/saved_object/timelines/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/timeline/saved_object/timelines/index.ts b/x-pack/plugins/security_solution/server/lib/timeline/saved_object/timelines/index.ts index cc28e0c9eb853..f7555409b95a9 100644 --- a/x-pack/plugins/security_solution/server/lib/timeline/saved_object/timelines/index.ts +++ b/x-pack/plugins/security_solution/server/lib/timeline/saved_object/timelines/index.ts @@ -623,7 +623,7 @@ const getSavedTimeline = async (request: FrameworkRequest, timelineId: string) = const getAllSavedTimeline = async (request: FrameworkRequest, options: SavedObjectsFindOptions) => { const userName = request.user?.username ?? UNAUTHENTICATED_USER; const savedObjectsClient = request.context.core.savedObjects.client; - if (options.searchFields != null && options.searchFields.includes('favorite.keySearch')) { + if (Array.isArray(options.searchFields) && options.searchFields.includes('favorite.keySearch')) { options.search = `${options.search != null ? options.search : ''} ${ userName != null ? convertStringToBase64(userName) : null }`;