From 110df593e0e2c74b5660e0a22016a1672506a39f Mon Sep 17 00:00:00 2001 From: Nils Lundquist Date: Thu, 11 Mar 2021 14:07:40 -0700 Subject: [PATCH 1/2] during merge of alternative match results only merge changed properties into value --- lib/types/alternatives.js | 90 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/lib/types/alternatives.js b/lib/types/alternatives.js index 875f2b090..207c67ece 100755 --- a/lib/types/alternatives.js +++ b/lib/types/alternatives.js @@ -2,6 +2,8 @@ const Assert = require('@hapi/hoek/lib/assert'); const Merge = require('@hapi/hoek/lib/merge'); +const Clone = require('@hapi/hoek/lib/clone'); +const Utils = require('@hapi/hoek/lib/utils'); const Any = require('./any'); const Common = require('../common'); @@ -12,6 +14,78 @@ const Ref = require('../ref'); const internals = {}; +// note: placed here purely for illustration purposes. will move to appropriate location before merging +// merges source properties into target, but only when they differ from the value in original +function differenceMerge(target, source, original, options) { + Assert(target && typeof target === 'object', 'Invalid target value: must be an object'); + Assert(source === null || source === undefined || typeof source === 'object', 'Invalid source value: must be null, undefined, or an object'); + Assert(original && typeof original === 'object', 'Invalid original value: must be an object'); + + if (!source) { + return target; + } + + options = Object.assign({ nullOverride: true, mergeArrays: true }, options); + + if (Array.isArray(source)) { + Assert(Array.isArray(target), 'Cannot merge array onto an object'); + if (!options.mergeArrays) { + target.length = 0; // Must not change target assignment + } + + for (let i = 0; i < source.length; ++i) { + target.push(Clone(source[i], { symbols: options.symbols })); + } + + return target; + } + + const keys = Utils.keys(source, options); + for (let i = 0; i < keys.length; ++i) { + const key = keys[i]; + if (key === '__proto__' || + !Object.prototype.propertyIsEnumerable.call(source, key)) { + + continue; + } + + const value = source[key]; + if (value && + typeof value === 'object') { + + if (target[key] === value) { + continue; // Can occur for shallow merges + } + + if (!target[key] || + typeof target[key] !== 'object' || + (Array.isArray(target[key]) !== Array.isArray(value)) || + value instanceof Date || + (Buffer && Buffer.isBuffer(value)) || // $lab:coverage:ignore$ + value instanceof RegExp) { + if (value !== original[key]) { + target[key] = Clone(value, { symbols: options.symbols }); + } + } + else { + differenceMerge(target[key], value, original[key], options); + } + } + else { + if (value !== original[key]) { + if (value !== null && + value !== undefined) { // Explicit to preserve empty strings + target[key] = value; + } + else if (options.nullOverride) { + target[key] = value; + } + } + } + } + + return target; +}; module.exports = Any.extend({ @@ -45,6 +119,7 @@ module.exports = Any.extend({ // Match all or one if (schema._flags.match) { + const originalValue = Clone(value) const matched = []; for (let i = 0; i < schema.$_terms.matches.length; ++i) { @@ -74,7 +149,14 @@ module.exports = Any.extend({ } const allobj = schema.$_terms.matches.reduce((acc, v) => acc && v.schema.type === 'object', true); - return allobj ? { value: matched.reduce((acc, v) => Merge(acc, v, { mergeArrays: false })) } : { value: matched[matched.length - 1] }; + + if (allobj) { + // add original value to set prior to merging any changed properties from matched subschemas + matched.unshift(value) + return { value: matched.reduce((acc, v) => differenceMerge(acc, v, originalValue, { mergeArrays: false })) } + } + + return { value: matched[matched.length - 1] } } // Match any @@ -138,7 +220,7 @@ module.exports = Any.extend({ const conditions = match.is ? [match] : match.switch; for (const item of conditions) { if (item.then && - item.otherwise) { + item.otherwise) { obj.$_setFlag('_endedSwitch', true, { clone: false }); break; @@ -200,7 +282,7 @@ module.exports = Any.extend({ const each = (item) => { if (Common.isSchema(item) && - item.type === 'array') { + item.type === 'array') { schema.$_setFlag('_arrayItems', true, { clone: false }); } @@ -330,4 +412,4 @@ internals.unmatched = function (failures, error) { } return { errors: error('alternatives.match', Errors.details(errors, { override: false })) }; -}; +}; \ No newline at end of file From d6b3cf47b7995c60b7a493cdf79f166f785f0dc0 Mon Sep 17 00:00:00 2001 From: Nils Lundquist Date: Thu, 11 Mar 2021 18:13:48 -0700 Subject: [PATCH 2/2] fix lint issues and prevent modification of input value --- lib/types/alternatives.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/types/alternatives.js b/lib/types/alternatives.js index 207c67ece..aca726b83 100755 --- a/lib/types/alternatives.js +++ b/lib/types/alternatives.js @@ -1,7 +1,6 @@ 'use strict'; const Assert = require('@hapi/hoek/lib/assert'); -const Merge = require('@hapi/hoek/lib/merge'); const Clone = require('@hapi/hoek/lib/clone'); const Utils = require('@hapi/hoek/lib/utils'); @@ -16,7 +15,8 @@ const internals = {}; // note: placed here purely for illustration purposes. will move to appropriate location before merging // merges source properties into target, but only when they differ from the value in original -function differenceMerge(target, source, original, options) { +const differenceMerge = function (target, source, original, options) { + Assert(target && typeof target === 'object', 'Invalid target value: must be an object'); Assert(source === null || source === undefined || typeof source === 'object', 'Invalid source value: must be null, undefined, or an object'); Assert(original && typeof original === 'object', 'Invalid original value: must be an object'); @@ -85,6 +85,7 @@ function differenceMerge(target, source, original, options) { } return target; + }; module.exports = Any.extend({ @@ -119,7 +120,7 @@ module.exports = Any.extend({ // Match all or one if (schema._flags.match) { - const originalValue = Clone(value) + const valueClone = Clone(value); const matched = []; for (let i = 0; i < schema.$_terms.matches.length; ++i) { @@ -152,11 +153,11 @@ module.exports = Any.extend({ if (allobj) { // add original value to set prior to merging any changed properties from matched subschemas - matched.unshift(value) - return { value: matched.reduce((acc, v) => differenceMerge(acc, v, originalValue, { mergeArrays: false })) } + matched.unshift(valueClone); + return { value: matched.reduce((acc, v) => differenceMerge(acc, v, value, { mergeArrays: false })) }; } - return { value: matched[matched.length - 1] } + return { value: matched[matched.length - 1] }; } // Match any @@ -412,4 +413,4 @@ internals.unmatched = function (failures, error) { } return { errors: error('alternatives.match', Errors.details(errors, { override: false })) }; -}; \ No newline at end of file +};