Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use setData operation when diffing geojson sources #5332

Merged
merged 9 commits into from
Sep 26, 2017
18 changes: 14 additions & 4 deletions src/style-spec/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ const operations = {
*/
removeSource: 'removeSource',

/*
* { command: 'setGeoJSONSourceData', args: ['sourceId', data] }
*/
setGeoJSONSourceData: 'setGeoJSONSourceData',

/*
* { command: 'setLayerZoomRange', args: ['layerId', 0, 22] }
*/
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down
20 changes: 19 additions & 1 deletion src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -42,7 +43,8 @@ const supportedDiffOperations = util.pick(diff.operations, [
'removeSource',
'setLayerZoomRange',
'setLight',
'setTransition'
'setTransition',
'setGeoJSONSourceData'
// 'setGlyphs',
// 'setSprite',
]);
Expand Down Expand Up @@ -500,6 +502,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
Expand Down
30 changes: 30 additions & 0 deletions test/unit/style-spec/diff.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
80 changes: 80 additions & 0 deletions test/unit/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`)));
Expand Down Expand Up @@ -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();
});

Expand Down Expand Up @@ -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()),
Expand Down