[Maps] Fix fit to bounds requests not getting canceled (elastic#67629)

* rename data request constants * register cancel callback * clean up
patrykkopycinski · May 29, 2020 · e28028b · e28028b
1 parent 043ecac
commit e28028b
Show file tree

Hide file tree

Showing 17 changed files with 219 additions and 147 deletions.
diff --git a/x-pack/plugins/maps/common/constants.ts b/x-pack/plugins/maps/common/constants.ts
@@ -72,11 +72,12 @@ export enum FIELD_ORIGIN {
 }
 export const JOIN_FIELD_NAME_PREFIX = '__kbnjoin__';
 
-export const SOURCE_DATA_ID_ORIGIN = 'source';
-export const META_ID_ORIGIN_SUFFIX = 'meta';
-export const SOURCE_META_ID_ORIGIN = `${SOURCE_DATA_ID_ORIGIN}_${META_ID_ORIGIN_SUFFIX}`;
-export const FORMATTERS_ID_ORIGIN_SUFFIX = 'formatters';
-export const SOURCE_FORMATTERS_ID_ORIGIN = `${SOURCE_DATA_ID_ORIGIN}_${FORMATTERS_ID_ORIGIN_SUFFIX}`;
+export const META_DATA_REQUEST_ID_SUFFIX = 'meta';
+export const FORMATTERS_DATA_REQUEST_ID_SUFFIX = 'formatters';
+export const SOURCE_DATA_REQUEST_ID = 'source';
+export const SOURCE_META_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_${META_DATA_REQUEST_ID_SUFFIX}`;
+export const SOURCE_FORMATTERS_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_${FORMATTERS_DATA_REQUEST_ID_SUFFIX}`;
+export const SOURCE_BOUNDS_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_bounds`;
 
 export const MIN_ZOOM = 0;
 export const MAX_ZOOM = 24;

diff --git a/x-pack/plugins/maps/public/actions/data_request_actions.ts b/x-pack/plugins/maps/public/actions/data_request_actions.ts
@@ -6,12 +6,15 @@
 /* eslint-disable @typescript-eslint/consistent-type-definitions */
 
 import { Dispatch } from 'redux';
+// @ts-ignore
+import turf from 'turf';
 import { FeatureCollection } from 'geojson';
 import { MapStoreState } from '../reducers/store';
-import { LAYER_TYPE, SOURCE_DATA_ID_ORIGIN } from '../../common/constants';
+import { LAYER_TYPE, SOURCE_DATA_REQUEST_ID } from '../../common/constants';
 import {
   getDataFilters,
   getDataRequestDescriptor,
+  getFittableLayers,
   getLayerById,
   getLayerList,
 } from '../selectors/map_selectors';
@@ -27,13 +30,15 @@ import {
   LAYER_DATA_LOAD_ENDED,
   LAYER_DATA_LOAD_ERROR,
   LAYER_DATA_LOAD_STARTED,
+  SET_GOTO,
   SET_LAYER_ERROR_STATUS,
   SET_LAYER_STYLE_META,
   UPDATE_LAYER_PROP,
   UPDATE_SOURCE_DATA_REQUEST,
 } from './map_action_constants';
 import { ILayer } from '../classes/layers/layer';
-import { DataMeta, MapFilters } from '../../common/descriptor_types';
+import { DataMeta, MapExtent, MapFilters } from '../../common/descriptor_types';
+import { DataRequestAbortError } from '../classes/util/data_request';
 
 export type DataRequestContext = {
   startLoading(dataId: string, requestToken: symbol, meta: DataMeta): void;
@@ -269,11 +274,107 @@ export function updateSourceDataRequest(layerId: string, newData: unknown) {
   return (dispatch: Dispatch) => {
     dispatch({
       type: UPDATE_SOURCE_DATA_REQUEST,
-      dataId: SOURCE_DATA_ID_ORIGIN,
+      dataId: SOURCE_DATA_REQUEST_ID,
       layerId,
       newData,
     });
 
     dispatch<any>(updateStyleMeta(layerId));
   };
 }
+
+export function fitToLayerExtent(layerId: string) {
+  return async (dispatch: Dispatch, getState: () => MapStoreState) => {
+    const targetLayer = getLayerById(layerId, getState());
+
+    if (targetLayer) {
+      try {
+        const bounds = await targetLayer.getBounds(
+          getDataRequestContext(dispatch, getState, layerId)
+        );
+        if (bounds) {
+          await dispatch(setGotoWithBounds(bounds));
+        }
+      } catch (error) {
+        if (!(error instanceof DataRequestAbortError)) {
+          // eslint-disable-next-line no-console
+          console.warn(
+            'Unhandled getBounds error for layer. Only DataRequestAbortError should be surfaced',
+            error
+          );
+        }
+        // new fitToLayerExtent request has superseded this thread of execution. Results no longer needed.
+        return;
+      }
+    }
+  };
+}
+
+export function fitToDataBounds() {
+  return async (dispatch: Dispatch, getState: () => MapStoreState) => {
+    const layerList = getFittableLayers(getState());
+
+    if (!layerList.length) {
+      return;
+    }
+
+    const boundsPromises = layerList.map(async (layer: ILayer) => {
+      return layer.getBounds(getDataRequestContext(dispatch, getState, layer.getId()));
+    });
+
+    let bounds;
+    try {
+      bounds = await Promise.all(boundsPromises);
+    } catch (error) {
+      if (!(error instanceof DataRequestAbortError)) {
+        // eslint-disable-next-line no-console
+        console.warn(
+          'Unhandled getBounds error for layer. Only DataRequestAbortError should be surfaced',
+          error
+        );
+      }
+      // new fitToDataBounds request has superseded this thread of execution. Results no longer needed.
+      return;
+    }
+
+    const corners = [];
+    for (let i = 0; i < bounds.length; i++) {
+      const b = bounds[i];
+
+      // filter out undefined bounds (uses Infinity due to turf responses)
+      if (
+        b === null ||
+        b.minLon === Infinity ||
+        b.maxLon === Infinity ||
+        b.minLat === -Infinity ||
+        b.maxLat === -Infinity
+      ) {
+        continue;
+      }
+
+      corners.push([b.minLon, b.minLat]);
+      corners.push([b.maxLon, b.maxLat]);
+    }
+
+    if (!corners.length) {
+      return;
+    }
+
+    const turfUnionBbox = turf.bbox(turf.multiPoint(corners));
+    const dataBounds = {
+      minLon: turfUnionBbox[0],
+      minLat: turfUnionBbox[1],
+      maxLon: turfUnionBbox[2],
+      maxLat: turfUnionBbox[3],
+    };
+
+    dispatch(setGotoWithBounds(dataBounds));
+  };
+}
+
+function setGotoWithBounds(bounds: MapExtent) {
+  return {
+    type: SET_GOTO,
+    bounds,
+  };
+}
diff --git a/x-pack/plugins/maps/public/actions/index.ts b/x-pack/plugins/maps/public/actions/index.ts
@@ -9,7 +9,12 @@ export * from './ui_actions';
 export * from './map_actions';
 export * from './map_action_constants';
 export * from './layer_actions';
-export { cancelAllInFlightRequests, DataRequestContext } from './data_request_actions';
+export {
+  cancelAllInFlightRequests,
+  DataRequestContext,
+  fitToLayerExtent,
+  fitToDataBounds,
+} from './data_request_actions';
 export {
   closeOnClickTooltip,
   openOnClickTooltip,

diff --git a/x-pack/plugins/maps/public/actions/map_actions.ts b/x-pack/plugins/maps/public/actions/map_actions.ts
@@ -12,11 +12,9 @@ import turfBooleanContains from '@turf/boolean-contains';
 import { Filter, Query, TimeRange } from 'src/plugins/data/public';
 import { MapStoreState } from '../reducers/store';
 import {
-  getLayerById,
   getDataFilters,
   getWaitingForMapReadyLayerListRaw,
   getQuery,
-  getFittableLayers,
 } from '../selectors/map_selectors';
 import {
   CLEAR_GOTO,
@@ -184,76 +182,6 @@ export function disableScrollZoom() {
   return { type: SET_SCROLL_ZOOM, scrollZoom: false };
 }
 
-export function fitToLayerExtent(layerId: string) {
-  return async (dispatch: Dispatch, getState: () => MapStoreState) => {
-    const targetLayer = getLayerById(layerId, getState());
-
-    if (targetLayer) {
-      const dataFilters = getDataFilters(getState());
-      const bounds = await targetLayer.getBounds(dataFilters);
-      if (bounds) {
-        await dispatch(setGotoWithBounds(bounds));
-      }
-    }
-  };
-}
-
-export function fitToDataBounds() {
-  return async (dispatch: Dispatch, getState: () => MapStoreState) => {
-    const layerList = getFittableLayers(getState());
-
-    if (!layerList.length) {
-      return;
-    }
-
-    const dataFilters = getDataFilters(getState());
-    const boundsPromises = layerList.map(async (layer) => {
-      return layer.getBounds(dataFilters);
-    });
-
-    const bounds = await Promise.all(boundsPromises);
-    const corners = [];
-    for (let i = 0; i < bounds.length; i++) {
-      const b = bounds[i];
-
-      // filter out undefined bounds (uses Infinity due to turf responses)
-      if (
-        b === null ||
-        b.minLon === Infinity ||
-        b.maxLon === Infinity ||
-        b.minLat === -Infinity ||
-        b.maxLat === -Infinity
-      ) {
-        continue;
-      }
-
-      corners.push([b.minLon, b.minLat]);
-      corners.push([b.maxLon, b.maxLat]);
-    }
-
-    if (!corners.length) {
-      return;
-    }
-
-    const turfUnionBbox = turf.bbox(turf.multiPoint(corners));
-    const dataBounds = {
-      minLon: turfUnionBbox[0],
-      minLat: turfUnionBbox[1],
-      maxLon: turfUnionBbox[2],
-      maxLat: turfUnionBbox[3],
-    };
-
-    dispatch(setGotoWithBounds(dataBounds));
-  };
-}
-
-export function setGotoWithBounds(bounds: MapExtent) {
-  return {
-    type: SET_GOTO,
-    bounds,
-  };
-}
-
 export function setGotoWithCenter({ lat, lon, zoom }: MapCenterAndZoom) {
   return {
     type: SET_GOTO,

diff --git a/x-pack/plugins/maps/public/classes/joins/inner_join.js b/x-pack/plugins/maps/public/classes/joins/inner_join.js
@@ -6,7 +6,10 @@
 
 import { ESTermSource } from '../sources/es_term_source';
 import { getComputedFieldNamePrefix } from '../styles/vector/style_util';
-import { META_ID_ORIGIN_SUFFIX, FORMATTERS_ID_ORIGIN_SUFFIX } from '../../../common/constants';
+import {
+  META_DATA_REQUEST_ID_SUFFIX,
+  FORMATTERS_DATA_REQUEST_ID_SUFFIX,
+} from '../../../common/constants';
 
 export class InnerJoin {
   constructor(joinDescriptor, leftSource) {
@@ -42,11 +45,11 @@ export class InnerJoin {
   }
 
   getSourceMetaDataRequestId() {
-    return `${this.getSourceDataRequestId()}_${META_ID_ORIGIN_SUFFIX}`;
+    return `${this.getSourceDataRequestId()}_${META_DATA_REQUEST_ID_SUFFIX}`;
   }
 
   getSourceFormattersDataRequestId() {
-    return `${this.getSourceDataRequestId()}_${FORMATTERS_ID_ORIGIN_SUFFIX}`;
+    return `${this.getSourceDataRequestId()}_${FORMATTERS_DATA_REQUEST_ID_SUFFIX}`;
   }
 
   getLeftField() {

diff --git a/x-pack/plugins/maps/public/classes/layers/layer.tsx b/x-pack/plugins/maps/public/classes/layers/layer.tsx
@@ -17,21 +17,16 @@ import {
   MAX_ZOOM,
   MB_SOURCE_ID_LAYER_ID_PREFIX_DELIMITER,
   MIN_ZOOM,
-  SOURCE_DATA_ID_ORIGIN,
+  SOURCE_DATA_REQUEST_ID,
 } from '../../../common/constants';
 import { copyPersistentState } from '../../reducers/util';
-import {
-  LayerDescriptor,
-  MapExtent,
-  MapFilters,
-  StyleDescriptor,
-} from '../../../common/descriptor_types';
+import { LayerDescriptor, MapExtent, StyleDescriptor } from '../../../common/descriptor_types';
 import { Attribution, ImmutableSourceProperty, ISource, SourceEditorArgs } from '../sources/source';
 import { DataRequestContext } from '../../actions';
 import { IStyle } from '../styles/style';
 
 export interface ILayer {
-  getBounds(mapFilters: MapFilters): Promise<MapExtent>;
+  getBounds(dataRequestContext: DataRequestContext): Promise<MapExtent | null>;
   getDataRequest(id: string): DataRequest | undefined;
   getDisplayName(source?: ISource): Promise<string>;
   getId(): string;
@@ -401,7 +396,7 @@ export class AbstractLayer implements ILayer {
   }
 
   getSourceDataRequest(): DataRequest | undefined {
-    return this.getDataRequest(SOURCE_DATA_ID_ORIGIN);
+    return this.getDataRequest(SOURCE_DATA_REQUEST_ID);
   }
 
   getDataRequest(id: string): DataRequest | undefined {
@@ -455,13 +450,8 @@ export class AbstractLayer implements ILayer {
     return sourceDataRequest ? sourceDataRequest.hasData() : false;
   }
 
-  async getBounds(mapFilters: MapFilters): Promise<MapExtent> {
-    return {
-      minLon: -180,
-      maxLon: 180,
-      minLat: -89,
-      maxLat: 89,
-    };
+  async getBounds(dataRequestContext: DataRequestContext): Promise<MapExtent | null> {
+    return null;
   }
 
   renderStyleEditor({

diff --git a/x-pack/plugins/maps/public/classes/layers/tile_layer/tile_layer.js b/x-pack/plugins/maps/public/classes/layers/tile_layer/tile_layer.js
@@ -6,7 +6,7 @@
 
 import { AbstractLayer } from '../layer';
 import _ from 'lodash';
-import { SOURCE_DATA_ID_ORIGIN, LAYER_TYPE, LAYER_STYLE_TYPE } from '../../../../common/constants';
+import { SOURCE_DATA_REQUEST_ID, LAYER_TYPE, LAYER_STYLE_TYPE } from '../../../../common/constants';
 import { TileStyle } from '../../styles/tile/tile_style';
 
 export class TileLayer extends AbstractLayer {
@@ -31,12 +31,12 @@ export class TileLayer extends AbstractLayer {
       return;
     }
     const requestToken = Symbol(`layer-source-refresh:${this.getId()} - source`);
-    startLoading(SOURCE_DATA_ID_ORIGIN, requestToken, dataFilters);
+    startLoading(SOURCE_DATA_REQUEST_ID, requestToken, dataFilters);
     try {
       const url = await this.getSource().getUrlTemplate();
-      stopLoading(SOURCE_DATA_ID_ORIGIN, requestToken, url, {});
+      stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, url, {});
     } catch (error) {
-      onLoadError(SOURCE_DATA_ID_ORIGIN, requestToken, error.message);
+      onLoadError(SOURCE_DATA_REQUEST_ID, requestToken, error.message);
     }
   }
 

diff --git a/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.tsx b/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.tsx
@@ -7,7 +7,7 @@
 import React from 'react';
 import { EuiIcon } from '@elastic/eui';
 import { VectorStyle } from '../../styles/vector/vector_style';
-import { SOURCE_DATA_ID_ORIGIN, LAYER_TYPE } from '../../../../common/constants';
+import { SOURCE_DATA_REQUEST_ID, LAYER_TYPE } from '../../../../common/constants';
 import { VectorLayer, VectorLayerArguments } from '../vector_layer/vector_layer';
 import { canSkipSourceUpdate } from '../../util/can_skip_fetch';
 import { ITiledSingleLayerVectorSource } from '../../sources/vector_source';
@@ -56,7 +56,7 @@ export class TiledVectorLayer extends VectorLayer {
     onLoadError,
     dataFilters,
   }: DataRequestContext) {
-    const requestToken: symbol = Symbol(`layer-${this.getId()}-${SOURCE_DATA_ID_ORIGIN}`);
+    const requestToken: symbol = Symbol(`layer-${this.getId()}-${SOURCE_DATA_REQUEST_ID}`);
     const searchFilters: VectorSourceRequestMeta = this._getSearchFilters(
       dataFilters,
       this.getSource(),
@@ -73,12 +73,12 @@ export class TiledVectorLayer extends VectorLayer {
       return null;
     }
 
-    startLoading(SOURCE_DATA_ID_ORIGIN, requestToken, searchFilters);
+    startLoading(SOURCE_DATA_REQUEST_ID, requestToken, searchFilters);
     try {
       const templateWithMeta = await this._source.getUrlTemplateWithMeta();
-      stopLoading(SOURCE_DATA_ID_ORIGIN, requestToken, templateWithMeta, {});
+      stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, templateWithMeta, {});
     } catch (error) {
-      onLoadError(SOURCE_DATA_ID_ORIGIN, requestToken, error.message);
+      onLoadError(SOURCE_DATA_REQUEST_ID, requestToken, error.message);
     }
   }