Use Hanami::DB::DatabaseURL in db commands

.github/workflows/database-e2e.sh

@@ -35,6 +35,11 @@ grep -q 'gem "hanami-cli", path:' testapp/Gemfile
 sed -i 's|^\([[:space:]]*\)gem "hanami-reloader".*|\1gem "hanami-reloader", github: "hanami/hanami-reloader", branch: "main"|' testapp/Gemfile
 echo 'gem "dry-cli", github: "dry-rb/dry-cli", branch: "main"' >> testapp/Gemfile
 
+# Use the same hanami-db as hanami-cli's own Gemfile, since hanami-cli may need unreleased hanami-db changes
+HANAMI_DB_GEM=$(grep '^gem "hanami-db"' "$GITHUB_WORKSPACE/Gemfile")
+sed -i "s|^\([[:space:]]*\)gem \"hanami-db\".*|\1$HANAMI_DB_GEM|" testapp/Gemfile
+grep -qF "$HANAMI_DB_GEM" testapp/Gemfile
+
 if [ "$JRUBY" = "true" ]; then
   case "$E2E_DATABASE_FLAG" in
     sqlite)

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Break Versioning](https://www.taoensso.com/break-ve
 ### Changed
 
 - Print clearer errors when Postgres CLI commands fail. (@mddelk in #437)
+- Raise `Hanami::CLI::UnsupportedDatabaseSchemeError` from `hanami db` commands for unsupported database URL schemes, and the new `Hanami::CLI::InvalidDatabaseURLError` for database URLs that cannot be parsed, rather than a `RuntimeError` or `URI::InvalidURIError`. Neither error includes the URL, since it may contain credentials. (@parndt in #463)
 - Ignore SQLite's `-wal`, `-shm` and `-journal` files in the `.gitignore` generated by `hanami new`, by widening `db/*.sqlite` to `db/*.sqlite*`. (@cllns in #451)
 - Use Dry CLI 2.0 (@timriley)
 - Commands now use the I/O streams given by Dry CLI 2.0's `Dry::CLI::Command`, and take `stdout:`, `stderr:` and `stdin:` in place of `out:` and `err:`. (@timriley)

lib/hanami/cli/commands/app/db/utils/database.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require "uri"
 require "delegate"
 
 module Hanami
@@ -23,35 +22,57 @@ def sql
                 end
               end
 
-              DATABASE_CLASS_RESOLVER = Hash.new { |_, key|
-                raise "#{key} is not a supported db scheme"
-              }.update(
-                "sqlite" => -> {
+              DATABASE_CLASS_RESOLVER = {
+                sqlite: -> {
                   require_relative("sqlite")
                   Sqlite
                 },
-                "postgres" => -> {
+                postgres: -> {
                   require_relative("postgres")
                   Postgres
                 },
-                "postgresql" => -> {
-                  require_relative("postgres")
-                  Postgres
-                },
-                "mysql" => -> {
-                  require_relative("mysql")
-                  Mysql
-                },
-                "mysql2" => -> {
+                mysql: -> {
                   require_relative("mysql")
                   Mysql
                 }
-              ).freeze
+              }.freeze
+
+              # Matches a URL's scheme, including any JDBC subprotocol, e.g. "jdbc:sqlserver"
+              DATABASE_SCHEME_MATCHER = /\A(?:jdbc:)?[a-z][a-z0-9+.-]*(?=:)/i
+              private_constant :DATABASE_SCHEME_MATCHER
 
               def self.database_class(database_url)
-                database_scheme = URI(database_url.delete_prefix("jdbc:")).scheme
-                DATABASE_CLASS_RESOLVER[database_scheme].call
+                # Required here rather than at the top of this file, since every app command loads
+                # this file, including in apps that do not use hanami-db.
+                require "hanami-db"
+
+                adapter = Hanami::DB::DatabaseURL.adapter(database_url)
+
+                DATABASE_CLASS_RESOLVER.fetch(adapter) {
+                  scheme = database_scheme(database_url)
+
+                  raise Hanami::CLI::InvalidDatabaseURLError.new(scheme) unless parseable?(database_url)
+
+                  raise Hanami::CLI::UnsupportedDatabaseSchemeError.new(scheme)
+                }.call
+              end
+
+              # Returns false when the URL cannot be parsed. Avoids raising from a `rescue`, since
+              # the URI error (whose message includes the URL) would become the new error's cause.
+              def self.parseable?(database_url)
+                Hanami::DB::DatabaseURL.uri(database_url)
+                true
+              rescue URI::Error
+                false
+              end
+              private_class_method :parseable?
+
+              # Returns the URL's scheme for use in error messages. Unlike parsing the URL, this
+              # cannot fail, and never exposes the credentials the URL may contain.
+              def self.database_scheme(database_url)
+                database_url.to_s[DATABASE_SCHEME_MATCHER] || "(none)"
               end
+              private_class_method :database_scheme
 
               def self.from_slice(slice:, system_call:)
                 provider = slice.container.providers[:db]
@@ -87,10 +108,10 @@ def database_url
                 slice.container.providers[:db].source.database_urls.fetch(gateway_name)
               end
 
-              # JDBC URLs (required to connect via JRuby) nest the real URL after "jdbc:", e.g.
-              # "jdbc:postgresql://localhost/app", so parse that nested URL instead.
+              # Parses the URL nested in JDBC URLs (required to connect via JRuby), e.g.
+              # "jdbc:postgresql://localhost/app".
               def database_uri
-                @database_uri ||= URI(database_url.delete_prefix("jdbc:"))
+                @database_uri ||= Hanami::DB::DatabaseURL.uri(database_url)
               end
 
               # JDBC drivers expect the user and password as query params, e.g.

lib/hanami/cli/commands/app/db/utils/sqlite.rb

@@ -72,7 +72,7 @@ def name
                 @name ||=
                   begin
                     raw =
-                      if database_url.start_with?("jdbc:")
+                      if Hanami::DB::DatabaseURL.jdbc?(database_url)
                         # For JDBC SQLite URIs like "jdbc:sqlite:db/app.sqlite3",
                         # we need to extract the path part after "jdbc:sqlite:"
                         # The standard URI.parse doesn't handle JDBC URIs well, so we remove the prefix manually

lib/hanami/cli/errors.rb

@@ -119,6 +119,19 @@ def initialize(scheme)
       end
     end
 
+    # Does not include the URL itself, since it may contain credentials.
+    #
+    # @since 3.1.0
+    # @api public
+    class InvalidDatabaseURLError < Error
+      def initialize(scheme)
+        super(
+          "`#{scheme}' database URL could not be parsed. " \
+          "Check it for characters that need escaping, such as in the password"
+        )
+      end
+    end
+
     # rubocop:disable Layout/LineLength
     # @since 2.2.0
     # @api public

spec/unit/hanami/cli/commands/app/db/utils/database_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "hanami/cli/commands/app/db/utils/database"
+
+RSpec.describe Hanami::CLI::Commands::App::DB::Utils::Database do
+  describe ".database_class" do
+    {
+      "sqlite://db/app.sqlite3" => "Sqlite",
+      "jdbc:sqlite:db/app.sqlite3" => "Sqlite",
+      "postgres://localhost/app" => "Postgres",
+      "postgresql://localhost/app" => "Postgres",
+      "jdbc:postgresql://localhost/app" => "Postgres",
+      "mysql://localhost/app" => "Mysql",
+      "mysql2://localhost/app" => "Mysql",
+      "jdbc:mysql://localhost/app" => "Mysql"
+    }.each do |url, class_name|
+      it "returns #{class_name} for #{url}" do
+        expect(described_class.database_class(url).name).to eq "Hanami::CLI::Commands::App::DB::Utils::#{class_name}"
+      end
+    end
+
+    it "raises an error for unsupported schemes, without including the URL's credentials" do
+      expect { described_class.database_class("oracle://user:secret@localhost/app") }
+        .to raise_error(Hanami::CLI::UnsupportedDatabaseSchemeError, "`oracle' is not a supported db scheme")
+    end
+
+    it "includes the JDBC subprotocol in the error for unsupported JDBC URLs" do
+      expect { described_class.database_class("jdbc:sqlserver://localhost;user=sa;password=secret") }
+        .to raise_error(Hanami::CLI::UnsupportedDatabaseSchemeError, "`jdbc:sqlserver' is not a supported db scheme")
+    end
+
+    it "raises an error for unsupported JDBC subprotocols" do
+      expect { described_class.database_class("jdbc:postgres://localhost/app") }
+        .to raise_error(Hanami::CLI::UnsupportedDatabaseSchemeError, "`jdbc:postgres' is not a supported db scheme")
+    end
+
+    it "raises an error for URLs that cannot be parsed, without including the URL's credentials" do
+      expect { described_class.database_class("postgres://user:se cret@localhost/app") }
+        .to raise_error(Hanami::CLI::InvalidDatabaseURLError) { |error|
+          expect(error.message).to start_with "`postgres' database URL could not be parsed"
+          expect(error.message).not_to include "cret"
+          expect(error.cause).to be_nil
+        }
+    end
+
+    it "raises an error for URLs without a scheme" do
+      expect { described_class.database_class("db/app.sqlite3") }
+        .to raise_error(Hanami::CLI::UnsupportedDatabaseSchemeError, "`(none)' is not a supported db scheme")
+    end
+  end
+end