From 4b005fb371c2e7af80df7da63be94509b1db038c Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Wed, 23 Jan 2013 01:05:23 -0800 Subject: DATABASE_URL parsing should turn numeric strings into numeric types, and the strings true and false into boolean types, in order to match how YAML would parse the same values from database.yml and prevent unexpected type errors in the database adapters. --- activerecord/CHANGELOG.md | 12 ++++++ .../connection_specification.rb | 19 +++++++++ .../connection_specification/resolver_test.rb | 48 +++++++++++++++++----- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b987104dd0..71de20a2b9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,17 @@ ## Rails 4.0.0 (unreleased) ## +* The DATABASE_URL environment variable now converts ints, floats, and + the strings true and false to Ruby types. For example, SQLite requires + that the timeout value is an integer, and PostgreSQL requires that the + prepared_statements option is a boolean. These now work as expected: + + Example: + + DATABASE_URL=sqlite3://localhost/test_db?timeout=500 + DATABASE_URL=postgresql://localhost/test_db?prepared_statements=false + + *Aaron Stone* + * Relation#merge now only overwrites where values on the LHS of the merge. Consider: diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 09250d3c01..d7cd34df22 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -62,6 +62,10 @@ module ActiveRecord ConnectionSpecification.new(spec, adapter_method) end + # For DATABASE_URL, accept a limited concept of ints and floats + SIMPLE_INT = /^\d+$/ + SIMPLE_FLOAT = /^\d+\.\d+$/ + def connection_url_to_hash(url) # :nodoc: config = URI.parse url adapter = config.scheme @@ -77,6 +81,21 @@ module ActiveRecord spec.map { |key,value| spec[key] = uri_parser.unescape(value) if value.is_a?(String) } if config.query options = Hash[config.query.split("&").map{ |pair| pair.split("=") }].symbolize_keys + # If anything looks numeric, make it numeric (e.g. pool count, timeout values, etc.) + options.map do |key,value| + options[key] = case value + when SIMPLE_INT + value.to_i + when SIMPLE_FLOAT + value.to_f + when 'true' + true + when 'false' + false + else + value + end + end spec.merge!(options) end spec diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb index 52de0efe7f..f0a2cdca1a 100644 --- a/activerecord/test/cases/connection_specification/resolver_test.rb +++ b/activerecord/test/cases/connection_specification/resolver_test.rb @@ -8,40 +8,66 @@ module ActiveRecord Resolver.new(spec, {}).spec.config end + def test_url_invalid_adapter + assert_raises(LoadError) do + resolve 'ridiculous://foo?encoding=utf8' + end + end + + # The abstract adapter is used simply to bypass the bit of code that + # checks that the adapter file can be required in. + def test_url_host_no_db - skip "only if mysql is available" unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) - spec = resolve 'mysql://foo?encoding=utf8' + spec = resolve 'abstract://foo?encoding=utf8' assert_equal({ - :adapter => "mysql", + :adapter => "abstract", :host => "foo", :encoding => "utf8" }, spec) end def test_url_host_db - skip "only if mysql is available" unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) - spec = resolve 'mysql://foo/bar?encoding=utf8' + spec = resolve 'abstract://foo/bar?encoding=utf8' assert_equal({ - :adapter => "mysql", + :adapter => "abstract", :database => "bar", :host => "foo", :encoding => "utf8" }, spec) end def test_url_port - skip "only if mysql is available" unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) - spec = resolve 'mysql://foo:123?encoding=utf8' + spec = resolve 'abstract://foo:123?encoding=utf8' assert_equal({ - :adapter => "mysql", + :adapter => "abstract", :port => 123, :host => "foo", :encoding => "utf8" }, spec) end + def test_url_query_numeric + spec = resolve 'abstract://foo:123?encoding=utf8&int=500&float=10.9' + assert_equal({ + :adapter => "abstract", + :port => 123, + :int => 500, + :float => 10.9, + :host => "foo", + :encoding => "utf8" }, spec) + end + + def test_url_query_boolean + spec = resolve 'abstract://foo:123?true=true&false=false' + assert_equal({ + :adapter => "abstract", + :port => 123, + :true => true, + :false => false, + :host => "foo" }, spec) + end + def test_encoded_password - skip "only if mysql is available" unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) password = 'am@z1ng_p@ssw0rd#!' encoded_password = URI.encode_www_form_component(password) - spec = resolve "mysql://foo:#{encoded_password}@localhost/bar" + spec = resolve "abstract://foo:#{encoded_password}@localhost/bar" assert_equal password, spec[:password] end end -- cgit v1.2.3