From 7429bc58a0dffa94636b21cc0cba1d19a5ae7a84 Mon Sep 17 00:00:00 2001 From: Jeremy Baker Date: Thu, 4 Feb 2016 01:32:45 -0800 Subject: [PATCH 1/3] Remove the assumption of schema in DATABASE_URL If you set the DATABASE_URL environment variable to `mydatabase` by accident, you end up getting a series of errors that are hard to trace. For example: ``` warning: already initialized constant ActiveRecord::Base::OrmAdapter ``` Turns out the cascade of errors is due to the error raised by `.tr` being called on `nil`. This commit makes sure that `scheme` is set before calling `.tr` on it. My previous iteration used `@uri.scheme.try(:tr, '-', '_')` but using the `&&` logical operator is a fair bit faster: http://stackoverflow.com/questions/26655032/try-vs-performance With this change, the error message becomes much more understandable: ``` FATAL: database "mydatabase" does not exist (ActiveRecord::NoDatabaseError) ``` --- .../connection_adapters/connection_specification.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index f633892dee4..4bc64473680 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_parser.parse(url) - @adapter = @uri.scheme.tr('-', '_') + @adapter = @uri.scheme && @uri.scheme.tr('-', '_') @adapter = "postgresql" if @adapter == "postgres" if @uri.opaque From d871696b4a8b021b67a3ce6c3cb5d5dcbbb578fe Mon Sep 17 00:00:00 2001 From: Jeremy Baker Date: Thu, 4 Feb 2016 10:59:52 -0800 Subject: [PATCH 2/3] Add a resolver test for the missing scheme --- .../connection_adapters/connection_specification_test.rb | 5 +++++ .../test/cases/connection_specification/resolver_test.rb | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/activerecord/test/cases/connection_adapters/connection_specification_test.rb b/activerecord/test/cases/connection_adapters/connection_specification_test.rb index ea2196cda2d..99fe7020ea0 100644 --- a/activerecord/test/cases/connection_adapters/connection_specification_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_specification_test.rb @@ -7,6 +7,11 @@ module ActiveRecord spec = ConnectionSpecification.new({ :a => :b }, "bar") assert_not_equal(spec.config.object_id, spec.dup.config.object_id) end + + def test_handle_missing_scheme + spec = ConnectionSpecification.new({ :url => 'testing' }, "bar") + assert_not_equal(spec.config.object_id, spec.dup.config.object_id) + end end end end diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb index 3c2f5d4219d..358b6ad5370 100644 --- a/activerecord/test/cases/connection_specification/resolver_test.rb +++ b/activerecord/test/cases/connection_specification/resolver_test.rb @@ -57,6 +57,12 @@ module ActiveRecord "encoding" => "utf8" }, spec) end + def test_url_missing_scheme + spec = resolve 'foo' + assert_equal({ + "database" => "foo" }, spec) + end + def test_url_host_db spec = resolve 'abstract://foo/bar?encoding=utf8' assert_equal({ From 96fdbd3be3e6c5c57e394019e8b812da6d705769 Mon Sep 17 00:00:00 2001 From: Jeremy Baker Date: Thu, 4 Feb 2016 11:13:05 -0800 Subject: [PATCH 3/3] Remove accidental additional test --- .../connection_adapters/connection_specification_test.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/activerecord/test/cases/connection_adapters/connection_specification_test.rb b/activerecord/test/cases/connection_adapters/connection_specification_test.rb index 99fe7020ea0..ea2196cda2d 100644 --- a/activerecord/test/cases/connection_adapters/connection_specification_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_specification_test.rb @@ -7,11 +7,6 @@ module ActiveRecord spec = ConnectionSpecification.new({ :a => :b }, "bar") assert_not_equal(spec.config.object_id, spec.dup.config.object_id) end - - def test_handle_missing_scheme - spec = ConnectionSpecification.new({ :url => 'testing' }, "bar") - assert_not_equal(spec.config.object_id, spec.dup.config.object_id) - end end end end