Skip to content

Fix validation logic for arrays attributes and array containers #814

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 4, 2016
Merged

Fix validation logic for arrays attributes and array containers #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
081bec4
coerce: add 'freeLength' val object option for 'info_array'
etpinard Aug 3, 2016
cfb3986
validate: don't mix up info array and array containers
etpinard Aug 3, 2016
d634cc4
set '_index' in isLinkedArray containers
etpinard Aug 3, 2016
e7391d5
validate: improve isLinkedToArray validation
etpinard Aug 3, 2016
a560e02
test: add _index to expected objects
etpinard Aug 4, 2016
9e3633f
fix typo noFixLength -> freeLength
etpinard Aug 4, 2016
bfe32f6
test: rm obsolete block in optionalOpts plot schema test
etpinard Aug 4, 2016
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
1 change: 1 addition & 0 deletions src/components/rangeselector/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function buttonsDefaults(containerIn, containerOut) {

coerce('label');

buttonOut._index = i;
buttonsOut.push(buttonOut);
}

Expand Down
1 change: 1 addition & 0 deletions src/components/updatemenus/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var buttonsAttrs = {
args: {
valType: 'info_array',
role: 'info',
freeLength: true,
items: [
{ valType: 'any' },
{ valType: 'any' },
Expand Down
1 change: 1 addition & 0 deletions src/components/updatemenus/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ function buttonsDefaults(menuIn, menuOut) {
coerce('args');
coerce('label');

buttonOut._index = i;
buttonsOut.push(buttonOut);
}

Expand Down
9 changes: 5 additions & 4 deletions src/lib/coerce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ exports.valObjects = {
'An {array} of plot information.'
].join(' '),
requiredOpts: ['items'],
otherOpts: ['dflt'],
otherOpts: ['dflt', 'freeLength'],
coerceFunction: function(v, propOut, dflt, opts) {
if(!Array.isArray(v)) {
propOut.set(dflt);
Expand All @@ -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;
Expand Down
37 changes: 30 additions & 7 deletions src/plot_api/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,30 +163,53 @@ 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));
}
else if(isPlainObject(valIn) && isPlainObject(valOut)) {
crawl(valIn, valOut, nestedSchema, list, base, p);
}
else if(nestedSchema.items && isArray(valIn)) {
var itemName = k.substr(0, k.length - 1);
else if(nestedSchema.items && !isInfoArray && isArray(valIn)) {
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)) {
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)) {
Expand Down
23 changes: 23 additions & 0 deletions test/jasmine/tests/lib_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 1 addition & 4 deletions test/jasmine/tests/plotschema_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
}
Expand Down
10 changes: 6 additions & 4 deletions test/jasmine/tests/range_selector_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ describe('range selector defaults:', function() {
expect(containerOut.rangeselector.buttons).toEqual([{
step: 'month',
stepmode: 'backward',
count: 1
count: 1,
_index: 0
}]);
});

Expand Down Expand Up @@ -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 }
]);
});

Expand All @@ -116,7 +117,8 @@ describe('range selector defaults:', function() {

expect(containerOut.rangeselector.buttons).toEqual([{
step: 'all',
label: 'full range'
label: 'full range',
_index: 0
}]);
});

Expand Down
6 changes: 4 additions & 2 deletions test/jasmine/tests/updatemenus_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});

Expand All @@ -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
});
});

Expand Down
67 changes: 59 additions & 8 deletions test/jasmine/tests/validate_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [{
Expand All @@ -154,7 +169,7 @@ describe('Plotly.validate', function() {
label: '1 month',
step: 'all',
count: 10
}, {
}, 'wont-work', {
title: '1 month'
}]
}
Expand All @@ -175,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',
Expand All @@ -197,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() {
Expand Down