[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geogra…

…phic 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 <[email protected]>
elastic · Mar 31, 2023 · 4504cd1 · 4504cd1
1 parent b692e34
commit 4504cd1
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 144 deletions.
diff --git a/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.test.ts b/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.test.ts
@@ -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: [] },
-              },
-            },
-          ],
-        },
-      },
-    } 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: [] },
-      },
+      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 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);
+  });
+});
diff --git a/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts b/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts
@@ -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;
 };
diff --git a/x-pack/plugins/maps/common/elasticsearch_util/spatial_filter_utils.test.ts b/x-pack/plugins/maps/common/elasticsearch_util/spatial_filter_utils.test.ts
@@ -6,16 +6,43 @@
  */
 
 import { Polygon } from 'geojson';
+import { DataViewBase } from '@kbn/es-query';
 import {
   createDistanceFilterWithMeta,
   createExtentFilter,
   buildGeoGridFilter,
   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],