From b860950b2862bd425913ccb67e420826280f55dc Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 4 Dec 2017 16:01:30 -0800 Subject: [PATCH] Clone property values on input and output This allows programs to mutate them internally -- a questionable practice, but supported by previous versions. --- src/style/properties.js | 14 ++-- test/unit/style/style.test.js | 144 ++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 7 deletions(-) diff --git a/src/style/properties.js b/src/style/properties.js index 44efd4e1ed2..e8dd11f6280 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -1,7 +1,7 @@ // @flow const assert = require('assert'); -const {extend, easeCubicInOut} = require('../util/util'); +const {clone, extend, easeCubicInOut} = require('../util/util'); const interpolate = require('../style-spec/util/interpolate'); const {normalizePropertyExpression} = require('../style-spec/expression'); const Color = require('../style-spec/util/color'); @@ -167,7 +167,7 @@ class Transitionable { } getValue(name: S): PropertyValueSpecification | void { - return this._values[name].value.value; + return clone(this._values[name].value.value); } setValue(name: S, value: PropertyValueSpecification | void) { @@ -176,18 +176,18 @@ class Transitionable { } // Note that we do not _remove_ an own property in the case where a value is being reset // to the default: the transition might still be non-default. - this._values[name].value = new PropertyValue(this._values[name].property, value === null ? undefined : value); + this._values[name].value = new PropertyValue(this._values[name].property, value === null ? undefined : clone(value)); } getTransition(name: S): TransitionSpecification | void { - return this._values[name].transition; + return clone(this._values[name].transition); } setTransition(name: S, value: TransitionSpecification | void) { if (!this._values.hasOwnProperty(name)) { this._values[name] = new TransitionablePropertyValue(this._values[name].property); } - this._values[name].transition = value || undefined; + this._values[name].transition = clone(value) || undefined; } serialize() { @@ -358,11 +358,11 @@ class Layout { } getValue(name: S) { - return this._values[name].value; + return clone(this._values[name].value); } setValue(name: S, value: *) { - this._values[name] = new PropertyValue(this._values[name].property, value === null ? undefined : value); + this._values[name] = new PropertyValue(this._values[name].property, value === null ? undefined : clone(value)); } serialize() { diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 6ff76232a3b..997f759e871 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -1313,6 +1313,150 @@ test('Style#setPaintProperty', (t) => { }); }); + t.test('#5802 clones the input', (t) => { + const style = new Style(new StubMap()); + style.loadJSON({ + "version": 8, + "sources": {}, + "layers": [ + { + "id": "background", + "type": "background" + } + ] + }); + + style.on('style.load', () => { + const value = {stops: [[0, 'red'], [10, 'blue']]}; + style.setPaintProperty('background', 'background-color', value); + t.notEqual(style.getPaintProperty('background', 'background-color'), value); + t.ok(style._changed); + + style.update({}); + t.notOk(style._changed); + + value.stops[0][0] = 1; + style.setPaintProperty('background', 'background-color', value); + t.ok(style._changed); + + t.end(); + }); + }); + + t.end(); +}); + +test('Style#getPaintProperty', (t) => { + t.test('#5802 clones the output', (t) => { + const style = new Style(new StubMap()); + style.loadJSON({ + "version": 8, + "sources": {}, + "layers": [ + { + "id": "background", + "type": "background" + } + ] + }); + + style.on('style.load', () => { + style.setPaintProperty('background', 'background-color', {stops: [[0, 'red'], [10, 'blue']]}); + style.update({}); + t.notOk(style._changed); + + const value = style.getPaintProperty('background', 'background-color'); + value.stops[0][0] = 1; + style.setPaintProperty('background', 'background-color', value); + t.ok(style._changed); + + t.end(); + }); + }); + + t.end(); +}); + +test('Style#setLayoutProperty', (t) => { + t.test('#5802 clones the input', (t) => { + const style = new Style(new StubMap()); + style.loadJSON({ + "version": 8, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [] + } + } + }, + "layers": [ + { + "id": "line", + "type": "line", + "source": "geojson" + } + ] + }); + + style.on('style.load', () => { + const value = {stops: [[0, 'butt'], [10, 'round']]}; + style.setLayoutProperty('line', 'line-cap', value); + t.notEqual(style.getLayoutProperty('line', 'line-cap'), value); + t.ok(style._changed); + + style.update({}); + t.notOk(style._changed); + + value.stops[0][0] = 1; + style.setLayoutProperty('line', 'line-cap', value); + t.ok(style._changed); + + t.end(); + }); + }); + + t.end(); +}); + +test('Style#getLayoutProperty', (t) => { + t.test('#5802 clones the output', (t) => { + const style = new Style(new StubMap()); + style.loadJSON({ + "version": 8, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [] + } + } + }, + "layers": [ + { + "id": "line", + "type": "line", + "source": "geojson" + } + ] + }); + + style.on('style.load', () => { + style.setLayoutProperty('line', 'line-cap', {stops: [[0, 'butt'], [10, 'round']]}); + style.update({}); + t.notOk(style._changed); + + const value = style.getLayoutProperty('line', 'line-cap'); + value.stops[0][0] = 1; + style.setLayoutProperty('line', 'line-cap', value); + t.ok(style._changed); + + t.end(); + }); + }); + t.end(); });