[Maps] Compare SearchFilters to determine whether mvt layers can skip…

… update (#93531)
elastic · Mar 11, 2021 · 9d365a4 · 9d365a4
1 parent fa3b2f0
commit 9d365a4
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 39 deletions.
diff --git a/x-pack/plugins/maps/public/classes/layers/blended_vector_layer/blended_vector_layer.ts b/x-pack/plugins/maps/public/classes/layers/blended_vector_layer/blended_vector_layer.ts
@@ -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;

diff --git a/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.test.tsx b/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.test.tsx
@@ -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,
@@ -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,
@@ -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 });
       });
     });

diff --git a/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.tsx b/x-pack/plugins/maps/public/classes/layers/tiled_vector_layer/tiled_vector_layer.tsx
@@ -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;
@@ -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);
     }

diff --git a/x-pack/plugins/maps/public/classes/layers/vector_layer/utils.tsx b/x-pack/plugins/maps/public/classes/layers/vector_layer/utils.tsx
@@ -73,6 +73,7 @@ export async function syncVectorSource({
     source,
     prevDataRequest,
     nextMeta: requestMeta,
+    extentAware: source.isFilterByMapBounds(),
   });
   if (canSkipFetch) {
     return {

diff --git a/x-pack/plugins/maps/public/classes/layers/vector_layer/vector_layer.tsx b/x-pack/plugins/maps/public/classes/layers/vector_layer/vector_layer.tsx
@@ -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 {

diff --git a/x-pack/plugins/maps/public/classes/util/can_skip_fetch.test.js b/x-pack/plugins/maps/public/classes/util/can_skip_fetch.test.js
@@ -135,6 +135,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(true);
@@ -154,6 +155,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(true);
@@ -173,6 +175,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);
@@ -189,6 +192,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);
@@ -219,6 +223,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);
@@ -238,6 +243,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);
@@ -257,6 +263,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);
@@ -273,6 +280,7 @@ describe('canSkipSourceUpdate', () => {
           source: queryAwareSourceMock,
           prevDataRequest,
           nextMeta,
+          extentAware: queryAwareSourceMock.isFilterByMapBounds(),
         });
 
         expect(canSkipUpdate).toBe(false);

diff --git a/x-pack/plugins/maps/public/classes/util/can_skip_fetch.ts b/x-pack/plugins/maps/public/classes/util/can_skip_fetch.ts
@@ -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();
@@ -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);
   }