[maps] Use label features from ES vector tile search API to fix multi…

…ple labels (elastic#132080) * [maps] mvt labels * eslint * only request labels when needed * update vector tile integration tests for hasLabels parameter * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * fix tests * fix test * only add _mvt_label_position filter when vector tiles are from ES vector tile search API * review feedback * include hasLabels in source data * fix jest test Co-authored-by: Kibana Machine <[email protected]>
klacabane · May 20, 2022 · d344088 · d344088
1 parent 1d8bc7e
commit d344088
Show file tree

Hide file tree

Showing 21 changed files with 229 additions and 22 deletions.
diff --git a/x-pack/plugins/maps/common/mvt_request_body.ts b/x-pack/plugins/maps/common/mvt_request_body.ts
@@ -21,6 +21,7 @@ export function getAggsTileRequest({
   encodedRequestBody,
   geometryFieldName,
   gridPrecision,
+  hasLabels,
   index,
   renderAs = RENDER_AS.POINT,
   x,
@@ -30,6 +31,7 @@ export function getAggsTileRequest({
   encodedRequestBody: string;
   geometryFieldName: string;
   gridPrecision: number;
+  hasLabels: boolean;
   index: string;
   renderAs: RENDER_AS;
   x: number;
@@ -50,20 +52,23 @@ export function getAggsTileRequest({
       aggs: requestBody.aggs,
       fields: requestBody.fields,
       runtime_mappings: requestBody.runtime_mappings,
+      with_labels: hasLabels,
     },
   };
 }
 
 export function getHitsTileRequest({
   encodedRequestBody,
   geometryFieldName,
+  hasLabels,
   index,
   x,
   y,
   z,
 }: {
   encodedRequestBody: string;
   geometryFieldName: string;
+  hasLabels: boolean;
   index: string;
   x: number;
   y: number;
@@ -86,6 +91,7 @@ export function getHitsTileRequest({
       ),
       runtime_mappings: requestBody.runtime_mappings,
       track_total_hits: typeof requestBody.size === 'number' ? requestBody.size + 1 : false,
+      with_labels: hasLabels,
     },
   };
 }

diff --git a/x-pack/plugins/maps/public/classes/layers/heatmap_layer/heatmap_layer.ts b/x-pack/plugins/maps/public/classes/layers/heatmap_layer/heatmap_layer.ts
@@ -87,6 +87,7 @@ export class HeatmapLayer extends AbstractLayer {
 
   async syncData(syncContext: DataRequestContext) {
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: this.getId(),
       layerName: await this.getDisplayName(),
       prevDataRequest: this.getSourceDataRequest(),

diff --git a/.../plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.test.ts b/.../plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.test.ts
@@ -52,6 +52,7 @@ describe('syncMvtSourceData', () => {
     const syncContext = new MockSyncContext({ dataFilters: {} });
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: undefined,
@@ -82,6 +83,7 @@ describe('syncMvtSourceData', () => {
       tileSourceLayer: 'aggs',
       tileUrl: 'https://example.com/{x}/{y}/{z}.pbf',
       refreshToken: '12345',
+      hasLabels: false,
     });
   });
 
@@ -99,6 +101,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -112,6 +115,7 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'aggs',
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -142,6 +146,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -155,6 +160,7 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'aggs',
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -182,6 +188,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -195,6 +202,7 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'aggs',
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -230,6 +238,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -243,6 +252,7 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'barfoo', // tileSourceLayer is different then mockSource
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -270,6 +280,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -283,6 +294,7 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'aggs',
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -310,6 +322,7 @@ describe('syncMvtSourceData', () => {
     };
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: {
@@ -323,6 +336,49 @@ describe('syncMvtSourceData', () => {
             tileSourceLayer: 'aggs',
             tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
             refreshToken: '12345',
+            hasLabels: false,
+          };
+        },
+      } as unknown as DataRequest,
+      requestMeta: { ...prevRequestMeta },
+      source: mockSource,
+      syncContext,
+    });
+    // @ts-expect-error
+    sinon.assert.calledOnce(syncContext.startLoading);
+    // @ts-expect-error
+    sinon.assert.calledOnce(syncContext.stopLoading);
+  });
+
+  test('Should re-sync when hasLabel state changes', async () => {
+    const syncContext = new MockSyncContext({ dataFilters: {} });
+    const prevRequestMeta = {
+      ...syncContext.dataFilters,
+      applyGlobalQuery: true,
+      applyGlobalTime: true,
+      applyForceRefresh: true,
+      fieldNames: [],
+      sourceMeta: {},
+      isForceRefresh: false,
+      isFeatureEditorOpenForLayer: false,
+    };
+
+    await syncMvtSourceData({
+      hasLabels: true,
+      layerId: 'layer1',
+      layerName: 'my layer',
+      prevDataRequest: {
+        getMeta: () => {
+          return prevRequestMeta;
+        },
+        getData: () => {
+          return {
+            tileMinZoom: 4,
+            tileMaxZoom: 14,
+            tileSourceLayer: 'aggs',
+            tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
+            refreshToken: '12345',
+            hasLabels: false,
           };
         },
       } as unknown as DataRequest,
@@ -340,6 +396,7 @@ describe('syncMvtSourceData', () => {
     const syncContext = new MockSyncContext({ dataFilters: {} });
 
     await syncMvtSourceData({
+      hasLabels: false,
       layerId: 'layer1',
       layerName: 'my layer',
       prevDataRequest: undefined,

diff --git a/x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.ts b/x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.ts
@@ -20,16 +20,19 @@ export interface MvtSourceData {
   tileMaxZoom: number;
   tileUrl: string;
   refreshToken: string;
+  hasLabels: boolean;
 }
 
 export async function syncMvtSourceData({
+  hasLabels,
   layerId,
   layerName,
   prevDataRequest,
   requestMeta,
   source,
   syncContext,
 }: {
+  hasLabels: boolean;
   layerId: string;
   layerName: string;
   prevDataRequest: DataRequest | undefined;
@@ -56,7 +59,10 @@ export async function syncMvtSourceData({
       },
     });
     const canSkip =
-      !syncContext.forceRefreshDueToDrawing && noChangesInSourceState && noChangesInSearchState;
+      !syncContext.forceRefreshDueToDrawing &&
+      noChangesInSourceState &&
+      noChangesInSearchState &&
+      prevData.hasLabels === hasLabels;
 
     if (canSkip) {
       return;
@@ -72,7 +78,7 @@ export async function syncMvtSourceData({
         ? uuid()
         : prevData.refreshToken;
 
-    const tileUrl = await source.getTileUrl(requestMeta, refreshToken);
+    const tileUrl = await source.getTileUrl(requestMeta, refreshToken, hasLabels);
     if (source.isESSource()) {
       syncContext.inspectorAdapters.vectorTiles.addLayer(layerId, layerName, tileUrl);
     }
@@ -82,6 +88,7 @@ export async function syncMvtSourceData({
       tileMinZoom: source.getMinZoom(),
       tileMaxZoom: source.getMaxZoom(),
       refreshToken,
+      hasLabels,
     };
     syncContext.stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, sourceData, {});
   } catch (error) {

diff --git a/x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_vector_layer.tsx b/x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_vector_layer.tsx
@@ -219,6 +219,7 @@ export class MvtVectorLayer extends AbstractVectorLayer {
     await this._syncSupportsFeatureEditing({ syncContext, source: this.getSource() });
 
     await syncMvtSourceData({
+      hasLabels: this.getCurrentStyle().hasLabels(),
       layerId: this.getId(),
       layerName: await this.getDisplayName(),
       prevDataRequest: this.getSourceDataRequest(),

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
@@ -736,7 +736,10 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
       }
     }
 
+    const isSourceGeoJson = !this.getSource().isMvt();
     const filterExpr = getPointFilterExpression(
+      isSourceGeoJson,
+      this.getSource().isESSource(),
       this._getJoinFilterExpression(),
       timesliceMaskConfig
     );
@@ -843,6 +846,7 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
     const isSourceGeoJson = !this.getSource().isMvt();
     const filterExpr = getLabelFilterExpression(
       isSourceGeoJson,
+      this.getSource().isESSource(),
       this._getJoinFilterExpression(),
       timesliceMaskConfig
     );

diff --git a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.test.ts b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.test.ts
@@ -306,10 +306,10 @@ describe('ESGeoGridSource', () => {
     });
 
     it('getTileUrl', async () => {
-      const tileUrl = await mvtGeogridSource.getTileUrl(vectorSourceRequestMeta, '1234');
+      const tileUrl = await mvtGeogridSource.getTileUrl(vectorSourceRequestMeta, '1234', false);
 
       expect(tileUrl).toEqual(
-        "rootdir/api/maps/mvt/getGridTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=undefined&gridPrecision=8&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'1'%3A('0'%3Asize%2C'1'%3A0)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A''))%2C'6'%3A('0'%3Aaggs%2C'1'%3A())))&renderAs=heatmap&token=1234"
+        "rootdir/api/maps/mvt/getGridTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=undefined&gridPrecision=8&hasLabels=false&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'1'%3A('0'%3Asize%2C'1'%3A0)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A''))%2C'6'%3A('0'%3Aaggs%2C'1'%3A())))&renderAs=heatmap&token=1234"
       );
     });
   });

diff --git a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.tsx b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.tsx
@@ -471,7 +471,11 @@ export class ESGeoGridSource extends AbstractESAggSource implements IMvtVectorSo
     return 'aggs';
   }
 
-  async getTileUrl(searchFilters: VectorSourceRequestMeta, refreshToken: string): Promise<string> {
+  async getTileUrl(
+    searchFilters: VectorSourceRequestMeta,
+    refreshToken: string,
+    hasLabels: boolean
+  ): Promise<string> {
     const indexPattern = await this.getIndexPattern();
     const searchSource = await this.makeSearchSource(searchFilters, 0);
     searchSource.setField('aggs', this.getValueAggsDsl(indexPattern));
@@ -484,6 +488,7 @@ export class ESGeoGridSource extends AbstractESAggSource implements IMvtVectorSo
 ?geometryFieldName=${this._descriptor.geoField}\
 &index=${indexPattern.title}\
 &gridPrecision=${this._getGeoGridPrecisionResolutionDelta()}\
+&hasLabels=${hasLabels}\
 &requestBody=${encodeMvtResponseBody(searchSource.getSearchRequestBody())}\
 &renderAs=${this._descriptor.requestType}\
 &token=${refreshToken}`;

diff --git a/x-pack/plugins/maps/public/classes/sources/es_search_source/es_search_source.test.ts b/x-pack/plugins/maps/public/classes/sources/es_search_source/es_search_source.test.ts
@@ -114,9 +114,9 @@ describe('ESSearchSource', () => {
           geoField: geoFieldName,
           indexPatternId: 'ipId',
         });
-        const tileUrl = await esSearchSource.getTileUrl(searchFilters, '1234');
+        const tileUrl = await esSearchSource.getTileUrl(searchFilters, '1234', false);
         expect(tileUrl).toBe(
-          `rootdir/api/maps/mvt/getTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foobar-title-*&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'1'%3A('0'%3Asize%2C'1'%3A1000)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A'tooltipField%3A%20foobar'))%2C'6'%3A('0'%3AfieldsFromSource%2C'1'%3A!(tooltipField%2CstyleField))%2C'7'%3A('0'%3Asource%2C'1'%3A!(tooltipField%2CstyleField))))&token=1234`
+          `rootdir/api/maps/mvt/getTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foobar-title-*&hasLabels=false&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'1'%3A('0'%3Asize%2C'1'%3A1000)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A'tooltipField%3A%20foobar'))%2C'6'%3A('0'%3AfieldsFromSource%2C'1'%3A!(tooltipField%2CstyleField))%2C'7'%3A('0'%3Asource%2C'1'%3A!(tooltipField%2CstyleField))))&token=1234`
         );
       });
     });

diff --git a/x-pack/plugins/maps/public/classes/sources/es_search_source/es_search_source.tsx b/x-pack/plugins/maps/public/classes/sources/es_search_source/es_search_source.tsx
@@ -810,7 +810,11 @@ export class ESSearchSource extends AbstractESSource implements IMvtVectorSource
     return 'hits';
   }
 
-  async getTileUrl(searchFilters: VectorSourceRequestMeta, refreshToken: string): Promise<string> {
+  async getTileUrl(
+    searchFilters: VectorSourceRequestMeta,
+    refreshToken: string,
+    hasLabels: boolean
+  ): Promise<string> {
     const indexPattern = await this.getIndexPattern();
     const indexSettings = await loadIndexSettings(indexPattern.title);
 
@@ -847,6 +851,7 @@ export class ESSearchSource extends AbstractESSource implements IMvtVectorSource
     return `${mvtUrlServicePath}\
 ?geometryFieldName=${this._descriptor.geoField}\
 &index=${indexPattern.title}\
+&hasLabels=${hasLabels}\
 &requestBody=${encodeMvtResponseBody(searchSource.getSearchRequestBody())}\
 &token=${refreshToken}`;
   }

diff --git a/x-pack/plugins/maps/public/classes/sources/vector_source/mvt_vector_source.ts b/x-pack/plugins/maps/public/classes/sources/vector_source/mvt_vector_source.ts
@@ -13,7 +13,11 @@ export interface IMvtVectorSource extends IVectorSource {
    * IMvtVectorSource.getTileUrl returns the tile source URL.
    * Append refreshToken as a URL parameter to force tile re-fetch on refresh (not required)
    */
-  getTileUrl(searchFilters: VectorSourceRequestMeta, refreshToken: string): Promise<string>;
+  getTileUrl(
+    searchFilters: VectorSourceRequestMeta,
+    refreshToken: string,
+    hasLabels: boolean
+  ): Promise<string>;
 
   /*
    * Tile vector sources can contain multiple layers. For example, elasticsearch _mvt tiles contain the layers "hits", "aggs", and "meta".

diff --git a/x-pack/plugins/maps/public/classes/styles/vector/style_util.ts b/x-pack/plugins/maps/public/classes/styles/vector/style_util.ts
@@ -94,9 +94,9 @@ export function makeMbClampedNumberExpression({
   ];
 }
 
-export function getHasLabel(label: StaticTextProperty | DynamicTextProperty) {
+export function getHasLabel(label: StaticTextProperty | DynamicTextProperty): boolean {
   return label.isDynamic()
     ? label.isComplete()
     : (label as StaticTextProperty).getOptions().value != null &&
-        (label as StaticTextProperty).getOptions().value.length;
+        (label as StaticTextProperty).getOptions().value.length > 0;
 }