From 4e01932d437a03a8fefea01c8ed2075d386a3cea Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 17 Sep 2019 14:27:41 -0400 Subject: [PATCH] Deprecate `to_h` and `to_legacy_hash` These should have been deprecated when I added them but for some reason I didn't. As we move away from passing hashes around we no longer need these methods, and since we no longer use configuration hashes as database configuration we can deprecate this in favor of using the database configuration objects and access the connection hashes from there. Co-authored-by: John Crepezzi --- .../active_record/database_configurations.rb | 7 +- .../database_config.rb | 4 - ...rge_and_resolve_default_url_config_test.rb | 215 +++++++++--------- .../cases/database_configurations_test.rb | 16 +- 4 files changed, 116 insertions(+), 126 deletions(-) diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index d84d57bb05..0fe9437be1 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -79,12 +79,11 @@ module ActiveRecord # Returns the DatabaseConfigurations object as a Hash. def to_h - configs = configurations.reverse.inject({}) do |memo, db_config| - memo.merge(db_config.to_legacy_hash) + configurations.inject({}) do |memo, db_config| + memo.merge(db_config.env_name => db_config.configuration_hash.stringify_keys) end - - Hash[configs.to_a.reverse] end + deprecate to_h: "You can use `ActiveRecord::Base.configurations.configs_for(env_name: 'env', spec_name: 'primary').configuration_hash` to get the configuration hashes." # Checks if the application's configurations are empty. # diff --git a/activerecord/lib/active_record/database_configurations/database_config.rb b/activerecord/lib/active_record/database_configurations/database_config.rb index 797da7fda7..6b34011bef 100644 --- a/activerecord/lib/active_record/database_configurations/database_config.rb +++ b/activerecord/lib/active_record/database_configurations/database_config.rb @@ -38,10 +38,6 @@ module ActiveRecord false end - def to_legacy_hash - { env_name => configuration_hash.stringify_keys } - end - def for_current_env? env_name == ActiveRecord::ConnectionHandling::DEFAULT_ENV.call end diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 9586fe6c8f..5a70acd66c 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -17,9 +17,9 @@ module ActiveRecord ENV["RAILS_ENV"] = @previous_rails_env end - def resolve_config(config) + def resolve_config(config, env_name = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call) configs = ActiveRecord::DatabaseConfigurations.new(config) - configs.to_h + configs.configs_for(env_name: env_name, spec_name: "primary")&.configuration_hash end def resolve_spec(spec, config) @@ -116,16 +116,21 @@ module ActiveRecord def test_jdbc_url config = { "production" => { "url" => "jdbc:postgres://localhost/foo" } } - actual = resolve_config(config) - assert_equal config, actual + actual = resolve_config(config, "production") + assert_equal config["production"].symbolize_keys, actual end def test_environment_does_not_exist_in_config_url_does_exist ENV["DATABASE_URL"] = "postgres://localhost/foo" - config = { "not_default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } - actual = resolve_config(config) - expect_prod = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } - assert_equal expect_prod, actual["default_env"] + config = { "not_default_env" => { "adapter" => "not_postgres", "database" => "not_foo" } } + actual = resolve_config(config, "default_env") + expect_prod = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } + + assert_equal expect_prod, actual end def test_url_with_hyphenated_scheme @@ -138,38 +143,38 @@ module ActiveRecord def test_string_connection config = { "default_env" => "postgres://localhost/foo" } - actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" - } - } + actual = resolve_config(config, "default_env") + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } + assert_equal expected, actual end def test_url_sub_key config = { "default_env" => { "url" => "postgres://localhost/foo" } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" - } - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } + assert_equal expected, actual end def test_hash config = { "production" => { "adapter" => "postgres", "database" => "foo" } } - actual = resolve_config(config) - assert_equal config, actual + actual = resolve_config(config, "production") + assert_equal config["production"].symbolize_keys, actual end def test_blank config = {} - actual = resolve_config(config) - assert_equal config, actual + actual = resolve_config(config, "default_env") + assert_nil actual end def test_blank_with_database_url @@ -177,17 +182,13 @@ module ActiveRecord config = {} actual = resolve_config(config) - expected = { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" } - assert_equal expected, actual["default_env"] - assert_nil actual["production"] - assert_nil actual["development"] - assert_nil actual["test"] - assert_nil actual[:default_env] - assert_nil actual[:production] - assert_nil actual[:development] - assert_nil actual[:test] + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } + + assert_equal expected, actual end def test_blank_with_database_url_with_rails_env @@ -196,20 +197,13 @@ module ActiveRecord config = {} actual = resolve_config(config) - expected = { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } - assert_equal expected, actual["not_production"] - assert_nil actual["production"] - assert_nil actual["default_env"] - assert_nil actual["development"] - assert_nil actual["test"] - assert_nil actual[:default_env] - assert_nil actual[:not_production] - assert_nil actual[:production] - assert_nil actual[:development] - assert_nil actual[:test] + assert_equal expected, actual end def test_blank_with_database_url_with_rack_env @@ -218,20 +212,13 @@ module ActiveRecord config = {} actual = resolve_config(config) - expected = { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } - assert_equal expected, actual["not_production"] - assert_nil actual["production"] - assert_nil actual["default_env"] - assert_nil actual["development"] - assert_nil actual["test"] - assert_nil actual[:default_env] - assert_nil actual[:not_production] - assert_nil actual[:production] - assert_nil actual[:development] - assert_nil actual[:test] + assert_equal expected, actual end def test_database_url_with_ipv6_host_and_port @@ -239,11 +226,14 @@ module ActiveRecord config = {} actual = resolve_config(config) - expected = { "adapter" => "postgresql", - "database" => "foo", - "host" => "::1", - "port" => 5454 } - assert_equal expected, actual["default_env"] + expected = { + adapter: "postgresql", + database: "foo", + host: "::1", + port: 5454 + } + + assert_equal expected, actual end def test_url_sub_key_with_database_url @@ -251,12 +241,12 @@ module ActiveRecord config = { "default_env" => { "url" => "postgres://localhost/foo" } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost" - } - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost" + } + assert_equal expected, actual end @@ -264,19 +254,21 @@ module ActiveRecord ENV["DATABASE_URL"] = "postgres://localhost/baz" config = { "default_env" => { "database" => "foo" }, "other_env" => { "url" => "postgres://foohost/bardb" } } - actual = resolve_config(config) - expected = { "default_env" => - { "database" => "baz", - "adapter" => "postgresql", - "host" => "localhost" - }, - "other_env" => - { "adapter" => "postgresql", - "database" => "bardb", - "host" => "foohost" - } - } - assert_equal expected, actual + expected = { + default_env: { + database: "baz", + adapter: "postgresql", + host: "localhost" + }, + other_env: { + adapter: "postgresql", + database: "bardb", + host: "foohost" + } + } + + assert_equal expected[:default_env], resolve_config(config, "default_env") + assert_equal expected[:other_env], resolve_config(config, "other_env") end def test_merge_no_conflicts_with_database_url @@ -284,13 +276,13 @@ module ActiveRecord config = { "default_env" => { "pool" => "5" } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost", - "pool" => "5" - } - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost", + pool: "5" + } + assert_equal expected, actual end @@ -299,13 +291,13 @@ module ActiveRecord config = { "default_env" => { "adapter" => "NOT-POSTGRES", "database" => "NOT-FOO", "pool" => "5" } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost", - "pool" => "5" - } - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost", + pool: "5" + } + assert_equal expected, actual end @@ -314,13 +306,13 @@ module ActiveRecord config = { "default_env" => { "adapter" => "postgresql", "pool" => "5" } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost", - "pool" => "5" - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost", + pool: "5" } + assert_equal expected, actual end @@ -329,12 +321,11 @@ module ActiveRecord config = { "default_env" => { "pool" => 5 } } actual = resolve_config(config) - expected = { "default_env" => - { "adapter" => "postgresql", - "database" => "foo", - "host" => "localhost", - "pool" => 5 - } + expected = { + adapter: "postgresql", + database: "foo", + host: "localhost", + pool: 5 } assert_equal expected, actual diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb index 6b863d71c6..6644bbdb36 100644 --- a/activerecord/test/cases/database_configurations_test.rb +++ b/activerecord/test/cases/database_configurations_test.rb @@ -48,11 +48,13 @@ class DatabaseConfigurationsTest < ActiveRecord::TestCase assert_equal "primary", config.spec_name end - def test_to_h_turns_db_config_object_back_into_a_hash + def test_to_h_turns_db_config_object_back_into_a_hash_and_is_deprecated configs = ActiveRecord::Base.configurations assert_equal "ActiveRecord::DatabaseConfigurations", configs.class.name - assert_equal "Hash", configs.to_h.class.name - assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"], ActiveRecord::Base.configurations.to_h.keys.sort + assert_deprecated do + assert_equal "Hash", configs.to_h.class.name + assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"], ActiveRecord::Base.configurations.to_h.keys.sort + end end end @@ -73,9 +75,11 @@ class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase end end - def test_can_turn_configurations_into_a_hash - assert ActiveRecord::Base.configurations.to_h.is_a?(Hash), "expected to be a hash but was not." - assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"].sort, ActiveRecord::Base.configurations.to_h.keys.sort + def test_can_turn_configurations_into_a_hash_and_is_deprecated + assert_deprecated do + assert ActiveRecord::Base.configurations.to_h.is_a?(Hash), "expected to be a hash but was not." + assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"].sort, ActiveRecord::Base.configurations.to_h.keys.sort + end end def test_each_is_deprecated