Skip to content

Commit

Permalink
[Maps] fix double fetch when filter pill is added
Browse files Browse the repository at this point in the history
  • Loading branch information
nreese committed Apr 8, 2020
1 parent 3457dde commit 6af3233
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 7 deletions.
18 changes: 18 additions & 0 deletions x-pack/legacy/plugins/maps/public/actions/map_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getOpenTooltips,
getQuery,
getDataRequestDescriptor,
getLayerSyncDataToken,
} from '../selectors/map_selectors';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { FLYOUT_STATE } from '../../../../../plugins/maps/public/reducers/ui';
Expand Down Expand Up @@ -59,6 +60,7 @@ import {
UPDATE_LAYER_PROP,
UPDATE_LAYER_STYLE,
SET_LAYER_STYLE_META,
SET_LAYER_SYNC_DATA_TOKEN,
UPDATE_SOURCE_PROP,
SET_REFRESH_CONFIG,
SET_MOUSE_COORDINATES,
Expand All @@ -84,6 +86,13 @@ import {
export * from '../../../../../plugins/maps/public/actions/map_actions';

function getLayerLoadingCallbacks(dispatch, getState, layerId) {
const syncToken = Symbol();
dispatch({
type: SET_LAYER_SYNC_DATA_TOKEN,
layerId,
syncToken,
});

return {
startLoading: (dataId, requestToken, meta) =>
dispatch(startDataLoad(layerId, dataId, requestToken, meta)),
Expand All @@ -94,6 +103,15 @@ function getLayerLoadingCallbacks(dispatch, getState, layerId) {
updateSourceData: newData => {
dispatch(updateSourceDataRequest(layerId, newData));
},
// Check to ensure data syncing action for layer is still active.
// Async actions may occur before startLoading is called.
// Use isDataSyncActive to check that the data sync is still active before startLoading is called for each data request.
isDataSyncActive: () => {
const currentToken = getLayerSyncDataToken(getState(), layerId);
return currentToken === syncToken;
},
// Check to ensure single data request for layer is still active.
// Use isRequestStillActive to check that the data request is still active after startLoading has been called.
isRequestStillActive: (dataId, requestToken) => {
const dataRequest = getDataRequestDescriptor(getState(), layerId, dataId);
if (!dataRequest) {
Expand Down
5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/maps/public/selectors/map_selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ export function getDataRequestDescriptor(state = {}, layerId, dataId) {
});
}

export function getLayerSyncDataToken(state = {}, layerId) {
const layerDescriptor = getLayerDescriptor(state, layerId);
return _.get(layerDescriptor, '__dataSyncToken');
}

export const getDataFilters = createSelector(
getMapExtent,
getMapBuffer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export type JoinDescriptor = {
};

export type LayerDescriptor = {
__dataSyncToken?: symbol;
__dataRequests?: DataRequestDescriptor[];
__isInErrorState?: boolean;
__errorMessage?: string;
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/actions/map_actions.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type SyncContext = {
stopLoading(dataId: string, requestToken: symbol, data: unknown, meta: DataMeta): void;
onLoadError(dataId: string, requestToken: symbol, errorMessage: string): void;
updateSourceData(newData: unknown): void;
isDataSyncActive(): boolean;
isRequestStillActive(dataId: string, requestToken: symbol): boolean;
registerCancelCallback(requestToken: symbol, callback: () => void): void;
dataFilters: MapFilters;
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/actions/map_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const TRIGGER_REFRESH_TIMER = 'TRIGGER_REFRESH_TIMER';
export const UPDATE_LAYER_PROP = 'UPDATE_LAYER_PROP';
export const UPDATE_LAYER_STYLE = 'UPDATE_LAYER_STYLE';
export const SET_LAYER_STYLE_META = 'SET_LAYER_STYLE_META';
export const SET_LAYER_SYNC_DATA_TOKEN = 'SET_LAYER_SYNC_DATA_TOKEN';
export const UPDATE_SOURCE_PROP = 'UPDATE_SOURCE_PROP';
export const SET_REFRESH_CONFIG = 'SET_REFRESH_CONFIG';
export const SET_MOUSE_COORDINATES = 'SET_MOUSE_COORDINATES';
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/maps/public/layers/util/data_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ export class DataRequest {
}

getMeta(): DataMeta {
return this.hasData()
? _.get(this._descriptor, 'dataMeta', {})
: _.get(this._descriptor, 'dataMetaAtStart', {});
if (this._descriptor.dataMetaAtStart) {
return this._descriptor.dataMetaAtStart;
} else if (this._descriptor.dataMeta) {
return this._descriptor.dataMeta;
} else {
return {};
}
}

hasData(): boolean {
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/maps/public/layers/vector_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export class VectorLayer extends AbstractLayer {

async _syncJoin({
join,
isDataSyncActive,
startLoading,
stopLoading,
onLoadError,
Expand All @@ -252,7 +253,8 @@ export class VectorLayer extends AbstractLayer {
prevDataRequest,
nextMeta: searchFilters,
});
if (canSkipFetch) {

if (canSkipFetch || !isDataSyncActive()) {
return {
dataHasChanged: false,
join: join,
Expand Down Expand Up @@ -360,6 +362,7 @@ export class VectorLayer extends AbstractLayer {
onLoadError,
registerCancelCallback,
dataFilters,
isDataSyncActive,
isRequestStillActive,
} = syncContext;
const dataRequestId = SOURCE_DATA_ID_ORIGIN;
Expand All @@ -371,7 +374,8 @@ export class VectorLayer extends AbstractLayer {
prevDataRequest,
nextMeta: searchFilters,
});
if (canSkipFetch) {

if (canSkipFetch || !isDataSyncActive()) {
return {
refreshed: false,
featureCollection: prevDataRequest.getData(),
Expand Down Expand Up @@ -458,6 +462,7 @@ export class VectorLayer extends AbstractLayer {
startLoading,
stopLoading,
onLoadError,
isDataSyncActive,
registerCancelCallback,
}) {
if (!source.isESSource() || dynamicStyleProps.length === 0) {
Expand All @@ -476,7 +481,8 @@ export class VectorLayer extends AbstractLayer {
};
const prevDataRequest = this.getDataRequest(dataRequestId);
const canSkipFetch = canSkipStyleMetaUpdate({ prevDataRequest, nextMeta });
if (canSkipFetch) {

if (canSkipFetch || !isDataSyncActive()) {
return;
}

Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/maps/public/reducers/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
UPDATE_LAYER_PROP,
UPDATE_LAYER_STYLE,
SET_LAYER_STYLE_META,
SET_LAYER_SYNC_DATA_TOKEN,
SET_JOINS,
UPDATE_SOURCE_PROP,
SET_REFRESH_CONFIG,
Expand Down Expand Up @@ -57,8 +58,13 @@ const updateLayerInList = (state, layerId, attribute, newValue) => {
if (!layerId) {
return state;
}

const { layerList } = state;
const layerIdx = getLayerIndex(layerList, layerId);
if (layerIdx === -1) {
return state;
}

const updatedLayer = {
...layerList[layerIdx],
// Update layer w/ new value. If no value provided, toggle boolean value
Expand Down Expand Up @@ -318,6 +324,8 @@ export function map(state = INITIAL_STATE, action) {
...state.layerList[index].style,
__styleMeta: styleMeta,
});
case SET_LAYER_SYNC_DATA_TOKEN:
return updateLayerInList(state, action.layerId, '__dataSyncToken', action.syncToken);
case SET_SCROLL_ZOOM:
return {
...state,
Expand Down Expand Up @@ -437,7 +445,7 @@ function updateWithDataResponse(state, action) {

dataRequest.data = action.data;
dataRequest.dataMeta = { ...dataRequest.dataMetaAtStart, ...action.meta };
dataRequest.dataMetaAtStart = null;
delete dataRequest.dataMetaAtStart;
return resetDataRequest(state, action, dataRequest);
}

Expand Down

0 comments on commit 6af3233

Please sign in to comment.