[8.7] [maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working (#153816) (#154162)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled,
geographic filters no longer working
(#153816)](https://github.com/elastic/kibana/pull/153816)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2023-03-31T13:59:01Z","message":"[maps]
fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic
filters no longer working (#153816)\n\nFixes
https://github.com/elastic/kibana/issues/153595\r\n\r\n###
Background\r\n\r\nWhen advanced setting
`courier:ignoreFilterIfFieldNotInIndex` is\r\nenabled, filters are
ignored when data view does not contain the\r\nfiltering field. The
logic on how this works is captured below
for\r\ncontext.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83\r\n```\r\n
.filter((filter) => {\r\n const indexPattern =
findIndexPattern(inputDataViews, filter.meta?.index);\r\n return
!ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter,
indexPattern);\r\n
})\r\n```\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20\r\n```\r\nexport
function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase
| null) {\r\n if (!filter.meta?.key || !indexPattern) {\r\n return
true;\r\n }\r\n\r\n // Fixes
https://github.com/elastic/kibana/issues/89878\r\n // Custom filters may
refer multiple fields. Validate the index id only.\r\n if
(filter.meta?.type === 'custom') {\r\n return filter.meta.index ===
indexPattern.id;\r\n }\r\n\r\n return indexPattern.fields.some((field)
=> field.name === filter.meta.key);\r\n}\r\n```\r\n\r\nThe problem is
that\r\n[mapSpatialFilter](https://github.com/elastic/kibana/blob/8.7/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts#L12)\r\nis
setting `meta.key` property. This causes `filterMatchesIndex`
check\r\nto fail for spatial filters.\r\n\r\nSpatial filters are
designed to work across data views and avoid the\r\nproblems that
`courier:ignoreFilterIfFieldNotInIndex` solves. As such,\r\nspatial
filters should not set `meta.key` property and not get removed\r\nwhen
`courier:ignoreFilterIfFieldNotInIndex` is enabled.\r\n\r\n### Test\r\n*
set advanced setting `courier:ignoreFilterIfFieldNotInIndex`
and\r\nrefresh browser\r\n* install 2 or more sample data sets\r\n*
create a new map\r\n* add documents layer from one sample data set\r\n*
add documents layer from another sample data set\r\n* create spatial
filter on
map,\r\nhttps://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters\r\n*
Verify filter is applied to both
layers\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4504cd18c07c0f0af94830558f7906dd77a2b217","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","auto-backport","Feature:Maps","v8.8.0","v8.7.1"],"number":153816,"url":"https://github.com/elastic/kibana/pull/153816","mergeCommit":{"message":"[maps]
fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic
filters no longer working (#153816)\n\nFixes
https://github.com/elastic/kibana/issues/153595\r\n\r\n###
Background\r\n\r\nWhen advanced setting
`courier:ignoreFilterIfFieldNotInIndex` is\r\nenabled, filters are
ignored when data view does not contain the\r\nfiltering field. The
logic on how this works is captured below
for\r\ncontext.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83\r\n```\r\n
.filter((filter) => {\r\n const indexPattern =
findIndexPattern(inputDataViews, filter.meta?.index);\r\n return
!ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter,
indexPattern);\r\n
})\r\n```\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20\r\n```\r\nexport
function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase
| null) {\r\n if (!filter.meta?.key || !indexPattern) {\r\n return
true;\r\n }\r\n\r\n // Fixes
https://github.com/elastic/kibana/issues/89878\r\n // Custom filters may
refer multiple fields. Validate the index id only.\r\n if
(filter.meta?.type === 'custom') {\r\n return filter.meta.index ===
indexPattern.id;\r\n }\r\n\r\n return indexPattern.fields.some((field)
=> field.name === filter.meta.key);\r\n}\r\n```\r\n\r\nThe problem is
that\r\n[mapSpatialFilter](https://github.com/elastic/kibana/blob/8.7/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts#L12)\r\nis
setting `meta.key` property. This causes `filterMatchesIndex`
check\r\nto fail for spatial filters.\r\n\r\nSpatial filters are
designed to work across data views and avoid the\r\nproblems that
`courier:ignoreFilterIfFieldNotInIndex` solves. As such,\r\nspatial
filters should not set `meta.key` property and not get removed\r\nwhen
`courier:ignoreFilterIfFieldNotInIndex` is enabled.\r\n\r\n### Test\r\n*
set advanced setting `courier:ignoreFilterIfFieldNotInIndex`
and\r\nrefresh browser\r\n* install 2 or more sample data sets\r\n*
create a new map\r\n* add documents layer from one sample data set\r\n*
add documents layer from another sample data set\r\n* create spatial
filter on
map,\r\nhttps://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters\r\n*
Verify filter is applied to both
layers\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4504cd18c07c0f0af94830558f7906dd77a2b217"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/153816","number":153816,"mergeCommit":{"message":"[maps]
fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic
filters no longer working (#153816)\n\nFixes
https://github.com/elastic/kibana/issues/153595\r\n\r\n###
Background\r\n\r\nWhen advanced setting
`courier:ignoreFilterIfFieldNotInIndex` is\r\nenabled, filters are
ignored when data view does not contain the\r\nfiltering field. The
logic on how this works is captured below
for\r\ncontext.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83\r\n```\r\n
.filter((filter) => {\r\n const indexPattern =
findIndexPattern(inputDataViews, filter.meta?.index);\r\n return
!ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter,
indexPattern);\r\n
})\r\n```\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20\r\n```\r\nexport
function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase
| null) {\r\n if (!filter.meta?.key || !indexPattern) {\r\n return
true;\r\n }\r\n\r\n // Fixes
https://github.com/elastic/kibana/issues/89878\r\n // Custom filters may
refer multiple fields. Validate the index id only.\r\n if
(filter.meta?.type === 'custom') {\r\n return filter.meta.index ===
indexPattern.id;\r\n }\r\n\r\n return indexPattern.fields.some((field)
=> field.name === filter.meta.key);\r\n}\r\n```\r\n\r\nThe problem is
that\r\n[mapSpatialFilter](https://github.com/elastic/kibana/blob/8.7/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts#L12)\r\nis
setting `meta.key` property. This causes `filterMatchesIndex`
check\r\nto fail for spatial filters.\r\n\r\nSpatial filters are
designed to work across data views and avoid the\r\nproblems that
`courier:ignoreFilterIfFieldNotInIndex` solves. As such,\r\nspatial
filters should not set `meta.key` property and not get removed\r\nwhen
`courier:ignoreFilterIfFieldNotInIndex` is enabled.\r\n\r\n### Test\r\n*
set advanced setting `courier:ignoreFilterIfFieldNotInIndex`
and\r\nrefresh browser\r\n* install 2 or more sample data sets\r\n*
create a new map\r\n* add documents layer from one sample data set\r\n*
add documents layer from another sample data set\r\n* create spatial
filter on
map,\r\nhttps://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters\r\n*
Verify filter is applied to both
layers\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4504cd18c07c0f0af94830558f7906dd77a2b217"}},{"branch":"8.7","label":"v8.7.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
This commit is contained in:
Kibana Machine 2023-03-31 11:43:48 -04:00 committed by GitHub
parent 7a8aa42ba7
commit 7947c8bb9a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 114 additions and 144 deletions

View file

@ -7,117 +7,31 @@
*/
import { mapSpatialFilter } from './map_spatial_filter';
import { mapFilter } from '../map_filter';
import { FilterMeta, Filter, FILTERS } from '@kbn/es-query';
describe('mapSpatialFilter()', () => {
test('should return the key for matching multi polygon filter', async () => {
describe('mapSpatialFilter', () => {
test('should set meta type field', async () => {
const filter = {
meta: {
key: 'location',
alias: 'my spatial filter',
type: FILTERS.SPATIAL_FILTER,
} as FilterMeta,
query: {
bool: {
should: [
{
geo_polygon: {
geoCoordinates: { points: [] },
},
},
],
},
},
query: {},
} as Filter;
const result = mapSpatialFilter(filter);
expect(result).toHaveProperty('key', 'location');
expect(result).toHaveProperty('value', '');
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
});
test('should return the key for matching polygon filter', async () => {
const filter = {
meta: {
key: 'location',
alias: 'my spatial filter',
type: FILTERS.SPATIAL_FILTER,
} as FilterMeta,
geo_polygon: {
geoCoordinates: { points: [] },
},
} as Filter;
const result = mapSpatialFilter(filter);
expect(result).toHaveProperty('key', 'location');
expect(result).toHaveProperty('value', '');
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
});
test('should return the key for matching multi field filter', async () => {
const filter = {
meta: {
alias: 'my spatial filter',
isMultiIndex: true,
type: FILTERS.SPATIAL_FILTER,
} as FilterMeta,
query: {
bool: {
should: [
{
bool: {
must: [
{
exists: {
field: 'geo.coordinates',
},
},
{
geo_distance: {
distance: '1000km',
'geo.coordinates': [120, 30],
},
},
],
},
},
{
bool: {
must: [
{
exists: {
field: 'location',
},
},
{
geo_distance: {
distance: '1000km',
location: [120, 30],
},
},
],
},
},
],
},
},
} as Filter;
const result = mapSpatialFilter(filter);
expect(result).toHaveProperty('key', 'query');
expect(result).toHaveProperty('value', '');
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
expect(result).toHaveProperty('key', undefined);
expect(result).toHaveProperty('value', undefined);
});
test('should return undefined for none matching', async () => {
const filter = {
meta: {
key: 'location',
alias: 'my spatial filter',
alias: 'my non-spatial filter',
} as FilterMeta,
geo_polygon: {
geoCoordinates: { points: [] },
},
query: {},
} as Filter;
try {
@ -127,3 +41,17 @@ describe('mapSpatialFilter()', () => {
}
});
});
describe('mapFilter', () => {
test('should set key and value properties to undefined', async () => {
const before = {
meta: { type: FILTERS.SPATIAL_FILTER } as FilterMeta,
query: {},
} as Filter;
const after = mapFilter(before);
expect(after).toHaveProperty('meta');
expect(after.meta).toHaveProperty('key', undefined);
expect(after.meta).toHaveProperty('value', undefined);
});
});

View file

@ -10,30 +10,17 @@ import { Filter, FILTERS } from '@kbn/es-query';
// Use mapSpatialFilter mapper to avoid bloated meta with value and params for spatial filters.
export const mapSpatialFilter = (filter: Filter) => {
if (
filter.meta &&
filter.meta.key &&
filter.meta.alias &&
filter.meta.type === FILTERS.SPATIAL_FILTER
) {
if (filter.meta?.type === FILTERS.SPATIAL_FILTER) {
return {
key: filter.meta.key,
type: filter.meta.type,
value: '',
// spatial filters support multiple fields across multiple data views
// do not provide "key" since filter does not provide a single field
key: undefined,
// default mapper puts stringified filter in "value"
// do not provide "value" to avoid bloating URL
value: undefined,
};
}
if (
filter.meta &&
filter.meta.type === FILTERS.SPATIAL_FILTER &&
filter.meta.isMultiIndex &&
filter.query?.bool?.should
) {
return {
key: 'query',
type: filter.meta.type,
value: '',
};
}
throw filter;
};

View file

@ -6,6 +6,7 @@
*/
import { Polygon } from 'geojson';
import { DataViewBase } from '@kbn/es-query';
import {
createDistanceFilterWithMeta,
createExtentFilter,
@ -13,9 +14,35 @@ import {
buildGeoShapeFilter,
extractFeaturesFromFilters,
} from './spatial_filter_utils';
import { buildQueryFromFilters } from '@kbn/es-query';
const geoFieldName = 'location';
describe('buildQueryFromFilters', () => {
it('should not drop spatial filters when ignoreFilterIfFieldNotInIndex is true', () => {
const query = buildQueryFromFilters(
[
createDistanceFilterWithMeta({
point: [100, 30],
distanceKm: 1000,
geoFieldNames: ['geo.coordinates'],
}),
createDistanceFilterWithMeta({
point: [120, 30],
distanceKm: 1000,
geoFieldNames: ['geo.coordinates', 'location'],
}),
],
{
id: 'myDataViewWithoutAnyMacthingFields',
fields: [],
} as unknown as DataViewBase,
{ ignoreFilterIfFieldNotInIndex: true }
);
expect(query.filter.length).toBe(2);
});
});
describe('createExtentFilter', () => {
it('should return elasticsearch geo_bounding_box filter', () => {
const mapExtent = {
@ -28,7 +55,7 @@ describe('createExtentFilter', () => {
meta: {
alias: null,
disabled: false,
key: 'location',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -65,7 +92,7 @@ describe('createExtentFilter', () => {
meta: {
alias: null,
disabled: false,
key: 'location',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -102,7 +129,7 @@ describe('createExtentFilter', () => {
meta: {
alias: null,
disabled: false,
key: 'location',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -139,7 +166,7 @@ describe('createExtentFilter', () => {
meta: {
alias: null,
disabled: false,
key: 'location',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -176,7 +203,7 @@ describe('createExtentFilter', () => {
meta: {
alias: null,
disabled: false,
key: 'location',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -276,7 +303,7 @@ describe('buildGeoGridFilter', () => {
meta: {
alias: 'intersects cluster 9/146/195',
disabled: false,
key: 'geo.coordinates',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -312,7 +339,6 @@ describe('buildGeoGridFilter', () => {
alias: 'intersects cluster 822af7fffffffff',
disabled: false,
isMultiIndex: true,
key: undefined,
negate: false,
type: 'spatial_filter',
},
@ -384,7 +410,7 @@ describe('buildGeoShapeFilter', () => {
meta: {
alias: 'intersects myShape',
disabled: false,
key: 'geo.coordinates',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -444,7 +470,6 @@ describe('buildGeoShapeFilter', () => {
alias: 'intersects myShape',
disabled: false,
isMultiIndex: true,
key: undefined,
negate: false,
type: 'spatial_filter',
},
@ -531,7 +556,7 @@ describe('createDistanceFilterWithMeta', () => {
meta: {
alias: 'within 1000km of 120, 30',
disabled: false,
key: 'geo.coordinates',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},
@ -566,7 +591,6 @@ describe('createDistanceFilterWithMeta', () => {
alias: 'within 1000km of 120, 30',
disabled: false,
isMultiIndex: true,
key: undefined,
negate: false,
type: 'spatial_filter',
},
@ -637,6 +661,37 @@ describe('extractFeaturesFromFilters', () => {
expect(extractFeaturesFromFilters([phraseFilter])).toEqual([]);
});
it('should ignore malformed spatial filers', () => {
expect(
extractFeaturesFromFilters([
{
meta: {
type: 'spatial_filter',
},
query: {
bool: {
must: [],
},
},
},
])
).toEqual([]);
expect(
extractFeaturesFromFilters([
{
meta: {
type: 'spatial_filter',
},
query: {
bool: {
should: [],
},
},
},
])
).toEqual([]);
});
it('should convert single field geo_distance filter to feature', () => {
const spatialFilter = createDistanceFilterWithMeta({
point: [-89.87125, 53.49454],

View file

@ -34,7 +34,7 @@ function createMultiGeoFieldFilter(
return {
meta: {
...meta,
key: geoFieldNames[0],
isMultiIndex: true,
},
query: {
bool: {
@ -54,7 +54,6 @@ function createMultiGeoFieldFilter(
return {
meta: {
...meta,
key: undefined,
isMultiIndex: true,
},
query: {
@ -114,7 +113,6 @@ export function buildGeoShapeFilter({
const meta: FilterMeta = {
type: SPATIAL_FILTER_TYPE,
negate: false,
key: geoFieldNames.length === 1 ? geoFieldNames[0] : undefined,
alias: `${getEsSpatialRelationLabel(relation)} ${geometryLabel}`,
disabled: false,
};
@ -159,7 +157,6 @@ export function buildGeoGridFilter({
{
type: SPATIAL_FILTER_TYPE,
negate: false,
key: geoFieldNames.length === 1 ? geoFieldNames[0] : undefined,
alias: i18n.translate('xpack.maps.common.esSpatialRelation.clusterFilterLabel', {
defaultMessage: 'intersects cluster {gridId}',
values: { gridId },
@ -236,18 +233,13 @@ export function extractFeaturesFromFilters(filters: GeoFilter[]): Feature[] {
})
.forEach((filter) => {
let geometry: Geometry | undefined;
if (filter.meta.isMultiIndex) {
const geoFieldName = filter?.query?.bool?.should?.[0]?.bool?.must?.[0]?.exists?.field;
const spatialClause = filter?.query?.bool?.should?.[0]?.bool?.must?.[1];
if (geoFieldName && spatialClause) {
geometry = extractGeometryFromFilter(geoFieldName, spatialClause);
}
} else {
const geoFieldName = filter.meta.key;
const spatialClause = filter?.query?.bool?.must?.[1];
if (geoFieldName && spatialClause) {
geometry = extractGeometryFromFilter(geoFieldName, spatialClause);
}
const must = filter?.query?.bool?.should?.length
? filter?.query?.bool?.should?.[0]?.bool?.must
: filter?.query?.bool?.must;
const geoFieldName = must?.[0]?.exists?.field;
const spatialClause = must?.[1];
if (geoFieldName && spatialClause) {
geometry = extractGeometryFromFilter(geoFieldName, spatialClause);
}
if (geometry) {

View file

@ -223,7 +223,7 @@ describe('ESGeoGridSource', () => {
meta: {
alias: null,
disabled: false,
key: 'bar',
isMultiIndex: true,
negate: false,
type: 'spatial_filter',
},

View file

@ -244,9 +244,17 @@ export class MapApp extends React.Component<Props, State> {
} else {
indexPatterns = await getIndexPatternsFromIds(nextIndexPatternIds);
}
if (this._isMounted) {
this.setState({ indexPatterns });
if (!this._isMounted) {
return;
}
// ignore results for outdated requests
if (!_.isEqual(nextIndexPatternIds, this._prevIndexPatternIds)) {
return;
}
this.setState({ indexPatterns });
}
_onQueryChange = ({