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
elastic · Oct 8, 2020 · 4dc6f3b · 4dc6f3b
1 parent fff3060
commit 4dc6f3b
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 44 deletions.
diff --git 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<FilterManager extends BaseFilterManager> {
   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
@@ -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
@@ -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,20 +49,30 @@ 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[] {
     const kbnFilters = _.flatten([
       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
@@ -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<IndexPatternsContract>;
     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
@@ -23,32 +23,34 @@ import { FilterManager } from './filter_manager';
 import {
   PhraseFilter,
   esFilters,
-  IndexPattern,
+  IndexPatternsContract,
   FilterManager as QueryFilterManager,
 } from '../../../../data/public';
 
 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
@@ -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<IndexPatternsContract>;
     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
@@ -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
@@ -142,6 +142,17 @@ export class ListControl extends Control<PhraseFilterManager> {
       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
@@ -134,18 +134,20 @@ export async function rangeControlFactory(
   deps: InputControlVisDependencies
 ): Promise<RangeControl> {
   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;
 }