[Maps] only provide visiblity check when vector layer has joins (elas…

…tic#51388) (elastic#51686) * [Maps] only provide visiblity check when vector layer has joins * clean up * properly set line filter expression * use ternary statements instead of if * review feedback
sorenlouv · Nov 26, 2019 · e32fb65 · e32fb65
1 parent a47a4b3
commit e32fb65
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 49 deletions.
diff --git a/x-pack/legacy/plugins/maps/common/constants.js b/x-pack/legacy/plugins/maps/common/constants.js
@@ -68,7 +68,7 @@ export const ZOOM_PRECISION = 2;
 export const ES_SIZE_LIMIT = 10000;
 
 export const FEATURE_ID_PROPERTY_NAME = '__kbn__feature_id__';
-export const FEATURE_VISIBLE_PROPERTY_NAME = '__kbn__isvisible__';
+export const FEATURE_VISIBLE_PROPERTY_NAME = '__kbn_isvisibleduetojoin__';
 
 export const MB_SOURCE_ID_LAYER_ID_PREFIX_DELIMITER = '_';
 

diff --git a/x-pack/legacy/plugins/maps/public/layers/util/mb_filter_expressions.js b/x-pack/legacy/plugins/maps/public/layers/util/mb_filter_expressions.js
@@ -0,0 +1,62 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { GEO_JSON_TYPE, FEATURE_VISIBLE_PROPERTY_NAME } from '../../../common/constants';
+
+const VISIBILITY_FILTER_CLAUSE = ['all',
+  [
+    '==',
+    ['get', FEATURE_VISIBLE_PROPERTY_NAME],
+    true
+  ]
+];
+
+const CLOSED_SHAPE_MB_FILTER = [
+  'any',
+  ['==', ['geometry-type'], GEO_JSON_TYPE.POLYGON],
+  ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POLYGON]
+];
+
+const VISIBLE_CLOSED_SHAPE_MB_FILTER = [
+  ...VISIBILITY_FILTER_CLAUSE,
+  CLOSED_SHAPE_MB_FILTER,
+];
+
+const ALL_SHAPE_MB_FILTER = [
+  'any',
+  ['==', ['geometry-type'], GEO_JSON_TYPE.POLYGON],
+  ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POLYGON],
+  ['==', ['geometry-type'], GEO_JSON_TYPE.LINE_STRING],
+  ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_LINE_STRING]
+];
+
+const VISIBLE_ALL_SHAPE_MB_FILTER = [
+  ...VISIBILITY_FILTER_CLAUSE,
+  ALL_SHAPE_MB_FILTER,
+];
+
+const POINT_MB_FILTER = [
+  'any',
+  ['==', ['geometry-type'], GEO_JSON_TYPE.POINT],
+  ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POINT]
+];
+
+const VISIBLE_POINT_MB_FILTER = [
+  ...VISIBILITY_FILTER_CLAUSE,
+  POINT_MB_FILTER,
+];
+
+export function getFillFilterExpression(hasJoins) {
+  return hasJoins ? VISIBLE_CLOSED_SHAPE_MB_FILTER : CLOSED_SHAPE_MB_FILTER;
+}
+
+export function getLineFilterExpression(hasJoins) {
+  return hasJoins ? VISIBLE_ALL_SHAPE_MB_FILTER : ALL_SHAPE_MB_FILTER;
+}
+
+export function getPointFilterExpression(hasJoins) {
+  return hasJoins ? VISIBLE_POINT_MB_FILTER : POINT_MB_FILTER;
+}
diff --git a/x-pack/legacy/plugins/maps/public/layers/vector_layer.js b/x-pack/legacy/plugins/maps/public/layers/vector_layer.js
@@ -10,7 +10,6 @@ import { AbstractLayer } from './layer';
 import { VectorStyle } from './styles/vector/vector_style';
 import { InnerJoin } from './joins/inner_join';
 import {
-  GEO_JSON_TYPE,
   FEATURE_ID_PROPERTY_NAME,
   SOURCE_DATA_ID_ORIGIN,
   FEATURE_VISIBLE_PROPERTY_NAME,
@@ -24,41 +23,11 @@ import { EuiIcon } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { DataRequestAbortError } from './util/data_request';
 import { assignFeatureIds } from './util/assign_feature_ids';
-
-const VISIBILITY_FILTER_CLAUSE = ['all',
-  [
-    '==',
-    ['get', FEATURE_VISIBLE_PROPERTY_NAME],
-    true
-  ]
-];
-
-const FILL_LAYER_MB_FILTER = [
-  ...VISIBILITY_FILTER_CLAUSE,
-  [
-    'any',
-    ['==', ['geometry-type'], GEO_JSON_TYPE.POLYGON],
-    ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POLYGON]
-  ]
-];
-
-const LINE_LAYER_MB_FILTER = [...VISIBILITY_FILTER_CLAUSE,
-  [
-    'any',
-    ['==', ['geometry-type'], GEO_JSON_TYPE.POLYGON],
-    ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POLYGON],
-    ['==', ['geometry-type'], GEO_JSON_TYPE.LINE_STRING],
-    ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_LINE_STRING]
-  ]
-];
-
-const POINT_LAYER_MB_FILTER = [...VISIBILITY_FILTER_CLAUSE,
-  [
-    'any',
-    ['==', ['geometry-type'], GEO_JSON_TYPE.POINT],
-    ['==', ['geometry-type'], GEO_JSON_TYPE.MULTI_POINT]
-  ]
-];
+import {
+  getFillFilterExpression,
+  getLineFilterExpression,
+  getPointFilterExpression,
+} from './util/mb_filter_expressions';
 
 export class VectorLayer extends AbstractLayer {
 
@@ -107,6 +76,10 @@ export class VectorLayer extends AbstractLayer {
     });
   }
 
