Skip to content

Commit

Permalink
add validation option to style setters (#7604)
Browse files Browse the repository at this point in the history 
* add validation option to style setters

* add StyleSetterOptions type
  • Loading branch information
@mollymerp
mollymerp authored Nov 26, 2018
1 parent c78fa7a commit aab7551
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 38 deletions.
16 changes: 10 additions & 6 deletions src/style/light.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { number as interpolate } from '../style-spec/util/interpolate';

import type {StylePropertySpecification} from '../style-spec/style-spec';
import type EvaluationParameters from './evaluation_parameters';

import type {StyleSetterOptions} from '../style/style';
import { Properties, Transitionable, Transitioning, PossiblyEvaluated, DataConstantProperty } from './properties';

import type {
Expand Down Expand Up @@ -86,13 +86,13 @@ class Light extends Evented {
return this._transitionable.serialize();
}

setLight(options?: LightSpecification) {
if (this._validate(validateLight, options)) {
setLight(light?: LightSpecification, options: StyleSetterOptions = {}) {
if (this._validate(validateLight, light, options)) {
return;
}

for (const name in options) {
const value = options[name];
for (const name in light) {
const value = light[name];
if (endsWith(name, TRANSITION_SUFFIX)) {
this._transitionable.setTransition(name.slice(0, -TRANSITION_SUFFIX.length), value);
} else {
Expand All @@ -113,7 +113,11 @@ class Light extends Evented {
this.properties = this._transitioning.possiblyEvaluate(parameters);
}

_validate(validate: Function, value: mixed) {
_validate(validate: Function, value: mixed, options?: {validate?: boolean}) {
if (options && options.validate === false) {
return false;
}

return emitValidationErrors(this, validate.call(validateStyle, extend({
value,
// Workaround for https://github.com/mapbox/mapbox-gl-js/issues/2407
Expand Down
29 changes: 15 additions & 14 deletions src/style/style.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ export type StyleOptions = {
localIdeographFontFamily?: string
};

export type StyleSetterOptions = {
validate?: boolean
};
/**
* @private
*/
Expand Down Expand Up @@ -200,9 +203,7 @@ class Style extends Evented {
});
}

loadJSON(json: StyleSpecification, options: {
validate?: boolean
} = {}) {
loadJSON(json: StyleSpecification, options: StyleSetterOptions = {}) {
this.fire(new Event('dataloading', {dataType: 'style'}));

this._request = browser.frame(() => {
Expand Down Expand Up @@ -481,7 +482,7 @@ class Style extends Evented {
return this.imageManager.listImages();
}

addSource(id: string, source: SourceSpecification, options?: {validate?: boolean}) {
addSource(id: string, source: SourceSpecification, options: StyleSetterOptions = {}) {
this._checkLoaded();

if (this.sourceCaches[id] !== undefined) {
Expand Down Expand Up @@ -567,7 +568,7 @@ class Style extends Evented {
* ID `before`, or appended if `before` is omitted.
* @param {string} [before] ID of an existing layer to insert before
*/
addLayer(layerObject: LayerSpecification | CustomLayerInterface, before?: string, options?: {validate?: boolean}) {
addLayer(layerObject: LayerSpecification | CustomLayerInterface, before?: string, options: StyleSetterOptions = {}) {
this._checkLoaded();

const id = layerObject.id;
Expand Down Expand Up @@ -733,7 +734,7 @@ class Style extends Evented {
this._updateLayer(layer);
}

setFilter(layerId: string, filter: ?FilterSpecification) {
setFilter(layerId: string, filter: ?FilterSpecification, options: StyleSetterOptions = {}) {
this._checkLoaded();

const layer = this.getLayer(layerId);
Expand All @@ -752,7 +753,7 @@ class Style extends Evented {
return;
}

if (this._validate(validateStyle.filter, `layers.${layer.id}.filter`, filter)) {
if (this._validate(validateStyle.filter, `layers.${layer.id}.filter`, filter, null, options)) {
return;
}

Expand All @@ -769,7 +770,7 @@ class Style extends Evented {
return clone(this.getLayer(layer).filter);
}

setLayoutProperty(layerId: string, name: string, value: any) {
setLayoutProperty(layerId: string, name: string, value: any, options: StyleSetterOptions = {}) {
this._checkLoaded();

const layer = this.getLayer(layerId);
Expand All @@ -780,7 +781,7 @@ class Style extends Evented {

if (deepEqual(layer.getLayoutProperty(name), value)) return;

layer.setLayoutProperty(name, value);
layer.setLayoutProperty(name, value, options);
this._updateLayer(layer);
}

Expand All @@ -800,7 +801,7 @@ class Style extends Evented {
return layer.getLayoutProperty(name);
}

setPaintProperty(layerId: string, name: string, value: any) {
setPaintProperty(layerId: string, name: string, value: any, options: StyleSetterOptions = {}) {
this._checkLoaded();

const layer = this.getLayer(layerId);
Expand All @@ -811,7 +812,7 @@ class Style extends Evented {

if (deepEqual(layer.getPaintProperty(name), value)) return;

const requiresRelayout = layer.setPaintProperty(name, value);
const requiresRelayout = layer.setPaintProperty(name, value, options);
if (requiresRelayout) {
this._updateLayer(layer);
}
Expand Down Expand Up @@ -1001,7 +1002,7 @@ class Style extends Evented {
return this.light.getLight();
}

setLight(lightOptions: LightSpecification) {
setLight(lightOptions: LightSpecification, options: StyleSetterOptions = {}) {
this._checkLoaded();

const light = this.light.getLight();
Expand All @@ -1022,11 +1023,11 @@ class Style extends Evented {
}, this.stylesheet.transition)
};

this.light.setLight(lightOptions);
this.light.setLight(lightOptions, options);
this.light.updateTransitions(parameters);
}

_validate(validate: ({}) => void, key: string, value: any, props: any, options?: {validate?: boolean}) {
_validate(validate: ({}) => void, key: string, value: any, props: any, options: StyleSetterOptions = {}) {
if (options && options.validate === false) {
return false;
}
Expand Down
7 changes: 4 additions & 3 deletions src/style/style_layer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
} from '../style-spec/types';
import type {CustomLayerInterface} from './style_layer/custom_style_layer';
import type Map from '../ui/map';
import type {StyleSetterOptions} from './style';

const TRANSITION_SUFFIX = '-transition';

Expand Down Expand Up @@ -115,7 +116,7 @@ class StyleLayer extends Evented {
return this._unevaluatedLayout.getValue(name);
}

setLayoutProperty(name: string, value: mixed, options: {validate: boolean}) {
setLayoutProperty(name: string, value: mixed, options: StyleSetterOptions = {}) {
if (value !== null && value !== undefined) {
const key = `layers.${this.id}.layout.${name}`;
if (this._validate(validateLayoutProperty, key, name, value, options)) {
Expand All @@ -139,7 +140,7 @@ class StyleLayer extends Evented {
}
}

setPaintProperty(name: string, value: mixed, options: {validate: boolean}) {
setPaintProperty(name: string, value: mixed, options: StyleSetterOptions = {}) {
if (value !== null && value !== undefined) {
const key = `layers.${this.id}.paint.${name}`;
if (this._validate(validatePaintProperty, key, name, value, options)) {
Expand Down Expand Up @@ -221,7 +222,7 @@ class StyleLayer extends Evented {
});
}

_validate(validate: Function, key: string, name: string, value: mixed, options: {validate: boolean}) {
_validate(validate: Function, key: string, name: string, value: mixed, options: StyleSetterOptions = {}) {
if (options && options.validate === false) {
return false;
}
Expand Down
28 changes: 18 additions & 10 deletions src/ui/map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import type {PointLike} from '@mapbox/point-geometry';
import type {LngLatLike} from '../geo/lng_lat';
import type {LngLatBoundsLike} from '../geo/lng_lat_bounds';
import type {RequestParameters} from '../util/ajax';
import type {StyleOptions} from '../style/style';
import type {StyleOptions, StyleSetterOptions} from '../style/style';
import type {MapEvent, MapDataEvent} from './events';
import type {CustomLayerInterface} from '../style/style_layer/custom_style_layer';

Expand All @@ -52,7 +52,6 @@ import type {
} from '../style-spec/types';

type ControlPosition = 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right';

/* eslint-disable no-use-before-define */
type IControl = {
onAdd(map: Map): HTMLElement;
Expand Down Expand Up @@ -1264,15 +1263,18 @@ class Map extends Camera {
* @param {string} layer The ID of the layer to which the filter will be applied.
* @param {Array | null | undefined} filter The filter, conforming to the Mapbox Style Specification's
* [filter definition](https://www.mapbox.com/mapbox-gl-js/style-spec/#other-filter). If `null` or `undefined` is provided, the function removes any existing filter from the layer.
* @param {Object} [options]
* @param {boolean} [options.validate=true] Whether to check if the filter conforms to the Mapbox GL Style Specification. Disabling validation is a performance optimization that should only be used if you have previously validated the values you will be passing to this function.
*
* @returns {Map} `this`
* @example
* map.setFilter('my-layer', ['==', 'name', 'USA']);
* @see [Filter features within map view](https://www.mapbox.com/mapbox-gl-js/example/filter-features-within-map-view/)
* @see [Highlight features containing similar data](https://www.mapbox.com/mapbox-gl-js/example/query-similar-features/)
* @see [Create a timeline animation](https://www.mapbox.com/mapbox-gl-js/example/timeline-animation/)
*/
setFilter(layer: string, filter: ?FilterSpecification) {
this.style.setFilter(layer, filter);
setFilter(layer: string, filter: ?FilterSpecification, options: StyleSetterOptions = {}) {
this.style.setFilter(layer, filter, options);
return this._update(true);
}

Expand Down Expand Up @@ -1308,15 +1310,17 @@ class Map extends Camera {
* @param {string} name The name of the paint property to set.
* @param {*} value The value of the paint propery to set.
* Must be of a type appropriate for the property, as defined in the [Mapbox Style Specification](https://www.mapbox.com/mapbox-gl-style-spec/).
* @param {Object} [options]
* @param {boolean} [options.validate=true] Whether to check if `value` conforms to the Mapbox GL Style Specification. Disabling validation is a performance optimization that should only be used if you have previously validated the values you will be passing to this function.
* @returns {Map} `this`
* @example
* map.setPaintProperty('my-layer', 'fill-color', '#faafee');
* @see [Change a layer's color with buttons](https://www.mapbox.com/mapbox-gl-js/example/color-switcher/)
* @see [Adjust a layer's opacity](https://www.mapbox.com/mapbox-gl-js/example/adjust-layer-opacity/)
* @see [Create a draggable point](https://www.mapbox.com/mapbox-gl-js/example/drag-a-point/)
*/
setPaintProperty(layer: string, name: string, value: any) {
this.style.setPaintProperty(layer, name, value);
setPaintProperty(layer: string, name: string, value: any, options: StyleSetterOptions = {}) {
this.style.setPaintProperty(layer, name, value, options);
return this._update(true);
}

Expand All @@ -1337,12 +1341,14 @@ class Map extends Camera {
* @param {string} layer The ID of the layer to set the layout property in.
* @param {string} name The name of the layout property to set.
* @param {*} value The value of the layout propery. Must be of a type appropriate for the property, as defined in the [Mapbox Style Specification](https://www.mapbox.com/mapbox-gl-style-spec/).
* @param {Object} [options]
* @param {boolean} [options.validate=true] Whether to check if `value` conforms to the Mapbox GL Style Specification. Disabling validation is a performance optimization that should only be used if you have previously validated the values you will be passing to this function.
* @returns {Map} `this`
* @example
* map.setLayoutProperty('my-layer', 'visibility', 'none');
*/
setLayoutProperty(layer: string, name: string, value: any) {
this.style.setLayoutProperty(layer, name, value);
setLayoutProperty(layer: string, name: string, value: any, options: StyleSetterOptions = {}) {
this.style.setLayoutProperty(layer, name, value, options);
return this._update(true);
}

Expand All @@ -1361,10 +1367,12 @@ class Map extends Camera {
* Sets the any combination of light values.
*
* @param light Light properties to set. Must conform to the [Mapbox Style Specification](https://www.mapbox.com/mapbox-gl-style-spec/#light).
* @param {Object} [options]
* @param {boolean} [options.validate=true] Whether to check if the filter conforms to the Mapbox GL Style Specification. Disabling validation is a performance optimization that should only be used if you have previously validated the values you will be passing to this function.
* @returns {Map} `this`
*/
setLight(light: LightSpecification) {
this.style.setLight(light);
setLight(light: LightSpecification, options: StyleSetterOptions = {}) {
this.style.setLight(light, options);
return this._update(true);
}

Expand Down
39 changes: 34 additions & 5 deletions test/unit/style/light.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,41 @@ test('Light#getLight', (t) => {
});

test('Light#setLight', (t) => {
const light = new Light({});
light.setLight({ color: 'red', "color-transition": { duration: 3000 }});
light.updateTransitions({ transition: true }, {});
light.recalculate({zoom: 16, zoomHistory: {}, now: 1500});
t.test('sets light', (t) => {
const light = new Light({});
light.setLight({ color: 'red', "color-transition": { duration: 3000 }});
light.updateTransitions({ transition: true }, {});
light.recalculate({zoom: 16, zoomHistory: {}, now: 1500});
t.deepEqual(light.properties.get('color'), new Color(1, 0.5, 0.5, 1));
t.end();
});

t.test('validates by default', (t) => {
const light = new Light({});
const lightSpy = t.spy(light, '_validate');
t.stub(console, 'error');
light.setLight({ color: 'notacolor'});
light.updateTransitions({ transition: false}, {});
light.recalculate({zoom: 16, zoomHistory: {}, now: 10});
t.ok(lightSpy.calledOnce);
t.ok(console.error.calledOnce);
t.deepEqual(lightSpy.args[0][2], {});
t.end();
});

t.deepEqual(light.properties.get('color'), new Color(1, 0.5, 0.5, 1));

t.test('respects validation option', (t) => {
const light = new Light({});

const lightSpy = t.spy(light, '_validate');
light.setLight({ color: [999]}, {validate: false});
light.updateTransitions({ transition: false}, {});
light.recalculate({zoom: 16, zoomHistory: {}, now: 10});

t.ok(lightSpy.calledOnce);
t.deepEqual(lightSpy.args[0][2], {validate: false});
t.deepEqual(light.properties.get('color'), [999]);
t.end();
});
t.end();
});
Loading

0 comments on commit aab7551

Please sign in to comment.