Deprecate use of string in establish_connection as connection lookup

kushkella · Dec 24, 2013 · ec11807 · ec11807
1 parent d2ed433
commit ec11807
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 15 deletions.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecated use of string argument as a configuration lookup in `ActiveRecord::Base.establish_connection`. Instead, a symbol must be given.
+
+    *José Valim*
+
 *   Fixed `update_column`, `update_columns`, and `update_all` to correctly serialize
     values for `array`, `hstore` and `json` column types in PostgreSQL.
 

diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
@@ -26,7 +26,7 @@ def resolve(config)
           if config
             resolve_connection config
           elsif defined?(Rails.env)
-            resolve_env_connection Rails.env
+            resolve_env_connection Rails.env.to_sym
           else
             raise AdapterNotSpecified
           end
@@ -55,22 +55,29 @@ def spec(config)
         def resolve_connection(spec) #:nodoc:
           case spec
           when Symbol, String
-            resolve_env_connection spec.to_s
+            resolve_env_connection spec
           when Hash
             resolve_hash_connection spec
           end
         end
 
         def resolve_env_connection(spec) # :nodoc:
           # Rails has historically accepted a string to mean either
-          # an environment key or a url spec. So we support both for
-          # now but it would be nice to limit the environment key only
-          # for symbols.
-          config = configurations.fetch(spec.to_s) do
-            resolve_string_connection(spec) if spec.is_a?(String)
+          # 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_string_connection and
+          # resolve_symbol_connection.
+          if config = configurations[spec.to_s]
+            if spec.is_a?(String)
+              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"
+            end
+            resolve_connection(config)
+          elsif spec.is_a?(String)
+            resolve_string_connection(spec)
+          else
+            raise(AdapterNotSpecified, "#{spec} database is not configured")
           end
-          raise(AdapterNotSpecified, "#{spec} database is not configured") unless config
-          resolve_connection(config)
         end
 
         def resolve_hash_connection(spec) # :nodoc:

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
@@ -32,6 +32,13 @@ module ConnectionHandling
     #     "postgres://myuser:mypass@localhost/somedatabase"
     #   )
     #
+    # In case <tt>ActiveRecord::Base.configurations</tt> is set (Rails
+    # automatically loads the contents of config/database.yml into it),
+    # a symbol can also be given as argument, representing a key in the
+    # configuration hash:
+    #
+    #   ActiveRecord::Base.establish_connection(:production)
+    #
     # The exceptions AdapterNotSpecified, AdapterNotFound and ArgumentError
     # may be returned on an error.
     def establish_connection(spec = ENV["DATABASE_URL"])

diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
@@ -191,7 +191,7 @@ class Klass < ActiveRecord::Base
     end
 
     def setup
-      Klass.establish_connection 'arunit'
+      Klass.establish_connection :arunit
       @connection = Klass.connection
     end
 

diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb
@@ -101,7 +101,7 @@ def test_associations_should_work_when_model_has_no_connection
           College.first.courses.first
         end
       ensure
-        ActiveRecord::Base.establish_connection 'arunit'
+        ActiveRecord::Base.establish_connection :arunit
       end
     end
   end

diff --git a/activerecord/test/cases/transaction_isolation_test.rb b/activerecord/test/cases/transaction_isolation_test.rb
@@ -28,8 +28,8 @@ class Tag2 < ActiveRecord::Base
     end
 
     setup do
-      Tag.establish_connection 'arunit'
-      Tag2.establish_connection 'arunit'
+      Tag.establish_connection :arunit
+      Tag2.establish_connection :arunit
       Tag.destroy_all
     end
 

diff --git a/activerecord/test/support/connection.rb b/activerecord/test/support/connection.rb
@@ -15,7 +15,7 @@ def self.connect
     puts "Using #{connection_name}"
     ActiveRecord::Base.logger = ActiveSupport::Logger.new("debug.log", 0, 100 * 1024 * 1024)
     ActiveRecord::Base.configurations = connection_config
-    ActiveRecord::Base.establish_connection 'arunit'
-    ARUnit2Model.establish_connection 'arunit2'
+    ActiveRecord::Base.establish_connection :arunit
+    ARUnit2Model.establish_connection :arunit2
   end
 end