Merge nil values by default

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Unreleased
 
-...
+* **WARNING:** `nil` values will from now on overwrite an existing value when merging configs! This change of behavior can be reverted via `config.merge_nil_values = false` in your Config initializer ([#196](https://github.com/railsconfig/config/pull/196))
 
 ## 1.6.1
 

README.md

@@ -271,6 +271,27 @@ located at `config/initializers/config.rb`.
 
 * `overwrite_arrays` - overwrite arrays found in previously loaded settings file. Default: `true`
 * `knockout_prefix` - ability to remove elements of the array set in earlier loaded settings file. Makes sense only when `overwrite_arrays = false`, otherwise array settings would be overwritten by default. Default: `nil`
+* `merge_nil_values` - `nil` values will overwrite an existing value when merging configs. Default: `true`.
+
+```ruby
+# merge_nil_values is true by default
+c = Config.load_files("./spec/fixtures/development.yml") # => #<Config::Options size=2, ...>
+c.size # => 2
+c.merge!(size: nil) => #<Config::Options size=nil, ...>
+c.size # => nil
+```
+
+```ruby
+# To reject nil values when merging settings:
+Config.setup do |config|
+  config.merge_nil_values = false
+end
+
+c = Config.load_files("./spec/fixtures/development.yml") # => #<Config::Options size=2, ...>
+c.size # => 2
+c.merge!(size: nil) => #<Config::Options size=nil, ...>
+c.size # => 2
+```
 
 Check [Deep Merge](https://github.com/danielsdeleo/deep_merge) for more details.
 

lib/config.rb

@@ -26,8 +26,9 @@ module Config
   @@fail_on_missing = false
 
   # deep_merge options
-  mattr_accessor :knockout_prefix, :overwrite_arrays
+  mattr_accessor :knockout_prefix, :merge_nil_values, :overwrite_arrays
   @@knockout_prefix = nil
+  @@merge_nil_values = true
   @@overwrite_arrays = true
 
   def self.setup

lib/config/options.rb

@@ -76,11 +76,14 @@ def reload!
         if conf.empty?
           conf = source_conf
         else
-          DeepMerge.deep_merge!(source_conf,
+          DeepMerge.deep_merge!(
+                                source_conf,
                                 conf,
                                 preserve_unmergeables: false,
                                 knockout_prefix:       Config.knockout_prefix,
-                                overwrite_arrays:      Config.overwrite_arrays)
+                                overwrite_arrays:      Config.overwrite_arrays,
+                                merge_nil_values:      Config.merge_nil_values
+                               )
         end
       end
 
@@ -125,10 +128,14 @@ def to_json(*args)
 
     def merge!(hash)
       current = to_hash
-      DeepMerge.deep_merge!(hash.dup,
+      DeepMerge.deep_merge!(
+                            hash.dup,
                             current,
                             preserve_unmergeables: false,
-                            overwrite_arrays:      Config.overwrite_arrays)
+                            knockout_prefix:       Config.knockout_prefix,
+                            overwrite_arrays:      Config.overwrite_arrays,
+                            merge_nil_values:      Config.merge_nil_values
+                           )
       marshal_load(__convert(current).marshal_dump)
       self
     end

lib/generators/config/templates/config.rb

@@ -6,6 +6,11 @@
   #
   # config.knockout_prefix = nil
 
+  # Overwrite an existing value when merging a `nil` value.
+  # When set to `false`, the existing value is retained after merge.
+  #
+  # config.merge_nil_values = true
+
   # Overwrite arrays found in previously loaded settings file. When set to `false`, arrays will be merged.
   #
   # config.overwrite_arrays = true

spec/config_spec.rb

@@ -198,7 +198,8 @@
 
   context "Merging nested hash at runtime" do
     let(:config) { Config.load_files("#{fixture_path}/deep_merge/config1.yml") }
-    let(:hash) { { :inner => { :something1 => 'changed1', :something3 => 'changed3' } } }
+    let(:hash) { { inner: { something1: 'changed1', something3: 'changed3' } } }
+    let(:hash_with_nil) { { inner: { something1: nil } } }
 
     it 'should preserve first level keys' do
       expect { config.merge!(hash) }.to_not change { config.keys }
@@ -210,11 +211,61 @@
     end
 
     it 'should add new nested key' do
-      expect { config.merge!(hash) }.to change { config.inner.something3 }.from(nil).to("changed3")
+      expect { config.merge!(hash) }
+        .to change { config.inner.something3 }.from(nil).to('changed3')
     end
 
     it 'should rewrite a merged value' do
-      expect { config.merge!(hash) }.to change { config.inner.something1 }.from('blah1').to('changed1')
+      expect { config.merge!(hash) }
+        .to change { config.inner.something1 }.from('blah1').to('changed1')
+    end
+
+    it 'should update a string to nil ' do
+      expect { config.merge!(hash_with_nil) }
+        .to change { config.inner.something1 }.from('blah1').to(nil)
+    end
+
+    it 'should update something nil to true' do
+      expect { config.merge!(inner: { somethingnil: true }) }
+        .to change { config.inner.somethingnil }.from(nil).to(true)
+    end
+
+    it 'should update something nil to false' do
+      expect { config.merge!(inner: { somethingnil: false }) }
+        .to change { config.inner.somethingnil }.from(nil).to(false)
+    end
+
+    it 'should update something false to true' do
+      expect { config.merge!(inner: { somethingfalse: true }) }
+        .to change { config.inner.somethingfalse }.from(false).to(true)
+    end
+
+    it 'should update something false to nil' do
+      expect { config.merge!(inner: { somethingfalse: nil }) }
+        .to change { config.inner.somethingfalse }.from(false).to(nil)
+    end
+
+    it 'should update something true to false' do
+      expect { config.merge!(inner: { somethingtrue: false }) }
+        .to change { config.inner.somethingtrue }.from(true).to(false)
+    end
+
+    it 'should update something true to nil' do
+      expect { config.merge!(inner: { somethingtrue: nil }) }
+        .to change { config.inner.somethingtrue }.from(true).to(nil)
+    end
+
+    context 'with Config.merge_nil_values = false' do
+      let(:config) do
+        Config.merge_nil_values = false
+        Config.load_files("#{fixture_path}/deep_merge/config1.yml")
+      end
+
+      it 'should not overwrite values with nil' do
+        old_value = config.inner.something1
+        config.merge!(hash_with_nil)
+        expect(config.inner.something1).to eq(old_value)
+      end
     end
   end
 
@@ -355,7 +406,7 @@
         end
 
         it 'should merge hashes from multiple configs' do
-          expect(config.inner.marshal_dump.keys.size).to eq(3)
+          expect(config.inner.marshal_dump.keys.size).to eq(6)
           expect(config.inner2.inner2_inner.marshal_dump.keys.size).to eq(3)
         end
 

spec/fixtures/deep_merge/config1.yml

@@ -3,6 +3,9 @@ server: google.com
 inner:
   something1: "blah1"
   something2: "blah2"
+  somethingnil:
+  somethingfalse: false
+  somethingtrue: true
 
 inner2:
   inner2_inner: