Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Maps] Do not check count for blended layers when layer is not visible #66460

Merged
merged 11 commits into from
May 15, 2020
Merged

[Maps] Do not check count for blended layers when layer is not visible #66460

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6977a16
[Maps] Do not check count for blended layers when layer is not visible
nreese May 13, 2020
29d4efe
Merge branch 'master' of github.com:elastic/kibana into no_count_check
nreese May 14, 2020
4bd7c9c
move visibility logic to map_actions where syncData is called
nreese May 14, 2020
5dada11
clean up
nreese May 14, 2020
3f6e75d
fix syntax error
nreese May 14, 2020
8dee6ca
remove async action
nreese May 14, 2020
af8ff7d
make syncDataForAllLayers a proper redux action
nreese May 14, 2020
382a3ce
simplify
nreese May 14, 2020
7ac07f7
fix functional test
nreese May 14, 2020
98bac55
Merge branch 'master' into no_count_check
elasticmachine May 14, 2020
3f11520
Merge branch 'master' into no_count_check
elasticmachine May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 43 additions & 38 deletions x-pack/plugins/maps/public/actions/map_actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,36 @@ function getLayerById(layerId, state) {
});
}

async function syncDataForAllLayers(dispatch, getState, dataFilters) {
const state = getState();
const layerList = getLayerList(state);
const syncs = layerList.map(layer => {
const loadingFunctions = getLayerLoadingCallbacks(dispatch, getState, layer.getId());
return layer.syncData({ ...loadingFunctions, dataFilters });
});
await Promise.all(syncs);
function syncDataForAllLayers() {
return async (dispatch, getState) => {
const syncPromises = getLayerList(getState()).map(async layer => {
return dispatch(syncDataForLayer(layer));
});
await Promise.all(syncPromises);
};
}

function syncDataForLayer(layer) {
return async (dispatch, getState) => {
const dataFilters = getDataFilters(getState());
if (!layer.isVisible() || !layer.showAtZoomLevel(dataFilters.zoom)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for pulling this out here. it really helps when the business-logic is gathered in the actions rather than sprinkled across multiple classes.

return;
}

await layer.syncData({
...getLayerLoadingCallbacks(dispatch, getState, layer.getId()),
dataFilters,
});
};
}

function syncDataForLayerId(layerId) {
return async (dispatch, getState) => {
const layer = getLayerById(layerId, getState());
if (layer) {
dispatch(syncDataForLayer(layer));
}
};
}

export function cancelAllInFlightRequests() {
Expand Down Expand Up @@ -192,7 +214,7 @@ export function rollbackToTrackedLayerStateForSelectedLayer() {
// syncDataForLayer may not trigger endDataLoad if no re-fetch is required
dispatch(updateStyleMeta(layerId));

dispatch(syncDataForLayer(layerId));
dispatch(syncDataForLayerId(layerId));
};
}

Expand Down Expand Up @@ -252,7 +274,7 @@ export function addLayer(layerDescriptor) {
type: ADD_LAYER,
layer: layerDescriptor,
});
dispatch(syncDataForLayer(layerDescriptor.id));
dispatch(syncDataForLayerId(layerDescriptor.id));
};
}

Expand Down Expand Up @@ -333,7 +355,7 @@ export function setLayerVisibility(layerId, makeVisible) {
visibility: makeVisible,
});
if (makeVisible) {
dispatch(syncDataForLayer(layerId));
dispatch(syncDataForLayer(layer));
}
};
}
Expand Down Expand Up @@ -466,8 +488,7 @@ export function mapExtentChanged(newMapConstants) {
...newMapConstants,
},
});
const newDataFilters = { ...dataFilters, ...newMapConstants };
await syncDataForAllLayers(dispatch, getState, newDataFilters);
await dispatch(syncDataForAllLayers());
};
}

Expand Down Expand Up @@ -751,7 +772,7 @@ export function updateSourceProp(layerId, propName, value, newLayerType) {
dispatch(updateLayerType(layerId, newLayerType));
}
await dispatch(clearMissingStyleProperties(layerId));
dispatch(syncDataForLayer(layerId));
dispatch(syncDataForLayerId(layerId));
};
}

Expand All @@ -771,20 +792,6 @@ function updateLayerType(layerId, newLayerType) {
};
}

