Skip to content

Commit 4504cd1

Browse files
[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working (#153816)
Fixes #153595 ### Background When advanced setting `courier:ignoreFilterIfFieldNotInIndex` is enabled, filters are ignored when data view does not contain the filtering field. The logic on how this works is captured below for context. https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83 ``` .filter((filter) => { const indexPattern = findIndexPattern(inputDataViews, filter.meta?.index); return !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern); }) ``` https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20 ``` export function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase | null) { if (!filter.meta?.key || !indexPattern) { return true; } // Fixes #89878 // Custom filters may refer multiple fields. Validate the index id only. if (filter.meta?.type === 'custom') { return filter.meta.index === indexPattern.id; } return indexPattern.fields.some((field) => field.name === filter.meta.key); } ``` The problem is that [mapSpatialFilter](https://github.com/elastic/kibana/blob/8.7/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts#L12) is setting `meta.key` property. This causes `filterMatchesIndex` check to fail for spatial filters. Spatial filters are designed to work across data views and avoid the problems that `courier:ignoreFilterIfFieldNotInIndex` solves. As such, spatial filters should not set `meta.key` property and not get removed when `courier:ignoreFilterIfFieldNotInIndex` is enabled. ### Test * set advanced setting `courier:ignoreFilterIfFieldNotInIndex` and refresh browser * install 2 or more sample data sets * create a new map * add documents layer from one sample data set * add documents layer from another sample data set * create spatial filter on map, https://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters * Verify filter is applied to both layers --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent b692e34 commit 4504cd1

File tree

6 files changed

+114
-144
lines changed

6 files changed

+114
-144
lines changed

src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.test.ts

Lines changed: 22 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -7,117 +7,31 @@
77
*/
88

99
import { mapSpatialFilter } from './map_spatial_filter';
10+
import { mapFilter } from '../map_filter';
1011
import { FilterMeta, Filter, FILTERS } from '@kbn/es-query';
1112

12-
describe('mapSpatialFilter()', () => {
13-
test('should return the key for matching multi polygon filter', async () => {
13+
describe('mapSpatialFilter', () => {
14+
test('should set meta type field', async () => {
1415
const filter = {
1516
meta: {
16-
key: 'location',
17-
alias: 'my spatial filter',
18-
type: FILTERS.SPATIAL_FILTER,
19-
} as FilterMeta,
20-
query: {
21-
bool: {
22-
should: [
23-
{
24-
geo_polygon: {
25-
geoCoordinates: { points: [] },
26-
},
27-
},
28-
],
29-
},
30-
},
31-
} as Filter;
32-
const result = mapSpatialFilter(filter);
33-
34-
expect(result).toHaveProperty('key', 'location');
35-
expect(result).toHaveProperty('value', '');
36-
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
37-
});
38-
39-
test('should return the key for matching polygon filter', async () => {
40-
const filter = {
41-
meta: {
42-
key: 'location',
43-
alias: 'my spatial filter',
4417
type: FILTERS.SPATIAL_FILTER,
4518
} as FilterMeta,
46-
geo_polygon: {
47-
geoCoordinates: { points: [] },
48-
},
19+
query: {},
4920
} as Filter;
5021
const result = mapSpatialFilter(filter);
5122

52-
expect(result).toHaveProperty('key', 'location');
53-
expect(result).toHaveProperty('value', '');
54-
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
55-
});
56-
57-
test('should return the key for matching multi field filter', async () => {
58-
const filter = {
59-
meta: {
60-
alias: 'my spatial filter',
61-
isMultiIndex: true,
62-
type: FILTERS.SPATIAL_FILTER,
63-
} as FilterMeta,
64-
query: {
65-
bool: {
66-
should: [
67-
{
68-
bool: {
69-
must: [
70-
{
71-
exists: {
72-
field: 'geo.coordinates',
73-
},
74-
},
75-
{
76-
geo_distance: {
77-
distance: '1000km',
78-
'geo.coordinates': [120, 30],
79-
},
80-
},
81-
],
82-
},
83-
},
84-
{
85-
bool: {
86-
must: [
87-
{
88-
exists: {
89-
field: 'location',
90-
},
91-
},
92-
{
93-
geo_distance: {
94-
distance: '1000km',
95-
location: [120, 30],
96-
},
97-
},
98-
],
99-
},
100-
},
101-
],
102-
},
103-
},
104-
} as Filter;
105-
const result = mapSpatialFilter(filter);
106-
107-
expect(result).toHaveProperty('key', 'query');
108-
expect(result).toHaveProperty('value', '');
10923
expect(result).toHaveProperty('type', FILTERS.SPATIAL_FILTER);
24+
expect(result).toHaveProperty('key', undefined);
25+
expect(result).toHaveProperty('value', undefined);
11026
});
11127

11228
test('should return undefined for none matching', async () => {
11329
const filter = {
11430
meta: {
11531
key: 'location',
116-
alias: 'my spatial filter',
32+
alias: 'my non-spatial filter',
11733
} as FilterMeta,
118-
geo_polygon: {
119-
geoCoordinates: { points: [] },
120-
},
34+
query: {},
12135
} as Filter;
12236

12337
try {
@@ -127,3 +41,17 @@ describe('mapSpatialFilter()', () => {
12741
}
12842
});
12943
});
44+
45+
describe('mapFilter', () => {
46+
test('should set key and value properties to undefined', async () => {
47+
const before = {
48+
meta: { type: FILTERS.SPATIAL_FILTER } as FilterMeta,
49+
query: {},
50+
} as Filter;
51+
const after = mapFilter(before);
52+
53+
expect(after).toHaveProperty('meta');
54+
expect(after.meta).toHaveProperty('key', undefined);
55+
expect(after.meta).toHaveProperty('value', undefined);
56+
});
57+
});

src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,30 +10,17 @@ import { Filter, FILTERS } from '@kbn/es-query';
1010

1111
// Use mapSpatialFilter mapper to avoid bloated meta with value and params for spatial filters.
1212
export const mapSpatialFilter = (filter: Filter) => {
13-
if (
14-
filter.meta &&
15-
filter.meta.key &&
16-
filter.meta.alias &&
17-
filter.meta.type === FILTERS.SPATIAL_FILTER
18-
) {
13+
if (filter.meta?.type === FILTERS.SPATIAL_FILTER) {
1914
return {
20-
key: filter.meta.key,
2115
type: filter.meta.type,
22-
value: '',
16+
// spatial filters support multiple fields across multiple data views
17+
// do not provide "key" since filter does not provide a single field
18+
key: undefined,
19+
// default mapper puts stringified filter in "value"
20+
// do not provide "value" to avoid bloating URL
21+
value: undefined,
2322
};
2423
}
2524

26-
if (
27-
filter.meta &&
28-
filter.meta.type === FILTERS.SPATIAL_FILTER &&
29-
filter.meta.isMultiIndex &&
30-
filter.query?.bool?.should
31-
) {
32-
return {
33-
key: 'query',
34-
type: filter.meta.type,
35-
value: '',
36-
};
37-
}
3825
throw filter;
3926
};

x-pack/plugins/maps/common/elasticsearch_util/spatial_filter_utils.test.ts

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,43 @@
66
*/
77

88
import { Polygon } from 'geojson';
9+
import { DataViewBase } from '@kbn/es-query';
910
import {
1011
createDistanceFilterWithMeta,
1112
createExtentFilter,
1213
buildGeoGridFilter,
1314
buildGeoShapeFilter,
1415
extractFeaturesFromFilters,
1516
} from './spatial_filter_utils';
17+
import { buildQueryFromFilters } from '@kbn/es-query';
1618

1719
const geoFieldName = 'location';
1820

21+
describe('buildQueryFromFilters', () => {
22+
it('should not drop spatial filters when ignoreFilterIfFieldNotInIndex is true', () => {
23+
const query = buildQueryFromFilters(
24+
[
25+
createDistanceFilterWithMeta({
26+
point: [100, 30],
27+
distanceKm: 1000,
28+
geoFieldNames: ['geo.coordinates'],
29+
}),
30+
createDistanceFilterWithMeta({
31+
point: [120, 30],
32+
distanceKm: 1000,
33+
geoFieldNames: ['geo.coordinates', 'location'],
34+
}),
35+
],
36+
{
37+
id: 'myDataViewWithoutAnyMacthingFields',
38+
fields: [],
39+
} as unknown as DataViewBase,
40+
{ ignoreFilterIfFieldNotInIndex: true }
41+
);
42+
expect(query.filter.length).toBe(2);
43+
});
44+
});
45+
1946
describe('createExtentFilter', () => {
2047
it('should return elasticsearch geo_bounding_box filter', () => {
2148
const mapExtent = {
@@ -28,7 +55,7 @@ describe('createExtentFilter', () => {
2855
meta: {
2956
alias: null,
3057
disabled: false,
31-
key: 'location',
58+
isMultiIndex: true,
3259
negate: false,
3360
type: 'spatial_filter',
3461
},
@@ -65,7 +92,7 @@ describe('createExtentFilter', () => {
6592
meta: {
6693
alias: null,
6794
disabled: false,
68-
key: 'location',
95+
isMultiIndex: true,
6996
negate: false,
7097
type: 'spatial_filter',
7198
},
@@ -102,7 +129,7 @@ describe('createExtentFilter', () => {
102129
meta: {
103130
alias: null,
104131
disabled: false,
105-
key: 'location',
132+
isMultiIndex: true,
106133
negate: false,
107134
type: 'spatial_filter',
108135
},
@@ -139,7 +166,7 @@ describe('createExtentFilter', () => {
139166
meta: {
140167
alias: null,
141168
disabled: false,
142-
key: 'location',
169+
isMultiIndex: true,
143170
negate: false,
144171
type: 'spatial_filter',
145172
},
@@ -176,7 +203,7 @@ describe('createExtentFilter', () => {
176203
meta: {
177204
alias: null,
178205
disabled: false,
179-
key: 'location',
206+
isMultiIndex: true,
180207
negate: false,
181208
type: 'spatial_filter',
182209
},
@@ -276,7 +303,7 @@ describe('buildGeoGridFilter', () => {
276303
meta: {
277304
alias: 'intersects cluster 9/146/195',
278305
disabled: false,
279-
key: 'geo.coordinates',
306+
isMultiIndex: true,
280307
negate: false,
281308
type: 'spatial_filter',
282309
},
@@ -312,7 +339,6 @@ describe('buildGeoGridFilter', () => {
312339
alias: 'intersects cluster 822af7fffffffff',
313340
disabled: false,
314341
isMultiIndex: true,
315-
key: undefined,
316342
negate: false,
317343
type: 'spatial_filter',
318344
},
@@ -384,7 +410,7 @@ describe('buildGeoShapeFilter', () => {
384410
meta: {
385411
alias: 'intersects myShape',
386412
disabled: false,
387-
key: 'geo.coordinates',
413+
isMultiIndex: true,
388414
negate: false,
389415
type: 'spatial_filter',
390416
},
@@ -444,7 +470,6 @@ describe('buildGeoShapeFilter', () => {
444470
alias: 'intersects myShape',
445471
disabled: false,
446472
isMultiIndex: true,
447-
key: undefined,
448473
negate: false,
449474
type: 'spatial_filter',
450475
},
@@ -531,7 +556,7 @@ describe('createDistanceFilterWithMeta', () => {
531556
meta: {
532557
alias: 'within 1000km of 120, 30',
533558
disabled: false,
534-
key: 'geo.coordinates',
559+
isMultiIndex: true,
535560
negate: false,
536561
type: 'spatial_filter',
537562
},
@@ -566,7 +591,6 @@ describe('createDistanceFilterWithMeta', () => {
566591
alias: 'within 1000km of 120, 30',
567592
disabled: false,
568593
isMultiIndex: true,
569-
key: undefined,
570594
negate: false,
571595
type: 'spatial_filter',
572596
},
@@ -637,6 +661,37 @@ describe('extractFeaturesFromFilters', () => {
637661
expect(extractFeaturesFromFilters([phraseFilter])).toEqual([]);
638662
});
639663

664+
it('should ignore malformed spatial filers', () => {
665+
expect(
666+
extractFeaturesFromFilters([
667+
{
668+
meta: {
669+
type: 'spatial_filter',
670+
},
671+
query: {
672+
bool: {
673+
must: [],
674+
},
675+
},
676+
},
677+
])
678+
).toEqual([]);
679+
expect(
680+
extractFeaturesFromFilters([
681+
{
682+
meta: {
683+
type: 'spatial_filter',
684+
},
685+
query: {
686+
bool: {
687+
should: [],
688+
},
689+
},
690+
},
691+
])
692+
).toEqual([]);
693+
});
694+
640695
it('should convert single field geo_distance filter to feature', () => {
641696
const spatialFilter = createDistanceFilterWithMeta({
642697
point: [-89.87125, 53.49454],

0 commit comments

Comments
 (0)