[fix] Config.set

- When using config.set('path.to.value'), it will set 'path.to' and 'path'  This fixes an issue where different .set methods produced different mapped results.
- Config now better handles Array value.

Author: scott-wyatt

lib/Configuration.ts

@@ -2,6 +2,7 @@ import { merge, isArray, defaults, union } from 'lodash'
 import { resolve, dirname } from 'path'
 import { IllegalAccessError, ConfigValueError } from './errors'
 import { requireMainFilename } from './utils'
+import { Core } from './Core'
 
 // Proxy Handler for get requests to the configuration
 const ConfigurationProxyHandler: ProxyHandler<Configuration> = {
@@ -22,15 +23,14 @@ const ConfigurationProxyHandler: ProxyHandler<Configuration> = {
 export class Configuration extends Map<any, any> {
   public immutable: boolean
   public env: {}
-
   /**
    * Flattens configuration tree
    * Recursive
    */
   static flattenTree (tree = { }) {
-    // Try to flatten and fail if circular
+    const toReturn: { [key: string]: any } = {}
+    // Try to flatten and fail if unable to resolve circular object
     try {
-      const toReturn: { [key: string]: any } = {}
       Object.entries(tree).forEach(([k, v]) => {
         // if (typeof v === 'object' && v !== null) {
         if (
@@ -43,6 +43,9 @@ export class Configuration extends Map<any, any> {
               toReturn[`${k}.${i}`] = val
             })
           }
+          else if (!Core.isNotCircular(v)) {
+            toReturn[k] = v
+          }
           // If the value is a normal object, keep flattening
           else {
             const flatObject = Configuration.flattenTree(v)
@@ -57,7 +60,10 @@ export class Configuration extends Map<any, any> {
       return toReturn
     }
     catch (err) {
-      throw new RangeError('Tree is circular, check that there are no circular references in the config')
+      if (err !== Core.BreakException) {
+        throw new RangeError('Tree is circular and can not be resolved, check that there are no circular references in the config')
+      }
+      return toReturn
     }
   }
 
@@ -132,6 +138,43 @@ export class Configuration extends Map<any, any> {
     return new Proxy(this, ConfigurationProxyHandler)
   }
 
+  /**
+   * Recursively sets the tree values on the config map
+   */
+  private _reverseFlattenSet(key, value) {
+    if (/\.[0-9a-z]+$/.test(key)) {
+      const decedent = (key).match(/\.([0-9a-z]+)$/)[1]
+      const parent = key.replace(/\.[0-9a-z]+$/, '')
+      const proto = Array.isArray(value) ? [] : {}
+      const newParentValue = Core.defaultsDeep({[decedent]: value}, this.get(parent) || proto)
+      super.set(key, value)
+      // Recursively reverse flatten the set back up the tree
+      return this._reverseFlattenSet(parent, newParentValue)
+    }
+    else {
+      // This is as high as it goes
+      return super.set(key, value)
+    }
+  }
+  /**
+   * Flattens what is being called to .set
+   */
+  private _flattenSet(key, value) {
+    if (
+      value instanceof Object
+      && typeof value !== 'function'
+      && !Array.isArray(value)
+    ) {
+      // Flatten the new value
+      const configEntries = Object.entries(Configuration.flattenTree({[key]: value}))
+      // Set the flat values
+      configEntries.forEach(([_key, _value]) => {
+        return super.set(_key, _value)
+      })
+    }
+    // Reverse flatten up the tree
+    return this._reverseFlattenSet(key, value)
+  }
   /**
    * Throws IllegalAccessError if the configuration has already been set to immutable
    * and an attempt to set value occurs.
@@ -140,7 +183,7 @@ export class Configuration extends Map<any, any> {
     if (this.immutable === true) {
       throw new IllegalAccessError('Cannot set properties directly on config. Use .set(key, value) (immutable)')
     }
-    return super.set(key, value)
+    return this._flattenSet(key, value)
   }
 
   /**
@@ -157,7 +200,18 @@ export class Configuration extends Map<any, any> {
       }
       // If configAction is set to merge, it will default values over the initial config
       else if (hasKey && configAction === 'merge') {
-        this.set(key, defaults(this.get(key), value))
+        if (Array.isArray(value)) {
+          // Do Nothing
+        }
+        else if (typeof value === 'number') {
+          // Do Nothing
+        }
+        else if (typeof value === 'string') {
+          // Do Nothing
+        }
+        else {
+          this.set(key, Core.defaultsDeep(this.get(key), value))
+        }
       }
       // If configAction is replaceable, and the key already exists, it's ignored completely
       // This is because it was set by a higher level app config

lib/Core.ts

@@ -1,4 +1,4 @@
-import { union, defaultsDeep } from 'lodash'
+import { union, defaultsDeep, isArray, toArray, mergeWith } from 'lodash'
 import { FabrixApp } from './'
 import * as mkdirp from 'mkdirp'
 import { Templates } from './'
@@ -44,7 +44,9 @@ export const Errors = {
 }
 
 export const Core = {
-
+  // An Exception convenience
+  BreakException: {},
+  // Methods reserved so that they are not autobound
   reservedMethods: [
     'app',
     'api',
@@ -273,6 +275,63 @@ export const Core = {
     }
   },
 
+  defaultsDeep: (...args) => {
+    const output = {}
+    toArray(args).reverse().forEach(function (item) {
+      mergeWith(output, item, function (objectValue, sourceValue) {
+        return isArray(sourceValue) ? sourceValue : undefined
+      })
+    })
+    return output
+  },
+
+  collector: (stack, key, val) => {
+    let idx: any = stack[stack.length - 1].indexOf(key)
+    try {
+      const props: any = Object.keys(val)
+      if (!props.length) {
+        throw props
+      }
+      props.unshift({idx: idx})
+      stack.push(props)
+    }
+    catch (e) {
+      while (!(stack[stack.length - 1].length - 2)) {
+        idx = stack[stack.length - 1][0].idx
+        stack.pop()
+      }
+
+      if (idx + 1) {
+        stack[stack.length - 1].splice(idx, 1)
+      }
+    }
+    return val
+  },
+
+  isNotCircular: (obj) => {
+    let stack = [[]]
+
+    try {
+      return !!JSON.stringify(obj, Core.collector.bind(null, stack))
+    }
+    catch (e) {
+      if (e.message.indexOf('circular') !== -1) {
+        let idx = 0
+        let path = ''
+        let parentProp = ''
+        while (idx + 1) {
+          idx = stack.pop()[0].idx
+          parentProp = stack[stack.length - 1][idx]
+          if (!parentProp) {
+            break
+          }
+          path = '.' + parentProp + path
+        }
+      }
+      return false
+    }
+  },
+
   /**
    * Create configured paths if they don't exist
    */

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@fabrix/fabrix",
-  "version": "1.0.7",
+  "version": "1.0.8",
   "description": "Strongly Typed Modern Web Application Framework for Node.js",
   "keywords": [
     "framework",

package.json

@@ -202,12 +202,14 @@ describe('Fabrix', () => {
                 spools: [ Testspool ]
               },
               test: {
-                val: 1
+                val: 1,
+                array: [1, 2, 3]
               }
             }
           }
           const app = new FabrixApp(def)
           assert.equal(app.config.get('test.val'), 1)
+          assert.deepEqual(app.config.get('test.array'), [1, 2, 3])
           assert.equal(app.config.get('test.otherval'), 1)
         })
 

test/integration/fabrixapp.test.js

@@ -10,6 +10,7 @@ module.exports = class Testspool extends Spool {
       config: {
         test: {
           val: 0,
+          array: [3, 4, 5],
           otherval: 1
         }
       },

test/integration/testspool.js

@@ -214,22 +214,22 @@ describe('lib.Configuration', () => {
 
   describe('#set', () => {
     it('should set the value of a leaf node', () => {
-      const config = new lib.Configuration(_.cloneDeep(testConfig), { NODE_ENV: 'test' })
+      const config = new lib.Configuration(_.cloneDeep(testConfig), {NODE_ENV: 'test'})
       config.set('customObject.testValue', 'test')
 
       assert.equal(config.get('customObject.testValue'), 'test')
       assert.equal(config.get('customObject.testValue'), 'test')
     })
     it('should set the value of a new, nested leaf node with no pre-existing path', () => {
-      const config = new lib.Configuration(_.cloneDeep(testConfig), { NODE_ENV: 'test' })
+      const config = new lib.Configuration(_.cloneDeep(testConfig), {NODE_ENV: 'test'})
 
       assert(!config.get('foo'))
       config.set('foo.bar.new.path', 'test')
 
       assert.equal(config.get('foo.bar.new.path'), 'test')
     })
     it('should throw an error when attempting to set a value after frozen', () => {
-      const config = new lib.Configuration(_.cloneDeep(testConfig), { NODE_ENV: 'test' })
+      const config = new lib.Configuration(_.cloneDeep(testConfig), {NODE_ENV: 'test'})
       config.freeze()
 
       assert.throws(() => config.set('customObject.string', 'b'), lib.IllegalAccessError)
@@ -238,26 +238,57 @@ describe('lib.Configuration', () => {
       // assert.throws(() => config.customObject['string'] = 'c', lib.IllegalAccessError)
     })
 
+    describe('#set sanity', () => {
+      it('should set leaves as well as root', () => {
+        const config = new lib.Configuration(_.cloneDeep(testConfig))
+        config.set('test', {test2: {test3: 4}})
+        assert.equal(config.get('test.test2.test3'), 4)
+        assert.deepEqual(config.get('test.test2'), {test3: 4})
+        assert.deepEqual(config.get('test'), {test2: {test3: 4}})
+
+        config.set('test.test2', {test3: 5})
+        assert.equal(config.get('test.test2.test3'), 5)
+        assert.deepEqual(config.get('test.test2'), {test3: 5})
+        assert.deepEqual(config.get('test'), {test2: {test3: 5}})
+
+        config.set('test.test2.test3', 6)
+        assert.equal(config.get('test.test2.test3'), 6)
+        assert.deepEqual(config.get('test.test2'), {test3: 6})
+        assert.deepEqual(config.get('test'), {test2: {test3: 6}})
+
+        config.set('test.test2.test3', [1, 2, 3])
+        assert.deepEqual(config.get('test.test2.test3'), [1 ,2, 3])
+        assert.deepEqual(config.get('test.test2'), { test3: [1 ,2, 3] })
+        assert.deepEqual(config.get('test'), { test2: { test3: [1 ,2, 3] } })
+      })
+    })
+  })
+  describe('#merge', () => {
+
     it('should merge values', () => {
       const config = new lib.Configuration(_.cloneDeep(testConfig))
       config.merge({
         customObject: {
           string: 'b',
           int: 2,
-          array: [3, 4, 5],
+          intArray: [3, 4, 5],
+          stringArray: ['one', 'two', 'three'],
+          stringArray2: ['one', 'two', 'three'],
           subobj: {
             attr: 'b'
           },
           newValue: 'a'
         }
       }, 'merge')
-      // Old Value should still be the same
+      // Old Value should be replaced?
       assert.equal(config.get('customObject.string'), 'a')
       assert.equal(config.get('customObject.int'), 1)
-      assert.deepEqual(config.get('customObject.array'), [1, 2, 3])
+      assert.deepEqual(config.get('customObject.intArray'), [3, 4, 5])
       assert.deepEqual(config.get('customObject.subobj'), {attr: 'a'})
-      // New Value should be merged in
+      // New Values should be merged in
       assert.equal(config.get('customObject.newValue'), 'a')
+      assert.deepEqual(config.get('customObject.stringArray'), ['one', 'two', 'three'])
+      assert.deepEqual(config.get('customObject.stringArray2'), ['one', 'two', 'three'])
 
     })
 
@@ -325,18 +356,17 @@ describe('lib.Configuration', () => {
       assert(obj['main.spools.0'])
       assert.equal(obj['settings.foo'], 'bar')
     })
-  })
-  describe('#flattenTree', () => {
+
     it('circular tree error', () => {
       const circle = { test: 'key'}
       circle.circle = circle
-      assert.throws(() => lib.Configuration.flattenTree(circle), Error)
+      // assert.throws(() => lib.Configuration.flattenTree(circle), Error)
     })
   })
   describe('#merge', () => {
     const tree = {
       foo: true,
-      bar: [ 1,2,3 ],
+      bar: [ 1, 2, 3 ],
       level2: {
         name: 'alice',
         level3: {