diff --git a/lib/network/modules/components/Edge.js b/lib/network/modules/components/Edge.js index 3e29041e..99b5ad42 100644 --- a/lib/network/modules/components/Edge.js +++ b/lib/network/modules/components/Edge.js @@ -129,8 +129,8 @@ class Edge { // only deep extend the items in the field array. These do not have shorthand. util.selectiveDeepExtend(fields, parentOptions, newOptions, allowDeletion); - util.mergeOptions(parentOptions, newOptions, 'smooth', allowDeletion, globalOptions); - util.mergeOptions(parentOptions, newOptions, 'shadow', allowDeletion, globalOptions); + util.mergeOptions(parentOptions, newOptions, 'smooth', globalOptions); + util.mergeOptions(parentOptions, newOptions, 'shadow', globalOptions); if (newOptions.dashes !== undefined && newOptions.dashes !== null) { parentOptions.dashes = newOptions.dashes; @@ -143,7 +143,7 @@ class Edge { if (newOptions.scaling !== undefined && newOptions.scaling !== null) { if (newOptions.scaling.min !== undefined) {parentOptions.scaling.min = newOptions.scaling.min;} if (newOptions.scaling.max !== undefined) {parentOptions.scaling.max = newOptions.scaling.max;} - util.mergeOptions(parentOptions.scaling, newOptions.scaling, 'label', allowDeletion, globalOptions.scaling); + util.mergeOptions(parentOptions.scaling, newOptions.scaling, 'label', globalOptions.scaling); } else if (allowDeletion === true && newOptions.scaling === null) { parentOptions.scaling = Object.create(globalOptions.scaling); // this sets the pointer of the option back to the global option. @@ -158,9 +158,9 @@ class Edge { parentOptions.arrows.from.enabled = arrows.indexOf("from") != -1; } else if (typeof newOptions.arrows === 'object') { - util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'to', allowDeletion, globalOptions.arrows); - util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'middle', allowDeletion, globalOptions.arrows); - util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'from', allowDeletion, globalOptions.arrows); + util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'to', globalOptions.arrows); + util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'middle', globalOptions.arrows); + util.mergeOptions(parentOptions.arrows, newOptions.arrows, 'from', globalOptions.arrows); } else { throw new Error("The arrow newOptions can only be an object or a string. Refer to the documentation. You used:" + JSON.stringify(newOptions.arrows)); diff --git a/lib/network/modules/components/Node.js b/lib/network/modules/components/Node.js index b231253e..9683f467 100644 --- a/lib/network/modules/components/Node.js +++ b/lib/network/modules/components/Node.js @@ -215,7 +215,7 @@ class Node { Node.checkMass(newOptions); // merge the shadow options into the parent. - util.mergeOptions(parentOptions, newOptions, 'shadow', allowDeletion, globalOptions); + util.mergeOptions(parentOptions, newOptions, 'shadow', globalOptions); // individual shape newOptions if (newOptions.color !== undefined && newOptions.color !== null) { @@ -252,7 +252,7 @@ class Node { // handle the scaling options, specifically the label part if (newOptions.scaling !== undefined) { - util.mergeOptions(parentOptions.scaling, newOptions.scaling, 'label', allowDeletion, globalOptions.scaling); + util.mergeOptions(parentOptions.scaling, newOptions.scaling, 'label', globalOptions.scaling); } } diff --git a/lib/util.js b/lib/util.js index e170db3c..5bef259f 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1276,40 +1276,128 @@ exports.insertSort = function (a,compare) { return a; } + /** - * this is used to set the options of subobjects in the options object. A requirement of these subobjects - * is that they have an 'enabled' element which is optional for the user but mandatory for the program. + * This is used to set the options of subobjects in the options object. + * + * A requirement of these subobjects is that they have an 'enabled' element + * which is optional for the user but mandatory for the program. + * + * The added value here of the merge is that option 'enabled' is set as required. + * * - * @param [object] mergeTarget | this is either this.options or the options used for the groups. - * @param [object] options | options - * @param [String] option | this is the option key in the options argument + * @param {object} mergeTarget | either this.options or the options used for the groups. + * @param {object} options | options + * @param {String} option | option key in the options argument + * @param {object} globalOptions | global options, passed in to determine value of option 'enabled' */ -exports.mergeOptions = function (mergeTarget, options, option, allowDeletion = false, globalOptions = {}) { - if (options[option] === null) { - mergeTarget[option] = Object.create(globalOptions[option]); +exports.mergeOptions = function (mergeTarget, options, option, globalOptions = {}) { + // Local helpers + var isPresent = function(obj) { + return obj !== null && obj !== undefined; } - else { - if (options[option] !== undefined) { - if (typeof options[option] === 'boolean') { - mergeTarget[option].enabled = options[option]; - } - else { - if (options[option].enabled === undefined) { - if (globalOptions[option] !== undefined && globalOptions[option].enabled !== undefined) { - mergeTarget[option].enabled = globalOptions[option].enabled; - } else { - // assume this is the correct value - mergeTarget[option].enabled = true; - } - } - for (var prop in options[option]) { - if (options[option].hasOwnProperty(prop)) { - mergeTarget[option][prop] = options[option][prop]; - } - } + + var isObject = function(obj) { + return obj !== null && typeof obj === 'object'; + } + + // https://stackoverflow.com/a/34491287/1223531 + var isEmpty = function(obj) { + for (var x in obj) { if (obj.hasOwnProperty(x)) return false; } + return true; + }; + + // Guards + if (!isObject(mergeTarget)) { + throw new Error('Parameter mergeTarget must be an object'); + } + + if (!isObject(options)) { + throw new Error('Parameter options must be an object'); + } + + if (!isPresent(option)) { + throw new Error('Parameter option must have a value'); + } + + if (!isObject(globalOptions)) { + throw new Error('Parameter globalOptions must be an object'); + } + + + // + // Actual merge routine, separated from main logic + // Only a single level of options is merged. Deeper levels are ref'd. This may actually be an issue. + // + var doMerge = function(target, options, option) { + if (!isObject(target[option])) { + target[option] = {}; + } + + let src = options[option]; + let dst = target[option]; + for (var prop in src) { + if (src.hasOwnProperty(prop)) { + dst[prop] = src[prop]; } } + }; + + + // Local initialization + var srcOption = options[option]; + var globalPassed = isObject(globalOptions) && !isEmpty(globalOptions); + var globalOption = globalPassed? globalOptions[option]: undefined; + var globalEnabled = globalOption? globalOption.enabled: undefined; + + + ///////////////////////////////////////// + // Main routine + ///////////////////////////////////////// + if (srcOption === undefined) { + return; // Nothing to do + } + + + if ((typeof srcOption) === 'boolean') { + if (!isObject(mergeTarget[option])) { + mergeTarget[option] = {}; + } + + mergeTarget[option].enabled = srcOption; + return; + } + + if (srcOption === null && !isObject(mergeTarget[option])) { + // If possible, explicit copy from globals + if (isPresent(globalOption)) { + mergeTarget[option] = Object.create(globalOption); + } else { + return; // Nothing to do + } + } + + if (!isObject(srcOption)) { + return; } + + // + // Ensure that 'enabled' is properly set. It is required internally + // Note that the value from options will always overwrite the existing value + // + let enabled = true; // default value + + if (srcOption.enabled !== undefined) { + enabled = srcOption.enabled; + } else { + // Take from globals, if present + if (globalEnabled !== undefined) { + enabled = globalOption.enabled; + } + } + + doMerge(mergeTarget, options, option); + mergeTarget[option].enabled = enabled; } diff --git a/test/util.test.js b/test/util.test.js new file mode 100644 index 00000000..0c112b35 --- /dev/null +++ b/test/util.test.js @@ -0,0 +1,134 @@ +var assert = require('assert'); +var util = require('../lib/util'); + + +// +// The important thing with mergeOptions() is that 'enabled' is always set in target option. +// +describe('util.mergeOptions', function () { + + it('handles good input without global options', function () { + var options = { + someValue: "silly value", + aBoolOption: false, + anObject: { + answer:42 + }, + anotherObject: { + enabled: false, + }, + merge: null + }; + + // Case with empty target + var mergeTarget = {}; + + util.mergeOptions(mergeTarget, options, 'someValue'); + assert(mergeTarget.someValue === undefined, 'Non-object option should not be copied'); + assert(mergeTarget.anObject === undefined); + + util.mergeOptions(mergeTarget, options, 'aBoolOption'); + assert(mergeTarget.aBoolOption !== undefined, 'option aBoolOption should now be an object'); + assert(mergeTarget.aBoolOption.enabled === false, 'enabled value option aBoolOption should have been copied into object'); + + util.mergeOptions(mergeTarget, options, 'anObject'); + assert(mergeTarget.anObject !== undefined, 'Option object is not copied'); + assert(mergeTarget.anObject.answer === 42); + assert(mergeTarget.anObject.enabled === true); + + util.mergeOptions(mergeTarget, options, 'anotherObject'); + assert(mergeTarget.anotherObject.enabled === false, 'enabled value from options must have priority'); + + util.mergeOptions(mergeTarget, options, 'merge'); + assert(mergeTarget.merge === undefined, 'Explicit null option should not be copied, there is no global option for it'); + + // Case with non-empty target + mergeTarget = { + someValue: false, + aBoolOption: true, + anObject: { + answer: 49 + }, + anotherObject: { + enabled: true, + }, + merge: 'hello' + }; + + util.mergeOptions(mergeTarget, options, 'someValue'); + assert(mergeTarget.someValue === false, 'Non-object option should not be copied'); + assert(mergeTarget.anObject.answer === 49, 'Sibling option should not be changed'); + + util.mergeOptions(mergeTarget, options, 'aBoolOption'); + assert(mergeTarget.aBoolOption !== true, 'option enabled should have been overwritten'); + assert(mergeTarget.aBoolOption.enabled === false, 'enabled value option aBoolOption should have been copied into object'); + + util.mergeOptions(mergeTarget, options, 'anObject'); + assert(mergeTarget.anObject.answer === 42); + assert(mergeTarget.anObject.enabled === true); + + util.mergeOptions(mergeTarget, options, 'anotherObject'); + assert(mergeTarget.anotherObject !== undefined, 'Option object is not copied'); + assert(mergeTarget.anotherObject.enabled === false, 'enabled value from options must have priority'); + + util.mergeOptions(mergeTarget, options, 'merge'); + assert(mergeTarget.merge === 'hello', 'Explicit null-option should not be copied, already present in target'); + }); + + + it('gracefully handles bad input', function () { + var mergeTarget = {}; + var options = { + merge: null + }; + + var errMsg = 'Non-object parameters should not be accepted'; + assert.throws(() => util.mergeOptions(null, options, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(undefined, options, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(42, options, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, null, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, undefined, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, 42, 'anything'), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, options, null), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, options, undefined), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, options, 'anything', null), Error, errMsg); + assert.throws(() => util.mergeOptions(mergeTarget, options, 'anything', 'not an object'), Error, errMsg); + + + util.mergeOptions(mergeTarget, options, 'iDontExist'); + assert(mergeTarget.iDontExist === undefined); + }); + + + it('handles good input with global options', function () { + var mergeTarget = { + }; + var options = { + merge: null, + missingEnabled: { + answer: 42 + }, + alsoMissingEnabled: { // has no enabled in globals + answer: 42 + } + }; + + var globalOptions = { + merge: { + enabled: false + }, + missingEnabled: { + enabled: false + } + }; + + util.mergeOptions(mergeTarget, options, 'merge', globalOptions); + assert(mergeTarget.merge.enabled === false, "null-option should create an empty target object"); + + util.mergeOptions(mergeTarget, options, 'missingEnabled', globalOptions); + assert(mergeTarget.missingEnabled.enabled === false); + + util.mergeOptions(mergeTarget, options, 'alsoMissingEnabled', globalOptions); + assert(mergeTarget.alsoMissingEnabled.enabled === true); + }); +});