From 3008ffe6da80bc89106f485d0d10687c53326184 Mon Sep 17 00:00:00 2001 From: Roberto Sebestyen Date: Wed, 11 Dec 2019 18:52:10 -0500 Subject: [PATCH] keep properties and values when merging objects with conflicting properties --- .prettierrc | 2 +- src/error-with-context.ts | 9 +++-- src/logger.ts | 32 +++++---------- src/safe-object-assign.ts | 83 ++++++++++++++++++++++++++++---------- test/logger.test.ts | 29 +++++++++---- test/merge-objects.test.ts | 48 +++++++++++++++++++++- 6 files changed, 146 insertions(+), 57 deletions(-) diff --git a/.prettierrc b/.prettierrc index 4ddba9a..91d9ce2 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,5 +1,5 @@ { - "printWidth": 120, + "printWidth": 180, "trailingComma": "all", "singleQuote": true } \ No newline at end of file diff --git a/src/error-with-context.ts b/src/error-with-context.ts index 2df516a..4df17f6 100644 --- a/src/error-with-context.ts +++ b/src/error-with-context.ts @@ -1,4 +1,5 @@ import { CaptureNestedStackTrace } from './capture-nested-stack-trace'; +import { safeObjectAssign } from './safe-object-assign'; export class ErrorWithContext extends Error { constructor(error: Error | string, extraContext: { [_: string]: any } = {}) { @@ -25,16 +26,16 @@ export class ErrorWithContext extends Error { if (typeof (error as any).extraContext === 'string') { // noinspection SuspiciousTypeOfGuard if (typeof extraContext === 'string') { - (this as any).extraContext = { ...{ message: (error as any).extraContext }, ...{ message2: extraContext } }; + (this as any).extraContext = safeObjectAssign({ message: (error as any).extraContext }, [], { message2: extraContext }); } else { - (this as any).extraContext = { ...{ message: (error as any).extraContext }, ...extraContext }; + (this as any).extraContext = safeObjectAssign({ message: (error as any).extraContext }, [], extraContext); } } else { // noinspection SuspiciousTypeOfGuard if (typeof extraContext === 'string') { - (this as any).extraContext = { ...(error as any).extraContext, ...{ message: extraContext } }; + (this as any).extraContext = safeObjectAssign((error as any).extraContext, [], { message: extraContext }); } else { - (this as any).extraContext = { ...(error as any).extraContext, ...extraContext }; + (this as any).extraContext = safeObjectAssign((error as any).extraContext, [], extraContext); } } } diff --git a/src/logger.ts b/src/logger.ts index d847be1..61b446d 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -5,6 +5,7 @@ import { ErrorWithContext } from './error-with-context'; import { FormatStackTrace } from './format-stack-trace'; import { getCallStack } from './get-call-stack'; import { getCallingFilename } from './get-calling-filename'; +import { safeObjectAssign } from './safe-object-assign'; import { sortObject } from './sort-object'; import { ToOneLine } from './to-one-line'; @@ -101,16 +102,16 @@ export function FormatErrorObject(object: any) { // Flatten message if it is an object if (typeof object.message === 'object') { - const messageObj = object.message; - delete returnData.message; - returnData = Object.assign(returnData, messageObj); + // const messageObj = object.message; + // delete returnData.message; + returnData = safeObjectAssign(returnData, ['message'], object.message); } // Combine extra context from ErrorWithContext if (object.extraContext) { const extraContext = object.extraContext; delete returnData.extraContext; - returnData = Object.assign(returnData, extraContext); + returnData = safeObjectAssign(returnData, ['message'], extraContext); } // Add stack trace if available @@ -119,7 +120,7 @@ export function FormatErrorObject(object: any) { const stackOneLine = FormatStackTrace.toNewLines(ToOneLine(stack)); delete returnData.stack; delete returnData.errCallStack; - returnData = Object.assign(returnData, { errCallStack: stackOneLine }); + returnData = safeObjectAssign(returnData, ['message'], { errCallStack: stackOneLine }); returnData.level = 'error'; // Lets put a space into the message when stack message exists @@ -219,16 +220,12 @@ export function NativeConsoleLog(...args: any[]) { function ifEverythingFailsLogger(functionName: string, err: Error) { if (consoleErrorBackup != null) { try { - consoleErrorBackup( - `{"level":"error","message":"Error: console-log-json: error while trying to process ${functionName} : ${err.message}"}`, - ); + consoleErrorBackup(`{"level":"error","message":"Error: console-log-json: error while trying to process ${functionName} : ${err.message}"}`); } catch (err) { throw new Error(`Failed to call ${functionName} and failed to fall back to native function`); } } else { - throw new Error( - 'Error: console-log-json: This is unexpected, there is no where to call console.log, this should never happen', - ); + throw new Error('Error: console-log-json: This is unexpected, there is no where to call console.log, this should never happen'); } } @@ -323,14 +320,7 @@ function filterNullOrUndefinedParameters(args: any): number { function findExplicitLogLevelAndUseIt(args: any, level: LOG_LEVEL) { let foundLevel = false; args.forEach((f: any) => { - if ( - !foundLevel && - f && - typeof f === 'object' && - Object.keys(f) && - Object.keys(f).length > 0 && - Object.keys(f)[0].toLowerCase() === 'level' - ) { + if (!foundLevel && f && typeof f === 'object' && Object.keys(f) && Object.keys(f).length > 0 && Object.keys(f)[0].toLowerCase() === 'level') { let specifiedLevelFromParameters: string = f[Object.keys(f)[0]]; // Normalize alternate log level strings @@ -506,7 +496,7 @@ function extractParametersFromArguments(args: any[]) { if (extraContext == null) { extraContext = f; } else { - extraContext = { ...extraContext, ...f }; + extraContext = safeObjectAssign(extraContext, ['message'], f); } } }); @@ -525,7 +515,7 @@ function extractParametersFromArguments(args: any[]) { // noinspection JSUnusedAssignment if (errorObject.name != null && errorObject.name.length > 0) { // noinspection JSUnusedAssignment - extraContext = { ...extraContext, ...{ '@errorObjectName': errorObject.name } }; + extraContext = safeObjectAssign(extraContext, ['message'], { '@errorObjectName': errorObject.name }); } // noinspection JSUnusedAssignment errorObject = new ErrorWithContext(errorObject, extraContext); diff --git a/src/safe-object-assign.ts b/src/safe-object-assign.ts index c892bef..14fbb3e 100644 --- a/src/safe-object-assign.ts +++ b/src/safe-object-assign.ts @@ -1,34 +1,71 @@ +import stringify from 'json-stringify-safe'; import { sortObject } from './sort-object'; +// tslint:disable-next-line:no-var-requires +/* tslint:disable:only-arrow-functions */ -export function mergeDeepSafe(target: any, ...sources: any): any { - if (!sources.length) { - return target; - } - const source = sources.shift(); +/** + * Safe deep merge two objects by handling circular references and conflicts + * + * in case of conflicting property, it will be merged with a modified property by adding a prefix + * @param target + * @param mergeStringProperties + * @param sources + */ +export function safeObjectAssign(target: any, mergeStringProperties: string[], ...sources: any): any { + const traversedProps = new Set(); + + function mergeDeep(theTarget: any, ...theSources: any): any { + if (!theSources.length) { + return theTarget; + } + let source = theSources.shift(); - if (isObject(target) && isObject(source)) { - for (const key in source) { - // noinspection JSUnfilteredForInLoop - if (isObject(source[key])) { + if (traversedProps.has(source)) { + source = { circular: 'circular' }; + } + traversedProps.add(source); + + if (isObject(theTarget) && isObject(source)) { + for (const key in source) { // noinspection JSUnfilteredForInLoop - if (!target[key]) { + if (isObject(source[key])) { + // noinspection JSUnfilteredForInLoop + if (!theTarget[key]) { + // noinspection JSUnfilteredForInLoop + Object.assign(theTarget, { [key]: {} }); + theTarget = sortObject(theTarget); + } // noinspection JSUnfilteredForInLoop - Object.assign(target, { [key]: {} }); - target = sortObject(target); + mergeDeep(theTarget[key], source[key]); + } else { + const targetMatchedKey = Object.keys(target).find(k => k.toLowerCase() === key); + if ( + targetMatchedKey != null && + mergeStringProperties != null && + mergeStringProperties.includes(targetMatchedKey) && + typeof theTarget[targetMatchedKey] === 'string' && + typeof source[targetMatchedKey] === 'string' + ) { + // merge the two strings together + theTarget[targetMatchedKey] = `${theTarget[targetMatchedKey]} - ${source[targetMatchedKey]}`; + } else { + // noinspection JSUnfilteredForInLoop + const targetKey = findNonConflictingKeyInTarget(theTarget, key); + // noinspection JSUnfilteredForInLoop + Object.assign(theTarget, { [targetKey]: source[key] }); + } + theTarget = sortObject(theTarget); } - // noinspection JSUnfilteredForInLoop - mergeDeepSafe(target[key], source[key]); - } else { - // noinspection JSUnfilteredForInLoop - const targetKey = findNonConflictingKeyInTarget(target, key); - // noinspection JSUnfilteredForInLoop - Object.assign(target, { [targetKey]: source[key] }); - target = sortObject(target); } } + + return mergeDeep(theTarget, ...theSources); } - return mergeDeepSafe(target, ...sources); + const targetCopy = JSON.parse(stringify(target)); + const sourcesCopy = JSON.parse(stringify(sources)); + + return mergeDeep(targetCopy, ...sourcesCopy); } function isObject(item: any) { @@ -38,8 +75,10 @@ function isObject(item: any) { function findNonConflictingKeyInTarget(target: any, key: string): string { const targetContainsKey = Object.keys(target).find(k => k.toLowerCase() === key); if (targetContainsKey != null) { - return findNonConflictingKeyInTarget(target, `_${key}`); + return findNonConflictingKeyInTarget(target, `${conflictResolutionPrefix}${key}`); } else { return key; } } + +const conflictResolutionPrefix = '_'; diff --git a/test/logger.test.ts b/test/logger.test.ts index d7e4626..d70f902 100644 --- a/test/logger.test.ts +++ b/test/logger.test.ts @@ -104,6 +104,7 @@ describe('logger', () => { } // assert + console.log(outputText[0]); expect(outputText[0]).contains('some outer error - this is the inner error 1234'); }); @@ -116,6 +117,7 @@ describe('logger', () => { const formatted = FormatErrorObject(sut); // assert + console.log(formatted); expect(formatted).contains('"contextInner":"dataInner"'); expect(formatted).contains('"contextForOuterError":"dataOuter"'); expect(formatted).contains('inner error 1234'); @@ -302,8 +304,8 @@ describe('logger', () => { console.log(outputText[0]); expect(JSON.parse(outputText[0]).level).eql("info"); - expect(JSON.parse(outputText[0]).circ.bob).eql("bob"); - expect(JSON.parse(outputText[0]).circ.circ).eql("[Circular ~.circ]"); + expect(JSON.parse(outputText[0]).bob).eql("bob"); + expect(JSON.parse(outputText[0]).circ).eql("[Circular ~]"); }); it('Handle where a string is passed to the logger that happens to be JSON, with new lines in it', async () => { @@ -515,22 +517,28 @@ describe('logger', () => { expect(testObj.message).eql("error-message"); }); - it('log errors with self referencing properties', async () => { + it('log works with self referencing properties', async () => { + // arrange const {originalWrite, outputText} = overrideStdOut(); LoggerAdaptToConsole(); + // action 1 const err1 = new Error('Error1'); (err1 as any).self = err1; console.log(err1); + // action 2 const objSelf:any = {"name": "objSelf"}; objSelf.self = objSelf; const err2 = new ErrorWithContext('Error2', objSelf); console.log(err2); + + // cleanup restoreStdOut(originalWrite); LoggerRestoreConsole(); + // assert outputText.forEach(l=>{ console.log(l); }); @@ -538,7 +546,7 @@ describe('logger', () => { expect(testObj.level).eql("error"); expect(testObj["@filename"]).include("/test/logger.test"); expect(testObj.message).eql("Error2"); - expect(testObj.self.self).eql("[Circular ~.self]"); + expect(testObj.self.self).eql(undefined); }); it('handle scenario where non traditional error object is passed', async () => { @@ -625,24 +633,31 @@ describe('logger', () => { }); it('logging in a different order produces same result', async () => { + // arrange const {originalWrite, outputText} = overrideStdOut(); LoggerAdaptToConsole(); const extraInfo1 = {firstName: 'homer', lastName: 'simpson'}; const extraInfo2 = {age: 25, location: 'mars'}; + + // action1 console.log(extraInfo1, 'hello world', extraInfo2); + // action2 console.log('hello world', extraInfo2, extraInfo1); + // cleanup restoreStdOut(originalWrite); LoggerRestoreConsole(); + // assert + console.log(outputText[0]); + console.log(outputText[1]); + outputText[0] = stripTimeStamp(outputText[0]); - outputText[0] = stripProperty(outputText[0], "@logCallStack"); outputText[1] = stripTimeStamp(outputText[1]); + outputText[0] = stripProperty(outputText[0], "@logCallStack"); outputText[1] = stripProperty(outputText[1], "@logCallStack"); - console.log(outputText[0]); - console.log(outputText[1]); expect(outputText[0]).equal(outputText[1]) }); diff --git a/test/merge-objects.test.ts b/test/merge-objects.test.ts index ec69a22..db2ef95 100644 --- a/test/merge-objects.test.ts +++ b/test/merge-objects.test.ts @@ -1,6 +1,6 @@ /* tslint:disable:only-arrow-functions */ import {expect} from 'chai' -import {mergeDeepSafe} from "../src"; +import {safeObjectAssign} from "../src"; describe('merge objects', function () { @@ -10,7 +10,7 @@ describe('merge objects', function () { const obj3 = {name: "Szekely"}; - const result = mergeDeepSafe(obj1, obj2, obj3); + const result = safeObjectAssign(obj1,[],obj2, obj3); console.log(result); expect(result).eql({ @@ -22,5 +22,49 @@ describe('merge objects', function () { one: 'one', two: 'two', }); + }); + + it('merges string values of objects with conflicting properties where specified', function () { + const obj1 = {name: "george", one:"one", last_name:"simpson"}; + const obj2 = {name: "castanza", two:"two", last_name:"arnold"}; + const obj3 = {name: "Szekely"}; + + + const result = safeObjectAssign(obj1,["last_name"],obj2, obj3); + + console.log(result); + expect(result).eql({ + __name: 'Szekely', + _name: 'castanza', + last_name: 'simpson - arnold', + name: 'george', + one: 'one', + two: 'two' + }); + }); + + it('merges self referencing objects', function () { + + const selfRefObj:any = {name1:"name1", inner: {}, other: {}}; + selfRefObj.self = selfRefObj; + selfRefObj.inner.bob = selfRefObj; + selfRefObj.other.self = "this should not be lost"; + + const obj2: any = {bob:"bob", name1:"name2"}; + + const result = safeObjectAssign(obj2, [], selfRefObj); + + expect(result).eql({ + "_name1": "name1", + "bob": "bob", + "inner": { + "bob": "[Circular ~.0]" + }, + "name1": "name2", + "other": { + "self": "this should not be lost" + }, + "self": "[Circular ~.0]" + }); }) }); \ No newline at end of file