Merge pull request #14633 from matthewd/narrow_database_url

Only apply DATABASE_URL for Rails.env

3 files changed, 54 insertions, 58 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
index e0715f7ce9..a8ab52be74 100644
--- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
@@ -33,7 +33,7 @@ module ActiveRecord
         def initialize(url)
           raise "Database URL cannot be empty" if url.blank?
           @uri     = URI.parse(url)
-          @adapter = @uri.scheme
+          @adapter = @uri.scheme.gsub('-', '_')
           @adapter = "postgresql" if @adapter == "postgres"
 
           if @uri.opaque
@@ -220,10 +220,10 @@ module ActiveRecord
           # an environment key or a URL spec, so we have deprecated
           # this ambiguous behaviour and in the future this function
           # can be removed in favor of resolve_url_connection.
-          if configurations.key?(spec)
+          if configurations.key?(spec) || spec !~ /:/
             ActiveSupport::Deprecation.warn "Passing a string to ActiveRecord::Base.establish_connection " \
               "for a configuration lookup is deprecated, please pass a symbol (#{spec.to_sym.inspect}) instead"
-            resolve_connection(configurations[spec])
+            resolve_symbol_connection(spec)
           else
             resolve_url_connection(spec)
           end
diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index bbb866cedf..31e7390bf7 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -58,9 +58,9 @@ module ActiveRecord
     end
 
     class MergeAndResolveDefaultUrlConfig # :nodoc:
-      def initialize(raw_configurations, url = ENV['DATABASE_URL'])
+      def initialize(raw_configurations)
         @raw_config = raw_configurations.dup
-        @url        = url
+        @env = DEFAULT_ENV.call.to_s
       end
 
       # Returns fully resolved connection hashes.
@@ -71,33 +71,10 @@ module ActiveRecord
 
       private
         def config
-          if @url
-            raw_merged_into_default
-          else
-            @raw_config
-          end
-        end
-
-        def raw_merged_into_default
-          default = default_url_hash
-
-          @raw_config.each do |env, values|
-            default[env] = values || {}
-            default[env].merge!("url" => @url) { |h, v1, v2| v1 || v2 } if default[env].is_a?(Hash)
-          end
-          default
-        end
-
-        # When the raw configuration is not present and ENV['DATABASE_URL']
-        # is available we return a hash with the connection information in
-        # the connection URL. This hash responds to any string key with
-        # resolved connection information.
-        def default_url_hash
-          Hash.new do |hash, key|
-            hash[key] = if key.is_a? String
-               ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(@url).to_hash
-            else
-              nil
+          @raw_config.dup.tap do |cfg|
+            if url = ENV['DATABASE_URL']
+              cfg[@env] ||= {}
+              cfg[@env]["url"] ||= url
             end
           end
         end
diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb
index e097449029..f2d18e812d 100644
--- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb
+++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb
@@ -25,35 +25,45 @@ module ActiveRecord
         ConnectionSpecification::Resolver.new(klass.new(config).resolve).spec(spec)
       end
 
