rubyconfig / config

Easiest way to add multi-environment yaml settings to Rails, Sinatra, Padrino and other Ruby projects.
Other
2.1k stars 230 forks source link

Hash sources get clobbered in `Settings.reload!` #320

Open wwboynton opened 2 years ago

wwboynton commented 2 years ago

When manually initializing Config with a YAML source, and then adding a hash source via .add_source!(), the Settings.reload!() method causes the hash source to be clobbered. Reversing the order of the sources by using prepend_source!() instead causes the YAML file to be totally clobbered leaving only the contents of the hash instead.

The reasons seems to be related to the fact that on line 43 of config/options.rb, we perform a DeepMerge of conf and source_conf. In this case, conf is an object of type Config::Options containing the configuration so far at that point in the reload process. The hash source, which is an object of type Config::Sources::HashSource, returns a plain hash when calling .load().

_So source_conf (which is set by calling source.load()) is a plain Hash, and conf is a Config::Options._

Per the DeepMerge documentation:

The deep_merge! method permits merging of arbitrary child elements. The two top level elements must be hashes.

When I changed line 43 of config/options.rb like this, it worked:

Before:

 43 DeepMerge.deep_merge!(
 44                       source_conf,
 45                       conf,
 46                       preserve_unmergeables: false,
 47                       knockout_prefix:       Config.knockout_prefix,
 48                       overwrite_arrays:      Config.overwrite_arrays,
 49                       merge_nil_values:      Config.merge_nil_values,
 50                       merge_hash_arrays:     Config.merge_hash_arrays
 51                      )

After:

 43 conf = DeepMerge.deep_merge!( # Manually set "conf" to this since the destination wouldn't stick
 44                       source_conf,
 45                       conf.to_hash, # Make sure this is also a hash
 46                       preserve_unmergeables: false,
 47                       knockout_prefix:       Config.knockout_prefix,
 48                       overwrite_arrays:      Config.overwrite_arrays,
 49                       merge_nil_values:      Config.merge_nil_values,
 50                       merge_hash_arrays:     Config.merge_hash_arrays
 51                      )

Alternatively you could just change line 40-41, I think:

 40         if conf.empty?
 41           conf = source_conf.to_hash

Once I made this change, I got the expected behaviour: the parameters in my hash were loaded after those in my YAML file, and merged together with priority given to the hash which is last in the sources list.

Happy to PR either of those methods, but wanted to make sure I wasn't doing something wrong or missing an unintended consequence of those changes.

pkuczynski commented 2 years ago

Have you tried running the test after making any of the changes? This might give you an answer if anything gets broken. Feel free to do a PR and it will test it automatically across different frameworks.