export function syncDataForLayer(layerId) {
return async (dispatch, getState) => {
const targetLayer = getLayerById(layerId, getState());
if (targetLayer) {
const dataFilters = getDataFilters(getState());
const loadingFunctions = getLayerLoadingCallbacks(dispatch, getState, layerId);
await targetLayer.syncData({
...loadingFunctions,
dataFilters,
});
}
};
}

export function updateLayerLabel(id, newLabel) {
return {
type: UPDATE_LAYER_PROP,
Expand Down Expand Up @@ -830,7 +837,7 @@ export function setLayerQuery(id, query) {
newValue: query,
});

dispatch(syncDataForLayer(id));
dispatch(syncDataForLayerId(id));
};
}

Expand Down Expand Up @@ -895,8 +902,7 @@ export function setQuery({ query, timeFilters, filters = [], refresh = false })
filters,
});

const dataFilters = getDataFilters(getState());
await syncDataForAllLayers(dispatch, getState, dataFilters);
await dispatch(syncDataForAllLayers());
};
}

Expand All @@ -909,13 +915,12 @@ export function setRefreshConfig({ isPaused, interval }) {
}

export function triggerRefreshTimer() {
return async (dispatch, getState) => {
return async dispatch => {
dispatch({
type: TRIGGER_REFRESH_TIMER,
});

const dataFilters = getDataFilters(getState());
await syncDataForAllLayers(dispatch, getState, dataFilters);
await dispatch(syncDataForAllLayers());
};
}

Expand Down Expand Up @@ -956,7 +961,7 @@ export function updateLayerStyle(layerId, styleDescriptor) {
dispatch(updateStyleMeta(layerId));

// Style update may require re-fetch, for example ES search may need to retrieve field used for dynamic styling
dispatch(syncDataForLayer(layerId));
dispatch(syncDataForLayerId(layerId));
};
}

Expand Down Expand Up @@ -994,12 +999,12 @@ export function setJoinsForLayer(layer, joins) {
return async dispatch => {
await dispatch({
type: SET_JOINS,
layer: layer,
joins: joins,
layer,
joins,
});

await dispatch(clearMissingStyleProperties(layer.getId()));
dispatch(syncDataForLayer(layer.getId()));
dispatch(syncDataForLayerId(layer.getId()));
};
}

Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/maps/public/classes/layers/tile_layer/tile_layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ export class TileLayer extends AbstractLayer {
}

