Skip to content

Commit

Permalink
Fix case when we want a UrlConfig but the URL is nil
Browse files Browse the repository at this point in the history
Previously if the `url` key in a config hash was nil we'd ignore the
configuration as invalid. This can happen when you're relying on a
`DATABASE_URL` in the env and that is not set in the environment.

```
production:
  <<: *default
  url: ENV['DATABASE_URL']
```

This PR fixes that case by checking if there is a `url` key in the
config instead of checking if the `url` is not nil in the config.

In addition to changing the conditional we then need to build a url hash
to merge with the original hash in the `UrlConfig` object.

Fixes rails#35091
  • Loading branch information
eileencodes committed Jan 30, 2019
1 parent 9bb07b7 commit dedcc19
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/database_configurations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,11 @@ def build_db_config_from_string(env_name, spec_name, config)
end

def build_db_config_from_hash(env_name, spec_name, config)
if url = config["url"]
if config.has_key?("url")
url = config["url"]
config_without_url = config.dup
config_without_url.delete "url"

ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url, config_without_url)
elsif config["database"] || (config.size == 1 && config.values.all? { |v| v.is_a? String })
ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,17 @@ def migrations_paths
end

private
def build_config(original_config, url)
if /^jdbc:/.match?(url)
hash = { "url" => url }

def build_url_hash(url)
if url.nil? || /^jdbc:/.match?(url)
{ "url" => url }
else
hash = ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash
ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash
end
end

def build_config(original_config, url)
hash = build_url_hash(url)

if original_config[env_name]
original_config[env_name].merge(hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ def test_resolver_with_database_uri_and_current_env_symbol_key_and_rails_env
assert_equal expected, actual
end

def test_resolver_with_nil_database_url_and_current_env
ENV["RAILS_ENV"] = "foo"
config = { "foo" => { "adapter" => "postgres", "url" => ENV["DATABASE_URL"] } }
actual = resolve_spec(:foo, config)
expected = { "adapter" => "postgres", "url" => nil, "name" => "foo" }
assert_equal expected, actual
end

def test_resolver_with_database_uri_and_current_env_symbol_key_and_rack_env
ENV["DATABASE_URL"] = "postgres://localhost/foo"
ENV["RACK_ENV"] = "foo"
Expand Down

0 comments on commit dedcc19

Please sign in to comment.