From 1252b832f45ca02af17543040e2ea2be0dc6fecf Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 27 Apr 2020 14:03:47 -0600 Subject: [PATCH] [Maps] geo_shape fit to bounds (#64303) * [Maps] geo_shape fit to bounds * handle results that cross dateline Co-authored-by: Elastic Machine --- .../layers/sources/es_source/es_source.js | 11 +- .../public/layers/util/can_skip_fetch.test.js | 149 +++++++----------- .../maps/public/layers/util/can_skip_fetch.ts | 16 +- .../maps/public/layers/vector_layer.js | 3 +- 4 files changed, 72 insertions(+), 107 deletions(-) diff --git a/x-pack/plugins/maps/public/layers/sources/es_source/es_source.js b/x-pack/plugins/maps/public/layers/sources/es_source/es_source.js index ccd8bc4a859d..87733e347aa2 100644 --- a/x-pack/plugins/maps/public/layers/sources/es_source/es_source.js +++ b/x-pack/plugins/maps/public/layers/sources/es_source/es_source.js @@ -18,7 +18,6 @@ import { i18n } from '@kbn/i18n'; import uuid from 'uuid/v4'; import { copyPersistentState } from '../../../reducers/util'; -import { ES_GEO_FIELD_TYPE } from '../../../../common/constants'; import { DataRequestAbortError } from '../../util/data_request'; import { expandToTileBoundaries } from '../es_geo_grid_source/geo_tile_utils'; @@ -176,9 +175,11 @@ export class AbstractESSource extends AbstractVectorSource { }; } + const minLon = esBounds.top_left.lon; + const maxLon = esBounds.bottom_right.lon; return { - minLon: esBounds.top_left.lon, - maxLon: esBounds.bottom_right.lon, + minLon: minLon > maxLon ? minLon - 360 : minLon, + maxLon, minLat: esBounds.bottom_right.lat, maxLat: esBounds.top_left.lat, }; @@ -223,9 +224,7 @@ export class AbstractESSource extends AbstractVectorSource { async supportsFitToBounds() { try { const geoField = await this._getGeoField(); - // geo_bounds aggregation only supports geo_point - // there is currently no backend support for getting bounding box of geo_shape field - return geoField.type !== ES_GEO_FIELD_TYPE.GEO_SHAPE; + return geoField.aggregatable; } catch (error) { return false; } diff --git a/x-pack/plugins/maps/public/layers/util/can_skip_fetch.test.js b/x-pack/plugins/maps/public/layers/util/can_skip_fetch.test.js index 2a4843c78635..bf1f9de6b2d2 100644 --- a/x-pack/plugins/maps/public/layers/util/can_skip_fetch.test.js +++ b/x-pack/plugins/maps/public/layers/util/can_skip_fetch.test.js @@ -8,101 +8,74 @@ import { canSkipSourceUpdate, updateDueToExtent } from './can_skip_fetch'; import { DataRequest } from './data_request'; describe('updateDueToExtent', () => { - it('should be false when the source is not extent aware', async () => { - const sourceMock = { - isFilterByMapBounds: () => { - return false; - }, + it('should be false when buffers are the same', async () => { + const oldBuffer = { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, }; - expect(updateDueToExtent(sourceMock)).toBe(false); + const newBuffer = { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, + }; + expect(updateDueToExtent({ buffer: oldBuffer }, { buffer: newBuffer })).toBe(false); }); - describe('source is extent aware', () => { - const sourceMock = { - isFilterByMapBounds: () => { - return true; - }, + it('should be false when the new buffer is contained in the old buffer', async () => { + const oldBuffer = { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, }; + const newBuffer = { + maxLat: 10, + maxLon: 100, + minLat: 5, + minLon: 95, + }; + expect(updateDueToExtent({ buffer: oldBuffer }, { buffer: newBuffer })).toBe(false); + }); - it('should be false when buffers are the same', async () => { - const oldBuffer = { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, - }; - const newBuffer = { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, - }; - expect(updateDueToExtent(sourceMock, { buffer: oldBuffer }, { buffer: newBuffer })).toBe( - false - ); - }); + it('should be true when the new buffer is contained in the old buffer and the past results were truncated', async () => { + const oldBuffer = { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, + }; + const newBuffer = { + maxLat: 10, + maxLon: 100, + minLat: 5, + minLon: 95, + }; + expect( + updateDueToExtent({ buffer: oldBuffer, areResultsTrimmed: true }, { buffer: newBuffer }) + ).toBe(true); + }); - it('should be false when the new buffer is contained in the old buffer', async () => { - const oldBuffer = { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, - }; - const newBuffer = { - maxLat: 10, - maxLon: 100, - minLat: 5, - minLon: 95, - }; - expect(updateDueToExtent(sourceMock, { buffer: oldBuffer }, { buffer: newBuffer })).toBe( - false - ); - }); + it('should be true when meta has no old buffer', async () => { + expect(updateDueToExtent()).toBe(true); + }); - it('should be true when the new buffer is contained in the old buffer and the past results were truncated', async () => { - const oldBuffer = { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, - }; - const newBuffer = { - maxLat: 10, - maxLon: 100, - minLat: 5, - minLon: 95, - }; - expect( - updateDueToExtent( - sourceMock, - { buffer: oldBuffer, areResultsTrimmed: true }, - { buffer: newBuffer } - ) - ).toBe(true); - }); - - it('should be true when meta has no old buffer', async () => { - expect(updateDueToExtent(sourceMock)).toBe(true); - }); - - it('should be true when the new buffer is not contained in the old buffer', async () => { - const oldBuffer = { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, - }; - const newBuffer = { - maxLat: 7.5, - maxLon: 92.5, - minLat: -2.5, - minLon: 82.5, - }; - expect(updateDueToExtent(sourceMock, { buffer: oldBuffer }, { buffer: newBuffer })).toBe( - true - ); - }); + it('should be true when the new buffer is not contained in the old buffer', async () => { + const oldBuffer = { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, + }; + const newBuffer = { + maxLat: 7.5, + maxLon: 92.5, + minLat: -2.5, + minLon: 82.5, + }; + expect(updateDueToExtent({ buffer: oldBuffer }, { buffer: newBuffer })).toBe(true); }); }); diff --git a/x-pack/plugins/maps/public/layers/util/can_skip_fetch.ts b/x-pack/plugins/maps/public/layers/util/can_skip_fetch.ts index 758cc35f41fb..8398bd7af39a 100644 --- a/x-pack/plugins/maps/public/layers/util/can_skip_fetch.ts +++ b/x-pack/plugins/maps/public/layers/util/can_skip_fetch.ts @@ -15,16 +15,7 @@ import { DataRequest } from './data_request'; const SOURCE_UPDATE_REQUIRED = true; const NO_SOURCE_UPDATE_REQUIRED = false; -export function updateDueToExtent( - source: ISource, - prevMeta: DataMeta = {}, - nextMeta: DataMeta = {} -) { - const extentAware = source.isFilterByMapBounds(); - if (!extentAware) { - return NO_SOURCE_UPDATE_REQUIRED; - } - +export function updateDueToExtent(prevMeta: DataMeta = {}, nextMeta: DataMeta = {}) { const { buffer: previousBuffer } = prevMeta; const { buffer: newBuffer } = nextMeta; @@ -134,7 +125,10 @@ export async function canSkipSourceUpdate({ updateDueToPrecisionChange = !_.isEqual(prevMeta.geogridPrecision, nextMeta.geogridPrecision); } - const updateDueToExtentChange = updateDueToExtent(source, prevMeta, nextMeta); + let updateDueToExtentChange = false; + if (extentAware) { + updateDueToExtentChange = updateDueToExtent(prevMeta, nextMeta); + } const updateDueToSourceMetaChange = !_.isEqual(prevMeta.sourceMeta, nextMeta.sourceMeta); diff --git a/x-pack/plugins/maps/public/layers/vector_layer.js b/x-pack/plugins/maps/public/layers/vector_layer.js index 17b7f8152d76..582e34bce2e9 100644 --- a/x-pack/plugins/maps/public/layers/vector_layer.js +++ b/x-pack/plugins/maps/public/layers/vector_layer.js @@ -175,8 +175,7 @@ export class VectorLayer extends AbstractLayer { } async getBounds(dataFilters) { - const isStaticLayer = - !this.getSource().isBoundsAware() || !this.getSource().isFilterByMapBounds(); + const isStaticLayer = !this.getSource().isBoundsAware(); if (isStaticLayer) { return this._getBoundsBasedOnData(); }