diff --git a/.github/workflows/database-e2e.sh b/.github/workflows/database-e2e.sh index c470f880..b531412f 100755 --- a/.github/workflows/database-e2e.sh +++ b/.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) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10e9f102..586ba33b 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/hanami/cli/commands/app/db/utils/database.rb b/lib/hanami/cli/commands/app/db/utils/database.rb index d4c11750..43c5be0f 100644 --- a/lib/hanami/cli/commands/app/db/utils/database.rb +++ b/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. diff --git a/lib/hanami/cli/commands/app/db/utils/sqlite.rb b/lib/hanami/cli/commands/app/db/utils/sqlite.rb index 11d7885c..ed1df63c 100644 --- a/lib/hanami/cli/commands/app/db/utils/sqlite.rb +++ b/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 diff --git a/lib/hanami/cli/errors.rb b/lib/hanami/cli/errors.rb index 46c26485..d961ec26 100644 --- a/lib/hanami/cli/errors.rb +++ b/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 diff --git a/spec/unit/hanami/cli/commands/app/db/utils/database_spec.rb b/spec/unit/hanami/cli/commands/app/db/utils/database_spec.rb new file mode 100644 index 00000000..2339e126 --- /dev/null +++ b/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