Allow deprecated non-symbol access to nested config_for hashes

A change to `Rails::Application.config_for` in
https://github.com/rails/rails/pull/33815 and
https://github.com/rails/rails/pull/33882 has altered the behaviour of
the returned object in a breaking manner. Before that change, nested
hashes returned from `config_for` could be accessed using non-symbol keys.
After the change, all keys are recursively symbolized so non-symbol access
fails to read the expected values.

This is a breaking change for any app that might be relying on the
nested hashes returned from `config_for` calls, and thus should be
deprecated before being removed from the codebase.

This commit introduces a temporary `NonSymbolAccessDeprecatedHash` class
that recursively wraps any nested hashes inside the `OrderedOptions`
object returned from `config_for` and issues a deprecation notice when a
non-symbol based access is performed.

This way, apps that are still relying on the ability to access these
nested hashes using non-symbol keys will be able to observe the
deprecation notices and have time to implement changes before non-symbol
access is removed for good.

A CHANGELOG entry is also added to note that non-symbol access to nested
`config_for` hashes is deprecated.

railties/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix non-symbol access to nested hashes returned from `Rails::Application.config_for`
+    being broken by allowing non-symbol access with a deprecation notice.
+
+    *Ufuk Kayserilioglu*
+
 *   Fix deeply nested namespace command printing.
 
     *Gannon McGibbon*

railties/lib/rails/application.rb

@@ -7,6 +7,7 @@ require "active_support/key_generator"
 require "active_support/message_verifier"
 require "active_support/encrypted_configuration"
 require "active_support/deprecation"
+require "active_support/hash_with_indifferent_access"
 require "rails/engine"
 require "rails/secrets"
 
@@ -230,8 +231,8 @@ module Rails
         config = YAML.load(ERB.new(yaml.read).result) || {}
         config = (config["shared"] || {}).merge(config[env] || {})
 
-        ActiveSupport::OrderedOptions.new.tap do |config_as_ordered_options|
-          config_as_ordered_options.update(config.deep_symbolize_keys)
+        ActiveSupport::OrderedOptions.new.tap do |options|
+          options.update(NonSymbolAccessDeprecatedHash.new(config))
         end
       else
         raise "Could not load configuration. No such file - #{yaml}"
@@ -590,5 +591,38 @@ module Rails
       def build_middleware
         config.app_middleware + super
       end
+
+      class NonSymbolAccessDeprecatedHash < HashWithIndifferentAccess # :nodoc:
+        def initialize(value = nil)
+          if value.is_a?(Hash)
+            value.each_pair { |k, v| self[k] = v }
+          else
+            super
+          end
+        end
+
+        def []=(key, value)
+          if value.is_a?(Hash)
+            value = self.class.new(value)
+          end
+          super(key.to_sym, value)
+        end
+
+        private
+
+          def convert_key(key)
+            unless key.kind_of?(Symbol)
+              ActiveSupport::Deprecation.warn(<<~MESSAGE.squish)
+                Accessing hashes returned from config_for by non-symbol keys
+                is deprecated and will be removed in Rails 6.1.
+                Use symbols for access instead.
+              MESSAGE
+
+              key = key.to_sym
+            end
+
+            key
+          end
+      end
   end
 end

railties/test/application/configuration_test.rb

@@ -1714,7 +1714,7 @@ module ApplicationTests
       assert_equal true, Rails.application.config.action_mailer.show_previews
     end
 
-    test "config_for loads custom configuration from yaml accessible as symbol" do
+    test "config_for loads custom configuration from yaml accessible as symbol or string" do
       app_file "config/custom.yml", <<-RUBY
       development:
         foo: 'bar'
@@ -1727,6 +1727,7 @@ module ApplicationTests
       app "development"
 
       assert_equal "bar", Rails.application.config.my_custom_config[:foo]
+      assert_equal "bar", Rails.application.config.my_custom_config["foo"]
     end
 
     test "config_for loads nested custom configuration from yaml as symbol keys" do
@@ -1746,6 +1747,25 @@ module ApplicationTests
       assert_equal 1, Rails.application.config.my_custom_config[:foo][:bar][:baz]
     end
 
+    test "config_for loads nested custom configuration from yaml with deprecated non-symbol access" do
+      app_file "config/custom.yml", <<-RUBY
+      development:
+        foo:
+          bar:
+            baz: 1
+      RUBY
+
+      add_to_config <<-RUBY
+        config.my_custom_config = config_for('custom')
+      RUBY
+
+      app "development"
+
+      assert_deprecated do
+        assert_equal 1, Rails.application.config.my_custom_config["foo"]["bar"]["baz"]
+      end
+    end
+
     test "config_for makes all hash methods available" do
       app_file "config/custom.yml", <<-RUBY
       development:
@@ -1770,6 +1790,87 @@ module ApplicationTests
       assert_equal actual[:bar], baz: 1
     end
 
+    test "config_for generates deprecation notice when nested hash methods are called with non-symbols" do
+      app_file "config/custom.yml", <<-RUBY
+      development:
+        foo:
+          bar: 1
+          baz: 2
+          qux:
+            boo: 3
+      RUBY
+
+      app "development"
+
+      actual = Rails.application.config_for("custom")[:foo]
+
+      # slice
+      assert_deprecated do
+        assert_equal({ bar: 1, baz: 2 }, actual.slice("bar", "baz"))
+      end
+
+      # except
+      assert_deprecated do
+        assert_equal({ qux: { boo: 3 } }, actual.except("bar", "baz"))
+      end
+
+      # dig
+      assert_deprecated do
+        assert_equal(3, actual.dig("qux", "boo"))
+      end
+
+      # fetch - hit
+      assert_deprecated do
+        assert_equal(1, actual.fetch("bar", 0))
+      end
+
+      # fetch - miss
+      assert_deprecated do
+        assert_equal(0, actual.fetch("does-not-exist", 0))
+      end
+
+      # fetch_values
+      assert_deprecated do
+        assert_equal([1, 2], actual.fetch_values("bar", "baz"))
+      end
+
+      # key? - hit
+      assert_deprecated do
+        assert(actual.key?("bar"))
+      end
+
+      # key? - miss
+      assert_deprecated do
+        assert_not(actual.key?("does-not-exist"))
+      end
+
+      # slice!
+      actual = Rails.application.config_for("custom")[:foo]
+
+      assert_deprecated do
+        slice = actual.slice!("bar", "baz")
+        assert_equal({ bar: 1, baz: 2 }, actual)
+        assert_equal({ qux: { boo: 3 } }, slice)
+      end
+
+      # extract!
+      actual = Rails.application.config_for("custom")[:foo]
+
+      assert_deprecated do
+        extracted = actual.extract!("bar", "baz")
+        assert_equal({ bar: 1, baz: 2 }, extracted)
+        assert_equal({ qux: { boo: 3 } }, actual)
+      end
+
+      # except!
+      actual = Rails.application.config_for("custom")[:foo]
+
+      assert_deprecated do
+        actual.except!("bar", "baz")
+        assert_equal({ qux: { boo: 3 } }, actual)
+      end
+    end
+
     test "config_for uses the Pathname object if it is provided" do
       app_file "config/custom.yml", <<-RUBY
       development: