From e94eec31ca160e29f221fdcd5d1f048f1b8cb117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 23 Jun 2016 17:16:00 -0400 Subject: [PATCH 01/14] add Lib.validate function: - to determine if an attribute val is valid --- src/lib/coerce.js | 19 +++ src/lib/index.js | 1 + test/jasmine/tests/lib_test.js | 223 +++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 9c67cb32014..a63496f6fdc 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -309,3 +309,22 @@ exports.coerceFont = function(coerce, attr, dfltObj) { return out; }; + +exports.validate = function(value, opts) { + var valObject = exports.valObjects[opts.valType]; + + if(opts.arrayOk && Array.isArray(value)) return true; + + if(valObject.validateFunction) { + return valObject.validateFunction(value, opts); + } + + var failed = {}, + out = failed, + propMock = { set: function(v) { out = v; } }; + + // 'failed' just something mutable that won't be === anything else + + valObject.coerceFunction(value, propMock, failed, opts); + return out !== failed; +}; diff --git a/src/lib/index.js b/src/lib/index.js index 32f3f811a67..0a3d8bf7854 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -21,6 +21,7 @@ lib.valObjects = coerceModule.valObjects; lib.coerce = coerceModule.coerce; lib.coerce2 = coerceModule.coerce2; lib.coerceFont = coerceModule.coerceFont; +lib.validate = coerceModule.validate; var datesModule = require('./dates'); lib.dateTime2ms = datesModule.dateTime2ms; diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index b736d31a73e..2db74f7154d 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -755,6 +755,229 @@ describe('Test lib.js:', function() { }); }); + fdescribe('validate', function() { + + function assert(shouldPass, shouldFail, valObject) { + shouldPass.forEach(function(v) { + var res = Lib.validate(v, valObject); + expect(res).toBe(true, JSON.stringify(v) + ' should pass'); + }); + + shouldFail.forEach(function(v) { + var res = Lib.validate(v, valObject); + expect(res).toBe(false, JSON.stringify(v) + ' should fail'); + }); + } + + it('should work for valType \'data_array\' where', function() { + var shouldPass = [[20], []], + shouldFail = ['a', {}, 20, undefined, null]; + + assert(shouldPass, shouldFail, { + valType: 'data_array' + }); + + assert(shouldPass, shouldFail, { + valType: 'data_array', + dflt: [1, 2] + }); + }); + + it('should work for valType \'enumerated\' where', function() { + assert(['a', 'b'], ['c', 1, null, undefined, ''], { + valType: 'enumerated', + values: ['a', 'b'], + dflt: 'a' + }); + + assert([1, '1', 2, '2'], ['c', 3, null, undefined, ''], { + valType: 'enumerated', + values: [1, 2], + coerceNumber: true, + dflt: 1 + }); + + assert(['a', 'b', [1, 2]], ['c', 1, null, undefined, ''], { + valType: 'enumerated', + values: ['a', 'b'], + arrayOk: true, + dflt: 'a' + }); + }); + + it('should work for valType \'boolean\' where', function() { + var shouldPass = [true, false], + shouldFail = ['a', 1, {}, [], null, undefined, '']; + + assert(shouldPass, shouldFail, { + valType: 'boolean', + dflt: true + }); + + assert(shouldPass, shouldFail, { + valType: 'boolean', + dflt: false + }); + }); + + it('should work for valType \'number\' where', function() { + var shouldPass = [20, '20', 1e6], + shouldFail = ['a', [], {}, null, undefined, '']; + + assert(shouldPass, shouldFail, { + valType: 'number' + }); + + assert(shouldPass, shouldFail, { + valType: 'number', + dflt: null + }); + + assert([20, '20'], [-10, '-10', 25, '25'], { + valType: 'number', + dflt: 20, + min: 0, + max: 21 + }); + + assert([20, '20', [1, 2]], ['a', {}], { + valType: 'number', + dflt: 20, + arrayOk: true + }); + }); + + it('should work for valType \'integer\' where', function() { + assert([1, 2, '3', '4'], ['a', 1.321321, {}, [], null, 2 / 3, undefined, null], { + valType: 'integer', + dflt: 1 + }); + + assert([1, 2, '3', '4'], [-1, '-2', 2.121, null, undefined, [], {}], { + valType: 'integer', + min: 0, + dflt: 1 + }); + }); + + it('should work for valType \'string\' where', function() { + + // This fails as the coerceFunction coerce all values + // (except when 'strict' is true) using String() + // + // Things like undefined, null, {}, [] should be considered valid! + // + // The question becomes how the change this while not + // scarifying perf?? + + assert(['3', '4', 'a'], [undefined, {}, [], null], { + valType: 'string', + dflt: 'a' + }); + + assert(['3', '4', 'a'], [undefined, {}, [], null, ''], { + valType: 'string', + dflt: 'a', + noBlank: true + }); + + assert(['3', '4'], [undefined, 1, {}, [], null, ''], { + valType: 'string', + dflt: 'a', + strict: true, + noBlank: true + }); + }); + + it('should work for valType \'color\' where', function() { + var shouldPass = ['red', '#d3d3d3', 'rgba(0,255,255,0.1)'], + shouldFail = [1, {}, [], 'rgq(233,122,332,1)', null, undefined]; + + assert(shouldPass, shouldFail, { + valType: 'color' + }); + }); + + it('should work for valType \'colorscale\' where', function() { + var good = [ [0, 'red'], [1, 'blue'] ], + bad = [ [0.1, 'red'], [1, 'blue'] ], + bad2 = [ [0], [1] ], + bad3 = [ ['red'], ['blue']], + bad4 = ['red', 'blue']; + + // Fails at [], should be an easy fix though. + + var shouldPass = ['Viridis', 'Greens', good], + shouldFail = ['red', 1, undefined, null, {}, [], bad, bad2, bad3, bad4]; + + assert(shouldPass, shouldFail, { + valType: 'colorscale' + }); + }); + + it('should work for valType \'angle\' where', function() { + var shouldPass = ['auto', '120', 270], + shouldFail = [{}, [], 'red', null, undefined, '']; + + assert(shouldPass, shouldFail, { + valType: 'angle', + dflt: 0 + }); + }); + + it('should work for valType \'subplotid\' where', function() { + var shouldPass = ['sp', 'sp1', 'sp4'], + shouldFail = [{}, [], 'sp0', 'spee1', null, undefined]; + + // This fails because coerceFunction depends on dflt + // having a length. + // + // Solution: don't use dflt as base string -> + // add other options to valObject + + assert(shouldPass, shouldFail, { + valType: 'subplotid', + dflt: 'sp' + }); + }); + + it('should work for valType \'flaglist\' where', function() { + var shouldPass = ['a', 'b', 'a+b', 'b+a', 'c'], + shouldFail = [{}, [], 'red', null, undefined, '', 'a + b']; + + assert(shouldPass, shouldFail, { + valType: 'flaglist', + flags: ['a', 'b'], + extras: ['c'] + }); + }); + + it('should work for valType \'any\' where', function() { + var shouldPass = ['', '120', null, false, {}, []], + shouldFail = [undefined]; + + assert(shouldPass, shouldFail, { + valType: 'any' + }); + }); + + it('should work for valType \'info_array\' where', function() { + var shouldPass = [[1, 2]], + shouldFail = [{}, [], ['aads', null], 'red', null, undefined, '']; + + // This fails. All array including [] are considered valid + + assert(shouldPass, shouldFail, { + valType: 'info_array', + items: [{ + valType: 'number', dflt: -20 + }, { + valType: 'number', dflt: 20 + }] + }); + }); + }); + describe('setCursor', function() { beforeEach(function() { From 9814edfff2d1f5ffa8c1d9010aa4648c20e5363e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 12:09:47 -0400 Subject: [PATCH 02/14] make is-valid colorscale array more strict --- .../colorscale/is_valid_scale_array.js | 32 +++++++++++-------- test/jasmine/tests/colorscale_test.js | 7 ++++ test/jasmine/tests/lib_test.js | 2 -- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/components/colorscale/is_valid_scale_array.js b/src/components/colorscale/is_valid_scale_array.js index 5c7bed6584e..ebf55b02b3d 100644 --- a/src/components/colorscale/is_valid_scale_array.js +++ b/src/components/colorscale/is_valid_scale_array.js @@ -14,20 +14,24 @@ var tinycolor = require('tinycolor2'); module.exports = function isValidScaleArray(scl) { var isValid = true, - highestVal = 0, - si; - - if(!Array.isArray(scl)) return false; - else { - if(+scl[0][0] !== 0 || +scl[scl.length - 1][0] !== 1) return false; - for(var i = 0; i < scl.length; i++) { - si = scl[i]; - if(si.length !== 2 || +si[0] < highestVal || !tinycolor(si[1]).isValid()) { - isValid = false; - break; - } - highestVal = +si[0]; + highestVal = 0; + + if(!Array.isArray(scl) || scl.length === 0) return false; + + if(!scl[0] || !scl[scl.length - 1]) return false; + + if(+scl[0][0] !== 0 || +scl[scl.length - 1][0] !== 1) return false; + + for(var i = 0; i < scl.length; i++) { + var si = scl[i]; + + if(si.length !== 2 || +si[0] < highestVal || !tinycolor(si[1]).isValid()) { + isValid = false; + break; } - return isValid; + + highestVal = +si[0]; } + + return isValid; }; diff --git a/test/jasmine/tests/colorscale_test.js b/test/jasmine/tests/colorscale_test.js index dba9936b7a1..3610c17cdec 100644 --- a/test/jasmine/tests/colorscale_test.js +++ b/test/jasmine/tests/colorscale_test.js @@ -20,6 +20,13 @@ describe('Test colorscale:', function() { it('should accept only array of 2-item arrays', function() { expect(isValidScale('a')).toBe(false); + expect(isValidScale([])).toBe(false); + expect(isValidScale([null, undefined])).toBe(false); + expect(isValidScale([{}, [1, 'rgb(0, 0, 200']])).toBe(false); + expect(isValidScale([[0, 'rgb(200, 0, 0)'], {}])).toBe(false); + expect(isValidScale([[0, 'rgb(0, 0, 200)'], undefined])).toBe(false); + expect(isValidScale([null, [1, 'rgb(0, 0, 200)']])).toBe(false); + expect(isValidScale(['a', 'b'])).toBe(false); expect(isValidScale(['a'])).toBe(false); expect(isValidScale([['a'], ['b']])).toBe(false); diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 2db74f7154d..305672fb7b2 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -905,8 +905,6 @@ describe('Test lib.js:', function() { bad3 = [ ['red'], ['blue']], bad4 = ['red', 'blue']; - // Fails at [], should be an easy fix though. - var shouldPass = ['Viridis', 'Greens', good], shouldFail = ['red', 1, undefined, null, {}, [], bad, bad2, bad3, bad4]; From 0b3040bcfbdb0106b370050a1c15e3c6f44d895d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 16:03:20 -0400 Subject: [PATCH 03/14] make string val type more strict: - now only coerce numbers, dates and boolean to string --- src/lib/coerce.js | 18 ++++++++++-------- test/jasmine/tests/lib_test.js | 25 ++++++++++++------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index a63496f6fdc..875da7f340b 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -99,16 +99,18 @@ exports.valObjects = { // TODO 'values shouldn't be in there (edge case: 'dash' in Scatter) otherOpts: ['dflt', 'noBlank', 'strict', 'arrayOk', 'values'], coerceFunction: function(v, propOut, dflt, opts) { - if(opts.strict === true && typeof v !== 'string') { - propOut.set(dflt); - return; - } + if(typeof v !== 'string') { + var okToCoerce = ( + typeof v === 'number' || + v instanceof Date || + typeof v === 'boolean' + ); - var s = String(v); - if(v === undefined || (opts.noBlank === true && !s)) { - propOut.set(dflt); + if(opts.strict === true || !okToCoerce) propOut.set(dflt); + else propOut.set(String(v)); } - else propOut.set(s); + else if(opts.noBlank && !v) propOut.set(dflt); + else propOut.set(v); } }, color: { diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 305672fb7b2..77365853f9e 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -526,13 +526,13 @@ describe('Test lib.js:', function() { .toEqual('42'); expect(coerce({s: [1, 2, 3]}, {}, stringAttrs, 's')) - .toEqual('1,2,3'); + .toEqual(dflt); expect(coerce({s: true}, {}, stringAttrs, 's')) .toEqual('true'); expect(coerce({s: {1: 2}}, {}, stringAttrs, 's')) - .toEqual('[object Object]'); // useless, but that's what it does!! + .toEqual(dflt); }); }); @@ -861,27 +861,26 @@ describe('Test lib.js:', function() { }); it('should work for valType \'string\' where', function() { + var date = new Date(2016, 1, 1); - // This fails as the coerceFunction coerce all values - // (except when 'strict' is true) using String() - // - // Things like undefined, null, {}, [] should be considered valid! - // - // The question becomes how the change this while not - // scarifying perf?? - - assert(['3', '4', 'a'], [undefined, {}, [], null], { + assert(['3', '4', 'a', 3, 1.2113, '', date, false], [undefined, {}, [], null], { valType: 'string', dflt: 'a' }); - assert(['3', '4', 'a'], [undefined, {}, [], null, ''], { + assert(['3', '4', 'a', 3, 1.2113, date, true], ['', undefined, {}, [], null], { valType: 'string', dflt: 'a', noBlank: true }); - assert(['3', '4'], [undefined, 1, {}, [], null, ''], { + assert(['3', '4', ''], [undefined, 1, {}, [], null, date, true, false], { + valType: 'string', + dflt: 'a', + strict: true + }); + + assert(['3', '4'], [undefined, 1, {}, [], null, date, '', true, false], { valType: 'string', dflt: 'a', strict: true, From a5bc7fecbcc8cffcc16d167f3015ef7d4a7fbf17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 17:14:32 -0400 Subject: [PATCH 04/14] add a few test cases for coerce + 'info_array' --- test/jasmine/tests/lib_test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 77365853f9e..eca2e5015f0 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -626,7 +626,22 @@ describe('Test lib.js:', function() { .toEqual([0.5, 1]); }); + it('should coerce unexpected input as best as it can', function() { + expect(coerce({range: [12]}, {}, infoArrayAttrs, 'range')) + .toEqual([12]); + expect(coerce({range: [12]}, {}, infoArrayAttrs, 'range', [-1, 20])) + .toEqual([12, 20]); + + expect(coerce({domain: [0.5]}, {}, infoArrayAttrs, 'domain')) + .toEqual([0.5, 1]); + + expect(coerce({range: ['-10', 100, 12]}, {}, infoArrayAttrs, 'range')) + .toEqual([-10, 100]); + + expect(coerce({domain: [0, 0.5, 1]}, {}, infoArrayAttrs, 'domain')) + .toEqual([0, 0.5]); + }); }); describe('subplotid valtype', function() { From 6c1938c412923cb5562bb9c0a786314f124670b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 17:14:48 -0400 Subject: [PATCH 05/14] add validateFunction for info_array --- src/lib/coerce.js | 14 ++++++++++++++ test/jasmine/tests/lib_test.js | 6 ++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 875da7f340b..e68fdb1cce0 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -241,6 +241,20 @@ exports.valObjects = { } propOut.set(vOut); + }, + validateFunction: function(v, opts) { + if(!Array.isArray(v)) return false; + + var items = opts.items; + + // valid when one item is valid (which is subject to debate) + for(var i = 0; i < items.length; i++) { + var isItemValid = exports.validate(v[i], opts.items[i]); + + if(isItemValid) return true; + } + + return false; } } }; diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index eca2e5015f0..66d1c84e374 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -770,7 +770,7 @@ describe('Test lib.js:', function() { }); }); - fdescribe('validate', function() { + describe('validate', function() { function assert(shouldPass, shouldFail, valObject) { shouldPass.forEach(function(v) { @@ -974,11 +974,9 @@ describe('Test lib.js:', function() { }); it('should work for valType \'info_array\' where', function() { - var shouldPass = [[1, 2]], + var shouldPass = [[1, 2], [10], [null, 10], [1, 10, null]], shouldFail = [{}, [], ['aads', null], 'red', null, undefined, '']; - // This fails. All array including [] are considered valid - assert(shouldPass, shouldFail, { valType: 'info_array', items: [{ From d304864ea2901ba401e216071c56e38d90e540da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 17:24:07 -0400 Subject: [PATCH 06/14] make 'dflt' a required opts for 'subplotid' val types --- src/lib/coerce.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index e68fdb1cce0..87c79aa474f 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -164,11 +164,11 @@ exports.valObjects = { subplotid: { description: [ 'An id string of a subplot type (given by dflt), optionally', - 'followed by an integer >1. e.g. if dflt=\'geo\', we can have', + 'followed by an integer >1. e.g. if dflt=\'geo\', we can have', '\'geo\', \'geo2\', \'geo3\', ...' ].join(' '), - requiredOpts: [], - otherOpts: ['dflt'], + requiredOpts: ['dflt'], + otherOpts: [], coerceFunction: function(v, propOut, dflt) { var dlen = dflt.length; if(typeof v === 'string' && v.substr(0, dlen) === dflt && From 877077b8412cc361495a8270c7953fdc02a35714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 17:24:24 -0400 Subject: [PATCH 07/14] add validate function for 'subplotid' val types --- src/lib/coerce.js | 12 ++++++++++++ test/jasmine/tests/lib_test.js | 10 ++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 87c79aa474f..0349bd8c3d3 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -177,6 +177,18 @@ exports.valObjects = { return; } propOut.set(dflt); + }, + validateFunction: function(v, opts) { + var dflt = opts.dflt, + dlen = dflt.length; + + if(v === dflt) return true; + if(typeof v !== 'string') return false; + if(v.substr(0, dlen) === dflt && idRegex.test(v.substr(dlen))) { + return true; + } + + return false; } }, flaglist: { diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 66d1c84e374..6701419c0a9 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -938,14 +938,8 @@ describe('Test lib.js:', function() { }); it('should work for valType \'subplotid\' where', function() { - var shouldPass = ['sp', 'sp1', 'sp4'], - shouldFail = [{}, [], 'sp0', 'spee1', null, undefined]; - - // This fails because coerceFunction depends on dflt - // having a length. - // - // Solution: don't use dflt as base string -> - // add other options to valObject + var shouldPass = ['sp', 'sp4', 'sp10'], + shouldFail = [{}, [], 'sp1', 'sp0', 'spee1', null, undefined, true]; assert(shouldPass, shouldFail, { valType: 'subplotid', From 1958dc6d5a2dce11b2400360e6fe1762c6c30269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 27 Jun 2016 19:36:24 -0400 Subject: [PATCH 08/14] add Plotly.validate: - to determine whether user 'data' and 'layout' are valid in plotly.js --- src/core.js | 1 + src/plot_api/validate.js | 156 ++++++++++++++++++++++++++++ test/jasmine/tests/validate_test.js | 109 +++++++++++++++++++ 3 files changed, 266 insertions(+) create mode 100644 src/plot_api/validate.js create mode 100644 test/jasmine/tests/validate_test.js diff --git a/src/core.js b/src/core.js index bae6c1accad..13502eec6ba 100644 --- a/src/core.js +++ b/src/core.js @@ -33,6 +33,7 @@ exports.setPlotConfig = require('./plot_api/set_plot_config'); exports.register = Plotly.register; exports.toImage = require('./plot_api/to_image'); exports.downloadImage = require('./snapshot/download'); +exports.validate = require('./plot_api/validate'); // plot icons exports.Icons = require('../build/ploticon'); diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js new file mode 100644 index 00000000000..8e09fa448c5 --- /dev/null +++ b/src/plot_api/validate.js @@ -0,0 +1,156 @@ +/** +* Copyright 2012-2016, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + + +'use strict'; + + +var Lib = require('../lib'); +var Plots = require('../plots/plots'); +var PlotSchema = require('./plot_schema'); + +var isPlainObject = Lib.isPlainObject; + +// validation error codes +var code2msgFunc = { + invisible: function(path) { + return 'trace ' + path + ' got defaulted to be not visible'; + }, + schema: function(path) { + return 'key ' + path.join('.') + ' is not part of the schema'; + }, + unused: function(path, valIn) { + var prefix = isPlainObject(valIn) ? 'container' : 'key'; + + return prefix + ' ' + path.join('.') + ' did not get coerced'; + }, + value: function(path, valIn) { + return 'key ' + path.join('.') + ' is set to an invalid value (' + valIn + ')'; + } +}; + +module.exports = function valiate(data, layout) { + if(!Array.isArray(data)) { + throw new Error('data must be an array'); + } + + if(!isPlainObject(layout)) { + throw new Error('layout must be an object'); + } + + var gd = { + data: Lib.extendDeep([], data), + layout: Lib.extendDeep({}, layout) + }; + Plots.supplyDefaults(gd); + + var schema = PlotSchema.get(); + + var dataOut = gd._fullData, + len = data.length, + dataList = new Array(len); + + for(var i = 0; i < len; i++) { + var traceIn = data[i]; + var traceList = dataList[i] = []; + + if(!isPlainObject(traceIn)) { + throw new Error('each data trace must be an object'); + } + + var traceOut = dataOut[i], + traceType = traceOut.type, + traceSchema = schema.traces[traceType].attributes; + + // PlotSchema does something fancy with trace 'type', reset it here + // to make the trace schema compatible with Lib.validate. + traceSchema.type = { + valType: 'enumerated', + values: [traceType] + }; + + if(traceOut.visible === false && traceIn.visible !== false) { + traceList.push(format('invisible', i)); + } + + crawl(traceIn, traceOut, traceSchema, traceList); + } + + var layoutOut = gd._fullLayout, + layoutSchema = fillLayoutSchema(schema, dataOut), + layoutList = []; + + crawl(layout, layoutOut, layoutSchema, layoutList); + + return { + data: dataList, + layout: layoutList + }; +}; + +function crawl(objIn, objOut, schema, list, path) { + path = path || []; + + var keys = Object.keys(objIn); + + for(var i = 0; i < keys.length; i++) { + var k = keys[i]; + + var p = path.slice(); + p.push(k); + + var valIn = objIn[k], + valOut = objOut[k]; + + if(isPlainObject(valIn) && isPlainObject(valOut)) { + crawl(valIn, valOut, schema[k], list, p); + } + else if(!(k in schema)) { + list.push(format('schema', p)); + } + else if(schema[k].items && Array.isArray(valIn)) { + var itemName = k.substr(0, k.length - 1); + + for(var j = 0; j < valIn.length; j++) { + p[p.length - 1] = k + '[' + j + ']'; + + crawl(valIn[j], valOut[j], schema[k].items[itemName], list, p); + } + } + else if(!(k in objOut)) { + list.push(format('unused', p, valIn)); + } + else if(!Lib.validate(valIn, schema[k])) { + list.push(format('value', p, valIn)); + } + } + + return list; +} + +// the 'full' layout schema depends on the traces types presents +function fillLayoutSchema(schema, dataOut) { + for(var i = 0; i < dataOut.length; i++) { + var traceType = dataOut[i].type, + traceLayoutAttr = schema.traces[traceType].layoutAttributes; + + if(traceLayoutAttr) { + Lib.extendFlat(schema.layout.layoutAttributes, traceLayoutAttr); + } + } + + return schema.layout.layoutAttributes; +} + +function format(code, path, valIn) { + return { + code: code, + path: path, + msg: code2msgFunc[code](path, valIn) + }; +} diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js new file mode 100644 index 00000000000..772d8be049e --- /dev/null +++ b/test/jasmine/tests/validate_test.js @@ -0,0 +1,109 @@ +var Plotly = require('@lib/index'); + + +describe('Plotly.validate', function() { + + function assertErrorShape(out, dataSize, layoutSize) { + var actualDataSize = out.data.map(function(t) { + return t.length; + }); + + expect(actualDataSize).toEqual(dataSize); + expect(out.layout.length).toEqual(layoutSize); + } + + function assertErrorContent(obj, code, path) { + expect(obj.code).toEqual(code); + expect(obj.path).toEqual(path); + + // TODO test msg + } + + it('should report when trace is defaulted to not be visible', function() { + var out = Plotly.validate([{ + type: 'scatter' + // missing 'x' and 'y + }], {}); + + assertErrorShape(out, [1], 0); + assertErrorContent(out.data[0][0], 'invisible', 0, ''); + }); + + it('should report when trace contains keys not part of the schema', function() { + var out = Plotly.validate([{ + x: [1, 2, 3], + markerColor: 'blue' + }], {}); + + assertErrorShape(out, [1], 0); + assertErrorContent(out.data[0][0], 'schema', ['markerColor'], ''); + }); + + it('should report when trace contains keys that are not coerced', function() { + var out = Plotly.validate([{ + x: [1, 2, 3], + mode: 'lines', + marker: { color: 'blue' } + }, { + x: [1, 2, 3], + mode: 'markers', + marker: { + color: 'blue', + cmin: 10 + } + }], {}); + + assertErrorShape(out, [1, 1], 0); + expect(out.layout.length).toEqual(0); + assertErrorContent(out.data[0][0], 'unused', ['marker'], ''); + assertErrorContent(out.data[1][0], 'unused', ['marker', 'cmin'], ''); + + }); + + it('should report when trace contains keys set to invalid values', function() { + var out = Plotly.validate([{ + x: [1, 2, 3], + mode: 'lines', + line: { width: 'a big number' } + }, { + x: [1, 2, 3], + mode: 'markers', + marker: { color: 10 } + }], {}); + + assertErrorShape(out, [1, 1], 0); + assertErrorContent(out.data[0][0], 'value', ['line', 'width'], ''); + assertErrorContent(out.data[1][0], 'value', ['marker', 'color'], ''); + }); + + it('should work with isLinkedToArray attributes', function() { + var out = Plotly.validate([], { + annotations: [{ + text: 'some text' + }, { + arrowSymbol: 'cat' + }], + xaxis: { + type: 'date', + rangeselector: { + buttons: [{ + label: '1 month', + step: 'all', + count: 10 + }, { + title: '1 month' + }] + } + }, + shapes: [{ + opacity: 'none' + }] + }); + + assertErrorShape(out, [], 4); + assertErrorContent(out.layout[0], 'schema', ['annotations[1]', 'arrowSymbol'], ''); + assertErrorContent(out.layout[1], 'unused', ['xaxis', 'rangeselector', 'buttons[0]', 'count'], ''); + assertErrorContent(out.layout[2], 'schema', ['xaxis', 'rangeselector', 'buttons[1]', 'title'], ''); + assertErrorContent(out.layout[3], 'value', ['shapes[0]', 'opacity'], ''); + }); +}); From f620a2292eb48f5e9e15aa0c2783ebcc5d445e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 6 Jul 2016 14:18:15 -0400 Subject: [PATCH 09/14] validate: add support for is isSubplotObj attributes --- src/plot_api/validate.js | 46 ++++++++++++++++++++++++----- test/jasmine/tests/validate_test.js | 30 +++++++++++++++++-- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js index 8e09fa448c5..8ecc7a07eb7 100644 --- a/src/plot_api/validate.js +++ b/src/plot_api/validate.js @@ -107,25 +107,27 @@ function crawl(objIn, objOut, schema, list, path) { var valIn = objIn[k], valOut = objOut[k]; - if(isPlainObject(valIn) && isPlainObject(valOut)) { - crawl(valIn, valOut, schema[k], list, p); - } - else if(!(k in schema)) { + var nestedSchema = getNestedSchema(schema, k); + + if(!isInSchema(schema, k)) { list.push(format('schema', p)); } - else if(schema[k].items && Array.isArray(valIn)) { + else if(isPlainObject(valIn) && isPlainObject(valOut)) { + crawl(valIn, valOut, nestedSchema, list, p); + } + else if(nestedSchema.items && Array.isArray(valIn)) { var itemName = k.substr(0, k.length - 1); for(var j = 0; j < valIn.length; j++) { p[p.length - 1] = k + '[' + j + ']'; - crawl(valIn[j], valOut[j], schema[k].items[itemName], list, p); + crawl(valIn[j], valOut[j], nestedSchema.items[itemName], list, p); } } else if(!(k in objOut)) { list.push(format('unused', p, valIn)); } - else if(!Lib.validate(valIn, schema[k])) { + else if(!Lib.validate(valIn, nestedSchema)) { list.push(format('value', p, valIn)); } } @@ -154,3 +156,33 @@ function format(code, path, valIn) { msg: code2msgFunc[code](path, valIn) }; } + +function isInSchema(schema, key) { + var parts = splitKey(key), + keyMinusId = parts.keyMinusId, + id = parts.id; + + if((keyMinusId in schema) && schema[keyMinusId]._isSubplotObj && id) { + return true; + } + + return (key in schema); +} + +function getNestedSchema(schema, key) { + var parts = splitKey(key); + + return schema[parts.keyMinusId]; +} + +function splitKey(key) { + var idRegex = /([2-9]|[1-9][0-9]+)$/; + + var keyMinusId = key.split(idRegex)[0], + id = key.substr(keyMinusId.length, key.length); + + return { + keyMinusId: keyMinusId, + id: id + }; +} diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index 772d8be049e..ca994ec603d 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -95,15 +95,41 @@ describe('Plotly.validate', function() { }] } }, + xaxis2: { + type: 'date', + rangeselector: { + buttons: [{ + title: '1 month' + }] + } + }, shapes: [{ opacity: 'none' }] }); - assertErrorShape(out, [], 4); + assertErrorShape(out, [], 5); assertErrorContent(out.layout[0], 'schema', ['annotations[1]', 'arrowSymbol'], ''); assertErrorContent(out.layout[1], 'unused', ['xaxis', 'rangeselector', 'buttons[0]', 'count'], ''); assertErrorContent(out.layout[2], 'schema', ['xaxis', 'rangeselector', 'buttons[1]', 'title'], ''); - assertErrorContent(out.layout[3], 'value', ['shapes[0]', 'opacity'], ''); + assertErrorContent(out.layout[3], 'schema', ['xaxis2', 'rangeselector', 'buttons[0]', 'title'], ''); + assertErrorContent(out.layout[4], 'value', ['shapes[0]', 'opacity'], ''); + }); + + it('should work with isSubplotObj attributes', function() { + var out = Plotly.validate([], { + xaxis2: { + range: 30 + }, + scene10: { + bgcolor: 'red' + }, + geo0: {}, + }); + + assertErrorShape(out, [], 4); + assertErrorContent(out.layout[0], 'value', ['xaxis2', 'range'], ''); + assertErrorContent(out.layout[1], 'unused', ['scene10'], ''); + assertErrorContent(out.layout[2], 'schema', ['geo0'], ''); }); }); From a2c52fefcda6187af3dd4a053cc475ff7677bfcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 6 Jul 2016 14:18:42 -0400 Subject: [PATCH 10/14] validate: add 'container' error code --- src/plot_api/validate.js | 6 ++++++ test/jasmine/tests/validate_test.js | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js index 8ecc7a07eb7..42677eb81e6 100644 --- a/src/plot_api/validate.js +++ b/src/plot_api/validate.js @@ -24,6 +24,9 @@ var code2msgFunc = { schema: function(path) { return 'key ' + path.join('.') + ' is not part of the schema'; }, + container: function(path) { + return 'key ' + path.join('.') + ' is supposed to be linked to a container'; + }, unused: function(path, valIn) { var prefix = isPlainObject(valIn) ? 'container' : 'key'; @@ -115,6 +118,9 @@ function crawl(objIn, objOut, schema, list, path) { else if(isPlainObject(valIn) && isPlainObject(valOut)) { crawl(valIn, valOut, nestedSchema, list, p); } + else if(!isPlainObject(valIn) && isPlainObject(valOut)) { + list.push(format('container', p, valIn)); + } else if(nestedSchema.items && Array.isArray(valIn)) { var itemName = k.substr(0, k.length - 1); diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index ca994ec603d..ab64acb910f 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -125,11 +125,13 @@ describe('Plotly.validate', function() { bgcolor: 'red' }, geo0: {}, + yaxis5: 'sup' }); assertErrorShape(out, [], 4); assertErrorContent(out.layout[0], 'value', ['xaxis2', 'range'], ''); assertErrorContent(out.layout[1], 'unused', ['scene10'], ''); assertErrorContent(out.layout[2], 'schema', ['geo0'], ''); + assertErrorContent(out.layout[3], 'container', ['yaxis5'], ''); }); }); From ad8db59d9a2a508ddd7ed19d5ba759f58ca7b8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 12 Jul 2016 14:16:19 -0400 Subject: [PATCH 11/14] on 2nd thought, only coerce numbers to strings --- src/lib/coerce.js | 6 +----- test/jasmine/tests/lib_test.js | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 0349bd8c3d3..681dea1474b 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -100,11 +100,7 @@ exports.valObjects = { otherOpts: ['dflt', 'noBlank', 'strict', 'arrayOk', 'values'], coerceFunction: function(v, propOut, dflt, opts) { if(typeof v !== 'string') { - var okToCoerce = ( - typeof v === 'number' || - v instanceof Date || - typeof v === 'boolean' - ); + var okToCoerce = (typeof v === 'number'); if(opts.strict === true || !okToCoerce) propOut.set(dflt); else propOut.set(String(v)); diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 6701419c0a9..e5105211622 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -529,7 +529,7 @@ describe('Test lib.js:', function() { .toEqual(dflt); expect(coerce({s: true}, {}, stringAttrs, 's')) - .toEqual('true'); + .toEqual(dflt); expect(coerce({s: {1: 2}}, {}, stringAttrs, 's')) .toEqual(dflt); @@ -878,12 +878,12 @@ describe('Test lib.js:', function() { it('should work for valType \'string\' where', function() { var date = new Date(2016, 1, 1); - assert(['3', '4', 'a', 3, 1.2113, '', date, false], [undefined, {}, [], null], { + assert(['3', '4', 'a', 3, 1.2113, ''], [undefined, {}, [], null, date, false], { valType: 'string', dflt: 'a' }); - assert(['3', '4', 'a', 3, 1.2113, date, true], ['', undefined, {}, [], null], { + assert(['3', '4', 'a', 3, 1.2113], ['', undefined, {}, [], null, date, true], { valType: 'string', dflt: 'a', noBlank: true From 09ed024068ec7e3db3351a8c44c881d69bef9745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 12 Jul 2016 14:20:50 -0400 Subject: [PATCH 12/14] clean up is-valid-colorscale logic --- src/components/colorscale/is_valid_scale_array.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/components/colorscale/is_valid_scale_array.js b/src/components/colorscale/is_valid_scale_array.js index ebf55b02b3d..95b0a625ef7 100644 --- a/src/components/colorscale/is_valid_scale_array.js +++ b/src/components/colorscale/is_valid_scale_array.js @@ -13,10 +13,9 @@ var tinycolor = require('tinycolor2'); module.exports = function isValidScaleArray(scl) { - var isValid = true, - highestVal = 0; + var highestVal = 0; - if(!Array.isArray(scl) || scl.length === 0) return false; + if(!Array.isArray(scl) || scl.length < 2) return false; if(!scl[0] || !scl[scl.length - 1]) return false; @@ -26,12 +25,11 @@ module.exports = function isValidScaleArray(scl) { var si = scl[i]; if(si.length !== 2 || +si[0] < highestVal || !tinycolor(si[1]).isValid()) { - isValid = false; - break; + return false; } highestVal = +si[0]; } - return isValid; + return true; }; From a2f2160893e7be5c33b9ff3ea504fb7f171c7a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 12 Jul 2016 14:33:15 -0400 Subject: [PATCH 13/14] make info_array validate func more strict: - only value that have same correct length and all item valid are consisdered valid. --- src/lib/coerce.js | 8 +++++--- test/jasmine/tests/lib_test.js | 8 ++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 681dea1474b..7f7e65a5900 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -255,14 +255,16 @@ exports.valObjects = { var items = opts.items; - // valid when one item is valid (which is subject to debate) + if(v.length !== items.length) return false; + + // valid when all items are valid for(var i = 0; i < items.length; i++) { var isItemValid = exports.validate(v[i], opts.items[i]); - if(isItemValid) return true; + if(!isItemValid) return false; } - return false; + return true; } } }; diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index e5105211622..9941494bfcc 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -968,8 +968,12 @@ describe('Test lib.js:', function() { }); it('should work for valType \'info_array\' where', function() { - var shouldPass = [[1, 2], [10], [null, 10], [1, 10, null]], - shouldFail = [{}, [], ['aads', null], 'red', null, undefined, '']; + var shouldPass = [[1, 2], [-20, '20']], + shouldFail = [ + {}, [], [10], [null, 10], ['aads', null], + 'red', null, undefined, '', + [1, 10, null] + ]; assert(shouldPass, shouldFail, { valType: 'info_array', From 54a6ed0ea20abeeb0be2e75a3927457d4d341d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 13 Jul 2016 15:00:03 -0400 Subject: [PATCH 14/14] validate: flatten Plotly.validate output - make Plotly.validate return a 1-level deep array of error objects - add 'container' and 'trace' key in each error object to locate the error in data trace or layout. - add 'astr' to error object (for easy plug into restyle and relayout) --- src/plot_api/validate.js | 229 +++++++++++++++++++++------- test/jasmine/tests/validate_test.js | 182 +++++++++++++++++----- 2 files changed, 320 insertions(+), 91 deletions(-) diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js index 42677eb81e6..58dc5829934 100644 --- a/src/plot_api/validate.js +++ b/src/plot_api/validate.js @@ -15,55 +15,75 @@ var Plots = require('../plots/plots'); var PlotSchema = require('./plot_schema'); var isPlainObject = Lib.isPlainObject; +var isArray = Array.isArray; -// validation error codes -var code2msgFunc = { - invisible: function(path) { - return 'trace ' + path + ' got defaulted to be not visible'; - }, - schema: function(path) { - return 'key ' + path.join('.') + ' is not part of the schema'; - }, - container: function(path) { - return 'key ' + path.join('.') + ' is supposed to be linked to a container'; - }, - unused: function(path, valIn) { - var prefix = isPlainObject(valIn) ? 'container' : 'key'; - - return prefix + ' ' + path.join('.') + ' did not get coerced'; - }, - value: function(path, valIn) { - return 'key ' + path.join('.') + ' is set to an invalid value (' + valIn + ')'; - } -}; +/** + * Validate a data array and layout object. + * + * @param {array} data + * @param {object} layout + * + * @return {array} array of error objects each containing: + * - {string} code + * error code ('object', 'array', 'schema', 'unused', 'invisible' or 'value') + * - {string} container + * container where the error occurs ('data' or 'layout') + * - {number} trace + * trace index of the 'data' container where the error occurs + * - {array} path + * nested path to the key that causes the error + * - {string} astr + * attribute string variant of 'path' compatible with Plotly.restyle and + * Plotly.relayout. + * - {string} msg + * error message (shown in console in logger config argument is enable) + */ module.exports = function valiate(data, layout) { - if(!Array.isArray(data)) { - throw new Error('data must be an array'); + var schema = PlotSchema.get(), + errorList = [], + gd = {}; + + var dataIn, layoutIn; + + if(isArray(data)) { + gd.data = Lib.extendDeep([], data); + dataIn = data; + } + else { + gd.data = []; + dataIn = []; + errorList.push(format('array', 'data')); } - if(!isPlainObject(layout)) { - throw new Error('layout must be an object'); + if(isPlainObject(layout)) { + gd.layout = Lib.extendDeep({}, layout); + layoutIn = layout; + } + else { + gd.layout = {}; + layoutIn = {}; + if(arguments.length > 1) { + errorList.push(format('object', 'layout')); + } } - var gd = { - data: Lib.extendDeep([], data), - layout: Lib.extendDeep({}, layout) - }; - Plots.supplyDefaults(gd); + // N.B. dataIn and layoutIn are in general not the same as + // gd.data and gd.layout after supplyDefaults as some attributes + // in gd.data and gd.layout (still) get mutated during this step. - var schema = PlotSchema.get(); + Plots.supplyDefaults(gd); var dataOut = gd._fullData, - len = data.length, - dataList = new Array(len); + len = dataIn.length; for(var i = 0; i < len; i++) { - var traceIn = data[i]; - var traceList = dataList[i] = []; + var traceIn = dataIn[i], + base = ['data', i]; if(!isPlainObject(traceIn)) { - throw new Error('each data trace must be an object'); + errorList.push(format('object', base)); + continue; } var traceOut = dataOut[i], @@ -78,25 +98,22 @@ module.exports = function valiate(data, layout) { }; if(traceOut.visible === false && traceIn.visible !== false) { - traceList.push(format('invisible', i)); + errorList.push(format('invisible', base)); } - crawl(traceIn, traceOut, traceSchema, traceList); + crawl(traceIn, traceOut, traceSchema, errorList, base); } var layoutOut = gd._fullLayout, - layoutSchema = fillLayoutSchema(schema, dataOut), - layoutList = []; + layoutSchema = fillLayoutSchema(schema, dataOut); - crawl(layout, layoutOut, layoutSchema, layoutList); + crawl(layoutIn, layoutOut, layoutSchema, errorList, 'layout'); - return { - data: dataList, - layout: layoutList - }; + // return undefined if no validation errors were found + return (errorList.length === 0) ? void(0) : errorList; }; -function crawl(objIn, objOut, schema, list, path) { +function crawl(objIn, objOut, schema, list, base, path) { path = path || []; var keys = Object.keys(objIn); @@ -113,28 +130,34 @@ function crawl(objIn, objOut, schema, list, path) { var nestedSchema = getNestedSchema(schema, k); if(!isInSchema(schema, k)) { - list.push(format('schema', p)); + list.push(format('schema', base, p)); } else if(isPlainObject(valIn) && isPlainObject(valOut)) { - crawl(valIn, valOut, nestedSchema, list, p); + crawl(valIn, valOut, nestedSchema, list, base, p); } - else if(!isPlainObject(valIn) && isPlainObject(valOut)) { - list.push(format('container', p, valIn)); - } - else if(nestedSchema.items && Array.isArray(valIn)) { + else if(nestedSchema.items && isArray(valIn)) { var itemName = k.substr(0, k.length - 1); for(var j = 0; j < valIn.length; j++) { - p[p.length - 1] = k + '[' + j + ']'; + var _nestedSchema = nestedSchema.items[itemName], + _p = p.slice(); - crawl(valIn[j], valOut[j], nestedSchema.items[itemName], list, p); + _p.push(j); + + crawl(valIn[j], valOut[j], _nestedSchema, list, base, _p); } } + else if(!isPlainObject(valIn) && isPlainObject(valOut)) { + list.push(format('object', base, p, valIn)); + } + else if(!isArray(valIn) && isArray(valOut) && nestedSchema.valType !== 'info_array') { + list.push(format('array', base, p, valIn)); + } else if(!(k in objOut)) { - list.push(format('unused', p, valIn)); + list.push(format('unused', base, p, valIn)); } else if(!Lib.validate(valIn, nestedSchema)) { - list.push(format('value', p, valIn)); + list.push(format('value', base, p, valIn)); } } @@ -155,11 +178,82 @@ function fillLayoutSchema(schema, dataOut) { return schema.layout.layoutAttributes; } -function format(code, path, valIn) { +// validation error codes +var code2msgFunc = { + object: function(base, astr) { + var prefix; + + if(base === 'layout' && astr === '') prefix = 'The layout argument'; + else if(base[0] === 'data') { + prefix = 'Trace ' + base[1] + ' in the data argument'; + } + else prefix = inBase(base) + 'key ' + astr; + + return prefix + ' must be linked to an object container'; + }, + array: function(base, astr) { + var prefix; + + if(base === 'data') prefix = 'The data argument'; + else prefix = inBase(base) + 'key ' + astr; + + return prefix + ' must be linked to an array container'; + }, + schema: function(base, astr) { + return inBase(base) + 'key ' + astr + ' is not part of the schema'; + }, + unused: function(base, astr, valIn) { + var target = isPlainObject(valIn) ? 'container' : 'key'; + + return inBase(base) + target + ' ' + astr + ' did not get coerced'; + }, + invisible: function(base) { + return 'Trace ' + base[1] + ' got defaulted to be not visible'; + }, + value: function(base, astr, valIn) { + return [ + inBase(base) + 'key ' + astr, + 'is set to an invalid value (' + valIn + ')' + ].join(' '); + } +}; + +function inBase(base) { + if(isArray(base)) return 'In data trace ' + base[1] + ', '; + + return 'In ' + base + ', '; +} + +function format(code, base, path, valIn) { + path = path || ''; + + var container, trace; + + // container is either 'data' or 'layout + // trace is the trace index if 'data', null otherwise + + if(isArray(base)) { + container = base[0]; + trace = base[1]; + } + else { + container = base; + trace = null; + } + + var astr = convertPathToAttributeString(path), + msg = code2msgFunc[code](base, astr, valIn); + + // log to console if logger config option is enabled + Lib.log(msg); + return { code: code, + container: container, + trace: trace, path: path, - msg: code2msgFunc[code](path, valIn) + astr: astr, + msg: msg }; } @@ -192,3 +286,24 @@ function splitKey(key) { id: id }; } + +function convertPathToAttributeString(path) { + if(!isArray(path)) return String(path); + + var astr = ''; + + for(var i = 0; i < path.length; i++) { + var p = path[i]; + + if(typeof p === 'number') { + astr = astr.substr(0, astr.length - 1) + '[' + p + ']'; + } + else { + astr += p; + } + + if(i < path.length - 1) astr += '.'; + } + + return astr; +} diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index ab64acb910f..4273b84a355 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -3,21 +3,61 @@ var Plotly = require('@lib/index'); describe('Plotly.validate', function() { - function assertErrorShape(out, dataSize, layoutSize) { - var actualDataSize = out.data.map(function(t) { - return t.length; + function assertErrorContent(obj, code, cont, trace, path, astr, msg) { + expect(obj.code).toEqual(code); + expect(obj.container).toEqual(cont); + expect(obj.trace).toEqual(trace); + expect(obj.path).toEqual(path); + expect(obj.astr).toEqual(astr); + expect(obj.msg).toEqual(msg); + } + + it('should return undefined when no errors are found', function() { + var out = Plotly.validate([{ + type: 'scatter', + x: [1, 2, 3] + }], { + title: 'my simple graph' }); - expect(actualDataSize).toEqual(dataSize); - expect(out.layout.length).toEqual(layoutSize); - } + expect(out).toBeUndefined(); + }); - function assertErrorContent(obj, code, path) { - expect(obj.code).toEqual(code); - expect(obj.path).toEqual(path); + it('should report when data is not an array', function() { + var out = Plotly.validate({ + type: 'scatter', + x: [1, 2, 3] + }); - // TODO test msg - } + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'array', 'data', null, '', '', + 'The data argument must be linked to an array container' + ); + }); + + it('should report when a data trace is not an object', function() { + var out = Plotly.validate([{ + type: 'scatter', + x: [1, 2, 3] + }, [1, 2, 3]]); + + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'object', 'data', 1, '', '', + 'Trace 1 in the data argument must be linked to an object container' + ); + }); + + it('should report when layout is not an object', function() { + var out = Plotly.validate([], [1, 2, 3]); + + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'object', 'layout', null, '', '', + 'The layout argument must be linked to an object container' + ); + }); it('should report when trace is defaulted to not be visible', function() { var out = Plotly.validate([{ @@ -25,8 +65,11 @@ describe('Plotly.validate', function() { // missing 'x' and 'y }], {}); - assertErrorShape(out, [1], 0); - assertErrorContent(out.data[0][0], 'invisible', 0, ''); + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'invisible', 'data', 0, '', '', + 'Trace 0 got defaulted to be not visible' + ); }); it('should report when trace contains keys not part of the schema', function() { @@ -35,8 +78,11 @@ describe('Plotly.validate', function() { markerColor: 'blue' }], {}); - assertErrorShape(out, [1], 0); - assertErrorContent(out.data[0][0], 'schema', ['markerColor'], ''); + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'schema', 'data', 0, ['markerColor'], 'markerColor', + 'In data trace 0, key markerColor is not part of the schema' + ); }); it('should report when trace contains keys that are not coerced', function() { @@ -53,11 +99,15 @@ describe('Plotly.validate', function() { } }], {}); - assertErrorShape(out, [1, 1], 0); - expect(out.layout.length).toEqual(0); - assertErrorContent(out.data[0][0], 'unused', ['marker'], ''); - assertErrorContent(out.data[1][0], 'unused', ['marker', 'cmin'], ''); - + expect(out.length).toEqual(2); + assertErrorContent( + out[0], 'unused', 'data', 0, ['marker'], 'marker', + 'In data trace 0, container marker did not get coerced' + ); + assertErrorContent( + out[1], 'unused', 'data', 1, ['marker', 'cmin'], 'marker.cmin', + 'In data trace 1, key marker.cmin did not get coerced' + ); }); it('should report when trace contains keys set to invalid values', function() { @@ -71,9 +121,15 @@ describe('Plotly.validate', function() { marker: { color: 10 } }], {}); - assertErrorShape(out, [1, 1], 0); - assertErrorContent(out.data[0][0], 'value', ['line', 'width'], ''); - assertErrorContent(out.data[1][0], 'value', ['marker', 'color'], ''); + expect(out.length).toEqual(2); + assertErrorContent( + out[0], 'value', 'data', 0, ['line', 'width'], 'line.width', + 'In data trace 0, key line.width is set to an invalid value (a big number)' + ); + assertErrorContent( + out[1], 'value', 'data', 1, ['marker', 'color'], 'marker.color', + 'In data trace 1, key marker.color is set to an invalid value (10)' + ); }); it('should work with isLinkedToArray attributes', function() { @@ -82,6 +138,8 @@ describe('Plotly.validate', function() { text: 'some text' }, { arrowSymbol: 'cat' + }, { + font: { color: 'wont-work' } }], xaxis: { type: 'date', @@ -103,17 +161,57 @@ describe('Plotly.validate', function() { }] } }, + xaxis3: { + type: 'date', + rangeselector: { + buttons: 'wont-work' + } + }, shapes: [{ opacity: 'none' }] }); - assertErrorShape(out, [], 5); - assertErrorContent(out.layout[0], 'schema', ['annotations[1]', 'arrowSymbol'], ''); - assertErrorContent(out.layout[1], 'unused', ['xaxis', 'rangeselector', 'buttons[0]', 'count'], ''); - assertErrorContent(out.layout[2], 'schema', ['xaxis', 'rangeselector', 'buttons[1]', 'title'], ''); - assertErrorContent(out.layout[3], 'schema', ['xaxis2', 'rangeselector', 'buttons[0]', 'title'], ''); - assertErrorContent(out.layout[4], 'value', ['shapes[0]', 'opacity'], ''); + expect(out.length).toEqual(7); + assertErrorContent( + out[0], 'schema', 'layout', null, + ['annotations', 1, 'arrowSymbol'], 'annotations[1].arrowSymbol', + 'In layout, key annotations[1].arrowSymbol is not part of the schema' + ); + assertErrorContent( + out[1], 'value', 'layout', null, + ['annotations', 2, 'font', 'color'], 'annotations[2].font.color', + 'In layout, key annotations[2].font.color is set to an invalid value (wont-work)' + ); + assertErrorContent( + out[2], 'unused', 'layout', null, + ['xaxis', 'rangeselector', 'buttons', 0, 'count'], + 'xaxis.rangeselector.buttons[0].count', + 'In layout, key xaxis.rangeselector.buttons[0].count did not get coerced' + ); + assertErrorContent( + out[3], 'schema', 'layout', null, + ['xaxis', 'rangeselector', 'buttons', 1, 'title'], + 'xaxis.rangeselector.buttons[1].title', + 'In layout, key xaxis.rangeselector.buttons[1].title is not part of the schema' + ); + assertErrorContent( + out[4], 'schema', 'layout', null, + ['xaxis2', 'rangeselector', 'buttons', 0, 'title'], + 'xaxis2.rangeselector.buttons[0].title', + 'In layout, key xaxis2.rangeselector.buttons[0].title is not part of the schema' + ); + assertErrorContent( + out[5], 'array', 'layout', null, + ['xaxis3', 'rangeselector', 'buttons'], + 'xaxis3.rangeselector.buttons', + 'In layout, key xaxis3.rangeselector.buttons must be linked to an array container' + ); + assertErrorContent( + out[6], 'value', 'layout', null, + ['shapes', 0, 'opacity'], 'shapes[0].opacity', + 'In layout, key shapes[0].opacity is set to an invalid value (none)' + ); }); it('should work with isSubplotObj attributes', function() { @@ -128,10 +226,26 @@ describe('Plotly.validate', function() { yaxis5: 'sup' }); - assertErrorShape(out, [], 4); - assertErrorContent(out.layout[0], 'value', ['xaxis2', 'range'], ''); - assertErrorContent(out.layout[1], 'unused', ['scene10'], ''); - assertErrorContent(out.layout[2], 'schema', ['geo0'], ''); - assertErrorContent(out.layout[3], 'container', ['yaxis5'], ''); + expect(out.length).toEqual(4); + assertErrorContent( + out[0], 'value', 'layout', null, + ['xaxis2', 'range'], 'xaxis2.range', + 'In layout, key xaxis2.range is set to an invalid value (30)' + ); + assertErrorContent( + out[1], 'unused', 'layout', null, + ['scene10'], 'scene10', + 'In layout, container scene10 did not get coerced' + ); + assertErrorContent( + out[2], 'schema', 'layout', null, + ['geo0'], 'geo0', + 'In layout, key geo0 is not part of the schema' + ); + assertErrorContent( + out[3], 'object', 'layout', null, + ['yaxis5'], 'yaxis5', + 'In layout, key yaxis5 must be linked to an object container' + ); }); });