From a157c0c2bb216a8479bddefe62196fb32f1b0ed5 Mon Sep 17 00:00:00 2001 From: Benjamin Quorning Date: Fri, 13 Oct 2017 16:10:09 +0200 Subject: [PATCH] Add a few warnings to the connection switcher --- .rubocop.yml | 10 +++--- .../configuration_parser.rb | 5 +++ .../connection_switcher.rb | 10 ++++-- test/configuration_parser_test.rb | 34 +++++++++---------- test/connection_switching_test.rb | 4 +-- test/database.yml | 2 +- test/database_parse_test.yml | 16 ++++----- test/database_tasks.yml | 4 +-- 8 files changed, 48 insertions(+), 37 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 97792feb..1db34ba5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -15,17 +15,17 @@ Layout/IndentHash: Lint/EndAlignment: Enabled: variable +Naming/FileName: + Exclude: + - 'lib/active_record_shards/connection_switcher-5-0.rb' + - 'lib/active_record_shards/connection_switcher-5-1.rb' + Style/Alias: EnforcedStyle: prefer_alias_method Style/EmptyMethod: EnforcedStyle: expanded -Style/FileName: - Exclude: - - 'lib/active_record_shards/connection_switcher-5-0.rb' - - 'lib/active_record_shards/connection_switcher-5-1.rb' - Style/FormatString: EnforcedStyle: percent diff --git a/lib/active_record_shards/configuration_parser.rb b/lib/active_record_shards/configuration_parser.rb index 8d81bbe9..b6293a7c 100644 --- a/lib/active_record_shards/configuration_parser.rb +++ b/lib/active_record_shards/configuration_parser.rb @@ -10,6 +10,11 @@ def explode(conf) conf.to_a.each do |env_name, env_config| next unless shards = env_config.delete('shards') + + unless shards.keys.all? { |shard_name| shard_name.is_a?(Integer) } + raise "All shard names must be integers: #{shards.keys.inspect}." + end + env_config['shard_names'] = shards.keys shards.each do |shard_name, shard_conf| expand_child!(env_config, shard_conf) diff --git a/lib/active_record_shards/connection_switcher.rb b/lib/active_record_shards/connection_switcher.rb index 8fe36ecf..012f3e22 100644 --- a/lib/active_record_shards/connection_switcher.rb +++ b/lib/active_record_shards/connection_switcher.rb @@ -120,7 +120,7 @@ def connection_specification_name name = current_shard_selection.resolve_connection_name(sharded: is_sharded?, configurations: configurations) unless configurations[name] || name == "primary" - raise ActiveRecord::AdapterNotSpecified, "No database defined by #{name} in database.yml" + raise ActiveRecord::AdapterNotSpecified, "No database defined by #{name} in your database config. (configurations: #{configurations.inspect})" end name @@ -144,7 +144,10 @@ def current_shard_id def shard_names unless config = configurations[shard_env] - raise "Did not find #{shard_env} in configurations, did you forget to add it to your database.yml ? (configurations: #{configurations.inspect})" + raise "Did not find #{shard_env} in configurations, did you forget to add it to your database config? (configurations: #{configurations.inspect})" + end + unless config.fetch(SHARD_NAMES_CONFIG_KEY, []).all? { |shard_name| shard_name.is_a?(Integer) } + raise "All shard names must be integers: #{config[SHARD_NAMES_CONFIG_KEY].inspect}." end config[SHARD_NAMES_CONFIG_KEY] || [] end @@ -158,6 +161,9 @@ def switch_connection(options) end if options.key?(:shard) + unless configurations[shard_env] + raise "Did not find #{shard_env} in configurations, did you forget to add it to your database config? (configurations: #{configurations.inspect})" + end current_shard_selection.shard = options[:shard] end diff --git a/test/configuration_parser_test.rb b/test/configuration_parser_test.rb index 40545da4..099916b9 100644 --- a/test/configuration_parser_test.rb +++ b/test/configuration_parser_test.rb @@ -19,42 +19,42 @@ "username" => "root", "password" => nil, "host" => "main_slave_host", - "shard_names" => ["a", "b"].to_set + "shard_names" => [500, 501].to_set }, @conf) end end describe "shard a" do describe "master" do - before { @conf = @exploded_conf["test_shard_a"] } + before { @conf = @exploded_conf["test_shard_500"] } it "be exploded" do @conf["shard_names"] = @conf["shard_names"].to_set assert_equal({ "adapter" => "mysql", "encoding" => "utf8", - "database" => "ars_test_shard_a", + "database" => "ars_test_shard_500", "port" => 123, "username" => "root", "password" => nil, - "host" => "shard_a_host", - "shard_names" => ["a", "b"].to_set + "host" => "shard_500_host", + "shard_names" => [500, 501].to_set }, @conf) end end describe "slave" do - before { @conf = @exploded_conf["test_shard_a_slave"] } + before { @conf = @exploded_conf["test_shard_500_slave"] } it "be exploded" do @conf["shard_names"] = @conf["shard_names"].to_set assert_equal({ "adapter" => "mysql", "encoding" => "utf8", - "database" => "ars_test_shard_a", + "database" => "ars_test_shard_500", "port" => 123, "username" => "root", "password" => nil, - "host" => "shard_a_slave_host", - "shard_names" => ["a", "b"].to_set + "host" => "shard_500_slave_host", + "shard_names" => [500, 501].to_set }, @conf) end end @@ -62,35 +62,35 @@ describe "shard b" do describe "master" do - before { @conf = @exploded_conf["test_shard_b"] } + before { @conf = @exploded_conf["test_shard_501"] } it "be exploded" do @conf["shard_names"] = @conf["shard_names"].to_set assert_equal({ "adapter" => "mysql", "encoding" => "utf8", - "database" => "ars_test_shard_b", + "database" => "ars_test_shard_501", "port" => 123, "username" => "root", "password" => nil, - "host" => "shard_b_host", - "shard_names" => ["a", "b"].to_set + "host" => "shard_501_host", + "shard_names" => [500, 501].to_set }, @conf) end end describe "slave" do - before { @conf = @exploded_conf["test_shard_b_slave"] } + before { @conf = @exploded_conf["test_shard_501_slave"] } it "be exploded" do @conf["shard_names"] = @conf["shard_names"].to_set assert_equal({ "adapter" => "mysql", "encoding" => "utf8", - "database" => "ars_test_shard_b_slave", + "database" => "ars_test_shard_501_slave", "port" => 123, "username" => "root", "password" => nil, - "host" => "shard_b_host", - "shard_names" => ["a", "b"].to_set + "host" => "shard_501_host", + "shard_names" => [500, 501].to_set }, @conf) end end diff --git a/test/connection_switching_test.rb b/test/connection_switching_test.rb index c5e2b540..b3327cf8 100644 --- a/test/connection_switching_test.rb +++ b/test/connection_switching_test.rb @@ -75,8 +75,8 @@ assert_includes(database_names, @shard_1_master.select_value("SELECT DATABASE()")) assert_equal(2, database_shards.size) - assert_includes(database_shards, "0") - assert_includes(database_shards, "1") + assert_includes(database_shards, 0) + assert_includes(database_shards, 1) end it "execute the block unsharded" do diff --git a/test/database.yml b/test/database.yml index 1c03bffc..64341980 100644 --- a/test/database.yml +++ b/test/database.yml @@ -11,7 +11,7 @@ mysql: &MYSQL test: <<: *MYSQL database: ars_test - shard_names: ['0', '1'] + shard_names: [0, 1] test_slave: <<: *MYSQL diff --git a/test/database_parse_test.yml b/test/database_parse_test.yml index 3191a35f..67aa1d8c 100644 --- a/test/database_parse_test.yml +++ b/test/database_parse_test.yml @@ -9,13 +9,13 @@ test: slave: host: main_slave_host shards: - a: - database: ars_test_shard_a - host: shard_a_host + 500: + database: ars_test_shard_500 + host: shard_500_host slave: - host: shard_a_slave_host - b: - database: ars_test_shard_b - host: shard_b_host + host: shard_500_slave_host + 501: + database: ars_test_shard_501 + host: shard_501_host slave: - database: ars_test_shard_b_slave + database: ars_test_shard_501_slave diff --git a/test/database_tasks.yml b/test/database_tasks.yml index 4842badb..9cff2c34 100644 --- a/test/database_tasks.yml +++ b/test/database_tasks.yml @@ -10,7 +10,7 @@ test: slave: database: ars_tasks_test_slave shards: - a: + 0: database: ars_tasks_test_shard_a - b: + 1: database: ars_tasks_test_shard_b