Fix buildEsQuery to ignore filters if not in index (#29880)

* Fix buildEsQuery to ignore filters if not in index

* Fix tests and move getEsQueryConfig to package

* Revert changes to TSVB

* Remove console log

* Review feedback
This commit is contained in:
Lukas Olson 2019-02-11 08:36:19 -07:00
parent e1700347fe
commit 9e3a320dc5
12 changed files with 125 additions and 44 deletions

View file

@ -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 expect from 'expect.js';
import { filterMatchesIndex } from '../filter_matches_index';
describe('filterMatchesIndex', function () {
it('should return true if the filter has no meta', () => {
const filter = {};
const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] };
expect(filterMatchesIndex(filter, indexPattern)).to.be(true);
});
it('should return true if no index pattern is passed', () => {
const filter = { meta: { index: 'foo', key: 'bar' } };
const indexPattern = undefined;
expect(filterMatchesIndex(filter, indexPattern)).to.be(true);
});
it('should return true if the filter key matches a field name', () => {
const filter = { meta: { index: 'foo', key: 'bar' } };
const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] };
expect(filterMatchesIndex(filter, indexPattern)).to.be(true);
});
it('should return false if the filter key does not match a field name', () => {
const filter = { meta: { index: 'foo', key: 'baz' } };
const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] };
expect(filterMatchesIndex(filter, indexPattern)).to.be(false);
});
});

View file

