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.

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] Compare SearchFilters to determine whether mvt layers can skip update #93531

Merged
merged 9 commits into from
Mar 11, 2021
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 @@ -130,13 +139,49 @@ describe('syncData', () => {
sinon.assert.notCalled(syncContext2.stopLoading);
});

it('Should resync when changes to syncContext', async () => {
const layer1: TiledVectorLayer = createLayer({}, {}, true);
thomasneirynck marked this conversation as resolved.
Show resolved Hide resolved
const syncContext1 = new MockSyncContext({
dataFilters: {
timeFilters: {
from: 'now',
to: '15m',
mode: 'relative',
},
},
});

await layer1.syncData(syncContext1);

const dataRequestDescriptor: DataRequestDescriptor = {
data: { ...defaultConfig },
dataId: 'source',
};
const layer2: TiledVectorLayer = createLayer(
{
__dataRequests: [dataRequestDescriptor],
},
{},
true
);
const syncContext2 = new MockSyncContext({
dataFilters: {
timeFilters: {
from: 'now',
to: '30m',
mode: 'relative',
},
},
});
await layer2.syncData(syncContext2);
// @ts-expect-error
sinon.assert.calledOnce(syncContext2.startLoading);
// @ts-expect-error
sinon.assert.calledOnce(syncContext2.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: {} });
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