+  _hasJoins() {
+    return this.getValidJoins().length > 0;
+  }
+
   isDataLoaded() {
     const sourceDataRequest = this.getSourceDataRequest();
     if (!sourceDataRequest || !sourceDataRequest.hasData()) {
@@ -495,12 +468,14 @@ export class VectorLayer extends AbstractLayer {
     }
 
     const sourceResult = await this._syncSource(syncContext);
-    if (!sourceResult.featureCollection || !sourceResult.featureCollection.features.length) {
+    if (
+      !sourceResult.featureCollection ||
+      !sourceResult.featureCollection.features.length ||
+      !this._hasJoins()) {
       return;
     }
 
     const joinStates = await this._syncJoins(syncContext);
-
     await this._performInnerJoins(sourceResult, joinStates, syncContext.updateSourceData);
   }
 
@@ -571,7 +546,11 @@ export class VectorLayer extends AbstractLayer {
         source: sourceId,
         paint: {}
       });
-      mbMap.setFilter(pointLayerId, POINT_LAYER_MB_FILTER);
+    }
+
+    const filterExpr = getPointFilterExpression(this._hasJoins());
+    if (filterExpr !== mbMap.getFilter(pointLayerId)) {
+      mbMap.setFilter(pointLayerId, filterExpr);
     }
 
     this._style.setMBPaintPropertiesForPoints({
@@ -592,7 +571,11 @@ export class VectorLayer extends AbstractLayer {
         type: 'symbol',
         source: sourceId,
       });
-      mbMap.setFilter(symbolLayerId, POINT_LAYER_MB_FILTER);
+    }
+
+    const filterExpr = getPointFilterExpression(this._hasJoins());
+    if (filterExpr !== mbMap.getFilter(symbolLayerId)) {
+      mbMap.setFilter(symbolLayerId, filterExpr);
     }
 
     this._style.setMBSymbolPropertiesForPoints({
@@ -606,14 +589,14 @@ export class VectorLayer extends AbstractLayer {
     const sourceId = this.getId();
     const fillLayerId = this._getMbPolygonLayerId();
     const lineLayerId = this._getMbLineLayerId();
+    const hasJoins = this._hasJoins();
     if (!mbMap.getLayer(fillLayerId)) {
       mbMap.addLayer({
         id: fillLayerId,
         type: 'fill',
         source: sourceId,
         paint: {}
       });
-      mbMap.setFilter(fillLayerId, FILL_LAYER_MB_FILTER);
     }
     if (!mbMap.getLayer(lineLayerId)) {
       mbMap.addLayer({
@@ -622,7 +605,6 @@ export class VectorLayer extends AbstractLayer {
         source: sourceId,
         paint: {}
       });
-      mbMap.setFilter(lineLayerId, LINE_LAYER_MB_FILTER);
     }
     this._style.setMBPaintProperties({
       alpha: this.getAlpha(),
@@ -632,9 +614,18 @@ export class VectorLayer extends AbstractLayer {
     });
 
     this.syncVisibilityWithMb(mbMap, fillLayerId);
+    mbMap.setLayerZoomRange(fillLayerId, this._descriptor.minZoom, this._descriptor.maxZoom);
+    const fillFilterExpr = getFillFilterExpression(hasJoins);
+    if (fillFilterExpr !== mbMap.getFilter(fillLayerId)) {
+      mbMap.setFilter(fillLayerId, fillFilterExpr);
+    }
+
     this.syncVisibilityWithMb(mbMap, lineLayerId);
     mbMap.setLayerZoomRange(lineLayerId, this._descriptor.minZoom, this._descriptor.maxZoom);
-    mbMap.setLayerZoomRange(fillLayerId, this._descriptor.minZoom, this._descriptor.maxZoom);
+    const lineFilterExpr = getLineFilterExpression(hasJoins);
+    if (lineFilterExpr !== mbMap.getFilter(lineLayerId)) {
+      mbMap.setFilter(lineLayerId, lineFilterExpr);
+    }
   }
 
   _syncStylePropertiesWithMb(mbMap) {

diff --git a/x-pack/test/functional/apps/maps/joins.js b/x-pack/test/functional/apps/maps/joins.js
@@ -102,7 +102,7 @@ export default function ({ getPageObjects, getService }) {
       const vectorSource = mapboxStyle.sources[VECTOR_SOURCE_ID];
 
       const visibilitiesOfFeatures = vectorSource.data.features.map(feature => {
-        return feature.properties.__kbn__isvisible__;
+        return feature.properties.__kbn_isvisibleduetojoin__;
       });
 
       expect(visibilitiesOfFeatures).to.eql([false, true, true, true]);
@@ -166,7 +166,7 @@ export default function ({ getPageObjects, getService }) {
         const vectorSource = mapboxStyle.sources[VECTOR_SOURCE_ID];
 
         const visibilitiesOfFeatures = vectorSource.data.features.map(feature => {
-          return feature.properties.__kbn__isvisible__;
+          return feature.properties.__kbn_isvisibleduetojoin__;
         });
 
         expect(visibilitiesOfFeatures).to.eql([false, true, false, false]);

diff --git a/x-pack/test/functional/apps/maps/mapbox_styles.js b/x-pack/test/functional/apps/maps/mapbox_styles.js
@@ -15,7 +15,7 @@ export const MAPBOX_STYLES = {
       'all',
       [
         '==',
-        ['get', '__kbn__isvisible__'],
+        ['get', '__kbn_isvisibleduetojoin__'],
         true
       ],
       [
@@ -89,7 +89,7 @@ export const MAPBOX_STYLES = {
       'all',
       [
         '==',
-        ['get', '__kbn__isvisible__'],
+        ['get', '__kbn_isvisibleduetojoin__'],
         true
       ],
       [
@@ -160,7 +160,7 @@ export const MAPBOX_STYLES = {
       'all',
       [
         '==',
-        ['get', '__kbn__isvisible__'],
+        ['get', '__kbn_isvisibleduetojoin__'],
         true
       ],
       [