@ -36,6 +36,7 @@ export function buildEsQuery(
config = {
allowLeadingWildcards: false,
queryStringOptions: {},
ignoreFilterIfFieldNotInIndex: false,
}) {
queries = Array.isArray(queries) ? queries : [queries];
filters = Array.isArray(filters) ? filters : [filters];
@ -45,7 +46,7 @@ export function buildEsQuery(
const kueryQuery = buildQueryFromKuery(indexPattern, queriesByLanguage.kuery, config.allowLeadingWildcards);
const luceneQuery = buildQueryFromLucene(queriesByLanguage.lucene, config.queryStringOptions);
const filterQuery = buildQueryFromFilters(filters, indexPattern);
const filterQuery = buildQueryFromFilters(filters, indexPattern, config.ignoreFilterIfFieldNotInIndex);
return {
bool: {

View file

@ -0,0 +1,28 @@
/*
* 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.
*/
// TODO: We should base this on something better than `filter.meta.key`. We should probably modify
// this to check if `filter.meta.index` matches `indexPattern.id` instead, but that's a breaking
// change.
export function filterMatchesIndex(filter, indexPattern) {
if (!filter.meta || !indexPattern) {
return true;
}
return indexPattern.fields.some(field => field.name === filter.meta.key);
}

View file

@ -19,6 +19,7 @@
import _ from 'lodash';
import { migrateFilter } from './migrate_filter';
import { filterMatchesIndex } from './filter_matches_index';
/**
* Create a filter that can be reversed for filters with negate set
@ -58,10 +59,11 @@ const cleanFilter = function (filter) {
return _.omit(filter, ['meta', '$state']);
};
export function buildQueryFromFilters(filters, indexPattern) {
export function buildQueryFromFilters(filters = [], indexPattern, ignoreFilterIfFieldNotInIndex) {
return {
must: (filters || [])
must: filters
.filter(filterNegate(false))
.filter(filter => !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern))
.map(translateToQuery)
.map(cleanFilter)
.map(filter => {
@ -69,8 +71,9 @@ export function buildQueryFromFilters(filters, indexPattern) {
}),
filter: [],
should: [],
must_not: (filters || [])
must_not: filters
.filter(filterNegate(true))
.filter(filter => !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern))
.map(translateToQuery)
.map(cleanFilter)
.map(filter => {

View file

@ -0,0 +1,25 @@
/*
* 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.
*/
export function getEsQueryConfig(config) {
const allowLeadingWildcards = config.get('query:allowLeadingWildcards');
const queryStringOptions = config.get('query:queryString:options');
const ignoreFilterIfFieldNotInIndex = config.get('courier:ignoreFilterIfFieldNotInIndex');
return { allowLeadingWildcards, queryStringOptions, ignoreFilterIfFieldNotInIndex };
}

View file

@ -22,3 +22,5 @@ export { buildQueryFromFilters } from './from_filters';
export { luceneStringToDsl } from './lucene_string_to_dsl';
export { migrateFilter } from './migrate_filter';
export { decorateQuery } from './decorate_query';
export { filterMatchesIndex } from './filter_matches_index';
export { getEsQueryConfig } from './get_es_query_config';

View file

@ -21,8 +21,10 @@ export async function getEsQueryConfig(req) {
const uiSettings = req.getUiSettingsService();
const allowLeadingWildcards = await uiSettings.get('query:allowLeadingWildcards');
const queryStringOptions = await uiSettings.get('query:queryString:options');
const ignoreFilterIfFieldNotInIndex = await uiSettings.get('courier:ignoreFilterIfFieldNotInIndex');
return {
allowLeadingWildcards,
queryStringOptions: JSON.parse(queryStringOptions),
ignoreFilterIfFieldNotInIndex,
};
}

View file

@ -18,7 +18,7 @@
*/
import _ from 'lodash';
import { buildEsQuery } from '@kbn/es-query';
import { buildEsQuery, getEsQueryConfig } from '@kbn/es-query';
import { timezoneProvider } from 'ui/vis/lib/timezone';
const TimelionRequestHandlerProvider = function (Private, Notifier, $http, config) {
@ -35,10 +35,7 @@ const TimelionRequestHandlerProvider = function (Private, Notifier, $http, confi
return new Promise((resolve, reject) => {
const expression = visParams.expression;
if (!expression) return;
const esQueryConfigs = {
allowLeadingWildcards: config.get('query:allowLeadingWildcards'),
queryStringOptions: config.get('query:queryString:options'),
};
const esQueryConfigs = getEsQueryConfig(config);
const httpResult = $http.post('../api/timelion/run', {
sheet: [expression],
extended: {

View file

@ -21,7 +21,7 @@ import { VegaParser } from './data_model/vega_parser';
import { SearchCache } from './data_model/search_cache';
import { TimeCache } from './data_model/time_cache';
import { timefilter } from 'ui/timefilter';
import { buildEsQuery } from '@kbn/es-query';
import { buildEsQuery, getEsQueryConfig } from '@kbn/es-query';
export function VegaRequestHandlerProvider(es, serviceSettings, config) {
const searchCache = new SearchCache(es, { max: 10, maxAge: 4 * 1000 });
@ -34,10 +34,7 @@ export function VegaRequestHandlerProvider(es, serviceSettings, config) {
handler({ timeRange, filters, query, visParams }) {
timeCache.setTimeRange(timeRange);
const esQueryConfigs = {
allowLeadingWildcards: config.get('query:allowLeadingWildcards'),
queryStringOptions: config.get('query:queryString:options'),
};
const esQueryConfigs = getEsQueryConfig(config);
const filtersDsl = buildEsQuery(undefined, query, filters, esQueryConfigs);
const vp = new VegaParser(visParams.spec, searchCache, timeCache, filtersDsl, serviceSettings);
return vp.parseAsync();

View file

@ -311,9 +311,7 @@ describe('SearchSource', function () {
}
};
state.index = {
fields: {
byName: {}
}
fields: []
};
searchSource._mergeProp(state, filter, 'filter');
expect(state.filters).to.eql([ filter ]);
@ -329,9 +327,7 @@ describe('SearchSource', function () {
}
};
state.index = {
fields: {
byName: {}
}
fields: []
};
searchSource._mergeProp(state, filter, 'filter');
expect(state.filters).to.be.empty();
@ -345,11 +341,7 @@ describe('SearchSource', function () {
}
};
state.index = {
fields: {
byName: {
bar: true
}
}
fields: [{ name: 'bar' }]
};
searchSource._mergeProp(state, filter, 'filter');
expect(state.filters).to.eql([ filter ]);

View file

@ -71,7 +71,7 @@
import _ from 'lodash';
import angular from 'angular';
import { buildEsQuery } from '@kbn/es-query';
import { buildEsQuery, getEsQueryConfig, filterMatchesIndex } from '@kbn/es-query';
import '../../promises';
import { NormalizeSortRequestProvider } from './_normalize_sort_request';
@ -147,15 +147,8 @@ export function SearchSourceProvider(Promise, Private, config) {
return disabled === undefined || disabled === false;
},
(filter, data) => {
if (!config.get('courier:ignoreFilterIfFieldNotInIndex')) {
return true;
}
if ('meta' in filter && 'index' in data) {
const field = data.index.fields.byName[filter.meta.key];
if (!field) return false;
}
return true;
const index = data.index || this.getField('index');
return !config.get('courier:ignoreFilterIfFieldNotInIndex') || filterMatchesIndex(filter, index);
}
];
}
@ -610,10 +603,7 @@ export function SearchSourceProvider(Promise, Private, config) {
}
try {
const esQueryConfigs = {
allowLeadingWildcards: config.get('query:allowLeadingWildcards'),
queryStringOptions: config.get('query:queryString:options'),
};
const esQueryConfigs = getEsQueryConfig(config);
flatData.body.query = buildEsQuery(flatData.index, flatData.query, flatData.filters, esQueryConfigs);
} catch (e) {
if (e.message === 'OutdatedKuerySyntaxError') {

View file

@ -8,7 +8,7 @@
import _ from 'lodash';
import $ from 'jquery';
import { buildEsQuery } from '@kbn/es-query';
import { buildEsQuery, getEsQueryConfig } from '@kbn/es-query';
import { addItemToRecentlyAccessed } from 'plugins/ml/util/recently_accessed';
import { mlJobService } from 'plugins/ml/services/job_service';
@ -49,10 +49,7 @@ export function SearchItemsProvider(Private, $route, config) {
if (fs.length) {
filters = fs;
}
const esQueryConfigs = {
allowLeadingWildcards: config.get('query:allowLeadingWildcards'),
queryStringOptions: config.get('query:queryString:options'),
};
const esQueryConfigs = getEsQueryConfig(config);
combinedQuery = buildEsQuery(indexPattern, [query], filters, esQueryConfigs);
}