Move connection resoluion logic to it's own testable class.

3 files changed, 115 insertions, 92 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 65570da0d9..f2e7f88011 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -5,6 +5,78 @@ module ActiveRecord
       def initialize (config, adapter_method)
         @config, @adapter_method = config, adapter_method
       end
+
+      ##
+      # Builds a ConnectionSpecification from user input
+      class Resolver # :nodoc:
+        attr_reader :config, :klass, :configurations
+
+        def initialize(config, klass, configurations)
+          @config         = config
+          @klass          = klass
+          @configurations = configurations
+        end
+
+        def spec
+          case config
+          when nil
+            raise AdapterNotSpecified unless defined?(Rails.env)
+            resolve_string_connection Rails.env
+          when Symbol, String
+            resolve_string_connection config.to_s
+          when Hash
+            resolve_hash_connection config
+          end
+        end
+
+        private
+        def resolve_string_connection(spec) # :nodoc:
+          hash = configurations.fetch(spec) do |k|
+            connection_url_to_hash(k)
+          end
+
+          raise(AdapterNotSpecified, "#{spec} database is not configured") unless hash
+
+          resolve_hash_connection hash
+        end
+
+        def resolve_hash_connection(spec) # :nodoc:
+          spec = spec.symbolize_keys
+
+          raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter)
+
+          begin
+            require "active_record/connection_adapters/#{spec[:adapter]}_adapter"
+          rescue LoadError => e
+            raise LoadError, "Please install the #{spec[:adapter]} adapter: `gem install activerecord-#{spec[:adapter]}-adapter` (#{e.message})", e.backtrace
+          end
+
+          adapter_method = "#{spec[:adapter]}_connection"
+          unless klass.respond_to?(adapter_method)
+            raise AdapterNotFound, "database configuration specifies nonexistent #{spec[:adapter]} adapter"
+          end
+
+          ConnectionSpecification.new(spec, adapter_method)
+        end
+
+        def connection_url_to_hash(url) # :nodoc:
+          config = URI.parse url
+          adapter = config.scheme
+          adapter = "postgresql" if adapter == "postgres"
+          spec = { :adapter  => adapter,
+                   :username => config.user,
+                   :password => config.password,
+                   :port     => config.port,
+                   :database => config.path.sub(%r{^/},""),
+                   :host     => config.host }
+          spec.reject!{ |_,value| !value }
+          if config.query
+            options = Hash[config.query.split("&").map{ |pair| pair.split("=") }].symbolize_keys
+            spec.merge!(options)
+          end
+          spec
+        end
+      end
     end
 
     ##
@@ -55,65 +127,9 @@ module ActiveRecord
     # The exceptions AdapterNotSpecified, AdapterNotFound and ArgumentError
     # may be returned on an error.
     def self.establish_connection(spec = ENV["DATABASE_URL"])
-      config = case spec
-               when nil
-                 raise AdapterNotSpecified unless defined?(Rails.env)
-                 resolve_string_connection Rails.env
-               when Symbol, String
-                 resolve_string_connection spec.to_s
-               when Hash
-                 resolve_hash_connection spec
-               end
-
-      connection_handler.establish_connection(name, config)
-    end
-
-    def self.resolve_string_connection(spec) # :nodoc:
-      hash = configurations.fetch(spec) do |k|
-        connection_url_to_hash(k)
-      end
-
-      raise(AdapterNotSpecified, "#{spec} database is not configured") unless hash
-
-      resolve_hash_connection hash
-    end
-
-    def self.resolve_hash_connection(spec) # :nodoc:
-      spec = spec.symbolize_keys
-
-      raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter)
-
-      begin
-        require "active_record/connection_adapters/#{spec[:adapter]}_adapter"
-      rescue LoadError => e
-        raise LoadError, "Please install the #{spec[:adapter]} adapter: `gem install activerecord-#{spec[:adapter]}-adapter` (#{e.message})", e.backtrace
-      end
-
-      adapter_method = "#{spec[:adapter]}_connection"
-      unless respond_to?(adapter_method)
-        raise AdapterNotFound, "database configuration specifies nonexistent #{spec[:adapter]} adapter"
-      end
-
+      resolver = ConnectionSpecification::Resolver.new spec, self, configurations
       remove_connection
