From 7d4495f2e2bc994909ae80e543d3a3e0476f9ae0 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Mon, 1 Mar 2021 17:35:50 -0700 Subject: [PATCH 1/4] Update geo shape filter queries to use bounding box logic --- .../elasticsearch_geo_utils.d.ts | 3 +-- .../elasticsearch_geo_utils.js | 27 ++++--------------- .../classes/sources/es_source/es_source.ts | 6 +---- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.d.ts b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.d.ts index d7d76ab8acd37..2a3741146d454 100644 --- a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.d.ts +++ b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.d.ts @@ -47,8 +47,7 @@ export interface ESPolygonFilter { export function createExtentFilter( mapExtent: MapExtent, - geoFieldName: string, - geoFieldType: ES_GEO_FIELD_TYPE + geoFieldName: string ): ESPolygonFilter | ESGeoBoundingBoxFilter; export function makeESBbox({ maxLat, maxLon, minLat, minLon }: MapExtent): ESBBox; diff --git a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js index c6b1c712925f0..cb80fa4401810 100644 --- a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js +++ b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js @@ -280,33 +280,16 @@ export function makeESBbox({ maxLat, maxLon, minLat, minLon }) { return esBbox; } -function createGeoBoundBoxFilter({ maxLat, maxLon, minLat, minLon }, geoFieldName) { - const boundingBox = makeESBbox({ maxLat, maxLon, minLat, minLon }); - return { - geo_bounding_box: { - [geoFieldName]: boundingBox, - }, - }; -} - -export function createExtentFilter(mapExtent, geoFieldName, geoFieldType) { - ensureGeoField(geoFieldType); - +export function createExtentFilter(mapExtent, geoFieldName) { // Extent filters are used to dynamically filter data for the current map view port. // Continue to use geo_bounding_box queries for extent filters // 1) geo_bounding_box queries are faster than polygon queries // 2) geo_shape benefits of pre-indexed shapes and - // compatability across multi-indices with geo_point and geo_shape do not apply to this use case. - if (geoFieldType === ES_GEO_FIELD_TYPE.GEO_POINT) { - return createGeoBoundBoxFilter(mapExtent, geoFieldName); - } - + // compatibility across multi-indices with geo_point and geo_shape do not apply to this use case. + const boundingBox = makeESBbox(mapExtent); return { - geo_shape: { - [geoFieldName]: { - shape: formatEnvelopeAsPolygon(mapExtent), - relation: ES_SPATIAL_RELATIONS.INTERSECTS, - }, + geo_bounding_box: { + [geoFieldName]: boundingBox, }, }; } diff --git a/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts b/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts index f6f0a234bcd67..945369829cfdf 100644 --- a/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts +++ b/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts @@ -236,11 +236,7 @@ export class AbstractESSource extends AbstractVectorSource implements IESSource typeof searchFilters.geogridPrecision === 'number' ? expandToTileBoundaries(searchFilters.buffer, searchFilters.geogridPrecision) : searchFilters.buffer; - const extentFilter = createExtentFilter( - buffer, - geoField.name, - geoField.type as ES_GEO_FIELD_TYPE - ); + const extentFilter = createExtentFilter(buffer, geoField.name); // @ts-expect-error allFilters.push(extentFilter); From 178650473360f5d4236f63aebb5f8221f0174024 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Mon, 1 Mar 2021 17:36:06 -0700 Subject: [PATCH 2/4] Update tests --- .../elasticsearch_geo_utils.test.js | 48 +++++-------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js index aed9ccbb96c3b..c1925985a48b8 100644 --- a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js +++ b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js @@ -397,7 +397,7 @@ describe('createExtentFilter', () => { minLat: 35, minLon: -89, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_point'); + const filter = createExtentFilter(mapExtent, geoFieldName); expect(filter).toEqual({ geo_bounding_box: { location: { @@ -415,7 +415,7 @@ describe('createExtentFilter', () => { minLat: -100, minLon: -190, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_point'); + const filter = createExtentFilter(mapExtent, geoFieldName); expect(filter).toEqual({ geo_bounding_box: { location: { @@ -433,7 +433,7 @@ describe('createExtentFilter', () => { minLat: 35, minLon: 100, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_point'); + const filter = createExtentFilter(mapExtent, geoFieldName); const leftLon = filter.geo_bounding_box.location.top_left[0]; const rightLon = filter.geo_bounding_box.location.bottom_right[0]; expect(leftLon).toBeGreaterThan(rightLon); @@ -454,7 +454,7 @@ describe('createExtentFilter', () => { minLat: 35, minLon: -200, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_point'); + const filter = createExtentFilter(mapExtent, geoFieldName); const leftLon = filter.geo_bounding_box.location.top_left[0]; const rightLon = filter.geo_bounding_box.location.bottom_right[0]; expect(leftLon).toBeGreaterThan(rightLon); @@ -477,52 +477,30 @@ describe('createExtentFilter', () => { minLat: 35, minLon: -89, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_shape'); + const filter = createExtentFilter(mapExtent, geoFieldName); expect(filter).toEqual({ - geo_shape: { + geo_bounding_box: { location: { - relation: 'INTERSECTS', - shape: { - coordinates: [ - [ - [-89, 39], - [-89, 35], - [-83, 35], - [-83, 39], - [-89, 39], - ], - ], - type: 'Polygon', - }, + top_left: [-89, 39], + bottom_right: [-83, 35], }, }, }); }); - it('should clamp longitudes to -180 to 180 when lonitude wraps globe', () => { + it('should clamp longitudes to -180 to 180 when longitude wraps globe', () => { const mapExtent = { maxLat: 39, maxLon: 209, minLat: 35, minLon: -191, }; - const filter = createExtentFilter(mapExtent, geoFieldName, 'geo_shape'); + const filter = createExtentFilter(mapExtent, geoFieldName); expect(filter).toEqual({ - geo_shape: { + geo_bounding_box: { location: { - relation: 'INTERSECTS', - shape: { - coordinates: [ - [ - [-180, 39], - [-180, 35], - [180, 35], - [180, 39], - [-180, 39], - ], - ], - type: 'Polygon', - }, + top_left: [-180, 39], + bottom_right: [180, 35], }, }, }); From 40bcf31e67fe4a9345f6e88fb1bd6ad80e28a066 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Tue, 2 Mar 2021 11:29:36 -0700 Subject: [PATCH 3/4] Review feedback. Remove comment. Remove redundant tests --- .../elasticsearch_geo_utils.js | 5 - .../elasticsearch_geo_utils.test.js | 180 ++++++++---------- 2 files changed, 79 insertions(+), 106 deletions(-) diff --git a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js index cb80fa4401810..47de8850c0e96 100644 --- a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js +++ b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.js @@ -281,11 +281,6 @@ export function makeESBbox({ maxLat, maxLon, minLat, minLon }) { } export function createExtentFilter(mapExtent, geoFieldName) { - // Extent filters are used to dynamically filter data for the current map view port. - // Continue to use geo_bounding_box queries for extent filters - // 1) geo_bounding_box queries are faster than polygon queries - // 2) geo_shape benefits of pre-indexed shapes and - // compatibility across multi-indices with geo_point and geo_shape do not apply to this use case. const boundingBox = makeESBbox(mapExtent); return { geo_bounding_box: { diff --git a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js index c1925985a48b8..9983bb9b84588 100644 --- a/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js +++ b/x-pack/plugins/maps/common/elasticsearch_util/elasticsearch_geo_utils.test.js @@ -389,121 +389,99 @@ describe('geoShapeToGeometry', () => { }); describe('createExtentFilter', () => { - describe('geo_point field', () => { - it('should return elasticsearch geo_bounding_box filter for geo_point field', () => { - const mapExtent = { - maxLat: 39, - maxLon: -83, - minLat: 35, - minLon: -89, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [-89, 39], - bottom_right: [-83, 35], - }, - }, - }); - }); - - it('should clamp longitudes to -180 to 180 and latitudes to -90 to 90', () => { - const mapExtent = { - maxLat: 120, - maxLon: 200, - minLat: -100, - minLon: -190, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [-180, 89], - bottom_right: [180, -89], - }, + it('should return elasticsearch geo_bounding_box filter', () => { + const mapExtent = { + maxLat: 39, + maxLon: -83, + minLat: 35, + minLon: -89, + }; + const filter = createExtentFilter(mapExtent, geoFieldName); + expect(filter).toEqual({ + geo_bounding_box: { + location: { + top_left: [-89, 39], + bottom_right: [-83, 35], }, - }); + }, }); + }); - it('should make left longitude greater then right longitude when area crosses 180 meridian east to west', () => { - const mapExtent = { - maxLat: 39, - maxLon: 200, - minLat: 35, - minLon: 100, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - const leftLon = filter.geo_bounding_box.location.top_left[0]; - const rightLon = filter.geo_bounding_box.location.bottom_right[0]; - expect(leftLon).toBeGreaterThan(rightLon); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [100, 39], - bottom_right: [-160, 35], - }, + it('should clamp longitudes to -180 to 180 and latitudes to -90 to 90', () => { + const mapExtent = { + maxLat: 120, + maxLon: 200, + minLat: -100, + minLon: -190, + }; + const filter = createExtentFilter(mapExtent, geoFieldName); + expect(filter).toEqual({ + geo_bounding_box: { + location: { + top_left: [-180, 89], + bottom_right: [180, -89], }, - }); + }, }); + }); - it('should make left longitude greater then right longitude when area crosses 180 meridian west to east', () => { - const mapExtent = { - maxLat: 39, - maxLon: -100, - minLat: 35, - minLon: -200, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - const leftLon = filter.geo_bounding_box.location.top_left[0]; - const rightLon = filter.geo_bounding_box.location.bottom_right[0]; - expect(leftLon).toBeGreaterThan(rightLon); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [160, 39], - bottom_right: [-100, 35], - }, + it('should make left longitude greater then right longitude when area crosses 180 meridian east to west', () => { + const mapExtent = { + maxLat: 39, + maxLon: 200, + minLat: 35, + minLon: 100, + }; + const filter = createExtentFilter(mapExtent, geoFieldName); + const leftLon = filter.geo_bounding_box.location.top_left[0]; + const rightLon = filter.geo_bounding_box.location.bottom_right[0]; + expect(leftLon).toBeGreaterThan(rightLon); + expect(filter).toEqual({ + geo_bounding_box: { + location: { + top_left: [100, 39], + bottom_right: [-160, 35], }, - }); + }, }); }); - describe('geo_shape field', () => { - it('should return elasticsearch geo_shape filter', () => { - const mapExtent = { - maxLat: 39, - maxLon: -83, - minLat: 35, - minLon: -89, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [-89, 39], - bottom_right: [-83, 35], - }, + it('should make left longitude greater then right longitude when area crosses 180 meridian west to east', () => { + const mapExtent = { + maxLat: 39, + maxLon: -100, + minLat: 35, + minLon: -200, + }; + const filter = createExtentFilter(mapExtent, geoFieldName); + const leftLon = filter.geo_bounding_box.location.top_left[0]; + const rightLon = filter.geo_bounding_box.location.bottom_right[0]; + expect(leftLon).toBeGreaterThan(rightLon); + expect(filter).toEqual({ + geo_bounding_box: { + location: { + top_left: [160, 39], + bottom_right: [-100, 35], }, - }); + }, }); + }); - it('should clamp longitudes to -180 to 180 when longitude wraps globe', () => { - const mapExtent = { - maxLat: 39, - maxLon: 209, - minLat: 35, - minLon: -191, - }; - const filter = createExtentFilter(mapExtent, geoFieldName); - expect(filter).toEqual({ - geo_bounding_box: { - location: { - top_left: [-180, 39], - bottom_right: [180, 35], - }, + it('should clamp longitudes to -180 to 180 when longitude wraps globe', () => { + const mapExtent = { + maxLat: 39, + maxLon: 209, + minLat: 35, + minLon: -191, + }; + const filter = createExtentFilter(mapExtent, geoFieldName); + expect(filter).toEqual({ + geo_bounding_box: { + location: { + top_left: [-180, 39], + bottom_right: [180, 35], }, - }); + }, }); }); }); From e85097514bc4a1f3fbd0739459ebf826e56c3676 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Tue, 2 Mar 2021 11:46:41 -0700 Subject: [PATCH 4/4] Remove unused import --- .../plugins/maps/public/classes/sources/es_source/es_source.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts b/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts index 945369829cfdf..6b99f1f8860c0 100644 --- a/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts +++ b/x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts @@ -34,7 +34,7 @@ import { import { IVectorStyle } from '../../styles/vector/vector_style'; import { IDynamicStyleProperty } from '../../styles/vector/properties/dynamic_style_property'; import { IField } from '../../fields/field'; -import { ES_GEO_FIELD_TYPE, FieldFormatter } from '../../../../common/constants'; +import { FieldFormatter } from '../../../../common/constants'; import { Adapters, RequestResponder,