diff --git a/src/plugins/discover/public/utils/get_sharing_data.test.ts b/src/plugins/discover/public/utils/get_sharing_data.test.ts index aef9bcff15403..cc37599ef12c0 100644 --- a/src/plugins/discover/public/utils/get_sharing_data.test.ts +++ b/src/plugins/discover/public/utils/get_sharing_data.test.ts @@ -11,7 +11,11 @@ import type { DataView } from 'src/plugins/data/common'; import type { DiscoverServices } from '../build_services'; import { dataPluginMock } from '../../../data/public/mocks'; import { createSearchSourceMock } from '../../../data/common/search/search_source/mocks'; -import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../common'; +import { + DOC_HIDE_TIME_COLUMN_SETTING, + SORT_DEFAULT_ORDER_SETTING, + SEARCH_FIELDS_FROM_SOURCE, +} from '../../common'; import { indexPatternMock } from '../__mocks__/index_pattern'; import { getSharingData, showPublicUrlSwitch } from './get_sharing_data'; @@ -23,6 +27,9 @@ describe('getSharingData', () => { data: dataPluginMock.createStartContract(), uiSettings: { get: (key: string) => { + if (key === SEARCH_FIELDS_FROM_SOURCE) { + return false; + } if (key === SORT_DEFAULT_ORDER_SETTING) { return 'desc'; } @@ -64,6 +71,91 @@ describe('getSharingData', () => { `); }); + test('getSearchSource does not add fields to the searchSource', async () => { + const index = { ...indexPatternMock } as DataView; + index.timeFieldName = 'cool-timefield'; + const searchSourceMock = createSearchSourceMock({ index }); + const { getSearchSource } = await getSharingData(searchSourceMock, {}, services); + expect(getSearchSource()).toMatchInlineSnapshot(` + Object { + "index": "the-index-pattern-id", + "sort": Array [ + Object { + "_doc": "desc", + }, + ], + } + `); + }); + + test(`getSearchSource does not add fields to the searchSource with 'discover:searchFieldsFromSource=true'`, async () => { + const originalGet = services.uiSettings.get; + services.uiSettings = { + get: (key: string, ...args: unknown[]) => { + if (key === SEARCH_FIELDS_FROM_SOURCE) { + return true; + } + return originalGet(key, ...args); + }, + } as unknown as IUiSettingsClient; + const index = { ...indexPatternMock } as DataView; + index.timeFieldName = 'cool-timefield'; + const searchSourceMock = createSearchSourceMock({ index }); + const { getSearchSource } = await getSharingData( + searchSourceMock, + { + columns: [ + 'cool-field-1', + 'cool-field-2', + 'cool-field-3', + 'cool-field-4', + 'cool-field-5', + 'cool-field-6', + ], + }, + services + ); + expect(getSearchSource()).toMatchInlineSnapshot(` + Object { + "index": "the-index-pattern-id", + "sort": Array [ + Object { + "_doc": "desc", + }, + ], + } + `); + }); + + test('getSearchSource does add fields to the searchSource when columns are selected', async () => { + const index = { ...indexPatternMock } as DataView; + index.timeFieldName = 'cool-timefield'; + const searchSourceMock = createSearchSourceMock({ index }); + const { getSearchSource } = await getSharingData( + searchSourceMock, + { + columns: [ + 'cool-field-1', + 'cool-field-2', + 'cool-field-3', + 'cool-field-4', + 'cool-field-5', + 'cool-field-6', + ], + }, + services + ); + expect(getSearchSource().fields).toStrictEqual([ + 'cool-timefield', + 'cool-field-1', + 'cool-field-2', + 'cool-field-3', + 'cool-field-4', + 'cool-field-5', + 'cool-field-6', + ]); + }); + test('fields have prepended timeField', async () => { const index = { ...indexPatternMock } as DataView; index.timeFieldName = 'cool-timefield'; diff --git a/src/plugins/discover/public/utils/get_sharing_data.ts b/src/plugins/discover/public/utils/get_sharing_data.ts index e14ae252da95e..cd00fc5e3c70e 100644 --- a/src/plugins/discover/public/utils/get_sharing_data.ts +++ b/src/plugins/discover/public/utils/get_sharing_data.ts @@ -10,7 +10,11 @@ import type { Capabilities } from 'kibana/public'; import type { IUiSettingsClient } from 'kibana/public'; import type { DataPublicPluginStart } from 'src/plugins/data/public'; import type { Filter, ISearchSource, SerializedSearchSourceFields } from 'src/plugins/data/common'; -import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../common'; +import { + DOC_HIDE_TIME_COLUMN_SETTING, + SEARCH_FIELDS_FROM_SOURCE, + SORT_DEFAULT_ORDER_SETTING, +} from '../../common'; import type { SavedSearch, SortOrder } from '../services/saved_searches'; import { getSortForSearchSource } from '../components/doc_table'; import { AppState } from '../application/main/services/discover_state'; @@ -72,6 +76,15 @@ export async function getSharingData( searchSource.setField('filter', filter); } + /* + * For downstream querying performance, the searchSource object must have fields set. + * Otherwise, the requests will ask for all fields, even if only a few are really needed. + * Discover does not set fields, since having all fields is needed for the UI. + */ + const useFieldsApi = !config.get(SEARCH_FIELDS_FROM_SOURCE); + if (useFieldsApi && columns.length) { + searchSource.setField('fields', columns); + } return searchSource.getSerializedFields(true); }, columns, diff --git a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx index b9bb529e93268..255fb91946a13 100644 --- a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx +++ b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx @@ -75,11 +75,7 @@ export class ReportingCsvPanelAction implements ActionDefinition public async getSearchSource(savedSearch: SavedSearch, _embeddable: ISearchEmbeddable) { const [{ uiSettings }, { data }] = await this.startServices$.pipe(first()).toPromise(); const { getSharingData } = await loadSharingDataHelpers(); - return await getSharingData( - savedSearch.searchSource, - savedSearch, // TODO: get unsaved state (using embeddable.searchScope): https://github.com/elastic/kibana/issues/43977 - { uiSettings, data } - ); + return await getSharingData(savedSearch.searchSource, savedSearch, { uiSettings, data }); } public isCompatible = async (context: ActionContext) => { diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/__snapshots__/generate_csv.test.ts.snap b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/__snapshots__/generate_csv.test.ts.snap index f27b0691e58ea..30f7721afe448 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/__snapshots__/generate_csv.test.ts.snap +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/__snapshots__/generate_csv.test.ts.snap @@ -12,9 +12,9 @@ exports[`fields from job.columns (7.13+ generated) columns can be top-level fiel " `; -exports[`fields from job.columns (7.13+ generated) empty columns defaults to using searchSource.getFields() 1`] = ` -"product -coconut +exports[`fields from job.columns (7.13+ generated) default column names come from tabify 1`] = ` +"\\"_id\\",\\"_index\\",\\"_score\\",category,product +\\"my-cool-id\\",\\"my-cool-index\\",\\"'-\\",\\"cool, rad\\",coconut " `; diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts index 60b4cc05448b8..0f6652943da25 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts @@ -69,21 +69,6 @@ const mockDataClientSearchDefault = jest.fn().mockImplementation( }, }) ); -const mockSearchSourceGetFieldDefault = jest.fn().mockImplementation((key: string) => { - switch (key) { - case 'fields': - return ['date', 'ip', 'message']; - case 'index': - return { - fields: { - getByName: jest.fn().mockImplementation(() => []), - getByType: jest.fn().mockImplementation(() => []), - }, - metaFields: ['_id', '_index', '_type', '_score'], - getFormatterForField: jest.fn(), - }; - } -}); const mockFieldFormatsRegistry = { deserialize: jest @@ -123,14 +108,26 @@ beforeEach(async () => { }) ); - searchSourceMock.getField = mockSearchSourceGetFieldDefault; + searchSourceMock.getField = jest.fn((key: string) => { + switch (key) { + case 'index': + return { + fields: { + getByName: jest.fn(() => []), + getByType: jest.fn(() => []), + }, + metaFields: ['_id', '_index', '_type', '_score'], + getFormatterForField: jest.fn(), + }; + } + }); }); const logger = createMockLevelLogger(); it('formats an empty search result to CSV content', async () => { const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, @@ -170,7 +167,7 @@ it('formats a search result to CSV content', async () => { }) ); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, @@ -193,12 +190,6 @@ it('formats a search result to CSV content', async () => { const HITS_TOTAL = 100; it('calculates the bytes of the content', async () => { - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['message']; - } - return mockSearchSourceGetFieldDefault(key); - }); mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ rawResponse: { @@ -215,7 +206,7 @@ it('calculates the bytes of the content', async () => { ); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['message'] }), mockConfig, { es: mockEsClient, @@ -267,7 +258,7 @@ it('warns if max size was reached', async () => { ); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, @@ -321,7 +312,7 @@ it('uses the scrollId to page all the data', async () => { }); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, @@ -361,12 +352,6 @@ it('uses the scrollId to page all the data', async () => { describe('fields from job.searchSource.getFields() (7.12 generated)', () => { it('cells can be multi-value', async () => { - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['_id', 'sku']; - } - return mockSearchSourceGetFieldDefault(key); - }); mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ rawResponse: { @@ -388,7 +373,7 @@ describe('fields from job.searchSource.getFields() (7.12 generated)', () => { ); const generateCsv = new CsvGenerator( - createMockJob({ searchSource: {} }), + createMockJob({ searchSource: {}, columns: ['_id', 'sku'] }), mockConfig, { es: mockEsClient, @@ -409,12 +394,6 @@ describe('fields from job.searchSource.getFields() (7.12 generated)', () => { }); it('provides top-level underscored fields as columns', async () => { - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['_id', '_index', 'date', 'message']; - } - return mockSearchSourceGetFieldDefault(key); - }); mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ rawResponse: { @@ -445,6 +424,7 @@ describe('fields from job.searchSource.getFields() (7.12 generated)', () => { fields: ['_id', '_index', '@date', 'message'], filter: [], }, + columns: ['_id', '_index', 'date', 'message'], }), mockConfig, { @@ -468,12 +448,6 @@ describe('fields from job.searchSource.getFields() (7.12 generated)', () => { }); it('sorts the fields when they are to be used as table column names', async () => { - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['*']; - } - return mockSearchSourceGetFieldDefault(key); - }); mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ rawResponse: { @@ -620,13 +594,7 @@ describe('fields from job.columns (7.13+ generated)', () => { expect(content).toMatchSnapshot(); }); - it('empty columns defaults to using searchSource.getFields()', async () => { - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['product']; - } - return mockSearchSourceGetFieldDefault(key); - }); + it('default column names come from tabify', async () => { mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ rawResponse: { @@ -694,7 +662,7 @@ describe('formulas', () => { ); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, @@ -736,15 +704,8 @@ describe('formulas', () => { }) ); - searchSourceMock.getField = jest.fn().mockImplementation((key: string) => { - if (key === 'fields') { - return ['date', 'ip', TEST_FORMULA]; - } - return mockSearchSourceGetFieldDefault(key); - }); - const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', TEST_FORMULA] }), mockConfig, { es: mockEsClient, @@ -797,7 +758,7 @@ describe('formulas', () => { ); const generateCsv = new CsvGenerator( - createMockJob({}), + createMockJob({ columns: ['date', 'ip', 'message'] }), mockConfig, { es: mockEsClient, diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index 804fc66c54758..9088423159cf0 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -16,8 +16,6 @@ import type { DataView, ISearchSource, ISearchStartSearchSource, - SearchFieldValue, - SearchSourceFields, } from '../../../../../../../src/plugins/data/common'; import { cellHasFormulas, @@ -50,23 +48,7 @@ interface Dependencies { fieldFormatsRegistry: IFieldFormatsRegistry; } -// Function to check if the field name values can be used as the header row -function isPlainStringArray( - fields: SearchFieldValue[] | string | boolean | undefined -): fields is string[] { - let result = true; - if (Array.isArray(fields)) { - fields.forEach((field) => { - if (typeof field !== 'string' || field === '*' || field === '_source') { - result = false; - } - }); - } - return result; -} - export class CsvGenerator { - private _columns?: string[]; private csvContainsFormulas = false; private maxSizeReached = false; private csvRowCount = 0; @@ -136,36 +118,10 @@ export class CsvGenerator { }; } - private getColumns(searchSource: ISearchSource, table: Datatable) { - if (this._columns != null) { - return this._columns; - } - - // if columns is not provided in job params, - // default to use fields/fieldsFromSource from the searchSource to get the ordering of columns - const getFromSearchSource = (): string[] => { - const fieldValues: Pick = { - fields: searchSource.getField('fields'), - fieldsFromSource: searchSource.getField('fieldsFromSource'), - }; - const fieldSource = fieldValues.fieldsFromSource ? 'fieldsFromSource' : 'fields'; - this.logger.debug(`Getting columns from '${fieldSource}' in search source.`); - - const fields = fieldValues[fieldSource]; - // Check if field name values are string[] and if the fields are user-defined - if (isPlainStringArray(fields)) { - return fields; - } - - // Default to using the table column IDs as the fields - const columnIds = table.columns.map((c) => c.id); - // Fields in the API response don't come sorted - they need to be sorted client-side - columnIds.sort(); - return columnIds; - }; - this._columns = this.job.columns?.length ? this.job.columns : getFromSearchSource(); - - return this._columns; + private getColumnsFromTabify(table: Datatable) { + const columnIds = table.columns.map((c) => c.id); + columnIds.sort(); + return columnIds; } private formatCellValues(formatters: Record) { @@ -379,9 +335,12 @@ export class CsvGenerator { break; } - // If columns exists in the job params, use it to order the CSV columns - // otherwise, get the ordering from the searchSource's fields / fieldsFromSource - const columns = this.getColumns(searchSource, table) || []; + let columns: string[]; + if (this.job.columns && this.job.columns.length > 0) { + columns = this.job.columns; + } else { + columns = this.getColumnsFromTabify(table); + } if (first) { first = false; diff --git a/x-pack/test/reporting_api_integration/reporting_and_security/download_csv_dashboard.ts b/x-pack/test/reporting_api_integration/reporting_and_security/download_csv_dashboard.ts index 9e99f5886894e..6694b5299cd67 100644 --- a/x-pack/test/reporting_api_integration/reporting_and_security/download_csv_dashboard.ts +++ b/x-pack/test/reporting_api_integration/reporting_and_security/download_csv_dashboard.ts @@ -233,6 +233,7 @@ export default function ({ getService }: FtrProviderContext) { query: { language: 'kuery', query: '' }, sort: [{ '@timestamp': 'desc' }], }, + columns: ['@timestamp', 'clientip', 'extension'], }) )) as supertest.Response; const { status: resStatus, text: resText, type: resType } = res; @@ -271,6 +272,7 @@ export default function ({ getService }: FtrProviderContext) { query: { language: 'kuery', query: '' }, sort: [{ '@timestamp': 'desc' }], }, + columns: ['@timestamp', 'clientip', 'extension'], }) )) as supertest.Response; const { status: resStatus, text: resText, type: resType } = res; @@ -301,6 +303,7 @@ export default function ({ getService }: FtrProviderContext) { fields: ['date', 'message'], filter: [], }, + columns: ['date', 'message'], }) ); const { status: resStatus, text: resText, type: resType } = res; @@ -322,6 +325,7 @@ export default function ({ getService }: FtrProviderContext) { fields: ['date', 'message'], filter: [], }, + columns: ['date', 'message'], }) ); const { status: resStatus, text: resText, type: resType } = res; @@ -378,6 +382,7 @@ export default function ({ getService }: FtrProviderContext) { }, ], }, + columns: ['name', 'power'], }) )) as supertest.Response;