1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Don't allow granular swapping for shards in legacy

This is a separate commit because I want it to be easy to revert if we
change our minds. After some discussion I think it is confusing that you
could swap shard but not role granularly in legacy mode. This change
forces users to always either have global swapping until moved off
legacy mode. This will prevent a situation where `AnimalsBase` can
change the shard granularly but the role globally.
This commit is contained in:
eileencodes 2020-10-28 18:07:04 -04:00
parent 31461d8a79
commit 6b110d7528
No known key found for this signature in database
GPG key ID: BA5C575120BBE8DF
3 changed files with 20 additions and 26 deletions

View file

@ -135,17 +135,21 @@ module ActiveRecord
# #
# The database kwarg is deprecated and will be removed in 6.2.0 without replacement. # The database kwarg is deprecated and will be removed in 6.2.0 without replacement.
def connected_to(database: nil, role: nil, shard: nil, prevent_writes: false, &blk) def connected_to(database: nil, role: nil, shard: nil, prevent_writes: false, &blk)
if shard && role && (!abstract_class? || self != Base) if legacy_connection_handling
raise NotImplementedError, "calling `connected_to` with `role` and `shard` is only allowed on ActiveRecord::Base or abstract classes" unless abstract_class? || self == Base if self != Base
elsif legacy_connection_handling && self != Base raise NotImplementedError, "`connected_to` can only be called on ActiveRecord::Base with legacy connection handling."
raise NotImplementedError, "`connected_to` can only be called on ActiveRecord::Base" end
elsif database else
ActiveSupport::Deprecation.warn("The database key in `connected_to` is deprecated. It will be removed in Rails 6.2.0 without replacement.") if self != Base && !abstract_class
raise NotImplementedError, "calling `connected_to` is only allowed on ActiveRecord::Base or abstract classes."
end
end end
if database && (role || shard) if database && (role || shard)
raise ArgumentError, "`connected_to` cannot accept a `database` argument with any other arguments." raise ArgumentError, "`connected_to` cannot accept a `database` argument with any other arguments."
elsif database elsif database
ActiveSupport::Deprecation.warn("The database key in `connected_to` is deprecated. It will be removed in Rails 6.2.0 without replacement.")
if database.is_a?(Hash) if database.is_a?(Hash)
role, database = database.first role, database = database.first
role = role.to_sym role = role.to_sym

View file

@ -1632,7 +1632,7 @@ class BasicsTest < ActiveRecord::TestCase
Bird.connected_to(role: :reading) { } Bird.connected_to(role: :reading) { }
end end
assert_equal "`connected_to` can only be called on ActiveRecord::Base", error.message assert_equal "`connected_to` can only be called on ActiveRecord::Base with legacy connection handling.", error.message
ensure ensure
clean_up_legacy_connection_handlers clean_up_legacy_connection_handlers
ActiveRecord::Base.legacy_connection_handling = old_value ActiveRecord::Base.legacy_connection_handling = old_value
@ -1643,7 +1643,7 @@ class BasicsTest < ActiveRecord::TestCase
Bird.connected_to(role: :reading, shard: :default) { } Bird.connected_to(role: :reading, shard: :default) { }
end end
assert_equal "calling `connected_to` with `role` and `shard` is only allowed on ActiveRecord::Base or abstract classes", error.message assert_equal "calling `connected_to` is only allowed on ActiveRecord::Base or abstract classes.", error.message
end end
test "can call connected_to with role and shard on abstract classes" do test "can call connected_to with role and shard on abstract classes" do

View file

@ -308,27 +308,17 @@ module ActiveRecord
assert_equal "primary_shard_one_replica", ActiveRecord::Base.connection_pool.db_config.name assert_equal "primary_shard_one_replica", ActiveRecord::Base.connection_pool.db_config.name
assert_equal "secondary_shard_one_replica", SecondaryBase.connection_pool.db_config.name assert_equal "secondary_shard_one_replica", SecondaryBase.connection_pool.db_config.name
SecondaryBase.connected_to(role: :writing, shard: :default) do ActiveRecord::Base.connected_to(role: :writing, shard: :default) do
# Note: Currently there is only granular shard swapping, not role swapping assert_equal "primary", ActiveRecord::Base.connection_pool.db_config.name
# so this switches the shard only for SecondaryBase but role for both
assert_equal "primary_shard_one", ActiveRecord::Base.connection_pool.db_config.name
assert_equal "secondary", SecondaryBase.connection_pool.db_config.name assert_equal "secondary", SecondaryBase.connection_pool.db_config.name
ActiveRecord::Base.connected_to(role: :writing, shard: :default) do ActiveRecord::Base.connected_to(role: :reading, shard: :shard_two) do
assert_equal "primary", ActiveRecord::Base.connection_pool.db_config.name # ActiveRecord::Base has no shard_two
assert_equal "secondary", SecondaryBase.connection_pool.db_config.name
end
SecondaryBase.connected_to(role: :reading, shard: :shard_two) do
# granular switching means only SecondaryBase uses shard_two
assert_equal "primary_shard_one_replica", ActiveRecord::Base.connection_pool.db_config.name
assert_equal "secondary_shard_two_replica", SecondaryBase.connection_pool.db_config.name assert_equal "secondary_shard_two_replica", SecondaryBase.connection_pool.db_config.name
end end
ActiveRecord::Base.connected_to(role: :writing) do ActiveRecord::Base.connected_to(role: :writing) do
# shard is inherited for AR::Base from outer most block :shard_one assert_equal "primary", ActiveRecord::Base.connection_pool.db_config.name
assert_equal "primary_shard_one", ActiveRecord::Base.connection_pool.db_config.name
# shard is inherited for SecondaryBase by second block :default
assert_equal "secondary", SecondaryBase.connection_pool.db_config.name assert_equal "secondary", SecondaryBase.connection_pool.db_config.name
end end
end end
@ -559,13 +549,13 @@ module ActiveRecord
shard_one_latch.count_down shard_one_latch.count_down
end end
SecondaryBase.connected_to(shard: :one, role: :secondary) do ActiveRecord::Base.connected_to(shard: :one, role: :secondary) do
shard_default_latch.count_down shard_default_latch.count_down
assert_equal "shard_key_one_secondary", ShardConnectionTestModel.connection.select_value("SELECT shard_key from shard_connection_test_models") assert_equal "shard_key_one_secondary", ShardConnectionTestModel.connection.select_value("SELECT shard_key from shard_connection_test_models")
assert_equal "shard_key_default_secondary_b", ShardConnectionTestModelB.connection.select_value("SELECT shard_key from shard_connection_test_models") assert_equal "shard_key_one_secondary_b", ShardConnectionTestModelB.connection.select_value("SELECT shard_key from shard_connection_test_models")
SomeOtherBase.connected_to(shard: :one, role: :secondary) do ActiveRecord::Base.connected_to(shard: :one, role: :secondary) do
assert_equal "shard_key_one_secondary", ShardConnectionTestModel.connection.select_value("SELECT shard_key from shard_connection_test_models") assert_equal "shard_key_one_secondary", ShardConnectionTestModel.connection.select_value("SELECT shard_key from shard_connection_test_models")
assert_equal "shard_key_one_secondary_b", ShardConnectionTestModelB.connection.select_value("SELECT shard_key from shard_connection_test_models") assert_equal "shard_key_one_secondary_b", ShardConnectionTestModelB.connection.select_value("SELECT shard_key from shard_connection_test_models")
end end