async syncData({ startLoading, stopLoading, onLoadError, dataFilters }) {
if (!this.isVisible() || !this.showAtZoomLevel(dataFilters.zoom)) {
return;
}
const sourceDataRequest = this.getSourceDataRequest();
if (sourceDataRequest) {
//data is immmutable
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ export class TiledVectorLayer extends VectorLayer {
}

async syncData(syncContext: SyncContext) {
if (!this.isVisible() || !this.showAtZoomLevel(syncContext.dataFilters.zoom)) {
return;
}

await this._syncSourceStyleMeta(syncContext, this._source, this._style);
await this._syncSourceFormatters(syncContext, this._source, this._style);
await this._syncMVTUrlTemplate(syncContext);
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/maps/public/classes/layers/vector_layer/vector_layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,6 @@ export class VectorLayer extends AbstractLayer {
// Given 2 above, which source/style to use can not be pulled from data request state.
// Therefore, source and style are provided as arugments and must be used instead of calling getSource or getCurrentStyle.
async _syncData(syncContext, source, style) {
if (!this.isVisible() || !this.showAtZoomLevel(syncContext.dataFilters.zoom)) {
return;
}

await this._syncSourceStyleMeta(syncContext, source, style);
await this._syncSourceFormatters(syncContext, source, style);
const sourceResult = await this._syncSource(syncContext, source, style);
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/maps/public/classes/layers/vector_tile_layer/vector_tile_layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ export class VectorTileLayer extends TileLayer {
}

async syncData({ startLoading, stopLoading, onLoadError, dataFilters }) {
if (!this.isVisible() || !this.showAtZoomLevel(dataFilters.zoom)) {
return;
}

const nextMeta = { tileLayerId: this.getSource().getTileLayerId() };
const canSkipSync = this._canSkipSync({
prevDataRequest: this.getSourceDataRequest(),
Expand Down
8 changes: 4 additions & 4 deletions x-pack/test/functional/es_archives/maps/kibana/data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
"title" : "document example top hits",
"description" : "",
"mapStateJSON" : "{\"zoom\":4.1,\"center\":{\"lon\":-100.61091,\"lat\":33.23887},\"timeFilters\":{\"from\":\"2015-09-20T00:00:00.000Z\",\"to\":\"2015-09-24T01:00:00.000Z\"},\"refreshConfig\":{\"isPaused\":true,\"interval\":1000},\"query\":{\"query\":\"\",\"language\":\"kuery\"}}",
"layerListJSON" : "[{\"id\":\"0hmz5\",\"sourceDescriptor\":{\"type\":\"EMS_TMS\",\"id\":\"road_map\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"TILE\",\"properties\":{}},\"type\":\"TILE\",\"minZoom\":0,\"maxZoom\":24},{\"id\":\"z52lq\",\"label\":\"logstash\",\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"id\":\"e1a5e1a6-676c-4a89-8ea9-0d91d64b73c6\",\"type\":\"ES_SEARCH\",\"geoField\":\"geo.coordinates\",\"limit\":2048,\"filterByMapBounds\":true,\"showTooltip\":true,\"tooltipProperties\":[],\"topHitsTimeField\":\"@timestamp\",\"useTopHits\":true,\"topHitsSplitField\":\"machine.os.raw\",\"topHitsSize\":2,\"indexPatternRefName\":\"layer_1_source_index_pattern\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#e6194b\"}},\"lineColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#FFFFFF\"}},\"lineWidth\":{\"type\":\"STATIC\",\"options\":{\"size\":1}},\"iconSize\":{\"type\":\"STATIC\",\"options\":{\"size\":10}}},\"previousStyle\":null},\"type\":\"VECTOR\"}]",
"layerListJSON" : "[{\"id\":\"z52lq\",\"label\":\"logstash\",\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"id\":\"e1a5e1a6-676c-4a89-8ea9-0d91d64b73c6\",\"type\":\"ES_SEARCH\",\"geoField\":\"geo.coordinates\",\"limit\":2048,\"filterByMapBounds\":true,\"showTooltip\":true,\"tooltipProperties\":[],\"topHitsTimeField\":\"@timestamp\",\"useTopHits\":true,\"topHitsSplitField\":\"machine.os.raw\",\"topHitsSize\":2,\"indexPatternRefName\":\"layer_1_source_index_pattern\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#e6194b\"}},\"lineColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#FFFFFF\"}},\"lineWidth\":{\"type\":\"STATIC\",\"options\":{\"size\":1}},\"iconSize\":{\"type\":\"STATIC\",\"options\":{\"size\":10}}},\"previousStyle\":null},\"type\":\"VECTOR\"}]",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed EMS base map layer from layerList. Tests using these maps look at mapbox style. EMS base map layer bloats mapbox style making the inspector very slow to render. Removing that layer speeds up tests a lot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, good call. Didn't occur to me, but when we moved to vector-tiles, this added a large amount of layers to the mb-map

"uiStateJSON" : "{\"isLayerTOCOpen\":true,\"openTOCDetails\":[]}",
"bounds" : {
"type" : "polygon",
Expand Down Expand Up @@ -467,7 +467,7 @@
"type": "envelope"
},
"description": "",
"layerListJSON" : "[{\"id\":\"0hmz5\",\"label\":\"EMS base layer (road_map)\",\"sourceDescriptor\":{\"type\":\"EMS_TMS\",\"id\":\"road_map\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"TILE\",\"properties\":{}},\"type\":\"VECTOR_TILE\",\"minZoom\":0,\"maxZoom\":24},{\"id\":\"n1t6f\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"id\":\"62eca1fc-fe42-11e8-8eb2-f2801f1b9fd1\",\"type\":\"ES_SEARCH\",\"geoField\":\"geometry\",\"limit\":2048,\"filterByMapBounds\":false,\"showTooltip\":true,\"tooltipProperties\":[\"name\"],\"applyGlobalQuery\":false,\"indexPatternRefName\":\"layer_1_source_index_pattern\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"DYNAMIC\",\"options\":{\"fieldMetaOptions\":{\"isEnabled\":false,\"sigma\":3},\"field\":{\"label\":\"max(prop1) group by meta_for_geo_shapes*.shape_name\",\"name\":\"__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name\",\"origin\":\"join\"},\"color\":\"Blues\"}},\"iconSize\":{\"type\":\"STATIC\",\"options\":{\"size\":10}}},\"temporary\":true,\"previousStyle\":null},\"type\":\"VECTOR\",\"joins\":[{\"leftField\":\"name\",\"right\":{\"id\":\"855ccb86-fe42-11e8-8eb2-f2801f1b9fd1\",\"indexPatternTitle\":\"meta_for_geo_shapes*\",\"term\":\"shape_name\",\"metrics\":[{\"type\":\"max\",\"field\":\"prop1\"}],\"applyGlobalQuery\":true,\"indexPatternRefName\":\"layer_1_join_0_index_pattern\"}}]}]",
"layerListJSON" : "[{\"id\":\"n1t6f\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"id\":\"62eca1fc-fe42-11e8-8eb2-f2801f1b9fd1\",\"type\":\"ES_SEARCH\",\"geoField\":\"geometry\",\"limit\":2048,\"filterByMapBounds\":false,\"showTooltip\":true,\"tooltipProperties\":[\"name\"],\"applyGlobalQuery\":false,\"indexPatternRefName\":\"layer_1_source_index_pattern\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"DYNAMIC\",\"options\":{\"fieldMetaOptions\":{\"isEnabled\":false,\"sigma\":3},\"field\":{\"label\":\"max(prop1) group by meta_for_geo_shapes*.shape_name\",\"name\":\"__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name\",\"origin\":\"join\"},\"color\":\"Blues\"}},\"iconSize\":{\"type\":\"STATIC\",\"options\":{\"size\":10}}},\"temporary\":true,\"previousStyle\":null},\"type\":\"VECTOR\",\"joins\":[{\"leftField\":\"name\",\"right\":{\"id\":\"855ccb86-fe42-11e8-8eb2-f2801f1b9fd1\",\"indexPatternTitle\":\"meta_for_geo_shapes*\",\"term\":\"shape_name\",\"metrics\":[{\"type\":\"max\",\"field\":\"prop1\"}],\"applyGlobalQuery\":true,\"indexPatternRefName\":\"layer_1_join_0_index_pattern\"}}]}]",
"mapStateJSON": "{\"zoom\":3.02,\"center\":{\"lon\":77.33426,\"lat\":-0.04647},\"timeFilters\":{\"from\":\"now-17m\",\"to\":\"now\",\"mode\":\"quick\"},\"refreshConfig\":{\"isPaused\":true,\"interval\":1000}}",
"title": "join example",
"uiStateJSON": "{\"isLayerTOCOpen\":true,\"openTOCDetails\":[\"n1t6f\"]}"
Expand Down Expand Up @@ -512,7 +512,7 @@
],
"type": "envelope"
},
"layerListJSON": "[{\"id\":\"0hmz5\",\"sourceDescriptor\":{\"type\":\"EMS_TMS\",\"id\":\"road_map\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"TILE\",\"properties\":{}},\"type\":\"TILE\",\"minZoom\":0,\"maxZoom\":24},{\"id\":\"3xlvm\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"resolution\": \"COARSE\",\"type\":\"ES_GEO_GRID\",\"id\":\"427aa49d-a552-4e7d-a629-67c47db27128\",\"indexPatternId\":\"c698b940-e149-11e8-a35a-370a8516603a\",\"geoField\":\"geo.coordinates\",\"requestType\":\"heatmap\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"HEATMAP\",\"refinement\":\"coarse\",\"properties\":{},\"previousStyle\":null},\"type\":\"HEATMAP\"}]",
"layerListJSON": "[{\"id\":\"3xlvm\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"resolution\": \"COARSE\",\"type\":\"ES_GEO_GRID\",\"id\":\"427aa49d-a552-4e7d-a629-67c47db27128\",\"indexPatternId\":\"c698b940-e149-11e8-a35a-370a8516603a\",\"geoField\":\"geo.coordinates\",\"requestType\":\"heatmap\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"HEATMAP\",\"refinement\":\"coarse\",\"properties\":{},\"previousStyle\":null},\"type\":\"HEATMAP\"}]",
"mapStateJSON": "{\"zoom\":3.59,\"center\":{\"lon\":-98.05765,\"lat\":38.32288},\"timeFilters\":{\"from\":\"2015-09-20T00:00:00.000Z\",\"to\":\"2015-09-20T01:00:00.000Z\"},\"refreshConfig\":{\"isPaused\":true,\"interval\":1000}}",
"title": "geo grid heatmap example",
"uiStateJSON": "{\"isDarkMode\":false}"
Expand Down Expand Up @@ -543,7 +543,7 @@
"type": "envelope"
},
"description": "",
"layerListJSON": "[{\"id\":\"0hmz5\",\"sourceDescriptor\":{\"type\":\"EMS_TMS\",\"id\":\"road_map\"},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"TILE\",\"properties\":{}},\"type\":\"TILE\",\"minZoom\":0,\"maxZoom\":24},{\"id\":\"g1xkv\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"resolution\": \"COARSE\",\"type\":\"ES_GEO_GRID\",\"id\":\"9305f6ea-4518-4c06-95b9-33321aa38d6a\",\"indexPatternId\":\"c698b940-e149-11e8-a35a-370a8516603a\",\"geoField\":\"geo.coordinates\",\"requestType\":\"grid\",\"metrics\":[{\"type\":\"count\"},{\"type\":\"max\",\"field\":\"bytes\"}]},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"DYNAMIC\",\"options\":{\"field\":{\"label\":\"max of bytes\",\"name\":\"max_of_bytes\",\"origin\":\"source\"},\"color\":\"Blues\"}},\"lineColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#cccccc\"}},\"lineWidth\":{\"type\":\"STATIC\",\"options\":{\"size\":1}},\"iconSize\":{\"type\":\"DYNAMIC\",\"options\":{\"field\":{\"label\":\"Count\",\"name\":\"doc_count\",\"origin\":\"source\"},\"minSize\":4,\"maxSize\":32}}},\"temporary\":true,\"previousStyle\":null},\"type\":\"VECTOR\"}]",
"layerListJSON": "[{\"id\":\"g1xkv\",\"label\":null,\"minZoom\":0,\"maxZoom\":24,\"sourceDescriptor\":{\"resolution\": \"COARSE\",\"type\":\"ES_GEO_GRID\",\"id\":\"9305f6ea-4518-4c06-95b9-33321aa38d6a\",\"indexPatternId\":\"c698b940-e149-11e8-a35a-370a8516603a\",\"geoField\":\"geo.coordinates\",\"requestType\":\"grid\",\"metrics\":[{\"type\":\"count\"},{\"type\":\"max\",\"field\":\"bytes\"}]},\"visible\":true,\"temporary\":false,\"style\":{\"type\":\"VECTOR\",\"properties\":{\"fillColor\":{\"type\":\"DYNAMIC\",\"options\":{\"field\":{\"label\":\"max of bytes\",\"name\":\"max_of_bytes\",\"origin\":\"source\"},\"color\":\"Blues\"}},\"lineColor\":{\"type\":\"STATIC\",\"options\":{\"color\":\"#cccccc\"}},\"lineWidth\":{\"type\":\"STATIC\",\"options\":{\"size\":1}},\"iconSize\":{\"type\":\"DYNAMIC\",\"options\":{\"field\":{\"label\":\"Count\",\"name\":\"doc_count\",\"origin\":\"source\"},\"minSize\":4,\"maxSize\":32}}},\"temporary\":true,\"previousStyle\":null},\"type\":\"VECTOR\"}]",
"mapStateJSON": "{\"zoom\":3.59,\"center\":{\"lon\":-98.05765,\"lat\":38.32288},\"timeFilters\":{\"from\":\"2015-09-20T00:00:00.000Z\",\"to\":\"2015-09-20T01:00:00.000Z\"},\"refreshConfig\":{\"isPaused\":true,\"interval\":1000}}",
"title": "geo grid vector grid example",
"uiStateJSON": "{\"isDarkMode\":false}"
Expand Down