From 3712e36e9461c40130928e16638bff36abfe88c3 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Tue, 1 Dec 2020 12:10:38 +0300 Subject: [PATCH] [Vega] Filter bar in Vega is not usable with non default index pattern. (#84090) * [Vega] Filtering is not working Closes: #81738 * fix CI * some work * some work * add tests for extract_index_pattern * simplify extract_index_pattern * fix type error * cleanup * Update vega_base_view.js * Update extract_index_pattern.test.ts * fix PR comments * fix some comments * findByTitle -> getByTitle * remove getByTitle * fix vega_base_view * fix jest * allow to set multiple indexes from top_nav Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/vis_type_vega/public/vega_type.ts # src/plugins/vis_type_vega/public/vega_visualization.test.js # src/plugins/vis_type_vega/public/vega_visualization.ts --- .../data/common/index_patterns/utils.ts | 34 ++--- src/plugins/data/public/mocks.ts | 1 + .../public/data_model/es_query_parser.ts | 1 + .../vis_type_vega/public/data_model/types.ts | 5 +- .../public/data_model/vega_parser.test.js | 6 +- .../public/lib/extract_index_pattern.test.ts | 125 ++++++++++++++++++ .../public/lib/extract_index_pattern.ts | 47 +++++++ src/plugins/vis_type_vega/public/plugin.ts | 2 - src/plugins/vis_type_vega/public/services.ts | 11 +- src/plugins/vis_type_vega/public/vega_type.ts | 14 ++ .../public/vega_view/vega_base_view.js | 50 ++++++- .../public/vega_visualization.js | 37 +----- .../public/vega_visualization.test.js | 3 +- .../components/visualize_top_nav.tsx | 35 +++-- .../translations/translations/ja-JP.json | 4 +- .../translations/translations/zh-CN.json | 4 +- 16 files changed, 285 insertions(+), 94 deletions(-) create mode 100644 src/plugins/vis_type_vega/public/lib/extract_index_pattern.test.ts create mode 100644 src/plugins/vis_type_vega/public/lib/extract_index_pattern.ts diff --git a/src/plugins/data/common/index_patterns/utils.ts b/src/plugins/data/common/index_patterns/utils.ts index d9e1cfa0d952af..b7e1f28d5d60f3 100644 --- a/src/plugins/data/common/index_patterns/utils.ts +++ b/src/plugins/data/common/index_patterns/utils.ts @@ -17,8 +17,8 @@ * under the License. */ -import { find } from 'lodash'; -import { SavedObjectsClientCommon, SavedObject } from '..'; +import type { IndexPatternSavedObjectAttrs } from './index_patterns'; +import type { SavedObjectsClientCommon } from '../types'; /** * Returns an object matching a given title @@ -27,24 +27,16 @@ import { SavedObjectsClientCommon, SavedObject } from '..'; * @param title {string} * @returns {Promise} */ -export async function findByTitle( - client: SavedObjectsClientCommon, - title: string -): Promise | void> { - if (!title) { - return Promise.resolve(); - } - - const savedObjects = await client.find({ - type: 'index-pattern', - perPage: 10, - search: `"${title}"`, - searchFields: ['title'], - fields: ['title'], - }); +export async function findByTitle(client: SavedObjectsClientCommon, title: string) { + if (title) { + const savedObjects = await client.find({ + type: 'index-pattern', + perPage: 10, + search: `"${title}"`, + searchFields: ['title'], + fields: ['title'], + }); - return find( - savedObjects, - (obj: SavedObject) => obj.attributes.title.toLowerCase() === title.toLowerCase() - ); + return savedObjects.find((obj) => obj.attributes.title.toLowerCase() === title.toLowerCase()); + } } diff --git a/src/plugins/data/public/mocks.ts b/src/plugins/data/public/mocks.ts index 1b83eb569b1a1c..67c1ff7e09dd74 100644 --- a/src/plugins/data/public/mocks.ts +++ b/src/plugins/data/public/mocks.ts @@ -64,6 +64,7 @@ const createStartContract = (): Start => { SearchBar: jest.fn().mockReturnValue(null), }, indexPatterns: ({ + find: jest.fn((search) => [{ id: search, title: search }]), createField: jest.fn(() => {}), createFieldList: jest.fn(() => []), ensureDefaultIndexPattern: jest.fn(), diff --git a/src/plugins/vis_type_vega/public/data_model/es_query_parser.ts b/src/plugins/vis_type_vega/public/data_model/es_query_parser.ts index 1aac8e25d5c738..79eb049fb6dcce 100644 --- a/src/plugins/vis_type_vega/public/data_model/es_query_parser.ts +++ b/src/plugins/vis_type_vega/public/data_model/es_query_parser.ts @@ -226,6 +226,7 @@ export class EsQueryParser { const requestObject = requests.find((item) => getRequestName(item, index) === data.name); if (requestObject) { + requestObject.dataObject.url = requestObject.url; requestObject.dataObject.values = data.rawResponse; } }); diff --git a/src/plugins/vis_type_vega/public/data_model/types.ts b/src/plugins/vis_type_vega/public/data_model/types.ts index 11bdf4f0640236..71b03e69fd92c8 100644 --- a/src/plugins/vis_type_vega/public/data_model/types.ts +++ b/src/plugins/vis_type_vega/public/data_model/types.ts @@ -82,8 +82,9 @@ interface Projection { name: string; } -interface RequestDataObject { +interface RequestDataObject { name?: string; + url?: TUrlData; values: SearchResponse; } @@ -186,7 +187,7 @@ export interface CacheBounds { max: number; } -interface Requests { +interface Requests> { url: TUrlData; name: string; dataObject: TRequestDataObject; diff --git a/src/plugins/vis_type_vega/public/data_model/vega_parser.test.js b/src/plugins/vis_type_vega/public/data_model/vega_parser.test.js index c9f8e0a4394ec4..9cdc6cd5130960 100644 --- a/src/plugins/vis_type_vega/public/data_model/vega_parser.test.js +++ b/src/plugins/vis_type_vega/public/data_model/vega_parser.test.js @@ -185,21 +185,21 @@ describe('VegaParser._resolveEsQueries', () => { 'es', check( { data: { name: 'requestId', url: { index: 'a' }, x: 1 } }, - { data: { name: 'requestId', values: [42], x: 1 } } + { data: { name: 'requestId', url: { index: 'a', body: {} }, values: [42], x: 1 } } ) ); test( 'es 2', check( { data: { name: 'requestId', url: { '%type%': 'elasticsearch', index: 'a' } } }, - { data: { name: 'requestId', values: [42] } } + { data: { name: 'requestId', url: { index: 'a', body: {} }, values: [42] } } ) ); test( 'es arr', check( { arr: [{ data: { name: 'requestId', url: { index: 'a' }, x: 1 } }] }, - { arr: [{ data: { name: 'requestId', values: [42], x: 1 } }] } + { arr: [{ data: { name: 'requestId', url: { index: 'a', body: {} }, values: [42], x: 1 } }] } ) ); test( diff --git a/src/plugins/vis_type_vega/public/lib/extract_index_pattern.test.ts b/src/plugins/vis_type_vega/public/lib/extract_index_pattern.test.ts new file mode 100644 index 00000000000000..a13428d539ad98 --- /dev/null +++ b/src/plugins/vis_type_vega/public/lib/extract_index_pattern.test.ts @@ -0,0 +1,125 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { dataPluginMock } from '../../../data/public/mocks'; +import { extractIndexPatternsFromSpec } from './extract_index_pattern'; +import { setData } from '../services'; + +import type { VegaSpec } from '../data_model/types'; + +const getMockedSpec = (mockedObj: any) => (mockedObj as unknown) as VegaSpec; + +describe('extractIndexPatternsFromSpec', () => { + const dataStart = dataPluginMock.createStartContract(); + + beforeAll(() => { + setData(dataStart); + }); + + test('should not throw errors if no index is specified', async () => { + const spec = getMockedSpec({ + data: {}, + }); + + const indexes = await extractIndexPatternsFromSpec(spec); + + expect(indexes).toMatchInlineSnapshot(`Array []`); + }); + + test('should extract single index pattern', async () => { + const spec = getMockedSpec({ + data: { + url: { + index: 'test', + }, + }, + }); + + const indexes = await extractIndexPatternsFromSpec(spec); + + expect(indexes).toMatchInlineSnapshot(` + Array [ + Object { + "id": "test", + "title": "test", + }, + ] + `); + }); + + test('should extract multiple index patterns', async () => { + const spec = getMockedSpec({ + data: [ + { + url: { + index: 'test1', + }, + }, + { + url: { + index: 'test2', + }, + }, + ], + }); + + const indexes = await extractIndexPatternsFromSpec(spec); + + expect(indexes).toMatchInlineSnapshot(` + Array [ + Object { + "id": "test1", + "title": "test1", + }, + Object { + "id": "test2", + "title": "test2", + }, + ] + `); + }); + + test('should filter empty values', async () => { + const spec = getMockedSpec({ + data: [ + { + url: { + wrong: 'wrong', + }, + }, + { + url: { + index: 'ok', + }, + }, + ], + }); + + const indexes = await extractIndexPatternsFromSpec(spec); + + expect(indexes).toMatchInlineSnapshot(` + Array [ + Object { + "id": "ok", + "title": "ok", + }, + ] + `); + }); +}); diff --git a/src/plugins/vis_type_vega/public/lib/extract_index_pattern.ts b/src/plugins/vis_type_vega/public/lib/extract_index_pattern.ts new file mode 100644 index 00000000000000..12cbd6f7ebbfaa --- /dev/null +++ b/src/plugins/vis_type_vega/public/lib/extract_index_pattern.ts @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { flatten } from 'lodash'; +import { getData } from '../services'; + +import type { Data, VegaSpec } from '../data_model/types'; +import type { IndexPattern } from '../../../data/public'; + +export const extractIndexPatternsFromSpec = async (spec: VegaSpec) => { + const { indexPatterns } = getData(); + let data: Data[] = []; + + if (Array.isArray(spec.data)) { + data = spec.data; + } else if (spec.data) { + data = [spec.data]; + } + + return flatten( + await Promise.all( + data.reduce>>((accumulator, currentValue) => { + if (currentValue.url?.index) { + accumulator.push(indexPatterns.find(currentValue.url.index)); + } + + return accumulator; + }, []) + ) + ); +}; diff --git a/src/plugins/vis_type_vega/public/plugin.ts b/src/plugins/vis_type_vega/public/plugin.ts index ce5c5130961c6b..5850f4fab5f8c9 100644 --- a/src/plugins/vis_type_vega/public/plugin.ts +++ b/src/plugins/vis_type_vega/public/plugin.ts @@ -25,7 +25,6 @@ import { Setup as InspectorSetup } from '../../inspector/public'; import { setNotifications, setData, - setSavedObjects, setInjectedVars, setUISettings, setMapsLegacyConfig, @@ -99,7 +98,6 @@ export class VegaPlugin implements Plugin, void> { public start(core: CoreStart, { data }: VegaPluginStartDependencies) { setNotifications(core.notifications); - setSavedObjects(core.savedObjects); setData(data); setInjectedMetadata(core.injectedMetadata); } diff --git a/src/plugins/vis_type_vega/public/services.ts b/src/plugins/vis_type_vega/public/services.ts index 455fe67dbc4233..43856c83248477 100644 --- a/src/plugins/vis_type_vega/public/services.ts +++ b/src/plugins/vis_type_vega/public/services.ts @@ -17,12 +17,7 @@ * under the License. */ -import { - CoreStart, - SavedObjectsStart, - NotificationsStart, - IUiSettingsClient, -} from 'src/core/public'; +import { CoreStart, NotificationsStart, IUiSettingsClient } from 'src/core/public'; import { DataPublicPluginStart } from '../../data/public'; import { createGetterSetter } from '../../kibana_utils/public'; @@ -40,10 +35,6 @@ export const [getInjectedMetadata, setInjectedMetadata] = createGetterSetter< CoreStart['injectedMetadata'] >('InjectedMetadata'); -export const [getSavedObjects, setSavedObjects] = createGetterSetter( - 'SavedObjects' -); - export const [getInjectedVars, setInjectedVars] = createGetterSetter<{ enableExternalUrls: boolean; emsTileLayerId: unknown; diff --git a/src/plugins/vis_type_vega/public/vega_type.ts b/src/plugins/vis_type_vega/public/vega_type.ts index 0496f765e5e99b..f90580cb013fe7 100644 --- a/src/plugins/vis_type_vega/public/vega_type.ts +++ b/src/plugins/vis_type_vega/public/vega_type.ts @@ -18,11 +18,15 @@ */ import { i18n } from '@kbn/i18n'; +import { parse } from 'hjson'; import { BaseVisTypeOptions } from 'src/plugins/visualizations/public'; import { DefaultEditorSize } from '../../vis_default_editor/public'; import { VegaVisualizationDependencies } from './plugin'; import { VegaVisEditor } from './components'; +import type { VegaSpec } from './data_model/types'; + +import { extractIndexPatternsFromSpec } from './lib/extract_index_pattern'; import { createVegaRequestHandler } from './vega_request_handler'; // @ts-expect-error import { createVegaVisualization } from './vega_visualization'; @@ -64,6 +68,16 @@ export const createVegaTypeDefinition = ( getSupportedTriggers: () => { return [VIS_EVENT_TO_TRIGGER.applyFilter]; }, + getUsedIndexPattern: async (visParams) => { + try { + const spec = parse(visParams.spec, { legacyRoot: false, keepWsc: true }); + + return extractIndexPatternsFromSpec(spec as VegaSpec); + } catch (e) { + // spec is invalid + } + return []; + }, inspectorAdapters: createInspectorAdapters, }; }; diff --git a/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js b/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js index 979432b2aed2a4..7d74da40c2b05d 100644 --- a/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js +++ b/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js @@ -27,7 +27,8 @@ import { i18n } from '@kbn/i18n'; import { TooltipHandler } from './vega_tooltip'; import { esFilters } from '../../../data/public'; -import { getEnableExternalUrls } from '../services'; +import { getEnableExternalUrls, getData } from '../services'; +import { extractIndexPatternsFromSpec } from '../lib/extract_index_pattern'; vega.scheme('elastic', euiPaletteColorBlind()); @@ -65,7 +66,6 @@ export class VegaBaseView { this._filterManager = opts.filterManager; this._applyFilter = opts.applyFilter; this._timefilter = opts.timefilter; - this._findIndex = opts.findIndex; this._view = null; this._vegaViewConfig = null; this._$messages = null; @@ -127,6 +127,48 @@ export class VegaBaseView { } } + /** + * Find index pattern by its title, if not given, gets it from spec or a defaults one + * @param {string} [index] + * @returns {Promise} index id + */ + async findIndex(index) { + const { indexPatterns } = getData(); + let idxObj; + + if (index) { + [idxObj] = await indexPatterns.find(index); + if (!idxObj) { + throw new Error( + i18n.translate('visTypeVega.vegaParser.baseView.indexNotFoundErrorMessage', { + defaultMessage: 'Index {index} not found', + values: { index: `"${index}"` }, + }) + ); + } + } else { + [idxObj] = await extractIndexPatternsFromSpec( + this._parser.isVegaLite ? this._parser.vlspec : this._parser.spec + ); + + if (!idxObj) { + const defaultIdx = await indexPatterns.getDefault(); + + if (defaultIdx) { + idxObj = defaultIdx; + } else { + throw new Error( + i18n.translate('visTypeVega.vegaParser.baseView.unableToFindDefaultIndexErrorMessage', { + defaultMessage: 'Unable to find default index', + }) + ); + } + } + } + + return idxObj.id; + } + createViewConfig() { const config = { // eslint-disable-next-line import/namespace @@ -261,7 +303,7 @@ export class VegaBaseView { * @param {string} [index] as defined in Kibana, or default if missing */ async addFilterHandler(query, index) { - const indexId = await this._findIndex(index); + const indexId = await this.findIndex(index); const filter = esFilters.buildQueryFilter(query, indexId); this._applyFilter({ filters: [filter] }); @@ -272,7 +314,7 @@ export class VegaBaseView { * @param {string} [index] as defined in Kibana, or default if missing */ async removeFilterHandler(query, index) { - const indexId = await this._findIndex(index); + const indexId = await this.findIndex(index); const filterToRemove = esFilters.buildQueryFilter(query, indexId); const currentFilters = this._filterManager.getFilters(); diff --git a/src/plugins/vis_type_vega/public/vega_visualization.js b/src/plugins/vis_type_vega/public/vega_visualization.js index 2d58e9cda60cdc..7782d4915dce97 100644 --- a/src/plugins/vis_type_vega/public/vega_visualization.js +++ b/src/plugins/vis_type_vega/public/vega_visualization.js @@ -17,50 +17,16 @@ * under the License. */ import { i18n } from '@kbn/i18n'; -import { getNotifications, getData, getSavedObjects } from './services'; +import { getNotifications, getData } from './services'; export const createVegaVisualization = ({ getServiceSettings }) => class VegaVisualization { constructor(el, vis) { this._el = el; this._vis = vis; - - this.savedObjectsClient = getSavedObjects(); this.dataPlugin = getData(); } - /** - * Find index pattern by its title, of if not given, gets default - * @param {string} [index] - * @returns {Promise} index id - */ - async findIndex(index) { - const { indexPatterns } = this.dataPlugin; - let idxObj; - - if (index) { - idxObj = indexPatterns.findByTitle(this.savedObjectsClient, index); - if (!idxObj) { - throw new Error( - i18n.translate('visTypeVega.visualization.indexNotFoundErrorMessage', { - defaultMessage: 'Index {index} not found', - values: { index: `"${index}"` }, - }) - ); - } - } else { - idxObj = await indexPatterns.getDefault(); - if (!idxObj) { - throw new Error( - i18n.translate('visTypeVega.visualization.unableToFindDefaultIndexErrorMessage', { - defaultMessage: 'Unable to find default index', - }) - ); - } - } - return idxObj.id; - } - /** * * @param {VegaParser} visData @@ -112,7 +78,6 @@ export const createVegaVisualization = ({ getServiceSettings }) => serviceSettings, filterManager, timefilter, - findIndex: this.findIndex.bind(this), }; if (vegaParser.useMap) { diff --git a/src/plugins/vis_type_vega/public/vega_visualization.test.js b/src/plugins/vis_type_vega/public/vega_visualization.test.js index dcf1722768075a..2a0976bd726e06 100644 --- a/src/plugins/vis_type_vega/public/vega_visualization.test.js +++ b/src/plugins/vis_type_vega/public/vega_visualization.test.js @@ -32,7 +32,7 @@ import { SearchAPI } from './data_model/search_api'; import { createVegaTypeDefinition } from './vega_type'; -import { setInjectedVars, setData, setSavedObjects, setNotifications } from './services'; +import { setInjectedVars, setData, setNotifications } from './services'; import { coreMock } from '../../../core/public/mocks'; import { dataPluginMock } from '../../data/public/mocks'; @@ -80,7 +80,6 @@ describe('VegaVisualizations', () => { enableExternalUrls: true, }); setData(dataPluginStart); - setSavedObjects(coreStart.savedObjects); setNotifications(coreStart.notifications); vegaVisualizationDependencies = { diff --git a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx index b207529c456a14..4b328801361465 100644 --- a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx +++ b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx @@ -20,7 +20,6 @@ import React, { memo, useCallback, useMemo, useState, useEffect } from 'react'; import { AppMountParameters, OverlayRef } from 'kibana/public'; -import _ from 'lodash'; import { i18n } from '@kbn/i18n'; import { useKibana } from '../../../../kibana_react/public'; import { @@ -31,6 +30,7 @@ import { } from '../types'; import { APP_NAME } from '../visualize_constants'; import { getTopNavConfig } from '../utils'; +import type { IndexPattern } from '../../../../data/public'; interface VisualizeTopNavProps { currentAppState: VisualizeAppState; @@ -118,7 +118,9 @@ const TopNav = ({ stateTransfer, onAppLeave, ]); - const [indexPattern, setIndexPattern] = useState(vis.data.indexPattern); + const [indexPatterns, setIndexPatterns] = useState( + vis.data.indexPattern ? [vis.data.indexPattern] : [] + ); const showDatePicker = () => { // tsvb loads without an indexPattern initially (TODO investigate). // hide timefilter only if timeFieldName is explicitly undefined. @@ -165,14 +167,27 @@ const TopNav = ({ ]); useEffect(() => { - if (!vis.data.indexPattern) { - services.data.indexPatterns.getDefault().then((index) => { - if (index) { - setIndexPattern(index); + const asyncSetIndexPattern = async () => { + let indexes: IndexPattern[] | undefined; + + if (vis.type.getUsedIndexPattern) { + indexes = await vis.type.getUsedIndexPattern(vis.params); + } + if (!indexes || !indexes.length) { + const defaultIndex = await services.data.indexPatterns.getDefault(); + if (defaultIndex) { + indexes = [defaultIndex]; } - }); + } + if (indexes) { + setIndexPatterns(indexes); + } + }; + + if (!vis.data.indexPattern) { + asyncSetIndexPattern(); } - }, [services.data.indexPatterns, vis.data.indexPattern]); + }, [vis.params, vis.type, services.data.indexPatterns, vis.data.indexPattern]); return isChromeVisible ? ( /** @@ -189,7 +204,7 @@ const TopNav = ({ onQuerySubmit={handleRefresh} savedQueryId={currentAppState.savedQuery} onSavedQueryIdChange={stateContainer.transitions.updateSavedQuery} - indexPatterns={indexPattern ? [indexPattern] : undefined} + indexPatterns={indexPatterns} screenTitle={vis.title} showAutoRefreshOnly={!showDatePicker()} showDatePicker={showDatePicker()} @@ -207,7 +222,7 @@ const TopNav = ({