Fixed support for multilevel settings loaded from ENV variables (insp…

…ired by @cbeer in [#144](#144))

CHANGELOG.md

@@ -1,9 +1,13 @@
 # Changelog
 
+## 1.2.1
+
+* Fixed support for multilevel settings loaded from ENV variables (inspired by @cbeer in [#144](https://github.com/railsconfig/config/pull/144))
+
 ## 1.2.0
 
-* Add ability to load settings from ENV variables ([#108](https://github.com/railsconfig/config/issues/108) thanks @vinceve and @spalladino)
-* Removed Rails 5 deprecation warnings for prepend_before_filter ([#141](https://github.com/railsconfig/config/pull/141)
+* Add ability to load settings from ENV variables ([#108](https://github.com/railsconfig/config/issues/108) thanks to @vinceve and @spalladino)
+* Removed Rails 5 deprecation warnings for prepend_before_filter ([#141](https://github.com/railsconfig/config/pull/141))
 
 ## 1.1.1
 
@@ -18,8 +22,8 @@
 
 * `RailsConfig` is now officially renamed to `Config`
 * Fixed array descent when converting to hash ([#89](https://github.com/railsconfig/config/pull/89))
-* Catch OpenStruct reserved keywords ([#95](https://github.com/railsconfig/config/pull/95) thanks @dudo)
-* Allows loading before app configuration process ([#107](https://github.com/railsconfig/config/pull/107) thanks @Antiarchitect)
+* Catch OpenStruct reserved keywords ([#95](https://github.com/railsconfig/config/pull/95) by @dudo)
+* Allows loading before app configuration process ([#107](https://github.com/railsconfig/config/pull/107) by @Antiarchitect)
 * `deep_merge` is now properly managed via gemspec ([#110](https://github.com/railsconfig/config/pull/110))
 * Added `prepend_source!` ([#102](https://github.com/railsconfig/config/pull/102))
 

lib/config/options.rb

@@ -29,23 +29,33 @@ def prepend_source!(source)
 
     def reload_env!
       return self if ENV.nil? || ENV.empty?
-      conf = Hash.new
-      ENV.each do |key, value|
-        next unless key.to_s.index(Config.env_prefix || Config.const_name) == 0
-        hash = Config.env_parse_values ? __value(value) : value
-        key.to_s.split(Config.env_separator).reverse[0...-1].each do |element|
-          element = case Config.env_converter
-          when :downcase then element.downcase
-          when nil then element
-          else raise "Invalid env converter: #{Config.env_converter}"
+
+      hash = Hash.new
+
+      ENV.each do |variable, value|
+        keys = variable.to_s.split(Config.env_separator)
+
+        next if keys.shift != (Config.env_prefix || Config.const_name)
+
+        keys.map! { |key|
+          case Config.env_converter
+            when :downcase then
+              key.downcase.to_sym
+            when nil then
+              key.to_sym
+            else
+              raise "Invalid ENV variables name converter: #{Config.env_converter}"
           end
+        }
 
-          hash = {element => hash}
-        end
-        DeepMerge.deep_merge!(hash, conf, :preserve_unmergeables => false)
+        leaf = keys[0...-1].inject(hash) { |h, key|
+          h[key] ||= {}
+        }
+
+        leaf[keys.last] = Config.env_parse_values ? __value(value) : value
       end
 
-      merge!(conf)
+      merge!(hash)
     end
 
     alias :load_env! :reload_env!
@@ -59,11 +69,10 @@ def reload!
         if conf.empty?
           conf = source_conf
         else
-          # see Options Details in lib/rails_config/vendor/deep_merge.rb
           DeepMerge.deep_merge!(source_conf,
                                 conf,
                                 preserve_unmergeables: false,
-                                knockout_prefix: Config.knockout_prefix)
+                                knockout_prefix:       Config.knockout_prefix)
         end
       end
 

lib/config/version.rb

@@ -1,3 +1,3 @@
 module Config
-  VERSION = '1.2.0'
+  VERSION = '1.2.1'
 end

spec/config_env_spec.rb

@@ -0,0 +1,143 @@
+require 'spec_helper'
+
+describe Config do
+
+  context 'when overriding settings via ENV variables is enabled' do
+    let(:config) do
+      Config.load_files "#{fixture_path}/settings.yml", "#{fixture_path}/multilevel.yml"
+    end
+
+    before :all do
+      Config.use_env = true
+    end
+
+    after :all do
+      Config.use_env = false
+    end
+
+    after :each do
+      ENV.clear
+
+      Config.env_prefix       = nil
+      Config.env_separator    = '.'
+      Config.env_converter    = nil
+      Config.env_parse_values = false
+    end
+
+    it 'should add new setting from ENV variable' do
+      ENV['Settings.new_var'] = 'value'
+
+      expect(config.new_var).to eq('value')
+    end
+
+    context 'should override existing setting with a value from ENV variable' do
+      it 'for a basic values' do
+        ENV['Settings.size'] = 'overwritten by env'
+
+        expect(config.size).to eq('overwritten by env')
+      end
+
+      it 'for multilevel sections' do
+        ENV['Settings.number_of_all_countries'] = '0'
+        ENV['Settings.world.countries.europe'] = '0'
+
+        expect(config.number_of_all_countries).to eq('0')
+        expect(config.world.countries.europe).to eq('0')
+        expect(config.world.countries.australia).to eq(1)
+      end
+    end
+
+    context 'and parsing ENV variables is enabled' do
+      before :each do
+        Config.env_parse_values = true
+      end
+
+      it 'should recognize numbers and expose them as integers' do
+        ENV['Settings.new_var'] = '123'
+
+        expect(config.new_var).to eq(123)
+        expect(config.new_var.is_a? Integer).to eq(true)
+      end
+
+      it 'should leave strings intact' do
+        ENV['Settings.new_var'] = 'foobar'
+
+        expect(config.new_var).to eq('foobar')
+        expect(config.new_var.is_a? String).to eq(true)
+      end
+    end
+
+    context 'and custom ENV variables prefix is defined' do
+      before :each do
+        Config.env_prefix = 'MyConfig'
+      end
+
+      it 'should load variables from the new prefix' do
+        ENV['MyConfig.new_var'] = 'value'
+
+        expect(config.new_var).to eq('value')
+      end
+
+      it 'should not load variables from the default prefix' do
+        ENV['Settings.new_var'] = 'value'
+
+        expect(config.new_var).to eq(nil)
+      end
+
+      it 'should skip ENV variable when partial prefix match' do
+        ENV['MyConfigs.new_var'] = 'value'
+
+        expect(config.new_var).to eq(nil)
+      end
+    end
+
+    context 'and custom ENV variables separator is defined' do
+      before :each do
+        Config.env_separator = '__'
+      end
+
+      it 'should load variables and correctly recognize the new separator' do
+        ENV['Settings__new_var']                  = 'value'
+        ENV['Settings__var.with.dot']             = 'value'
+        ENV['Settings__world__countries__europe'] = '0'
+
+        expect(config.new_var).to eq('value')
+        expect(config['var.with.dot']).to eq('value')
+        expect(config.world.countries.europe).to eq('0')
+      end
+
+      it 'should ignore variables wit default separator' do
+        ENV['Settings.new_var'] = 'value'
+
+        expect(config.new_var).to eq(nil)
+      end
+    end
+
+    context 'and variable names conversion is considered' do
+      it 'should downcase variable names when :downcase conversion enabled' do
+        ENV['Settings.NEW_VAR'] = 'value'
+        Config.env_converter    = :downcase
+
+        expect(config.new_var).to eq('value')
+      end
+
+      it 'should not change variable names by default' do
+        ENV['Settings.NEW_VAR'] = 'value'
+
+        expect(config.new_var).to eq(nil)
+        expect(config.NEW_VAR).to eq('value')
+      end
+    end
+
+    it 'should always load ENV variables when reloading settings from files' do
+      ENV['Settings.new_var'] = 'value'
+
+      expect(config.new_var).to eq('value')
+
+      Config.reload!
+
+      expect(config.new_var).to eq('value')
+    end
+
+  end
+end

spec/config_spec.rb

@@ -90,95 +90,7 @@
     expect(Settings.size).to eq(2)
   end
 
-  context "ENV variables" do
-    let(:config) do
-      Config.load_files("#{fixture_path}/settings.yml")
-    end
-
-    before :all do
-      load_env("#{fixture_path}/env/settings.yml")
-      Config.use_env = true
-    end
-
-    after :all do
-      Config.use_env = false
-    end
-
-    after :each do
-      Config.env_prefix = nil
-      Config.env_separator = "."
-      Config.env_converter = nil
-      Config.env_parse_values = false
-    end
-
-    it "should load basic ENV variables" do
-      config.load_env!
-      expect(config.test_var).to eq("123")
-    end
-
-    it "should parse ENV variables as numeric" do
-      Config.env_parse_values = true
-      config.load_env!
-      expect(config.test_var).to eq(123)
-    end
-
-    it "should leave ENV variables as strings" do
-      Config.env_parse_values = true
-      config.load_env!
-      expect(config.test_str_var).to eq("foobar")
-    end
-
-    it "should load nested sections" do
-      config.load_env!
-      expect(config.hash_test.one).to eq("1-1")
-    end
-
-    it "should use env specific prefix" do
-      ENV['MyConfig.hash_test.three'] = "1-3"
-      Config.env_prefix = "MyConfig"
-      config.load_env!
-      expect(config.hash_test.one).to be_nil
-      expect(config.hash_test.three).to eq("1-3")
-    end
-
-    it "should use env specific separator" do
-      ENV['Settings__hash_test__four'] = "1-4"
-      Config.env_separator = "__"
-      config.load_env!
-      expect(config.hash_test.four).to eq("1-4")
-    end
-
-    it "should convert env keys to downcase" do
-      ENV['Settings.HASH_TEST.FIVE'] = "1-5"
-      Config.env_converter = :downcase
-      config.load_env!
-      expect(config.hash_test.five).to eq("1-5")
-    end
 
-    it "should parse env-like keys" do
-      ENV['APP__HASH_TEST__SIX'] = "1-6"
-      Config.env_converter = :downcase
-      Config.env_prefix = "APP"
-      Config.env_separator = "__"
-      config.load_env!
-      expect(config.hash_test.six).to eq("1-6")
-    end
-
-    it "should override settings from files" do
-      Config.load_and_set_settings ["#{fixture_path}/settings.yml"]
-
-      expect(Settings.server).to eq("google.com")
-      expect(Settings.size).to eq("3")
-    end
-
-    it "should reload env" do
-      Config.load_and_set_settings ["#{fixture_path}/settings.yml"]
-      Config.reload!
-
-      expect(Settings.server).to eq("google.com")
-      expect(Settings.size).to eq("3")
-    end
-  end
 
   context "Nested Settings" do
     let(:config) do

spec/fixtures/multilevel.yml

@@ -0,0 +1,10 @@
+world:
+  capitals:
+    europe:
+      germany: 'Berlin'
+      poland: 'Warsaw'
+    australia: 'Sydney'
+  countries:
+    europe: 50
+    australia: 1
+  number_of_all_countries: 196