Skip to content

Commit

Permalink
Add api-supported-mode to spec.validate (#8663)
Browse files Browse the repository at this point in the history
The Mapbox Styles API uses stricter stylesheet validation than Mapbox GL JS. This commit adds a new function to the style spec and a new option to the mapbox-gl-validate CLI tool that will inform the user if their style is fit to be uploaded to the Mapbox style API.
  • Loading branch information
samanpwbb authored Sep 25, 2019
1 parent 30db469 commit fe621bb
Show file tree
Hide file tree
Showing 44 changed files with 1,096 additions and 38 deletions.
2 changes: 2 additions & 0 deletions src/style-spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,5 @@ $ gl-style-validate style.json

Will validate the given style JSON and print errors to stdout. Provide a
`--json` flag to get JSON output.

To validate that a style can be uploaded to the Mapbox Styles API, use the `--mapbox-api-supported` flag.
12 changes: 10 additions & 2 deletions src/style-spec/bin/gl-style-validate
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@


var argv = require('minimist')(process.argv.slice(2), {
boolean: 'json'
boolean: 'json',
boolean: 'mapbox-api-supported'
}),
validate = require('../').validate,
validateMapboxApiSupported = require('../').validateMapboxApiSupported
rw = require('rw'),
status = 0;

Expand All @@ -17,7 +19,12 @@ if (!argv._.length) {
}

argv._.forEach(function(file) {
var errors = validate(rw.readFileSync(file, 'utf8'));
var errors;
if (argv['mapbox-api-supported']) {
errors = validateMapboxApiSupported(rw.readFileSync(file, 'utf8'));
} else {
errors = validate(rw.readFileSync(file, 'utf8'));
}
if (errors.length) {
if (argv.json) {
process.stdout.write(JSON.stringify(errors, null, 2));
Expand All @@ -39,4 +46,5 @@ function help() {
console.log('');
console.log('options:');
console.log('--json output errors as json');
console.log('--mapbox-api-supported validate compatibility with Mapbox Styles API');
}
14 changes: 14 additions & 0 deletions src/style-spec/read_style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import ParsingError from './error/parsing_error';
import jsonlint from '@mapbox/jsonlint-lines-primitives';

export default function readStyle(style) {
if (style instanceof String || typeof style === 'string' || style instanceof Buffer) {
try {
return jsonlint.parse(style.toString());
} catch (e) {
throw new ParsingError(e);
}
}

return style;
}
2 changes: 2 additions & 0 deletions src/style-spec/style-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import convertFunction from './function/convert';
import {eachSource, eachLayer, eachProperty} from './visit';

import validate from './validate_style';
import validateMapboxApiSupported from './validate_mapbox_api_supported';

const expression = {
StyleExpression,
Expand Down Expand Up @@ -111,6 +112,7 @@ export {
Color,
styleFunction as function,
validate,
validateMapboxApiSupported,
visit
};

Expand Down
171 changes: 171 additions & 0 deletions src/style-spec/validate_mapbox_api_supported.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// @flow

import validateStyle from './validate_style.min';
import {v8} from './style-spec';
import readStyle from './read_style';
import ValidationError from './error/validation_error';
import getType from './util/get_type';

const SUPPORTED_SPEC_VERSION = 8;
const MAX_SOURCES_IN_STYLE = 15;

function isValid(value: ?string, regex: RegExp): boolean {
if (!value || getType(value) !== 'string') return true;
return !!value.match(regex);
}

function getSourceCount(source: Object): number {
if (source.url) {
return source.url.split(',').length;
} else {
return 0;
}
}

function getAllowedKeyErrors(obj: Object, keys: Array<*>, path: ?string): Array<?ValidationError> {
const allowed = new Set(keys);
const errors = [];
Object.keys(obj).forEach(k => {
if (!allowed.has(k)) {
const prop = path ? `${path}.${k}` : null;
errors.push(new ValidationError(prop, obj[k], `Unsupported property "${k}"`));
}
});
return errors;
}

function getSourceErrors(source: Object, i: number): Array<?ValidationError> {
const errors = [];

/*
* Inlined sources are not supported by the Mapbox Styles API, so only
* "type", "url", and "tileSize" properties are valid
*/
const sourceKeys = ['type', 'url', 'tileSize'];
errors.push(...getAllowedKeyErrors(source, sourceKeys, 'source'));

/*
* "sprite" is optional. If present, valid examples:
* mapbox://mapbox.abcd1234
* mapbox://penny.abcd1234
* mapbox://mapbox.abcd1234,penny.abcd1234
*/
const sourceUrlPattern = /^mapbox:\/\/([^/]*)$/;
if (!isValid(source.url, sourceUrlPattern)) {
errors.push(new ValidationError(`sources[${i}]`, source.url, 'Style must reference sources hosted by Mapbox'));
}

return errors;
}

function getSourcesErrors(sources: Object): Array<?ValidationError> {
const errors = [];
let count = 0;

Object.keys(sources).forEach((s: string, i: number) => {
const sourceErrors = getSourceErrors(sources[s], i);

// If source has errors, skip counting
if (!sourceErrors.length) {
count = count + getSourceCount(sources[s]);
}

errors.push(...sourceErrors);
});

if (count > MAX_SOURCES_IN_STYLE) {
errors.push(new ValidationError('sources', null, `Styles must contain ${MAX_SOURCES_IN_STYLE} or fewer sources`));
}

return errors;
}

function getRootErrors(style: Object, specKeys: Array<any>): Array<?ValidationError> {
const errors = [];

/*
* The following keys are optional but fully managed by the Mapbox Styles
* API. Values on stylesheet on POST or PATCH will be ignored: "owner",
* "id", "cacheControl", "draft", "created", "modified"
*
* The following keys are optional. The Mapbox Styles API respects value on
* stylesheet on PATCH, but ignores the value on POST: "visibility"
*/
const optionalRootProperties = [
'owner',
'id',
'cacheControl',
'draft',
'created',
'modified',
'visibility'
];

const allowedKeyErrors = getAllowedKeyErrors(style, [...specKeys, ...optionalRootProperties]);
errors.push(...allowedKeyErrors);

if (style.version > SUPPORTED_SPEC_VERSION || style.version < SUPPORTED_SPEC_VERSION) {
errors.push(new ValidationError('version', style.version, `style version must be ${SUPPORTED_SPEC_VERSION}`));
}

/*
* "glyphs" is optional. If present, valid examples:
* mapbox://fonts/penny/{fontstack}/{range}.pbf
* mapbox://fonts/mapbox/{fontstack}/{range}.pbf
*/
const glyphUrlPattern = /^mapbox:\/\/fonts\/([^/]*)\/{fontstack}\/{range}.pbf$/;
if (!isValid(style.glyphs, glyphUrlPattern)) {
errors.push(new ValidationError('glyphs', style.glyphs, 'Styles must reference glyphs hosted by Mapbox'));
}

/*
* "sprite" is optional. If present, valid examples:
* mapbox://sprites/penny/abcd1234
* mapbox://sprites/mapbox/abcd1234/draft
* mapbox://sprites/cyrus/abcd1234/abcd1234
*/
const spriteUrlPattern = /^mapbox:\/\/sprites\/([^/]*)\/([^/]*)\/?([^/]*)?$/;
if (!isValid(style.sprite, spriteUrlPattern)) {
errors.push(new ValidationError('sprite', style.sprite, 'Styles must reference sprites hosted by Mapbox'));
}

/*
* "visibility" is optional. If present, valid examples:
* "private"
* "public"
*/
const visibilityPattern = /^(public|private)$/;
if (!isValid(style.visibility, visibilityPattern)) {
errors.push(new ValidationError('visibility', style.visibility, 'Style visibility must be public or private'));
}

return errors;
}

/**
* Validate a Mapbox GL style against the style specification and check for
* compatibility with the Mapbox Styles API.
*
* @param {Object} style The style to be validated.
* @returns {Array<ValidationError>}
* @example
* var validateMapboxApiSupported = require('mapbox-gl-style-spec/lib/validate_style_mapbox_api_supported.js');
* var errors = validateMapboxApiSupported(style);
*/
export default function validateMapboxApiSupported(style: Object): Array<?ValidationError> {
let s = style;
try {
s = readStyle(s);
} catch (e) {
return [e];
}

let errors = validateStyle(s, v8)
.concat(getRootErrors(s, Object.keys(v8.$root)));

if (s.sources) {
errors = errors.concat(getSourcesErrors(s.sources));
}

return errors;
}
21 changes: 9 additions & 12 deletions src/style-spec/validate_style.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@

import validateStyleMin from './validate_style.min';
import ParsingError from './error/parsing_error';
import jsonlint from '@mapbox/jsonlint-lines-primitives';
import {v8} from './style-spec';
import readStyle from './read_style';

/**
* Validate a Mapbox GL style against the style specification.
Expand All @@ -20,18 +19,16 @@ import {v8} from './style-spec';
* var errors = validate(style);
*/

export default function validateStyle(style, styleSpec) {
if (style instanceof String || typeof style === 'string' || style instanceof Buffer) {
try {
style = jsonlint.parse(style.toString());
} catch (e) {
return [new ParsingError(e)];
}
}
export default function validateStyle(style, styleSpec = v8) {
let s = style;

styleSpec = styleSpec || v8;
try {
s = readStyle(s);
} catch (e) {
return [e];
}

return validateStyleMin(style, styleSpec);
return validateStyleMin(s, styleSpec);
}

export const source = validateStyleMin.source;
Expand Down
3 changes: 1 addition & 2 deletions src/style-spec/validate_style.min.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ import validateLayoutProperty from './validate/validate_layout_property';
* var validate = require('mapbox-gl-style-spec/lib/validate_style.min');
* var errors = validate(style);
*/
function validateStyleMin(style, styleSpec) {
styleSpec = styleSpec || latestStyleSpec;
function validateStyleMin(style, styleSpec = latestStyleSpec) {

let errors = [];

Expand Down
14 changes: 14 additions & 0 deletions test/unit/style-spec/fixture/bad-color.output-api-supported.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
"message": "layers[0].paint.fill-color: color expected, array found",
"line": 16
},
{
"message": "layers[0].paint.fill-outline-color: color expected, \"not a color\" found",
"line": 17
},
{
"message": "layers[1].paint.fill-outline-color: color expected, array found",
"line": 26
}
]
14 changes: 14 additions & 0 deletions test/unit/style-spec/fixture/constants.output-api-supported.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
"message": "constants: constants have been deprecated as of v8",
"line": 9
},
{
"message": "layers[1].paint.fill-color: color expected, array found",
"line": 28
},
{
"message": "Unsupported property \"constants\"",
"line": 9
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
{
"message": "Unsupported property \"extrakey\"",
"line": 13
}
]
62 changes: 62 additions & 0 deletions test/unit/style-spec/fixture/filters.output-api-supported.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
[
{
"message": "layers[0].filter: array expected, object found",
"line": 15
},
{
"message": "layers[1].filter: filter array must have at least 1 element",
"line": 22
},
{
"message": "layers[2].filter[0]: Unknown expression \"=\". If you wanted a literal array, use [\"literal\", [...]].",
"line": 29
},
{
"message": "layers[3].filter: Expected two or three arguments.",
"line": 40
},
{
"message": "layers[4].filter: Expected two or three arguments.",
"line": 47
},
{
"message": "layers[5].filter[1]: string expected, number found",
"line": 59
},
{
"message": "layers[6].filter[2]: Expected an array with at least one element. If you wanted a literal array, use [\"literal\", []].",
"line": 68
},
{
"message": "layers[7].filter[2]: expected one of [Point, LineString, Polygon], \"value\" found",
"line": 82
},
{
"message": "layers[8].filter: \"$type\" cannot be use with operator \">\"",
"line": 90
},
{
"message": "layers[8].filter[2]: expected one of [Point, LineString, Polygon], \"value\" found",
"line": 93
},
{
"message": "layers[9].filter[1]: filter array must have at least 1 element",
"line": 103
},
{
"message": "layers[12].filter[2]: Expected object but found string instead.",
"line": 131
},
{
"message": "layers[13].filter[1]: Bare objects invalid. Use [\"literal\", {...}] instead.",
"line": 142
},
{
"message": "layers[14].filter[2][1]: string expected, array found",
"line": 152
},
{
"message": "layers[15].filter: \"feature-state\" data expressions are not supported with filters.",
"line": 159
}
]
Loading

0 comments on commit fe621bb

Please sign in to comment.