From 720021bdce142386bd17675f5c7b84299efc69f9 Mon Sep 17 00:00:00 2001 From: Diana Derevyankina <54894989+DziyanaDzeraviankina@users.noreply.github.com> Date: Thu, 8 Oct 2020 13:35:35 +0300 Subject: [PATCH] Input controls crashes if index pattern is not available (#79431) * Input controls crashes if index pattern is not available Closes #72664 * Replace index pattern null type with undefined * Move index pattern checks into FilterManager init function * Rename indexPatternService to indexPatternsService to match other variables names --- .../public/control/control.ts | 2 +- .../filter_manager/filter_manager.test.ts | 15 ++++++++-- .../control/filter_manager/filter_manager.ts | 30 +++++++++++++++---- .../phrase_filter_manager.test.ts | 26 +++++++++++----- .../filter_manager/phrase_filter_manager.ts | 14 +++++---- .../range_filter_manager.test.ts | 20 +++++++++---- .../filter_manager/range_filter_manager.ts | 5 ++-- .../public/control/list_control_factory.ts | 24 ++++++++------- .../public/control/range_control_factory.ts | 8 +++-- 9 files changed, 100 insertions(+), 44 deletions(-) diff --git a/src/plugins/input_control_vis/public/control/control.ts b/src/plugins/input_control_vis/public/control/control.ts index da2dc7bab7cf7a..303b340e5905b5 100644 --- a/src/plugins/input_control_vis/public/control/control.ts +++ b/src/plugins/input_control_vis/public/control/control.ts @@ -83,7 +83,7 @@ export abstract class Control { format = (value: any) => { const indexPattern = this.filterManager.getIndexPattern(); const field = this.filterManager.getField(); - if (field) { + if (field && indexPattern) { return indexPattern.getFormatterForField(field).convert(value); } diff --git a/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.test.ts b/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.test.ts index e0eeddf93f67e3..141f879f248a9c 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.test.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.test.ts @@ -21,7 +21,11 @@ import expect from '@kbn/expect'; import { FilterManager } from './filter_manager'; import { coreMock } from '../../../../../core/public/mocks'; -import { Filter, IndexPattern, FilterManager as QueryFilterManager } from '../../../../data/public'; +import { + Filter, + FilterManager as QueryFilterManager, + IndexPatternsContract, +} from '../../../../data/public'; const setupMock = coreMock.createSetup(); @@ -39,7 +43,6 @@ describe('FilterManager', function () { const controlId = 'control1'; describe('findFilters', function () { - const indexPatternMock = {} as IndexPattern; let kbnFilters: Filter[]; const queryFilterMock = new QueryFilterManager(setupMock.uiSettings); queryFilterMock.getAppFilters = () => kbnFilters; @@ -48,7 +51,13 @@ describe('FilterManager', function () { let filterManager: FilterManagerTest; beforeEach(() => { kbnFilters = []; - filterManager = new FilterManagerTest(controlId, 'field1', indexPatternMock, queryFilterMock); + filterManager = new FilterManagerTest( + controlId, + 'field1', + '1', + {} as IndexPatternsContract, + queryFilterMock + ); }); test('should not find filters that are not controlled by any visualization', function () { diff --git a/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.ts b/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.ts index ece3f7a88ba37c..de948ccda86639 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/filter_manager.ts @@ -19,14 +19,22 @@ import _ from 'lodash'; -import { FilterManager as QueryFilterManager, IndexPattern, Filter } from '../../../../data/public'; +import { + FilterManager as QueryFilterManager, + IndexPattern, + Filter, + IndexPatternsContract, +} from '../../../../data/public'; export abstract class FilterManager { + protected indexPattern: IndexPattern | undefined; + constructor( public controlId: string, public fieldName: string, - public indexPattern: IndexPattern, - public queryFilter: QueryFilterManager + private indexPatternId: string, + private indexPatternsService: IndexPatternsContract, + protected queryFilter: QueryFilterManager ) {} /** @@ -41,12 +49,22 @@ export abstract class FilterManager { abstract getValueFromFilterBar(): any; - getIndexPattern(): IndexPattern { + async init() { + try { + if (!this.indexPattern) { + this.indexPattern = await this.indexPatternsService.get(this.indexPatternId); + } + } catch (e) { + // invalid index pattern id + } + } + + getIndexPattern(): IndexPattern | undefined { return this.indexPattern; } getField() { - return this.indexPattern.fields.getByName(this.fieldName); + return this.indexPattern?.fields.getByName(this.fieldName); } findFilters(): Filter[] { @@ -54,7 +72,7 @@ export abstract class FilterManager { this.queryFilter.getAppFilters(), this.queryFilter.getGlobalFilters(), ]); - return kbnFilters.filter((kbnFilter) => { + return kbnFilters.filter((kbnFilter: Filter) => { return _.get(kbnFilter, 'meta.controlledBy') === this.controlId; }); } diff --git a/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.ts b/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.ts index ec1f9d42aa67b3..066d87230f9091 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.ts @@ -19,7 +19,12 @@ import expect from '@kbn/expect'; -import { Filter, IndexPattern, FilterManager as QueryFilterManager } from '../../../../data/public'; +import { + Filter, + IndexPattern, + FilterManager as QueryFilterManager, + IndexPatternsContract, +} from '../../../../data/public'; import { PhraseFilterManager } from './phrase_filter_manager'; describe('PhraseFilterManager', function () { @@ -42,15 +47,20 @@ describe('PhraseFilterManager', function () { }, }, } as IndexPattern; + const indexPatternsServiceMock = ({ + get: jest.fn().mockReturnValue(Promise.resolve(indexPatternMock)), + } as unknown) as jest.Mocked; const queryFilterMock: QueryFilterManager = {} as QueryFilterManager; let filterManager: PhraseFilterManager; - beforeEach(() => { + beforeEach(async () => { filterManager = new PhraseFilterManager( controlId, 'field1', - indexPatternMock, + '1', + indexPatternsServiceMock, queryFilterMock ); + await filterManager.init(); }); test('should create match phrase filter from single value', function () { @@ -89,10 +99,11 @@ describe('PhraseFilterManager', function () { constructor( id: string, fieldName: string, - indexPattern: IndexPattern, + indexPatternId: string, + indexPatternsService: IndexPatternsContract, queryFilter: QueryFilterManager ) { - super(id, fieldName, indexPattern, queryFilter); + super(id, fieldName, indexPatternId, indexPatternsService, queryFilter); this.mockFilters = []; } @@ -105,14 +116,15 @@ describe('PhraseFilterManager', function () { } } - const indexPatternMock: IndexPattern = {} as IndexPattern; + const indexPatternsServiceMock = {} as IndexPatternsContract; const queryFilterMock: QueryFilterManager = {} as QueryFilterManager; let filterManager: MockFindFiltersPhraseFilterManager; beforeEach(() => { filterManager = new MockFindFiltersPhraseFilterManager( controlId, 'field1', - indexPatternMock, + '1', + indexPatternsServiceMock, queryFilterMock ); }); diff --git a/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.ts b/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.ts index 03ed6c5520decd..f3a2b4085b923a 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.ts @@ -23,7 +23,7 @@ import { FilterManager } from './filter_manager'; import { PhraseFilter, esFilters, - IndexPattern, + IndexPatternsContract, FilterManager as QueryFilterManager, } from '../../../../data/public'; @@ -31,24 +31,26 @@ export class PhraseFilterManager extends FilterManager { constructor( controlId: string, fieldName: string, - indexPattern: IndexPattern, + indexPatternId: string, + indexPatternsService: IndexPatternsContract, queryFilter: QueryFilterManager ) { - super(controlId, fieldName, indexPattern, queryFilter); + super(controlId, fieldName, indexPatternId, indexPatternsService, queryFilter); } createFilter(phrases: any): PhraseFilter { + const indexPattern = this.getIndexPattern()!; let newFilter: PhraseFilter; - const value = this.indexPattern.fields.getByName(this.fieldName); + const value = indexPattern.fields.getByName(this.fieldName); if (!value) { throw new Error(`Unable to find field with name: ${this.fieldName} on indexPattern`); } if (phrases.length === 1) { - newFilter = esFilters.buildPhraseFilter(value, phrases[0], this.indexPattern); + newFilter = esFilters.buildPhraseFilter(value, phrases[0], indexPattern); } else { - newFilter = esFilters.buildPhrasesFilter(value, phrases, this.indexPattern); + newFilter = esFilters.buildPhrasesFilter(value, phrases, indexPattern); } newFilter.meta.key = this.fieldName; diff --git a/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.test.ts b/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.test.ts index 8556312d10f4ab..2a665670060495 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.test.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.test.ts @@ -25,6 +25,7 @@ import { RangeFilterMeta, IndexPattern, FilterManager as QueryFilterManager, + IndexPatternsContract, } from '../../../../data/public'; describe('RangeFilterManager', function () { @@ -46,15 +47,20 @@ describe('RangeFilterManager', function () { }, }, } as IndexPattern; + const indexPatternsServiceMock = ({ + get: jest.fn().mockReturnValue(Promise.resolve(indexPatternMock)), + } as unknown) as jest.Mocked; const queryFilterMock: QueryFilterManager = {} as QueryFilterManager; let filterManager: RangeFilterManager; - beforeEach(() => { + beforeEach(async () => { filterManager = new RangeFilterManager( controlId, 'field1', - indexPatternMock, + '1', + indexPatternsServiceMock, queryFilterMock ); + await filterManager.init(); }); test('should create range filter from slider value', function () { @@ -75,10 +81,11 @@ describe('RangeFilterManager', function () { constructor( id: string, fieldName: string, - indexPattern: IndexPattern, + indexPatternId: string, + indexPatternsService: IndexPatternsContract, queryFilter: QueryFilterManager ) { - super(id, fieldName, indexPattern, queryFilter); + super(id, fieldName, indexPatternId, indexPatternsService, queryFilter); this.mockFilters = []; } @@ -91,14 +98,15 @@ describe('RangeFilterManager', function () { } } - const indexPatternMock: IndexPattern = {} as IndexPattern; + const indexPatternsServiceMock = {} as IndexPatternsContract; const queryFilterMock: QueryFilterManager = {} as QueryFilterManager; let filterManager: MockFindFiltersRangeFilterManager; beforeEach(() => { filterManager = new MockFindFiltersRangeFilterManager( controlId, 'field1', - indexPatternMock, + '1', + indexPatternsServiceMock, queryFilterMock ); }); diff --git a/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.ts b/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.ts index 1a884cf267c411..39e4bfa19e5dcb 100644 --- a/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.ts +++ b/src/plugins/input_control_vis/public/control/filter_manager/range_filter_manager.ts @@ -61,11 +61,12 @@ export class RangeFilterManager extends FilterManager { * @return {object} range filter */ createFilter(value: SliderValue): RangeFilter { + const indexPattern = this.getIndexPattern()!; const newFilter = esFilters.buildRangeFilter( // TODO: Fix type to be required - this.indexPattern.fields.getByName(this.fieldName) as IFieldType, + indexPattern.fields.getByName(this.fieldName) as IFieldType, toRange(value), - this.indexPattern + indexPattern ); newFilter.meta.key = this.fieldName; newFilter.meta.controlledBy = this.controlId; diff --git a/src/plugins/input_control_vis/public/control/list_control_factory.ts b/src/plugins/input_control_vis/public/control/list_control_factory.ts index 325eb471d510b1..74ee09117887e1 100644 --- a/src/plugins/input_control_vis/public/control/list_control_factory.ts +++ b/src/plugins/input_control_vis/public/control/list_control_factory.ts @@ -142,6 +142,17 @@ export class ListControl extends Control { timeout: `${settings.autocompleteTimeout}ms`, terminate_after: Number(settings.autocompleteTerminateAfter), }; + + // dynamic options are only allowed on String fields but the setting defaults to true so it could + // be enabled for non-string fields (since UI input is hidden for non-string fields). + // If field is not string, then disable dynamic options. + const field = indexPattern?.fields + .getAll() + .find(({ name }) => name === this.controlParams.fieldName); + if (field && field.type !== 'string') { + this.options.dynamicOptions = false; + } + const aggs = termsAgg({ field: indexPattern.fields.getByName(fieldName), size: this.options.dynamicOptions ? null : _.get(this.options, 'size', 5), @@ -213,27 +224,20 @@ export async function listControlFactory( deps: InputControlVisDependencies ) { const [, { data: dataPluginStart }] = await deps.core.getStartServices(); - const indexPattern = await dataPluginStart.indexPatterns.get(controlParams.indexPattern); - - // dynamic options are only allowed on String fields but the setting defaults to true so it could - // be enabled for non-string fields (since UI input is hidden for non-string fields). - // If field is not string, then disable dynamic options. - const field = indexPattern.fields.getAll().find(({ name }) => name === controlParams.fieldName); - if (field && field.type !== 'string') { - controlParams.options.dynamicOptions = false; - } const listControl = new ListControl( controlParams, new PhraseFilterManager( controlParams.id, controlParams.fieldName, - indexPattern, + controlParams.indexPattern, + dataPluginStart.indexPatterns, deps.data.query.filterManager ), useTimeFilter, dataPluginStart.search.searchSource, deps ); + await listControl.filterManager.init(); return listControl; } diff --git a/src/plugins/input_control_vis/public/control/range_control_factory.ts b/src/plugins/input_control_vis/public/control/range_control_factory.ts index eac79ca5fcca86..9cb1a0e25a8db8 100644 --- a/src/plugins/input_control_vis/public/control/range_control_factory.ts +++ b/src/plugins/input_control_vis/public/control/range_control_factory.ts @@ -134,18 +134,20 @@ export async function rangeControlFactory( deps: InputControlVisDependencies ): Promise { const [, { data: dataPluginStart }] = await deps.core.getStartServices(); - const indexPattern = await dataPluginStart.indexPatterns.get(controlParams.indexPattern); - return new RangeControl( + const rangeControl = new RangeControl( controlParams, new RangeFilterManager( controlParams.id, controlParams.fieldName, - indexPattern, + controlParams.indexPattern, + dataPluginStart.indexPatterns, deps.data.query.filterManager ), useTimeFilter, dataPluginStart.search.searchSource, deps ); + await rangeControl.filterManager.init(); + return rangeControl; }