Merge pull request #9041 from sodabrew/patch-1

Convert int, float, and bools from ENV['DATABASE_URL'] query args

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