Use setData operation when diffing geojson sources (#5332)

This change makes setStyle() smarter by using GeoJSONSource#setData() when possible, rather than always removing/re-adding the source. It _doesn't_ currently defer the setData() work to the once-per-frame batch update (Style#update()), but that should happen as part of #3692

* setData for geojson source on setStyle diff

* add style tests for setData

* diff test for setData

* remove from SourceCache and test uses setState

* remove from SourceCache and test uses setState

* remove leftover

* add setData back to Source interface

* add flow check and validation to Style#setGeoJSONSourceData

src/style-spec/diff.js

@@ -43,6 +43,11 @@ const operations = {
      */
     removeSource: 'removeSource',
 
+    /*
+     * { command: 'setGeoJSONSourceData', args: ['sourceId', data] }
+     */
+    setGeoJSONSourceData: 'setGeoJSONSourceData',
+
     /*
      * { command: 'setLayerZoomRange', args: ['layerId', 0, 22] }
      */
@@ -117,10 +122,15 @@ function diffSources(before, after, commands, sourcesRemoved) {
         if (!before.hasOwnProperty(sourceId)) {
             commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
         } else if (!isEqual(before[sourceId], after[sourceId])) {
-            // no update command, must remove then add
-            commands.push({ command: operations.removeSource, args: [sourceId] });
-            commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
-            sourcesRemoved[sourceId] = true;
+            if (before[sourceId].type === 'geojson' && after[sourceId].type === 'geojson') {
+                // geojson sources use setGeoJSONSourceData command to update
+                commands.push({ command: operations.setGeoJSONSourceData, args: [sourceId, after[sourceId].data] });
+            } else {
+                // no update command, must remove then add
+                commands.push({ command: operations.removeSource, args: [sourceId] });
+                commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
+                sourcesRemoved[sourceId] = true;
+            }
         }
     }
 }

src/style/style.js

@@ -19,6 +19,7 @@ const getSourceType = require('../source/source').getType;
 const setSourceType = require('../source/source').setType;
 const QueryFeatures = require('../source/query_features');
 const SourceCache = require('../source/source_cache');
+const GeoJSONSource = require('../source/geojson_source');
 const styleSpec = require('../style-spec/reference/latest');
 const MapboxGLFunction = require('../style-spec/function');
 const getWorkerPool = require('../util/global_worker_pool');
@@ -42,7 +43,8 @@ const supportedDiffOperations = util.pick(diff.operations, [
     'removeSource',
     'setLayerZoomRange',
     'setLight',
-    'setTransition'
+    'setTransition',
+    'setGeoJSONSourceData'
     // 'setGlyphs',
     // 'setSprite',
 ]);
@@ -503,6 +505,22 @@ class Style extends Evented {
         this._changed = true;
     }
 
+    /**
+    * Set the data of a GeoJSON source, given its id.
+    * @param {string} id id of the source
+    * @param {GeoJSON|string} data GeoJSON source
+    */
+    setGeoJSONSourceData(id: string, data: GeoJSON | string) {
+        this._checkLoaded();
+
+        assert(this.sourceCaches[id] !== undefined, 'There is no source with this ID');
+        const geojsonSource: GeoJSONSource = (this.sourceCaches[id].getSource(): any);
+        assert(geojsonSource.type === 'geojson');
+
+        geojsonSource.setData(data);
+        this._changed = true;
+    }
+
     /**
      * Get a source by id.
      * @param {string} id id of the desired source

test/unit/style-spec/diff.test.js

@@ -111,6 +111,36 @@ t('diff', (t) => {
         { command: 'addSource', args: ['foo', 1] }
     ], 'add a source');
 
+    t.deepEqual(diffStyles({
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] }
+            }
+        }
+    }, {
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: {
+                    type: 'FeatureCollection',
+                    features: [{
+                        type: 'Feature',
+                        geometry: { type: 'Point', coordinates: [10, 20] }
+                    }]
+                }
+            }
+        }
+    }), [
+        { command: 'setGeoJSONSourceData', args: ['foo', {
+            type: 'FeatureCollection',
+            features: [{
+                type: 'Feature',
+                geometry: { type: 'Point', coordinates: [10, 20] }
+            }]
+        }]}
+    ], 'update a geojson source');
+
     t.deepEqual(diffStyles({}, {
         metadata: { 'mapbox:author': 'nobody' }
     }), [], 'ignore style metadata');

test/unit/style/style.test.js

@@ -405,6 +405,7 @@ test('Style#setState', (t) => {
             'setFilter',
             'addSource',
             'removeSource',
+            'setGeoJSONSourceData',
             'setLayerZoomRange',
             'setLight'
         ].forEach((method) => t.stub(style, method).callsFake(() => t.fail(`${method} called`)));
@@ -456,6 +457,49 @@ test('Style#setState', (t) => {
         });
     });
 
+    t.test('sets GeoJSON source data if different', (t) => {
+        const initialState = createStyleJSON({
+            "sources": { "source-id": createGeoJSONSource() }
+        });
+
+        const geoJSONSourceData = {
+            "type": "FeatureCollection",
+            "features": [
+                {
+                    "type": "Feature",
+                    "geometry": {
+                        "type": "Point",
+                        "coordinates": [125.6, 10.1]
+                    }
+                }
+            ]
+        };
+
+        const nextState = createStyleJSON({
+            "sources": {
+                "source-id": {
+                    "type": "geojson",
+                    "data": geoJSONSourceData
+                }
+            }
+        });
+
+        const style = new Style(initialState);
+
+        style.on('style.load', () => {
+            const geoJSONSource = style.sourceCaches['source-id'].getSource();
+            t.spy(style, 'setGeoJSONSourceData');
+            t.spy(geoJSONSource, 'setData');
+            const didChange = style.setState(nextState);
+
+            t.ok(style.setGeoJSONSourceData.calledWith('source-id', geoJSONSourceData));
+            t.ok(geoJSONSource.setData.calledWith(geoJSONSourceData));
+            t.ok(didChange);
+            t.same(style.stylesheet, nextState);
+            t.end();
+        });
+    });
+
     t.end();
 });
 
@@ -644,6 +688,42 @@ test('Style#removeSource', (t) => {
     t.end();
 });
 
+test('Style#setData', (t) => {
+    t.test('throws before loaded', (t) => {
+        const style = new Style(createStyleJSON({
+            "sources": { "source-id": createGeoJSONSource() }
+        }), new StubMap());
+        const geoJSONSourceData = {
+            "type": "FeatureCollection",
+            "features": [
+                {
+                    "type": "Feature",
+                    "geometry": { "type": "Point", "coordinates": [125.6, 10.1] }
+                }
+            ]
+        };
+        t.throws(() => {
+            style.setGeoJSONSourceData('source-id', geoJSONSourceData);
+        }, Error, /load/i);
+        style.on('style.load', () => {
+            t.end();
+        });
+    });
+
+    t.test('throws on non-existence', (t) => {
+        const style = new Style(createStyleJSON(), new StubMap()),
+            geoJSONSourceData = { type: "FeatureCollection", "features": [] };
+        style.on('style.load', () => {
+            t.throws(() => {
+                style.setGeoJSONSourceData('source-id', geoJSONSourceData);
+            }, Error, /There is no source with this ID/);
+            t.end();
+        });
+    });
+
+    t.end();
+});
+
 test('Style#addLayer', (t) => {
     t.test('throw before loaded', (t) => {
         const style = new Style(createStyleJSON(), new StubMap()),