Skip to content

Commit

Permalink
Input controls crashes if index pattern is not available (elastic#79431
Browse files Browse the repository at this point in the history
…) (elastic#79979)

* Input controls crashes if index pattern is not available

Closes elastic#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
  • Loading branch information
DianaDerevyankina authored Oct 8, 2020
1 parent 9408f41 commit 2549191
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/plugins/input_control_vis/public/control/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
Expand All @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {}

/**
Expand All @@ -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;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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 () {
Expand Down Expand Up @@ -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 = [];
}

Expand All @@ -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
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
RangeFilterMeta,
IndexPattern,
FilterManager as QueryFilterManager,
IndexPatternsContract,
} from '../../../../data/public';

describe('RangeFilterManager', function () {
Expand All @@ -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 () {
Expand All @@ -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 = [];
}

Expand All @@ -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
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 2549191

Please sign in to comment.