aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoreileencodes <eileencodes@gmail.com>2019-07-30 09:06:48 -0400
committereileencodes <eileencodes@gmail.com>2019-08-01 16:33:58 -0400
commitdde03a9234e9b7fe802a45d511720c0ac3bf7617 (patch)
tree7af0a1b82060d9b9635ad615ffc198c5a9e4c03e
parent4f1bd290f0d834b30976b6c08bcee9b15c13b2ad (diff)
downloadrails-dde03a9234e9b7fe802a45d511720c0ac3bf7617.tar.gz
rails-dde03a9234e9b7fe802a45d511720c0ac3bf7617.tar.bz2
rails-dde03a9234e9b7fe802a45d511720c0ac3bf7617.zip
Introduce InvalidConfigurationError
In our app at work we had a faked config like this: ``` { "foo" => :bar, "bar" => { "adapter" => "memory" } } ``` This config is invalid. You can't say for foo env just have a symbol, nor would this work if you had fa foo env with just a string. A configuration must be a url or an adapter or a database. Otherwise it's invalid and we can't parse it. When this was just yaml turned into hashes you could get away with passing whatever. It wouldn't work but it wouldn't blow up either. Now that we're using objects we were returning `nil` for these but that just means we either blow up on `for_current_env` or compact the `nil`'s. I think it's a better user experience to not build the configs and raise an appropriate error. This is also an invalid config because if you do pass a string here it should be a URL. ``` { "foo" => "bar", "bar" => { "adapter" => "memory" } } ```
-rw-r--r--activerecord/lib/active_record/database_configurations.rb10
-rw-r--r--activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb16
2 files changed, 23 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb
index c56a099769..268be34b21 100644
--- a/activerecord/lib/active_record/database_configurations.rb
+++ b/activerecord/lib/active_record/database_configurations.rb
@@ -9,6 +9,8 @@ module ActiveRecord
# objects (either a HashConfig or UrlConfig) that are constructed from the
# application's database configuration hash or URL string.
class DatabaseConfigurations
+ class InvalidConfigurationError < StandardError; end
+
attr_reader :configurations
delegate :any?, to: :configurations
@@ -146,17 +148,19 @@ module ActiveRecord
build_db_config_from_string(env_name, spec_name, config)
when Hash
build_db_config_from_hash(env_name, spec_name, config.stringify_keys)
+ else
+ raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash."
end
end
def build_db_config_from_string(env_name, spec_name, config)
url = config
uri = URI.parse(url)
- if uri&.scheme
+ if uri.scheme
ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url)
+ else
+ raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash."
end
- rescue URI::InvalidURIError
- ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config)
end
def build_db_config_from_hash(env_name, spec_name, config)
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 ee2972101f..2ac249b478 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
@@ -28,6 +28,22 @@ module ActiveRecord
resolver.resolve(spec, spec)
end
+ def test_invalid_string_config
+ config = { "foo" => "bar" }
+
+ assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do
+ resolve_config(config)
+ end
+ end
+
+ def test_invalid_symbol_config
+ config = { "foo" => :bar }
+
+ assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do
+ resolve_config(config)
+ end
+ end
+
def test_resolver_with_database_uri_and_current_env_symbol_key
ENV["DATABASE_URL"] = "postgres://localhost/foo"
config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } }