From 081bec493885036950eaf7cfdc3664e4e41115ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 3 Aug 2016 19:02:04 -0400 Subject: [PATCH 1/7] coerce: add 'freeLength' val object option for 'info_array' - by default, input info array must have same length as the declared version, now with 'freeLength' this doesn't have to be. - set freeLength to true for the updatem menus 'args' attribute --- src/components/updatemenus/attributes.js | 1 + src/lib/coerce.js | 9 +++++---- test/jasmine/tests/lib_test.js | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/components/updatemenus/attributes.js b/src/components/updatemenus/attributes.js index a27157200b1..a8703528302 100644 --- a/src/components/updatemenus/attributes.js +++ b/src/components/updatemenus/attributes.js @@ -27,6 +27,7 @@ var buttonsAttrs = { args: { valType: 'info_array', role: 'info', + freeLength: true, items: [ { valType: 'any' }, { valType: 'any' }, diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 7f7e65a5900..195ab99af76 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -233,7 +233,7 @@ exports.valObjects = { 'An {array} of plot information.' ].join(' '), requiredOpts: ['items'], - otherOpts: ['dflt'], + otherOpts: ['dflt', 'noFixLength'], coerceFunction: function(v, propOut, dflt, opts) { if(!Array.isArray(v)) { propOut.set(dflt); @@ -255,10 +255,11 @@ exports.valObjects = { var items = opts.items; - if(v.length !== items.length) return false; + // when free length is off, input and declared lengths must match + if(!opts.freeLength && v.length !== items.length) return false; - // valid when all items are valid - for(var i = 0; i < items.length; i++) { + // valid when all input items are valid + for(var i = 0; i < v.length; i++) { var isItemValid = exports.validate(v[i], opts.items[i]); if(!isItemValid) return false; diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index b25821f9fd0..76d0dff5b1e 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -985,6 +985,29 @@ describe('Test lib.js:', function() { }] }); }); + + it('should work for valType \'info_array\' (freeLength case)', function() { + var shouldPass = [ + ['marker.color', 'red'], + [{ 'marker.color': 'red' }, [1, 2]] + ]; + var shouldFail = [ + ['marker.color', 'red', 'red'], + [{ 'marker.color': 'red' }, [1, 2], 'blue'] + ]; + + assert(shouldPass, shouldFail, { + valType: 'info_array', + freeLength: true, + items: [{ + valType: 'any' + }, { + valType: 'any' + }, { + valType: 'number' + }] + }); + }); }); describe('setCursor', function() { From cfb39867da7e308ee95e0b29ee1ee18d0b0ecc3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 3 Aug 2016 19:04:47 -0400 Subject: [PATCH 2/7] validate: don't mix up info array and array containers --- src/plot_api/validate.js | 7 ++++--- test/jasmine/tests/validate_test.js | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js index fc9584fe353..3c5709d6171 100644 --- a/src/plot_api/validate.js +++ b/src/plot_api/validate.js @@ -163,7 +163,8 @@ function crawl(objIn, objOut, schema, list, base, path) { var valIn = objIn[k], valOut = objOut[k]; - var nestedSchema = getNestedSchema(schema, k); + var nestedSchema = getNestedSchema(schema, k), + isInfoArray = (nestedSchema || {}).valType === 'info_array'; if(!isInSchema(schema, k)) { list.push(format('schema', base, p)); @@ -171,7 +172,7 @@ function crawl(objIn, objOut, schema, list, base, path) { else if(isPlainObject(valIn) && isPlainObject(valOut)) { crawl(valIn, valOut, nestedSchema, list, base, p); } - else if(nestedSchema.items && isArray(valIn)) { + else if(nestedSchema.items && !isInfoArray && isArray(valIn)) { var itemName = k.substr(0, k.length - 1); for(var j = 0; j < valIn.length; j++) { @@ -186,7 +187,7 @@ function crawl(objIn, objOut, schema, list, base, path) { else if(!isPlainObject(valIn) && isPlainObject(valOut)) { list.push(format('object', base, p, valIn)); } - else if(!isArray(valIn) && isArray(valOut) && nestedSchema.valType !== 'info_array') { + else if(!isArray(valIn) && isArray(valOut) && !isInfoArray) { list.push(format('array', base, p, valIn)); } else if(!(k in objOut)) { diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index 5ce33eb738f..b177c71fb87 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -138,6 +138,21 @@ describe('Plotly.validate', function() { ); }); + it('should work with info arrays', function() { + var out = Plotly.validate([{ + y: [1, 2, 2] + }], { + xaxis: { range: [0, 10] }, + yaxis: { range: 'not-gonna-work' }, + }); + + expect(out.length).toEqual(1); + assertErrorContent( + out[0], 'value', 'layout', null, ['yaxis', 'range'], 'yaxis.range', + 'In layout, key yaxis.range is set to an invalid value (not-gonna-work)' + ); + }); + it('should work with isLinkedToArray attributes', function() { var out = Plotly.validate([], { annotations: [{ From d634cc478cc5e5625870459ac5162fdc4919c6da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 3 Aug 2016 19:10:42 -0400 Subject: [PATCH 3/7] set '_index' in isLinkedArray containers - so that we can keep track of 'skipped-over' containers e.g. 'buttons' items are skipped over when their content is invalid --- src/components/rangeselector/defaults.js | 1 + src/components/updatemenus/defaults.js | 1 + 2 files changed, 2 insertions(+) diff --git a/src/components/rangeselector/defaults.js b/src/components/rangeselector/defaults.js index c616d41d261..fb486fdaa4c 100644 --- a/src/components/rangeselector/defaults.js +++ b/src/components/rangeselector/defaults.js @@ -69,6 +69,7 @@ function buttonsDefaults(containerIn, containerOut) { coerce('label'); + buttonOut._index = i; buttonsOut.push(buttonOut); } diff --git a/src/components/updatemenus/defaults.js b/src/components/updatemenus/defaults.js index 52c79efa5e1..5574d5f5040 100644 --- a/src/components/updatemenus/defaults.js +++ b/src/components/updatemenus/defaults.js @@ -86,6 +86,7 @@ function buttonsDefaults(menuIn, menuOut) { coerce('args'); coerce('label'); + buttonOut._index = i; buttonsOut.push(buttonOut); } From e7391d5c7874d2feffc06f7dbde0c0b6e792a9a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 3 Aug 2016 19:12:37 -0400 Subject: [PATCH 4/7] validate: improve isLinkedToArray validation - make sure that invalid and skipped-over containers lead to validation errors. - make sure to track the input index to match schema/value with the correct input container --- src/plot_api/validate.js | 30 ++++++++++++++--- test/jasmine/tests/validate_test.js | 52 ++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/plot_api/validate.js b/src/plot_api/validate.js index 3c5709d6171..d034cca38d1 100644 --- a/src/plot_api/validate.js +++ b/src/plot_api/validate.js @@ -173,15 +173,37 @@ function crawl(objIn, objOut, schema, list, base, path) { crawl(valIn, valOut, nestedSchema, list, base, p); } else if(nestedSchema.items && !isInfoArray && isArray(valIn)) { - var itemName = k.substr(0, k.length - 1); + var itemName = k.substr(0, k.length - 1), + indexList = []; - for(var j = 0; j < valIn.length; j++) { + var j, _p; + + // loop over valOut items while keeping track of their + // corresponding input container index (given by _index) + for(j = 0; j < valOut.length; j++) { var _nestedSchema = nestedSchema.items[itemName], - _p = p.slice(); + _index = valOut[j]._index || j; + + _p = p.slice(); + _p.push(_index); + if(isPlainObject(valIn[_index]) && isPlainObject(valOut[j])) { + indexList.push(_index); + crawl(valIn[_index], valOut[j], _nestedSchema, list, base, _p); + } + } + + // loop over valIn to determine where it went wrong for some items + for(j = 0; j < valIn.length; j++) { + _p = p.slice(); _p.push(j); - crawl(valIn[j], valOut[j], _nestedSchema, list, base, _p); + if(!isPlainObject(valIn[j])) { + list.push(format('object', base, _p, valIn[j])); + } + else if(indexList.indexOf(j) === -1) { + list.push(format('unused', base, _p)); + } } } else if(!isPlainObject(valIn) && isPlainObject(valOut)) { diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index b177c71fb87..8a55323a6bd 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -169,7 +169,7 @@ describe('Plotly.validate', function() { label: '1 month', step: 'all', count: 10 - }, { + }, 'wont-work', { title: '1 month' }] } @@ -190,10 +190,25 @@ describe('Plotly.validate', function() { }, shapes: [{ opacity: 'none' + }], + updatemenus: [{ + buttons: [{ + method: 'restyle', + args: ['marker.color', 'red'] + }] + }, 'wont-work', { + buttons: [{ + method: 'restyle', + args: null + }, { + method: 'relayout', + args: ['marker.color', 'red'], + title: 'not-gonna-work' + }, 'wont-work'] }] }); - expect(out.length).toEqual(7); + expect(out.length).toEqual(12); assertErrorContent( out[0], 'schema', 'layout', null, ['annotations', 1, 'arrowSymbol'], 'annotations[1].arrowSymbol', @@ -212,27 +227,48 @@ describe('Plotly.validate', function() { ); 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' + ['xaxis', 'rangeselector', 'buttons', 2, 'title'], + 'xaxis.rangeselector.buttons[2].title', + 'In layout, key xaxis.rangeselector.buttons[2].title is not part of the schema' ); assertErrorContent( - out[4], 'schema', 'layout', null, + out[4], 'object', 'layout', null, + ['xaxis', 'rangeselector', 'buttons', 1], + 'xaxis.rangeselector.buttons[1]', + 'In layout, key xaxis.rangeselector.buttons[1] must be linked to an object container' + ); + assertErrorContent( + out[5], '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, + out[6], '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, + out[7], 'value', 'layout', null, ['shapes', 0, 'opacity'], 'shapes[0].opacity', 'In layout, key shapes[0].opacity is set to an invalid value (none)' ); + assertErrorContent( + out[8], 'schema', 'layout', null, + ['updatemenus', 2, 'buttons', 1, 'title'], 'updatemenus[2].buttons[1].title', + 'In layout, key updatemenus[2].buttons[1].title is not part of the schema' + ); + assertErrorContent( + out[9], 'unused', 'layout', null, + ['updatemenus', 2, 'buttons', 0], 'updatemenus[2].buttons[0]', + 'In layout, key updatemenus[2].buttons[0] did not get coerced' + ); + assertErrorContent( + out[10], 'object', 'layout', null, + ['updatemenus', 2, 'buttons', 2], 'updatemenus[2].buttons[2]', + 'In layout, key updatemenus[2].buttons[2] must be linked to an object container' + ); }); it('should work with isSubplotObj attributes', function() { From a560e025deaeab6139a9b5730c5608df1069c1f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 4 Aug 2016 11:08:24 -0400 Subject: [PATCH 5/7] test: add _index to expected objects --- test/jasmine/tests/range_selector_test.js | 10 ++++++---- test/jasmine/tests/updatemenus_test.js | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/test/jasmine/tests/range_selector_test.js b/test/jasmine/tests/range_selector_test.js index a88b05738a2..bfc17b46457 100644 --- a/test/jasmine/tests/range_selector_test.js +++ b/test/jasmine/tests/range_selector_test.js @@ -55,7 +55,8 @@ describe('range selector defaults:', function() { expect(containerOut.rangeselector.buttons).toEqual([{ step: 'month', stepmode: 'backward', - count: 1 + count: 1, + _index: 0 }]); }); @@ -96,8 +97,8 @@ describe('range selector defaults:', function() { expect(containerOut.rangeselector.visible).toBe(true); expect(containerOut.rangeselector.buttons).toEqual([ - { step: 'year', stepmode: 'backward', count: 10 }, - { step: 'month', stepmode: 'backward', count: 6 } + { step: 'year', stepmode: 'backward', count: 10, _index: 0 }, + { step: 'month', stepmode: 'backward', count: 6, _index: 1 } ]); }); @@ -116,7 +117,8 @@ describe('range selector defaults:', function() { expect(containerOut.rangeselector.buttons).toEqual([{ step: 'all', - label: 'full range' + label: 'full range', + _index: 0 }]); }); diff --git a/test/jasmine/tests/updatemenus_test.js b/test/jasmine/tests/updatemenus_test.js index 8d9f639db33..e0bac3fff7e 100644 --- a/test/jasmine/tests/updatemenus_test.js +++ b/test/jasmine/tests/updatemenus_test.js @@ -64,7 +64,8 @@ describe('update menus defaults', function() { expect(layoutOut.updatemenus[0].buttons[0]).toEqual({ method: 'relayout', args: ['title', 'Hello World'], - label: '' + label: '', + _index: 1 }); }); @@ -87,7 +88,8 @@ describe('update menus defaults', function() { expect(layoutOut.updatemenus[0].buttons[0]).toEqual({ method: 'relayout', args: ['title', 'Hello World'], - label: '' + label: '', + _index: 1 }); }); From 9e3633f3fa89a5fe770d093f8e715da762eb5390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 4 Aug 2016 13:28:45 -0400 Subject: [PATCH 6/7] fix typo noFixLength -> freeLength --- src/lib/coerce.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index 195ab99af76..6409692ec3c 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -233,7 +233,7 @@ exports.valObjects = { 'An {array} of plot information.' ].join(' '), requiredOpts: ['items'], - otherOpts: ['dflt', 'noFixLength'], + otherOpts: ['dflt', 'freeLength'], coerceFunction: function(v, propOut, dflt, opts) { if(!Array.isArray(v)) { propOut.set(dflt); From bfe32f656cb32e35d860dcb669d23c74306a8364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 4 Aug 2016 13:29:07 -0400 Subject: [PATCH 7/7] test: rm obsolete block in optionalOpts plot schema test --- test/jasmine/tests/plotschema_test.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/jasmine/tests/plotschema_test.js b/test/jasmine/tests/plotschema_test.js index 5f2195bfe28..2c8bacf3651 100644 --- a/test/jasmine/tests/plotschema_test.js +++ b/test/jasmine/tests/plotschema_test.js @@ -86,10 +86,7 @@ describe('plot schema', function() { .concat(['valType', 'description', 'role']); Object.keys(attr).forEach(function(key) { - // handle the histogram marker.color case - if(opts.indexOf(key) === -1 && opts[key] === undefined) return; - - expect(opts.indexOf(key) !== -1).toBe(true); + expect(opts.indexOf(key) !== -1).toBe(true, key, attr); }); } }