Fix case where multiple sections with same settings could modify shar…

…ed configuration (#48)

* Fix case where multiple sections with same settings could modify shared configuration

* Optimize safe merge of overridden settings

- Add safe option to mergeDeep that prevents mutation from to object
- Use safe option in non-hot code paths

index.js

@@ -126,7 +126,7 @@ Ycb.prototype = {
             config = {};
 
         context = context || {};
-        options = mergeDeep(this.options, options || {});
+        options = mergeDeep(this.options, options || {}, true);
 
         lookupPaths = this._getLookupPaths(context, options);
 
@@ -409,7 +409,8 @@ Ycb.prototype = {
                             this.settings[key] ? (' onto ' + this.settings[key].__ycb_source__) : ''
                         ));
                 }
-                mergeDeep(section, this.settings[key]);
+                // Clone original settings so that we don't override shared settings
+                this.settings[key] = mergeDeep(section, this.settings[key], true);
             }
         }, this);
     },

lib/mergeDeep.js

@@ -1,8 +1,17 @@
 var isA = require('./isA');
 var cloneDeep = require('./cloneDeep');
 
-module.exports = function mergeDeep(from, to) {
+module.exports = function mergeDeep(from, to, safe) {
+    var newTo = to;
     var key;
+    if (safe) {
+        newTo = {};
+        for (key in to) {
+            if (to.hasOwnProperty(key)) {
+                newTo[key] = to[key];
+            }
+        }
+    }
     for (key in from) {
         if (from.hasOwnProperty(key)) {
             // Property in destination object set; update its value.
@@ -13,14 +22,14 @@ module.exports = function mergeDeep(from, to) {
                 } else {
                     mergeToObj = {};
                 }
-                to[key] = mergeDeep(from[key], mergeToObj);
+                newTo[key] = mergeDeep(from[key], mergeToObj, safe);
             } else if (isA(from[key], Array)) {
-                to[key] = cloneDeep(from[key]);
+                newTo[key] = cloneDeep(from[key]);
             } else {
                 // Other literals are copied
-                to[key] = from[key];
+                newTo[key] = from[key];
             }
         }
     }
-    return to;
+    return newTo;
 };

tests/fixtures/overridden-multisetting.json

@@ -0,0 +1,27 @@
+[
+    {
+        "settings": ["master"],
+        "foo": null,
+        "bar": 0,
+        "baz": false,
+        "oof": 4,
+        "rab": 5,
+        "zab": 6,
+        "__ycb_source__": "tests/fixtures/simple-4.json"
+    },
+    {
+        "settings": ["lang:fr,es"],
+        "foo": 1,
+        "bar": 2,
+        "baz": 3,
+        "oof": null,
+        "rab": 0,
+        "zab": false,
+        "__ycb_source__": "tests/fixtures/simple-4.json"
+    },
+    {
+        "settings": ["lang:es"],
+        "foo": 3,
+        "__ycb_source__": "tests/fixtures/simple-4.json"
+    }
+]

tests/unit/index.js

@@ -534,6 +534,34 @@ describe('ycb unit tests', function () {
             }, config);
         });
 
+        it('should not merge matched settings', function () {
+            var bundle,
+                ycb;
+
+            bundle = readFixtureFile('dimensions.json')
+                .concat(readFixtureFile('overridden-multisetting.json'));
+            ycb = new libycb.Ycb(bundle);
+            var config = ycb.read({
+                'lang': 'fr'
+            });
+            assert.equal(config.foo, 1, 'fr should be 1');
+
+            config = ycb.read({
+                'lang': 'es'
+            });
+            assert.equal(config.foo, 3, 'es should be 3');
+
+            config = ycb.read({
+                'lang': ['es', 'fr']
+            });
+            assert.equal(config.foo, 3, '[es,fr] should be 3');
+
+            config = ycb.read({
+                'lang': ['fr', 'es']
+            });
+            assert.equal(config.foo, 1, '[fr,es] should be 1');
+        });
+
         it('should not pollute master settings with dimension values', function () {
             var bundle,
                 ycb,