Fix for layer not found error in featuresAt function (#1194)

* issue #1183 updated featuresAt function to check if style layers exist before querying against them. updated existing test cases to account for this check and added a new test case to ensure missing layers are not included in the queryRenderedFeatures call.

* use map.getLayer instead of map.style.getLayer. update tests.

src/lib/features_at.js

@@ -29,7 +29,8 @@ function featuresAt(event, bbox, ctx, buffer) {
   const box = (event) ? mapEventToBoundingBox(event, buffer) : bbox;
 
   const queryParams = {};
-  if (ctx.options.styles) queryParams.layers = ctx.options.styles.map(s => s.id);
+
+  if (ctx.options.styles) queryParams.layers = ctx.options.styles.map(s => s.id).filter(id => ctx.map.getLayer(id) != null);
 
   const features = ctx.map.queryRenderedFeatures(box, queryParams)
     .filter(feature => META_TYPES.indexOf(feature.properties.meta) !== -1);

test/features_at.test.js

@@ -1,12 +1,27 @@
 import test from 'tape';
-import {stub} from 'sinon';
 import featuresAt from '../src/lib/features_at';
+import styles from '../src/lib/theme';
+import * as Constants from '../src/constants';
+import setupOptions from '../src/options';
 
-function createMockContext() {
-  return {
-    options: {},
-    map: {
-      queryRenderedFeatures: stub().returns([{
+/**
+ * Mock of the addLayers function in setup
+ */
+function addLayers(ctx) {
+  // drawn features style
+  ctx.map.addSource(Constants.sources.COLD, {
+    data: {
+      type: Constants.geojsonTypes.FEATURE_COLLECTION,
+      features: []
+    },
+    type: 'geojson'
+  });
+
+  // hot features style
+  ctx.map.addSource(Constants.sources.HOT, {
+    data: {
+      type: Constants.geojsonTypes.FEATURE_COLLECTION,
+      features: [{
         type: 'Feature',
         properties: {
           meta: 'feature',
@@ -46,20 +61,96 @@ function createMockContext() {
           type: 'Point',
           coordinates: [10, 10]
         }
-      }])
+      }]
+    },
+    type: 'geojson'
+  });
+
+  ctx.options.styles.forEach((style) => {
+    ctx.map.addLayer(style);
+  });
+}
+
+/**
+ * Mock context with a simplified mapbox-gl-js map (including some source/style/layer interactions)
+ */
+function createMockContext() {
+  const _layers = {};
+  const sources = {};
+  const addSource = (id, source) => {
+    _layers[id] = source;
+  };
+  const style = {
+    _layers,
+    getLayer: id => _layers[id],
+    addLayer: ((layerObject) => {
+      addSource(layerObject.id, layerObject);
+    }),
+    addSource,
+    removeSource: (id) => {
+      delete _layers[id];
+    },
+    queryRenderedFeatures: (bbox, params) => {
+      const features = [];
+      const includedSources = {};
+      if (params && params.layers) {
+        for (const layerId of params.layers) {
+          const layer = _layers[layerId];
+          if (!layer) {
+            // this layer is not in the style.layers array
+            throw new ErrorEvent(new Error(`The layer '${layerId}' does not exist in the map's style and cannot be queried for features.`));
+          }
+          includedSources[layer.source] = true;
+        }
+      }
+      Object.keys(includedSources).filter(source => includedSources[source] != null).forEach((source) => {
+        if (sources[source] && sources[source].data) {
+          features.push(...sources[source].data.features);
+        }
+      });
+      return features;
     }
   };
+
+  const context = {
+    options: {
+      styles
+    },
+    map: {
+      setStyle: (newStyle) => {
+        Object.keys(_layers).forEach(key => delete _layers[key]);
+        Object.values(newStyle).forEach((s) => {
+          style.addLayer(s);
+        });
+      },
+      addSource: (id, source) => {
+        sources[id] = source;
+        style.addSource(id, source);
+      },
+      removeSource: (id) => {
+        style.removeSource(id);
+        delete sources[id];
+      },
+      getLayer: id => style.getLayer(id),
+      addLayer: (layer) => {
+        style.addLayer(layer);
+      },
+      style,
+      queryRenderedFeatures: (bbox, params) => style.queryRenderedFeatures(bbox, params)
+    }
+  };
+
+  context.options = setupOptions(context.options);
+
+  addLayers(context);
+
+  return context;
 }
 
 test('featuresAt with click bounding box', (t) => {
   const mockContext = createMockContext();
   const result = featuresAt.click(null, [[10, 10], [20, 20]], mockContext);
 
-  t.equal(mockContext.map.queryRenderedFeatures.callCount, 1);
-  t.deepEqual(mockContext.map.queryRenderedFeatures.getCall(0).args, [
-    [[10, 10], [20, 20]],
-    {}
-  ]);
   t.deepEqual(result, [{
     type: 'Feature',
     properties: {
@@ -89,11 +180,6 @@ test('featuresAt with touch bounding box', (t) => {
   const mockContext = createMockContext();
   const result = featuresAt.touch(null, [[10, 10], [20, 20]], mockContext);
 
-  t.equal(mockContext.map.queryRenderedFeatures.callCount, 1);
-  t.deepEqual(mockContext.map.queryRenderedFeatures.getCall(0).args, [
-    [[10, 10], [20, 20]],
-    {}
-  ]);
   t.deepEqual(result, [{
     type: 'Feature',
     properties: {
@@ -119,3 +205,41 @@ test('featuresAt with touch bounding box', (t) => {
   t.end();
 });
 
+test('featuresAt should not include missing style layers', (t) => {
+  const mockContext = createMockContext();
+
+  // mock of map's setStyle, which will remove all mapbox-gl-draw styles until the data event is fired, in which mapbox-gl-draw adds back in the styles.
+  mockContext.map.setStyle({});
+
+  // featuresAt should return no features if the styles have not finished adding back in
+  let result = featuresAt.touch(null, [[10, 10], [20, 20]], mockContext);
+  t.deepEqual(result, [], 'sorts, filters based on properties.meta, removes duplicates');
+
+  // mock adding layers back, similar to data event that fires and mapbox-gl-draw subsequently checks for any missing layers and adds them back in.
+  addLayers(mockContext);
+
+  result = featuresAt.touch(null, [[10, 10], [20, 20]], mockContext);
+  t.deepEqual(result, [{
+    type: 'Feature',
+    properties: {
+      meta: 'vertex',
+      id: 'baz'
+    },
+    geometry: {
+      type: 'Point',
+      coordinates: [10, 10]
+    }
+  }, {
+    type: 'Feature',
+    properties: {
+      meta: 'feature',
+      id: 'foo'
+    },
+    geometry: {
+      type: 'LineString',
+      coordinates: [[0, 0], [1, 1], [2, 2]]
+    }
+  }], 'sorts, filters based on properties.meta, removes duplicates');
+
+  t.end();
+});

test/utils/create_map.js

@@ -7,6 +7,16 @@ class MockMap extends Evented {
     super();
 
     this.sources = {};
+    this.style = {
+      _layers: {},
+      getLayer: id => this.style._layers[id],
+      addSource: (id, source) => {
+        this.style._layers[id] = source;
+      },
+      removeSource: (id) => {
+        delete this.style._layers[id];
+      },
+    };
     this.options = {
       container: document.createElement('div'),
       ...options
@@ -34,11 +44,16 @@ class MockMap extends Evented {
     return true;
   }
 
+  getLayer(id) {
+    return this.style.getLayer(id);
+  }
+
   getContainer() {
     return this.options.container;
   }
 
   addSource(name, source) {
+    this.style.addSource(name, source);
     this.sources[name] = source;
   }
   removeSource(name) {
@@ -55,7 +70,6 @@ class MockMap extends Evented {
   }
 
   addLayer() {}
-  getLayer() {}
 
   queryRenderedFeatures([p0, p1]) {
     if (!Array.isArray(p0)) p0 = [p0.x, p0.y];