Fix url config PR test failures

README.md

@@ -189,24 +189,42 @@ connections:
     url: 'mysql2://db_username:db_password@localhost:3306/db_name'
 ```
 
-However, `ENV['DATABASE_URL']` is not respected due to the need for multiple databases.
-
 We recommend, if using environmental variables, to interpolate them via ERb.
 
 ```yml
 connections:
   - role: master
     blacklist_duration: 0
-    url: <%= ENV['DATABASE_URL'] %>
+    url: <%= ENV['DATABASE_URL_MASTER'] %>
   - role: slave
     url: <%= ENV['DATABASE_URL_SLAVE'] %>
 ```
-For more information on url parsing, see
-[ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig](https://github.com/rails/rails/blob/4-0-stable/activerecord/lib/active_record/connection_handling.rb)
-and
-[ActiveRecord::Core::ConnectionSpecification::Resolver](https://github.com/rails/rails/blob/3-0-stable/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb)
-in Rails, as used by
-[ConfigParser](https://github.com/taskrabbit/makara/blob/master/lib/makara/config_parser.rb).
+
+**Important**: *Do NOT use `ENV['DATABASE_URL']`*, as it inteferes with the the database configuration
+initialization and may cause Makara not to complete the configuration.  For the moment, it is easier
+to use a different ENV variable than to hook into the database initialization in all the supported
+Rails.
+
+For more information on url parsing, as used in
+[ConfigParser](https://github.com/taskrabbit/makara/blob/master/lib/makara/config_parser.rb), see:
+
+- 3.0
+    [ActiveRecord::Base::ConnectionSpecification.new](https://github.com/rails/rails/blob/3-0-stable/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb#L3-L7)
+- 3.0
+    [ActiveRecord::Base::ConnectionSpecification.new](https://github.com/rails/rails/blob/3-1-stable/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb#L3-L7)
+- 3.2
+  [ActiveRecord::Base::ConnectionSpecification::Resolver.send(:connection_url_to_hash, url_config[:url])](https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb#L60-L77)
+- 4.0
+ [ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver.send(:connection_url_to_hash, url_config[:url])](https://github.com/rails/rails/blob/4-0-stable/activerecord/lib/active_record/connection_adapters/connection_specification.rb#L68-L92)
+  - [ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig](https://github.com/rails/rails/blob/4-0-stable/activerecord/lib/active_record/connection_handling.rb)
+- 4.1
+ [ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash](https://github.com/rails/rails/blob/4-1-stable/activerecord/lib/active_record/connection_adapters/connection_specification.rb#L17-L121)
+  - ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig.new(url_config).resolve
+- 4.2
+ [ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash](https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/connection_handling.rb#L60-L81)
+- master
+ [ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash](https://github.com/rails/rails/blob/97b980b4e61aea3cee429bdee4b2eae2329905cd/activerecord/lib/active_record/connection_handling.rb#L60-L81)
+
 
 
 ## Custom error matchers:

gemfiles/ar41.gemfile

@@ -9,7 +9,7 @@ gem 'activerecord', '4.1.0'
 gem 'rspec'
 gem 'rack', '1.6.0'
 gem 'timecop'
-gem 'mysql2', :platform => :ruby
+gem 'mysql2', '~> 0.3.10', :platform => :ruby
 gem 'activerecord-jdbcmysql-adapter', :platform => :jruby
 gem 'pg', :platform => :ruby
 gem 'activerecord-jdbcpostgresql-adapter', :platform => :jruby

lib/makara/config_parser.rb

@@ -26,35 +26,118 @@ class ConfigParser
       :sticky => true
     }
 
+    # ConnectionUrlResolver is borrowed from Rails 4-2 since its location and implementation
+    # vary slightly among Rails versions, but the behavior is the same.  Thus, borrowing the
+    # class should be the most future-safe way to parse a database url.
+    #
+    # Expands a connection string into a hash.
+    class ConnectionUrlResolver # :nodoc:
+
+      # == Example
+      #
+      #   url = "postgresql://foo:bar@localhost:9000/foo_test?pool=5&timeout=3000"
+      #   ConnectionUrlResolver.new(url).to_hash
+      #   # => {
+      #     "adapter"  => "postgresql",
+      #     "host"     => "localhost",
+      #     "port"     => 9000,
+      #     "database" => "foo_test",
+      #     "username" => "foo",
+      #     "password" => "bar",
+      #     "pool"     => "5",
+      #     "timeout"  => "3000"
+      #   }
+      def initialize(url)
+        raise "Database URL cannot be empty" if url.blank?
+        @uri     = URI.parse(url)
+        @adapter = @uri.scheme.tr('-', '_')
+        @adapter = "postgresql" if @adapter == "postgres"
+
+        if @uri.opaque
+          @uri.opaque, @query = @uri.opaque.split('?', 2)
+        else
+          @query = @uri.query
+        end
+      end
+
+      # Converts the given URL to a full connection hash.
+      def to_hash
+        config = raw_config.reject { |_,value| value.blank? }
+        config.map { |key,value| config[key] = URI.unescape(value) if value.is_a? String }
+        config
+      end
+
+      private
+
+      def uri
+        @uri
+      end
+
+      # Converts the query parameters of the URI into a hash.
+      #
+      #   "localhost?pool=5&reaping_frequency=2"
+      #   # => { "pool" => "5", "reaping_frequency" => "2" }
+      #
+      # returns empty hash if no query present.
+      #
+      #   "localhost"
+      #   # => {}
+      def query_hash
+        Hash[(@query || '').split("&").map { |pair| pair.split("=") }]
+      end
+
+      def raw_config
+        if uri.opaque
+          query_hash.merge({
+            "adapter"  => @adapter,
+            "database" => uri.opaque })
+        else
+          query_hash.merge({
+            "adapter"  => @adapter,
+            "username" => uri.user,
+            "password" => uri.password,
+            "port"     => uri.port,
+            "database" => database_from_path,
+            "host"     => uri.host })
+        end
+      end
+
+      # Returns name of the database.
+      def database_from_path
+        if @adapter == 'sqlite3'
+          # 'sqlite3:/foo' is absolute, because that makes sense. The
+          # corresponding relative version, 'sqlite3:foo', is handled
+          # elsewhere, as an "opaque".
+
+          uri.path
+        else
+          # Only SQLite uses a filename as the "database" name; for
+          # anything else, a leading slash would be silly.
+
+          uri.path.sub(%r{^/}, "")
+        end
+      end
+    end
+
     # NOTE: url format must be, e.g.
     # url: mysql2://...
     # NOT
     # url: mysql2_makara://...
     # since the '_' in the protocol (mysql2_makara) makes the URI invalid
     # NOTE: Does not use ENV['DATABASE_URL']
     def self.merge_and_resolve_default_url_config(config)
+      if ENV['DATABASE_URL']
+        Logging::Logger.log "Please rename DATABASE_URL to use in the database.yml", :warn
+      end
       return config unless config.key?(:url)
-      url_config = config.slice(:url)
-      url_config = connection_url_to_hash(url_config)
-      url_config = url_config[:url].symbolize_keys
+      url = config[:url]
+      url_config = ConnectionUrlResolver.new(url).to_hash
+      url_config = url_config.symbolize_keys
       url_config.delete(:adapter)
       config.delete(:url)
       config.update(url_config)
     end
 
-    def self.connection_url_to_hash(url_config)
-      if defined?(ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig)
-        ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig.new(url_config).resolve
-      elsif defined?(ActiveRecord::Core::ConnectionSpecification::Resolver)
-        {
-          :url =>
-          ActiveRecord::Core::ConnectionSpecification::Resolver.send(:connection_url_to_hash, url_config[:url])
-        }
-      else
-        fail 'Unknown connection_url_hash method'
-      end
-    end
-
     attr_reader :makara_config
 
     def initialize(config)