-      def test_resolver_with_database_uri_and_known_key
+      def test_resolver_with_database_uri_and_current_env_symbol_key
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
-        config   = { "production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
-        actual   = resolve(:production, config)
+        config   = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        actual   = resolve(:default_env, config)
         expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" }
         assert_equal expected, actual
       end
 
-      def test_resolver_with_database_uri_and_known_string_key
+      def test_resolver_with_database_uri_and_and_current_env_string_key
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
-        config   = { "production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
-        actual   = assert_deprecated { resolve("production", config) }
+        config   = { "default_env" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        actual   = assert_deprecated { resolve("default_env", config) }
         expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" }
         assert_equal expected, actual
       end
 
-      def test_resolver_with_database_uri_and_unknown_symbol_key
+      def test_resolver_with_database_uri_and_known_key
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
-        config   = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        config   = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } }
         actual   = resolve(:production, config)
-        expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" }
+        expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost" }
         assert_equal expected, actual
       end
 
-      def test_resolver_with_database_uri_and_unknown_string_key
+      def test_resolver_with_database_uri_and_unknown_symbol_key
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
         config   = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
         assert_raises AdapterNotSpecified do
-          spec("production", config)
+          resolve(:production, config)
+        end
+      end
+
+      def test_resolver_with_database_uri_and_unknown_string_key
+        ENV['DATABASE_URL'] = "postgres://localhost/foo"
+        config   = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        assert_deprecated do
+          assert_raises AdapterNotSpecified do
+            spec("production", config)
+          end
         end
       end
 
@@ -73,16 +83,24 @@ module ActiveRecord
 
       def test_environment_does_not_exist_in_config_url_does_exist
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
-        config      = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        config      = { "not_default_env" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
         actual      = klass.new(config).resolve
         expect_prod = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" }
-        assert_equal expect_prod, actual["production"]
+        assert_equal expect_prod, actual["default_env"]
+      end
+
+      def test_url_with_hyphenated_scheme
+        ENV['DATABASE_URL'] = "ibm-db://localhost/foo"
+        config   = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } }
+        actual   = resolve(:default_env, config)
+        expected = { "adapter"=>"ibm_db", "database"=>"foo", "host"=>"localhost" }
+        assert_equal expected, actual
       end
 
       def test_string_connection
-        config   = { "production" => "postgres://localhost/foo" }
+        config   = { "default_env" => "postgres://localhost/foo" }
         actual   = klass.new(config).resolve
-        expected = { "production" =>
+        expected = { "default_env" =>
                      { "adapter"  => "postgresql",
                        "database" => "foo",
                        "host"     => "localhost"
@@ -92,9 +110,9 @@ module ActiveRecord
       end
 
       def test_url_sub_key
-        config   = { "production" => { "url" => "postgres://localhost/foo" } }
+        config   = { "default_env" => { "url" => "postgres://localhost/foo" } }
         actual   = klass.new(config).resolve
-        expected = { "production" =>
+        expected = { "default_env" =>
                      { "adapter"  => "postgresql",
                        "database" => "foo",
                        "host"     => "localhost"
@@ -123,9 +141,10 @@ module ActiveRecord
         expected = { "adapter"  => "postgresql",
                      "database" => "foo",
                      "host"     => "localhost" }
-        assert_equal expected, actual["production"]
-        assert_equal expected, actual["development"]
-        assert_equal expected, actual["test"]
+        assert_equal expected, actual["default_env"]
+        assert_equal nil,      actual["production"]
+        assert_equal nil,      actual["development"]
+        assert_equal nil,      actual["test"]
         assert_equal nil,      actual[:production]
         assert_equal nil,      actual[:development]
         assert_equal nil,      actual[:test]
@@ -134,9 +153,9 @@ module ActiveRecord
       def test_url_sub_key_with_database_url
         ENV['DATABASE_URL'] = "NOT-POSTGRES://localhost/NOT_FOO"
 
-        config   = { "production" => { "url" => "postgres://localhost/foo" } }
+        config   = { "default_env" => { "url" => "postgres://localhost/foo" } }
         actual   = klass.new(config).resolve
-        expected = { "production" =>
+        expected = { "default_env" =>
                     { "adapter"  => "postgresql",
                        "database" => "foo",
                        "host"     => "localhost"
@@ -148,9 +167,9 @@ module ActiveRecord
       def test_merge_no_conflicts_with_database_url
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
 
-        config   = {"production" => { "pool" => "5" } }
+        config   = {"default_env" => { "pool" => "5" } }
         actual   = klass.new(config).resolve
-        expected = { "production" =>
+        expected = { "default_env" =>
                      { "adapter"  => "postgresql",
                        "database" => "foo",
                        "host"     => "localhost",
@@ -163,9 +182,9 @@ module ActiveRecord
       def test_merge_conflicts_with_database_url
         ENV['DATABASE_URL'] = "postgres://localhost/foo"
 
-        config   = {"production" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } }
+        config   = {"default_env" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } }
         actual   = klass.new(config).resolve
-        expected = { "production" =>
+        expected = { "default_env" =>
                      { "adapter"  => "postgresql",
                        "database" => "foo",
                        "host"     => "localhost",