From 5e260574a43d5c2fef8b170138baa9f7e10bfb24 Mon Sep 17 00:00:00 2001
From: John Crepezzi <seejohnrun@github.com>
Date: Thu, 25 Jul 2019 20:08:10 -0400
Subject: Only merge DATABASE_URL settings into the current env
This commit fixes a regression where when the `DATABASE_URL` environment
variable was set and the current Rails environment had a valid configuration
defined in the database config, settings from the environment variable would
affect _all_ environments (not just the current one).
---
.../lib/active_record/database_configurations.rb | 38 ++++++++++++----------
.../merge_and_resolve_default_url_config_test.rb | 16 +++++++++
2 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb
index e122628b05..8baa0f5af6 100644
--- a/activerecord/lib/active_record/database_configurations.rb
+++ b/activerecord/lib/active_record/database_configurations.rb
@@ -105,18 +105,20 @@ module ActiveRecord
return configs if configs.is_a?(Array)
db_configs = configs.flat_map do |env_name, config|
- if config.is_a?(Hash) && config.all? { |k, v| v.is_a?(Hash) }
+ if config.is_a?(Hash) && config.all? { |_, v| v.is_a?(Hash) }
walk_configs(env_name.to_s, config)
else
build_db_config_from_raw_config(env_name.to_s, "primary", config)
end
- end.compact
+ end
- if url = ENV["DATABASE_URL"]
- merge_url_with_configs(url, db_configs)
- else
- db_configs
+ current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s
+
+ unless db_configs.find(&:for_current_env?)
+ db_configs << environment_url_config(current_env, "primary", {})
end
+
+ merge_db_environment_variables(current_env, db_configs.compact)
end
def walk_configs(env_name, config)
@@ -156,22 +158,22 @@ module ActiveRecord
end
end
- def merge_url_with_configs(url, configs)
- env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s
+ def merge_db_environment_variables(current_env, configs)
+ configs.map do |config|
+ next config if config.url_config? || config.env_name != current_env
- if configs.find(&:for_current_env?)
- configs.map do |config|
- if config.url_config?
- config
- else
- ActiveRecord::DatabaseConfigurations::UrlConfig.new(config.env_name, config.spec_name, url, config.config)
- end
- end
- else
- configs + [ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, "primary", url)]
+ url_config = environment_url_config(current_env, config.spec_name, config.config)
+ url_config || config
end
end
+ def environment_url_config(env, spec_name, config)
+ url = ENV["DATABASE_URL"]
+ return unless url
+
+ ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, spec_name, url, config)
+ end
+
def method_missing(method, *args, &blk)
case method
when :each, :first
diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb
index 15a045ed23..95e57f42e3 100644
--- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb
+++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb
@@ -347,6 +347,22 @@ module ActiveRecord
assert_equal expected, actual
end
end
+
+ def test_does_not_change_other_environments
+ ENV["DATABASE_URL"] = "postgres://localhost/foo"
+ config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" }, "default_env" => {} }
+
+ actual = resolve_spec(:production, config)
+ assert_equal config["production"].merge("name" => "production"), actual
+
+ actual = resolve_spec(:default_env, config)
+ assert_equal({
+ "host" => "localhost",
+ "database" => "foo",
+ "adapter" => "postgresql",
+ "name" => "default_env"
+ }, actual)
+ end
end
end
end
--
cgit v1.2.3