Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e482029
Update ems utils to better handle no service results. Prevent excess …
kindsun Jan 16, 2019
6b911c3
Update tile layer sync to return promise and handle errors related to…
kindsun Jan 16, 2019
abafa1c
Add flow for updating tms layers with error status/message
kindsun Jan 16, 2019
ad4184b
Handle promises, if returned, on syncLayerWithMB. Update TMS error st…
kindsun Jan 16, 2019
58651ae
Exclude layers that mapbox didn't add to map but are tracked in layer…
kindsun Jan 16, 2019
bfc1db4
Move datarequest handling to vector layer. Use relevant data load/err…
kindsun Jan 16, 2019
4a6f50e
Don't try to get attributions on errored layer
kindsun Jan 16, 2019
3b2bba5
Handle 'includeElasticMapsService' configuration
kindsun Jan 16, 2019
43a021f
Move data requests back to layer level for heatmap usage
kindsun Jan 17, 2019
ab80f12
Update all layers to set top-level layer error status and message. Co…
kindsun Jan 17, 2019
b4c16ce
Update tile sync function to more reliably confirm load status after …
kindsun Jan 17, 2019
76b357c
Remove unnecessary, and annoying, clear temp layers on tms error
kindsun Jan 17, 2019
eca78ef
Clean up
kindsun Jan 17, 2019
7b75249
More clean up
kindsun Jan 17, 2019
0c3dd1f
Merge branch 'master' into fix/mapsAppHandleDisabling
kindsun Jan 18, 2019
2497b9c
Merge remote-tracking branch 'upstream/master' into fix/mapsAppHandle…
kindsun Jan 24, 2019
adf4962
Review feedback
kindsun Jan 24, 2019
0a205a3
Review feedback. Test cleanup
kindsun Jan 25, 2019
11fcf24
Merge remote-tracking branch 'upstream/master' into fix/mapsAppHandle…
kindsun Jan 25, 2019
14e94cb
Test fixes and review feedback
kindsun Jan 28, 2019
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
11 changes: 11 additions & 0 deletions x-pack/plugins/gis/public/actions/store_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { timeService } from '../kibana_services';
export const SET_SELECTED_LAYER = 'SET_SELECTED_LAYER';
export const UPDATE_LAYER_ORDER = 'UPDATE_LAYER_ORDER';
export const ADD_LAYER = 'ADD_LAYER';
export const SET_LAYER_ERROR_STATUS = 'SET_LAYER_ERROR_STATUS';
export const ADD_WAITING_FOR_MAP_READY_LAYER = 'ADD_WAITING_FOR_MAP_READY_LAYER';
export const CLEAR_WAITING_FOR_MAP_READY_LAYER_LIST = 'CLEAR_WAITING_FOR_MAP_READY_LAYER_LIST';
export const REMOVE_LAYER = 'REMOVE_LAYER';
Expand Down Expand Up @@ -108,6 +109,16 @@ export function addLayer(layerDescriptor) {
};
}

export function setLayerErrorStatus(id, errorMessage) {
return dispatch => {
dispatch({
type: SET_LAYER_ERROR_STATUS,
layerId: id,
errorMessage,
});
};
}

