From 1ed8b11d457b79bcd954f321314abbcfe652ca2a Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Thu, 26 Oct 2023 10:51:05 -0300 Subject: [PATCH] [Search Source] Update search source to use minimal data view spec for serialization (#169460) ## Summary Currently we use the full data view spec whenever we serialize fields for a search source with an ad hoc data view, except when otherwise specified (currently only when serializing for alert URLs). As a result, whenever we persist a serialized search source containing an ad hoc data view within a saved object, we end up persisting the entire data view field list as well. For data views that target hundreds or thousands of fields, this can result in huge saved objects that exceed Kibana's max payload size, causing issues like the one seen in #168573. Does it ever make sense to persist the full data view spec now that we have a minimal spec (strips all field list data except popularity counts, custom labels, etc.)? I don't think it does, so this PR updates search source to always use the minimal data view spec for ad hoc data views, limiting the impact of large field lists. Barring a good reason not to do this, I think we should make this the standard behaviour. Fixes #168573. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f0bf4d2e9536ed74fb3ea13d75f23c5b305b5817) --- .../search/search_source/search_source.test.ts | 14 +++++++------- .../common/search/search_source/search_source.ts | 6 ++---- .../discover/public/utils/get_sharing_data.ts | 2 +- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/plugins/data/common/search/search_source/search_source.test.ts b/src/plugins/data/common/search/search_source/search_source.test.ts index 8ceaab7e12a18c..4dfd9aea3b6df9 100644 --- a/src/plugins/data/common/search/search_source/search_source.test.ts +++ b/src/plugins/data/common/search/search_source/search_source.test.ts @@ -919,7 +919,7 @@ describe('SearchSource', () => { const localDataView = { id: 'local-123', isPersisted: () => false, - toSpec: () => ({ id: 'local-123' }), + toMinimalSpec: () => ({ id: 'local-123' }), } as DataView; searchSource.setField('index', localDataView); const { searchSourceJSON, references } = searchSource.serialize(); @@ -1029,7 +1029,7 @@ describe('SearchSource', () => { const indexPattern123 = { id: '123', isPersisted: jest.fn(() => true), - toSpec: jest.fn(), + toMinimalSpec: jest.fn(), } as unknown as DataView; test('should return serialized fields', () => { @@ -1038,7 +1038,7 @@ describe('SearchSource', () => { return filter; }); const serializedFields = searchSource.getSerializedFields(); - expect(indexPattern123.toSpec).toHaveBeenCalledTimes(0); + expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(0); expect(serializedFields).toMatchSnapshot(); }); @@ -1048,18 +1048,18 @@ describe('SearchSource', () => { const childSearchSource = searchSource.createChild(); childSearchSource.setField('timeout', '100'); const serializedFields = childSearchSource.getSerializedFields(true); - expect(indexPattern123.toSpec).toHaveBeenCalledTimes(0); + expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(0); expect(serializedFields).toMatchObject({ timeout: '100', parent: { index: '123', from: 123 }, }); }); - test('should use spec', () => { + test('should use minimal spec for ad hoc data view', () => { indexPattern123.isPersisted = jest.fn(() => false); searchSource.setField('index', indexPattern123); - searchSource.getSerializedFields(true, false); - expect(indexPattern123.toSpec).toHaveBeenCalledWith(false); + searchSource.getSerializedFields(true); + expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(1); }); }); diff --git a/src/plugins/data/common/search/search_source/search_source.ts b/src/plugins/data/common/search/search_source/search_source.ts index b42cb7fdf4f250..191cac0cde4a9a 100644 --- a/src/plugins/data/common/search/search_source/search_source.ts +++ b/src/plugins/data/common/search/search_source/search_source.ts @@ -946,7 +946,7 @@ export class SearchSource { /** * serializes search source fields (which can later be passed to {@link ISearchStartSearchSource}) */ - public getSerializedFields(recurse = false, includeFields = true): SerializedSearchSourceFields { + public getSerializedFields(recurse = false): SerializedSearchSourceFields { const { filter: originalFilters, aggs: searchSourceAggs, @@ -961,9 +961,7 @@ export class SearchSource { ...searchSourceFields, }; if (index) { - serializedSearchSourceFields.index = index.isPersisted() - ? index.id - : index.toSpec(includeFields); + serializedSearchSourceFields.index = index.isPersisted() ? index.id : index.toMinimalSpec(); } if (sort) { serializedSearchSourceFields.sort = !Array.isArray(sort) ? [sort] : sort; diff --git a/src/plugins/discover/public/utils/get_sharing_data.ts b/src/plugins/discover/public/utils/get_sharing_data.ts index 7cd6b3821024b1..0e243385272c40 100644 --- a/src/plugins/discover/public/utils/get_sharing_data.ts +++ b/src/plugins/discover/public/utils/get_sharing_data.ts @@ -118,7 +118,7 @@ export async function getSharingData( searchSource.setField('fields', fields); } - return searchSource.getSerializedFields(true, false); + return searchSource.getSerializedFields(true); }, columns, };