diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 94729c9db545b02d669e2aa7bd77c875bdfa6093..c7d6186493dd6462ba5a8f19d984812489fc9b14 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* `connects_to` can only be called on `ActiveRecord::Base` or abstract classes. + + Ensure that `connects_to` can only be called from `ActiveRecord::Base` or abstract classes. This protects the application from opening duplicate or too many connections. + + *Eileen M. Uchitelle*, *John Crepezzi* + * All connection adapters `execute` now raises `ActiveRecord::ConnectionNotEstablished` rather than `ActiveRecord::InvalidStatement` when they encounter a connection error. diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 8ddf8dc183e08ba5e4d13dc08b4a1b87fcadb7c7..62e588f2bfd3000b32713232c41b5a49664e4cc8 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -79,6 +79,8 @@ def establish_connection(config_or_env = nil) # # Returns an array of database connections. def connects_to(database: {}, shards: {}) + raise NotImplementedError, "connects_to can only be called on ActiveRecord::Base or abstract classes" unless self == Base || abstract_class? + if database.present? && shards.present? raise ArgumentError, "connects_to can only accept a `database` or `shards` argument, but not both arguments." end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 1f3f2b28adbf4db5a43896d5f6837976fae5ff0e..8846d9830ddf586aca0cbc3048e6fa7e9da62695 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1661,6 +1661,14 @@ def test_protected_environments_are_stored_as_an_array_of_string end end + test "cannot call connects_to on non-abstract or non-ActiveRecord::Base classes" do + error = assert_raises(NotImplementedError) do + Bird.connects_to(database: { writing: :arunit }) + end + + assert_equal "connects_to can only be called on ActiveRecord::Base or abstract classes", error.message + end + test "cannot call connected_to on subclasses of ActiveRecord::Base" do error = assert_raises(NotImplementedError) do Bird.connected_to(role: :reading) { } diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb index d184af3b7ae9a31e4a44193441e575cc8d6ca22d..5cab75c703b2584c8b0fc7a94cbcc674acc2d02c 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb @@ -24,7 +24,11 @@ def teardown clean_up_connection_handler end - class MultiConnectionTestModel < ActiveRecord::Base + class SecondaryBase < ActiveRecord::Base + self.abstract_class = true + end + + class MultiConnectionTestModel < SecondaryBase end def test_multiple_connection_handlers_works_in_a_threaded_environment @@ -33,7 +37,7 @@ def test_multiple_connection_handlers_works_in_a_threaded_environment # We need to use a role for reading not named reading, otherwise we'll prevent writes # and won't be able to write to the second connection. - MultiConnectionTestModel.connects_to database: { writing: { database: tf_writing.path, adapter: "sqlite3" }, secondary: { database: tf_reading.path, adapter: "sqlite3" } } + SecondaryBase.connects_to database: { writing: { database: tf_writing.path, adapter: "sqlite3" }, secondary: { database: tf_reading.path, adapter: "sqlite3" } } MultiConnectionTestModel.connection.execute("CREATE TABLE `multi_connection_test_models` (connection_role VARCHAR (255))") MultiConnectionTestModel.connection.execute("INSERT INTO multi_connection_test_models VALUES ('writing')") @@ -73,7 +77,7 @@ def test_multiple_connection_handlers_works_in_a_threaded_environment def test_loading_relations_with_multi_db_connection_handlers # We need to use a role for reading not named reading, otherwise we'll prevent writes # and won't be able to write to the second connection. - MultiConnectionTestModel.connects_to database: { writing: { database: ":memory:", adapter: "sqlite3" }, secondary: { database: ":memory:", adapter: "sqlite3" } } + SecondaryBase.connects_to database: { writing: { database: ":memory:", adapter: "sqlite3" }, secondary: { database: ":memory:", adapter: "sqlite3" } } relation = ActiveRecord::Base.connected_to(role: :secondary) do MultiConnectionTestModel.connection.execute("CREATE TABLE `multi_connection_test_models` (connection_role VARCHAR (255))") diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_sharding_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_sharding_db_test.rb index 6d0e17320a7b378efa369a963d5e199bc1746711..aea3b902dfb867b4601d76316973754f215fa87f 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_sharding_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_sharding_db_test.rb @@ -264,15 +264,23 @@ def test_calling_connected_to_on_a_non_existent_shard_raises end end - class ShardConnectionTestModel < ActiveRecord::Base + class SecondaryBase < ActiveRecord::Base + self.abstract_class = true end - class ShardConnectionTestModelB < ActiveRecord::Base + class ShardConnectionTestModel < SecondaryBase + end + + class SomeOtherBase < ActiveRecord::Base + self.abstract_class = true + end + + class ShardConnectionTestModelB < SomeOtherBase end def test_same_shards_across_clusters - ShardConnectionTestModel.connects_to shards: { one: { writing: { database: ":memory:", adapter: "sqlite3" } } } - ShardConnectionTestModelB.connects_to shards: { one: { writing: { database: ":memory:", adapter: "sqlite3" } } } + SecondaryBase.connects_to shards: { one: { writing: { database: ":memory:", adapter: "sqlite3" } } } + SomeOtherBase.connects_to shards: { one: { writing: { database: ":memory:", adapter: "sqlite3" } } } ActiveRecord::Base.connected_to(shard: :one) do ShardConnectionTestModel.connection.execute("CREATE TABLE `shard_connection_test_models` (shard_key VARCHAR (255))") @@ -287,7 +295,7 @@ def test_same_shards_across_clusters end def test_sharding_separation - ShardConnectionTestModel.connects_to shards: { + SecondaryBase.connects_to shards: { default: { writing: { database: ":memory:", adapter: "sqlite3" } }, one: { writing: { database: ":memory:", adapter: "sqlite3" } } } @@ -323,7 +331,7 @@ def test_swapping_shards_in_a_multi_threaded_environment tf_default = Tempfile.open "shard_key_default" tf_shard_one = Tempfile.open "shard_key_one" - ShardConnectionTestModel.connects_to shards: { + SecondaryBase.connects_to shards: { default: { writing: { database: tf_default.path, adapter: "sqlite3" } }, one: { writing: { database: tf_shard_one.path, adapter: "sqlite3" } } } @@ -368,7 +376,7 @@ def test_swapping_shards_and_roles_in_a_multi_threaded_environment tf_default_reading = Tempfile.open "shard_key_default_reading" tf_shard_one_reading = Tempfile.open "shard_key_one_reading" - ShardConnectionTestModel.connects_to shards: { + SecondaryBase.connects_to shards: { default: { writing: { database: tf_default.path, adapter: "sqlite3" }, secondary: { database: tf_default_reading.path, adapter: "sqlite3" } }, one: { writing: { database: tf_shard_one.path, adapter: "sqlite3" }, secondary: { database: tf_shard_one_reading.path, adapter: "sqlite3" } } }