From 21c7199c0f0a0187298621631fbc3864c93ec8b9 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 2 Dec 2019 11:52:54 +0100 Subject: [PATCH 1/2] Allow non-hash values in config files Fix: https://github.com/rails/rails/issues/37800 --- railties/lib/rails/application.rb | 20 +++++----- .../test/application/configuration_test.rb | 37 +++++++++++++------ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 2ea0a45c78..29b13f3e7a 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -220,27 +220,25 @@ module Rails # config.middleware.use ExceptionNotifier, config_for(:exception_notification) # end def config_for(name, env: Rails.env) - if name.is_a?(Pathname) - yaml = name - else - yaml = Pathname.new("#{paths["config"].existent.first}/#{name}.yml") - end + yaml = name.is_a?(Pathname) ? name : Pathname.new("#{paths["config"].existent.first}/#{name}.yml") if yaml.exist? require "erb" - config = YAML.load(ERB.new(yaml.read).result, symbolize_names: true) || {} - config = (config[:shared] || {}).deep_merge(config[env.to_sym] || {}) + all_configs = YAML.load(ERB.new(yaml.read).result, symbolize_names: true) || {} + config, shared = all_configs[env.to_sym], all_configs[:shared] - ActiveSupport::OrderedOptions.new.tap do |options| - options.update(config) + if config.is_a?(Hash) + ActiveSupport::OrderedOptions.new.update(shared&.deep_merge(config) || config) + else + config || shared end else raise "Could not load configuration. No such file - #{yaml}" end - rescue Psych::SyntaxError => e + rescue Psych::SyntaxError => error raise "YAML syntax error occurred while parsing #{yaml}. " \ "Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \ - "Error: #{e.message}" + "Error: #{error.message}" end # Stores some of the Rails initial environment parameters which diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index b00afc484c..8768f141d6 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1958,6 +1958,22 @@ module ApplicationTests assert_equal({ baz: 1 }, actual[:bar]) end + test "config_for does not assume config is a hash" do + app_file "config/custom.yml", <<~RUBY + development: + - foo + - bar + RUBY + + add_to_config <<~RUBY + config.my_custom_config = config_for('custom') + RUBY + + app "development" + + assert_equal %w( foo bar ), Rails.application.config.my_custom_config + end + test "config_for uses the Pathname object if it is provided" do app_file "config/custom.yml", <<-RUBY development: @@ -1985,19 +2001,19 @@ module ApplicationTests assert_equal "Could not load configuration. No such file - #{app_path}/config/custom.yml", exception.message end - test "config_for without the environment configured returns an empty hash" do - app_file "config/custom.yml", <<-RUBY - test: - key: 'custom key' + test "config_for without the environment configured returns nil" do + app_file "config/custom.yml", <<~RUBY + test: + key: 'custom key' RUBY - add_to_config <<-RUBY + add_to_config <<~RUBY config.my_custom_config = config_for('custom') RUBY app "development" - assert_equal({}, Rails.application.config.my_custom_config) + assert_nil Rails.application.config.my_custom_config end test "config_for implements shared configuration as secrets case found" do @@ -2055,17 +2071,16 @@ module ApplicationTests assert_equal({ baz: 1, qux: 2 }, Rails.application.config.my_custom_config[:foo][:bar]) end - test "config_for with empty file returns an empty hash" do - app_file "config/custom.yml", <<-RUBY - RUBY + test "config_for with empty file returns nil" do + app_file "config/custom.yml", "" - add_to_config <<-RUBY + add_to_config <<~RUBY config.my_custom_config = config_for('custom') RUBY app "development" - assert_equal({}, Rails.application.config.my_custom_config) + assert_nil Rails.application.config.my_custom_config end test "config_for containing ERB tags should evaluate" do From 5c3b6c73c4d3eab072cff242e59b959bffd641eb Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Tue, 17 Dec 2019 02:02:27 +0100 Subject: [PATCH 2/2] Add helper method to slim down config_for test set up --- .../test/application/configuration_test.rb | 165 +++++++----------- 1 file changed, 60 insertions(+), 105 deletions(-) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 8768f141d6..827a0fdca1 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1902,13 +1902,9 @@ module ApplicationTests end test "config_for loads custom configuration from yaml accessible as symbol or string" do - app_file "config/custom.yml", <<-RUBY - development: - foo: 'bar' - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + set_custom_config <<~RUBY + development: + foo: "bar" RUBY app "development" @@ -1918,15 +1914,11 @@ module ApplicationTests end test "config_for loads nested custom configuration from yaml as symbol keys" do - app_file "config/custom.yml", <<-RUBY - development: - foo: - bar: - baz: 1 - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + set_custom_config <<~RUBY + development: + foo: + bar: + baz: 1 RUBY app "development" @@ -1935,21 +1927,16 @@ module ApplicationTests end test "config_for makes all hash methods available" do - app_file "config/custom.yml", <<-RUBY - development: - foo: 0 - bar: - baz: 1 - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + set_custom_config <<~RUBY + development: + foo: 0 + bar: + baz: 1 RUBY app "development" actual = Rails.application.config.my_custom_config - assert_equal({ foo: 0, bar: { baz: 1 } }, actual) assert_equal([ :foo, :bar ], actual.keys) assert_equal([ 0, baz: 1], actual.values) @@ -1959,29 +1946,21 @@ module ApplicationTests end test "config_for does not assume config is a hash" do - app_file "config/custom.yml", <<~RUBY + set_custom_config <<~RUBY development: - foo - bar RUBY - add_to_config <<~RUBY - config.my_custom_config = config_for('custom') - RUBY - app "development" assert_equal %w( foo bar ), Rails.application.config.my_custom_config end test "config_for uses the Pathname object if it is provided" do - app_file "config/custom.yml", <<-RUBY - development: - key: 'custom key' - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for(Pathname.new(Rails.root.join("config/custom.yml"))) + set_custom_config <<~RUBY, "Pathname.new(Rails.root.join('config/custom.yml'))" + development: + key: 'custom key' RUBY app "development" @@ -2002,68 +1981,52 @@ module ApplicationTests end test "config_for without the environment configured returns nil" do - app_file "config/custom.yml", <<~RUBY + set_custom_config <<~RUBY test: key: 'custom key' RUBY - add_to_config <<~RUBY - config.my_custom_config = config_for('custom') - RUBY - app "development" assert_nil Rails.application.config.my_custom_config end - test "config_for implements shared configuration as secrets case found" do - app_file "config/custom.yml", <<-RUBY - shared: - foo: :bar - test: - foo: :baz - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + test "config_for shared config is overriden" do + set_custom_config <<~RUBY + shared: + foo: :from_shared + test: + foo: :from_env RUBY app "test" - assert_equal(:baz, Rails.application.config.my_custom_config[:foo]) + assert_equal :from_env, Rails.application.config.my_custom_config[:foo] end - test "config_for implements shared configuration as secrets case not found" do - app_file "config/custom.yml", <<-RUBY - shared: - foo: :bar - test: - foo: :baz - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + test "config_for shared config is returned when environment is missing" do + set_custom_config <<~RUBY + shared: + foo: :from_shared + test: + foo: :from_env RUBY app "development" - assert_equal(:bar, Rails.application.config.my_custom_config[:foo]) + assert_equal :from_shared, Rails.application.config.my_custom_config[:foo] end test "config_for merges shared configuration deeply" do - app_file "config/custom.yml", <<-RUBY - shared: - foo: - bar: - baz: 1 - development: - foo: - bar: - qux: 2 - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + set_custom_config <<~RUBY + shared: + foo: + bar: + baz: 1 + development: + foo: + bar: + qux: 2 RUBY app "development" @@ -2072,11 +2035,7 @@ module ApplicationTests end test "config_for with empty file returns nil" do - app_file "config/custom.yml", "" - - add_to_config <<~RUBY - config.my_custom_config = config_for('custom') - RUBY + set_custom_config "" app "development" @@ -2084,13 +2043,9 @@ module ApplicationTests end test "config_for containing ERB tags should evaluate" do - app_file "config/custom.yml", <<-RUBY - development: - key: <%= 'custom key' %> - RUBY - - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') + set_custom_config <<~RUBY + development: + key: <%= 'custom key' %> RUBY app "development" @@ -2099,33 +2054,25 @@ module ApplicationTests end test "config_for with syntax error show a more descriptive exception" do - app_file "config/custom.yml", <<-RUBY - development: - key: foo: + set_custom_config <<~RUBY + development: + key: foo: RUBY - add_to_config <<-RUBY - config.my_custom_config = config_for('custom') - RUBY - - exception = assert_raises(RuntimeError) do + error = assert_raises RuntimeError do app "development" end - - assert_match "YAML syntax error occurred while parsing", exception.message + assert_match "YAML syntax error occurred while parsing", error.message end test "config_for allows overriding the environment" do - app_file "config/custom.yml", <<-RUBY + set_custom_config <<~RUBY, "'custom', env: 'production'" test: key: 'walrus' production: - key: 'unicorn' + key: 'unicorn' RUBY - add_to_config <<-RUBY - config.my_custom_config = config_for('custom', env: 'production') - RUBY require "#{app_path}/config/environment" assert_equal "unicorn", Rails.application.config.my_custom_config[:key] @@ -2544,5 +2491,13 @@ module ApplicationTests def force_lazy_load_hooks yield # Tasty clarifying sugar, homie! We only need to reference a constant to load it. end + + def set_custom_config(contents, config_source = "custom".inspect) + app_file "config/custom.yml", contents + + add_to_config <<~RUBY + config.my_custom_config = config_for(#{config_source}) + RUBY + end end end