export function toggleLayerVisible(layerId) {
return {
type: TOGGLE_LAYER_VISIBLE,
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/gis/public/components/map/mb/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
mapDestroyed,
setMouseCoordinates,
clearMouseCoordinates,
clearGoto
clearGoto,
setLayerErrorStatus,
} from '../../../actions/store_actions';
import { getLayerList, getMapReady, getGoto } from "../../../selectors/map_selectors";

Expand Down Expand Up @@ -45,7 +46,9 @@ function mapDispatchToProps(dispatch) {
},
clearGoto: () => {
dispatch(clearGoto());
}
},
setLayerErrorStatus: (id, msg) =>
dispatch(setLayerErrorStatus(id, msg))
};
}

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/gis/public/components/map/mb/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ export function syncLayerOrder(mbMap, layerList) {
const mbLayers = mbMap.getStyle().layers.slice();
const currentLayerOrder = _.uniq( // Consolidate layers and remove suffix
mbLayers.map(({ id }) => id.substring(0, id.lastIndexOf('_'))));
const newLayerOrder = layerList.map(l => l.getId());
const newLayerOrder = layerList.map(l => l.getId())
.filter(layerId => currentLayerOrder.includes(layerId));
let netPos = 0;
let netNeg = 0;
const movementArr = currentLayerOrder.reduce((accu, id, idx) => {
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/gis/public/components/map/mb/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,14 @@ export class MBMapContainer extends React.Component {
if (!isMapReady) {
return;
}

removeOrphanedSourcesAndLayers(this._mbMap, layerList);
layerList.forEach((layer) => {
layer.syncLayerWithMB(this._mbMap);
layerList.forEach(layer => {
if (!layer.hasErrors()) {
Promise.resolve(layer.syncLayerWithMB(this._mbMap))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This causes the syncing operation to unhook from the callstack, and completion is not guaranteed.

.catch(({ message }) =>
this.props.setLayerErrorStatus(layer.getId(), message));
}
});
syncLayerOrder(this._mbMap, layerList);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ export class TOCEntry extends React.Component {
alignItems="center"
responsive={false}
className={
layer.isVisible() && layer.showAtZoomLevel(zoom) && !layer.dataHasLoadError() ? 'gisTocEntry-visible' : 'gisTocEntry-notVisible'
layer.isVisible() && layer.showAtZoomLevel(zoom)
&& !layer.hasErrors() ? 'gisTocEntry-visible' : 'gisTocEntry-notVisible'
}
>
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ export class LayerTocActions extends Component {
_renderIcon() {
const { zoom, layer } = this.props;
let smallLegendIcon;
if (layer.dataHasLoadError()) {
if (layer.hasErrors()) {
smallLegendIcon = (
<EuiIconTip
aria-label="Load warning"
size="m"
type="alert"
color="warning"
content={layer.getDataLoadError()}
content={layer.getErrors()}
/>
);
} else if (layer.isLayerLoading()) {
Expand Down
27 changes: 12 additions & 15 deletions x-pack/plugins/gis/public/shared/layers/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export class AbstractLayer {
this._descriptor = AbstractLayer.createDescriptor(layerDescriptor);
this._source = source;
this._style = style;

if (this._descriptor.dataRequests) {
this._dataRequests = this._descriptor.dataRequests.map(dataRequest => new DataRequest(dataRequest));
} else {
Expand Down Expand Up @@ -64,7 +63,10 @@ export class AbstractLayer {
}

async getAttributions() {
return await this._source.getAttributions();
if (!this.hasErrors()) {
return await this._source.getAttributions();
}
return [];
}

getLabel() {
Expand Down Expand Up @@ -139,21 +141,20 @@ export class AbstractLayer {
return this._source.renderSourceSettingsEditor({ onChange });
};

getSourceDataRequest() {
return this._dataRequests.find(dataRequest => dataRequest.getDataId() === 'source');
}

isLayerLoading() {
return this._dataRequests.some(dataRequest => dataRequest.isLoading());
}

dataHasLoadError() {
return this._dataRequests.some(dataRequest => dataRequest.hasLoadError());
hasErrors() {
return _.get(this._descriptor, 'isInErrorState', false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is isInErrorState needed? Why not just check if this._descriptor.errorMessage exists or not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not needed, just preferred

}

getDataLoadError() {
const loadErrors = this._dataRequests
.filter(dataRequest => dataRequest.hasLoadError())
.map(dataRequest => {
return dataRequest._descriptor.dataLoadError;
});
return loadErrors.join(',');
getErrors() {
return this.hasErrors() ? this._descriptor.errorMessage : '';
}

toLayerDescriptor() {
Expand Down Expand Up @@ -219,10 +220,6 @@ export class AbstractLayer {
return style.renderEditor(options);
}

getSourceDataRequest() {
return this._dataRequests.find(dataRequest => dataRequest.getDataId() === 'source');
}

getIndexPatternIds() {
return [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export class EMSTMSSource extends AbstractTMSSource {
}

_getTMSOptions() {
if(!this._emsTileServices) {
return;
}

return this._emsTileServices.find(service => {
return service.id === this._descriptor.id;
});
Expand All @@ -96,9 +100,11 @@ export class EMSTMSSource extends AbstractTMSSource {

async getAttributions() {
const service = this._getTMSOptions();
const attributions = service.attributionMarkdown.split('|');
if (!service || !service.attributionMarkdown) {
return [];
}

return attributions.map((attribution) => {
return service.attributionMarkdown.split('|').map((attribution) => {
attribution = attribution.trim();
//this assumes attribution is plain markdown link
const extractLink = /\[(.*)\]\((.*)\)/;
Expand All @@ -112,8 +118,9 @@ export class EMSTMSSource extends AbstractTMSSource {

getUrlTemplate() {
const service = this._getTMSOptions();
if (!service || !service.url) {
throw new Error('Can not generate EMS TMS url template');
}
return service.url;
}


}
75 changes: 58 additions & 17 deletions x-pack/plugins/gis/public/shared/layers/tile_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import React from 'react';
import { EuiIcon } from '@elastic/eui';
import { TileStyle } from '../layers/styles/tile_style';

const TMS_LOAD_TIMEOUT = 32000;

export class TileLayer extends AbstractLayer {

static type = "TILE";
Expand All @@ -30,29 +32,65 @@ export class TileLayer extends AbstractLayer {
return tileLayerDescriptor;
}

_tileLoadErrorTracker(map, url) {
let tileLoad;
map.on('dataloading', ({ tile }) => {
if (tile && tile.request) {
// If at least one tile loads, endpoint/resource is valid
tile.request.onloadend = ({ loaded }) => {
if (loaded) {
tileLoad = true;
}
};
}
});

return new Promise((resolve, reject) => {
let tileLoadTimer = null;

syncLayerWithMB(mbMap) {
const clearChecks = () => {
clearTimeout(tileLoadTimer);
map.off('dataloading');
};

tileLoadTimer = setTimeout(() => {
if (!tileLoad) {
reject(new Error(`Tiles from "${url}" could not be loaded`));
} else {
resolve();
}
clearChecks();
}, TMS_LOAD_TIMEOUT);
});
}

async syncLayerWithMB(mbMap) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we make this signature async, than the client-code should take into account the async-nature of this method.

In practice, the completion of this method (https://github.com/thomasneirynck/kibana/blob/93fe5b7b2f566cc4519cc257834b110eb8f8248b/x-pack/plugins/gis/public/components/map/mb/view.js#L194) now no longer guarantees that the map has synced.

const source = mbMap.getSource(this.getId());
const layerId = this.getId() + '_raster';
if (!source) {
const url = this._source.getUrlTemplate();
mbMap.addSource(this.getId(), {
type: 'raster',
tiles: [url],
tileSize: 256,
scheme: 'xyz',
});

mbMap.addLayer({
id: layerId,
type: 'raster',
source: this.getId(),
minzoom: 0,
maxzoom: 22,
});

if (source) {
return;
}

const url = this._source.getUrlTemplate();
const sourceId = this.getId();
mbMap.addSource(sourceId, {
type: 'raster',
tiles: [url],
tileSize: 256,
scheme: 'xyz',
});

mbMap.addLayer({
id: layerId,
type: 'raster',
source: sourceId,
minzoom: 0,
maxzoom: 22,
});

await this._tileLoadErrorTracker(mbMap, url);

mbMap.setLayoutProperty(layerId, 'visibility', this.isVisible() ? 'visible' : 'none');
mbMap.setLayerZoomRange(layerId, this._descriptor.minZoom, this._descriptor.maxZoom);
this._style && this._style.setMBPaintProperties({
Expand All @@ -73,5 +111,8 @@ export class TileLayer extends AbstractLayer {
/>
);
}
isLayerLoading() {
return false;
}

}
8 changes: 0 additions & 8 deletions x-pack/plugins/gis/public/shared/layers/util/data_request.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ export class DataRequest {
this._descriptor = descriptor;
}

hasLoadError() {
return !!this._descriptor.dataHasLoadError;
}

getLoadError() {
return this._descriptor.dataLoadError;
}

getData() {
return this._descriptor.data;
}
Expand Down
Loading