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

add validation option to style setters #7604

Merged
merged 2 commits into from
Nov 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/style/light.js
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
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
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
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
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