From 7a0ffa8459cc2304976911dce8cf17a5e3da5808 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Tue, 25 Feb 2020 11:11:48 -0700 Subject: [PATCH] review feedback --- ...son.test.js => convert_to_geojson.test.ts} | 0 .../es_geo_grid_source/es_geo_grid_source.js | 249 ++++++++++-------- .../es_geo_grid_source/geo_tile_utils.js | 14 - ...lines.test.js => convert_to_lines.test.ts} | 0 4 files changed, 143 insertions(+), 120 deletions(-) rename x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/{convert_to_geojson.test.js => convert_to_geojson.test.ts} (100%) rename x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/{convert_to_lines.test.js => convert_to_lines.test.ts} (100%) diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.test.js b/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.test.ts similarity index 100% rename from x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.test.js rename to x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.test.ts diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js b/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js index ae1b1ebaa4557..9906c10ede001 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js @@ -164,126 +164,163 @@ export class ESGeoGridSource extends AbstractESAggSource { ); } - async getGeoJsonWithMeta(layerName, searchFilters, registerCancelCallback, isRequestStillActive) { - const indexPattern = await this.getIndexPattern(); - const searchSource = await this._makeSearchSource(searchFilters, 0); - - let bucketsPerGrid = 1; - this.getMetricFields().forEach(metricField => { - if (metricField.getAggType() === AGG_TYPE.TERMS) { - // each terms aggregation increases the overall number of buckets per grid - bucketsPerGrid++; - } - }); - - const features = []; - // Do not use composite aggregation when there are no terms sub-aggregations - // see https://github.com/elastic/kibana/pull/57875#issuecomment-590515482 for explanation on using separate code paths - if (bucketsPerGrid === 1) { - searchSource.setField('aggs', { - gridSplit: { - geotile_grid: { - field: this._descriptor.geoField, - precision: searchFilters.geogridPrecision, - }, - aggs: { - gridCentroid: { - geo_centroid: { - field: this._descriptor.geoField, + async compositeAggRequest({ + searchSource, + indexPattern, + precision, + layerName, + registerCancelCallback, + bucketsPerGrid, + isRequestStillActive, + }) { + const gridsPerRequest = Math.floor(DEFAULT_MAX_BUCKETS_LIMIT / bucketsPerGrid); + const aggs = { + compositeSplit: { + composite: { + size: gridsPerRequest, + sources: [ + { + gridSplit: { + geotile_grid: { + field: this._descriptor.geoField, + precision, + }, }, }, - ...this.getValueAggsDsl(indexPattern), + ], + }, + aggs: { + gridCentroid: { + geo_centroid: { + field: this._descriptor.geoField, + }, }, + ...this.getValueAggsDsl(indexPattern), }, - }); + }, + }; + + const features = []; + let requestCount = 0; + let afterKey = null; + while (true) { + if (!isRequestStillActive()) { + // Stop paging through results if request is obsolete + throw new DataRequestAbortError(); + } + + requestCount++; + + // circuit breaker to ensure reasonable number of requests + if (requestCount > 5) { + throw new Error( + i18n.translate('xpack.maps.source.esGrid.compositePaginationErrorMessage', { + defaultMessage: `{layerName} is causing too many requests. Reduce "Grid resolution" and/or reduce the number of top term "Metrics".`, + values: { layerName }, + }) + ); + } + if (afterKey) { + aggs.compositeSplit.composite.after = afterKey; + } + searchSource.setField('aggs', aggs); + const requestId = afterKey ? `${this.getId()} afterKey ${afterKey.geoSplit}` : this.getId(); const esResponse = await this._runEsQuery({ - requestId: this.getId(), - requestName: layerName, + requestId, + requestName: `${layerName} (${requestCount})`, searchSource, registerCancelCallback, - requestDescription: i18n.translate('xpack.maps.source.esGrid.inspectorDescription', { - defaultMessage: 'Elasticsearch geo grid aggregation request', - }), + requestDescription: i18n.translate( + 'xpack.maps.source.esGrid.compositeInspectorDescription', + { + defaultMessage: 'Elasticsearch geo grid aggregation request: {requestId}', + values: { requestId }, + } + ), }); - features.push(...convertRegularRespToGeoJson(esResponse, this._descriptor.requestType)); - } else { - const gridsPerRequest = Math.floor(DEFAULT_MAX_BUCKETS_LIMIT / bucketsPerGrid); - const aggs = { - compositeSplit: { - composite: { - size: gridsPerRequest, - sources: [ - { - gridSplit: { - geotile_grid: { - field: this._descriptor.geoField, - precision: searchFilters.geogridPrecision, - }, - }, - }, - ], - }, - aggs: { - gridCentroid: { - geo_centroid: { - field: this._descriptor.geoField, - }, + features.push(...convertCompositeRespToGeoJson(esResponse, this._descriptor.requestType)); + + afterKey = esResponse.aggregations.compositeSplit.after_key; + if (esResponse.aggregations.compositeSplit.buckets.length < gridsPerRequest) { + // Finished because request did not get full resultset back + break; + } + } + + return features; + } + + // Do not use composite aggregation when there are no terms sub-aggregations + // see https://github.com/elastic/kibana/pull/57875#issuecomment-590515482 for explanation on using separate code paths + async nonCompositeAggRequest({ + searchSource, + indexPattern, + precision, + layerName, + registerCancelCallback, + }) { + searchSource.setField('aggs', { + gridSplit: { + geotile_grid: { + field: this._descriptor.geoField, + precision, + }, + aggs: { + gridCentroid: { + geo_centroid: { + field: this._descriptor.geoField, }, - ...this.getValueAggsDsl(indexPattern), }, + ...this.getValueAggsDsl(indexPattern), }, - }; - - let requestCount = 0; - let afterKey = null; - while (true) { - if (!isRequestStillActive()) { - // Stop paging through results if request is obsolete - throw new DataRequestAbortError(); - } - - requestCount++; - - // circuit breaker to ensure reasonable number of requests - if (requestCount > 5) { - throw new Error( - i18n.translate('xpack.maps.source.esGrid.compositePaginationErrorMessage', { - defaultMessage: `{layerName} is causing too many requests. Reduce "Grid resolution" and/or reduce the number of top term "Metrics".`, - values: { layerName }, - }) - ); - } - - if (afterKey) { - aggs.compositeSplit.composite.after = afterKey; - } - searchSource.setField('aggs', aggs); - const requestId = afterKey ? `${this.getId()} afterKey ${afterKey.geoSplit}` : this.getId(); - const esResponse = await this._runEsQuery({ - requestId, - requestName: `${layerName} (${requestCount})`, - searchSource, - registerCancelCallback, - requestDescription: i18n.translate( - 'xpack.maps.source.esGrid.compositeInspectorDescription', - { - defaultMessage: 'Elasticsearch geo grid aggregation request: {requestId}', - values: { requestId }, - } - ), - }); - - features.push(...convertCompositeRespToGeoJson(esResponse, this._descriptor.requestType)); - - afterKey = esResponse.aggregations.compositeSplit.after_key; - if (esResponse.aggregations.compositeSplit.buckets.length < gridsPerRequest) { - // Finished because request did not get full resultset back - break; - } + }, + }); + + const esResponse = await this._runEsQuery({ + requestId: this.getId(), + requestName: layerName, + searchSource, + registerCancelCallback, + requestDescription: i18n.translate('xpack.maps.source.esGrid.inspectorDescription', { + defaultMessage: 'Elasticsearch geo grid aggregation request', + }), + }); + + return convertRegularRespToGeoJson(esResponse, this._descriptor.requestType); + } + + async getGeoJsonWithMeta(layerName, searchFilters, registerCancelCallback, isRequestStillActive) { + const indexPattern = await this.getIndexPattern(); + const searchSource = await this._makeSearchSource(searchFilters, 0); + + let bucketsPerGrid = 1; + this.getMetricFields().forEach(metricField => { + if (metricField.getAggType() === AGG_TYPE.TERMS) { + // each terms aggregation increases the overall number of buckets per grid + bucketsPerGrid++; } - } + }); + + const features = + bucketsPerGrid === 1 + ? await this.nonCompositeAggRequest({ + searchSource, + indexPattern, + precision: searchFilters.geogridPrecision, + layerName, + registerCancelCallback, + }) + : await this.compositeAggRequest({ + searchSource, + indexPattern, + precision: searchFilters.geogridPrecision, + layerName, + registerCancelCallback, + bucketsPerGrid, + isRequestStillActive, + }); return { data: { diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js b/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js index e1d998ea8d506..73039bf3bfc9f 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js @@ -118,17 +118,3 @@ export function expandToTileBoundaries(extent, zoom) { maxLat: tileToLatitude(upperLeftY, tileCount), }; } - -export function getTileCountInExtent(extent, zoom) { - const tileCount = getTileCount(zoom); - - const upperLeftX = longitudeToTile(extent.minLon, tileCount); - const upperLeftY = latitudeToTile(Math.min(extent.maxLat, 90), tileCount); - const lowerRightX = longitudeToTile(extent.maxLon, tileCount); - const lowerRightY = latitudeToTile(Math.max(extent.minLat, -90), tileCount); - - const tilesY = Math.abs(upperLeftY - lowerRightY); - const tilesX = Math.abs(upperLeftX - lowerRightX); - - return tilesX * tilesY; -} diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.test.js b/x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.test.ts similarity index 100% rename from x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.test.js rename to x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.test.ts