diff --git a/kolibri/core/assets/src/objectSpecs.js b/kolibri/core/assets/src/objectSpecs.js index d5a8c3ed4c9..588ff96b6b2 100644 --- a/kolibri/core/assets/src/objectSpecs.js +++ b/kolibri/core/assets/src/objectSpecs.js @@ -41,7 +41,7 @@ import isArray from 'lodash/isArray'; import isBoolean from 'lodash/isBoolean'; import isDate from 'lodash/isDate'; import isFunction from 'lodash/isFunction'; -import isObject from 'lodash/isObject'; +import isPlainObject from 'lodash/isPlainObject'; import isNumber from 'lodash/isNumber'; import isString from 'lodash/isString'; import isSymbol from 'lodash/isSymbol'; @@ -75,10 +75,22 @@ function _validateObjectData(data, options, dataKey) { // object sub-spec if (hasData && options.spec) { - if (!isObject(data)) { - return _fail('Only objects can have sub-specs', dataKey, data); + if (!isPlainObject(data) && !isArray(data)) { + return _fail('Only objects or arrays can have sub-specs', dataKey, data); } - if (!validateObject(data, options.spec)) { + + // If it is an array, we will validate each item in the array + if (isArray(data)) { + for (const item of data) { + if (!validateObject(item, options.spec.spec)) { + return _fail('Object in Array sub-spec failed', dataKey, item); + } + } + } + + // Here we know it is an Object, but we need to be sure it isn't an Array to avoid + // checking it again after we already validated the Array in the block above. + if (isPlainObject(data) && !validateObject(data, options.spec)) { return _fail('Validator sub-spec failed', dataKey, data); } } @@ -98,7 +110,7 @@ function _validateObjectData(data, options, dataKey) { return _fail('Expected Date', dataKey, data); } else if (options.type === Function && !isFunction(data)) { return _fail('Expected Function', dataKey, data); - } else if (options.type === Object && !isObject(data)) { + } else if (options.type === Object && !isPlainObject(data)) { return _fail('Expected Object', dataKey, data); } else if (options.type === Number && !isNumber(data)) { return _fail('Expected Number', dataKey, data); @@ -142,7 +154,7 @@ export function validateObject(object, spec) { let isValid = true; for (const dataKey in spec) { const options = spec[dataKey]; - if (!isObject(options)) { + if (!isPlainObject(options)) { logging.error(`Expected an Object for '${dataKey}' in spec. Got:`, options); isValid = false; continue; diff --git a/kolibri/core/assets/src/styles/themeSpec.js b/kolibri/core/assets/src/styles/themeSpec.js index 62d6ac20766..a48624b3199 100644 --- a/kolibri/core/assets/src/styles/themeSpec.js +++ b/kolibri/core/assets/src/styles/themeSpec.js @@ -172,7 +172,7 @@ export default { }, logos: { type: Array, - default: [], + default: () => [], spec: _imageSpec, }, }; diff --git a/kolibri/core/assets/test/objectSpecs.spec.js b/kolibri/core/assets/test/objectSpecs.spec.js index f48618ccc22..7a581ba2c8b 100644 --- a/kolibri/core/assets/test/objectSpecs.spec.js +++ b/kolibri/core/assets/test/objectSpecs.spec.js @@ -46,6 +46,26 @@ const simpleSpec = { }; }, }, + arrayOfNum: { + type: Array, + default: () => [], + spec: { + type: Number, + required: true, + }, + }, + arrayOfObj: { + type: Array, + default: () => [], + spec: { + type: Object, + default: null, + spec: { + prop_C: { type: String, default: 'val_C' }, + prop_D: { type: String, default: 'val_D' }, + }, + }, + }, }; describe('validateObject basic operation', () => { @@ -99,6 +119,14 @@ describe('validateObject basic operation', () => { }; expect(validateObject(obj, simpleSpec)).toBe(true); }); + test('validateObject should test all children of an Array that has a spec', () => { + const obj = { + str1: 'A', + arrayOfNum: [1, 2, 3, 4, 5], + arrayOfObj: [{ prop_C: 'val_C', prop_D: 'val_D' }], + }; + expect(validateObject(obj, simpleSpec)).toBe(true); + }); }); describe('validateObject rejects bad specs', () => { @@ -126,7 +154,7 @@ describe('validateObject rejects bad specs', () => { }; expect(validateObject(obj, badSpec)).toBe(false); }); - test('non-object cannot have sub-spec', () => { + test('non-object/array cannot have sub-spec', () => { const badSpec = { str1: { type: String, @@ -156,6 +184,16 @@ describe('validateObject rejects bad specs', () => { }; expect(validateObject(obj, badSpec)).toBe(false); }); + test('A specced array rejects an array even if any value is bad', () => { + const badNumArrayObj = { + arrayOfNum: [1, 2, 3, 4, 5, 'A'], + }; + expect(validateObject(badNumArrayObj, simpleSpec)).toBe(false); + const badObjArrayObj = { + arrayOfObj: [{ foo: 'bar', prop_c: 'val_c' }], + }; + expect(validateObject(badObjArrayObj, simpleSpec)).toBe(false); + }); }); describe('objectWithDefaults fills defaults', () => {