-      ConnectionSpecification.new(spec, adapter_method)
-    end
-
-    def self.connection_url_to_hash(url) # :nodoc:
-      config = URI.parse url
-      adapter = config.scheme
-      adapter = "postgresql" if adapter == "postgres"
-      spec = { :adapter  => adapter,
-               :username => config.user,
-               :password => config.password,
-               :port     => config.port,
-               :database => config.path.sub(%r{^/},""),
-               :host     => config.host }
-      spec.reject!{ |_,value| !value }
-      if config.query
-        options = Hash[config.query.split("&").map{ |pair| pair.split("=") }].symbolize_keys
-        spec.merge!(options)
-      end
-      spec
+      connection_handler.establish_connection name, resolver.spec
     end
 
     class << self
diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb
index 2ce61f214b..a1d1177289 100644
--- a/activerecord/test/cases/connection_management_test.rb
+++ b/activerecord/test/cases/connection_management_test.rb
@@ -25,40 +25,6 @@ module ActiveRecord
         assert ActiveRecord::Base.connection_handler.active_connections?
       end
 
-      class FakeBase < ActiveRecord::Base
-        def self.establish_connection spec
-          String === spec ? super : spec
-        end
-      end
-
-      def test_url_host_no_db
-        spec = FakeBase.connection_url_to_hash 'postgres://foo?encoding=utf8'
-        assert_equal({
-          :adapter  => "postgresql",
-          :database => "",
-          :host     => "foo",
-          :encoding => "utf8" }, spec)
-      end
-
-      def test_url_host_db
-        spec = FakeBase.connection_url_to_hash 'postgres://foo/bar?encoding=utf8'
-        assert_equal({
-          :adapter  => "postgresql",
-          :database => "bar",
-          :host     => "foo",
-          :encoding => "utf8" }, spec)
-      end
-
-      def test_url_port
-        spec = FakeBase.connection_url_to_hash 'postgres://foo:123?encoding=utf8'
-        assert_equal({
-          :adapter  => "postgresql",
-          :database => "",
-          :port     => 123,
-          :host     => "foo",
-          :encoding => "utf8" }, spec)
-      end
-
       def test_app_delegation
         manager = ConnectionManagement.new(@app)
 
diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb
new file mode 100644
index 0000000000..1ca9a00c85
--- /dev/null
+++ b/activerecord/test/cases/connection_specification/resolver_test.rb
@@ -0,0 +1,41 @@
+require "cases/helper"
+
+module ActiveRecord
+  class Base
+    class ConnectionSpecification
+      class ResolverTest < ActiveRecord::TestCase
+        def resolve(spec)
+          Resolver.new(spec, ActiveRecord::Base, {}).spec.config
+        end
+
+        def test_url_host_no_db
+          spec = resolve 'postgres://foo?encoding=utf8'
+          assert_equal({
+            :adapter  => "postgresql",
+            :database => "",
+            :host     => "foo",
+            :encoding => "utf8" }, spec)
+        end
+
+        def test_url_host_db
+          spec = resolve 'postgres://foo/bar?encoding=utf8'
+          assert_equal({
+            :adapter  => "postgresql",
+            :database => "bar",
+            :host     => "foo",
+            :encoding => "utf8" }, spec)
+        end
+
+        def test_url_port
+          spec = resolve 'postgres://foo:123?encoding=utf8'
+          assert_equal({
+            :adapter  => "postgresql",
+            :database => "",
+            :port     => 123,
+            :host     => "foo",
+            :encoding => "utf8" }, spec)
+        end
+      end
+    end
+  end
+end