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..76c82f0d 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 print a clear error for database URLs that cannot be parsed, rather than raising 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/Gemfile b/Gemfile index a51f9ca2..07a9d53b 100644 --- a/Gemfile +++ b/Gemfile @@ -15,7 +15,7 @@ end gem "hanami", github: "hanami/hanami", branch: "main" gem "hanami-assets", github: "hanami/hanami-assets", branch: "main" gem "hanami-action", github: "hanami/hanami-action", branch: "main" -gem "hanami-db", github: "hanami/hanami-db", branch: "main" +gem "hanami-db", github: "hanami/hanami-db", branch: "invalid-database-url-error" gem "hanami-router", github: "hanami/hanami-router", branch: "main" gem "hanami-utils", github: "hanami/hanami-utils", branch: "main" diff --git a/lib/hanami/cli/commands/app/db/utils/database.rb b/lib/hanami/cli/commands/app/db/utils/database.rb index d4c11750..b68d27a1 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,36 +22,45 @@ 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 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) { + ensure_parseable(database_url) + + scheme = Hanami::DB::DatabaseURL.scheme(database_url) || "(none)" + raise Hanami::CLI::UnsupportedDatabaseSchemeError.new(scheme) + }.call end + # Re-raises hanami-db's error for URLs that cannot be parsed as a CLI error, so the CLI + # prints its message rather than a backtrace. + def self.ensure_parseable(database_url) + Hanami::DB::DatabaseURL.uri(database_url) + rescue Hanami::DB::InvalidDatabaseURLError => exception + raise Hanami::CLI::Error, exception.message + end + private_class_method :ensure_parseable + def self.from_slice(slice:, system_call:) provider = slice.container.providers[:db] raise "No :db provider for #{slice}" unless provider @@ -87,10 +95,13 @@ 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". + # + # Needs no handling for URLs that cannot be parsed, since `.database_class` has already + # rejected them. 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/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..5b305f0c --- /dev/null +++ b/spec/unit/hanami/cli/commands/app/db/utils/database_spec.rb @@ -0,0 +1,52 @@ +# 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::Error) { |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_an_instance_of Hanami::DB::InvalidDatabaseURLError + expect(error.cause.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