Skip to content

Commit 4b64345

Browse files
etpinardetpinard
authored
Merge pull request #814 from plotly/validate-missing-items
Fix validation logic for arrays attributes and array containers
2 parents 9125f21 + bfe32f6 commit 4b64345

File tree

10 files changed

+131
-29
lines changed

10 files changed

+131
-29
lines changed

‎src/components/rangeselector/defaults.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ function buttonsDefaults(containerIn, containerOut) {
6969

7070
coerce('label');
7171

72+
buttonOut._index = i;
7273
buttonsOut.push(buttonOut);
7374
}
7475

‎src/components/updatemenus/attributes.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var buttonsAttrs = {
2727
args: {
2828
valType: 'info_array',
2929
role: 'info',
30+
freeLength: true,
3031
items: [
3132
{ valType: 'any' },
3233
{ valType: 'any' },

‎src/components/updatemenus/defaults.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ function buttonsDefaults(menuIn, menuOut) {
8686
coerce('args');
8787
coerce('label');
8888

89+
buttonOut._index = i;
8990
buttonsOut.push(buttonOut);
9091
}
9192

‎src/lib/coerce.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ exports.valObjects = {
233233
'An {array} of plot information.'
234234
].join(' '),
235235
requiredOpts: ['items'],
236-
otherOpts: ['dflt'],
236+
otherOpts: ['dflt', 'freeLength'],
237237
coerceFunction: function(v, propOut, dflt, opts) {
238238
if(!Array.isArray(v)) {
239239
propOut.set(dflt);
@@ -255,10 +255,11 @@ exports.valObjects = {
255255

256256
var items = opts.items;
257257

258-
if(v.length !== items.length) return false;
258+
// when free length is off, input and declared lengths must match
259+
if(!opts.freeLength && v.length !== items.length) return false;
259260

260-
// valid when all items are valid
261-
for(var i = 0; i < items.length; i++) {
261+
// valid when all input items are valid
262+
for(var i = 0; i < v.length; i++) {
262263
var isItemValid = exports.validate(v[i], opts.items[i]);
263264

264265
if(!isItemValid) return false;

‎src/plot_api/validate.js‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,30 +163,53 @@ function crawl(objIn, objOut, schema, list, base, path) {
163163
var valIn = objIn[k],
164164
valOut = objOut[k];
165165

166-
var nestedSchema = getNestedSchema(schema, k);
166+
var nestedSchema = getNestedSchema(schema, k),
167+
isInfoArray = (nestedSchema || {}).valType === 'info_array';
167168

168169
if(!isInSchema(schema, k)) {
169170
list.push(format('schema', base, p));
170171
}
171172
else if(isPlainObject(valIn) && isPlainObject(valOut)) {
172173
crawl(valIn, valOut, nestedSchema, list, base, p);
173174
}
174-
else if(nestedSchema.items && isArray(valIn)) {
175-
var itemName = k.substr(0, k.length - 1);
175+
else if(nestedSchema.items && !isInfoArray && isArray(valIn)) {
176+
var itemName = k.substr(0, k.length - 1),
177+
indexList = [];
176178

177-
for(var j = 0; j < valIn.length; j++) {
179+
var j, _p;
180+
181+
// loop over valOut items while keeping track of their
182+
// corresponding input container index (given by _index)
183+
for(j = 0; j < valOut.length; j++) {
178184
var _nestedSchema = nestedSchema.items[itemName],
179-
_p = p.slice();
185+
_index = valOut[j]._index || j;
186+
187+
_p = p.slice();
188+
_p.push(_index);
180189

190+
if(isPlainObject(valIn[_index]) && isPlainObject(valOut[j])) {
191+
indexList.push(_index);
192+
crawl(valIn[_index], valOut[j], _nestedSchema, list, base, _p);
193+
}
194+
}
195+
196+
// loop over valIn to determine where it went wrong for some items
197+
for(j = 0; j < valIn.length; j++) {
198+
_p = p.slice();
181199
_p.push(j);
182200

183-
crawl(valIn[j], valOut[j], _nestedSchema, list, base, _p);
201+
if(!isPlainObject(valIn[j])) {
202+
list.push(format('object', base, _p, valIn[j]));
203+
}
204+
else if(indexList.indexOf(j) === -1) {
205+
list.push(format('unused', base, _p));
206+
}
184207
}
185208
}
186209
else if(!isPlainObject(valIn) && isPlainObject(valOut)) {
187210
list.push(format('object', base, p, valIn));
188211
}
189-
else if(!isArray(valIn) && isArray(valOut) && nestedSchema.valType !== 'info_array') {
212+
else if(!isArray(valIn) && isArray(valOut) && !isInfoArray) {
190213
list.push(format('array', base, p, valIn));
191214
}
192215
else if(!(k in objOut)) {

‎test/jasmine/tests/lib_test.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,29 @@ describe('Test lib.js:', function() {
985985
}]
986986
});
987987
});
988+
989+
it('should work for valType \'info_array\' (freeLength case)', function() {
990+
var shouldPass = [
991+
['marker.color', 'red'],
992+
[{ 'marker.color': 'red' }, [1, 2]]
993+
];
994+
var shouldFail = [
995+
['marker.color', 'red', 'red'],
996+
[{ 'marker.color': 'red' }, [1, 2], 'blue']
997+
];
998+
999+
assert(shouldPass, shouldFail, {
1000+
valType: 'info_array',
1001+
freeLength: true,
1002+
items: [{
1003+
valType: 'any'
1004+
}, {
1005+
valType: 'any'
1006+
}, {
1007+
valType: 'number'
1008+
}]
1009+
});
1010+
});
9881011
});
9891012

9901013
describe('setCursor', function() {

‎test/jasmine/tests/plotschema_test.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,7 @@ describe('plot schema', function() {
8383
.concat(['valType', 'description', 'role']);
8484

8585
Object.keys(attr).forEach(function(key) {
86-
// handle the histogram marker.color case
87-
if(opts.indexOf(key) === -1 && opts[key] === undefined) return;
88-
89-
expect(opts.indexOf(key) !== -1).toBe(true);
86+
expect(opts.indexOf(key) !== -1).toBe(true, key, attr);
9087
});
9188
}
9289
}

‎test/jasmine/tests/range_selector_test.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ describe('range selector defaults:', function() {
5555
expect(containerOut.rangeselector.buttons).toEqual([{
5656
step: 'month',
5757
stepmode: 'backward',
58-
count: 1
58+
count: 1,
59+
_index: 0
5960
}]);
6061
});
6162

@@ -96,8 +97,8 @@ describe('range selector defaults:', function() {
9697

9798
expect(containerOut.rangeselector.visible).toBe(true);
9899
expect(containerOut.rangeselector.buttons).toEqual([
99-
{ step: 'year', stepmode: 'backward', count: 10 },
100-
{ step: 'month', stepmode: 'backward', count: 6 }
100+
{ step: 'year', stepmode: 'backward', count: 10, _index: 0 },
101+
{ step: 'month', stepmode: 'backward', count: 6, _index: 1 }
101102
]);
102103
});
103104

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

117118
expect(containerOut.rangeselector.buttons).toEqual([{
118119
step: 'all',
119-
label: 'full range'
120+
label: 'full range',
121+
_index: 0
120122
}]);
121123
});
122124

‎test/jasmine/tests/updatemenus_test.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ describe('update menus defaults', function() {
6464
expect(layoutOut.updatemenus[0].buttons[0]).toEqual({
6565
method: 'relayout',
6666
args: ['title', 'Hello World'],
67-
label: ''
67+
label: '',
68+
_index: 1
6869
});
6970
});
7071

@@ -87,7 +88,8 @@ describe('update menus defaults', function() {
8788
expect(layoutOut.updatemenus[0].buttons[0]).toEqual({
8889
method: 'relayout',
8990
args: ['title', 'Hello World'],
90-
label: ''
91+
label: '',
92+
_index: 1
9193
});
9294
});
9395

‎test/jasmine/tests/validate_test.js‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,21 @@ describe('Plotly.validate', function() {
138138
);
139139
});
140140

141+
it('should work with info arrays', function() {
142+
var out = Plotly.validate([{
143+
y: [1, 2, 2]
144+
}], {
145+
xaxis: { range: [0, 10] },
146+
yaxis: { range: 'not-gonna-work' },
147+
});
148+
149+
expect(out.length).toEqual(1);
150+
assertErrorContent(
151+
out[0], 'value', 'layout', null, ['yaxis', 'range'], 'yaxis.range',
152+
'In layout, key yaxis.range is set to an invalid value (not-gonna-work)'
153+
);
154+
});
155+
141156
it('should work with isLinkedToArray attributes', function() {
142157
var out = Plotly.validate([], {
143158
annotations: [{
@@ -154,7 +169,7 @@ describe('Plotly.validate', function() {
154169
label: '1 month',
155170
step: 'all',
156171
count: 10
157-
}, {
172+
}, 'wont-work', {
158173
title: '1 month'
159174
}]
160175
}
@@ -175,10 +190,25 @@ describe('Plotly.validate', function() {
175190
},
176191
shapes: [{
177192
opacity: 'none'
193+
}],
194+
updatemenus: [{
195+
buttons: [{
196+
method: 'restyle',
197+
args: ['marker.color', 'red']
198+
}]
199+
}, 'wont-work', {
200+
buttons: [{
201+
method: 'restyle',
202+
args: null
203+
}, {
204+
method: 'relayout',
205+
args: ['marker.color', 'red'],
206+
title: 'not-gonna-work'
207+
}, 'wont-work']
178208
}]
179209
});
180210

181-
expect(out.length).toEqual(7);
211+
expect(out.length).toEqual(12);
182212
assertErrorContent(
183213
out[0], 'schema', 'layout', null,
184214
['annotations', 1, 'arrowSymbol'], 'annotations[1].arrowSymbol',
@@ -197,27 +227,48 @@ describe('Plotly.validate', function() {
197227
);
198228
assertErrorContent(
199229
out[3], 'schema', 'layout', null,
200-
['xaxis', 'rangeselector', 'buttons', 1, 'title'],
201-
'xaxis.rangeselector.buttons[1].title',
202-
'In layout, key xaxis.rangeselector.buttons[1].title is not part of the schema'
230+
['xaxis', 'rangeselector', 'buttons', 2, 'title'],
231+
'xaxis.rangeselector.buttons[2].title',
232+
'In layout, key xaxis.rangeselector.buttons[2].title is not part of the schema'
203233
);
204234
assertErrorContent(
205-
out[4], 'schema', 'layout', null,
235+
out[4], 'object', 'layout', null,
236+
['xaxis', 'rangeselector', 'buttons', 1],
237+
'xaxis.rangeselector.buttons[1]',
238+
'In layout, key xaxis.rangeselector.buttons[1] must be linked to an object container'
239+
);
240+
assertErrorContent(
241+
out[5], 'schema', 'layout', null,
206242
['xaxis2', 'rangeselector', 'buttons', 0, 'title'],
207243
'xaxis2.rangeselector.buttons[0].title',
208244
'In layout, key xaxis2.rangeselector.buttons[0].title is not part of the schema'
209245
);
210246
assertErrorContent(
211-
out[5], 'array', 'layout', null,
247+
out[6], 'array', 'layout', null,
212248
['xaxis3', 'rangeselector', 'buttons'],
213249
'xaxis3.rangeselector.buttons',
214250
'In layout, key xaxis3.rangeselector.buttons must be linked to an array container'
215251
);
216252
assertErrorContent(
217-
out[6], 'value', 'layout', null,
253+
out[7], 'value', 'layout', null,
218254
['shapes', 0, 'opacity'], 'shapes[0].opacity',
219255
'In layout, key shapes[0].opacity is set to an invalid value (none)'
220256
);
257+
assertErrorContent(
258+
out[8], 'schema', 'layout', null,
259+
['updatemenus', 2, 'buttons', 1, 'title'], 'updatemenus[2].buttons[1].title',
260+
'In layout, key updatemenus[2].buttons[1].title is not part of the schema'
261+
);
262+
assertErrorContent(
263+
out[9], 'unused', 'layout', null,
264+
['updatemenus', 2, 'buttons', 0], 'updatemenus[2].buttons[0]',
265+
'In layout, key updatemenus[2].buttons[0] did not get coerced'
266+
);
267+
assertErrorContent(
268+
out[10], 'object', 'layout', null,
269+
['updatemenus', 2, 'buttons', 2], 'updatemenus[2].buttons[2]',
270+
'In layout, key updatemenus[2].buttons[2] must be linked to an object container'
271+
);
221272
});
222273

223274
it('should work with isSubplotObj attributes', function() {

0 commit comments

Comments
 (0)