Merge pull request #14569 from matthewd/sqlite_relative_deprecated

Revise 'sqlite3:' URL handling for smoother upgrades

Conflicts:
	activerecord/CHANGELOG.md

activerecord/CHANGELOG.md

+*   Switch `sqlite3:///` URLs (which were temporarily
+    deprecated in 4.1) from relative to absolute.
+
+    If you still want the previous interpretation, you should replace
+    `sqlite3:///my/path` with `sqlite3:my/path`.
+
+    *Matthew Draper*
+
 *   Treat blank UUID values as `nil`.
 
     Example:

activerecord/lib/active_record/connection_adapters/connection_specification.rb

@@ -35,7 +35,12 @@ def initialize(url)
           @uri     = URI.parse(url)
           @adapter = @uri.scheme
           @adapter = "postgresql" if @adapter == "postgres"
-          @query   = @uri.query || ''
+
+          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.
@@ -65,30 +70,38 @@ def uri_parser
         #   "localhost"
         #   # => {}
         def query_hash
-          Hash[@query.split("&").map { |pair| pair.split("=") }]
+          Hash[(@query || '').split("&").map { |pair| pair.split("=") }]
         end
 
         def raw_config
-          query_hash.merge({
-            "adapter"  => @adapter,
-            "username" => uri.user,
-            "password" => uri.password,
-            "port"     => uri.port,
-            "database" => database,
-            "host"     => uri.host })
+          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.
-        # Sqlite3 expects this to be a full path or `:memory:`.
-        def database
+        def database_from_path
           if @adapter == 'sqlite3'
-            if '/:memory:' == uri.path
-              ':memory:'
-            else
-              uri.path
-            end
+            # 'sqlite3:/foo' is absolute, because that makes sense. The
+            # corresponding relative version, 'sqlite3:foo', is handled
+            # elsewhere, as an "opaque".
+
+            uri.path
           else
-            uri.path.sub(%r{^/},"")
+            # 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

activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb

@@ -31,7 +31,7 @@ def test_bad_connection
         def test_connect_with_url
           original_connection = ActiveRecord::Base.remove_connection
           tf = Tempfile.open 'whatever'
-          url = "sqlite3://#{tf.path}"
+          url = "sqlite3:#{tf.path}"
           ActiveRecord::Base.establish_connection(url)
           assert ActiveRecord::Base.connection
         ensure
@@ -42,7 +42,7 @@ def test_connect_with_url
 
         def test_connect_memory_with_url
           original_connection = ActiveRecord::Base.remove_connection
-          url = "sqlite3:///:memory:"
+          url = "sqlite3::memory:"
           ActiveRecord::Base.establish_connection(url)
           assert ActiveRecord::Base.connection
         ensure

activerecord/test/cases/connection_specification/resolver_test.rb

@@ -82,15 +82,34 @@ def test_encoded_password
           assert_equal password, spec["password"]
         end
 
-        def test_url_host_db_for_sqlite3
-          spec = resolve 'sqlite3://foo:bar@dburl:9000/foo_test'
+        def test_url_with_authority_for_sqlite3
+          spec = resolve 'sqlite3:///foo_test'
           assert_equal('/foo_test', spec["database"])
         end
 
-        def test_url_host_memory_db_for_sqlite3
-          spec = resolve 'sqlite3://foo:bar@dburl:9000/:memory:'
+        def test_url_absolute_path_for_sqlite3
+          spec = resolve 'sqlite3:/foo_test'
+          assert_equal('/foo_test', spec["database"])
+        end
+
+        def test_url_relative_path_for_sqlite3
+          spec = resolve 'sqlite3:foo_test'
+          assert_equal('foo_test', spec["database"])
+        end
+
+        def test_url_memory_db_for_sqlite3
+          spec = resolve 'sqlite3::memory:'
           assert_equal(':memory:', spec["database"])
         end
+
+        def test_url_sub_key_for_sqlite3
+          spec = resolve :production, 'production' => {"url" => 'sqlite3:foo?encoding=utf8'}
+          assert_equal({
+            "adapter"  => "sqlite3",
+            "database" => "foo",
+            "encoding" => "utf8" }, spec)
+        end
+
       end
     end
   end

activerecord/test/cases/fixtures_test.rb

@@ -254,7 +254,7 @@ def test_serialized_fixtures
 
   def test_fixtures_are_set_up_with_database_env_variable
     db_url_tmp = ENV['DATABASE_URL']
-    ENV['DATABASE_URL'] = "sqlite3:///:memory:"
+    ENV['DATABASE_URL'] = "sqlite3::memory:"
     ActiveRecord::Base.stubs(:configurations).returns({})
     test_case = Class.new(ActiveRecord::TestCase) do
       fixtures :accounts

guides/source/configuring.md

@@ -580,13 +580,13 @@ The only way to explicitly not use the connection information in `ENV['DATABASE_
 ```
 $ cat config/database.yml
 development:
-  url: sqlite3://localhost/NOT_my_database
+  url: sqlite3:NOT_my_database
 
 $ echo $DATABASE_URL
 postgresql://localhost/my_database
 
 $ rails runner 'puts ActiveRecord::Base.connections'
-{"development"=>{"adapter"=>"sqlite3", "host"=>"localhost", "database"=>"NOT_my_database"}}
+{"development"=>{"adapter"=>"sqlite3", "database"=>"NOT_my_database"}}
 ```
 
 Here the connection information in `ENV['DATABASE_URL']` is ignored, note the different adapter and database name.

railties/lib/rails/generators/rails/app/templates/config/databases/sqlite3.yml

@@ -23,8 +23,10 @@ test:
 # Do not keep production credentials in the repository,
 # instead read the configuration from the environment.
 #
-# Example:
-#   sqlite3://myuser:mypass@localhost/full/path/to/somedatabase
+# Examples:
+#   sqlite3::memory:
+#   sqlite3:db/production.sqlite3
+#   sqlite3:/full/path/to/database.sqlite3
 #
 production:
   url: <%%= ENV["DATABASE_URL"] %>

railties/test/application/initializers/frameworks_test.rb

@@ -216,8 +216,8 @@ def from_bar_helper
         require "#{app_path}/config/environment"
         orig_database_url = ENV.delete("DATABASE_URL")
         orig_rails_env, Rails.env = Rails.env, 'development'
-        database_url_db_name = File.join(app_path, "db/database_url_db.sqlite3")
-        ENV["DATABASE_URL"] = "sqlite3://:@localhost/#{database_url_db_name}"
+        database_url_db_name = "db/database_url_db.sqlite3"
+        ENV["DATABASE_URL"] = "sqlite3:#{database_url_db_name}"
         ActiveRecord::Base.establish_connection
         assert ActiveRecord::Base.connection
         assert_match(/#{database_url_db_name}/, ActiveRecord::Base.connection_config[:database])

railties/test/application/rake/dbs_test.rb

@@ -17,11 +17,11 @@ def teardown
       end
 
       def database_url_db_name
-        File.join(app_path, "db/database_url_db.sqlite3")
+        "db/database_url_db.sqlite3"
       end
 
       def set_database_url
-        ENV['DATABASE_URL'] = File.join("sqlite3://:@localhost", database_url_db_name)
+        ENV['DATABASE_URL'] = "sqlite3:#{database_url_db_name}"
         # ensure it's using the DATABASE_URL
         FileUtils.rm_rf("#{app_path}/config/database.yml")
       end