Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash sources get clobbered in Settings.reload! #320

Open
wwboynton opened this issue Jun 5, 2022 · 1 comment
Open

Hash sources get clobbered in Settings.reload! #320

wwboynton opened this issue Jun 5, 2022 · 1 comment

Comments

@wwboynton
Copy link

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants