Skip to content

Commit

Permalink
[Maps] Compare SearchFilters to determine whether mvt layers can skip…
Browse files Browse the repository at this point in the history
… update (#93531) (#94479)
  • Loading branch information
thomasneirynck authored Mar 11, 2021
1 parent 698617d commit ef4dc35
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,12 @@ export class BlendedVectorLayer extends VectorLayer implements IVectorLayer {
this.getSource(),
this.getCurrentStyle()
);
const source = this.getSource();
const canSkipFetch = await canSkipSourceUpdate({
source: this.getSource(),
source,
prevDataRequest: this.getDataRequest(dataRequestId),
nextMeta: searchFilters,
extentAware: source.isFilterByMapBounds(),
});

let activeSource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const defaultConfig = {

function createLayer(
layerOptions: Partial<VectorLayerDescriptor> = {},
sourceOptions: Partial<TiledSingleLayerVectorSourceDescriptor> = {}
sourceOptions: Partial<TiledSingleLayerVectorSourceDescriptor> = {},
isTimeAware: boolean = false
): TiledVectorLayer {
const sourceDescriptor: TiledSingleLayerVectorSourceDescriptor = {
type: SOURCE_TYPES.MVT_SINGLE_LAYER,
Expand All @@ -47,6 +48,14 @@ function createLayer(
...sourceOptions,
};
const mvtSource = new MVTSingleLayerVectorSource(sourceDescriptor);
if (isTimeAware) {
mvtSource.isTimeAware = async () => {
return true;
};
mvtSource.getApplyGlobalTime = () => {
return true;
};
}

const defaultLayerOptions = {
...layerOptions,
Expand Down Expand Up @@ -107,62 +116,75 @@ describe('syncData', () => {
});

it('Should not resync when no changes to source params', async () => {
const layer1: TiledVectorLayer = createLayer({}, {});
const syncContext1 = new MockSyncContext({ dataFilters: {} });

await layer1.syncData(syncContext1);

const dataRequestDescriptor: DataRequestDescriptor = {
data: { ...defaultConfig },
dataId: 'source',
};
const layer2: TiledVectorLayer = createLayer(
const layer: TiledVectorLayer = createLayer(
{
__dataRequests: [dataRequestDescriptor],
},
{}
);
const syncContext2 = new MockSyncContext({ dataFilters: {} });
await layer2.syncData(syncContext2);
const syncContext = new MockSyncContext({ dataFilters: {} });
await layer.syncData(syncContext);
// @ts-expect-error
sinon.assert.notCalled(syncContext2.startLoading);
sinon.assert.notCalled(syncContext.startLoading);
// @ts-expect-error
sinon.assert.notCalled(syncContext2.stopLoading);
sinon.assert.notCalled(syncContext.stopLoading);
});

it('Should resync when changes to syncContext', async () => {
const dataRequestDescriptor: DataRequestDescriptor = {
data: { ...defaultConfig },
dataId: 'source',
};
const layer: TiledVectorLayer = createLayer(
{
__dataRequests: [dataRequestDescriptor],
},
{},
true
);
const syncContext = new MockSyncContext({
dataFilters: {
timeFilters: {
from: 'now',
to: '30m',
mode: 'relative',
},
},
});
await layer.syncData(syncContext);
// @ts-expect-error
sinon.assert.calledOnce(syncContext.startLoading);
// @ts-expect-error
sinon.assert.calledOnce(syncContext.stopLoading);
});

describe('Should resync when changes to source params: ', () => {
[
{ layerName: 'barfoo' },
{ urlTemplate: 'https://sub.example.com/{z}/{x}/{y}.pbf' },
{ minSourceZoom: 1 },
{ maxSourceZoom: 12 },
].forEach((changes) => {
[{ layerName: 'barfoo' }, { minSourceZoom: 1 }, { maxSourceZoom: 12 }].forEach((changes) => {
it(`change in ${Object.keys(changes).join(',')}`, async () => {
const layer1: TiledVectorLayer = createLayer({}, {});
const syncContext1 = new MockSyncContext({ dataFilters: {} });

await layer1.syncData(syncContext1);

const dataRequestDescriptor: DataRequestDescriptor = {
data: defaultConfig,
dataId: 'source',
};
const layer2: TiledVectorLayer = createLayer(
const layer: TiledVectorLayer = createLayer(
{
__dataRequests: [dataRequestDescriptor],
},
changes
);
const syncContext2 = new MockSyncContext({ dataFilters: {} });
await layer2.syncData(syncContext2);
const syncContext = new MockSyncContext({ dataFilters: {} });
await layer.syncData(syncContext);

// @ts-expect-error
sinon.assert.calledOnce(syncContext2.startLoading);
sinon.assert.calledOnce(syncContext.startLoading);
// @ts-expect-error
sinon.assert.calledOnce(syncContext2.stopLoading);
sinon.assert.calledOnce(syncContext.stopLoading);

// @ts-expect-error
const call = syncContext2.stopLoading.getCall(0);
const call = syncContext.stopLoading.getCall(0);
expect(call.args[2]).toEqual({ ...defaultConfig, ...changes });
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
VectorSourceRequestMeta,
} from '../../../../common/descriptor_types';
import { MVTSingleLayerVectorSourceConfig } from '../../sources/mvt_single_layer_vector_source/types';
import { canSkipSourceUpdate } from '../../util/can_skip_fetch';

export class TiledVectorLayer extends VectorLayer {
static type = LAYER_TYPE.TILED_VECTOR;
Expand Down Expand Up @@ -68,26 +69,30 @@ export class TiledVectorLayer extends VectorLayer {
this._style as IVectorStyle
);
const prevDataRequest = this.getSourceDataRequest();

const templateWithMeta = await this._source.getUrlTemplateWithMeta(searchFilters);
const dataRequest = await this._source.getUrlTemplateWithMeta(searchFilters);
if (prevDataRequest) {
const data: MVTSingleLayerVectorSourceConfig = prevDataRequest.getData() as MVTSingleLayerVectorSourceConfig;
if (data) {
const canSkipBecauseNoChanges =
const noChangesInSourceState: boolean =
data.layerName === this._source.getLayerName() &&
data.minSourceZoom === this._source.getMinZoom() &&
data.maxSourceZoom === this._source.getMaxZoom() &&
data.urlTemplate === templateWithMeta.urlTemplate;

if (canSkipBecauseNoChanges) {
data.maxSourceZoom === this._source.getMaxZoom();
const noChangesInSearchState: boolean = await canSkipSourceUpdate({
extentAware: false, // spatial extent knowledge is already fully automated by tile-loading based on pan-zooming
source: this.getSource(),
prevDataRequest,
nextMeta: searchFilters,
});
const canSkip = noChangesInSourceState && noChangesInSearchState;
if (canSkip) {
return null;
}
}
}

startLoading(SOURCE_DATA_REQUEST_ID, requestToken, searchFilters);
try {
stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, templateWithMeta, {});
stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, dataRequest, {});
} catch (error) {
onLoadError(SOURCE_DATA_REQUEST_ID, requestToken, error.message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export async function syncVectorSource({
source,
prevDataRequest,
nextMeta: requestMeta,
extentAware: source.isFilterByMapBounds(),
});
if (canSkipFetch) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
source: joinSource,
prevDataRequest,
nextMeta: searchFilters,
extentAware: false, // join-sources are term-aggs that are spatially unaware (e.g. ESTermSource/TableSource).
});
if (canSkipFetch) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(true);
Expand All @@ -154,6 +155,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(true);
Expand All @@ -173,6 +175,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand All @@ -189,6 +192,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand Down Expand Up @@ -219,6 +223,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand All @@ -238,6 +243,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand All @@ -257,6 +263,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand All @@ -273,6 +280,7 @@ describe('canSkipSourceUpdate', () => {
source: queryAwareSourceMock,
prevDataRequest,
nextMeta,
extentAware: queryAwareSourceMock.isFilterByMapBounds(),
});

expect(canSkipUpdate).toBe(false);
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/maps/public/classes/util/can_skip_fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,15 @@ export async function canSkipSourceUpdate({
source,
prevDataRequest,
nextMeta,
extentAware,
}: {
source: ISource;
prevDataRequest: DataRequest | undefined;
nextMeta: DataMeta;
extentAware: boolean;
}): Promise<boolean> {
const timeAware = await source.isTimeAware();
const refreshTimerAware = await source.isRefreshTimerAware();
const extentAware = source.isFilterByMapBounds();
const isFieldAware = source.isFieldAware();
const isQueryAware = source.isQueryAware();
const isGeoGridPrecisionAware = source.isGeoGridPrecisionAware();
Expand Down Expand Up @@ -132,11 +133,12 @@ export async function canSkipSourceUpdate({
}

let updateDueToPrecisionChange = false;
let updateDueToExtentChange = false;

if (isGeoGridPrecisionAware) {
updateDueToPrecisionChange = !_.isEqual(prevMeta.geogridPrecision, nextMeta.geogridPrecision);
}

let updateDueToExtentChange = false;
if (extentAware) {
updateDueToExtentChange = updateDueToExtent(prevMeta, nextMeta);
}
Expand Down

0 comments on commit ef4dc35

Please sign in to comment.