From af5a66203ce98eac9fd1b3d15cf897cead162f95 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Fri, 23 Jun 2023 15:26:28 -0500 Subject: [PATCH] fix: support multiple databases We used to monkeypatch freely some classes. However, Rails is now supporting multiple databases, and these patches are in the way of this usage. Fixes #251 --- CHANGELOG.md | 7 ++--- bin/console | 1 + .../connection_adapters/cockroachdb/column.rb | 6 +---- .../cockroachdb/quoting.rb | 6 +++++ .../cockroachdb/schema_statements.rb | 2 +- .../connection_adapters/cockroachdb/type.rb | 7 +++-- test/cases/connection_adapters/type_test.rb | 26 +++++++++++++++++++ .../PostgreSQLAdapter/QuotingTest.rb | 9 ++++--- 8 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 test/cases/connection_adapters/type_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index d3854837..3e59ff52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,10 @@ ## Ongoing -- Add support for sql load in rake tasks (#275). -- Add support for sql dump in rake tasks (#273). -- Add support for table optimize hints (#266). +- Fix Multiple Database connections ([#283](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/)). +- Add support for sql load in rake tasks ([#275](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/)). +- Add support for sql dump in rake tasks ([#273](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/)). +- Add support for table optimize hints ([#266](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/)). ## 7.0.2 - 2023-05-23 diff --git a/bin/console b/bin/console index d4e395a5..a88aae76 100755 --- a/bin/console +++ b/bin/console @@ -15,6 +15,7 @@ require "active_record/connection_adapters/cockroachdb/database_tasks" begin retried = false ActiveRecord::Base.establish_connection( + #Alternative version: "cockroachdb://root@localhost:26257/ar_crdb_console" adapter: "cockroachdb", host: "localhost", port: 26257, diff --git a/lib/active_record/connection_adapters/cockroachdb/column.rb b/lib/active_record/connection_adapters/cockroachdb/column.rb index 4f22b18a..db37d875 100644 --- a/lib/active_record/connection_adapters/cockroachdb/column.rb +++ b/lib/active_record/connection_adapters/cockroachdb/column.rb @@ -1,7 +1,7 @@ module ActiveRecord module ConnectionAdapters module CockroachDB - module PostgreSQLColumnMonkeyPatch + class Column < PostgreSQLColumn # most functions taken from activerecord-postgis-adapter spatial_column # https://github.com/rgeo/activerecord-postgis-adapter/blob/master/lib/active_record/connection_adapters/postgis/spatial_column.rb def initialize(name, default, sql_type_metadata = nil, null = true, @@ -93,9 +93,5 @@ def to_type_name(geometric_type) end end end - - class PostgreSQLColumn - prepend CockroachDB::PostgreSQLColumnMonkeyPatch - end end end diff --git a/lib/active_record/connection_adapters/cockroachdb/quoting.rb b/lib/active_record/connection_adapters/cockroachdb/quoting.rb index 8cea5789..289cc159 100644 --- a/lib/active_record/connection_adapters/cockroachdb/quoting.rb +++ b/lib/active_record/connection_adapters/cockroachdb/quoting.rb @@ -19,6 +19,12 @@ module Quoting # converting to WKB, so this does it automatically. def quote(value) if value.is_a?(Numeric) + # NOTE: The fact that integers are quoted is important and helps + # mitigate a potential vulnerability. + # + # See + # - https://nvd.nist.gov/vuln/detail/CVE-2022-44566 + # - https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/280#discussion_r1288692977 "'#{quote_string(value.to_s)}'" elsif RGeo::Feature::Geometry.check_type(value) "'#{RGeo::WKRep::WKBGenerator.new(hex_format: true, type_format: :ewkb, emit_ewkb_srid: true).generate(value)}'" diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index d7d8c42e..637f3331 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb @@ -90,7 +90,7 @@ def new_column_from_field(table_name, field) # {:dimension=>2, :has_m=>false, :has_z=>false, :name=>"latlon", :srid=>0, :type=>"GEOMETRY"} spatial = spatial_column_info(table_name).get(column_name, type_metadata.sql_type) - PostgreSQL::Column.new( + CockroachDB::Column.new( column_name, default_value, type_metadata, diff --git a/lib/active_record/connection_adapters/cockroachdb/type.rb b/lib/active_record/connection_adapters/cockroachdb/type.rb index a46c249a..77770fb9 100644 --- a/lib/active_record/connection_adapters/cockroachdb/type.rb +++ b/lib/active_record/connection_adapters/cockroachdb/type.rb @@ -4,8 +4,11 @@ class << self # Return :postgresql instead of :cockroachdb for current_adapter_name so # we can continue using the ActiveRecord::Types defined in # PostgreSQLAdapter. - def adapter_name_from(_model) - :postgresql + def adapter_name_from(model) + name = model.connection_db_config.adapter.to_sym + return :postgresql if name == :cockroachdb + + name end end end diff --git a/test/cases/connection_adapters/type_test.rb b/test/cases/connection_adapters/type_test.rb new file mode 100644 index 00000000..dd1e5eff --- /dev/null +++ b/test/cases/connection_adapters/type_test.rb @@ -0,0 +1,26 @@ +require "cases/helper_cockroachdb" +require "models/account" + +module CockroachDB + module ConnectionAdapters + class TypeTest < ActiveRecord::TestCase + fixtures :accounts + class SqliteModel < ActiveRecord::Base + establish_connection( + adapter: "sqlite3", + database: "tmp/some" + ) + end + def test_type_can_be_used_with_various_db + assert_equal( + :postgresql, + ActiveRecord::Type.adapter_name_from(Account) + ) + assert_equal( + :sqlite3, + ActiveRecord::Type.adapter_name_from(SqliteModel) + ) + end + end + end +end diff --git a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapter/QuotingTest.rb b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapter/QuotingTest.rb index ea418e70..e1bb2f1b 100644 --- a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapter/QuotingTest.rb +++ b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapter/QuotingTest.rb @@ -1,3 +1,6 @@ -exclude :test_do_not_raise_when_int_is_not_wider_than_64bit, "replaced for correct quoting" -exclude :test_do_not_raise_when_raise_int_wider_than_64bit_is_false, "replaced for correct quoting" -exclude :test_raise_when_int_is_wider_than_64bit, "since integers are stringified there is not 64bit limit" +comment = "This adapter quotes integers as string, as CRDB is capable of " \ + "implicitely converting, and checking for out of bound errors. " \ + "See quoting.rb for more information." +exclude :test_do_not_raise_when_int_is_not_wider_than_64bit, comment +exclude :test_do_not_raise_when_raise_int_wider_than_64bit_is_false, comment +exclude :test_raise_when_int_is_wider_than_64bit, comment