From c6f60c8cbf56f608ad56549bf866b0416728a7a1 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Fri, 21 Apr 2017 16:48:08 -0400 Subject: [PATCH] Merge nil values by default Previously, if you merged in {a: nil}, it would not overwrite an existing value. In #13, this was found to be an undesirable default behavior. Now, we will merge nil values by default. You can retain the old behavior via `config.merge_nil_values = false` in your Config initializer. This required changes to deep merge, found in: https://github.com/danielsdeleo/deep_merge/pull/33 This was released in deep_merge 1.2.0+. Fixes #13 --- CHANGELOG.md | 1 + README.md | 22 +++++++++ lib/config.rb | 3 +- lib/config/options.rb | 1 + lib/generators/config/templates/config.rb | 5 ++ spec/config_spec.rb | 59 +++++++++++++++++++++-- spec/fixtures/deep_merge/config1.yml | 3 ++ 7 files changed, 89 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0923bbb..007eaff9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +* **WARNING:** nil values will now overwrite an existing value. 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)) * Make dry-validation dependency less strict allowing to use newer versions ([#183](https://github.com/railsconfig/config/pull/183)) * Fix `key?` and `has_key?`, which raise NoMethodError in non Rails environment, by using ActiveSupport `#delegate` implicitly ([#185](https://github.com/railsconfig/config/pull/185)) * Update `deep_merge` dependency to latest version (v1.2.1) ([#191](https://github.com/railsconfig/config/pull/191)) diff --git a/README.md b/README.md index 8f92a940..15bd9769 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,28 @@ located at `config/initializers/config.rb`. ### Merge customization * `overwrite_arrays` - overwrite arrays found in previously loaded settings file. Default: `true` +* `merge_nil_values` - overwrite a value with a nil value. Default: `true`. + +```ruby +# merge_nil_values is true by default +c = Config.load_files("./spec/fixtures/development.yml") # => # +c.size # => 2 +c.merge!(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") # => # +c.size # => 2 +c.merge!(size: nil) => # +c.size # => 2 +``` + * `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` Check [Deep Merge](https://github.com/danielsdeleo/deep_merge) for more details. diff --git a/lib/config.rb b/lib/config.rb index 1b5e1f43..a6322641 100644 --- a/lib/config.rb +++ b/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 diff --git a/lib/config/options.rb b/lib/config/options.rb index 81127b12..28599b4f 100644 --- a/lib/config/options.rb +++ b/lib/config/options.rb @@ -127,6 +127,7 @@ def merge!(hash) current = to_hash DeepMerge.deep_merge!(hash.dup, current, + merge_nil_values: Config.merge_nil_values, preserve_unmergeables: false, overwrite_arrays: Config.overwrite_arrays) marshal_load(__convert(current).marshal_dump) diff --git a/lib/generators/config/templates/config.rb b/lib/generators/config/templates/config.rb index f3252122..8e276df9 100644 --- a/lib/generators/config/templates/config.rb +++ b/lib/generators/config/templates/config.rb @@ -6,6 +6,11 @@ # # config.knockout_prefix = nil + # When merging nil values for settings, overwrite an existing value with nil. + # When set to `false`, the original value is retained. + # + # 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 diff --git a/spec/config_spec.rb b/spec/config_spec.rb index 736bcd5f..cf4f8c88 100644 --- a/spec/config_spec.rb +++ b/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 diff --git a/spec/fixtures/deep_merge/config1.yml b/spec/fixtures/deep_merge/config1.yml index 0ffefd8f..1ba74eda 100644 --- a/spec/fixtures/deep_merge/config1.yml +++ b/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: