diff --git a/lib/util.js b/lib/util.js index e65d973a..cced85de 100644 --- a/lib/util.js +++ b/lib/util.js @@ -107,24 +107,49 @@ exports.assignAllKeys = function (obj, value) { /** - * Fill an object with a possibly partially defined other object. Only copies values if the a object has an object requiring values. + * Copy property from b to a if property present in a. + * If property in b explicitly set to null, delete it if `allowDeletion` set. + * + * Internal helper routine, should not be exported. Not added to `exports` for that reason. + * + * @param {object} a target object + * @param {object} b source object + * @param {string} prop name of property to copy to a + * @param {boolean} allowDeletion if true, delete property in a if explicitly set to null in b + * @private + */ +function copyOrDelete(a, b, prop, allowDeletion) { + var doDeletion = false; + if (allowDeletion === true) { + doDeletion = (b[prop] === null && a[prop] !== undefined); + } + + if (doDeletion) { + delete a[prop]; + } else { + a[prop] = b[prop]; // Remember, this is a reference copy! + } +} + + +/** + * Fill an object with a possibly partially defined other object. + * + * Only copies values for the properties already present in a. * That means an object is not created on a property if only the b object has it. + * * @param {object} a * @param {object} b - * @param {boolean} [allowDeletion=false] + * @param {boolean} [allowDeletion=false] if true, delete properties in a that are explicitly set to null in b */ exports.fillIfDefined = function (a, b, allowDeletion = false) { + // NOTE: iteration of properties of a + // NOTE: prototype properties iterated over as well for (var prop in a) { if (b[prop] !== undefined) { - if (typeof b[prop] !== 'object') { - if ((b[prop] === undefined || b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } - } - else { + if (b[prop] === null || typeof b[prop] !== 'object') { // Note: typeof null === 'object' + copyOrDelete(a, b, prop, allowDeletion); + } else { if (typeof a[prop] === 'object') { exports.fillIfDefined(a[prop], b[prop], allowDeletion); } @@ -134,24 +159,6 @@ exports.fillIfDefined = function (a, b, allowDeletion = false) { }; - -/** - * Extend object a with the properties of object b or a series of objects - * Only properties with defined values are copied - * @param {Object} a - * @param {...Object} b - * @return {Object} a - */ -exports.protoExtend = function (a, b) { // eslint-disable-line no-unused-vars - for (var i = 1; i < arguments.length; i++) { - var other = arguments[i]; - for (var prop in other) { - a[prop] = other[prop]; - } - } - return a; -}; - /** * Extend object a with the properties of object b or a series of objects * Only properties with defined values are copied @@ -197,118 +204,112 @@ exports.selectiveExtend = function (props, a, b) { // eslint-disable-line no-un return a; }; + /** - * Extend object a with selected properties of object b or a series of objects - * Only properties with defined values are copied - * @param {Array.} props - * @param {Object} a - * @param {Object} b - * @param {boolean} [allowDeletion=false] - * @return {Object} a + * Extend object a with selected properties of object b. + * Only properties with defined values are copied. + * + * **Note:** Previous version of this routine implied that multiple source objects + * could be used; however, the implementation was **wrong**. + * Since multiple (>1) sources weren't used anywhere in the `vis.js` code, + * this has been removed + * + * @param {Array.} props names of first-level properties to copy over + * @param {object} a target object + * @param {object} b source object + * @param {boolean} [allowDeletion=false] if true, delete property in a if explicitly set to null in b + * @returns {Object} a */ exports.selectiveDeepExtend = function (props, a, b, allowDeletion = false) { // TODO: add support for Arrays to deepExtend if (Array.isArray(b)) { throw new TypeError('Arrays are not supported by deepExtend'); } - for (var i = 2; i < arguments.length; i++) { - var other = arguments[i]; - for (var p = 0; p < props.length; p++) { - var prop = props[p]; - if (other.hasOwnProperty(prop)) { - if (b[prop] && b[prop].constructor === Object) { - if (a[prop] === undefined) { - a[prop] = {}; - } - if (a[prop].constructor === Object) { - exports.deepExtend(a[prop], b[prop], false, allowDeletion); - } - else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } - } - } else if (Array.isArray(b[prop])) { - throw new TypeError('Arrays are not supported by deepExtend'); - } else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } - } + for (var p = 0; p < props.length; p++) { + var prop = props[p]; + if (b.hasOwnProperty(prop)) { + if (b[prop] && b[prop].constructor === Object) { + if (a[prop] === undefined) { + a[prop] = {}; + } + if (a[prop].constructor === Object) { + exports.deepExtend(a[prop], b[prop], false, allowDeletion); + } + else { + copyOrDelete(a, b, prop, allowDeletion); + } + } else if (Array.isArray(b[prop])) { + throw new TypeError('Arrays are not supported by deepExtend'); + } else { + copyOrDelete(a, b, prop, allowDeletion); } } } return a; }; + /** - * Extend object a with selected properties of object b or a series of objects + * Extend object `a` with properties of object `b`, ignoring properties which are explicitly specified + * to be excluded. + * + * The properties of `b` are considered for copying. + * Properties which are themselves objects are are also extended. * Only properties with defined values are copied - * @param {Array.} props - * @param {Object} a - * @param {Object} b - * @param {boolean} [allowDeletion=false] + * + * @param {Array.} propsToExclude names of properties which should *not* be copied + * @param {Object} a object to extend + * @param {Object} b object to take properties from for extension + * @param {boolean} [allowDeletion=false] if true, delete properties in a that are explicitly set to null in b * @return {Object} a */ -exports.selectiveNotDeepExtend = function (props, a, b, allowDeletion = false) { +exports.selectiveNotDeepExtend = function (propsToExclude, a, b, allowDeletion = false) { // TODO: add support for Arrays to deepExtend + // NOTE: array properties have an else-below; apparently, there is a problem here. if (Array.isArray(b)) { throw new TypeError('Arrays are not supported by deepExtend'); } + for (var prop in b) { - if (b.hasOwnProperty(prop)) { - if (props.indexOf(prop) == -1) { - if (b[prop] && b[prop].constructor === Object) { - if (a[prop] === undefined) { - a[prop] = {}; - } - if (a[prop].constructor === Object) { - exports.deepExtend(a[prop], b[prop]); - } - else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } - } - } else if (Array.isArray(b[prop])) { - a[prop] = []; - for (let i = 0; i < b[prop].length; i++) { - a[prop].push(b[prop][i]); - } - } else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } - } + if (!b.hasOwnProperty(prop)) continue; // Handle local properties only + if (propsToExclude.indexOf(prop) !== -1) continue; // In exclusion list, skip + + if (b[prop] && b[prop].constructor === Object) { + if (a[prop] === undefined) { + a[prop] = {}; + } + if (a[prop].constructor === Object) { + exports.deepExtend(a[prop], b[prop]); // NOTE: allowDeletion not propagated! + } + else { + copyOrDelete(a, b, prop, allowDeletion); + } + } else if (Array.isArray(b[prop])) { + a[prop] = []; + for (let i = 0; i < b[prop].length; i++) { + a[prop].push(b[prop][i]); } + } else { + copyOrDelete(a, b, prop, allowDeletion); } } + return a; }; + /** * Deep extend an object a with the properties of object b + * * @param {Object} a * @param {Object} b - * @param {boolean} [protoExtend] --> optional parameter. If true, the prototype values will also be extended. - * (ie. the options objects that inherit from others will also get the inherited options) - * @param {boolean} [allowDeletion] --> optional parameter. If true, the values of fields that are null will be deleted + * @param {boolean} [protoExtend=false] If true, the prototype values will also be extended. + * (ie. the options objects that inherit from others will also get the inherited options) + * @param {boolean} [allowDeletion=false] If true, the values of fields that are null will be deleted * @returns {Object} */ -exports.deepExtend = function (a, b, protoExtend, allowDeletion) { +exports.deepExtend = function (a, b, protoExtend=false, allowDeletion=false) { for (var prop in b) { if (b.hasOwnProperty(prop) || protoExtend === true) { if (b[prop] && b[prop].constructor === Object) { @@ -316,15 +317,10 @@ exports.deepExtend = function (a, b, protoExtend, allowDeletion) { a[prop] = {}; } if (a[prop].constructor === Object) { - exports.deepExtend(a[prop], b[prop], protoExtend); + exports.deepExtend(a[prop], b[prop], protoExtend); // NOTE: allowDeletion not propagated! } else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } + copyOrDelete(a, b, prop, allowDeletion); } } else if (Array.isArray(b[prop])) { a[prop] = []; @@ -332,18 +328,14 @@ exports.deepExtend = function (a, b, protoExtend, allowDeletion) { a[prop].push(b[prop][i]); } } else { - if ((b[prop] === null) && a[prop] !== undefined && allowDeletion === true) { - delete a[prop]; - } - else { - a[prop] = b[prop]; - } + copyOrDelete(a, b, prop, allowDeletion); } } } return a; }; + /** * Test whether all elements in two arrays are equal. * @param {Array} a diff --git a/test/Network.test.js b/test/Network.test.js index 2b450a15..8d6e4b6e 100644 --- a/test/Network.test.js +++ b/test/Network.test.js @@ -17,6 +17,7 @@ var Network = vis.network; var jsdom_global = require('jsdom-global'); var stdout = require('test-console').stdout; var Validator = require("./../lib/shared/Validator").default; +var {allOptions, configureOptions} = require('./../lib/network/options.js'); //var {printStyle} = require('./../lib/shared/Validator'); @@ -178,6 +179,49 @@ function assertNumEdges(network, expectedPresent, expectedVisible) { }; +/** + * Check if the font options haven't changed. + * + * This is to guard against future code changes; a lot of the code deals with particular properties of + * the font options. + * If any assertion fails here, all code in Network handling fonts should be checked. + */ +function checkFontProperties(fontItem, checkStrict = true) { + var knownProperties = [ + 'color', + 'size', + 'face', + 'background', + 'strokeWidth', + 'strokeColor', + 'align', + 'multi', + 'vadjust', + 'bold', + 'boldital', + 'ital', + 'mono', + ]; + + // All properties in fontItem should be known + for (var prop in fontItem) { + if (prop === '__type__') continue; // Skip special field in options definition + if (!fontItem.hasOwnProperty(prop)) continue; + assert(knownProperties.indexOf(prop) !== -1, "Unknown font option '" + prop + "'"); + } + + if (!checkStrict) return; + + // All known properties should be present + var keys = Object.keys(fontItem); + for (var n in knownProperties) { + var prop = knownProperties[n]; + assert(keys.indexOf(prop) !== -1, "Missing known font option '" + prop + "'"); + } +} + + + describe('Network', function () { before(function() { @@ -250,6 +294,13 @@ describe('Network', function () { describe('Node', function () { + it('has known font options', function () { + var network = createNetwork({}); + checkFontProperties(network.nodesHandler.defaultOptions.font); + checkFontProperties(allOptions.nodes.font); + checkFontProperties(configureOptions.nodes.font, false); + }); + /** * NOTE: choosify tests of Node and Edge are parallel @@ -293,6 +344,14 @@ describe('Node', function () { describe('Edge', function () { + it('has known font options', function () { + var network = createNetwork({}); + checkFontProperties(network.edgesHandler.defaultOptions.font); + checkFontProperties(allOptions.edges.font); + checkFontProperties(configureOptions.edges.font, false); + }); + + /** * NOTE: choosify tests of Node and Edge are parallel * TODO: consolidate this is necessary diff --git a/test/util.test.js b/test/util.test.js index 0c112b35..c5d7fc28 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -2,10 +2,345 @@ var assert = require('assert'); var util = require('../lib/util'); +describe('util', function () { + +/** + * Tests for copy and extend methods. + * + * Goal: to cover all possible paths within the tested method(s) + * + * + * **NOTES** + * + * - All these methods have the inherent flaw that it's possible to define properties + * on an object with value 'undefined'. e.g. in `node`: + * + * > a = { b:undefined } + * > a.hasOwnProperty('b') + * true + * + * The logic for handling this in the code is minimal and accidental. For the time being, + * this flaw is ignored. + */ +describe('extend routines', function () { + + /** + * Check if values have been copied over from b to a as intended + */ + function checkExtended(a, b, checkCopyTarget = false) { + var result = { + color: 'green', + sub: { + enabled: false, + sub2: { + font: 'awesome' + } + } + }; + + assert(a.color !== undefined && a.color === result.color); + assert(a.notInSource === true); + if (checkCopyTarget) { + assert(a.notInTarget === true); + } else { + assert(a.notInTarget === undefined); + } + + var sub = a.sub; + assert(sub !== undefined); + assert(sub.enabled !== undefined && sub.enabled === result.sub.enabled); + assert(sub.notInSource === true); + if (checkCopyTarget) { + assert(sub.notInTarget === true); + } else { + assert(sub.notInTarget === undefined); + } + + sub = a.sub.sub2; + assert(sub !== undefined); + assert(sub !== undefined && sub.font !== undefined && sub.font === result.sub.sub2.font); + assert(sub.notInSource === true); + assert(a.subNotInSource !== undefined); + if (checkCopyTarget) { + assert(a.subNotInTarget.enabled === true); + assert(sub.notInTarget === true); + } else { + assert(a.subNotInTarget === undefined); + assert(sub.notInTarget === undefined); + } + } + + + /** + * Spot check on values of a unchanged as intended + */ + function testAUnchanged(a) { + var sub = a.sub; + assert(sub !== undefined); + assert(sub.enabled !== undefined && sub.enabled === true); + assert(sub.notInSource === true); + assert(sub.notInTarget === undefined); + assert(sub.deleteThis === true); + + sub = a.sub.sub2; + assert(sub !== undefined); + assert(sub !== undefined && sub.font !== undefined && sub.font === 'arial'); + assert(sub.notInSource === true); + assert(sub.notInTarget === undefined); + + assert(a.subNotInSource !== undefined); + assert(a.subNotInTarget === undefined); + } + + + function initA() { + return { + color: 'red', + notInSource: true, + sub: { + enabled: true, + notInSource: true, + sub2: { + font: 'arial', + notInSource: true, + }, + deleteThis: true, + }, + subNotInSource: { + enabled: true, + }, + deleteThis: true, + subDeleteThis: { + enabled: true, + }, + }; + } + + + beforeEach(function() { + this.a = initA(); + + this.b = { + color: 'green', + notInTarget: true, + sub: { + enabled: false, + notInTarget: true, + sub2: { + font: 'awesome', + notInTarget: true, + }, + deleteThis: null, + }, + subNotInTarget: { + enabled: true, + }, + deleteThis: null, + subDeleteThis: null + }; + }); + + + it('performs fillIfDefined() as advertized', function () { + var a = this.a; + var b = this.b; + + util.fillIfDefined(a, b); + checkExtended(a, b); + + // NOTE: if allowDeletion === false, null values are copied over! + // This is due to existing logic; it might not be the intention and hence a bug + assert(a.sub.deleteThis === null); + assert(a.deleteThis === null); + assert(a.subDeleteThis === null); + }); + + + it('performs fillIfDefined() as advertized with deletion', function () { + var a = this.a; + var b = this.b; + + util.fillIfDefined(a, b, true); // thrid param: allowDeletion + checkExtended(a, b); + + // Following should be removed now + assert(a.sub.deleteThis === undefined); + assert(a.deleteThis === undefined); + assert(a.subDeleteThis === undefined); + }); + + + it('performs selectiveDeepExtend() as advertized', function () { + var a = this.a; + var b = this.b; + + // pedantic: copy nothing + util.selectiveDeepExtend([], a, b); + assert(a.color !== undefined && a.color === 'red'); + assert(a.notInSource === true); + assert(a.notInTarget === undefined); + + // pedantic: copy nonexistent property (nothing happens) + assert(b.iDontExist === undefined); + util.selectiveDeepExtend(['iDontExist'], a, b, true); + assert(a.iDontExist === undefined); + + // At this point nothing should have changed yet. + testAUnchanged(a); + + // Copy one property + util.selectiveDeepExtend(['color'], a, b); + assert(a.color !== undefined && a.color === 'green'); + + // Copy property Object + var sub = a.sub; + assert(sub.deleteThis === true); // pre + util.selectiveDeepExtend(['sub'], a, b); + assert(sub !== undefined); + assert(sub.enabled !== undefined && sub.enabled === false); + assert(sub.notInSource === true); + assert(sub.notInTarget === true); + assert(sub.deleteThis === null); + + + // Copy new Objects + assert(a.notInTarget === undefined); // pre + assert(a.subNotInTarget === undefined); // pre + util.selectiveDeepExtend(['notInTarget', 'subNotInTarget'], a, b); + assert(a.notInTarget === true); + assert(a.subNotInTarget.enabled === true); + + // Copy null objects + assert(a.deleteThis !== null); // pre + assert(a.subDeleteThis !== null); // pre + util.selectiveDeepExtend(['deleteThis', 'subDeleteThis'], a, b); + + // NOTE: if allowDeletion === false, null values are copied over! + // This is due to existing logic; it might not be the intention and hence a bug + assert(a.deleteThis === null); + assert(a.subDeleteThis === null); + }); + + + it('performs selectiveDeepExtend() as advertized with deletion', function () { + var a = this.a; + var b = this.b; + + // Only test expected differences here with test allowDeletion === false + + // Copy object property with properties to be deleted + var sub = a.sub; + assert(sub.deleteThis === true); // pre + util.selectiveDeepExtend(['sub'], a, b, true); + assert(sub.deleteThis === undefined); // should be deleted + + // Spot check on rest of properties in `a.sub` - there should have been copied + sub = a.sub; + assert(sub !== undefined); + assert(sub.enabled !== undefined && sub.enabled === false); + assert(sub.notInSource === true); + assert(sub.notInTarget === true); + + // Copy null objects + assert(a.deleteThis === true); // pre + assert(a.subDeleteThis !== undefined); // pre + assert(a.subDeleteThis.enabled === true); // pre + util.selectiveDeepExtend(['deleteThis', 'subDeleteThis'], a, b, true); + assert(a.deleteThis === undefined); // should be deleted + assert(a.subDeleteThis === undefined); // should be deleted + }); + + + it('performs selectiveNotDeepExtend() as advertized', function () { + var a = this.a; + var b = this.b; + + // Exclude all properties, nothing copied + util.selectiveNotDeepExtend(Object.keys(b), a, b); + testAUnchanged(a); + + // Exclude nothing, everything copied + util.selectiveNotDeepExtend([], a, b); + checkExtended(a, b, true); + + // Exclude some + a = initA(); + assert(a.notInTarget === undefined); // pre + assert(a.subNotInTarget === undefined); // pre + util.selectiveNotDeepExtend(['notInTarget', 'subNotInTarget'], a, b); + assert(a.notInTarget === undefined); // not copied + assert(a.subNotInTarget === undefined); // not copied + assert(a.sub.notInTarget === true); // copied! + }); + + + it('performs selectiveNotDeepExtend() as advertized with deletion', function () { + var a = this.a; + var b = this.b; + + // Exclude all properties, nothing copied + util.selectiveNotDeepExtend(Object.keys(b), a, b, true); + testAUnchanged(a); + + // Exclude nothing, everything copied and some deleted + util.selectiveNotDeepExtend([], a, b, true); + checkExtended(a, b, true); + + // Exclude some + a = initA(); + assert(a.notInTarget === undefined); // pre + assert(a.subNotInTarget === undefined); // pre + assert(a.deleteThis === true); // pre + assert(a.subDeleteThis !== undefined); // pre + assert(a.sub.deleteThis === true); // pre + assert(a.subDeleteThis.enabled === true); // pre + util.selectiveNotDeepExtend(['notInTarget', 'subNotInTarget'], a, b, true); + assert(a.deleteThis === undefined); // should be deleted + assert(a.sub.deleteThis !== undefined); // not deleted! Original logic, could be a bug + assert(a.subDeleteThis === undefined); // should be deleted + // Spot check: following should be same as allowDeletion === false + assert(a.notInTarget === undefined); // not copied + assert(a.subNotInTarget === undefined); // not copied + assert(a.sub.notInTarget === true); // copied! + }); + + + /** + * NOTE: parameter `protoExtend` not tested here! + */ + it('performs deepExtend() as advertized', function () { + var a = this.a; + var b = this.b; + + util.deepExtend(a, b); + checkExtended(a, b, true); + }); + + + /** + * NOTE: parameter `protoExtend` not tested here! + */ + it('performs deepExtend() as advertized with delete', function () { + var a = this.a; + var b = this.b; + + // Copy null objects + assert(a.deleteThis === true); // pre + assert(a.subDeleteThis !== undefined); // pre + assert(a.subDeleteThis.enabled === true); // pre + util.deepExtend(a, b, false, true); + checkExtended(a, b, true); // Normal copy should be good + assert(a.deleteThis === undefined); // should be deleted + assert(a.subDeleteThis === undefined); // should be deleted + assert(a.sub.deleteThis !== undefined); // not deleted!!! Original logic, could be a bug + }); +}); // extend routines + + // // The important thing with mergeOptions() is that 'enabled' is always set in target option. // -describe('util.mergeOptions', function () { +describe('mergeOptions', function () { it('handles good input without global options', function () { var options = { @@ -131,4 +466,6 @@ describe('util.mergeOptions', function () { util.mergeOptions(mergeTarget, options, 'alsoMissingEnabled', globalOptions); assert(mergeTarget.alsoMissingEnabled.enabled === true); }); -}); + +}); // mergeOptions +}); // util